Prohibit application tokens to access repositories with GUEST permission #1093
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation:
We are aiming to implement stricter and more explicit access control in
Central Dogma.
It has been observed that many tokens are accessing repositories as a
GUEST role without being registered with the project. If the token is
exposed to the outside, an attacker could gain access to all
repositories that allow GUEST access.
To resolve the issue, I propose restricting new tokens from accessing
repositories that have GUEST permission. For backward compatibility,
the existing tokens are still allowed to access repositories with GUEST
permission.
Modifications:
- Add `allowGuestAccess` field to `Token` class to indicate if a token
is allowed to access a repository with the GUEST role.
- If the value is null in `tokens.json`, it defaults to `true` for
backward compatibility.
- Fixed `MetadataService` to prohibit tokens whose `allowGuestAccess` is
false from accessing repositories with the GUEST role.
Result:
New application tokens no longer have access to repositories with the
`GUEST` role.
jrhee17
approved these changes
Feb 10, 2025
| @JsonProperty("creation") UserAndTimestamp creation, | ||
| @JsonProperty("deactivation") @Nullable UserAndTimestamp deactivation, | ||
| @JsonProperty("deletion") @Nullable UserAndTimestamp deletion) { | ||
| assert isAdmin != null || isSystemAdmin != null; | ||
| this.appId = Util.validateFileName(appId, "appId"); | ||
| this.secret = Util.validateFileName(secret, "secret"); | ||
| this.isSystemAdmin = isSystemAdmin != null ? isSystemAdmin : isAdmin; | ||
| // Allow guest access by default for backward compatibility. |
There was a problem hiding this comment.
Note: Although allowGuestAccess=true is allowed via API, it isn't allowed via UI. I understood the reason is this field will eventually be deprecated and guest access won't be allowed entirely.
There was a problem hiding this comment.
Right. Users can't set allowGuestAccess=true either in the UI or the REST API.
allowGuestAccess is true for old tokens or administrative tokens.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
We are aiming to implement stricter and more explicit access control in Central Dogma.
It has been observed that many tokens are accessing repositories as a GUEST role without being registered with the project. If the token is exposed to the outside, an attacker could gain access to all repositories that allow GUEST access.
To resolve the issue, I propose restricting new tokens from accessing repositories that have GUEST permission. For backward compatibility, the existing tokens are still allowed to access repositories with GUEST permission.
Modifications:
allowGuestAccessfield toTokenclass to indicate if a token is allowed to access a repository with the GUEST role.tokens.json, it defaults totruefor backward compatibility.MetadataServiceto prohibit tokens whoseallowGuestAccessis false from accessing repositories with the GUEST role.Result:
New application tokens no longer have access to repositories with the
GUESTrole.