Remove Group principal from Secrets Manager permissions

Erick Bajaorequested to merge
eb-gsm-group-principal-deprecation-step3 into master

What does this MR do and why?

GitLab Secrets Manager lets owners grant secrets permissions to principals: User, MemberRole, Group, Role. The Group principal is being removed. This is step 3 of that removal. Step 1 stopped honouring Group grants at login by dropping the groups JWT claim. Step 2 is the frontend MR that hides the Group tab and stops sending GROUP. This MR removes Group from the backend and the GraphQL schema.

This MR targets the step 1 branch, so its diff shows only step 3 changes. It will be retargeted to master once step 1 merges.

Where this fits

Step Change MR
1 Stop sending the groups JWT claim and drop the grps line from the CEL for new namespaces. Group grants stop working. !254051 (merged)
2 Frontend hides the Group tab and stops sending GROUP. !253896 (merged)
3 Remove Group from the backend and the GraphQL schema. !254392 (merged) 👈 You are here
4 Delete leftover group policies in OpenBao. Existing namespaces keep the old CEL text, which is harmless with the claim gone. Later milestone

Merge order is 1, then 2, then 3.

Backend changes

  • Group is removed from PRINCIPAL_TYPES in SecretsManagement::BaseSecretsPermission, together with its validation and the two group lookup hooks on ProjectSecretsPermission and GroupSecretsPermission. Creating or updating a permission with type Group now fails with "must be one of: User, Role, MemberRole". Deleting one returns "Invalid principal".
  • The policy name helper returns nil for Group. The permission list parser skips users/direct/group_* and api/users/direct/group_* policies, so any policy left in OpenBao is invisible to the UI and API.

GraphQL changes

  • The GROUP value is removed from the PrincipalType enum.
  • The groupPath argument is removed from PrincipalInput, and id is now required. The PrincipalInputValidator and the PermissionPrincipalHelpers mutation concern are deleted. The four permission mutations no longer resolve a group path.
  • The group field is removed from the Principal type.
  • Descriptions no longer mention Group. doc/api/graphql/reference/_index.md and the introspection JSON files under public/-/graphql/ are regenerated.

Breaking change

The permission mutations (projectSecretsPermissionUpdate, projectSecretsPermissionDelete, groupSecretsPermissionUpdate, groupSecretsPermissionDelete) are not marked experiment. Removing an enum value, an argument, and a field from them is a breaking schema change under the normal deprecation policy.

The team chose to remove rather than deprecate, for these reasons:

  • Secrets Manager is still in beta.
  • Group grants have granted no access since step 1 merged.
  • The feature had very few users of this option, and they have been told.
  • Keeping a deprecated value that returns an error for a full cycle adds confusion for little benefit.

Reviewers should weigh in if they disagree with this approach.

Behavior and compatibility

  • Group policies left in OpenBao are not deleted here. They grant nothing and are not listed. A later step deletes them directly through the OpenBao API.
  • The grps mapping in the CEL of namespaces provisioned before step 1 also stays until that cleanup. It is harmless because the claim is absent.
  • Self-managed instances get this on upgrade, with no opt-out.

Merge order

This MR must merge after step 1 and after the frontend MR. The frontend on master still sends GROUP and queries principal { group { ... } }. The frontend GraphQL schema lint would fail against this schema until the frontend MR lands.

Testing

The full secrets management spec tree passes locally against GDK OpenBao (2423 examples, 0 failures). A spec helper create_legacy_group_policies writes a group policy straight into OpenBao so specs can assert leftover policies are ignored. Coverage includes:

  • Model: Group is rejected with a clear error, and the policy name is nil.
  • Service: update rejects Group, delete rejects Group and leaves the leftover policy untouched, list skips leftover policies under both the users/ and api/users/ prefixes.
  • GraphQL: GROUP is rejected by the schema, a missing id is rejected by the schema, list queries never return GROUP, and userPermissions returns all false when a leftover policy exists, for both project and group.
  • Mount level with real JWTs: a leftover policy is denied on the project API mount (own group, project share, group to group share) and on the group API mount.

References

Screenshots or screen recordings

No UI changes in this MR. The UI change is in the frontend MR (step 2).

How to set up and validate locally

  1. Have OpenBao running in GDK (gdk status openbao) and the secrets manager enabled for a project you own.
  2. Run the projectSecretsPermissionUpdate GraphQL mutation with principal: { id: <group id>, type: GROUP }. The request is rejected by the schema because GROUP is no longer an enum value.
  3. Run the same mutation with principal: { type: USER } and no id. It is rejected because id is required.
  4. Optional, to see a leftover policy being ignored: in a Rails console, write a policy named users/direct/group_<group id> into the project's OpenBao namespace. The spec helper create_legacy_group_policies in ee/spec/support/helpers/secrets_management/gitlab_secrets_manager_helpers.rb shows the paths. Then query projectSecretsPermissions. The group policy is not listed, and a member of that group still cannot read secrets.

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.

Edited by Erick Bajao

Merge request reports