Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to Allowed Domains #5979

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ Metrics/ClassLength:
# A calculated magnitude based on number of assignments,
# branches, and conditions.
Metrics/AbcSize:
Max: 90
Max: 95

Metrics/ParameterLists:
CountKeywordArgs: false
Expand All @@ -82,10 +82,10 @@ RSpec/AnyInstance:
Enabled: false

Metrics/CyclomaticComplexity:
Max: 20
Max: 25

Metrics/PerceivedComplexity:
Max: 20
Max: 25

Rails/Exit:
Exclude:
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/api/v1/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def show
render_data data: user, status: :ok
end

# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
# rubocop:disable Metrics/AbcSize
# POST /api/v1/users.json
# Creates and saves a new user record in the database with the provided parameters
def create
Expand All @@ -62,7 +62,7 @@ def create
create_user_params[:language] = current_user&.language || I18n.default_locale if create_user_params[:language].blank?

# renders an error if the user is signing up with an invalid domain based off site settings
return render_error errors: Rails.configuration.custom_error_msgs[:unauthorized], status: :forbidden unless valid_domain?
return render_error errors: Rails.configuration.custom_error_msgs[:banned_user], status: :forbidden unless valid_domain?

user = UserCreator.new(user_params: create_user_params.except(:invite_token), provider: current_provider, role: default_role).call

Expand Down Expand Up @@ -97,7 +97,7 @@ def create
render_error errors: Rails.configuration.custom_error_msgs[:record_invalid], status: :bad_request
end
end
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
# rubocop:enable Metrics/AbcSize

# PATCH /api/v1/users/:id.json
# Updates the values of a user
Expand Down
5 changes: 3 additions & 2 deletions app/controllers/external_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@ def create_user
return redirect_to root_path(error: Rails.configuration.custom_error_msgs[:invite_token_invalid])
end

return render_error status: :forbidden unless valid_domain?(user_info[:email])
# Redirect to root if the user doesn't exist and has an invalid domain
return redirect_to root_path(error: Rails.configuration.custom_error_msgs[:banned_user]) if new_user && !valid_domain?(user_info[:email])

# Create the user if they dont exist
# Create the user if they don't exist
if new_user
user = UserCreator.new(user_params: user_info, provider: current_provider, role: default_role).call
user.save!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export default function Registration() {
<input
className="form-control"
placeholder={t('admin.site_settings.registration.enter_allowed_domains_rule')}
defaultValue={siteSettings?.AllowedDomains}
/>
<Button
variant="brand"
Expand Down
3 changes: 3 additions & 0 deletions app/javascript/components/home/HomePage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ export default function HomePage() {
case 'SignupError':
toast.error(t('toast.error.users.signup_error'));
break;
case 'BannedUser':
toast.error(t('toast.error.users.banned'));
break;
default:
}
if (error) { setSearchParams(searchParams.delete('error')); }
Expand Down
2 changes: 2 additions & 0 deletions app/javascript/hooks/mutations/users/useCreateUser.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ export default function useCreateUser() {
toast.error(t('toast.error.users.invalid_invite'));
} else if (err.response.data.errors === 'EmailAlreadyExists') {
toast.error(t('toast.error.users.email_exists'));
} else if (err.response.data.errors === 'BannedUser') {
toast.error(t('toast.error.users.banned'));
} else {
toast.error(t('toast.error.problem_completing_action'));
}
Expand Down
9 changes: 9 additions & 0 deletions spec/controllers/external_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,15 @@

expect { get :create_user, params: { provider: 'openid_connect' } }.not_to change(User, :count)
end

it 'does not affect existing users with different domains' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]

create(:user, external_id: OmniAuth.config.mock_auth[:openid_connect][:uid])

get :create_user, params: { provider: 'openid_connect' }
expect(response).not_to redirect_to(root_path(error: Rails.configuration.custom_error_msgs[:banned_user]))
end
end

context 'restricted domain set to multiple domain' do
Expand Down
Loading