Commit d733a966 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'allow-owner-to-run-ci-builds' into 'master'

Allow owners to fetch source code in CI builds

Due to different way of handling owners of a project, they were not allowed to fetch CI sources for project.

This adds a separate code path for handling owners, that are not admins.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23437

See merge request !6943
parents 90072d61 517dd4a3
...@@ -2,11 +2,11 @@ class ProjectPolicy < BasePolicy ...@@ -2,11 +2,11 @@ class ProjectPolicy < BasePolicy
def rules def rules
team_access!(user) team_access!(user)
owner = user.admin? || owner = project.owner == user ||
project.owner == user ||
(project.group && project.group.has_owner?(user)) (project.group && project.group.has_owner?(user))
owner_access! if owner owner_access! if user.admin? || owner
team_member_owner_access! if owner
if project.public? || (project.internal? && !user.external?) if project.public? || (project.internal? && !user.external?)
guest_access! guest_access!
...@@ -16,7 +16,7 @@ class ProjectPolicy < BasePolicy ...@@ -16,7 +16,7 @@ class ProjectPolicy < BasePolicy
can! :read_build if project.public_builds? can! :read_build if project.public_builds?
if project.request_access_enabled && if project.request_access_enabled &&
!(owner || project.team.member?(user) || project_group_member?(user)) !(owner || user.admin? || project.team.member?(user) || project_group_member?(user))
can! :request_access can! :request_access
end end
end end
...@@ -135,6 +135,10 @@ class ProjectPolicy < BasePolicy ...@@ -135,6 +135,10 @@ class ProjectPolicy < BasePolicy
can! :destroy_issue can! :destroy_issue
end end
def team_member_owner_access!
team_member_reporter_access!
end
# Push abilities on the users team role # Push abilities on the users team role
def team_access!(user) def team_access!(user)
access = project.team.max_member_access(user.id) access = project.team.max_member_access(user.id)
......
...@@ -122,6 +122,14 @@ describe Gitlab::GitAccess, lib: true do ...@@ -122,6 +122,14 @@ describe Gitlab::GitAccess, lib: true do
describe 'build authentication_abilities permissions' do describe 'build authentication_abilities permissions' do
let(:authentication_abilities) { build_authentication_abilities } let(:authentication_abilities) { build_authentication_abilities }
describe 'owner' do
let(:project) { create(:project, namespace: user.namespace) }
context 'pull code' do
it { expect(subject).to be_allowed }
end
end
describe 'reporter user' do describe 'reporter user' do
before { project.team << [user, :reporter] } before { project.team << [user, :reporter] }
......
...@@ -6,6 +6,7 @@ describe ProjectPolicy, models: true do ...@@ -6,6 +6,7 @@ describe ProjectPolicy, models: true do
let(:dev) { create(:user) } let(:dev) { create(:user) }
let(:master) { create(:user) } let(:master) { create(:user) }
let(:owner) { create(:user) } let(:owner) { create(:user) }
let(:admin) { create(:admin) }
let(:project) { create(:empty_project, :public, namespace: owner.namespace) } let(:project) { create(:empty_project, :public, namespace: owner.namespace) }
let(:guest_permissions) do let(:guest_permissions) do
...@@ -152,6 +153,19 @@ describe ProjectPolicy, models: true do ...@@ -152,6 +153,19 @@ describe ProjectPolicy, models: true do
context 'owner' do context 'owner' do
let(:current_user) { owner } let(:current_user) { owner }
it do
is_expected.to include(*guest_permissions)
is_expected.to include(*reporter_permissions)
is_expected.to include(*team_member_reporter_permissions)
is_expected.to include(*developer_permissions)
is_expected.to include(*master_permissions)
is_expected.to include(*owner_permissions)
end
end
context 'admin' do
let(:current_user) { admin }
it do it do
is_expected.to include(*guest_permissions) is_expected.to include(*guest_permissions)
is_expected.to include(*reporter_permissions) is_expected.to include(*reporter_permissions)
......
...@@ -284,7 +284,17 @@ describe 'Git LFS API and storage' do ...@@ -284,7 +284,17 @@ describe 'Git LFS API and storage' do
let(:authorization) { authorize_ci_project } let(:authorization) { authorize_ci_project }
shared_examples 'can download LFS only from own projects' do shared_examples 'can download LFS only from own projects' do
context 'for own project' do context 'for owned project' do
let(:project) { create(:empty_project, namespace: user.namespace) }
let(:update_permissions) do
project.lfs_objects << lfs_object
end
it_behaves_like 'responds with a file'
end
context 'for member of project' do
let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:pipeline) { create(:ci_empty_pipeline, project: project) }
let(:update_permissions) do let(:update_permissions) do
......
...@@ -245,6 +245,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -245,6 +245,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
it_behaves_like 'a pullable' it_behaves_like 'a pullable'
end end
context 'when you are owner' do
let(:project) { create(:empty_project, namespace: current_user.namespace) }
it_behaves_like 'a pullable'
end
end end
context 'for private' do context 'for private' do
...@@ -266,6 +272,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -266,6 +272,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
it_behaves_like 'a pullable' it_behaves_like 'a pullable'
end end
context 'when you are owner' do
let(:project) { create(:empty_project, namespace: current_user.namespace) }
it_behaves_like 'a pullable'
end
end end
end end
end end
...@@ -276,6 +288,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -276,6 +288,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
end end
context 'disallow for all' do context 'disallow for all' do
context 'when you are member' do
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :public) }
before do before do
...@@ -284,6 +297,13 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do ...@@ -284,6 +297,13 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
it_behaves_like 'an inaccessible' it_behaves_like 'an inaccessible'
end end
context 'when you are owner' do
let(:project) { create(:empty_project, :public, namespace: current_user.namespace) }
it_behaves_like 'an inaccessible'
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