From 6b34a049e3d2392110cd9329654bc9ae1dfea474 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 18 Oct 2023 14:55:45 -0600 Subject: [PATCH 01/19] Remove `admin_vulnerability` from Developer role This change introduces a feature flag that can be enabled on a Project or Group actor to disable the `:admin_vulnerability` permission on the Developer role. --- ee/app/policies/ee/group_policy.rb | 8 +++ ee/app/policies/ee/project_policy.rb | 7 ++- ...eveloper_access_to_admin_vulnerability.yml | 8 +++ .../vulnerabilities_controller_spec.rb | 1 + .../security/vulnerability_report_spec.rb | 1 + .../finding/create_merge_request_spec.rb | 2 + .../security/finding/dismiss_spec.rb | 1 + .../vulnerabilities/bulk_dismiss_spec.rb | 3 +- .../mutations/vulnerabilities/confirm_spec.rb | 1 + .../create_external_issue_link_spec.rb | 1 + .../mutations/vulnerabilities/create_spec.rb | 1 + .../mutations/vulnerabilities/dismiss_spec.rb | 1 + .../mutations/vulnerabilities/resolve_spec.rb | 1 + .../revert_to_detected_spec.rb | 1 + ee/spec/helpers/security_helper_spec.rb | 1 + .../helpers/vulnerabilities_helper_spec.rb | 2 + ee/spec/policies/group_policy_spec.rb | 30 ++++++++- ee/spec/policies/project_policy_spec.rb | 61 +++++++++++++++++++ .../security/finding/create_issue_spec.rb | 1 + .../finding/create_merge_request_spec.rb | 3 +- .../finding/revert_to_detected_spec.rb | 1 + .../vulnerabilities/bulk_dismiss_spec.rb | 4 ++ .../create_external_issue_link_spec.rb | 1 + .../requests/api/internal/kubernetes_spec.rb | 2 + ee/spec/requests/api/vulnerabilities_spec.rb | 35 +++++++++++ .../security/findings/dismiss_service_spec.rb | 1 + .../bulk_dismiss_service_spec.rb | 4 +- .../vulnerabilities/confirm_service_spec.rb | 5 ++ .../vulnerabilities/create_service_spec.rb | 1 + .../vulnerabilities/dismiss_service_spec.rb | 6 ++ ...eate_from_security_finding_service_spec.rb | 1 + .../manually_create_service_spec.rb | 1 + .../vulnerabilities/resolve_service_spec.rb | 5 ++ .../revert_to_detected_service_spec.rb | 5 ++ .../create_issue_service_spec.rb | 1 + .../create_merge_request_service_spec.rb | 1 + ...board_vulnerability_create_service_spec.rb | 1 + ...oard_vulnerability_resolve_service_spec.rb | 1 + .../vulnerabilities/update_service_spec.rb | 1 + .../create_service_spec.rb | 1 + .../destroy_service_spec.rb | 1 + ..._create_state_transition_for_same_state.rb | 1 + 42 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 ee/config/feature_flags/development/disable_developer_access_to_admin_vulnerability.yml diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index 5a162d3c861598..9178e5ce744b61 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -247,6 +247,11 @@ module GroupPolicy Ability.allowed?(@user, :developer_access, security_orchestration_policy_configuration.security_policy_management_project) end + condition(:developer_access_to_admin_vulnerability) do + ::Feature.disabled?(:disable_developer_access_to_admin_vulnerability, subject) && + can?(:developer_access) + end + rule { user_banned_from_namespace }.prevent_all rule { public_group | logged_in_viewable }.policy do @@ -470,6 +475,9 @@ module GroupPolicy rule { security_dashboard_enabled & developer }.policy do enable :read_group_security_dashboard + end + + rule { security_dashboard_enabled & (owner | developer_access_to_admin_vulnerability) }.policy do enable :admin_vulnerability end diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 079f18a98c7544..9f68d343ea0805 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -267,6 +267,11 @@ module ProjectPolicy ).has_ability? end + condition(:developer_access_to_admin_vulnerability) do + ::Feature.disabled?(:disable_developer_access_to_admin_vulnerability, subject) && + can?(:developer_access) + end + with_scope :subject condition(:suggested_reviewers_available) do @subject.can_suggest_reviewers? @@ -446,7 +451,7 @@ module ProjectPolicy enable :read_vulnerability end - rule { can?(:read_security_resource) & can?(:developer_access) }.policy do + rule { can?(:read_security_resource) & (owner | developer_access_to_admin_vulnerability) }.policy do enable :admin_vulnerability end diff --git a/ee/config/feature_flags/development/disable_developer_access_to_admin_vulnerability.yml b/ee/config/feature_flags/development/disable_developer_access_to_admin_vulnerability.yml new file mode 100644 index 00000000000000..999fe673f5692f --- /dev/null +++ b/ee/config/feature_flags/development/disable_developer_access_to_admin_vulnerability.yml @@ -0,0 +1,8 @@ +--- +name: disable_developer_access_to_admin_vulnerability +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134579 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/429122 +milestone: '16.6' +type: development +group: group::authorization +default_enabled: false diff --git a/ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb b/ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb index e0155e59547f4b..67c3114096a994 100644 --- a/ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb +++ b/ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb @@ -10,6 +10,7 @@ render_views before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) group.add_developer(user) stub_licensed_features(security_dashboard: true) sign_in(user) diff --git a/ee/spec/features/projects/security/vulnerability_report_spec.rb b/ee/spec/features/projects/security/vulnerability_report_spec.rb index 4d71ecb6699234..4a4cd204ede258 100644 --- a/ee/spec/features/projects/security/vulnerability_report_spec.rb +++ b/ee/spec/features/projects/security/vulnerability_report_spec.rb @@ -61,6 +61,7 @@ security_dashboard: true, sast: true ) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) sign_in(user) end diff --git a/ee/spec/graphql/mutations/security/finding/create_merge_request_spec.rb b/ee/spec/graphql/mutations/security/finding/create_merge_request_spec.rb index 3fdcc237dd228b..712eb88a016bb2 100644 --- a/ee/spec/graphql/mutations/security/finding/create_merge_request_spec.rb +++ b/ee/spec/graphql/mutations/security/finding/create_merge_request_spec.rb @@ -68,6 +68,7 @@ before do stub_licensed_features(security_dashboard: true) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) allow_next_instance_of(Commits::CommitPatchService) do |service| allow(service).to receive(:execute).and_return({ status: :success }) @@ -120,6 +121,7 @@ before do stub_licensed_features(security_dashboard: true) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(current_user) end diff --git a/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb b/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb index 8dcf3ede0a902b..b66196638687e6 100644 --- a/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb +++ b/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb @@ -38,6 +38,7 @@ context 'when the user has access to the project' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) security_finding.project.add_developer(user) end diff --git a/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb index a29202062a5cd4..8f9bb9e0e5a7bf 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb @@ -20,6 +20,7 @@ before_all do project.add_developer(user) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end subject do @@ -46,7 +47,7 @@ expect(queries.occurrences_starting_with('SELECT "projects"').values.sum).to eq(1) expect(queries.occurrences_starting_with('UPDATE "vulnerabilities"').values.sum).to eq(1) expect(queries.occurrences_starting_with('UPDATE "vulnerability_reads"').values.sum).to eq(1) - expect(queries.count).to be <= 15 + expect(queries.count).to be <= 16 end end end diff --git a/ee/spec/graphql/mutations/vulnerabilities/confirm_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/confirm_spec.rb index 4f2cd7833af68f..6d77696c6bdc0e 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/confirm_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/confirm_spec.rb @@ -34,6 +34,7 @@ context 'when user has access to the project', :aggregate_failures do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) vulnerability.project.add_developer(user) end diff --git a/ee/spec/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb index c544b3de51237a..279d14a7bde2b1 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb @@ -24,6 +24,7 @@ context 'when user has access to the project' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) vulnerability.project.add_developer(user) allow_next_instance_of(::VulnerabilityExternalIssueLinks::CreateService) do |create_service| allow(create_service).to receive(:execute).and_return(result) diff --git a/ee/spec/graphql/mutations/vulnerabilities/create_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/create_spec.rb index 744812c4a39abc..c61e9dfe60dc53 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/create_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/create_spec.rb @@ -9,6 +9,7 @@ let(:mutated_vulnerability) { subject[:vulnerability] } before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/graphql/mutations/vulnerabilities/dismiss_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/dismiss_spec.rb index 12df1b8831c2a7..d4b8fbbefedac9 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/dismiss_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/dismiss_spec.rb @@ -29,6 +29,7 @@ context 'when user has access to the project' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) vulnerability.project.add_developer(user) end diff --git a/ee/spec/graphql/mutations/vulnerabilities/resolve_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/resolve_spec.rb index 71c18e446c8c2e..78cd00b4907f91 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/resolve_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/resolve_spec.rb @@ -27,6 +27,7 @@ context 'when user has access to the project', :aggregate_failures do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) vulnerability.project.add_developer(user) end diff --git a/ee/spec/graphql/mutations/vulnerabilities/revert_to_detected_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/revert_to_detected_spec.rb index 442253dc85fd30..529e538acc5e4b 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/revert_to_detected_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/revert_to_detected_spec.rb @@ -35,6 +35,7 @@ context 'when user has access to the project' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) vulnerability.project.add_developer(user) end diff --git a/ee/spec/helpers/security_helper_spec.rb b/ee/spec/helpers/security_helper_spec.rb index d824bc9e900406..94f79863de74e8 100644 --- a/ee/spec/helpers/security_helper_spec.rb +++ b/ee/spec/helpers/security_helper_spec.rb @@ -13,6 +13,7 @@ before do stub_licensed_features(security_dashboard: true) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.namespace.add_maintainer(current_user) if has_group create(:users_security_dashboard_project, user: current_user, project: project) end diff --git a/ee/spec/helpers/vulnerabilities_helper_spec.rb b/ee/spec/helpers/vulnerabilities_helper_spec.rb index 7c705ff612ddf8..c22c0a9f0feb74 100644 --- a/ee/spec/helpers/vulnerabilities_helper_spec.rb +++ b/ee/spec/helpers/vulnerabilities_helper_spec.rb @@ -104,6 +104,7 @@ context 'when user can manage related issues' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -132,6 +133,7 @@ context 'when user can admin vulnerabilities' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 2f24a3d96d450a..9e0ebd69355316 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -1465,13 +1465,29 @@ def stub_group_saml_config(enabled) context 'with developer' do let(:current_user) { developer } - it { is_expected.to be_allowed(:admin_vulnerability) } + it { is_expected.to be_disallowed(:admin_vulnerability) } + + context "with `disable_developer_access_to_admin_vulnerability` disabled" do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { is_expected.to be_allowed(:admin_vulnerability) } + end end context 'with maintainer' do let(:current_user) { maintainer } - it { is_expected.to be_allowed(:admin_vulnerability) } + it { is_expected.to be_disallowed(:admin_vulnerability) } + + context "with `disable_developer_access_to_admin_vulnerability` disabled" do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { is_expected.to be_allowed(:admin_vulnerability) } + end end context 'with owner' do @@ -1500,7 +1516,15 @@ def stub_group_saml_config(enabled) group.add_developer(auditor) end - it { is_expected.to be_allowed(:admin_vulnerability) } + it { is_expected.to be_disallowed(:admin_vulnerability) } + + context "with `disable_developer_access_to_admin_vulnerability` disabled" do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { is_expected.to be_allowed(:admin_vulnerability) } + end end end end diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 097f120f0776d2..cce1bcc72bc6b1 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -1859,6 +1859,7 @@ before do allow(project.root_namespace).to receive(:read_only?).and_return(read_only) allow(project).to receive(:design_management_enabled?).and_return(true) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true, license_scanning: true, quality_management: true) end @@ -3113,6 +3114,66 @@ def create_member_role(member, abilities = member_role_abilities) end end + describe "#admin_vulnerability" do + before do + stub_licensed_features(security_dashboard: true) + end + + context "with guest" do + let(:current_user) { guest } + + it { is_expected.to be_disallowed(:admin_vulnerability) } + end + + context "with reporter" do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:admin_vulnerability) } + end + + context "with developer" do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:admin_vulnerability) } + + context "with `disable_developer_access_to_admin_vulnerability` disabled" do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { is_expected.to be_allowed(:admin_vulnerability) } + end + end + + context "with maintainer" do + let(:current_user) { maintainer } + + it { is_expected.to be_disallowed(:admin_vulnerability) } + + context "with `disable_developer_access_to_admin_vulnerability` disabled" do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { is_expected.to be_allowed(:admin_vulnerability) } + end + end + + context "with owner" do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:admin_vulnerability) } + + context "with `disable_developer_access_to_admin_vulnerability` disabled" do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { is_expected.to be_allowed(:admin_vulnerability) } + end + end + end + describe 'read_observability_metrics policy' do let(:current_user) { reporter } diff --git a/ee/spec/requests/api/graphql/mutations/security/finding/create_issue_spec.rb b/ee/spec/requests/api/graphql/mutations/security/finding/create_issue_spec.rb index a9c2cd4e5de6ca..ee6b711efb6e73 100644 --- a/ee/spec/requests/api/graphql/mutations/security/finding/create_issue_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/security/finding/create_issue_spec.rb @@ -62,6 +62,7 @@ context 'when the user has permission' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(current_user) end diff --git a/ee/spec/requests/api/graphql/mutations/security/finding/create_merge_request_spec.rb b/ee/spec/requests/api/graphql/mutations/security/finding/create_merge_request_spec.rb index b4850d4bf93667..cda3de303aad30 100644 --- a/ee/spec/requests/api/graphql/mutations/security/finding/create_merge_request_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/security/finding/create_merge_request_spec.rb @@ -66,7 +66,8 @@ allow(service).to receive(:execute).and_return({ status: :success }) end - security_finding.project.add_maintainer(current_user) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + security_finding.project.add_developer(current_user) end context 'with valid parameters' do diff --git a/ee/spec/requests/api/graphql/mutations/security/finding/revert_to_detected_spec.rb b/ee/spec/requests/api/graphql/mutations/security/finding/revert_to_detected_spec.rb index 83cc2ca82f43cf..baf3db98881a94 100644 --- a/ee/spec/requests/api/graphql/mutations/security/finding/revert_to_detected_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/security/finding/revert_to_detected_spec.rb @@ -68,6 +68,7 @@ end before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) security_finding.project.add_developer(current_user) end diff --git a/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb b/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb index d8574fc8c6e9d3..85cea5655adb0b 100644 --- a/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb @@ -37,6 +37,10 @@ def mutation_response project.add_developer(current_user) end + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + context "when security_dashboard is disabled" do before do stub_licensed_features(security_dashboard: false) diff --git a/ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb b/ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb index 488e0a6aee4a49..d6e1dd422b0238 100644 --- a/ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb @@ -36,6 +36,7 @@ def mutation_response context 'when the user has permission' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) vulnerability.project.add_developer(current_user) end diff --git a/ee/spec/requests/api/internal/kubernetes_spec.rb b/ee/spec/requests/api/internal/kubernetes_spec.rb index e262d3c518ba49..0c5c1a6e0e19a6 100644 --- a/ee/spec/requests/api/internal/kubernetes_spec.rb +++ b/ee/spec/requests/api/internal/kubernetes_spec.rb @@ -213,6 +213,7 @@ def send_request(headers: {}, params: {}) context 'is authenticated for an agent' do before do stub_licensed_features(security_dashboard: true) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_maintainer(agent.created_by_user) end @@ -334,6 +335,7 @@ def send_request(headers: {}, params: {}) context 'is authenticated for an agent' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/requests/api/vulnerabilities_spec.rb b/ee/spec/requests/api/vulnerabilities_spec.rb index 2e306cb4628e47..83bf0b91231e9d 100644 --- a/ee/spec/requests/api/vulnerabilities_spec.rb +++ b/ee/spec/requests/api/vulnerabilities_spec.rb @@ -21,6 +21,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -50,6 +51,10 @@ end describe 'permissions', :enable_admin_mode do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + it { expect { get_vulnerabilities }.to be_allowed_for(:admin) } it { expect { get_vulnerabilities }.to be_allowed_for(:owner).of(project) } it { expect { get_vulnerabilities }.to be_allowed_for(:maintainer).of(project) } @@ -72,6 +77,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -96,6 +102,10 @@ end describe 'permissions', :enable_admin_mode do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + it { expect { get_vulnerability }.to be_allowed_for(:admin) } it { expect { get_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { get_vulnerability }.to be_allowed_for(:maintainer).of(project) } @@ -119,6 +129,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -168,6 +179,10 @@ end describe 'permissions', :enable_admin_mode do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + it { expect { create_vulnerability }.to be_allowed_for(:admin) } it { expect { create_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { create_vulnerability }.to be_allowed_for(:maintainer).of(project) } @@ -194,6 +209,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -243,6 +259,10 @@ end describe 'permissions', :enable_admin_mode do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + it { expect { dismiss_vulnerability }.to be_allowed_for(:admin) } it { expect { dismiss_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { dismiss_vulnerability }.to be_allowed_for(:maintainer).of(project) } @@ -269,6 +289,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -316,6 +337,10 @@ end describe 'permissions', :enable_admin_mode do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + it { expect { resolve_vulnerability }.to be_allowed_for(:admin) } it { expect { resolve_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { resolve_vulnerability }.to be_allowed_for(:maintainer).of(project) } @@ -347,6 +372,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -378,6 +404,10 @@ end describe 'permissions', :enable_admin_mode do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + it { expect { confirm_vulnerability }.to be_allowed_for(:admin) } it { expect { confirm_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { confirm_vulnerability }.to be_allowed_for(:maintainer).of(project) } @@ -409,6 +439,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -470,6 +501,10 @@ end describe 'permissions', :enable_admin_mode do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) } it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:owner).of(project) } it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:maintainer).of(project) } diff --git a/ee/spec/services/security/findings/dismiss_service_spec.rb b/ee/spec/services/security/findings/dismiss_service_spec.rb index 5282f056deb01c..1f7e1c8abd3b64 100644 --- a/ee/spec/services/security/findings/dismiss_service_spec.rb +++ b/ee/spec/services/security/findings/dismiss_service_spec.rb @@ -21,6 +21,7 @@ describe '#execute' do context 'when the user is authorized' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) finding.project.add_developer(user) end diff --git a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb index b429c209e63457..21b1b3b5e93b29 100644 --- a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb @@ -18,6 +18,7 @@ end before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true) end @@ -128,8 +129,7 @@ queries = ActiveRecord::QueryRecorder.new do service.execute end - - expect(queries.count).to eq(14) + expect(queries.count).to eq(16) end end diff --git a/ee/spec/services/vulnerabilities/confirm_service_spec.rb b/ee/spec/services/vulnerabilities/confirm_service_spec.rb index 1122d18b30889b..a8ff7ac3013c02 100644 --- a/ee/spec/services/vulnerabilities/confirm_service_spec.rb +++ b/ee/spec/services/vulnerabilities/confirm_service_spec.rb @@ -22,6 +22,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -82,6 +83,10 @@ end describe 'permissions' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + context 'when admin mode is enabled', :enable_admin_mode do it { expect { confirm_vulnerability }.to be_allowed_for(:admin) } end diff --git a/ee/spec/services/vulnerabilities/create_service_spec.rb b/ee/spec/services/vulnerabilities/create_service_spec.rb index c95b610586b7f0..70b02d71686448 100644 --- a/ee/spec/services/vulnerabilities/create_service_spec.rb +++ b/ee/spec/services/vulnerabilities/create_service_spec.rb @@ -55,6 +55,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end diff --git a/ee/spec/services/vulnerabilities/dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/dismiss_service_spec.rb index 25891b355addd5..f704272ee53125 100644 --- a/ee/spec/services/vulnerabilities/dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/dismiss_service_spec.rb @@ -61,6 +61,7 @@ context 'when vulnerability state is different from the requested state' do context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -168,6 +169,7 @@ let(:vulnerability) { create(:vulnerability, :with_state_transition, :dismissed, project: project, to_state: :dismissed) } before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -193,6 +195,10 @@ end describe 'permissions' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + context 'when admin mode is enabled', :enable_admin_mode do it { expect { dismiss_vulnerability }.to be_allowed_for(:admin) } end diff --git a/ee/spec/services/vulnerabilities/find_or_create_from_security_finding_service_spec.rb b/ee/spec/services/vulnerabilities/find_or_create_from_security_finding_service_spec.rb index 29503a0fbf487f..441cd7fe69d37a 100644 --- a/ee/spec/services/vulnerabilities/find_or_create_from_security_finding_service_spec.rb +++ b/ee/spec/services/vulnerabilities/find_or_create_from_security_finding_service_spec.rb @@ -6,6 +6,7 @@ feature_category: :vulnerability_management do before do stub_licensed_features(security_dashboard: true) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end diff --git a/ee/spec/services/vulnerabilities/manually_create_service_spec.rb b/ee/spec/services/vulnerabilities/manually_create_service_spec.rb index dad17155ea7ca3..09c1b84983088f 100644 --- a/ee/spec/services/vulnerabilities/manually_create_service_spec.rb +++ b/ee/spec/services/vulnerabilities/manually_create_service_spec.rb @@ -16,6 +16,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end diff --git a/ee/spec/services/vulnerabilities/resolve_service_spec.rb b/ee/spec/services/vulnerabilities/resolve_service_spec.rb index f0c6392dbb729e..a49db7e5b6f153 100644 --- a/ee/spec/services/vulnerabilities/resolve_service_spec.rb +++ b/ee/spec/services/vulnerabilities/resolve_service_spec.rb @@ -23,6 +23,7 @@ context 'when vulnerability state is different from the requested state' do context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -77,6 +78,10 @@ end describe 'permissions' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + context 'when admin mode is enabled', :enable_admin_mode do it { expect { resolve_vulnerability }.to be_allowed_for(:admin) } end diff --git a/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb b/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb index 67df8a6829b393..d553e475ed9bbd 100644 --- a/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb +++ b/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb @@ -51,6 +51,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end @@ -99,6 +100,10 @@ end describe 'permissions' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + context 'when admin mode is enabled', :enable_admin_mode do it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) } end diff --git a/ee/spec/services/vulnerabilities/security_finding/create_issue_service_spec.rb b/ee/spec/services/vulnerabilities/security_finding/create_issue_service_spec.rb index 1e31b06b68b164..396c6a28d58c6e 100644 --- a/ee/spec/services/vulnerabilities/security_finding/create_issue_service_spec.rb +++ b/ee/spec/services/vulnerabilities/security_finding/create_issue_service_spec.rb @@ -6,6 +6,7 @@ feature_category: :vulnerability_management do before do stub_licensed_features(security_dashboard: true) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end diff --git a/ee/spec/services/vulnerabilities/security_finding/create_merge_request_service_spec.rb b/ee/spec/services/vulnerabilities/security_finding/create_merge_request_service_spec.rb index 18b80df5ca5649..aaf06e04334060 100644 --- a/ee/spec/services/vulnerabilities/security_finding/create_merge_request_service_spec.rb +++ b/ee/spec/services/vulnerabilities/security_finding/create_merge_request_service_spec.rb @@ -36,6 +36,7 @@ before do stub_licensed_features(security_dashboard: true) + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) group.add_developer(user) end diff --git a/ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb b/ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb index 23488dd17e0f1e..cf2738c1f10497 100644 --- a/ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb +++ b/ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb @@ -51,6 +51,7 @@ context 'with authorized user' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end diff --git a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb index 91ab0ed80efa7a..522a851b140c16 100644 --- a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb +++ b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb @@ -35,6 +35,7 @@ context 'with feature enabled' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/services/vulnerabilities/update_service_spec.rb b/ee/spec/services/vulnerabilities/update_service_spec.rb index 453783c8d61f20..8accd94b3ca926 100644 --- a/ee/spec/services/vulnerabilities/update_service_spec.rb +++ b/ee/spec/services/vulnerabilities/update_service_spec.rb @@ -22,6 +22,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end diff --git a/ee/spec/services/vulnerability_feedback/create_service_spec.rb b/ee/spec/services/vulnerability_feedback/create_service_spec.rb index c67519a454e202..5995997c25366a 100644 --- a/ee/spec/services/vulnerability_feedback/create_service_spec.rb +++ b/ee/spec/services/vulnerability_feedback/create_service_spec.rb @@ -17,6 +17,7 @@ let(:security_finding) { security_findings.first } before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) group.add_developer(user) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/services/vulnerability_feedback/destroy_service_spec.rb b/ee/spec/services/vulnerability_feedback/destroy_service_spec.rb index 69fe76443466d9..5aec8eebb9dbf4 100644 --- a/ee/spec/services/vulnerability_feedback/destroy_service_spec.rb +++ b/ee/spec/services/vulnerability_feedback/destroy_service_spec.rb @@ -10,6 +10,7 @@ let(:service_object) { described_class.new(project, user, vulnerability_feedback, revert_vulnerability_state: revert_vulnerability_state) } before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/support/shared_examples/services/vulnerabilities/does_not_create_state_transition_for_same_state.rb b/ee/spec/support/shared_examples/services/vulnerabilities/does_not_create_state_transition_for_same_state.rb index 576d8ccaa5b95b..e57eda00693bdd 100644 --- a/ee/spec/support/shared_examples/services/vulnerabilities/does_not_create_state_transition_for_same_state.rb +++ b/ee/spec/support/shared_examples/services/vulnerabilities/does_not_create_state_transition_for_same_state.rb @@ -6,6 +6,7 @@ context 'with an authorized user with proper permissions' do before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(user) end -- GitLab From c331fddb7744f726a0ce2f8261ee1be160342289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= Date: Thu, 26 Oct 2023 18:03:08 +0000 Subject: [PATCH 02/19] Apply 1 suggestion(s) to 1 file(s) --- ee/app/policies/ee/group_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index 9178e5ce744b61..8ab1b18989c951 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -477,7 +477,7 @@ module GroupPolicy enable :read_group_security_dashboard end - rule { security_dashboard_enabled & (owner | developer_access_to_admin_vulnerability) }.policy do + rule { security_dashboard_enabled & (maintainer | developer_access_to_admin_vulnerability) }.policy do enable :admin_vulnerability end -- GitLab From 76e2100399828b9b6bae329f18e35d29cb649a7d Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 26 Oct 2023 12:07:01 -0600 Subject: [PATCH 03/19] Ensure that maintainers will continue to have admin_vulnerability --- ee/app/policies/ee/project_policy.rb | 2 +- ee/spec/policies/group_policy_spec.rb | 2 +- ee/spec/policies/project_policy_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 9f68d343ea0805..2c2f0d7c4b0284 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -451,7 +451,7 @@ module ProjectPolicy enable :read_vulnerability end - rule { can?(:read_security_resource) & (owner | developer_access_to_admin_vulnerability) }.policy do + rule { can?(:read_security_resource) & (maintainer | developer_access_to_admin_vulnerability) }.policy do enable :admin_vulnerability end diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 9e0ebd69355316..2078b5fdbd1355 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -1479,7 +1479,7 @@ def stub_group_saml_config(enabled) context 'with maintainer' do let(:current_user) { maintainer } - it { is_expected.to be_disallowed(:admin_vulnerability) } + it { is_expected.to be_allowed(:admin_vulnerability) } context "with `disable_developer_access_to_admin_vulnerability` disabled" do before do diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index cce1bcc72bc6b1..da34444ecb2c2d 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3148,7 +3148,7 @@ def create_member_role(member, abilities = member_role_abilities) context "with maintainer" do let(:current_user) { maintainer } - it { is_expected.to be_disallowed(:admin_vulnerability) } + it { is_expected.to be_allowed(:admin_vulnerability) } context "with `disable_developer_access_to_admin_vulnerability` disabled" do before do -- GitLab From b7760abd198830bcb602edc11da279e4921d531c Mon Sep 17 00:00:00 2001 From: mo khan Date: Mon, 30 Oct 2023 11:34:44 -0600 Subject: [PATCH 04/19] Revert change to query count assertion --- ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb | 2 +- ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb index 8f9bb9e0e5a7bf..5c81b399cbe135 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb @@ -47,7 +47,7 @@ expect(queries.occurrences_starting_with('SELECT "projects"').values.sum).to eq(1) expect(queries.occurrences_starting_with('UPDATE "vulnerabilities"').values.sum).to eq(1) expect(queries.occurrences_starting_with('UPDATE "vulnerability_reads"').values.sum).to eq(1) - expect(queries.count).to be <= 16 + expect(queries.count).to be <= 15 end end end diff --git a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb index 21b1b3b5e93b29..586a477f7c10cb 100644 --- a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb @@ -129,7 +129,7 @@ queries = ActiveRecord::QueryRecorder.new do service.execute end - expect(queries.count).to eq(16) + expect(queries.count).to eq(14) end end -- GitLab From 154eac379841ce41fbb33c5025609fe91e600538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= Date: Wed, 1 Nov 2023 20:26:19 +0000 Subject: [PATCH 05/19] Apply 1 suggestion(s) to 1 file(s) --- ee/spec/policies/group_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 2078b5fdbd1355..43d6ff7a97bc53 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -1467,7 +1467,7 @@ def stub_group_saml_config(enabled) it { is_expected.to be_disallowed(:admin_vulnerability) } - context "with `disable_developer_access_to_admin_vulnerability` disabled" do + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do before do stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end -- GitLab From 227c3e884298ed25cf6628b791762101a6bceb5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= Date: Wed, 1 Nov 2023 20:26:39 +0000 Subject: [PATCH 06/19] Apply 1 suggestion(s) to 1 file(s) --- ee/spec/policies/group_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 43d6ff7a97bc53..8d8e08f5ca5856 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -1481,7 +1481,7 @@ def stub_group_saml_config(enabled) it { is_expected.to be_allowed(:admin_vulnerability) } - context "with `disable_developer_access_to_admin_vulnerability` disabled" do + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do before do stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end -- GitLab From 7a0b5d3c3b23c4e0e7b573b1e00ecca88485f732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= Date: Wed, 1 Nov 2023 20:26:58 +0000 Subject: [PATCH 07/19] Apply 1 suggestion(s) to 1 file(s) --- ee/spec/policies/group_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 8d8e08f5ca5856..a5463880d67e97 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -1518,7 +1518,7 @@ def stub_group_saml_config(enabled) it { is_expected.to be_disallowed(:admin_vulnerability) } - context "with `disable_developer_access_to_admin_vulnerability` disabled" do + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do before do stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end -- GitLab From 6e0ef86d972fa77740375f2d8914305a32fc7d68 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 1 Nov 2023 14:31:20 -0600 Subject: [PATCH 08/19] Delegate to group actor for feature flag --- ee/app/policies/ee/project_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 2c2f0d7c4b0284..5c8aee63e69349 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -268,7 +268,7 @@ module ProjectPolicy end condition(:developer_access_to_admin_vulnerability) do - ::Feature.disabled?(:disable_developer_access_to_admin_vulnerability, subject) && + ::Feature.disabled?(:disable_developer_access_to_admin_vulnerability, subject&.group) && can?(:developer_access) end -- GitLab From b8237bdfa7b6c686c31fae091188b88aa7e52290 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 1 Nov 2023 14:32:25 -0600 Subject: [PATCH 09/19] Remove spec for maintainer --- ee/spec/policies/project_policy_spec.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index da34444ecb2c2d..450c8b76436fbf 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3149,14 +3149,6 @@ def create_member_role(member, abilities = member_role_abilities) let(:current_user) { maintainer } it { is_expected.to be_allowed(:admin_vulnerability) } - - context "with `disable_developer_access_to_admin_vulnerability` disabled" do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - - it { is_expected.to be_allowed(:admin_vulnerability) } - end end context "with owner" do -- GitLab From f812e27abc558b190eb72f41f4ca6e43ccdf8e20 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 1 Nov 2023 14:32:39 -0600 Subject: [PATCH 10/19] Remove spec for owner --- ee/spec/policies/project_policy_spec.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 450c8b76436fbf..45b2c36670feeb 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3155,14 +3155,6 @@ def create_member_role(member, abilities = member_role_abilities) let(:current_user) { owner } it { is_expected.to be_allowed(:admin_vulnerability) } - - context "with `disable_developer_access_to_admin_vulnerability` disabled" do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - - it { is_expected.to be_allowed(:admin_vulnerability) } - end end end -- GitLab From 197a23b7f1059cef737d4a25d8525a887ed7c2fc Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 1 Nov 2023 16:05:43 -0600 Subject: [PATCH 11/19] Add specs for GraphQL endpoints requiring admin_vulnerability --- ee/spec/factories/member_roles.rb | 5 + .../admin_vulnerability/request_spec.rb | 282 +++++++++++++++++- spec/support/helpers/graphql_helpers.rb | 1 + 3 files changed, 275 insertions(+), 13 deletions(-) diff --git a/ee/spec/factories/member_roles.rb b/ee/spec/factories/member_roles.rb index 1c6f9806ee902c..9132fde33640ad 100644 --- a/ee/spec/factories/member_roles.rb +++ b/ee/spec/factories/member_roles.rb @@ -8,5 +8,10 @@ trait(:developer) { base_access_level { Gitlab::Access::DEVELOPER } } trait(:reporter) { base_access_level { Gitlab::Access::REPORTER } } trait(:guest) { base_access_level { Gitlab::Access::GUEST } } + + trait :admin_vulnerability do + admin_vulnerability { true } + read_vulnerability { true } + end end end diff --git a/ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb b/ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb index 29eb7bb8ee0253..b300f7d477de2b 100644 --- a/ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb +++ b/ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb @@ -5,25 +5,18 @@ RSpec.describe 'User with admin_vulnerability custom role', feature_category: :system_access do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository, :in_group) } - let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) } + let_it_be(:role) { create(:member_role, :guest, :admin_vulnerability, namespace: project.group) } + let_it_be(:membership) { create(:group_member, :guest, user: user, source: project.group, member_role: role) } before do stub_licensed_features(custom_roles: true, security_dashboard: true) - - group_member = create(:group_member, :guest, user: user, source: project.group) - create( - :member_role, - :guest, - admin_vulnerability: true, - read_code: false, - read_vulnerability: true, - members: [group_member], - namespace: project.group - ) - sign_in(user) end describe Projects::Security::VulnerabilitiesController do + before do + sign_in(user) + end + describe "#new" do it 'user has access via a custom role' do get new_project_security_vulnerability_path(project) @@ -35,4 +28,267 @@ end end end + + describe Mutations::Security::Finding::CreateIssue do + include GraphqlHelpers + + pending "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:security_finding_create_issue, { + project: project.to_global_id.to_s, + uuid: SecureRandom.uuid + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:security_finding_create_issue) + expect(mutation_response).to be_present + expect(mutation_response['errors']).to be_empty + end + end + + describe Mutations::Security::Finding::CreateMergeRequest do + include GraphqlHelpers + + pending "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:security_finding_create_merge_request, { + uuid: SecureRandom.uuid + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:security_finding_create_merge_request) + expect(mutation_response).to be_present + expect(mutation_response['errors']).to be_empty + end + end + + describe Mutations::Security::Finding::Dismiss do + include GraphqlHelpers + + let_it_be(:pipeline) { create(:ee_ci_pipeline, project: project) } + let_it_be(:build) { create(:ci_build, :success, project: project, pipeline: pipeline) } + let_it_be(:scan) { create(:security_scan, build: build) } + let_it_be(:security_finding) do + create(:security_finding, :with_finding_data, scan: scan, remediation_byte_offsets: [{ + "start_byte" => 0, + "end_byte" => 1 + }]) + end + + pending "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:security_finding_dismiss, { + uuid: security_finding.uuid, + comment: "dismissal feedback", + dismissal_reason: "USED_IN_TESTS" + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:security_finding_dismiss) + expect(mutation_response).to be_present + expect(mutation_response["securityFinding"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Security::Finding::RevertToDetected do + include GraphqlHelpers + + pending "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:security_finding_revert_to_detected, { + uuid: SecureRandom.uuid + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:security_finding_revert_to_detected) + expect(mutation_response).to be_present + expect(mutation_response['errors']).to be_empty + end + end + + describe Mutations::Vulnerabilities::BulkDismiss do + include GraphqlHelpers + + let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) } + + it "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerabilities_dismiss, { + vulnerability_ids: [vulnerability.to_global_id.to_s], + comment: 'Dismissal Feedback', + dismissal_reason: 'USED_IN_TESTS' + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerabilities_dismiss) + expect(mutation_response).to be_present + expect(mutation_response["vulnerabilities"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Vulnerabilities::Confirm do + include GraphqlHelpers + + let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) } + + it "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerability_confirm, { + id: vulnerability.to_global_id.to_s, + comment: "A comment" + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerability_confirm) + expect(mutation_response).to be_present + expect(mutation_response["vulnerability"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Vulnerabilities::Create do + include GraphqlHelpers + + it "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerability_create, { + project: project.to_global_id, + name: "example", + description: "example", + scanner: { + id: "my-custom-scanner", + name: "example", + url: "https://example.org", + vendor: { name: "example" }, + version: "1.0.0" + }, + identifiers: [{ + name: "example", + url: "https://example.org/example" + }], + state: "DETECTED", + severity: "UNKNOWN", + confidence: "UNKNOWN", + solution: "curl -s 'https://unpkg.com/emoji.json@13.1.0/emoji.json' | jq -r '.[] | .char'", + message: "example" + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerability_create) + expect(mutation_response).to be_present + expect(mutation_response["vulnerability"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Vulnerabilities::CreateExternalIssueLink do + include GraphqlHelpers + + let_it_be(:vulnerability) { create(:vulnerability, project: project) } + + pending "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerability_external_issue_link_create, { + id: vulnerability.to_global_id.to_s, + link_type: 'CREATED', + external_tracker: 'JIRA' + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerability_external_issue_link_create) + expect(mutation_response).to be_present + expect(mutation_response["externalIssueLink"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Vulnerabilities::CreateIssueLink do + include GraphqlHelpers + + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:vulnerability) { create(:vulnerability, project: project) } + + it "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerability_issue_link_create, { + issue_id: issue.to_global_id.to_s, + vulnerability_ids: [vulnerability.to_global_id.to_s] + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerability_issue_link_create) + expect(mutation_response).to be_present + expect(mutation_response["issueLinks"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Vulnerabilities::DestroyExternalIssueLink do + include GraphqlHelpers + + let_it_be(:vulnerability) { create(:vulnerability, project: project) } + let_it_be(:external_issue_link) { create(:vulnerabilities_external_issue_link, vulnerability: vulnerability) } + + it "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerability_external_issue_link_destroy, { + id: external_issue_link.to_global_id.to_s + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerability_external_issue_link_destroy) + expect(mutation_response).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Vulnerabilities::Dismiss do + include GraphqlHelpers + + let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) } + + pending "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerability_dismiss, { + id: vulnerability.to_global_id.to_s, + comment: "comment", + dismissal_reason: "USED_IN_TESTS" + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerability_dismiss) + expect(mutation_response).to be_present + expect(mutation_response["vulnerability"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Vulnerabilities::Resolve do + include GraphqlHelpers + + let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) } + + it "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerability_resolve, { + id: vulnerability.to_global_id.to_s, + comment: "resolved" + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerability_resolve) + expect(mutation_response).to be_present + expect(mutation_response["vulnerability"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end + + describe Mutations::Vulnerabilities::RevertToDetected do + include GraphqlHelpers + + let_it_be(:vulnerability) { create(:vulnerability, :dismissed, :with_findings, project: project) } + + it "has access via a custom role" do + post_graphql_mutation(graphql_mutation(:vulnerability_revert_to_detected, { + id: vulnerability.to_global_id.to_s, + comment: "comment" + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + mutation_response = graphql_mutation_response(:vulnerability_revert_to_detected) + expect(mutation_response).to be_present + expect(mutation_response["vulnerability"]).to be_present + expect(mutation_response["errors"]).to be_empty + end + end end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 5eba982e3da70c..085340d6cb96cc 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -2,6 +2,7 @@ module GraphqlHelpers def self.included(base) + base.include(::ApiHelpers) base.include(::Gitlab::Graphql::Laziness) end -- GitLab From a5af24c90b53f3e65ce8408cae51d3c64a31614a Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 2 Nov 2023 11:17:58 -0600 Subject: [PATCH 12/19] Use maintainer access in tests and ensure admin mode is still supported --- ee/app/policies/ee/group_policy.rb | 2 +- ee/app/policies/ee/project_policy.rb | 2 +- .../vulnerabilities_controller_spec.rb | 3 +- .../security/vulnerability_report_spec.rb | 3 +- .../finding/create_merge_request_spec.rb | 18 +- .../security/finding/dismiss_spec.rb | 3 +- .../vulnerabilities/bulk_dismiss_spec.rb | 3 +- .../mutations/vulnerabilities/confirm_spec.rb | 3 +- .../create_external_issue_link_spec.rb | 3 +- .../mutations/vulnerabilities/create_spec.rb | 3 +- .../mutations/vulnerabilities/dismiss_spec.rb | 3 +- .../mutations/vulnerabilities/resolve_spec.rb | 3 +- .../revert_to_detected_spec.rb | 3 +- ee/spec/helpers/security_helper_spec.rb | 1 - .../helpers/vulnerabilities_helper_spec.rb | 6 +- ee/spec/policies/group_policy_spec.rb | 12 +- ee/spec/policies/project_policy_spec.rb | 1 - .../security/finding/create_issue_spec.rb | 3 +- .../finding/create_merge_request_spec.rb | 3 +- .../finding/revert_to_detected_spec.rb | 3 +- .../vulnerabilities/bulk_dismiss_spec.rb | 6 +- .../create_external_issue_link_spec.rb | 3 +- .../requests/api/internal/kubernetes_spec.rb | 2 - ee/spec/requests/api/vulnerabilities_spec.rb | 170 +++++++++++++----- .../security/findings/dismiss_service_spec.rb | 3 +- .../bulk_dismiss_service_spec.rb | 3 +- .../vulnerabilities/confirm_service_spec.rb | 30 +++- .../vulnerabilities/create_service_spec.rb | 3 +- .../vulnerabilities/dismiss_service_spec.rb | 35 +++- ...eate_from_security_finding_service_spec.rb | 3 +- .../manually_create_service_spec.rb | 3 +- .../vulnerabilities/resolve_service_spec.rb | 30 +++- .../revert_to_detected_service_spec.rb | 30 +++- .../create_issue_service_spec.rb | 3 +- .../create_merge_request_service_spec.rb | 3 +- ...board_vulnerability_create_service_spec.rb | 3 +- ...oard_vulnerability_resolve_service_spec.rb | 3 +- .../vulnerabilities/update_service_spec.rb | 3 +- .../create_service_spec.rb | 3 +- .../destroy_service_spec.rb | 3 +- ..._and_compliance_feature_shared_examples.rb | 4 +- ..._create_state_transition_for_same_state.rb | 3 +- 42 files changed, 271 insertions(+), 159 deletions(-) diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index 8ab1b18989c951..ca156731076e51 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -477,7 +477,7 @@ module GroupPolicy enable :read_group_security_dashboard end - rule { security_dashboard_enabled & (maintainer | developer_access_to_admin_vulnerability) }.policy do + rule { security_dashboard_enabled & (can?(:maintainer_access) | developer_access_to_admin_vulnerability) }.policy do enable :admin_vulnerability end diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 5c8aee63e69349..0413ed58446725 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -451,7 +451,7 @@ module ProjectPolicy enable :read_vulnerability end - rule { can?(:read_security_resource) & (maintainer | developer_access_to_admin_vulnerability) }.policy do + rule { can?(:read_security_resource) & (can?(:maintainer_access) | developer_access_to_admin_vulnerability) }.policy do enable :admin_vulnerability end diff --git a/ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb b/ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb index 67c3114096a994..f1c7356bba6aaf 100644 --- a/ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb +++ b/ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb @@ -10,8 +10,7 @@ render_views before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - group.add_developer(user) + group.add_maintainer(user) stub_licensed_features(security_dashboard: true) sign_in(user) end diff --git a/ee/spec/features/projects/security/vulnerability_report_spec.rb b/ee/spec/features/projects/security/vulnerability_report_spec.rb index 4a4cd204ede258..4278f3189581cc 100644 --- a/ee/spec/features/projects/security/vulnerability_report_spec.rb +++ b/ee/spec/features/projects/security/vulnerability_report_spec.rb @@ -61,8 +61,7 @@ security_dashboard: true, sast: true ) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) sign_in(user) end diff --git a/ee/spec/graphql/mutations/security/finding/create_merge_request_spec.rb b/ee/spec/graphql/mutations/security/finding/create_merge_request_spec.rb index 712eb88a016bb2..43e1ed22a741a1 100644 --- a/ee/spec/graphql/mutations/security/finding/create_merge_request_spec.rb +++ b/ee/spec/graphql/mutations/security/finding/create_merge_request_spec.rb @@ -68,7 +68,6 @@ before do stub_licensed_features(security_dashboard: true) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) allow_next_instance_of(Commits::CommitPatchService) do |service| allow(service).to receive(:execute).and_return({ status: :success }) @@ -121,15 +120,22 @@ before do stub_licensed_features(security_dashboard: true) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_developer(current_user) end - it 'returns an error' do - response = execute - expect(response[:errors]).not_to be_empty - expect(response[:merge_request]).to be_blank + it { expect { execute }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it 'returns an error' do + response = execute + expect(response[:errors]).not_to be_empty + expect(response[:merge_request]).to be_blank + end end end end diff --git a/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb b/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb index b66196638687e6..3a272fc49d6eac 100644 --- a/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb +++ b/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb @@ -38,8 +38,7 @@ context 'when the user has access to the project' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - security_finding.project.add_developer(user) + security_finding.project.add_maintainer(user) end context 'when the dismissal is successful' do diff --git a/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb index 5c81b399cbe135..a70fac66a5cb0b 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb @@ -19,8 +19,7 @@ end before_all do - project.add_developer(user) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + project.add_maintainer(user) end subject do diff --git a/ee/spec/graphql/mutations/vulnerabilities/confirm_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/confirm_spec.rb index 6d77696c6bdc0e..2a861b1348e829 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/confirm_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/confirm_spec.rb @@ -34,8 +34,7 @@ context 'when user has access to the project', :aggregate_failures do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - vulnerability.project.add_developer(user) + vulnerability.project.add_maintainer(user) end context 'when comment is not provided' do diff --git a/ee/spec/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb index 279d14a7bde2b1..d88a4095a357e6 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb @@ -24,8 +24,7 @@ context 'when user has access to the project' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - vulnerability.project.add_developer(user) + vulnerability.project.add_maintainer(user) allow_next_instance_of(::VulnerabilityExternalIssueLinks::CreateService) do |create_service| allow(create_service).to receive(:execute).and_return(result) end diff --git a/ee/spec/graphql/mutations/vulnerabilities/create_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/create_spec.rb index c61e9dfe60dc53..509a286136ca5f 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/create_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/create_spec.rb @@ -4,12 +4,11 @@ RSpec.describe Mutations::Vulnerabilities::Create, feature_category: :vulnerability_management do include GraphqlHelpers let_it_be_with_reload(:project) { create(:project) } - let_it_be(:user) { create(:user).tap { |user| project.add_developer(user) } } + let_it_be(:user) { create(:user).tap { |user| project.add_maintainer(user) } } let(:mutated_vulnerability) { subject[:vulnerability] } before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/graphql/mutations/vulnerabilities/dismiss_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/dismiss_spec.rb index d4b8fbbefedac9..c223c2a6c106c1 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/dismiss_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/dismiss_spec.rb @@ -29,8 +29,7 @@ context 'when user has access to the project' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - vulnerability.project.add_developer(user) + vulnerability.project.add_maintainer(user) end it 'returns the dismissed vulnerability' do diff --git a/ee/spec/graphql/mutations/vulnerabilities/resolve_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/resolve_spec.rb index 78cd00b4907f91..6c2260e2e908a4 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/resolve_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/resolve_spec.rb @@ -27,8 +27,7 @@ context 'when user has access to the project', :aggregate_failures do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - vulnerability.project.add_developer(user) + vulnerability.project.add_maintainer(user) end it 'returns the resolved vulnerability' do diff --git a/ee/spec/graphql/mutations/vulnerabilities/revert_to_detected_spec.rb b/ee/spec/graphql/mutations/vulnerabilities/revert_to_detected_spec.rb index 529e538acc5e4b..010a930ec01059 100644 --- a/ee/spec/graphql/mutations/vulnerabilities/revert_to_detected_spec.rb +++ b/ee/spec/graphql/mutations/vulnerabilities/revert_to_detected_spec.rb @@ -35,8 +35,7 @@ context 'when user has access to the project' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - vulnerability.project.add_developer(user) + vulnerability.project.add_maintainer(user) end context 'and no comment is provided' do diff --git a/ee/spec/helpers/security_helper_spec.rb b/ee/spec/helpers/security_helper_spec.rb index 94f79863de74e8..d824bc9e900406 100644 --- a/ee/spec/helpers/security_helper_spec.rb +++ b/ee/spec/helpers/security_helper_spec.rb @@ -13,7 +13,6 @@ before do stub_licensed_features(security_dashboard: true) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.namespace.add_maintainer(current_user) if has_group create(:users_security_dashboard_project, user: current_user, project: project) end diff --git a/ee/spec/helpers/vulnerabilities_helper_spec.rb b/ee/spec/helpers/vulnerabilities_helper_spec.rb index c22c0a9f0feb74..d0a5dcb7e06d26 100644 --- a/ee/spec/helpers/vulnerabilities_helper_spec.rb +++ b/ee/spec/helpers/vulnerabilities_helper_spec.rb @@ -104,8 +104,7 @@ context 'when user can manage related issues' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it { is_expected.to include(can_modify_related_issues: true) } @@ -133,8 +132,7 @@ context 'when user can admin vulnerabilities' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it { is_expected.to include(can_admin: true) } diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index a5463880d67e97..235a8317e0df53 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -1474,20 +1474,16 @@ def stub_group_saml_config(enabled) it { is_expected.to be_allowed(:admin_vulnerability) } end + + context 'with admin mode enabled', :enable_admin_mode do + it { is_expected.to be_allowed(:admin_vulnerability) } + end end context 'with maintainer' do let(:current_user) { maintainer } it { is_expected.to be_allowed(:admin_vulnerability) } - - context 'with `disable_developer_access_to_admin_vulnerability` disabled' do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - - it { is_expected.to be_allowed(:admin_vulnerability) } - end end context 'with owner' do diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 45b2c36670feeb..147e4bf1885931 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -1859,7 +1859,6 @@ before do allow(project.root_namespace).to receive(:read_only?).and_return(read_only) allow(project).to receive(:design_management_enabled?).and_return(true) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true, license_scanning: true, quality_management: true) end diff --git a/ee/spec/requests/api/graphql/mutations/security/finding/create_issue_spec.rb b/ee/spec/requests/api/graphql/mutations/security/finding/create_issue_spec.rb index ee6b711efb6e73..7b4f0730f78998 100644 --- a/ee/spec/requests/api/graphql/mutations/security/finding/create_issue_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/security/finding/create_issue_spec.rb @@ -62,8 +62,7 @@ context 'when the user has permission' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(current_user) + project.add_maintainer(current_user) end context 'with valid parameters' do diff --git a/ee/spec/requests/api/graphql/mutations/security/finding/create_merge_request_spec.rb b/ee/spec/requests/api/graphql/mutations/security/finding/create_merge_request_spec.rb index cda3de303aad30..b4850d4bf93667 100644 --- a/ee/spec/requests/api/graphql/mutations/security/finding/create_merge_request_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/security/finding/create_merge_request_spec.rb @@ -66,8 +66,7 @@ allow(service).to receive(:execute).and_return({ status: :success }) end - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - security_finding.project.add_developer(current_user) + security_finding.project.add_maintainer(current_user) end context 'with valid parameters' do diff --git a/ee/spec/requests/api/graphql/mutations/security/finding/revert_to_detected_spec.rb b/ee/spec/requests/api/graphql/mutations/security/finding/revert_to_detected_spec.rb index baf3db98881a94..3082b37abc1e48 100644 --- a/ee/spec/requests/api/graphql/mutations/security/finding/revert_to_detected_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/security/finding/revert_to_detected_spec.rb @@ -68,8 +68,7 @@ end before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - security_finding.project.add_developer(current_user) + security_finding.project.add_maintainer(current_user) end shared_examples 'properly sets the security finding state' do diff --git a/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb b/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb index 85cea5655adb0b..a4f7c511fdd97f 100644 --- a/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/vulnerabilities/bulk_dismiss_spec.rb @@ -34,11 +34,7 @@ def mutation_response context "when the user has access" do before_all do - project.add_developer(current_user) - end - - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + project.add_maintainer(current_user) end context "when security_dashboard is disabled" do diff --git a/ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb b/ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb index d6e1dd422b0238..eb2ac7fc47b06d 100644 --- a/ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb @@ -36,8 +36,7 @@ def mutation_response context 'when the user has permission' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - vulnerability.project.add_developer(current_user) + vulnerability.project.add_maintainer(current_user) end context 'when security_dashboard is disabled' do diff --git a/ee/spec/requests/api/internal/kubernetes_spec.rb b/ee/spec/requests/api/internal/kubernetes_spec.rb index 0c5c1a6e0e19a6..e262d3c518ba49 100644 --- a/ee/spec/requests/api/internal/kubernetes_spec.rb +++ b/ee/spec/requests/api/internal/kubernetes_spec.rb @@ -213,7 +213,6 @@ def send_request(headers: {}, params: {}) context 'is authenticated for an agent' do before do stub_licensed_features(security_dashboard: true) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) project.add_maintainer(agent.created_by_user) end @@ -335,7 +334,6 @@ def send_request(headers: {}, params: {}) context 'is authenticated for an agent' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/requests/api/vulnerabilities_spec.rb b/ee/spec/requests/api/vulnerabilities_spec.rb index 83bf0b91231e9d..d0f2d3baf65fd2 100644 --- a/ee/spec/requests/api/vulnerabilities_spec.rb +++ b/ee/spec/requests/api/vulnerabilities_spec.rb @@ -21,8 +21,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it 'returns all vulnerabilities of a project', :aggregate_failures do @@ -51,10 +50,6 @@ end describe 'permissions', :enable_admin_mode do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - it { expect { get_vulnerabilities }.to be_allowed_for(:admin) } it { expect { get_vulnerabilities }.to be_allowed_for(:owner).of(project) } it { expect { get_vulnerabilities }.to be_allowed_for(:maintainer).of(project) } @@ -64,6 +59,21 @@ it { expect { get_vulnerabilities }.to be_denied_for(:reporter).of(project) } it { expect { get_vulnerabilities }.to be_denied_for(:guest).of(project) } it { expect { get_vulnerabilities }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { expect { get_vulnerabilities }.to be_allowed_for(:admin) } + it { expect { get_vulnerabilities }.to be_allowed_for(:owner).of(project) } + it { expect { get_vulnerabilities }.to be_allowed_for(:maintainer).of(project) } + it { expect { get_vulnerabilities }.to be_allowed_for(:developer).of(project) } + it { expect { get_vulnerabilities }.to be_allowed_for(:auditor) } + it { expect { get_vulnerabilities }.to be_denied_for(:reporter).of(project) } + it { expect { get_vulnerabilities }.to be_denied_for(:guest).of(project) } + it { expect { get_vulnerabilities }.to be_denied_for(:anonymous) } + end end end @@ -77,8 +87,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it 'returns the desired vulnerability', :aggregate_failures do @@ -102,10 +111,6 @@ end describe 'permissions', :enable_admin_mode do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - it { expect { get_vulnerability }.to be_allowed_for(:admin) } it { expect { get_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { get_vulnerability }.to be_allowed_for(:maintainer).of(project) } @@ -115,6 +120,22 @@ it { expect { get_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { get_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { get_vulnerability }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { expect { get_vulnerability }.to be_allowed_for(:admin) } + it { expect { get_vulnerability }.to be_allowed_for(:owner).of(project) } + it { expect { get_vulnerability }.to be_allowed_for(:maintainer).of(project) } + it { expect { get_vulnerability }.to be_allowed_for(:developer).of(project) } + it { expect { get_vulnerability }.to be_allowed_for(:auditor) } + + it { expect { get_vulnerability }.to be_denied_for(:reporter).of(project) } + it { expect { get_vulnerability }.to be_denied_for(:guest).of(project) } + it { expect { get_vulnerability }.to be_denied_for(:anonymous) } + end end end @@ -129,8 +150,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it 'creates a vulnerability from finding and attaches it to the vulnerability', :aggregate_failures do @@ -179,19 +199,31 @@ end describe 'permissions', :enable_admin_mode do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - it { expect { create_vulnerability }.to be_allowed_for(:admin) } it { expect { create_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { create_vulnerability }.to be_allowed_for(:maintainer).of(project) } - it { expect { create_vulnerability }.to be_allowed_for(:developer).of(project) } + it { expect { create_vulnerability }.to be_denied_for(:developer).of(project) } it { expect { create_vulnerability }.to be_denied_for(:auditor) } it { expect { create_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { create_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { create_vulnerability }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { expect { create_vulnerability }.to be_allowed_for(:admin) } + it { expect { create_vulnerability }.to be_allowed_for(:owner).of(project) } + it { expect { create_vulnerability }.to be_allowed_for(:maintainer).of(project) } + it { expect { create_vulnerability }.to be_allowed_for(:developer).of(project) } + + it { expect { create_vulnerability }.to be_denied_for(:auditor) } + it { expect { create_vulnerability }.to be_denied_for(:reporter).of(project) } + it { expect { create_vulnerability }.to be_denied_for(:guest).of(project) } + it { expect { create_vulnerability }.to be_denied_for(:anonymous) } + end end end @@ -209,8 +241,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it_behaves_like 'responds with "not found" for an unknown vulnerability ID' @@ -259,19 +290,31 @@ end describe 'permissions', :enable_admin_mode do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - it { expect { dismiss_vulnerability }.to be_allowed_for(:admin) } it { expect { dismiss_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { dismiss_vulnerability }.to be_allowed_for(:maintainer).of(project) } - it { expect { dismiss_vulnerability }.to be_allowed_for(:developer).of(project) } + it { expect { dismiss_vulnerability }.to be_denied_for(:developer).of(project) } it { expect { dismiss_vulnerability }.to be_denied_for(:auditor) } it { expect { dismiss_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { dismiss_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { dismiss_vulnerability }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { expect { dismiss_vulnerability }.to be_allowed_for(:admin) } + it { expect { dismiss_vulnerability }.to be_allowed_for(:owner).of(project) } + it { expect { dismiss_vulnerability }.to be_allowed_for(:maintainer).of(project) } + it { expect { dismiss_vulnerability }.to be_allowed_for(:developer).of(project) } + + it { expect { dismiss_vulnerability }.to be_denied_for(:auditor) } + it { expect { dismiss_vulnerability }.to be_denied_for(:reporter).of(project) } + it { expect { dismiss_vulnerability }.to be_denied_for(:guest).of(project) } + it { expect { dismiss_vulnerability }.to be_denied_for(:anonymous) } + end end end @@ -289,8 +332,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it 'resolves a vulnerability and its associated findings', :freeze_time, :aggregate_failures do @@ -337,19 +379,31 @@ end describe 'permissions', :enable_admin_mode do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - it { expect { resolve_vulnerability }.to be_allowed_for(:admin) } it { expect { resolve_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { resolve_vulnerability }.to be_allowed_for(:maintainer).of(project) } - it { expect { resolve_vulnerability }.to be_allowed_for(:developer).of(project) } + it { expect { resolve_vulnerability }.to be_denied_for(:developer).of(project) } it { expect { resolve_vulnerability }.to be_denied_for(:auditor) } it { expect { resolve_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { resolve_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { resolve_vulnerability }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { expect { resolve_vulnerability }.to be_allowed_for(:admin) } + it { expect { resolve_vulnerability }.to be_allowed_for(:owner).of(project) } + it { expect { resolve_vulnerability }.to be_allowed_for(:maintainer).of(project) } + it { expect { resolve_vulnerability }.to be_allowed_for(:developer).of(project) } + + it { expect { resolve_vulnerability }.to be_denied_for(:auditor) } + it { expect { resolve_vulnerability }.to be_denied_for(:reporter).of(project) } + it { expect { resolve_vulnerability }.to be_denied_for(:guest).of(project) } + it { expect { resolve_vulnerability }.to be_denied_for(:anonymous) } + end end end @@ -372,8 +426,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it 'confirms a vulnerability and its associated findings', :freeze_time, :aggregate_failures do @@ -404,19 +457,31 @@ end describe 'permissions', :enable_admin_mode do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - it { expect { confirm_vulnerability }.to be_allowed_for(:admin) } it { expect { confirm_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { confirm_vulnerability }.to be_allowed_for(:maintainer).of(project) } - it { expect { confirm_vulnerability }.to be_allowed_for(:developer).of(project) } + it { expect { confirm_vulnerability }.to be_denied_for(:developer).of(project) } it { expect { confirm_vulnerability }.to be_denied_for(:auditor) } it { expect { confirm_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { confirm_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { confirm_vulnerability }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { expect { confirm_vulnerability }.to be_allowed_for(:admin) } + it { expect { confirm_vulnerability }.to be_allowed_for(:owner).of(project) } + it { expect { confirm_vulnerability }.to be_allowed_for(:maintainer).of(project) } + it { expect { confirm_vulnerability }.to be_allowed_for(:developer).of(project) } + + it { expect { confirm_vulnerability }.to be_denied_for(:auditor) } + it { expect { confirm_vulnerability }.to be_denied_for(:reporter).of(project) } + it { expect { confirm_vulnerability }.to be_denied_for(:guest).of(project) } + it { expect { confirm_vulnerability }.to be_denied_for(:anonymous) } + end end end @@ -439,8 +504,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it 'reverts a vulnerability and its associated findings to detected state', :freeze_time, :aggregate_failures do @@ -501,19 +565,31 @@ end describe 'permissions', :enable_admin_mode do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) } it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:owner).of(project) } it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:maintainer).of(project) } - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:developer).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:developer).of(project) } it { expect { revert_vulnerability_to_detected }.to be_denied_for(:auditor) } it { expect { revert_vulnerability_to_detected }.to be_denied_for(:reporter).of(project) } it { expect { revert_vulnerability_to_detected }.to be_denied_for(:guest).of(project) } it { expect { revert_vulnerability_to_detected }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) } + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:owner).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:maintainer).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:developer).of(project) } + + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:auditor) } + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:reporter).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:guest).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:anonymous) } + end end end end diff --git a/ee/spec/services/security/findings/dismiss_service_spec.rb b/ee/spec/services/security/findings/dismiss_service_spec.rb index 1f7e1c8abd3b64..7c8b0c20591633 100644 --- a/ee/spec/services/security/findings/dismiss_service_spec.rb +++ b/ee/spec/services/security/findings/dismiss_service_spec.rb @@ -21,8 +21,7 @@ describe '#execute' do context 'when the user is authorized' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - finding.project.add_developer(user) + finding.project.add_maintainer(user) end context 'when comment is added' do diff --git a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb index 586a477f7c10cb..bb8387c2c26591 100644 --- a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb @@ -14,11 +14,10 @@ describe '#execute' do before_all do - project.add_developer(user) + project.add_maintainer(user) end before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/services/vulnerabilities/confirm_service_spec.rb b/ee/spec/services/vulnerabilities/confirm_service_spec.rb index a8ff7ac3013c02..dad88eed75ed6a 100644 --- a/ee/spec/services/vulnerabilities/confirm_service_spec.rb +++ b/ee/spec/services/vulnerabilities/confirm_service_spec.rb @@ -22,8 +22,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end context 'when vulnerability state is different from the requested state' do @@ -83,10 +82,6 @@ end describe 'permissions' do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - context 'when admin mode is enabled', :enable_admin_mode do it { expect { confirm_vulnerability }.to be_allowed_for(:admin) } end @@ -103,5 +98,28 @@ it { expect { confirm_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { confirm_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { confirm_vulnerability }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + context 'when admin mode is enabled', :enable_admin_mode do + it { expect { confirm_vulnerability }.to be_allowed_for(:admin) } + end + + context 'when admin mode is disabled' do + it { expect { confirm_vulnerability }.to be_denied_for(:admin) } + end + + it { expect { confirm_vulnerability }.to be_allowed_for(:owner).of(project) } + it { expect { confirm_vulnerability }.to be_allowed_for(:maintainer).of(project) } + it { expect { confirm_vulnerability }.to be_allowed_for(:developer).of(project) } + + it { expect { confirm_vulnerability }.to be_denied_for(:auditor) } + it { expect { confirm_vulnerability }.to be_denied_for(:reporter).of(project) } + it { expect { confirm_vulnerability }.to be_denied_for(:guest).of(project) } + it { expect { confirm_vulnerability }.to be_denied_for(:anonymous) } + end end end diff --git a/ee/spec/services/vulnerabilities/create_service_spec.rb b/ee/spec/services/vulnerabilities/create_service_spec.rb index 70b02d71686448..ca7ac0096f0dd4 100644 --- a/ee/spec/services/vulnerabilities/create_service_spec.rb +++ b/ee/spec/services/vulnerabilities/create_service_spec.rb @@ -55,8 +55,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService' diff --git a/ee/spec/services/vulnerabilities/dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/dismiss_service_spec.rb index f704272ee53125..82f7fc9bed35a7 100644 --- a/ee/spec/services/vulnerabilities/dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/dismiss_service_spec.rb @@ -61,8 +61,7 @@ context 'when vulnerability state is different from the requested state' do context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it_behaves_like 'calls vulnerability statistics utility services in order' @@ -169,8 +168,7 @@ let(:vulnerability) { create(:vulnerability, :with_state_transition, :dismissed, project: project, to_state: :dismissed) } before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it { expect { dismiss_vulnerability }.not_to raise_error } @@ -195,10 +193,6 @@ end describe 'permissions' do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - context 'when admin mode is enabled', :enable_admin_mode do it { expect { dismiss_vulnerability }.to be_allowed_for(:admin) } end @@ -209,11 +203,34 @@ it { expect { dismiss_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { dismiss_vulnerability }.to be_allowed_for(:maintainer).of(project) } - it { expect { dismiss_vulnerability }.to be_allowed_for(:developer).of(project) } + it { expect { dismiss_vulnerability }.to be_denied_for(:developer).of(project) } it { expect { dismiss_vulnerability }.to be_denied_for(:auditor) } it { expect { dismiss_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { dismiss_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { dismiss_vulnerability }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + context 'when admin mode is enabled', :enable_admin_mode do + it { expect { dismiss_vulnerability }.to be_allowed_for(:admin) } + end + + context 'when admin mode is disabled' do + it { expect { dismiss_vulnerability }.to be_denied_for(:admin) } + end + + it { expect { dismiss_vulnerability }.to be_allowed_for(:owner).of(project) } + it { expect { dismiss_vulnerability }.to be_allowed_for(:maintainer).of(project) } + it { expect { dismiss_vulnerability }.to be_allowed_for(:developer).of(project) } + + it { expect { dismiss_vulnerability }.to be_denied_for(:auditor) } + it { expect { dismiss_vulnerability }.to be_denied_for(:reporter).of(project) } + it { expect { dismiss_vulnerability }.to be_denied_for(:guest).of(project) } + it { expect { dismiss_vulnerability }.to be_denied_for(:anonymous) } + end end end diff --git a/ee/spec/services/vulnerabilities/find_or_create_from_security_finding_service_spec.rb b/ee/spec/services/vulnerabilities/find_or_create_from_security_finding_service_spec.rb index 441cd7fe69d37a..9b74e4633f9ecc 100644 --- a/ee/spec/services/vulnerabilities/find_or_create_from_security_finding_service_spec.rb +++ b/ee/spec/services/vulnerabilities/find_or_create_from_security_finding_service_spec.rb @@ -6,8 +6,7 @@ feature_category: :vulnerability_management do before do stub_licensed_features(security_dashboard: true) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end let(:security_finding_uuid) { security_findings.first.uuid } diff --git a/ee/spec/services/vulnerabilities/manually_create_service_spec.rb b/ee/spec/services/vulnerabilities/manually_create_service_spec.rb index 09c1b84983088f..910624f5d1b0c4 100644 --- a/ee/spec/services/vulnerabilities/manually_create_service_spec.rb +++ b/ee/spec/services/vulnerabilities/manually_create_service_spec.rb @@ -16,8 +16,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end context 'with valid parameters' do diff --git a/ee/spec/services/vulnerabilities/resolve_service_spec.rb b/ee/spec/services/vulnerabilities/resolve_service_spec.rb index a49db7e5b6f153..3bc0da112c1f1a 100644 --- a/ee/spec/services/vulnerabilities/resolve_service_spec.rb +++ b/ee/spec/services/vulnerabilities/resolve_service_spec.rb @@ -23,8 +23,7 @@ context 'when vulnerability state is different from the requested state' do context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it_behaves_like 'calls vulnerability statistics utility services in order' @@ -78,10 +77,6 @@ end describe 'permissions' do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - context 'when admin mode is enabled', :enable_admin_mode do it { expect { resolve_vulnerability }.to be_allowed_for(:admin) } end @@ -98,5 +93,28 @@ it { expect { resolve_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { resolve_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { resolve_vulnerability }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + context 'when admin mode is enabled', :enable_admin_mode do + it { expect { resolve_vulnerability }.to be_allowed_for(:admin) } + end + + context 'when admin mode is disabled' do + it { expect { resolve_vulnerability }.to be_denied_for(:admin) } + end + + it { expect { resolve_vulnerability }.to be_allowed_for(:owner).of(project) } + it { expect { resolve_vulnerability }.to be_allowed_for(:maintainer).of(project) } + it { expect { resolve_vulnerability }.to be_allowed_for(:developer).of(project) } + + it { expect { resolve_vulnerability }.to be_denied_for(:auditor) } + it { expect { resolve_vulnerability }.to be_denied_for(:reporter).of(project) } + it { expect { resolve_vulnerability }.to be_denied_for(:guest).of(project) } + it { expect { resolve_vulnerability }.to be_denied_for(:anonymous) } + end end end diff --git a/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb b/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb index d553e475ed9bbd..237ae12ea69874 100644 --- a/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb +++ b/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb @@ -51,8 +51,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end context 'when vulnerability state is different from the requested state' do @@ -100,10 +99,6 @@ end describe 'permissions' do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - context 'when admin mode is enabled', :enable_admin_mode do it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) } end @@ -120,5 +115,28 @@ it { expect { revert_vulnerability_to_detected }.to be_denied_for(:reporter).of(project) } it { expect { revert_vulnerability_to_detected }.to be_denied_for(:guest).of(project) } it { expect { revert_vulnerability_to_detected }.to be_denied_for(:anonymous) } + + context 'with `disable_developer_access_to_admin_vulnerability` disabled' do + before do + stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) + end + + context 'when admin mode is enabled', :enable_admin_mode do + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) } + end + + context 'when admin mode is disabled' do + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:admin) } + end + + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:owner).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:maintainer).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:developer).of(project) } + + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:auditor) } + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:reporter).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:guest).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:anonymous) } + end end end diff --git a/ee/spec/services/vulnerabilities/security_finding/create_issue_service_spec.rb b/ee/spec/services/vulnerabilities/security_finding/create_issue_service_spec.rb index 396c6a28d58c6e..889ec5ab7d717b 100644 --- a/ee/spec/services/vulnerabilities/security_finding/create_issue_service_spec.rb +++ b/ee/spec/services/vulnerabilities/security_finding/create_issue_service_spec.rb @@ -6,8 +6,7 @@ feature_category: :vulnerability_management do before do stub_licensed_features(security_dashboard: true) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end let_it_be(:project) { create(:project, :repository) } diff --git a/ee/spec/services/vulnerabilities/security_finding/create_merge_request_service_spec.rb b/ee/spec/services/vulnerabilities/security_finding/create_merge_request_service_spec.rb index aaf06e04334060..8f93c5b2c26d49 100644 --- a/ee/spec/services/vulnerabilities/security_finding/create_merge_request_service_spec.rb +++ b/ee/spec/services/vulnerabilities/security_finding/create_merge_request_service_spec.rb @@ -36,8 +36,7 @@ before do stub_licensed_features(security_dashboard: true) - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - group.add_developer(user) + group.add_maintainer(user) end context 'when user does not have permission to read_security_resource' do diff --git a/ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb b/ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb index cf2738c1f10497..67e436b2100405 100644 --- a/ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb +++ b/ee/spec/services/vulnerabilities/starboard_vulnerability_create_service_spec.rb @@ -51,8 +51,7 @@ context 'with authorized user' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end context 'with feature enabled' do diff --git a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb index 522a851b140c16..2fcf7a93d47f20 100644 --- a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb +++ b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb @@ -30,12 +30,11 @@ describe "#execute" do context 'with authorized user' do before_all do - project.add_developer(user) + project.add_maintainer(user) end context 'with feature enabled' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/services/vulnerabilities/update_service_spec.rb b/ee/spec/services/vulnerabilities/update_service_spec.rb index 8accd94b3ca926..1350e2ea91ba46 100644 --- a/ee/spec/services/vulnerabilities/update_service_spec.rb +++ b/ee/spec/services/vulnerabilities/update_service_spec.rb @@ -22,8 +22,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService' diff --git a/ee/spec/services/vulnerability_feedback/create_service_spec.rb b/ee/spec/services/vulnerability_feedback/create_service_spec.rb index 5995997c25366a..c58b61c9cdafcc 100644 --- a/ee/spec/services/vulnerability_feedback/create_service_spec.rb +++ b/ee/spec/services/vulnerability_feedback/create_service_spec.rb @@ -17,8 +17,7 @@ let(:security_finding) { security_findings.first } before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - group.add_developer(user) + group.add_maintainer(user) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/services/vulnerability_feedback/destroy_service_spec.rb b/ee/spec/services/vulnerability_feedback/destroy_service_spec.rb index 5aec8eebb9dbf4..5d58ed681d7f8b 100644 --- a/ee/spec/services/vulnerability_feedback/destroy_service_spec.rb +++ b/ee/spec/services/vulnerability_feedback/destroy_service_spec.rb @@ -10,8 +10,7 @@ let(:service_object) { described_class.new(project, user, vulnerability_feedback, revert_vulnerability_state: revert_vulnerability_state) } before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) stub_licensed_features(security_dashboard: true) end diff --git a/ee/spec/support/shared_examples/controllers/projects/security_and_compliance_feature_shared_examples.rb b/ee/spec/support/shared_examples/controllers/projects/security_and_compliance_feature_shared_examples.rb index 0133fc4e3f965a..9019cbfe305fce 100644 --- a/ee/spec/support/shared_examples/controllers/projects/security_and_compliance_feature_shared_examples.rb +++ b/ee/spec/support/shared_examples/controllers/projects/security_and_compliance_feature_shared_examples.rb @@ -8,7 +8,7 @@ context 'when user has role that enables sufficient access' do before do - group.add_developer(user) + group.add_maintainer(user) end it { is_expected.to have_gitlab_http_status(:not_found) } @@ -34,7 +34,7 @@ context 'when user has role that enables sufficient access' do before do - group.add_developer(user) + group.add_maintainer(user) end it { is_expected.not_to have_gitlab_http_status(:not_found) } diff --git a/ee/spec/support/shared_examples/services/vulnerabilities/does_not_create_state_transition_for_same_state.rb b/ee/spec/support/shared_examples/services/vulnerabilities/does_not_create_state_transition_for_same_state.rb index e57eda00693bdd..771b2d6ce0a53b 100644 --- a/ee/spec/support/shared_examples/services/vulnerabilities/does_not_create_state_transition_for_same_state.rb +++ b/ee/spec/support/shared_examples/services/vulnerabilities/does_not_create_state_transition_for_same_state.rb @@ -6,8 +6,7 @@ context 'with an authorized user with proper permissions' do before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - project.add_developer(user) + project.add_maintainer(user) end it 'does not create a state transition entry' do -- GitLab From ffba143118385471db9b166dd2a056f67aba0f0c Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 2 Nov 2023 11:45:34 -0600 Subject: [PATCH 13/19] Update assertion to disable developer access to admin_vulnerability functionality --- ee/spec/services/vulnerabilities/confirm_service_spec.rb | 2 +- ee/spec/services/vulnerabilities/resolve_service_spec.rb | 2 +- .../services/vulnerabilities/revert_to_detected_service_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/spec/services/vulnerabilities/confirm_service_spec.rb b/ee/spec/services/vulnerabilities/confirm_service_spec.rb index dad88eed75ed6a..55c704b98c50a8 100644 --- a/ee/spec/services/vulnerabilities/confirm_service_spec.rb +++ b/ee/spec/services/vulnerabilities/confirm_service_spec.rb @@ -92,7 +92,7 @@ it { expect { confirm_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { confirm_vulnerability }.to be_allowed_for(:maintainer).of(project) } - it { expect { confirm_vulnerability }.to be_allowed_for(:developer).of(project) } + it { expect { confirm_vulnerability }.to be_denied_for(:developer).of(project) } it { expect { confirm_vulnerability }.to be_denied_for(:auditor) } it { expect { confirm_vulnerability }.to be_denied_for(:reporter).of(project) } diff --git a/ee/spec/services/vulnerabilities/resolve_service_spec.rb b/ee/spec/services/vulnerabilities/resolve_service_spec.rb index 3bc0da112c1f1a..d1b80ef676b00b 100644 --- a/ee/spec/services/vulnerabilities/resolve_service_spec.rb +++ b/ee/spec/services/vulnerabilities/resolve_service_spec.rb @@ -87,7 +87,7 @@ it { expect { resolve_vulnerability }.to be_allowed_for(:owner).of(project) } it { expect { resolve_vulnerability }.to be_allowed_for(:maintainer).of(project) } - it { expect { resolve_vulnerability }.to be_allowed_for(:developer).of(project) } + it { expect { resolve_vulnerability }.to be_denied_for(:developer).of(project) } it { expect { resolve_vulnerability }.to be_denied_for(:auditor) } it { expect { resolve_vulnerability }.to be_denied_for(:reporter).of(project) } diff --git a/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb b/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb index 237ae12ea69874..56a59aa0d4731b 100644 --- a/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb +++ b/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb @@ -109,7 +109,7 @@ it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:owner).of(project) } it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:maintainer).of(project) } - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:developer).of(project) } + it { expect { revert_vulnerability_to_detected }.to be_denied_for(:developer).of(project) } it { expect { revert_vulnerability_to_detected }.to be_denied_for(:auditor) } it { expect { revert_vulnerability_to_detected }.to be_denied_for(:reporter).of(project) } -- GitLab From 59c8991341b5f7bb28ac8a1eaf7163f36c8dc55a Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 2 Nov 2023 11:55:12 -0600 Subject: [PATCH 14/19] Increase query count by 1 --- .../services/vulnerabilities/manually_create_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/vulnerabilities/manually_create_service_spec.rb b/ee/spec/services/vulnerabilities/manually_create_service_spec.rb index 910624f5d1b0c4..edf4cfe7d32614 100644 --- a/ee/spec/services/vulnerabilities/manually_create_service_spec.rb +++ b/ee/spec/services/vulnerabilities/manually_create_service_spec.rb @@ -87,7 +87,7 @@ end it 'does not exceed query limit' do - expect { subject }.not_to exceed_query_limit(27) + expect { subject }.not_to exceed_query_limit(28) end it 'creates a new Vulnerability' do -- GitLab From 518783413fda146f7035bceb7f778c75a346d31e Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 2 Nov 2023 12:20:41 -0600 Subject: [PATCH 15/19] Tag specs with feature_category --- ee/spec/graphql/mutations/security/finding/dismiss_spec.rb | 2 +- ee/spec/helpers/security_helper_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb b/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb index 3a272fc49d6eac..aba11e277607c3 100644 --- a/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb +++ b/ee/spec/graphql/mutations/security/finding/dismiss_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Mutations::Security::Finding::Dismiss do +RSpec.describe Mutations::Security::Finding::Dismiss, feature_category: :vulnerability_management do include GraphqlHelpers let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } diff --git a/ee/spec/helpers/security_helper_spec.rb b/ee/spec/helpers/security_helper_spec.rb index d824bc9e900406..b61702187e8ace 100644 --- a/ee/spec/helpers/security_helper_spec.rb +++ b/ee/spec/helpers/security_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe SecurityHelper do +RSpec.describe SecurityHelper, feature_category: :vulnerability_management do describe '#instance_security_dashboard_data' do let_it_be(:group) { create(:group) } let_it_be(:has_group) { true } -- GitLab From ed93b86a7eca593a4623a376f269d0775b605b1f Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 2 Nov 2023 14:26:27 -0600 Subject: [PATCH 16/19] Move admin mode to outer context --- ee/spec/policies/group_policy_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 235a8317e0df53..ca38092a82484f 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -1474,10 +1474,6 @@ def stub_group_saml_config(enabled) it { is_expected.to be_allowed(:admin_vulnerability) } end - - context 'with admin mode enabled', :enable_admin_mode do - it { is_expected.to be_allowed(:admin_vulnerability) } - end end context 'with maintainer' do @@ -1492,6 +1488,12 @@ def stub_group_saml_config(enabled) it { is_expected.to be_allowed(:admin_vulnerability) } end + context 'with admin', :enable_admin_mode do + let(:current_user) { admin } + + it { is_expected.to be_allowed(:admin_vulnerability) } + end + context 'with auditor' do let(:current_user) { auditor } -- GitLab From 024e93de83abe43fccbef79c12519396e56cf43a Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 2 Nov 2023 14:38:19 -0600 Subject: [PATCH 17/19] Remove duplicate specs that do not define a change in behaviour --- ee/spec/requests/api/vulnerabilities_spec.rb | 71 ------------------- .../bulk_dismiss_service_spec.rb | 1 + .../vulnerabilities/confirm_service_spec.rb | 15 ---- .../vulnerabilities/dismiss_service_spec.rb | 15 ---- .../vulnerabilities/resolve_service_spec.rb | 15 ---- .../revert_to_detected_service_spec.rb | 15 ---- 6 files changed, 1 insertion(+), 131 deletions(-) diff --git a/ee/spec/requests/api/vulnerabilities_spec.rb b/ee/spec/requests/api/vulnerabilities_spec.rb index d0f2d3baf65fd2..879e9f5aafc644 100644 --- a/ee/spec/requests/api/vulnerabilities_spec.rb +++ b/ee/spec/requests/api/vulnerabilities_spec.rb @@ -59,21 +59,6 @@ it { expect { get_vulnerabilities }.to be_denied_for(:reporter).of(project) } it { expect { get_vulnerabilities }.to be_denied_for(:guest).of(project) } it { expect { get_vulnerabilities }.to be_denied_for(:anonymous) } - - context 'with `disable_developer_access_to_admin_vulnerability` disabled' do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - - it { expect { get_vulnerabilities }.to be_allowed_for(:admin) } - it { expect { get_vulnerabilities }.to be_allowed_for(:owner).of(project) } - it { expect { get_vulnerabilities }.to be_allowed_for(:maintainer).of(project) } - it { expect { get_vulnerabilities }.to be_allowed_for(:developer).of(project) } - it { expect { get_vulnerabilities }.to be_allowed_for(:auditor) } - it { expect { get_vulnerabilities }.to be_denied_for(:reporter).of(project) } - it { expect { get_vulnerabilities }.to be_denied_for(:guest).of(project) } - it { expect { get_vulnerabilities }.to be_denied_for(:anonymous) } - end end end @@ -120,22 +105,6 @@ it { expect { get_vulnerability }.to be_denied_for(:reporter).of(project) } it { expect { get_vulnerability }.to be_denied_for(:guest).of(project) } it { expect { get_vulnerability }.to be_denied_for(:anonymous) } - - context 'with `disable_developer_access_to_admin_vulnerability` disabled' do - before do - stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) - end - - it { expect { get_vulnerability }.to be_allowed_for(:admin) } - it { expect { get_vulnerability }.to be_allowed_for(:owner).of(project) } - it { expect { get_vulnerability }.to be_allowed_for(:maintainer).of(project) } - it { expect { get_vulnerability }.to be_allowed_for(:developer).of(project) } - it { expect { get_vulnerability }.to be_allowed_for(:auditor) } - - it { expect { get_vulnerability }.to be_denied_for(:reporter).of(project) } - it { expect { get_vulnerability }.to be_denied_for(:guest).of(project) } - it { expect { get_vulnerability }.to be_denied_for(:anonymous) } - end end end @@ -214,15 +183,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - it { expect { create_vulnerability }.to be_allowed_for(:admin) } - it { expect { create_vulnerability }.to be_allowed_for(:owner).of(project) } - it { expect { create_vulnerability }.to be_allowed_for(:maintainer).of(project) } it { expect { create_vulnerability }.to be_allowed_for(:developer).of(project) } - - it { expect { create_vulnerability }.to be_denied_for(:auditor) } - it { expect { create_vulnerability }.to be_denied_for(:reporter).of(project) } - it { expect { create_vulnerability }.to be_denied_for(:guest).of(project) } - it { expect { create_vulnerability }.to be_denied_for(:anonymous) } end end end @@ -305,15 +266,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - it { expect { dismiss_vulnerability }.to be_allowed_for(:admin) } - it { expect { dismiss_vulnerability }.to be_allowed_for(:owner).of(project) } - it { expect { dismiss_vulnerability }.to be_allowed_for(:maintainer).of(project) } it { expect { dismiss_vulnerability }.to be_allowed_for(:developer).of(project) } - - it { expect { dismiss_vulnerability }.to be_denied_for(:auditor) } - it { expect { dismiss_vulnerability }.to be_denied_for(:reporter).of(project) } - it { expect { dismiss_vulnerability }.to be_denied_for(:guest).of(project) } - it { expect { dismiss_vulnerability }.to be_denied_for(:anonymous) } end end end @@ -394,15 +347,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - it { expect { resolve_vulnerability }.to be_allowed_for(:admin) } - it { expect { resolve_vulnerability }.to be_allowed_for(:owner).of(project) } - it { expect { resolve_vulnerability }.to be_allowed_for(:maintainer).of(project) } it { expect { resolve_vulnerability }.to be_allowed_for(:developer).of(project) } - - it { expect { resolve_vulnerability }.to be_denied_for(:auditor) } - it { expect { resolve_vulnerability }.to be_denied_for(:reporter).of(project) } - it { expect { resolve_vulnerability }.to be_denied_for(:guest).of(project) } - it { expect { resolve_vulnerability }.to be_denied_for(:anonymous) } end end end @@ -472,15 +417,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - it { expect { confirm_vulnerability }.to be_allowed_for(:admin) } - it { expect { confirm_vulnerability }.to be_allowed_for(:owner).of(project) } - it { expect { confirm_vulnerability }.to be_allowed_for(:maintainer).of(project) } it { expect { confirm_vulnerability }.to be_allowed_for(:developer).of(project) } - - it { expect { confirm_vulnerability }.to be_denied_for(:auditor) } - it { expect { confirm_vulnerability }.to be_denied_for(:reporter).of(project) } - it { expect { confirm_vulnerability }.to be_denied_for(:guest).of(project) } - it { expect { confirm_vulnerability }.to be_denied_for(:anonymous) } end end end @@ -580,15 +517,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) } - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:owner).of(project) } - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:maintainer).of(project) } it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:developer).of(project) } - - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:auditor) } - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:reporter).of(project) } - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:guest).of(project) } - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:anonymous) } end end end diff --git a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb index bb8387c2c26591..22f660cb9cdd45 100644 --- a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb @@ -128,6 +128,7 @@ queries = ActiveRecord::QueryRecorder.new do service.execute end + expect(queries.count).to eq(14) end end diff --git a/ee/spec/services/vulnerabilities/confirm_service_spec.rb b/ee/spec/services/vulnerabilities/confirm_service_spec.rb index 55c704b98c50a8..e97d0d69a61629 100644 --- a/ee/spec/services/vulnerabilities/confirm_service_spec.rb +++ b/ee/spec/services/vulnerabilities/confirm_service_spec.rb @@ -104,22 +104,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - context 'when admin mode is enabled', :enable_admin_mode do - it { expect { confirm_vulnerability }.to be_allowed_for(:admin) } - end - - context 'when admin mode is disabled' do - it { expect { confirm_vulnerability }.to be_denied_for(:admin) } - end - - it { expect { confirm_vulnerability }.to be_allowed_for(:owner).of(project) } - it { expect { confirm_vulnerability }.to be_allowed_for(:maintainer).of(project) } it { expect { confirm_vulnerability }.to be_allowed_for(:developer).of(project) } - - it { expect { confirm_vulnerability }.to be_denied_for(:auditor) } - it { expect { confirm_vulnerability }.to be_denied_for(:reporter).of(project) } - it { expect { confirm_vulnerability }.to be_denied_for(:guest).of(project) } - it { expect { confirm_vulnerability }.to be_denied_for(:anonymous) } end end end diff --git a/ee/spec/services/vulnerabilities/dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/dismiss_service_spec.rb index 82f7fc9bed35a7..6199e81b8629d7 100644 --- a/ee/spec/services/vulnerabilities/dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/dismiss_service_spec.rb @@ -215,22 +215,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - context 'when admin mode is enabled', :enable_admin_mode do - it { expect { dismiss_vulnerability }.to be_allowed_for(:admin) } - end - - context 'when admin mode is disabled' do - it { expect { dismiss_vulnerability }.to be_denied_for(:admin) } - end - - it { expect { dismiss_vulnerability }.to be_allowed_for(:owner).of(project) } - it { expect { dismiss_vulnerability }.to be_allowed_for(:maintainer).of(project) } it { expect { dismiss_vulnerability }.to be_allowed_for(:developer).of(project) } - - it { expect { dismiss_vulnerability }.to be_denied_for(:auditor) } - it { expect { dismiss_vulnerability }.to be_denied_for(:reporter).of(project) } - it { expect { dismiss_vulnerability }.to be_denied_for(:guest).of(project) } - it { expect { dismiss_vulnerability }.to be_denied_for(:anonymous) } end end end diff --git a/ee/spec/services/vulnerabilities/resolve_service_spec.rb b/ee/spec/services/vulnerabilities/resolve_service_spec.rb index d1b80ef676b00b..0e51b2ec8fed2b 100644 --- a/ee/spec/services/vulnerabilities/resolve_service_spec.rb +++ b/ee/spec/services/vulnerabilities/resolve_service_spec.rb @@ -99,22 +99,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - context 'when admin mode is enabled', :enable_admin_mode do - it { expect { resolve_vulnerability }.to be_allowed_for(:admin) } - end - - context 'when admin mode is disabled' do - it { expect { resolve_vulnerability }.to be_denied_for(:admin) } - end - - it { expect { resolve_vulnerability }.to be_allowed_for(:owner).of(project) } - it { expect { resolve_vulnerability }.to be_allowed_for(:maintainer).of(project) } it { expect { resolve_vulnerability }.to be_allowed_for(:developer).of(project) } - - it { expect { resolve_vulnerability }.to be_denied_for(:auditor) } - it { expect { resolve_vulnerability }.to be_denied_for(:reporter).of(project) } - it { expect { resolve_vulnerability }.to be_denied_for(:guest).of(project) } - it { expect { resolve_vulnerability }.to be_denied_for(:anonymous) } end end end diff --git a/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb b/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb index 56a59aa0d4731b..1789dc2f31b6b5 100644 --- a/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb +++ b/ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb @@ -121,22 +121,7 @@ stub_feature_flags(disable_developer_access_to_admin_vulnerability: false) end - context 'when admin mode is enabled', :enable_admin_mode do - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:admin) } - end - - context 'when admin mode is disabled' do - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:admin) } - end - - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:owner).of(project) } - it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:maintainer).of(project) } it { expect { revert_vulnerability_to_detected }.to be_allowed_for(:developer).of(project) } - - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:auditor) } - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:reporter).of(project) } - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:guest).of(project) } - it { expect { revert_vulnerability_to_detected }.to be_denied_for(:anonymous) } end end end -- GitLab From 0e2ef6dd95a86af893be7b22a1ea5224257cda1e Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 2 Nov 2023 14:57:30 -0600 Subject: [PATCH 18/19] Add guest trait to fix failing specs --- ee/spec/factories/member_roles.rb | 2 +- .../api/graphql/mutations/vulnerabilities/confirm_spec.rb | 2 +- .../api/graphql/mutations/vulnerabilities/resolve_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/spec/factories/member_roles.rb b/ee/spec/factories/member_roles.rb index 7140b305453cb2..9132fde33640ad 100644 --- a/ee/spec/factories/member_roles.rb +++ b/ee/spec/factories/member_roles.rb @@ -14,4 +14,4 @@ read_vulnerability { true } end end -end \ No newline at end of file +end diff --git a/ee/spec/requests/api/graphql/mutations/vulnerabilities/confirm_spec.rb b/ee/spec/requests/api/graphql/mutations/vulnerabilities/confirm_spec.rb index 91e6aa73d64120..6eb7e17947c1f1 100644 --- a/ee/spec/requests/api/graphql/mutations/vulnerabilities/confirm_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/vulnerabilities/confirm_spec.rb @@ -27,7 +27,7 @@ end context "with `admin_vulnerability` enabled" do - let(:role) { create(:member_role, :admin_vulnerability, namespace: project.group) } + let(:role) { create(:member_role, :guest, :admin_vulnerability, namespace: project.group) } it "returns a successful response" do post_graphql_mutation(mutation, current_user: current_user) diff --git a/ee/spec/requests/api/graphql/mutations/vulnerabilities/resolve_spec.rb b/ee/spec/requests/api/graphql/mutations/vulnerabilities/resolve_spec.rb index a62894e5b636c5..9ec8a5d9edd947 100644 --- a/ee/spec/requests/api/graphql/mutations/vulnerabilities/resolve_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/vulnerabilities/resolve_spec.rb @@ -26,7 +26,7 @@ end context "with `admin_vulnerability` enabled" do - let(:role) { create(:member_role, :admin_vulnerability, namespace: project.group) } + let(:role) { create(:member_role, :guest, :admin_vulnerability, namespace: project.group) } it "returns a successful response" do post_graphql_mutation(mutation, current_user: current_user) -- GitLab From 9e3e7fa136559daec545bbb19426dbe43b31da34 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 3 Nov 2023 13:52:06 -0600 Subject: [PATCH 19/19] Fix failing spec with let_it_be_with_reload --- ee/spec/policies/project_policy_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 965e889d689efc..a63a038d9386cf 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -2340,8 +2340,8 @@ def expect_private_project_permissions_as_if_non_member end describe 'inviting a group' do - let(:current_user) { developer } - let(:project) { public_project } + let_it_be_with_reload(:current_user) { developer } + let_it_be_with_reload(:project) { public_project } let_it_be(:banned_group) { create(:group) } let_it_be(:banned_subgroup) { create(:group, parent: banned_group) } -- GitLab