diff options
author | Pavel Shutsin <pshutsin@gitlab.com> | 2019-03-18 17:36:34 +0300 |
---|---|---|
committer | Pavel Shutsin <pshutsin@gitlab.com> | 2019-03-19 15:38:16 +0300 |
commit | 8ee1927db90d43205b4e6f8bd13f209c74b41bd1 (patch) | |
tree | 247e5f813947c1bdeb838e2776835208e6a7e2bc /spec | |
parent | a4b18040778d7272bd8fbbb3746e199699ffd893 (diff) | |
download | gitlab-ce-8ee1927db90d43205b4e6f8bd13f209c74b41bd1.tar.gz |
Move out link\unlink ability checks to a policy
We can extend the policy in EE for additional behavior
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/omniauth_callbacks_controller_spec.rb | 27 | ||||
-rw-r--r-- | spec/helpers/auth_helper_spec.rb | 38 | ||||
-rw-r--r-- | spec/policies/identity_provider_policy_spec.rb | 30 |
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 |