Commit d801dd17 authored by Bob Van Landuyt's avatar Bob Van Landuyt

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.
parent f7f13f9d
class GlobalPolicy < BasePolicy class GlobalPolicy < BasePolicy
desc "User is blocked" desc "User is blocked"
with_options scope: :user, score: 0 with_options scope: :user, score: 0
condition(:blocked) { @user.blocked? } condition(:blocked) { @user&.blocked? }
desc "User is an internal user" desc "User is an internal user"
with_options scope: :user, score: 0 with_options scope: :user, score: 0
condition(:internal) { @user.internal? } condition(:internal) { @user&.internal? }
desc "User's access has been locked" desc "User's access has been locked"
with_options scope: :user, score: 0 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 condition(:required_terms_not_accepted, scope: :user, score: 0) do
@user&.required_terms_not_accepted? @user&.required_terms_not_accepted?
...@@ -19,8 +19,6 @@ class GlobalPolicy < BasePolicy ...@@ -19,8 +19,6 @@ class GlobalPolicy < BasePolicy
rule { anonymous }.policy do rule { anonymous }.policy do
prevent :log_in prevent :log_in
prevent :access_api
prevent :access_git
prevent :receive_notifications prevent :receive_notifications
prevent :use_quick_actions prevent :use_quick_actions
prevent :create_group prevent :create_group
......
...@@ -91,21 +91,31 @@ describe GlobalPolicy do ...@@ -91,21 +91,31 @@ describe GlobalPolicy do
end end
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 'API access' do
describe 'regular user' do context 'regular user' do
it { is_expected.to be_allowed(:access_api) } it { is_expected.to be_allowed(:access_api) }
end end
describe 'admin' do context 'admin' do
let(:current_user) { create(:admin) } let(:current_user) { create(:admin) }
it { is_expected.to be_allowed(:access_api) } it { is_expected.to be_allowed(:access_api) }
end end
describe 'anonymous' do context 'anonymous' do
let(:current_user) { nil } let(:current_user) { nil }
it { is_expected.not_to be_allowed(:access_api) } it { is_expected.to be_allowed(:access_api) }
end end
context 'when terms are enforced' do context 'when terms are enforced' do
...@@ -113,12 +123,20 @@ describe GlobalPolicy do ...@@ -113,12 +123,20 @@ describe GlobalPolicy do
enforce_terms enforce_terms
end 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
it 'allows access to the API when the user accepted the terms' do context 'admin' do
accept_terms(current_user) let(:current_user) { create(:admin) }
is_expected.to be_allowed(:access_api) it_behaves_like 'access allowed when terms accepted', :access_api
end
context 'anonymous' do
let(:current_user) { nil }
it { is_expected.to be_allowed(:access_api) }
end end
end end
end end
...@@ -137,7 +155,7 @@ describe GlobalPolicy do ...@@ -137,7 +155,7 @@ describe GlobalPolicy do
describe 'anonymous' do describe 'anonymous' do
let(:current_user) { nil } let(:current_user) { nil }
it { is_expected.not_to be_allowed(:access_git) } it { is_expected.to be_allowed(:access_git) }
end end
context 'when terms are enforced' do context 'when terms are enforced' do
...@@ -145,12 +163,20 @@ describe GlobalPolicy do ...@@ -145,12 +163,20 @@ describe GlobalPolicy do
enforce_terms enforce_terms
end 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 'allows access to git when terms are accepted' do it_behaves_like 'access allowed when terms accepted', :access_git
accept_terms(current_user) end
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 end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment