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
The following commit(s) were added to refs/heads/master by this push:
new 87351b15dc JAMES-3948 FIX virtual users should not be listed (#1753)
87351b15dc is described below
commit 87351b15dca1e14bf9a0abbaea0003e1ccedea06
Author: Benoit TELLIER <btell...@linagora.com>
AuthorDate: Thu Oct 19 08:32:29 2023 +0700
JAMES-3948 FIX virtual users should not be listed (#1753)
Those are only authorizedAccesses,
authenticated through other means (OIDC,
certificates) that do not correspond to an
actual user (no mails of their own). They
only access mails of others.
Cassandra partial row for keeping track
which users we have access to causes James
to mistakenly list those entries into local
users.
---
.../data/CassandraDelegationStoreModule.java | 1 +
.../user/cassandra/CassandraDelegationStore.java | 28 ++++++++++++++++++++--
.../james/user/cassandra/CassandraUsersDAO.java | 16 ++++++-------
.../cassandra/CassandraDelegationStoreTest.java | 18 +++++++++++++-
4 files changed, 52 insertions(+), 11 deletions(-)
diff --git
a/server/container/guice/data-cassandra/src/main/java/org/apache/james/modules/data/CassandraDelegationStoreModule.java
b/server/container/guice/data-cassandra/src/main/java/org/apache/james/modules/data/CassandraDelegationStoreModule.java
index 1776ddaa2a..12631b6189 100644
---
a/server/container/guice/data-cassandra/src/main/java/org/apache/james/modules/data/CassandraDelegationStoreModule.java
+++
b/server/container/guice/data-cassandra/src/main/java/org/apache/james/modules/data/CassandraDelegationStoreModule.java
@@ -41,6 +41,7 @@ public class CassandraDelegationStoreModule extends
AbstractModule {
@Override
public void configure() {
bind(DelegationStore.class).to(CassandraDelegationStore.class);
+
bind(CassandraDelegationStore.UserExistencePredicate.class).to(CassandraDelegationStore.UserExistencePredicateImplementation.class);
bind(Authorizator.class).to(DelegationStoreAuthorizator.class);
Multibinder<CassandraModule> cassandraDataDefinitions =
Multibinder.newSetBinder(binder(), CassandraModule.class);
cassandraDataDefinitions.addBinding().toInstance(org.apache.james.user.cassandra.CassandraUsersRepositoryModule.MODULE);
diff --git
a/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraDelegationStore.java
b/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraDelegationStore.java
index 9fc32c1bfa..7dc03979ba 100644
---
a/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraDelegationStore.java
+++
b/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraDelegationStore.java
@@ -23,14 +23,37 @@ import javax.inject.Inject;
import org.apache.james.core.Username;
import org.apache.james.user.api.DelegationStore;
+import org.apache.james.user.api.UsersRepository;
import org.reactivestreams.Publisher;
+import reactor.core.publisher.Mono;
+
public class CassandraDelegationStore implements DelegationStore {
+ public interface UserExistencePredicate {
+ Mono<Boolean> exists(Username username);
+ }
+
+ public static class UserExistencePredicateImplementation implements
UserExistencePredicate {
+ private final UsersRepository usersRepository;
+
+ @Inject
+ UserExistencePredicateImplementation(UsersRepository usersRepository) {
+ this.usersRepository = usersRepository;
+ }
+
+ @Override
+ public Mono<Boolean> exists(Username username) {
+ return Mono.from(usersRepository.containsReactive(username));
+ }
+ }
+
private final CassandraUsersDAO cassandraUsersDAO;
+ private final UserExistencePredicate userExistencePredicate;
@Inject
- public CassandraDelegationStore(CassandraUsersDAO cassandraUsersDAO) {
+ public CassandraDelegationStore(CassandraUsersDAO cassandraUsersDAO,
UserExistencePredicate userExistencePredicate) {
this.cassandraUsersDAO = cassandraUsersDAO;
+ this.userExistencePredicate = userExistencePredicate;
}
@Override
@@ -45,7 +68,8 @@ public class CassandraDelegationStore implements
DelegationStore {
@Override
public Publisher<Void> addAuthorizedUser(Username baseUser, Username
userWithAccess) {
- return cassandraUsersDAO.addAuthorizedUsers(baseUser, userWithAccess);
+ return userExistencePredicate.exists(userWithAccess)
+ .flatMap(targetUserExists ->
cassandraUsersDAO.addAuthorizedUsers(baseUser, userWithAccess,
targetUserExists));
}
@Override
diff --git
a/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersDAO.java
b/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersDAO.java
index fd6083f15c..de3887cbd9 100644
---
a/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersDAO.java
+++
b/server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersDAO.java
@@ -52,7 +52,6 @@ import org.reactivestreams.Publisher;
import com.datastax.oss.driver.api.core.CqlSession;
import com.datastax.oss.driver.api.core.cql.BatchStatementBuilder;
import com.datastax.oss.driver.api.core.cql.BatchType;
-import com.datastax.oss.driver.api.core.cql.BoundStatement;
import com.datastax.oss.driver.api.core.cql.PreparedStatement;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
@@ -200,15 +199,16 @@ public class CassandraUsersDAO implements UsersDAO {
}
}
- public Mono<Void> addAuthorizedUsers(Username baseUser, Username
userWithAccess) {
+ public Mono<Void> addAuthorizedUsers(Username baseUser, Username
userWithAccess, boolean targetUserExists) {
BatchStatementBuilder batchBuilder = new
BatchStatementBuilder(BatchType.LOGGED);
- BoundStatement addAuthorizedStatement =
addAuthorizedUsersStatement.bind()
+ batchBuilder.addStatement(addAuthorizedUsersStatement.bind()
.setString(NAME, baseUser.asString())
- .setSet(AUTHORIZED_USERS,
ImmutableSet.of(userWithAccess.asString()), String.class);
- batchBuilder.addStatement(addAuthorizedStatement);
- batchBuilder.addStatement(addDelegatedToUsersStatement.bind()
- .setString(NAME, userWithAccess.asString())
- .setSet(DELEGATED_USERS, ImmutableSet.of(baseUser.asString()),
String.class));
+ .setSet(AUTHORIZED_USERS,
ImmutableSet.of(userWithAccess.asString()), String.class));
+ if (targetUserExists) {
+ batchBuilder.addStatement(addDelegatedToUsersStatement.bind()
+ .setString(NAME, userWithAccess.asString())
+ .setSet(DELEGATED_USERS, ImmutableSet.of(baseUser.asString()),
String.class));
+ }
return executor.executeVoid(batchBuilder.build());
}
diff --git
a/server/data/data-cassandra/src/test/java/org/apache/james/user/cassandra/CassandraDelegationStoreTest.java
b/server/data/data-cassandra/src/test/java/org/apache/james/user/cassandra/CassandraDelegationStoreTest.java
index f190189e39..0cc4d8e678 100644
---
a/server/data/data-cassandra/src/test/java/org/apache/james/user/cassandra/CassandraDelegationStoreTest.java
+++
b/server/data/data-cassandra/src/test/java/org/apache/james/user/cassandra/CassandraDelegationStoreTest.java
@@ -19,14 +19,19 @@
package org.apache.james.user.cassandra;
+import static org.assertj.core.api.Assertions.assertThat;
+
import org.apache.james.backends.cassandra.CassandraClusterExtension;
import org.apache.james.core.Username;
import org.apache.james.user.api.DelegationStore;
import org.apache.james.user.api.DelegationStoreContract;
import org.apache.james.user.api.UsersRepositoryException;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
+import reactor.core.publisher.Mono;
+
public class CassandraDelegationStoreTest implements DelegationStoreContract {
@RegisterExtension
static CassandraClusterExtension cassandraCluster = new
CassandraClusterExtension(CassandraUsersRepositoryModule.MODULE);
@@ -37,7 +42,7 @@ public class CassandraDelegationStoreTest implements
DelegationStoreContract {
@BeforeEach
void setUp() {
cassandraUsersDAO = new
CassandraUsersDAO(cassandraCluster.getCassandraCluster().getConf());
- testee = new CassandraDelegationStore(cassandraUsersDAO);
+ testee = new CassandraDelegationStore(cassandraUsersDAO, any ->
Mono.just(true));
}
@Override
@@ -53,4 +58,15 @@ public class CassandraDelegationStoreTest implements
DelegationStoreContract {
throw new RuntimeException(e);
}
}
+
+ @Test
+ void virtualUsersShouldNotBeListed() {
+ testee = new CassandraDelegationStore(cassandraUsersDAO, any ->
Mono.just(false));
+ addUser(BOB);
+
+ Mono.from(testee().addAuthorizedUser(ALICE).forUser(BOB)).block();
+
+ assertThat(cassandraUsersDAO.listReactive().collectList().block())
+ .containsOnly(BOB);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
