Commit 5e8bb1fc authored by Balazs Nagy's avatar Balazs Nagy

Add support for SAML required_groups

parent 43371992
...@@ -156,7 +156,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -156,7 +156,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
message << " Create a GitLab account first, and then connect it to your #{label} account." message << " Create a GitLab account first, and then connect it to your #{label} account."
end end
flash[:notice] = message flash[:alert] = message
redirect_to new_user_session_path redirect_to new_user_session_path
end end
......
---
title: julian7 Add required_groups option to SAML config, to restrict access to GitLab
to specific SAML groups.
merge_request: 3223
author: Balazs Nagy
type: added
...@@ -176,6 +176,33 @@ tell GitLab which groups are external via the `external_groups:` element: ...@@ -176,6 +176,33 @@ tell GitLab which groups are external via the `external_groups:` element:
} } } }
``` ```
## Required groups
>**Note:**
This setting is only available on GitLab 10.2 EE and above.
This setting works like `External Groups` setting. Just like there, your IdP has to
pass Group Information to GitLab, you have to tell GitLab where to look or the
groups SAML response, and which group membership should be requisite for logging in.
When `required_groups` is not set or it is empty, anyone with proper authentication
will be able to use the service.
Example:
```yaml
{ name: 'saml',
label: 'Our SAML Provider',
groups_attribute: 'Groups',
required_groups: ['Developers', 'Managers', 'Admins'],
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'
} }
```
## Admin Groups ## Admin Groups
>**Note:** >**Note:**
......
...@@ -14,6 +14,10 @@ module Gitlab ...@@ -14,6 +14,10 @@ module Gitlab
options[:external_groups] options[:external_groups]
end end
def required_groups
Array(options[:required_groups])
end
def admin_groups def admin_groups
options[:admin_groups] options[:admin_groups]
end end
......
...@@ -17,6 +17,14 @@ module Gitlab ...@@ -17,6 +17,14 @@ module Gitlab
user ||= find_or_build_ldap_user if auto_link_ldap_user? user ||= find_or_build_ldap_user if auto_link_ldap_user?
user ||= build_new_user if signup_enabled? user ||= build_new_user if signup_enabled?
if user_in_required_group?
unblock_user(user, "in required group") if user.persisted? && user.blocked?
elsif user.persisted?
block_user(user, "not in required group") unless user.blocked?
else
user = nil
end
if user if user
user.external = !(auth_hash.groups & Gitlab::Saml::Config.external_groups).empty? if external_users_enabled? user.external = !(auth_hash.groups & Gitlab::Saml::Config.external_groups).empty? if external_users_enabled?
user.admin = !(auth_hash.groups & Gitlab::Saml::Config.admin_groups).empty? if admin_groups_enabled? user.admin = !(auth_hash.groups & Gitlab::Saml::Config.admin_groups).empty? if admin_groups_enabled?
...@@ -32,6 +40,28 @@ module Gitlab ...@@ -32,6 +40,28 @@ module Gitlab
protected protected
def block_user(user, reason)
user.ldap_block
log_user_changes(user, "#{reason}, blocking")
end
def unblock_user(user, reason)
user.activate
log_user_changes(user, "#{reason}, unblocking")
end
def log_user_changes(user, message)
Gitlab::AppLogger.info(
"SAML(#{auth_hash.provider}) account \"#{auth_hash.uid}\" #{message} " \
"Gitlab user \"#{user.name}\" (#{user.email})"
)
end
def user_in_required_group?
required_groups = Gitlab::Saml::Config.required_groups
required_groups.empty? || !(auth_hash.groups & required_groups).empty?
end
def auto_link_saml_user? def auto_link_saml_user?
Gitlab.config.omniauth.auto_link_saml_user Gitlab.config.omniauth.auto_link_saml_user
end end
......
...@@ -36,6 +36,10 @@ describe Gitlab::Saml::User do ...@@ -36,6 +36,10 @@ describe Gitlab::Saml::User do
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', external_groups: groups, args: {} } }) allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', external_groups: groups, args: {} } })
end end
def stub_saml_required_group_config(groups)
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', required_groups: groups, args: {} } })
end
def stub_saml_admin_group_config(groups) def stub_saml_admin_group_config(groups)
allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', admin_groups: groups, args: {} } }) allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', admin_groups: groups, args: {} } })
end end
...@@ -185,6 +189,47 @@ describe Gitlab::Saml::User do ...@@ -185,6 +189,47 @@ describe Gitlab::Saml::User do
end end
end end
context 'required groups' do
context 'not defined' do
it 'lets anyone in' do
saml_user.save
expect(gl_user).to be_valid
end
end
context 'are defined' do
before do
stub_omniauth_config(block_auto_created_users: false)
end
it 'lets members in' do
stub_saml_required_group_config(%w(Developers))
saml_user.save
expect(gl_user).to be_valid
end
it 'unblocks already blocked members' do
stub_saml_required_group_config(%w(Developers))
saml_user.save.ldap_block
expect(saml_user.find_user).to be_active
end
it 'does not allow non-members' do
stub_saml_required_group_config(%w(ArchitectureAstronauts))
expect { saml_user.save }.to raise_error Gitlab::OAuth::SignupDisabledError
end
it 'blocks non-members' do
orig_groups = auth_hash.extra.raw_info["groups"]
auth_hash.extra.raw_info.add("groups", "ArchitectureAstronauts")
stub_saml_required_group_config(%w(ArchitectureAstronauts))
saml_user.save
auth_hash.extra.raw_info.set("groups", orig_groups)
expect(saml_user.find_user).to be_ldap_blocked
end
end
end
context 'admin groups' do context 'admin groups' do
context 'are defined' do context 'are defined' do
it 'marks the user as admin' do it 'marks the user as admin' do
......
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