Commit f014127e authored by Patricio Cano's avatar Patricio Cano

Decouple SAML authentication from the default Omniauth logic

parent 7f7eef2a
...@@ -64,6 +64,9 @@ v 8.5.0 (unreleased) ...@@ -64,6 +64,9 @@ v 8.5.0 (unreleased)
- Fix broken link to project in build notification emails - Fix broken link to project in build notification emails
- Ability to see and sort on vote count from Issues and MR lists - 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 - 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 v 8.4.4
- Update omniauth-saml gem to 1.4.2 - Update omniauth-saml gem to 1.4.2
......
...@@ -42,6 +42,26 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -42,6 +42,26 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
end end
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 def omniauth_error
@provider = params[:provider] @provider = params[:provider]
@error = params[:error] @error = params[:error]
...@@ -65,25 +85,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -65,25 +85,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
log_audit_event(current_user, with: oauth['provider']) log_audit_event(current_user, with: oauth['provider'])
redirect_to profile_account_path, notice: 'Authentication method updated' redirect_to profile_account_path, notice: 'Authentication method updated'
else else
@user = Gitlab::OAuth::User.new(oauth) oauth_user = Gitlab::OAuth::User.new(oauth)
@user.save oauth_user.save
@user = oauth_user.gl_user
# Only allow properly saved users to login. continue_login_process
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 end
rescue Gitlab::OAuth::SignupDisabledError rescue Gitlab::OAuth::SignupDisabledError
label = Gitlab::OAuth::Provider.label_for(oauth['provider']) label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
...@@ -104,6 +110,18 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -104,6 +110,18 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
session[:service_tickets][provider] = ticket session[:service_tickets][provider] = ticket
end 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 def oauth
@oauth ||= request.env['omniauth.auth'] @oauth ||= request.env['omniauth.auth']
end end
......
...@@ -288,15 +288,22 @@ production: &base ...@@ -288,15 +288,22 @@ production: &base
# auto_sign_in_with_provider: saml # auto_sign_in_with_provider: saml
# CAUTION! # CAUTION!
# This allows users to login without having a user account first (default: false). # This allows users to login without having a user account first. Define the allowed
# providers using an array, e.g. ["saml", "twitter"]
# User accounts will be created automatically when authentication was successful. # User accounts will be created automatically when authentication was successful.
allow_single_sign_on: false allow_single_sign_on: ["saml"]
# Locks down those users until they have been cleared by the admin (default: true). # Locks down those users until they have been cleared by the admin (default: true).
block_auto_created_users: true block_auto_created_users: true
# Look up new users in LDAP servers. If a match is found (same uid), automatically # 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) # link the omniauth identity with the LDAP account. (default: false)
auto_link_ldap_user: 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 ## Auth providers
# Uncomment the following lines and fill in the data of the auth provider you want to use # 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: # If your favorite auth provider is not listed you can use others:
......
...@@ -24,6 +24,10 @@ module Gitlab ...@@ -24,6 +24,10 @@ module Gitlab
update_user_attributes update_user_attributes
end end
def save
super('LDAP')
end
# instance methods # instance methods
def gl_user def gl_user
@gl_user ||= find_by_uid_and_provider || find_by_email || build_new_user @gl_user ||= find_by_uid_and_provider || find_by_email || build_new_user
......
...@@ -26,7 +26,7 @@ module Gitlab ...@@ -26,7 +26,7 @@ module Gitlab
gl_user.try(:valid?) gl_user.try(:valid?)
end end
def save def save(provider = 'OAuth')
unauthorized_to_create unless gl_user unauthorized_to_create unless gl_user
if needs_blocking? if needs_blocking?
...@@ -36,10 +36,10 @@ module Gitlab ...@@ -36,10 +36,10 @@ module Gitlab
gl_user.save! gl_user.save!
end end
log.info "(OAuth) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}" log.info "(#{provider}) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
gl_user gl_user
rescue ActiveRecord::RecordInvalid => e rescue ActiveRecord::RecordInvalid => e
log.info "(OAuth) Error saving user: #{gl_user.errors.full_messages}" log.info "(#{provider}) Error saving user: #{gl_user.errors.full_messages}"
return self, e.record.errors return self, e.record.errors
end end
...@@ -105,7 +105,8 @@ module Gitlab ...@@ -105,7 +105,8 @@ module Gitlab
end end
def signup_enabled? def signup_enabled?
Gitlab.config.omniauth.allow_single_sign_on providers = Gitlab.config.omniauth.allow_single_sign_on
providers.include?(auth_hash.provider)
end end
def block_after_signup? def block_after_signup?
......
# 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
...@@ -42,7 +42,7 @@ describe Gitlab::OAuth::User, lib: true do ...@@ -42,7 +42,7 @@ describe Gitlab::OAuth::User, lib: true do
describe 'signup' do describe 'signup' do
shared_examples "to verify compliance with allow_single_sign_on" do shared_examples "to verify compliance with allow_single_sign_on" do
context "with allow_single_sign_on enabled" do context "with allow_single_sign_on enabled" do
before { stub_omniauth_config(allow_single_sign_on: true) } before { stub_omniauth_config(allow_single_sign_on: ['twitter']) }
it "creates a user from Omniauth" do it "creates a user from Omniauth" do
oauth_user.save oauth_user.save
...@@ -55,7 +55,7 @@ describe Gitlab::OAuth::User, lib: true do ...@@ -55,7 +55,7 @@ describe Gitlab::OAuth::User, lib: true do
end end
context "with allow_single_sign_on disabled (Default)" do context "with allow_single_sign_on disabled (Default)" do
before { stub_omniauth_config(allow_single_sign_on: false) } before { stub_omniauth_config(allow_single_sign_on: []) }
it "throws an error" do it "throws an error" do
expect{ oauth_user.save }.to raise_error StandardError expect{ oauth_user.save }.to raise_error StandardError
end end
...@@ -135,7 +135,7 @@ describe Gitlab::OAuth::User, lib: true do ...@@ -135,7 +135,7 @@ describe Gitlab::OAuth::User, lib: true do
describe 'blocking' do describe 'blocking' do
let(:provider) { 'twitter' } let(:provider) { 'twitter' }
before { stub_omniauth_config(allow_single_sign_on: true) } before { stub_omniauth_config(allow_single_sign_on: ['twitter']) }
context 'signup with omniauth only' do context 'signup with omniauth only' do
context 'dont block on create' do context 'dont block on create' do
......
This diff is collapsed.
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