Merge branch '38822-oauth-search-case-insensitive' into 'master'
Changing OAuth lookup to be case insensitive Closes #38822 See merge request gitlab-org/gitlab-ce!15312
This commit is contained in:
commit
e68ee8af4d
|
@ -54,7 +54,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
|
|||
if current_user
|
||||
log_audit_event(current_user, with: :saml)
|
||||
# Update SAML identity if data has changed.
|
||||
identity = current_user.identities.find_by(extern_uid: oauth['uid'], provider: :saml)
|
||||
identity = current_user.identities.with_extern_uid(:saml, oauth['uid']).take
|
||||
if identity.nil?
|
||||
current_user.identities.create(extern_uid: oauth['uid'], provider: :saml)
|
||||
redirect_to profile_account_path, notice: 'Authentication method updated'
|
||||
|
@ -98,7 +98,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
|
|||
def handle_omniauth
|
||||
if current_user
|
||||
# Add new authentication method
|
||||
current_user.identities.find_or_create_by(extern_uid: oauth['uid'], provider: oauth['provider'])
|
||||
current_user.identities
|
||||
.with_extern_uid(oauth['provider'], oauth['uid'])
|
||||
.first_or_create(extern_uid: oauth['uid'])
|
||||
log_audit_event(current_user, with: oauth['provider'])
|
||||
redirect_to profile_account_path, notice: 'Authentication method updated'
|
||||
else
|
||||
|
|
|
@ -1,18 +1,27 @@
|
|||
class Identity < ActiveRecord::Base
|
||||
include Sortable
|
||||
include CaseSensitivity
|
||||
|
||||
belongs_to :user
|
||||
|
||||
validates :provider, presence: true
|
||||
validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
|
||||
validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider, case_sensitive: false }
|
||||
validates :user_id, uniqueness: { scope: :provider }
|
||||
|
||||
scope :with_provider, ->(provider) { where(provider: provider) }
|
||||
scope :with_extern_uid, ->(provider, extern_uid) do
|
||||
extern_uid = Gitlab::LDAP::Person.normalize_dn(extern_uid) if provider.starts_with?('ldap')
|
||||
where(extern_uid: extern_uid, provider: provider)
|
||||
iwhere(extern_uid: normalize_uid(provider, extern_uid)).with_provider(provider)
|
||||
end
|
||||
|
||||
def ldap?
|
||||
provider.starts_with?('ldap')
|
||||
end
|
||||
|
||||
def self.normalize_uid(provider, uid)
|
||||
if provider.to_s.starts_with?('ldap')
|
||||
Gitlab::LDAP::Person.normalize_dn(uid)
|
||||
else
|
||||
uid.to_s
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -269,8 +269,7 @@ class User < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def for_github_id(id)
|
||||
joins(:identities)
|
||||
.where(identities: { provider: :github, extern_uid: id.to_s })
|
||||
joins(:identities).merge(Identity.with_extern_uid(:github, id))
|
||||
end
|
||||
|
||||
# Find a User by their primary email or any associated secondary email
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: OAuth identity lookups case-insensitive
|
||||
merge_request: 15312
|
||||
author:
|
||||
type: fixed
|
|
@ -9,11 +9,8 @@ module Gitlab
|
|||
class User < Gitlab::OAuth::User
|
||||
class << self
|
||||
def find_by_uid_and_provider(uid, provider)
|
||||
uid = Gitlab::LDAP::Person.normalize_dn(uid)
|
||||
identity = ::Identity.with_extern_uid(provider, uid).take
|
||||
|
||||
identity = ::Identity
|
||||
.where(provider: provider)
|
||||
.where(extern_uid: uid).last
|
||||
identity && identity.user
|
||||
end
|
||||
end
|
||||
|
|
|
@ -157,7 +157,7 @@ module Gitlab
|
|||
end
|
||||
|
||||
def find_by_uid_and_provider
|
||||
identity = Identity.find_by(provider: auth_hash.provider, extern_uid: auth_hash.uid)
|
||||
identity = Identity.with_extern_uid(auth_hash.provider, auth_hash.uid).take
|
||||
identity && identity.user
|
||||
end
|
||||
|
||||
|
|
|
@ -662,4 +662,13 @@ describe Gitlab::OAuth::User do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '.find_by_uid_and_provider' do
|
||||
let!(:existing_user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') }
|
||||
|
||||
it 'normalizes extern_uid' do
|
||||
allow(oauth_user.auth_hash).to receive(:uid).and_return('MY-UID')
|
||||
expect(oauth_user.find_user).to eql gl_user
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -33,5 +33,15 @@ describe Identity do
|
|||
expect(identity).to eq(ldap_identity)
|
||||
end
|
||||
end
|
||||
|
||||
context 'any other provider' do
|
||||
let!(:test_entity) { create(:identity, provider: 'test_provider', extern_uid: 'test_uid') }
|
||||
|
||||
it 'the extern_uid lookup is case insensitive' do
|
||||
identity = described_class.with_extern_uid('test_provider', 'TEST_UID').first
|
||||
|
||||
expect(identity).to eq(test_entity)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue