Skip to content

Resolve "Enterprise User with disabled PATs can still create PATs"

Bogdan Denkovychrequested to merge
fix-issue-477979 into master

NOTE: AppSec confirmed that we can fix this security issue directly on canonical, see https://gitlab.com/gitlab-org/gitlab/-/issues/477979#note_2341110087.

What does this MR do and why?

#369504 (closed) added ability for groups to disable personal access tokens for their enterprise users.

https://gitlab.com/gitlab-org/gitlab/-/issues/477979 reported that enterprise users with disabled personal access tokens by their enterprise group can still create personal access tokens, despite UI does not allow that, by using gitlab-shell.

By following an implementation of the GitLab instance setting that disables personal access tokens, this MR amends disablement check for personal access tokens creation at the policy level by checking the user enterprise group setting state too.

We use PersonalAccessTokens::CreateService to create personal access tokens. This class uses create_user_personal_access_token policy to permit token creation. For that reason that MR adds specs to this class too, to confirm that the issue is fixed, for better test coverage, and to prevent regression in the future.

With this change, during the manual testing while following steps to reproduce, I see

remote: 
remote: ========================================================================
remote: 
remote: ERROR: Failed to create token: Not permitted to create

remote: 
remote: ========================================================================
remote:

Note that https://gitlab.com/gitlab-org/gitlab/-/issues/507779 will be fixed by a separate MR.

References

Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.

MR acceptance checklist

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

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Edited by Bogdan Denkovych

Merge request reports