You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2022/11/21 03:39:11 UTC

[james-project] 01/03: JAMES-3756 Provide a fluent API for delegation in order to prevent confusions

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 13ce548863db2d04ae9a8130716d24092309ba2e
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Nov 4 11:40:51 2022 +0700

    JAMES-3756 Provide a fluent API for delegation in order to prevent confusions
    
    It is very easy to be confused between the two arguments
    and I believe staged methods makes this way easier to
    understand and might prevent errors down the road...
---
 .../src/main/java/org/apache/james/mailbox/Authorizator.java |  8 ++++++++
 .../main/java/org/apache/james/mailbox/SessionProvider.java  | 12 ++++++++++++
 .../org/apache/james/mailbox/store/SessionProviderImpl.java  |  2 +-
 .../apache/james/imap/processor/AbstractAuthProcessor.java   |  7 +++----
 .../apache/james/imap/processor/AuthenticateProcessor.java   |  6 +++---
 .../main/java/org/apache/james/user/api/DelegationStore.java | 12 ++++++++++++
 .../org/apache/james/smtpserver/UsersRepositoryAuthHook.java |  2 +-
 .../java/org/apache/james/webadmin/routes/UserRoutes.java    |  9 +++++++--
 8 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/Authorizator.java b/mailbox/api/src/main/java/org/apache/james/mailbox/Authorizator.java
index 3f9948a92f..f647e62c36 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/Authorizator.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/Authorizator.java
@@ -27,6 +27,10 @@ import org.apache.james.mailbox.exception.MailboxException;
  */
 public interface Authorizator {
 
+    interface FluentAuthorizator {
+        AuthorizationState canLoginAs(Username otherUserId) throws MailboxException;
+    }
+
     enum AuthorizationState {
         ALLOWED,
         FORBIDDEN,
@@ -34,5 +38,9 @@ public interface Authorizator {
     }
 
     AuthorizationState canLoginAsOtherUser(Username userId, Username otherUserId) throws MailboxException;
+
+    default FluentAuthorizator user(Username userId) {
+        return otherUserId -> canLoginAsOtherUser(userId, otherUserId);
+    }
 }
 
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/SessionProvider.java b/mailbox/api/src/main/java/org/apache/james/mailbox/SessionProvider.java
index e5ed5051b1..9b21e4448e 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/SessionProvider.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/SessionProvider.java
@@ -23,6 +23,10 @@ import org.apache.james.core.Username;
 import org.apache.james.mailbox.exception.MailboxException;
 
 public interface SessionProvider {
+    interface DelegationLogin {
+        MailboxSession as(Username other) throws MailboxException;
+    }
+
     /**
      * Return the delimiter to use for folders
      *
@@ -75,6 +79,10 @@ public interface SessionProvider {
      */
     MailboxSession loginAsOtherUser(Username givenUserid, String passwd, Username otherUserId) throws MailboxException;
 
+    default DelegationLogin authenticate(Username givenUserid, String passwd) {
+        return otherUserId -> loginAsOtherUser(givenUserid, passwd, otherUserId);
+    }
+
     /**
      * Checking given user can log in as another user
      * When delegated and authorized, a session for the other user will be supplied
@@ -90,6 +98,10 @@ public interface SessionProvider {
      */
     MailboxSession loginAsOtherUser(Username givenUserid, Username otherUserId) throws MailboxException;
 
+    default DelegationLogin authenticate(Username givenUserid) {
+        return otherUserId -> loginAsOtherUser(givenUserid, otherUserId);
+    }
+
     /**
      * <p>
      * Logs the session out, freeing any resources. Clients who open session
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/SessionProviderImpl.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/SessionProviderImpl.java
index fe14f75c64..396673a3c1 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/SessionProviderImpl.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/SessionProviderImpl.java
@@ -76,7 +76,7 @@ public class SessionProviderImpl implements SessionProvider {
 
     @Override
     public MailboxSession loginAsOtherUser(Username givenUserid, Username otherUserId) throws MailboxException {
-        Authorizator.AuthorizationState authorizationState = authorizator.canLoginAsOtherUser(givenUserid, otherUserId);
+        Authorizator.AuthorizationState authorizationState = authorizator.user(givenUserid).canLoginAs(otherUserId);
         switch (authorizationState) {
             case ALLOWED:
                 return createSystemSession(otherUserId);
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java
index c93f609d18..aad9178810 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java
@@ -99,10 +99,9 @@ public abstract class AbstractAuthProcessor<R extends ImapRequest> extends Abstr
             return;
         }
         Username otherUser = authenticationAttempt.getDelegateUserName().orElseThrow();
-        doAuthWithDelegation(() -> getMailboxManager().loginAsOtherUser(
-                givenUser,
-                authenticationAttempt.getPassword(),
-                otherUser),
+        doAuthWithDelegation(() -> getMailboxManager()
+                .authenticate(givenUser, authenticationAttempt.getPassword())
+                .as(otherUser),
             session,
             request, responder);
     }
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java
index e782569c64..f9f35c65ae 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java
@@ -204,9 +204,9 @@ public class AuthenticateProcessor extends AbstractAuthProcessor<AuthenticateReq
             .ifPresentOrElse(authenticatedUser -> {
                 Username associatedUser = Username.of(oidcInitialResponse.getAssociatedUser());
                 if (!associatedUser.equals(authenticatedUser)) {
-                    doAuthWithDelegation(() -> getMailboxManager().loginAsOtherUser(
-                            authenticatedUser,
-                            associatedUser),
+                    doAuthWithDelegation(() -> getMailboxManager()
+                            .authenticate(authenticatedUser)
+                            .as(associatedUser),
                         session, request, responder);
                 } else {
                     authSuccess(authenticatedUser, session, request, responder);
diff --git a/server/data/data-api/src/main/java/org/apache/james/user/api/DelegationStore.java b/server/data/data-api/src/main/java/org/apache/james/user/api/DelegationStore.java
index acbbe5c890..b8cd22b5f8 100644
--- a/server/data/data-api/src/main/java/org/apache/james/user/api/DelegationStore.java
+++ b/server/data/data-api/src/main/java/org/apache/james/user/api/DelegationStore.java
@@ -23,6 +23,10 @@ import org.apache.james.core.Username;
 import org.reactivestreams.Publisher;
 
 public interface DelegationStore {
+    interface Fluent {
+        Publisher<Void> forUser(Username baseUser);
+    }
+
     /**
      * @return Lists of the users authorized to impersonnate to baseUser.
      */
@@ -32,5 +36,13 @@ public interface DelegationStore {
 
     Publisher<Void> addAuthorizedUser(Username baseUser, Username userWithAccess);
 
+    default Fluent addAuthorizedUser(Username userWithAccess) {
+        return baseUser -> addAuthorizedUser(baseUser, userWithAccess);
+    }
+
     Publisher<Void> removeAuthorizedUser(Username baseUser, Username userWithAccess);
+
+    default Fluent removeAuthorizedUser(Username userWithAccess) {
+        return baseUser -> removeAuthorizedUser(baseUser, userWithAccess);
+    }
 }
diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/UsersRepositoryAuthHook.java b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/UsersRepositoryAuthHook.java
index c48e89a35a..882e4bf18c 100644
--- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/UsersRepositoryAuthHook.java
+++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/UsersRepositoryAuthHook.java
@@ -93,7 +93,7 @@ public class UsersRepositoryAuthHook implements AuthHook {
 
     private HookResult doAuthWithDelegation(SMTPSession session, Username authenticatedUser, Username associatedUser) {
         try {
-            if (Authorizator.AuthorizationState.ALLOWED.equals(authorizator.canLoginAsOtherUser(authenticatedUser, associatedUser))) {
+            if (Authorizator.AuthorizationState.ALLOWED.equals(authorizator.user(authenticatedUser).canLoginAs(associatedUser))) {
                 return saslSuccess(session, associatedUser);
             }
         } catch (MailboxException e) {
diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
index 62b30760a2..61b7fff33b 100644
--- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
+++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
@@ -324,7 +324,10 @@ public class UserRoutes implements Routes {
                     .message(String.format("Delegated user '%s' does not exist", delegatedUser.asString()))
                     .haltError();
             } else {
-                Mono.from(delegationStore.addAuthorizedUser(baseUser, delegatedUser)).block();
+                Mono.from(delegationStore
+                    .addAuthorizedUser(delegatedUser)
+                    .forUser(baseUser))
+                    .block();
                 return Constants.EMPTY_BODY;
             }
         } catch (NotImplementedException e) {
@@ -355,7 +358,9 @@ public class UserRoutes implements Routes {
                     .message(String.format("Delegated user '%s' does not exist", delegatedUser.asString()))
                     .haltError();
             } else {
-                Mono.from(delegationStore.removeAuthorizedUser(baseUser, delegatedUser)).block();
+                Mono.from(delegationStore.removeAuthorizedUser(delegatedUser)
+                    .forUser(baseUser))
+                    .block();
                 return Constants.EMPTY_BODY;
             }
         } catch (NotImplementedException e) {


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org