diff --git a/api/src/main/java/org/openmrs/User.java b/api/src/main/java/org/openmrs/User.java index 8a38d47da230..58ab2e9727ee 100644 --- a/api/src/main/java/org/openmrs/User.java +++ b/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; } diff --git a/api/src/main/java/org/openmrs/api/db/hibernate/HibernateUserDAO.java b/api/src/main/java/org/openmrs/api/db/hibernate/HibernateUserDAO.java index 177d6f705b58..ef2fe78d4065 100644 --- a/api/src/main/java/org/openmrs/api/db/hibernate/HibernateUserDAO.java +++ b/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); diff --git a/api/src/test/java/org/openmrs/api/db/UserDAOTest.java b/api/src/test/java/org/openmrs/api/db/UserDAOTest.java index 33916740895c..ac55e2b60a63 100644 --- a/api/src/test/java/org/openmrs/api/db/UserDAOTest.java +++ b/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")); + + } + } diff --git a/web/src/test/java/org/openmrs/web/test/WebTestHelper.java b/web/src/test/java/org/openmrs/web/test/WebTestHelper.java index 13593f42d3ca..738ec3b57efe 100644 --- a/web/src/test/java/org/openmrs/web/test/WebTestHelper.java +++ b/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);