From f965979e84a9bd6564614037f1e242fce0e3b80e Mon Sep 17 00:00:00 2001 From: Eugenia Grieff Date: Tue, 15 Oct 2024 12:04:17 +0200 Subject: [PATCH 1/5] Add Planner role Add a new static role that defines Product Manager abilities Add group policies for Planner role Add project policies for Planner role Add issue, epic and issuable policies Changelog: added --- .../javascripts/access_level/constants.js | 12 + app/finders/issues/confidentiality_filter.rb | 2 +- app/graphql/resolvers/projects_resolver.rb | 4 + app/graphql/types/member_access_level_enum.rb | 1 + app/helpers/projects_helper.rb | 1 + app/models/group.rb | 4 + app/models/issue.rb | 4 +- app/models/member.rb | 1 + app/models/project.rb | 2 +- app/models/project_team.rb | 12 + app/models/system/broadcast_message.rb | 1 + app/models/user.rb | 2 +- app/policies/board_policy.rb | 6 +- app/policies/group_policy.rb | 18 +- app/policies/issuable_policy.rb | 7 +- app/policies/issue_policy.rb | 6 +- app/policies/merge_request_policy.rb | 2 +- .../group_project_namespace_shared_policy.rb | 4 + app/policies/project_policy.rb | 43 +- .../groups/open_issues_count_service.rb | 8 +- .../projects/open_issues_count_service.rb | 10 +- .../destroy/confidential_issue_service.rb | 2 +- .../todos/destroy/entity_leave_service.rb | 34 +- doc/api/graphql/reference/index.md | 2 + ee/app/models/ee/group.rb | 2 +- ee/app/policies/ee/group_policy.rb | 12 +- ee/app/policies/ee/project_policy.rb | 8 +- ee/app/policies/epic_policy.rb | 11 +- .../ee/todos/destroy/entity_leave_service.rb | 4 +- ee/bin/custom-ability | 1 + ee/config/custom_abilities/read_code.yml | 2 +- .../custom_abilities/read_crm_contact.yml | 2 +- ee/lib/elastic/latest/epic_class_proxy.rb | 2 +- .../table/drawer/role_details_drawer_spec.js | 4 +- .../components/table/drawer/utils_spec.js | 5 +- .../members/components/table/max_role_spec.js | 6 +- .../members/standard_roles_resolver_spec.rb | 2 +- .../billable_users_utils_spec.rb | 2 +- ee/spec/models/ee/group_spec.rb | 11 +- ee/spec/models/members/member_role_spec.rb | 2 +- ee/spec/policies/epic_policy_spec.rb | 32 + ee/spec/policies/group_policy_spec.rb | 117 ++- ee/spec/policies/issuable_policy_spec.rb | 20 + ee/spec/policies/issue_policy_spec.rb | 7 + ee/spec/policies/project_policy_spec.rb | 263 +++--- .../requirement_policy_spec.rb | 3 +- ee/spec/policies/work_item_policy_spec.rb | 3 + .../api/graphql/group/standard_roles_spec.rb | 3 + .../members/group_standard_roles_spec.rb | 9 + .../members/instance_standard_roles_spec.rb | 8 + .../authorize_proxy_user_service_spec.rb | 4 +- .../approve_access_request_service_spec.rb | 15 + .../destroy/entity_leave_service_spec.rb | 2 +- .../requirement_policy_shared_examples.rb | 6 + lib/gitlab/access.rb | 4 + locale/gitlab.pot | 9 + spec/factories/groups.rb | 2 + spec/factories/member.rb | 1 + spec/factories/projects.rb | 2 + spec/factories/user_highest_roles.rb | 1 + spec/factories/users.rb | 2 + .../table/drawer/role_details_drawer_spec.js | 4 +- spec/frontend/members/mock_data.js | 1 + .../resolvers/merge_requests_resolver_spec.rb | 5 +- .../types/current_user_todos_type_spec.rb | 4 +- .../types/member_access_level_enum_spec.rb | 2 +- spec/helpers/groups_helper_spec.rb | 3 + spec/models/group_spec.rb | 1 + spec/models/members/project_member_spec.rb | 1 + spec/models/project_spec.rb | 2 +- spec/models/project_team_spec.rb | 30 +- spec/models/user_highest_role_spec.rb | 1 + spec/models/user_spec.rb | 24 +- spec/policies/board_policy_spec.rb | 49 +- spec/policies/group_policy_spec.rb | 138 +++- spec/policies/issuable_policy_spec.rb | 20 + spec/policies/issue_policy_spec.rb | 130 +-- spec/policies/merge_request_policy_spec.rb | 168 +++- spec/policies/project_policy_spec.rb | 753 +++++++++--------- spec/policies/work_item_policy_spec.rb | 2 + spec/presenters/member_presenter_spec.rb | 1 + spec/requests/api/ci/jobs_spec.rb | 2 +- spec/requests/api/members_spec.rb | 2 +- .../groups/open_issues_count_service_spec.rb | 2 +- .../open_issues_count_service_spec.rb | 2 +- .../destroy/entity_leave_service_spec.rb | 34 +- .../policies/group_policy_shared_context.rb | 11 + .../policies/project_policy_shared_context.rb | 15 + ...redacted_search_results_shared_examples.rb | 2 +- .../group_level_work_items_shared_examples.rb | 57 ++ ...roject_level_work_items_shared_examples.rb | 32 + .../project_policy_shared_examples.rb | 111 +++ .../policies/wiki_policies_shared_examples.rb | 9 + 93 files changed, 1651 insertions(+), 744 deletions(-) diff --git a/app/assets/javascripts/access_level/constants.js b/app/assets/javascripts/access_level/constants.js index c13d5fee5f0ee1..2059264c694f8f 100644 --- a/app/assets/javascripts/access_level/constants.js +++ b/app/assets/javascripts/access_level/constants.js @@ -4,6 +4,7 @@ import { __, s__ } from '~/locale'; export const ACCESS_LEVEL_NO_ACCESS_INTEGER = 0; export const ACCESS_LEVEL_MINIMAL_ACCESS_INTEGER = 5; export const ACCESS_LEVEL_GUEST_INTEGER = 10; +export const ACCESS_LEVEL_PLANNER_INTEGER = 15; export const ACCESS_LEVEL_REPORTER_INTEGER = 20; export const ACCESS_LEVEL_DEVELOPER_INTEGER = 30; export const ACCESS_LEVEL_MAINTAINER_INTEGER = 40; @@ -14,6 +15,7 @@ export const ACCESS_LEVEL_ADMIN_INTEGER = 60; export const ACCESS_LEVEL_NO_ACCESS_STRING = 'NO_ACCESS'; export const ACCESS_LEVEL_MINIMAL_ACCESS_STRING = 'MINIMAL_ACCESS'; export const ACCESS_LEVEL_GUEST_STRING = 'GUEST'; +export const ACCESS_LEVEL_PLANNER_STRING = 'PLANNER'; export const ACCESS_LEVEL_REPORTER_STRING = 'REPORTER'; export const ACCESS_LEVEL_DEVELOPER_STRING = 'DEVELOPER'; export const ACCESS_LEVEL_MAINTAINER_STRING = 'MAINTAINER'; @@ -23,6 +25,7 @@ export const ACCESS_LEVELS_INTEGER_TO_STRING = { [ACCESS_LEVEL_NO_ACCESS_INTEGER]: ACCESS_LEVEL_NO_ACCESS_STRING, [ACCESS_LEVEL_MINIMAL_ACCESS_INTEGER]: ACCESS_LEVEL_MINIMAL_ACCESS_STRING, [ACCESS_LEVEL_GUEST_INTEGER]: ACCESS_LEVEL_GUEST_STRING, + [ACCESS_LEVEL_PLANNER_INTEGER]: ACCESS_LEVEL_PLANNER_STRING, [ACCESS_LEVEL_REPORTER_INTEGER]: ACCESS_LEVEL_REPORTER_STRING, [ACCESS_LEVEL_DEVELOPER_INTEGER]: ACCESS_LEVEL_DEVELOPER_STRING, [ACCESS_LEVEL_MAINTAINER_INTEGER]: ACCESS_LEVEL_MAINTAINER_STRING, @@ -32,6 +35,7 @@ export const ACCESS_LEVELS_INTEGER_TO_STRING = { const ACCESS_LEVEL_NO_ACCESS = __('No access'); const ACCESS_LEVEL_MINIMAL_ACCESS = __('Minimal Access'); const ACCESS_LEVEL_GUEST = __('Guest'); +const ACCESS_LEVEL_PLANNER = __('Planner'); const ACCESS_LEVEL_REPORTER = __('Reporter'); const ACCESS_LEVEL_DEVELOPER = __('Developer'); const ACCESS_LEVEL_MAINTAINER = __('Maintainer'); @@ -56,6 +60,14 @@ export const BASE_ROLES = [ 'MemberRole|The Guest role is for users who need visibility into a project or group but should not have the ability to make changes, such as external stakeholders.', ), }, + { + value: 'PLANNER', + text: ACCESS_LEVEL_PLANNER, + accessLevel: ACCESS_LEVEL_PLANNER_INTEGER, + memberRoleId: null, + occupiesSeat: true, + description: s__('MemberRole|The Planner role is for users who need to manage projects.'), + }, { value: 'REPORTER', text: ACCESS_LEVEL_REPORTER, diff --git a/app/finders/issues/confidentiality_filter.rb b/app/finders/issues/confidentiality_filter.rb index 282e56637fa5a5..c277635f5f5f47 100644 --- a/app/finders/issues/confidentiality_filter.rb +++ b/app/finders/issues/confidentiality_filter.rb @@ -2,7 +2,7 @@ module Issues class ConfidentialityFilter < Issuables::BaseFilter - CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::REPORTER + CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::PLANNER def initialize(current_user:, parent:, assignee_filter:, related_groups: nil, **kwargs) @current_user = current_user diff --git a/app/graphql/resolvers/projects_resolver.rb b/app/graphql/resolvers/projects_resolver.rb index 7c0b79a404a313..1bc621a2e3e2c0 100644 --- a/app/graphql/resolvers/projects_resolver.rb +++ b/app/graphql/resolvers/projects_resolver.rb @@ -35,6 +35,10 @@ class ProjectsResolver < BaseResolver required: false, description: 'Filter projects by programming language name (case insensitive). For example: "css" or "ruby".' + before_connection_authorization do |projects, current_user| + ::Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects, current_user).execute + end + def resolve_with_lookahead(**args) validate_args!(args) diff --git a/app/graphql/types/member_access_level_enum.rb b/app/graphql/types/member_access_level_enum.rb index 1789067ce0bbf7..96e3034955173a 100644 --- a/app/graphql/types/member_access_level_enum.rb +++ b/app/graphql/types/member_access_level_enum.rb @@ -10,6 +10,7 @@ def self.descriptions end value 'GUEST', value: Gitlab::Access::GUEST, description: descriptions[Gitlab::Access::GUEST] + value 'PLANNER', value: Gitlab::Access::PLANNER, description: descriptions[Gitlab::Access::PLANNER] value 'REPORTER', value: Gitlab::Access::REPORTER, description: descriptions[Gitlab::Access::REPORTER] value 'DEVELOPER', value: Gitlab::Access::DEVELOPER, description: descriptions[Gitlab::Access::DEVELOPER] value 'MAINTAINER', value: Gitlab::Access::MAINTAINER, description: descriptions[Gitlab::Access::MAINTAINER] diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 5996c34afa91cc..12fc46181af625 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -771,6 +771,7 @@ def localized_access_names Gitlab::Access::NO_ACCESS => _('No access'), Gitlab::Access::MINIMAL_ACCESS => _("Minimal Access"), Gitlab::Access::GUEST => _('Guest'), + Gitlab::Access::PLANNER => _('Planner'), Gitlab::Access::REPORTER => _('Reporter'), Gitlab::Access::DEVELOPER => _('Developer'), Gitlab::Access::MAINTAINER => _('Maintainer'), diff --git a/app/models/group.rb b/app/models/group.rb index b17c85dbe5cf2a..fd9db4013bcb8d 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -553,6 +553,10 @@ def add_guest(user, current_user = nil) add_member(user, :guest, current_user: current_user) end + def add_planner(user, current_user = nil) + add_member(user, :planner, current_user: current_user) + end + def add_reporter(user, current_user = nil) add_member(user, :reporter, current_user: current_user) end diff --git a/app/models/issue.rb b/app/models/issue.rb index e4fa7164b62cea..ed653f50c30456 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -795,7 +795,7 @@ def project_level_readable_by?(user) elsif project.personal? && project.team.owner?(user) true elsif confidential? && !assignee_or_author?(user) - project.member?(user, Gitlab::Access::REPORTER) + project.member?(user, Gitlab::Access::PLANNER) elsif project.public? || (project.internal? && !user.external?) project.feature_available?(:issues, user) else @@ -808,7 +808,7 @@ def group_level_readable_by?(user) return false unless namespace.is_a?(::Group) if confidential? && !assignee_or_author?(user) - namespace.member?(user, Gitlab::Access::REPORTER) + namespace.member?(user, Gitlab::Access::PLANNER) else namespace.member?(user) end diff --git a/app/models/member.rb b/app/models/member.rb index 880ace110d055d..ad554ed43d7690 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -189,6 +189,7 @@ class Member < ApplicationRecord scope :has_access, -> { active.where('access_level > 0') } scope :guests, -> { active.where(access_level: GUEST) } + scope :planners, -> { active.where(access_level: PLANNER) } scope :reporters, -> { active.where(access_level: REPORTER) } scope :developers, -> { active.where(access_level: DEVELOPER) } scope :maintainers, -> { active.where(access_level: MAINTAINER) } diff --git a/app/models/project.rb b/app/models/project.rb index f966f1dcf13aec..47bde7e7e57d82 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -538,7 +538,7 @@ def self.integration_association_name(name) with_options to: :team do delegate :members, prefix: true delegate :add_member, :add_members, :member? - delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_owner, :add_role + delegate :add_guest, :add_planner, :add_reporter, :add_developer, :add_maintainer, :add_owner, :add_role delegate :has_user? end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 5aac606207883c..2ef643e7e188df 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -11,6 +11,10 @@ def add_guest(user, current_user: nil) add_member(user, :guest, current_user: current_user) end + def add_planner(user, current_user: nil) + add_member(user, :planner, current_user: current_user) + end + def add_reporter(user, current_user: nil) add_member(user, :reporter, current_user: current_user) end @@ -89,6 +93,10 @@ def guests @guests ||= fetch_members(Gitlab::Access::GUEST) end + def planners + @planners ||= fetch_members(Gitlab::Access::PLANNER) + end + def reporters @reporters ||= fetch_members(Gitlab::Access::REPORTER) end @@ -152,6 +160,10 @@ def guest?(user) max_member_access(user.id) == Gitlab::Access::GUEST end + def planner?(user) + max_member_access(user.id) == Gitlab::Access::PLANNER + end + def reporter?(user) max_member_access(user.id) == Gitlab::Access::REPORTER end diff --git a/app/models/system/broadcast_message.rb b/app/models/system/broadcast_message.rb index 8bcaf4a1e584a9..28fd9c0422cd8c 100644 --- a/app/models/system/broadcast_message.rb +++ b/app/models/system/broadcast_message.rb @@ -7,6 +7,7 @@ class BroadcastMessage < ApplicationRecord ALLOWED_TARGET_ACCESS_LEVELS = [ Gitlab::Access::GUEST, + Gitlab::Access::PLANNER, Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER, diff --git a/app/models/user.rb b/app/models/user.rb index fef2aad235528a..b43fbc6f988f21 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1424,7 +1424,7 @@ def owned_projects # # This logic is duplicated from `Ability#project_abilities` into a SQL form. def projects_where_can_admin_issues - authorized_projects(Gitlab::Access::REPORTER).non_archived.with_issues_enabled + authorized_projects(Gitlab::Access::PLANNER).non_archived.with_issues_enabled end # rubocop: disable CodeReuse/ServiceClass diff --git a/app/policies/board_policy.rb b/app/policies/board_policy.rb index eaffffd68afc3a..b95232252e54bd 100644 --- a/app/policies/board_policy.rb +++ b/app/policies/board_policy.rb @@ -16,11 +16,11 @@ class BoardPolicy < BasePolicy enable :read_issue end - condition(:reporter_of_group_projects) do + condition(:planner_of_group_projects) do next unless @user group_projects_for(user: @user, group: @subject.resource_parent) - .visible_to_user_and_access_level(@user, ::Gitlab::Access::REPORTER) + .visible_to_user_and_access_level(@user, ::Gitlab::Access::PLANNER) .exists? end @@ -28,7 +28,7 @@ class BoardPolicy < BasePolicy enable :create_non_backlog_issues end - rule { is_group_board & reporter_of_group_projects }.policy do + rule { is_group_board & planner_of_group_projects }.policy do enable :create_non_backlog_issues end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 20915e0f42dca0..e9e9f29eb8847f 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -13,6 +13,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy condition(:has_access) { access_level != GroupMember::NO_ACCESS } condition(:guest) { access_level >= GroupMember::GUEST } + condition(:planner) { access_level == GroupMember::PLANNER } condition(:developer) { access_level >= GroupMember::DEVELOPER } condition(:owner) { access_level >= GroupMember::OWNER } condition(:maintainer) { access_level >= GroupMember::MAINTAINER } @@ -135,6 +136,21 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :award_emoji end + rule { planner }.policy do + enable :planner_access + enable :guest_access + enable :admin_label + enable :admin_milestone + enable :admin_issue_board + enable :admin_issue_board_list + enable :admin_issue + enable :update_issue + enable :destroy_issue + enable :read_confidential_issues + enable :read_crm_organization + enable :read_crm_contact + end + rule { admin | organization_owner }.policy do enable :read_group end @@ -403,7 +419,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy rule { can?(:admin_group) | can?(:admin_runner) }.enable :admin_group_or_admin_runner # Should be matched with ProjectPolicy#read_internal_note - rule { admin | reporter }.enable :read_internal_note + rule { admin | reporter | planner }.enable :read_internal_note rule { can?(:remove_group) }.enable :view_edit_page diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb index 9af84818730aff..64d242d937300e 100644 --- a/app/policies/issuable_policy.rb +++ b/app/policies/issuable_policy.rb @@ -12,6 +12,11 @@ class IssuablePolicy < BasePolicy @user && @subject.assignee_or_author?(@user) end + desc "User has planner or reporter access" + condition(:planner_or_reporter_access) do + can?(:reporter_access) || can?(:planner_access) + end + condition(:is_author) { @subject&.author == @user } condition(:is_incident) { @subject.incident_type_issue? } @@ -53,7 +58,7 @@ class IssuablePolicy < BasePolicy enable :admin_incident_management_timeline_event end - rule { can?(:reporter_access) }.policy do + rule { planner_or_reporter_access }.policy do enable :create_timelog end diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index 20150cc7c3e87c..c961d37670e426 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -13,10 +13,12 @@ def epics_license_available? false end + # rubocop:disable Cop/UserAdmin -- specifically check the admin attribute desc "User can read confidential issues" condition(:can_read_confidential) do - @user && (@user.admin? || can?(:reporter_access) || assignee_or_author?) # rubocop:disable Cop/UserAdmin + @user && (@user.admin? || planner_or_reporter_access? || assignee_or_author?) end + # rubocop:enable Cop/UserAdmin desc "Project belongs to a group, crm is enabled and user can read contacts in source group" condition(:can_read_crm_contacts, scope: :subject) do @@ -145,7 +147,7 @@ def epics_license_available? enable :set_issue_crm_contacts end - rule { can?(:reporter_access) }.policy do + rule { planner_or_reporter_access }.policy do enable :mark_note_as_internal end diff --git a/app/policies/merge_request_policy.rb b/app/policies/merge_request_policy.rb index 27bcd816a08e8f..a76770ee0549d5 100644 --- a/app/policies/merge_request_policy.rb +++ b/app/policies/merge_request_policy.rb @@ -44,7 +44,7 @@ class MergeRequestPolicy < IssuablePolicy enable :set_merge_request_metadata end - rule { can?(:reporter_access) }.policy do + rule { planner_or_reporter_access }.policy do enable :mark_note_as_internal end diff --git a/app/policies/namespaces/group_project_namespace_shared_policy.rb b/app/policies/namespaces/group_project_namespace_shared_policy.rb index 295ab59076a97a..650db3ddc0fb4b 100644 --- a/app/policies/namespaces/group_project_namespace_shared_policy.rb +++ b/app/policies/namespaces/group_project_namespace_shared_policy.rb @@ -19,6 +19,10 @@ class GroupProjectNamespaceSharedPolicy < ::NamespacePolicy enable :reopen_issue end + rule { can?(:planner_access) }.policy do + enable :reopen_issue + end + rule { can?(:guest_access) }.policy do enable :read_work_item enable :read_issue diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index e08f9d470a1bc1..f11744197621d0 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -15,6 +15,9 @@ class ProjectPolicy < BasePolicy desc "User has guest access" condition(:guest) { team_member? } + desc "User has planner access" + condition(:planner) { team_access_level == Gitlab::Access::PLANNER } + desc "User has reporter access" condition(:reporter) { team_access_level >= Gitlab::Access::REPORTER } @@ -311,6 +314,11 @@ class ProjectPolicy < BasePolicy Feature.enabled?(:hide_projects_of_banned_users) && @subject.created_and_owned_by_banned_user? end + desc "User has either planner or reporter access" + condition(:planner_or_reporter_access) do + can?(:reporter_access) || can?(:planner_access) + end + # `:read_project` may be prevented in EE, but `:read_project_for_iids` should # not. rule { guest | admin | organization_owner }.enable :read_project_for_iids @@ -320,6 +328,7 @@ class ProjectPolicy < BasePolicy rule { can?(:read_all_resources) }.enable :read_confidential_issues rule { guest }.enable :guest_access + rule { planner }.enable :planner_access rule { reporter }.enable :reporter_access rule { developer }.enable :developer_access rule { maintainer }.enable :maintainer_access @@ -329,6 +338,7 @@ class ProjectPolicy < BasePolicy rule { can?(:owner_access) }.policy do enable :guest_access + enable :planner_access enable :reporter_access enable :developer_access enable :maintainer_access @@ -379,6 +389,29 @@ class ProjectPolicy < BasePolicy enable :read_upload end + rule { can?(:planner_access) }.policy do + enable :guest_access + enable :admin_issue_board + enable :admin_issue_board_list + enable :update_issue + enable :reopen_issue + enable :admin_issue + enable :destroy_issue + enable :read_confidential_issues + enable :create_design + enable :update_design + enable :move_design + enable :destroy_design + enable :admin_label + enable :admin_milestone + enable :download_wiki_code + enable :create_wiki + enable :admin_wiki + enable :read_merge_request + enable :download_code + enable :export_work_items + end + rule { can?(:reporter_access) & can?(:create_issue) }.enable :create_incident rule { can?(:reporter_access) & can?(:read_environment) }.enable :read_freeze_period @@ -503,8 +536,8 @@ class ProjectPolicy < BasePolicy rule { owner | admin | organization_owner | guest | group_member | group_requester }.prevent :request_access rule { ~request_access_enabled }.prevent :request_access - rule { can?(:developer_access) & can?(:create_issue) }.enable :import_issues - rule { can?(:reporter_access) & can?(:create_work_item) }.enable :import_work_items + rule { (can?(:planner_access) | can?(:developer_access)) & can?(:create_issue) }.enable :import_issues + rule { planner_or_reporter_access & can?(:create_work_item) }.enable :import_work_items rule { can?(:developer_access) }.policy do enable :create_package @@ -747,6 +780,7 @@ class ProjectPolicy < BasePolicy # If this project is public or internal we want to prevent all aside from a few public policies rule { public_or_internal & ~project_allowed_for_job_token_by_scope }.policy do prevent :guest_access + prevent :planner_access prevent :public_access prevent :reporter_access prevent :developer_access @@ -854,6 +888,7 @@ class ProjectPolicy < BasePolicy # `:read_project_for_iids` is not prevented by this condition, as it is # used for cross-project reference checks. prevent :guest_access + prevent :planner_access prevent :public_access prevent :public_user_access prevent :reporter_access @@ -1013,7 +1048,7 @@ class ProjectPolicy < BasePolicy end # Should be matched with GroupPolicy#read_internal_note - rule { admin | can?(:reporter_access) }.enable :read_internal_note + rule { admin | planner_or_reporter_access }.enable :read_internal_note rule { can?(:developer_access) & namespace_catalog_available }.policy do enable :read_namespace_catalog @@ -1117,7 +1152,7 @@ def team_access_level return -1 if @user.nil? return -1 unless user_is_user? - lookup_access_level! + @team_access_level ||= lookup_access_level! end def lookup_access_level! diff --git a/app/services/groups/open_issues_count_service.rb b/app/services/groups/open_issues_count_service.rb index 6ab2fda0893ff1..b5e24f98e8c14b 100644 --- a/app/services/groups/open_issues_count_service.rb +++ b/app/services/groups/open_issues_count_service.rb @@ -37,12 +37,12 @@ def cache_key_name end def public_only? - !user_is_at_least_reporter? + !user_is_at_least_planner? end - def user_is_at_least_reporter? - strong_memoize(:user_is_at_least_reporter) do - group.member?(user, Gitlab::Access::REPORTER) + def user_is_at_least_planner? + strong_memoize(:user_is_at_least_planner) do + group.member?(user, Gitlab::Access::PLANNER) end end diff --git a/app/services/projects/open_issues_count_service.rb b/app/services/projects/open_issues_count_service.rb index 55ffbbbb8c9b86..09e2f992e1b9e8 100644 --- a/app/services/projects/open_issues_count_service.rb +++ b/app/services/projects/open_issues_count_service.rb @@ -21,12 +21,12 @@ def cache_key_name end def public_only? - !user_is_at_least_reporter? + !user_is_at_least_planner? end - def user_is_at_least_reporter? - strong_memoize(:user_is_at_least_reporter) do - @project.member?(@user, Gitlab::Access::REPORTER) + def user_is_at_least_planner? + strong_memoize(:user_is_at_least_planner) do + @project.member?(@user, Gitlab::Access::PLANNER) end end @@ -57,7 +57,7 @@ def refresh_cache(&block) end end - # We only show issues count including confidential for reporters, who are allowed to view confidential issues. + # We only show issues count including confidential for planners, who are allowed to view confidential issues. # This will still show a discrepancy on issues number but should be less than before. # Check https://gitlab.com/gitlab-org/gitlab-foss/issues/38418 description. diff --git a/app/services/todos/destroy/confidential_issue_service.rb b/app/services/todos/destroy/confidential_issue_service.rb index 331c4a12681998..42f9ce66e77c07 100644 --- a/app/services/todos/destroy/confidential_issue_service.rb +++ b/app/services/todos/destroy/confidential_issue_service.rb @@ -3,7 +3,7 @@ module Todos module Destroy # Service class for deleting todos that belongs to confidential issues. - # It deletes todos for users that are not at least reporters, issue author or assignee. + # It deletes todos for users that are not at least planners, issue author or assignee. # # Accepts issue_id or project_id as argument. # When issue_id is passed it deletes matching todos for one confidential issue. diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index 387c5ce063a89f..f0056756765156 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -19,8 +19,8 @@ def initialize(user_id, entity_id, entity_type) def execute return unless entity && user - # if at least reporter, all entities including confidential issues can be accessed - return if user_has_reporter_access? + # if at least planner, all entities including confidential issues can be accessed + return if user_has_planner_access? remove_confidential_resource_todos remove_group_todos @@ -52,7 +52,7 @@ def remove_confidential_resource_todos Todo .for_type(Issue.name) .for_internal_notes - .for_project(non_authorized_reporter_projects) # Only Reporter+ can read internal notes + .for_project(non_authorized_planner_projects) # Only Planner+ can read internal notes .for_user(user) .delete_all end @@ -65,9 +65,9 @@ def remove_project_todos .for_user(user) .delete_all - # MRs require reporter access, so remove those todos that are not authorized + # MRs require planner access, so remove those todos that are not authorized Todo - .for_project(non_authorized_reporter_projects) + .for_project(non_authorized_planner_projects) .for_type(MergeRequest.name) .for_user(user) .delete_all @@ -87,30 +87,30 @@ def projects when Project { id: entity.id } when Namespace - { namespace_id: non_authorized_reporter_groups } + { namespace_id: non_authorized_planner_groups } end Project.where(condition) # rubocop: disable CodeReuse/ActiveRecord end - def authorized_reporter_projects - user.authorized_projects(Gitlab::Access::REPORTER).select(:id) + def authorized_planner_projects + user.authorized_projects(Gitlab::Access::PLANNER).select(:id) end def authorized_guest_projects user.authorized_projects(Gitlab::Access::GUEST).select(:id) end - def non_authorized_reporter_projects - projects.id_not_in(authorized_reporter_projects) + def non_authorized_planner_projects + projects.id_not_in(authorized_planner_projects) end def non_authorized_guest_projects projects.id_not_in(authorized_guest_projects) end - def authorized_reporter_groups - GroupsFinder.new(user, min_access_level: Gitlab::Access::REPORTER).execute.select(:id) + def authorized_planner_groups + GroupsFinder.new(user, min_access_level: Gitlab::Access::PLANNER).execute.select(:id) end # rubocop: disable CodeReuse/ActiveRecord @@ -124,15 +124,15 @@ def unauthorized_private_groups end # rubocop: enable CodeReuse/ActiveRecord - def non_authorized_reporter_groups + def non_authorized_planner_groups entity.self_and_descendants.select(:id) - .id_not_in(authorized_reporter_groups) + .id_not_in(authorized_planner_groups) end - def user_has_reporter_access? + def user_has_planner_access? return unless entity.is_a?(Namespace) - entity.member?(User.find(user.id), Gitlab::Access::REPORTER) + entity.member?(User.find(user.id), Gitlab::Access::PLANNER) end def confidential_issues @@ -141,7 +141,7 @@ def confidential_issues Issue .in_projects(projects) .confidential_only - .not_in_projects(authorized_reporter_projects) + .not_in_projects(authorized_planner_projects) .not_authored_by(user) .id_not_in(assigned_ids) end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 746186d38f6dbb..6943af3107bac5 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -38873,6 +38873,7 @@ Access level of a group or project member. | `MAINTAINER` | The Maintainer role is primarily used for managing code reviews, approvals, and administrative settings for projects. This role can also manage project memberships. | | `MINIMAL_ACCESS` | The Minimal Access role is for users who need the least amount of access into groups and projects. You can assign this role as a default, before giving a user another role with more permissions. | | `OWNER` | The Owner role is normally assigned to the individual or team responsible for managing and maintaining the group or creating the project. This role has the highest level of administrative control, and can manage all aspects of the group or project, including managing other Owners. | +| `PLANNER` | The Planner role is for users who need access to product management and planning tools. | | `REPORTER` | The Reporter role is suitable for team members who need to stay informed about a project or group but do not actively contribute code. | ### `MemberAccessLevelName` @@ -38885,6 +38886,7 @@ Name of access levels of a group or project member. | `GUEST` | Guest access. | | `MAINTAINER` | Maintainer access. | | `OWNER` | Owner access. | +| `PLANNER` | Planner access. | | `REPORTER` | Reporter access. | ### `MemberApprovalStatusType` diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index ba93b3097a65eb..d26376020c76d0 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -394,7 +394,7 @@ def filter_groups_user_can(groups:, user:, action:) access_level = if action == :read_confidential_epic - ::Gitlab::Access::REPORTER + ::Gitlab::Access::PLANNER else ::Gitlab::Access::GUEST end diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index a5e5f29b945562..74f58d4ec158c9 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -298,6 +298,12 @@ module GroupPolicy enable :list_subgroup_epics end + rule { planner }.policy do + enable :create_wiki + enable :admin_wiki + enable :download_wiki_code + end + rule { reporter }.policy do enable :admin_issue_board_list enable :view_productivity_analytics @@ -430,7 +436,7 @@ module GroupPolicy enable :read_iteration_cadence end - rule { reporter & iterations_available }.policy do + rule { (reporter | planner) & iterations_available }.policy do enable :create_iteration enable :admin_iteration enable :create_iteration_cadence @@ -441,7 +447,7 @@ module GroupPolicy enable :rollover_issues end - rule { reporter & epics_available }.policy do + rule { (reporter | planner) & epics_available }.policy do enable :create_epic enable :admin_epic enable :update_epic @@ -450,7 +456,7 @@ module GroupPolicy enable :admin_epic_board_list end - rule { owner & epics_available }.enable :destroy_epic + rule { (owner | planner) & epics_available }.enable :destroy_epic rule { can?(:read_group) & custom_fields_available }.policy do enable :read_custom_field diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 6ded3d027d350b..3030d7b2d694d4 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -379,7 +379,7 @@ module ProjectPolicy enable :read_project_security_exclusions end - rule { can?(:reporter_access) & iterations_available }.policy do + rule { planner_or_reporter_access & iterations_available }.policy do enable :create_iteration enable :admin_iteration end @@ -747,7 +747,7 @@ module ProjectPolicy rule { can?(:read_project) & requirements_available }.enable :read_requirement - rule { requirements_available & (reporter | admin) }.policy do + rule { requirements_available & (planner | reporter | admin) }.policy do enable :create_requirement enable :create_requirement_test_report enable :admin_requirement @@ -758,7 +758,9 @@ module ProjectPolicy rule { requirements_available & (owner | admin) }.enable :destroy_requirement - rule { quality_management_available & can?(:reporter_access) & can?(:create_issue) }.enable :create_test_case + rule { quality_management_available & planner_or_reporter_access & can?(:create_issue) }.policy do + enable :create_test_case + end rule { compliance_framework_available & can?(:owner_access) }.enable :admin_compliance_framework diff --git a/ee/app/policies/epic_policy.rb b/ee/app/policies/epic_policy.rb index 84e1be0a986ca7..e36a43fed6afae 100644 --- a/ee/app/policies/epic_policy.rb +++ b/ee/app/policies/epic_policy.rb @@ -7,6 +7,11 @@ class EpicPolicy < BasePolicy condition(:is_group_member) { @subject.group.member?(@user) } + desc "User has planner or reporter access" + condition(:planner_or_reporter_access) do + can?(:reporter_access) || can?(:planner_access) + end + condition(:service_desk_enabled) do @subject.group.has_project_with_service_desk_enabled? end @@ -72,7 +77,7 @@ class EpicPolicy < BasePolicy rule { can?(:owner_access) | can?(:maintainer_access) }.enable :admin_note desc 'User cannot read confidential epics' - rule { confidential & ~can?(:reporter_access) }.policy do + rule { confidential & ~planner_or_reporter_access }.policy do prevent :create_epic prevent :update_epic prevent :admin_epic @@ -112,7 +117,7 @@ class EpicPolicy < BasePolicy # Special case to not prevent support bot # assigning issues to confidential epics using quick actions # when group has projects with service desk enabled. - rule { confidential & ~can?(:reporter_access) & ~(support_bot & service_desk_enabled) }.policy do + rule { confidential & ~planner_or_reporter_access & ~(support_bot & service_desk_enabled) }.policy do prevent :read_epic prevent :read_epic_iid end @@ -126,7 +131,7 @@ class EpicPolicy < BasePolicy enable :set_confidentiality end - rule { can?(:reporter_access) }.policy do + rule { planner_or_reporter_access }.policy do enable :mark_note_as_internal end diff --git a/ee/app/services/ee/todos/destroy/entity_leave_service.rb b/ee/app/services/ee/todos/destroy/entity_leave_service.rb index 1c7f7b2511a301..30f917c4c92409 100644 --- a/ee/app/services/ee/todos/destroy/entity_leave_service.rb +++ b/ee/app/services/ee/todos/destroy/entity_leave_service.rb @@ -24,7 +24,7 @@ def remove_confidential_resource_todos ::Todo .for_type(::Epic.name) .for_internal_notes - .for_group(non_authorized_reporter_groups) # Only reporter+ can read internal notes + .for_group(non_authorized_planner_groups) # Only planner+ can read internal notes .for_user(user) .delete_all end @@ -33,7 +33,7 @@ def remove_confidential_resource_todos def confidential_epics ::Epic - .in_selected_groups(non_authorized_reporter_groups) + .in_selected_groups(non_authorized_planner_groups) .confidential end end diff --git a/ee/bin/custom-ability b/ee/bin/custom-ability index 189c028a101b9d..1b301cf7d05b19 100755 --- a/ee/bin/custom-ability +++ b/ee/bin/custom-ability @@ -21,6 +21,7 @@ require_relative '../../lib/gitlab/popen' LEVELS = { 'Not applicable' => 0, 'Guest' => 10, + 'Planner' => 15, 'Reporter' => 20, 'Developer' => 30, 'Maintainer' => 40, diff --git a/ee/config/custom_abilities/read_code.yml b/ee/config/custom_abilities/read_code.yml index 222cc53e9501d4..78a7d39e0b6415 100644 --- a/ee/config/custom_abilities/read_code.yml +++ b/ee/config/custom_abilities/read_code.yml @@ -12,4 +12,4 @@ skip_seat_consumption: true feature_flag: customizable_roles feature_flag_enabled_milestone: '15.9' feature_flag_enabled_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110810 -available_from_access_level: 20 +available_from_access_level: 15 diff --git a/ee/config/custom_abilities/read_crm_contact.yml b/ee/config/custom_abilities/read_crm_contact.yml index 48f4a7fcbc47ba..db3d23db0f0dfd 100644 --- a/ee/config/custom_abilities/read_crm_contact.yml +++ b/ee/config/custom_abilities/read_crm_contact.yml @@ -9,4 +9,4 @@ milestone: '17.1' group_ability: true project_ability: false requirements: [] -available_from_access_level: 20 +available_from_access_level: 15 diff --git a/ee/lib/elastic/latest/epic_class_proxy.rb b/ee/lib/elastic/latest/epic_class_proxy.rb index f10e8feb3b51da..54476afbe57f0f 100644 --- a/ee/lib/elastic/latest/epic_class_proxy.rb +++ b/ee/lib/elastic/latest/epic_class_proxy.rb @@ -80,7 +80,7 @@ def group end def group_ids_user_can_read_epics(confidential: false) - min_access_level = confidential ? ::Gitlab::Access::REPORTER : ::Gitlab::Access::GUEST + min_access_level = confidential ? ::Gitlab::Access::PLANNER : ::Gitlab::Access::GUEST finder_params = { min_access_level: min_access_level } if options[:search_level] == 'group' finder_params[:filter_group_ids] = group.self_and_descendants.pluck_primary_key diff --git a/ee/spec/frontend/members/components/table/drawer/role_details_drawer_spec.js b/ee/spec/frontend/members/components/table/drawer/role_details_drawer_spec.js index 2c845ab60bb692..58aaad5c67e473 100644 --- a/ee/spec/frontend/members/components/table/drawer/role_details_drawer_spec.js +++ b/ee/spec/frontend/members/components/table/drawer/role_details_drawer_spec.js @@ -16,7 +16,9 @@ import { describe('Role details drawer', () => { const { permissions } = updateableCustomRoleMember.customRoles[1]; const dropdownItems = roleDropdownItems(updateableCustomRoleMember); - const currentRole = dropdownItems.flatten[6]; + const currentRole = dropdownItems.flatten.find( + (role) => role.memberRoleId === updateableCustomRoleMember.accessLevel.memberRoleId, + ); let wrapper; const createWrapper = ({ member = updateableCustomRoleMember } = {}) => { diff --git a/ee/spec/frontend/members/components/table/drawer/utils_spec.js b/ee/spec/frontend/members/components/table/drawer/utils_spec.js index c2d7e94ae585ec..14e6fb1b616171 100644 --- a/ee/spec/frontend/members/components/table/drawer/utils_spec.js +++ b/ee/spec/frontend/members/components/table/drawer/utils_spec.js @@ -68,7 +68,10 @@ describe('Role details drawer utils', () => { const mockAxios = new MockAdapter(axios); mockAxios.onPut(memberPath).replyOnce(HTTP_STATUS_OK); - const customRole = roleDropdownItems(upgradedMember).flatten[6]; + const customRole = roleDropdownItems(upgradedMember).flatten.find( + (role) => role.memberRoleId === upgradedMember.accessLevel.memberRoleId, + ); + await callRoleUpdateApi(member, customRole); expect(mockAxios.history.put).toHaveLength(1); diff --git a/ee/spec/frontend/members/components/table/max_role_spec.js b/ee/spec/frontend/members/components/table/max_role_spec.js index a685d0944b7673..dc517af7f9dfb7 100644 --- a/ee/spec/frontend/members/components/table/max_role_spec.js +++ b/ee/spec/frontend/members/components/table/max_role_spec.js @@ -182,9 +182,11 @@ describe('MaxRole', () => { createComponent(); expect(findListbox().props('items')[0].text).toBe('Default roles'); - expect(findListbox().props('items')[0].options).toHaveLength(6); + expect(findListbox().props('items')[0].options).toHaveLength( + Object.keys(member.validRoles).length, + ); expect(findListbox().props('items')[1].text).toBe('Custom roles'); - expect(findListbox().props('items')[1].options).toHaveLength(2); + expect(findListbox().props('items')[1].options).toHaveLength(member.customRoles.length); }); it('calls `updateMemberRole` Vuex action', async () => { diff --git a/ee/spec/graphql/resolvers/members/standard_roles_resolver_spec.rb b/ee/spec/graphql/resolvers/members/standard_roles_resolver_spec.rb index 99b4f4adb08dd9..87038caa31974a 100644 --- a/ee/spec/graphql/resolvers/members/standard_roles_resolver_spec.rb +++ b/ee/spec/graphql/resolvers/members/standard_roles_resolver_spec.rb @@ -24,7 +24,7 @@ it 'returns the totals for each standard role' do expect(result).to be_present - expect(result.count).to eq(6) + expect(result.count).to eq(7) roles_with_members = [::Gitlab::Access::MAINTAINER, ::Gitlab::Access::DEVELOPER] diff --git a/ee/spec/lib/gitlab_subscriptions/billable_users_utils_spec.rb b/ee/spec/lib/gitlab_subscriptions/billable_users_utils_spec.rb index 2a47417f318378..30fc7a04cd6fdd 100644 --- a/ee/spec/lib/gitlab_subscriptions/billable_users_utils_spec.rb +++ b/ee/spec/lib/gitlab_subscriptions/billable_users_utils_spec.rb @@ -5,7 +5,7 @@ RSpec.describe GitlabSubscriptions::BillableUsersUtils, feature_category: :consumables_cost_management do let_it_be(:dummy_class) { Class.new { include GitlabSubscriptions::BillableUsersUtils }.new } - billable_roles = [Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, + billable_roles = [Gitlab::Access::PLANNER, Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER, Gitlab::Access::OWNER, Gitlab::Access::ADMIN].freeze sm_non_billable_roles = [Gitlab::Access::NO_ACCESS].freeze diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index ef6065579af5f2..27ff488a8e532b 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -571,9 +571,9 @@ end end - context 'when user is reporter' do + context 'when user is planner' do before do - private_subgroup_1.add_reporter(user) + private_subgroup_1.add_planner(user) end it_behaves_like 'a filter for permissioned groups' do @@ -581,11 +581,11 @@ end end - context 'when user is reporter via shared group' do - let(:shared_group_access) { GroupMember::REPORTER } + context 'when user is planner via shared group' do + let(:shared_group_access) { GroupMember::PLANNER } before do - shared_with_group.add_reporter(user) + shared_with_group.add_planner(user) end it_behaves_like 'a filter for permissioned groups' do @@ -3216,6 +3216,7 @@ def webhook_headers { "Minimal Access" => 5, "Guest" => 10, + "Planner" => 15, "Reporter" => 20, "Developer" => 30, "Maintainer" => 40, diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb index cb12bafb84ae87..68dbf8a3babe3b 100644 --- a/ee/spec/models/members/member_role_spec.rb +++ b/ee/spec/models/members/member_role_spec.rb @@ -375,7 +375,7 @@ describe '.levels_sentence' do it 'returns the list of access levels with names' do expect(described_class.levels_sentence).to eq( - "10 (Guest), 20 (Reporter), 30 (Developer), 40 (Maintainer), and 50 (Owner)" + "10 (Guest), 15 (Planner), 20 (Reporter), 30 (Developer), 40 (Maintainer), and 50 (Owner)" ) end end diff --git a/ee/spec/policies/epic_policy_spec.rb b/ee/spec/policies/epic_policy_spec.rb index 8809da21b296b1..6769c89c41c045 100644 --- a/ee/spec/policies/epic_policy_spec.rb +++ b/ee/spec/policies/epic_policy_spec.rb @@ -125,6 +125,21 @@ it_behaves_like 'cannot resolve epic discussions' end + context 'planner group member' do + before do + group.add_planner(user) + end + + it_behaves_like 'can manage epics' + it_behaves_like 'can comment on epics' + it_behaves_like 'cannot edit epic comments' + it_behaves_like 'cannot resolve epic discussions' + + it 'can destroy epics' do + is_expected.to be_allowed(:destroy_epic) + end + end + context 'reporter group member' do before do group.add_reporter(user) @@ -306,6 +321,14 @@ it_behaves_like 'all epic permissions disabled' end + context 'when user is planner' do + before do + group.add_planner(user) + end + + it_behaves_like 'all reporter epic permissions enabled' + end + context 'when user is reporter' do before do group.add_reporter(user) @@ -391,6 +414,15 @@ it_behaves_like 'cannot edit epic comments' end + context 'when user is planner' do + before do + group.add_planner(user) + end + + it_behaves_like 'can comment on epics' + it_behaves_like 'cannot edit epic comments' + end + context 'user who is reporter' do before do group.add_reporter(user) diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 3581ef5502de74..72b78e20a495e1 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -86,6 +86,12 @@ def stub_group_saml_config(enabled) it { is_expected.to be_disallowed(:destroy_epic) } end + context 'when user is planner' do + let(:current_user) { planner } + + it { is_expected.to be_allowed(*epic_rules) } + end + context 'when user is guest' do let(:current_user) { guest } @@ -142,30 +148,31 @@ def stub_group_saml_config(enabled) end context 'when iterations feature is enabled' do + let(:read_actions) { [:read_iteration, :read_iteration_cadence] } + let(:edit_actions) { [:create_iteration, :admin_iteration, :create_iteration_cadence, :admin_iteration_cadence] } + before do stub_licensed_features(iterations: true) end - context 'when user is a developer' do - let(:current_user) { developer } - - it { is_expected.to be_allowed(:read_iteration, :create_iteration, :admin_iteration, :read_iteration_cadence, :create_iteration_cadence, :admin_iteration_cadence) } - end - - context 'when user is a guest' do - let(:current_user) { guest } - - it { is_expected.to be_allowed(:read_iteration, :read_iteration_cadence) } - it { is_expected.to be_disallowed(:create_iteration, :admin_iteration, :create_iteration_cadence, :admin_iteration_cadence) } + where(:role, :actions, :allowed) do + :none | ref(:read_actions) | false + :none | ref(:edit_actions) | false + :guest | ref(:read_actions) | true + :guest | ref(:edit_actions) | false + :planner | ref(:read_actions) | true + :planner | ref(:edit_actions) | true + :reporter | ref(:read_actions) | true + :reporter | ref(:edit_actions) | true end - context 'when user is logged out' do - let(:current_user) { nil } + with_them do + let(:current_user) { try(role) } - it { is_expected.to be_disallowed(:read_iteration, :create_iteration, :admin_iteration, :create_iteration_cadence) } + it { is_expected.to(allowed ? be_allowed(*actions) : be_disallowed(*actions)) } end - context 'when project is private' do + context 'when project is public' do let(:group) { create(:group, :public, :owner_subgroup_creation_only) } context 'when user is logged out' do @@ -368,6 +375,7 @@ def stub_group_saml_config(enabled) describe ':read_product_analytics', :enable_admin_mode do where(:role, :allowed) do :guest | false + :planner | false :reporter | true :developer | true :admin | true @@ -1436,6 +1444,13 @@ def stub_group_saml_config(enabled) it { is_expected.to be_disallowed(:admin_ldap_group_links) } end + context 'planners' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:override_group_member) } + it { is_expected.to be_disallowed(:admin_ldap_group_links) } + end + context 'reporter' do let(:current_user) { reporter } @@ -1668,6 +1683,7 @@ def stub_group_saml_config(enabled) :admin_vulnerability | :auditor | nil | false :admin_vulnerability | :developer | nil | false :admin_vulnerability | :guest | nil | false + :admin_vulnerability | :planner | nil | false :admin_vulnerability | :maintainer | nil | true :admin_vulnerability | :owner | nil | true :admin_vulnerability | :reporter | nil | false @@ -1676,6 +1692,7 @@ def stub_group_saml_config(enabled) :read_dependency | :auditor | nil | true :read_dependency | :developer | nil | true :read_dependency | :guest | nil | false + :read_dependency | :planner | nil | false :read_dependency | :maintainer | nil | true :read_dependency | :owner | nil | true :read_dependency | :reporter | nil | false @@ -1684,6 +1701,7 @@ def stub_group_saml_config(enabled) :read_group_security_dashboard | :auditor | nil | true :read_group_security_dashboard | :developer | nil | true :read_group_security_dashboard | :guest | nil | false + :read_group_security_dashboard | :planner | nil | false :read_group_security_dashboard | :maintainer | nil | true :read_group_security_dashboard | :owner | nil | true :read_group_security_dashboard | :reporter | nil | false @@ -1692,6 +1710,7 @@ def stub_group_saml_config(enabled) :read_licenses | :auditor | nil | true :read_licenses | :developer | nil | true :read_licenses | :guest | nil | false + :read_licenses | :planner | nil | false :read_licenses | :maintainer | nil | true :read_licenses | :owner | nil | true :read_licenses | :reporter | nil | false @@ -1700,6 +1719,7 @@ def stub_group_saml_config(enabled) :read_vulnerability | :auditor | nil | true :read_vulnerability | :developer | nil | true :read_vulnerability | :guest | nil | false + :read_vulnerability | :planner | nil | false :read_vulnerability | :maintainer | nil | true :read_vulnerability | :owner | nil | true :read_vulnerability | :reporter | nil | false @@ -1862,6 +1882,12 @@ def stub_group_saml_config(enabled) it { is_expected.to be_disallowed(*abilities) } end + context 'with planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(*abilities) } + end + context 'with guest' do let(:current_user) { guest } @@ -2121,6 +2147,14 @@ def stub_group_saml_config(enabled) is_expected.to be_disallowed(action) end end + + context 'planner' do + let(:current_user) { planner } + + it 'is not allowed' do + is_expected.to be_disallowed(action) + end + end end describe 'view_productivity_analytics' do @@ -2280,6 +2314,7 @@ def stub_group_saml_config(enabled) where(:role, :admin_mode, :allowed) do :guest | nil | false + :planner | nil | false :reporter | nil | false :developer | nil | false :maintainer | nil | false @@ -2304,6 +2339,7 @@ def stub_group_saml_config(enabled) where(:role, :admin_mode, :allowed) do :guest | nil | false + :planner | nil | false :reporter | nil | false :developer | nil | true :maintainer | nil | true @@ -2508,6 +2544,8 @@ def set_access_level(access_level) where(:role, :licensed, :admin_mode, :root_group, :allowed) do :guest | true | nil | true | false :guest | false | nil | true | false + :planner | true | nil | true | false + :planner | false | nil | true | false :reporter | true | nil | true | false :reporter | false | nil | true | false :developer | true | nil | true | false @@ -2543,6 +2581,7 @@ def set_access_level(access_level) where(:role, :allowed) do :guest | false + :planner | false :reporter | false :developer | false :maintainer | false @@ -2619,6 +2658,7 @@ def set_access_level(access_level) where(:role, :allowed) do :guest | true + :planner | true :reporter | true :developer | true :maintainer | true @@ -2669,6 +2709,8 @@ def set_access_level(access_level) where(:role, :eligible_for_trial, :admin_mode, :allowed) do :guest | true | nil | false :guest | false | nil | false + :planner | true | nil | false + :planner | false | nil | false :reporter | true | nil | false :reporter | false | nil | false :developer | true | nil | false @@ -2706,6 +2748,7 @@ def set_access_level(access_level) :maintainer | true | nil | false :developer | true | nil | false :reporter | true | nil | false + :planner | true | nil | false :guest | true | nil | false end @@ -2759,6 +2802,7 @@ def set_access_level(access_level) :maintainer | true :developer | true :reporter | true + :planner | false :guest | false :non_group_member | false :auditor | true @@ -2799,6 +2843,7 @@ def set_access_level(access_level) :maintainer | true :developer | true :reporter | true + :planner | false :guest | false :non_group_member | false end @@ -2822,6 +2867,7 @@ def set_access_level(access_level) :maintainer | false :developer | false :reporter | false + :planner | false :guest | false :non_group_member | false end @@ -3064,6 +3110,7 @@ def expect_private_group_permissions_as_if_non_member :maintainer | nil | false :developer | nil | false :reporter | nil | false + :planner | nil | false :guest | nil | false end @@ -4051,6 +4098,12 @@ def create_member_role(member, abilities = member_role_abilities) it { is_expected.to be_allowed(:read_saved_replies, :create_saved_replies, :update_saved_replies, :destroy_saved_replies) } end + context 'when the user is a planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:read_saved_replies, :create_saved_replies, :update_saved_replies, :destroy_saved_replies) } + end + context 'when the user is a guest member of the group' do let(:current_user) { guest } @@ -4264,33 +4317,41 @@ def create_member_role(member, abilities = member_role_abilities) describe 'bulk_admin_epic' do context 'when bulk_edit_feature_available is true' do - let(:current_user) { reporter } - before do stub_licensed_features(epics: true, group_bulk_edit: true) end - it { is_expected.to be_allowed(:bulk_admin_epic) } + context 'when user is planner or reporter' do + where(role: %w[planner reporter]) + + with_them do + let(:current_user) { public_send(role) } + + it { is_expected.to be_allowed(:bulk_admin_epic) } + end + end + + context 'when user is not reporter or better' do + let(:current_user) { guest } + + it { is_expected.to be_disallowed(:bulk_admin_epic) } + end end context 'when bulk_edit_feature_available is false' do - let(:current_user) { reporter } - before do stub_licensed_features(epics: true, group_bulk_edit: false) end - it { is_expected.to be_disallowed(:bulk_admin_epic) } - end + context 'when user is guest, planner or reporter' do + where(role: %w[guest planner reporter]) - context 'when user is not reporter or better' do - let(:current_user) { guest } + with_them do + let(:current_user) { public_send(role) } - before do - stub_licensed_features(epics: true, group_bulk_edit: true) + it { is_expected.to be_disallowed(:bulk_admin_epic) } + end end - - it { is_expected.to be_disallowed(:bulk_admin_epic) } end end end diff --git a/ee/spec/policies/issuable_policy_spec.rb b/ee/spec/policies/issuable_policy_spec.rb index 7b79ddafa051b8..36437b4482f4c0 100644 --- a/ee/spec/policies/issuable_policy_spec.rb +++ b/ee/spec/policies/issuable_policy_spec.rb @@ -5,15 +5,18 @@ RSpec.describe IssuablePolicy, :models do let_it_be(:non_member) { create(:user) } let_it_be(:guest) { create(:user) } + let_it_be(:planner) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:developer) { create(:user) } let(:guest_issue) { create(:issue, project: project, author: guest) } + let(:planner_issue) { create(:issue, project: project, author: planner) } let(:reporter_issue) { create(:issue, project: project, author: reporter) } let(:incident_issue) { create(:incident, project: project, author: developer) } before do project.add_guest(guest) + project.add_planner(planner) project.add_reporter(reporter) project.add_developer(developer) end @@ -34,14 +37,21 @@ def permissions(user, issue) expect(permissions(guest, incident_issue)).to be_disallowed(:read_issuable_resource_link) end + it 'disallows planners' do + expect(permissions(planner, incident_issue)).to be_disallowed(:admin_issuable_resource_link) + expect(permissions(planner, incident_issue)).to be_disallowed(:read_issuable_resource_link) + end + it 'disallows all on non-incident issue type' do expect(permissions(non_member, issue)).to be_disallowed(:admin_issuable_resource_link) expect(permissions(guest, issue)).to be_disallowed(:admin_issuable_resource_link) expect(permissions(developer, issue)).to be_disallowed(:admin_issuable_resource_link) + expect(permissions(planner, issue)).to be_disallowed(:admin_issuable_resource_link) expect(permissions(reporter, issue)).to be_disallowed(:admin_issuable_resource_link) expect(permissions(non_member, issue)).to be_disallowed(:read_issuable_resource_link) expect(permissions(guest, issue)).to be_disallowed(:read_issuable_resource_link) expect(permissions(developer, issue)).to be_disallowed(:read_issuable_resource_link) + expect(permissions(planner, issue)).to be_disallowed(:read_issuable_resource_link) expect(permissions(reporter, issue)).to be_disallowed(:read_issuable_resource_link) end end @@ -62,6 +72,11 @@ def permissions(user, issue) expect(permissions(guest, guest_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) end + it 'allows planners to create and delete metric images' do + expect(permissions(planner, issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) + expect(permissions(planner, planner_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) + end + it 'allows reporters to create and delete metric images' do expect(permissions(reporter, issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) expect(permissions(reporter, reporter_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) @@ -121,6 +136,11 @@ def permissions(user, issue) expect(permissions(guest, guest_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) end + it 'allows planners to create and delete metric images' do + expect(permissions(planner, issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) + expect(permissions(planner, planner_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) + end + it 'allows reporters to create and delete metric images' do expect(permissions(reporter, issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) expect(permissions(reporter, reporter_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :update_issuable_metric_image, :destroy_issuable_metric_image) diff --git a/ee/spec/policies/issue_policy_spec.rb b/ee/spec/policies/issue_policy_spec.rb index fa9e9d33e2ad1e..a72703a2fd7afc 100644 --- a/ee/spec/policies/issue_policy_spec.rb +++ b/ee/spec/policies/issue_policy_spec.rb @@ -5,6 +5,7 @@ RSpec.describe IssuePolicy, feature_category: :team_planning do let_it_be(:user) { create(:user) } let_it_be(:guest) { create(:user) } + let_it_be(:planner) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:owner) { create(:user) } let_it_be(:support_bot) { Users::Internal.support_bot } @@ -13,6 +14,7 @@ let_it_be(:root_group) do create(:group, :public).tap do |g| + g.add_planner(planner) g.add_reporter(reporter) g.add_owner(owner) end @@ -20,6 +22,7 @@ let_it_be(:group) do create(:group, :public, parent: root_group).tap do |g| + g.add_planner(planner) g.add_reporter(reporter) g.add_owner(owner) end @@ -109,6 +112,7 @@ def permissions(user, issue) it 'dis-allows it for members', :aggregate_failures do expect(permissions(guest, group_issue)).to be_disallowed(:reopen_issue) expect(permissions(reporter, group_issue)).to be_disallowed(:reopen_issue) + expect(permissions(planner, group_issue)).to be_disallowed(:reopen_issue) expect(permissions(owner, group_issue)).to be_disallowed(:reopen_issue) end end @@ -121,6 +125,7 @@ def permissions(user, issue) it 'allows it for members', :aggregate_failures do expect(permissions(guest, group_issue)).to be_disallowed(:reopen_issue) expect(permissions(reporter, group_issue)).to be_allowed(:reopen_issue) + expect(permissions(planner, group_issue)).to be_allowed(:reopen_issue) expect(permissions(owner, group_issue)).to be_allowed(:reopen_issue) end end @@ -245,6 +250,7 @@ def permissions(user, issue) ) # allows read permissions + expect(permissions(planner, group_issue)).to be_disallowed(:read_internal_note, :read_crm_contacts) expect(permissions(reporter, group_issue)).to be_disallowed(:read_internal_note, :read_crm_contacts) # allows some permissions that modify the issue @@ -284,6 +290,7 @@ def permissions(user, issue) ) # allows read permissions + expect(permissions(planner, group_issue)).to be_allowed(:read_internal_note, :read_crm_contacts) expect(permissions(reporter, group_issue)).to be_allowed(:read_internal_note, :read_crm_contacts) # allows some permissions that modify the issue diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index e2937f32eee6f9..99ae433c93d3d9 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -25,6 +25,7 @@ context 'basic permissions' do let(:additional_guest_permissions) { %i[read_limit_alert] } + let(:additional_planner_permissions) { [:read_software_license_policy] } let(:additional_reporter_permissions) do %i[read_software_license_policy admin_value_stream read_product_analytics] end @@ -66,6 +67,7 @@ it_behaves_like 'project policies as anonymous' it_behaves_like 'project policies as guest' + it_behaves_like 'project policies as planner' it_behaves_like 'project policies as reporter' it_behaves_like 'project policies as developer' it_behaves_like 'project policies as maintainer' @@ -76,6 +78,9 @@ context 'auditor' do let(:current_user) { auditor } let(:auditor_permission_exclusions) { [:fork_project, :create_merge_request_in] } + let(:auditor_as_guest_exclusions) do + %i[create_note read_confidential_issues create_project create_issue create_note upload_file admin_issue_link] + end before do stub_licensed_features(security_dashboard: true, license_scanning: true) @@ -87,6 +92,7 @@ is_expected.to be_disallowed(*maintainer_permissions) is_expected.to be_disallowed(*owner_permissions) is_expected.to be_disallowed(*(guest_permissions - auditor_permissions)) + is_expected.to be_disallowed(*(planner_permissions - auditor_permissions - [:read_confidential_issues])) is_expected.to be_allowed(*auditor_permission_exclusions) is_expected.to be_allowed(*auditor_permissions) end @@ -101,6 +107,7 @@ is_expected.to be_disallowed(*maintainer_permissions) is_expected.to be_disallowed(*owner_permissions) is_expected.to be_disallowed(*(guest_permissions - auditor_permissions)) + is_expected.to be_disallowed(*(planner_permissions - auditor_permissions - [:read_confidential_issues])) is_expected.to be_disallowed(*auditor_permission_exclusions) is_expected.to be_allowed(*(auditor_permissions - auditor_permission_exclusions)) end @@ -116,6 +123,7 @@ is_expected.to be_disallowed(*(developer_permissions - auditor_permissions)) is_expected.to be_disallowed(*maintainer_permissions) is_expected.to be_disallowed(*owner_permissions) + is_expected.to be_disallowed(*(planner_permissions - auditor_permissions - auditor_as_guest_exclusions)) is_expected.to be_allowed(*(guest_permissions - auditor_permissions)) is_expected.to be_allowed(*auditor_permissions) end @@ -237,6 +245,7 @@ where(:the_user, :allowed, :disallowed) do ref(:developer) | [:read_iteration, :create_iteration, :admin_iteration] | [] + ref(:planner) | [:read_iteration, :create_iteration, :admin_iteration] | [] ref(:guest) | [:read_iteration] | [:create_iteration, :admin_iteration] ref(:non_member) | [:read_iteration] | [:create_iteration, :admin_iteration] ref(:anonymous) | [:read_iteration] | [:create_iteration, :admin_iteration] @@ -752,40 +761,20 @@ end end - context 'with owner' do - let(:current_user) { owner } - - it { is_expected.to be_allowed(permission) } - end - - context 'with maintainer' do - let(:current_user) { maintainer } - - it { is_expected.to be_allowed(permission) } - end - - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_disallowed(permission) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(permission) } - end - - context 'with non member' do - let(:current_user) { non_member } + %w[owner maintainer].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(permission) } + it { is_expected.to be_allowed(permission) } + end end - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous non_member guest planner reporter].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(permission) } + it { is_expected.to be_disallowed(permission) } + end end end end @@ -971,7 +960,7 @@ end context 'with less than developer role' do - where(role: %w[reporter guest]) + where(role: %w[reporter planner guest]) with_them do let(:current_user) { public_send(role) } @@ -1102,46 +1091,20 @@ end end - context 'with owner' do - let(:current_user) { owner } - - it { is_expected.to be_allowed(:admin_software_license_policy) } - end - - context 'with maintainer' do - let(:current_user) { maintainer } - - it { is_expected.to be_allowed(:admin_software_license_policy) } - end + %w[owner maintainer].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - context 'with developer' do - let(:current_user) { developer } - - it { is_expected.to be_disallowed(:admin_software_license_policy) } - end - - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_disallowed(:admin_software_license_policy) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:admin_software_license_policy) } - end - - context 'with non member' do - let(:current_user) { non_member } - - it { is_expected.to be_disallowed(:admin_software_license_policy) } + it { is_expected.to be_allowed(:admin_software_license_policy) } + end end - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous non_member guest planner reporter developer].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:admin_software_license_policy) } + it { is_expected.to be_disallowed(:admin_software_license_policy) } + end end end @@ -1194,46 +1157,20 @@ end end - context 'with owner' do - let(:current_user) { owner } - - it { is_expected.to be_allowed(:read_dependency) } - end - - context 'with maintainer' do - let(:current_user) { maintainer } + %w[owner maintainer developer reporter planner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:read_dependency) } - end - - context 'with developer' do - let(:current_user) { developer } - - it { is_expected.to be_allowed(:read_dependency) } - end - - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_allowed(:read_dependency) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:read_dependency) } - end - - context 'with non member' do - let(:current_user) { non_member } - - it { is_expected.to be_disallowed(:read_dependency) } + it { is_expected.to be_allowed(:read_dependency) } + end end - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous non_member guest].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:read_dependency) } + it { is_expected.to be_disallowed(:read_dependency) } + end end end end @@ -1258,7 +1195,7 @@ context 'with private project' do let(:project) { private_project } - where(role: %w[owner maintainer developer reporter]) + where(role: %w[owner maintainer developer reporter planner]) with_them do let(:current_user) { public_send(role) } @@ -1278,22 +1215,12 @@ end end - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:read_licenses) } - end - - context 'with non member' do - let(:current_user) { non_member } - - it { is_expected.to be_disallowed(:read_licenses) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous non_member guest].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:read_licenses) } + it { is_expected.to be_disallowed(:read_licenses) } + end end end end @@ -1317,6 +1244,7 @@ where(:role, :admin_mode, :allowed) do :anonymous | nil | false :guest | nil | false + :planner | nil | false :reporter | nil | false :developer | nil | true :maintainer | nil | true @@ -1597,6 +1525,7 @@ describe ':read_enterprise_ai_analytics' do let(:project) { private_project_in_group } let(:guest) { inherited_guest } + let(:planner) { inherited_planner } let(:reporter) { inherited_reporter } context 'when on SAAS', :saas do @@ -1651,6 +1580,7 @@ where(:role, :admin_mode, :allowed) do :guest | nil | false + :planner | nil | true :reporter | nil | true :developer | nil | true :maintainer | nil | true @@ -1687,6 +1617,7 @@ context 'with merge request approvers rules available in license' do where(:role, :setting, :admin_mode, :allowed) do :guest | true | nil | false + :planner | true | nil | false :reporter | true | nil | false :developer | true | nil | false :maintainer | false | nil | true @@ -1715,6 +1646,7 @@ context 'with merge request approvers rules not available in license' do where(:role, :setting, :admin_mode, :allowed) do :guest | true | nil | false + :planner | true | nil | false :reporter | true | nil | false :developer | true | nil | false :maintainer | false | nil | true @@ -1745,7 +1677,8 @@ let(:project) { private_project } where(:role, :licensed, :allowed) do - :guest | true | false + :guest | true | false + :planner | true | false :reporter | true | false :developer | true | false :maintainer | false | false @@ -1798,6 +1731,7 @@ where(:role, :admin_mode, :allowed) do :guest | nil | false + :planner | nil | true :reporter | nil | true :developer | nil | true :maintainer | nil | true @@ -1829,16 +1763,19 @@ shared_examples_for 'prevents CI cancellation ability' do context 'when feature is enabled' do where(:restricted_role, :actual_role, :allowed) do + :developer | :planner | false :developer | :guest | false :developer | :reporter | false :developer | :developer | true :developer | :maintainer | true :developer | :owner | true + :maintainer | :planner | false :maintainer | :guest | false :maintainer | :reporter | false :maintainer | :developer | false :maintainer | :maintainer | true :maintainer | :owner | true + :no_one | :planner | false :no_one | :guest | false :no_one | :reporter | false :no_one | :developer | false @@ -1948,6 +1885,8 @@ where(:role, :feature_enabled, :admin_mode, :allowed) do :guest | false | nil | false :guest | true | nil | false + :planner | false | nil | false + :planner | true | nil | false :reporter | false | nil | false :reporter | true | nil | false :developer | false | nil | false @@ -1987,6 +1926,7 @@ where(:role, :admin_mode, :allowed) do :guest | nil | false + :planner | nil | false :reporter | nil | true :developer | nil | true :maintainer | nil | true @@ -2016,6 +1956,7 @@ where(:role, :admin_mode, :allowed) do :guest | nil | false + :planner | nil | false :reporter | nil | false :developer | nil | false :maintainer | nil | true @@ -2055,6 +1996,7 @@ where(:role, :admin_mode, :allowed) do :guest | nil | false + :planner | nil | false :reporter | nil | true :developer | nil | true :maintainer | nil | true @@ -2084,6 +2026,7 @@ where(:role, :admin_mode, :allowed) do :guest | nil | false + :planner | nil | false :reporter | nil | false :developer | nil | false :maintainer | nil | true @@ -2319,38 +2262,24 @@ context 'when analytics is disabled for the project' do let(:project) { project_with_analytics_disabled } - context 'for guest user' do - let(:current_user) { guest } + %w[guest planner developer admin auditor].each do |role| + context "for #{role} user" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(*all_read_analytics_permissions) } - end - - context 'for developer' do - let(:current_user) { developer } - - it { is_expected.to be_disallowed(*all_read_analytics_permissions) } - end - - context 'for admin', :enable_admin_mode do - let(:current_user) { admin } - - it { is_expected.to be_disallowed(*all_read_analytics_permissions) } - end - - context 'for auditor' do - let(:current_user) { auditor } - - it { is_expected.to be_disallowed(*all_read_analytics_permissions) } + it { is_expected.to be_disallowed(*all_read_analytics_permissions) } + end end end context 'when analytics is private for the project' do let(:project) { project_with_analytics_private } - context 'for guest user' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "for #{role} user" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(*all_read_analytics_permissions) } + it { is_expected.to be_disallowed(*all_read_analytics_permissions) } + end end context 'for developer' do @@ -2375,13 +2304,15 @@ context 'when analytics is enabled for the project' do let(:project) { project_with_analytics_enabled } - context 'for guest user' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "for #{role} user" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:read_project_merge_request_analytics) } - it { is_expected.to be_disallowed(:read_code_review_analytics) } - it { is_expected.to be_disallowed(:read_cycle_analytics) } - it { is_expected.to be_allowed(:read_issue_analytics) } + it { is_expected.to be_disallowed(:read_project_merge_request_analytics) } + it { is_expected.to be_disallowed(:read_code_review_analytics) } + it { is_expected.to be_disallowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_issue_analytics) } + end end context 'for developer' do @@ -2410,18 +2341,21 @@ where(:role, :project_visibility, :allowed) do :guest | 'public' | true + :planner | 'public' | true :reporter | 'public' | true :developer | 'public' | true :maintainer | 'public' | true :owner | 'public' | true :admin | 'public' | true :guest | 'internal' | true + :planner | 'internal' | true :reporter | 'internal' | true :developer | 'internal' | true :maintainer | 'internal' | true :owner | 'internal' | true :admin | 'internal' | true :guest | 'private' | false + :planner | 'private' | false :reporter | 'private' | true :developer | 'private' | true :maintainer | 'private' | true @@ -2532,6 +2466,7 @@ def expect_private_project_permissions_as_if_non_member where(:role, :allowed) do :guest | false + :planner | false :reporter | false :developer | false :maintainer | true @@ -2696,6 +2631,7 @@ def expect_private_project_permissions_as_if_non_member where(:role, :allowed) do :guest | true + :planner | true :reporter | true :developer | true :maintainer | true @@ -2742,6 +2678,7 @@ def expect_private_project_permissions_as_if_non_member where(:role, :allowed) do :guest | true + :planner | true :reporter | true :developer | true :maintainer | true @@ -3854,6 +3791,12 @@ def create_member_role(member, abilities = member_role_abilities) it { is_expected.to be_disallowed(:admin_vulnerability) } end + context "with planner" do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:admin_vulnerability) } + end + context "with reporter" do let(:current_user) { reporter } @@ -3914,18 +3857,22 @@ def create_member_role(member, abilities = member_role_abilities) where(:feature_flag_enabled, :licensed_feature, :current_user, :allowed) do true | true | ref(:owner) | true true | true | ref(:reporter) | true + true | true | ref(:planner) | true true | true | ref(:guest) | true true | true | ref(:non_member) | false true | false | ref(:owner) | false true | false | ref(:reporter) | false + true | false | ref(:planner) | false true | false | ref(:guest) | false true | false | ref(:non_member) | false false | true | ref(:owner) | false false | true | ref(:reporter) | false + false | true | ref(:planner) | false false | true | ref(:guest) | false false | true | ref(:non_member) | false false | false | ref(:owner) | false false | false | ref(:reporter) | false + false | false | ref(:planner) | false false | false | ref(:guest) | false false | false | ref(:non_member) | false end @@ -3947,18 +3894,22 @@ def create_member_role(member, abilities = member_role_abilities) where(:feature_flag_enabled, :licensed_feature, :current_user, :allowed) do true | true | ref(:owner) | true true | true | ref(:reporter) | true + true | true | ref(:planner) | false true | true | ref(:guest) | false true | true | ref(:non_member) | false true | false | ref(:owner) | false true | false | ref(:reporter) | false + true | false | ref(:planner) | false true | false | ref(:guest) | false true | false | ref(:non_member) | false false | true | ref(:owner) | false false | true | ref(:reporter) | false + false | true | ref(:planner) | false false | true | ref(:guest) | false false | true | ref(:non_member) | false false | false | ref(:owner) | false false | false | ref(:reporter) | false + false | false | ref(:planner) | false false | false | ref(:guest) | false false | false | ref(:non_member) | false end @@ -4219,10 +4170,12 @@ def create_member_role(member, abilities = member_role_abilities) where(:saas_feature_enabled, :current_user, :match_expected_result) do true | ref(:owner) | be_allowed(:read_google_cloud_artifact_registry) true | ref(:reporter) | be_allowed(:read_google_cloud_artifact_registry) + true | ref(:planner) | be_disallowed(:read_google_cloud_artifact_registry) true | ref(:guest) | be_disallowed(:read_google_cloud_artifact_registry) true | ref(:non_member) | be_disallowed(:read_google_cloud_artifact_registry) false | ref(:owner) | be_disallowed(:read_google_cloud_artifact_registry) false | ref(:reporter) | be_disallowed(:read_google_cloud_artifact_registry) + false | ref(:planner) | be_disallowed(:read_google_cloud_artifact_registry) false | ref(:guest) | be_disallowed(:read_google_cloud_artifact_registry) false | ref(:non_member) | be_disallowed(:read_google_cloud_artifact_registry) end @@ -4337,26 +4290,31 @@ def create_member_role(member, abilities = member_role_abilities) true | true | true | ref(:owner) | be_allowed(:duo_workflow) true | true | true | ref(:maintainer) | be_allowed(:duo_workflow) true | true | true | ref(:developer) | be_allowed(:duo_workflow) + true | true | true | ref(:planner) | be_disallowed(:duo_workflow) true | true | true | ref(:guest) | be_disallowed(:duo_workflow) true | true | true | ref(:non_member) | be_disallowed(:duo_workflow) true | false | true | ref(:owner) | be_disallowed(:duo_workflow) true | false | true | ref(:maintainer) | be_disallowed(:duo_workflow) true | false | true | ref(:developer) | be_disallowed(:duo_workflow) + true | false | true | ref(:planner) | be_disallowed(:duo_workflow) true | false | true | ref(:guest) | be_disallowed(:duo_workflow) true | false | true | ref(:non_member) | be_disallowed(:duo_workflow) false | true | true | ref(:owner) | be_disallowed(:duo_workflow) false | true | true | ref(:maintainer) | be_disallowed(:duo_workflow) false | true | true | ref(:developer) | be_disallowed(:duo_workflow) + false | true | true | ref(:planner) | be_disallowed(:duo_workflow) false | true | true | ref(:guest) | be_disallowed(:duo_workflow) false | true | true | ref(:non_member) | be_disallowed(:duo_workflow) false | false | true | ref(:owner) | be_disallowed(:duo_workflow) false | false | true | ref(:maintainer) | be_disallowed(:duo_workflow) false | false | true | ref(:developer) | be_disallowed(:duo_workflow) + false | false | true | ref(:planner) | be_disallowed(:duo_workflow) false | false | true | ref(:guest) | be_disallowed(:duo_workflow) false | false | true | ref(:non_member) | be_disallowed(:duo_workflow) false | false | false | ref(:owner) | be_disallowed(:duo_workflow) false | false | false | ref(:maintainer) | be_disallowed(:duo_workflow) false | false | false | ref(:developer) | be_disallowed(:duo_workflow) + false | false | false | ref(:planner) | be_disallowed(:duo_workflow) false | false | false | ref(:guest) | be_disallowed(:duo_workflow) false | false | false | ref(:non_member) | be_disallowed(:duo_workflow) end @@ -4411,6 +4369,7 @@ def create_member_role(member, abilities = member_role_abilities) ref(:owner) | be_allowed(:read_pre_receive_secret_detection_info) ref(:maintainer) | be_allowed(:read_pre_receive_secret_detection_info) ref(:developer) | be_allowed(:read_pre_receive_secret_detection_info) + ref(:planner) | be_disallowed(:read_pre_receive_secret_detection_info) ref(:guest) | be_disallowed(:read_pre_receive_secret_detection_info) ref(:non_member) | be_disallowed(:read_pre_receive_secret_detection_info) end @@ -4442,6 +4401,7 @@ def create_member_role(member, abilities = member_role_abilities) where(:role, :allowed) do :guest | false + :planner | false :reporter | false :developer | false :maintainer | true @@ -4466,6 +4426,7 @@ def create_member_role(member, abilities = member_role_abilities) where(:role, :allowed) do :guest | false + :planner | false :reporter | false :developer | true :maintainer | true @@ -4504,12 +4465,14 @@ def create_member_role(member, abilities = member_role_abilities) true | ref(:maintainer) | true true | ref(:developer) | true true | ref(:guest) | false + true | ref(:planner) | false true | ref(:reporter) | false true | ref(:non_member) | false false | ref(:owner) | false false | ref(:maintainer) | false false | ref(:developer) | false false | ref(:guest) | false + false | ref(:planner) | false false | ref(:reporter) | false false | ref(:non_member) | false end diff --git a/ee/spec/policies/requirements_management/requirement_policy_spec.rb b/ee/spec/policies/requirements_management/requirement_policy_spec.rb index 621091ccab6ff0..c5b2df8ec6f831 100644 --- a/ee/spec/policies/requirements_management/requirement_policy_spec.rb +++ b/ee/spec/policies/requirements_management/requirement_policy_spec.rb @@ -5,12 +5,13 @@ RSpec.describe RequirementsManagement::RequirementPolicy do let_it_be(:owner) { create(:user) } let_it_be(:admin) { create(:admin) } + let_it_be(:planner) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:developer) { create(:user) } let_it_be(:maintainer) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:project) do - create(:project, :public, namespace: owner.namespace, reporters: reporter, developers: developer, + create(:project, :public, namespace: owner.namespace, planners: planner, reporters: reporter, developers: developer, maintainers: maintainer, guests: guest) end diff --git a/ee/spec/policies/work_item_policy_spec.rb b/ee/spec/policies/work_item_policy_spec.rb index 342655639a028c..795299b7659745 100644 --- a/ee/spec/policies/work_item_policy_spec.rb +++ b/ee/spec/policies/work_item_policy_spec.rb @@ -4,11 +4,13 @@ RSpec.describe WorkItemPolicy, feature_category: :team_planning do let_it_be(:guest) { create(:user) } + let_it_be(:planner) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:owner) { create(:user) } let_it_be(:group) do create(:group, :public).tap do |g| g.add_guest(guest) + g.add_planner(planner) g.add_reporter(reporter) g.add_owner(owner) end @@ -34,6 +36,7 @@ def permissions(user, work_item) # allows read permissions expect(permissions(reporter, work_item)).to be_allowed(:read_internal_note, :read_crm_contacts, :reopen_issue) + expect(permissions(planner, work_item)).to be_allowed(:read_internal_note, :read_crm_contacts, :reopen_issue) # allows some permissions that modify the issue expect(permissions(owner, work_item)).to be_allowed( diff --git a/ee/spec/requests/api/graphql/group/standard_roles_spec.rb b/ee/spec/requests/api/graphql/group/standard_roles_spec.rb index d68d59477f2c6b..9217a721a48576 100644 --- a/ee/spec/requests/api/graphql/group/standard_roles_spec.rb +++ b/ee/spec/requests/api/graphql/group/standard_roles_spec.rb @@ -61,6 +61,7 @@ def run_query(query) [ [::Gitlab::Access::MINIMAL_ACCESS, 0], [::Gitlab::Access::GUEST, 0], + [::Gitlab::Access::PLANNER, 0], [::Gitlab::Access::REPORTER, 0], [::Gitlab::Access::DEVELOPER, 2], [::Gitlab::Access::MAINTAINER, 2], @@ -90,6 +91,7 @@ def run_query(query) [ [::Gitlab::Access::MINIMAL_ACCESS, 0], [::Gitlab::Access::GUEST, 0], + [::Gitlab::Access::PLANNER, 0], [::Gitlab::Access::REPORTER, 0], [::Gitlab::Access::DEVELOPER, 2], [::Gitlab::Access::MAINTAINER, 2], @@ -122,6 +124,7 @@ def run_query(query) [ [::Gitlab::Access::MINIMAL_ACCESS, 0], [::Gitlab::Access::GUEST, 0], + [::Gitlab::Access::PLANNER, 0], [::Gitlab::Access::REPORTER, 0], [::Gitlab::Access::DEVELOPER, 2 + 3], [::Gitlab::Access::MAINTAINER, 2 + 3], diff --git a/ee/spec/requests/api/graphql/members/group_standard_roles_spec.rb b/ee/spec/requests/api/graphql/members/group_standard_roles_spec.rb index 6a260823aa68b0..3655203885e236 100644 --- a/ee/spec/requests/api/graphql/members/group_standard_roles_spec.rb +++ b/ee/spec/requests/api/graphql/members/group_standard_roles_spec.rb @@ -26,12 +26,14 @@ def standard_roles_query let_it_be(:user) { create(:user) } let_it_be(:group_1) { create(:group) } let_it_be(:subgroup) { create(:group, parent: group_1) } + let_it_be(:subgroup_2) { create(:group, parent: group_1) } let_it_be(:project) { create(:project, group: group_1) } let_it_be(:group_2) { create(:group) } let_it_be(:member_1) { create(:group_member, :guest, group: group_1, user: user) } let_it_be(:member_g2) { create(:group_member, :developer, group: group_2, user: user) } let_it_be(:member_2) { create(:group_member, :maintainer, group: subgroup, user: user) } let_it_be(:member_3) { create(:project_member, :guest, project: project, user: user) } + let_it_be(:member_4) { create(:group_member, :planner, group: subgroup_2, user: user) } subject(:roles) do graphql_data.dig('group', 'standardRoles', 'nodes') @@ -59,6 +61,13 @@ def standard_roles_query 'usersCount' => 1, 'detailsPath' => '/admin/application_settings/roles_and_permissions/GUEST' }, + { + 'accessLevel' => 15, + 'name' => 'Planner', + 'membersCount' => 1, + 'usersCount' => 1, + 'detailsPath' => '/admin/application_settings/roles_and_permissions/PLANNER' + }, { 'accessLevel' => 20, 'name' => 'Reporter', diff --git a/ee/spec/requests/api/graphql/members/instance_standard_roles_spec.rb b/ee/spec/requests/api/graphql/members/instance_standard_roles_spec.rb index 5013ccb1bd6cf9..785b3ac7134a67 100644 --- a/ee/spec/requests/api/graphql/members/instance_standard_roles_spec.rb +++ b/ee/spec/requests/api/graphql/members/instance_standard_roles_spec.rb @@ -25,6 +25,7 @@ def standard_roles_query let_it_be(:member_1) { create(:group_member, :guest, user: user) } let_it_be(:member_2) { create(:group_member, :maintainer, user: user) } let_it_be(:member_3) { create(:project_member, :guest, user: user) } + let_it_be(:member_4) { create(:group_member, :planner, user: user) } subject(:roles) do graphql_data.dig('standardRoles', 'nodes') @@ -59,6 +60,13 @@ def standard_roles_query 'usersCount' => 1, 'detailsPath' => '/admin/application_settings/roles_and_permissions/GUEST' }, + { + 'accessLevel' => 15, + 'name' => 'Planner', + 'membersCount' => 1, + 'usersCount' => 1, + 'detailsPath' => '/admin/application_settings/roles_and_permissions/PLANNER' + }, { 'accessLevel' => 20, 'name' => 'Reporter', diff --git a/ee/spec/services/ee/clusters/agents/authorize_proxy_user_service_spec.rb b/ee/spec/services/ee/clusters/agents/authorize_proxy_user_service_spec.rb index 78eadff996c36f..3c37e53f29fd76 100644 --- a/ee/spec/services/ee/clusters/agents/authorize_proxy_user_service_spec.rb +++ b/ee/spec/services/ee/clusters/agents/authorize_proxy_user_service_spec.rb @@ -43,8 +43,8 @@ expect(service_response).to be_success expect(service_response.payload[:access_as]).to eq({ user: { - projects: [{ id: deployment_project.id, roles: %i[guest reporter developer] }], - groups: [{ id: deployment_group.id, roles: %i[guest reporter developer maintainer] }] + projects: [{ id: deployment_project.id, roles: %i[guest planner reporter developer] }], + groups: [{ id: deployment_group.id, roles: %i[guest planner reporter developer maintainer] }] } }) end diff --git a/ee/spec/services/ee/members/approve_access_request_service_spec.rb b/ee/spec/services/ee/members/approve_access_request_service_spec.rb index 8c3f94ba388411..b9e3698ff94e37 100644 --- a/ee/spec/services/ee/members/approve_access_request_service_spec.rb +++ b/ee/spec/services/ee/members/approve_access_request_service_spec.rb @@ -128,6 +128,21 @@ end end + context 'for planner member role' do + let(:current_role) { Gitlab::Access::PLANNER } + let(:higher_role) { Gitlab::Access::REPORTER } + + it_behaves_like 'updating members using custom permission' + + context 'with the default (developer) role of the requester' do + let(:params) { {} } + + it 'raises an error' do + expect { approve_access_request }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end + end + context 'for reporter member role' do let(:current_role) { Gitlab::Access::REPORTER } let(:higher_role) { Gitlab::Access::DEVELOPER } diff --git a/ee/spec/services/ee/todos/destroy/entity_leave_service_spec.rb b/ee/spec/services/ee/todos/destroy/entity_leave_service_spec.rb index 7e0e6fed14671f..5cb2df4b30d606 100644 --- a/ee/spec/services/ee/todos/destroy/entity_leave_service_spec.rb +++ b/ee/spec/services/ee/todos/destroy/entity_leave_service_spec.rb @@ -31,7 +31,7 @@ context 'when user is still member of ancestor group' do before do - group.add_reporter(user) + group.add_planner(user) end it 'does not remove todos targeting confidential epics in the group' do diff --git a/ee/spec/support/shared_examples/policies/requirement_policy_shared_examples.rb b/ee/spec/support/shared_examples/policies/requirement_policy_shared_examples.rb index c1878e7bfaabef..9f84a972fd0082 100644 --- a/ee/spec/support/shared_examples/policies/requirement_policy_shared_examples.rb +++ b/ee/spec/support/shared_examples/policies/requirement_policy_shared_examples.rb @@ -59,6 +59,12 @@ it_behaves_like 'user with manage permissions' end + context 'with planner' do + let(:current_user) { planner } + + it_behaves_like 'user with manage permissions' + end + context 'with guest' do let(:current_user) { guest } diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index 7ac49209a29a2b..a921e512a52947 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -12,6 +12,7 @@ module Access NO_ACCESS = 0 MINIMAL_ACCESS = 5 GUEST = 10 + PLANNER = 15 REPORTER = 20 DEVELOPER = 30 MAINTAINER = 40 @@ -45,6 +46,7 @@ def all_values def options { "Guest" => GUEST, + "Planner" => PLANNER, "Reporter" => REPORTER, "Developer" => DEVELOPER, "Maintainer" => MAINTAINER @@ -67,6 +69,7 @@ def option_descriptions { NO_ACCESS => s_('MemberRole|The None role is assigned to the invited group users of a shared project when project sharing is disabled in group setting.'), GUEST => s_('MemberRole|The Guest role is for users who need visibility into a project or group but should not have the ability to make changes, such as external stakeholders.'), + PLANNER => s_('MemberRole|The Planner role is for users who need access to product management and planning tools.'), REPORTER => s_('MemberRole|The Reporter role is suitable for team members who need to stay informed about a project or group but do not actively contribute code.'), DEVELOPER => s_('MemberRole|The Developer role gives users access to contribute code while restricting sensitive administrative actions.'), MAINTAINER => s_('MemberRole|The Maintainer role is primarily used for managing code reviews, approvals, and administrative settings for projects. This role can also manage project memberships.'), @@ -77,6 +80,7 @@ def option_descriptions def sym_options { guest: GUEST, + planner: PLANNER, reporter: REPORTER, developer: DEVELOPER, maintainer: MAINTAINER diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 35170536b069ed..54f70e0d5d9d31 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -33719,6 +33719,12 @@ msgstr "" msgid "MemberRole|The Owner role is normally assigned to the individual or team responsible for managing and maintaining the group or creating the project. This role has the highest level of administrative control, and can manage all aspects of the group or project, including managing other Owners." msgstr "" +msgid "MemberRole|The Planner role is for users who need access to product management and planning tools." +msgstr "" + +msgid "MemberRole|The Planner role is for users who need to manage projects." +msgstr "" + msgid "MemberRole|The Reporter role is suitable for team members who need to stay informed about a project or group but do not actively contribute code." msgstr "" @@ -41258,6 +41264,9 @@ msgstr "" msgid "Plan:" msgstr "" +msgid "Planner" +msgstr "" + msgid "PlantUML" msgstr "" diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index 8b8a058201b147..979e7ea9b32788 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -11,6 +11,7 @@ transient do # rubocop:disable Lint/EmptyBlock -- block is required by factorybot guests {} + planners {} reporters {} developers {} maintainers {} @@ -29,6 +30,7 @@ create(:namespace_settings, namespace: group) unless group.namespace_settings group.add_members(Array.wrap(evaluator.guests), :guest) + group.add_members(Array.wrap(evaluator.planners), :planner) group.add_members(Array.wrap(evaluator.reporters), :reporter) group.add_members(Array.wrap(evaluator.developers), :developer) group.add_members(Array.wrap(evaluator.maintainers), :maintainer) diff --git a/spec/factories/member.rb b/spec/factories/member.rb index a27a48770d460c..cc8add9fbe7190 100644 --- a/spec/factories/member.rb +++ b/spec/factories/member.rb @@ -8,6 +8,7 @@ user trait(:guest) { access_level { Gitlab::Access::GUEST } } + trait(:planner) { access_level { Gitlab::Access::PLANNER } } trait(:reporter) { access_level { Gitlab::Access::REPORTER } } trait(:developer) { access_level { Gitlab::Access::DEVELOPER } } trait(:maintainer) { access_level { Gitlab::Access::MAINTAINER } } diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index ae3a83ca8dc1b3..c5a8656519bca8 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -76,6 +76,7 @@ # rubocop:disable Lint/EmptyBlock -- block is required by factorybot guests {} + planners {} reporters {} developers {} maintainers {} @@ -165,6 +166,7 @@ project.create_ci_project_mirror!(namespace_id: project.namespace_id) unless project.ci_project_mirror project.add_members(Array.wrap(evaluator.guests), :guest) + project.add_members(Array.wrap(evaluator.planners), :planner) project.add_members(Array.wrap(evaluator.reporters), :reporter) project.add_members(Array.wrap(evaluator.developers), :developer) project.add_members(Array.wrap(evaluator.maintainers), :maintainer) diff --git a/spec/factories/user_highest_roles.rb b/spec/factories/user_highest_roles.rb index ee5794b55fb14a..04e3e344b4c4e0 100644 --- a/spec/factories/user_highest_roles.rb +++ b/spec/factories/user_highest_roles.rb @@ -6,6 +6,7 @@ user trait(:guest) { highest_access_level { GroupMember::GUEST } } + trait(:planner) { highest_access_level { GroupMember::PLANNER } } trait(:reporter) { highest_access_level { GroupMember::REPORTER } } trait(:developer) { highest_access_level { GroupMember::DEVELOPER } } trait(:maintainer) { highest_access_level { GroupMember::MAINTAINER } } diff --git a/spec/factories/users.rb b/spec/factories/users.rb index ac53454ddc4d6a..1e67cd1487c1e3 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -207,6 +207,7 @@ transient do # rubocop:disable Lint/EmptyBlock -- block is required by factorybot guest_of {} + planner_of {} reporter_of {} developer_of {} maintainer_of {} @@ -216,6 +217,7 @@ after(:create) do |user, evaluator| Array.wrap(evaluator.guest_of).each { |target| target.add_guest(user) } + Array.wrap(evaluator.planner_of).each { |target| target.add_planner(user) } Array.wrap(evaluator.reporter_of).each { |target| target.add_reporter(user) } Array.wrap(evaluator.developer_of).each { |target| target.add_developer(user) } Array.wrap(evaluator.maintainer_of).each { |target| target.add_maintainer(user) } diff --git a/spec/frontend/members/components/table/drawer/role_details_drawer_spec.js b/spec/frontend/members/components/table/drawer/role_details_drawer_spec.js index d81bc13260172e..9cd31b944ad108 100644 --- a/spec/frontend/members/components/table/drawer/role_details_drawer_spec.js +++ b/spec/frontend/members/components/table/drawer/role_details_drawer_spec.js @@ -16,7 +16,9 @@ jest.mock('~/lib/utils/dom_utils', () => ({ describe('Role details drawer', () => { const dropdownItems = roleDropdownItems(updateableMember); - const currentRole = dropdownItems.flatten[5]; + const currentRole = dropdownItems.flatten.find( + (role) => role.accessLevel === updateableMember.accessLevel.integerValue, + ); const newRole = dropdownItems.flatten[2]; const saveRoleStub = jest.fn(); let wrapper; diff --git a/spec/frontend/members/mock_data.js b/spec/frontend/members/mock_data.js index 75a766fcd31272..841db09e21e543 100644 --- a/spec/frontend/members/mock_data.js +++ b/spec/frontend/members/mock_data.js @@ -48,6 +48,7 @@ export const member = { validRoles: { 'Minimal Access': 5, Guest: 10, + Planner: 15, Reporter: 20, Developer: 30, Maintainer: 40, diff --git a/spec/graphql/resolvers/merge_requests_resolver_spec.rb b/spec/graphql/resolvers/merge_requests_resolver_spec.rb index 014d5067d1b287..9e9704ddcd8261 100644 --- a/spec/graphql/resolvers/merge_requests_resolver_spec.rb +++ b/spec/graphql/resolvers/merge_requests_resolver_spec.rb @@ -96,8 +96,9 @@ end it 'can batch-resolve merge requests from different projects', :request_store do - # 2 queries for project_authorizations, and 2 for merge_requests - results = batch_sync(max_queries: queries_per_project * 2) do + # 2 queries for organization_users, 2 for project_authorizations, and 2 for merge_requests + extra_auth_queries = 2 + results = batch_sync(max_queries: (queries_per_project + extra_auth_queries) * 2) do a = resolve_mr(project, iids: [iid_1]) b = resolve_mr(project, iids: [iid_2]) c = resolve_mr(other_project, iids: [other_iid]) diff --git a/spec/graphql/types/current_user_todos_type_spec.rb b/spec/graphql/types/current_user_todos_type_spec.rb index 2b33a705ae211c..84bc173bf5b2c8 100644 --- a/spec/graphql/types/current_user_todos_type_spec.rb +++ b/spec/graphql/types/current_user_todos_type_spec.rb @@ -165,11 +165,11 @@ expect do execute_query(query_type, graphql: query_without_state_arguments) - end.not_to exceed_query_limit(control) # at present this is 3 + end.not_to exceed_query_limit(control).with_threshold(1) # at present this is 4 expect do execute_query(query_type, graphql: with_state_arguments) - end.not_to exceed_query_limit(control).with_threshold(1) + end.not_to exceed_query_limit(control).with_threshold(2) end it 'returns correct data' do diff --git a/spec/graphql/types/member_access_level_enum_spec.rb b/spec/graphql/types/member_access_level_enum_spec.rb index cb079f848e0154..03bb4fb343b165 100644 --- a/spec/graphql/types/member_access_level_enum_spec.rb +++ b/spec/graphql/types/member_access_level_enum_spec.rb @@ -6,6 +6,6 @@ specify { expect(described_class.graphql_name).to eq('MemberAccessLevel') } it 'exposes all the existing access levels' do - expect(described_class.values.keys).to include(*%w[GUEST REPORTER DEVELOPER MAINTAINER OWNER]) + expect(described_class.values.keys).to include(*%w[GUEST PLANNER REPORTER DEVELOPER MAINTAINER OWNER]) end end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 386b5916a60038..4648b71a00edfc 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -667,6 +667,7 @@ expect(subject).to eq( { 'Guest' => 10, + 'Planner' => 15, 'Reporter' => 20, 'Developer' => 30 } @@ -683,6 +684,7 @@ expect(subject).to eq( { 'Guest' => 10, + 'Planner' => 15, 'Reporter' => 20, 'Developer' => 30, 'Maintainer' => 40, @@ -714,6 +716,7 @@ expect(helper.access_level_roles_user_can_assign(grand_parent, group.access_level_roles)).to be_empty expect(helper.access_level_roles_user_can_assign(parent, group.access_level_roles)).to eq({ 'Guest' => ::Gitlab::Access::GUEST, + 'Planner' => ::Gitlab::Access::PLANNER, 'Reporter' => ::Gitlab::Access::REPORTER, 'Developer' => ::Gitlab::Access::DEVELOPER }) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d1aa8f41e99b99..3341123a6ee192 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -3197,6 +3197,7 @@ def define_cache_expectations(cache_key) expect(group.access_level_roles).to eq( { 'Guest' => 10, + 'Planner' => 15, 'Reporter' => 20, 'Developer' => 30, 'Maintainer' => 40, diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 70f843be0e1789..891d349b6c465a 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -113,6 +113,7 @@ it 'returns Gitlab::Access.options' do expect(access_levels).to eq({ "Guest" => 10, + "Planner" => 15, "Reporter" => 20, "Developer" => 30 }) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 180df9cc7be870..b05cc68d5d9a34 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1215,7 +1215,7 @@ describe 'delegation' do let_it_be(:project) { create(:project) } - [:add_guest, :add_reporter, :add_developer, :add_maintainer, :add_member, :add_members].each do |method| + [:add_guest, :add_planner, :add_reporter, :add_developer, :add_maintainer, :add_member, :add_members].each do |method| it { is_expected.to delegate_method(method).to(:team) } end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 728b42abe53e78..d7bf81869c1f30 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -7,6 +7,7 @@ let(:maintainer) { create(:user) } let(:reporter) { create(:user) } + let(:planner) { create(:user) } let(:guest) { create(:user) } let(:nonmember) { create(:user) } @@ -15,6 +16,7 @@ before do project.add_maintainer(maintainer) + project.add_planner(planner) project.add_reporter(reporter) project.add_guest(guest) end @@ -22,6 +24,7 @@ describe 'members collection' do it { expect(project.team.maintainers).to include(maintainer) } it { expect(project.team.maintainers).not_to include(guest) } + it { expect(project.team.maintainers).not_to include(planner) } it { expect(project.team.maintainers).not_to include(reporter) } it { expect(project.team.maintainers).not_to include(nonmember) } end @@ -29,10 +32,12 @@ describe 'access methods' do it { expect(project.team.maintainer?(maintainer)).to be_truthy } it { expect(project.team.maintainer?(guest)).to be_falsey } + it { expect(project.team.maintainer?(planner)).to be_falsey } it { expect(project.team.maintainer?(reporter)).to be_falsey } it { expect(project.team.maintainer?(nonmember)).to be_falsey } it { expect(project.team.member?(nonmember)).to be_falsey } it { expect(project.team.member?(guest)).to be_truthy } + it { expect(project.team.member?(planner, Gitlab::Access::PLANNER)).to be_truthy } it { expect(project.team.member?(reporter, Gitlab::Access::REPORTER)).to be_truthy } it { expect(project.team.member?(guest, Gitlab::Access::REPORTER)).to be_falsey } it { expect(project.team.member?(nonmember, Gitlab::Access::GUEST)).to be_falsey } @@ -46,6 +51,7 @@ before do group.add_maintainer(maintainer) group.add_reporter(reporter) + group.add_planner(planner) group.add_guest(guest) # If user is a group and a project member - GitLab uses highest permission @@ -57,14 +63,17 @@ describe 'members collection' do it { expect(project.team.reporters).to include(reporter) } + it { expect(project.team.planners).to include(planner) } it { expect(project.team.maintainers).to include(maintainer) } it { expect(project.team.maintainers).to include(guest) } it { expect(project.team.maintainers).not_to include(reporter) } + it { expect(project.team.maintainers).not_to include(planner) } it { expect(project.team.maintainers).not_to include(nonmember) } end describe 'access methods' do it { expect(project.team.reporter?(reporter)).to be_truthy } + it { expect(project.team.planner?(planner)).to be_truthy } it { expect(project.team.maintainer?(maintainer)).to be_truthy } it { expect(project.team.maintainer?(guest)).to be_truthy } it { expect(project.team.maintainer?(reporter)).to be_falsey } @@ -244,12 +253,14 @@ before do project.add_maintainer(maintainer) project.add_reporter(reporter) + project.add_planner(planner) project.add_guest(guest) project.request_access(requester) end it { expect(project.team.find_member(maintainer.id)).to be_a(ProjectMember) } it { expect(project.team.find_member(reporter.id)).to be_a(ProjectMember) } + it { expect(project.team.find_member(planner.id)).to be_a(ProjectMember) } it { expect(project.team.find_member(guest.id)).to be_a(ProjectMember) } it { expect(project.team.find_member(nonmember.id)).to be_nil } it { expect(project.team.find_member(requester.id)).to be_nil } @@ -263,12 +274,14 @@ before do group.add_maintainer(maintainer) group.add_reporter(reporter) + group.add_planner(planner) group.add_guest(guest) group.request_access(requester) end it { expect(project.team.find_member(maintainer.id)).to be_a(GroupMember) } it { expect(project.team.find_member(reporter.id)).to be_a(GroupMember) } + it { expect(project.team.find_member(planner.id)).to be_a(GroupMember) } it { expect(project.team.find_member(guest.id)).to be_a(GroupMember) } it { expect(project.team.find_member(nonmember.id)).to be_nil } it { expect(project.team.find_member(requester.id)).to be_nil } @@ -411,11 +424,13 @@ before do project.add_maintainer(maintainer) project.add_reporter(reporter) + project.add_planner(planner) project.add_guest(guest) end it { expect(project.team.contributor?(maintainer.id)).to be false } it { expect(project.team.contributor?(reporter.id)).to be false } + it { expect(project.team.contributor?(planner.id)).to be false } it { expect(project.team.contributor?(guest.id)).to be false } end @@ -445,12 +460,14 @@ before do project.add_maintainer(maintainer) project.add_reporter(reporter) + project.add_planner(planner) project.add_guest(guest) project.request_access(requester) end it { expect(project.team.max_member_access(maintainer.id)).to eq(Gitlab::Access::MAINTAINER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } + it { expect(project.team.max_member_access(planner.id)).to eq(Gitlab::Access::PLANNER) } it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } @@ -465,10 +482,12 @@ group.add_maintainer(maintainer) group.add_reporter(reporter) + group.add_planner(planner) end it { expect(project.team.max_member_access(maintainer.id)).to eq(Gitlab::Access::DEVELOPER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } + it { expect(project.team.max_member_access(planner.id)).to eq(Gitlab::Access::PLANNER) } it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } @@ -479,6 +498,7 @@ it { expect(project.team.max_member_access(maintainer.id)).to eq(Gitlab::Access::NO_ACCESS) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(planner.id)).to eq(Gitlab::Access::NO_ACCESS) } end end end @@ -492,12 +512,14 @@ before do group.add_maintainer(maintainer) group.add_reporter(reporter) + group.add_planner(planner) group.add_guest(guest) group.request_access(requester) end it { expect(project.team.max_member_access(maintainer.id)).to eq(Gitlab::Access::MAINTAINER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } + it { expect(project.team.max_member_access(planner.id)).to eq(Gitlab::Access::PLANNER) } it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } @@ -644,6 +666,7 @@ def contributors(users) let(:maintainer) { create(:user) } let(:reporter) { create(:user) } + let(:planner) { create(:user) } let(:guest) { create(:user) } let(:promoted_guest) { create(:user) } @@ -655,13 +678,17 @@ def contributors(users) let(:second_user_without_access) { create(:user) } let(:users) do - [maintainer, reporter, promoted_guest, guest, group_developer, second_developer, user_without_access].map(&:id) + [ + maintainer, reporter, planner, promoted_guest, guest, + group_developer, second_developer, user_without_access + ].map(&:id) end let(:expected) do { maintainer.id => Gitlab::Access::MAINTAINER, reporter.id => Gitlab::Access::REPORTER, + planner.id => Gitlab::Access::PLANNER, promoted_guest.id => Gitlab::Access::DEVELOPER, guest.id => Gitlab::Access::GUEST, group_developer.id => Gitlab::Access::DEVELOPER, @@ -673,6 +700,7 @@ def contributors(users) before do project.add_maintainer(maintainer) project.add_reporter(reporter) + project.add_planner(planner) project.add_guest(promoted_guest) project.add_guest(guest) diff --git a/spec/models/user_highest_role_spec.rb b/spec/models/user_highest_role_spec.rb index cf86a996ba5816..56a7e22871ab71 100644 --- a/spec/models/user_highest_role_spec.rb +++ b/spec/models/user_highest_role_spec.rb @@ -31,6 +31,7 @@ let(:expected_allowed_values) do [ Gitlab::Access::GUEST, + Gitlab::Access::PLANNER, Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER, diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d3b8dec0c3acd4..e18bd8ab0334ef 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3973,6 +3973,7 @@ def login_method(login) end [ + Gitlab::Access::PLANNER, Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER, @@ -5297,18 +5298,22 @@ def login_method(login) describe '#projects_where_can_admin_issues' do let(:user) { create(:user) } - it 'includes projects for which the user access level is above or equal to reporter' do + it 'includes projects for which the user access level is above or equal to planner' do + planner_project = create(:project) { |p| p.add_planner(user) } reporter_project = create(:project) { |p| p.add_reporter(user) } developer_project = create(:project) { |p| p.add_developer(user) } maintainer_project = create(:project) { |p| p.add_maintainer(user) } - expect(user.projects_where_can_admin_issues.to_a).to match_array([maintainer_project, developer_project, reporter_project]) + expect(user.projects_where_can_admin_issues.to_a).to match_array( + [maintainer_project, developer_project, reporter_project, planner_project] + ) expect(user.can?(:admin_issue, maintainer_project)).to eq(true) expect(user.can?(:admin_issue, developer_project)).to eq(true) expect(user.can?(:admin_issue, reporter_project)).to eq(true) + expect(user.can?(:admin_issue, planner_project)).to eq(true) end - it 'does not include for which the user access level is below reporter' do + it 'does not include for which the user access level is below planner' do project = create(:project) guest_project = create(:project) { |p| p.add_guest(user) } @@ -6906,11 +6911,13 @@ def add_user(access) let(:maintainer_project) { create(:project) } let(:reporter_project) { create(:project) } let(:developer_project) { create(:project) } + let(:planner_project) { create(:project) } let(:guest_project) { create(:project) } let(:no_access_project) { create(:project) } let(:projects) do - [owner_project, maintainer_project, reporter_project, developer_project, guest_project, no_access_project].map(&:id) + [owner_project, maintainer_project, reporter_project, + developer_project, planner_project, guest_project, no_access_project].map(&:id) end let(:expected) do @@ -6920,6 +6927,7 @@ def add_user(access) reporter_project.id => Gitlab::Access::REPORTER, developer_project.id => Gitlab::Access::DEVELOPER, guest_project.id => Gitlab::Access::GUEST, + planner_project.id => Gitlab::Access::PLANNER, no_access_project.id => Gitlab::Access::NO_ACCESS } end @@ -6929,6 +6937,7 @@ def add_user(access) maintainer_project.add_maintainer(user) reporter_project.add_reporter(user) developer_project.add_developer(user) + planner_project.add_planner(user) guest_project.add_guest(user) end @@ -6987,11 +6996,13 @@ def access_levels(projects) let(:maintainer_group) { create(:group) } let(:reporter_group) { create(:group) } let(:developer_group) { create(:group) } + let(:planner_group) { create(:group) } let(:guest_group) { create(:group) } let(:no_access_group) { create(:group) } let(:groups) do - [owner_group, maintainer_group, reporter_group, developer_group, guest_group, no_access_group].map(&:id) + [owner_group, maintainer_group, reporter_group, developer_group, + planner_group, guest_group, no_access_group, planner_group].map(&:id) end let(:expected) do @@ -7000,6 +7011,7 @@ def access_levels(projects) maintainer_group.id => Gitlab::Access::MAINTAINER, reporter_group.id => Gitlab::Access::REPORTER, developer_group.id => Gitlab::Access::DEVELOPER, + planner_group.id => Gitlab::Access::PLANNER, guest_group.id => Gitlab::Access::GUEST, no_access_group.id => Gitlab::Access::NO_ACCESS } @@ -7010,6 +7022,7 @@ def access_levels(projects) maintainer_group.add_maintainer(user) reporter_group.add_reporter(user) developer_group.add_developer(user) + planner_group.add_planner(user) guest_group.add_guest(user) end @@ -8092,6 +8105,7 @@ def access_levels(groups) context 'when memberships exist' do it 'returns the highest access level for non requested memberships' do create(:group_member, :reporter, user_id: user.id) + create(:project_member, :planner, user_id: user.id) create(:project_member, :guest, user_id: user.id) create(:project_member, :maintainer, user_id: user.id, requested_at: Time.current) diff --git a/spec/policies/board_policy_spec.rb b/spec/policies/board_policy_spec.rb index 3786fb222668cd..f6558dbee69892 100644 --- a/spec/policies/board_policy_spec.rb +++ b/spec/policies/board_policy_spec.rb @@ -2,12 +2,12 @@ require 'spec_helper' -RSpec.describe BoardPolicy do - let(:user) { create(:user) } - let(:project) { create(:project, :private) } - let(:group) { create(:group, :private) } - let(:group_board) { create(:board, group: group) } - let(:project_board) { create(:board, project: project) } +RSpec.describe BoardPolicy, feature_category: :portfolio_management do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:group) { create(:group, :private) } + let_it_be(:group_board) { create(:board, group: group) } + let_it_be(:project_board) { create(:board, project: project) } let(:board_permissions) do [ @@ -21,7 +21,7 @@ subject { described_class.new(user, group_board) } context 'user has access' do - before do + before_all do group.add_developer(user) end @@ -41,7 +41,7 @@ subject { described_class.new(user, project_board) } context 'user has access' do - before do + before_all do project.add_developer(user) end @@ -83,7 +83,7 @@ context 'when user can admin project issues' do it 'allows to add non backlog issues from issue board' do - project.add_reporter(current_user) + project.add_planner(current_user) expect_allowed(:create_non_backlog_issues) end @@ -99,29 +99,34 @@ end context 'for group boards' do - let!(:current_user) { create(:user) } - let!(:project_1) { create(:project, namespace: group) } - let!(:project_2) { create(:project, namespace: group) } - let!(:group_board) { create(:board, group: group) } + let_it_be(:guest) { create(:user) } + let_it_be(:planner) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:group_board) { create(:board, group: group) } + let_it_be(:project_2) do + create(:project, namespace: group, guests: guest, planners: planner, reporters: reporter) + end + + let(:current_user) { nil } subject { described_class.new(current_user, group_board) } it_behaves_like 'with admin' - context 'when user is at least reporter in one of the child projects' do - it 'allows to add non backlog issues from issue board' do - project_2.add_reporter(current_user) + context 'with planner or reporter role in a child project' do + where(role: %w[planner reporter]) - expect_allowed(:create_non_backlog_issues) + with_them do + let(:current_user) { public_send(role) } + + it { expect_allowed(:create_non_backlog_issues) } end end - context 'when user is not a reporter from any child projects' do - it 'does not allow to add non backlog issues from issue board' do - project_2.add_guest(current_user) + context 'when user is not at least a planner from any child projects' do + let(:current_user) { guest } - expect_disallowed(:create_non_backlog_issues) - end + it { expect_disallowed(:create_non_backlog_issues) } end end end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index b2dbe59241ebd2..544cd7176bcdab 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -14,6 +14,8 @@ specify do expect_allowed(*public_permissions) expect_disallowed(:upload_file) + expect_disallowed(*(guest_permissions - public_permissions)) + expect_disallowed(*(planner_permissions - guest_permissions)) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -29,6 +31,8 @@ specify do expect_allowed(*public_permissions) expect_disallowed(:upload_file) + expect_disallowed(*(guest_permissions - public_permissions)) + expect_disallowed(*(planner_permissions - guest_permissions)) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -47,6 +51,8 @@ specify do expect_disallowed(*public_permissions) + expect_disallowed(*guest_permissions) + expect_disallowed(*planner_permissions) expect_disallowed(*reporter_permissions) expect_disallowed(*owner_permissions) end @@ -62,6 +68,8 @@ specify do expect_disallowed(*public_permissions) + expect_disallowed(*guest_permissions) + expect_disallowed(*planner_permissions) expect_disallowed(*reporter_permissions) expect_disallowed(*owner_permissions) end @@ -96,6 +104,7 @@ specify do expect_disallowed(*public_permissions) expect_disallowed(*guest_permissions) + expect_disallowed(*planner_permissions) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -109,6 +118,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_disallowed(*(planner_permissions - guest_permissions)) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -120,12 +130,31 @@ end end + context 'planners' do + let(:current_user) { planner } + + specify do + expect_allowed(*public_permissions) + expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) + expect_disallowed(*(reporter_permissions - planner_permissions)) + expect_disallowed(*developer_permissions) + expect_disallowed(*maintainer_permissions) + expect_disallowed(*(owner_permissions - [:destroy_issue])) + end + + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { planner.id } + end + end + context 'reporter' do let(:current_user) { reporter } specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - [:destroy_issue])) expect_allowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -143,6 +172,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - [:destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -165,6 +195,7 @@ it 'allows permissions from lower roles' do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - [:destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) end @@ -179,6 +210,7 @@ it 'allows every maintainer permission' do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions - [:destroy_issue]) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) expect_allowed(*maintainer_permissions) @@ -197,6 +229,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) expect_allowed(*maintainer_permissions) @@ -214,6 +247,7 @@ specify do expect_disallowed(*public_permissions) expect_disallowed(*guest_permissions) + expect_disallowed(*planner_permissions) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -224,6 +258,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) expect_allowed(*maintainer_permissions) @@ -247,6 +282,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) expect_allowed(*maintainer_permissions) @@ -274,6 +310,7 @@ it :aggregate_failures do expect_allowed(:read_resource_access_tokens, :destroy_resource_access_tokens) expect_disallowed(*guest_permissions) + expect_disallowed(*planner_permissions) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -290,6 +327,7 @@ it :aggregate_failures do expect_disallowed(:read_resource_access_tokens, :destroy_resource_access_tokens) expect_disallowed(*guest_permissions) + expect_disallowed(*planner_permissions) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -323,6 +361,7 @@ specify do expect_disallowed(*public_permissions) expect_disallowed(*guest_permissions) + expect_disallowed(*planner_permissions) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -336,6 +375,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_disallowed(*(planner_permissions - guest_permissions)) expect_disallowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -343,12 +383,27 @@ end end + context 'planners' do + let(:current_user) { planner } + + specify do + expect_allowed(*public_permissions) + expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) + expect_disallowed(*(reporter_permissions - planner_permissions)) + expect_disallowed(*developer_permissions) + expect_disallowed(*maintainer_permissions) + expect_disallowed(*(owner_permissions - [:destroy_issue])) + end + end + context 'reporter' do let(:current_user) { reporter } specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - [:destroy_issue])) expect_allowed(*reporter_permissions) expect_disallowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -362,6 +417,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - [:destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) expect_disallowed(*maintainer_permissions) @@ -375,6 +431,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - [:destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) end @@ -391,6 +448,7 @@ specify do expect_allowed(*public_permissions) expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) expect_allowed(*developer_permissions) expect_allowed(*maintainer_permissions) @@ -578,26 +636,31 @@ context 'create_projects' do context 'without visibility levels restricted' do where(:project_creation_level, :current_user, :create_projects_allowed?) do + nil | lazy { planner } | false nil | lazy { reporter } | false nil | lazy { developer } | true nil | lazy { maintainer } | true nil | lazy { owner } | true nil | lazy { admin } | true + ::Gitlab::Access::NO_ONE_PROJECT_ACCESS | lazy { planner } | false ::Gitlab::Access::NO_ONE_PROJECT_ACCESS | lazy { reporter } | false ::Gitlab::Access::NO_ONE_PROJECT_ACCESS | lazy { developer } | false ::Gitlab::Access::NO_ONE_PROJECT_ACCESS | lazy { maintainer } | false ::Gitlab::Access::NO_ONE_PROJECT_ACCESS | lazy { owner } | false ::Gitlab::Access::NO_ONE_PROJECT_ACCESS | lazy { admin } | false + ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | lazy { planner } | false ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | lazy { reporter } | false ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | lazy { developer } | false ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | lazy { maintainer } | true ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | lazy { owner } | true ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | lazy { admin } | true + ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | lazy { planner } | false ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | lazy { reporter } | false ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | lazy { developer } | true ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | lazy { maintainer } | true ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | lazy { owner } | true ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | lazy { admin } | true + ::Gitlab::Access::ADMINISTRATOR_PROJECT_ACCESS | lazy { planner } | false ::Gitlab::Access::ADMINISTRATOR_PROJECT_ACCESS | lazy { reporter } | false ::Gitlab::Access::ADMINISTRATOR_PROJECT_ACCESS | lazy { developer } | false ::Gitlab::Access::ADMINISTRATOR_PROJECT_ACCESS | lazy { maintainer } | false @@ -678,6 +741,12 @@ context 'when group has no project creation level set' do let(:project_creation_level) { nil } + context 'planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:import_projects) } + end + context 'reporter' do let(:current_user) { reporter } @@ -706,6 +775,12 @@ context 'when group has project creation level set to no one' do let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } + context 'planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:import_projects) } + end + context 'reporter' do let(:current_user) { reporter } @@ -734,6 +809,12 @@ context 'when group has project creation level set to maintainer only' do let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } + context 'planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:import_projects) } + end + context 'reporter' do let(:current_user) { reporter } @@ -762,6 +843,12 @@ context 'when group has project creation level set to developers + maintainer' do let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS } + context 'planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:import_projects) } + end + context 'reporter' do let(:current_user) { reporter } @@ -794,6 +881,12 @@ group.update!(subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS) end + context 'planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:create_subgroup) } + end + context 'reporter' do let(:current_user) { reporter } @@ -824,6 +917,12 @@ group.update!(subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) end + context 'planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:create_subgroup) } + end + context 'reporter' do let(:current_user) { reporter } @@ -878,7 +977,7 @@ end end - %w[guest reporter developer maintainer owner].each do |role| + %w[guest planner reporter developer maintainer owner].each do |role| context role do let(:current_user) { send(role) } @@ -976,6 +1075,12 @@ it { is_expected.to be_disallowed(:create_jira_connect_subscription) } end + context 'with planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:create_jira_connect_subscription) } + end + context 'with guest' do let(:current_user) { guest } @@ -1026,6 +1131,12 @@ it { is_expected.to be_allowed(:read_package) } end + context 'with planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:read_package) } + end + context 'with guest' do let(:current_user) { guest } @@ -1278,6 +1389,12 @@ it { is_expected.to be_disallowed(:update_runners_registration_token) } end + context 'with planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:update_runners_registration_token) } + end + context 'with guest' do let(:current_user) { guest } @@ -1408,6 +1525,12 @@ it { is_expected.to be_disallowed(:register_group_runners) } end + context 'with planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:register_group_runners) } + end + context 'with guest' do let(:current_user) { guest } @@ -1508,6 +1631,12 @@ it { is_expected.to be_disallowed(:create_runner) } end + context 'with planner' do + let(:current_user) { planner } + + it { is_expected.to be_disallowed(:create_runner) } + end + context 'with guest' do let(:current_user) { guest } @@ -1564,6 +1693,12 @@ specify { is_expected.to be_disallowed(:read_group_all_available_runners) } end + context 'with planner' do + let(:current_user) { planner } + + specify { is_expected.to be_disallowed(:read_group_all_available_runners) } + end + context 'with guest' do let(:current_user) { guest } @@ -1633,6 +1768,7 @@ :maintainer | nil | false :developer | nil | false :reporter | nil | false + :planner | nil | false :guest | nil | false end diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb index 71cf341b5847ed..f6767ac88e3633 100644 --- a/spec/policies/issuable_policy_spec.rb +++ b/spec/policies/issuable_policy_spec.rb @@ -5,6 +5,7 @@ RSpec.describe IssuablePolicy, :models do let_it_be(:user) { create(:user) } let_it_be(:guest) { create(:user) } + let_it_be(:planner) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:developer) { create(:user) } let_it_be(:project) { create(:project, :public) } @@ -15,6 +16,7 @@ before do project.add_developer(developer) project.add_guest(guest) + project.add_planner(planner) project.add_reporter(reporter) end @@ -46,6 +48,10 @@ def permissions(user, issuable) expect(permissions(guest, issue)).to be_allowed(:read_incident_management_timeline_event) end + it 'disallows planners from managing timeline events' do + expect(permissions(planner, issue)).to be_disallowed(:admin_incident_management_timeline_event) + end + it 'disallows reporters from managing timeline events' do expect(permissions(reporter, issue)).to be_disallowed(:admin_incident_management_timeline_event) end @@ -79,6 +85,10 @@ def permissions(user, issuable) expect(permissions(guest, issue)).to be_allowed(:read_incident_management_timeline_event) end + it 'disallows planners from managing timeline events' do + expect(permissions(planner, issue)).to be_disallowed(:admin_incident_management_timeline_event) + end + it 'disallows reporters from managing timeline events' do expect(permissions(reporter, issue)).to be_disallowed(:admin_incident_management_timeline_event) end @@ -169,6 +179,16 @@ def permissions(user, issuable) end end + context 'when user is at planner of the project' do + it 'allows timelogs creation' do + expect(permissions(planner, issue)).to be_allowed(:create_timelog) + end + + it 'allows reading internal notes' do + expect(permissions(planner, issue)).to be_allowed(:read_internal_note) + end + end + context 'when user is at least reporter of the project' do it 'allows timelogs creation' do expect(permissions(reporter, issue)).to be_allowed(:create_timelog) diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index e82ad79f2a4e90..442dc9de565ff0 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -11,6 +11,7 @@ let_it_be(:group) { create(:group, :public) } let_it_be(:admin) { create(:user, :admin) } let_it_be(:guest) { create(:user) } + let_it_be(:planner) { create(:user) } let_it_be(:author) { create(:user) } let_it_be(:assignee) { create(:user) } let_it_be(:reporter) { create(:user) } @@ -80,6 +81,7 @@ def permissions(user, issue) project.add_guest(guest) project.add_guest(author) project.add_guest(assignee) + project.add_planner(planner) project.add_reporter(reporter) group.add_reporter(reporter_from_group_link) @@ -101,6 +103,12 @@ def permissions(user, issue) expect(permissions(guest, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end + it 'allows planners to read, update, admin and create confidential notes' do + expect(permissions(planner, issue)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) + expect(permissions(planner, issue_no_assignee)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) + expect(permissions(planner, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality, :mark_note_as_internal, :admin_issue_relation) + end + it 'allows reporters to read, update, admin and create confidential notes' do expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) @@ -156,6 +164,11 @@ def permissions(user, issue) expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation, :award_emoji, :admin_issue_link) end + it 'allows planners to read, update, and admin confidential issues' do + expect(permissions(planner, confidential_issue)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation, :award_emoji) + expect(permissions(planner, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation, :award_emoji) + end + it 'allows reporters to read, update, and admin confidential issues' do expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation, :award_emoji) expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation, :award_emoji) @@ -205,6 +218,7 @@ def permissions(user, issue) before_all do project.add_guest(guest) + project.add_planner(planner) project.add_reporter(reporter) project.add_maintainer(maintainer) project.add_owner(owner) @@ -246,6 +260,14 @@ def permissions(user, issue) expect(permissions(guest, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) end + it 'allows planners to read, update, reopen, and admin issues' do + expect(permissions(planner, issue)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) + expect(permissions(planner, issue_no_assignee)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) + expect(permissions(planner, issue_locked)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) + expect(permissions(planner, issue_locked)).to be_disallowed(:reopen_issue) + expect(permissions(planner, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) + end + it 'allows reporters to read, update, reopen, and admin issues' do expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality, :admin_issue_relation) @@ -402,6 +424,11 @@ def permissions(user, issue) expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end + it 'allows planners to read, update, and admin confidential issues' do + expect(permissions(planner, confidential_issue)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :admin_issue_relation) + expect(permissions(planner, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + end + it 'allows reporters to read, update, and admin confidential issues' do expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :admin_issue_relation) expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_note, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) @@ -463,6 +490,7 @@ def permissions(user, issue) # with notes widget enabled, even guests can access notes expect(permissions(guest, issue)).to be_allowed(:create_note, :read_note) expect(permissions(guest, issue)).to be_disallowed(:read_internal_note, :mark_note_as_internal, :set_note_created_at, :admin_note) + expect(permissions(planner, issue)).to be_allowed(:create_note, :read_note, :read_internal_note, :mark_note_as_internal) expect(permissions(reporter, issue)).to be_allowed(:create_note, :read_note, :read_internal_note, :mark_note_as_internal) expect(permissions(reporter, issue)).to be_disallowed(:admin_note) expect(permissions(maintainer, issue)).to be_allowed(:create_note, :read_note, :read_internal_note, :mark_note_as_internal, :admin_note) @@ -483,10 +511,12 @@ def permissions(user, issue) let_it_be(:non_member_user) { create(:user) } let_it_be(:guest) { create(:user, guest_of: [private_project, public_project]) } + let_it_be(:planner) { create(:user, planner_of: [private_project, public_project]) } let_it_be(:guest_author) { create(:user, guest_of: [private_project, public_project]) } let_it_be(:reporter) { create(:user, reporter_of: [private_project, public_project]) } let_it_be(:group_guest) { create(:user, guest_of: [private_group, public_group]) } + let_it_be(:group_planner) { create(:user, planner_of: [private_group, public_group]) } let_it_be(:group_guest_author) { create(:user, guest_of: [private_group, public_group]) } let_it_be(:group_reporter) { create(:user, reporter_of: [private_group, public_group]) } @@ -538,77 +568,79 @@ def permissions(user, issue) let(:issue) { create(:issue, project: project) } let(:policies) { described_class.new(user, issue) } - context 'when project reporter' do - it 'is disallowed' do - project.add_reporter(user) - - expect(policies).to be_disallowed(:read_crm_contacts) - expect(policies).to be_disallowed(:set_issue_crm_contacts) - end - end - - context 'when subgroup reporter' do - it 'is allowed' do - subgroup.add_reporter(user) + %i[planner reporter].each do |role| + context "when project #{role}" do + it 'is disallowed' do + project.try(:"add_#{role}", user) - expect(policies).to be_disallowed(:read_crm_contacts) - expect(policies).to be_disallowed(:set_issue_crm_contacts) + expect(policies).to be_disallowed(:read_crm_contacts) + expect(policies).to be_disallowed(:set_issue_crm_contacts) + end end - end - context 'when root group reporter' do - it 'is allowed' do - subgroup.parent.add_reporter(user) + context "when subgroup #{role}" do + it 'is allowed' do + subgroup.try(:"add_#{role}", user) - expect(policies).to be_allowed(:read_crm_contacts) - expect(policies).to be_allowed(:set_issue_crm_contacts) + expect(policies).to be_disallowed(:read_crm_contacts) + expect(policies).to be_disallowed(:set_issue_crm_contacts) + end end - end - - context 'when crm disabled on subgroup' do - let(:subgroup) { create(:group, :crm_disabled, parent: create(:group)) } - it 'is disallowed' do - subgroup.parent.add_reporter(user) + context "when root group #{role}" do + it 'is allowed' do + subgroup.parent.try(:"add_#{role}", user) - expect(policies).to be_disallowed(:read_crm_contacts) - expect(policies).to be_disallowed(:set_issue_crm_contacts) + expect(policies).to be_allowed(:read_crm_contacts) + expect(policies).to be_allowed(:set_issue_crm_contacts) + end end - end - context 'when personal namespace' do - let(:project) { create(:project) } + context 'when crm disabled on subgroup' do + let(:subgroup) { create(:group, :crm_disabled, parent: create(:group)) } - it 'is disallowed' do - project.add_reporter(user) + it 'is disallowed' do + subgroup.parent.try(:"add_#{role}", user) - expect(policies).to be_disallowed(:read_crm_contacts) - expect(policies).to be_disallowed(:set_issue_crm_contacts) + expect(policies).to be_disallowed(:read_crm_contacts) + expect(policies).to be_disallowed(:set_issue_crm_contacts) + end end - end - context 'when custom crm_group configured' do - let_it_be(:crm_settings) { create(:crm_settings, source_group: create(:group)) } - let_it_be(:subgroup) { create(:group, parent: create(:group), crm_settings: crm_settings) } - let_it_be(:project) { create(:project, group: subgroup) } + context 'when personal namespace' do + let(:project) { create(:project) } - context 'when custom crm_group guest' do it 'is disallowed' do - subgroup.parent.add_reporter(user) - crm_settings.source_group.add_guest(user) + project.try(:"add_#{role}", user) expect(policies).to be_disallowed(:read_crm_contacts) expect(policies).to be_disallowed(:set_issue_crm_contacts) end end - context 'when custom crm_group reporter' do - it 'is allowed' do - subgroup.parent.add_reporter(user) - crm_settings.source_group.add_reporter(user) + context 'when custom crm_group configured' do + let_it_be(:crm_settings) { create(:crm_settings, source_group: create(:group)) } + let_it_be(:subgroup) { create(:group, parent: create(:group), crm_settings: crm_settings) } + let_it_be(:project) { create(:project, group: subgroup) } - expect(policies).to be_allowed(:read_crm_contacts) - expect(policies).to be_allowed(:set_issue_crm_contacts) + context 'when custom crm_group guest' do + it 'is disallowed' do + subgroup.parent.try(:"add_#{role}", user) + crm_settings.source_group.add_guest(user) + + expect(policies).to be_disallowed(:read_crm_contacts) + expect(policies).to be_disallowed(:set_issue_crm_contacts) + end + end + + context "when custom crm_group #{role}" do + it 'is allowed' do + subgroup.parent.try(:"add_#{role}", user) + crm_settings.source_group.try(:"add_#{role}", user) + + expect(policies).to be_allowed(:read_crm_contacts) + expect(policies).to be_allowed(:set_issue_crm_contacts) + end end end end diff --git a/spec/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb index 12b38fe1d93216..19f4d79a68460b 100644 --- a/spec/policies/merge_request_policy_spec.rb +++ b/spec/policies/merge_request_policy_spec.rb @@ -4,9 +4,11 @@ RSpec.describe MergeRequestPolicy, feature_category: :code_review_workflow do include ExternalAuthorizationServiceHelpers + using RSpec::Parameterized::TableSyntax let_it_be(:guest) { create(:user) } let_it_be(:author) { create(:user) } + let_it_be(:planner) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:developer) { create(:user) } let_it_be(:non_team_member) { create(:user) } @@ -16,6 +18,34 @@ def permissions(user, merge_request) described_class.new(user, merge_request) end + # :policy, :is_allowed + def permission_table_for_guest + :read_merge_request | true + :create_todo | true + :create_note | true + :update_subscription | true + :create_merge_request_in | true + :create_merge_request_from | false + :approve_merge_request | false + :update_merge_request | false + :reset_merge_request_approvals | false + :mark_note_as_internal | false + end + + # :policy, :is_allowed + def permission_table_for_reporter + :read_merge_request | true + :create_todo | true + :create_note | true + :update_subscription | true + :create_merge_request_in | true + :create_merge_request_from | false + :approve_merge_request | false + :update_merge_request | false + :reset_merge_request_approvals | false + :mark_note_as_internal | true + end + mr_perms = %i[create_merge_request_in create_merge_request_from read_merge_request @@ -36,19 +66,9 @@ def permissions(user, merge_request) end end - shared_examples_for 'a user with reporter access' do - using RSpec::Parameterized::TableSyntax - + shared_examples_for 'a user with limited access' do where(:policy, :is_allowed) do - :create_merge_request_in | true - :read_merge_request | true - :create_todo | true - :create_note | true - :update_subscription | true - :create_merge_request_from | false - :approve_merge_request | false - :update_merge_request | false - :mark_note_as_internal | true + permission_table end with_them do @@ -74,6 +94,7 @@ def permissions(user, merge_request) before do project.add_guest(guest) project.add_guest(author) + project.add_planner(planner) project.add_developer(developer) project.add_developer(bot) end @@ -105,6 +126,30 @@ def permissions(user, merge_request) end end + context 'and the user is a planner' do + let(:user) { planner } + + it do + is_expected.to be_allowed(:update_merge_request) + end + + it do + is_expected.to be_allowed(:reopen_merge_request) + end + + it do + is_expected.to be_allowed(:approve_merge_request) + end + + it do + is_expected.to be_allowed(:mark_note_as_internal) + end + + it do + is_expected.to be_disallowed(:reset_merge_request_approvals) + end + end + context 'and the user is a bot' do let(:user) { bot } @@ -113,6 +158,46 @@ def permissions(user, merge_request) end end end + + context 'and user is not author' do + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project, author: author) + end + + describe 'a guest' do + let(:permission_table) { permission_table_for_guest } + + subject { permissions(guest, merge_request) } + + it_behaves_like 'a user with limited access' + end + + describe 'a planner' do + let(:permission_table) { permission_table_for_reporter } + + subject { permissions(planner, merge_request) } + + it_behaves_like 'a user with limited access' + end + end + + context 'with private project' do + let_it_be(:project) { create(:project, :private) } + + describe 'a guest' do + subject { guest } + + it_behaves_like 'a denied user' + end + + describe 'a planner' do + let(:permission_table) { permission_table_for_reporter } + + subject { permissions(planner, merge_request) } + + it_behaves_like 'a user with limited access' + end + end end context 'when merge requests have been disabled' do @@ -134,6 +219,12 @@ def permissions(user, merge_request) it_behaves_like 'a denied user' end + describe 'a planner' do + subject { planner } + + it_behaves_like 'a denied user' + end + describe 'a developer' do subject { developer } @@ -163,6 +254,12 @@ def permissions(user, merge_request) it_behaves_like 'a denied user' end + describe 'a planner' do + subject { planner } + + it_behaves_like 'a denied user' + end + describe 'a developer' do subject { developer } @@ -189,6 +286,10 @@ def permissions(user, merge_request) expect(permissions(developer, merge_request)).to be_allowed(:reopen_merge_request) end + it 'prevents planner from reopening merge request' do + expect(permissions(planner, merge_request)).to be_disallowed(:reopen_merge_request) + end + it 'prevents guest from reopening merge request' do expect(permissions(guest, merge_request)).to be_disallowed(:reopen_merge_request) end @@ -205,6 +306,10 @@ def permissions(user, merge_request) expect(permissions(developer, merge_request_locked)).to be_disallowed(:reopen_merge_request) end + it 'prevents planners from reopening merge request' do + expect(permissions(planner, merge_request_locked)).to be_disallowed(:reopen_merge_request) + end + it 'prevents guests from reopening merge request' do expect(permissions(guest, merge_request_locked)).to be_disallowed(:reopen_merge_request) end @@ -244,6 +349,7 @@ def permissions(user, merge_request) before_all do group.add_guest(guest) group.add_guest(author) + group.add_planner(planner) group.add_reporter(reporter) group.add_developer(developer) group.add_developer(bot) @@ -276,6 +382,22 @@ def permissions(user, merge_request) end end + describe 'a planner' do + let(:permission_table) { permission_table_for_reporter } + + subject { permissions(planner, merge_request) } + + it_behaves_like 'a user with limited access' + end + + describe 'a reporter' do + let(:permission_table) { permission_table_for_reporter } + + subject { permissions(reporter, merge_request) } + + it_behaves_like 'a user with limited access' + end + context 'and merge requests are private' do before do project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) @@ -288,10 +410,18 @@ def permissions(user, merge_request) it_behaves_like 'a denied user' end + describe 'a planner' do + subject { planner } + + it_behaves_like 'a denied user' + end + describe 'a reporter' do + let(:permission_table) { permission_table_for_reporter } + subject { permissions(reporter, merge_request) } - it_behaves_like 'a user with reporter access' + it_behaves_like 'a user with limited access' end describe 'a developer' do @@ -319,10 +449,20 @@ def permissions(user, merge_request) it_behaves_like 'a denied user' end + describe 'a planner' do + let(:permission_table) { permission_table_for_reporter } + + subject { permissions(planner, merge_request) } + + it_behaves_like 'a user with limited access' + end + describe 'a reporter' do + let(:permission_table) { permission_table_for_reporter } + subject { permissions(reporter, merge_request) } - it_behaves_like 'a user with reporter access' + it_behaves_like 'a user with limited access' end describe 'a developer' do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index dde138a862d9d6..4b61a53b424c0b 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -17,6 +17,7 @@ before_all do project_with_runner_registration_token.add_guest(guest) + project_with_runner_registration_token.add_planner(planner) project_with_runner_registration_token.add_reporter(reporter) project_with_runner_registration_token.add_developer(developer) project_with_runner_registration_token.add_maintainer(maintainer) @@ -105,13 +106,18 @@ def set_access_level(access_level) expect_disallowed(*mr_permissions) end - context 'for a guest in a private project' do + context "for a guest in a private project" do let(:current_user) { guest } let(:project) { private_project } - it 'disallows the guest from all merge request permissions' do - expect_disallowed(*mr_permissions) - end + it { expect_disallowed(*mr_permissions) } + end + + context "for a planner in a private project" do + let(:current_user) { planner } + let(:project) { private_project } + + it { expect_disallowed(*(mr_permissions - [:read_merge_request, :create_merge_request_in])) } end end @@ -178,32 +184,42 @@ def set_access_level(access_level) context 'when project is public' do let(:project) { public_project } - context 'when the current_user is guest' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "when the current_user is #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:create_merge_request_in) } + it { is_expected.to be_allowed(:create_merge_request_in) } + end end end context 'when project is internal' do let(:project) { internal_project } - context 'when the current_user is guest' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "when the current_user is #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:create_merge_request_in) } + it { is_expected.to be_allowed(:create_merge_request_in) } + end end end context 'when project is private' do let(:project) { private_project } - context 'when the current_user is guest' do + context "when the current_user is guest" do let(:current_user) { guest } it { is_expected.not_to be_allowed(:create_merge_request_in) } end + context 'when the current_user is planner' do + let(:current_user) { planner } + + it { is_expected.to be_allowed(:create_merge_request_in) } + end + context 'when the current_user is reporter or above' do let(:current_user) { reporter } @@ -221,30 +237,36 @@ def set_access_level(access_level) context 'when project is public' do let(:project) { public_project } - context 'when the current_user is guest' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "when the current_user is #{role}" do + let(:current_user) { send(role) } - it { is_expected.not_to be_allowed(:create_merge_request_in) } + it { is_expected.not_to be_allowed(:create_merge_request_in) } + end end end context 'when project is internal' do let(:project) { internal_project } - context 'when the current_user is guest' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "when the current_user is #{role}" do + let(:current_user) { send(role) } - it { is_expected.not_to be_allowed(:create_merge_request_in) } + it { is_expected.not_to be_allowed(:create_merge_request_in) } + end end end context 'when project is private' do let(:project) { private_project } - context 'when the current_user is guest' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "when the current_user is #{role}" do + let(:current_user) { send(role) } - it { is_expected.not_to be_allowed(:create_merge_request_in) } + it { is_expected.not_to be_allowed(:create_merge_request_in) } + end end context 'when the current_user is reporter or above' do @@ -446,6 +468,7 @@ def set_access_level(access_level) it_behaves_like 'project policies as anonymous' it_behaves_like 'project policies as guest' + it_behaves_like 'project policies as planner' it_behaves_like 'project policies as reporter' it_behaves_like 'project policies as developer' it_behaves_like 'project policies as maintainer' @@ -499,7 +522,7 @@ def set_access_level(access_level) end end - %w[guest reporter developer anonymous].each do |role| + %w[guest planner reporter developer anonymous].each do |role| context "with #{role}" do let(:current_user) { send(role) } @@ -521,7 +544,7 @@ def set_access_level(access_level) end context 'importing work items' do - %w[reporter developer maintainer owner].each do |role| + %w[reporter planner developer maintainer owner].each do |role| context "with #{role}" do let(:current_user) { send(role) } @@ -559,7 +582,7 @@ def set_access_level(access_level) end end - %w[guest reporter developer anonymous].each do |role| + %w[guest planner reporter developer anonymous].each do |role| context "with #{role}" do let(:current_user) { send(role) } @@ -597,6 +620,7 @@ def set_access_level(access_level) before_all do project.add_guest(guest) + project.add_planner(planner) project.add_reporter(reporter) project.add_developer(developer) project.add_maintainer(maintainer) @@ -607,6 +631,7 @@ def set_access_level(access_level) expect(described_class.new(owner_of_different_thing, project)).to be_disallowed(:owner_access) expect(described_class.new(non_member, project)).to be_disallowed(:owner_access) expect(described_class.new(guest, project)).to be_disallowed(:owner_access) + expect(described_class.new(planner, project)).to be_disallowed(:owner_access) expect(described_class.new(reporter, project)).to be_disallowed(:owner_access) expect(described_class.new(developer, project)).to be_disallowed(:owner_access) expect(described_class.new(maintainer, project)).to be_disallowed(:owner_access) @@ -622,6 +647,7 @@ def set_access_level(access_level) context 'group members' do before_all do group.add_guest(guest) + group.add_planner(planner) group.add_reporter(reporter) group.add_developer(developer) group.add_maintainer(maintainer) @@ -633,6 +659,7 @@ def set_access_level(access_level) expect(described_class.new(owner_of_different_thing, project)).to be_disallowed(:owner_access) expect(described_class.new(non_member, project)).to be_disallowed(:owner_access) expect(described_class.new(guest, project)).to be_disallowed(:owner_access) + expect(described_class.new(planner, project)).to be_disallowed(:owner_access) expect(described_class.new(reporter, project)).to be_disallowed(:owner_access) expect(described_class.new(developer, project)).to be_disallowed(:owner_access) expect(described_class.new(maintainer, project)).to be_disallowed(:owner_access) @@ -648,6 +675,7 @@ def set_access_level(access_level) expect(described_class.new(owner, project)).to be_allowed(:read_incident_management_timeline_event_tag) expect(described_class.new(developer, project)).to be_allowed(:read_incident_management_timeline_event_tag) expect(described_class.new(guest, project)).to be_allowed(:read_incident_management_timeline_event_tag) + expect(described_class.new(planner, project)).to be_allowed(:read_incident_management_timeline_event_tag) expect(described_class.new(admin, project)).to be_allowed(:read_incident_management_timeline_event_tag) end end @@ -664,16 +692,18 @@ def set_access_level(access_level) end end - context 'when user is a developer/guest/reporter' do + context 'when user is a developer/guest/planner/reporter' do it 'disallows creation' do expect(described_class.new(developer, project)).to be_disallowed(:admin_incident_management_timeline_event_tag) expect(described_class.new(guest, project)).to be_disallowed(:admin_incident_management_timeline_event_tag) + expect(described_class.new(planner, project)).to be_disallowed(:admin_incident_management_timeline_event_tag) expect(described_class.new(reporter, project)).to be_disallowed(:admin_incident_management_timeline_event_tag) end it 'disallows reading the import error' do expect(described_class.new(developer, project)).to be_disallowed(:read_import_error) expect(described_class.new(guest, project)).to be_disallowed(:read_import_error) + expect(described_class.new(planner, project)).to be_disallowed(:read_import_error) expect(described_class.new(reporter, project)).to be_disallowed(:read_import_error) end end @@ -764,10 +794,12 @@ def set_access_level(access_level) context 'project member' do let(:project) { private_project } - context 'guest' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context role do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:fork_project) } + it { is_expected.to be_disallowed(:fork_project) } + end end %w[reporter developer maintainer].each do |role| @@ -796,12 +828,15 @@ def set_access_level(access_level) where(:project_visibility, :role, :allowed) do :public | :anonymous | false :public | :guest | false + :public | :planner | false :public | :reporter | true :internal | :anonymous | false :internal | :guest | true + :internal | :planner | true :internal | :reporter | true :private | :anonymous | false :private | :guest | true + :private | :planner | true :private | :reporter | true end @@ -829,12 +864,15 @@ def set_access_level(access_level) where(:project_visibility, :role, :allowed) do :public | :anonymous | false :public | :guest | false + :public | :planner | false :public | :reporter | true :internal | :anonymous | false :internal | :guest | false + :internal | :planner | false :internal | :reporter | true :private | :anonymous | false :private | :guest | false + :private | :planner | false :private | :reporter | true end @@ -869,7 +907,7 @@ def set_access_level(access_level) end end - %w[guest reporter developer maintainer owner].each do |role| + %w[guest planner reporter developer maintainer owner].each do |role| context role do let(:current_user) { send(role) } @@ -897,7 +935,7 @@ def set_access_level(access_level) end end - %w[guest reporter developer maintainer owner].each do |role| + %w[guest planner reporter developer maintainer owner].each do |role| context role do let(:current_user) { send(role) } @@ -923,6 +961,7 @@ def set_access_level(access_level) where(:user_role, :minimum_role, :allowed) do :guest | :developer | false + :planner | :developer | false :reporter | :developer | false :developer | :developer | false :maintainer | :developer | true @@ -961,48 +1000,56 @@ def set_access_level(access_level) :maintainer | :no_one_allowed | true | false :owner | :no_one_allowed | true | false :guest | :no_one_allowed | true | false + :planner | :no_one_allowed | true | false :reporter | :no_one_allowed | true | false :anonymous | :no_one_allowed | true | false :developer | :developer | true | true :maintainer | :developer | true | true :owner | :developer | true | true :guest | :developer | true | false + :planner | :developer | true | false :reporter | :developer | true | false :anonymous | :developer | true | false :developer | :maintainer | true | false :maintainer | :maintainer | true | true :owner | :maintainer | true | true :guest | :maintainer | true | false + :planner | :maintainer | true | false :reporter | :maintainer | true | false :anonymous | :maintainer | true | false :developer | :owner | true | false :maintainer | :owner | true | false :owner | :owner | true | true :guest | :owner | true | false + :planner | :owner | true | false :reporter | :owner | true | false :anonymous | :owner | true | false :developer | :no_one_allowed | false | true :maintainer | :no_one_allowed | false | true :owner | :no_one_allowed | false | true :guest | :no_one_allowed | false | true + :planner | :no_one_allowed | false | true :reporter | :no_one_allowed | false | true :anonymous | :no_one_allowed | false | true :developer | :developer | false | true :maintainer | :developer | false | true :owner | :developer | false | true :guest | :developer | false | true + :planner | :developer | false | true :reporter | :developer | false | true :anonymous | :developer | false | true :developer | :maintainer | false | true :maintainer | :maintainer | false | true :owner | :maintainer | false | true :guest | :maintainer | false | true + :planner | :maintainer | false | true :reporter | :maintainer | false | true :anonymous | :maintainer | false | true :developer | :owner | false | true :maintainer | :owner | false | true :owner | :owner | false | true :guest | :owner | false | true + :planner | :owner | false | true :reporter | :owner | false | true :anonymous | :owner | false | true end @@ -1152,16 +1199,12 @@ def set_access_level(access_level) it { is_expected.to be_allowed(:read_deployment) } end - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:metrics_dashboard) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous guest planner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:metrics_dashboard) } + it { is_expected.to be_disallowed(:metrics_dashboard) } + end end end @@ -1178,20 +1221,14 @@ def set_access_level(access_level) it { is_expected.to be_allowed(:read_deployment) } end - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_allowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } - it { is_expected.to be_allowed(:read_deployment) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous guest planner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } - it { is_expected.to be_allowed(:read_deployment) } + it { is_expected.to be_allowed(:metrics_dashboard) } + it { is_expected.to be_disallowed(:read_prometheus) } + it { is_expected.to be_allowed(:read_deployment) } + end end end end @@ -1208,18 +1245,13 @@ def set_access_level(access_level) it { is_expected.to be_allowed(:read_deployment) } end - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous guest planner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } + it { is_expected.to be_disallowed(:metrics_dashboard) } + it { is_expected.to be_disallowed(:read_prometheus) } + end end end @@ -1236,12 +1268,14 @@ def set_access_level(access_level) it { is_expected.to be_allowed(:read_deployment) } end - context 'with guest' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } - it { is_expected.to be_allowed(:read_deployment) } + it { is_expected.to be_allowed(:metrics_dashboard) } + it { is_expected.to be_disallowed(:read_prometheus) } + it { is_expected.to be_allowed(:read_deployment) } + end end context 'with anonymous' do @@ -1265,18 +1299,13 @@ def set_access_level(access_level) it { is_expected.to be_allowed(:read_deployment) } end - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous guest planner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } + it { is_expected.to be_disallowed(:metrics_dashboard) } + it { is_expected.to be_disallowed(:read_prometheus) } + end end end @@ -1289,18 +1318,13 @@ def set_access_level(access_level) it { is_expected.to be_allowed(:read_deployment) } end - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous guest planner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:metrics_dashboard) } - it { is_expected.to be_disallowed(:read_prometheus) } + it { is_expected.to be_disallowed(:metrics_dashboard) } + it { is_expected.to be_disallowed(:read_prometheus) } + end end end end @@ -1310,22 +1334,12 @@ def set_access_level(access_level) project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::DISABLED) end - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_disallowed(:metrics_dashboard) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:metrics_dashboard) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous guest planner reporter].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:metrics_dashboard) } + it { is_expected.to be_disallowed(:metrics_dashboard) } + end end end end @@ -1507,60 +1521,42 @@ def set_access_level(access_level) it { is_expected.to be_allowed(:create_web_ide_terminal) } end - context 'with developer' do - let(:current_user) { developer } - - it { is_expected.to be_disallowed(:create_web_ide_terminal) } - end - - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_disallowed(:create_web_ide_terminal) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:create_web_ide_terminal) } - end - - context 'with non member' do - let(:current_user) { non_member } - - it { is_expected.to be_disallowed(:create_web_ide_terminal) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous non_member guest planner reporter developer].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:create_web_ide_terminal) } + it { is_expected.to be_disallowed(:create_web_ide_terminal) } + end end end describe 'read_repository_graphs' do - let(:current_user) { guest } + %w[guest planner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - before do - allow(subject).to receive(:allowed?).with(:read_repository_graphs).and_call_original - allow(subject).to receive(:allowed?).with(:download_code).and_return(can_download_code) - end + before do + allow(subject).to receive(:allowed?).with(:read_repository_graphs).and_call_original + allow(subject).to receive(:allowed?).with(:download_code).and_return(can_download_code) + end - context 'when user can download_code' do - let(:can_download_code) { true } + context 'when user can download_code' do + let(:can_download_code) { true } - it { is_expected.to be_allowed(:read_repository_graphs) } - end + it { is_expected.to be_allowed(:read_repository_graphs) } + end - context 'when user cannot download_code' do - let(:can_download_code) { false } + context 'when user cannot download_code' do + let(:can_download_code) { false } - it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + end + end end end context 'security configuration feature' do - %w[guest reporter].each do |role| + %w[guest planner reporter].each do |role| context role do let(:current_user) { send(role) } @@ -1582,7 +1578,7 @@ def set_access_level(access_level) end context 'infrastructure google cloud feature' do - %w[guest reporter developer].each do |role| + %w[guest planner reporter developer].each do |role| context role do let(:current_user) { send(role) } @@ -1604,7 +1600,7 @@ def set_access_level(access_level) end context 'infrastructure aws feature' do - %w[guest reporter developer].each do |role| + %w[guest planner reporter developer].each do |role| context role do let(:current_user) { send(role) } @@ -1631,8 +1627,8 @@ def set_access_level(access_level) let(:current_user) { reporter } let(:guest_design_abilities) { %i[read_design read_design_activity] } - let(:reporter_design_abilities) { %i[create_design destroy_design move_design update_design] } - let(:design_abilities) { guest_design_abilities + reporter_design_abilities } + let(:reporter_and_planner_design_abilities) { %i[create_design destroy_design move_design update_design] } + let(:design_abilities) { guest_design_abilities + reporter_and_planner_design_abilities } context 'when design management is not available' do before do @@ -1649,11 +1645,19 @@ def set_access_level(access_level) it { is_expected.to be_allowed(*design_abilities) } - context 'when user has below reporter access' do + %w[planner reporter].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } + + it { is_expected.to be_allowed(*design_abilities) } + end + end + + context 'when user is a guest' do let(:current_user) { guest } it { is_expected.to be_allowed(*guest_design_abilities) } - it { is_expected.not_to be_allowed(*reporter_design_abilities) } + it { is_expected.not_to be_allowed(*reporter_and_planner_design_abilities) } end end end @@ -1705,46 +1709,12 @@ def set_access_level(access_level) it_behaves_like 'package access with repository disabled' end - context 'with owner' do - let(:current_user) { owner } - - it { is_expected.to be_allowed(:read_package) } - end - - context 'with maintainer' do - let(:current_user) { maintainer } - - it { is_expected.to be_allowed(:read_package) } - end - - context 'with developer' do - let(:current_user) { developer } - - it { is_expected.to be_allowed(:read_package) } - end - - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_allowed(:read_package) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_allowed(:read_package) } - end - - context 'with non member' do - let(:current_user) { non_member } - - it { is_expected.to be_allowed(:read_package) } - end - - context 'with anonymous' do - let(:current_user) { anonymous } + %w[anonymous non_member guest planner reporter developer maintainer owner].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:read_package) } + it { is_expected.to be_allowed(:read_package) } + end end end @@ -1769,7 +1739,7 @@ def set_access_level(access_level) end end - %i[developer reporter guest non_member anonymous].each do |role| + %i[developer reporter planner guest non_member anonymous].each do |role| context "with #{role}" do let(:current_user) { public_send(role) } @@ -1809,7 +1779,7 @@ def set_access_level(access_level) end end - %i[developer reporter guest non_member anonymous].each do |role| + %i[developer reporter planner guest non_member anonymous].each do |role| context "with #{role}" do let(:current_user) { public_send(role) } @@ -1835,7 +1805,7 @@ def set_access_level(access_level) end end - %i[owner maintainer developer reporter guest non_member anonymous].each do |role| + %i[owner maintainer developer reporter planner guest non_member anonymous].each do |role| context "with #{role}" do let(:current_user) { public_send(role) } @@ -1875,7 +1845,7 @@ def set_access_level(access_level) end end - %i[developer reporter guest non_member anonymous].each do |role| + %i[developer reporter planner guest non_member anonymous].each do |role| context "with #{role}" do let(:current_user) { public_send(role) } @@ -1901,7 +1871,7 @@ def set_access_level(access_level) end end - %i[owner maintainer developer reporter guest non_member anonymous].each do |role| + %i[owner maintainer developer reporter planner guest non_member anonymous].each do |role| context "with #{role}" do let(:current_user) { public_send(role) } @@ -1929,7 +1899,7 @@ def set_access_level(access_level) end end - %i[owner maintainer developer reporter guest non_member anonymous].each do |role| + %i[owner maintainer developer reporter planner guest non_member anonymous].each do |role| context "with #{role}" do let(:current_user) { public_send(role) } @@ -1996,6 +1966,10 @@ def set_access_level(access_level) project_with_analytics_private.add_guest(guest) project_with_analytics_enabled.add_guest(guest) + project_with_analytics_disabled.add_guest(planner) + project_with_analytics_private.add_guest(planner) + project_with_analytics_enabled.add_guest(planner) + project_with_analytics_disabled.add_reporter(reporter) project_with_analytics_private.add_reporter(reporter) project_with_analytics_enabled.add_reporter(reporter) @@ -2008,93 +1982,67 @@ def set_access_level(access_level) context 'when analytics is disabled for the project' do let(:project) { project_with_analytics_disabled } - context 'for guest user' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:read_cycle_analytics) } - it { is_expected.to be_disallowed(:read_insights) } - it { is_expected.to be_disallowed(:read_repository_graphs) } - it { is_expected.to be_disallowed(:read_ci_cd_analytics) } - end - - context 'for reporter user' do - let(:current_user) { reporter } + %w[guest planner reporter developer].each do |role| + context "for #{role} user" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:read_cycle_analytics) } - it { is_expected.to be_disallowed(:read_insights) } - it { is_expected.to be_disallowed(:read_repository_graphs) } - it { is_expected.to be_disallowed(:read_ci_cd_analytics) } - end - - context 'for developer' do - let(:current_user) { developer } - - it { is_expected.to be_disallowed(:read_cycle_analytics) } - it { is_expected.to be_disallowed(:read_insights) } - it { is_expected.to be_disallowed(:read_repository_graphs) } - it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + it { is_expected.to be_disallowed(:read_cycle_analytics) } + it { is_expected.to be_disallowed(:read_insights) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end end end context 'when analytics is private for the project' do let(:project) { project_with_analytics_private } - context 'for guest user' do - let(:current_user) { guest } - - it { is_expected.to be_allowed(:read_cycle_analytics) } - it { is_expected.to be_allowed(:read_insights) } - it { is_expected.to be_disallowed(:read_repository_graphs) } - it { is_expected.to be_disallowed(:read_ci_cd_analytics) } - end - - context 'for reporter user' do - let(:current_user) { reporter } + %w[guest planner].each do |role| + context "for #{role} user" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:read_cycle_analytics) } - it { is_expected.to be_allowed(:read_insights) } - it { is_expected.to be_allowed(:read_repository_graphs) } - it { is_expected.to be_allowed(:read_ci_cd_analytics) } + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end end - context 'for developer' do - let(:current_user) { developer } + %w[reporter developer].each do |role| + context "for #{role} user" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:read_cycle_analytics) } - it { is_expected.to be_allowed(:read_insights) } - it { is_expected.to be_allowed(:read_repository_graphs) } - it { is_expected.to be_allowed(:read_ci_cd_analytics) } + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } + it { is_expected.to be_allowed(:read_repository_graphs) } + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end end end context 'when analytics is enabled for the project' do let(:project) { project_with_analytics_enabled } - context 'for guest user' do - let(:current_user) { guest } - - it { is_expected.to be_allowed(:read_cycle_analytics) } - it { is_expected.to be_allowed(:read_insights) } - it { is_expected.to be_disallowed(:read_repository_graphs) } - it { is_expected.to be_disallowed(:read_ci_cd_analytics) } - end + %w[guest planner].each do |role| + context "for #{role} user" do + let(:current_user) { send(role) } - context 'for reporter user' do - let(:current_user) { reporter } - - it { is_expected.to be_allowed(:read_cycle_analytics) } - it { is_expected.to be_allowed(:read_insights) } - it { is_expected.to be_allowed(:read_repository_graphs) } - it { is_expected.to be_allowed(:read_ci_cd_analytics) } + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end end - context 'for developer' do - let(:current_user) { developer } + %w[reporter developer].each do |role| + context "for #{role} user" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:read_cycle_analytics) } - it { is_expected.to be_allowed(:read_insights) } - it { is_expected.to be_allowed(:read_repository_graphs) } - it { is_expected.to be_allowed(:read_ci_cd_analytics) } + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } + it { is_expected.to be_allowed(:read_repository_graphs) } + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end end end end @@ -2102,7 +2050,7 @@ def set_access_level(access_level) context 'project member' do let(:project) { private_project } - %w[guest reporter developer maintainer].each do |role| + %w[guest planner reporter developer maintainer].each do |role| context role do let(:current_user) { send(role) } @@ -2131,13 +2079,13 @@ def set_access_level(access_level) end context 'project member' do - %w[guest reporter developer maintainer].each do |role| + %w[guest planner reporter developer maintainer].each do |role| context role do before do project.add_member(current_user, role.to_sym) end - if role == 'guest' + if role == 'guest' || role == 'planner' it { is_expected.to be_disallowed(:read_ci_cd_analytics) } else it { is_expected.to be_allowed(:read_ci_cd_analytics) } @@ -2165,7 +2113,7 @@ def set_access_level(access_level) end context 'project member' do - %w[guest reporter developer maintainer].each do |role| + %w[guest planner reporter developer maintainer].each do |role| context role do before do project.add_member(current_user, role.to_sym) @@ -2195,13 +2143,13 @@ def set_access_level(access_level) let(:current_user) { create(:user) } context 'project member' do - %w[guest reporter developer maintainer].each do |role| + %w[guest planner reporter developer maintainer].each do |role| context role do before do project.add_member(current_user, role.to_sym) end - if role == 'guest' + if role == 'guest' || role == 'planner' it { is_expected.to be_disallowed(:read_ci_cd_analytics) } else it { is_expected.to be_allowed(:read_ci_cd_analytics) } @@ -2244,38 +2192,47 @@ def set_access_level(access_level) where(:project_visibility, :access_level, :role, :allowed) do :public | ProjectFeature::ENABLED | :maintainer | true :public | ProjectFeature::ENABLED | :developer | true + :public | ProjectFeature::ENABLED | :planner | true :public | ProjectFeature::ENABLED | :guest | true :public | ProjectFeature::ENABLED | :anonymous | true :public | ProjectFeature::PRIVATE | :maintainer | true :public | ProjectFeature::PRIVATE | :developer | true + :public | ProjectFeature::PRIVATE | :planner | false :public | ProjectFeature::PRIVATE | :guest | false :public | ProjectFeature::PRIVATE | :anonymous | false :public | ProjectFeature::DISABLED | :maintainer | false :public | ProjectFeature::DISABLED | :developer | false + :public | ProjectFeature::DISABLED | :planner | false :public | ProjectFeature::DISABLED | :guest | false :public | ProjectFeature::DISABLED | :anonymous | false :internal | ProjectFeature::ENABLED | :maintainer | true :internal | ProjectFeature::ENABLED | :developer | true + :internal | ProjectFeature::ENABLED | :planner | true :internal | ProjectFeature::ENABLED | :guest | true :internal | ProjectFeature::ENABLED | :anonymous | false :internal | ProjectFeature::PRIVATE | :maintainer | true :internal | ProjectFeature::PRIVATE | :developer | true + :internal | ProjectFeature::PRIVATE | :planner | false :internal | ProjectFeature::PRIVATE | :guest | false :internal | ProjectFeature::PRIVATE | :anonymous | false :internal | ProjectFeature::DISABLED | :maintainer | false :internal | ProjectFeature::DISABLED | :developer | false + :internal | ProjectFeature::DISABLED | :planner | false :internal | ProjectFeature::DISABLED | :guest | false :internal | ProjectFeature::DISABLED | :anonymous | false :private | ProjectFeature::ENABLED | :maintainer | true :private | ProjectFeature::ENABLED | :developer | true + :private | ProjectFeature::ENABLED | :planner | false :private | ProjectFeature::ENABLED | :guest | false :private | ProjectFeature::ENABLED | :anonymous | false :private | ProjectFeature::PRIVATE | :maintainer | true :private | ProjectFeature::PRIVATE | :developer | true + :private | ProjectFeature::PRIVATE | :planner | false :private | ProjectFeature::PRIVATE | :guest | false :private | ProjectFeature::PRIVATE | :anonymous | false :private | ProjectFeature::DISABLED | :maintainer | false :private | ProjectFeature::DISABLED | :developer | false + :private | ProjectFeature::DISABLED | :planner | false :private | ProjectFeature::DISABLED | :guest | false :private | ProjectFeature::DISABLED | :anonymous | false end @@ -2313,38 +2270,47 @@ def set_access_level(access_level) where(:project_visibility, :access_level, :role, :allowed) do :public | ProjectFeature::ENABLED | :maintainer | true :public | ProjectFeature::ENABLED | :developer | true + :public | ProjectFeature::ENABLED | :planne | true :public | ProjectFeature::ENABLED | :guest | true :public | ProjectFeature::ENABLED | :anonymous | true :public | ProjectFeature::PRIVATE | :maintainer | true :public | ProjectFeature::PRIVATE | :developer | true + :public | ProjectFeature::PRIVATE | :planne | true :public | ProjectFeature::PRIVATE | :guest | true :public | ProjectFeature::PRIVATE | :anonymous | false :public | ProjectFeature::DISABLED | :maintainer | false :public | ProjectFeature::DISABLED | :developer | false + :public | ProjectFeature::DISABLED | :planner | false :public | ProjectFeature::DISABLED | :guest | false :public | ProjectFeature::DISABLED | :anonymous | false :internal | ProjectFeature::ENABLED | :maintainer | true :internal | ProjectFeature::ENABLED | :developer | true + :internal | ProjectFeature::ENABLED | :planner | true :internal | ProjectFeature::ENABLED | :guest | true :internal | ProjectFeature::ENABLED | :anonymous | false :internal | ProjectFeature::PRIVATE | :maintainer | true :internal | ProjectFeature::PRIVATE | :developer | true + :internal | ProjectFeature::PRIVATE | :planner | true :internal | ProjectFeature::PRIVATE | :guest | true :internal | ProjectFeature::PRIVATE | :anonymous | false :internal | ProjectFeature::DISABLED | :maintainer | false :internal | ProjectFeature::DISABLED | :developer | false + :internal | ProjectFeature::DISABLED | :planner | false :internal | ProjectFeature::DISABLED | :guest | false :internal | ProjectFeature::DISABLED | :anonymous | false :private | ProjectFeature::ENABLED | :maintainer | true :private | ProjectFeature::ENABLED | :developer | true + :private | ProjectFeature::ENABLED | :planner | false :private | ProjectFeature::ENABLED | :guest | false :private | ProjectFeature::ENABLED | :anonymous | false :private | ProjectFeature::PRIVATE | :maintainer | true :private | ProjectFeature::PRIVATE | :developer | true + :private | ProjectFeature::PRIVATE | :planner | false :private | ProjectFeature::PRIVATE | :guest | false :private | ProjectFeature::PRIVATE | :anonymous | false :private | ProjectFeature::DISABLED | :maintainer | false :private | ProjectFeature::DISABLED | :developer | false + :private | ProjectFeature::DISABLED | :planner | false :private | ProjectFeature::DISABLED | :guest | false :private | ProjectFeature::DISABLED | :anonymous | false end @@ -2384,38 +2350,47 @@ def set_access_level(access_level) where(:project_visibility, :access_level, :role, :allowed) do :public | ProjectFeature::ENABLED | :maintainer | true :public | ProjectFeature::ENABLED | :developer | true + :public | ProjectFeature::ENABLED | :planner | true :public | ProjectFeature::ENABLED | :guest | true :public | ProjectFeature::ENABLED | :anonymous | true :public | ProjectFeature::PRIVATE | :maintainer | true :public | ProjectFeature::PRIVATE | :developer | true + :public | ProjectFeature::PRIVATE | :planner | true :public | ProjectFeature::PRIVATE | :guest | true :public | ProjectFeature::PRIVATE | :anonymous | false :public | ProjectFeature::DISABLED | :maintainer | false :public | ProjectFeature::DISABLED | :developer | false + :public | ProjectFeature::DISABLED | :planner | false :public | ProjectFeature::DISABLED | :guest | false :public | ProjectFeature::DISABLED | :anonymous | false :internal | ProjectFeature::ENABLED | :maintainer | true :internal | ProjectFeature::ENABLED | :developer | true + :internal | ProjectFeature::ENABLED | :planner | true :internal | ProjectFeature::ENABLED | :guest | true :internal | ProjectFeature::ENABLED | :anonymous | false :internal | ProjectFeature::PRIVATE | :maintainer | true :internal | ProjectFeature::PRIVATE | :developer | true + :internal | ProjectFeature::PRIVATE | :planner | true :internal | ProjectFeature::PRIVATE | :guest | true :internal | ProjectFeature::PRIVATE | :anonymous | false :internal | ProjectFeature::DISABLED | :maintainer | false :internal | ProjectFeature::DISABLED | :developer | false + :internal | ProjectFeature::DISABLED | :planner | false :internal | ProjectFeature::DISABLED | :guest | false :internal | ProjectFeature::DISABLED | :anonymous | false :private | ProjectFeature::ENABLED | :maintainer | true :private | ProjectFeature::ENABLED | :developer | true + :private | ProjectFeature::ENABLED | :planner | false :private | ProjectFeature::ENABLED | :guest | false :private | ProjectFeature::ENABLED | :anonymous | false :private | ProjectFeature::PRIVATE | :maintainer | true :private | ProjectFeature::PRIVATE | :developer | true + :private | ProjectFeature::PRIVATE | :planner | false :private | ProjectFeature::PRIVATE | :guest | false :private | ProjectFeature::PRIVATE | :anonymous | false :private | ProjectFeature::DISABLED | :maintainer | false :private | ProjectFeature::DISABLED | :developer | false + :private | ProjectFeature::DISABLED | :planner | false :private | ProjectFeature::DISABLED | :guest | false :private | ProjectFeature::DISABLED | :anonymous | false end @@ -2452,38 +2427,47 @@ def set_access_level(access_level) where(:project_visibility, :access_level, :role, :allowed) do :public | ProjectFeature::ENABLED | :maintainer | true :public | ProjectFeature::ENABLED | :developer | true + :public | ProjectFeature::ENABLED | :planner | true :public | ProjectFeature::ENABLED | :guest | true :public | ProjectFeature::ENABLED | :anonymous | true :public | ProjectFeature::PRIVATE | :maintainer | true :public | ProjectFeature::PRIVATE | :developer | true + :public | ProjectFeature::PRIVATE | :planner | true :public | ProjectFeature::PRIVATE | :guest | true :public | ProjectFeature::PRIVATE | :anonymous | false :public | ProjectFeature::DISABLED | :maintainer | false :public | ProjectFeature::DISABLED | :developer | false + :public | ProjectFeature::DISABLED | :planner | false :public | ProjectFeature::DISABLED | :guest | false :public | ProjectFeature::DISABLED | :anonymous | false :internal | ProjectFeature::ENABLED | :maintainer | true :internal | ProjectFeature::ENABLED | :developer | true + :internal | ProjectFeature::ENABLED | :planner | true :internal | ProjectFeature::ENABLED | :guest | true :internal | ProjectFeature::ENABLED | :anonymous | false :internal | ProjectFeature::PRIVATE | :maintainer | true :internal | ProjectFeature::PRIVATE | :developer | true + :internal | ProjectFeature::PRIVATE | :planner | true :internal | ProjectFeature::PRIVATE | :guest | true :internal | ProjectFeature::PRIVATE | :anonymous | false :internal | ProjectFeature::DISABLED | :maintainer | false :internal | ProjectFeature::DISABLED | :developer | false + :internal | ProjectFeature::DISABLED | :planner | false :internal | ProjectFeature::DISABLED | :guest | false :internal | ProjectFeature::DISABLED | :anonymous | false :private | ProjectFeature::ENABLED | :maintainer | true :private | ProjectFeature::ENABLED | :developer | true + :private | ProjectFeature::ENABLED | :planner | true :private | ProjectFeature::ENABLED | :guest | true :private | ProjectFeature::ENABLED | :anonymous | false :private | ProjectFeature::PRIVATE | :maintainer | true :private | ProjectFeature::PRIVATE | :developer | true + :private | ProjectFeature::PRIVATE | :planner | true :private | ProjectFeature::PRIVATE | :guest | true :private | ProjectFeature::PRIVATE | :anonymous | false :private | ProjectFeature::DISABLED | :maintainer | false :private | ProjectFeature::DISABLED | :developer | false + :private | ProjectFeature::DISABLED | :planner | false :private | ProjectFeature::DISABLED | :guest | false :private | ProjectFeature::DISABLED | :anonymous | false end @@ -2512,6 +2496,7 @@ def set_access_level(access_level) :maintainer | true :developer | true :reporter | false + :planner | false :guest | false end @@ -2547,38 +2532,47 @@ def set_access_level(access_level) where(:project_visibility, :access_level, :role, :allowed) do :public | ProjectFeature::ENABLED | :maintainer | true :public | ProjectFeature::ENABLED | :developer | true + :public | ProjectFeature::ENABLED | :planner | true :public | ProjectFeature::ENABLED | :guest | true :public | ProjectFeature::ENABLED | :anonymous | true :public | ProjectFeature::PRIVATE | :maintainer | true :public | ProjectFeature::PRIVATE | :developer | true + :public | ProjectFeature::PRIVATE | :planner | true :public | ProjectFeature::PRIVATE | :guest | true :public | ProjectFeature::PRIVATE | :anonymous | false :public | ProjectFeature::DISABLED | :maintainer | false :public | ProjectFeature::DISABLED | :developer | false + :public | ProjectFeature::DISABLED | :planner | false :public | ProjectFeature::DISABLED | :guest | false :public | ProjectFeature::DISABLED | :anonymous | false :internal | ProjectFeature::ENABLED | :maintainer | true :internal | ProjectFeature::ENABLED | :developer | true + :internal | ProjectFeature::ENABLED | :planner | true :internal | ProjectFeature::ENABLED | :guest | true :internal | ProjectFeature::ENABLED | :anonymous | false :internal | ProjectFeature::PRIVATE | :maintainer | true :internal | ProjectFeature::PRIVATE | :developer | true + :internal | ProjectFeature::PRIVATE | :planner | true :internal | ProjectFeature::PRIVATE | :guest | true :internal | ProjectFeature::PRIVATE | :anonymous | false :internal | ProjectFeature::DISABLED | :maintainer | false :internal | ProjectFeature::DISABLED | :developer | false + :internal | ProjectFeature::DISABLED | :planner | false :internal | ProjectFeature::DISABLED | :guest | false :internal | ProjectFeature::DISABLED | :anonymous | false :private | ProjectFeature::ENABLED | :maintainer | true :private | ProjectFeature::ENABLED | :developer | true + :private | ProjectFeature::ENABLED | :planner | true :private | ProjectFeature::ENABLED | :guest | true :private | ProjectFeature::ENABLED | :anonymous | false :private | ProjectFeature::PRIVATE | :maintainer | true :private | ProjectFeature::PRIVATE | :developer | true + :private | ProjectFeature::PRIVATE | :planner | true :private | ProjectFeature::PRIVATE | :guest | true :private | ProjectFeature::PRIVATE | :anonymous | false :private | ProjectFeature::DISABLED | :maintainer | false :private | ProjectFeature::DISABLED | :developer | false + :private | ProjectFeature::DISABLED | :planner | false :private | ProjectFeature::DISABLED | :guest | false :private | ProjectFeature::DISABLED | :anonymous | false end @@ -2655,7 +2649,7 @@ def set_access_level(access_level) end end - %w[reporter guest].each do |role| + %w[reporter planner guest].each do |role| context "when the role is #{role}" do let(:current_user) { public_send(role) } @@ -2681,7 +2675,7 @@ def set_access_level(access_level) project.project_feature.update!(security_and_compliance_access_level: Featurable::DISABLED) end - %w[owner maintainer developer reporter guest].each do |role| + %w[owner maintainer developer reporter planner guest].each do |role| context "when the role is #{role}" do let(:current_user) { public_send(role) } @@ -2762,6 +2756,12 @@ def set_access_level(access_level) :reporter | false | :different | true | false :reporter | true | :different | true | false :reporter | false | :different | false | true + :planner | false | :same | true | true + :planner | true | :same | true | true + :planner | false | :same | false | true + :planner | false | :different | true | false + :planner | true | :different | true | false + :planner | false | :different | false | true :guest | false | :same | true | true :guest | true | :same | true | true :guest | false | :same | false | true @@ -2821,72 +2821,84 @@ def set_access_level(access_level) where(:project_visibility, :external_user, :token_scope_enabled, :role, :allowed) do :private | false | false | :anonymous | false + :private | false | false | :planner | true :private | false | false | :guest | true :private | false | false | :reporter | true :private | false | false | :developer | true :private | false | false | :maintainer | true :private | false | false | :owner | true :public | false | false | :anonymous | false + :public | false | false | :planner | true :public | false | false | :guest | true :public | false | false | :reporter | true :public | false | false | :developer | true :public | false | false | :maintainer | true :public | false | false | :owner | true :internal | false | false | :anonymous | false + :internal | false | false | :planner | true :internal | false | false | :guest | true :internal | false | false | :reporter | true :internal | false | false | :developer | true :internal | false | false | :maintainer | true :internal | false | false | :owner | true :private | true | false | :anonymous | false + :private | true | false | :planner | false :private | true | false | :guest | false :private | true | false | :reporter | false :private | true | false | :developer | false :private | true | false | :maintainer | false :private | true | false | :owner | false :public | true | false | :anonymous | false + :public | true | false | :planner | false :public | true | false | :guest | false :public | true | false | :reporter | false :public | true | false | :developer | false :public | true | false | :maintainer | false :public | true | false | :owner | false :internal | true | false | :anonymous | false + :internal | true | false | :planner | false :internal | true | false | :guest | false :internal | true | false | :reporter | false :internal | true | false | :developer | false :internal | true | false | :maintainer | false :internal | true | false | :owner | false :private | false | true | :anonymous | false + :private | false | true | :planner | true :private | false | true | :guest | true :private | false | true | :reporter | true :private | false | true | :developer | true :private | false | true | :maintainer | true :private | false | true | :owner | true :public | false | true | :anonymous | false + :public | false | true | :planner | true :public | false | true | :guest | true :public | false | true | :reporter | true :public | false | true | :developer | true :public | false | true | :maintainer | true :public | false | true | :owner | true :internal | false | true | :anonymous | false + :internal | false | true | :planner | true :internal | false | true | :guest | true :internal | false | true | :reporter | true :internal | false | true | :developer | true :internal | false | true | :maintainer | true :internal | false | true | :owner | true :private | true | true | :anonymous | false + :private | true | true | :planner | false :private | true | true | :guest | false :private | true | true | :reporter | false :private | true | true | :developer | false :private | true | true | :maintainer | false :private | true | true | :owner | false :public | true | true | :anonymous | false + :public | true | true | :planner | false :public | true | true | :guest | false :public | true | true | :reporter | false :public | true | true | :developer | false :public | true | true | :maintainer | false :public | true | true | :owner | false :internal | true | true | :anonymous | false + :internal | true | true | :planner | false :internal | true | true | :guest | false :internal | true | true | :reporter | false :internal | true | true | :developer | false @@ -2956,6 +2968,7 @@ def set_access_level(access_level) :public | ProjectFeature::ENABLED | :maintainer | true :public | ProjectFeature::ENABLED | :developer | true :public | ProjectFeature::ENABLED | :reporter | true + :public | ProjectFeature::ENABLED | :planner | true :public | ProjectFeature::ENABLED | :guest | true :public | ProjectFeature::ENABLED | :anonymous | true :public | ProjectFeature::PRIVATE | :admin | true @@ -2963,6 +2976,7 @@ def set_access_level(access_level) :public | ProjectFeature::PRIVATE | :maintainer | true :public | ProjectFeature::PRIVATE | :developer | true :public | ProjectFeature::PRIVATE | :reporter | true + :public | ProjectFeature::PRIVATE | :planner | false :public | ProjectFeature::PRIVATE | :guest | false :public | ProjectFeature::PRIVATE | :anonymous | false :public | ProjectFeature::DISABLED | :admin | false @@ -2970,6 +2984,7 @@ def set_access_level(access_level) :public | ProjectFeature::DISABLED | :maintainer | false :public | ProjectFeature::DISABLED | :developer | false :public | ProjectFeature::DISABLED | :reporter | false + :public | ProjectFeature::DISABLED | :planner | false :public | ProjectFeature::DISABLED | :guest | false :public | ProjectFeature::DISABLED | :anonymous | false :internal | ProjectFeature::ENABLED | :admin | true @@ -2977,6 +2992,7 @@ def set_access_level(access_level) :internal | ProjectFeature::ENABLED | :maintainer | true :internal | ProjectFeature::ENABLED | :developer | true :internal | ProjectFeature::ENABLED | :reporter | true + :internal | ProjectFeature::ENABLED | :planner | true :internal | ProjectFeature::ENABLED | :guest | true :internal | ProjectFeature::ENABLED | :anonymous | false :internal | ProjectFeature::PRIVATE | :admin | true @@ -2984,6 +3000,7 @@ def set_access_level(access_level) :internal | ProjectFeature::PRIVATE | :maintainer | true :internal | ProjectFeature::PRIVATE | :developer | true :internal | ProjectFeature::PRIVATE | :reporter | true + :internal | ProjectFeature::PRIVATE | :planner | false :internal | ProjectFeature::PRIVATE | :guest | false :internal | ProjectFeature::PRIVATE | :anonymous | false :internal | ProjectFeature::DISABLED | :admin | false @@ -2991,6 +3008,7 @@ def set_access_level(access_level) :internal | ProjectFeature::DISABLED | :maintainer | false :internal | ProjectFeature::DISABLED | :developer | false :internal | ProjectFeature::DISABLED | :reporter | false + :internal | ProjectFeature::DISABLED | :planner | false :internal | ProjectFeature::DISABLED | :guest | false :internal | ProjectFeature::DISABLED | :anonymous | false :private | ProjectFeature::ENABLED | :admin | true @@ -2998,6 +3016,7 @@ def set_access_level(access_level) :private | ProjectFeature::ENABLED | :maintainer | true :private | ProjectFeature::ENABLED | :developer | true :private | ProjectFeature::ENABLED | :reporter | true + :private | ProjectFeature::ENABLED | :planner | false :private | ProjectFeature::ENABLED | :guest | false :private | ProjectFeature::ENABLED | :anonymous | false :private | ProjectFeature::PRIVATE | :admin | true @@ -3005,6 +3024,7 @@ def set_access_level(access_level) :private | ProjectFeature::PRIVATE | :maintainer | true :private | ProjectFeature::PRIVATE | :developer | true :private | ProjectFeature::PRIVATE | :reporter | true + :private | ProjectFeature::PRIVATE | :planner | false :private | ProjectFeature::PRIVATE | :guest | false :private | ProjectFeature::PRIVATE | :anonymous | false :private | ProjectFeature::DISABLED | :admin | false @@ -3012,6 +3032,7 @@ def set_access_level(access_level) :private | ProjectFeature::DISABLED | :maintainer | false :private | ProjectFeature::DISABLED | :developer | false :private | ProjectFeature::DISABLED | :reporter | false + :private | ProjectFeature::DISABLED | :planner | false :private | ProjectFeature::DISABLED | :guest | false :private | ProjectFeature::DISABLED | :anonymous | false end @@ -3022,6 +3043,7 @@ def set_access_level(access_level) before do enable_admin_mode!(admin) if role == :admin + allow(current_user).to receive(:external?).and_return(false) project.project_feature.update!(container_registry_access_level: access_level) end @@ -3043,23 +3065,35 @@ def set_access_level(access_level) end end - context 'with external guest users' do - where(:project_visibility, :access_level, :allowed) do - :public | ProjectFeature::ENABLED | true - :public | ProjectFeature::PRIVATE | false - :public | ProjectFeature::DISABLED | false + context 'with external guest and planner users' do + where(:project_visibility, :access_level, :role, :allowed) do + :public | ProjectFeature::ENABLED | :guest | true + :public | ProjectFeature::PRIVATE | :guest | false + :public | ProjectFeature::DISABLED | :guest | false + + :internal | ProjectFeature::ENABLED | :guest | true + :internal | ProjectFeature::PRIVATE | :guest | false + :internal | ProjectFeature::DISABLED | :guest | false + + :private | ProjectFeature::ENABLED | :guest | false + :private | ProjectFeature::PRIVATE | :guest | false + :private | ProjectFeature::DISABLED | :guest | false + + :public | ProjectFeature::ENABLED | :planner | true + :public | ProjectFeature::PRIVATE | :planner | false + :public | ProjectFeature::DISABLED | :planner | false - :internal | ProjectFeature::ENABLED | true - :internal | ProjectFeature::PRIVATE | false - :internal | ProjectFeature::DISABLED | false + :internal | ProjectFeature::ENABLED | :planner | true + :internal | ProjectFeature::PRIVATE | :planner | false + :internal | ProjectFeature::DISABLED | :planner | false - :private | ProjectFeature::ENABLED | false - :private | ProjectFeature::PRIVATE | false - :private | ProjectFeature::DISABLED | false + :private | ProjectFeature::ENABLED | :planner | false + :private | ProjectFeature::PRIVATE | :planner | false + :private | ProjectFeature::DISABLED | :planner | false end with_them do - let(:current_user) { guest } + let(:current_user) { send(role) } let(:project) { send("#{project_visibility}_project") } before do @@ -3091,7 +3125,7 @@ def permissions_abilities(role) maintainer_operations_permissions when :developer developer_operations_permissions - when :reporter, :guest + when :reporter, :guest, :planner guest_operations_permissions when :anonymous anonymous_operations_permissions @@ -3134,7 +3168,7 @@ def permissions_abilities(role) end end - %w[guest reporter developer].each do |role| + %w[guest planner reporter developer].each do |role| context role do let(:current_user) { send(role) } @@ -3240,28 +3274,12 @@ def permissions_abilities(role) end end - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_disallowed(:register_project_runners) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:register_project_runners) } - end - - context 'with non member' do - let(:current_user) { create(:user) } - - it { is_expected.to be_disallowed(:register_project_runners) } - end - - context 'with anonymous' do - let(:current_user) { nil } + %w[anonymous non_member guest planner reporter developer].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:register_project_runners) } + it { is_expected.to be_disallowed(:register_project_runners) } + end end end @@ -3322,28 +3340,12 @@ def permissions_abilities(role) it { is_expected.to be_allowed(:create_runner) } end - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_disallowed(:create_runner) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:create_runner) } - end - - context 'with developer' do - let(:current_user) { developer } - - it { is_expected.to be_disallowed(:create_runner) } - end - - context 'with anonymous' do - let(:current_user) { nil } + %w[anonymous guest planner reporter developer].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:create_runner) } + it { is_expected.to be_disallowed(:create_runner) } + end end end @@ -3372,28 +3374,12 @@ def permissions_abilities(role) it { is_expected.to be_allowed(:create_runner) } end - context 'with reporter' do - let(:current_user) { reporter } - - it { is_expected.to be_disallowed(:create_runner) } - end - - context 'with guest' do - let(:current_user) { guest } - - it { is_expected.to be_disallowed(:create_runner) } - end - - context 'with developer' do - let(:current_user) { developer } - - it { is_expected.to be_disallowed(:create_runner) } - end - - context 'with anonymous' do - let(:current_user) { nil } + %w[anonymous guest planner reporter developer].each do |role| + context "with #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_disallowed(:create_runner) } + it { is_expected.to be_disallowed(:create_runner) } + end end end @@ -3412,16 +3398,12 @@ def permissions_abilities(role) it { is_expected.to be_allowed(:read_project_runners) } end - context 'with reporter' do - let(:user) { reporter } - - it { is_expected.to be_disallowed(:read_project_runners) } - end - - context 'when the user is not part of the project' do - let(:user) { non_member } + %w[non_member guest planner reporter].each do |role| + context "with #{role}" do + let(:user) { send(role) } - it { is_expected.to be_disallowed(:read_project_runners) } + it { is_expected.to be_disallowed(:read_project_runners) } + end end end @@ -3433,6 +3415,7 @@ def permissions_abilities(role) :maintainer | true :developer | true :reporter | false + :planner | false :guest | false end @@ -3459,22 +3442,12 @@ def permissions_abilities(role) end context 'when user is an inherited member from the group' do - context 'and user is a guest' do - let(:current_user) { inherited_guest } - - it { is_expected.to be_allowed(:read_milestone) } - end - - context 'and user is a reporter' do - let(:current_user) { inherited_reporter } + %w[guest planner reporter developer].each do |role| + context "and user is a #{role}" do + let(:current_user) { send(role) } - it { is_expected.to be_allowed(:read_milestone) } - end - - context 'and user is a developer' do - let(:current_user) { inherited_developer } - - it { is_expected.to be_allowed(:read_milestone) } + it { is_expected.to be_allowed(:read_milestone) } + end end end end @@ -3495,6 +3468,7 @@ def permissions_abilities(role) :maintainer | true :developer | true :reporter | true + :planner | true :guest | true with_them do @@ -3513,6 +3487,7 @@ def permissions_abilities(role) :maintainer | true :developer | true :reporter | true + :planner | true :guest | false end @@ -3562,6 +3537,7 @@ def permissions_abilities(role) :maintainer | false :developer | false :reporter | false + :planner | false :guest | false end @@ -3578,21 +3554,25 @@ def permissions_abilities(role) where(:ability, :current_user, :access_level, :allowed) do :admin_pages | ref(:maintainer) | Featurable::ENABLED | true :admin_pages | ref(:reporter) | Featurable::ENABLED | false + :admin_pages | ref(:planner) | Featurable::ENABLED | false :admin_pages | ref(:guest) | Featurable::ENABLED | false :admin_pages | ref(:non_member) | Featurable::ENABLED | false :update_pages | ref(:maintainer) | Featurable::ENABLED | true :update_pages | ref(:reporter) | Featurable::ENABLED | false + :update_pages | ref(:planner) | Featurable::ENABLED | false :update_pages | ref(:guest) | Featurable::ENABLED | false :update_pages | ref(:non_member) | Featurable::ENABLED | false :remove_pages | ref(:maintainer) | Featurable::ENABLED | true :remove_pages | ref(:reporter) | Featurable::ENABLED | false + :remove_pages | ref(:planner) | Featurable::ENABLED | false :remove_pages | ref(:guest) | Featurable::ENABLED | false :remove_pages | ref(:non_member) | Featurable::ENABLED | false :read_pages | ref(:maintainer) | Featurable::ENABLED | true :read_pages | ref(:reporter) | Featurable::ENABLED | false + :read_pages | ref(:planner) | Featurable::ENABLED | false :read_pages | ref(:guest) | Featurable::ENABLED | false :read_pages | ref(:non_member) | Featurable::ENABLED | false @@ -3600,6 +3580,9 @@ def permissions_abilities(role) :read_pages_content | ref(:reporter) | Featurable::ENABLED | true :read_pages_content | ref(:reporter) | Featurable::PRIVATE | true :read_pages_content | ref(:reporter) | Featurable::DISABLED | false + :read_pages_content | ref(:planner) | Featurable::ENABLED | true + :read_pages_content | ref(:planner) | Featurable::PRIVATE | true + :read_pages_content | ref(:planner) | Featurable::DISABLED | false :read_pages_content | ref(:guest) | Featurable::ENABLED | true :read_pages_content | ref(:guest) | Featurable::PRIVATE | true :read_pages_content | ref(:guest) | Featurable::DISABLED | false @@ -3628,6 +3611,7 @@ def permissions_abilities(role) Featurable::DISABLED | ref(:anonymous) | false Featurable::DISABLED | ref(:non_member) | false Featurable::DISABLED | ref(:guest) | false + Featurable::DISABLED | ref(:planner) | false Featurable::DISABLED | ref(:reporter) | false Featurable::DISABLED | ref(:developer) | false Featurable::DISABLED | ref(:maintainer) | false @@ -3635,6 +3619,7 @@ def permissions_abilities(role) Featurable::ENABLED | ref(:anonymous) | true Featurable::ENABLED | ref(:non_member) | true Featurable::ENABLED | ref(:guest) | true + Featurable::ENABLED | ref(:planner) | true Featurable::ENABLED | ref(:reporter) | true Featurable::ENABLED | ref(:developer) | true Featurable::ENABLED | ref(:maintainer) | true @@ -3642,6 +3627,7 @@ def permissions_abilities(role) Featurable::PRIVATE | ref(:anonymous) | false Featurable::PRIVATE | ref(:non_member) | false Featurable::PRIVATE | ref(:guest) | true + Featurable::PRIVATE | ref(:planner) | true Featurable::PRIVATE | ref(:reporter) | true Featurable::PRIVATE | ref(:developer) | true Featurable::PRIVATE | ref(:maintainer) | true @@ -3669,6 +3655,7 @@ def permissions_abilities(role) Featurable::DISABLED | ref(:anonymous) | false Featurable::DISABLED | ref(:non_member) | false Featurable::DISABLED | ref(:guest) | false + Featurable::DISABLED | ref(:planner) | false Featurable::DISABLED | ref(:reporter) | false Featurable::DISABLED | ref(:developer) | false Featurable::DISABLED | ref(:maintainer) | false @@ -3676,6 +3663,7 @@ def permissions_abilities(role) Featurable::ENABLED | ref(:anonymous) | false Featurable::ENABLED | ref(:non_member) | false Featurable::ENABLED | ref(:guest) | true + Featurable::ENABLED | ref(:planner) | true Featurable::ENABLED | ref(:reporter) | true Featurable::ENABLED | ref(:developer) | true Featurable::ENABLED | ref(:maintainer) | true @@ -3683,6 +3671,7 @@ def permissions_abilities(role) Featurable::PRIVATE | ref(:anonymous) | false Featurable::PRIVATE | ref(:non_member) | false Featurable::PRIVATE | ref(:guest) | true + Featurable::PRIVATE | ref(:planner) | true Featurable::PRIVATE | ref(:reporter) | true Featurable::PRIVATE | ref(:developer) | true Featurable::PRIVATE | ref(:maintainer) | true @@ -3710,6 +3699,7 @@ def permissions_abilities(role) Featurable::DISABLED | ref(:anonymous) | false Featurable::DISABLED | ref(:non_member) | false Featurable::DISABLED | ref(:guest) | false + Featurable::DISABLED | ref(:planner) | false Featurable::DISABLED | ref(:reporter) | false Featurable::DISABLED | ref(:developer) | false Featurable::DISABLED | ref(:maintainer) | false @@ -3717,6 +3707,7 @@ def permissions_abilities(role) Featurable::ENABLED | ref(:anonymous) | false Featurable::ENABLED | ref(:non_member) | false Featurable::ENABLED | ref(:guest) | true + Featurable::ENABLED | ref(:planner) | true Featurable::ENABLED | ref(:reporter) | true Featurable::ENABLED | ref(:developer) | true Featurable::ENABLED | ref(:maintainer) | true @@ -3724,6 +3715,7 @@ def permissions_abilities(role) Featurable::PRIVATE | ref(:anonymous) | false Featurable::PRIVATE | ref(:non_member) | false Featurable::PRIVATE | ref(:guest) | true + Featurable::PRIVATE | ref(:planner) | true Featurable::PRIVATE | ref(:reporter) | true Featurable::PRIVATE | ref(:developer) | true Featurable::PRIVATE | ref(:maintainer) | true @@ -3756,6 +3748,9 @@ def permissions_abilities(role) ref(:guest) | Featurable::ENABLED | false ref(:guest) | Featurable::PRIVATE | false ref(:guest) | Featurable::DISABLED | false + ref(:planner) | Featurable::ENABLED | false + ref(:planner) | Featurable::PRIVATE | false + ref(:planner) | Featurable::DISABLED | false ref(:reporter) | Featurable::ENABLED | false ref(:reporter) | Featurable::PRIVATE | false ref(:reporter) | Featurable::DISABLED | false @@ -3790,6 +3785,7 @@ def permissions_abilities(role) Featurable::DISABLED | ref(:anonymous) | false Featurable::DISABLED | ref(:non_member) | false Featurable::DISABLED | ref(:guest) | false + Featurable::DISABLED | ref(:planner) | false Featurable::DISABLED | ref(:reporter) | false Featurable::DISABLED | ref(:developer) | false Featurable::DISABLED | ref(:maintainer) | false @@ -3797,6 +3793,7 @@ def permissions_abilities(role) Featurable::ENABLED | ref(:anonymous) | true Featurable::ENABLED | ref(:non_member) | true Featurable::ENABLED | ref(:guest) | true + Featurable::ENABLED | ref(:planner) | true Featurable::ENABLED | ref(:reporter) | true Featurable::ENABLED | ref(:developer) | true Featurable::ENABLED | ref(:maintainer) | true @@ -3804,6 +3801,7 @@ def permissions_abilities(role) Featurable::PRIVATE | ref(:anonymous) | false Featurable::PRIVATE | ref(:non_member) | false Featurable::PRIVATE | ref(:guest) | true + Featurable::PRIVATE | ref(:planner) | true Featurable::PRIVATE | ref(:reporter) | true Featurable::PRIVATE | ref(:developer) | true Featurable::PRIVATE | ref(:maintainer) | true @@ -3831,6 +3829,7 @@ def permissions_abilities(role) Featurable::DISABLED | ref(:anonymous) | false Featurable::DISABLED | ref(:non_member) | false Featurable::DISABLED | ref(:guest) | false + Featurable::DISABLED | ref(:planner) | false Featurable::DISABLED | ref(:reporter) | false Featurable::DISABLED | ref(:developer) | false Featurable::DISABLED | ref(:maintainer) | false @@ -3838,6 +3837,7 @@ def permissions_abilities(role) Featurable::ENABLED | ref(:anonymous) | false Featurable::ENABLED | ref(:non_member) | false Featurable::ENABLED | ref(:guest) | true + Featurable::ENABLED | ref(:planner) | true Featurable::ENABLED | ref(:reporter) | true Featurable::ENABLED | ref(:developer) | true Featurable::ENABLED | ref(:maintainer) | true @@ -3845,6 +3845,7 @@ def permissions_abilities(role) Featurable::PRIVATE | ref(:anonymous) | false Featurable::PRIVATE | ref(:non_member) | false Featurable::PRIVATE | ref(:guest) | true + Featurable::PRIVATE | ref(:planner) | true Featurable::PRIVATE | ref(:reporter) | true Featurable::PRIVATE | ref(:developer) | true Featurable::PRIVATE | ref(:maintainer) | true @@ -3872,6 +3873,7 @@ def permissions_abilities(role) Featurable::DISABLED | ref(:anonymous) | false Featurable::DISABLED | ref(:non_member) | false Featurable::DISABLED | ref(:guest) | false + Featurable::DISABLED | ref(:planner) | false Featurable::DISABLED | ref(:reporter) | false Featurable::DISABLED | ref(:developer) | false Featurable::DISABLED | ref(:maintainer) | false @@ -3879,6 +3881,7 @@ def permissions_abilities(role) Featurable::ENABLED | ref(:anonymous) | false Featurable::ENABLED | ref(:non_member) | false Featurable::ENABLED | ref(:guest) | true + Featurable::ENABLED | ref(:planner) | true Featurable::ENABLED | ref(:reporter) | true Featurable::ENABLED | ref(:developer) | true Featurable::ENABLED | ref(:maintainer) | true @@ -3886,6 +3889,7 @@ def permissions_abilities(role) Featurable::PRIVATE | ref(:anonymous) | false Featurable::PRIVATE | ref(:non_member) | false Featurable::PRIVATE | ref(:guest) | true + Featurable::PRIVATE | ref(:planner) | true Featurable::PRIVATE | ref(:reporter) | true Featurable::PRIVATE | ref(:developer) | true Featurable::PRIVATE | ref(:maintainer) | true @@ -3919,6 +3923,9 @@ def permissions_abilities(role) true | ref(:guest) | Featurable::ENABLED | false true | ref(:guest) | Featurable::PRIVATE | false true | ref(:guest) | Featurable::DISABLED | false + true | ref(:planner) | Featurable::ENABLED | false + true | ref(:planner) | Featurable::PRIVATE | false + true | ref(:planner) | Featurable::DISABLED | false true | ref(:reporter) | Featurable::ENABLED | false true | ref(:reporter) | Featurable::PRIVATE | false true | ref(:reporter) | Featurable::DISABLED | false @@ -3997,6 +4004,9 @@ def permissions_abilities(role) :maintainer | :private | true | true | true | false :developer | :public | true | true | true | false :reporter | :public | true | true | false | false + :planner | :public | true | true | false | false + :planner | :private | true | true | false | false + :planner | :internal | true | true | false | false :guest | :public | true | true | false | false :guest | :private | true | true | false | false :guest | :internal | true | true | false | false @@ -4025,6 +4035,7 @@ def permissions_abilities(role) stub_feature_flags(allow_push_repository_for_job_token: false) if ff_disabled project.add_guest(guest) + project.add_planner(planner) project.add_reporter(reporter) project.add_developer(developer) project.add_maintainer(maintainer) @@ -4077,6 +4088,8 @@ def user_subject(role) developer when :guest guest + when :planner + planner when :anonymous anonymous end diff --git a/spec/policies/work_item_policy_spec.rb b/spec/policies/work_item_policy_spec.rb index a634105e0be3fb..d251c688b43367 100644 --- a/spec/policies/work_item_policy_spec.rb +++ b/spec/policies/work_item_policy_spec.rb @@ -14,9 +14,11 @@ let_it_be(:guest) { create(:user, guest_of: [private_project, public_project]) } let_it_be(:guest_author) { create(:user, guest_of: [private_project, public_project]) } + let_it_be(:planner) { create(:user, planner_of: [private_project, public_project]) } let_it_be(:reporter) { create(:user, reporter_of: [private_project, public_project]) } let_it_be(:group_guest) { create(:user, guest_of: [private_group, public_group]) } + let_it_be(:group_planner) { create(:user, planner_of: [private_group, public_group]) } let_it_be(:group_guest_author) { create(:user, guest_of: [private_group, public_group]) } let_it_be(:group_reporter) { create(:user, reporter_of: [private_group, public_group]) } diff --git a/spec/presenters/member_presenter_spec.rb b/spec/presenters/member_presenter_spec.rb index c0ed8fdb3a3de6..ebf2e847a08745 100644 --- a/spec/presenters/member_presenter_spec.rb +++ b/spec/presenters/member_presenter_spec.rb @@ -48,6 +48,7 @@ it 'returns all roles for the root group' do expect(described_class.new(root_member).valid_level_roles).to eq( 'Guest' => Gitlab::Access::GUEST, + 'Planner' => Gitlab::Access::PLANNER, 'Reporter' => Gitlab::Access::REPORTER, 'Developer' => Gitlab::Access::DEVELOPER, 'Maintainer' => Gitlab::Access::MAINTAINER, diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index e48736fb91df10..6388ef049d7f12 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -269,7 +269,7 @@ def perform_request expect(json_response.dig('project', 'groups')).to match_array([{ 'id' => group.id }]) expect(json_response.dig('user', 'id')).to eq(api_user.id) expect(json_response.dig('user', 'username')).to eq(api_user.username) - expect(json_response.dig('user', 'roles_in_project')).to match_array %w[guest reporter developer] + expect(json_response.dig('user', 'roles_in_project')).to match_array %w[guest planner reporter developer] expect(json_response).not_to include('environment') end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 48d8bd904e4e45..87b9055c919550 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -909,7 +909,7 @@ it 'returns 400 when access level is not valid' do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), - params: { access_level: 15 } + params: { access_level: 25 } expect(response).to have_gitlab_http_status(:bad_request) end diff --git a/spec/services/groups/open_issues_count_service_spec.rb b/spec/services/groups/open_issues_count_service_spec.rb index b336f48ebf983d..362d5e8c72304a 100644 --- a/spec/services/groups/open_issues_count_service_spec.rb +++ b/spec/services/groups/open_issues_count_service_spec.rb @@ -39,7 +39,7 @@ context 'when user is provided' do context 'when user can read confidential issues' do before do - group.add_reporter(user) + group.add_planner(user) end it 'returns the right count with confidential issues' do diff --git a/spec/services/projects/open_issues_count_service_spec.rb b/spec/services/projects/open_issues_count_service_spec.rb index 89405f06f5ddc2..52688ae321c1ce 100644 --- a/spec/services/projects/open_issues_count_service_spec.rb +++ b/spec/services/projects/open_issues_count_service_spec.rb @@ -24,7 +24,7 @@ context 'when user can read confidential issues' do before do - project.add_reporter(user) + project.add_planner(user) end it 'returns the right count with confidential issues' do diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb index 1ced2eda799f6d..80d83ef6e6788d 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -66,8 +66,8 @@ def set_access(object, user, access_name) case access_name when :developer object.add_developer(user) - when :reporter - object.add_reporter(user) + when :planner + object.add_planner(user) when :guest object.add_guest(user) end @@ -91,11 +91,11 @@ def set_access(object, user, access_name) context 'access permissions' do where(:group_access, :project_access, :method_name) do [ - [nil, :reporter, :does_not_remove_any_todos], + [nil, :planner, :does_not_remove_any_todos], [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], - [:reporter, nil, :does_not_remove_any_todos], + [:planner, nil, :does_not_remove_any_todos], [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], - [:guest, :reporter, :does_not_remove_any_todos], + [:guest, :planner, :does_not_remove_any_todos], [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -123,11 +123,11 @@ def set_access(object, user, access_name) context 'access permissions' do where(:group_access, :project_access, :method_name) do [ - [nil, :reporter, :does_not_remove_any_todos], + [nil, :planner, :does_not_remove_any_todos], [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], - [:reporter, nil, :does_not_remove_any_todos], + [:planner, nil, :does_not_remove_any_todos], [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], - [:guest, :reporter, :does_not_remove_any_todos], + [:guest, :planner, :does_not_remove_any_todos], [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -176,11 +176,11 @@ def set_access(object, user, access_name) context 'access permissions' do where(:group_access, :project_access, :method_name) do [ - [nil, :reporter, :does_not_remove_any_todos], + [nil, :planner, :does_not_remove_any_todos], [nil, :guest, :removes_confidential_issues_and_internal_notes_todos], - [:reporter, nil, :does_not_remove_any_todos], + [:planner, nil, :does_not_remove_any_todos], [:guest, nil, :removes_confidential_issues_and_internal_notes_todos], - [:guest, :reporter, :does_not_remove_any_todos], + [:guest, :planner, :does_not_remove_any_todos], [:guest, :guest, :removes_confidential_issues_and_internal_notes_todos] ] end @@ -221,11 +221,11 @@ def set_access(object, user, access_name) context 'access permissions' do where(:group_access, :project_access, :method_name) do [ - [nil, :reporter, :does_not_remove_any_todos], + [nil, :planner, :does_not_remove_any_todos], [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], - [:reporter, nil, :does_not_remove_any_todos], + [:planner, nil, :does_not_remove_any_todos], [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], - [:guest, :reporter, :does_not_remove_any_todos], + [:guest, :planner, :does_not_remove_any_todos], [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -336,11 +336,11 @@ def set_access(object, user, access_name) where(:group_access, :project_access, :method_name) do [ [nil, nil, :removes_confidential_issues_and_internal_notes_todos], - [nil, :reporter, :does_not_remove_any_todos], + [nil, :planner, :does_not_remove_any_todos], [nil, :guest, :removes_confidential_issues_and_internal_notes_todos], - [:reporter, nil, :does_not_remove_any_todos], + [:planner, nil, :does_not_remove_any_todos], [:guest, nil, :removes_confidential_issues_and_internal_notes_todos], - [:guest, :reporter, :does_not_remove_any_todos], + [:guest, :planner, :does_not_remove_any_todos], [:guest, :guest, :removes_confidential_issues_and_internal_notes_todos] ] end diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index e223e2f82e42a8..d97dd19cfe5b51 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -8,6 +8,7 @@ end let_it_be(:guest) { create(:user, guest_of: group) } + let_it_be(:planner) { create(:user, planner_of: group) } let_it_be(:reporter) { create(:user, reporter_of: group) } let_it_be(:developer) { create(:user, developer_of: group) } let_it_be(:maintainer) { create(:user, maintainer_of: group) } @@ -22,6 +23,8 @@ %i[ read_group read_counts read_issue read_namespace read_label read_issue_board_list read_milestone read_issue_board + read_group_activity read_group_issues read_group_boards read_group_labels + read_group_milestones read_group_merge_requests ] end @@ -33,6 +36,14 @@ ] end + let(:planner_permissions) do + guest_permissions + %i[ + admin_label admin_milestone admin_issue_board admin_issue_board_list + admin_issue update_issue destroy_issue read_confidential_issues read_internal_note + read_crm_contact read_crm_organization + ] + end + let(:reporter_permissions) do %i[ admin_label diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index 8a6b75eb449d21..bd4c26cb23ade4 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -3,10 +3,12 @@ RSpec.shared_context 'ProjectPolicy context' do let_it_be(:anonymous) { nil } let_it_be(:guest) { create(:user) } + let_it_be(:planner) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:developer) { create(:user) } let_it_be(:maintainer) { create(:user) } let_it_be(:inherited_guest) { create(:user) } + let_it_be(:inherited_planner) { create(:user) } let_it_be(:inherited_reporter) { create(:user) } let_it_be(:inherited_developer) { create(:user) } let_it_be(:inherited_maintainer) { create(:user) } @@ -32,6 +34,15 @@ ] end + let(:base_planner_permissions) do + base_guest_permissions + + %i[ + admin_issue admin_issue_board admin_issue_board_list admin_label admin_milestone + read_confidential_issues update_issue reopen_issue destroy_issue read_internal_note + download_wiki_code create_wiki admin_wiki export_work_items + ] + end + let(:base_reporter_permissions) do %i[ admin_issue admin_label admin_milestone admin_issue_board_list @@ -107,23 +118,27 @@ # Used in EE specs let(:additional_guest_permissions) { [] } + let(:additional_planner_permissions) { [] } let(:additional_reporter_permissions) { [] } let(:additional_maintainer_permissions) { [] } let(:additional_owner_permissions) { [] } let(:guest_permissions) { base_guest_permissions + additional_guest_permissions } + let(:planner_permissions) { base_planner_permissions + additional_planner_permissions } let(:reporter_permissions) { base_reporter_permissions + additional_reporter_permissions } let(:maintainer_permissions) { base_maintainer_permissions + additional_maintainer_permissions } let(:owner_permissions) { base_owner_permissions + additional_owner_permissions } before_all do group.add_guest(inherited_guest) + group.add_planner(inherited_planner) group.add_reporter(inherited_reporter) group.add_developer(inherited_developer) group.add_maintainer(inherited_maintainer) [private_project, internal_project, public_project, public_project_in_group].each do |project| project.add_guest(guest) + project.add_planner(planner) project.add_reporter(reporter) project.add_developer(developer) project.add_maintainer(maintainer) diff --git a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb index f2052f4202d197..307a6b6851b107 100644 --- a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb +++ b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb @@ -172,7 +172,7 @@ def kaminari_array(*objects) context 'with :with_api_entity_associations' do let(:unredacted_results) { ar_relation(MergeRequest.with_api_entity_associations, readable, unreadable) } - it_behaves_like "redaction limits N+1 queries", limit: 8 + it_behaves_like "redaction limits N+1 queries", limit: 9 end end diff --git a/spec/support/shared_examples/policies/group_level_work_items_shared_examples.rb b/spec/support/shared_examples/policies/group_level_work_items_shared_examples.rb index fcb13b04f8c963..b8d6bbc8016101 100644 --- a/spec/support/shared_examples/policies/group_level_work_items_shared_examples.rb +++ b/spec/support/shared_examples/policies/group_level_work_items_shared_examples.rb @@ -34,6 +34,18 @@ ) end + it 'checks project planner abilities' do + # disallowed + expect(permissions(planner, work_item)).to be_disallowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :delete_work_item, + :admin_parent_link, :set_work_item_metadata, :admin_work_item_link, :create_note + ) + expect(permissions(planner, confidential_work_item)).to be_disallowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :delete_work_item, + :admin_parent_link, :set_work_item_metadata, :admin_work_item_link, :create_note + ) + end + it 'checks project reporter abilities' do # disallowed expect(permissions(reporter, work_item)).to be_disallowed( @@ -66,6 +78,18 @@ ) end + it 'checks group planner abilities' do + # disallowed + expect(permissions(group_planner, work_item)).to be_disallowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :delete_work_item, + :admin_parent_link, :set_work_item_metadata, :admin_work_item_link, :create_note + ) + expect(permissions(group_planner, confidential_work_item)).to be_disallowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :delete_work_item, + :admin_parent_link, :set_work_item_metadata, :admin_work_item_link, :create_note + ) + end + it 'checks group reporter abilities' do # disallowed expect(permissions(group_reporter, work_item)).to be_disallowed( @@ -108,6 +132,23 @@ ) end + it 'checks project planner abilities' do + # allowed + expect(permissions(planner, work_item)).to be_allowed( + :read_work_item, :read_issue, :read_note, :create_note + ) + + # disallowed + expect(permissions(planner, work_item)).to be_disallowed( + :admin_work_item, :update_work_item, :delete_work_item, :admin_parent_link, :set_work_item_metadata, + :admin_work_item_link + ) + expect(permissions(planner, confidential_work_item)).to be_disallowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :admin_parent_link, + :set_work_item_metadata, :admin_work_item_link, :create_note + ) + end + it 'checks project reporter abilities' do # allowed expect(permissions(reporter, work_item)).to be_allowed( @@ -159,6 +200,22 @@ ) end + it 'checks group planner abilities' do + # allowed + expect(permissions(group_planner, work_item)).to be_allowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :admin_parent_link, + :set_work_item_metadata, :admin_work_item_link, :create_note + ) + expect(permissions(group_planner, confidential_work_item)).to be_allowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :admin_parent_link, + :set_work_item_metadata, :admin_work_item_link, :create_note + ) + + # disallowed + expect(permissions(group_planner, work_item)).to be_allowed(:delete_work_item) + expect(permissions(group_planner, confidential_work_item)).to be_allowed(:delete_work_item) + end + it 'checks group reporter abilities' do # allowed expect(permissions(group_reporter, work_item)).to be_allowed( diff --git a/spec/support/shared_examples/policies/project_level_work_items_shared_examples.rb b/spec/support/shared_examples/policies/project_level_work_items_shared_examples.rb index daaea4a88e5d5c..00a45c8a5c8bc3 100644 --- a/spec/support/shared_examples/policies/project_level_work_items_shared_examples.rb +++ b/spec/support/shared_examples/policies/project_level_work_items_shared_examples.rb @@ -35,6 +35,38 @@ ) end + it 'checks planner abilities' do + # allowed + expect(permissions(planner, project_work_item)).to be_allowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :admin_parent_link, + :set_work_item_metadata, :admin_work_item_link, :create_note + ) + expect(permissions(planner, project_confidential_work_item)).to be_allowed(:read_work_item, :read_issue, + :read_note, :admin_work_item, :update_work_item, :admin_parent_link, :set_work_item_metadata, + :admin_work_item_link, :create_note + ) + + # disallowed + expect(permissions(planner, project_work_item)).to be_allowed(:delete_work_item) + expect(permissions(planner, project_confidential_work_item)).to be_allowed(:delete_work_item) + end + + it 'checks group planner abilities' do + # allowed + expect(permissions(group_planner, project_work_item)).to be_allowed( + :read_work_item, :read_issue, :read_note, :admin_work_item, :update_work_item, :admin_parent_link, + :set_work_item_metadata, :admin_work_item_link, :create_note + ) + expect(permissions(group_planner, project_confidential_work_item)).to be_allowed(:read_work_item, + :read_issue, :read_note, :admin_work_item, :update_work_item, :admin_parent_link, :set_work_item_metadata, + :admin_work_item_link, :create_note + ) + + # disallowed + expect(permissions(group_planner, project_work_item)).to be_allowed(:delete_work_item) + expect(permissions(group_planner, project_confidential_work_item)).to be_allowed(:delete_work_item) + end + it 'checks reporter abilities' do # allowed expect(permissions(reporter, project_work_item)).to be_allowed( diff --git a/spec/support/shared_examples/policies/project_policy_shared_examples.rb b/spec/support/shared_examples/policies/project_policy_shared_examples.rb index 953a036c0e557a..820328d0154c26 100644 --- a/spec/support/shared_examples/policies/project_policy_shared_examples.rb +++ b/spec/support/shared_examples/policies/project_policy_shared_examples.rb @@ -195,6 +195,109 @@ end end +RSpec.shared_examples 'project policies as planner' do + let(:disallowed_reporter_public_permissions) do + %i[ + create_snippet create_incident daily_statistics metrics_dashboard read_harbor_registry + read_prometheus read_sentry_issue read_external_emails + ] + end + + let(:disallowed_reporter_permissions) do + disallowed_reporter_public_permissions + + %i[fork_project read_commit_status read_container_image read_deployment read_environment] + end + + context 'as a direct project member' do + context 'abilities for public projects' do + let(:project) { public_project } + let(:current_user) { planner } + + specify do + expect_allowed(*public_permissions) + expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) + expect_allowed(*(base_reporter_permissions - disallowed_reporter_public_permissions)) + expect_disallowed(*disallowed_reporter_public_permissions) + expect_disallowed(*(developer_permissions - [:create_wiki])) + expect_disallowed(*(maintainer_permissions - [:admin_wiki])) + expect_disallowed(*(owner_permissions - [:destroy_issue])) + end + end + + context 'abilities for non-public projects' do + let(:project) { private_project } + let(:current_user) { planner } + + specify do + expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) + expect_allowed(*(base_reporter_permissions - disallowed_reporter_permissions)) + expect_disallowed(*disallowed_reporter_permissions) + expect_disallowed(*(developer_permissions - [:create_wiki])) + expect_disallowed(*(maintainer_permissions - [:admin_wiki])) + expect_disallowed(*(owner_permissions - [:destroy_issue])) + end + + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { planner.id } + end + + it_behaves_like 'archived project policies' do + let(:regular_abilities) { planner_permissions } + end + + context 'public builds enabled' do + specify do + expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) + expect_allowed(:read_build, :read_pipeline) + end + end + + context 'when public builds disabled' do + before do + project.update!(public_builds: false) + end + + specify do + expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) + expect_disallowed(:read_build, :read_pipeline) + end + end + + context 'when builds are disabled' do + before do + project.project_feature.update!(builds_access_level: ProjectFeature::DISABLED) + end + + specify do + expect_disallowed(:read_build) + expect_allowed(:read_pipeline) + end + end + end + end + + context 'as an inherited member from the group' do + context 'abilities for private projects' do + let(:project) { private_project_in_group } + let(:current_user) { inherited_planner } + + specify do + expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) + expect_allowed(*(base_reporter_permissions - disallowed_reporter_permissions)) + expect_disallowed(*disallowed_reporter_permissions) + expect_disallowed(*(developer_permissions - [:create_wiki])) + expect_disallowed(*(maintainer_permissions - [:admin_wiki])) + expect_disallowed(*(owner_permissions - [:destroy_issue])) + end + end + end +end + RSpec.shared_examples 'project policies as reporter' do context 'abilities for non-public projects' do let(:project) { private_project } @@ -202,6 +305,7 @@ specify do expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - %i[create_wiki admin_wiki destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*team_member_reporter_permissions) expect_disallowed(*developer_permissions) @@ -225,6 +329,7 @@ specify do expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - %i[create_wiki admin_wiki destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*team_member_reporter_permissions) expect_disallowed(*developer_permissions) @@ -242,6 +347,7 @@ specify do expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - %i[admin_wiki destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*team_member_reporter_permissions) expect_allowed(*developer_permissions) @@ -265,6 +371,7 @@ specify do expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - %i[admin_wiki destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*team_member_reporter_permissions) expect_allowed(*developer_permissions) @@ -282,6 +389,7 @@ it do expect_allowed(*guest_permissions) + expect_allowed(*(planner_permissions - [:destroy_issue])) expect_allowed(*reporter_permissions) expect_allowed(*team_member_reporter_permissions) expect_allowed(*developer_permissions) @@ -306,6 +414,7 @@ it do expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) expect_allowed(*team_member_reporter_permissions) expect_allowed(*developer_permissions) @@ -330,6 +439,7 @@ it do expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) expect_disallowed(*team_member_reporter_permissions) expect_allowed(*developer_permissions) @@ -361,6 +471,7 @@ it do expect_allowed(*guest_permissions) + expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) expect_disallowed(*team_member_reporter_permissions) expect_allowed(*developer_permissions) diff --git a/spec/support/shared_examples/policies/wiki_policies_shared_examples.rb b/spec/support/shared_examples/policies/wiki_policies_shared_examples.rb index b9d4709efd5982..7efd0ebcada1c5 100644 --- a/spec/support/shared_examples/policies/wiki_policies_shared_examples.rb +++ b/spec/support/shared_examples/policies/wiki_policies_shared_examples.rb @@ -15,6 +15,7 @@ permissions[:reporter] = permissions[:guest] + %i[download_wiki_code] permissions[:developer] = permissions[:reporter] + %i[create_wiki] permissions[:maintainer] = permissions[:developer] + %i[admin_wiki] + permissions[:planner] = permissions[:maintainer] permissions[:all] = permissions[:maintainer] end end @@ -26,6 +27,7 @@ :public | :enabled | :maintainer | :maintainer :public | :enabled | :developer | :developer :public | :enabled | :reporter | :reporter + :public | :enabled | :planner | :planner :public | :enabled | :guest | :guest :public | :enabled | :non_member | :guest :public | :enabled | :anonymous | :guest @@ -34,6 +36,7 @@ :public | :private | :maintainer | :maintainer :public | :private | :developer | :developer :public | :private | :reporter | :reporter + :public | :private | :planner | :planner :public | :private | :guest | :guest :public | :private | :non_member | nil :public | :private | :anonymous | nil @@ -42,6 +45,7 @@ :public | :disabled | :maintainer | nil :public | :disabled | :developer | nil :public | :disabled | :reporter | nil + :public | :disabled | :planner | nil :public | :disabled | :guest | nil :public | :disabled | :non_member | nil :public | :disabled | :anonymous | nil @@ -50,6 +54,7 @@ :internal | :enabled | :maintainer | :maintainer :internal | :enabled | :developer | :developer :internal | :enabled | :reporter | :reporter + :internal | :enabled | :planner | :planner :internal | :enabled | :guest | :guest :internal | :enabled | :non_member | :guest :internal | :enabled | :anonymous | nil @@ -58,6 +63,7 @@ :internal | :private | :maintainer | :maintainer :internal | :private | :developer | :developer :internal | :private | :reporter | :reporter + :internal | :private | :planner | :planner :internal | :private | :guest | :guest :internal | :private | :non_member | nil :internal | :private | :anonymous | nil @@ -66,6 +72,7 @@ :internal | :disabled | :maintainer | nil :internal | :disabled | :developer | nil :internal | :disabled | :reporter | nil + :internal | :disabled | :planner | nil :internal | :disabled | :guest | nil :internal | :disabled | :non_member | nil :internal | :disabled | :anonymous | nil @@ -74,6 +81,7 @@ :private | :private | :maintainer | :maintainer :private | :private | :developer | :developer :private | :private | :reporter | :reporter + :private | :private | :planner | :planner :private | :private | :guest | :guest :private | :private | :non_member | nil :private | :private | :anonymous | nil @@ -82,6 +90,7 @@ :private | :disabled | :maintainer | nil :private | :disabled | :developer | nil :private | :disabled | :reporter | nil + :private | :disabled | :planner | nil :private | :disabled | :guest | nil :private | :disabled | :non_member | nil :private | :disabled | :anonymous | nil -- GitLab From e8be0062ba6bf510d31a7f59991c9e6bd05085c1 Mon Sep 17 00:00:00 2001 From: Eugenia Grieff Date: Tue, 19 Nov 2024 15:54:19 +0100 Subject: [PATCH 2/5] Fix spec failure for redaced search results --- .../features/search/redacted_search_results_shared_examples.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb index 307a6b6851b107..5bad260d48b992 100644 --- a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb +++ b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb @@ -172,7 +172,7 @@ def kaminari_array(*objects) context 'with :with_api_entity_associations' do let(:unredacted_results) { ar_relation(MergeRequest.with_api_entity_associations, readable, unreadable) } - it_behaves_like "redaction limits N+1 queries", limit: 9 + it_behaves_like "redaction limits N+1 queries", limit: 10 end end -- GitLab From 25e81b169d0422d5bd71709e80283724cd4ee230 Mon Sep 17 00:00:00 2001 From: Eugenia Grieff Date: Wed, 20 Nov 2024 10:36:49 +0100 Subject: [PATCH 3/5] Apply backend review feedback --- app/policies/group_policy.rb | 1 + app/policies/project_policy.rb | 1 + app/services/groups/open_issues_count_service.rb | 2 ++ app/services/todos/destroy/entity_leave_service.rb | 4 +++- spec/policies/board_policy_spec.rb | 4 ++-- spec/policies/merge_request_policy_spec.rb | 2 +- 6 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index e9e9f29eb8847f..768206b9fdaee0 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -13,6 +13,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy condition(:has_access) { access_level != GroupMember::NO_ACCESS } condition(:guest) { access_level >= GroupMember::GUEST } + # This is not a linear condition (some policies available for planner might not be available for higher access levels) condition(:planner) { access_level == GroupMember::PLANNER } condition(:developer) { access_level >= GroupMember::DEVELOPER } condition(:owner) { access_level >= GroupMember::OWNER } diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index f11744197621d0..8417ad406f88f8 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -15,6 +15,7 @@ class ProjectPolicy < BasePolicy desc "User has guest access" condition(:guest) { team_member? } + # This is not a linear condition (some policies available for planner might not be available for higher access levels) desc "User has planner access" condition(:planner) { team_access_level == Gitlab::Access::PLANNER } diff --git a/app/services/groups/open_issues_count_service.rb b/app/services/groups/open_issues_count_service.rb index b5e24f98e8c14b..70ef3daa08fe72 100644 --- a/app/services/groups/open_issues_count_service.rb +++ b/app/services/groups/open_issues_count_service.rb @@ -37,6 +37,8 @@ def cache_key_name end def public_only? + # Although PLANNER is not a linear access level, it can be considered so for the purpose of issues visibility + # because the same permissions apply to all levels higher than Gitlab::Access::PLANNER !user_is_at_least_planner? end diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index f0056756765156..b49cbacc75c33d 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -19,7 +19,9 @@ def initialize(user_id, entity_id, entity_type) def execute return unless entity && user - # if at least planner, all entities including confidential issues can be accessed + # If at least planner, all entities including confidential issues can be accessed. Although PLANNER is not a + # linear access level, it can be considered so for the purpose of issuables visibility because the same + # permissions apply to all levels higher than Gitlab::Access::PLANNER return if user_has_planner_access? remove_confidential_resource_todos diff --git a/spec/policies/board_policy_spec.rb b/spec/policies/board_policy_spec.rb index f6558dbee69892..fcb452cf1693df 100644 --- a/spec/policies/board_policy_spec.rb +++ b/spec/policies/board_policy_spec.rb @@ -4,8 +4,8 @@ RSpec.describe BoardPolicy, feature_category: :portfolio_management do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :private) } - let_it_be(:group) { create(:group, :private) } + let_it_be_with_reload(:project) { create(:project, :private) } + let_it_be_with_reload(:group) { create(:group, :private) } let_it_be(:group_board) { create(:board, group: group) } let_it_be(:project_board) { create(:board, project: project) } diff --git a/spec/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb index 19f4d79a68460b..a5ad504c87dffa 100644 --- a/spec/policies/merge_request_policy_spec.rb +++ b/spec/policies/merge_request_policy_spec.rb @@ -173,7 +173,7 @@ def permission_table_for_reporter end describe 'a planner' do - let(:permission_table) { permission_table_for_reporter } + let(:permission_table) { permission_table_for_reporter } # same as reporter because MR is public subject { permissions(planner, merge_request) } -- GitLab From d497702f18804a6198608b23b9dcdeb5fcc1f9ca Mon Sep 17 00:00:00 2001 From: Eugenia Grieff Date: Wed, 20 Nov 2024 10:54:24 +0100 Subject: [PATCH 4/5] Apply technical writer review feedback --- app/assets/javascripts/access_level/constants.js | 4 +++- lib/gitlab/access.rb | 2 +- locale/gitlab.pot | 8 ++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/access_level/constants.js b/app/assets/javascripts/access_level/constants.js index 2059264c694f8f..105c9f392cfcf2 100644 --- a/app/assets/javascripts/access_level/constants.js +++ b/app/assets/javascripts/access_level/constants.js @@ -66,7 +66,9 @@ export const BASE_ROLES = [ accessLevel: ACCESS_LEVEL_PLANNER_INTEGER, memberRoleId: null, occupiesSeat: true, - description: s__('MemberRole|The Planner role is for users who need to manage projects.'), + description: s__( + 'MemberRole|The Planner role is suitable for team members who need to manage projects and track work items but do not need to contribute code, such as project managers and scrum masters.', + ), }, { value: 'REPORTER', diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index a921e512a52947..422ee036411aad 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -69,7 +69,7 @@ def option_descriptions { NO_ACCESS => s_('MemberRole|The None role is assigned to the invited group users of a shared project when project sharing is disabled in group setting.'), GUEST => s_('MemberRole|The Guest role is for users who need visibility into a project or group but should not have the ability to make changes, such as external stakeholders.'), - PLANNER => s_('MemberRole|The Planner role is for users who need access to product management and planning tools.'), + PLANNER => s_('The Planner role is suitable for team members who need to manage projects and track work items but do not need to contribute code.'), REPORTER => s_('MemberRole|The Reporter role is suitable for team members who need to stay informed about a project or group but do not actively contribute code.'), DEVELOPER => s_('MemberRole|The Developer role gives users access to contribute code while restricting sensitive administrative actions.'), MAINTAINER => s_('MemberRole|The Maintainer role is primarily used for managing code reviews, approvals, and administrative settings for projects. This role can also manage project memberships.'), diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 54f70e0d5d9d31..025a8bb0da605d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -33719,10 +33719,7 @@ msgstr "" msgid "MemberRole|The Owner role is normally assigned to the individual or team responsible for managing and maintaining the group or creating the project. This role has the highest level of administrative control, and can manage all aspects of the group or project, including managing other Owners." msgstr "" -msgid "MemberRole|The Planner role is for users who need access to product management and planning tools." -msgstr "" - -msgid "MemberRole|The Planner role is for users who need to manage projects." +msgid "MemberRole|The Planner role is suitable for team members who need to manage projects and track work items but do not need to contribute code, such as project managers and scrum masters." msgstr "" msgid "MemberRole|The Reporter role is suitable for team members who need to stay informed about a project or group but do not actively contribute code." @@ -55191,6 +55188,9 @@ msgstr "" msgid "The Mattermost token." msgstr "" +msgid "The Planner role is suitable for team members who need to manage projects and track work items but do not need to contribute code." +msgstr "" + msgid "The Slack notifications integration is deprecated and will be removed in a future release. To continue to receive notifications from Slack, use the GitLab for Slack app instead. %{learn_more_link_start}Learn more%{link_end}." msgstr "" -- GitLab From 22e985a913d4cf16a7d66d6f8013942dc409c8b7 Mon Sep 17 00:00:00 2001 From: Eugenia Grieff Date: Wed, 20 Nov 2024 13:11:04 +0100 Subject: [PATCH 5/5] Update GraphQL schema and documentation --- doc/api/graphql/reference/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 6943af3107bac5..e58bcba5a7c289 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -38873,7 +38873,7 @@ Access level of a group or project member. | `MAINTAINER` | The Maintainer role is primarily used for managing code reviews, approvals, and administrative settings for projects. This role can also manage project memberships. | | `MINIMAL_ACCESS` | The Minimal Access role is for users who need the least amount of access into groups and projects. You can assign this role as a default, before giving a user another role with more permissions. | | `OWNER` | The Owner role is normally assigned to the individual or team responsible for managing and maintaining the group or creating the project. This role has the highest level of administrative control, and can manage all aspects of the group or project, including managing other Owners. | -| `PLANNER` | The Planner role is for users who need access to product management and planning tools. | +| `PLANNER` | The Planner role is suitable for team members who need to manage projects and track work items but do not need to contribute code. | | `REPORTER` | The Reporter role is suitable for team members who need to stay informed about a project or group but do not actively contribute code. | ### `MemberAccessLevelName` -- GitLab