-
Notifications
You must be signed in to change notification settings - Fork 86
User password updates via trusted clients #318
Comments
After some initial testing it seems that allowing password updates through /v1/users/:id by enabling private fields (as I outlined in the diff in the original post) causes an issue whereby if the password field is ever omitted from a PATCH request then the 'hash' field is entirely removed from the user model. This causes a user to not be able to log in at all as they have no password set. I suspect this is caused by the way modinha-redis loads and updates the document and it could even be a bug in modinha-redis? Anyway, this highlighted how if something goes wrong when updating a password via PATCH /users/:id it could have really bad consequences and I think this gives even more weight to the argument for having a separate route just to update a password. I've created a proof-of-concept for a PATCH route /v1/rest/userpassword/:id which has been working well during my testing. @christiansmith if you have any thoughts around this from an architectural standpoint let me know and I can tidy up the code and submit as a PR for further review? |
@hedleysmith, I don't believe there's a bug in A distinct endpoint for updating passwords via API does make sense vs just handling it with Having a view for authenticated users to update their passwords (and profile) is certainly a necessity. I'd prefer to handle that in a separate issue as it warrants a broader discussion. We've had some users in the past ask about prompting for missing/required profile information and password creation after the first completion of an outbound auth flow (e.g., "sign in with GitHub"). Seems like an opportunity to solve both of those problems at once. |
@christiansmith brilliant, thanks for clarifying that. I've just added a pull request of how this could work here: #321 - I've tested this out and it seems to work fine, happy to iterate on this if you have any comments and then write some tests / update documentation. Have created a new issue here #322 for discussing the ability for users to update their profile and password information directly from Anvil. |
@hedleysmith thanks for the PR. I'll pull down your fork, kick the tires manually, and get back to you with feedback. At first glance it looks pretty good. |
Add new route for updating user passwords #318
Currently if a trusted client makes a PATCH request to /v1/users/:id and includes an updated password it is ignored as it is a 'private' field. There are some instances where updating a password through the REST API would be very useful, such as from an administration interface or from an 'update password' form on a separate domain.
This raises a number of questions:
I thought I'd open this issue before putting forward any code as there may be other design and security considerations here.
One way I figured out how to quickly allow password updates via REST is:
The text was updated successfully, but these errors were encountered: