[fix][core] stop returning credentials from the current user endpoint (24.05) - #7940
Open
ar2rsawseen wants to merge 1 commit into
Open
[fix][core] stop returning credentials from the current user endpoint (24.05)#7940ar2rsawseen wants to merge 1 commit into
ar2rsawseen wants to merge 1 commit into
Conversation
/o/users/me answered with the caller's member document after deleting only the password, so the response carried the account's api_key and, for members using two factor auth, the stored secret for that factor. The neighbouring reads already treat both as sensitive: getUserById and getAllUsers project api_key away, and the member event payloads delete it with a comment saying it must never be forwarded. This was the one read path that returned it. Answer with a copy of the member that has the password, the api_key and the two factor object removed. Nothing in the product reads the key from here: the dashboard takes it from the server rendered globals, and a member who wants their own key has the /api-key route. Also refuse a token that was restricted to specific applications on this endpoint. It reports the caller's own account and belongs to no application, so a token deliberately limited to some applications has no business reading it. The restriction in verify_token is only compared when the request itself names an application, and this request never does, so without this an app restricted token still reached account level data. Tokens with no application restriction keep working exactly as before, which is what the dashboard and the existing suites use.
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
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.
Backport of #7939 to release.24.05.
What
/o/users/mereturned the caller's member document with only the password removed:So the response carried the account's
api_key, and for members using two factor auth the stored secret for that factor as well. The neighbouring reads already treat both as sensitive:getUserByIdandgetAllUsersprojectapi_keyaway, and the member event payloads delete it with a comment saying it must never be forwarded. This was the one read path that handed it out.Separately, a token that was restricted to specific applications could reach this endpoint. The restriction in
verify_tokenis only compared when the request itself names an application, and this request never does, so the restriction was not applied.Change
password,api_keyandtwo_factor_authremoved. A copy, so nothing later in the request sees a mutated member.Tokens with no application restriction behave exactly as before.
Why this is safe to change
api_keyfrom the server rendered globals, and there is a dedicated/api-keyroute for a member who wants their own key. No frontend code calls/o/users/meat all.email,full_name,global_adminandusername./o/users/meintest/2.api/14.authorize.token.jsis created with noapp, so it is unrestricted and unaffected. The token inApp-scoped token enforces its app restrictionis pinned to^/o/actions, so it never reaches this endpoint either.two_factor_auth.enabledremains available where the UI actually reads it, from the user listing.Verification
Two cases added:
test/2.api/02.read.user.js: the response still carries the account's own fields but has noapi_key, nopasswordand notwo_factor_auth.test/2.api/14.authorize.token.js: a token restricted to one application, with no endpoint restriction, is refused on/o/users/me. This is a different token shape from the existing app restriction case, which is endpoint pinned.Lint and
node --checkclean on all changed files.