REST should require you to send the old password when updating your password

padmavati · December 29, 2016, 5:39pm

Hello,

In Bahmni we have recently developed the Change Password feature for logged in user.Please refer to the following link https://bahmni.atlassian.net/wiki/display/BAH/Features For developing this feature we have used UserResource2_0 in webservices rest.But this end point doesn’t support verification old password.

github.com

openmrs/openmrs-module-webservices.rest/blob/master/omod-2.0/src/main/java/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs2_0/UserResource2_0.java#L45


			description.removeProperty("secretQuestion");
		}
		return description;
	}
	
	/**
	 * @see org.openmrs.module.webservices.rest.web.resource.impl.DelegatingCrudResource#save(java.lang.Object)
	 */
	@Override
	public UserAndPassword1_8 save(UserAndPassword1_8 user) {
		User openmrsUser = new User();
		if (user.getUser().getUserId() == null) {
			openmrsUser = Context.getUserService().createUser(user.getUser(), user.getPassword());
		} else {
			openmrsUser = Context.getUserService().saveUser(user.getUser());
			Context.refreshAuthenticatedUser();
		}
		
		return new UserAndPassword1_8(openmrsUser);
		
	}

Our use case is we need to verify the old password as well, to make sure that person whoever is changing the password is authorized. Capturing new password is not sufficient enough to make sure that the person is authorized. It can happen that one can login to Bahmni and just leave it open for some time and step out for some work/loosing of device.We thought that from security point of view also it is good to capture old password as well. Any inputs/thoughts on this ? @darius @vinay @bharatak @angshuonline

darius · January 3, 2017, 10:07pm

(@padmavati I have moved this post to the #dev category so that more OpenMRS devs see it.)

I agree that it is a bug that the OpenMRS REST API allows a user to change their password without needing to provide the existing one. The underlying Java API does require this (here) and we should maintain the same in the REST API.

So, the right thing to do is to create a ticket and fix this in the REST module.

Want to do that?

padmavati · January 4, 2017, 12:34am

Hi @darius

Thanks for the reply. Yes I would like to take this up.

darius · January 4, 2017, 12:43am

@padmavati, great! Go ahead and create a ticket on issues.openmrs.org (requesting access from help.openmrs.org if you need it) and share a link here for reference.

pushpa446 · January 5, 2017, 4:02am

We have raised an issue in openmrs. Following is the link

padmavati · January 27, 2017, 9:06am

Pull request to accept the old password has been merged. Can you please release webservices-rest ? Bahmni 88 release is dependent on this change password change.

dkayiwa · January 27, 2017, 1:17pm

@padmavati are you referring to this? https://issues.openmrs.org/browse/RESTWS-634 If yes, the ticket status is In Progress, and with a comment that needs a response.

darius · January 27, 2017, 4:51pm

Actually this was already merged. The ticket was not updated to reflect this, but I have updated it now.

dkayiwa · January 29, 2017, 9:48pm

@padmavati are you doing the remaining work that Darius mentioned on the ticket?

padmavati · January 30, 2017, 3:46am

@dkayiwa Yes, we are doing the remaining work.

dkayiwa · January 30, 2017, 9:39am

@padmavati feel free to ping someone on your team to do the release. Am sure you already have a good number of them with the necessary permissions.

padmavati · January 30, 2017, 9:40am

Thanks daniel. Will ask some one from our team.

darius · January 31, 2017, 5:56am

@dkayiwa, @raff, we want to release this version of the RESTWS module very soon, since it’s on the critical path for us to get our Bahmni release out, and we’re in the last stages of testing.

However the ticket RESTWS-627 has code committed, but it’s in the Design state. Please help us figure out what to do with this ticket.

darius · January 31, 2017, 10:49pm

FYI I poked around about RESTWS-627 and the discussion thread for it is: Module identifiers in RESTWS module