Move password logic to manager

This commit is contained in:
René Pfeuffer
2018-10-17 11:58:37 +02:00
parent 24b1241ed7
commit 9bfb2cdadb
24 changed files with 285 additions and 218 deletions

View File

@@ -13,6 +13,7 @@ import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import sonia.scm.user.InvalidPasswordException;
import sonia.scm.user.User;
import sonia.scm.user.UserManager;
import sonia.scm.web.VndMediaType;
@@ -27,6 +28,7 @@ import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;
@@ -97,6 +99,7 @@ public class MeResourceTest {
public void shouldEncryptPasswordBeforeChanging() throws Exception {
String newPassword = "pwd123";
String encryptedNewPassword = "encrypted123";
String encryptedOldPassword = "encryptedOld";
String oldPassword = "secret";
String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword);
MockHttpRequest request = MockHttpRequest
@@ -104,33 +107,32 @@ public class MeResourceTest {
.contentType(VndMediaType.PASSWORD_CHANGE)
.content(content.getBytes());
MockHttpResponse response = new MockHttpResponse();
when(passwordService.encryptPassword(newPassword)).thenReturn(encryptedNewPassword);
when(passwordService.encryptPassword(oldPassword)).thenReturn("secret");
ArgumentCaptor<User> modifyUserCaptor = ArgumentCaptor.forClass(User.class);
doNothing().when(userManager).modify(modifyUserCaptor.capture(), any());
when(passwordService.encryptPassword(oldPassword)).thenReturn(encryptedOldPassword);
ArgumentCaptor<String> encryptedOldPasswordCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<String> encryptedNewPasswordCaptor = ArgumentCaptor.forClass(String.class);
doNothing().when(userManager).changePasswordForLoggedInUser(encryptedOldPasswordCaptor.capture(), encryptedNewPasswordCaptor.capture());
dispatcher.invoke(request, response);
assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus());
verify(userManager).modify(any(), any());
User updatedUser = modifyUserCaptor.getValue();
assertEquals(encryptedNewPassword, updatedUser.getPassword());
assertEquals(encryptedNewPassword, encryptedNewPasswordCaptor.getValue());
assertEquals(encryptedOldPassword, encryptedOldPasswordCaptor.getValue());
}
@Test
@SubjectAware(username = "trillian", password = "secret")
public void shouldGet400OnChangePasswordOfUserWithNonDefaultType() throws Exception {
public void shouldGet400OnMissingOldPassword() throws Exception {
originalUser.setType("not an xml type");
String newPassword = "pwd123";
String oldPassword = "secret";
String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword);
String content = String.format("{ \"newPassword\": \"%s\" }", newPassword);
MockHttpRequest request = MockHttpRequest
.put("/" + MeResource.ME_PATH_V2 + "password")
.contentType(VndMediaType.PASSWORD_CHANGE)
.content(content.getBytes());
MockHttpResponse response = new MockHttpResponse();
when(passwordService.encryptPassword(eq(newPassword))).thenReturn("encrypted123");
when(passwordService.encryptPassword(eq(oldPassword))).thenReturn("secret");
dispatcher.invoke(request, response);
@@ -139,17 +141,34 @@ public class MeResourceTest {
@Test
@SubjectAware(username = "trillian", password = "secret")
public void shouldGet400OnChangePasswordIfOldPasswordDoesNotMatchOriginalPassword() throws Exception {
public void shouldGet400OnMissingEmptyPassword() throws Exception {
String newPassword = "pwd123";
String oldPassword = "notEncriptedSecret";
String oldPassword = "";
String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword);
MockHttpRequest request = MockHttpRequest
.put("/" + MeResource.ME_PATH_V2 + "password")
.contentType(VndMediaType.PASSWORD_CHANGE)
.content(content.getBytes());
MockHttpResponse response = new MockHttpResponse();
when(passwordService.encryptPassword(newPassword)).thenReturn("encrypted123");
when(passwordService.encryptPassword(eq(oldPassword))).thenReturn("differentThanSecret");
dispatcher.invoke(request, response);
assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus());
}
@Test
@SubjectAware(username = "trillian", password = "secret")
public void shouldMapExceptionFromManager() throws Exception {
String newPassword = "pwd123";
String oldPassword = "secret";
String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword);
MockHttpRequest request = MockHttpRequest
.put("/" + MeResource.ME_PATH_V2 + "password")
.contentType(VndMediaType.PASSWORD_CHANGE)
.content(content.getBytes());
MockHttpResponse response = new MockHttpResponse();
doThrow(InvalidPasswordException.class).when(userManager).changePasswordForLoggedInUser(any(), any());
dispatcher.invoke(request, response);

View File

@@ -14,7 +14,9 @@ import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import sonia.scm.NotFoundException;
import sonia.scm.PageResult;
import sonia.scm.user.ChangePasswordNotAllowedException;
import sonia.scm.user.User;
import sonia.scm.user.UserManager;
import sonia.scm.web.VndMediaType;
@@ -31,6 +33,7 @@ import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -142,7 +145,7 @@ public class UserRootResourceTest {
String content = String.format("{\"newPassword\": \"%s\"}", newPassword);
MockHttpRequest request = MockHttpRequest
.put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password")
.contentType(VndMediaType.PASSWORD_CHANGE)
.contentType(VndMediaType.PASSWORD_OVERWRITE)
.content(content.getBytes());
MockHttpResponse response = new MockHttpResponse();
when(passwordService.encryptPassword(newPassword)).thenReturn("encrypted123");
@@ -150,26 +153,61 @@ public class UserRootResourceTest {
dispatcher.invoke(request, response);
assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus());
verify(userManager).modify(any(), any());
User updatedUser = userCaptor.getValue();
assertEquals("encrypted123", updatedUser.getPassword());
verify(userManager).overwritePassword("Neo", "encrypted123");
}
@Test
public void shouldGet400OnChangePasswordOfUserWithNonDefaultType() throws Exception {
public void shouldGet400OnOverwritePasswordWhenManagerThrowsNotAllowed() throws Exception {
originalUser.setType("not an xml type");
String newPassword = "pwd123";
String content = String.format("{\"newPassword\": \"%s\"}", newPassword);
MockHttpRequest request = MockHttpRequest
.put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password")
.contentType(VndMediaType.PASSWORD_CHANGE)
.contentType(VndMediaType.PASSWORD_OVERWRITE)
.content(content.getBytes());
MockHttpResponse response = new MockHttpResponse();
doThrow(ChangePasswordNotAllowedException.class).when(userManager).overwritePassword(any(), any());
dispatcher.invoke(request, response);
assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus());
}
@Test
public void shouldGet404OnOverwritePasswordWhenNotFound() throws Exception {
originalUser.setType("not an xml type");
String newPassword = "pwd123";
String content = String.format("{\"newPassword\": \"%s\"}", newPassword);
MockHttpRequest request = MockHttpRequest
.put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password")
.contentType(VndMediaType.PASSWORD_OVERWRITE)
.content(content.getBytes());
MockHttpResponse response = new MockHttpResponse();
doThrow(NotFoundException.class).when(userManager).overwritePassword(any(), any());
dispatcher.invoke(request, response);
assertEquals(HttpServletResponse.SC_NOT_FOUND, response.getStatus());
}
@Test
public void shouldEncryptPasswordOnOverwritePassword() throws Exception {
originalUser.setType("not an xml type");
String newPassword = "pwd123";
String content = String.format("{\"newPassword\": \"%s\"}", newPassword);
MockHttpRequest request = MockHttpRequest
.put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password")
.contentType(VndMediaType.PASSWORD_OVERWRITE)
.content(content.getBytes());
MockHttpResponse response = new MockHttpResponse();
when(passwordService.encryptPassword(newPassword)).thenReturn("encrypted123");
dispatcher.invoke(request, response);
assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus());
assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus());
verify(userManager).overwritePassword("Neo", "encrypted123");
}
@Test

View File

@@ -65,7 +65,7 @@ public class UserToUserDtoMapperTest {
}
@Test
public void shouldGetPasswordLinkOnlyForDefaultUserType() {
public void shouldGetPasswordLinkForAdmin() {
User user = createDefaultUser();
when(subject.isPermitted("user:modify:abc")).thenReturn(true);
when(userManager.isTypeDefault(eq(user))).thenReturn(true);
@@ -73,14 +73,15 @@ public class UserToUserDtoMapperTest {
UserDto userDto = mapper.map(user);
assertEquals("expected password link with modify permission", expectedBaseUri.resolve("abc/password").toString(), userDto.getLinks().getLinkBy("password").get().getHref());
}
when(subject.isPermitted("user:modify:abc")).thenReturn(false);
userDto = mapper.map(user);
assertEquals("expected password link on mission modify permission", expectedBaseUri.resolve("abc/password").toString(), userDto.getLinks().getLinkBy("password").get().getHref());
@Test
public void shouldGetPasswordLinkOnlyForDefaultUserType() {
User user = createDefaultUser();
when(subject.isPermitted("user:modify:abc")).thenReturn(true);
when(userManager.isTypeDefault(eq(user))).thenReturn(false);
userDto = mapper.map(user);
UserDto userDto = mapper.map(user);
assertFalse("expected no password link", userDto.getLinks().getLinkBy("password").isPresent());
}

View File

@@ -159,7 +159,7 @@ public class DefaultAuthorizationCollectorTest {
AuthorizationInfo authInfo = collector.collect();
assertThat(authInfo.getRoles(), Matchers.contains(Role.USER));
assertThat(authInfo.getStringPermissions(), hasSize(4));
assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:autocomplete", "group:autocomplete", "user:changeOwnPassword", "user:read:trillian"));
assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:autocomplete", "group:autocomplete", "user:changePassword:trillian", "user:read:trillian"));
assertThat(authInfo.getObjectPermissions(), nullValue());
}
@@ -204,7 +204,7 @@ public class DefaultAuthorizationCollectorTest {
AuthorizationInfo authInfo = collector.collect();
assertThat(authInfo.getRoles(), Matchers.containsInAnyOrder(Role.USER));
assertThat(authInfo.getObjectPermissions(), nullValue());
assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:autocomplete", "group:autocomplete", "user:changeOwnPassword", "repository:read,pull:one", "repository:read,pull,push:two", "user:read:trillian"));
assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:autocomplete", "group:autocomplete", "user:changePassword:trillian", "repository:read,pull:one", "repository:read,pull,push:two", "user:read:trillian"));
}
/**
@@ -225,7 +225,7 @@ public class DefaultAuthorizationCollectorTest {
AuthorizationInfo authInfo = collector.collect();
assertThat(authInfo.getRoles(), Matchers.containsInAnyOrder(Role.USER));
assertThat(authInfo.getObjectPermissions(), nullValue());
assertThat(authInfo.getStringPermissions(), containsInAnyOrder("one:one", "two:two", "user:read:trillian", "user:autocomplete", "group:autocomplete", "user:changeOwnPassword"));
assertThat(authInfo.getStringPermissions(), containsInAnyOrder("one:one", "two:two", "user:read:trillian", "user:autocomplete", "group:autocomplete", "user:changePassword:trillian"));
}
private void authenticate(User user, String group, String... groups) {

View File

@@ -39,9 +39,13 @@ import com.github.sdorra.shiro.ShiroRule;
import com.github.sdorra.shiro.SubjectAware;
import com.google.common.collect.Lists;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import sonia.scm.NotFoundException;
import sonia.scm.store.JAXBConfigurationStoreFactory;
import sonia.scm.user.xml.XmlUserDAO;
import sonia.scm.util.MockUtil;
@@ -70,6 +74,10 @@ public class DefaultUserManagerTest extends UserManagerTestBase
@Rule
public ShiroRule shiro = new ShiroRule();
private UserDAO userDAO = mock(UserDAO.class);
private User trillian;
/**
* Method description
*
@@ -82,6 +90,16 @@ public class DefaultUserManagerTest extends UserManagerTestBase
return new DefaultUserManager(createXmlUserDAO());
}
@Before
public void initDao() {
trillian = UserTestData.createTrillian();
trillian.setPassword("oldEncrypted");
userDAO = mock(UserDAO.class);
when(userDAO.getType()).thenReturn("xml");
when(userDAO.get("trillian")).thenReturn(trillian);
}
/**
* Method description
*
@@ -89,7 +107,6 @@ public class DefaultUserManagerTest extends UserManagerTestBase
@Test
public void testDefaultAccountAfterFristStart()
{
UserDAO userDAO = mock(UserDAO.class);
List<User> users = Lists.newArrayList(new User("tuser"));
when(userDAO.getAll()).thenReturn(users);
@@ -108,8 +125,6 @@ public class DefaultUserManagerTest extends UserManagerTestBase
@SuppressWarnings("unchecked")
public void testDefaultAccountCreation()
{
UserDAO userDAO = mock(UserDAO.class);
when(userDAO.getAll()).thenReturn(Collections.EMPTY_LIST);
UserManager userManager = new DefaultUserManager(userDAO);
@@ -118,6 +133,55 @@ public class DefaultUserManagerTest extends UserManagerTestBase
verify(userDAO, times(2)).add(any(User.class));
}
@Test(expected = InvalidPasswordException.class)
public void shouldFailChangePasswordForWrongOldPassword() {
UserManager userManager = new DefaultUserManager(userDAO);
userManager.changePasswordForLoggedInUser("wrongPassword", "---");
}
@Test
public void shouldSucceedChangePassword() {
ArgumentCaptor<User> userCaptor = ArgumentCaptor.forClass(User.class);
doNothing().when(userDAO).modify(userCaptor.capture());
UserManager userManager = new DefaultUserManager(userDAO);
userManager.changePasswordForLoggedInUser("oldEncrypted", "newEncrypted");
Assertions.assertThat(userCaptor.getValue().getPassword()).isEqualTo("newEncrypted");
}
@Test(expected = ChangePasswordNotAllowedException.class)
public void shouldFailOverwritePasswordForWrongType() {
trillian.setType("wrongType");
UserManager userManager = new DefaultUserManager(userDAO);
userManager.overwritePassword("trillian", "---");
}
@Test(expected = NotFoundException.class)
public void shouldFailOverwritePasswordForMissingUser() {
UserManager userManager = new DefaultUserManager(userDAO);
userManager.overwritePassword("notExisting", "---");
}
@Test
public void shouldSucceedOverwritePassword() {
ArgumentCaptor<User> userCaptor = ArgumentCaptor.forClass(User.class);
doNothing().when(userDAO).modify(userCaptor.capture());
UserManager userManager = new DefaultUserManager(userDAO);
userManager.overwritePassword("trillian", "newEncrypted");
Assertions.assertThat(userCaptor.getValue().getPassword()).isEqualTo("newEncrypted");
}
//~--- methods --------------------------------------------------------------
private XmlUserDAO createXmlUserDAO() {