Skip to content

Commit

Permalink
Merge pull request openmrs#347 from michaelpigg/TRUNK-3877
Browse files Browse the repository at this point in the history 
TRUNK-3877: Changing login password deletes secret answer
  • Loading branch information
@dkayiwa
dkayiwa committed Aug 1, 2013
2 parents d2c2423 + e423b39 commit 3c3b010
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 4 deletions.
3 changes: 3 additions & 0 deletions api/src/main/java/org/openmrs/User.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,14 +411,17 @@ public void setUsername(String username) {
}

/**
* @deprecated Use LoginCredentials
* @return Returns the secretQuestion.
*/
public String getSecretQuestion() {
return secretQuestion;
}

/**
* @deprecated Use LoginCredentials
* @param secretQuestion The secretQuestion to set.
*
*/
public void setSecretQuestion(String secretQuestion) {
this.secretQuestion = secretQuestion;
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/org/openmrs/api/db/hibernate/HibernateUserDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
115 changes: 112 additions & 3 deletions api/src/test/java/org/openmrs/api/db/UserDAOTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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
Expand Down Expand Up @@ -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"));

}

}

0 comments on commit 3c3b010

Please sign in to comment.