SEBSERV-8 #more tests and bug fixes
This commit is contained in:
		
							parent
							
								
									ad9865bfa3
								
							
						
					
					
						commit
						70d66e6806
					
				
					 9 changed files with 130 additions and 48 deletions
				
			
		|  | @ -6,7 +6,7 @@ | |||
|  * file, You can obtain one at http://mozilla.org/MPL/2.0/. | ||||
|  */ | ||||
| 
 | ||||
| package ch.ethz.seb.sebserver.webservice.datalayer; | ||||
| package ch.ethz.seb.sebserver.gbl.model; | ||||
| 
 | ||||
| import java.io.Serializable; | ||||
| import java.util.Collections; | ||||
|  | @ -17,6 +17,9 @@ import org.springframework.http.HttpStatus; | |||
| import org.springframework.http.ResponseEntity; | ||||
| import org.springframework.validation.FieldError; | ||||
| 
 | ||||
| import com.fasterxml.jackson.annotation.JsonCreator; | ||||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||||
| 
 | ||||
| import ch.ethz.seb.sebserver.gbl.util.Utils; | ||||
| 
 | ||||
| public class APIMessage implements Serializable { | ||||
|  | @ -75,16 +78,21 @@ public class APIMessage implements Serializable { | |||
|         } | ||||
|     } | ||||
| 
 | ||||
|     @JsonProperty("messageCode") | ||||
|     public final String messageCode; | ||||
|     @JsonProperty("systemMessage") | ||||
|     public final String systemMessage; | ||||
|     @JsonProperty("details") | ||||
|     public final String details; | ||||
|     @JsonProperty("attributes") | ||||
|     public final List<String> attributes; | ||||
| 
 | ||||
|     @JsonCreator | ||||
|     public APIMessage( | ||||
|             final String messageCode, | ||||
|             final String systemMessage, | ||||
|             final String details, | ||||
|             final List<String> attributes) { | ||||
|             @JsonProperty("messageCode") final String messageCode, | ||||
|             @JsonProperty("systemMessage") final String systemMessage, | ||||
|             @JsonProperty("details") final String details, | ||||
|             @JsonProperty("attributes") final List<String> attributes) { | ||||
| 
 | ||||
|         this.messageCode = messageCode; | ||||
|         this.systemMessage = systemMessage; | ||||
|  | @ -139,6 +147,11 @@ public class APIMessage implements Serializable { | |||
|             this.apiMessage = errorMessage.of(); | ||||
|         } | ||||
| 
 | ||||
|         public APIMessageException(final ErrorMessage errorMessage, final String detail, final String... attributes) { | ||||
|             super(); | ||||
|             this.apiMessage = errorMessage.of(detail, attributes); | ||||
|         } | ||||
| 
 | ||||
|         public APIMessage getAPIMessage() { | ||||
|             return this.apiMessage; | ||||
|         } | ||||
|  | @ -51,18 +51,18 @@ public final class UserInfo implements GrantEntity, Serializable { | |||
| 
 | ||||
|     /** Full name of the user */ | ||||
|     @NotNull | ||||
|     @Size(min = 3, max = 255, message = "userInfo:name:size:{min}:{max}:${validatedValue}") | ||||
|     @Size(min = 3, max = 255, message = "user:name:size:{min}:{max}:${validatedValue}") | ||||
|     @JsonProperty(USER.ATTR_NAME) | ||||
|     public final String name; | ||||
| 
 | ||||
|     /** The internal user name */ | ||||
|     @NotNull | ||||
|     @Size(min = 3, max = 255, message = "userInfo:username:size:{min}:{max}:${validatedValue}") | ||||
|     @Size(min = 3, max = 255, message = "user:username:size:{min}:{max}:${validatedValue}") | ||||
|     @JsonProperty(USER.ATTR_USER_NAME) | ||||
|     public final String userName; | ||||
| 
 | ||||
|     /** E-mail address of the user */ | ||||
|     @Email(message = "userInfo:email:email:_:_:${validatedValue}") | ||||
|     @Email(message = "user:email:email:_:_:${validatedValue}") | ||||
|     @JsonProperty(USER.ATTR_EMAIL) | ||||
|     public final String email; | ||||
| 
 | ||||
|  | @ -83,7 +83,7 @@ public final class UserInfo implements GrantEntity, Serializable { | |||
| 
 | ||||
|     /** The users roles in a unmodifiable set. Is never null */ | ||||
|     @NotNull | ||||
|     @NotEmpty(message = "userInfo:roles:notEmpty:_:_:_") | ||||
|     @NotEmpty(message = "user:roles:notEmpty:_:_:_") | ||||
|     @JsonProperty(USER_ROLE.REFERENCE_NAME) | ||||
|     public final Set<String> roles; | ||||
| 
 | ||||
|  |  | |||
|  | @ -26,7 +26,7 @@ public final class UserMod implements GrantEntity { | |||
|     @JsonProperty(ATTR_NAME_USER_INFO) | ||||
|     private final UserInfo userInfo; | ||||
| 
 | ||||
|     @Size(min = 8, max = 255, message = "userInfo:password:size:{min}:{max}:${validatedValue}") | ||||
|     @Size(min = 8, max = 255, message = "user:password:size:{min}:{max}:${validatedValue}") | ||||
|     @JsonProperty(ATTR_NAME_NEW_PASSWORD) | ||||
|     private final String newPassword; | ||||
| 
 | ||||
|  |  | |||
|  | @ -11,6 +11,7 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.dao; | |||
| import java.util.Collection; | ||||
| import java.util.function.Predicate; | ||||
| 
 | ||||
| import ch.ethz.seb.sebserver.gbl.model.Entity; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserFilter; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserInfo; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserMod; | ||||
|  | @ -77,11 +78,16 @@ public interface UserDAO extends EntityDAO<UserInfo> { | |||
|      * not null on UserMod instance are updated within the existing user record. | ||||
|      * | ||||
|      * @param userMod UserMod instance containing new user record data | ||||
|      * @param principal the user principal that requests the save/modification | ||||
|      * @return A Result of UserInfo where the successfully saved/modified user data is available or a reported | ||||
|      *         exception on error case */ | ||||
|     Result<UserInfo> save(SEBServerUser principal, UserMod userMod); | ||||
|     Result<UserInfo> save(UserMod userMod); | ||||
| 
 | ||||
|     Result<UserInfo> delete(SEBServerUser principal, Long id); | ||||
|     /** Use this to delete a user and all its relationships by id | ||||
|      * | ||||
|      * @param id the identifier if the user to delete | ||||
|      * @param archive indicates whether the user and all its relations should be archived (inactive and anonymous) or | ||||
|      *            hard deleted | ||||
|      * @return Result of a collection of all entities that has been deleted (or archived) */ | ||||
|     Result<Collection<Entity>> delete(Long id, boolean archive); | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -34,14 +34,15 @@ import org.springframework.transaction.interceptor.TransactionInterceptor; | |||
| import org.springframework.util.CollectionUtils; | ||||
| 
 | ||||
| import ch.ethz.seb.sebserver.WebSecurityConfig; | ||||
| import ch.ethz.seb.sebserver.gbl.model.Entity; | ||||
| import ch.ethz.seb.sebserver.gbl.model.EntityType; | ||||
| import ch.ethz.seb.sebserver.gbl.model.APIMessage.APIMessageException; | ||||
| import ch.ethz.seb.sebserver.gbl.model.APIMessage.ErrorMessage; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserFilter; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserInfo; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserMod; | ||||
| import ch.ethz.seb.sebserver.gbl.util.Result; | ||||
| import ch.ethz.seb.sebserver.gbl.util.Utils; | ||||
| import ch.ethz.seb.sebserver.webservice.datalayer.APIMessage.APIMessageException; | ||||
| import ch.ethz.seb.sebserver.webservice.datalayer.APIMessage.ErrorMessage; | ||||
| import ch.ethz.seb.sebserver.webservice.datalayer.batis.mapper.RoleRecordDynamicSqlSupport; | ||||
| import ch.ethz.seb.sebserver.webservice.datalayer.batis.mapper.RoleRecordMapper; | ||||
| import ch.ethz.seb.sebserver.webservice.datalayer.batis.mapper.UserRecordDynamicSqlSupport; | ||||
|  | @ -160,7 +161,7 @@ public class UserDaoImpl implements UserDAO { | |||
| 
 | ||||
|     @Override | ||||
|     @Transactional | ||||
|     public Result<UserInfo> save(final SEBServerUser principal, final UserMod userMod) { | ||||
|     public Result<UserInfo> save(final UserMod userMod) { | ||||
|         if (userMod == null) { | ||||
|             return Result.ofError(new NullPointerException("userMod has null-reference")); | ||||
|         } | ||||
|  | @ -168,11 +169,9 @@ public class UserDaoImpl implements UserDAO { | |||
|         try { | ||||
| 
 | ||||
|             final UserInfo userInfo = userMod.getUserInfo(); | ||||
|             if (userInfo.uuid != null) { | ||||
|                 return updateUser(userMod); | ||||
|             } else { | ||||
|                 return createNewUser(principal, userMod); | ||||
|             } | ||||
|             return (userInfo.uuid != null) | ||||
|                     ? updateUser(userMod) | ||||
|                     : createNewUser(userMod); | ||||
| 
 | ||||
|         } catch (final Throwable t) { | ||||
|             log.error("Unexpected error while saving User data: ", t); | ||||
|  | @ -183,8 +182,10 @@ public class UserDaoImpl implements UserDAO { | |||
| 
 | ||||
|     @Override | ||||
|     @Transactional | ||||
|     public Result<UserInfo> delete(final SEBServerUser principal, final Long id) { | ||||
|     public Result<Collection<Entity>> delete(final Long id, final boolean force) { | ||||
| 
 | ||||
|         // TODO clarify within discussion about inactivate, archive and delete user related data | ||||
| 
 | ||||
|         return Result.ofError(new RuntimeException("TODO")); | ||||
|     } | ||||
| 
 | ||||
|  | @ -234,7 +235,7 @@ public class UserDaoImpl implements UserDAO { | |||
|                 }); | ||||
|     } | ||||
| 
 | ||||
|     private Result<UserInfo> createNewUser(final SEBServerUser principal, final UserMod userMod) { | ||||
|     private Result<UserInfo> createNewUser(final UserMod userMod) { | ||||
|         final UserInfo userInfo = userMod.getUserInfo(); | ||||
| 
 | ||||
|         if (!userMod.newPasswordMatch()) { | ||||
|  |  | |||
|  | @ -8,6 +8,7 @@ | |||
| 
 | ||||
| package ch.ethz.seb.sebserver.webservice.weblayer.api; | ||||
| 
 | ||||
| import java.util.Arrays; | ||||
| import java.util.Collection; | ||||
| import java.util.stream.Collectors; | ||||
| 
 | ||||
|  | @ -25,7 +26,8 @@ import org.springframework.web.bind.annotation.ExceptionHandler; | |||
| import org.springframework.web.context.request.WebRequest; | ||||
| import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; | ||||
| 
 | ||||
| import ch.ethz.seb.sebserver.webservice.datalayer.APIMessage; | ||||
| import ch.ethz.seb.sebserver.gbl.model.APIMessage; | ||||
| import ch.ethz.seb.sebserver.gbl.model.APIMessage.APIMessageException; | ||||
| import ch.ethz.seb.sebserver.webservice.servicelayer.authorization.PermissionDeniedException; | ||||
| 
 | ||||
| @Order(Ordered.HIGHEST_PRECEDENCE) | ||||
|  | @ -92,4 +94,14 @@ public class APIExceptionHandler extends ResponseEntityExceptionHandler { | |||
|                 .createErrorResponse(ex.getMessage()); | ||||
|     } | ||||
| 
 | ||||
|     @ExceptionHandler(APIMessageException.class) | ||||
|     public ResponseEntity<Object> handleAPIMessageException( | ||||
|             final APIMessageException ex, | ||||
|             final WebRequest request) { | ||||
| 
 | ||||
|         return new ResponseEntity<>( | ||||
|                 Arrays.asList(ex.getAPIMessage()), | ||||
|                 HttpStatus.BAD_REQUEST); | ||||
|     } | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -13,6 +13,8 @@ import java.util.Collection; | |||
| import java.util.function.Predicate; | ||||
| import java.util.stream.Collectors; | ||||
| 
 | ||||
| import javax.validation.Valid; | ||||
| 
 | ||||
| import org.springframework.context.ApplicationEventPublisher; | ||||
| import org.springframework.security.core.Authentication; | ||||
| import org.springframework.web.bind.annotation.PathVariable; | ||||
|  | @ -29,7 +31,6 @@ import ch.ethz.seb.sebserver.gbl.profile.WebServiceProfile; | |||
| import ch.ethz.seb.sebserver.gbl.util.Result; | ||||
| import ch.ethz.seb.sebserver.webservice.servicelayer.authorization.AuthorizationGrantService; | ||||
| import ch.ethz.seb.sebserver.webservice.servicelayer.authorization.PrivilegeType; | ||||
| import ch.ethz.seb.sebserver.webservice.servicelayer.authorization.SEBServerUser; | ||||
| import ch.ethz.seb.sebserver.webservice.servicelayer.authorization.UserService; | ||||
| import ch.ethz.seb.sebserver.webservice.servicelayer.dao.UserActivityLogDAO; | ||||
| import ch.ethz.seb.sebserver.webservice.servicelayer.dao.UserActivityLogDAO.ActivityType; | ||||
|  | @ -126,33 +127,24 @@ public class UserAccountController { | |||
| 
 | ||||
|     @RequestMapping(value = "/create", method = RequestMethod.PUT) | ||||
|     public UserInfo createUser( | ||||
|             @RequestBody final UserMod userData, | ||||
|             @Valid @RequestBody final UserMod userData, | ||||
|             final Principal principal) { | ||||
| 
 | ||||
|         return _saveUser( | ||||
|                 userData, | ||||
|                 this.userService.extractFromPrincipal(principal), | ||||
|                 PrivilegeType.WRITE) | ||||
|                         .getOrThrow(); | ||||
|         return _saveUser(userData, PrivilegeType.WRITE) | ||||
|                 .getOrThrow(); | ||||
|     } | ||||
| 
 | ||||
|     @RequestMapping(value = "/save", method = RequestMethod.POST) | ||||
|     public UserInfo saveUser( | ||||
|             @RequestBody final UserMod userData, | ||||
|             @Valid @RequestBody final UserMod userData, | ||||
|             final Principal principal) { | ||||
| 
 | ||||
|         return _saveUser( | ||||
|                 userData, | ||||
|                 this.userService.extractFromPrincipal(principal), | ||||
|                 PrivilegeType.MODIFY) | ||||
|                         .getOrThrow(); | ||||
|         return _saveUser(userData, PrivilegeType.MODIFY) | ||||
|                 .getOrThrow(); | ||||
| 
 | ||||
|     } | ||||
| 
 | ||||
|     private Result<UserInfo> _saveUser( | ||||
|             final UserMod userData, | ||||
|             final SEBServerUser admin, | ||||
|             final PrivilegeType privilegeType) { | ||||
|     private Result<UserInfo> _saveUser(final UserMod userData, final PrivilegeType privilegeType) { | ||||
| 
 | ||||
|         final ActivityType actionType = (userData.getUserInfo().uuid == null) | ||||
|                 ? ActivityType.CREATE | ||||
|  | @ -160,12 +152,14 @@ public class UserAccountController { | |||
| 
 | ||||
|         return this.authorizationGrantService | ||||
|                 .checkGrantOnEntity(userData, privilegeType) | ||||
|                 .flatMap(data -> this.userDao.save(admin, data)) | ||||
|                 .flatMap(this.userDao::save) | ||||
|                 .flatMap(userInfo -> this.userActivityLogDAO.logUserActivity(actionType, userInfo)) | ||||
|                 .flatMap(userInfo -> { | ||||
|                     // handle password change; revoke access tokens if password has changed | ||||
|                     this.applicationEventPublisher.publishEvent( | ||||
|                             new RevokeTokenEndpoint.RevokeTokenEvent(this, userInfo.userName)); | ||||
|                     if (userData.passwordChangeRequest() && userData.newPasswordMatch()) { | ||||
|                         this.applicationEventPublisher.publishEvent( | ||||
|                                 new RevokeTokenEndpoint.RevokeTokenEvent(this, userInfo.userName)); | ||||
|                     } | ||||
|                     return Result.of(userInfo); | ||||
|                 }); | ||||
|     } | ||||
|  |  | |||
|  | @ -54,7 +54,7 @@ public class RevokeTokenEndpoint { | |||
|     } | ||||
| 
 | ||||
|     @EventListener(RevokeTokenEvent.class) | ||||
|     private void revokeAccessToken(final RevokeTokenEvent event) { | ||||
|     void revokeAccessToken(final RevokeTokenEvent event) { | ||||
|         final String clientId = this.adminAPIClientDetails.getClientId(); | ||||
|         final Collection<OAuth2AccessToken> tokens = this.tokenStore | ||||
|                 .findTokensByClientIdAndUserName(clientId, event.userName); | ||||
|  |  | |||
|  | @ -28,6 +28,7 @@ import org.springframework.test.context.jdbc.Sql; | |||
| 
 | ||||
| import com.fasterxml.jackson.core.type.TypeReference; | ||||
| 
 | ||||
| import ch.ethz.seb.sebserver.gbl.model.APIMessage; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserActivityLog; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserFilter; | ||||
| import ch.ethz.seb.sebserver.gbl.model.user.UserInfo; | ||||
|  | @ -207,7 +208,7 @@ public class UserAPITest extends AdministrationAPIIntegrationTest { | |||
|                 null, 1L, "NewTestUser", "NewTestUser", | ||||
|                 "", true, Locale.CANADA, DateTimeZone.UTC, | ||||
|                 new HashSet<>(Arrays.asList(UserRole.EXAM_ADMIN.name()))); | ||||
|         final UserMod newUser = new UserMod(userInfo, "123", "123"); | ||||
|         final UserMod newUser = new UserMod(userInfo, "12345678", "12345678"); | ||||
|         final String newUserJson = this.jsonMapper.writeValueAsString(newUser); | ||||
| 
 | ||||
|         final String token = getSebAdminAccess(); | ||||
|  | @ -347,7 +348,7 @@ public class UserAPITest extends AdministrationAPIIntegrationTest { | |||
|                 null, 2L, "NewTestUser", "NewTestUser", | ||||
|                 "", true, Locale.CANADA, DateTimeZone.UTC, | ||||
|                 new HashSet<>(Arrays.asList(UserRole.EXAM_ADMIN.name()))); | ||||
|         final UserMod newUser = new UserMod(userInfo, "123", "123"); | ||||
|         final UserMod newUser = new UserMod(userInfo, "12345678", "12345678"); | ||||
|         final String newUserJson = this.jsonMapper.writeValueAsString(newUser); | ||||
| 
 | ||||
|         final String token = getAdminInstitution1Access(); | ||||
|  | @ -372,7 +373,7 @@ public class UserAPITest extends AdministrationAPIIntegrationTest { | |||
|                 null, 2L, "NewTestUser", "NewTestUser", | ||||
|                 "", true, Locale.CANADA, DateTimeZone.UTC, | ||||
|                 new HashSet<>(Arrays.asList(UserRole.EXAM_ADMIN.name()))); | ||||
|         final UserMod newUser = new UserMod(userInfo, "123", "123"); | ||||
|         final UserMod newUser = new UserMod(userInfo, "12345678", "12345678"); | ||||
|         final String newUserJson = this.jsonMapper.writeValueAsString(newUser); | ||||
| 
 | ||||
|         final String token = getExamAdmin1(); | ||||
|  | @ -441,6 +442,61 @@ public class UserAPITest extends AdministrationAPIIntegrationTest { | |||
|                 .andReturn().getResponse().getContentAsString(); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void modifyUserPasswordInvalidPasswords() throws Exception { | ||||
|         final String sebAdminToken = getSebAdminAccess(); | ||||
|         final UserInfo examAdmin1 = this.jsonMapper.readValue( | ||||
|                 this.mockMvc.perform(get(this.endpoint + RestAPI.ENDPOINT_USER_ACCOUNT + "/user4") | ||||
|                         .header("Authorization", "Bearer " + sebAdminToken)) | ||||
|                         .andExpect(status().isOk()) | ||||
|                         .andReturn().getResponse().getContentAsString(), | ||||
|                 new TypeReference<UserInfo>() { | ||||
|                 }); | ||||
| 
 | ||||
|         // must be longer then 8 chars | ||||
|         UserMod modifiedUser = new UserMod( | ||||
|                 UserInfo.of(examAdmin1), | ||||
|                 "new", | ||||
|                 "new"); | ||||
|         String modifiedUserJson = this.jsonMapper.writeValueAsString(modifiedUser); | ||||
| 
 | ||||
|         List<APIMessage> messages = this.jsonMapper.readValue( | ||||
|                 this.mockMvc.perform(post(this.endpoint + RestAPI.ENDPOINT_USER_ACCOUNT + "/save") | ||||
|                         .header("Authorization", "Bearer " + sebAdminToken) | ||||
|                         .contentType(MediaType.APPLICATION_JSON_UTF8) | ||||
|                         .content(modifiedUserJson)) | ||||
|                         .andExpect(status().isBadRequest()) | ||||
|                         .andReturn().getResponse().getContentAsString(), | ||||
|                 new TypeReference<List<APIMessage>>() { | ||||
|                 }); | ||||
| 
 | ||||
|         assertNotNull(messages); | ||||
|         assertTrue(1 == messages.size()); | ||||
|         assertEquals("1200", messages.get(0).messageCode); | ||||
|         assertEquals("[user, password, size, 8, 255, new]", String.valueOf(messages.get(0).getAttributes())); | ||||
| 
 | ||||
|         // wrong password retype | ||||
|         modifiedUser = new UserMod( | ||||
|                 UserInfo.of(examAdmin1), | ||||
|                 "12345678", | ||||
|                 "87654321"); | ||||
|         modifiedUserJson = this.jsonMapper.writeValueAsString(modifiedUser); | ||||
| 
 | ||||
|         messages = this.jsonMapper.readValue( | ||||
|                 this.mockMvc.perform(post(this.endpoint + RestAPI.ENDPOINT_USER_ACCOUNT + "/save") | ||||
|                         .header("Authorization", "Bearer " + sebAdminToken) | ||||
|                         .contentType(MediaType.APPLICATION_JSON_UTF8) | ||||
|                         .content(modifiedUserJson)) | ||||
|                         .andExpect(status().isBadRequest()) | ||||
|                         .andReturn().getResponse().getContentAsString(), | ||||
|                 new TypeReference<List<APIMessage>>() { | ||||
|                 }); | ||||
| 
 | ||||
|         assertNotNull(messages); | ||||
|         assertTrue(1 == messages.size()); | ||||
|         assertEquals("1300", messages.get(0).messageCode); | ||||
|     } | ||||
| 
 | ||||
|     private UserInfo getUserInfo(final String name, final Collection<UserInfo> infos) { | ||||
|         return infos | ||||
|                 .stream() | ||||
|  |  | |||
		Loading…
	
	Add table
		
		Reference in a new issue
	
	 anhefti
						anhefti