Skip to content

Commit

Permalink
Rename methods to avoid confusion between OAuth and OmniAuth
Browse files Browse the repository at this point in the history
  • Loading branch information
ClearlyClaire committed Feb 14, 2024
1 parent 7857ed3 commit b33a9d4
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 17 deletions.
2 changes: 1 addition & 1 deletion app/controllers/auth/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 11 additions & 11 deletions app/models/concerns/omniauthable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ def email_present?
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 ||= reattach_for_oauth(auth)
user ||= create_for_oauth(auth)
user ||= reattach_for_auth(auth)
user ||= create_for_auth(auth)

if identity.user.nil?
identity.user = user
Expand All @@ -40,7 +40,9 @@ def find_for_oauth(auth, signed_in_resource = nil)
user
end

def reattach_for_oauth(auth)
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.
Expand All @@ -52,7 +54,7 @@ def reattach_for_oauth(auth)

return unless ENV['ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH'] == 'true'

email, email_is_verified = email_from_oauth(auth)
email, email_is_verified = email_from_auth(auth)
return unless email_is_verified

user = User.find_by(email: email)
Expand All @@ -61,12 +63,12 @@ def reattach_for_oauth(auth)
user
end

def create_for_oauth(auth)
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

email, email_is_verified = email_from_oauth(auth)
email, email_is_verified = email_from_auth(auth)

user = User.new(user_params_from_auth(email, auth))

Expand All @@ -81,9 +83,7 @@ def create_for_oauth(auth)
user
end

private

def email_from_oauth(auth)
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
Expand Down
2 changes: 1 addition & 1 deletion app/models/identity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions spec/models/identity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
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
expect(described_class).to receive(:find_or_create_by).with(uid: auth.uid, provider: auth.provider)
described_class.find_for_oauth(auth)
described_class.find_for_omniauth(auth)
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
2 changes: 1 addition & 1 deletion spec/requests/omniauth_callbacks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@

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
Expand Down

0 comments on commit b33a9d4

Please sign in to comment.