User password updates via trusted clients

[hedleysmith]

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:

• Conceptually, should password updates be allowed through the regular /v1/user/:id endpoint? I think it makes sense, as email address updates are allowed already which are also a key component for users and for user security (e.g if a malicious user updated an email address to their own they could easily take over an account through the password recovery facility).
• If updating passwords through /user/:id is not desirable, would a separate endpoint (e.g /changePassword) purely for modifying passwords be a sensible alternative? The OIDC spec seems to not make much mention of password changes so it should fit in with the spec and the general design of Connect, however this could make for more API calls and might not fit so obviously with REST principles.
• A separate but related feature would be to allow users to update their own password by adding a new view within Anvil. This could even be extended to a whole 'edit my profile' view. How this view operates and which endpoints it uses would make sense to be in-line with how clients access & update this information.

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:

routes/rest/v1/users.js

server.patch('/v1/users/:id', authorize, function (req, res, next) {
- User.patch(req.params.id, req.body, function (err, instance)
+ User.patch(req.params.id, req.body, { private: true }, function (err, instance)

[hedleysmith]

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?

[christiansmith]

@hedleysmith, I don't believe there's a bug in modinha-redis, this is simply a model-specific bit of logic. You can see how we handled password setting for user creation in User.insert(), which is overridden from the default method provided by modinha-redis. There's also a setter method for hash defined in the schema.

A distinct endpoint for updating passwords via API does make sense vs just handling it with PATCH /v1/users/:id. For implementing this, there's already the User.changePassword() method that's used at the /resetPassword endpoint (see routes/recovery.js). You can see that this uses the existing User.patch() implementation.

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.

[hedleysmith]

@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.

[christiansmith]

@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.