Commit a9eee14b authored by Jan Provaznik's avatar Jan Provaznik

Merge branch 'if-personal_project_owners_reverted' into 'master'

Owners of personal projects to have OWNER access level

See merge request gitlab-org/gitlab!80825
parents 578557a4 f2a87a4b
......@@ -40,7 +40,7 @@ module Projects
avenues = [authorizable_project_members]
avenues << if project.personal?
project_owner_acting_as_maintainer
project_owner
else
authorizable_group_members
end
......@@ -85,9 +85,15 @@ module Projects
Member.from_union(members)
end
def project_owner_acting_as_maintainer
# workaround until we migrate Project#owners to have membership with
# OWNER access level
def project_owner
user_id = project.namespace.owner.id
access_level = Gitlab::Access::MAINTAINER
access_level = if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml)
Gitlab::Access::OWNER
else
Gitlab::Access::MAINTAINER
end
Member
.from(generate_from_statement([[user_id, access_level]])) # rubocop: disable CodeReuse/ActiveRecord
......
......@@ -8,8 +8,14 @@ module SelectForProjectAuthorization
select("projects.id AS project_id", "members.access_level")
end
def select_as_maintainer_for_project_authorization
select(["projects.id AS project_id", "#{Gitlab::Access::MAINTAINER} AS access_level"])
# workaround until we migrate Project#owners to have membership with
# OWNER access level
def select_project_owner_for_project_authorization
if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml)
select(["projects.id AS project_id", "#{Gitlab::Access::OWNER} AS access_level"])
else
select(["projects.id AS project_id", "#{Gitlab::Access::MAINTAINER} AS access_level"])
end
end
end
end
......@@ -94,9 +94,16 @@ class ProjectMember < Member
override :access_level_inclusion
def access_level_inclusion
return if access_level.in?(Gitlab::Access.values)
errors.add(:access_level, "is not included in the list")
allowed_values = if ::Feature.enabled?(:personal_project_owner_with_owner_access,
default_enabled: :yaml)
Gitlab::Access.all_values
else
Gitlab::Access.values
end
unless access_level.in?(allowed_values)
errors.add(:access_level, "is not included in the list")
end
end
override :refresh_member_authorized_projects
......
......@@ -459,7 +459,7 @@ class Project < ApplicationRecord
delegate :name, to: :owner, allow_nil: true, prefix: true
delegate :members, to: :team, prefix: true
delegate :add_user, :add_users, to: :team
delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_role, to: :team
delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_owner, :add_role, to: :team
delegate :group_runners_enabled, :group_runners_enabled=, to: :ci_cd_settings, allow_nil: true
delegate :root_ancestor, to: :namespace, allow_nil: true
delegate :last_pipeline, to: :commit, allow_nil: true
......
......@@ -23,6 +23,10 @@ class ProjectTeam
add_user(user, :maintainer, current_user: current_user)
end
def add_owner(user, current_user: nil)
add_user(user, :owner, current_user: current_user)
end
def add_role(user, role, current_user: nil)
public_send(:"add_#{role}", user, current_user: current_user) # rubocop:disable GitlabSecurity/PublicSend
end
......@@ -103,7 +107,9 @@ class ProjectTeam
if group
group.owners
else
[project.owner]
# workaround until we migrate Project#owners to have membership with
# OWNER access level
Array.wrap(fetch_members(Gitlab::Access::OWNER)) | Array.wrap(project.owner)
end
end
......
......@@ -4,7 +4,7 @@ module Members
module Projects
class CreatorService < Members::CreatorService
def self.access_levels
Gitlab::Access.sym_options
Gitlab::Access.sym_options_with_owner
end
private
......
......@@ -14,6 +14,7 @@ module NotificationRecipients
return [] unless project
add_recipients(project.team.maintainers, :mention, nil)
add_recipients(project.team.owners, :mention, nil)
end
def acting_user
......
......@@ -147,7 +147,11 @@ module Projects
priority: UserProjectAccessChangedService::LOW_PRIORITY
)
else
@project.add_maintainer(@project.namespace.owner, current_user: current_user)
if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml)
@project.add_owner(@project.namespace.owner, current_user: current_user)
else
@project.add_maintainer(@project.namespace.owner, current_user: current_user)
end
end
end
......
---
name: personal_project_owner_with_owner_access
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78193
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351919
milestone: '14.8'
type: development
group: group::workspace
default_enabled: false
......@@ -33,14 +33,27 @@ usernames. A GitLab administrator can configure the GitLab instance to
## Project members permissions
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/219299) in GitLab 14.8, personal namespace owners appear with Owner role in new projects in their namespace. Introduced [with a flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`. Disabled by default.
FLAG:
On self-managed GitLab, personal namespace owners appearing with the Owner role in new projects in their namespace is disabled. To make it available,
ask an administrator to [enable the feature flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`.
The feature is not ready for production use.
On GitLab.com, this feature is not available.
A user's role determines what permissions they have on a project. The Owner role provides all permissions but is
available only:
- For group owners. The role is inherited for a group's projects.
- For Administrators.
Personal namespace owners have the same permissions as an Owner, but are displayed with the Maintainer role on projects created in their personal namespace.
For more information, see [projects members documentation](project/members/index.md).
Personal [namespace](group/index.md#namespaces) owners:
- Are displayed as having the Maintainer role on projects in the namespace, but have the same permissions as a user with the Owner role.
- (Disabled by default) In GitLab 14.8 and later, for new projects in the namespace, are displayed as having the Owner role.
For more information about how to manage project members, see
[members of a project](project/members/index.md).
The following table lists project permissions available for each role:
......
......@@ -22,7 +22,7 @@ module Gitlab
user.projects_with_active_memberships.select_for_project_authorization,
# The personal projects of the user.
user.personal_projects.select_as_maintainer_for_project_authorization,
user.personal_projects.select_project_owner_for_project_authorization,
# Projects that belong directly to any of the groups the user has
# access to.
......
......@@ -665,7 +665,7 @@ RSpec.describe Projects::ProjectMembersController do
sign_in(user)
end
it 'does not create a member' do
it 'creates a member' do
expect do
post :create, params: {
user_ids: stranger.id,
......@@ -673,7 +673,9 @@ RSpec.describe Projects::ProjectMembersController do
access_level: Member::OWNER,
project_id: project
}
end.to change { project.members.count }.by(0)
end.to change { project.members.count }.by(1)
expect(project.team_members).to include(user)
end
end
......
......@@ -11,21 +11,40 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
context 'for a personal project' do
let_it_be(:project) { create(:project) }
shared_examples_for 'includes access level of the owner of the project as Maintainer' do
it 'includes access level of the owner of the project as Maintainer' do
expect(subject).to(
contain_exactly(
hash_including(
'user_id' => project.namespace.owner.id,
'access_level' => Gitlab::Access::MAINTAINER
shared_examples_for 'includes access level of the owner of the project' do
context 'when personal_project_owner_with_owner_access feature flag is enabled' do
it 'includes access level of the owner of the project as Owner' do
expect(subject).to(
contain_exactly(
hash_including(
'user_id' => project.namespace.owner.id,
'access_level' => Gitlab::Access::OWNER
)
)
)
)
end
end
context 'when personal_project_owner_with_owner_access feature flag is disabled' do
before do
stub_feature_flags(personal_project_owner_with_owner_access: false)
end
it 'includes access level of the owner of the project as Maintainer' do
expect(subject).to(
contain_exactly(
hash_including(
'user_id' => project.namespace.owner.id,
'access_level' => Gitlab::Access::MAINTAINER
)
)
)
end
end
end
context 'when the project owner is a member of the project' do
it_behaves_like 'includes access level of the owner of the project as Maintainer'
it_behaves_like 'includes access level of the owner of the project'
end
context 'when the project owner is not explicitly a member of the project' do
......@@ -33,7 +52,7 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
project.members.find_by(user_id: project.namespace.owner.id).destroy!
end
it_behaves_like 'includes access level of the owner of the project as Maintainer'
it_behaves_like 'includes access level of the owner of the project'
end
end
......@@ -84,17 +103,32 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
context 'for a project within a group' do
context 'project in a root group' do
it 'includes access levels of users who are direct members of the parent group' do
group_member = create(:group_member, :developer, source: group)
context 'includes access levels of users who are direct members of the parent group' do
it 'when access level is developer' do
group_member = create(:group_member, :developer, source: group)
expect(subject).to(
include(
hash_including(
'user_id' => group_member.user.id,
'access_level' => Gitlab::Access::DEVELOPER
expect(subject).to(
include(
hash_including(
'user_id' => group_member.user.id,
'access_level' => Gitlab::Access::DEVELOPER
)
)
)
)
end
it 'when access level is owner' do
group_member = create(:group_member, :owner, source: group)
expect(subject).to(
include(
hash_including(
'user_id' => group_member.user.id,
'access_level' => Gitlab::Access::OWNER
)
)
)
end
end
end
......
......@@ -34,12 +34,28 @@ RSpec.describe Gitlab::ProjectAuthorizations do
.to include(owned_project.id, other_project.id, group_project.id)
end
it 'includes the correct access levels' do
mapping = map_access_levels(authorizations)
context 'when personal_project_owner_with_owner_access feature flag is enabled' do
it 'includes the correct access levels' do
mapping = map_access_levels(authorizations)
expect(mapping[owned_project.id]).to eq(Gitlab::Access::OWNER)
expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER)
expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER)
end
end
expect(mapping[owned_project.id]).to eq(Gitlab::Access::MAINTAINER)
expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER)
expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER)
context 'when personal_project_owner_with_owner_access feature flag is disabled' do
before do
stub_feature_flags(personal_project_owner_with_owner_access: false)
end
it 'includes the correct access levels' do
mapping = map_access_levels(authorizations)
expect(mapping[owned_project.id]).to eq(Gitlab::Access::MAINTAINER)
expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER)
expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER)
end
end
end
......
......@@ -225,7 +225,7 @@ RSpec.describe ProjectTeam do
let_it_be(:maintainer) { create(:user) }
let_it_be(:developer) { create(:user) }
let_it_be(:guest) { create(:user) }
let_it_be(:project) { create(:project, namespace: maintainer.namespace) }
let_it_be(:project) { create(:project, group: create(:group)) }
let_it_be(:access_levels) { [Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER] }
subject(:members_with_access_levels) { project.team.members_with_access_levels(access_levels) }
......
......@@ -3717,7 +3717,7 @@ RSpec.describe User do
context 'with min_access_level' do
let!(:user) { create(:user) }
let!(:project) { create(:project, :private, namespace: user.namespace) }
let!(:project) { create(:project, :private, group: create(:group)) }
before do
project.add_developer(user)
......
......@@ -675,13 +675,30 @@ RSpec.describe API::Members do
end
context 'adding owner to project' do
it 'returns 403' do
expect do
post api("/projects/#{project.id}/members", maintainer),
params: { user_id: stranger.id, access_level: Member::OWNER }
context 'when personal_project_owner_with_owner_access feature flag is enabled' do
it 'returns created status' do
expect do
post api("/projects/#{project.id}/members", maintainer),
params: { user_id: stranger.id, access_level: Member::OWNER }
expect(response).to have_gitlab_http_status(:bad_request)
end.not_to change { project.members.count }
expect(response).to have_gitlab_http_status(:created)
end.to change { project.members.count }.by(1)
end
end
context 'when personal_project_owner_with_owner_access feature flag is disabled' do
before do
stub_feature_flags(personal_project_owner_with_owner_access: false)
end
it 'returns created status' do
expect do
post api("/projects/#{project.id}/members", maintainer),
params: { user_id: stranger.id, access_level: Member::OWNER }
expect(response).to have_gitlab_http_status(:bad_request)
end.not_to change { project.members.count }
end
end
end
......
......@@ -40,7 +40,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
it 'is called' do
ProjectAuthorization.delete_all
expect(callback).to receive(:call).with(project.id, Gitlab::Access::MAINTAINER).once
expect(callback).to receive(:call).with(project.id, Gitlab::Access::OWNER).once
service.execute
end
......@@ -60,20 +60,20 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
to_be_removed = [project2.id]
to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER }
]
expect(service.execute).to eq([to_be_removed, to_be_added])
end
it 'finds duplicate entries that has to be removed' do
[Gitlab::Access::MAINTAINER, Gitlab::Access::REPORTER].each do |access_level|
[Gitlab::Access::OWNER, Gitlab::Access::REPORTER].each do |access_level|
user.project_authorizations.create!(project: project, access_level: access_level)
end
to_be_removed = [project.id]
to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER }
]
expect(service.execute).to eq([to_be_removed, to_be_added])
......@@ -85,7 +85,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
to_be_removed = [project.id]
to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER }
]
expect(service.execute).to eq([to_be_removed, to_be_added])
......@@ -143,16 +143,16 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
end
it 'sets the keys to the project IDs' do
expect(hash.keys).to eq([project.id])
expect(hash.keys).to match_array([project.id])
end
it 'sets the values to the access levels' do
expect(hash.values).to eq([Gitlab::Access::MAINTAINER])
expect(hash.values).to match_array([Gitlab::Access::OWNER])
end
context 'personal projects' do
it 'includes the project with the right access level' do
expect(hash[project.id]).to eq(Gitlab::Access::MAINTAINER)
expect(hash[project.id]).to eq(Gitlab::Access::OWNER)
end
end
......@@ -242,7 +242,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
value = hash.values[0]
expect(value.project_id).to eq(project.id)
expect(value.access_level).to eq(Gitlab::Access::MAINTAINER)
expect(value.access_level).to eq(Gitlab::Access::OWNER)
end
end
......@@ -267,7 +267,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
end
it 'includes the access level for every row' do
expect(row.access_level).to eq(Gitlab::Access::MAINTAINER)
expect(row.access_level).to eq(Gitlab::Access::OWNER)
end
end
end
......@@ -283,7 +283,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
rows = service.fresh_authorizations.to_a
expect(rows.length).to eq(1)
expect(rows.first.access_level).to eq(Gitlab::Access::MAINTAINER)
expect(rows.first.access_level).to eq(Gitlab::Access::OWNER)
end
context 'every returned row' do
......@@ -294,7 +294,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
end
it 'includes the access level' do
expect(row.access_level).to eq(Gitlab::Access::MAINTAINER)
expect(row.access_level).to eq(Gitlab::Access::OWNER)
end
end
end
......
......@@ -9,8 +9,8 @@ RSpec.describe Members::Projects::CreatorService do
end
describe '.access_levels' do
it 'returns Gitlab::Access.sym_options' do
expect(described_class.access_levels).to eq(Gitlab::Access.sym_options)
it 'returns Gitlab::Access.sym_options_with_owner' do
expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner)
end
end
end
......@@ -3312,7 +3312,7 @@ RSpec.describe NotificationService, :mailer do
describe "##{sym}" do
subject(:notify!) { notification.send(sym, domain) }
it 'emails current watching maintainers' do
it 'emails current watching maintainers and owners' do
expect(Notify).to receive(:"#{sym}_email").at_least(:once).and_call_original
notify!
......@@ -3410,7 +3410,7 @@ RSpec.describe NotificationService, :mailer do
reset_delivered_emails!
end
it 'emails current watching maintainers' do
it 'emails current watching maintainers and owners' do
notification.remote_mirror_update_failed(remote_mirror)
should_only_email(u_maintainer1, u_maintainer2, u_owner)
......
......@@ -116,14 +116,34 @@ RSpec.describe Projects::CreateService, '#execute' do
end
context 'user namespace' do
it 'creates a project in user namespace' do
project = create_project(user, opts)
context 'when personal_project_owner_with_owner_access feature flag is enabled' do
it 'creates a project in user namespace' do
project = create_project(user, opts)
expect(project).to be_valid
expect(project.first_owner).to eq(user)
expect(project.team.maintainers).to include(user)
expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project)
expect(project).to be_valid
expect(project.first_owner).to eq(user)
expect(project.team.maintainers).not_to include(user)
expect(project.team.owners).to contain_exactly(user)
expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project)
end
end
context 'when personal_project_owner_with_owner_access feature flag is disabled' do
before do
stub_feature_flags(personal_project_owner_with_owner_access: false)
end
it 'creates a project in user namespace' do
project = create_project(user, opts)
expect(project).to be_valid
expect(project.first_owner).to eq(user)
expect(project.team.maintainers).to contain_exactly(user)
expect(project.team.owners).to contain_exactly(user)
expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project)
end
end
end
......@@ -162,7 +182,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to be_persisted
expect(project.owner).to eq(user)
expect(project.first_owner).to eq(user)
expect(project.team.maintainers).to contain_exactly(user)
expect(project.team.owners).to contain_exactly(user)
expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project)
end
......
......@@ -52,7 +52,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
it 'is called' do
ProjectAuthorization.delete_all
expect(callback).to receive(:call).with(project.id, Gitlab::Access::MAINTAINER).once
expect(callback).to receive(:call).with(project.id, Gitlab::Access::OWNER).once
service.execute
end
......@@ -73,7 +73,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
to_be_removed = [project_authorization.project_id]
to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER }
]
expect(service).to receive(:update_authorizations)
......@@ -83,14 +83,14 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
end
it 'removes duplicate entries' do
[Gitlab::Access::MAINTAINER, Gitlab::Access::REPORTER].each do |access_level|
[Gitlab::Access::OWNER, Gitlab::Access::REPORTER].each do |access_level|
user.project_authorizations.create!(project: project, access_level: access_level)
end
to_be_removed = [project.id]
to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER }
]
expect(service).to(
receive(:update_authorizations)
......@@ -103,7 +103,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
project_authorization = ProjectAuthorization.where(
project_id: project.id,
user_id: user.id,
access_level: Gitlab::Access::MAINTAINER)
access_level: Gitlab::Access::OWNER)
expect(project_authorization).to exist
end
......@@ -116,7 +116,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
to_be_removed = [project_authorization.project_id]
to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER }
]
expect(service).to receive(:update_authorizations)
......
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