Merge branch 'master' of https://github.com/openmrs/openmrs-core into…

… 4012

api/src/main/java/org/openmrs/User.java

@@ -411,15 +411,20 @@ public void setUsername(String username) {
 	}
 
 	/**
+	 * @deprecated Use LoginCredentials
 	 * @return Returns the secretQuestion.
 	 */
+	@Deprecated
 	public String getSecretQuestion() {
 		return secretQuestion;
 	}
 
 	/**
+	 * @deprecated Use LoginCredentials
 	 * @param secretQuestion The secretQuestion to set.
+	 *
 	 */
+	@Deprecated
 	public void setSecretQuestion(String secretQuestion) {
 		this.secretQuestion = secretQuestion;
 	}

api/src/main/java/org/openmrs/api/db/hibernate/HibernateUserDAO.java

@@ -280,7 +280,7 @@ private void updateUserPassword(String newHashedPassword, String salt, Integer c
 		if (changeForUser == null)
 			throw new DAOException("Couldn't find user to set password for userId=" + userIdToChange);
 		User changedByUser = getUser(changedBy);
-		LoginCredential credentials = new LoginCredential();
+		LoginCredential credentials = getLoginCredential(changeForUser);
 		credentials.setUserId(userIdToChange);
 		credentials.setHashedPassword(newHashedPassword);
 		credentials.setSalt(salt);

api/src/test/java/org/openmrs/api/db/UserDAOTest.java

@@ -2,18 +2,30 @@
 
 import java.util.Date;
 
-import org.junit.Assert;
+import static org.junit.Assert.*;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.openmrs.Person;
 import org.openmrs.PersonName;
 import org.openmrs.User;
+import org.openmrs.api.UserService;
 import org.openmrs.api.context.Context;
+import org.openmrs.api.context.UserContext;
 import org.openmrs.test.BaseContextSensitiveTest;
 import org.openmrs.test.Verifies;
+import org.openmrs.util.Security;
 
 public class UserDAOTest extends BaseContextSensitiveTest {
 
+	public static final String SECRET_QUESTION = "What is the answer?";
+
+	public static final String SECRET_ANSWER = "42";
+
+	public static final String PASSWORD = "Openmr5xy";
+
+	private User userJoe;
+
 	private UserDAO dao = null;
 
 	/**
@@ -24,6 +36,18 @@ public class UserDAOTest extends BaseContextSensitiveTest {
 	 */
 	@Before
 	public void runBeforeEachTest() throws Exception {
+		PersonName name = new PersonName("Joe", "J", "Doe");
+		name.setDateCreated(new Date());
+		Person person = new Person();
+		person.setDateCreated(new Date());
+		person.setPersonDateCreated(person.getDateCreated());
+		person.setGender("M");
+		userJoe = new User();
+		userJoe.setSystemId("100-30");
+		userJoe.setPerson(person);
+		userJoe.addName(name);
+		userJoe.setUsername("juser");
+		userJoe.setDateCreated(new Date());
 
 		if (dao == null)
 			// fetch the dao from the spring application context
@@ -56,13 +80,98 @@ public void getUsers_shouldEscapeSqlWildcardsInSearchPhrase() throws Exception {
 
 			//we expect only one matching name or or systemId  to be returned
 			int size = dao.getUsers(wildcard + "ca", null, false, null, null).size();
-			Assert.assertEquals(1, size);
+			assertEquals(1, size);
 
 			//if actually the search returned the matching name or system id
 			String userName = (dao.getUsers(wildcard + "ca", null, false, null, null).get(0).getUsername());
-			Assert.assertEquals("Test failed since no user containing the character " + wildcard + " was found, ", wildcard
+			assertEquals("Test failed since no user containing the character " + wildcard + " was found, ", wildcard
 			        + "test" + wildcard, userName);
 
 		}
 	}
+
+	@Test
+	@Verifies(value = "creates a new user", method = "saveUser(u, pwd)")
+	public void saveUser_shouldCreateNewUser() throws Exception {
+		dao.saveUser(userJoe, "Openmr5xy");
+		User u2 = dao.getUser(userJoe.getId());
+		assertNotNull("User should have been returned", u2);
+	}
+
+	@Test
+	@Verifies(value = "should not overwrite user secret question or answer on password change", method = "changePassword(User, String)")
+	public void updateUserPassword_shouldNotOverwriteUserSecretQuestionOrAnswer() throws Exception {
+		dao.saveUser(userJoe, PASSWORD);
+		dao.changeQuestionAnswer(userJoe, SECRET_QUESTION, SECRET_ANSWER);
+		LoginCredential lc = dao.getLoginCredential(userJoe);
+		assertEquals("question should be set", SECRET_QUESTION, lc.getSecretQuestion());
+		assertEquals("answer should be set", SECRET_ANSWER, lc.getSecretAnswer());
+		dao.changePassword(userJoe, "Openmr6zz");
+		lc = dao.getLoginCredential(userJoe);
+		assertEquals("question should not have changed", SECRET_QUESTION, lc.getSecretQuestion());
+		assertEquals("answer should not have changed", SECRET_ANSWER, lc.getSecretAnswer());
+	}
+
+	@Test
+	@Verifies(value = "should not overwrite user secret question or answer when saving existing user", method = "saveUser(User, String)")
+	public void saveUser_shouldNotOverwriteUserSecretQuestionOrAnswer() throws Exception {
+		dao.saveUser(userJoe, PASSWORD);
+		dao.changeQuestionAnswer(userJoe, SECRET_QUESTION, SECRET_ANSWER);
+		LoginCredential lc = dao.getLoginCredential(userJoe);
+		assertEquals("question should be set", SECRET_QUESTION, lc.getSecretQuestion());
+		assertEquals("answer should be set", SECRET_ANSWER, lc.getSecretAnswer());
+		userJoe.setUserProperty("foo", "bar");
+		dao.saveUser(userJoe, PASSWORD);
+		lc = dao.getLoginCredential(userJoe);
+		assertEquals("question should not have changed", SECRET_QUESTION, lc.getSecretQuestion());
+		assertEquals("answer should not have changed", SECRET_ANSWER, lc.getSecretAnswer());
+	}
+
+	@Test
+	@Verifies(value = "should not overwrite user secret question or answer when changing password", method = "changePassword(String, String)")
+	public void changePassword_shouldNotOverwriteUserSecretQuestionOrAnswer() throws Exception {
+		dao.saveUser(userJoe, PASSWORD);
+		dao.changeQuestionAnswer(userJoe, SECRET_QUESTION, SECRET_ANSWER);
+		LoginCredential lc = dao.getLoginCredential(userJoe);
+		assertEquals("question should be set", SECRET_QUESTION, lc.getSecretQuestion());
+		assertEquals("answer should be set", SECRET_ANSWER, lc.getSecretAnswer());
+		Context.authenticate(userJoe.getUsername(), PASSWORD);
+		dao.changePassword(PASSWORD, PASSWORD + "foo");
+		lc = dao.getLoginCredential(userJoe);
+		assertEquals("question should not have changed", SECRET_QUESTION, lc.getSecretQuestion());
+		assertEquals("answer should not have changed", SECRET_ANSWER, lc.getSecretAnswer());
+	}
+
+	@Test
+	@Verifies(value = "should not overwrite user secret question or answer when saving hashed password", method = "changeHashedPassword(User, String, String)")
+	public void changeHashedPassword_shouldNotOverwriteUserSecretQuestionOrAnswer() throws Exception {
+		dao.saveUser(userJoe, PASSWORD);
+		dao.changeQuestionAnswer(userJoe, SECRET_QUESTION, SECRET_ANSWER);
+		LoginCredential lc = dao.getLoginCredential(userJoe);
+		assertEquals("question should be set", SECRET_QUESTION, lc.getSecretQuestion());
+		assertEquals("answer should be set", SECRET_ANSWER, lc.getSecretAnswer());
+		userJoe.setUserProperty("foo", "bar");
+		dao.changeHashedPassword(userJoe, "VakesJkw1", Security.getRandomToken());
+		lc = dao.getLoginCredential(userJoe);
+		assertEquals("question should not have changed", SECRET_QUESTION, lc.getSecretQuestion());
+		assertEquals("answer should not have changed", SECRET_ANSWER, lc.getSecretAnswer());
+	}
+
+	@Test
+	@Verifies(value = "should return true when supplied secret answer matches", method = "isSecretAnswer(User, String)")
+	public void isSecretAnswer_shouldReturnTrueWhenTheAnswerMatches() {
+		dao.saveUser(userJoe, PASSWORD);
+		dao.changeQuestionAnswer(userJoe, SECRET_QUESTION, SECRET_ANSWER);
+		assertTrue(dao.isSecretAnswer(userJoe, SECRET_ANSWER));
+	}
+
+	@Test
+	@Verifies(value = "should return false when supplied secret answer does not match", method = "isSecretAnswer(User, String)")
+	public void isSecretAnswer_shouldReturnFalseWhenTheAnswerDoesNotMatch() {
+		dao.saveUser(userJoe, PASSWORD);
+		dao.changeQuestionAnswer(userJoe, SECRET_QUESTION, SECRET_ANSWER);
+		assertFalse(dao.isSecretAnswer(userJoe, "foo"));
+
+	}
+
 }

web/src/test/java/org/openmrs/web/test/WebTestHelper.java

@@ -114,10 +114,9 @@ public Response handle(final HttpServletRequest request) throws Exception {
 
 		HandlerExecutionChain handlerChain = null;
 		for (HandlerMapping handlerMapping : handlerMappings) {
-			if (handlerChain == null) {
-				handlerChain = handlerMapping.getHandler(request);
-			} else {
-				Assert.fail("The requested URI has more than one mapping: " + request.getRequestURI());
+			handlerChain = handlerMapping.getHandler(request);
+			if (handlerChain != null) {
+				break;
 			}
 		}
 		Assert.assertNotNull("The requested URI has no mapping: " + request.getRequestURI(), handlerChain);