diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-05-09 13:55:31 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-05-10 18:02:27 +0300 |
commit | d801dd177483a8375f1656654ca3638c18550204 (patch) | |
tree | 91dbc59dd7dfb437308e2d39238c024528268064 | |
parent | f7f13f9db0da92c7b43481dfe5559f317711e533 (diff) |
Allows `access_(git|api)` to anonymous users
The `access_git` and `access_api` were currently never checked for
anonymous users. And they would also be allowed access:
An anonymous user can clone and pull from a public repo
An anonymous user can request public information from the API
So the policy didn't actually reflect what we were enforcing.
-rw-r--r-- | app/policies/global_policy.rb | 10 | ||||
-rw-r--r-- | spec/policies/global_policy_spec.rb | 52 |
2 files changed, 43 insertions, 19 deletions
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index e2736d3bb61..1cf5515d9d7 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -1,17 +1,17 @@ class GlobalPolicy < BasePolicy desc "User is blocked" with_options scope: :user, score: 0 - condition(:blocked) { @user.blocked? } + condition(:blocked) { @user&.blocked? } desc "User is an internal user" with_options scope: :user, score: 0 - condition(:internal) { @user.internal? } + condition(:internal) { @user&.internal? } desc "User's access has been locked" with_options scope: :user, score: 0 - condition(:access_locked) { @user.access_locked? } + condition(:access_locked) { @user&.access_locked? } - condition(:can_create_fork, scope: :user) { @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } } + condition(:can_create_fork, scope: :user) { @user && @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } } condition(:required_terms_not_accepted, scope: :user, score: 0) do @user&.required_terms_not_accepted? @@ -19,8 +19,6 @@ class GlobalPolicy < BasePolicy rule { anonymous }.policy do prevent :log_in - prevent :access_api - prevent :access_git prevent :receive_notifications prevent :use_quick_actions prevent :create_group diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index 91d37db035a..873673b50ef 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -91,21 +91,31 @@ describe GlobalPolicy do end end + shared_examples 'access allowed when terms accepted' do |ability| + it { is_expected.not_to be_allowed(ability) } + + it "allows #{ability} when the user accepted the terms" do + accept_terms(current_user) + + is_expected.to be_allowed(ability) + end + end + describe 'API access' do - describe 'regular user' do + context 'regular user' do it { is_expected.to be_allowed(:access_api) } end - describe 'admin' do + context 'admin' do let(:current_user) { create(:admin) } it { is_expected.to be_allowed(:access_api) } end - describe 'anonymous' do + context 'anonymous' do let(:current_user) { nil } - it { is_expected.not_to be_allowed(:access_api) } + it { is_expected.to be_allowed(:access_api) } end context 'when terms are enforced' do @@ -113,12 +123,20 @@ describe GlobalPolicy do enforce_terms end - it { is_expected.not_to be_allowed(:access_api) } + context 'regular user' do + it_behaves_like 'access allowed when terms accepted', :access_api + end + + context 'admin' do + let(:current_user) { create(:admin) } + + it_behaves_like 'access allowed when terms accepted', :access_api + end - it 'allows access to the API when the user accepted the terms' do - accept_terms(current_user) + context 'anonymous' do + let(:current_user) { nil } - is_expected.to be_allowed(:access_api) + it { is_expected.to be_allowed(:access_api) } end end end @@ -137,7 +155,7 @@ describe GlobalPolicy do describe 'anonymous' do let(:current_user) { nil } - it { is_expected.not_to be_allowed(:access_git) } + it { is_expected.to be_allowed(:access_git) } end context 'when terms are enforced' do @@ -145,12 +163,20 @@ describe GlobalPolicy do enforce_terms end - it { is_expected.not_to be_allowed(:access_git) } + context 'regular user' do + it_behaves_like 'access allowed when terms accepted', :access_git + end + + context 'admin' do + let(:current_user) { create(:admin) } + + it_behaves_like 'access allowed when terms accepted', :access_git + end - it 'allows access to git when terms are accepted' do - accept_terms(current_user) + context 'anonymous' do + let(:current_user) { nil } - is_expected.to be_allowed(:access_git) + it { is_expected.to be_allowed(:access_git) } end end end |