Skip to content

Commit

Permalink
Fix and add test for self-registration
Browse files Browse the repository at this point in the history
  • Loading branch information
oliverguenther committed Oct 18, 2024
1 parent 9ad16a0 commit 35c82d3
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 19 deletions.
2 changes: 1 addition & 1 deletion app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def password_change_not_possible(user)
if user.ldap_auth_source
user.ldap_auth_source.name
else
user.authentication_provider
user.human_authentication_provider
end
open_project_headers "Type" => "Account"

Expand Down
7 changes: 6 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,12 @@ def name(formatter = nil)
def authentication_provider
return if identity_url.blank?

identity_url.split(":", 2).first.titleize
identity_url.split(":", 2).first
end

# Return user's authentication provider for display
def human_authentication_provider
authentication_provider&.titleize
end

##
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/form/authentication/_external.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="form--field -reduced-margin">
<%= styled_label_tag nil, I18n.t('user.authentication_provider') %>
<div class="form--field-container">
<%= @user.authentication_provider %>
<%= @user.human_authentication_provider %>
</div>
</div>
<div class="form--field-instructions">
Expand Down
90 changes: 75 additions & 15 deletions modules/openid_connect/spec/requests/openid_connect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,13 @@
end

describe "sign-up and login" do
let(:limit_self_registration) { false }

before do
create(:oidc_provider, slug: "keycloak", limit_self_registration: false)
create(:oidc_provider, slug: "keycloak", limit_self_registration:)
end

it "works" do
it "logs in the user" do
##
# it should redirect to the provider's openid connect authentication endpoint
click_on_signin("keycloak")
Expand Down Expand Up @@ -115,7 +117,77 @@
expect(response.location).to match /\/my\/page/
end

context "with a custom claim and mapping" do
context "with self-registration disabled and provider respecting that",
with_settings: {
self_registration: 0
} do
let(:limit_self_registration) { true }

it "does not allow registration" do
click_on_signin("keycloak")
redirect_from_provider("keycloak")

expect(response).to have_http_status :found
expect(response.location).to match /\/login$/
expect(flash[:error]).to include "User registration is limited for the Single sign-on provider 'keycloak'"

user = User.find_by(mail: user_info[:email])
expect(user).to be_registered
expect(user).not_to be_active
end
end

context "with self-registration manual and provider respecting that",
with_settings: {
self_registration: 2
} do
let(:limit_self_registration) { true }

it "does not allow registration" do
##
# it should redirect to the provider's openid connect authentication endpoint
click_on_signin("keycloak")

##
# it should redirect back from the provider to the login page
redirect_from_provider("keycloak")

expect(response).to have_http_status :found
expect(response.location).to match /\/login$/
expect(flash[:notice]).to eq "Your account was created and is now pending administrator approval."

user = User.find_by(mail: user_info[:email])
expect(user).to be_registered
expect(user).not_to be active
end
end

context "with self-registration disabled and provider ignoring that",
with_settings: {
self_registration: 0
} do
let(:limit_self_registration) { false }

it "does not allow registration" do
click_on_signin("keycloak")
redirect_from_provider("keycloak")

expect(response).to have_http_status :found
expect(response.location).to match /\/\?first_time_user=true$/

user = User.find_by(mail: user_info[:email])
expect(user).to be_active
end
end

context "with a custom attribute mapping" do
let!(:provider) do
create(:oidc_provider,
slug: "keycloak",
limit_self_registration:,
mapping_login: :foobar)
end

let(:user_info) do
{
sub: "87117114115116",
Expand All @@ -127,18 +199,6 @@
}
end

before do
allow(Setting).to receive(:plugin_openproject_openid_connect).and_return(
"providers" => {
"heroku" => {
"attribute_map" => { login: :foobar },
"identifier" => "does not",
"secret" => "matter"
}
}
)
end

it "maps to the login" do
skip "Mapping is not supported yet"
click_on_signin
Expand Down
6 changes: 5 additions & 1 deletion spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,12 @@
user.save!
end

it "returns the internal provider slug" do
expect(user._authentication_provider).to eql("test_provider")
end

it "creates a human readable name" do
expect(user.authentication_provider).to eql("Test Provider")
expect(user.human_authentication_provider).to eql("Test Provider")
end
end

Expand Down

0 comments on commit 35c82d3

Please sign in to comment.