Block PAT creation for service accounts pending deletion

What does this MR do and why?

Service account personal access tokens (PATs) could still be created while the service account was pending deletion.

When a service account is deleted, it is not removed immediately. The delete API enqueues DeleteUserWorker, which runs Users::DestroyService. That service blocks the user and creates a Users::GhostUserMigration record, and a cron worker finishes the deletion later. During this pending-deletion window, owners and admins could still create new PATs for the service account through the UI and the API.

The root cause is in app/policies/user_policy.rb. The rules enabling :create_personal_access_token and :rotate_personal_access_token use ~blocked. blocked comes from BasePolicy and means @user&.blocked?, i.e. the state of the person performing the action, not the target service account. Nothing checked the target account's own state.

This MR adds a rule in app/policies/user_policy.rb that uses the existing subject-scoped blocked_user condition (@subject.blocked?) to prevent :create_personal_access_token and :rotate_personal_access_token. Checking the ghost user migration is not needed, because Users::DestroyService blocks the user before it creates the Users::GhostUserMigration record, so a pending-deletion account is always blocked.

PersonalAccessTokens::CreateService checks this policy, so the fix covers the group, project, and instance service account creation APIs, and the UI, since the UI's "Manage access tokens" page calls the same REST API. The API now returns 422 with {"message":"Not permitted to create"}, the same response already used for other denied creations.

Behaviour change

The rule applies to any blocked user, not only service accounts. User#blocked? is true for the blocked, ldap_blocked, blocked_pending_approval and banned states. Previously an admin could still create or rotate a PAT for a blocked human user (for example with the admin POST /users/:id/personal_access_tokens endpoint), because ~blocked only checked the admin. Now that is denied too. This was agreed in review, since PATs of blocked users cannot be used anyway.

Scope and follow-up

The service-account rotate endpoints (POST .../personal_access_tokens/:token_id/rotate) call PersonalAccessTokens::RotateService, which does not check this policy. Rotation on those endpoints is not blocked by this MR.

For a soft delete, the account is only blocked once DeleteUserWorker runs. The short window between the delete request and the worker starting is not covered.

The UI does not yet disable the "Add new token" button or show a dedicated message for a pending-deletion account ahead of the request. The list API entity (API::Entities::ServiceAccount) does not expose the account's state, so surfacing that in the UI would be a separate change.

Resolves #602352

Changelog: fixed

References

Screenshots or screen recordings

Before After

How to set up and validate locally

On a local GDK at http://127.0.0.1:3000, as an admin:

  1. Create a group service account via API:

    curl --request POST --header "PRIVATE-TOKEN: <admin_token>" "http://127.0.0.1:3000/api/v4/groups/<group_id>/service_accounts"
  2. Block the account from the Rails console (gdk rails c). The delete worker blocks the account first, so this is the same state as pending deletion. Doing it by hand is easier because the real deletion finishes quickly in the background:

    sa = User.find(<service_account_id>)
    sa.block!
  3. Try to create a token:

    curl --request POST --header "PRIVATE-TOKEN: <admin_token>" --data "name=test&scopes[]=api&expires_at=<a date within a year>" "http://127.0.0.1:3000/api/v4/groups/<group_id>/service_accounts/<service_account_id>/personal_access_tokens"
  4. Expected: 422 with {"message":"Not permitted to create"}. On master it returns 201 with a token.

  5. Optional UI check: Group > Settings > Service accounts > Manage access tokens for that account > Add new token. An error is shown and no token is created.

Run the specs:

bin/rspec spec/policies/user_policy_spec.rb spec/requests/api/group_service_accounts_spec.rb

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 #602352

Edited by Sahil Bhatt

Merge request reports

Loading
Loading