Move out link\unlink ability checks to a policy

We can extend the policy in EE for additional behavior

6 files changed, 47 insertions, 22 deletions

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index cc2bb99f55b..e90e8278c13 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -3,6 +3,7 @@
 class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   include AuthenticatesWithTwoFactor
   include Devise::Controllers::Rememberable
+  include AuthHelper
 
   protect_from_forgery except: [:kerberos, :saml, :cas3, :failure], with: :exception, prepend: true
 
@@ -80,10 +81,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     end
 
     if current_user
+      return render_403 unless link_provider_allowed?(oauth['provider'])
+
       log_audit_event(current_user, with: oauth['provider'])
 
       identity_linker ||= auth_module::IdentityLinker.new(current_user, oauth)
-
       identity_linker.link
 
       if identity_linker.changed?
diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb
index b0d65f284af..0d2a6145d0e 100644
--- a/app/controllers/profiles/accounts_controller.rb
+++ b/app/controllers/profiles/accounts_controller.rb
@@ -14,7 +14,7 @@ class Profiles::AccountsController < Profiles::ApplicationController
 
     return render_404 unless identity
 
-    if unlink_allowed?(provider)
+    if unlink_provider_allowed?(provider)
       identity.destroy
     else
       flash[:alert] = "You are not allowed to unlink your primary login account"
diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb
index 2b1d6f49878..b4ee648361c 100644
--- a/app/helpers/auth_helper.rb
+++ b/app/helpers/auth_helper.rb
@@ -100,8 +100,12 @@ module AuthHelper
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  def unlink_allowed?(provider)
-    %w(saml cas3).exclude?(provider.to_s)
+  def unlink_provider_allowed?(provider)
+    IdentityProviderPolicy.new(current_user, provider).can?(:unlink)
+  end
+
+  def link_provider_allowed?(provider)
+    IdentityProviderPolicy.new(current_user, provider).can?(:link)
   end
 
   extend self
diff --git a/app/policies/identity_provider_policy.rb b/app/policies/identity_provider_policy.rb
new file mode 100644
index 00000000000..d34cdd5bdd4
--- /dev/null
+++ b/app/policies/identity_provider_policy.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class IdentityProviderPolicy < BasePolicy
+  desc "Provider is SAML or CAS3"
+  condition(:protected_provider, scope: :subject, score: 0) { %w(saml cas3).include?(@subject.to_s) }
+
+  rule { anonymous }.prevent_all
+
+  rule { default }.policy do
+    enable :unlink
+    enable :link
+  end
+
+  rule { protected_provider }.prevent(:unlink)
+end
diff --git a/app/views/profiles/accounts/_providers.html.haml b/app/views/profiles/accounts/_providers.html.haml
new file mode 100644
index 00000000000..068f9cc70f7
--- /dev/null
+++ b/app/views/profiles/accounts/_providers.html.haml
@@ -0,0 +1,21 @@
+%label.label-bold
+  = s_('Profiles|Connected Accounts')
+  %p= s_('Profiles|Click on icon to activate signin with one of the following services')
+  - providers.each do |provider|
+    - unlink_allowed = unlink_provider_allowed?(provider)
+    - link_allowed = link_provider_allowed?(provider)
+    - if unlink_allowed || link_allowed
+      .provider-btn-group
+        .provider-btn-image
+          = provider_image_tag(provider)
+        - if auth_active?(provider)
+          - if unlink_allowed
+            = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do
+              = s_('Profiles|Disconnect')
+          - else
+            %a.provider-btn
+              = s_('Profiles|Active')
+        - elsif link_allowed
+          = link_to omniauth_authorize_path(:user, provider), method: :post, class: 'provider-btn not-active' do
+            = s_('Profiles|Connect')
+  = render_if_exists 'profiles/accounts/group_saml_unlink_buttons', group_saml_identities: group_saml_identities
diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml
index ee2c5a13b8a..e6380817c8f 100644
--- a/app/views/profiles/accounts/show.html.haml
+++ b/app/views/profiles/accounts/show.html.haml
@@ -29,24 +29,7 @@
       %p
         = s_('Profiles|Activate signin with one of the following services')
     .col-lg-8
-      %label.label-bold
-        = s_('Profiles|Connected Accounts')
-      %p= s_('Profiles|Click on icon to activate signin with one of the following services')
-      - button_based_providers.each do |provider|
-        .provider-btn-group
-          .provider-btn-image
-            = provider_image_tag(provider)
-          - if auth_active?(provider)
-            - if unlink_allowed?(provider)
-              = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do
-                = s_('Profiles|Disconnect')
-            - else
-              %a.provider-btn
-                = s_('Profiles|Active')
-          - else
-            = link_to omniauth_authorize_path(:user, provider), method: :post, class: 'provider-btn not-active' do
-              = s_('Profiles|Connect')
-      = render_if_exists 'profiles/accounts/group_saml_unlink_buttons', group_saml_identities: local_assigns[:group_saml_identities]
+      = render 'providers', providers: button_based_providers, group_saml_identities: local_assigns[:group_saml_identities]
   %hr
 - if current_user.can_change_username?
   .row.prepend-top-default