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