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 rc...@apache.org on 2019/11/26 02:03:07 UTC
[james-project] 02/03: JAMES-2949 Standardize case support for
UsersRepository
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 4b3aaf5c7703ea89f9a9fe926e4b701fe4f98e49
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Wed Nov 20 11:10:37 2019 +0700
JAMES-2949 Standardize case support for UsersRepository
---
.../main/java/org/apache/james/core/Username.java | 6 +-
.../org/apache/james/user/api/UsersRepository.java | 10 +-
.../user/cassandra/CassandraUsersRepository.java | 10 +-
.../cassandra/CassandraUsersRepositoryTest.java | 10 +-
.../james/user/jpa/JpaUsersRepositoryTest.java | 4 +-
.../user/ldap/ReadOnlyUsersLDAPRepository.java | 10 --
.../james/user/lib/AbstractUsersRepository.java | 11 +-
.../user/lib/AbstractUsersRepositoryTest.java | 122 ++++++++++++++++++++-
.../james/user/memory/MemoryUsersRepository.java | 4 +-
9 files changed, 151 insertions(+), 36 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 73e6aaf..8ae8117 100644
--- a/core/src/main/java/org/apache/james/core/Username.java
+++ b/core/src/main/java/org/apache/james/core/Username.java
@@ -122,7 +122,11 @@ public class Username {
}
public String asId() {
- return asString().toLowerCase(Locale.US);
+ return toLowerCase().asString();
+ }
+
+ public Username toLowerCase() {
+ return new Username(localPart.toLowerCase(Locale.US), domainPart);
}
public boolean equalsAsId(String otherId) {
diff --git a/server/data/data-api/src/main/java/org/apache/james/user/api/UsersRepository.java b/server/data/data-api/src/main/java/org/apache/james/user/api/UsersRepository.java
index 742fbc3..a5ffdad 100644
--- a/server/data/data-api/src/main/java/org/apache/james/user/api/UsersRepository.java
+++ b/server/data/data-api/src/main/java/org/apache/james/user/api/UsersRepository.java
@@ -132,11 +132,15 @@ public interface UsersRepository {
/**
* Returns username to be used for a given MailAddress
*
- * @param mailAddress
* @return Username used by James for this mailAddress
- * @throws UsersRepositoryException
*/
- Username getUser(MailAddress mailAddress) throws UsersRepositoryException;
+ default Username getUser(MailAddress mailAddress) throws UsersRepositoryException {
+ if (supportVirtualHosting()) {
+ return Username.of(mailAddress.asString()).toLowerCase();
+ } else {
+ return Username.of(mailAddress.getLocalPart()).toLowerCase();
+ }
+ }
/**
* Returns one of the possible mail addresses to be used to send a mail to that user
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 f1ac2a8..9f73dfa 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
@@ -84,7 +84,7 @@ public class CassandraUsersRepository extends AbstractUsersRepository {
}
private PreparedStatement prepareListStatement(Session session) {
- return session.prepare(select(REALNAME)
+ return session.prepare(select(NAME)
.from(TABLE_NAME));
}
@@ -109,7 +109,7 @@ public class CassandraUsersRepository extends AbstractUsersRepository {
}
private PreparedStatement prepareGetUserStatement(Session session) {
- return session.prepare(select(REALNAME, PASSWORD, ALGORITHM)
+ return session.prepare(select(NAME, PASSWORD, ALGORITHM)
.from(TABLE_NAME)
.where(eq(NAME, bindMarker(NAME))));
}
@@ -119,7 +119,7 @@ public class CassandraUsersRepository extends AbstractUsersRepository {
return executor.executeSingleRow(
getUserStatement.bind()
.setString(NAME, name.asId()))
- .map(row -> new DefaultUser(Username.of(row.getString(REALNAME)), row.getString(PASSWORD), row.getString(ALGORITHM)))
+ .map(row -> new DefaultUser(Username.of(row.getString(NAME)), row.getString(PASSWORD), row.getString(ALGORITHM)))
.blockOptional()
.orElse(null);
}
@@ -145,7 +145,7 @@ public class CassandraUsersRepository extends AbstractUsersRepository {
public void removeUser(Username name) throws UsersRepositoryException {
boolean executed = executor.executeReturnApplied(
removeUserStatement.bind()
- .setString(NAME, name.asString()))
+ .setString(NAME, name.asId()))
.block();
if (!executed) {
@@ -178,7 +178,7 @@ public class CassandraUsersRepository extends AbstractUsersRepository {
@Override
public Iterator<Username> list() throws UsersRepositoryException {
return executor.executeRows(listStatement.bind())
- .map(row -> row.getString(REALNAME))
+ .map(row -> row.getString(NAME))
.map(Username::of)
.toIterable()
.iterator();
diff --git a/server/data/data-cassandra/src/test/java/org/apache/james/user/cassandra/CassandraUsersRepositoryTest.java b/server/data/data-cassandra/src/test/java/org/apache/james/user/cassandra/CassandraUsersRepositoryTest.java
index 0341928..7009cd7 100644
--- a/server/data/data-cassandra/src/test/java/org/apache/james/user/cassandra/CassandraUsersRepositoryTest.java
+++ b/server/data/data-cassandra/src/test/java/org/apache/james/user/cassandra/CassandraUsersRepositoryTest.java
@@ -19,6 +19,7 @@
package org.apache.james.user.cassandra;
+import org.apache.commons.configuration2.BaseHierarchicalConfiguration;
import org.apache.james.backends.cassandra.CassandraCluster;
import org.apache.james.backends.cassandra.DockerCassandraRule;
import org.apache.james.user.lib.AbstractUsersRepository;
@@ -48,8 +49,11 @@ public class CassandraUsersRepositoryTest extends AbstractUsersRepositoryTest {
cassandra.closeCluster();
}
- @Override
- protected AbstractUsersRepository getUsersRepository() {
- return new CassandraUsersRepository(domainList, cassandra.getConf());
+ @Override protected AbstractUsersRepository getUsersRepository() throws Exception {
+ CassandraUsersRepository cassandraUsersRepository = new CassandraUsersRepository(domainList, cassandra.getConf());
+ BaseHierarchicalConfiguration configuration = new BaseHierarchicalConfiguration();
+ configuration.addProperty("enableVirtualHosting", "true");
+ cassandraUsersRepository.configure(configuration);
+ return cassandraUsersRepository;
}
}
diff --git a/server/data/data-jpa/src/test/java/org/apache/james/user/jpa/JpaUsersRepositoryTest.java b/server/data/data-jpa/src/test/java/org/apache/james/user/jpa/JpaUsersRepositoryTest.java
index f81f3e2..1cff3ee 100644
--- a/server/data/data-jpa/src/test/java/org/apache/james/user/jpa/JpaUsersRepositoryTest.java
+++ b/server/data/data-jpa/src/test/java/org/apache/james/user/jpa/JpaUsersRepositoryTest.java
@@ -47,7 +47,9 @@ public class JpaUsersRepositoryTest extends AbstractUsersRepositoryTest {
protected AbstractUsersRepository getUsersRepository() throws Exception {
JPAUsersRepository repos = new JPAUsersRepository(domainList);
repos.setEntityManagerFactory(JPA_TEST_CLUSTER.getEntityManagerFactory());
- repos.configure(new BaseHierarchicalConfiguration());
+ BaseHierarchicalConfiguration configuration = new BaseHierarchicalConfiguration();
+ configuration.addProperty("enableVirtualHosting", "true");
+ repos.configure(configuration);
return repos;
}
}
diff --git a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
index 812da90..ebbd794 100644
--- a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
+++ b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
@@ -657,16 +657,6 @@ public class ReadOnlyUsersLDAPRepository implements UsersRepository, Configurabl
return ldapConfiguration.supportsVirtualHosting();
}
-
- @Override
- public Username getUser(MailAddress mailAddress) {
- if (supportVirtualHosting()) {
- return Username.of(mailAddress.asString());
- } else {
- return Username.of(mailAddress.getLocalPart());
- }
- }
-
@Override
public boolean isAdministrator(Username username) {
if (ldapConfiguration.getAdministratorId().isPresent()) {
diff --git a/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java b/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java
index 7dfcd17..2d0cb7f 100644
--- a/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java
+++ b/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java
@@ -122,15 +122,6 @@ public abstract class AbstractUsersRepository implements UsersRepository, Config
*/
protected abstract void doAddUser(Username username, String password) throws UsersRepositoryException;
- @Override
- public Username getUser(MailAddress mailAddress) throws UsersRepositoryException {
- if (supportVirtualHosting()) {
- return Username.of(mailAddress.asString());
- } else {
- return Username.of(mailAddress.getLocalPart());
- }
- }
-
@VisibleForTesting void setAdministratorId(Optional<Username> username) {
this.administratorId = username;
}
@@ -138,7 +129,7 @@ public abstract class AbstractUsersRepository implements UsersRepository, Config
@Override
public boolean isAdministrator(Username username) throws UsersRepositoryException {
if (administratorId.isPresent()) {
- return administratorId.get().equals(username);
+ return administratorId.get().toLowerCase().equals(username.toLowerCase());
}
return false;
}
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 0503efb..203f431 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
@@ -19,6 +19,7 @@
package org.apache.james.user.lib;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.ArrayList;
import java.util.Iterator;
@@ -52,10 +53,12 @@ public abstract class AbstractUsersRepositoryTest {
protected abstract AbstractUsersRepository getUsersRepository() throws Exception;
private Username user1;
+ private Username user1CaseVariation;
private Username user2;
private Username user3;
private Username admin;
-
+ private Username adminCaseVariation;
+
public void setUp() throws Exception {
domainList = new SimpleDomainList();
domainList.addDomain(DOMAIN);
@@ -63,7 +66,9 @@ public abstract class AbstractUsersRepositoryTest {
user1 = login("username");
user2 = login("username2");
user3 = login("username3");
+ user1CaseVariation = login("uSeRnaMe");
admin = login("admin");
+ adminCaseVariation = login("adMin");
}
public void tearDown() throws Exception {
@@ -139,6 +144,121 @@ public abstract class AbstractUsersRepositoryTest {
//Then
assertThat(usersRepository.contains(user2)).isTrue();
}
+
+ @Test
+ public void containsShouldPreserveCaseVariation() throws UsersRepositoryException {
+ usersRepository.addUser(user1CaseVariation, "password2");
+
+ assertThat(usersRepository.contains(user1CaseVariation)).isTrue();
+ }
+
+ @Test
+ public void containsShouldBeCaseInsentive() throws UsersRepositoryException {
+ usersRepository.addUser(user1CaseVariation, "password2");
+
+ assertThat(usersRepository.contains(user1)).isTrue();
+ }
+
+ @Test
+ public void containsShouldBeCaseInsentiveWhenOriginalValueLowerCased() throws UsersRepositoryException {
+ usersRepository.addUser(user1, "password2");
+
+ assertThat(usersRepository.contains(user1CaseVariation)).isTrue();
+ }
+
+ @Test
+ public void addUserShouldDisableCaseVariationWhenOriginalValueLowerCased() throws UsersRepositoryException {
+ usersRepository.addUser(user1, "password2");
+
+ assertThatThrownBy(() -> usersRepository.addUser(user1CaseVariation, "pass"))
+ .isInstanceOf(UsersRepositoryException.class);
+ }
+
+ @Test
+ public void addUserShouldDisableCaseVariation() throws UsersRepositoryException {
+ usersRepository.addUser(user1CaseVariation, "password2");
+
+ assertThatThrownBy(() -> usersRepository.addUser(user1, "pass"))
+ .isInstanceOf(UsersRepositoryException.class);
+ }
+
+ @Test
+ public void listShouldReturnLowerCaseUser() throws UsersRepositoryException {
+ usersRepository.addUser(user1CaseVariation, "password2");
+
+ assertThat(usersRepository.list())
+ .toIterable()
+ .containsExactly(user1);
+ }
+
+ @Test
+ public void removeUserShouldBeCaseInsentiveOnCaseVariationUser() throws UsersRepositoryException {
+ usersRepository.addUser(user1CaseVariation, "password2");
+
+ usersRepository.removeUser(user1);
+
+ assertThat(usersRepository.list())
+ .toIterable()
+ .isEmpty();
+ }
+
+ @Test
+ public void removeUserShouldBeCaseInsentive() throws UsersRepositoryException {
+ usersRepository.addUser(user1, "password2");
+
+ usersRepository.removeUser(user1CaseVariation);
+
+ assertThat(usersRepository.list())
+ .toIterable()
+ .isEmpty();
+ }
+
+ @Test
+ public void getUserByNameShouldBeCaseInsentive() throws UsersRepositoryException {
+ usersRepository.addUser(user1, "password2");
+
+ assertThat(usersRepository.getUserByName(user1CaseVariation).getUserName())
+ .isEqualTo(user1);
+ }
+
+ @Test
+ public void getUserByNameShouldReturnLowerCaseAddedUser() throws UsersRepositoryException {
+ usersRepository.addUser(user1CaseVariation, "password2");
+
+ assertThat(usersRepository.getUserByName(user1).getUserName())
+ .isEqualTo(user1);
+ }
+
+ @Test
+ public void getUserShouldBeCaseInsentive() throws Exception {
+ assertThat(usersRepository.getUser(user1CaseVariation.asMailAddress()))
+ .isEqualTo(user1);
+ }
+
+ @Test
+ public void isAdministratorShouldBeCaseInsentive() throws Exception {
+ usersRepository.setAdministratorId(Optional.of(admin));
+ assertThat(usersRepository.isAdministrator(adminCaseVariation))
+ .isTrue();
+ }
+
+ @Test
+ public void testShouldBeCaseInsentiveOnCaseVariationUser() throws UsersRepositoryException {
+ String password = "password2";
+ usersRepository.addUser(user1CaseVariation, password);
+
+ assertThat(usersRepository.test(user1, password))
+ .isTrue();
+ }
+
+ @Test
+ public void testShouldBeCaseInsentive() throws UsersRepositoryException {
+ String password = "password2";
+ usersRepository.addUser(user1, password);
+
+ assertThat(usersRepository.test(user1CaseVariation, password))
+ .isTrue();
+ }
@Test
public void addUserShouldAddAUserWhenNotEmptyRepository() throws UsersRepositoryException {
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 7a4de59..a658f3d 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
@@ -71,8 +71,8 @@ public class MemoryUsersRepository extends AbstractUsersRepository {
}
@Override
- protected void doAddUser(Username username, String password) throws UsersRepositoryException {
- DefaultUser user = new DefaultUser(username, algo);
+ protected void doAddUser(Username username, String password) {
+ DefaultUser user = new DefaultUser(username.toLowerCase(), algo);
user.setPassword(password);
userByName.put(username.asId(), user);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org