This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 733d51b0f5b63c42240cc08e1fb570d5f348fe51 Author: Raphael Ouazana <raphael.ouaz...@linagora.com> AuthorDate: Fri Nov 8 15:47:02 2019 +0100 Request for comment: make UsersRepository case insensitive. Probably need some migrations instructions and to write an ADR about this? --- .../main/java/org/apache/james/core/Username.java | 5 +++++ .../james/mailbox/jpa/mail/JPAMailboxMapper.java | 20 ++++++-------------- .../user/cassandra/CassandraUsersRepository.java | 8 +++----- .../apache/james/user/jpa/JPAUsersRepository.java | 8 ++++---- .../org/apache/james/user/jpa/model/JPAUser.java | 2 +- .../james/user/lib/AbstractUsersRepositoryTest.java | 9 +++++---- .../james/user/memory/MemoryUsersRepository.java | 21 ++++++++------------- 7 files changed, 32 insertions(+), 41 deletions(-) diff --git a/core/src/main/java/org/apache/james/core/Username.java b/core/src/main/java/org/apache/james/core/Username.java index 3c3d5dd..b29e9b8 100644 --- a/core/src/main/java/org/apache/james/core/Username.java +++ b/core/src/main/java/org/apache/james/core/Username.java @@ -20,6 +20,7 @@ package org.apache.james.core; import java.util.List; +import java.util.Locale; import java.util.Objects; import java.util.Optional; @@ -119,6 +120,10 @@ public class Username { .orElse(localPart); } + public String asId() { + return asString().toLowerCase(Locale.US); + } + public MailAddress asMailAddress() throws AddressException { Preconditions.checkState(hasDomainPart()); return new MailAddress(localPart, domainPart.get()); diff --git a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAMailboxMapper.java b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAMailboxMapper.java index 29114b5..b38874a 100644 --- a/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAMailboxMapper.java +++ b/mailbox/jpa/src/main/java/org/apache/james/mailbox/jpa/mail/JPAMailboxMapper.java @@ -128,20 +128,12 @@ public class JPAMailboxMapper extends JPATransactionalMapper implements MailboxM @Override public Mailbox findMailboxByPath(MailboxPath mailboxPath) throws MailboxException, MailboxNotFoundException { try { - if (mailboxPath.getUser() == null) { - return getEntityManager().createNamedQuery("findMailboxByName", JPAMailbox.class) - .setParameter("nameParam", mailboxPath.getName()) - .setParameter("namespaceParam", mailboxPath.getNamespace()) - .getSingleResult() - .toMailbox(); - } else { - return getEntityManager().createNamedQuery("findMailboxByNameWithUser", JPAMailbox.class) - .setParameter("nameParam", mailboxPath.getName()) - .setParameter("namespaceParam", mailboxPath.getNamespace()) - .setParameter("userParam", mailboxPath.getUser()) - .getSingleResult() - .toMailbox(); - } + return getEntityManager().createNamedQuery("findMailboxByNameWithUser", JPAMailbox.class) + .setParameter("nameParam", mailboxPath.getName()) + .setParameter("namespaceParam", mailboxPath.getNamespace()) + .setParameter("userParam", mailboxPath.getUser().asString()) + .getSingleResult() + .toMailbox(); } catch (NoResultException e) { throw new MailboxNotFoundException(mailboxPath); } catch (PersistenceException e) { diff --git a/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersRepository.java b/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersRepository.java index 0f07fcb..cd8070b 100644 --- a/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersRepository.java +++ b/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersRepository.java @@ -33,7 +33,6 @@ import static org.apache.james.user.cassandra.tables.CassandraUserTable.REALNAME import static org.apache.james.user.cassandra.tables.CassandraUserTable.TABLE_NAME; import java.util.Iterator; -import java.util.Locale; import java.util.Optional; import javax.inject.Inject; @@ -117,9 +116,8 @@ public class CassandraUsersRepository extends AbstractUsersRepository { public User getUserByName(Username name) { return executor.executeSingleRow( getUserStatement.bind() - .setString(NAME, name.asString().toLowerCase(Locale.US))) + .setString(NAME, name.asId())) .map(row -> new DefaultUser(Username.of(row.getString(REALNAME)), row.getString(PASSWORD), row.getString(ALGORITHM))) - .filter(user -> user.hasUsername(name)) .blockOptional() .orElse(null); } @@ -133,7 +131,7 @@ public class CassandraUsersRepository extends AbstractUsersRepository { .setString(REALNAME, defaultUser.getUserName().asString()) .setString(PASSWORD, defaultUser.getHashedPassword()) .setString(ALGORITHM, defaultUser.getHashAlgorithm()) - .setString(NAME, defaultUser.getUserName().asString().toLowerCase(Locale.US))) + .setString(NAME, defaultUser.getUserName().asId())) .block(); if (!executed) { @@ -196,7 +194,7 @@ public class CassandraUsersRepository extends AbstractUsersRepository { user.setPassword(password); boolean executed = executor.executeReturnApplied( insertStatement.bind() - .setString(NAME, user.getUserName().asString().toLowerCase(Locale.US)) + .setString(NAME, user.getUserName().asId()) .setString(REALNAME, user.getUserName().asString()) .setString(PASSWORD, user.getHashedPassword()) .setString(ALGORITHM, user.getHashAlgorithm())) diff --git a/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/JPAUsersRepository.java b/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/JPAUsersRepository.java index 45f8fd7..369bb38 100644 --- a/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/JPAUsersRepository.java +++ b/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/JPAUsersRepository.java @@ -40,11 +40,11 @@ import org.apache.james.user.api.UsersRepositoryException; import org.apache.james.user.api.model.User; import org.apache.james.user.jpa.model.JPAUser; import org.apache.james.user.lib.AbstractUsersRepository; - -import com.github.steveash.guavate.Guavate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.github.steveash.guavate.Guavate; + /** * JPA based UserRepository */ @@ -87,7 +87,7 @@ public class JPAUsersRepository extends AbstractUsersRepository { EntityManager entityManager = entityManagerFactory.createEntityManager(); try { - return (JPAUser) entityManager.createNamedQuery("findUserByName").setParameter("name", name.asString()).getSingleResult(); + return (JPAUser) entityManager.createNamedQuery("findUserByName").setParameter("name", name.asId()).getSingleResult(); } catch (NoResultException e) { return null; } catch (PersistenceException e) { @@ -160,7 +160,7 @@ public class JPAUsersRepository extends AbstractUsersRepository { final EntityTransaction transaction = entityManager.getTransaction(); try { transaction.begin(); - if (entityManager.createNamedQuery("deleteUserByName").setParameter("name", name.asString()).executeUpdate() < 1) { + if (entityManager.createNamedQuery("deleteUserByName").setParameter("name", name.asId()).executeUpdate() < 1) { transaction.commit(); throw new UsersRepositoryException("User " + name.asString() + " does not exist"); } else { diff --git a/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/model/JPAUser.java b/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/model/JPAUser.java index 289cef7..fc805c5 100644 --- a/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/model/JPAUser.java +++ b/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/model/JPAUser.java @@ -42,7 +42,7 @@ import com.google.common.hash.Hashing; @Entity(name = "JamesUser") @Table(name = "JAMES_USER") @NamedQueries({ - @NamedQuery(name = "findUserByName", query = "SELECT user FROM JamesUser user WHERE user.name=:name"), + @NamedQuery(name = "findUserByName", query = "SELECT user FROM JamesUser user WHERE LOWER(user.name)=:name"), @NamedQuery(name = "deleteUserByName", query = "DELETE FROM JamesUser user WHERE user.name=:name"), @NamedQuery(name = "containsUser", query = "SELECT COUNT(user) FROM JamesUser user WHERE user.name=:name"), @NamedQuery(name = "countUsers", query = "SELECT COUNT(user) FROM JamesUser user"), diff --git a/server/data/data-library/src/test/java/org/apache/james/user/lib/AbstractUsersRepositoryTest.java b/server/data/data-library/src/test/java/org/apache/james/user/lib/AbstractUsersRepositoryTest.java index f297179..3919ea3 100644 --- a/server/data/data-library/src/test/java/org/apache/james/user/lib/AbstractUsersRepositoryTest.java +++ b/server/data/data-library/src/test/java/org/apache/james/user/lib/AbstractUsersRepositoryTest.java @@ -179,13 +179,14 @@ public abstract class AbstractUsersRepositoryTest { } @Test - public void getUserByNameShouldReturnNullWhenDifferentCase() throws UsersRepositoryException { + public void getUserByNameShouldReturnUserWhenDifferentCase() throws UsersRepositoryException { //Given usersRepository.addUser(login("username"), "password"); //When User actual = usersRepository.getUserByName(login("uSERNAMe")); //Then - assertThat(actual).isNull(); + assertThat(actual).isNotNull(); + assertThat(actual.getUserName()).isEqualTo(user1); } @Test @@ -243,13 +244,13 @@ public abstract class AbstractUsersRepositoryTest { } @Test - public void testShouldReturnFalseWhenAUserHasAnIncorrectCaseName() throws UsersRepositoryException { + public void testShouldReturnTrueWhenAUserHasAnIncorrectCaseName() throws UsersRepositoryException { //Given usersRepository.addUser(login("username"), "password"); //When boolean actual = usersRepository.test(login("userName"), "password"); //Then - assertThat(actual).isFalse(); + assertThat(actual).isTrue(); } @Test diff --git a/server/data/data-memory/src/main/java/org/apache/james/user/memory/MemoryUsersRepository.java b/server/data/data-memory/src/main/java/org/apache/james/user/memory/MemoryUsersRepository.java index 99f2ec2..f7d74c2 100644 --- a/server/data/data-memory/src/main/java/org/apache/james/user/memory/MemoryUsersRepository.java +++ b/server/data/data-memory/src/main/java/org/apache/james/user/memory/MemoryUsersRepository.java @@ -21,7 +21,6 @@ package org.apache.james.user.memory; import java.util.HashMap; import java.util.Iterator; -import java.util.Locale; import java.util.Map; import java.util.Optional; @@ -73,12 +72,12 @@ public class MemoryUsersRepository extends AbstractUsersRepository { protected void doAddUser(Username username, String password) throws UsersRepositoryException { DefaultUser user = new DefaultUser(username, algo); user.setPassword(password); - userByName.put(toKey(username), user); + userByName.put(username.asId(), user); } @Override public User getUserByName(Username name) throws UsersRepositoryException { - return userByName.get(toKey(name)); + return userByName.get(name.asId()); } @Override @@ -87,24 +86,24 @@ public class MemoryUsersRepository extends AbstractUsersRepository { if (existingUser == null) { throw new UsersRepositoryException("Please provide an existing user to update"); } - userByName.put(toKey(user.getUserName()), user); + userByName.put(user.getUserName().asId(), user); } @Override public void removeUser(Username name) throws UsersRepositoryException { - if (userByName.remove(toKey(name)) == null) { + if (userByName.remove(name.asId()) == null) { throw new UsersRepositoryException("unable to remove unknown user " + name.asString()); } } @Override public boolean contains(Username name) throws UsersRepositoryException { - return userByName.containsKey(toKey(name)); + return userByName.containsKey(name.asId()); } @Override public boolean test(Username name, final String password) throws UsersRepositoryException { - return Optional.ofNullable(userByName.get(toKey(name))) + return Optional.ofNullable(userByName.get(name.asId())) .map(user -> user.verifyPassword(password)) .orElse(false); } @@ -116,13 +115,9 @@ public class MemoryUsersRepository extends AbstractUsersRepository { @Override public Iterator<Username> list() throws UsersRepositoryException { - return userByName.values() + return userByName.keySet() .stream() - .map(User::getUserName) + .map(Username::of) .iterator(); } - - private String toKey(Username username) { - return username.asString().toLowerCase(Locale.US); - } } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org