Fix user creation failure handling in OAuth paths (#29207)

Co-authored-by: Matt Jankowski <matt@jankowski.online>
This commit is contained in:
Claire 2024-02-14 22:49:45 +01:00
parent 6499850ac4
commit 9e5af6bb58
4 changed files with 154 additions and 0 deletions

View file

@ -24,6 +24,9 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController
session["devise.#{provider}_data"] = request.env['omniauth.auth'] session["devise.#{provider}_data"] = request.env['omniauth.auth']
redirect_to new_user_registration_url redirect_to new_user_registration_url
end end
rescue ActiveRecord::RecordInvalid
flash[:alert] = I18n.t('devise.failure.omniauth_user_creation_failure') if is_navigational_format?
redirect_to new_user_session_url
end end
end end

View file

@ -12,6 +12,7 @@ en:
last_attempt: You have one more attempt before your account is locked. last_attempt: You have one more attempt before your account is locked.
locked: Your account is locked. locked: Your account is locked.
not_found_in_database: Invalid %{authentication_keys} or password. not_found_in_database: Invalid %{authentication_keys} or password.
omniauth_user_creation_failure: Error creating an account for this identity.
pending: Your account is still under review. pending: Your account is still under review.
timeout: Your session expired. Please sign in again to continue. timeout: Your session expired. Please sign in again to continue.
unauthenticated: You need to sign in or sign up before continuing. unauthenticated: You need to sign in or sign up before continuing.

View file

@ -0,0 +1,143 @@
# frozen_string_literal: true
require 'rails_helper'
describe 'OmniAuth callbacks' do
shared_examples 'omniauth provider callbacks' do |provider|
subject { post send :"user_#{provider}_omniauth_callback_path" }
context 'with full information in response' do
before do
mock_omniauth(provider, {
provider: provider.to_s,
uid: '123',
info: {
verified: 'true',
email: 'user@host.example',
},
})
end
context 'without a matching user' do
it 'creates a user and an identity and redirects to root path' do
expect { subject }
.to change(User, :count)
.by(1)
.and change(Identity, :count)
.by(1)
.and change(LoginActivity, :count)
.by(1)
expect(User.last.email).to eq('user@host.example')
expect(Identity.find_by(user: User.last).uid).to eq('123')
expect(response).to redirect_to(root_path)
end
end
context 'with a matching user and no matching identity' do
before do
Fabricate(:user, email: 'user@host.example')
end
context 'when ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH is set to true' do
around do |example|
ClimateControl.modify ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH: 'true' do
example.run
end
end
it 'matches the existing user, creates an identity, and redirects to root path' do
expect { subject }
.to not_change(User, :count)
.and change(Identity, :count)
.by(1)
.and change(LoginActivity, :count)
.by(1)
expect(Identity.find_by(user: User.last).uid).to eq('123')
expect(response).to redirect_to(root_path)
end
end
context 'when ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH is not set to true' do
it 'does not match the existing user or create an identity, and redirects to login page' do
expect { subject }
.to not_change(User, :count)
.and not_change(Identity, :count)
.and not_change(LoginActivity, :count)
expect(response).to redirect_to(new_user_session_url)
end
end
end
context 'with a matching user and a matching identity' do
before do
user = Fabricate(:user, email: 'user@host.example')
Fabricate(:identity, user: user, uid: '123', provider: provider)
end
it 'matches the existing records and redirects to root path' do
expect { subject }
.to not_change(User, :count)
.and not_change(Identity, :count)
.and change(LoginActivity, :count)
.by(1)
expect(response).to redirect_to(root_path)
end
end
end
context 'with a response missing email address' do
before do
mock_omniauth(provider, {
provider: provider.to_s,
uid: '123',
info: {
verified: 'true',
},
})
end
it 'redirects to the auth setup page' do
expect { subject }
.to change(User, :count)
.by(1)
.and change(Identity, :count)
.by(1)
.and change(LoginActivity, :count)
.by(1)
expect(response).to redirect_to(auth_setup_path(missing_email: '1'))
end
end
context 'when a user cannot be built' do
before do
allow(User).to receive(:find_for_omniauth).and_return(User.new)
end
it 'redirects to the new user signup page' do
expect { subject }
.to not_change(User, :count)
.and not_change(Identity, :count)
.and not_change(LoginActivity, :count)
expect(response).to redirect_to(new_user_registration_url)
end
end
end
describe '#openid_connect', if: ENV['OIDC_ENABLED'] == 'true' && ENV['OIDC_SCOPE'].present? do
include_examples 'omniauth provider callbacks', :openid_connect
end
describe '#cas', if: ENV['CAS_ENABLED'] == 'true' do
include_examples 'omniauth provider callbacks', :cas
end
describe '#saml', if: ENV['SAML_ENABLED'] == 'true' do
include_examples 'omniauth provider callbacks', :saml
end
end

View file

@ -0,0 +1,7 @@
# frozen_string_literal: true
OmniAuth.config.test_mode = true
def mock_omniauth(provider, data)
OmniAuth.config.mock_auth[provider] = OmniAuth::AuthHash.new(data)
end