<!--IssueSummary start--> <details> <summary> Everyone can contribute. [Help move this issue forward](https://handbook.gitlab.com/handbook/marketing/developer-relations/contributor-success/community-contributors-workflows/#contributor-links) while earning points, leveling up and collecting rewards. </summary> - [Close this issue](https://contributors.gitlab.com/manage-issue?action=close&projectId=278964&issueIid=623457) </details> <!--IssueSummary end--> ## Why are we doing this work First acceptance criteria item of Iteration 2 in [&22561](https://gitlab.com/groups/gitlab-org/-/work_items/22561): *"Remove 'Group' tab from permissions table."* This issue is the backend half. Removing the tab alone would leave the `Group` principal reachable through the API. Rationale, from [@sayobittencourt](https://gitlab.com/sayobittencourt) on [&22561](https://gitlab.com/groups/gitlab-org/-/work_items/22561#note_3740233394), with the first bullet corrected after testing: - **Hard to discover, but it does work.** At project level the search does not return groups, because it only returns resources *below* the current level and a project is the last level. The group can still be added by entering its URL path in the form. Verified on a test project by [@iamricecake](https://gitlab.com/iamricecake), and the count below found 13 group grants on project-level resources in production. The related claim that Subgroup A cannot grant to Subgroup B has not been verified, though the 5 cross-group grants found in the count suggest it is also possible. - **Poor usability.** The group must be entered by URL path rather than name, which breaks on rename and is error-prone. - **Unclear semantics, possible security concern.** It is not obvious what is being granted, possibly permissions for everyone in one group to manage secrets in another. - **Deprecation directive.** ~"group::authorization" has security concerns with Group Links. Per the [group sharing transition plan](https://gitlab.com/gitlab-com/tenant-scale/product/teams/-/blob/master/decisions/group-sharing-transition-plan.md) we need to deprecate anything related to group linking and inviting. - **Teams supersede it.** Organizations will introduce Teams, which is a much better fit: grant a named set of users access to secrets in one click, rather than a group as a proxy for its membership. ## Approach: count first, then decide whether to deprecate Confirmed with [@jrandazzo](https://gitlab.com/jrandazzo): we want the exact number of existing group grants, and we want it before deciding anything. **Step 1. Count the existing group grants.** Done, see [Count results](#count-results) below. Runbook kept in [How to count](#how-to-count) for the re-run. **Step 2. Decide whether to deprecate the `Group` principal at all.** Open. This is the decision the count was gathered for, and it is with [@jrandazzo](https://gitlab.com/jrandazzo). The two data points pull in different directions: 39 grants is small enough that any option stays manageable, but it is about 44% of all direct grants, which suggests deliberate use rather than accident. **Step 3. If we deprecate, decide what happens to the existing 39.** - Delete the policies on the backend, so the tab can be removed outright. This silently removes access someone granted on purpose, so it needs a deprecation announcement. - Or keep the tab in a conditional read-only form: rendered only when group grants exist for the resource, delete allowed, no **Add** button, disappearing once the last one is removed. This is extra frontend scope which [#623352](https://gitlab.com/gitlab-org/gitlab/-/work_items/623352) does not cover today. Classifying the 39 by hierarchy relationship (self, ancestor, cross-hierarchy, grantee since deleted) would inform this decision. It is not needed for Step 2, since the count already establishes that every one of them was created deliberately. **Step 4. If we deprecate, ship it.** Block new group grants, and handle the existing ones according to the Step 3 decision. **One caveat to record.** The count is a snapshot, not a stable fact. Group grants can still be created right up until a block ships, so the number will drift. Re-run the count before merging any change that blocks them, to confirm it has not changed materially. Whatever we do, we should not ship "stop accepting, hide the tab, leave the policies enforced". See the security note below. ## Count results Swept on 2026-08-27 from a read-only production Rails console over Teleport, by [@iamricecake](https://gitlab.com/iamricecake). | | Namespaces checked | Group grants found | Errors | |---|---|---|---| | Group level | 548 | 26 | 0 | | Project level | 191 | 13 | 0 | | **Total** | **739** | **39** | **0** | Zero errors, so the sweep was complete and 39 is the exact number rather than a lower bound. All direct principals seen, for context: | Principal | Group level | Project level | Total | |---|---|---|---| | `Role` | 614 | 218 | 832 | | `User` | 13 | 32 | 45 | | `Group` | 26 | 13 | 39 | | `MemberRole` | 2 | 2 | 4 | `Group` is 39 of 88 direct grants, about 44%. At group level it is the most common direct principal, roughly twice as common as `User`. At project level `User` leads. Every one of the 39 was created deliberately. Provisioning creates only the owner `Role` policy, so no code path produces a `Group` policy on its own. ## Security note Group grants are genuinely enforced today. OpenBao unions the group-membership policy into a user's effective capabilities. Leaving them enforced while hiding the tab causes two problems: - **Lost visibility.** An owner checking who can access their secrets gets an incomplete list. - **Membership drift.** A group grant resolves through current group membership, so anyone added to that group later automatically gains secrets access. Nobody granted it to them, and with the tab gone nobody can see or remove it. The enforcement is visible in the ACL itself. `ProjectSecretsManagers::UserHelper#user_auth_cel_program` builds the user's token policies as: ``` policies: (uid != "" ? [base + "/direct/user_" + uid] : []) + (mrid != "" ? [base + "/direct/member_role_" + mrid] : []) + grps.map(g, base + "/direct/group_" + string(g)) + (rid != "" ? [base + "/roles/" + rid] : []), ``` `grps` comes from the token's `groups` claim, so every group the user belongs to unions a `users/direct/group_<id>` policy into their effective capabilities at login. That is the membership-drift mechanism spelled out in code: the grant is resolved from live membership on every authentication, not from a stored list of users. ## How to count Kept as a runbook, since the count needs re-running before any change that blocks new grants. Collapsed, because the decision no longer depends on it. <details> <summary>Expand the runbook</summary> **This cannot be counted in SQL.** `BaseSecretsPermission` is an `ActiveModel::Model`, not ActiveRecord. Permissions only exist as ACL policies inside OpenBao, and `SecretsPermissions::ListServiceHelpers` rebuilds the permission objects from those policies on every read. Policy naming is deterministic, from `SecretsManagement::SecretsManagers::UserHelper#policy_name_for_principal`: | Principal type | Policy name | |---|---| | `User` | `users/direct/user_<id>` | | `MemberRole` | `users/direct/member_role_<id>` | | `Group` | `users/direct/group_<id>` | | `Role` | `users/roles/<access_level>` | So a group grant is always `users/direct/group_<group_id>` and can be counted from the policy name alone, without reading the policy body. Listing: use `client.list_policies(type: :users)`, which is what `ListServiceHelpers` already does. It hits `sys/policies/detailed/acl/users` and returns fully qualified keys, which is why the service splits the key into three parts and expects `users` at index 0. Filter with `key.start_with?('users/direct/group_')`. **Two sweeps are needed, one for groups and one for projects.** See the namespacing section below. The naming and the filter are identical for both, because `ProjectSecretsManagers::UserHelper` only adds the CEL program and does not override `policy_name_for_principal` or `user_path`; both come from the shared `SecretsManagers::UserHelper` on `BaseSecretsManager`. ### Namespacing: resolved An earlier draft of this issue suspected that project policies live under a `project_<id>/` prefix inside a parent OpenBao namespace, and that iterating `ProjectSecretsManager` would therefore look in the wrong place. **That is not the case.** Projects get their own OpenBao namespace, a sibling of the group's. `ProjectSecretsManagers::ProvisionService#enable_namespaces` creates three levels: ```ruby base_secrets_manager_client.enable_namespace(secrets_manager.org_path) # org_<org_id> org_secrets_manager_client.enable_namespace(secrets_manager.namespace_path) # group_<root_ns_id> namespace_secrets_manager_client.enable_namespace(secrets_manager.project_path) # project_<project_id> ``` `ProjectSecretsManager#full_project_namespace_path` is therefore `org_X/group_Y/project_Z`, a sibling of `GroupSecretsManager#full_group_namespace_path` (`org_X/group_Y/group_Z`). Confirmed on production: `org_1/group_785414/project_77505350`. Project permissions live at `acl/users` inside the project's own namespace, and `ProjectSecretsPermissions::ListService` reads them with `project_secrets_manager_client` through the same shared `ListServiceHelpers`. So iterating `ProjectSecretsManager` separately is exactly right. `list_project_policies` is not the read path for project permissions: it has no callers in application code, only in `ee/spec/lib/secrets_management/secrets_manager_client_spec.rb` and `ee/spec/requests/secrets_management/authentication_boundaries_spec.rb`, where it asserts what a project-scoped JWT may reach from a parent namespace. It can be ignored for counting purposes. ### Getting a client **There is no `client` method on the secrets manager models.** An earlier draft of this section used `sm.client`. That does not exist: neither `BaseSecretsManager`, `GroupSecretsManager` nor `ProjectSecretsManager` defines it. The client is built in the service layer, in `ee/app/services/secrets_management/group_base_service.rb`: ```ruby def base_secrets_manager_client jwt = GroupSecretsManagerJwt.new(current_user: current_user, group: group).encoded SecretsManagerClient.new(jwt: jwt) end def group_secrets_manager_client base_secrets_manager_client.with_namespace(group.secrets_manager.full_group_namespace_path) end ``` `ProjectBaseService` mirrors this with `ProjectSecretsManagerJwt` and `full_project_namespace_path`. Note the asymmetry: these helpers are **private** on `GroupBaseService` but **public** on `ProjectBaseService`, so the group sweep needs `send` and the project sweep does not. The least error-prone way to get a correctly scoped client from a console is to instantiate the list service and reach for its helper, which keeps JWT construction and namespace scoping in one place rather than duplicating them in the script: ```ruby # group SecretsManagement::GroupSecretsPermissions::ListService .new(group, current_user) .send(:group_secrets_manager_client) # project SecretsManagement::ProjectSecretsPermissions::ListService .new(project, current_user) .project_secrets_manager_client ``` **`current_user` must be a real user, for both.** Verified on `gprd`: passing nil fails on every namespace with ``` failed to perform inline authentication: missing name in alias (SecretsManagement::SecretsManagerClient::ServiceUnavailableError) ``` surfaced as an HTTP 500 from `GET /v1/<namespace>/sys/policies/detailed/acl/users?list=true`. The `app` JWT role is configured with `user_claim: user_id` (see `doc/administration/secrets_manager/maintenance.md`), and `GroupSecretsManagerJwt#resource_claims` sets `user_id: current_user&.id.to_s`, which is `""` for a nil user. `.compact` strips nils but not empty strings, so OpenBao receives an empty `user_id` and cannot build the identity alias. `GlobalSecretsManagerJwt` avoids this by hardcoding `user_id: SYSTEM_UID`; neither resource JWT has an equivalent fallback. The project side is the same or worse: `ProjectSecretsManagerJwt` delegates to `JSONWebToken::UserProjectTokenClaims#user_claims`, which sets `user_id: user&.id.to_s` with no `.compact` at all. Note that this failure looks like OpenBao instability but is not. During a sweep, `ServiceUnavailableError` on *every* namespace means the JWT is wrong, not that OpenBao is degraded. **Group membership is not required.** The `app` role has fixed `token_policies: ["secrets_manager"]` and authenticates on `bound_subject: gitlab_secrets_manager`, which `GlobalSecretsManagerJwt#payload` supplies as `sub: SYSTEM_UID`. The `user_id` claim only names the OpenBao identity alias; nothing resolves the caller's membership. On the GitLab side, `ListServiceHelpers#execute` only checks `resource.secrets_manager&.active?`, with membership and role checks living in the GraphQL resolvers that a console call bypasses. This is deliberate but worth stating plainly: the sweep reads every group's and project's ACL policies through a privileged system role, including namespaces the operator has no membership in. It is read-only and it is the same path the app takes on every permissions page load, but the OpenBao audit log will attribute thousands of namespace reads to one user. Flag that to whoever approves the Teleport session. Use your own account rather than a group or project owner's, so the attribution is honest. For contrast, `user_client` in the same services uses `GroupUserJwt` / `ProjectUserJwt` and the `all_users` role, which *does* resolve membership and member roles. The permissions `ListService` intentionally uses the secrets manager client instead. ### The group sweep Resumable, because Teleport is the only vehicle we have and the session can drop. Note the last `last_id=` printed and re-run with `LAST_ID` set to it. ```ruby LAST_ID = 0 # resume point: last `last_id=` printed by a previous run BATCH = 100 PAUSE = 0.05 # seconds between OpenBao list calls sweep_user = User.find_by_username('<your-username>') # required, see above found = [] errors = [] tally = Hash.new(0) seen = 0 SecretsManagement::GroupSecretsManager .active .where('id > ?', LAST_ID) .each_batch(of: BATCH) do |batch| batch.preload(group: :organization).each do |sm| seen += 1 begin client = SecretsManagement::GroupSecretsPermissions::ListService .new(sm.group, sweep_user) .send(:group_secrets_manager_client) client.list_policies(type: :users) do |policy| key = policy['key'] kind = case key when %r{\Ausers/direct/user_} then 'User' when %r{\Ausers/direct/member_role_} then 'MemberRole' when %r{\Ausers/direct/group_} then 'Group' when %r{\Ausers/roles/} then 'Role' end tally[kind] += 1 if kind found << [sm.id, sm.group_id, key] if kind == 'Group' end rescue StandardError => e errors << [sm.id, sm.group_id, e.class.name, e.message] end sleep PAUSE end puts "last_id=#{batch.maximum(:id)} seen=#{seen} found=#{found.size} errors=#{errors.size} #{tally.inspect}" end puts "GROUP TOTAL seen=#{seen} found=#{found.size} errors=#{errors.size}" puts tally.inspect puts errors.group_by { |e| e[2] }.transform_values(&:size).inspect puts found.inspect ``` Notes on the shape: - `.active` replaces the per-record `sm.active?` check. - `found` entries are `[secrets_manager_id, group_id, policy_key]`, so both the granting namespace and the granted-to group are recoverable afterwards. - `tally` exists to give a zero result a denominator. `Group => 0` alongside a healthy `User` count means the sweep provably parsed real direct grants and found no group ones. `Group => 0` with nothing but `Role` counts is a much weaker claim and probably means something is wrong. - The `if kind` guard matters: `case` with no `else` returns nil, and without the guard a nil key lands in the tally instead of being skipped. - On resume, `found`, `errors`, `tally` and `seen` all reset. Record the printed totals for each chunk and sum them at the end. ### The project sweep Symmetric. `project_secrets_manager_client` is public on `ProjectBaseService`, so no `send` here. ```ruby LAST_ID = 0 BATCH = 100 PAUSE = 0.05 sweep_user = User.find_by_username('<your-username>') found = [] errors = [] tally = Hash.new(0) seen = 0 SecretsManagement::ProjectSecretsManager .active .where('id > ?', LAST_ID) .each_batch(of: BATCH) do |batch| batch.preload(:project).each do |sm| seen += 1 begin client = SecretsManagement::ProjectSecretsPermissions::ListService .new(sm.project, sweep_user) .project_secrets_manager_client client.list_policies(type: :users) do |policy| key = policy['key'] kind = case key when %r{\Ausers/direct/user_} then 'User' when %r{\Ausers/direct/member_role_} then 'MemberRole' when %r{\Ausers/direct/group_} then 'Group' when %r{\Ausers/roles/} then 'Role' end tally[kind] += 1 if kind found << [sm.id, sm.project_id, key] if kind == 'Group' end rescue StandardError => e errors << [sm.id, sm.project_id, e.class.name, e.message] end sleep PAUSE end puts "last_id=#{batch.maximum(:id)} seen=#{seen} found=#{found.size} errors=#{errors.size} #{tally.inspect}" end puts "PROJECT TOTAL seen=#{seen} found=#{found.size} errors=#{errors.size}" puts tally.inspect puts errors.group_by { |e| e[2] }.transform_values(&:size).inspect puts found.inspect ``` The project sweep is heavier per record. `UserProjectTokenClaims#user_claims` calls `user_access_level`, which is `project.team.human_max_access(sweep_user.id)`, so every project JWT does a membership lookup on top of `root_ancestor` and `full_path`. Still read-only, just slower per namespace than the group sweep. Run them as separate pastes, since they share variable names and each has its own `LAST_ID` resume point. ### Dry run one namespace first Before either wide sweep, run the loop body against a single known-provisioned secrets manager with no `rescue`, so mistakes raise instead of being tallied: ```ruby me = User.find_by_username('<your-username>') sm = SecretsManagement::GroupSecretsManager.active.first group = sm.group client = SecretsManagement::GroupSecretsPermissions::ListService .new(group, me) .send(:group_secrets_manager_client) puts "group=#{group.full_path} namespace=#{sm.full_group_namespace_path}" keys = client.list_policies(type: :users).map { |p| p['key'] } puts "total_user_policies=#{keys.size}" puts keys.sort puts keys.select { |k| k.start_with?('users/direct/group_') } ``` `total_user_policies` should be at least 1, because `ProvisionService#create_owner_policy` creates a `Role` policy under `users/roles/`. An empty array means the listing or the namespace scoping is wrong, not that the resource has no permissions. Cross-check the name filter against the app's own parsing, which is what actually classifies a policy as a `Group` grant. If the two diverge, `extract_principal_info_from_policy` in `ListServiceHelpers` sees something `start_with?` does not, and the sweep's filter needs fixing first: ```ruby result = SecretsManagement::GroupSecretsPermissions::ListService.new(group, me).execute perms = result.payload[:secrets_permissions] puts perms.group_by(&:principal_type).transform_values(&:size).inspect puts perms.select { |p| p.principal_type == 'Group' }.map(&:principal_id).inspect ``` On 2026-08-27 this matched exactly for a project holding one `User` and one `Role` policy, returning `{"User"=>1, "Role"=>1}` and an empty Group list, which is the evidence that the sweep's classification agrees with the application. ### Execution caveats - **A silent zero is the main hazard.** A bare `rescue StandardError; next` turns any mistake in the loop body into `found.size == 0`, which reads as "no group grants exist" rather than "the sweep never ran". Always report the error count and the tally next to the total, and treat `found=0, errors=N` as a failed run rather than an answer. - **Where the sleep goes.** `list_policies` is one LIST call per namespace and then yields per policy from an in-memory hash, so a sleep inside the block throttles nothing. It belongs in the outer per-secrets-manager loop, which is the HTTP call. The loop is serialized and therefore already bounded to roughly 20-50 requests per second; `PAUSE` is cheap insurance on top of that. - **Sustained load on OpenBao.** One list call per provisioned namespace, across every group *and* every project. - **N+1 on the DB side.** `full_group_namespace_path` reads `group.organization_id` and `group.root_ancestor`; the project equivalent adds `human_max_access`. Each namespace therefore costs several queries beyond the OpenBao call. The `preload` calls cover part of it; `root_ancestor` remains one query per record. - **Read-only node is fine.** `list_policies` is a GET with `list: true`, and inline auth mints a request-scoped token that OpenBao never persists. No DB or OpenBao writes. </details> ## Implementation plan - [x] Confirm how project policies are namespaced. **Resolved:** projects get their own OpenBao namespace at `org_X/group_Y/project_Z`, a sibling of the group namespace, so `ProjectSecretsManager` must be iterated separately with `project_secrets_manager_client`. `list_project_policies` is unused in application code and is not the read path. See the namespacing section above. - [x] Run both sweeps and record the exact counts. **26 at group level, 13 at project level, 39 total, 0 errors across 739 namespaces.** See [Count results](#count-results). - [ ] **Decide whether to deprecate the `Group` principal at all.** With [@jrandazzo](https://gitlab.com/jrandazzo). Everything below applies only if we decide to deprecate: - [ ] Decide and record the Step 3 approach for the existing 39. Classifying them by hierarchy relationship would inform this. - [ ] Reject `Group` on the permission create and update paths. `validate_principal_types` in `ee/app/models/secrets_management/base_secrets_permission.rb` derives from `PRINCIPAL_TYPES`, so the change needs to distinguish "valid for write" from "valid for read/delete". - [ ] Guard the policy generation path. If provisioning or any repair or re-sync job rebuilds policies from stored state, blocking the mutation alone is not enough and a group policy could come back. - [ ] Handle existing grants per the Step 3 decision. Note that cleanup is not a Rails migration, since the data lives in OpenBao. It means iterating both group and project namespaces through `SecretsManagerClient`. - [ ] Re-run the count immediately before merging the block, to confirm the number has not drifted. - [ ] Affected mutations. The principal type is an argument, so no mutation is removed: `ProjectSecretsPermissions::{Update,Delete}`, `GroupSecretsPermissions::{Update,Delete}`, and the legacy `Permissions::{Update,Delete}`. - [ ] Update `doc/ci/secrets/secrets_manager/_index.md`, which currently says *"Select Add to add permissions rules for specific users, groups, or roles."* ## Sequencing If we deprecate, do it **before** [#596011](https://gitlab.com/gitlab-org/gitlab/-/work_items/596011) removes the `experiment:` annotations from the Secrets Management GraphQL mutations. While they are still experiment level, changing or removing the `Group` value is cheap. Afterwards we would owe a formal deprecation cycle. This is a reason to reach the Step 2 decision reasonably soon, even if the answer is "not yet". Coordinate merge order with [#623352](https://gitlab.com/gitlab-org/gitlab/-/work_items/623352). The tab must not disappear before existing grants are either cleaned up or made removable. ## Acceptance criteria - [x] Exact count of existing `Group` grants recorded for both group and project namespaces, with the number of failed namespaces for each - [ ] Decision recorded on whether to deprecate the `Group` principal If we deprecate: - [ ] Approach for the existing grants chosen and recorded - [ ] `Group` is rejected as a principal type when creating or updating a secrets permission, with a clear error - [ ] No policy generation path can recreate a group policy - [ ] No group grant is left enforced but unreachable - [ ] Docs updated ## Related - [#623352](https://gitlab.com/gitlab-org/gitlab/-/work_items/623352) frontend counterpart, removes the tab from the permissions table - [#621682](https://gitlab.com/gitlab-org/gitlab/-/work_items/621682) design