diff --git a/config/authz/permissions/definitions_todo.txt b/config/authz/permissions/definitions_todo.txt index 5081bbef1d35d21688ef67766e3f93d1bb294ee2..4fb0f7047f2db37614fd215ac3084a72d2e0144e 100644 --- a/config/authz/permissions/definitions_todo.txt +++ b/config/authz/permissions/definitions_todo.txt @@ -671,7 +671,6 @@ read_scan read_secret_push_protection_info read_secure_files read_security_attribute -read_security_category read_security_configuration read_security_inventory read_security_orchestration_policies diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 8d1737a575d453dd9a3adf2f746f376ae77818f3..2acbef0ac555ae7ef20ebc3abbd8b41008957618 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -49965,7 +49965,7 @@ Member role permission. | `ADMIN_PROTECTED_ENVIRONMENTS` | Create, read, update, and delete protected environments. | | `ADMIN_PUSH_RULES` | Configure push rules for repositories at the group or project level. | | `ADMIN_RUNNERS` | Create, view, edit, and delete group or project Runners. Includes configuring Runner settings. | -| `ADMIN_SECURITY_ATTRIBUTES` | Edit the security attributes belonging to a top-level group. | +| `ADMIN_SECURITY_ATTRIBUTES` | Manage the security attributes belonging to a top-level group. Also requires the `read_security_attribute` permission. | | `ADMIN_SECURITY_TESTING` | Edit and manage security testing configurations and settings. | | `ADMIN_TERRAFORM_STATE` | Execute terraform commands, lock/unlock terraform state files, and remove file versions. | | `ADMIN_VULNERABILITY` | Edit the status, linked issue, and severity of a vulnerability object. Also requires the `read_vulnerability` permission. | @@ -49988,6 +49988,7 @@ Member role permission. | `READ_CRM_CONTACT` | Read CRM contact. | | `READ_DEPENDENCY` | Allows read-only access to the dependencies and licenses. | | `READ_RUNNERS` | Allows read-only access to group or project runners, including the runner fleet dashboard. | +| `READ_SECURITY_ATTRIBUTE` | Allows read-only access to the security categories and attributes belonging to a top-level group. | | `READ_VULNERABILITY` | Read vulnerability reports and security dashboards. | | `REMOVE_GROUP` | Ability to delete or restore a group. This ability does not allow deleting top-level groups. Review the Retention period settings to prevent accidental deletion. | | `REMOVE_PROJECT` | Allows deletion of projects. | @@ -50007,7 +50008,7 @@ Member role standard permission. | `ADMIN_PROTECTED_ENVIRONMENTS` | Create, read, update, and delete protected environments. | | `ADMIN_PUSH_RULES` | Configure push rules for repositories at the group or project level. | | `ADMIN_RUNNERS` | Create, view, edit, and delete group or project Runners. Includes configuring Runner settings. | -| `ADMIN_SECURITY_ATTRIBUTES` {{< icon name="warning-solid" >}} | **Introduced** in GitLab 18.2. **Status**: Experiment. Edit the security attributes belonging to a top-level group. | +| `ADMIN_SECURITY_ATTRIBUTES` {{< icon name="warning-solid" >}} | **Introduced** in GitLab 18.2. **Status**: Experiment. Manage the security attributes belonging to a top-level group. Also requires the `read_security_attribute` permission. | | `ADMIN_SECURITY_TESTING` {{< icon name="warning-solid" >}} | **Introduced** in GitLab 17.9. **Status**: Experiment. Edit and manage security testing configurations and settings. | | `ADMIN_TERRAFORM_STATE` | Execute terraform commands, lock/unlock terraform state files, and remove file versions. | | `ADMIN_VULNERABILITY` | Edit the status, linked issue, and severity of a vulnerability object. Also requires the `read_vulnerability` permission. | @@ -50024,6 +50025,7 @@ Member role standard permission. | `READ_CRM_CONTACT` | Read CRM contact. | | `READ_DEPENDENCY` | Allows read-only access to the dependencies and licenses. | | `READ_RUNNERS` | Allows read-only access to group or project runners, including the runner fleet dashboard. | +| `READ_SECURITY_ATTRIBUTE` {{< icon name="warning-solid" >}} | **Introduced** in GitLab 18.6. **Status**: Experiment. Allows read-only access to the security categories and attributes belonging to a top-level group. | | `READ_VULNERABILITY` | Read vulnerability reports and security dashboards. | | `REMOVE_GROUP` | Ability to delete or restore a group. This ability does not allow deleting top-level groups. Review the Retention period settings to prevent accidental deletion. | | `REMOVE_PROJECT` | Allows deletion of projects. | diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index d56488608333d2ad3cbcf2c8888b2a64ebd4ac76..d5c9f1c9e91b3f3fc3a81f0343488a4c46bf1ae0 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -52274,8 +52274,6 @@ definitions: type: boolean remove_project: type: boolean - admin_security_attributes: - type: boolean manage_security_policy_link: type: boolean admin_compliance_framework: @@ -52304,6 +52302,8 @@ definitions: type: boolean admin_runners: type: boolean + admin_security_attributes: + type: boolean admin_security_testing: type: boolean admin_terraform_state: @@ -52328,6 +52328,8 @@ definitions: type: boolean read_runners: type: boolean + read_security_attribute: + type: boolean read_admin_subscription: type: boolean read_admin_monitoring: diff --git a/ee/app/assets/javascripts/roles_and_permissions/components/manage_role/utils.js b/ee/app/assets/javascripts/roles_and_permissions/components/manage_role/utils.js index 2656d7ab14ba264c09390e4095536d2d8b33cd24..2bf91ffd7827ccd34a2f1bcbd5bd1293c6e3f086 100644 --- a/ee/app/assets/javascripts/roles_and_permissions/components/manage_role/utils.js +++ b/ee/app/assets/javascripts/roles_and_permissions/components/manage_role/utils.js @@ -45,6 +45,12 @@ export const getCustomPermissionsTreeTemplate = () => [ name: s__('MemberRole|Secrets management'), permissions: [{ value: 'ADMIN_CICD_VARIABLES' }], }, + { + name: s__('MemberRole|Security asset inventories'), + permissions: [ + { value: 'ADMIN_SECURITY_ATTRIBUTES', children: [{ value: 'READ_SECURITY_ATTRIBUTE' }] }, + ], + }, { name: s__('MemberRole|Security policy management'), permissions: [{ value: 'MANAGE_SECURITY_POLICY_LINK' }], diff --git a/ee/app/graphql/ee/types/group_type.rb b/ee/app/graphql/ee/types/group_type.rb index e8799b30b2a6182b3c97f8caf45657268ec8f6f9..d0c03057f1149caa3e93155bf5ec3a14e2f90f84 100644 --- a/ee/app/graphql/ee/types/group_type.rb +++ b/ee/app/graphql/ee/types/group_type.rb @@ -389,7 +389,6 @@ module GroupType field :security_categories, [::Types::Security::CategoryType], null: true, description: 'Security categories for the group.', - authorize: :admin_security_attributes, resolver: ::Resolvers::Security::CategoryResolver field :compliance_requirement_control_coverage, diff --git a/ee/app/graphql/resolvers/security/attributes_resolver.rb b/ee/app/graphql/resolvers/security/attributes_resolver.rb index 925044471546787f665b38a0cd825b4a3058f8a7..4ccdc068abac8fa7b54eba574fa1ef1db2d1fff5 100644 --- a/ee/app/graphql/resolvers/security/attributes_resolver.rb +++ b/ee/app/graphql/resolvers/security/attributes_resolver.rb @@ -15,6 +15,10 @@ def resolve project = object.is_a?(::Project) ? object : object.try(:project) return ::Security::Attribute.none unless project + # Use authorized_resource? instead of authorize! to avoid raising an exception + # This returns a boolean, allowing graceful handling when permission is denied + return ::Security::Attribute.none unless authorized_resource?(project.namespace) + return [] unless ::Feature.enabled?(:security_categories_and_attributes, project.root_ancestor) project.security_attributes.include_category diff --git a/ee/app/graphql/resolvers/security/category_resolver.rb b/ee/app/graphql/resolvers/security/category_resolver.rb index ad057827ab72c037ff25220d7fcb0eda9b439578..2de1692b3b1c204e09914c9eb3a899477e37e8e0 100644 --- a/ee/app/graphql/resolvers/security/category_resolver.rb +++ b/ee/app/graphql/resolvers/security/category_resolver.rb @@ -7,7 +7,7 @@ class CategoryResolver < BaseResolver type Types::Security::CategoryType.connection_type, null: true - authorize :admin_security_attributes + authorize :read_security_attribute description 'Resolves security categories for a group.' diff --git a/ee/app/graphql/types/security/attribute_type.rb b/ee/app/graphql/types/security/attribute_type.rb index da4f855ab9769ca86b839509c34ed7d492815740..04d84b8d2f9797816cce87a0daf7f696bf1df2c2 100644 --- a/ee/app/graphql/types/security/attribute_type.rb +++ b/ee/app/graphql/types/security/attribute_type.rb @@ -2,12 +2,10 @@ module Types module Security - class AttributeType < BaseObject + class AttributeType < BaseObject # rubocop:disable Graphql/AuthorizeTypes -- Authorization is done in resolver layer based on viewing context graphql_name 'SecurityAttribute' description 'A security attribute' - authorize :read_security_attribute - field :color, Types::ColorType, null: false, description: 'Color of the security attribute.' diff --git a/ee/app/graphql/types/security/category_type.rb b/ee/app/graphql/types/security/category_type.rb index 1eb7701ea48878a95fb38bd8be12ff8792735dfd..8cec72ad36578481f4ba666cd5891aa9d9e31bbd 100644 --- a/ee/app/graphql/types/security/category_type.rb +++ b/ee/app/graphql/types/security/category_type.rb @@ -2,12 +2,10 @@ module Types module Security - class CategoryType < BaseObject + class CategoryType < BaseObject # rubocop:disable Graphql/AuthorizeTypes -- Authorization is done in resolver layer based on viewing context graphql_name 'SecurityCategory' description 'A security category' - authorize :read_security_category - field :description, GraphQL::Types::String, null: true, description: 'Description of the security category.' diff --git a/ee/app/models/security/category.rb b/ee/app/models/security/category.rb index 5f26467aac2948bae2d2108774317a198811a442..9db2758e7f692d9a6a106bee93b4b53fbe75fd37 100644 --- a/ee/app/models/security/category.rb +++ b/ee/app/models/security/category.rb @@ -5,6 +5,10 @@ class Category < ::SecApplicationRecord self.table_name = 'security_categories' MAX_ATTRIBUTES = 50 + def self.declarative_policy_class + 'Security::AttributePolicy' + end + belongs_to :namespace, optional: false has_many :security_attributes, class_name: 'Security::Attribute', inverse_of: :security_category diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index c118ccc70ae3a03f20f997e7b38f37c77a56d165..89467e1c79e4a8d18fdf1e508a6cf5ffae72dff2 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -663,6 +663,12 @@ module GroupPolicy rule { custom_role_enables_admin_security_attributes }.enable(:admin_security_attributes) + rule { can?(:admin_security_attributes) }.policy do + enable :read_security_attribute + end + + rule { custom_role_enables_read_security_attribute }.enable(:read_security_attribute) + rule { ~security_inventory_available }.prevent :read_security_inventory rule { custom_role_enables_read_vulnerability }.policy do diff --git a/ee/app/policies/security/category_policy.rb b/ee/app/policies/security/category_policy.rb deleted file mode 100644 index 8a112b1826f4cc77fd25309778775a0592fbe9ff..0000000000000000000000000000000000000000 --- a/ee/app/policies/security/category_policy.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module Security - class CategoryPolicy < BasePolicy - delegate { @subject.namespace } - - rule { can?(:admin_security_attributes) }.policy do - enable :read_security_category - end - end -end diff --git a/ee/config/custom_abilities/admin_security_attributes.yml b/ee/config/custom_abilities/admin_security_attributes.yml index 25b3442600fc13e7e2ebe80e28b52dae4635d901..8ddb30e8e44d36e667bf95241d153762699610b4 100644 --- a/ee/config/custom_abilities/admin_security_attributes.yml +++ b/ee/config/custom_abilities/admin_security_attributes.yml @@ -1,11 +1,13 @@ --- -title: Edit security attributes +title: Manage security attributes name: admin_security_attributes -description: Edit the security attributes belonging to a top-level group. +description: Manage the security attributes belonging to a top-level group. Also requires the `read_security_attribute` permission. introduced_by_issue: https://gitlab.com/groups/gitlab-org/-/epics/18010 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195554 feature_category: security_asset_inventories milestone: '18.2' group_ability: true enabled_for_group_access_levels: [40, 50] +requirements: + - read_security_attribute project_ability: false diff --git a/ee/config/custom_abilities/read_security_attribute.yml b/ee/config/custom_abilities/read_security_attribute.yml new file mode 100644 index 0000000000000000000000000000000000000000..f2593b947a3dfb7f6b4d645069ec4b0e790b5bbc --- /dev/null +++ b/ee/config/custom_abilities/read_security_attribute.yml @@ -0,0 +1,11 @@ +--- +title: View security attributes +name: read_security_attribute +description: Allows read-only access to the security categories and attributes belonging to a top-level group. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/567237 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209670 +feature_category: security_asset_inventories +milestone: '18.6' +group_ability: true +enabled_for_group_access_levels: [40, 50] +project_ability: false diff --git a/ee/config/feature_flags/beta/custom_ability_read_security_attribute.yml b/ee/config/feature_flags/beta/custom_ability_read_security_attribute.yml new file mode 100644 index 0000000000000000000000000000000000000000..5237123922d9261098f34f08158350b1971b5527 --- /dev/null +++ b/ee/config/feature_flags/beta/custom_ability_read_security_attribute.yml @@ -0,0 +1,10 @@ +--- +name: custom_ability_read_security_attribute +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/567237 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209670 +rollout_issue_url: +milestone: '18.6' +group: group::security platform management +type: beta +default_enabled: false diff --git a/ee/spec/graphql/ee/types/attribute_type_spec.rb b/ee/spec/graphql/ee/types/attribute_type_spec.rb index 513f42e5205a3a3fd42634a53699d692440edaff..d491891b97a3a1b55d4b24d2410608dd30bd11f4 100644 --- a/ee/spec/graphql/ee/types/attribute_type_spec.rb +++ b/ee/spec/graphql/ee/types/attribute_type_spec.rb @@ -17,7 +17,6 @@ end it { expect(described_class.graphql_name).to eq('SecurityAttribute') } - it { expect(described_class).to require_graphql_authorizations(:read_security_attribute) } describe 'fields' do it { expect(described_class).to have_graphql_field(:id, resolver_method: :resolve_id) } diff --git a/ee/spec/graphql/ee/types/category_type_spec.rb b/ee/spec/graphql/ee/types/category_type_spec.rb index 9fbc7110ecfd92548d96fdcfba704b0fa2573899..e1148520b418b2bbacf3c23029b5be190a5fd244 100644 --- a/ee/spec/graphql/ee/types/category_type_spec.rb +++ b/ee/spec/graphql/ee/types/category_type_spec.rb @@ -18,7 +18,6 @@ end it { expect(described_class.graphql_name).to eq('SecurityCategory') } - it { expect(described_class).to require_graphql_authorizations(:read_security_category) } describe 'fields' do it { expect(described_class).to have_graphql_field(:id, resolver_method: :resolve_id) } diff --git a/ee/spec/graphql/resolvers/security/attributes_resolver_spec.rb b/ee/spec/graphql/resolvers/security/attributes_resolver_spec.rb index ea3775f3e69aa8081f543ce5983135e4523d403a..c3fa5c608d3bb2492ae8d55c97fcaf5693ace30f 100644 --- a/ee/spec/graphql/resolvers/security/attributes_resolver_spec.rb +++ b/ee/spec/graphql/resolvers/security/attributes_resolver_spec.rb @@ -41,7 +41,7 @@ context 'when user has permission' do before_all do - project.add_maintainer(current_user) + sub_group.add_maintainer(current_user) end context 'when project has linked attributes' do diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 90f28c23f4e104461324413a8c0b822da25e5df1..b4f7e66a52a4731be76f7561da29176540977923 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -4494,7 +4494,7 @@ def create_member_role(member, abilities = member_role_abilities) end context 'for a member role with the `admin_security_attributes` ability' do - let(:member_role_abilities) { { admin_security_attributes: true } } + let(:member_role_abilities) { { admin_security_attributes: true, read_security_attribute: true } } let(:allowed_abilities) { [:admin_security_attributes] } it_behaves_like 'custom roles abilities' @@ -5111,12 +5111,14 @@ def create_member_role(member, abilities = member_role_abilities) let(:current_user) { maintainer } it { is_expected.to be_allowed(:admin_security_attributes) } + it { is_expected.to be_allowed(:read_security_attribute) } end context 'when user is developer' do let(:current_user) { developer } it { is_expected.to be_disallowed(:admin_security_attributes) } + it { is_expected.to be_disallowed(:read_security_attribute) } end end end diff --git a/ee/spec/policies/security/attribute_policy_spec.rb b/ee/spec/policies/security/attribute_policy_spec.rb index 9833d40f7eabba87ea06568c4fe74b8d5423bdc2..c8786217d86fa74d435f03382e3a2a229e14452c 100644 --- a/ee/spec/policies/security/attribute_policy_spec.rb +++ b/ee/spec/policies/security/attribute_policy_spec.rb @@ -47,4 +47,20 @@ it { is_expected.to be_disallowed(:read_security_attribute) } end + + context 'when user has reporter access to root group' do + before_all do + root_group.add_reporter(user) + end + + it { is_expected.to be_disallowed(:read_security_attribute) } + end + + context 'when user has guest access to root group' do + before_all do + root_group.add_guest(user) + end + + it { is_expected.to be_disallowed(:read_security_attribute) } + end end diff --git a/ee/spec/policies/security/category_policy_spec.rb b/ee/spec/policies/security/category_policy_spec.rb deleted file mode 100644 index 4d112c90ebd7d202133a5f8e2691eba2a2eb4375..0000000000000000000000000000000000000000 --- a/ee/spec/policies/security/category_policy_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Security::CategoryPolicy, feature_category: :security_asset_inventories do - let_it_be(:root_group) { create(:group) } - let_it_be(:group) { create(:group, parent: root_group) } - let_it_be(:user) { create(:user) } - let_it_be(:category) { create(:security_category, namespace: root_group) } - - subject { described_class.new(user, category) } - - context 'when user is maintainer of root group' do - before_all do - root_group.add_maintainer(user) - end - - it { is_expected.to be_allowed(:read_security_category) } - end - - context 'when user is maintainer of subgroup' do - before_all do - group.add_maintainer(user) - end - - it { is_expected.to be_disallowed(:read_security_category) } - end - - context 'when user has developer access to root_group' do - before_all do - root_group.add_developer(user) - end - - it { is_expected.to be_disallowed(:read_security_category) } - end - - context 'when user has no access to any group' do - subject { described_class.new(user, category) } - - it { is_expected.to be_disallowed(:read_security_category) } - end - - context 'when user is owner of subgroup' do - before_all do - group.add_owner(user) - end - - it { is_expected.to be_disallowed(:read_security_category) } - end -end diff --git a/ee/spec/requests/custom_roles/read_security_attribute/request_spec.rb b/ee/spec/requests/custom_roles/read_security_attribute/request_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..05dff4d48817c881188260178a891432257b179f --- /dev/null +++ b/ee/spec/requests/custom_roles/read_security_attribute/request_spec.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User with read_security_attribute custom role', feature_category: :security_asset_inventories do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:user) { create(:user) } + let_it_be(:role) { create(:member_role, :reporter, namespace: group, read_security_attribute: true) } + let_it_be(:membership) { create(:group_member, :reporter, member_role: role, user: user, group: group) } + + before do + stub_licensed_features(custom_roles: true, security_attributes: true) + stub_feature_flags(security_categories_and_attributes: true) + + # Clear and stub GraphqlKnownOperations to avoid "undefined method `map' for String" error + Gitlab::Webpack::GraphqlKnownOperations.clear_memoization! + allow(Gitlab::Webpack::GraphqlKnownOperations).to receive(:load).and_return(['SubgroupsAndProjects']) + end + + describe "GraphQL queries" do + let_it_be(:category) { create(:security_category, namespace: group) } + let_it_be(:attribute) { create(:security_attribute, namespace: group, security_category: category) } + let_it_be(:project_attribute) do + create(:project_to_security_attribute, project: project, security_attribute: attribute) + end + + let(:query) do + <<~GQL + query SubgroupsAndProjects($fullPath: ID!, $projectsFirst: Int, + $projectsAfter: String, $search: String, $hasSearch: Boolean!) { + group(fullPath: $fullPath) { + id + projects( + first: $projectsFirst + after: $projectsAfter + search: $search + includeSubgroups: $hasSearch + includeArchived: false + ) @skip(if: $hasSearch) { + pageInfo { + hasNextPage + endCursor + } + nodes { + id + name + path + fullPath + securityAttributes { + nodes { + id + securityCategory { + id + name + } + name + description + color + } + } + } + } + } + } + GQL + end + + let(:variables) do + { + fullPath: group.full_path, + search: "", + hasSearch: false, + projectsFirst: 20, + projectsAfter: nil + } + end + + describe 'security attributes query for projects' do + it 'can access security attributes for projects' do + post_graphql(query, current_user: user, variables: variables) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_blank + + projects_data = graphql_data.dig('group', 'projects', 'nodes') + expect(projects_data).to be_present + expect(projects_data.first.dig('securityAttributes', 'nodes')).not_to be_empty + end + end + + context 'when user does not have the custom role' do + let_it_be(:user_without_permission) { create(:user) } + let_it_be(:membership_without_role) do + create(:group_member, :reporter, user: user_without_permission, group: group) + end + + it 'cannot access security attributes' do + post_graphql(query, current_user: user_without_permission, variables: variables) + + expect(response).to have_gitlab_http_status(:success) + + # Authorization should prevent access - either through errors or empty results + if graphql_errors.blank? + projects_data = graphql_data.dig('group', 'projects', 'nodes') + expect(projects_data.first.dig('securityAttributes', 'nodes')).to be_empty + else + expect(graphql_errors).to be_present + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5b4b01827d17825a7f9e0c7ab41a0e6d26748c57..ebb43f0c4167a44fac5a7665428dc2329034609f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -40427,6 +40427,9 @@ msgstr "" msgid "MemberRole|Secrets management" msgstr "" +msgid "MemberRole|Security asset inventories" +msgstr "" + msgid "MemberRole|Security policy dependency" msgstr ""