You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2019/11/18 02:50:36 UTC
[james-project] 02/44: Request for comment: make UsersRepository
case insensitive.
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 <ra...@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-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org