diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index f205a7a820e7c77c1635b22aa953d3339d58cc75..524000a60faac62d0b8000d9df369e1872dcffdf 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -379,10 +379,6 @@ module ProjectPolicy rule { can?(:developer_access) }.policy do enable :admin_issue_board - enable :read_vulnerability_feedback - enable :create_vulnerability_feedback - enable :destroy_vulnerability_feedback - enable :update_vulnerability_feedback enable :admin_feature_flags_issue_links enable :read_project_audit_events enable :read_product_analytics @@ -422,7 +418,6 @@ module ProjectPolicy rule { security_dashboard_enabled & can?(:developer_access) }.policy do enable :read_security_resource - enable :read_vulnerability_scanner end rule { coverage_fuzzing_enabled & can?(:developer_access) }.policy do @@ -464,6 +459,18 @@ module ProjectPolicy enable :admin_vulnerability end + rule { can?(:admin_vulnerability) }.policy do + enable :read_vulnerability + enable :create_vulnerability_feedback + enable :destroy_vulnerability_feedback + enable :update_vulnerability_feedback + end + + rule { can?(:read_vulnerability) }.policy do + enable :read_vulnerability_feedback + enable :read_vulnerability_scanner + end + rule { security_and_compliance_disabled }.policy do prevent :admin_vulnerability prevent :read_vulnerability @@ -537,7 +544,6 @@ module ProjectPolicy rule { auditor & security_dashboard_enabled }.policy do enable :read_security_resource - enable :read_vulnerability_scanner end rule { auditor & oncall_schedules_available }.policy do diff --git a/ee/spec/controllers/projects/issues_controller_spec.rb b/ee/spec/controllers/projects/issues_controller_spec.rb index 5f118835e4b0d6b33b359cc50222d670eb6f5550..7e5244699aec85be2723b8341056b7f4b6b3d8e8 100644 --- a/ee/spec/controllers/projects/issues_controller_spec.rb +++ b/ee/spec/controllers/projects/issues_controller_spec.rb @@ -131,6 +131,7 @@ def perform(method, action, opts = {}) before do stub_licensed_features(security_dashboard: true) + namespace.add_maintainer(user) end it 'overwrites the default fields' do diff --git a/ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb b/ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb index c804728194bcb0b88f1170661103e5d92f2a3d5f..cc8226f31a97ccb4fa7fe1756260238e80137bb9 100644 --- a/ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb +++ b/ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb @@ -9,7 +9,8 @@ let_it_be(:guest) { create(:user) } before do - group.add_developer(user) + stub_licensed_features(security_dashboard: true) + group.add_maintainer(user) end describe 'GET #count' do @@ -281,10 +282,6 @@ def list_feedbacks(params = {}) } end - before do - stub_licensed_features(security_dashboard: true) - end - shared_examples 'vulnerability response' do let_it_be(:vulnerability) { create(:vulnerability) } let_it_be(:finding) { create(:vulnerabilities_finding, vulnerability: vulnerability) } diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index d5f716c492ccd385e502c41459dfb8fcdfeae26d..eef4d1dc836274897f8978c28c19297c948bc8ce 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -702,6 +702,30 @@ end describe 'vulnerability feedback permissions' do + before do + stub_licensed_features(security_dashboard: true) + end + + context 'with developer' do + let(:current_user) { developer } + + it { is_expected.to be_allowed(:read_vulnerability_feedback) } + it { is_expected.to be_disallowed(:create_vulnerability_feedback) } + it { is_expected.to be_disallowed(:update_vulnerability_feedback) } + it { is_expected.to be_disallowed(:destroy_vulnerability_feedback) } + + 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(:read_vulnerability_feedback) } + it { is_expected.to be_allowed(:create_vulnerability_feedback) } + it { is_expected.to be_allowed(:update_vulnerability_feedback) } + it { is_expected.to be_allowed(:destroy_vulnerability_feedback) } + end + end + where(permission: %i[ read_vulnerability_feedback create_vulnerability_feedback @@ -734,12 +758,6 @@ it { is_expected.to be_allowed(permission) } end - context 'with developer' do - let(:current_user) { developer } - - it { is_expected.to be_allowed(permission) } - end - context 'with reporter' do let(:current_user) { reporter } @@ -2683,7 +2701,9 @@ def create_member_role(member, abilities = member_role_abilities) :access_security_and_compliance, :create_vulnerability_export, :read_security_resource, - :read_vulnerability + :read_vulnerability, + :read_vulnerability_feedback, + :read_vulnerability_scanner ] end @@ -2696,7 +2716,16 @@ def create_member_role(member, abilities = member_role_abilities) context 'for a member role with admin_vulnerability true' do let(:member_role_abilities) { { read_vulnerability: true, admin_vulnerability: true } } - let(:allowed_abilities) { [:read_vulnerability, :admin_vulnerability] } + let(:allowed_abilities) do + [ + :admin_vulnerability, + :create_vulnerability_feedback, + :destroy_vulnerability_feedback, + :read_vulnerability, + :read_vulnerability_feedback, + :update_vulnerability_feedback + ] + end it_behaves_like 'custom roles abilities' end diff --git a/ee/spec/policies/vulnerabilities/feedback_policy_spec.rb b/ee/spec/policies/vulnerabilities/feedback_policy_spec.rb index c14ea10c3af1aa48a42ee4bc1b995183549cd41c..c022b79580fc8d0e09bd27ea5711b6d8e18f5132 100644 --- a/ee/spec/policies/vulnerabilities/feedback_policy_spec.rb +++ b/ee/spec/policies/vulnerabilities/feedback_policy_spec.rb @@ -99,6 +99,10 @@ end describe 'update_vulnerability_feedback' do + before do + stub_licensed_features(security_dashboard: true) + end + context 'when feedback type is issue' do let(:vulnerability_feedback) { Vulnerabilities::Feedback.new(project: project, feedback_type: :issue) } @@ -125,6 +129,10 @@ end describe 'destroy_vulnerability_feedback' do + before do + stub_licensed_features(security_dashboard: true) + end + context 'when feedback type is issue' do let(:vulnerability_feedback) { Vulnerabilities::Feedback.new(project: project, feedback_type: :issue) } diff --git a/ee/spec/presenters/merge_request_presenter_spec.rb b/ee/spec/presenters/merge_request_presenter_spec.rb index 54acade74d63f563dcb1ae9fbbf4c39c5a3bf7a2..4d969f2f83570570b97b4009a1a23b223834a455 100644 --- a/ee/spec/presenters/merge_request_presenter_spec.rb +++ b/ee/spec/presenters/merge_request_presenter_spec.rb @@ -62,7 +62,11 @@ end end - describe 'create vulnerability feedback paths' do + describe 'create vulnerability feedback paths', feature_category: :vulnerability_management do + before do + stub_licensed_features(security_dashboard: true) + end + where(:create_feedback_path) do [ :create_vulnerability_feedback_issue_path, diff --git a/ee/spec/requests/api/vulnerability_findings_spec.rb b/ee/spec/requests/api/vulnerability_findings_spec.rb index dbcaaef6efa13b8170edc3cfb9b2fd0cb279a9f7..dd570674afd4c1325ae8aee92f1dc2fae877bf2f 100644 --- a/ee/spec/requests/api/vulnerability_findings_spec.rb +++ b/ee/spec/requests/api/vulnerability_findings_spec.rb @@ -58,7 +58,7 @@ context 'with an authorized user with proper permissions' do before do create(:vulnerability_statistic, project: project, pipeline: pipeline) - project.add_developer(user) + project.add_maintainer(user) end # Because fixture reports that power :ee_ci_job_artifact factory contain long report lists, 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 b300f7d477de2b47808f4ba2b0672a6ebe5e0ab3..72c456ff6dfdb0bd9362ad572ded5f22996cf6ea 100644 --- a/ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb +++ b/ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb @@ -73,7 +73,7 @@ }]) end - pending "has access via a custom role" do + it "has access via a custom role" do post_graphql_mutation(graphql_mutation(:security_finding_dismiss, { uuid: security_finding.uuid, comment: "dismissal feedback", @@ -239,7 +239,7 @@ let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) } - pending "has access via a custom role" do + it "has access via a custom role" do post_graphql_mutation(graphql_mutation(:vulnerability_dismiss, { id: vulnerability.to_global_id.to_s, comment: "comment", diff --git a/ee/spec/serializers/merge_request_widget_entity_spec.rb b/ee/spec/serializers/merge_request_widget_entity_spec.rb index a9bc34a8d1939fd431dc8cf793d0756af5391cc5..ea3ebf068e8e49792ebe5d2ce766791c61adb95a 100644 --- a/ee/spec/serializers/merge_request_widget_entity_spec.rb +++ b/ee/spec/serializers/merge_request_widget_entity_spec.rb @@ -257,6 +257,7 @@ def create_all_artifacts describe '#can_read_vulnerability_feedback' do context 'when user has permissions to read vulnerability feedback' do before do + stub_licensed_features(security_dashboard: true) project.add_developer(user) end diff --git a/ee/spec/serializers/vulnerabilities/feedback_entity_spec.rb b/ee/spec/serializers/vulnerabilities/feedback_entity_spec.rb index 90d86ba3bd631e5c1343c11356caf9266428fb9c..f98a4dde4b3865923670336dfd6e9cfffdd03b7e 100644 --- a/ee/spec/serializers/vulnerabilities/feedback_entity_spec.rb +++ b/ee/spec/serializers/vulnerabilities/feedback_entity_spec.rb @@ -136,7 +136,8 @@ context 'when allowed to destroy vulnerability feedback' do before do - project.add_developer(user) + stub_licensed_features(security_dashboard: true) + project.add_maintainer(user) end it 'contains destroy vulnerability feedback dismissal path' do diff --git a/ee/spec/services/ci/compare_security_reports_service_spec.rb b/ee/spec/services/ci/compare_security_reports_service_spec.rb index 3e01f9ee187becc1b6da3ed8f13e6df4d3631e56..db5403a0df1103379178a4ea1faf4fa570c2b966 100644 --- a/ee/spec/services/ci/compare_security_reports_service_spec.rb +++ b/ee/spec/services/ci/compare_security_reports_service_spec.rb @@ -29,7 +29,7 @@ def collect_ids(collection) describe '#execute DS' do before do - stub_licensed_features(dependency_scanning: true) + stub_licensed_features(security_dashboard: true, dependency_scanning: true) end let(:service) { described_class.new(project, current_user, report_type: 'dependency_scanning') } @@ -100,7 +100,7 @@ def collect_ids(collection) describe '#execute CS' do before do - stub_licensed_features(container_scanning: true) + stub_licensed_features(security_dashboard: true, container_scanning: true) end let(:service) { described_class.new(project, current_user, report_type: 'container_scanning') } @@ -149,7 +149,7 @@ def collect_ids(collection) describe '#execute DAST' do before do - stub_licensed_features(dast: true) + stub_licensed_features(security_dashboard: true, dast: true) end let(:service) { described_class.new(project, current_user, report_type: 'dast') } @@ -207,7 +207,7 @@ def collect_ids(collection) describe '#execute SAST' do before do - stub_licensed_features(sast: true) + stub_licensed_features(security_dashboard: true, sast: true) end let(:service) { described_class.new(project, current_user, report_type: 'sast') } @@ -257,7 +257,7 @@ def collect_ids(collection) describe '#execute SECRET DETECTION' do before do - stub_licensed_features(secret_detection: true) + stub_licensed_features(security_dashboard: true, secret_detection: true) end let(:service) { described_class.new(project, current_user, report_type: 'secret_detection') } diff --git a/ee/spec/services/ee/vulnerability_feedback_module/update_service_spec.rb b/ee/spec/services/ee/vulnerability_feedback_module/update_service_spec.rb index 44e239b8d645bbc9ab0e0a83d49a0997797f276f..ce5911b1fab0ecc6d2e9608b3cb307230c06922b 100644 --- a/ee/spec/services/ee/vulnerability_feedback_module/update_service_spec.rb +++ b/ee/spec/services/ee/vulnerability_feedback_module/update_service_spec.rb @@ -11,7 +11,8 @@ let(:service) { described_class.new(project, user, comment: comment) } before do - group.add_developer(user) + stub_licensed_features(security_dashboard: true) + group.add_maintainer(user) end subject(:result) { service.execute(vuln_feedback) } @@ -75,7 +76,7 @@ let(:service) { described_class.new(project, second_user, vuln_feedback) } before do - group.add_developer(second_user) + group.add_maintainer(second_user) end it 'sets second user as the comment author' do diff --git a/ee/spec/services/vulnerabilities/destroy_dismissal_feedback_service_spec.rb b/ee/spec/services/vulnerabilities/destroy_dismissal_feedback_service_spec.rb index 8bd5540c63755d3ffed8e37f33d20829755df9b4..c6dec27fe1ff0a446dff07e9c10662ba87cfa94c 100644 --- a/ee/spec/services/vulnerabilities/destroy_dismissal_feedback_service_spec.rb +++ b/ee/spec/services/vulnerabilities/destroy_dismissal_feedback_service_spec.rb @@ -22,6 +22,10 @@ describe '#execute' do subject(:destroy_feedback) { described_class.new(user, vulnerability).execute } + before do + stub_licensed_features(security_dashboard: true) + end + context 'without necessary permissions' do it 'raises `Gitlab::Access::AccessDeniedError` error' do expect { destroy_feedback }.to raise_error(Gitlab::Access::AccessDeniedError) @@ -31,7 +35,7 @@ context 'with necessary permissions' do before do - project.add_developer(user) + project.add_maintainer(user) end it 'destroys the feedback records associated with the findings of the given vulnerability' do diff --git a/ee/spec/services/vulnerability_feedback/create_service_spec.rb b/ee/spec/services/vulnerability_feedback/create_service_spec.rb index c58b61c9cdafccbf1aadfcbbea57d902ee656c36..290ce0f758656f2172ab2ff61dd3b5d701152778 100644 --- a/ee/spec/services/vulnerability_feedback/create_service_spec.rb +++ b/ee/spec/services/vulnerability_feedback/create_service_spec.rb @@ -161,7 +161,7 @@ end it 'does not dismiss the existing vulnerability' do - expect { result }.not_to change { finding.vulnerability.reload.state }.from('detected') + expect { result }.to raise_error(Gitlab::Access::AccessDeniedError) end end