Merge branch '3143-saml_required_groups' into 'master'

Add support for SAML required_groups

Closes #3143

See merge request gitlab-org/gitlab-ee!3223

app/controllers/omniauth_callbacks_controller.rb

@@ -156,7 +156,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
       message << " Create a GitLab account first, and then connect it to your #{label} account."
     end
 
-    flash[:notice] = message
+    flash[:alert] = message
 
     redirect_to new_user_session_path
   end

changelogs/unreleased-ee/3143-saml_required_groups.yml

+---
+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

doc/integration/saml.md

@@ -179,6 +179,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
 
 >**Note:**

lib/gitlab/saml/config.rb

@@ -14,6 +14,10 @@ module Gitlab
           options[:external_groups]
         end
 
+        def required_groups
+          Array(options[:required_groups])
+        end
+
         def admin_groups
           options[:admin_groups]
         end

lib/gitlab/saml/user.rb

@@ -17,6 +17,14 @@ module Gitlab
         user ||= find_or_build_ldap_user if auto_link_ldap_user?
         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
           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?
@@ -32,6 +40,28 @@ module Gitlab
 
       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?
         Gitlab.config.omniauth.auto_link_saml_user
       end

spec/lib/gitlab/saml/user_spec.rb

@@ -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: {} } })
     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)
       allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', admin_groups: groups, args: {} } })
     end
@@ -185,6 +189,47 @@ describe Gitlab::Saml::User do
         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 'are defined' do
           it 'marks the user as admin' do