-
Notifications
You must be signed in to change notification settings - Fork 25k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Formal support for "password_hash" in Put User #35242
Conversation
For some time, the PutUser REST API has supported storing a pre-hashed password for a user. The change adds validation and tests around that feature so that it can be documented & officially supported.
Pinging @elastic/es-security |
I will raise follow up PRs for
|
@elasticmachine test this please I think the CI failure will be resolved by 81daf4c |
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.
LGTM. I think the commit message and title of this pr needs s/password_user/password_hash
@jkakavas Ping. Do you want to review? I'd like to merge once I can get CI to pass. |
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.
I'm slightly worried that we don't deal well enough with a PutUser request that has both a password
and a password_hash
parameter. As is, whatever is last in the parameters order will be the actual user password.
Since PutUserRequest
only knows about passwordHash
, we can't do the validation there but I was wondering if we should validate this in PutRequestBuilder
and throw a friendly please fix your input
message, and add an accompanying test for it.
Also, just a reminder to update API Docs also
@jkakavas I've added a check for setting the password_hash twice - can you review again? |
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.
LGTM
PR elastic#35242 formalised support for the password_hash field in the body of the Put User security API. Since this field is now validated and tested, it can also be documented. The Put User API also supports a "refresh" query parameter that was not documented. This commit adds it to the docs.
Update PutUserRequest to support password_hash (see: elastic#35242) This also updates the documentation to bring it in line with our more recent approach to HLRC docs.
For some time, the PutUser REST API has supported storing a pre-hashed password for a user. The change adds validation and tests around that feature so that it can be documented & officially supported. It also prevents the request from containing both a "password" and a "password_hash".
PR #35242 formalised support for the password_hash field in the body of the Put User security API. Since this field is now validated and tested, it can also be documented. The Put User API also supports a "refresh" query parameter that was not documented. This commit adds it to the docs.
PR #35242 formalised support for the password_hash field in the body of the Put User security API. Since this field is now validated and tested, it can also be documented. The Put User API also supports a "refresh" query parameter that was not documented. This commit adds it to the docs.
Update PutUserRequest to support password_hash (see: #35242) This also updates the documentation to bring it in line with our more recent approach to HLRC docs.
Update PutUserRequest to support password_hash (see: #35242) This also updates the documentation to bring it in line with our more recent approach to HLRC docs.
For some time, the PutUser REST API has supported storing a pre-hashed
password for a user. The change adds validation and tests around that
feature.
Relates: #34729