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 bt...@apache.org on 2020/03/26 02:41:37 UTC
[james-project] 15/16: JAMES-3087 Sequentially process
authentication strategies
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 <bt...@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("user@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-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org