diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb
index 707b50ef9e..9b83de945b 100644
--- a/app/controllers/auth/omniauth_callbacks_controller.rb
+++ b/app/controllers/auth/omniauth_callbacks_controller.rb
@@ -7,7 +7,7 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController
def self.provides_callback_for(provider)
define_method provider do
@provider = provider
- @user = User.find_for_oauth(request.env['omniauth.auth'], current_user)
+ @user = User.find_for_omniauth(request.env['omniauth.auth'], current_user)
if @user.persisted?
record_login_activity
diff --git a/app/models/concerns/user/omniauthable.rb b/app/models/concerns/user/omniauthable.rb
index 113bfda230..396a0598f8 100644
--- a/app/models/concerns/user/omniauthable.rb
+++ b/app/models/concerns/user/omniauthable.rb
@@ -19,17 +19,18 @@ module User::Omniauthable
end
class_methods do
- def find_for_oauth(auth, signed_in_resource = nil)
+ def find_for_omniauth(auth, signed_in_resource = nil)
# EOLE-SSO Patch
auth.uid = (auth.uid[0][:uid] || auth.uid[0][:user]) if auth.uid.is_a? Hashie::Array
- identity = Identity.find_for_oauth(auth)
+ identity = Identity.find_for_omniauth(auth)
# If a signed_in_resource is provided it always overrides the existing user
# to prevent the identity being locked with accidentally created accounts.
# Note that this may leave zombie accounts (with no associated identity) which
# can be cleaned up at a later date.
user = signed_in_resource || identity.user
- user ||= create_for_oauth(auth)
+ user ||= reattach_for_auth(auth)
+ user ||= create_for_auth(auth)
if identity.user.nil?
identity.user = user
@@ -39,19 +40,35 @@ module User::Omniauthable
user
end
- def create_for_oauth(auth)
- # Check if the user exists with provided email. If no email was provided,
+ private
+
+ def reattach_for_auth(auth)
+ # If allowed, check if a user exists with the provided email address,
+ # and return it if they does not have an associated identity with the
+ # current authentication provider.
+
+ # This can be used to provide a choice of alternative auth providers
+ # or provide smooth gradual transition between multiple auth providers,
+ # but this is discouraged because any insecure provider will put *all*
+ # local users at risk, regardless of which provider they registered with.
+
+ return unless ENV['ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH'] == 'true'
+
+ email, email_is_verified = email_from_auth(auth)
+ return unless email_is_verified
+
+ user = User.find_by(email: email)
+ return if user.nil? || Identity.exists?(provider: auth.provider, user_id: user.id)
+
+ user
+ end
+
+ def create_for_auth(auth)
+ # Create a user for the given auth params. If no email was provided,
# we assign a temporary email and ask the user to verify it on
# the next step via Auth::SetupController.show
- strategy = Devise.omniauth_configs[auth.provider.to_sym].strategy
- assume_verified = strategy&.security&.assume_email_is_verified
- email_is_verified = auth.info.verified || auth.info.verified_email || auth.info.email_verified || assume_verified
- email = auth.info.verified_email || auth.info.email
-
- user = User.find_by(email: email) if email_is_verified
-
- return user unless user.nil?
+ email, email_is_verified = email_from_auth(auth)
user = User.new(user_params_from_auth(email, auth))
@@ -66,7 +83,14 @@ module User::Omniauthable
user
end
- private
+ def email_from_auth(auth)
+ strategy = Devise.omniauth_configs[auth.provider.to_sym].strategy
+ assume_verified = strategy&.security&.assume_email_is_verified
+ email_is_verified = auth.info.verified || auth.info.verified_email || auth.info.email_verified || assume_verified
+ email = auth.info.verified_email || auth.info.email
+
+ [email, email_is_verified]
+ end
def user_params_from_auth(email, auth)
{
diff --git a/app/models/identity.rb b/app/models/identity.rb
index c95a68a6f6..77821b78fa 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -17,7 +17,7 @@ class Identity < ApplicationRecord
validates :uid, presence: true, uniqueness: { scope: :provider }
validates :provider, presence: true
- def self.find_for_oauth(auth)
+ def self.find_for_omniauth(auth)
find_or_create_by(uid: auth.uid, provider: auth.provider)
end
end
diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
index 7022454443..d5a2ffbc86 100644
--- a/spec/models/identity_spec.rb
+++ b/spec/models/identity_spec.rb
@@ -3,19 +3,19 @@
require 'rails_helper'
RSpec.describe Identity do
- describe '.find_for_oauth' do
+ describe '.find_for_omniauth' do
let(:auth) { Fabricate(:identity, user: Fabricate(:user)) }
it 'calls .find_or_create_by' do
allow(described_class).to receive(:find_or_create_by)
- described_class.find_for_oauth(auth)
+ described_class.find_for_omniauth(auth)
expect(described_class).to have_received(:find_or_create_by).with(uid: auth.uid, provider: auth.provider)
end
it 'returns an instance of Identity' do
- expect(described_class.find_for_oauth(auth)).to be_instance_of described_class
+ expect(described_class.find_for_omniauth(auth)).to be_instance_of described_class
end
end
end
diff --git a/spec/requests/omniauth_callbacks_spec.rb b/spec/requests/omniauth_callbacks_spec.rb
index 0d37c41140..b478ca1ce6 100644
--- a/spec/requests/omniauth_callbacks_spec.rb
+++ b/spec/requests/omniauth_callbacks_spec.rb
@@ -96,7 +96,7 @@ describe 'OmniAuth callbacks' do
context 'when a user cannot be built' do
before do
- allow(User).to receive(:find_for_oauth).and_return(User.new)
+ allow(User).to receive(:find_for_omniauth).and_return(User.new)
end
it 'redirects to the new user signup page' do