- Notifications
You must be signed in to change notification settings - Fork1k
feat: add API endpoint to update token API keys#19983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:thomask33/09-25-resource_scoped_api_keys_in_codersdk
Are you sure you want to change the base?
feat: add API endpoint to update token API keys#19983
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ThomasK33 commentedSep 26, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stackon Graphite.
This stack of pull requests is managed byGraphite. Learn more aboutstacking. |
88f66b9 tob0bed7fCompare6d17ef1 to6fa5fe1Compareb0bed7f toacc9e28Compare59e7235 to8ab1479Compareacc9e28 to53815d9Compare8ab1479 to4fbe05fCompare53815d9 to326dbdcCompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| require.NotEmpty(t,resp.Key) | ||
| } | ||
| funcTestTokenCreationAllowsElevatedScopes(t*testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do these scoped API keys allow creating other API keys? If so, do these API keys inherit the original scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Scoped API keys allow the creation of other API keys. The newly created keys do not inherit the original key's scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Isn't that basically privilege escalation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It depends on one's perspective.
Not inheriting the original scopes allows an admin to use that API key to provision just-in-time API keys for specific use cases and revoke them later. If we enforced inheritance, the issuing API key would need all the permissions required for those actions, which could introduce the risk of creating an overly broad API key.
AWS IAM addresses this withService Control Policies (SCPs)—which act as organization-wide upper limits—andpermission boundaries, which serve as per-identity upper limits. These need to be explicitly applied to prevent this kind of privilege escalation.
For Coder, this could or should be considered as a later addition. Also, once we have the OAuth client credentials flow in place, we can advise users and admins to use that instead.
| // Valid typed allow list should succeed. | ||
| resp,err:=client.CreateToken(ctx,codersdk.Me, codersdk.CreateTokenRequest{ | ||
| Scopes: []codersdk.APIKeyScope{codersdk.APIKeyScopeWorkspaceRead}, | ||
| AllowList: []codersdk.APIAllowListTarget{codersdk.AllowResourceTarget(codersdk.ResourceWorkspace,uuid.New())}, | ||
| }) | ||
| require.NoError(t,err) | ||
| require.NotEmpty(t,resp.Key) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Are wildcard resource targets allowed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
They are supported.
4fbe05f toe1c95e4Comparefb77dfe toc8a7549Compare3acf64c to790294fComparec8a7549 toc50a0acCompare9ba5e36 to541b300Compare56cd860 tod38ed5bCompare541b300 to2a701edCompare| // Default to coder:all for backward compatibility when nothing is provided. | ||
| iflen(scopes)==0&&string(scope)=="" { | ||
| return database.APIKeyScopes{database.ApiKeyScopeCoderAll},nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should error if the user tries to set both right?
| // Default to coder:all for backward compatibility when nothing is provided. | |
| iflen(scopes)==0&&string(scope)=="" { | |
| return database.APIKeyScopes{database.ApiKeyScopeCoderAll},nil | |
| } | |
| // Default to coder:all for backward compatibility when nothing is provided. | |
| iflen(scopes)==0&&string(scope)=="" { | |
| return database.APIKeyScopes{database.ApiKeyScopeCoderAll},nil | |
| } | |
| iflen(scopes)>0&&string(scope)!="" { | |
| returnnil,xerrors.New("provide either scope or scopes, not both") | |
| } |
| returnout,nil | ||
| } | ||
| funcnormalizeTokenAllowList(entries []codersdk.APIAllowListTarget) (database.AllowList,bool,error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do we need thebool return? It just returns if the input list is non-zero right?
It's only used here
Lines 92 to 101 in2a701ed
| ifallowList,provided,err:=normalizeTokenAllowList(createToken.AllowList);err!=nil { | |
| httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{ | |
| Message:"Failed to create API key.", | |
| Detail:err.Error(), | |
| }) | |
| return | |
| }elseifprovided { | |
| params.AllowList=allowList | |
| } | |
But theparams.AllowList is not set ifprovided == false. So the boolean return does not do anything.
Theparams.AllowList defaults tonil, and ifprovided == false, the return value isnil for the returned list.
So this is equivalent
allowList,err:=normalizeTokenAllowList(createToken.AllowList)iferr!=nil {// .. Handle}params.AllowList=allowList
d38ed5b to5cb53f7Compare2a701ed to29af648Compare5cb53f7 to6248fffCompare29af648 to1a42b23Compare6248fff to67ed32bCompare1d1732b to518faffCompare4beef99 to86f7de9Compare4de874d tofd7a0fbCompare86f7de9 to854efb0Compare854efb0 to87fb51cComparefd7a0fb tod06529eCompare87fb51c to3bdb267Compared06529e toa343213Compare| r.Get("/tokenconfig",api.tokenConfig) | ||
| r.Route("/{keyname}",func(r chi.Router) { | ||
| r.Get("/",api.apiKeyByName) | ||
| r.Patch("/",api.patchToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why do we need to update api keys?
Is this a common practice elsewhere? Can the user just make a new api key with the new scopes? I feel strange allowing someone to escalate a token after it has been created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is this required for oauth, or has product requested it?
I would prefer not to add this if it is not requested/required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is this a common practice elsewhere? Can the user just make a new api key with the new scopes? I feel strange allowing someone to escalate a token after it has been created.
It is a common practice. GitHub, for example, allows you to update a personal access token and attach additional permissions to it or remove permissions. This is not only an escalation but also a de-escalation.
Is this required for oauth, or has product requested it?
I would prefer not to add this if it is not requested/required.
It's part of the API key scopes RFC.
Implements RFC-compliant PATCH /users/{user}/keys/tokens/{keyname}endpoint enabling updates to API key authorization parametersincluding scopes, allow lists, and expiration times.Key changes:- Add patchToken handler with comprehensive validation- Extract scope and allow list normalization into reusable functions - Implement authorization checks preventing non-owners from minting elevated scopes with wildcard allow lists- Add UpdateAPIKeyAuthorization database query with audit support- Generate OpenAPI documentation and TypeScript client typesThe endpoint supports partial updates with proper authorizationcontext validation and maintains backward compatibility withexisting token creation patterns.3bdb267 toc41112aComparea343213 to95b0229Compare
Add API for updating token API keys
This PR adds a new PATCH endpoint for updating token API keys. The endpoint allows updating:
scopeorscopesfield)The implementation includes:
UpdateTokenRequesttype in the SDKpatchTokenhandler in the APIThis enables users to modify existing API tokens without having to create new ones, which is particularly useful for adjusting permissions or extending token lifetimes.