Commit 936be025 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'saml-external-groups' into 'master'

Allow SAML to identify external users and set them as such

Related to #4009

Fixes #14577

This allows SAML to retrieve group information form the `SAML Response`
and match that to a setting that will flag all matching users as external.

See merge request !3530
parents 730625f0 8110e753
...@@ -10,6 +10,7 @@ v 8.7.0 (unreleased) ...@@ -10,6 +10,7 @@ v 8.7.0 (unreleased)
- Allow back dating on issues when created through the API - Allow back dating on issues when created through the API
- Fix Error 500 after renaming a project path (Stan Hu) - Fix Error 500 after renaming a project path (Stan Hu)
- Fix avatar stretching by providing a cropping feature - Fix avatar stretching by providing a cropping feature
- Allow SAML to handle external users based on user's information !3530
- Add endpoints to archive or unarchive a project !3372 - Add endpoints to archive or unarchive a project !3372
- Add links to CI setup documentation from project settings and builds pages - Add links to CI setup documentation from project settings and builds pages
- Handle nil descriptions in Slack issue messages (Stan Hu) - Handle nil descriptions in Slack issue messages (Stan Hu)
......
...@@ -55,7 +55,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -55,7 +55,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
end end
else else
saml_user = Gitlab::Saml::User.new(oauth) saml_user = Gitlab::Saml::User.new(oauth)
saml_user.save saml_user.save if saml_user.changed?
@user = saml_user.gl_user @user = saml_user.gl_user
continue_login_process continue_login_process
......
...@@ -345,6 +345,8 @@ production: &base ...@@ -345,6 +345,8 @@ production: &base
# #
# - { name: 'saml', # - { name: 'saml',
# label: 'Our SAML Provider', # label: 'Our SAML Provider',
# groups_attribute: 'Groups',
# external_groups: ['Contractors', 'Freelancers'],
# args: { # args: {
# assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', # 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_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8',
...@@ -352,6 +354,7 @@ production: &base ...@@ -352,6 +354,7 @@ production: &base
# issuer: 'https://gitlab.example.com', # issuer: 'https://gitlab.example.com',
# name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' # name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient'
# } } # } }
#
# - { name: 'crowd', # - { name: 'crowd',
# args: { # args: {
# crowd_server_url: 'CROWD SERVER URL', # crowd_server_url: 'CROWD SERVER URL',
......
...@@ -131,8 +131,75 @@ On the sign in page there should now be a SAML button below the regular sign in ...@@ -131,8 +131,75 @@ On the sign in page there should now be a SAML button below the regular sign in
Click the icon to begin the authentication process. If everything goes well the user Click the icon to begin the authentication process. If everything goes well the user
will be returned to GitLab and will be signed in. will be returned to GitLab and will be signed in.
## External Groups
>**Note:**
This setting is only available on GitLab 8.7 and above.
SAML login includes support for external groups. You can define in the SAML
settings which groups, to which your users belong in your IdP, you wish to be
marked as [external](../permissions/permissions.md).
### Requirements
First you need to tell GitLab where to look for group information. For this you
need to make sure that your IdP server sends a specific `AttributeStament` along
with the regular SAML response. Here is an example:
```xml
<saml:AttributeStatement>
<saml:Attribute Name="Groups">
<saml:AttributeValue xsi:type="xs:string">SecurityGroup</saml:AttributeValue>
<saml:AttributeValue xsi:type="xs:string">Developers</saml:AttributeValue>
<saml:AttributeValue xsi:type="xs:string">Designers</saml:AttributeValue>
</saml:Attribute>
</saml:AttributeStatement>
```
The name of the attribute can be anything you like, but it must contain the groups
to which a user belongs. In order to tell GitLab where to find these groups, you need
to add a `groups_attribute:` element to your SAML settings. You will also need to
tell GitLab which groups are external via the `external_groups:` element:
```yaml
{ name: 'saml',
label: 'Our SAML Provider',
groups_attribute: 'Groups',
external_groups: ['Freelancers', 'Interns'],
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:transient'
} }
```
## Customization ## Customization
### `auto_sign_in_with_provider`
You can add this setting to your GitLab configuration to automatically redirect you
to your SAML server for authentication, thus removing the need to click a button
before actually signing in.
For omnibus package:
```ruby
gitlab_rails['omniauth_auto_sign_in_with_provider'] = 'saml'
```
For installations from source:
```yaml
omniauth:
auto_sign_in_with_provider: saml
```
Please keep in mind that every sign in attempt will be redirected to the SAML server,
so you will not be able to sign in using local credentials. Make sure that at least one
of the SAML users has admin permissions.
### `attribute_statements` ### `attribute_statements`
>**Note:** >**Note:**
...@@ -205,6 +272,10 @@ To bypass this you can add `skip_before_action :verify_authenticity_token` to th ...@@ -205,6 +272,10 @@ To bypass this you can add `skip_before_action :verify_authenticity_token` to th
where it can then be seen in the usual logs, or as a flash message in the login where it can then be seen in the usual logs, or as a flash message in the login
screen. screen.
That file is located at `/opt/gitlab/embedded/service/gitlab-rails/app/controllers`
for Omnibus installations and by default on `/home/git/gitlab/app/controllers` for
installations from source.
### Invalid audience ### Invalid audience
This error means that the IdP doesn't recognize GitLab as a valid sender and This error means that the IdP doesn't recognize GitLab as a valid sender and
......
module Gitlab
module Saml
class AuthHash < Gitlab::OAuth::AuthHash
def groups
get_raw(Gitlab::Saml::Config.groups)
end
private
def get_raw(key)
# Needs to call `all` because of https://git.io/vVo4u
# otherwise just the first value is returned
auth_hash.extra[:raw_info].all[key]
end
end
end
end
module Gitlab
module Saml
class Config
class << self
def options
Gitlab.config.omniauth.providers.find { |provider| provider.name == 'saml' }
end
def groups
options[:groups_attribute]
end
def external_groups
options[:external_groups]
end
end
end
end
end
...@@ -18,7 +18,7 @@ module Gitlab ...@@ -18,7 +18,7 @@ module Gitlab
@user ||= find_or_create_ldap_user @user ||= find_or_create_ldap_user
end end
if auto_link_saml_enabled? if auto_link_saml_user?
@user ||= find_by_email @user ||= find_by_email
end end
...@@ -26,6 +26,16 @@ module Gitlab ...@@ -26,6 +26,16 @@ module Gitlab
@user ||= build_new_user @user ||= build_new_user
end end
if external_users_enabled?
# Check if there is overlap between the user's groups and the external groups
# setting then set user as external or internal.
if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
@user.external = false
else
@user.external = true
end
end
@user @user
end end
...@@ -37,11 +47,23 @@ module Gitlab ...@@ -37,11 +47,23 @@ module Gitlab
end end
end end
def changed?
gl_user.changed? || gl_user.identities.any?(&:changed?)
end
protected protected
def auto_link_saml_enabled? def auto_link_saml_user?
Gitlab.config.omniauth.auto_link_saml_user Gitlab.config.omniauth.auto_link_saml_user
end end
def external_users_enabled?
!Gitlab::Saml::Config.external_groups.nil?
end
def auth_hash=(auth_hash)
@auth_hash = Gitlab::Saml::AuthHash.new(auth_hash)
end
end end
end end
end end
...@@ -5,7 +5,7 @@ describe Gitlab::Saml::User, lib: true do ...@@ -5,7 +5,7 @@ describe Gitlab::Saml::User, lib: true do
let(:gl_user) { saml_user.gl_user } let(:gl_user) { saml_user.gl_user }
let(:uid) { 'my-uid' } let(:uid) { 'my-uid' }
let(:provider) { 'saml' } let(:provider) { 'saml' }
let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash) } let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash, extra: { raw_info: OneLogin::RubySaml::Attributes.new({ 'groups' => %w(Developers Freelancers Designers) }) }) }
let(:info_hash) do let(:info_hash) do
{ {
name: 'John', name: 'John',
...@@ -23,10 +23,20 @@ describe Gitlab::Saml::User, lib: true do ...@@ -23,10 +23,20 @@ describe Gitlab::Saml::User, lib: true do
allow(Gitlab::LDAP::Config).to receive_messages(messages) allow(Gitlab::LDAP::Config).to receive_messages(messages)
end end
def stub_basic_saml_config
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', args: {} } })
end
def stub_saml_group_config(groups)
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', external_groups: groups, args: {} } })
end
before { stub_basic_saml_config }
describe 'account exists on server' do describe 'account exists on server' do
before { stub_omniauth_config({ allow_single_sign_on: ['saml'], auto_link_saml_user: true }) } 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') } let!(:existing_user) { create(:user, email: 'john@mail.com', username: 'john') }
context 'and should bind with SAML' do
it 'adds the SAML identity to the existing user' do it 'adds the SAML identity to the existing user' do
saml_user.save saml_user.save
expect(gl_user).to be_valid expect(gl_user).to be_valid
...@@ -36,6 +46,35 @@ describe Gitlab::Saml::User, lib: true do ...@@ -36,6 +46,35 @@ describe Gitlab::Saml::User, lib: true do
expect(identity.provider).to eql 'saml' expect(identity.provider).to eql 'saml'
end end
end end
context 'external groups' do
context 'are defined' do
it 'marks the user as external' do
stub_saml_group_config(%w(Freelancers))
saml_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_truthy
end
end
before { stub_saml_group_config(%w(Interns)) }
context 'are defined but the user does not belong there' do
it 'does not mark the user as external' do
saml_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_falsey
end
end
context 'user was external, now should not be' do
it 'should make user internal' do
existing_user.update_attribute('external', true)
saml_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_falsey
end
end
end
end end
describe 'no account exists on server' do describe 'no account exists on server' do
...@@ -68,6 +107,26 @@ describe Gitlab::Saml::User, lib: true do ...@@ -68,6 +107,26 @@ describe Gitlab::Saml::User, lib: true do
end end
end end
context 'external groups' do
context 'are defined' do
it 'marks the user as external' do
stub_saml_group_config(%w(Freelancers))
saml_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_truthy
end
end
context 'are defined but the user does not belong there' do
it 'does not mark the user as external' do
stub_saml_group_config(%w(Interns))
saml_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_falsey
end
end
end
context 'with auto_link_ldap_user disabled (default)' do 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'] }) } 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' include_examples 'to verify compliance with allow_single_sign_on'
...@@ -76,12 +135,6 @@ describe Gitlab::Saml::User, lib: true do ...@@ -76,12 +135,6 @@ describe Gitlab::Saml::User, lib: true do
context 'with auto_link_ldap_user enabled' do context 'with auto_link_ldap_user enabled' do
before { stub_omniauth_config({ auto_link_ldap_user: true, auto_link_saml_user: false }) } 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 context 'and at least one LDAP provider is defined' do
before { stub_ldap_config(providers: %w(ldapmain)) } before { stub_ldap_config(providers: %w(ldapmain)) }
...@@ -89,19 +142,18 @@ describe Gitlab::Saml::User, lib: true do ...@@ -89,19 +142,18 @@ describe Gitlab::Saml::User, lib: true do
before do before do
allow(ldap_user).to receive(:uid) { uid } allow(ldap_user).to receive(:uid) { uid }
allow(ldap_user).to receive(:username) { 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(:email) { %w(john@mail.com john2@example.com) }
allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } 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) allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user)
end end
context 'and no account for the LDAP user' do context 'and no account for the LDAP user' do
it 'creates a user with dual LDAP and SAML identities' do it 'creates a user with dual LDAP and SAML identities' do
saml_user.save saml_user.save
expect(gl_user).to be_valid expect(gl_user).to be_valid
expect(gl_user.username).to eql uid expect(gl_user.username).to eql uid
expect(gl_user.email).to eql 'johndoe@example.com' expect(gl_user.email).to eql 'john@mail.com'
expect(gl_user.identities.length).to eql 2 expect(gl_user.identities.length).to eql 2
identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } 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' }, expect(identities_as_hash).to match_array([ { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' },
...@@ -111,13 +163,13 @@ describe Gitlab::Saml::User, lib: true do ...@@ -111,13 +163,13 @@ describe Gitlab::Saml::User, lib: true do
end end
context 'and LDAP user has an account already' do 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') } let!(:existing_user) { create(:omniauth_user, email: 'john@mail.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') }
it "adds the omniauth identity to the LDAP account" do it 'adds the omniauth identity to the LDAP account' do
saml_user.save saml_user.save
expect(gl_user).to be_valid expect(gl_user).to be_valid
expect(gl_user.username).to eql 'john' expect(gl_user.username).to eql 'john'
expect(gl_user.email).to eql 'john@example.com' expect(gl_user.email).to eql 'john@mail.com'
expect(gl_user.identities.length).to eql 2 expect(gl_user.identities.length).to eql 2
identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } 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' }, expect(identities_as_hash).to match_array([ { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' },
...@@ -126,19 +178,13 @@ describe Gitlab::Saml::User, lib: true do ...@@ -126,19 +178,13 @@ describe Gitlab::Saml::User, lib: true do
end end
end 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 end
end end
describe 'blocking' do describe 'blocking' do
before { stub_omniauth_config({ allow_saml_sign_up: true, auto_link_saml_user: true }) } before { stub_omniauth_config({ allow_single_sign_on: ['saml'], auto_link_saml_user: true }) }
context 'signup with SAML only' do context 'signup with SAML only' do
context 'dont block on create' do context 'dont block on create' do
...@@ -162,64 +208,6 @@ describe Gitlab::Saml::User, lib: true do ...@@ -162,64 +208,6 @@ describe Gitlab::Saml::User, lib: true do
end 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 context 'sign-in' do
before do before do
saml_user.save saml_user.save
...@@ -245,26 +233,6 @@ describe Gitlab::Saml::User, lib: true do ...@@ -245,26 +233,6 @@ describe Gitlab::Saml::User, lib: true do
expect(gl_user).not_to be_blocked expect(gl_user).not_to be_blocked
end end
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
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