From 43e1b9568320da3814c58345584fc0e4b06b87f7 Mon Sep 17 00:00:00 2001 From: open-schnick Date: Fri, 20 Nov 2020 15:24:33 +0100 Subject: [PATCH 1/8] fixed tests, added shallow logic. --- .../business/AccessTokenBusinessService.java | 6 ++--- .../business/UserAuthorizationService.java | 15 ++++++++--- .../user/business/UserBusinessService.java | 4 +++ .../domain/user/rest/UserRestController.java | 9 ++++--- .../domain/user/rest/UserRestService.java | 27 +++++++++---------- .../user/rest/UserRestServiceInterface.java | 2 +- .../AccessTokenBusinessServiceUnitTest.java | 12 ++++----- .../UserAuthorizationServiceUnitTest.java | 15 ++++++----- .../user/rest/UserRestControllerUnitTest.java | 9 +++---- .../resources/UserEditInformation.feature | 1 + 10 files changed, 58 insertions(+), 42 deletions(-) diff --git a/src/main/java/de/filefighter/rest/domain/token/business/AccessTokenBusinessService.java b/src/main/java/de/filefighter/rest/domain/token/business/AccessTokenBusinessService.java index e64a07d2..3d53a940 100644 --- a/src/main/java/de/filefighter/rest/domain/token/business/AccessTokenBusinessService.java +++ b/src/main/java/de/filefighter/rest/domain/token/business/AccessTokenBusinessService.java @@ -37,7 +37,7 @@ public AccessToken getValidAccessTokenForUser(User user) { accessTokenEntity = AccessTokenEntity .builder() .validUntil(currentTimeSeconds + ACCESS_TOKEN_DURATION_IN_SECONDS) - .value(this.generateRandomTokenValue()) + .value(generateRandomTokenValue()) .userId(user.getId()) .build(); accessTokenEntity = accessTokenRepository.save(accessTokenEntity); @@ -47,7 +47,7 @@ public AccessToken getValidAccessTokenForUser(User user) { accessTokenEntity = AccessTokenEntity .builder() .validUntil(currentTimeSeconds + ACCESS_TOKEN_DURATION_IN_SECONDS) - .value(this.generateRandomTokenValue()) + .value(generateRandomTokenValue()) .userId(user.getId()) .build(); accessTokenEntity = accessTokenRepository.save(accessTokenEntity); @@ -80,7 +80,7 @@ public AccessToken findAccessTokenByValue(String accessTokenValue) { } - public AccessToken validateAccessTokenValue(String accessTokenValue) { + public AccessToken validateAccessTokenValueWithHeader(String accessTokenValue) { String cleanValue = validateAuthorizationHeader(AUTHORIZATION_BEARER_PREFIX, accessTokenValue); AccessTokenEntity accessTokenEntity = accessTokenRepository.findByValue(cleanValue); if (null == accessTokenEntity) diff --git a/src/main/java/de/filefighter/rest/domain/user/business/UserAuthorizationService.java b/src/main/java/de/filefighter/rest/domain/user/business/UserAuthorizationService.java index bf6e20fe..b9d0bee9 100644 --- a/src/main/java/de/filefighter/rest/domain/user/business/UserAuthorizationService.java +++ b/src/main/java/de/filefighter/rest/domain/user/business/UserAuthorizationService.java @@ -1,5 +1,6 @@ package de.filefighter.rest.domain.user.business; +import de.filefighter.rest.domain.common.Utils; import de.filefighter.rest.domain.token.data.dto.AccessToken; import de.filefighter.rest.domain.user.data.dto.User; import de.filefighter.rest.domain.user.data.persistance.UserEntity; @@ -15,6 +16,9 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; +import static de.filefighter.rest.configuration.RestConfiguration.AUTHORIZATION_BASIC_PREFIX; +import static de.filefighter.rest.configuration.RestConfiguration.AUTHORIZATION_BEARER_PREFIX; + @Service public class UserAuthorizationService { @@ -28,7 +32,9 @@ public UserAuthorizationService(UserRepository userRepository, UserDtoService us this.userDtoService = userDtoService; } - public User authenticateUserWithUsernameAndPassword(String base64encodedUserAndPassword) { + public User authenticateUserWithUsernameAndPassword(String base64encodedUserAndPasswordWithHeader) { + String base64encodedUserAndPassword = Utils.validateAuthorizationHeader(AUTHORIZATION_BASIC_PREFIX, base64encodedUserAndPasswordWithHeader); + String decodedUsernameAndPassword = ""; try { byte[] decodedValue = Base64.getDecoder().decode(base64encodedUserAndPassword); @@ -54,17 +60,20 @@ public User authenticateUserWithUsernameAndPassword(String base64encodedUserAndP } public User authenticateUserWithRefreshToken(String refreshToken) { - UserEntity userEntity = userRepository.findByRefreshToken(refreshToken); + String cleanValue = Utils.validateAuthorizationHeader(AUTHORIZATION_BEARER_PREFIX, refreshToken); + UserEntity userEntity = userRepository.findByRefreshToken(cleanValue); if (null == userEntity) throw new UserNotAuthenticatedException("No user found for this Refresh Token."); return userDtoService.createDto(userEntity); } - public void authenticateUserWithAccessToken(AccessToken accessToken) { + public User authenticateUserWithAccessToken(AccessToken accessToken) { UserEntity userEntity = userRepository.findByUserId(accessToken.getUserId()); if (null == userEntity) throw new UserNotAuthenticatedException(accessToken.getUserId()); + + return userDtoService.createDto(userEntity); } public void authenticateUserWithAccessTokenAndGroup(AccessToken accessToken, Groups groups) { diff --git a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java index 34a17c80..69d6c771 100644 --- a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java +++ b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java @@ -140,4 +140,8 @@ public void passwordIsValid(String password) { if (!pattern.matcher(password).matches()) throw new UserNotRegisteredException("Password needs to be at least 8 characters long and, contains at least one uppercase and lowercase letter and a number."); } + + public String updateUser(long userId, UserRegisterForm updatedUser, User authenticatedUser) { + return null; + } } diff --git a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestController.java b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestController.java index 85998f47..0b5c9ea2 100644 --- a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestController.java +++ b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestController.java @@ -64,13 +64,14 @@ public ResponseEntity getUserInfo( return userRestService.getUserByUserIdAuthenticateWithAccessToken(accessToken, userId); } - @PutMapping(USER_BASE_URI + "edit") - public ResponseEntity updateUser( + @PutMapping(USER_BASE_URI + "{userId}/edit") + public ResponseEntity updateUser( + @PathVariable long userId, @RequestHeader(value = "Authorization", defaultValue = AUTHORIZATION_BEARER_PREFIX + "token") String accessToken, @RequestBody UserRegisterForm updatedUser) { LOG.info("Updated User and Token {}, with form {}.", accessToken, updatedUser); - return userRestService.updateUserWithAccessToken(updatedUser, accessToken); + return userRestService.updateUserByUserIdAuthenticateWithAccessToken(updatedUser, userId, accessToken); } @GetMapping(USER_BASE_URI + "find") @@ -78,7 +79,7 @@ public ResponseEntity findUserByUsername( @RequestHeader(value = "Authorization", defaultValue = AUTHORIZATION_BEARER_PREFIX + "token") String accessToken, @RequestParam(name = "username", value = "username") String username ) { - LOG.info("Requested finding User with the username {} and Token {}", username, accessToken); + LOG.info("Requested finding User with the username {} and Token {}", username, accessToken); return userRestService.findUserByUsernameAndAccessToken(username, accessToken); } } diff --git a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestService.java b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestService.java index 99075988..fe35d1a4 100644 --- a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestService.java +++ b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestService.java @@ -1,6 +1,5 @@ package de.filefighter.rest.domain.user.rest; -import de.filefighter.rest.domain.common.Utils; import de.filefighter.rest.domain.token.business.AccessTokenBusinessService; import de.filefighter.rest.domain.token.data.dto.AccessToken; import de.filefighter.rest.domain.token.data.dto.RefreshToken; @@ -13,8 +12,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; -import static de.filefighter.rest.configuration.RestConfiguration.AUTHORIZATION_BASIC_PREFIX; -import static de.filefighter.rest.configuration.RestConfiguration.AUTHORIZATION_BEARER_PREFIX; import static de.filefighter.rest.domain.user.group.Groups.ADMIN; @@ -33,36 +30,38 @@ public UserRestService(UserBusinessService userBusinessService, UserAuthorizatio @Override public ResponseEntity getUserByUserIdAuthenticateWithAccessToken(String accessToken, long userId) { - AccessToken validAccessToken = accessTokenBusinessService.validateAccessTokenValue(accessToken); + AccessToken validAccessToken = accessTokenBusinessService.validateAccessTokenValueWithHeader(accessToken); userAuthorizationService.authenticateUserWithAccessToken(validAccessToken); User user = userBusinessService.getUserById(userId); return new ResponseEntity<>(user, HttpStatus.OK); } @Override - public ResponseEntity getRefreshTokenWithUsernameAndPassword(String base64encodedUserAndPassword) { - String cleanValue = Utils.validateAuthorizationHeader(AUTHORIZATION_BASIC_PREFIX, base64encodedUserAndPassword); - User authenticatedUser = userAuthorizationService.authenticateUserWithUsernameAndPassword(cleanValue); + public ResponseEntity getRefreshTokenWithUsernameAndPassword(String base64encodedUserAndPasswordWithHeader) { + User authenticatedUser = userAuthorizationService.authenticateUserWithUsernameAndPassword(base64encodedUserAndPasswordWithHeader); RefreshToken refreshToken = userBusinessService.getRefreshTokenForUser(authenticatedUser); return new ResponseEntity<>(refreshToken, HttpStatus.OK); } @Override - public ResponseEntity getAccessTokenByRefreshToken(String refreshToken) { - String cleanValue = Utils.validateAuthorizationHeader(AUTHORIZATION_BEARER_PREFIX, refreshToken); - User user = userAuthorizationService.authenticateUserWithRefreshToken(cleanValue); + public ResponseEntity getAccessTokenByRefreshToken(String refreshTokenWithHeader) { + User user = userAuthorizationService.authenticateUserWithRefreshToken(refreshTokenWithHeader); AccessToken accessToken = accessTokenBusinessService.getValidAccessTokenForUser(user); return new ResponseEntity<>(accessToken, HttpStatus.OK); } @Override - public ResponseEntity updateUserWithAccessToken(UserRegisterForm updatedUser, String accessToken) { - return null; + public ResponseEntity updateUserByUserIdAuthenticateWithAccessToken(UserRegisterForm updatedUser, long userId, String accessTokenValue) { + AccessToken accessToken = accessTokenBusinessService.validateAccessTokenValueWithHeader(accessTokenValue); + User authenticatedUser = userAuthorizationService.authenticateUserWithAccessToken(accessToken); + String message = userBusinessService.updateUser(userId, updatedUser, authenticatedUser); + ServerResponse response = new ServerResponse(HttpStatus.CREATED, message); + return new ResponseEntity<>(response, HttpStatus.CREATED); } @Override public ResponseEntity registerNewUserWithAccessToken(UserRegisterForm newUser, String accessToken) { - AccessToken validAccessToken = accessTokenBusinessService.validateAccessTokenValue(accessToken); + AccessToken validAccessToken = accessTokenBusinessService.validateAccessTokenValueWithHeader(accessToken); userAuthorizationService.authenticateUserWithAccessTokenAndGroup(validAccessToken, ADMIN); userBusinessService.registerNewUser(newUser); return new ResponseEntity<>(new ServerResponse(HttpStatus.CREATED, "User successfully created."), HttpStatus.CREATED); @@ -70,7 +69,7 @@ public ResponseEntity registerNewUserWithAccessToken(UserRegiste @Override public ResponseEntity findUserByUsernameAndAccessToken(String username, String accessToken) { - AccessToken token = accessTokenBusinessService.validateAccessTokenValue(accessToken); + AccessToken token = accessTokenBusinessService.validateAccessTokenValueWithHeader(accessToken); userAuthorizationService.authenticateUserWithAccessToken(token); User foundUser = userBusinessService.findUserByUsername(username); return new ResponseEntity<>(foundUser, HttpStatus.OK); diff --git a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestServiceInterface.java b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestServiceInterface.java index ed121e17..b653caa4 100644 --- a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestServiceInterface.java +++ b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestServiceInterface.java @@ -11,7 +11,7 @@ public interface UserRestServiceInterface { ResponseEntity getUserByUserIdAuthenticateWithAccessToken(String accessToken, long userId); ResponseEntity getRefreshTokenWithUsernameAndPassword(String base64encodedUserAndPassword); ResponseEntity getAccessTokenByRefreshToken(String refreshToken); - ResponseEntity updateUserWithAccessToken(UserRegisterForm updatedUser, String accessToken); + ResponseEntity updateUserByUserIdAuthenticateWithAccessToken(UserRegisterForm updatedUser, long userId, String accessToken); ResponseEntity registerNewUserWithAccessToken(UserRegisterForm newUser, String accessToken); ResponseEntity findUserByUsernameAndAccessToken(String username, String accessToken); } diff --git a/src/test/java/de/filefighter/rest/domain/token/business/AccessTokenBusinessServiceUnitTest.java b/src/test/java/de/filefighter/rest/domain/token/business/AccessTokenBusinessServiceUnitTest.java index 398254ec..af098330 100644 --- a/src/test/java/de/filefighter/rest/domain/token/business/AccessTokenBusinessServiceUnitTest.java +++ b/src/test/java/de/filefighter/rest/domain/token/business/AccessTokenBusinessServiceUnitTest.java @@ -152,7 +152,7 @@ void findAccessTokenByValueSuccessfully() { @Test void generateRandomTokenValue() { - String generatedToken = accessTokenBusinessService.generateRandomTokenValue(); + String generatedToken = AccessTokenBusinessService.generateRandomTokenValue(); assertEquals(36, generatedToken.length()); } @@ -162,13 +162,13 @@ void validateAccessTokenValueWithWrongHeader() { String header1 = ""; assertThrows(RequestDidntMeetFormalRequirementsException.class, () -> - accessTokenBusinessService.validateAccessTokenValue(header0) + accessTokenBusinessService.validateAccessTokenValueWithHeader(header0) ); assertThrows(RequestDidntMeetFormalRequirementsException.class, () -> - accessTokenBusinessService.validateAccessTokenValue(header1) + accessTokenBusinessService.validateAccessTokenValueWithHeader(header1) ); assertThrows(RequestDidntMeetFormalRequirementsException.class, () -> - accessTokenBusinessService.validateAccessTokenValue(AUTHORIZATION_BEARER_PREFIX) + accessTokenBusinessService.validateAccessTokenValueWithHeader(AUTHORIZATION_BEARER_PREFIX) ); } @@ -179,7 +179,7 @@ void validateAccessTokenValueButTokenDoesNotExist() { when(accessTokenRepositoryMock.findByValue("something")).thenReturn(null); assertThrows(UserNotAuthenticatedException.class, () -> - accessTokenBusinessService.validateAccessTokenValue(header) + accessTokenBusinessService.validateAccessTokenValueWithHeader(header) ); } @@ -192,7 +192,7 @@ void validateAccessTokenValue() { when(accessTokenRepositoryMock.findByValue("something")).thenReturn(accessTokenEntity); when(accessTokenDtoServiceMock.createDto(accessTokenEntity)).thenReturn(expected); - AccessToken actual = accessTokenBusinessService.validateAccessTokenValue(header); + AccessToken actual = accessTokenBusinessService.validateAccessTokenValueWithHeader(header); assertEquals(expected, actual); } diff --git a/src/test/java/de/filefighter/rest/domain/user/business/UserAuthorizationServiceUnitTest.java b/src/test/java/de/filefighter/rest/domain/user/business/UserAuthorizationServiceUnitTest.java index f524c720..b72374d0 100644 --- a/src/test/java/de/filefighter/rest/domain/user/business/UserAuthorizationServiceUnitTest.java +++ b/src/test/java/de/filefighter/rest/domain/user/business/UserAuthorizationServiceUnitTest.java @@ -1,5 +1,6 @@ package de.filefighter.rest.domain.user.business; +import de.filefighter.rest.configuration.RestConfiguration; import de.filefighter.rest.domain.token.data.dto.AccessToken; import de.filefighter.rest.domain.user.data.dto.User; import de.filefighter.rest.domain.user.data.persistance.UserEntity; @@ -9,6 +10,7 @@ import de.filefighter.rest.rest.exceptions.RequestDidntMeetFormalRequirementsException; import org.junit.jupiter.api.Test; +import static de.filefighter.rest.configuration.RestConfiguration.AUTHORIZATION_BASIC_PREFIX; import static de.filefighter.rest.configuration.RestConfiguration.AUTHORIZATION_BEARER_PREFIX; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mock; @@ -24,9 +26,9 @@ class UserAuthorizationServiceUnitTest { @Test void authenticateUserWithUsernameAndPasswordThrows() { - String matchesButIsNotSupportedEncoding = "���"; - String matchesButDoesNotMeetRequirements = "dWdhYnVnYQ=="; - String matchesButUserWasNotFound = "dXNlcjp1c2Vy"; + String matchesButIsNotSupportedEncoding = AUTHORIZATION_BASIC_PREFIX + "���"; + String matchesButDoesNotMeetRequirements = AUTHORIZATION_BASIC_PREFIX + "dWdhYnVnYQ=="; + String matchesButUserWasNotFound = AUTHORIZATION_BASIC_PREFIX + "dXNlcjpwYXNzd29yZA=="; assertThrows(RuntimeException.class, () -> userAuthorizationService.authenticateUserWithUsernameAndPassword(matchesButIsNotSupportedEncoding) @@ -35,7 +37,7 @@ void authenticateUserWithUsernameAndPasswordThrows() { userAuthorizationService.authenticateUserWithUsernameAndPassword(matchesButDoesNotMeetRequirements) ); - when(userRepositoryMock.findByUsernameAndPassword("user", "user")).thenReturn(null); + when(userRepositoryMock.findByUsernameAndPassword("user", "password")).thenReturn(null); assertThrows(UserNotAuthenticatedException.class, () -> userAuthorizationService.authenticateUserWithUsernameAndPassword(matchesButUserWasNotFound)); @@ -43,7 +45,7 @@ void authenticateUserWithUsernameAndPasswordThrows() { @Test void authenticateUserWithUsernameAndPasswordWorksCorrectly() { - String header = "dXNlcjpwYXNzd29yZA=="; // user:password + String header = AUTHORIZATION_BASIC_PREFIX + "dXNlcjpwYXNzd29yZA=="; // user:password User dummyUser = User.builder().build(); UserEntity dummyEntity = UserEntity.builder().build(); @@ -68,13 +70,14 @@ void authenticateUserWithRefreshTokenThrowsExceptions() { @Test void authenticateUserWithRefreshTokenWorksCorrectly() { String refreshToken = "Something"; + String authString = AUTHORIZATION_BEARER_PREFIX + refreshToken; UserEntity dummyEntity = UserEntity.builder().build(); User dummyUser = User.builder().build(); when(userRepositoryMock.findByRefreshToken(refreshToken)).thenReturn(dummyEntity); when(userDtoServiceMock.createDto(dummyEntity)).thenReturn(dummyUser); - User actualUser = userAuthorizationService.authenticateUserWithRefreshToken(refreshToken); + User actualUser = userAuthorizationService.authenticateUserWithRefreshToken(authString); assertEquals(dummyUser, actualUser); } diff --git a/src/test/java/de/filefighter/rest/domain/user/rest/UserRestControllerUnitTest.java b/src/test/java/de/filefighter/rest/domain/user/rest/UserRestControllerUnitTest.java index da99ac80..95abde30 100644 --- a/src/test/java/de/filefighter/rest/domain/user/rest/UserRestControllerUnitTest.java +++ b/src/test/java/de/filefighter/rest/domain/user/rest/UserRestControllerUnitTest.java @@ -75,14 +75,13 @@ void getUserInfoWithAccessToken() { @Test void updateUserWithAccessToken() { - User user = User.builder().id(420).groups(null).username("kevin").build(); - ResponseEntity expectedUser = new ResponseEntity<>(user, OK); + ResponseEntity expectedResponse = new ResponseEntity<>(new ServerResponse(CREATED, "uga"), CREATED); UserRegisterForm userRegisterForm = UserRegisterForm.builder().build(); - when(userRestServiceMock.updateUserWithAccessToken(userRegisterForm, "token")).thenReturn(expectedUser); - ResponseEntity actualUser = userRestController.updateUser("token", userRegisterForm); + when(userRestServiceMock.updateUserByUserIdAuthenticateWithAccessToken(userRegisterForm, 0, "token")).thenReturn(expectedResponse); + ResponseEntity actualResponse = userRestController.updateUser(0, "token", userRegisterForm); - assertEquals(expectedUser, actualUser); + assertEquals(expectedResponse, actualResponse); } @Test diff --git a/src/test/resources/UserEditInformation.feature b/src/test/resources/UserEditInformation.feature index 5882d829..9f3e7c34 100644 --- a/src/test/resources/UserEditInformation.feature +++ b/src/test/resources/UserEditInformation.feature @@ -10,6 +10,7 @@ # Scenario: Successful change of username # When user requests change of username with value "kangaroo" and accessToken "accessToken" # Then response contains key "message" and value "Username successfully changed." +# And response contains key "status" and value "" # And response status code is 201 # # Scenario: Successful change of password From 390bcd37699650ff6d890b506a4b4c7acd591ca6 Mon Sep 17 00:00:00 2001 From: open-schnick Date: Fri, 20 Nov 2020 15:29:39 +0100 Subject: [PATCH 2/8] updated feature files, and step def. --- .../cucumber/UserEditInformationSteps.java | 16 +-- .../resources/UserEditInformation.feature | 97 +++++++++---------- 2 files changed, 53 insertions(+), 60 deletions(-) diff --git a/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java b/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java index e2341aa1..2f82a265 100644 --- a/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java +++ b/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java @@ -11,19 +11,19 @@ import static de.filefighter.rest.configuration.RestConfiguration.*; public class UserEditInformationSteps extends RestApplicationIntegrationTest { - @When("user requests change of username with value {string} and accessToken {string}") - public void userRequestsChangeOfUsernameWithValueAndAccessTokenAndId(String newUsername, String accessToken) { + @When("user requests change of username with value {string} userId {long} and accessToken {string}") + public void userRequestsChangeOfUsernameWithValueAndAccessTokenAndId(String newUsername, long userId, String accessToken) { String authHeaderString = AUTHORIZATION_BEARER_PREFIX + accessToken; - String url = BASE_API_URI + USER_BASE_URI + "edit"; + String url = BASE_API_URI + USER_BASE_URI + userId + "/edit"; HashMap authHeader = new HashMap<>(); authHeader.put("Authorization", authHeaderString); - String postBody= serializeUserRequest(null,null,null,newUsername); - executeRestApiCall(HttpMethod.PUT, url, authHeader,postBody); + String postBody = serializeUserRequest(null, null, null, newUsername); + executeRestApiCall(HttpMethod.PUT, url, authHeader, postBody); } - @When("user requests change of password with value {string} and accessToken {string} and id {string}") + @When("user requests change of password with value {string} userId {long} and accessToken {string}") public void userRequestsChangeOfPasswordWithValueAndAccessTokenAndId(String newPassword, String accessToken, String userId) { String authHeaderString = AUTHORIZATION_BEARER_PREFIX + accessToken; String url = BASE_API_URI + USER_BASE_URI + userId + "/edit"; @@ -32,9 +32,9 @@ public void userRequestsChangeOfPasswordWithValueAndAccessTokenAndId(String newP authHeader.put("Authorization", authHeaderString); - String postBody=serializeUserRequest(newPassword,null,newPassword,null); + String postBody = serializeUserRequest(newPassword, null, newPassword, null); - executeRestApiCall(HttpMethod.GET, url, authHeader,postBody); + executeRestApiCall(HttpMethod.GET, url, authHeader, postBody); } } diff --git a/src/test/resources/UserEditInformation.feature b/src/test/resources/UserEditInformation.feature index 9f3e7c34..3e40e88e 100644 --- a/src/test/resources/UserEditInformation.feature +++ b/src/test/resources/UserEditInformation.feature @@ -1,52 +1,45 @@ -#Feature: Edit User Details -# As a user -# I want to be able to change my username and password -# -# Background: -# Given database is empty -# And user with id 1234 exists and has username "user", password "secure_password" -# And accessToken with value "accessToken" exists for user 1234 -# -# Scenario: Successful change of username -# When user requests change of username with value "kangaroo" and accessToken "accessToken" -# Then response contains key "message" and value "Username successfully changed." -# And response contains key "status" and value "" -# And response status code is 201 -# -# Scenario: Successful change of password -# When user requests change of password with value "pig-system" and accessToken "accessToken" and id "1234" -# Then response contains key "message" and value "Password successfully changed." -# And response status code is 201 -# -# Scenario: Failed change of username; new username equals old username -# When user requests change of username with value "user" and accessToken "accessToken" -# Then response contains key "message" and value "No changes." -# And response status code is 409 -# And response contains key "status" and value "conflict" -# -# Scenario: Failed change of username; new username already assigned -# Given user with id 1235 exists and has username "kangaroo", password "secure_password" -# When user requests change of username with value "kangaroo" and accessToken "accessToken" -# Then response contains key "message" and value "Username already assigned." -# And response status code is 409 -# And response contains key "status" and value "conflict" -# -# Scenario: Failed change of password; new password equals old password -# When user requests change of password with value "secure_password" and accessToken "accessToken" and id "1234" -# Then response contains key "message" and value "No changes." -# And response status code is 409 -# And response contains key "status" and value "conflict" -# -# Scenario: Failed change of password; new password contains username -# When user requests change of password with value "user123" and accessToken "accessToken" and id "1234" -# Then response contains key "message" and value "Username must not appear in password." -# And response status code is 409 -# And response contains key "status" and value "conflict" -# -# Scenario: Failed change of password; new password appears in list of top 10k passwords -# When user requests change of password with value "vietnam" and accessToken "accessToken" and id "1234" -# Then response status code is 409 -# And response contains key "message" and value "Password must not appear in the top 10000 most common passwords." -# And response contains key "status" and value "conflict" -# -# #https://github.com/iryndin/10K-Most-Popular-Passwords/blob/master/passwords.txt \ No newline at end of file +Feature: Edit User Details + As a user + I want to be able to change my username and password + + Background: + Given database is empty + And user with id 1234 exists and has username "user", password "secure_password" + And accessToken with value "accessToken" exists for user 1234 + + Scenario: Successful change of username + When user requests change of username with value "kangaroo" userId 1234 and accessToken "accessToken" + Then response contains key "message" and value "Username successfully changed." + And response contains key "status" and value "Created" + And response status code is 201 + + Scenario: Successful change of password + When user requests change of password with value "pig-system" userId 1234 and accessToken "accessToken" + Then response contains key "message" and value "Password successfully changed." + And response contains key "status" and value "Created" + And response status code is 201 + + Scenario: Failed change of username; new username equals old username + When user requests change of username with value "user" userId 1234 and accessToken "accessToken" + Then response contains key "message" and value "No changes." + And response status code is 409 + And response contains key "status" and value "Conflict" + + Scenario: Failed change of username; new username already assigned + Given user with id 1235 exists and has username "kangaroo", password "secure_password" + When user requests change of username with value "kangaroo" userId 1234 and accessToken "accessToken" + Then response contains key "message" and value "Username already assigned." + And response status code is 409 + And response contains key "status" and value "Conflict" + + Scenario: Failed change of password; new password equals old password + When user requests change of password with value "secure_password" userId 1234 and accessToken "accessToken" + Then response contains key "message" and value "No changes." + And response status code is 409 + And response contains key "status" and value "Conflict" + + Scenario: Failed change of password; new password contains username + When user requests change of password with value "user123" userId 1234 and accessToken "accessToken" + Then response contains key "message" and value "Username must not appear in password." + And response status code is 409 + And response contains key "status" and value "Conflict" \ No newline at end of file From 29d7aeece7ded7b42bfd243ba2baa70f1feea27e Mon Sep 17 00:00:00 2001 From: open-schnick Date: Fri, 20 Nov 2020 23:27:22 +0100 Subject: [PATCH 3/8] FF-106 Write Logic for User Edit --- .../user/business/UserBusinessService.java | 85 ++++++++++++++++--- .../user/exceptions/UserNotUpdatedAdvise.java | 22 +++++ .../exceptions/UserNotUpdatedException.java | 11 +++ .../domain/user/rest/UserRestController.java | 2 +- .../domain/user/rest/UserRestService.java | 4 +- .../business/UserBusinessServiceUnitTest.java | 4 +- 6 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 src/main/java/de/filefighter/rest/domain/user/exceptions/UserNotUpdatedAdvise.java create mode 100644 src/main/java/de/filefighter/rest/domain/user/exceptions/UserNotUpdatedException.java diff --git a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java index 69d6c771..ca15b682 100644 --- a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java +++ b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java @@ -9,13 +9,20 @@ import de.filefighter.rest.domain.user.data.persistance.UserRepository; import de.filefighter.rest.domain.user.exceptions.UserNotFoundException; import de.filefighter.rest.domain.user.exceptions.UserNotRegisteredException; +import de.filefighter.rest.domain.user.exceptions.UserNotUpdatedException; import de.filefighter.rest.domain.user.group.GroupRepository; +import de.filefighter.rest.domain.user.group.Groups; import de.filefighter.rest.rest.exceptions.RequestDidntMeetFormalRequirementsException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; import org.springframework.stereotype.Service; +import java.util.Arrays; import java.util.regex.Pattern; import static de.filefighter.rest.domain.common.Utils.stringIsValid; @@ -26,16 +33,19 @@ public class UserBusinessService { private final UserRepository userRepository; private final UserDtoService userDtoService; private final GroupRepository groupRepository; + private final MongoTemplate mongoTemplate; + private static final Logger LOG = LoggerFactory.getLogger(UserBusinessService.class); @Value("${filefighter.disable-password-check}") public boolean passwordCheckDisabled; - public UserBusinessService(UserRepository userRepository, UserDtoService userDtoService, GroupRepository groupRepository) { + public UserBusinessService(UserRepository userRepository, UserDtoService userDtoService, GroupRepository groupRepository, MongoTemplate mongoTemplate) { this.userRepository = userRepository; this.userDtoService = userDtoService; this.groupRepository = groupRepository; + this.mongoTemplate = mongoTemplate; } public long getUserCount() { @@ -97,14 +107,15 @@ public void registerNewUser(UserRegisterForm newUser) { // check pws. String password = newUser.getPassword(); - passwordIsValid(password); + if (!passwordIsValid(password)) + throw new UserNotRegisteredException("Password needs to be at least 8 characters long and, contains at least one uppercase and lowercase letter and a number."); String confirmationPassword = newUser.getConfirmationPassword(); if (!password.contentEquals(confirmationPassword)) throw new UserNotRegisteredException("Passwords do not match."); - if(password.toLowerCase().contains(username.toLowerCase())) + if (password.toLowerCase().contains(username.toLowerCase())) throw new UserNotRegisteredException("Username must not appear in password."); //check groups @@ -130,18 +141,70 @@ public void registerNewUser(UserRegisterForm newUser) { .build()); } - public void passwordIsValid(String password) { + public boolean passwordIsValid(String password) { if (!Utils.stringIsValid(password)) - throw new UserNotRegisteredException("Password was empty."); + return false; - if(this.passwordCheckDisabled) return; + if (this.passwordCheckDisabled) return true; - Pattern pattern = Pattern.compile("^(?=.*[0-9])(?=.*[a-z])(?=.*[A-Z])(?=\\S+$).{8,20}$"); - if (!pattern.matcher(password).matches()) - throw new UserNotRegisteredException("Password needs to be at least 8 characters long and, contains at least one uppercase and lowercase letter and a number."); + Pattern pattern = Pattern.compile("^(?=.*[0-9])(?=.*[a-z])(?=.*[A-Z])(?=\\S+).{8,20}$"); + return pattern.matcher(password).matches(); } - public String updateUser(long userId, UserRegisterForm updatedUser, User authenticatedUser) { - return null; + public void updateUser(long userId, UserRegisterForm updatedUser, User authenticatedUser) { + if (userId != authenticatedUser.getId() && Arrays.stream(authenticatedUser.getGroups()).anyMatch(g -> g == Groups.ADMIN)) + throw new UserNotUpdatedException("Only Admins are allowed to update other users."); + + UserEntity userEntityToUpdate = userRepository.findByUserId(userId); + boolean userToUpdateIsAdmin = Arrays.stream(userEntityToUpdate.getGroupIds()).anyMatch(g -> groupRepository.getGroupById(g) == Groups.ADMIN); + + Update newUpdate = new Update(); + + // username + if (null != updatedUser.getUsername()) { + if (!stringIsValid(updatedUser.getUsername())) + throw new UserNotUpdatedException("Wanted to change username, but username was not valid."); + + newUpdate.set("username", updatedUser.getUsername()); + } + + // pw + if (null != updatedUser.getPassword()) { + if (!stringIsValid(updatedUser.getPassword())) + throw new UserNotUpdatedException("Wanted to change password, but password was not valid."); + + String password = updatedUser.getPassword(); + String confirmation = updatedUser.getConfirmationPassword(); + + if (passwordIsValid(password)) + throw new UserNotUpdatedException("Password needs to be at least 8 characters long and, contains at least one uppercase and lowercase letter and a number."); + + if (!password.contentEquals(confirmation)) + throw new UserNotUpdatedException("Passwords do not match."); + + if (password.toLowerCase().contains(userEntityToUpdate.getLowercaseUsername())) + throw new UserNotUpdatedException("Username must not appear in password."); + + newUpdate.set("password", password); + } + + // groups + if (null != updatedUser.getGroupIds()) { + try { + for (Groups group : groupRepository.getGroupsByIds(updatedUser.getGroupIds())) { + if (group == Groups.ADMIN && !userToUpdateIsAdmin) + throw new UserNotUpdatedException("Only admins can add users to group " + Groups.ADMIN.getDisplayName()); + } + } catch (IllegalArgumentException exception) { + throw new UserNotUpdatedException("One or more groups do not exist."); + } + + newUpdate.set("groupIds", updatedUser.getGroupIds()); + } + + Query query = new Query(); + query.addCriteria(Criteria.where("userId").is(userId)); + mongoTemplate.findAndModify(query, newUpdate, UserEntity.class); } } + diff --git a/src/main/java/de/filefighter/rest/domain/user/exceptions/UserNotUpdatedAdvise.java b/src/main/java/de/filefighter/rest/domain/user/exceptions/UserNotUpdatedAdvise.java new file mode 100644 index 00000000..24bd8ab7 --- /dev/null +++ b/src/main/java/de/filefighter/rest/domain/user/exceptions/UserNotUpdatedAdvise.java @@ -0,0 +1,22 @@ +package de.filefighter.rest.domain.user.exceptions; + +import de.filefighter.rest.rest.ServerResponse; +import org.slf4j.LoggerFactory; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ControllerAdvice +class UserNotUpdatedAdvise { + + @ResponseBody + @ExceptionHandler(UserNotUpdatedException.class) + @ResponseStatus(HttpStatus.CONFLICT) + ResponseEntity userNotUpdatedExceptionHandler(UserNotUpdatedException ex) { + LoggerFactory.getLogger(UserNotUpdatedException.class).warn(ex.getMessage()); + return new ResponseEntity<>(new ServerResponse(HttpStatus.CONFLICT, ex.getMessage()), HttpStatus.CONFLICT); + } +} \ No newline at end of file diff --git a/src/main/java/de/filefighter/rest/domain/user/exceptions/UserNotUpdatedException.java b/src/main/java/de/filefighter/rest/domain/user/exceptions/UserNotUpdatedException.java new file mode 100644 index 00000000..0c29e4c8 --- /dev/null +++ b/src/main/java/de/filefighter/rest/domain/user/exceptions/UserNotUpdatedException.java @@ -0,0 +1,11 @@ +package de.filefighter.rest.domain.user.exceptions; + +public class UserNotUpdatedException extends RuntimeException{ + public UserNotUpdatedException() { + super("User could not get updated"); + } + + public UserNotUpdatedException(String reason) { + super("User could not get updated. "+reason); + } +} diff --git a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestController.java b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestController.java index 0b5c9ea2..4c379aec 100644 --- a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestController.java +++ b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestController.java @@ -70,7 +70,7 @@ public ResponseEntity updateUser( @RequestHeader(value = "Authorization", defaultValue = AUTHORIZATION_BEARER_PREFIX + "token") String accessToken, @RequestBody UserRegisterForm updatedUser) { - LOG.info("Updated User and Token {}, with form {}.", accessToken, updatedUser); + LOG.info("Updated User {} and Token {}, with form {}.", userId, accessToken, updatedUser); return userRestService.updateUserByUserIdAuthenticateWithAccessToken(updatedUser, userId, accessToken); } diff --git a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestService.java b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestService.java index fe35d1a4..013d01b5 100644 --- a/src/main/java/de/filefighter/rest/domain/user/rest/UserRestService.java +++ b/src/main/java/de/filefighter/rest/domain/user/rest/UserRestService.java @@ -54,8 +54,8 @@ public ResponseEntity getAccessTokenByRefreshToken(String refreshTo public ResponseEntity updateUserByUserIdAuthenticateWithAccessToken(UserRegisterForm updatedUser, long userId, String accessTokenValue) { AccessToken accessToken = accessTokenBusinessService.validateAccessTokenValueWithHeader(accessTokenValue); User authenticatedUser = userAuthorizationService.authenticateUserWithAccessToken(accessToken); - String message = userBusinessService.updateUser(userId, updatedUser, authenticatedUser); - ServerResponse response = new ServerResponse(HttpStatus.CREATED, message); + userBusinessService.updateUser(userId, updatedUser, authenticatedUser); + ServerResponse response = new ServerResponse(HttpStatus.CREATED, "User successfully updated."); return new ResponseEntity<>(response, HttpStatus.CREATED); } diff --git a/src/test/java/de/filefighter/rest/domain/user/business/UserBusinessServiceUnitTest.java b/src/test/java/de/filefighter/rest/domain/user/business/UserBusinessServiceUnitTest.java index e715dc55..e397b366 100644 --- a/src/test/java/de/filefighter/rest/domain/user/business/UserBusinessServiceUnitTest.java +++ b/src/test/java/de/filefighter/rest/domain/user/business/UserBusinessServiceUnitTest.java @@ -11,6 +11,7 @@ import de.filefighter.rest.rest.exceptions.RequestDidntMeetFormalRequirementsException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.data.mongodb.core.MongoTemplate; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; @@ -22,11 +23,12 @@ class UserBusinessServiceUnitTest { private final UserRepository userRepositoryMock = mock(UserRepository.class); private final UserDtoService userDtoServiceMock = mock(UserDtoService.class); private final GroupRepository groupRepositoryMock = mock(GroupRepository.class); + private final MongoTemplate mongoTemplateMock = mock(MongoTemplate.class); private UserBusinessService userBusinessService; @BeforeEach void setUp() { - userBusinessService = new UserBusinessService(userRepositoryMock, userDtoServiceMock, groupRepositoryMock); + userBusinessService = new UserBusinessService(userRepositoryMock, userDtoServiceMock, groupRepositoryMock, mongoTemplateMock); } @Test From 642715f13625ca7f061413f5941f893cfbcebc1b Mon Sep 17 00:00:00 2001 From: open-schnick Date: Sat, 21 Nov 2020 00:10:48 +0100 Subject: [PATCH 4/8] Tweaked Cucumber Steps. Fixed Serialization. --- .../user/business/UserBusinessService.java | 45 ++++++++++++++----- .../java/de/filefighter/rest/TestUtils.java | 2 +- .../cucumber/UserEditInformationSteps.java | 7 +-- .../resources/UserEditInformation.feature | 20 ++------- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java index ca15b682..cf980584 100644 --- a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java +++ b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java @@ -151,30 +151,51 @@ public boolean passwordIsValid(String password) { return pattern.matcher(password).matches(); } - public void updateUser(long userId, UserRegisterForm updatedUser, User authenticatedUser) { + public void updateUser(long userId, UserRegisterForm userToUpdate, User authenticatedUser) { + if (null == userToUpdate) + throw new UserNotUpdatedException("No updates specified."); + if (userId != authenticatedUser.getId() && Arrays.stream(authenticatedUser.getGroups()).anyMatch(g -> g == Groups.ADMIN)) throw new UserNotUpdatedException("Only Admins are allowed to update other users."); UserEntity userEntityToUpdate = userRepository.findByUserId(userId); - boolean userToUpdateIsAdmin = Arrays.stream(userEntityToUpdate.getGroupIds()).anyMatch(g -> groupRepository.getGroupById(g) == Groups.ADMIN); + boolean userToUpdateIsAdmin = false; + + for (Groups group : groupRepository.getGroupsByIds(userToUpdate.getGroupIds())) { + if (group == Groups.ADMIN) { + userToUpdateIsAdmin = true; + break; + } + } Update newUpdate = new Update(); // username - if (null != updatedUser.getUsername()) { - if (!stringIsValid(updatedUser.getUsername())) + String username = userToUpdate.getUsername(); + if (null != username) { + if (!stringIsValid(username)) throw new UserNotUpdatedException("Wanted to change username, but username was not valid."); - newUpdate.set("username", updatedUser.getUsername()); + User user = null; + try { + user = this.findUserByUsername(username); + } catch (UserNotFoundException ignored) { + LOG.info("Username '{}' is free to use.", username); + } + + if (null != user) + throw new UserNotUpdatedException("Username already taken."); + + newUpdate.set("username", username); } // pw - if (null != updatedUser.getPassword()) { - if (!stringIsValid(updatedUser.getPassword())) + if (null != userToUpdate.getPassword()) { + if (!stringIsValid(userToUpdate.getPassword())) throw new UserNotUpdatedException("Wanted to change password, but password was not valid."); - String password = updatedUser.getPassword(); - String confirmation = updatedUser.getConfirmationPassword(); + String password = userToUpdate.getPassword(); + String confirmation = userToUpdate.getConfirmationPassword(); if (passwordIsValid(password)) throw new UserNotUpdatedException("Password needs to be at least 8 characters long and, contains at least one uppercase and lowercase letter and a number."); @@ -189,9 +210,9 @@ public void updateUser(long userId, UserRegisterForm updatedUser, User authentic } // groups - if (null != updatedUser.getGroupIds()) { + if (null != userToUpdate.getGroupIds()) { try { - for (Groups group : groupRepository.getGroupsByIds(updatedUser.getGroupIds())) { + for (Groups group : groupRepository.getGroupsByIds(userToUpdate.getGroupIds())) { if (group == Groups.ADMIN && !userToUpdateIsAdmin) throw new UserNotUpdatedException("Only admins can add users to group " + Groups.ADMIN.getDisplayName()); } @@ -199,7 +220,7 @@ public void updateUser(long userId, UserRegisterForm updatedUser, User authentic throw new UserNotUpdatedException("One or more groups do not exist."); } - newUpdate.set("groupIds", updatedUser.getGroupIds()); + newUpdate.set("groupIds", userToUpdate.getGroupIds()); } Query query = new Query(); diff --git a/src/test/java/de/filefighter/rest/TestUtils.java b/src/test/java/de/filefighter/rest/TestUtils.java index e4d19641..4e0247ee 100644 --- a/src/test/java/de/filefighter/rest/TestUtils.java +++ b/src/test/java/de/filefighter/rest/TestUtils.java @@ -14,7 +14,7 @@ public static String serializeUserRequest(String confirmationPassword, int[] gro jsonString.append("\"groupIds\": ").append(Arrays.toString(groupIds)).append(","); } if (password != null) { - jsonString.append("\"password\": \"").append(password).append(username != null?"\",":""); + jsonString.append("\"password\": \"").append(password).append(username != null ? "\"," : "\""); } if (username != null) { jsonString.append("\"username\": \"").append(username).append("\""); diff --git a/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java b/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java index 2f82a265..7ee67c45 100644 --- a/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java +++ b/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java @@ -24,17 +24,14 @@ public void userRequestsChangeOfUsernameWithValueAndAccessTokenAndId(String newU } @When("user requests change of password with value {string} userId {long} and accessToken {string}") - public void userRequestsChangeOfPasswordWithValueAndAccessTokenAndId(String newPassword, String accessToken, String userId) { + public void userRequestsChangeOfPasswordWithValueAndAccessTokenAndId(String newPassword, long userId, String accessToken) { String authHeaderString = AUTHORIZATION_BEARER_PREFIX + accessToken; String url = BASE_API_URI + USER_BASE_URI + userId + "/edit"; HashMap authHeader = new HashMap<>(); authHeader.put("Authorization", authHeaderString); - - String postBody = serializeUserRequest(newPassword, null, newPassword, null); - - executeRestApiCall(HttpMethod.GET, url, authHeader, postBody); + executeRestApiCall(HttpMethod.PUT, url, authHeader, postBody); } } diff --git a/src/test/resources/UserEditInformation.feature b/src/test/resources/UserEditInformation.feature index 3e40e88e..2e6ceb62 100644 --- a/src/test/resources/UserEditInformation.feature +++ b/src/test/resources/UserEditInformation.feature @@ -9,37 +9,25 @@ Feature: Edit User Details Scenario: Successful change of username When user requests change of username with value "kangaroo" userId 1234 and accessToken "accessToken" - Then response contains key "message" and value "Username successfully changed." + Then response contains key "message" and value "User successfully updated." And response contains key "status" and value "Created" And response status code is 201 Scenario: Successful change of password When user requests change of password with value "pig-system" userId 1234 and accessToken "accessToken" - Then response contains key "message" and value "Password successfully changed." + Then response contains key "message" and value "User successfully updated." And response contains key "status" and value "Created" And response status code is 201 - Scenario: Failed change of username; new username equals old username - When user requests change of username with value "user" userId 1234 and accessToken "accessToken" - Then response contains key "message" and value "No changes." - And response status code is 409 - And response contains key "status" and value "Conflict" - Scenario: Failed change of username; new username already assigned Given user with id 1235 exists and has username "kangaroo", password "secure_password" When user requests change of username with value "kangaroo" userId 1234 and accessToken "accessToken" - Then response contains key "message" and value "Username already assigned." - And response status code is 409 - And response contains key "status" and value "Conflict" - - Scenario: Failed change of password; new password equals old password - When user requests change of password with value "secure_password" userId 1234 and accessToken "accessToken" - Then response contains key "message" and value "No changes." + Then response contains key "message" and value "User could not get updated. Username already taken." And response status code is 409 And response contains key "status" and value "Conflict" Scenario: Failed change of password; new password contains username When user requests change of password with value "user123" userId 1234 and accessToken "accessToken" - Then response contains key "message" and value "Username must not appear in password." + Then response contains key "message" and value "User could not get updated. Username must not appear in password." And response status code is 409 And response contains key "status" and value "Conflict" \ No newline at end of file From fc1f1718f4afd176a687c768a442955d89bb1f76 Mon Sep 17 00:00:00 2001 From: open-schnick Date: Sat, 21 Nov 2020 13:34:43 +0100 Subject: [PATCH 5/8] Added additional check, fixed some bugs. --- .../user/business/UserBusinessService.java | 24 ++++++++++--------- .../cucumber/UserEditInformationSteps.java | 12 ++++++++++ .../resources/UserEditInformation.feature | 8 ++++++- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java index cf980584..622796f3 100644 --- a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java +++ b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java @@ -155,20 +155,16 @@ public void updateUser(long userId, UserRegisterForm userToUpdate, User authenti if (null == userToUpdate) throw new UserNotUpdatedException("No updates specified."); - if (userId != authenticatedUser.getId() && Arrays.stream(authenticatedUser.getGroups()).anyMatch(g -> g == Groups.ADMIN)) + if(null == authenticatedUser.getGroups()) + throw new UserNotUpdatedException("Authenticated User is not allowed"); + + boolean authenticatedUserIsAdmin = Arrays.stream(authenticatedUser.getGroups()).anyMatch(g -> g == Groups.ADMIN); + if (userId != authenticatedUser.getId() && !authenticatedUserIsAdmin) throw new UserNotUpdatedException("Only Admins are allowed to update other users."); UserEntity userEntityToUpdate = userRepository.findByUserId(userId); - boolean userToUpdateIsAdmin = false; - - for (Groups group : groupRepository.getGroupsByIds(userToUpdate.getGroupIds())) { - if (group == Groups.ADMIN) { - userToUpdateIsAdmin = true; - break; - } - } - Update newUpdate = new Update(); + boolean changesWereMade = false; // username String username = userToUpdate.getUsername(); @@ -186,6 +182,7 @@ public void updateUser(long userId, UserRegisterForm userToUpdate, User authenti if (null != user) throw new UserNotUpdatedException("Username already taken."); + changesWereMade = true; newUpdate.set("username", username); } @@ -206,6 +203,7 @@ public void updateUser(long userId, UserRegisterForm userToUpdate, User authenti if (password.toLowerCase().contains(userEntityToUpdate.getLowercaseUsername())) throw new UserNotUpdatedException("Username must not appear in password."); + changesWereMade = true; newUpdate.set("password", password); } @@ -213,16 +211,20 @@ public void updateUser(long userId, UserRegisterForm userToUpdate, User authenti if (null != userToUpdate.getGroupIds()) { try { for (Groups group : groupRepository.getGroupsByIds(userToUpdate.getGroupIds())) { - if (group == Groups.ADMIN && !userToUpdateIsAdmin) + if (group == Groups.ADMIN && !authenticatedUserIsAdmin) throw new UserNotUpdatedException("Only admins can add users to group " + Groups.ADMIN.getDisplayName()); } } catch (IllegalArgumentException exception) { throw new UserNotUpdatedException("One or more groups do not exist."); } + changesWereMade = true; newUpdate.set("groupIds", userToUpdate.getGroupIds()); } + if(!changesWereMade) + throw new UserNotUpdatedException("No changes were made."); + Query query = new Query(); query.addCriteria(Criteria.where("userId").is(userId)); mongoTemplate.findAndModify(query, newUpdate, UserEntity.class); diff --git a/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java b/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java index 7ee67c45..b2f4d708 100644 --- a/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java +++ b/src/test/java/de/filefighter/rest/cucumber/UserEditInformationSteps.java @@ -34,4 +34,16 @@ public void userRequestsChangeOfPasswordWithValueAndAccessTokenAndId(String newP executeRestApiCall(HttpMethod.PUT, url, authHeader, postBody); } + + @When("user requests change of password with no changes, userId {long} and accessToken {string}") + public void userRequestsChangeOfPasswordWithNoChangesUserIdLongAndAccessTokenString(long userId, String accessToken) { + String authHeaderString = AUTHORIZATION_BEARER_PREFIX + accessToken; + String url = BASE_API_URI + USER_BASE_URI + userId + "/edit"; + + HashMap authHeader = new HashMap<>(); + authHeader.put("Authorization", authHeaderString); + String postBody = serializeUserRequest(null, null, null, null); + + executeRestApiCall(HttpMethod.PUT, url, authHeader, postBody); + } } diff --git a/src/test/resources/UserEditInformation.feature b/src/test/resources/UserEditInformation.feature index 2e6ceb62..3160849d 100644 --- a/src/test/resources/UserEditInformation.feature +++ b/src/test/resources/UserEditInformation.feature @@ -30,4 +30,10 @@ Feature: Edit User Details When user requests change of password with value "user123" userId 1234 and accessToken "accessToken" Then response contains key "message" and value "User could not get updated. Username must not appear in password." And response status code is 409 - And response contains key "status" and value "Conflict" \ No newline at end of file + And response contains key "status" and value "Conflict" + + Scenario: Failed change of user. No Changes + When user requests change of password with no changes, userId 1234 and accessToken "accessToken" + Then response contains key "message" and value "User could not get updated. No changes were made." + And response contains key "status" and value "Conflict" + And response status code is 409 From 91faab18cea1261b9da7ebec5881c86449aba510 Mon Sep 17 00:00:00 2001 From: open-schnick Date: Sat, 21 Nov 2020 14:35:58 +0100 Subject: [PATCH 6/8] FF-106 Wrote UnitTests for User Edit --- .../user/business/UserBusinessService.java | 8 +- .../business/UserBusinessServiceUnitTest.java | 141 +++++++++++++++++- 2 files changed, 139 insertions(+), 10 deletions(-) diff --git a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java index 622796f3..041d367d 100644 --- a/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java +++ b/src/main/java/de/filefighter/rest/domain/user/business/UserBusinessService.java @@ -188,13 +188,13 @@ public void updateUser(long userId, UserRegisterForm userToUpdate, User authenti // pw if (null != userToUpdate.getPassword()) { - if (!stringIsValid(userToUpdate.getPassword())) - throw new UserNotUpdatedException("Wanted to change password, but password was not valid."); - String password = userToUpdate.getPassword(); String confirmation = userToUpdate.getConfirmationPassword(); - if (passwordIsValid(password)) + if (!stringIsValid(password) || !stringIsValid(confirmation)) + throw new UserNotUpdatedException("Wanted to change password, but password was not valid."); + + if (!passwordIsValid(password)) throw new UserNotUpdatedException("Password needs to be at least 8 characters long and, contains at least one uppercase and lowercase letter and a number."); if (!password.contentEquals(confirmation)) diff --git a/src/test/java/de/filefighter/rest/domain/user/business/UserBusinessServiceUnitTest.java b/src/test/java/de/filefighter/rest/domain/user/business/UserBusinessServiceUnitTest.java index e397b366..3cfab6ef 100644 --- a/src/test/java/de/filefighter/rest/domain/user/business/UserBusinessServiceUnitTest.java +++ b/src/test/java/de/filefighter/rest/domain/user/business/UserBusinessServiceUnitTest.java @@ -7,7 +7,9 @@ import de.filefighter.rest.domain.user.data.persistance.UserRepository; import de.filefighter.rest.domain.user.exceptions.UserNotFoundException; import de.filefighter.rest.domain.user.exceptions.UserNotRegisteredException; +import de.filefighter.rest.domain.user.exceptions.UserNotUpdatedException; import de.filefighter.rest.domain.user.group.GroupRepository; +import de.filefighter.rest.domain.user.group.Groups; import de.filefighter.rest.rest.exceptions.RequestDidntMeetFormalRequirementsException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -140,17 +142,17 @@ void findUserByUsernameWorksCorrectly() { } @Test - void passwordIsValidThrows() { + void passwordIsValidReturnsFalse() { String isEmpty = ""; String[] doNotMatch = new String[]{"pw", "password", "Password", "Password\\", "asdfghjkljasdasda123AS?213+dfghjkfghjkghjk"}; - assertThrows(UserNotRegisteredException.class, () -> - userBusinessService.passwordIsValid(isEmpty)); + boolean actualState = userBusinessService.passwordIsValid(isEmpty); for (String string : doNotMatch) { - assertThrows(UserNotRegisteredException.class, () -> - userBusinessService.passwordIsValid(string)); + assertFalse(userBusinessService.passwordIsValid(string)); } + + assertFalse(actualState); } @Test @@ -202,7 +204,7 @@ void registerNewUserThrows() { } @Test - void registerNewUserWorks(){ + void registerNewUserWorks() { String username = "username"; String password = "validPassword1234"; String confPassword = "validPassword1234"; @@ -217,4 +219,131 @@ void registerNewUserWorks(){ assertDoesNotThrow(() -> userBusinessService.registerNewUser(userRegisterForm)); } + + @Test + void updateUserThrows() { + final UserRegisterForm userRegisterForm = null; + long userId = 420; + User authenticatedUser = User.builder().build(); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser)); + + UserRegisterForm userRegisterForm1 = UserRegisterForm.builder().build(); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm1, authenticatedUser)); + + authenticatedUser.setGroups(new Groups[]{Groups.UNDEFINED}); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm1, authenticatedUser)); + + authenticatedUser.setGroups(new Groups[]{Groups.ADMIN}); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm1, authenticatedUser)); + } + + @Test + void updateUserNameThrows() { + final UserRegisterForm userRegisterForm = UserRegisterForm.builder().build(); + long userId = 420; + User authenticatedUser = User.builder().id(userId).groups(new Groups[]{Groups.FAMILY}).build(); + UserEntity dummyEntity = UserEntity.builder().build(); + + userRegisterForm.setUsername(""); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser)); + + String validUserName = "ValidUserNameButExists."; + userRegisterForm.setUsername(validUserName); + when(userRepositoryMock.findByLowercaseUsername(validUserName.toLowerCase())).thenReturn(dummyEntity); + when(userDtoServiceMock.createDto(dummyEntity)).thenReturn(User.builder().build()); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser)); + } + + + @Test + void updateUserNameWorks() { + final UserRegisterForm userRegisterForm = UserRegisterForm.builder().username("newUserName").build(); + long userId = 420; + User authenticatedUser = User.builder().id(userId).groups(new Groups[]{Groups.FAMILY}).build(); + + assertDoesNotThrow(() -> userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser)); + } + + @Test + void updatePasswordThrows() { + final UserRegisterForm userRegisterForm = UserRegisterForm.builder().build(); + long userId = 420; + User authenticatedUser = User.builder().id(userId).groups(new Groups[]{Groups.FAMILY}).build(); + UserEntity dummyEntity = UserEntity.builder().userId(userId).lowercaseUsername("password").build(); + + userRegisterForm.setPassword(""); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser), "Wanted to change password, but password was not valid."); + + userRegisterForm.setPassword("somepw"); + userRegisterForm.setConfirmationPassword("somepw"); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser), "Password needs to be at least 8 characters long and, contains at least one uppercase and lowercase letter and a number."); + + userRegisterForm.setPassword("Somepw12345"); + userRegisterForm.setConfirmationPassword("Somepw1234"); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser), "Passwords do not match."); + + String validPassword ="ValidPassword1234!="; + userRegisterForm.setPassword(validPassword); + userRegisterForm.setConfirmationPassword(validPassword); + when(userRepositoryMock.findByUserId(userId)).thenReturn(dummyEntity); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser), "Username must not appear in password."); + } + + @Test + void updatePasswordWorks() { + String password = "validPassword1234"; + final UserRegisterForm userRegisterForm = UserRegisterForm.builder().password(password).confirmationPassword(password).build(); + long userId = 420; + User authenticatedUser = User.builder().id(userId).groups(new Groups[]{Groups.FAMILY}).build(); + UserEntity dummyEntity = UserEntity.builder().userId(userId).lowercaseUsername("UGABUGA").build(); + + when(userRepositoryMock.findByUserId(userId)).thenReturn(dummyEntity); + assertDoesNotThrow(() -> userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser)); + } + + @Test + void updateGroupsThrows() { + final UserRegisterForm userRegisterForm = UserRegisterForm.builder().build(); + long userId = 420; + User authenticatedUser = User.builder().id(userId).groups(new Groups[]{Groups.FAMILY}).build(); + UserEntity dummyEntity = UserEntity.builder().userId(userId).lowercaseUsername("password").build(); + + long[] groups = new long[]{0}; + userRegisterForm.setGroupIds(groups); + when(userRepositoryMock.findByUserId(userId)).thenReturn(dummyEntity); + when(groupRepositoryMock.getGroupsByIds(groups)).thenReturn(new Groups[]{Groups.ADMIN}); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser)); + + groups = new long[]{123032,1230213}; + userRegisterForm.setGroupIds(groups); + when(userRepositoryMock.findByUserId(userId)).thenReturn(dummyEntity); + when(groupRepositoryMock.getGroupsByIds(groups)).thenThrow(new IllegalArgumentException("id doesnt belong to a group")); + assertThrows(UserNotUpdatedException.class, () -> + userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser)); + } + + @Test + void updateGroupsWorks() { + final UserRegisterForm userRegisterForm = UserRegisterForm.builder().build(); + long userId = 420; + User authenticatedUser = User.builder().id(userId).groups(new Groups[]{Groups.FAMILY}).build(); + UserEntity dummyEntity = UserEntity.builder().userId(userId).lowercaseUsername("password").build(); + + long[] groups = new long[]{0}; + userRegisterForm.setGroupIds(groups); + when(userRepositoryMock.findByUserId(userId)).thenReturn(dummyEntity); + when(groupRepositoryMock.getGroupsByIds(groups)).thenReturn(new Groups[]{Groups.FAMILY}); + assertDoesNotThrow(() -> userBusinessService.updateUser(userId, userRegisterForm, authenticatedUser)); + } } \ No newline at end of file From 4aa831bd33a9882d2008bf52e2668165f2128274 Mon Sep 17 00:00:00 2001 From: open-schnick Date: Sat, 21 Nov 2020 14:46:43 +0100 Subject: [PATCH 7/8] Damn you password check. --- src/test/resources/UserEditInformation.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/resources/UserEditInformation.feature b/src/test/resources/UserEditInformation.feature index 3160849d..8b0bbf5e 100644 --- a/src/test/resources/UserEditInformation.feature +++ b/src/test/resources/UserEditInformation.feature @@ -14,7 +14,7 @@ Feature: Edit User Details And response status code is 201 Scenario: Successful change of password - When user requests change of password with value "pig-system" userId 1234 and accessToken "accessToken" + When user requests change of password with value "pigSystem1234" userId 1234 and accessToken "accessToken" Then response contains key "message" and value "User successfully updated." And response contains key "status" and value "Created" And response status code is 201 @@ -27,7 +27,7 @@ Feature: Edit User Details And response contains key "status" and value "Conflict" Scenario: Failed change of password; new password contains username - When user requests change of password with value "user123" userId 1234 and accessToken "accessToken" + When user requests change of password with value "User123asd" userId 1234 and accessToken "accessToken" Then response contains key "message" and value "User could not get updated. Username must not appear in password." And response status code is 409 And response contains key "status" and value "Conflict" From 596c144cf8facab38b3bf9d43dce5197af6984e7 Mon Sep 17 00:00:00 2001 From: open-schnick Date: Sat, 21 Nov 2020 14:49:22 +0100 Subject: [PATCH 8/8] Bumped Version to v0.0.5 --- pom.xml | 2 +- src/main/resources/application.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index ec86f801..a8fdc88e 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ de.filefighter rest - 0.0.4 + 0.0.5 RestApi RestApi for FileFighter diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 10eb542e..05b88aea 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -8,5 +8,5 @@ spring.data.mongodb.host=localhost spring.data.mongodb.port=27017 #-------------------Custom------------------ filefighter.version=0.0.4 -filefighter.date=19.11.2020 +filefighter.date=21.11.2020 filefighter.disable-password-check=false \ No newline at end of file