Merge branch 'disable-saml-account-unlink' into 'master'

Disable the unlink feature for SAML connected accounts (social login).

This disables the ability to manually unlink your SAML account, if you have one connected. In certain scenarios, the only allowed login mechanism can be SAML, and if you unlink your account you will be locked out of the system (configuration dependent).

Fixes #18613

See merge request !4662

4 files changed, 35 insertions, 4 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e16f5c331e5..68287f0e6b9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -59,6 +59,7 @@ v 8.9.0 (unreleased)
   - Add option to project to only allow merge requests to be merged if the build succeeds (Rui Santos)
   - Added navigation shortcuts to the project pipelines, milestones, builds and forks page. !4393
   - Fix issues filter when ordering by milestone
+  - Disable SAML account unlink feature
   - Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3
   - Bamboo Service: Fix missing credentials & URL handling when base URL contains a path (Benjamin Schmid)
   - TeamCity Service: Fix URL handling when base URL contains a path
diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb
index 175afbf8425..69959fe3687 100644
--- a/app/controllers/profiles/accounts_controller.rb
+++ b/app/controllers/profiles/accounts_controller.rb
@@ -5,7 +5,7 @@ class Profiles::AccountsController < Profiles::ApplicationController
 
   def unlink
     provider = params[:provider]
-    current_user.identities.find_by(provider: provider).destroy
+    current_user.identities.find_by(provider: provider).destroy unless provider.to_s == 'saml'
     redirect_to profile_account_path
   end
 end
diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml
index 3d2a245ecbd..8efe486e01b 100644
--- a/app/views/profiles/accounts/show.html.haml
+++ b/app/views/profiles/accounts/show.html.haml
@@ -62,10 +62,14 @@
           .provider-btn-image
             = provider_image_tag(provider)
           - if auth_active?(provider)
-            = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do
-              Disconnect
+            - if provider.to_s == 'saml'
+              %a.provider-btn
+                Active
+            - else
+              = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do
+                Disconnect
           - else
-            = link_to user_omniauth_authorize_path(provider), method: :post, class: "provider-btn #{'not-active' if !auth_active?(provider)}", "data-no-turbolink" => "true" do
+            = link_to user_omniauth_authorize_path(provider), method: :post, class: 'provider-btn not-active', "data-no-turbolink" => "true" do
               Connect
   %hr
 - if current_user.can_change_username?
diff --git a/spec/controllers/profiles/accounts_controller_spec.rb b/spec/controllers/profiles/accounts_controller_spec.rb
new file mode 100644
index 00000000000..4eafc11abaa
--- /dev/null
+++ b/spec/controllers/profiles/accounts_controller_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe Profiles::AccountsController do
+
+  let(:user) { create(:omniauth_user, provider: 'saml') }
+
+  before do
+    sign_in(user)
+  end
+
+  it 'does not allow to unlink SAML connected account' do
+    identity = user.identities.last
+    delete :unlink, provider: 'saml'
+    updated_user = User.find(user.id)
+
+    expect(response.status).to eq(302)
+    expect(updated_user.identities.size).to eq(1)
+    expect(updated_user.identities).to include(identity)
+  end
+
+  it 'does allow to delete other linked accounts' do
+    user.identities.create(provider: 'twitter', extern_uid: 'twitter_123')
+
+    expect { delete :unlink, provider: 'twitter' }.to change(Identity.all, :size).by(-1)
+  end
+end