From 1b6f9fb44a445e9c22108ec2de2992af8914289c Mon Sep 17 00:00:00 2001 From: lukaskabc Date: Mon, 22 Jul 2024 17:26:43 +0200 Subject: [PATCH 1/9] [Enhancement #482] backend endpoint --- .../dao/PasswordChangeRequestDao.java | 15 +- .../AdminBasedRegistrationController.java | 24 +- .../termit/rest/PasswordChangeController.java | 13 +- .../business/PasswordChangeService.java | 77 ----- .../termit/service/business/UserService.java | 90 +++++- .../notification/PasswordChangeNotifier.java | 31 +- ...asswordChangeRequestRepositoryService.java | 4 +- .../resources/template/cs/password-create.vm | 24 ++ .../resources/template/en/password-create.vm | 24 ++ .../kbss/termit/environment/Environment.java | 1 + .../environment/config/TestServiceConfig.java | 9 +- .../dao/PasswordChangeRequestDaoTest.java | 2 +- .../AdminBasedRegistrationControllerTest.java | 77 ++++- .../rest/PasswordChangeControllerTest.java | 18 +- .../business/PasswordChangeServiceTest.java | 205 ------------ .../service/business/UserServiceTest.java | 302 +++++++++++++++++- 16 files changed, 574 insertions(+), 342 deletions(-) delete mode 100644 src/main/java/cz/cvut/kbss/termit/service/business/PasswordChangeService.java create mode 100644 src/main/resources/template/cs/password-create.vm create mode 100644 src/main/resources/template/en/password-create.vm delete mode 100644 src/test/java/cz/cvut/kbss/termit/service/business/PasswordChangeServiceTest.java diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDao.java b/src/main/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDao.java index 62353caa4..1cf181959 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDao.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDao.java @@ -3,7 +3,7 @@ import cz.cvut.kbss.jopa.model.EntityManager; import cz.cvut.kbss.termit.exception.PersistenceException; import cz.cvut.kbss.termit.model.PasswordChangeRequest; -import cz.cvut.kbss.termit.util.Configuration; +import cz.cvut.kbss.termit.model.UserAccount; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Repository; @@ -13,19 +13,16 @@ @Repository public class PasswordChangeRequestDao extends BaseDao { - private final Configuration.Persistence persistenceConfig; - @Autowired - public PasswordChangeRequestDao(EntityManager em, Configuration configuration) { + public PasswordChangeRequestDao(EntityManager em) { super(PasswordChangeRequest.class, em); - this.persistenceConfig = configuration.getPersistence(); } - public List findAllByUsername(String username) { - Objects.requireNonNull(username); + public List findAllByUserAccount(UserAccount userAccount) { + Objects.requireNonNull(userAccount); try { - return em.createQuery("SELECT DISTINCT t FROM " + type.getSimpleName() + " t WHERE t.userAccount.username = :username", type) - .setParameter("username", username, persistenceConfig.getLanguage()) + return em.createQuery("SELECT DISTINCT t FROM " + type.getSimpleName() + " t WHERE t.userAccount = :userAccount", type) + .setParameter("userAccount", userAccount) .getResultList(); } catch (RuntimeException e) { throw new PersistenceException(e); diff --git a/src/main/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationController.java b/src/main/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationController.java index c0a6c03af..2bcba08f8 100644 --- a/src/main/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationController.java +++ b/src/main/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationController.java @@ -21,16 +21,20 @@ import cz.cvut.kbss.termit.model.UserAccount; import cz.cvut.kbss.termit.security.SecurityConstants; import cz.cvut.kbss.termit.service.business.UserService; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.security.SecurityRequirement; +import io.swagger.v3.oas.annotations.tags.Tag; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.context.annotation.Profile; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; @@ -41,12 +45,12 @@ * Available only if internal security is used. */ @ConditionalOnProperty(prefix = "termit.security", name = "provider", havingValue = "internal", matchIfMissing = true) +@Tag(name = "Admin User Registration", description = "Allows admins to register new users.") @RestController @RequestMapping("/users") -@Profile("admin-registration-only") public class AdminBasedRegistrationController { - private static final Logger LOG = LoggerFactory.getLogger(FreeRegistrationController.class); + private static final Logger LOG = LoggerFactory.getLogger(AdminBasedRegistrationController.class); private final UserService userService; @@ -56,11 +60,17 @@ public AdminBasedRegistrationController(UserService userService) { LOG.debug("Instantiating admin-based registration controller."); } + @Operation(security = {@SecurityRequirement(name="bearer-key")}, + description = "Creates a new user account. If the password is blank, the account is locked, and an email will be sent to the new user with a link to create a password.") + @ApiResponses({ + @ApiResponse(responseCode = "201", description = "User created"), + @ApiResponse(responseCode = "409", description = "User data are invalid") + }) @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") - @PostMapping(consumes = {MediaType.APPLICATION_JSON_VALUE, JsonLd.MEDIA_TYPE}) + @PutMapping(consumes = {MediaType.APPLICATION_JSON_VALUE, JsonLd.MEDIA_TYPE}) public ResponseEntity createUser(@RequestBody UserAccount user) { - userService.persist(user); - LOG.info("User {} successfully registered.", user); + userService.adminCreateUser(user); + LOG.info("User {} successfully registered by {}.", user, userService.getCurrent().getUsername()); return new ResponseEntity<>(HttpStatus.CREATED); } } diff --git a/src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java b/src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java index 8c800e732..e00b5e44b 100644 --- a/src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java +++ b/src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java @@ -2,7 +2,7 @@ import cz.cvut.kbss.jsonld.JsonLd; import cz.cvut.kbss.termit.dto.PasswordChangeDto; -import cz.cvut.kbss.termit.service.business.PasswordChangeService; +import cz.cvut.kbss.termit.service.business.UserService; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.responses.ApiResponse; @@ -30,11 +30,12 @@ public class PasswordChangeController { private static final Logger LOG = LoggerFactory.getLogger(PasswordChangeController.class); - private final PasswordChangeService passwordChangeService; + private final UserService userService; @Autowired - public PasswordChangeController(PasswordChangeService passwordChangeService) { - this.passwordChangeService = passwordChangeService; + public PasswordChangeController(UserService userService) { + this.userService = userService; + LOG.debug("Instantiating password change controller."); } @Operation(description = "Requests a password reset for the specified username.") @@ -47,7 +48,7 @@ public PasswordChangeController(PasswordChangeService passwordChangeService) { public ResponseEntity requestPasswordReset( @Parameter(description = "Username of the user") @RequestBody String username) { LOG.info("Password reset requested for user {}.", username); - passwordChangeService.requestPasswordReset(username); + userService.requestPasswordReset(username); return new ResponseEntity<>(HttpStatus.NO_CONTENT); } @@ -62,7 +63,7 @@ public ResponseEntity changePassword( @Parameter( description = "Token with URI for password reset") @RequestBody PasswordChangeDto passwordChangeDto) { LOG.info("Password change requested with token {}", passwordChangeDto.getToken()); - passwordChangeService.changePassword(passwordChangeDto); + userService.changePassword(passwordChangeDto); return new ResponseEntity<>(HttpStatus.NO_CONTENT); } } diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/PasswordChangeService.java b/src/main/java/cz/cvut/kbss/termit/service/business/PasswordChangeService.java deleted file mode 100644 index ae3e4be16..000000000 --- a/src/main/java/cz/cvut/kbss/termit/service/business/PasswordChangeService.java +++ /dev/null @@ -1,77 +0,0 @@ -package cz.cvut.kbss.termit.service.business; - -import cz.cvut.kbss.termit.dto.PasswordChangeDto; -import cz.cvut.kbss.termit.exception.AuthorizationException; -import cz.cvut.kbss.termit.exception.InvalidPasswordChangeRequestException; -import cz.cvut.kbss.termit.exception.NotFoundException; -import cz.cvut.kbss.termit.model.PasswordChangeRequest; -import cz.cvut.kbss.termit.model.UserAccount; -import cz.cvut.kbss.termit.service.notification.PasswordChangeNotifier; -import cz.cvut.kbss.termit.service.repository.PasswordChangeRequestRepositoryService; -import cz.cvut.kbss.termit.service.repository.UserRepositoryService; -import cz.cvut.kbss.termit.util.Configuration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; - -import java.time.Instant; -import java.util.function.Supplier; - -@Service -public class PasswordChangeService { - private static final Logger LOG = LoggerFactory.getLogger(PasswordChangeService.class); - - public static final String INVALID_TOKEN_ERROR_MESSAGE_ID = "resetPassword.invalidToken"; - - private final Configuration.Security securityConfig; - private final PasswordChangeRequestRepositoryService passwordChangeRequestRepositoryService; - private final UserRepositoryService userRepositoryService; - private final PasswordChangeNotifier passwordChangeNotifier; - - public PasswordChangeService(PasswordChangeRequestRepositoryService passwordChangeRequestRepositoryService, - Configuration configuration, UserRepositoryService userRepositoryService, - PasswordChangeNotifier passwordChangeNotifier) { - this.passwordChangeRequestRepositoryService = passwordChangeRequestRepositoryService; - this.securityConfig = configuration.getSecurity(); - this.userRepositoryService = userRepositoryService; - this.passwordChangeNotifier = passwordChangeNotifier; - } - - @Transactional - public void requestPasswordReset(String username) { - // delete any existing request for the user - passwordChangeRequestRepositoryService.findAllByUsername(username) - .forEach(passwordChangeRequestRepositoryService::remove); - - UserAccount account = userRepositoryService.findByUsername(username) - .orElseThrow(() -> NotFoundException.create(UserAccount.class, username)); - PasswordChangeRequest request = passwordChangeRequestRepositoryService.create(account); - passwordChangeNotifier.sendEmail(request); - } - - private boolean isValid(PasswordChangeRequest request) { - return request.getCreatedAt().plus(securityConfig.getPasswordChangeRequestValidity()).isAfter(Instant.now()); - } - - @Transactional - public void changePassword(PasswordChangeDto passwordChangeDto) { - Supplier exception = () -> new InvalidPasswordChangeRequestException("Invalid or expired password change link", INVALID_TOKEN_ERROR_MESSAGE_ID); - PasswordChangeRequest request = passwordChangeRequestRepositoryService.find(passwordChangeDto.getUri()) - .orElseThrow(exception); - - if (!request.getToken().equals(passwordChangeDto.getToken()) || !isValid(request)) { - throw exception.get(); - } - - UserAccount account = userRepositoryService.find(request.getUserAccount().getUri()) - .orElseThrow(exception); - - passwordChangeRequestRepositoryService.remove(request); - - account.setPassword(passwordChangeDto.getNewPassword()); - userRepositoryService.update(account); - - LOG.info("Password changed for user {}.", account.getUsername()); - } -} diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/UserService.java b/src/main/java/cz/cvut/kbss/termit/service/business/UserService.java index b947cfb2e..486056985 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/UserService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/UserService.java @@ -17,19 +17,25 @@ */ package cz.cvut.kbss.termit.service.business; +import cz.cvut.kbss.termit.dto.PasswordChangeDto; import cz.cvut.kbss.termit.dto.RdfsResource; import cz.cvut.kbss.termit.dto.mapper.DtoMapper; import cz.cvut.kbss.termit.event.LoginAttemptsThresholdExceeded; import cz.cvut.kbss.termit.exception.AuthorizationException; +import cz.cvut.kbss.termit.exception.InvalidPasswordChangeRequestException; import cz.cvut.kbss.termit.exception.NotFoundException; import cz.cvut.kbss.termit.exception.UnsupportedOperationException; import cz.cvut.kbss.termit.exception.ValidationException; +import cz.cvut.kbss.termit.model.PasswordChangeRequest; import cz.cvut.kbss.termit.model.UserAccount; import cz.cvut.kbss.termit.model.UserRole; import cz.cvut.kbss.termit.rest.dto.UserUpdateDto; +import cz.cvut.kbss.termit.service.notification.PasswordChangeNotifier; +import cz.cvut.kbss.termit.service.repository.PasswordChangeRequestRepositoryService; import cz.cvut.kbss.termit.service.repository.UserRepositoryService; import cz.cvut.kbss.termit.service.repository.UserRoleRepositoryService; import cz.cvut.kbss.termit.service.security.SecurityUtils; +import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Utils; import cz.cvut.kbss.termit.util.Vocabulary; import org.jetbrains.annotations.NotNull; @@ -42,9 +48,12 @@ import org.springframework.transaction.annotation.Transactional; import java.net.URI; +import java.time.Instant; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.UUID; +import java.util.function.Supplier; import java.util.stream.Collectors; /** @@ -54,6 +63,7 @@ public class UserService { private static final Logger LOG = LoggerFactory.getLogger(UserService.class); + public static final String INVALID_TOKEN_ERROR_MESSAGE_ID = "resetPassword.invalidToken"; private final UserRepositoryService repositoryService; @@ -65,14 +75,26 @@ public class UserService { private final DtoMapper dtoMapper; + private final Configuration.Security securityConfig; + + private final PasswordChangeRequestRepositoryService passwordChangeRequestRepositoryService; + + private final PasswordChangeNotifier passwordChangeNotifier; + @Autowired public UserService(UserRepositoryService repositoryService, UserRoleRepositoryService userRoleRepositoryService, - AccessControlListService aclService, SecurityUtils securityUtils, DtoMapper dtoMapper) { + AccessControlListService aclService, SecurityUtils securityUtils, DtoMapper dtoMapper, + Configuration configuration, + PasswordChangeRequestRepositoryService passwordChangeRequestRepositoryService, + PasswordChangeNotifier passwordChangeNotifier) { this.repositoryService = repositoryService; this.userRoleRepositoryService = userRoleRepositoryService; this.aclService = aclService; this.securityUtils = securityUtils; this.dtoMapper = dtoMapper; + this.securityConfig = configuration.getSecurity(); + this.passwordChangeRequestRepositoryService = passwordChangeRequestRepositoryService; + this.passwordChangeNotifier = passwordChangeNotifier; } /** @@ -137,6 +159,27 @@ private void updateLastSeen(UserAccount account) { repositoryService.update(account); } + /** + * Persists a new user. + * When a password is null or blank, + * a random password is generated and an email for password creation is sent to the user. + * @param account + */ + @Transactional + public void adminCreateUser(UserAccount account) { + if (account.getPassword() == null || account.getPassword().isBlank()) { + // generate random password + account.setPassword(UUID.randomUUID() + UUID.randomUUID().toString()); + account.lock(); + persist(account); + + PasswordChangeRequest passwordChangeRequest = createPasswordChangeRequest(account); + passwordChangeNotifier.sendCreatePasswordEmail(passwordChangeRequest); + } else { + persist(account); + } + } + /** * Persists the specified user account. * @@ -215,7 +258,7 @@ public void unlock(UserAccount account, String newPassword) { } private void ensureNotOwnAccount(UserAccount account, String operation) { - if (securityUtils.getCurrentUser().equals(account)) { + if (securityUtils.isAuthenticated() && securityUtils.getCurrentUser().equals(account)) { throw new UnsupportedOperationException("Cannot " + operation + " your own account!"); } } @@ -307,4 +350,47 @@ public List getManagedAssets(@NonNull UserAccount user) { return aclService.findAssetsByAgentWithSecurityAccess(user.toUser()).stream() .map(dtoMapper::assetToRdfsResource).collect(Collectors.toList()); } + + private PasswordChangeRequest createPasswordChangeRequest(UserAccount userAccount) { + // delete any existing request for the user + passwordChangeRequestRepositoryService.findAllByUserAccount(userAccount) + .forEach(passwordChangeRequestRepositoryService::remove); + + return passwordChangeRequestRepositoryService.create(userAccount); + } + + @Transactional + public void requestPasswordReset(String username) { + final UserAccount account = repositoryService.findByUsername(username) + .orElseThrow(() -> NotFoundException.create(UserAccount.class, username)); + PasswordChangeRequest request = createPasswordChangeRequest(account); + passwordChangeNotifier.sendPasswordResetEmail(request); + } + + private boolean isValid(PasswordChangeRequest request) { + return request.getCreatedAt().plus(securityConfig.getPasswordChangeRequestValidity()).isAfter(Instant.now()); + } + + /** + * Changes the user's password if there is a valid password change request. + * Unlocks the user account if it is locked. + */ + @Transactional + public void changePassword(PasswordChangeDto passwordChangeDto) { + Supplier exception = () -> new InvalidPasswordChangeRequestException("Invalid or expired password change link", INVALID_TOKEN_ERROR_MESSAGE_ID); + PasswordChangeRequest request = passwordChangeRequestRepositoryService.find(passwordChangeDto.getUri()) + .orElseThrow(exception); + + if (!request.getToken().equals(passwordChangeDto.getToken()) || !isValid(request)) { + throw exception.get(); + } + + UserAccount account = repositoryService.find(request.getUserAccount().getUri()) + .orElseThrow(exception); + + passwordChangeRequestRepositoryService.remove(request); + + unlock(account, passwordChangeDto.getNewPassword()); + LOG.info("Password changed for user {}.", account.getUsername()); + } } diff --git a/src/main/java/cz/cvut/kbss/termit/service/notification/PasswordChangeNotifier.java b/src/main/java/cz/cvut/kbss/termit/service/notification/PasswordChangeNotifier.java index e0e199f35..eea26b9d5 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/notification/PasswordChangeNotifier.java +++ b/src/main/java/cz/cvut/kbss/termit/service/notification/PasswordChangeNotifier.java @@ -16,6 +16,7 @@ @Component public class PasswordChangeNotifier { private static final String PASSWORD_RESET_TEMPLATE = "password-change.vm"; + private static final String CREATE_PASSWORD_TEMPLATE = "password-create.vm"; private final Configuration config; private final MessageComposer messageComposer; @@ -27,7 +28,7 @@ public PasswordChangeNotifier(Configuration configuration, MessageComposer messa this.postman = postman; } - private String buildResetLink(PasswordChangeRequest request) { + private String buildPasswordResetLink(PasswordChangeRequest request) { return UriComponentsBuilder.fromHttpUrl(config.getUrl()) .fragment("/reset-password/" + request.getToken() + "/" + @@ -36,29 +37,43 @@ private String buildResetLink(PasswordChangeRequest request) { ).toUriString(); } + private String buildCreatePasswordLink(PasswordChangeRequest request) { + return UriComponentsBuilder.fromHttpUrl(config.getUrl()) + .fragment("/create-password/" + + request.getToken() + "/" + + URLEncoder.encode(request.getUri().toString(), + StandardCharsets.UTF_8) + ).toUriString(); + } + /** - * Creates message from ${@link #PASSWORD_RESET_TEMPLATE} template. + * Creates a message from specified template and frontend link. * * @param request Password reset request + * @param frontendLink a link to the frontend, mapped as {@code resetLink} for the template + * @param templateName Name of the template * @return message */ - private Message createMessage(PasswordChangeRequest request) { + private Message createMessage(PasswordChangeRequest request, String frontendLink, String templateName) { Map variables = Map.of( - "resetLink", buildResetLink(request), + "resetLink", frontendLink, "username", request.getUserAccount().getUsername(), "validity", config.getSecurity().getPasswordChangeRequestValidity() ); return Message.to(request.getUserAccount().getUsername()) - .content(messageComposer.composeMessage(PASSWORD_RESET_TEMPLATE, variables)) + .content(messageComposer.composeMessage(templateName, variables)) .subject(new MessageFormatter(config.getPersistence().getLanguage()).formatMessage( "password-change.email.subject")) .build(); } - public void sendEmail(PasswordChangeRequest request) { - Message message = createMessage(request); + public void sendPasswordResetEmail(PasswordChangeRequest request) { + Message message = createMessage(request, buildPasswordResetLink(request), PASSWORD_RESET_TEMPLATE); postman.sendMessage(message); } - + public void sendCreatePasswordEmail(PasswordChangeRequest request) { + Message message = createMessage(request, buildCreatePasswordLink(request), CREATE_PASSWORD_TEMPLATE); + postman.sendMessage(message); + } } diff --git a/src/main/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java b/src/main/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java index e3b861c22..324db884f 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java @@ -43,8 +43,8 @@ public PasswordChangeRequest create(UserAccount userAccount) { return request; } - public List findAllByUsername(String username) { - List loaded = passwordChangeRequestDao.findAllByUsername(username); + public List findAllByUserAccount(UserAccount userAccount) { + List loaded = passwordChangeRequestDao.findAllByUserAccount(userAccount); return loaded.stream().map(this::postLoad).toList(); } diff --git a/src/main/resources/template/cs/password-create.vm b/src/main/resources/template/cs/password-create.vm new file mode 100644 index 000000000..b4ddfad0f --- /dev/null +++ b/src/main/resources/template/cs/password-create.vm @@ -0,0 +1,24 @@ + + +Dobrý den, + +

+ Byli jste administrátorem zaregistrováni do systému TermIt. + Na odkazu níže si prosím vytvořte heslo. +

+

+ Vytvořit heslo +

+

+ Odkaz je platný ${validity} hodin. +

+ +S pozdravem,
+TermIt Postman +
+

+ -------------------------------------------------------------------------------
+ Toto je automatická zpráva. Prosím, neodpovídejte na ni. +

+ + diff --git a/src/main/resources/template/en/password-create.vm b/src/main/resources/template/en/password-create.vm new file mode 100644 index 000000000..244c5b191 --- /dev/null +++ b/src/main/resources/template/en/password-create.vm @@ -0,0 +1,24 @@ + + +Hello, + +

+ You have been registered in the TermIt system by the administrator. + Please use the link below to create a password. +

+

+ Create password +

+

+ The link is valid for ${validity} hours. +

+ +Kind regards,
+TermIt Postman +
+

+ -------------------------------------------------------------------------------
+ This is an automated message. Please do not reply to it. +

+ + diff --git a/src/test/java/cz/cvut/kbss/termit/environment/Environment.java b/src/test/java/cz/cvut/kbss/termit/environment/Environment.java index a19c77b98..3b76b438f 100644 --- a/src/test/java/cz/cvut/kbss/termit/environment/Environment.java +++ b/src/test/java/cz/cvut/kbss/termit/environment/Environment.java @@ -29,6 +29,7 @@ import cz.cvut.kbss.termit.model.UserAccount; import cz.cvut.kbss.termit.security.model.AuthenticationToken; import cz.cvut.kbss.termit.security.model.TermItUserDetails; +import cz.cvut.kbss.termit.service.security.SecurityUtils; import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Constants; import cz.cvut.kbss.termit.util.Vocabulary; diff --git a/src/test/java/cz/cvut/kbss/termit/environment/config/TestServiceConfig.java b/src/test/java/cz/cvut/kbss/termit/environment/config/TestServiceConfig.java index 973b1424f..17fdc0458 100644 --- a/src/test/java/cz/cvut/kbss/termit/environment/config/TestServiceConfig.java +++ b/src/test/java/cz/cvut/kbss/termit/environment/config/TestServiceConfig.java @@ -34,17 +34,21 @@ import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.mail.javamail.JavaMailSender; +import org.springframework.mail.javamail.JavaMailSenderImpl; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; import org.springframework.web.client.RestTemplate; +import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.Set; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @TestConfiguration @ComponentScan(basePackages = "cz.cvut.kbss.termit.service") @@ -94,7 +98,10 @@ public ClassPathResource termStatesLanguageFile() { @Bean public JavaMailSender javaMailSender() { - return mock(JavaMailSender.class); + JavaMailSender sender = mock(JavaMailSenderImpl.class); + when(sender.createMimeMessage()).thenCallRealMethod(); + when(sender.createMimeMessage(any(InputStream.class))).thenCallRealMethod(); + return sender; } @Bean diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java index b756e674b..dad4147bb 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java @@ -46,7 +46,7 @@ void findAllByUsernameReturnsAllResults() { transactional(() -> em.persist(passwordChangeRequest)); transactional(() -> em.persist(secondPasswordChangeRequest)); - final List result = sut.findAllByUsername(user.getUsername()); + final List result = sut.findAllByUserAccount(user); assertTrue(result.stream().anyMatch(r -> passwordChangeRequest.getUri().equals(r.getUri()))); assertTrue(result.stream().anyMatch(r -> secondPasswordChangeRequest.getUri().equals(r.getUri()))); assertEquals(2, result.size()); diff --git a/src/test/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationControllerTest.java b/src/test/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationControllerTest.java index 18ab1b6c3..f2554624c 100644 --- a/src/test/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationControllerTest.java +++ b/src/test/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationControllerTest.java @@ -19,29 +19,54 @@ import cz.cvut.kbss.termit.environment.Environment; import cz.cvut.kbss.termit.environment.Generator; +import cz.cvut.kbss.termit.environment.config.TestPersistenceConfig; import cz.cvut.kbss.termit.environment.config.TestRestSecurityConfig; +import cz.cvut.kbss.termit.environment.config.TestServiceConfig; +import cz.cvut.kbss.termit.model.PasswordChangeRequest; import cz.cvut.kbss.termit.model.UserAccount; import cz.cvut.kbss.termit.service.business.UserService; +import cz.cvut.kbss.termit.service.mail.Postman; +import cz.cvut.kbss.termit.service.notification.PasswordChangeNotifier; +import cz.cvut.kbss.termit.service.security.SecurityUtils; +import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Vocabulary; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; -import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.context.annotation.Import; +import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer; +import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.context.annotation.aspectj.EnableSpringConfigured; import org.springframework.http.MediaType; import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.transaction.annotation.EnableTransactionManagement; import static cz.cvut.kbss.termit.util.Constants.REST_MAPPING_PATH; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @WebMvcTest(AdminBasedRegistrationController.class) -@Import({TestRestSecurityConfig.class}) -@ActiveProfiles(profiles = {"admin-registration-only"}) +@EnableAspectJAutoProxy(proxyTargetClass = true) +@EnableTransactionManagement +@ExtendWith(SpringExtension.class) +@EnableSpringConfigured +@EnableConfigurationProperties({Configuration.class}) +@ContextConfiguration(classes = { + TestRestSecurityConfig.class, + TestPersistenceConfig.class, + TestServiceConfig.class}, initializers = {ConfigDataApplicationContextInitializer.class}) +@ActiveProfiles("test") class AdminBasedRegistrationControllerTest extends BaseControllerTestRunner { private static final String PATH = REST_MAPPING_PATH + "/users"; @@ -49,29 +74,59 @@ class AdminBasedRegistrationControllerTest extends BaseControllerTestRunner { @Autowired private MockMvc mockMvc; - @MockBean + @SpyBean + private SecurityUtils securityUtils; + + @SpyBean + private Postman postman; + + @SpyBean + private PasswordChangeNotifier passwordChangeNotifier; + + @SpyBean private UserService userService; @Test void createUserPersistsUserWhenCalledByAdmin() throws Exception { - final UserAccount admin = Generator.generateUserAccount(); + final UserAccount admin = Generator.generateUserAccountWithPassword(); admin.addType(Vocabulary.s_c_administrator_termitu); Environment.setCurrentUser(admin); - final UserAccount user = Generator.generateUserAccount(); - mockMvc.perform(post(PATH).content(toJson(user)) + when(securityUtils.getCurrentUser()).thenReturn(admin); + userService.persist(admin); + final UserAccount user = Generator.generateUserAccountWithPassword(); + mockMvc.perform(put(PATH).content(toJson(user)) .contentType(MediaType.APPLICATION_JSON_VALUE)) .andExpect(status().isCreated()); - verify(userService).persist(user); + verify(userService).adminCreateUser(user); } @Test void createUserThrowsForbiddenForNonAdminUser() throws Exception { final UserAccount admin = Generator.generateUserAccount(); Environment.setCurrentUser(admin); + when(securityUtils.getCurrentUser()).thenReturn(admin); final UserAccount user = Generator.generateUserAccount(); - mockMvc.perform(post(PATH).content(toJson(user)) + mockMvc.perform(put(PATH).content(toJson(user)) .contentType(MediaType.APPLICATION_JSON_VALUE)) .andExpect(status().isForbidden()); verify(userService, never()).persist(any()); } + + @Test + void createUserSendsEmailWhenPasswordIsEmpty() throws Exception { + final UserAccount admin = Generator.generateUserAccountWithPassword(); + admin.addType(Vocabulary.s_c_administrator_termitu); + Environment.setCurrentUser(admin); + when(securityUtils.getCurrentUser()).thenReturn(admin); + userService.persist(admin); + final UserAccount user = Generator.generateUserAccount(); + mockMvc.perform(put(PATH).content(toJson(user)) + .contentType(MediaType.APPLICATION_JSON_VALUE)) + .andExpect(status().isCreated()); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(PasswordChangeRequest.class); + verify(passwordChangeNotifier).sendCreatePasswordEmail(argumentCaptor.capture()); + assertEquals(user, argumentCaptor.getValue().getUserAccount()); + verify(postman).sendMessage(any()); + } } diff --git a/src/test/java/cz/cvut/kbss/termit/rest/PasswordChangeControllerTest.java b/src/test/java/cz/cvut/kbss/termit/rest/PasswordChangeControllerTest.java index f65173e89..7b45c84cf 100644 --- a/src/test/java/cz/cvut/kbss/termit/rest/PasswordChangeControllerTest.java +++ b/src/test/java/cz/cvut/kbss/termit/rest/PasswordChangeControllerTest.java @@ -6,7 +6,7 @@ import cz.cvut.kbss.termit.exception.InvalidPasswordChangeRequestException; import cz.cvut.kbss.termit.exception.NotFoundException; import cz.cvut.kbss.termit.model.UserAccount; -import cz.cvut.kbss.termit.service.business.PasswordChangeService; +import cz.cvut.kbss.termit.service.business.UserService; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -17,7 +17,7 @@ import java.util.UUID; -import static cz.cvut.kbss.termit.service.business.PasswordChangeService.INVALID_TOKEN_ERROR_MESSAGE_ID; +import static cz.cvut.kbss.termit.service.business.UserService.INVALID_TOKEN_ERROR_MESSAGE_ID; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.refEq; import static org.mockito.Mockito.doThrow; @@ -31,7 +31,7 @@ class PasswordChangeControllerTest extends BaseControllerTestRunner { @Mock - private PasswordChangeService passwordChangeService; + private UserService userService; @InjectMocks private PasswordChangeController sut; @@ -49,7 +49,7 @@ void passwordResetRequestsPasswordReset() throws Exception { .content(userAccount.getUsername()) .contentType(MediaType.TEXT_PLAIN)) .andExpect(status().isNoContent()); - verify(passwordChangeService).requestPasswordReset(userAccount.getUsername()); + verify(userService).requestPasswordReset(userAccount.getUsername()); } @Test @@ -58,14 +58,14 @@ void passwordChangeRequestedInvalidUserNotFoundReturned() throws Exception { final String username = userAccount.getUsername(); doThrow(NotFoundException.create(UserAccount.class, username)) - .when(passwordChangeService).requestPasswordReset(username); + .when(userService).requestPasswordReset(username); mockMvc.perform(post("/password") .content(username) .contentType(MediaType.TEXT_PLAIN)) .andExpect(status().isNotFound()); - verify(passwordChangeService).requestPasswordReset(eq(username)); + verify(userService).requestPasswordReset(eq(username)); } @Test @@ -77,7 +77,7 @@ void passwordChangeChangesPassword() throws Exception { mockMvc.perform(put("/password").content(toJson(dto)).contentType(MediaType.APPLICATION_JSON_VALUE)) .andExpect(status().isNoContent()); - verify(passwordChangeService).changePassword(refEq(dto)); + verify(userService).changePassword(refEq(dto)); } @Test @@ -88,12 +88,12 @@ void passwordChangeRequestedWithInvalidTokenConflictReturned() throws Exception dto.setToken(UUID.randomUUID().toString()); doThrow(new InvalidPasswordChangeRequestException("Invalid or expired password change link", INVALID_TOKEN_ERROR_MESSAGE_ID)) - .when(passwordChangeService).changePassword(refEq(dto)); + .when(userService).changePassword(refEq(dto)); mockMvc.perform(put("/password").content(toJson(dto)).contentType(MediaType.APPLICATION_JSON_VALUE)) .andExpect(jsonPath("messageId").value(INVALID_TOKEN_ERROR_MESSAGE_ID)) .andExpect(status().isConflict()); - verify(passwordChangeService).changePassword(refEq(dto)); + verify(userService).changePassword(refEq(dto)); } } diff --git a/src/test/java/cz/cvut/kbss/termit/service/business/PasswordChangeServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/business/PasswordChangeServiceTest.java deleted file mode 100644 index b008264d5..000000000 --- a/src/test/java/cz/cvut/kbss/termit/service/business/PasswordChangeServiceTest.java +++ /dev/null @@ -1,205 +0,0 @@ -package cz.cvut.kbss.termit.service.business; - -import cz.cvut.kbss.termit.dto.PasswordChangeDto; -import cz.cvut.kbss.termit.environment.Generator; -import cz.cvut.kbss.termit.exception.InvalidPasswordChangeRequestException; -import cz.cvut.kbss.termit.exception.NotFoundException; -import cz.cvut.kbss.termit.model.PasswordChangeRequest; -import cz.cvut.kbss.termit.model.UserAccount; -import cz.cvut.kbss.termit.service.notification.PasswordChangeNotifier; -import cz.cvut.kbss.termit.service.repository.PasswordChangeRequestRepositoryService; -import cz.cvut.kbss.termit.service.repository.UserRepositoryService; -import cz.cvut.kbss.termit.util.Configuration; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Spy; -import org.mockito.junit.jupiter.MockitoExtension; - -import java.time.Instant; -import java.util.List; -import java.util.Optional; -import java.util.UUID; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -@ExtendWith(MockitoExtension.class) -public class PasswordChangeServiceTest { - - @Spy - Configuration configuration = new Configuration(); - - @Mock - PasswordChangeRequestRepositoryService passwordChangeRequestRepositoryService; - - @Mock - UserRepositoryService userRepositoryService; - - @Mock - PasswordChangeNotifier passwordChangeNotifier; - - @InjectMocks - PasswordChangeService sut; - - @Test - void requestPasswordResetRequestCreatedAndEmailSent() { - final UserAccount userAccount = Generator.generateUserAccount(); - final PasswordChangeRequest request = new PasswordChangeRequest(); - - when(passwordChangeRequestRepositoryService.findAllByUsername(userAccount.getUsername())) - .thenReturn(List.of()); - when(userRepositoryService.findByUsername(userAccount.getUsername())) - .thenReturn(Optional.of(userAccount)); - when(passwordChangeRequestRepositoryService.create(userAccount)).thenReturn(request); - - - sut.requestPasswordReset(userAccount.getUsername()); - - verify(passwordChangeRequestRepositoryService).create(userAccount); - verify(passwordChangeNotifier).sendEmail(request); - } - - @Test - void requestPasswordResetSinglePreviousRequestRemoved() { - final UserAccount userAccount = Generator.generateUserAccount(); - final PasswordChangeRequest oldRequest = new PasswordChangeRequest(); - final PasswordChangeRequest request = new PasswordChangeRequest(); - - when(passwordChangeRequestRepositoryService.findAllByUsername(userAccount.getUsername())) - .thenReturn(List.of(oldRequest)); - when(userRepositoryService.findByUsername(userAccount.getUsername())) - .thenReturn(Optional.of(userAccount)); - when(passwordChangeRequestRepositoryService.create(userAccount)).thenReturn(request); - - sut.requestPasswordReset(userAccount.getUsername()); - - verify(passwordChangeRequestRepositoryService).remove(oldRequest); - } - - @Test - void requestPasswordResetAllPreviousRequestsRemoved() { - final UserAccount userAccount = Generator.generateUserAccount(); - final PasswordChangeRequest oldRequest_A = new PasswordChangeRequest(); - final PasswordChangeRequest oldRequest_B = new PasswordChangeRequest(); - final PasswordChangeRequest oldRequest_C = new PasswordChangeRequest(); - final PasswordChangeRequest request = new PasswordChangeRequest(); - - when(passwordChangeRequestRepositoryService.findAllByUsername(userAccount.getUsername())) - .thenReturn(List.of(oldRequest_A, oldRequest_B, oldRequest_C)); - when(userRepositoryService.findByUsername(userAccount.getUsername())) - .thenReturn(Optional.of(userAccount)); - when(passwordChangeRequestRepositoryService.create(userAccount)).thenReturn(request); - - sut.requestPasswordReset(userAccount.getUsername()); - - verify(passwordChangeRequestRepositoryService).remove(oldRequest_A); - verify(passwordChangeRequestRepositoryService).remove(oldRequest_B); - verify(passwordChangeRequestRepositoryService).remove(oldRequest_C); - } - - @Test - void requestPasswordResetInvalidUsernameExceptionThrown() { - final String username = Generator.generateUserAccount().getUsername(); - when(userRepositoryService.findByUsername(username)) - .thenReturn(Optional.empty()); - - assertThrows(NotFoundException.class, () -> - sut.requestPasswordReset(username) - ); - - verify(userRepositoryService).findByUsername(username); - } - - @Test - void changePasswordValidRequestPasswordChanged() { - final UserAccount account = Generator.generateUserAccountWithPassword(); - final String originalPassword = account.getPassword(); - final PasswordChangeRequest request = new PasswordChangeRequest(); - request.setCreatedAt(Instant.now()); - request.setToken(UUID.randomUUID().toString()); - request.setUserAccount(account); - request.setUri(Generator.generateUri()); - - final PasswordChangeDto dto = new PasswordChangeDto(); - dto.setToken(request.getToken()); - dto.setNewPassword(UUID.randomUUID().toString()); - dto.setUri(Generator.generateUri()); - - when(passwordChangeRequestRepositoryService.find(dto.getUri())) - .thenReturn(Optional.of(request)); - when(userRepositoryService.find(account.getUri())) - .thenReturn(Optional.of(account)); - - sut.changePassword(dto); - - verify(passwordChangeRequestRepositoryService).remove(request); - verify(userRepositoryService).update(account); - assertEquals(dto.getNewPassword(), account.getPassword()); - assertNotEquals(originalPassword, account.getPassword()); - } - - @Test - void changePasswordRequestNotFoundExceptionThrown() { - final PasswordChangeDto dto = new PasswordChangeDto(); - dto.setUri(Generator.generateUri()); - - when(passwordChangeRequestRepositoryService.find(dto.getUri())) - .thenReturn(Optional.empty()); - - assertThrows(InvalidPasswordChangeRequestException.class, () -> - sut.changePassword(dto) - ); - verify(passwordChangeRequestRepositoryService).find(dto.getUri()); - } - - @Test - void changePasswordExpiredRequestExceptionThrown() { - final PasswordChangeRequest request = new PasswordChangeRequest(); - request.setCreatedAt(Instant.now().minus(configuration.getSecurity() - .getPasswordChangeRequestValidity()) - .minusNanos(1)); - request.setUri(Generator.generateUri()); - request.setToken(UUID.randomUUID().toString()); - - final PasswordChangeDto dto = new PasswordChangeDto(); - dto.setToken(request.getToken()); - dto.setUri(request.getUri()); - - when(passwordChangeRequestRepositoryService.find(dto.getUri())) - .thenReturn(Optional.of(request)); - - assertThrows(InvalidPasswordChangeRequestException.class, () -> - sut.changePassword(dto) - ); - verify(passwordChangeRequestRepositoryService).find(dto.getUri()); - } - - @Test - void changePasswordValidURINotMatchingTokenExceptionThrown() { - final PasswordChangeRequest request = new PasswordChangeRequest(); - request.setCreatedAt(Instant.now()); - request.setUri(Generator.generateUri()); - request.setToken(UUID.randomUUID().toString()); - - final PasswordChangeDto dto = new PasswordChangeDto(); - dto.setToken("non-existing-token"); - dto.setUri(request.getUri()); - - when(passwordChangeRequestRepositoryService.find(dto.getUri())) - .thenReturn(Optional.of(request)); - - assertThrows(InvalidPasswordChangeRequestException.class, () -> - sut.changePassword(dto) - ); - verify(passwordChangeRequestRepositoryService).find(dto.getUri()); - } - - -} diff --git a/src/test/java/cz/cvut/kbss/termit/service/business/UserServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/business/UserServiceTest.java index 5df647fbd..6a93c52e7 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/business/UserServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/business/UserServiceTest.java @@ -17,38 +17,64 @@ */ package cz.cvut.kbss.termit.service.business; +import cz.cvut.kbss.termit.dto.PasswordChangeDto; import cz.cvut.kbss.termit.dto.RdfsResource; import cz.cvut.kbss.termit.dto.mapper.DtoMapper; import cz.cvut.kbss.termit.environment.Environment; import cz.cvut.kbss.termit.environment.Generator; import cz.cvut.kbss.termit.event.LoginAttemptsThresholdExceeded; import cz.cvut.kbss.termit.exception.AuthorizationException; +import cz.cvut.kbss.termit.exception.InvalidPasswordChangeRequestException; +import cz.cvut.kbss.termit.exception.NotFoundException; import cz.cvut.kbss.termit.exception.UnsupportedOperationException; import cz.cvut.kbss.termit.exception.ValidationException; +import cz.cvut.kbss.termit.model.PasswordChangeRequest; import cz.cvut.kbss.termit.model.User; import cz.cvut.kbss.termit.model.UserAccount; import cz.cvut.kbss.termit.model.UserRole; import cz.cvut.kbss.termit.rest.dto.UserUpdateDto; +import cz.cvut.kbss.termit.service.notification.PasswordChangeNotifier; +import cz.cvut.kbss.termit.service.repository.PasswordChangeRequestRepositoryService; import cz.cvut.kbss.termit.service.repository.UserRepositoryService; import cz.cvut.kbss.termit.service.repository.UserRoleRepositoryService; import cz.cvut.kbss.termit.service.security.SecurityUtils; +import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Vocabulary; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.*; +import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; import java.net.URI; +import java.time.Instant; import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Optional; +import java.util.UUID; import static cz.cvut.kbss.termit.environment.util.ContainsSameEntities.containsSameEntities; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class UserServiceTest { @@ -65,9 +91,19 @@ class UserServiceTest { @Mock private AccessControlListService aclService; + // required as dependency for UserService @Spy private DtoMapper dtoMapper = Environment.getDtoMapper(); + @Spy + Configuration configuration = new Configuration(); + + @Mock + PasswordChangeRequestRepositoryService passwordChangeRequestRepositoryService; + + @Mock + PasswordChangeNotifier passwordChangeNotifier; + @InjectMocks private UserService sut; @@ -161,6 +197,7 @@ void existsChecksForUsernameExistenceInRepositoryService() { @Test void unlockUnlocksUserAccountAndUpdatesItViaRepositoryService() { + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(Generator.generateUserAccount()); final UserAccount account = Generator.generateUserAccount(); account.lock(); @@ -172,6 +209,7 @@ void unlockUnlocksUserAccountAndUpdatesItViaRepositoryService() { @Test void unlockSetsNewPasswordOnAccountSpecifiedAsArgument() { + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(Generator.generateUserAccount()); final UserAccount account = Generator.generateUserAccount(); account.lock(); @@ -186,6 +224,7 @@ void unlockSetsNewPasswordOnAccountSpecifiedAsArgument() { void unlockThrowsUnsupportedOperationExceptionWhenAttemptingToUnlockOwnAccount() { final UserAccount account = Generator.generateUserAccount(); account.lock(); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(account); assertThrows(UnsupportedOperationException.class, () -> sut.unlock(account, "newPassword")); verify(repositoryServiceMock, never()).update(any()); @@ -193,6 +232,7 @@ void unlockThrowsUnsupportedOperationExceptionWhenAttemptingToUnlockOwnAccount() @Test void disableDisablesUserAccountAndUpdatesItViaRepositoryService() { + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(Generator.generateUserAccount()); final UserAccount account = Generator.generateUserAccount(); sut.disable(account); @@ -204,6 +244,7 @@ void disableDisablesUserAccountAndUpdatesItViaRepositoryService() { @Test void disableThrowsUnsupportedOperationExceptionWhenAttemptingToDisableOwnAccount() { final UserAccount account = Generator.generateUserAccount(); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(account); assertThrows(UnsupportedOperationException.class, () -> sut.disable(account)); verify(repositoryServiceMock, never()).update(any()); @@ -211,6 +252,7 @@ void disableThrowsUnsupportedOperationExceptionWhenAttemptingToDisableOwnAccount @Test void enableEnablesUserAccountAndUpdatesItViaRepositoryService() { + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(Generator.generateUserAccount()); final UserAccount account = Generator.generateUserAccount(); account.disable(); @@ -224,6 +266,7 @@ void enableEnablesUserAccountAndUpdatesItViaRepositoryService() { void enableThrowsUnsupportedOperationExceptionWhenAttemptingToEnableOwnAccount() { final UserAccount account = Generator.generateUserAccount(); account.disable(); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(account); assertThrows(UnsupportedOperationException.class, () -> sut.enable(account)); verify(repositoryServiceMock, never()).update(any()); @@ -336,6 +379,7 @@ void updateInvokesRepositoryServiceWhenDataAreValid() { @Test void changeRoleThrowsUnsupportedOperationExceptionWhenAttemptingToChangeOwnRole() { final UserAccount ua = Generator.generateUserAccount(); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(ua); assertThrows(UnsupportedOperationException.class, @@ -352,6 +396,7 @@ void changeRoleReplacesPreviouslyAssignedRoleTypeWithSpecifiedOne() { final UserRole rThree = new UserRole(URI.create(Vocabulary.s_c_omezeny_uzivatel_termitu)); final List roles = Arrays.asList(rOne, rTwo, rThree); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); when(securityUtilsMock.getCurrentUser()).thenReturn(current); when(roleServiceMock.findAll()).thenReturn(roles); sut.changeRole(ua, Vocabulary.s_c_administrator_termitu); @@ -372,4 +417,253 @@ void getManagedAssetsRetrievesManagedAssetsForSpecifiedUserAndReturnsThemAsRdfsR verify(aclService).findAssetsByAgentWithSecurityAccess(ua.toUser()); assertThat(result, containsSameEntities(assets)); } + + + @Test + void requestPasswordResetRequestCreatedAndEmailSent() { + final UserAccount userAccount = Generator.generateUserAccount(); + final PasswordChangeRequest request = new PasswordChangeRequest(); + + when(passwordChangeRequestRepositoryService.findAllByUserAccount(userAccount)) + .thenReturn(List.of()); + when(repositoryServiceMock.findByUsername(userAccount.getUsername())) + .thenReturn(Optional.of(userAccount)); + when(passwordChangeRequestRepositoryService.create(userAccount)).thenReturn(request); + + + sut.requestPasswordReset(userAccount.getUsername()); + + verify(passwordChangeRequestRepositoryService).create(userAccount); + verify(passwordChangeNotifier).sendPasswordResetEmail(request); + } + + @Test + void requestPasswordResetSinglePreviousRequestRemoved() { + final UserAccount userAccount = Generator.generateUserAccount(); + final PasswordChangeRequest oldRequest = new PasswordChangeRequest(); + final PasswordChangeRequest request = new PasswordChangeRequest(); + + when(passwordChangeRequestRepositoryService.findAllByUserAccount(userAccount)) + .thenReturn(List.of(oldRequest)); + when(repositoryServiceMock.findByUsername(userAccount.getUsername())) + .thenReturn(Optional.of(userAccount)); + when(passwordChangeRequestRepositoryService.create(userAccount)).thenReturn(request); + + sut.requestPasswordReset(userAccount.getUsername()); + + verify(passwordChangeRequestRepositoryService).remove(oldRequest); + } + + @Test + void requestPasswordResetAllPreviousRequestsRemoved() { + final UserAccount userAccount = Generator.generateUserAccount(); + final PasswordChangeRequest oldRequest_A = new PasswordChangeRequest(); + final PasswordChangeRequest oldRequest_B = new PasswordChangeRequest(); + final PasswordChangeRequest oldRequest_C = new PasswordChangeRequest(); + final PasswordChangeRequest request = new PasswordChangeRequest(); + + when(passwordChangeRequestRepositoryService.findAllByUserAccount(userAccount)) + .thenReturn(List.of(oldRequest_A, oldRequest_B, oldRequest_C)); + when(repositoryServiceMock.findByUsername(userAccount.getUsername())) + .thenReturn(Optional.of(userAccount)); + when(passwordChangeRequestRepositoryService.create(userAccount)).thenReturn(request); + + sut.requestPasswordReset(userAccount.getUsername()); + + verify(passwordChangeRequestRepositoryService).remove(oldRequest_A); + verify(passwordChangeRequestRepositoryService).remove(oldRequest_B); + verify(passwordChangeRequestRepositoryService).remove(oldRequest_C); + } + + @Test + void requestPasswordResetInvalidUsernameExceptionThrown() { + final String username = Generator.generateUserAccount().getUsername(); + when(repositoryServiceMock.findByUsername(username)) + .thenReturn(Optional.empty()); + + assertThrows(NotFoundException.class, () -> + sut.requestPasswordReset(username) + ); + + verify(repositoryServiceMock).findByUsername(username); + } + + @Test + void changePasswordValidRequestPasswordChanged() { + final UserAccount account = Generator.generateUserAccountWithPassword(); + final String originalPassword = account.getPassword(); + final PasswordChangeRequest request = new PasswordChangeRequest(); + request.setCreatedAt(Instant.now()); + request.setToken(UUID.randomUUID().toString()); + request.setUserAccount(account); + request.setUri(Generator.generateUri()); + + final PasswordChangeDto dto = new PasswordChangeDto(); + dto.setToken(request.getToken()); + dto.setNewPassword(UUID.randomUUID().toString()); + dto.setUri(Generator.generateUri()); + + when(passwordChangeRequestRepositoryService.find(dto.getUri())) + .thenReturn(Optional.of(request)); + when(repositoryServiceMock.find(account.getUri())) + .thenReturn(Optional.of(account)); + + sut.changePassword(dto); + + verify(passwordChangeRequestRepositoryService).remove(request); + verify(repositoryServiceMock).update(account); + assertEquals(dto.getNewPassword(), account.getPassword()); + assertNotEquals(originalPassword, account.getPassword()); + } + + @Test + void changePasswordRequestNotFoundExceptionThrown() { + final PasswordChangeDto dto = new PasswordChangeDto(); + dto.setUri(Generator.generateUri()); + + when(passwordChangeRequestRepositoryService.find(dto.getUri())) + .thenReturn(Optional.empty()); + + assertThrows(InvalidPasswordChangeRequestException.class, () -> + sut.changePassword(dto) + ); + verify(passwordChangeRequestRepositoryService).find(dto.getUri()); + } + + @Test + void changePasswordExpiredRequestExceptionThrown() { + final PasswordChangeRequest request = new PasswordChangeRequest(); + request.setCreatedAt(Instant.now().minus(configuration.getSecurity() + .getPasswordChangeRequestValidity()) + .minusNanos(1)); + request.setUri(Generator.generateUri()); + request.setToken(UUID.randomUUID().toString()); + + final PasswordChangeDto dto = new PasswordChangeDto(); + dto.setToken(request.getToken()); + dto.setUri(request.getUri()); + + when(passwordChangeRequestRepositoryService.find(dto.getUri())) + .thenReturn(Optional.of(request)); + + assertThrows(InvalidPasswordChangeRequestException.class, () -> + sut.changePassword(dto) + ); + verify(passwordChangeRequestRepositoryService).find(dto.getUri()); + } + + @Test + void changePasswordValidURINotMatchingTokenExceptionThrown() { + final PasswordChangeRequest request = new PasswordChangeRequest(); + request.setCreatedAt(Instant.now()); + request.setUri(Generator.generateUri()); + request.setToken(UUID.randomUUID().toString()); + + final PasswordChangeDto dto = new PasswordChangeDto(); + dto.setToken("non-existing-token"); + dto.setUri(request.getUri()); + + when(passwordChangeRequestRepositoryService.find(dto.getUri())) + .thenReturn(Optional.of(request)); + + assertThrows(InvalidPasswordChangeRequestException.class, () -> + sut.changePassword(dto) + ); + verify(passwordChangeRequestRepositoryService).find(dto.getUri()); + } + + @Test + void changePasswordUnlocksLockedAccount() { + final UserAccount user = Generator.generateUserAccount(); + user.lock(); + + final PasswordChangeRequest request = new PasswordChangeRequest(); + request.setCreatedAt(Instant.now().minusNanos(1)); + request.setUri(Generator.generateUri()); + request.setToken(UUID.randomUUID().toString()); + request.setUserAccount(user); + + final PasswordChangeDto dto = new PasswordChangeDto(); + dto.setToken(request.getToken()); + dto.setNewPassword(UUID.randomUUID().toString()); + dto.setUri(request.getUri()); + + when(passwordChangeRequestRepositoryService.find(dto.getUri())) + .thenReturn(Optional.of(request)); + when(repositoryServiceMock.find(user.getUri())).thenReturn(Optional.of(user)); + + assertTrue(user.isLocked()); + + sut.changePassword(dto); + + assertFalse(user.isLocked()); + verify(passwordChangeRequestRepositoryService).find(dto.getUri()); + verify(repositoryServiceMock).update(user); + } + + + @Test + void adminCreateUserCreatesUserWithPassword() { + final UserAccount user = Generator.generateUserAccountWithPassword(); + final UserAccount admin = Generator.generateUserAccountWithPassword(); + admin.addType(Vocabulary.s_c_administrator_termitu); + + Environment.setCurrentUser(admin); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); + when(securityUtilsMock.getCurrentUser()).thenReturn(admin); + + sut.adminCreateUser(user); + + verifyNoInteractions(passwordChangeNotifier); + verifyNoInteractions(passwordChangeRequestRepositoryService); + verify(repositoryServiceMock).persist(user); + } + + @Test + void adminCreateUserWithoutPasswordSendsEmail() { + final UserAccount user = Generator.generateUserAccount(); + final UserAccount admin = Generator.generateUserAccountWithPassword(); + admin.addType(Vocabulary.s_c_administrator_termitu); + final PasswordChangeRequest request = new PasswordChangeRequest(); + + Environment.setCurrentUser(admin); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); + when(securityUtilsMock.getCurrentUser()).thenReturn(admin); + when(passwordChangeRequestRepositoryService.create(user)).thenReturn(request); + + sut.adminCreateUser(user); + + verify(passwordChangeNotifier).sendCreatePasswordEmail(request); + } + + @Test + void adminCreateUserWithoutPasswordCreatesLockedAccount() { + final UserAccount user = Generator.generateUserAccount(); + final UserAccount admin = Generator.generateUserAccountWithPassword(); + admin.addType(Vocabulary.s_c_administrator_termitu); + + Environment.setCurrentUser(admin); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); + when(securityUtilsMock.getCurrentUser()).thenReturn(admin); + + sut.adminCreateUser(user); + + assertTrue(user.isLocked()); + } + + @Test + void adminCreateUserWithPasswordCreatesUnlockedAccount() { + final UserAccount user = Generator.generateUserAccountWithPassword(); + final UserAccount admin = Generator.generateUserAccountWithPassword(); + admin.addType(Vocabulary.s_c_administrator_termitu); + + Environment.setCurrentUser(admin); + when(securityUtilsMock.isAuthenticated()).thenReturn(true); + when(securityUtilsMock.getCurrentUser()).thenReturn(admin); + + sut.adminCreateUser(user); + + assertFalse(user.isLocked()); + } + } From 64eaa02601af09c94119f86182e4a76713c75203 Mon Sep 17 00:00:00 2001 From: lukaskabc Date: Wed, 24 Jul 2024 10:05:56 +0200 Subject: [PATCH 2/9] PR adjustments --- .../termit/rest/AdminBasedRegistrationController.java | 6 +++--- .../kbss/termit/rest/PasswordChangeController.java | 1 - .../cvut/kbss/termit/service/business/UserService.java | 3 +-- .../PasswordChangeRequestRepositoryService.java | 4 ++-- .../persistence/dao/PasswordChangeRequestDaoTest.java | 8 +++----- .../rest/AdminBasedRegistrationControllerTest.java | 10 +++++----- .../kbss/termit/service/business/UserServiceTest.java | 10 +++++----- 7 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationController.java b/src/main/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationController.java index 2bcba08f8..08e3519a8 100644 --- a/src/main/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationController.java +++ b/src/main/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationController.java @@ -34,7 +34,7 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; @@ -47,7 +47,7 @@ @ConditionalOnProperty(prefix = "termit.security", name = "provider", havingValue = "internal", matchIfMissing = true) @Tag(name = "Admin User Registration", description = "Allows admins to register new users.") @RestController -@RequestMapping("/users") +@RequestMapping("/admin/users") public class AdminBasedRegistrationController { private static final Logger LOG = LoggerFactory.getLogger(AdminBasedRegistrationController.class); @@ -67,7 +67,7 @@ public AdminBasedRegistrationController(UserService userService) { @ApiResponse(responseCode = "409", description = "User data are invalid") }) @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") - @PutMapping(consumes = {MediaType.APPLICATION_JSON_VALUE, JsonLd.MEDIA_TYPE}) + @PostMapping(consumes = {MediaType.APPLICATION_JSON_VALUE, JsonLd.MEDIA_TYPE}) public ResponseEntity createUser(@RequestBody UserAccount user) { userService.adminCreateUser(user); LOG.info("User {} successfully registered by {}.", user, userService.getCurrent().getUsername()); diff --git a/src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java b/src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java index e00b5e44b..ad5cea4ed 100644 --- a/src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java +++ b/src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java @@ -35,7 +35,6 @@ public class PasswordChangeController { @Autowired public PasswordChangeController(UserService userService) { this.userService = userService; - LOG.debug("Instantiating password change controller."); } @Operation(description = "Requests a password reset for the specified username.") diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/UserService.java b/src/main/java/cz/cvut/kbss/termit/service/business/UserService.java index 486056985..a5022f675 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/UserService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/UserService.java @@ -48,7 +48,6 @@ import org.springframework.transaction.annotation.Transactional; import java.net.URI; -import java.time.Instant; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -368,7 +367,7 @@ public void requestPasswordReset(String username) { } private boolean isValid(PasswordChangeRequest request) { - return request.getCreatedAt().plus(securityConfig.getPasswordChangeRequestValidity()).isAfter(Instant.now()); + return request.getCreatedAt().plus(securityConfig.getPasswordChangeRequestValidity()).isAfter(Utils.timestamp()); } /** diff --git a/src/main/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java b/src/main/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java index 324db884f..66e4b1cad 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java @@ -6,10 +6,10 @@ import cz.cvut.kbss.termit.model.UserAccount; import cz.cvut.kbss.termit.persistence.dao.GenericDao; import cz.cvut.kbss.termit.persistence.dao.PasswordChangeRequestDao; +import cz.cvut.kbss.termit.util.Utils; import jakarta.validation.Validator; import org.springframework.stereotype.Service; -import java.time.Instant; import java.util.List; import java.util.UUID; @@ -36,7 +36,7 @@ public PasswordChangeRequest create(UserAccount userAccount) { PasswordChangeRequest request = new PasswordChangeRequest(); request.setUserAccount(userAccount); request.setToken(UUID.randomUUID().toString()); - request.setCreatedAt(Instant.now()); + request.setCreatedAt(Utils.timestamp()); passwordChangeRequestDao.persist(request); postPersist(request); diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java index dad4147bb..d8c84e2a7 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java @@ -4,16 +4,14 @@ import cz.cvut.kbss.termit.environment.Generator; import cz.cvut.kbss.termit.model.PasswordChangeRequest; import cz.cvut.kbss.termit.model.UserAccount; +import cz.cvut.kbss.termit.util.Utils; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import java.time.Instant; import java.util.List; -import java.util.Optional; import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; class PasswordChangeRequestDaoTest extends BaseDaoTestRunner { @@ -41,8 +39,8 @@ void findAllByUsernameReturnsAllResults() { secondPasswordChangeRequest.setToken(ANOTHER_TOKEN); passwordChangeRequest.setUserAccount(user); secondPasswordChangeRequest.setUserAccount(user); - passwordChangeRequest.setCreatedAt(Instant.now()); - secondPasswordChangeRequest.setCreatedAt(Instant.now()); + passwordChangeRequest.setCreatedAt(Utils.timestamp()); + secondPasswordChangeRequest.setCreatedAt(Utils.timestamp()); transactional(() -> em.persist(passwordChangeRequest)); transactional(() -> em.persist(secondPasswordChangeRequest)); diff --git a/src/test/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationControllerTest.java b/src/test/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationControllerTest.java index f2554624c..3c50beb10 100644 --- a/src/test/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationControllerTest.java +++ b/src/test/java/cz/cvut/kbss/termit/rest/AdminBasedRegistrationControllerTest.java @@ -53,7 +53,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @WebMvcTest(AdminBasedRegistrationController.class) @@ -69,7 +69,7 @@ @ActiveProfiles("test") class AdminBasedRegistrationControllerTest extends BaseControllerTestRunner { - private static final String PATH = REST_MAPPING_PATH + "/users"; + private static final String PATH = REST_MAPPING_PATH + "/admin/users"; @Autowired private MockMvc mockMvc; @@ -94,7 +94,7 @@ void createUserPersistsUserWhenCalledByAdmin() throws Exception { when(securityUtils.getCurrentUser()).thenReturn(admin); userService.persist(admin); final UserAccount user = Generator.generateUserAccountWithPassword(); - mockMvc.perform(put(PATH).content(toJson(user)) + mockMvc.perform(post(PATH).content(toJson(user)) .contentType(MediaType.APPLICATION_JSON_VALUE)) .andExpect(status().isCreated()); verify(userService).adminCreateUser(user); @@ -106,7 +106,7 @@ void createUserThrowsForbiddenForNonAdminUser() throws Exception { Environment.setCurrentUser(admin); when(securityUtils.getCurrentUser()).thenReturn(admin); final UserAccount user = Generator.generateUserAccount(); - mockMvc.perform(put(PATH).content(toJson(user)) + mockMvc.perform(post(PATH).content(toJson(user)) .contentType(MediaType.APPLICATION_JSON_VALUE)) .andExpect(status().isForbidden()); verify(userService, never()).persist(any()); @@ -120,7 +120,7 @@ void createUserSendsEmailWhenPasswordIsEmpty() throws Exception { when(securityUtils.getCurrentUser()).thenReturn(admin); userService.persist(admin); final UserAccount user = Generator.generateUserAccount(); - mockMvc.perform(put(PATH).content(toJson(user)) + mockMvc.perform(post(PATH).content(toJson(user)) .contentType(MediaType.APPLICATION_JSON_VALUE)) .andExpect(status().isCreated()); diff --git a/src/test/java/cz/cvut/kbss/termit/service/business/UserServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/business/UserServiceTest.java index 6a93c52e7..f7bf983ba 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/business/UserServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/business/UserServiceTest.java @@ -39,6 +39,7 @@ import cz.cvut.kbss.termit.service.repository.UserRoleRepositoryService; import cz.cvut.kbss.termit.service.security.SecurityUtils; import cz.cvut.kbss.termit.util.Configuration; +import cz.cvut.kbss.termit.util.Utils; import cz.cvut.kbss.termit.util.Vocabulary; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -51,7 +52,6 @@ import org.mockito.junit.jupiter.MockitoExtension; import java.net.URI; -import java.time.Instant; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -493,7 +493,7 @@ void changePasswordValidRequestPasswordChanged() { final UserAccount account = Generator.generateUserAccountWithPassword(); final String originalPassword = account.getPassword(); final PasswordChangeRequest request = new PasswordChangeRequest(); - request.setCreatedAt(Instant.now()); + request.setCreatedAt(Utils.timestamp()); request.setToken(UUID.randomUUID().toString()); request.setUserAccount(account); request.setUri(Generator.generateUri()); @@ -533,7 +533,7 @@ void changePasswordRequestNotFoundExceptionThrown() { @Test void changePasswordExpiredRequestExceptionThrown() { final PasswordChangeRequest request = new PasswordChangeRequest(); - request.setCreatedAt(Instant.now().minus(configuration.getSecurity() + request.setCreatedAt(Utils.timestamp().minus(configuration.getSecurity() .getPasswordChangeRequestValidity()) .minusNanos(1)); request.setUri(Generator.generateUri()); @@ -555,7 +555,7 @@ void changePasswordExpiredRequestExceptionThrown() { @Test void changePasswordValidURINotMatchingTokenExceptionThrown() { final PasswordChangeRequest request = new PasswordChangeRequest(); - request.setCreatedAt(Instant.now()); + request.setCreatedAt(Utils.timestamp()); request.setUri(Generator.generateUri()); request.setToken(UUID.randomUUID().toString()); @@ -578,7 +578,7 @@ void changePasswordUnlocksLockedAccount() { user.lock(); final PasswordChangeRequest request = new PasswordChangeRequest(); - request.setCreatedAt(Instant.now().minusNanos(1)); + request.setCreatedAt(Utils.timestamp().minusMillis(1)); request.setUri(Generator.generateUri()); request.setToken(UUID.randomUUID().toString()); request.setUserAccount(user); From 572e500589c034d2d1881f5821e57ff0b4b7c0fa Mon Sep 17 00:00:00 2001 From: lukaskabc Date: Thu, 25 Jul 2024 09:03:04 +0200 Subject: [PATCH 3/9] [Enhancement kbss-cvut/termit-ui#393] Support to load a label in a specific language --- .../kbss/termit/persistence/dao/DataDao.java | 30 +++++++++++++++++-- .../cvut/kbss/termit/rest/DataController.java | 14 +++++++-- .../notification/MessageAssetFactory.java | 2 +- .../repository/DataRepositoryService.java | 6 ++-- .../kbss/termit/rest/DataControllerTest.java | 4 +-- .../notification/MessageAssetFactoryTest.java | 2 +- 6 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/dao/DataDao.java b/src/main/java/cz/cvut/kbss/termit/persistence/dao/DataDao.java index ded878cee..9d7838fd3 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/dao/DataDao.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/dao/DataDao.java @@ -30,6 +30,7 @@ import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Configuration.Persistence; import cz.cvut.kbss.termit.util.TypeAwareResource; +import jakarta.annotation.Nullable; import org.eclipse.rdf4j.model.Resource; import org.eclipse.rdf4j.model.ValueFactory; import org.eclipse.rdf4j.repository.RepositoryConnection; @@ -41,7 +42,13 @@ import java.io.ByteArrayOutputStream; import java.net.URI; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; import java.util.stream.Collectors; @Repository @@ -139,13 +146,30 @@ public Optional find(URI id) { /** * Gets the {@link RDFS#LABEL} of a resource with the specified identifier. *

- * Note that the label has to have matching language tag or no language tag at all (matching tag is preferred). + * Note that the label has to have language tag matching the configured persistence unit language + * or no language tag at all (matching tag is preferred). * * @param id Resource ({@link RDFS#RESOURCE}) identifier * @return Matching resource identifier (if found) */ public Optional getLabel(URI id) { + return getLabel(id, null); + } + + /** + * Gets the {@link RDFS#LABEL} of a resource with the specified identifier. + *

+ * Note that the label has to have matching language tag or no language tag at all (matching tag is preferred). + * + * @param id Resource ({@link RDFS#RESOURCE}) identifier + * @param language Label language, if null, configured persistence unit language is used instead + * @return Matching resource identifier (if found) + */ + public Optional getLabel(URI id, @Nullable String language) { Objects.requireNonNull(id); + if(language == null) { + language = config.getLanguage(); + } if (!id.isAbsolute()) { return Optional.of(id.toString()); } @@ -159,7 +183,7 @@ public Optional getLabel(URI id) { String.class) .setParameter("x", id).setParameter("has-label", RDFS_LABEL) .setParameter("has-title", URI.create(DC.Terms.TITLE)) - .setParameter("tag", config.getLanguage(), null).getSingleResult()); + .setParameter("tag", language, null).getSingleResult()); } catch (NoResultException | NoUniqueResultException e) { return Optional.empty(); } diff --git a/src/main/java/cz/cvut/kbss/termit/rest/DataController.java b/src/main/java/cz/cvut/kbss/termit/rest/DataController.java index 23570df99..b072fc78c 100644 --- a/src/main/java/cz/cvut/kbss/termit/rest/DataController.java +++ b/src/main/java/cz/cvut/kbss/termit/rest/DataController.java @@ -35,7 +35,12 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; import java.net.URI; import java.util.List; @@ -92,8 +97,11 @@ public RdfsResource getById(@Parameter(description = "Identifier of the resource }) @GetMapping(value = "/label") public String getLabel(@Parameter(description = "Resource identifier.") - @RequestParam("iri") URI id) { - return dataService.getLabel(id).orElseThrow( + @RequestParam("iri") URI id, + @Parameter(description = "Label language") + @RequestParam(value = "language", required = false) String language + ) { + return dataService.getLabel(id, language).orElseThrow( () -> new NotFoundException("Resource with id " + id + " not found or it has no matching label.")); } } diff --git a/src/main/java/cz/cvut/kbss/termit/service/notification/MessageAssetFactory.java b/src/main/java/cz/cvut/kbss/termit/service/notification/MessageAssetFactory.java index f7e40da89..ae143419c 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/notification/MessageAssetFactory.java +++ b/src/main/java/cz/cvut/kbss/termit/service/notification/MessageAssetFactory.java @@ -57,7 +57,7 @@ private MessageLabelExtractor(DataRepositoryService dataService) { @Override public void visitTerm(AbstractTerm term) { - this.label = term.getPrimaryLabel() + " (" + dataService.getLabel(term.getVocabulary()).orElse("") + ")"; + this.label = term.getPrimaryLabel() + " (" + dataService.getLabel(term.getVocabulary(), null).orElse("") + ")"; } @Override diff --git a/src/main/java/cz/cvut/kbss/termit/service/repository/DataRepositoryService.java b/src/main/java/cz/cvut/kbss/termit/service/repository/DataRepositoryService.java index fdbb2e60e..486f82cd5 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/repository/DataRepositoryService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/repository/DataRepositoryService.java @@ -19,6 +19,7 @@ import cz.cvut.kbss.termit.dto.RdfsResource; import cz.cvut.kbss.termit.persistence.dao.DataDao; +import jakarta.annotation.Nullable; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -71,9 +72,10 @@ public void persistProperty(RdfsResource property) { * Gets the label of a resource with the specified identifier. * * @param id Resource identifier + * @param language Label language, if null, configured persistence unit language is used instead * @return Matching resource identifier (if found) */ - public Optional getLabel(URI id) { - return dataDao.getLabel(id); + public Optional getLabel(URI id, @Nullable String language) { + return dataDao.getLabel(id, language); } } diff --git a/src/test/java/cz/cvut/kbss/termit/rest/DataControllerTest.java b/src/test/java/cz/cvut/kbss/termit/rest/DataControllerTest.java index 33771355a..22a26ce6d 100644 --- a/src/test/java/cz/cvut/kbss/termit/rest/DataControllerTest.java +++ b/src/test/java/cz/cvut/kbss/termit/rest/DataControllerTest.java @@ -100,7 +100,7 @@ void getByIdThrowsNotFoundExceptionForUnknownResourceIdentifier() throws Excepti void getLabelReturnsLabelOfResourceWithSpecifiedIdAsString() throws Exception { final URI uri = Generator.generateUri(); final String label = "Test term"; - when(dataServiceMock.getLabel(uri)).thenReturn(Optional.of(label)); + when(dataServiceMock.getLabel(uri, null)).thenReturn(Optional.of(label)); final MvcResult mvcResult = mockMvc.perform(get("/data/label").param("iri", uri.toString())) .andExpect(status().isOk()).andReturn(); assertEquals(label, readValue(mvcResult, String.class)); @@ -109,7 +109,7 @@ void getLabelReturnsLabelOfResourceWithSpecifiedIdAsString() throws Exception { @Test void getLabelThrowsNotFoundExceptionWhenLabelIsNotFound() throws Exception { final URI uri = Generator.generateUri(); - when(dataServiceMock.getLabel(any())).thenReturn(Optional.empty()); + when(dataServiceMock.getLabel(any(), any())).thenReturn(Optional.empty()); mockMvc.perform(get("/data/label").param("iri", uri.toString())).andExpect(status().isNotFound()); } diff --git a/src/test/java/cz/cvut/kbss/termit/service/notification/MessageAssetFactoryTest.java b/src/test/java/cz/cvut/kbss/termit/service/notification/MessageAssetFactoryTest.java index c52936f5b..4f2522d9d 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/notification/MessageAssetFactoryTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/notification/MessageAssetFactoryTest.java @@ -68,7 +68,7 @@ void createAddsTermVocabularyLabelInParenthesesAsMessageAssetLabel() throws Exce final String vocabularyLabel = "Vocabulary " + Generator.randomInt(0, 1000); final Term term = Generator.generateTermWithId(); when(linkBuilder.linkTo(term)).thenReturn(term.getUri().toString()); - when(dataService.getLabel(term.getVocabulary())).thenReturn(Optional.of(vocabularyLabel)); + when(dataService.getLabel(term.getVocabulary(), null)).thenReturn(Optional.of(vocabularyLabel)); // Simulate autowired configuration final Field configField = Term.class.getDeclaredField("config"); configField.setAccessible(true); From 528857e2ea87456890e122ee429696e0ca16bc30 Mon Sep 17 00:00:00 2001 From: lukaskabc Date: Tue, 30 Jul 2024 14:44:58 +0200 Subject: [PATCH 4/9] [Enhancement kbss-cvut/termit-ui#479] Remove lang filter from root term selects --- .../kbss/termit/persistence/dao/TermDao.java | 21 +++++++++++++------ .../termit/service/business/TermService.java | 4 ++++ .../repository/TermRepositoryService.java | 6 ++++++ .../termit/persistence/dao/TermDaoTest.java | 13 ++++++++++++ 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/dao/TermDao.java b/src/main/java/cz/cvut/kbss/termit/persistence/dao/TermDao.java index 07fae2fe1..4000e2b5d 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/dao/TermDao.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/dao/TermDao.java @@ -390,6 +390,8 @@ public List findAllIncludingImported(Vocabulary vocabulary) { /** * Loads a page of root terms (terms without a parent) contained in the specified vocabulary. + *

+ * Terms with a label in the instance language are prepended. * * @param vocabulary Vocabulary whose root terms should be returned * @param pageSpec Page specification @@ -401,13 +403,14 @@ public List findAllRoots(Vocabulary vocabulary, Pageable pageSpec, Coll Objects.requireNonNull(vocabulary); Objects.requireNonNull(pageSpec); TypedQuery query = em.createNativeQuery("SELECT DISTINCT ?term WHERE {" + + "SELECT DISTINCT ?term ?hasLocaleLabel WHERE {" + "GRAPH ?context { " + "?term a ?type ;" + "?hasLabel ?label ." + "?vocabulary ?hasGlossary/?hasTerm ?term ." + - "FILTER (lang(?label) = ?labelLang) ." + + "BIND((lang(?label) = ?labelLang) as ?hasLocaleLabel) ." + "FILTER (?term NOT IN (?included))" + - "}} ORDER BY " + orderSentence("?label"), + "}} ORDER BY DESC(?hasLocaleLabel) " + orderSentence("?label") + "}", TermDto.class); query = setCommonFindAllRootsQueryParams(query, false); try { @@ -453,6 +456,8 @@ private static String r(String string, String from, String to) { /** * Loads a page of root terms (terms without a parent). + *

+ * Terms with a label in the instance language are prepended. * * @param pageSpec Page specification * @param includeTerms Identifiers of terms which should be a part of the result. Optional @@ -462,13 +467,14 @@ private static String r(String string, String from, String to) { public List findAllRoots(Pageable pageSpec, Collection includeTerms) { Objects.requireNonNull(pageSpec); TypedQuery query = em.createNativeQuery("SELECT DISTINCT ?term WHERE {" + + "SELECT DISTINCT ?term ?hasLocaleLabel WHERE {" + "?term a ?type ; " + "?hasLabel ?label . " + "?vocabulary ?hasGlossary/?hasTerm ?term . " + - "FILTER (lang(?label) = ?labelLang) . " + + "BIND((lang(?label) = ?labelLang) as ?hasLocaleLabel) ." + "FILTER (?term NOT IN (?included)) . " + "FILTER NOT EXISTS {?term a ?snapshot .} " + - "} ORDER BY " + orderSentence("?label"), + "} ORDER BY DESC(?hasLocaleLabel) " + orderSentence("?label") + "}", TermDto.class); query = setCommonFindAllRootsQueryParams(query, false); try { @@ -533,6 +539,8 @@ private void recursivelyLoadParentTermSubTerms(TermDto term) { *

* This method basically does a transitive closure of the vocabulary import relationship and retrieves a page of * root terms from this closure. + *

+ * Terms with a label in the instance language are prepended. * * @param vocabulary The last vocabulary in the vocabulary import chain * @param pageSpec Page specification @@ -544,13 +552,14 @@ public List findAllRootsIncludingImports(Vocabulary vocabulary, Pageabl Objects.requireNonNull(vocabulary); Objects.requireNonNull(pageSpec); TypedQuery query = em.createNativeQuery("SELECT DISTINCT ?term WHERE {" + + "SELECT DISTINCT ?term ?hasLocaleLabel WHERE {" + "?term a ?type ;" + "?hasLabel ?label ." + "?vocabulary ?imports* ?parent ." + "?parent ?hasGlossary/?hasTerm ?term ." + - "FILTER (lang(?label) = ?labelLang) ." + + "BIND((lang(?label) = ?labelLang) as ?hasLocaleLabel) ." + "FILTER (?term NOT IN (?included))" + - "} ORDER BY " + orderSentence("?label"), + "} ORDER BY DESC(?hasLocaleLabel) " + orderSentence("?label") + "}", TermDto.class); query = setCommonFindAllRootsQueryParams(query, true); try { diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/TermService.java b/src/main/java/cz/cvut/kbss/termit/service/business/TermService.java index 624e7be13..50e561a1d 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/TermService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/TermService.java @@ -175,6 +175,8 @@ public List findAllRoots(Vocabulary vocabulary, Pageable pageSpec, Coll * Retrieves root terms (terms without parent). *

* The page specification parameter allows configuration of the number of results and their offset. + *

+ * Terms with a label in the instance language are prepended. * * @param pageSpec Paging specification * @param includeTerms Identifiers of terms which should be a part of the result. Optional @@ -191,6 +193,8 @@ public List findAllRoots(Pageable pageSpec, Collection includeTerm *

* Basically, this does a transitive closure over the vocabulary import relationship, starting at the specified * vocabulary, and returns all parent-less terms. + *

+ * Terms with a label in the instance language are prepended. * * @param vocabulary Base vocabulary for the vocabulary import closure * @param pageSpec Page specifying result number and position diff --git a/src/main/java/cz/cvut/kbss/termit/service/repository/TermRepositoryService.java b/src/main/java/cz/cvut/kbss/termit/service/repository/TermRepositoryService.java index 23d5e2171..85fe06927 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/repository/TermRepositoryService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/repository/TermRepositoryService.java @@ -240,6 +240,8 @@ public List findAllIncludingImported(Vocabulary vocabulary) { /** * Finds all root terms (terms without parent term) in the specified vocabulary. + *

+ * Terms with a label in the instance language are prepended. * * @param vocabulary Vocabulary whose terms should be returned * @param pageSpec Page specifying result number and position @@ -255,6 +257,8 @@ public List findAllRoots(Vocabulary vocabulary, Pageable pageSpec, /** * Finds all root terms (terms without parent term). + *

+ * Terms with a label in the instance language are prepended. * * @param pageSpec Page specifying result number and position * @param includeTerms Identifiers of terms which should be a part of the result. Optional @@ -273,6 +277,8 @@ public List findAllRoots(Pageable pageSpec, *

* Basically, this does a transitive closure over the vocabulary import relationship, starting at the specified * vocabulary, and returns all parent-less terms. + *

+ * Terms with a label in the instance language are prepended. * * @param vocabulary Base vocabulary for the vocabulary import closure * @param pageSpec Page specifying result number and position diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java index 9acc6a73f..3b70c160b 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java @@ -743,6 +743,19 @@ void findAllRootsOrdersResultsInLexicographicOrderForCzech() { .collect(Collectors.toList())); } + @Test + void findAllRootsReturnsTermsThatAreMissingDefaultLanguageLabel() { + configuration.getPersistence().setLanguage("cs"); + // these terms will be missing Czech labels + persistTerms("en", "Germany", "China", "Spain", "Syria"); + final List result = sut.findAllRoots(vocabulary, Constants.DEFAULT_PAGE_SPEC, Collections.emptyList()); + assertEquals(4, result.size()); + assertEquals(Arrays + .asList("China", "Germany", "Spain", "Syria"), + result.stream().map(r -> r.getLabel().get("en")) + .toList()); + } + private void persistTerms(String lang, String... labels) { transactional(() -> Arrays.stream(labels).forEach(label -> { final Term parent = Generator.generateTermWithId(); From e61de0e9dbac0eb7bd94b3013d77dcc745c5ac55 Mon Sep 17 00:00:00 2001 From: lukaskabc Date: Wed, 31 Jul 2024 14:02:55 +0200 Subject: [PATCH 5/9] Multilang label ordering --- .../kbss/termit/persistence/dao/TermDao.java | 6 ++-- .../termit/persistence/dao/TermDaoTest.java | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/dao/TermDao.java b/src/main/java/cz/cvut/kbss/termit/persistence/dao/TermDao.java index 4000e2b5d..95d458480 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/dao/TermDao.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/dao/TermDao.java @@ -410,7 +410,7 @@ public List findAllRoots(Vocabulary vocabulary, Pageable pageSpec, Coll "?vocabulary ?hasGlossary/?hasTerm ?term ." + "BIND((lang(?label) = ?labelLang) as ?hasLocaleLabel) ." + "FILTER (?term NOT IN (?included))" + - "}} ORDER BY DESC(?hasLocaleLabel) " + orderSentence("?label") + "}", + "}} ORDER BY DESC(?hasLocaleLabel) lang(?label) " + orderSentence("?label") + "}", TermDto.class); query = setCommonFindAllRootsQueryParams(query, false); try { @@ -474,7 +474,7 @@ public List findAllRoots(Pageable pageSpec, Collection includeTerm "BIND((lang(?label) = ?labelLang) as ?hasLocaleLabel) ." + "FILTER (?term NOT IN (?included)) . " + "FILTER NOT EXISTS {?term a ?snapshot .} " + - "} ORDER BY DESC(?hasLocaleLabel) " + orderSentence("?label") + "}", + "} ORDER BY DESC(?hasLocaleLabel) lang(?label) " + orderSentence("?label") + "}", TermDto.class); query = setCommonFindAllRootsQueryParams(query, false); try { @@ -559,7 +559,7 @@ public List findAllRootsIncludingImports(Vocabulary vocabulary, Pageabl "?parent ?hasGlossary/?hasTerm ?term ." + "BIND((lang(?label) = ?labelLang) as ?hasLocaleLabel) ." + "FILTER (?term NOT IN (?included))" + - "} ORDER BY DESC(?hasLocaleLabel) " + orderSentence("?label") + "}", + "} ORDER BY DESC(?hasLocaleLabel) lang(?label) " + orderSentence("?label") + "}", TermDto.class); query = setCommonFindAllRootsQueryParams(query, true); try { diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java index 3b70c160b..62838ba45 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java @@ -756,6 +756,37 @@ void findAllRootsReturnsTermsThatAreMissingDefaultLanguageLabel() { .toList()); } + /** + * terms should be ordered - by language and then lexicographically, with the default language always first + */ + @Test + void findAllRootsReturnsTermsInMultipleLanguagesWithoutPrimaryLabelInCorrectOrder() { + configuration.getPersistence().setLanguage("cs"); + persistTerms("cs", "Německo", "Čína"); + persistTerms("af", "Duitsland", "Sjina"); + persistTerms("en", "Germany", "China"); + persistTerms("pl", "Niemcy", "Chiny"); + persistTerms("da", "Tyskland", "Kina"); + // lang order is: af, cs, da, en, pl + final List result = sut.findAllRoots(vocabulary, Constants.DEFAULT_PAGE_SPEC, Collections.emptyList()); + + // map results to another language than English if possible + final List labels = result.stream().map(r -> { + final Optional lang = r.getLabel().getLanguages().stream().filter(l -> !l.equals("en")).findAny(); + return r.getLabel().get(lang.orElse("en")); + }).toList(); + + final List expectedOrder = Arrays.asList( + "Čína", "Německo", // Czech as first, its default language + "Duitsland", "Sjina", + "Kina", "Tyskland", + "China", "Germany", + "Chiny", "Niemcy"); + + assertEquals(10, result.size()); + assertEquals(expectedOrder, labels); + } + private void persistTerms(String lang, String... labels) { transactional(() -> Arrays.stream(labels).forEach(label -> { final Term parent = Generator.generateTermWithId(); From 13e146fc0f13252b4d55e8dda2c97bd1bc76c12c Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Mon, 5 Aug 2024 10:40:43 +0200 Subject: [PATCH 6/9] [Fix] Ensure validation message always has a language tag. --- .../cz/cvut/kbss/termit/persistence/validation/Validator.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/validation/Validator.java b/src/main/java/cz/cvut/kbss/termit/persistence/validation/Validator.java index 83405ee3e..caeea219b 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/validation/Validator.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/validation/Validator.java @@ -24,6 +24,7 @@ import cz.cvut.kbss.termit.model.validation.ValidationResult; import cz.cvut.kbss.termit.persistence.context.VocabularyContextMapper; import cz.cvut.kbss.termit.util.Configuration; +import cz.cvut.kbss.termit.util.Constants; import cz.cvut.kbss.termit.util.Utils; import org.apache.jena.rdf.model.Literal; import org.apache.jena.rdf.model.Model; @@ -152,7 +153,8 @@ public List validate(final Collection vocabularyIris) { final MultilingualString messages = new MultilingualString(result.getMessages().stream() .map(RDFNode::asLiteral) .collect(Collectors.toMap( - Literal::getLanguage, + lit -> lit.getLanguage().isBlank() ? + Constants.DEFAULT_LANGUAGE : lit.getLanguage(), Literal::getLexicalForm))); return new ValidationResult() From b0edc7b965c7cf10124399c586d29e0a188747a7 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Mon, 5 Aug 2024 10:41:14 +0200 Subject: [PATCH 7/9] [Fix] Fix accidental DTO usage when removing vocabulary. --- .../cvut/kbss/termit/service/business/VocabularyService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java b/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java index e0374c4f8..ff5ec6430 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java @@ -282,8 +282,9 @@ public void runTextAnalysisOnAllVocabularies() { @Transactional @PreAuthorize("@vocabularyAuthorizationService.canRemove(#asset)") public void remove(Vocabulary asset) { - aclService.findFor(asset).ifPresent(aclService::remove); - repositoryService.remove(asset); + Vocabulary toRemove = repositoryService.findRequired(asset.getUri()); + aclService.findFor(toRemove).ifPresent(aclService::remove); + repositoryService.remove(toRemove); } /** From 10765cbb54f7142a4172d3e308522b3d33eecbf6 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Mon, 5 Aug 2024 11:21:09 +0200 Subject: [PATCH 8/9] [Fix] Fix broken tests. --- .../cz/cvut/kbss/termit/persistence/validation/Validator.java | 2 +- .../java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java | 2 +- .../kbss/termit/service/repository/VocabularyServiceTest.java | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/validation/Validator.java b/src/main/java/cz/cvut/kbss/termit/persistence/validation/Validator.java index caeea219b..2b0b8a767 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/validation/Validator.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/validation/Validator.java @@ -114,7 +114,7 @@ private Model initValidationModel(com.github.sgov.server.Validator validator, St // Currently, only using content rules, not OntoUml, as TermIt does not support adding OntoUml rules validator.getModelRules().stream() .filter(r -> MODEL_RULES_TO_ADD.stream().anyMatch(s -> r.toString().contains(s))) - .collect(Collectors.toList()) + .toList() ); final Model validationModel = com.github.sgov.server.Validator.getRulesModel(rules); loadOverrideRules(validationModel, language); diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java index 62838ba45..6f24abf49 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/dao/TermDaoTest.java @@ -790,7 +790,7 @@ void findAllRootsReturnsTermsInMultipleLanguagesWithoutPrimaryLabelInCorrectOrde private void persistTerms(String lang, String... labels) { transactional(() -> Arrays.stream(labels).forEach(label -> { final Term parent = Generator.generateTermWithId(); - parent.getLabel().set(lang, label); + parent.setLabel(MultilingualString.create(label, lang)); parent.setGlossary(vocabulary.getGlossary().getUri()); vocabulary.getGlossary().addRootTerm(parent); em.merge(vocabulary.getGlossary(), descriptorFactory.glossaryDescriptor(vocabulary)); diff --git a/src/test/java/cz/cvut/kbss/termit/service/repository/VocabularyServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/repository/VocabularyServiceTest.java index e9f76513e..1a2492388 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/repository/VocabularyServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/repository/VocabularyServiceTest.java @@ -297,6 +297,7 @@ void findEnhancesResultWithAccessLevelOfCurrentUser() { @Test void removeRemovesAccessControlList() { final Vocabulary vocabulary = Generator.generateVocabularyWithId(); + when(repositoryService.findRequired(vocabulary.getUri())).thenReturn(vocabulary); final AccessControlList acl = Generator.generateAccessControlList(true); when(aclService.findFor(vocabulary)).thenReturn(Optional.of(acl)); From 548fd6e063d661e06125d5321e4e759b375f112f Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Mon, 5 Aug 2024 11:29:15 +0200 Subject: [PATCH 9/9] [3.1.3] Bump version. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 302be74ac..488fe27c3 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ termit - 3.1.2 + 3.1.3 TermIt Terminology manager based on Semantic Web technologies. ${packaging}