diff --git a/domain/src/main/java/org/fao/geonet/repository/UserRepositoryCustom.java b/domain/src/main/java/org/fao/geonet/repository/UserRepositoryCustom.java index 65e3162a22e..21148980e14 100644 --- a/domain/src/main/java/org/fao/geonet/repository/UserRepositoryCustom.java +++ b/domain/src/main/java/org/fao/geonet/repository/UserRepositoryCustom.java @@ -61,7 +61,7 @@ public interface UserRepositoryCustom { */ @Nonnull List> findAllByGroupOwnerNameAndProfile(@Nonnull Collection metadataIds, - @Nullable Profile profil); + @Nullable Profile profile); /** * Find all the users that own at least one metadata element. diff --git a/domain/src/main/java/org/fao/geonet/repository/UserRepositoryCustomImpl.java b/domain/src/main/java/org/fao/geonet/repository/UserRepositoryCustomImpl.java index 4c8b54ad482..4585548d9fe 100644 --- a/domain/src/main/java/org/fao/geonet/repository/UserRepositoryCustomImpl.java +++ b/domain/src/main/java/org/fao/geonet/repository/UserRepositoryCustomImpl.java @@ -47,36 +47,37 @@ public class UserRepositoryCustomImpl implements UserRepositoryCustom { @PersistenceContext - private EntityManager _entityManager; + private EntityManager entityManager; @Override public User findOne(final String userId) { - return _entityManager.find(User.class, Integer.valueOf(userId)); + return entityManager.find(User.class, Integer.valueOf(userId)); } @Override - public User findOneByEmail(final String email) { - CriteriaBuilder cb = _entityManager.getCriteriaBuilder(); + public User findOneByEmail(@Nonnull final String email) { + CriteriaBuilder cb = entityManager.getCriteriaBuilder(); CriteriaQuery query = cb.createQuery(User.class); Root root = query.from(User.class); Join joinedEmailAddresses = root.join(User_.emailAddresses); // Case in-sensitive email search - query.where( cb.equal(cb.lower(joinedEmailAddresses), email.toLowerCase())); - final List resultList = _entityManager.createQuery(query).getResultList(); + query.where(cb.equal(cb.lower(joinedEmailAddresses), email.toLowerCase())); + query.orderBy(cb.asc(root.get(User_.username))); + final List resultList = entityManager.createQuery(query).getResultList(); if (resultList.isEmpty()) { return null; } if (resultList.size() > 1) { - Log.error(Constants.DOMAIN_LOG_MODULE, "The database is inconsistent. There are multiple users with the email address: " + - email); + Log.error(Constants.DOMAIN_LOG_MODULE, String.format("The database is inconsistent. There are multiple users with the email address: %s", + email)); } return resultList.get(0); } @Override - public User findOneByEmailAndSecurityAuthTypeIsNullOrEmpty(final String email) { - CriteriaBuilder cb = _entityManager.getCriteriaBuilder(); + public User findOneByEmailAndSecurityAuthTypeIsNullOrEmpty(@Nonnull final String email) { + CriteriaBuilder cb = entityManager.getCriteriaBuilder(); CriteriaQuery query = cb.createQuery(User.class); Root root = query.from(User.class); Join joinedEmailAddresses = root.join(User_.emailAddresses); @@ -85,33 +86,44 @@ public User findOneByEmailAndSecurityAuthTypeIsNullOrEmpty(final String email) { query.where(cb.and( // Case in-sensitive email search cb.equal(cb.lower(joinedEmailAddresses), email.toLowerCase()), - cb.or(cb.isNull(authTypePath), cb.equal(cb.trim(authTypePath), "")))); - List results = _entityManager.createQuery(query).getResultList(); + cb.or(cb.isNull(authTypePath), cb.equal(cb.trim(authTypePath), ""))) + ).orderBy(cb.asc(root.get(User_.username))); + List results = entityManager.createQuery(query).getResultList(); if (results.isEmpty()) { return null; } else { + if (results.size() > 1) { + Log.error(Constants.DOMAIN_LOG_MODULE, String.format("The database is inconsistent. There are multiple users with the email address: %s", + email)); + } return results.get(0); } } @Override - public User findOneByUsernameAndSecurityAuthTypeIsNullOrEmpty(final String username) { - CriteriaBuilder cb = _entityManager.getCriteriaBuilder(); + public User findOneByUsernameAndSecurityAuthTypeIsNullOrEmpty(@Nonnull final String username) { + CriteriaBuilder cb = entityManager.getCriteriaBuilder(); CriteriaQuery query = cb.createQuery(User.class); Root root = query.from(User.class); final Path authTypePath = root.get(User_.security).get(UserSecurity_.authType); final Path usernamePath = root.get(User_.username); // Case in-sensitive username search - query.where(cb.and(cb.equal(cb.lower(usernamePath), username.toLowerCase()), cb.or(cb.isNull(authTypePath), cb.equal(cb.trim(authTypePath), "")))); - List results = _entityManager.createQuery(query).getResultList(); - + query.where(cb.and( + cb.equal(cb.lower(usernamePath), username.toLowerCase()), + cb.or(cb.isNull(authTypePath), cb.equal(cb.trim(authTypePath), ""))) + ).orderBy(cb.asc(root.get(User_.username))); + List results = entityManager.createQuery(query).getResultList(); if (results.isEmpty()) { return null; } else { + if (results.size() > 1) { + Log.error(Constants.DOMAIN_LOG_MODULE, String.format("The database is inconsistent. There are multiple users with username: %s", + username)); + } return results.get(0); } } @@ -119,7 +131,7 @@ public User findOneByUsernameAndSecurityAuthTypeIsNullOrEmpty(final String usern @Nonnull @Override public List findDuplicatedUsernamesCaseInsensitive() { - CriteriaBuilder cb = _entityManager.getCriteriaBuilder(); + CriteriaBuilder cb = entityManager.getCriteriaBuilder(); CriteriaQuery query = cb.createQuery(String.class); Root userRoot = query.from(User.class); @@ -127,7 +139,7 @@ public List findDuplicatedUsernamesCaseInsensitive() { query.groupBy(cb.lower(userRoot.get(User_.username))); query.having(cb.gt(cb.count(userRoot), 1)); - return _entityManager.createQuery(query).getResultList(); + return entityManager.createQuery(query).getResultList(); } @Override @@ -143,8 +155,8 @@ public List> findAllByGroupOwnerNameAndProfile(@Nonnull fina } private List> findAllByGroupOwnerNameAndProfileInternal(@Nonnull final Collection metadataIds, - @Nullable final Profile profile, boolean draft) { - CriteriaBuilder cb = _entityManager.getCriteriaBuilder(); + @Nullable final Profile profile, boolean draft) { + CriteriaBuilder cb = entityManager.getCriteriaBuilder(); CriteriaQuery query = cb.createQuery(Tuple.class); Root userRoot = query.from(User.class); @@ -152,22 +164,20 @@ private List> findAllByGroupOwnerNameAndProfileInternal(@Non Predicate metadataPredicate; Predicate ownerPredicate; - Root metadataRoot = null; - Root metadataDraftRoot = null; if (!draft) { - metadataRoot = query.from(Metadata.class); + Root metadataRoot = query.from(Metadata.class); query.multiselect(metadataRoot.get(Metadata_.id), userRoot); metadataPredicate = metadataRoot.get(Metadata_.id).in(metadataIds); ownerPredicate = cb.equal(metadataRoot.get(Metadata_.sourceInfo).get(MetadataSourceInfo_.groupOwner), userGroupRoot.get(UserGroup_.id).get(UserGroupId_.groupId)); } else { - metadataDraftRoot = query.from(MetadataDraft.class); - query.multiselect(metadataDraftRoot.get(MetadataDraft_.id), userRoot); - metadataPredicate = metadataDraftRoot.get(Metadata_.id).in(metadataIds); + Root metadataRoot = query.from(MetadataDraft.class); + query.multiselect(metadataRoot.get(MetadataDraft_.id), userRoot); + metadataPredicate = metadataRoot.get(MetadataDraft_.id).in(metadataIds); - ownerPredicate = cb.equal(metadataDraftRoot.get(Metadata_.sourceInfo).get(MetadataSourceInfo_.groupOwner), + ownerPredicate = cb.equal(metadataRoot.get(MetadataDraft_.sourceInfo).get(MetadataSourceInfo_.groupOwner), userGroupRoot.get(UserGroup_.id).get(UserGroupId_.groupId)); } @@ -186,7 +196,7 @@ private List> findAllByGroupOwnerNameAndProfileInternal(@Non List> results = new ArrayList<>(); - for (Tuple result : _entityManager.createQuery(query).getResultList()) { + for (Tuple result : entityManager.createQuery(query).getResultList()) { Integer mdId = (Integer) result.get(0); User user = (User) result.get(1); results.add(Pair.read(mdId, user)); @@ -197,7 +207,7 @@ private List> findAllByGroupOwnerNameAndProfileInternal(@Non @Nonnull @Override public List findAllUsersThatOwnMetadata() { - final CriteriaBuilder cb = _entityManager.getCriteriaBuilder(); + final CriteriaBuilder cb = entityManager.getCriteriaBuilder(); final CriteriaQuery query = cb.createQuery(User.class); final Root metadataRoot = query.from(Metadata.class); @@ -210,13 +220,13 @@ public List findAllUsersThatOwnMetadata() { query.where(ownerExpression); query.distinct(true); - return _entityManager.createQuery(query).getResultList(); + return entityManager.createQuery(query).getResultList(); } @Nonnull @Override public List findAllUsersInUserGroups(@Nonnull final Specification userGroupSpec) { - final CriteriaBuilder cb = _entityManager.getCriteriaBuilder(); + final CriteriaBuilder cb = entityManager.getCriteriaBuilder(); final CriteriaQuery query = cb.createQuery(User.class); final Root userGroupRoot = query.from(UserGroup.class); @@ -229,7 +239,7 @@ public List findAllUsersInUserGroups(@Nonnull final Specification> found = _userRepo.findAllByGroupOwnerNameAndProfile(Arrays.asList(md1.getId()), null); - Collections.sort(found, Comparator.comparing(s -> s.two().getName())); + List> found = userRepo.findAllByGroupOwnerNameAndProfile(List.of(md1.getId()), null); + found.sort(Comparator.comparing(s -> s.two().getName())); assertEquals(2, found.size()); assertEquals(md1.getId(), found.get(0).one().intValue()); @@ -248,9 +248,9 @@ public void testFindAllByGroupOwnerNameAndProfile() { assertEquals(editUser, found.get(0).two()); assertEquals(reviewerUser, found.get(1).two()); - found = _userRepo.findAllByGroupOwnerNameAndProfile(Arrays.asList(md1.getId()), null); + found = userRepo.findAllByGroupOwnerNameAndProfile(List.of(md1.getId()), null); // Sort by user name descending - Collections.sort(found, Comparator.comparing(s -> s.two().getName(), Comparator.reverseOrder())); + found.sort(Comparator.comparing(s -> s.two().getName(), Comparator.reverseOrder())); assertEquals(2, found.size()); assertEquals(md1.getId(), found.get(0).one().intValue()); @@ -259,7 +259,7 @@ public void testFindAllByGroupOwnerNameAndProfile() { assertEquals(reviewerUser, found.get(0).two()); - found = _userRepo.findAllByGroupOwnerNameAndProfile(Arrays.asList(md1.getId(), md2.getId()), null); + found = userRepo.findAllByGroupOwnerNameAndProfile(Arrays.asList(md1.getId(), md2.getId()), null); assertEquals(4, found.size()); int md1Found = 0; @@ -277,21 +277,21 @@ public void testFindAllByGroupOwnerNameAndProfile() { @Test public void testFindAllUsersInUserGroups() { - Group group1 = _groupRepo.save(GroupRepositoryTest.newGroup(_inc)); - Group group2 = _groupRepo.save(GroupRepositoryTest.newGroup(_inc)); + Group group1 = groupRepo.save(GroupRepositoryTest.newGroup(_inc)); + Group group2 = groupRepo.save(GroupRepositoryTest.newGroup(_inc)); - User editUser = _userRepo.save(newUser().setProfile(Profile.Editor)); - User reviewerUser = _userRepo.save(newUser().setProfile(Profile.Reviewer)); - User registeredUser = _userRepo.save(newUser().setProfile(Profile.RegisteredUser)); - _userRepo.save(newUser().setProfile(Profile.Administrator)); + User editUser = userRepo.save(newUser().setProfile(Profile.Editor)); + User reviewerUser = userRepo.save(newUser().setProfile(Profile.Reviewer)); + User registeredUser = userRepo.save(newUser().setProfile(Profile.RegisteredUser)); + userRepo.save(newUser().setProfile(Profile.Administrator)); - _userGroupRepository.save(new UserGroup().setGroup(group1).setUser(editUser).setProfile(Profile.Editor)); - _userGroupRepository.save(new UserGroup().setGroup(group2).setUser(registeredUser).setProfile(Profile.RegisteredUser)); - _userGroupRepository.save(new UserGroup().setGroup(group2).setUser(reviewerUser).setProfile(Profile.Editor)); - _userGroupRepository.save(new UserGroup().setGroup(group1).setUser(reviewerUser).setProfile(Profile.Reviewer)); + userGroupRepository.save(new UserGroup().setGroup(group1).setUser(editUser).setProfile(Profile.Editor)); + userGroupRepository.save(new UserGroup().setGroup(group2).setUser(registeredUser).setProfile(Profile.RegisteredUser)); + userGroupRepository.save(new UserGroup().setGroup(group2).setUser(reviewerUser).setProfile(Profile.Editor)); + userGroupRepository.save(new UserGroup().setGroup(group1).setUser(reviewerUser).setProfile(Profile.Reviewer)); - List found = Lists.transform(_userRepo.findAllUsersInUserGroups(UserGroupSpecs.hasGroupId(group1.getId())), - new Function() { + List found = Lists.transform(userRepo.findAllUsersInUserGroups(UserGroupSpecs.hasGroupId(group1.getId())), + new Function<>() { @Nullable @Override @@ -304,7 +304,7 @@ public Integer apply(@Nullable User input) { assertTrue(found.contains(editUser.getId())); assertTrue(found.contains(reviewerUser.getId())); - found = Lists.transform(_userRepo.findAllUsersInUserGroups(Specification.not(UserGroupSpecs.hasProfile(Profile.RegisteredUser) + found = Lists.transform(userRepo.findAllUsersInUserGroups(Specification.not(UserGroupSpecs.hasProfile(Profile.RegisteredUser) )), new Function() { @Nullable @@ -323,21 +323,20 @@ public Integer apply(@Nullable User input) { @Test public void testFindAllUsersThatOwnMetadata() { - - User editUser = _userRepo.save(newUser().setProfile(Profile.Editor)); - User reviewerUser = _userRepo.save(newUser().setProfile(Profile.Reviewer)); - _userRepo.save(newUser().setProfile(Profile.RegisteredUser)); - _userRepo.save(newUser().setProfile(Profile.Administrator)); + User editUser = userRepo.save(newUser().setProfile(Profile.Editor)); + User reviewerUser = userRepo.save(newUser().setProfile(Profile.Reviewer)); + userRepo.save(newUser().setProfile(Profile.RegisteredUser)); + userRepo.save(newUser().setProfile(Profile.Administrator)); Metadata md1 = MetadataRepositoryTest.newMetadata(_inc); md1.getSourceInfo().setOwner(editUser.getId()); - _metadataRepo.save(md1); + metadataRepo.save(md1); Metadata md2 = MetadataRepositoryTest.newMetadata(_inc); md2.getSourceInfo().setOwner(reviewerUser.getId()); - _metadataRepo.save(md2); + metadataRepo.save(md2); - List found = _userRepo.findAllUsersThatOwnMetadata(); + List found = userRepo.findAllUsersThatOwnMetadata(); assertEquals(2, found.size()); boolean editUserFound = false; @@ -363,12 +362,12 @@ public void testFindDuplicatedUsernamesCaseInsensitive() { User userNonDuplicated1 = newUser(); usernameDuplicated1.setUsername("userNamE1"); usernameDuplicated2.setUsername("usERNAME1"); - _userRepo.save(usernameDuplicated1); - _userRepo.save(usernameDuplicated2); - _userRepo.save(userNonDuplicated1); + userRepo.save(usernameDuplicated1); + userRepo.save(usernameDuplicated2); + userRepo.save(userNonDuplicated1); - List duplicatedUsernames = _userRepo.findDuplicatedUsernamesCaseInsensitive(); - assertThat("Duplicated usernames don't match the expected ones", + List duplicatedUsernames = userRepo.findDuplicatedUsernamesCaseInsensitive(); + MatcherAssert.assertThat("Duplicated usernames don't match the expected ones", duplicatedUsernames, CoreMatchers.is(Lists.newArrayList("username1"))); assertEquals(1, duplicatedUsernames.size()); diff --git a/services/src/main/java/org/fao/geonet/api/users/PasswordApi.java b/services/src/main/java/org/fao/geonet/api/users/PasswordApi.java index 4360fd7875a..00e4010dad8 100644 --- a/services/src/main/java/org/fao/geonet/api/users/PasswordApi.java +++ b/services/src/main/java/org/fao/geonet/api/users/PasswordApi.java @@ -76,6 +76,7 @@ public class PasswordApi { public static final String LOGGER = Geonet.GEONETWORK + ".api.user"; public static final String DATE_FORMAT = "yyyy-MM-dd"; + public static final String USER_PASSWORD_SENT = "user_password_sent"; @Autowired LanguageUtils languageUtils; @Autowired @@ -85,14 +86,13 @@ public class PasswordApi { @Autowired FeedbackLanguages feedbackLanguages; - @Autowired(required=false) + @Autowired(required = false) SecurityProviderConfiguration securityProviderConfiguration; @io.swagger.v3.oas.annotations.Operation(summary = "Update user password", description = "Get a valid changekey by email first and then update your password.") - @RequestMapping( + @PatchMapping( value = "/{username}", - method = RequestMethod.PATCH, produces = MediaType.TEXT_PLAIN_VALUE) @ResponseStatus(value = HttpStatus.CREATED) @ResponseBody @@ -100,13 +100,12 @@ public ResponseEntity updatePassword( @Parameter(description = "The user name", required = true) @PathVariable - String username, + String username, @Parameter(description = "The new password and a valid change key", required = true) @RequestBody - PasswordUpdateParameter passwordAndChangeKey, - HttpServletRequest request) - throws Exception { + PasswordUpdateParameter passwordAndChangeKey, + HttpServletRequest request) { Locale locale = languageUtils.parseAcceptLanguage(request.getLocales()); ResourceBundle messages = ResourceBundle.getBundle("org.fao.geonet.api.Messages", locale); Locale[] feedbackLocales = feedbackLanguages.getLocales(locale); @@ -208,9 +207,8 @@ public ResponseEntity updatePassword( "reset his password. User MUST have an email to get the link. " + "LDAP users will not be able to retrieve their password " + "using this service.") - @RequestMapping( + @PutMapping( value = "/actions/forgot-password", - method = RequestMethod.PUT, produces = MediaType.TEXT_PLAIN_VALUE) @ResponseStatus(value = HttpStatus.CREATED) @ResponseBody @@ -218,9 +216,8 @@ public ResponseEntity sendPasswordByEmail( @Parameter(description = "The user name", required = true) @RequestParam - String username, - HttpServletRequest request) - throws Exception { + String username, + HttpServletRequest request) { Locale locale = languageUtils.parseAcceptLanguage(request.getLocales()); ResourceBundle messages = ResourceBundle.getBundle("org.fao.geonet.api.Messages", locale); Locale[] feedbackLocales = feedbackLanguages.getLocales(locale); @@ -239,7 +236,7 @@ public ResponseEntity sendPasswordByEmail( // Return response not providing details about the issue, that should be logged. return new ResponseEntity<>(String.format( - messages.getString("user_password_sent"), + messages.getString(USER_PASSWORD_SENT), XslUtil.encodeForJavaScript(username) ), HttpStatus.CREATED); } @@ -251,7 +248,7 @@ public ResponseEntity sendPasswordByEmail( // Return response not providing details about the issue, that should be logged. return new ResponseEntity<>(String.format( - messages.getString("user_password_sent"), + messages.getString(USER_PASSWORD_SENT), XslUtil.encodeForJavaScript(username) ), HttpStatus.CREATED); } @@ -263,7 +260,7 @@ public ResponseEntity sendPasswordByEmail( // Return response not providing details about the issue, that should be logged. return new ResponseEntity<>(String.format( - messages.getString("user_password_sent"), + messages.getString(USER_PASSWORD_SENT), XslUtil.encodeForJavaScript(username) ), HttpStatus.CREATED); } @@ -317,7 +314,7 @@ public ResponseEntity sendPasswordByEmail( } return new ResponseEntity<>(String.format( - messages.getString("user_password_sent"), + messages.getString(USER_PASSWORD_SENT), XslUtil.encodeForJavaScript(username) ), HttpStatus.CREATED); }