fix(coderd): allow user-admin password resets to succeed#26537
Open
geokat wants to merge 1 commit into
Open
Conversation
User Admin password resets could update the target user's hashed password but fail while revoking that user's API keys. The transaction then rolled back and returned HTTP 500, so the password was never changed. Add a user-scoped API key revoker actor and use it in both password reset flows so key revocation succeeds without broader system auth. Refs: https://linear.app/codercom/issue/PLAT-316
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
Resetting a password revokes all of the target user's API keys via
DeleteAPIKeysByUserID, whosedbauthzcheck requiresapi_key:deleteon those keys. Theuser-adminrole has full access toResourceUserbut no permissions onResourceApiKey, so the revocation failed authorization inside the transaction and surfaced as a 500. Owners worked only because they hold wildcard permissions.Fix
Add a narrowly-scoped
AsAPIKeyRevokerdbauthzactor that can delete only the API keys owned by a specific user, and nothing else. Use it for the key revocation in both the admin password-reset path (putUserPassword) and the one-time-passcode reset path, which previously leaned on the much broaderAsSystemRestricted.Refs: https://linear.app/codercom/issue/PLAT-316