Move out link\unlink ability checks to a policy

We can extend the policy in EE for additional behavior

3 files changed, 86 insertions, 9 deletions

diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb
index e0da23ca0b8..06c6f49f7cc 100644
--- a/spec/controllers/omniauth_callbacks_controller_spec.rb
+++ b/spec/controllers/omniauth_callbacks_controller_spec.rb
@@ -113,6 +113,33 @@ describe OmniauthCallbacksController, type: :controller do
           expect(request.env['warden']).to be_authenticated
         end
 
+        context 'when user has no linked provider' do
+          let(:user) { create(:user) }
+
+          before do
+            sign_in user
+          end
+
+          it 'links identity' do
+            expect do
+              post provider
+              user.reload
+            end.to change { user.identities.count }.by(1)
+          end
+
+          context 'and is not allowed to link the provider' do
+            before do
+              allow_any_instance_of(IdentityProviderPolicy).to receive(:can?).with(:link).and_return(false)
+            end
+
+            it 'returns 403' do
+              post provider
+
+              expect(response).to have_gitlab_http_status(403)
+            end
+          end
+        end
+
         shared_context 'sign_up' do
           let(:user) { double(email: 'new@example.com') }
 
diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb
index f0c2e4768ec..2ba8b3dbf22 100644
--- a/spec/helpers/auth_helper_spec.rb
+++ b/spec/helpers/auth_helper_spec.rb
@@ -97,17 +97,37 @@ describe AuthHelper do
     end
   end
 
-  describe 'unlink_allowed?' do
-    [:saml, :cas3].each do |provider|
-      it "returns true if the provider is #{provider}" do
-        expect(helper.unlink_allowed?(provider)).to be false
-      end
+  describe '#link_provider_allowed?' do
+    let(:policy) { instance_double('IdentityProviderPolicy') }
+    let(:current_user) { instance_double('User') }
+    let(:provider) { double }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(current_user)
+      allow(IdentityProviderPolicy).to receive(:new).with(current_user, provider).and_return(policy)
     end
 
-    [:twitter, :facebook, :google_oauth2, :gitlab, :github, :bitbucket, :crowd, :auth0, :authentiq].each do |provider|
-      it "returns false if the provider is #{provider}" do
-        expect(helper.unlink_allowed?(provider)).to be true
-      end
+    it 'delegates to identity provider policy' do
+      allow(policy).to receive(:can?).with(:link).and_return('policy_link_result')
+
+      expect(helper.link_provider_allowed?(provider)).to eq 'policy_link_result'
+    end
+  end
+
+  describe '#unlink_provider_allowed?' do
+    let(:policy) { instance_double('IdentityProviderPolicy') }
+    let(:current_user) { instance_double('User') }
+    let(:provider) { double }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(current_user)
+      allow(IdentityProviderPolicy).to receive(:new).with(current_user, provider).and_return(policy)
+    end
+
+    it 'delegates to identity provider policy' do
+      allow(policy).to receive(:can?).with(:unlink).and_return('policy_unlink_result')
+
+      expect(helper.unlink_provider_allowed?(provider)).to eq 'policy_unlink_result'
     end
   end
 end
diff --git a/spec/policies/identity_provider_policy_spec.rb b/spec/policies/identity_provider_policy_spec.rb
new file mode 100644
index 00000000000..2520469d4e7
--- /dev/null
+++ b/spec/policies/identity_provider_policy_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe IdentityProviderPolicy do
+  subject(:policy) { described_class.new(user, provider) }
+  let(:user) { User.new }
+  let(:provider) { :a_provider }
+
+  describe '#rules' do
+    it { is_expected.to be_allowed(:link) }
+    it { is_expected.to be_allowed(:unlink) }
+
+    context 'when user is anonymous' do
+      let(:user) { nil }
+
+      it { is_expected.not_to be_allowed(:link) }
+      it { is_expected.not_to be_allowed(:unlink) }
+    end
+
+    %w[saml cas3].each do |provider_name|
+      context "when provider is #{provider_name}" do
+        let(:provider) { provider_name }
+
+        it { is_expected.to be_allowed(:link) }
+        it { is_expected.not_to be_allowed(:unlink) }
+      end
+    end
+  end
+end