Commit 85a67799 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'dm-ldap-identity-normalize-dn' into 'master'

Normalize LDAP DN when looking up identity

Closes #39559

See merge request gitlab-org/gitlab-ce!15103
parents 01c95492 8399de0c
......@@ -7,7 +7,10 @@ class Identity < ActiveRecord::Base
validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
validates :user_id, uniqueness: { scope: :provider }
scope :with_extern_uid, ->(provider, extern_uid) { where(extern_uid: extern_uid, 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)
end
def ldap?
provider.starts_with?('ldap')
......
---
title: Normalize LDAP DN when looking up identity
merge_request:
author:
type: fixed
......@@ -4,7 +4,7 @@ module Gitlab
module LDAP
class AuthHash < Gitlab::OAuth::AuthHash
def uid
Gitlab::LDAP::Person.normalize_dn(super)
@uid ||= Gitlab::LDAP::Person.normalize_dn(super)
end
private
......
......@@ -9,10 +9,11 @@ module Gitlab
class User < Gitlab::OAuth::User
class << self
def find_by_uid_and_provider(uid, provider)
# LDAP distinguished name is case-insensitive
uid = Gitlab::LDAP::Person.normalize_dn(uid)
identity = ::Identity
.where(provider: provider)
.iwhere(extern_uid: uid).last
.where(extern_uid: uid).last
identity && identity.user
end
end
......
require 'spec_helper'
describe Gitlab::LDAP::Authentication do
let(:user) { create(:omniauth_user, extern_uid: dn) }
let(:dn) { 'uid=john,ou=people,dc=example,dc=com' }
let(:dn) { 'uid=John Smith, ou=People, dc=example, dc=com' }
let(:user) { create(:omniauth_user, extern_uid: Gitlab::LDAP::Person.normalize_dn(dn)) }
let(:login) { 'john' }
let(:password) { 'password' }
......
......@@ -44,23 +44,25 @@ describe Gitlab::LDAP::User do
end
describe '.find_by_uid_and_provider' do
let(:dn) { 'CN=John Åström, CN=Users, DC=Example, DC=com' }
it 'retrieves the correct user' do
special_info = {
name: 'John Åström',
email: 'john@example.com',
nickname: 'jastrom'
}
special_hash = OmniAuth::AuthHash.new(uid: 'CN=John Åström,CN=Users,DC=Example,DC=com', provider: 'ldapmain', info: special_info)
special_hash = OmniAuth::AuthHash.new(uid: dn, provider: 'ldapmain', info: special_info)
special_chars_user = described_class.new(special_hash)
user = special_chars_user.save
expect(described_class.find_by_uid_and_provider(special_hash.uid, special_hash.provider)).to eq user
expect(described_class.find_by_uid_and_provider(dn, 'ldapmain')).to eq user
end
end
describe 'find or create' do
it "finds the user if already existing" do
create(:omniauth_user, extern_uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain')
create(:omniauth_user, extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain')
expect { ldap_user.save }.not_to change { User.count }
end
......
......@@ -4,7 +4,7 @@ describe Gitlab::OAuth::User do
let(:oauth_user) { described_class.new(auth_hash) }
let(:gl_user) { oauth_user.gl_user }
let(:uid) { 'my-uid' }
let(:dn) { 'uid=user1,ou=People,dc=example' }
let(:dn) { 'uid=user1,ou=people,dc=example' }
let(:provider) { 'my-provider' }
let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash) }
let(:info_hash) do
......
......@@ -7,7 +7,7 @@ describe Gitlab::Saml::User do
let(:saml_user) { described_class.new(auth_hash) }
let(:gl_user) { saml_user.gl_user }
let(:uid) { 'my-uid' }
let(:dn) { 'uid=user1,ou=People,dc=example' }
let(:dn) { 'uid=user1,ou=people,dc=example' }
let(:provider) { 'saml' }
let(:raw_info_attr) { { 'groups' => %w(Developers Freelancers Designers) } }
let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash, extra: { raw_info: OneLogin::RubySaml::Attributes.new(raw_info_attr) }) }
......
require 'spec_helper'
RSpec.describe Identity do
describe Identity do
describe 'relations' do
it { is_expected.to belong_to(:user) }
end
......@@ -22,4 +22,16 @@ RSpec.describe Identity do
expect(other_identity.ldap?).to be_falsey
end
end
describe '.with_extern_uid' do
context 'LDAP identity' do
let!(:ldap_identity) { create(:identity, provider: 'ldapmain', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com') }
it 'finds the identity when the DN is formatted differently' do
identity = described_class.with_extern_uid('ldapmain', 'uid=John Smith, ou=People, dc=example, dc=com').first
expect(identity).to eq(ldap_identity)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment