Skip to content
Snippets Groups Projects
Commit 873b0db2 authored by Douwe Maan's avatar Douwe Maan
Browse files

Revert "Merge branch 'saml-decoupling' into 'master' 

This reverts commit c04e22fb, reversing
changes made to 0feab326.
parent 6524fbea
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
CHANGELOG
+ 0
− 3
View file @ 873b0db2
@@ -64,9 +64,6 @@ v 8.5.0 (unreleased)
- Fix broken link to project in build notification emails
- Ability to see and sort on vote count from Issues and MR lists
- Fix builds scheduler when first build in stage was allowed to fail
- Allow SAML users to login with no previous account without having to allow
all Omniauth providers to do so.
- Allow existing users to auto link their SAML credentials by logging in via SAML
 
v 8.4.4
- Update omniauth-saml gem to 1.4.2
app/controllers/omniauth_callbacks_controller.rb
+ 18
− 36
View file @ 873b0db2
@@ -42,26 +42,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
end
end
 
def saml
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)
if identity.nil?
current_user.identities.create(extern_uid: oauth['uid'], provider: :saml)
redirect_to profile_account_path, notice: 'Authentication method updated'
else
redirect_to after_sign_in_path_for(current_user)
end
else
saml_user = Gitlab::Saml::User.new(oauth)
saml_user.save
@user = saml_user.gl_user
continue_login_process
end
end
def omniauth_error
@provider = params[:provider]
@error = params[:error]
@@ -85,11 +65,25 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
log_audit_event(current_user, with: oauth['provider'])
redirect_to profile_account_path, notice: 'Authentication method updated'
else
oauth_user = Gitlab::OAuth::User.new(oauth)
oauth_user.save
@user = oauth_user.gl_user
@user = Gitlab::OAuth::User.new(oauth)
@user.save
 
continue_login_process
# Only allow properly saved users to login.
if @user.persisted? && @user.valid?
log_audit_event(@user.gl_user, with: oauth['provider'])
sign_in_and_redirect(@user.gl_user)
else
error_message =
if @user.gl_user.errors.any?
@user.gl_user.errors.map do |attribute, message|
"#{attribute} #{message}"
end.join(", ")
else
''
end
redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return
end
end
rescue Gitlab::OAuth::SignupDisabledError
label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
@@ -110,18 +104,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
session[:service_tickets][provider] = ticket
end
 
def continue_login_process
# Only allow properly saved users to login.
if @user.persisted? && @user.valid?
log_audit_event(@user, with: oauth['provider'])
sign_in_and_redirect(@user)
else
error_message = @user.errors.full_messages.to_sentence
redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return
end
end
def oauth
@oauth ||= request.env['omniauth.auth']
end
config/gitlab.yml.example
+ 2
− 9
View file @ 873b0db2
@@ -288,22 +288,15 @@ production: &base
# auto_sign_in_with_provider: saml
 
# CAUTION!
# This allows users to login without having a user account first. Define the allowed
# providers using an array, e.g. ["saml", "twitter"]
# This allows users to login without having a user account first (default: false).
# User accounts will be created automatically when authentication was successful.
allow_single_sign_on: ["saml"]
allow_single_sign_on: false
# Locks down those users until they have been cleared by the admin (default: true).
block_auto_created_users: true
# Look up new users in LDAP servers. If a match is found (same uid), automatically
# link the omniauth identity with the LDAP account. (default: false)
auto_link_ldap_user: false
 
# Allow users with existing accounts to login and auto link their account via SAML
# login, without having to do a manual login first and manually add SAML
# (default: false)
auto_link_saml_user: false
## Auth providers
# Uncomment the following lines and fill in the data of the auth provider you want to use
# If your favorite auth provider is not listed you can use others:
lib/gitlab/ldap/user.rb
+ 0
− 4
View file @ 873b0db2
@@ -24,10 +24,6 @@ module Gitlab
update_user_attributes
end
 
def save
super('LDAP')
end
# instance methods
def gl_user
@gl_user ||= find_by_uid_and_provider || find_by_email || build_new_user
lib/gitlab/o_auth/user.rb
+ 4
− 5
View file @ 873b0db2
@@ -26,7 +26,7 @@ module Gitlab
gl_user.try(:valid?)
end
 
def save(provider = 'OAuth')
def save
unauthorized_to_create unless gl_user
 
if needs_blocking?
@@ -36,10 +36,10 @@ module Gitlab
gl_user.save!
end
 
log.info "(#{provider}) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
log.info "(OAuth) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
gl_user
rescue ActiveRecord::RecordInvalid => e
log.info "(#{provider}) Error saving user: #{gl_user.errors.full_messages}"
log.info "(OAuth) Error saving user: #{gl_user.errors.full_messages}"
return self, e.record.errors
end
 
@@ -105,8 +105,7 @@ module Gitlab
end
 
def signup_enabled?
providers = Gitlab.config.omniauth.allow_single_sign_on
providers.include?(auth_hash.provider)
Gitlab.config.omniauth.allow_single_sign_on
end
 
def block_after_signup?
lib/gitlab/saml/user.rb deleted 100644 → 0
+ 0
− 47
View file @ 6524fbea
# SAML extension for User model
#
# * Find GitLab user based on SAML uid and provider
# * Create new user from SAML data
#
module Gitlab
module Saml
class User < Gitlab::OAuth::User
def save
super('SAML')
end
def gl_user
@user ||= find_by_uid_and_provider
if auto_link_ldap_user?
@user ||= find_or_create_ldap_user
end
if auto_link_saml_enabled?
@user ||= find_by_email
end
if signup_enabled?
@user ||= build_new_user
end
@user
end
def find_by_email
if auth_hash.has_email?
user = ::User.find_by(email: auth_hash.email.downcase)
user.identities.new(extern_uid: auth_hash.uid, provider: auth_hash.provider) if user
user
end
end
protected
def auto_link_saml_enabled?
Gitlab.config.omniauth.auto_link_saml_user
end
end
end
end
spec/lib/gitlab/o_auth/user_spec.rb
+ 3
− 3
View file @ 873b0db2
@@ -42,7 +42,7 @@ describe Gitlab::OAuth::User, lib: true do
describe 'signup' do
shared_examples "to verify compliance with allow_single_sign_on" do
context "with allow_single_sign_on enabled" do
before { stub_omniauth_config(allow_single_sign_on: ['twitter']) }
before { stub_omniauth_config(allow_single_sign_on: true) }
 
it "creates a user from Omniauth" do
oauth_user.save
@@ -55,7 +55,7 @@ describe Gitlab::OAuth::User, lib: true do
end
 
context "with allow_single_sign_on disabled (Default)" do
before { stub_omniauth_config(allow_single_sign_on: []) }
before { stub_omniauth_config(allow_single_sign_on: false) }
it "throws an error" do
expect{ oauth_user.save }.to raise_error StandardError
end
@@ -135,7 +135,7 @@ describe Gitlab::OAuth::User, lib: true do
 
describe 'blocking' do
let(:provider) { 'twitter' }
before { stub_omniauth_config(allow_single_sign_on: ['twitter']) }
before { stub_omniauth_config(allow_single_sign_on: true) }
 
context 'signup with omniauth only' do
context 'dont block on create' do
spec/lib/gitlab/saml/user_spec.rb deleted 100644 → 0
+ 0
− 271
View file @ 6524fbea
require 'spec_helper'
describe Gitlab::Saml::User, lib: true do
let(:saml_user) { described_class.new(auth_hash) }
let(:gl_user) { saml_user.gl_user }
let(:uid) { 'my-uid' }
let(:provider) { 'saml' }
let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash) }
let(:info_hash) do
{
name: 'John',
email: 'john@mail.com'
}
end
let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, 'ldapmain') }
describe '#save' do
def stub_omniauth_config(messages)
allow(Gitlab.config.omniauth).to receive_messages(messages)
end
def stub_ldap_config(messages)
allow(Gitlab::LDAP::Config).to receive_messages(messages)
end
describe 'account exists on server' do
before { stub_omniauth_config({ allow_single_sign_on: ['saml'], auto_link_saml_user: true }) }
context 'and should bind with SAML' do
let!(:existing_user) { create(:user, email: 'john@mail.com', username: 'john') }
it 'adds the SAML identity to the existing user' do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).to eq existing_user
identity = gl_user.identities.first
expect(identity.extern_uid).to eql uid
expect(identity.provider).to eql 'saml'
end
end
end
describe 'no account exists on server' do
shared_examples 'to verify compliance with allow_single_sign_on' do
context 'with allow_single_sign_on enabled' do
before { stub_omniauth_config(allow_single_sign_on: ['saml']) }
it 'creates a user from SAML' do
saml_user.save
expect(gl_user).to be_valid
identity = gl_user.identities.first
expect(identity.extern_uid).to eql uid
expect(identity.provider).to eql 'saml'
end
end
context 'with allow_single_sign_on default (["saml"])' do
before { stub_omniauth_config(allow_single_sign_on: ['saml']) }
it 'should not throw an error' do
expect{ saml_user.save }.not_to raise_error
end
end
context 'with allow_single_sign_on disabled' do
before { stub_omniauth_config(allow_single_sign_on: []) }
it 'should throw an error' do
expect{ saml_user.save }.to raise_error StandardError
end
end
end
context 'with auto_link_ldap_user disabled (default)' do
before { stub_omniauth_config({ auto_link_ldap_user: false, auto_link_saml_user: false, allow_single_sign_on: ['saml'] }) }
include_examples 'to verify compliance with allow_single_sign_on'
end
context 'with auto_link_ldap_user enabled' do
before { stub_omniauth_config({ auto_link_ldap_user: true, auto_link_saml_user: false }) }
context 'and no LDAP provider defined' do
before { stub_ldap_config(providers: []) }
include_examples 'to verify compliance with allow_single_sign_on'
end
context 'and at least one LDAP provider is defined' do
before { stub_ldap_config(providers: %w(ldapmain)) }
context 'and a corresponding LDAP person' do
before do
allow(ldap_user).to receive(:uid) { uid }
allow(ldap_user).to receive(:username) { uid }
allow(ldap_user).to receive(:email) { ['johndoe@example.com','john2@example.com'] }
allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' }
allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user)
end
context 'and no account for the LDAP user' do
it 'creates a user with dual LDAP and SAML identities' do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user.username).to eql uid
expect(gl_user.email).to eql 'johndoe@example.com'
expect(gl_user.identities.length).to eql 2
identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } }
expect(identities_as_hash).to match_array([ { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' },
{ provider: 'saml', extern_uid: uid }
])
end
end
context 'and LDAP user has an account already' do
let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') }
it "adds the omniauth identity to the LDAP account" do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user.username).to eql 'john'
expect(gl_user.email).to eql 'john@example.com'
expect(gl_user.identities.length).to eql 2
identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } }
expect(identities_as_hash).to match_array([ { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' },
{ provider: 'saml', extern_uid: uid }
])
end
end
end
context 'and no corresponding LDAP person' do
before { allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(nil) }
include_examples 'to verify compliance with allow_single_sign_on'
end
end
end
end
describe 'blocking' do
before { stub_omniauth_config({ allow_saml_sign_up: true, auto_link_saml_user: true }) }
context 'signup with SAML only' do
context 'dont block on create' do
before { stub_omniauth_config(block_auto_created_users: false) }
it 'should not block the user' do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).not_to be_blocked
end
end
context 'block on create' do
before { stub_omniauth_config(block_auto_created_users: true) }
it 'should block user' do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).to be_blocked
end
end
end
context 'signup with linked omniauth and LDAP account' do
before do
stub_omniauth_config(auto_link_ldap_user: true)
allow(ldap_user).to receive(:uid) { uid }
allow(ldap_user).to receive(:username) { uid }
allow(ldap_user).to receive(:email) { ['johndoe@example.com','john2@example.com'] }
allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' }
allow(saml_user).to receive(:ldap_person).and_return(ldap_user)
end
context "and no account for the LDAP user" do
context 'dont block on create (LDAP)' do
before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: false) }
it do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).not_to be_blocked
end
end
context 'block on create (LDAP)' do
before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: true) }
it do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).to be_blocked
end
end
end
context 'and LDAP user has an account already' do
let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') }
context 'dont block on create (LDAP)' do
before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: false) }
it do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).not_to be_blocked
end
end
context 'block on create (LDAP)' do
before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: true) }
it do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).not_to be_blocked
end
end
end
end
context 'sign-in' do
before do
saml_user.save
saml_user.gl_user.activate
end
context 'dont block on create' do
before { stub_omniauth_config(block_auto_created_users: false) }
it do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).not_to be_blocked
end
end
context 'block on create' do
before { stub_omniauth_config(block_auto_created_users: true) }
it do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).not_to be_blocked
end
end
context 'dont block on create (LDAP)' do
before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: false) }
it do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).not_to be_blocked
end
end
context 'block on create (LDAP)' do
before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: true) }
it do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user).not_to be_blocked
end
end
end
end
end
end
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