diff options
-rw-r--r-- | app/controllers/omniauth_callbacks_controller.rb | 2 | ||||
-rw-r--r-- | doc/integration/saml.md | 75 | ||||
-rw-r--r-- | lib/gitlab/auth/o_auth/user.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/auth/saml/auth_hash.rb | 15 | ||||
-rw-r--r-- | lib/gitlab/auth/saml/config.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/auth/saml/user.rb | 4 | ||||
-rw-r--r-- | spec/controllers/omniauth_callbacks_controller_spec.rb | 189 | ||||
-rw-r--r-- | spec/dependencies/omniauth_saml_spec.rb | 22 | ||||
-rw-r--r-- | spec/features/users/login_spec.rb | 35 | ||||
-rw-r--r-- | spec/fixtures/authentication/saml_response.xml | 42 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/o_auth/user_spec.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/saml/auth_hash_spec.rb | 51 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/saml/user_spec.rb | 41 | ||||
-rw-r--r-- | spec/support/helpers/login_helpers.rb | 36 |
14 files changed, 435 insertions, 93 deletions
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index ba62d2d5142..1547d4b5972 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -119,7 +119,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController set_remember_me(user) - if user.two_factor_enabled? + if user.two_factor_enabled? && !auth_user.bypass_two_factor? prompt_for_two_factor(user) else sign_in_and_redirect(user) diff --git a/doc/integration/saml.md b/doc/integration/saml.md index 3f49432ce93..db06efdae53 100644 --- a/doc/integration/saml.md +++ b/doc/integration/saml.md @@ -179,6 +179,81 @@ tell GitLab which groups are external via the `external_groups:` element: } } ``` +## Bypass two factor authentication + +If you want some SAML authentication methods to count as 2FA on a per session basis, you can register them in the +`upstream_two_factor_authn_contexts` list: + +**For Omnibus installations:** + +1. Edit `/etc/gitlab/gitlab.rb`: + + ```ruby + gitlab_rails['omniauth_providers'] = [ + { + name: 'saml', + args: { + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', + idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', + idp_sso_target_url: 'https://login.example.com/idp', + issuer: 'https://gitlab.example.com', + name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', + upstream_two_factor_authn_contexts: + %w( + urn:oasis:names:tc:SAML:2.0:ac:classes:CertificateProtectedTransport + urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS + urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN + ) + + }, + label: 'Company Login' # optional label for SAML login button, defaults to "Saml" + } + ] + ``` + +1. Save the file and [reconfigure][] GitLab for the changes to take effect. + +--- + +**For installations from source:** + +1. Edit `config/gitlab.yml`: + + ```yaml + - { + name: 'saml', + args: { + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', + idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', + idp_sso_target_url: 'https://login.example.com/idp', + issuer: 'https://gitlab.example.com', + name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', + upstream_two_factor_authn_contexts: + [ + 'urn:oasis:names:tc:SAML:2.0:ac:classes:CertificateProtectedTransport', + 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS', + 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN' + ] + + }, + label: 'Company Login' # optional label for SAML login button, defaults to "Saml" + } + ``` + +1. Save the file and [restart GitLab][] for the changes ot take effect + + +In addition to the changes in GitLab, make sure that your Idp is returning the +`AuthnContext`. For example: + +```xml + <saml:AuthnStatement> + <saml:AuthnContext> + <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:MediumStrongCertificateProtectedTransport</saml:AuthnContextClassRef> + </saml:AuthnContext> + </saml:AuthnStatement> +``` + ## Customization ### `auto_sign_in_with_provider` diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb index 6c5d0788a0a..e7283b2f9e8 100644 --- a/lib/gitlab/auth/o_auth/user.rb +++ b/lib/gitlab/auth/o_auth/user.rb @@ -74,6 +74,10 @@ module Gitlab gl_user end + def bypass_two_factor? + false + end + protected def should_save? diff --git a/lib/gitlab/auth/saml/auth_hash.rb b/lib/gitlab/auth/saml/auth_hash.rb index c345a7e3f6c..3bc5e2864df 100644 --- a/lib/gitlab/auth/saml/auth_hash.rb +++ b/lib/gitlab/auth/saml/auth_hash.rb @@ -6,6 +6,17 @@ module Gitlab Array.wrap(get_raw(Gitlab::Auth::Saml::Config.groups)) end + def authn_context + response_object = auth_hash.extra[:response_object] + return nil if response_object.blank? + + document = response_object.decrypted_document + document ||= response_object.document + return nil if document.blank? + + extract_authn_context(document) + end + private def get_raw(key) @@ -13,6 +24,10 @@ module Gitlab # otherwise just the first value is returned auth_hash.extra[:raw_info].all[key] end + + def extract_authn_context(document) + REXML::XPath.first(document, "//saml:AuthnStatement/saml:AuthnContext/saml:AuthnContextClassRef/text()").to_s + end end end end diff --git a/lib/gitlab/auth/saml/config.rb b/lib/gitlab/auth/saml/config.rb index 5fa9581f837..625dab7c6f4 100644 --- a/lib/gitlab/auth/saml/config.rb +++ b/lib/gitlab/auth/saml/config.rb @@ -7,6 +7,10 @@ module Gitlab Gitlab::Auth::OAuth::Provider.config_for('saml') end + def upstream_two_factor_authn_contexts + options.args[:upstream_two_factor_authn_contexts] + end + def groups options[:groups_attribute] end diff --git a/lib/gitlab/auth/saml/user.rb b/lib/gitlab/auth/saml/user.rb index b8c84c37cd5..6c3b75f3eb0 100644 --- a/lib/gitlab/auth/saml/user.rb +++ b/lib/gitlab/auth/saml/user.rb @@ -34,6 +34,10 @@ module Gitlab gl_user.changed? || gl_user.identities.any?(&:changed?) end + def bypass_two_factor? + saml_config.upstream_two_factor_authn_contexts&.include?(auth_hash.authn_context) + end + protected def saml_config diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 5f0e8c5eca9..b23f183fec8 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -1,127 +1,162 @@ require 'spec_helper' -describe OmniauthCallbacksController do +describe OmniauthCallbacksController, type: :controller do include LoginHelpers - let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) } - - before do - mock_auth_hash(provider.to_s, extern_uid, user.email) - stub_omniauth_provider(provider, context: request) - end - - context 'when the user is on the last sign in attempt' do - let(:extern_uid) { 'my-uid' } + describe 'omniauth' do + let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) } before do - user.update(failed_attempts: User.maximum_attempts.pred) - subject.response = ActionDispatch::Response.new + mock_auth_hash(provider.to_s, extern_uid, user.email) + stub_omniauth_provider(provider, context: request) end - context 'when using a form based provider' do - let(:provider) { :ldap } - - it 'locks the user when sign in fails' do - allow(subject).to receive(:params).and_return(ActionController::Parameters.new(username: user.username)) - request.env['omniauth.error.strategy'] = OmniAuth::Strategies::LDAP.new(nil) - - subject.send(:failure) + context 'when the user is on the last sign in attempt' do + let(:extern_uid) { 'my-uid' } - expect(user.reload).to be_access_locked + before do + user.update(failed_attempts: User.maximum_attempts.pred) + subject.response = ActionDispatch::Response.new end - end - context 'when using a button based provider' do - let(:provider) { :github } + context 'when using a form based provider' do + let(:provider) { :ldap } - it 'does not lock the user when sign in fails' do - request.env['omniauth.error.strategy'] = OmniAuth::Strategies::GitHub.new(nil) + it 'locks the user when sign in fails' do + allow(subject).to receive(:params).and_return(ActionController::Parameters.new(username: user.username)) + request.env['omniauth.error.strategy'] = OmniAuth::Strategies::LDAP.new(nil) - subject.send(:failure) + subject.send(:failure) - expect(user.reload).not_to be_access_locked + expect(user.reload).to be_access_locked + end end - end - end - context 'strategies' do - context 'github' do - let(:extern_uid) { 'my-uid' } - let(:provider) { :github } + context 'when using a button based provider' do + let(:provider) { :github } - it 'allows sign in' do - post provider + it 'does not lock the user when sign in fails' do + request.env['omniauth.error.strategy'] = OmniAuth::Strategies::GitHub.new(nil) - expect(request.env['warden']).to be_authenticated - end - - shared_context 'sign_up' do - let(:user) { double(email: 'new@example.com') } + subject.send(:failure) - before do - stub_omniauth_setting(block_auto_created_users: false) + expect(user.reload).not_to be_access_locked end end + end - context 'sign up' do - include_context 'sign_up' + context 'strategies' do + context 'github' do + let(:extern_uid) { 'my-uid' } + let(:provider) { :github } - it 'is allowed' do + it 'allows sign in' do post provider expect(request.env['warden']).to be_authenticated end - end - - context 'when OAuth is disabled' do - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - settings = Gitlab::CurrentSettings.current_application_settings - settings.update(disabled_oauth_sign_in_sources: [provider.to_s]) - end - it 'prevents login via POST' do - post provider + shared_context 'sign_up' do + let(:user) { double(email: 'new@example.com') } - expect(request.env['warden']).not_to be_authenticated + before do + stub_omniauth_setting(block_auto_created_users: false) + end end - it 'shows warning when attempting login' do - post provider - - expect(response).to redirect_to new_user_session_path - expect(flash[:alert]).to eq('Signing in using GitHub has been disabled') - end + context 'sign up' do + include_context 'sign_up' - it 'allows linking the disabled provider' do - user.identities.destroy_all - sign_in(user) + it 'is allowed' do + post provider - expect { post provider }.to change { user.reload.identities.count }.by(1) + expect(request.env['warden']).to be_authenticated + end end - context 'sign up' do - include_context 'sign_up' + context 'when OAuth is disabled' do + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + settings = Gitlab::CurrentSettings.current_application_settings + settings.update(disabled_oauth_sign_in_sources: [provider.to_s]) + end - it 'is prevented' do + it 'prevents login via POST' do post provider expect(request.env['warden']).not_to be_authenticated end + + it 'shows warning when attempting login' do + post provider + + expect(response).to redirect_to new_user_session_path + expect(flash[:alert]).to eq('Signing in using GitHub has been disabled') + end + + it 'allows linking the disabled provider' do + user.identities.destroy_all + sign_in(user) + + expect { post provider }.to change { user.reload.identities.count }.by(1) + end + + context 'sign up' do + include_context 'sign_up' + + it 'is prevented' do + post provider + + expect(request.env['warden']).not_to be_authenticated + end + end + end + end + + context 'auth0' do + let(:extern_uid) { '' } + let(:provider) { :auth0 } + + it 'does not allow sign in without extern_uid' do + post 'auth0' + + expect(request.env['warden']).not_to be_authenticated + expect(response.status).to eq(302) + expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.') end end end + end + + describe '#saml' do + let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml') } + let(:mock_saml_response) { File.read('spec/fixtures/authentication/saml_response.xml') } + let(:saml_config) { mock_saml_config_with_upstream_two_factor_authn_contexts } + + before do + stub_omniauth_saml_config({ enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], + providers: [saml_config] }) + mock_auth_hash('saml', 'my-uid', user.email, mock_saml_response) + request.env["devise.mapping"] = Devise.mappings[:user] + request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth'] + post :saml, params: { SAMLResponse: mock_saml_response } + end - context 'auth0' do - let(:extern_uid) { '' } - let(:provider) { :auth0 } + context 'when worth two factors' do + let(:mock_saml_response) do + File.read('spec/fixtures/authentication/saml_response.xml') + .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN') + end - it 'does not allow sign in without extern_uid' do - post 'auth0' + it 'expects user to be signed_in' do + expect(request.env['warden']).to be_authenticated + end + end + context 'when not worth two factors' do + it 'expects user to provide second factor' do + expect(response).to render_template('devise/sessions/two_factor') expect(request.env['warden']).not_to be_authenticated - expect(response.status).to eq(302) - expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.') end end end diff --git a/spec/dependencies/omniauth_saml_spec.rb b/spec/dependencies/omniauth_saml_spec.rb new file mode 100644 index 00000000000..ccc604dc230 --- /dev/null +++ b/spec/dependencies/omniauth_saml_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' +require 'omniauth/strategies/saml' + +describe 'processing of SAMLResponse in dependencies' do + let(:mock_saml_response) { File.read('spec/fixtures/authentication/saml_response.xml') } + let(:saml_strategy) { OmniAuth::Strategies::SAML.new({}) } + let(:session_mock) { {} } + let(:settings) { OpenStruct.new({ soft: false, idp_cert_fingerprint: 'something' }) } + let(:auth_hash) { Gitlab::Auth::Saml::AuthHash.new(saml_strategy) } + + subject { auth_hash.authn_context } + + before do + allow(saml_strategy).to receive(:session).and_return(session_mock) + allow_any_instance_of(OneLogin::RubySaml::Response).to receive(:is_valid?).and_return(true) + saml_strategy.send(:handle_response, mock_saml_response, {}, settings ) { } + end + + it 'can extract AuthnContextClassRef from SAMLResponse param' do + is_expected.to eq 'urn:oasis:names:tc:SAML:2.0:ac:classes:Password' + end +end diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 1f8d31a5c88..24a2c89f50b 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -177,14 +177,35 @@ feature 'Login' do end context 'logging in via OAuth' do - it 'shows 2FA prompt after OAuth login' do - stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config]) - user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml') - gitlab_sign_in_via('saml', user, 'my-uid') + let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')} + let(:mock_saml_response) do + File.read('spec/fixtures/authentication/saml_response.xml') + end - expect(page).to have_content('Two-Factor Authentication') - enter_code(user.current_otp) - expect(current_path).to eq root_path + before do + stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], + providers: [mock_saml_config_with_upstream_two_factor_authn_contexts]) + gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response) + end + + context 'when authn_context is worth two factors' do + let(:mock_saml_response) do + File.read('spec/fixtures/authentication/saml_response.xml') + .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + end + + it 'signs user in without prompting for second factor' do + expect(page).not_to have_content('Two-Factor Authentication') + expect(current_path).to eq root_path + end + end + + context 'when authn_context is not worth two factors' do + it 'shows 2FA prompt after OAuth login' do + expect(page).to have_content('Two-Factor Authentication') + enter_code(user.current_otp) + expect(current_path).to eq root_path + end end end end diff --git a/spec/fixtures/authentication/saml_response.xml b/spec/fixtures/authentication/saml_response.xml new file mode 100644 index 00000000000..ac7b662be22 --- /dev/null +++ b/spec/fixtures/authentication/saml_response.xml @@ -0,0 +1,42 @@ +<?xml version='1.0'?> +<samlp:Response xmlns:samlp='urn:oasis:names:tc:SAML:2.0:protocol' xmlns:saml='urn:oasis:names:tc:SAML:2.0:assertion' ID='pfxb9b71715-2202-9a51-8ae5-689d5b9dd25a' Version='2.0' IssueInstant='2014-07-17T01:01:48Z' Destination='http://sp.example.com/demo1/index.php?acs' InResponseTo='ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685'> + <saml:Issuer>http://idp.example.com/metadata.php</saml:Issuer><ds:Signature xmlns:ds='http://www.w3.org/2000/09/xmldsig#'> + <ds:SignedInfo><ds:CanonicalizationMethod Algorithm='http://www.w3.org/2001/10/xml-exc-c14n#'/> + <ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/> + <ds:Reference URI='#pfxb9b71715-2202-9a51-8ae5-689d5b9dd25a'><ds:Transforms><ds:Transform Algorithm='http://www.w3.org/2000/09/xmldsig#enveloped-signature'/><ds:Transform Algorithm='http://www.w3.org/2001/10/xml-exc-c14n#'/></ds:Transforms><ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#sha1'/><ds:DigestValue>z0Y25hsUHVJJnYhgB5LzPVjqbgM=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>NSdsZopzNX4kJETipLNbU+7dG4GPTj5e40iSBaUeUMc1UUSX4UCe9Qx6R9ADEkEQgNekgYaCFOuY90kLNh9Ky0Czq8gd4w7ykQJEVJ7VF7LakmG8dPedHAKyAMAuZ8y3mNGye31vtR9frYaznCVoxB3eAi9rbVOXkQtdOTRMHec=</ds:SignatureValue> + <ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIICajCCAdOgAwIBAgIBADANBgkqhkiG9w0BAQ0FADBSMQswCQYDVQQGEwJ1czETMBEGA1UECAwKQ2FsaWZvcm5pYTEVMBMGA1UECgwMT25lbG9naW4gSW5jMRcwFQYDVQQDDA5zcC5leGFtcGxlLmNvbTAeFw0xNDA3MTcxNDEyNTZaFw0xNTA3MTcxNDEyNTZaMFIxCzAJBgNVBAYTAnVzMRMwEQYDVQQIDApDYWxpZm9ybmlhMRUwEwYDVQQKDAxPbmVsb2dpbiBJbmMxFzAVBgNVBAMMDnNwLmV4YW1wbGUuY29tMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDZx+ON4IUoIWxgukTb1tOiX3bMYzYQiwWPUNMp+Fq82xoNogso2bykZG0yiJm5o8zv/sd6pGouayMgkx/2FSOdc36T0jGbCHuRSbtia0PEzNIRtmViMrt3AeoWBidRXmZsxCNLwgIV6dn2WpuE5Az0bHgpZnQxTKFek0BMKU/d8wIDAQABo1AwTjAdBgNVHQ4EFgQUGHxYqZYyX7cTxKVODVgZwSTdCnwwHwYDVR0jBBgwFoAUGHxYqZYyX7cTxKVODVgZwSTdCnwwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQ0FAAOBgQByFOl+hMFICbd3DJfnp2Rgd/dqttsZG/tyhILWvErbio/DEe98mXpowhTkC04ENprOyXi7ZbUqiicF89uAGyt1oqgTUCD1VsLahqIcmrzgumNyTwLGWo17WDAa1/usDhetWAMhgzF/Cnf5ek0nK00m0YZGyc4LzgD0CROMASTWNg==</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature> + <samlp:Status> + <samlp:StatusCode Value='urn:oasis:names:tc:SAML:2.0:status:Success'/> + </samlp:Status> + <saml:Assertion xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xmlns:xs='http://www.w3.org/2001/XMLSchema' ID='_d71a3a8e9fcc45c9e9d248ef7049393fc8f04e5f75' Version='2.0' IssueInstant='2014-07-17T01:01:48Z'> + <saml:Issuer>http://idp.example.com/metadata.php</saml:Issuer> + <saml:Subject> + <saml:NameID SPNameQualifier='http://sp.example.com/demo1/metadata.php' Format='urn:oasis:names:tc:SAML:2.0:nameid-format:transient'>_ce3d2948b4cf20146dee0a0b3dd6f69b6cf86f62d7</saml:NameID> + <saml:SubjectConfirmation Method='urn:oasis:names:tc:SAML:2.0:cm:bearer'> + <saml:SubjectConfirmationData NotOnOrAfter='2024-01-18T06:21:48Z' Recipient='http://sp.example.com/demo1/index.php?acs' InResponseTo='ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685'/> + </saml:SubjectConfirmation> + </saml:Subject> + <saml:Conditions NotBefore='2014-07-17T01:01:18Z' NotOnOrAfter='2024-01-18T06:21:48Z'> + <saml:AudienceRestriction> + <saml:Audience>http://sp.example.com/demo1/metadata.php</saml:Audience> + </saml:AudienceRestriction> + </saml:Conditions> + <saml:AuthnStatement AuthnInstant='2014-07-17T01:01:48Z' SessionNotOnOrAfter='2024-07-17T09:01:48Z' SessionIndex='_be9967abd904ddcae3c0eb4189adbe3f71e327cf93'> + <saml:AuthnContext> + <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:Password</saml:AuthnContextClassRef> + </saml:AuthnContext> + </saml:AuthnStatement> + <saml:AttributeStatement> + <saml:Attribute Name='uid' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'> + <saml:AttributeValue xsi:type='xs:string'>test</saml:AttributeValue> + </saml:Attribute> + <saml:Attribute Name='mail' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'> + <saml:AttributeValue xsi:type='xs:string'>test@example.com</saml:AttributeValue> + </saml:Attribute> + <saml:Attribute Name='eduPersonAffiliation' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'> + <saml:AttributeValue xsi:type='xs:string'>users</saml:AttributeValue> + <saml:AttributeValue xsi:type='xs:string'>examplerole1</saml:AttributeValue> + </saml:Attribute> + </saml:AttributeStatement> + </saml:Assertion> +</samlp:Response> diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb index 64f3d09a25b..3a8667e434d 100644 --- a/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -779,4 +779,12 @@ describe Gitlab::Auth::OAuth::User do end end end + + describe '#bypass_two_factor?' do + subject { oauth_user.bypass_two_factor? } + + it 'returns always false' do + is_expected.to be_falsey + end + end end diff --git a/spec/lib/gitlab/auth/saml/auth_hash_spec.rb b/spec/lib/gitlab/auth/saml/auth_hash_spec.rb index bb950e6bbf8..76f49e778fb 100644 --- a/spec/lib/gitlab/auth/saml/auth_hash_spec.rb +++ b/spec/lib/gitlab/auth/saml/auth_hash_spec.rb @@ -37,4 +37,55 @@ describe Gitlab::Auth::Saml::AuthHash do end end end + + describe '#authn_context' do + let(:auth_hash_data) do + { + provider: 'saml', + uid: 'some_uid', + info: + { + name: 'mockuser', + email: 'mock@email.ch', + image: 'mock_user_thumbnail_url' + }, + credentials: + { + token: 'mock_token', + secret: 'mock_secret' + }, + extra: + { + raw_info: + { + info: + { + name: 'mockuser', + email: 'mock@email.ch', + image: 'mock_user_thumbnail_url' + } + } + } + } + end + + subject(:saml_auth_hash) { described_class.new(OmniAuth::AuthHash.new(auth_hash_data)) } + + context 'with response_object' do + before do + auth_hash_data[:extra][:response_object] = { document: + saml_xml(File.read('spec/fixtures/authentication/saml_response.xml')) } + end + + it 'can extract authn_context' do + expect(saml_auth_hash.authn_context).to eq 'urn:oasis:names:tc:SAML:2.0:ac:classes:Password' + end + end + + context 'without response_object' do + it 'returns an empty string' do + expect(saml_auth_hash.authn_context).to be_nil + end + end + end end diff --git a/spec/lib/gitlab/auth/saml/user_spec.rb b/spec/lib/gitlab/auth/saml/user_spec.rb index 62514ca0688..c523f5e177f 100644 --- a/spec/lib/gitlab/auth/saml/user_spec.rb +++ b/spec/lib/gitlab/auth/saml/user_spec.rb @@ -400,4 +400,45 @@ describe Gitlab::Auth::Saml::User do end end end + + describe '#bypass_two_factor?' do + let(:saml_config) { mock_saml_config_with_upstream_two_factor_authn_contexts } + + subject { saml_user.bypass_two_factor? } + + context 'with authn_contexts_worth_two_factors configured' do + before do + stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [saml_config]) + end + + it 'returns true when authn_context is worth two factors' do + allow(saml_user.auth_hash).to receive(:authn_context).and_return('urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + is_expected.to be_truthy + end + + it 'returns false when authn_context is not worth two factors' do + allow(saml_user.auth_hash).to receive(:authn_context).and_return('urn:oasis:names:tc:SAML:2.0:ac:classes:Password') + is_expected.to be_falsey + end + + it 'returns false when authn_context is blank' do + is_expected.to be_falsey + end + end + + context 'without auth_contexts_worth_two_factors_configured' do + before do + stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config]) + end + + it 'returns false when authn_context is present' do + allow(saml_user.auth_hash).to receive(:authn_context).and_return('urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + is_expected.to be_falsey + end + + it 'returns false when authn_context is blank' do + is_expected.to be_falsey + end + end + end end diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index 329f18cd288..87cfb6c04dc 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -46,8 +46,8 @@ module LoginHelpers @current_user = user end - def gitlab_sign_in_via(provider, user, uid) - mock_auth_hash(provider, uid, user.email) + def gitlab_sign_in_via(provider, user, uid, saml_response = nil) + mock_auth_hash(provider, uid, user.email, saml_response) visit new_user_session_path click_link provider end @@ -87,7 +87,7 @@ module LoginHelpers click_link "oauth-login-#{provider}" end - def mock_auth_hash(provider, uid, email) + def mock_auth_hash(provider, uid, email, saml_response = nil) # The mock_auth configuration allows you to set per-provider (or default) # authentication hashes to return during integration testing. OmniAuth.config.mock_auth[provider.to_sym] = OmniAuth::AuthHash.new({ @@ -109,12 +109,21 @@ module LoginHelpers email: email, image: 'mock_user_thumbnail_url' } + }, + response_object: { + document: saml_xml(saml_response) } } }) Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym] end + def saml_xml(raw_saml_response) + return '' if raw_saml_response.blank? + + XMLSecurity::SignedDocument.new(raw_saml_response, []) + end + def mock_saml_config OpenStruct.new(name: 'saml', label: 'saml', args: { assertion_consumer_service_url: 'https://localhost:3443/users/auth/saml/callback', @@ -125,6 +134,14 @@ module LoginHelpers }) end + def mock_saml_config_with_upstream_two_factor_authn_contexts + config = mock_saml_config + config.args[:upstream_two_factor_authn_contexts] = %w(urn:oasis:names:tc:SAML:2.0:ac:classes:CertificateProtectedTransport + urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS + urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN) + config + end + def stub_omniauth_provider(provider, context: Rails.application) env = env_from_context(context) @@ -140,13 +157,16 @@ module LoginHelpers env['omniauth.error.strategy'] = strategy end - def stub_omniauth_saml_config(messages) - set_devise_mapping(context: Rails.application) - Rails.application.routes.disable_clear_and_finalize = true - Rails.application.routes.draw do + def stub_omniauth_saml_config(messages, context: Rails.application) + set_devise_mapping(context: context) + routes = Rails.application.routes + routes.disable_clear_and_finalize = true + routes.formatter.clear + routes.draw do post '/users/auth/saml' => 'omniauth_callbacks#saml' end - allow(Gitlab::Auth::OAuth::Provider).to receive_messages(providers: [:saml], config_for: mock_saml_config) + saml_config = messages.key?(:providers) ? messages[:providers].first : mock_saml_config + allow(Gitlab::Auth::OAuth::Provider).to receive_messages(providers: [:saml], config_for: saml_config) stub_omniauth_setting(messages) stub_saml_authorize_path_helpers end |