I believe Enable vulnerability reports for security managers (!225982 - merged) enables everything we need for this one:

@mclausen35 does this look good to you?

I believe Enable vulnerability reports for security managers (!225982 - merged) enables everything we need for this one:

I noticed that severity isn't adjustable from this page, but after digging it looks like this feature isn't enabled on this page, only the vulnerability show page, eg http://127.0.0.1:3000/root_group/root_project/-/security/vulnerabilities/544

@mclausen35 does this sound accurate to you? And does this video confirm the behavior is correct?

What does this MR do and why?

• Replaces role-based condition checks (target_project_developer, target_project_reporter) in EE::MergeRequestPolicy with permission grants from role definition YAML files, using prevent rules instead of role-gated enable rules
• Moves approve_merge_request, provide_status_check_response, and retry_failed_status_checks to developer.yml and read_external_status_check_response to reporter.yml
• Removes the can_approve? override in the EE policy since approve_merge_request is now granted directly through the developer role definition
• Adds prevent :approve_merge_request to ProjectPolicy when merge requests are disabled, closing a gap where the permission could leak through the role definition

References

Screenshots or screen recordings

How to set up and validate locally

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Closes #593261

Looks good @imand3r, on to @dlrussel for maintainer.

Question (non-blocking): Any idea why this was here? did developer not have update_merge_request maybe?

Comment (non-blocking): We're delegating from merge_request_policy to the project policy, which is why we can move this here, right?

What does this MR do and why?

Refactor custom dashboard policy to use granular permissions

Replace reporter_access and developer_access checks in DashboardPolicy with delegated namespace conditions and specific permissions. Add custom dashboard permissions to reporter and developer role definitions. Update permission validation to support underscore-prefixed permission names.

References

Screenshots or screen recordings

How to set up and validate locally

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #593274

What does this MR do and why?

Allow Security Managers to access project runners settings.

References

Implements #590075.

Screenshots

	Owner	Security Manager	Notes
Project sidebar menu and assigned project runners			No Create project runner button, no kebab menu for runner registration. If the current user is the creator of a runner, they can perform actions on the runner (resume/pause, edit, delete). The user CANNOT unassign any of the listed runners.
Other available project runners			This tab displays assignable project runners visible to the current user. If they are the creator of a runner, they can perform actions on the runner (resume/pause, edit, delete). The user CANNOT assign any of the listed runners to the project.
Group runners			Group runners toggle is not displayed. This tab displays group runners visible to the current user. If they are the creator of a runner, they can perform actions on the runner (resume/pause, edit, delete).
Instance runners			Instance runners toggle is not displayed

How to set up and validate locally

1. Start GDK with support for Security Manager role

   $ export GITLAB_SECURITY_MANAGER_ROLE=true
   $ gdk start

2. Login with root then navigate to a project (I recommend http://localhost:3000/gitlab-org/gitlab-shell)

3. Add another user as a Security Manager

4. Logout then login with the Security Manager user

5. Verify that the project sidebar displays Settings -> CI/CD menu item

6. Click on Settings -> CI/CD menu item

7. Verify that Runners settings section is displayed

8. Verify that UI elements Security Managers are unauthorized to view are not displayed (refer to Screenshots section above)

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Approved! @hmehra will you take backend maintainer review, please?

While our handbook suggests to keep EE/CE code separated, our auth architecture ADR suggests what you're saying.

Hey @dlrussel, a few questions to start 🏓

Question: Do we need to be calling prevent on these on https://gitlab.com/gitlab-org/gitlab/-/blob/fix/public-access-guest-access-separation/app/policies/project_policy.rb#L811

Question: Do we need to add a prevent leveraging allow_guest_plus_roles_to_pull_packages_enabled for this?

Question: It looks like the permissions that used to be granted with guest_access now provided thru public_authenticated, is that right?

@eugielimpin done!

@bwill I lost your approval in the process, could you kindly reapprove?

Jay (97bc9ed0) at 18 Mar 18:00

... and 224 more commits

@eugielimpin I suppose because admin_vulnerability_merge_request_link was the only thing removed from the read_security_resource block, which is enabled by the read_vulnerability custom permission. Now looking at that I'm assuming the read_vulnerability permission was probably over privileged based off its description: "Read vulnerability reports and security dashboards.".

This problem probably exists beyond here, I can see someone conveniently adding a permission by adding it to a block, but in unknowingly be adding permissions to a role that shouldn't have that. If caught they could then prevent it, but thats seems wrong and just extra code, not really convenient after all. I wonder whats the most effective way to build our permission groups as I'm assuming we'll hit that problem there as well. For example, whats going to be the overlap between role x and role y as far as the depth of the feature that they should have access to? The only thing I can think of so far is read only vs all access for features.

I think the hard thing is that the non-hierarchical roles are really specific, security manager can modify the severity of a vulnerability but can't create a merge request. Maybe those details just belong in the role for now.

Updating as workflowin dev as per this slack convo:

Here's my proposal to avoid breaking changes while also allowing us to authorize Security Manager role to update DAST on-demand scans:


Keep the can_push_to_branch? check to keep the behavior as-is for Developer+ roles
Introduce update_on_demand_dast_scan that will be enabled only for Security Managers
Change the check to can_push_to_branch? || user.can?(:update_on_demand_dast_scan, project)