Commit 53b09735 authored by Imre Farkas's avatar Imre Farkas Committed by Jan Provaznik

Owners of personal projects to have OWNER access level

Owners of personal projects are displayed as maintainers in the members
page. This change fixes inconsistency for new projects. It is behind
the personal_project_owner_with_owner_access feature flag.
parent a5b471a6
......@@ -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
# 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
......@@ -445,7 +445,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
......
......@@ -146,10 +146,14 @@ module Projects
blocking: false,
priority: UserProjectAccessChangedService::LOW_PRIORITY
)
else
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
def create_readme
commit_attrs = {
......
---
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:
......
......@@ -33,7 +33,13 @@ module Gitlab
MAINTAINER_SUBGROUP_ACCESS = 1
class << self
delegate :values, to: :options
def values
if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml)
options_with_owner.values
else
options.values
end
end
def all_values
options_with_owner.values
......
......@@ -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,7 +11,25 @@ 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
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(
......@@ -23,9 +41,10 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
)
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,7 +103,8 @@ 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
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(
......@@ -96,6 +116,20 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
)
)
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
context 'project in a subgroup' do
......
......@@ -34,6 +34,21 @@ RSpec.describe Gitlab::ProjectAuthorizations do
.to include(owned_project.id, other_project.id, group_project.id)
end
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
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)
......@@ -42,6 +57,7 @@ RSpec.describe Gitlab::ProjectAuthorizations do
expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER)
end
end
end
context 'unapproved access request' do
let_it_be(:group) { create(:group) }
......
......@@ -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)
......
......@@ -673,7 +673,23 @@ RSpec.describe API::Members do
end
context 'adding owner to project' do
it 'returns 403' do
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(: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 }
......@@ -682,6 +698,7 @@ RSpec.describe API::Members do
end.not_to change { project.members.count }
end
end
end
context 'remove bot from project' do
it 'returns a 403 forbidden' do
......
......@@ -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,17 +116,37 @@ RSpec.describe Projects::CreateService, '#execute' do
end
context 'user namespace' do
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.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
describe 'after create actions' do
it 'invalidate personal_projects_count caches' do
expect(user).to receive(:invalidate_personal_projects_count)
......@@ -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