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/08/11 09:39:20 UTC

[james-project] 03/10: JAMES-3351 Reject request immediately when error

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 a16087cdd045eb79ccd3ff6675388a70668c6629
Author: LanKhuat <dl...@linagora.com>
AuthorDate: Tue Jul 28 14:17:22 2020 +0700

    JAMES-3351 Reject request immediately when error
---
 .../java/org/apache/james/jmap/http/AuthenticationRoutes.java    | 1 +
 .../test/java/org/apache/james/jmap/http/AuthenticatorTest.java  | 9 ++++-----
 .../src/main/java/org/apache/james/jmap/http/Authenticator.java  | 3 +--
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java
index 987ed7a..35e0a69 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java
@@ -149,6 +149,7 @@ public class AuthenticationRoutes implements JMAPRoutes {
             return authenticator.authenticate(req)
                 .flatMap(session -> returnEndPointsResponse(resp)
                     .subscriberContext(jmapAuthContext(session)))
+                .onErrorResume(IllegalArgumentException.class, e -> handleBadRequest(resp, LOGGER, e))
                 .onErrorResume(BadRequestException.class, e -> handleBadRequest(resp, LOGGER, e))
                 .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e)))
                 .onErrorResume(InternalErrorException.class, e -> handleInternalError(resp, e))
diff --git a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/AuthenticatorTest.java b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/AuthenticatorTest.java
index f009783..66b5cb4 100644
--- a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/AuthenticatorTest.java
+++ b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/AuthenticatorTest.java
@@ -30,7 +30,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.james.core.Username;
 import org.apache.james.jmap.api.access.AccessToken;
 import org.apache.james.jmap.api.access.AccessTokenRepository;
-import org.apache.james.jmap.exceptions.MailboxSessionCreationException;
 import org.apache.james.jmap.exceptions.UnauthorizedException;
 import org.apache.james.jmap.memory.access.MemoryAccessTokenRepository;
 import org.apache.james.mailbox.MailboxSession;
@@ -50,7 +49,7 @@ public class AuthenticatorTest {
     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 DENY = httpRequest -> Mono.error(new UnauthorizedException(null));
     private static final AuthenticationStrategy ALLOW = httpRequest -> Mono.just(mock(MailboxSession.class));
 
     private HttpServerRequest mockedRequest;
@@ -148,7 +147,7 @@ public class AuthenticatorTest {
     }
 
     @Test
-    public void filterShouldNotThrowWhenChainingAuthorizationStrategies() {
+    public void filterShouldThrowWhenChainingAuthorizationStrategies() {
         AccessToken token = AccessToken.fromString(TOKEN);
         when(mockedHeaders.get(AUTHORIZATION_HEADERS))
             .thenReturn(TOKEN);
@@ -157,7 +156,7 @@ public class AuthenticatorTest {
 
         Authenticator authFilter = Authenticator.of(new RecordingMetricFactory(), DENY, ALLOW);
 
-        assertThatCode(() -> authFilter.authenticate(mockedRequest).block())
-            .doesNotThrowAnyException();
+        assertThatThrownBy(() -> authFilter.authenticate(mockedRequest).block())
+            .isInstanceOf(UnauthorizedException.class);
     }
 }
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/http/Authenticator.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/http/Authenticator.java
index 49fb6f5..0a7fe3d 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/http/Authenticator.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/http/Authenticator.java
@@ -53,8 +53,7 @@ public class Authenticator {
         return Mono.from(metricFactory.decoratePublisherWithTimerMetric("JMAP-authentication-filter",
             Flux.fromIterable(authMethods)
                 .concatMap(auth -> auth.createMailboxSession(request))
-                .onErrorContinue((throwable, nothing) -> LOGGER.error("Error while trying to authenticate with JMAP", throwable))
                 .next()
-                .switchIfEmpty(Mono.error(new UnauthorizedException()))));
+                .switchIfEmpty(Mono.error(new UnauthorizedException("Unexpected error")))));
     }
 }


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