When working on RA-833 , i realized that when a user is updated via REST, the user resource only saves the user account but doesn’t update their password if specified.
I think the user resource needs to update the user’s password if specified after saving the user? I created RESTWS-528 to address this and added it to the admin UI sprint since it is a blocker to one of the tickets
Yes, you should be allowed to change a user’s password via REST. (We particularly have the UserAndPassword helper class as an artifact of this.)
Please remember that to change a user’s password, you either need to (a) be that user and also submit your old password, or (b) has some elevated privilege.
(I.e. don’t forget the security check on this.)
Also, please make sure neither the password or salt is ever exposed (i.e., we should never be exposing password or salt outside the Java API, much less a REST API) – i.e., setting a password via REST is okay, but retrieving a password (or salt) – even in a response – is not.
There are some industry-wide “best practices” to this. One should not be able to change the password, but only reset it unless you are logged in as that user. This is particularly important so that the user cannot just login to another users account, even though you are the admin. Obviously with database access, you can change salts + hash and then do this… but then it means a deeper breach. I know we don’t implement this meaningfully in OpenMRS now… But we should think about this, not just for REST. But thinking about it now (and a ticket in core after we agree about this is good) that we are doing in REST is my recommendation.
I’ve seen a case where an admin user changed the user password, logged in as that user, made a change with the user not knowing it at all. There is no way for non-repudiation principle in the way its currently implemented in OpenMRS.
As it stands now there is a “Become This User” function (available to superusers) in the legacy admin UI. It’s a fair point that for non-repudiation we should not re-implement this in the reference application (and, perhaps, should remove it from the old UI, in the context of what @sunbiz says).
@wyclif, what is the specific thing in the Admin UI sprint that is triggering this? Is it just the ability for a user to change their own password via My Account? Or are we doing the Edit User use case?
I agree with @sunbiz, ideally, admins should not change a user’s password, secret question and answer, the legacy UI does let anyone with the appropriate privileges change these fields on the user form which is wrong and i will be excluding them from the edit account form in the new UI except when they are creating a new account.
So these are changes that need to be made:
changePassword page if force password is set to true, this is implemented in the old UI, again this behavior needs to be provided by the API i think, when a user authenticates and force password user property is set to true, it should throw a specific password related exception that the web layer can catch and act appropriately upon, i will create a platform ticket for this too.And in REST module:
forcePassword property to UserAndPassword resource, so that admins can force users to set their passwordsSounds right?
How does a headless platform (e.g., Java API or REST API) enforce a password change?
For the Java API, would any calls request when a user’s account has forcePassword == true throw an UnauthorizedException with some sort of clue as to why?
For the REST API, would any request made to the API when a user’s account has forcePassword == true return a 302 Temporary Redirect with Location pointing to a URI for updating the password? Or would it return a 401 Unauthorized with an error message stating that the password must be changed?
@burke , the API currently already throws several types of Exceptions that are sub classes of PasswordException e.g WeakPasswordException, we can add another subclass PasswordChangeRequiredException and this is thrown by the API when a user authenticates and the user property is set to true. The web layer should catch this exception type when a user logs in.
As for REST, the assumption is that the user some how already authenticated with the API by they time they have an authenticated jsessionId and it is safe to assume that change password should be have been enforced during authentication so i wouldn’t worry about that
@darius in the admin UI module, the changePassword form is processed by a traditional Spring controller and that works fine. The edit user form in the legacy UI has fields for an admin to set a user’s password which am saying we need to get rid of in the admin UI’s edit user form except when creating a new user account, when editing we should only have the forcePasswordChange user property, that way the admin never changes a user’s password but rather forces them to change it so that the next time they login into the system they get redirected to the changePassword page. FYI the legacy UI already does this bit when a user logs in
We cannot assume all REST calls will come from a web app already authenticated with the API. For example, if you have written a plugin for a (non-OpenMRS) lab or pharmacy system to fetch data from the local OpenMRS instance in the background and the administrator turns on forcePasswordChange on the account being used, how will the REST API respond to your code’s “authenticated” call to GET /ws/rest/v1/encounter/{uuid}? It can’t return the encounter . It could simply respond with 401 Unauthorized, but it would be nice to provide some hint of why the account is suddenly unauthorized.
I like the idea for the PasswordChangeException. Whenever this exception is thrown, the omod-common (org.openmrs.module.webservices.rest.web.response) gives 302 redirect response pointing to the /session resource, where one is supposed to get authenticated.
I had a chance to discuss this with Burke and this is what we agreed on, it is not very different from what has been suggested on this thread.
What needs to be done now in the reference application?
In the admin UI module when creating a new user, the admin sets a password for the user which is technically temporary because a user account can’t be created in the API without a password, when the form is submitted and the user had been saved, the code would also set the user property for force password change to true, this is already an existing a standard user property defined in core and probably in the future we need to add it as a column in the user table. Upon a user’s first successful login attempt, the login controller in the reference application module should check the user property, if it is set to true, it redirects the user to the change my password form. When the user submits the change password form and the operation is successful, the controller should set the user property back to false if the authenticated user is actually that user and we are done.
Now what do we want to do in the long run?
We want to move this behavior to the API with the option of the admin to set a global property to opt out of it for flexibility in case of what i call ‘clueless and controlling’ admins who want to create passwords for all their users who forget the user can actually change it later to something else anyways OR in Burke’s case of automated systems communicating with OpenMRS i.e where there is nothing like a user interface to send a user to change their password, this global property can be used to by pass the force password change, though i still think this is easy to get around because the admin can edit the associated user account and unset the user property.
The way we would implement it in the API is such that, when you save a user account, it should set the user property to true for a new user account if the admin hasn’t opted out. And whenever Context.authenticate is called, upon successful login it throws an exception say a new one PasswordChangeException if force password change is set to true for that user. Then we tweak the changePassword logic in core to unset the user property upon a successful password change if the authenticated user is actually that user.
Then we get rid of the logic that sets the user property from the admin UI module and update the login controller in the reference application to redirect the user to the change password form whenever this exception type is thrown. Same thing in REST, the AuthorizationFilter filter should always catch this exception type and send back the appropriate http code and message to inform the client about what went wrong.
I can create the platform tickets if you agree, what do you think?
Do you guys agree with the above solution? We need to get this done ASAP.
By the way there is a ChangePasswordFilter in core which will redirect an authenticated user to the change password form in core upon any request they make if the property is set to true, so we might just need to re-redirect that request to the changePassword form in the admin UI module.
@wyclif, if you’re going to ask for feedback right before a long weekend you need to write a shorter message, or highlight the key points. 
I think what you’re saying is:
I agree with these.
Thanks Darius! For your Second bullet, it is more like both i.e the first time the user logs in we force them to change the password. Also the next time they make an authenticated request the filter in core will automatically force them and we can’t control that except to redirect them to the form in the new UI.
Created the reference application ticket:
hey guys, so seems like this is not available in OpenMRS 1.12, right? I am wondering as part of the fix (in 2.0), can user change his own password - from Wyclif’s comment on ticket 528 18th May 2016, seems like it can’t be done? also, wondering what would be the exact payload from the change password request?
{
"username": "user1" ,
"password": "Testing123"
}
If so, then this means, only if I have logged on, I can change password of someone else. Let me know if my understanding is correct.
You don’t need to specify the username, the user is fetched by uuid which is already included in the URL
Hmm. seems like I can’t get it to work with openmrs rest service 2.15 + openmrs core 1.12.x. As a superuser (Admin) I tried to change password of another user by a simple HTTP POST to
“openmrs/ws/rest/v1/user/” with headers of Content-Type: application/json
{
"password" : "Test12345"
}
No luck! I just get a 302 found response to the call and get automatically redirected to the GET call of the same URL. Unless I am missing something … Does this webservices module version work only with openmrs core 2.0? If so, then we should change it so.
Also, should we be thinking of the usecase, where a user wants to change his own password (not logging through openmrs console but using REST api).
Currently we don’t support a user changing their own password via rest but there has been discussions to support it and should be possible at some point.
In my earlier comment, i did mention that in rest the uuid should be part if the URL when updating so your case you should post to openmrs/ws/rest/v1/user/uuid