From 1a168279fa3eb87c2061917707397af21e7b26ea Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Mon, 4 Apr 2016 19:09:12 -0500
Subject: [PATCH 01/10] Prepare SAML for group retrieval

---
 lib/gitlab/saml/auth_hash.rb | 17 ++++++++++++++
 lib/gitlab/saml/config.rb    | 22 ++++++++++++++++++
 lib/gitlab/saml/user.rb      | 43 ++++++++++++++++++++++++++++++++++--
 3 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 lib/gitlab/saml/auth_hash.rb
 create mode 100644 lib/gitlab/saml/config.rb

diff --git a/lib/gitlab/saml/auth_hash.rb b/lib/gitlab/saml/auth_hash.rb
new file mode 100644
index 0000000000..5ffccc0e10
--- /dev/null
+++ b/lib/gitlab/saml/auth_hash.rb
@@ -0,0 +1,17 @@
+module Gitlab
+  module Saml
+    class AuthHash < Gitlab::OAuth::AuthHash
+
+      def groups
+        get_raw(Gitlab::Saml::Config.groups)
+      end
+
+      private
+
+      def get_raw(key)
+        auth_hash.extra[:raw_info][key]
+      end
+
+    end
+  end
+end
diff --git a/lib/gitlab/saml/config.rb b/lib/gitlab/saml/config.rb
new file mode 100644
index 0000000000..dade4c0fa6
--- /dev/null
+++ b/lib/gitlab/saml/config.rb
@@ -0,0 +1,22 @@
+# Load a specific server configuration
+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
diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb
index b1e30110ef..14eda337d9 100644
--- a/lib/gitlab/saml/user.rb
+++ b/lib/gitlab/saml/user.rb
@@ -7,6 +7,11 @@ module Gitlab
   module Saml
     class User < Gitlab::OAuth::User
 
+      def initialize(auth_hash)
+        super
+        update_user_attributes
+      end
+
       def save
         super('SAML')
       end
@@ -18,7 +23,7 @@ module Gitlab
           @user ||= find_or_create_ldap_user
         end
 
-        if auto_link_saml_enabled?
+        if auto_link_saml_user?
           @user ||= find_by_email
         end
 
@@ -37,11 +42,45 @@ module Gitlab
         end
       end
 
+      def changed?
+        gl_user.changed? || gl_user.identities.any?(&:changed?)
+      end
+
       protected
 
-      def auto_link_saml_enabled?
+      def build_new_user
+        user = super
+        if external_users_enabled?
+          unless (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
+            user.external = true
+          end
+        end
+        user
+      end
+
+      def auto_link_saml_user?
         Gitlab.config.omniauth.auto_link_saml_user
       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
+
+      def update_user_attributes
+        if persisted?
+          if external_users_enabled?
+            if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
+              gl_user.external = false
+            else
+              gl_user.external = true
+            end
+          end
+        end
+      end
     end
   end
 end

From d14f080c0a7e4cfb2893a9fb2462a650f02a6ef9 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Mon, 4 Apr 2016 19:09:46 -0500
Subject: [PATCH 02/10] Remove unnecessary LDAP tests from SAML tests

---
 spec/lib/gitlab/saml/user_spec.rb | 66 ++++---------------------------
 1 file changed, 7 insertions(+), 59 deletions(-)

diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb
index de7cd99d49..6f5cf3a1cf 100644
--- a/spec/lib/gitlab/saml/user_spec.rb
+++ b/spec/lib/gitlab/saml/user_spec.rb
@@ -23,6 +23,12 @@ describe Gitlab::Saml::User, lib: true do
       allow(Gitlab::LDAP::Config).to receive_messages(messages)
     end
 
+    def stub_saml_config(messages)
+      allow(Gitlab::Saml::Config).to receive_messages(messages)
+    end
+
+    before { stub_saml_config({ options: { name: 'saml', args: {} } }) }
+
     describe 'account exists on server' do
       before { stub_omniauth_config({ allow_single_sign_on: ['saml'], auto_link_saml_user: true }) }
       context 'and should bind with SAML' do
@@ -138,7 +144,7 @@ describe Gitlab::Saml::User, lib: true do
     end
 
     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 'dont block on create' do
@@ -162,64 +168,6 @@ describe Gitlab::Saml::User, lib: true do
         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
         before do
           saml_user.save

From 943d8d4b90fbdc4f68d548c0566343903f895138 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Mon, 4 Apr 2016 19:10:17 -0500
Subject: [PATCH 03/10] Config examples

---
 config/gitlab.yml.example | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index fb1c3476f6..c607e32a05 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -345,6 +345,8 @@ production: &base
       #
       # - { name: 'saml',
       #     label: 'Our SAML Provider',
+      #     groups_attribute: 'Groups',
+      #     external_groups: ['Contractors', 'Freelancers'],
       #     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',

From e99855bfe4b4741d33d5575fdf1f0bc2edd85844 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Mon, 4 Apr 2016 19:10:59 -0500
Subject: [PATCH 04/10] Avoid saving again if the user attributes haven't
 changed

---
 app/controllers/omniauth_callbacks_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 21135f7d60..d28e96c3f1 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -55,7 +55,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
       end
     else
       saml_user = Gitlab::Saml::User.new(oauth)
-      saml_user.save
+      saml_user.save if saml_user.changed?
       @user = saml_user.gl_user
 
       continue_login_process

From 518ec6b2660c55beba2833ce71b93774ed0a6c2a Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Tue, 5 Apr 2016 19:20:18 -0500
Subject: [PATCH 05/10] Changed config syntax and  improved how chaanges in
 group memberships are handled when external groups is set up

---
 lib/gitlab/saml/config.rb |  4 ++--
 lib/gitlab/saml/user.rb   | 39 ++++++++++++---------------------------
 2 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/lib/gitlab/saml/config.rb b/lib/gitlab/saml/config.rb
index dade4c0fa6..2b3cf840f6 100644
--- a/lib/gitlab/saml/config.rb
+++ b/lib/gitlab/saml/config.rb
@@ -9,11 +9,11 @@ module Gitlab
         end
 
         def groups
-          options['groups_attribute']
+          options[:groups_attribute]
         end
 
         def external_groups
-          options['external_groups']
+          options[:external_groups]
         end
       end
 
diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb
index 14eda337d9..6ab165cf51 100644
--- a/lib/gitlab/saml/user.rb
+++ b/lib/gitlab/saml/user.rb
@@ -7,11 +7,6 @@ module Gitlab
   module Saml
     class User < Gitlab::OAuth::User
 
-      def initialize(auth_hash)
-        super
-        update_user_attributes
-      end
-
       def save
         super('SAML')
       end
@@ -31,6 +26,18 @@ module Gitlab
           @user ||= build_new_user
         end
 
+        if external_users_enabled?
+          # Check if there is overlap between the user's groups and the external groups
+          # setting and set user as external or internal.
+          if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
+            # Avoid an unnecessary change of values and the subsequent save
+            @user.external = false if @user.external
+          else
+            # Avoid an unnecessary change of values and the subsequent save
+            @user.external = true unless @user.external
+          end
+        end
+
         @user
       end
 
@@ -48,16 +55,6 @@ module Gitlab
 
       protected
 
-      def build_new_user
-        user = super
-        if external_users_enabled?
-          unless (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
-            user.external = true
-          end
-        end
-        user
-      end
-
       def auto_link_saml_user?
         Gitlab.config.omniauth.auto_link_saml_user
       end
@@ -69,18 +66,6 @@ module Gitlab
       def auth_hash=(auth_hash)
         @auth_hash = Gitlab::Saml::AuthHash.new(auth_hash)
       end
-
-      def update_user_attributes
-        if persisted?
-          if external_users_enabled?
-            if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
-              gl_user.external = false
-            else
-              gl_user.external = true
-            end
-          end
-        end
-      end
     end
   end
 end

From 7efaf22bccb16b381f7e76054d084e741006fc5f Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Tue, 5 Apr 2016 19:22:58 -0500
Subject: [PATCH 06/10] Removed extra LDAP tests and added tests for the
 external groups feature

---
 spec/lib/gitlab/saml/user_spec.rb | 68 +++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 22 deletions(-)

diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb
index 6f5cf3a1cf..84a26af940 100644
--- a/spec/lib/gitlab/saml/user_spec.rb
+++ b/spec/lib/gitlab/saml/user_spec.rb
@@ -5,7 +5,7 @@ describe Gitlab::Saml::User, lib: true do
   let(:gl_user) { saml_user.gl_user }
   let(:uid) { 'my-uid' }
   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: { groups: %w(Developers Freelancers Designers) } }) }
   let(:info_hash) do
     {
       name: 'John',
@@ -31,8 +31,8 @@ describe Gitlab::Saml::User, lib: true do
 
     describe 'account exists on server' do
       before { stub_omniauth_config({ allow_single_sign_on: ['saml'], auto_link_saml_user: true }) }
+      let!(:existing_user) { create(:user, email: 'john@mail.com', username: 'john') }
       context 'and should bind with SAML' do
-        let!(:existing_user) { create(:user, email: 'john@mail.com', username: 'john') }
         it 'adds the SAML identity to the existing user' do
           saml_user.save
           expect(gl_user).to be_valid
@@ -42,6 +42,32 @@ describe Gitlab::Saml::User, lib: true do
           expect(identity.provider).to eql 'saml'
         end
       end
+
+      context 'external groups' do
+        context 'are defined' do
+          before { stub_saml_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Freelancers), args: {} } }) }
+          it 'marks the user as external' do
+            saml_user.save
+            expect(gl_user.external).to be_truthy
+          end
+        end
+
+        before { stub_saml_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Interns), args: {} } }) }
+        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.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.external).to be_falsey
+          end
+        end
+      end
     end
 
     describe 'no account exists on server' do
@@ -74,6 +100,24 @@ describe Gitlab::Saml::User, lib: true do
         end
       end
 
+      context 'external groups' do
+        context 'are defined' do
+          before { stub_saml_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Freelancers), args: {} } }) }
+          it 'marks the user as external' do
+            saml_user.save
+            expect(gl_user.external).to be_truthy
+          end
+        end
+
+        before { stub_saml_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Interns), args: {} } }) }
+        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.external).to be_falsey
+          end
+        end
+      end
+
       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'] }) }
         include_examples 'to verify compliance with allow_single_sign_on'
@@ -193,26 +237,6 @@ describe Gitlab::Saml::User, lib: true do
             expect(gl_user).not_to be_blocked
           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

From 3a36fa895724aedb4bd919ec91cc00a24415e712 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Wed, 6 Apr 2016 16:03:35 -0500
Subject: [PATCH 07/10] Fix error that was causing only one group to be
 returned and corrected specs to use the proper attribute type

---
 lib/gitlab/saml/auth_hash.rb      | 4 +++-
 spec/lib/gitlab/saml/user_spec.rb | 7 ++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/saml/auth_hash.rb b/lib/gitlab/saml/auth_hash.rb
index 5ffccc0e10..3414d24ca7 100644
--- a/lib/gitlab/saml/auth_hash.rb
+++ b/lib/gitlab/saml/auth_hash.rb
@@ -9,7 +9,9 @@ module Gitlab
       private
 
       def get_raw(key)
-        auth_hash.extra[:raw_info][key]
+        # Needs to call `all` because of https://github.com/onelogin/ruby-saml/blob/master/lib/onelogin/ruby-saml/attributes.rb#L78
+        # otherwise just the first value is returned
+        auth_hash.extra[:raw_info].all[key]
       end
 
     end
diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb
index 84a26af940..ed7b507dba 100644
--- a/spec/lib/gitlab/saml/user_spec.rb
+++ b/spec/lib/gitlab/saml/user_spec.rb
@@ -5,7 +5,7 @@ describe Gitlab::Saml::User, lib: true do
   let(:gl_user) { saml_user.gl_user }
   let(:uid) { 'my-uid' }
   let(:provider) { 'saml' }
-  let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash, extra: { raw_info: { groups: %w(Developers Freelancers Designers) } }) }
+  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
     {
       name: 'John',
@@ -48,6 +48,7 @@ describe Gitlab::Saml::User, lib: true do
           before { stub_saml_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Freelancers), args: {} } }) }
           it 'marks the user as external' do
             saml_user.save
+            expect(gl_user).to be_valid
             expect(gl_user.external).to be_truthy
           end
         end
@@ -56,6 +57,7 @@ describe Gitlab::Saml::User, lib: true do
         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
@@ -64,6 +66,7 @@ describe Gitlab::Saml::User, lib: true 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
@@ -105,6 +108,7 @@ describe Gitlab::Saml::User, lib: true do
           before { stub_saml_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Freelancers), args: {} } }) }
           it 'marks the user as external' do
             saml_user.save
+            expect(gl_user).to be_valid
             expect(gl_user.external).to be_truthy
           end
         end
@@ -113,6 +117,7 @@ describe Gitlab::Saml::User, lib: true do
         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

From b95ac0962c96c933bc85b89558733055a21d3dd5 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Wed, 6 Apr 2016 16:04:11 -0500
Subject: [PATCH 08/10] Added documentation

---
 config/gitlab.yml.example |  1 +
 doc/integration/saml.md   | 72 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index c607e32a05..ddee467b14 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -354,6 +354,7 @@ production: &base
       #             issuer: 'https://gitlab.example.com',
       #             name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient'
       #           } }
+      #
       # - { name: 'crowd',
       #     args: {
       #       crowd_server_url: 'CROWD SERVER URL',
diff --git a/doc/integration/saml.md b/doc/integration/saml.md
index 1c3dc707f6..684f0a5b8c 100644
--- a/doc/integration/saml.md
+++ b/doc/integration/saml.md
@@ -131,8 +131,76 @@ 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
 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, for that you need 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
 
+### `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`
 
 >**Note:**
@@ -205,6 +273,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
 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
 
 This error means that the IdP doesn't recognize GitLab as a valid sender and

From eb0f1de36c6ebf13d86540bb79d7fb07fc3642f0 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Wed, 6 Apr 2016 16:21:58 -0500
Subject: [PATCH 09/10] Added CHANGELOG item

---
 CHANGELOG               | 1 +
 lib/gitlab/saml/user.rb | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index f72bb670ec..1b3fd97a1c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -8,6 +8,7 @@ v 8.7.0 (unreleased)
   - Expose label description in API (Mariusz Jachimowicz)
   - Allow back dating on issues when created through the API
   - 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 links to CI setup documentation from project settings and builds pages
   - Handle nil descriptions in Slack issue messages (Stan Hu)
diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb
index 6ab165cf51..73fc443a02 100644
--- a/lib/gitlab/saml/user.rb
+++ b/lib/gitlab/saml/user.rb
@@ -33,7 +33,6 @@ module Gitlab
             # Avoid an unnecessary change of values and the subsequent save
             @user.external = false if @user.external
           else
-            # Avoid an unnecessary change of values and the subsequent save
             @user.external = true unless @user.external
           end
         end

From 8110e7530902de8744ff985f08938306e2c38367 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Wed, 6 Apr 2016 18:12:25 -0500
Subject: [PATCH 10/10] Implemented suggested fixes

---
 doc/integration/saml.md           |  3 +--
 lib/gitlab/saml/auth_hash.rb      |  2 +-
 lib/gitlab/saml/config.rb         |  1 -
 lib/gitlab/saml/user.rb           |  7 +++---
 spec/lib/gitlab/saml/user_spec.rb | 41 ++++++++++++-------------------
 5 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/doc/integration/saml.md b/doc/integration/saml.md
index 684f0a5b8c..8a7205caaa 100644
--- a/doc/integration/saml.md
+++ b/doc/integration/saml.md
@@ -159,8 +159,7 @@ with the regular SAML response. Here is an example:
 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, for that you need the `external_groups:`
-element:
+tell GitLab which groups are external via the `external_groups:` element:
 
 ```yaml
 { name: 'saml',
diff --git a/lib/gitlab/saml/auth_hash.rb b/lib/gitlab/saml/auth_hash.rb
index 3414d24ca7..32c1c9ec5b 100644
--- a/lib/gitlab/saml/auth_hash.rb
+++ b/lib/gitlab/saml/auth_hash.rb
@@ -9,7 +9,7 @@ module Gitlab
       private
 
       def get_raw(key)
-        # Needs to call `all` because of https://github.com/onelogin/ruby-saml/blob/master/lib/onelogin/ruby-saml/attributes.rb#L78
+        # 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
diff --git a/lib/gitlab/saml/config.rb b/lib/gitlab/saml/config.rb
index 2b3cf840f6..0f40c00f54 100644
--- a/lib/gitlab/saml/config.rb
+++ b/lib/gitlab/saml/config.rb
@@ -1,4 +1,3 @@
-# Load a specific server configuration
 module Gitlab
   module Saml
     class Config
diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb
index 73fc443a02..c1072452ab 100644
--- a/lib/gitlab/saml/user.rb
+++ b/lib/gitlab/saml/user.rb
@@ -28,12 +28,11 @@ module Gitlab
 
         if external_users_enabled?
           # Check if there is overlap between the user's groups and the external groups
-          # setting and set user as external or internal.
+          # setting then set user as external or internal.
           if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
-            # Avoid an unnecessary change of values and the subsequent save
-            @user.external = false if @user.external
+            @user.external = false
           else
-            @user.external = true unless @user.external
+            @user.external = true
           end
         end
 
diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb
index ed7b507dba..c2a51d9249 100644
--- a/spec/lib/gitlab/saml/user_spec.rb
+++ b/spec/lib/gitlab/saml/user_spec.rb
@@ -23,11 +23,15 @@ describe Gitlab::Saml::User, lib: true do
       allow(Gitlab::LDAP::Config).to receive_messages(messages)
     end
 
-    def stub_saml_config(messages)
-      allow(Gitlab::Saml::Config).to receive_messages(messages)
+    def stub_basic_saml_config
+      allow(Gitlab::Saml::Config).to receive_messages({ options: { name: 'saml', args: {} } })
     end
 
-    before { stub_saml_config({ options: { name: 'saml', args: {} } }) }
+    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
       before { stub_omniauth_config({ allow_single_sign_on: ['saml'], auto_link_saml_user: true }) }
@@ -45,15 +49,15 @@ describe Gitlab::Saml::User, lib: true do
 
       context 'external groups' do
         context 'are defined' do
-          before { stub_saml_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Freelancers), args: {} } }) }
           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_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Interns), args: {} } }) }
+        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
@@ -105,17 +109,17 @@ describe Gitlab::Saml::User, lib: true do
 
       context 'external groups' do
         context 'are defined' do
-          before { stub_saml_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Freelancers), args: {} } }) }
           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_config({ options: { name: 'saml', groups_attribute: 'groups', external_groups: %w(Interns), args: {} } }) }
         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
@@ -131,12 +135,6 @@ describe Gitlab::Saml::User, lib: true do
       context 'with auto_link_ldap_user enabled' do
         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
           before { stub_ldap_config(providers: %w(ldapmain)) }
 
@@ -144,19 +142,18 @@ describe Gitlab::Saml::User, lib: true do
             before do
               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(:email) { %w(john@mail.com john2@example.com) }
               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)
             end
 
             context 'and no account for the LDAP user' do
-
               it 'creates a user with dual LDAP and SAML identities' do
                 saml_user.save
 
                 expect(gl_user).to be_valid
                 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
                 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' },
@@ -166,13 +163,13 @@ describe Gitlab::Saml::User, lib: true do
             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') }
-              it "adds the omniauth identity to the LDAP account" do
+              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
                 saml_user.save
 
                 expect(gl_user).to be_valid
                 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
                 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' },
@@ -181,12 +178,6 @@ describe Gitlab::Saml::User, lib: true do
               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