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 6ba313032776098da7e265c96fc26ca9c7ab57d6
Author: Benoit Tellier <btell...@linagora.com>
AuthorDate: Tue Mar 24 14:38:11 2020 +0700

    JAMES-3087 Sequentially process authentication strategies
    
    Solutions relying on JWT were also execution standard JMAP authentication
    
    This is because flatMap is eager.
    
    Sequentially processing avoids verbose logs of invalid access token format.
---
 .../jmap/http/AuthenticationReactiveFilter.java    | 11 +++--
 .../http/AuthenticationReactiveFilterTest.java     | 48 +++++++++++-----------
 2 files changed, 31 insertions(+), 28 deletions(-)

diff --git 
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationReactiveFilter.java
 
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationReactiveFilter.java
index 2a68298..4955bc5 100644
--- 
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationReactiveFilter.java
+++ 
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationReactiveFilter.java
@@ -29,6 +29,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -37,6 +38,10 @@ import reactor.netty.http.server.HttpServerRequest;
 public class AuthenticationReactiveFilter {
     private static final Logger LOGGER = 
LoggerFactory.getLogger(AuthenticationReactiveFilter.class);
 
+    static AuthenticationReactiveFilter of(MetricFactory metricFactory, 
AuthenticationStrategy... authenticationStrategies) {
+        return new 
AuthenticationReactiveFilter(ImmutableList.copyOf(authenticationStrategies), 
metricFactory);
+    }
+
     private final List<AuthenticationStrategy> authMethods;
     private final MetricFactory metricFactory;
 
@@ -49,10 +54,10 @@ public class AuthenticationReactiveFilter {
 
     public Mono<MailboxSession> authenticate(HttpServerRequest request) {
         return 
Mono.from(metricFactory.runPublishingTimerMetric("JMAP-authentication-filter",
-            Flux.fromStream(authMethods.stream())
-                .flatMap(auth -> auth.createMailboxSession(request))
+            Flux.fromIterable(authMethods)
+                .concatMap(auth -> auth.createMailboxSession(request))
                 .onErrorContinue((throwable, nothing) -> LOGGER.error("Error 
while trying to authenticate with JMAP", throwable))
-                .singleOrEmpty()
+                .next()
                 .switchIfEmpty(Mono.error(new UnauthorizedException()))));
     }
 }
diff --git 
a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/AuthenticationReactiveFilterTest.java
 
b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/AuthenticationReactiveFilterTest.java
index b236aa8..16041b9 100644
--- 
a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/AuthenticationReactiveFilterTest.java
+++ 
b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/AuthenticationReactiveFilterTest.java
@@ -18,13 +18,14 @@
  ****************************************************************/
 package org.apache.james.jmap.http;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.james.core.Username;
 import org.apache.james.jmap.api.access.AccessToken;
@@ -45,11 +46,13 @@ import reactor.core.publisher.Mono;
 import reactor.netty.http.server.HttpServerRequest;
 
 public class AuthenticationReactiveFilterTest {
-    private static final boolean AUTHORIZED = true;
     private static final String TOKEN = "df991d2a-1c5a-4910-a90f-808b6eda133e";
     private static final String AUTHORIZATION_HEADERS = "Authorization";
     private static final Username USERNAME = Username.of("u...@domain.tld");
 
+    private static final AuthenticationStrategy DENY = httpRequest -> 
Mono.error(new MailboxSessionCreationException(null));
+    private static final AuthenticationStrategy ALLOW = httpRequest -> 
Mono.just(mock(MailboxSession.class));
+
     private HttpServerRequest mockedRequest;
     private HttpHeaders mockedHeaders;
     private AccessTokenRepository accessTokenRepository;
@@ -68,9 +71,7 @@ public class AuthenticationReactiveFilterTest {
         when(mockedRequest.requestHeaders())
             .thenReturn(mockedHeaders);
 
-        List<AuthenticationStrategy> fakeAuthenticationStrategies = 
ImmutableList.of(new FakeAuthenticationStrategy(!AUTHORIZED));
-
-        testee = new 
AuthenticationReactiveFilter(fakeAuthenticationStrategies, new 
RecordingMetricFactory());
+        testee = AuthenticationReactiveFilter.of(new RecordingMetricFactory(), 
DENY);
     }
 
     @Test
@@ -81,7 +82,21 @@ public class AuthenticationReactiveFilterTest {
         assertThatThrownBy(() -> testee.authenticate(mockedRequest).block())
             .isInstanceOf(UnauthorizedException.class);
     }
-    
+
+    @Test
+    public void authenticationStrategiesShouldNotBeEagerlySubScribed() {
+        AtomicBoolean called = new AtomicBoolean(false);
+
+        testee = AuthenticationReactiveFilter.of(new RecordingMetricFactory(),
+            ALLOW,
+            req -> Mono.fromRunnable(() -> called.set(true)));
+        assertThat(called.get()).isFalse();
+
+        testee.authenticate(mockedRequest).block();
+
+        assertThat(called.get()).isFalse();
+    }
+
     @Test
     public void filterShouldReturnUnauthorizedOnInvalidAuthorizationHeader() {
         when(mockedHeaders.get(AUTHORIZATION_HEADERS))
@@ -118,7 +133,7 @@ public class AuthenticationReactiveFilterTest {
 
         accessTokenRepository.addToken(USERNAME, token).block();
 
-        AuthenticationReactiveFilter authFilter = new 
AuthenticationReactiveFilter(ImmutableList.of(new 
FakeAuthenticationStrategy(AUTHORIZED)), new RecordingMetricFactory());
+        AuthenticationReactiveFilter authFilter = 
AuthenticationReactiveFilter.of(new RecordingMetricFactory(), ALLOW);
 
         assertThatCode(() -> authFilter.authenticate(mockedRequest).block())
             .doesNotThrowAnyException();
@@ -132,26 +147,9 @@ public class AuthenticationReactiveFilterTest {
 
         accessTokenRepository.addToken(USERNAME, token).block();
 
-        AuthenticationReactiveFilter authFilter = new 
AuthenticationReactiveFilter(ImmutableList.of(new 
FakeAuthenticationStrategy(!AUTHORIZED), new 
FakeAuthenticationStrategy(AUTHORIZED)), new RecordingMetricFactory());
+        AuthenticationReactiveFilter authFilter = 
AuthenticationReactiveFilter.of(new RecordingMetricFactory(), DENY, ALLOW);
 
         assertThatCode(() -> authFilter.authenticate(mockedRequest).block())
             .doesNotThrowAnyException();
     }
-
-    private static class FakeAuthenticationStrategy implements 
AuthenticationStrategy {
-
-        private final boolean isAuthorized;
-
-        private FakeAuthenticationStrategy(boolean isAuthorized) {
-            this.isAuthorized = isAuthorized;
-        }
-
-        @Override
-        public Mono<MailboxSession> createMailboxSession(HttpServerRequest 
httpRequest) {
-            if (!isAuthorized) {
-                return Mono.error(new MailboxSessionCreationException(null));
-            }
-            return Mono.just(mock(MailboxSession.class));
-        }
-    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org

Reply via email to