JAMES-1784 Remove checkAuthorizationHeader in AuthenticationStrategy
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/62627357 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/62627357 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/62627357 Branch: refs/heads/master Commit: 62627357717844ed0630b68fb5541836639763af Parents: 3c7cc85 Author: Antoine Duprat <adup...@linagora.com> Authored: Mon Jul 4 13:48:31 2016 +0200 Committer: Antoine Duprat <adup...@linagora.com> Committed: Fri Jul 8 09:54:06 2016 +0200 ---------------------------------------------------------------------- .../jmap/AccessTokenAuthenticationStrategy.java | 22 +---- .../apache/james/jmap/AuthenticationFilter.java | 13 ++- .../james/jmap/AuthenticationStrategy.java | 2 - .../james/jmap/JWTAuthenticationStrategy.java | 6 -- .../AccessTokenAuthenticationStrategyTest.java | 80 +++++++------------ .../james/jmap/AuthenticationFilterTest.java | 27 +++++-- .../jmap/JWTAuthenticationStrategyTest.java | 84 +++++--------------- 7 files changed, 76 insertions(+), 158 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/62627357/server/protocols/jmap/src/main/java/org/apache/james/jmap/AccessTokenAuthenticationStrategy.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/AccessTokenAuthenticationStrategy.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/AccessTokenAuthenticationStrategy.java index d190ebd..57d903a 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/AccessTokenAuthenticationStrategy.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/AccessTokenAuthenticationStrategy.java @@ -25,7 +25,6 @@ import javax.servlet.http.HttpServletRequest; import org.apache.james.jmap.api.AccessTokenManager; import org.apache.james.jmap.api.access.AccessToken; -import org.apache.james.jmap.api.access.exceptions.NotAnAccessTokenException; import org.apache.james.jmap.exceptions.MailboxSessionCreationException; import org.apache.james.jmap.exceptions.NoValidAuthHeaderException; import org.apache.james.jmap.utils.HeadersAuthenticationExtractor; @@ -58,6 +57,7 @@ public class AccessTokenAuthenticationStrategy implements AuthenticationStrategy Optional<String> username = authenticationExtractor.authHeaders(httpRequest) .map(AccessToken::fromString) + .filter(accessTokenManager::isValid) .map(accessTokenManager::getUsernameFromToken) .findFirst(); @@ -70,24 +70,4 @@ public class AccessTokenAuthenticationStrategy implements AuthenticationStrategy } throw new NoValidAuthHeaderException(); } - - @Override - public boolean checkAuthorizationHeader(HttpServletRequest httpRequest) { - return authenticationExtractor.authHeaders(httpRequest) - .map(this::accessTokenFrom) - .anyMatch(this::isValid); - } - - private Optional<AccessToken> accessTokenFrom(String header) { - try { - return Optional.of(AccessToken.fromString(header)); - } catch (NotAnAccessTokenException e) { - return Optional.empty(); - } - } - - private boolean isValid(Optional<AccessToken> token) { - return token.map(accessTokenManager::isValid) - .orElse(false); - } } http://git-wip-us.apache.org/repos/asf/james-project/blob/62627357/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationFilter.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationFilter.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationFilter.java index d62c1c2..14823a5 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationFilter.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationFilter.java @@ -20,6 +20,7 @@ package org.apache.james.jmap; import java.io.IOException; import java.util.List; +import java.util.stream.Stream; import javax.inject.Inject; import javax.servlet.Filter; @@ -67,9 +68,9 @@ public class AuthenticationFilter implements Filter { try { HttpServletRequest requestWithSession = authMethods.stream() - .filter(auth -> auth.checkAuthorizationHeader(httpRequest)) + .flatMap(auth -> createSession(auth, httpRequest)) .findFirst() - .map(auth -> addSessionToRequest(httpRequest, createSession(auth, httpRequest))) + .map(mailboxSession -> addSessionToRequest(httpRequest, mailboxSession)) .orElseThrow(UnauthorizedException::new); chain.doFilter(requestWithSession, response); @@ -85,8 +86,12 @@ public class AuthenticationFilter implements Filter { return httpRequest; } - private MailboxSession createSession(AuthenticationStrategy authenticationMethod, HttpServletRequest httpRequest) { - return authenticationMethod.createMailboxSession(httpRequest); + private Stream<MailboxSession> createSession(AuthenticationStrategy authenticationMethod, HttpServletRequest httpRequest) { + try { + return Stream.of(authenticationMethod.createMailboxSession(httpRequest)); + } catch (Exception e) { + return Stream.empty(); + } } @Override http://git-wip-us.apache.org/repos/asf/james-project/blob/62627357/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationStrategy.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationStrategy.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationStrategy.java index d75247a..aab2286 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationStrategy.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/AuthenticationStrategy.java @@ -25,6 +25,4 @@ import org.apache.james.mailbox.MailboxSession; public interface AuthenticationStrategy { MailboxSession createMailboxSession(HttpServletRequest httpRequest); - - boolean checkAuthorizationHeader(HttpServletRequest httpRequest); } http://git-wip-us.apache.org/repos/asf/james-project/blob/62627357/server/protocols/jmap/src/main/java/org/apache/james/jmap/JWTAuthenticationStrategy.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JWTAuthenticationStrategy.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JWTAuthenticationStrategy.java index 9d9c659..a7e91be 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JWTAuthenticationStrategy.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JWTAuthenticationStrategy.java @@ -72,12 +72,6 @@ public class JWTAuthenticationStrategy implements AuthenticationStrategy { .orElseThrow(() -> new NoValidAuthHeaderException()); } - @Override - public boolean checkAuthorizationHeader(HttpServletRequest httpRequest) { - return extractTokensFromAuthHeaders(authenticationExtractor.authHeaders(httpRequest)) - .anyMatch(tokenManager::verify); - } - private Stream<String> extractTokensFromAuthHeaders(Stream<String> authHeaders) { return authHeaders .filter(h -> h.startsWith(AUTHORIZATION_HEADER_PREFIX)) http://git-wip-us.apache.org/repos/asf/james-project/blob/62627357/server/protocols/jmap/src/test/java/org/apache/james/jmap/AccessTokenAuthenticationStrategyTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/AccessTokenAuthenticationStrategyTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/AccessTokenAuthenticationStrategyTest.java index 91e18e2..b1c71b6 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/AccessTokenAuthenticationStrategyTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/AccessTokenAuthenticationStrategyTest.java @@ -80,17 +80,38 @@ public class AccessTokenAuthenticationStrategyTest { } @Test + public void createMailboxSessionShouldThrowWhenAuthHeaderIsInvalid() throws Exception { + String username = "123456789"; + MailboxSession fakeMailboxSession = mock(MailboxSession.class); + + when(mockedMailboxManager.createSystemSession(eq(username), any(Logger.class))) + .thenReturn(fakeMailboxSession); + + UUID authHeader = UUID.randomUUID(); + when(mockedAccessTokenManager.getUsernameFromToken(AccessToken.fromString(authHeader.toString()))) + .thenReturn(username); + when(mockAuthenticationExtractor.authHeaders(request)) + .thenReturn(Stream.of(authHeader.toString())); + + + assertThatThrownBy(() -> testee.createMailboxSession(request)) + .isExactlyInstanceOf(NoValidAuthHeaderException.class); + } + + @Test public void createMailboxSessionShouldThrowWhenMailboxExceptionHasOccurred() throws Exception { String username = "username"; when(mockedMailboxManager.createSystemSession(eq(username), any(Logger.class))) .thenThrow(new MailboxException()); UUID authHeader = UUID.randomUUID(); - when(mockedAccessTokenManager.getUsernameFromToken(AccessToken.fromString(authHeader.toString()))) + AccessToken accessToken = AccessToken.fromString(authHeader.toString()); + when(mockedAccessTokenManager.getUsernameFromToken(accessToken)) .thenReturn(username); when(mockAuthenticationExtractor.authHeaders(request)) .thenReturn(Stream.of(authHeader.toString())); - + when(mockedAccessTokenManager.isValid(accessToken)) + .thenReturn(true); assertThatThrownBy(() -> testee.createMailboxSession(request)) .isExactlyInstanceOf(MailboxSessionCreationException.class); @@ -105,63 +126,16 @@ public class AccessTokenAuthenticationStrategyTest { .thenReturn(fakeMailboxSession); UUID authHeader = UUID.randomUUID(); - when(mockedAccessTokenManager.getUsernameFromToken(AccessToken.fromString(authHeader.toString()))) + AccessToken accessToken = AccessToken.fromString(authHeader.toString()); + when(mockedAccessTokenManager.getUsernameFromToken(accessToken)) .thenReturn(username); when(mockAuthenticationExtractor.authHeaders(request)) .thenReturn(Stream.of(authHeader.toString())); + when(mockedAccessTokenManager.isValid(accessToken)) + .thenReturn(true); MailboxSession result = testee.createMailboxSession(request); assertThat(result).isEqualTo(fakeMailboxSession); } - - @Test - public void checkAuthorizationHeaderShouldReturnFalseWhenAuthHeaderIsEmpty() { - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(Stream.empty()); - - assertThat(testee.checkAuthorizationHeader(request)).isFalse(); - } - - @Test - public void checkAuthorizationHeaderShouldReturnFalseWhenAuthHeaderIsInvalid() { - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(Stream.of("bad")); - - assertThat(testee.checkAuthorizationHeader(request)).isFalse(); - } - - @Test - public void checkAuthorizationHeaderShouldReturnFalseWhenAuthHeadersAreInvalid() { - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(Stream.of("bad", "alsobad")); - - assertThat(testee.checkAuthorizationHeader(request)).isFalse(); - } - - @Test - public void checkAuthorizationHeaderShouldReturnTrueWhenAuthHeaderIsValid() { - - String validToken = UUID.randomUUID().toString(); - when(mockedAccessTokenManager.isValid(AccessToken.fromString(validToken))) - .thenReturn(true); - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(Stream.of(validToken)); - - - assertThat(testee.checkAuthorizationHeader(request)).isTrue(); - } - - @Test - public void checkAuthorizationHeaderShouldReturnTrueWhenOneAuthHeaderIsValid() { - - String validToken = UUID.randomUUID().toString(); - when(mockedAccessTokenManager.isValid(AccessToken.fromString(validToken))) - .thenReturn(true); - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(Stream.of("bad", validToken)); - - - assertThat(testee.checkAuthorizationHeader(request)).isTrue(); - } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/62627357/server/protocols/jmap/src/test/java/org/apache/james/jmap/AuthenticationFilterTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/AuthenticationFilterTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/AuthenticationFilterTest.java index 837ff9f..8a189b7 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/AuthenticationFilterTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/AuthenticationFilterTest.java @@ -34,6 +34,7 @@ import javax.servlet.http.HttpServletResponse; 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.memory.access.MemoryAccessTokenRepository; import org.apache.james.mailbox.MailboxSession; import org.junit.Before; @@ -99,6 +100,20 @@ public class AuthenticationFilterTest { } @Test + public void filterShouldChainAuthorizationStrategy() throws Exception { + AccessToken token = AccessToken.fromString(TOKEN); + when(mockedRequest.getHeader("Authorization")) + .thenReturn(TOKEN); + + accessTokenRepository.addToken("u...@domain.tld", token); + + AuthenticationFilter sut = new AuthenticationFilter(ImmutableList.of(new FakeAuthenticationStrategy(false), new FakeAuthenticationStrategy(true))); + sut.doFilter(mockedRequest, mockedResponse, filterChain); + + verify(filterChain).doFilter(any(ServletRequest.class), eq(mockedResponse)); + } + + @Test public void filterShouldReturnUnauthorizedOnBadAuthorizationHeader() throws Exception { when(mockedRequest.getHeader("Authorization")) .thenReturn("bad"); @@ -119,7 +134,7 @@ public class AuthenticationFilterTest { verify(mockedResponse).sendError(HttpServletResponse.SC_UNAUTHORIZED); } - private class FakeAuthenticationStrategy implements AuthenticationStrategy { + private static class FakeAuthenticationStrategy implements AuthenticationStrategy { private final boolean isAuthorized; @@ -129,12 +144,10 @@ public class AuthenticationFilterTest { @Override public MailboxSession createMailboxSession(HttpServletRequest httpRequest) { - return null; - } - - @Override - public boolean checkAuthorizationHeader(HttpServletRequest httpRequest) { - return isAuthorized; + if (!isAuthorized) { + throw new MailboxSessionCreationException(null); + } + return mock(MailboxSession.class); } } } http://git-wip-us.apache.org/repos/asf/james-project/blob/62627357/server/protocols/jmap/src/test/java/org/apache/james/jmap/JWTAuthenticationStrategyTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/JWTAuthenticationStrategyTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/JWTAuthenticationStrategyTest.java index c96b7a8..4a60578 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/JWTAuthenticationStrategyTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/JWTAuthenticationStrategyTest.java @@ -70,6 +70,25 @@ public class JWTAuthenticationStrategyTest { } @Test + public void createMailboxSessionShouldThrownWhenAuthHeadersIsInvalid() throws Exception { + String username = "123456789"; + String validAuthHeader = "valid"; + String fakeAuthHeaderWithPrefix = JWTAuthenticationStrategy.AUTHORIZATION_HEADER_PREFIX + validAuthHeader; + MailboxSession fakeMailboxSession = mock(MailboxSession.class); + + when(stubTokenVerifier.verify(validAuthHeader)).thenReturn(false); + when(stubTokenVerifier.extractLogin(validAuthHeader)).thenReturn(username); + when(mockedMailboxManager.createSystemSession(eq(username), any(Logger.class))) + .thenReturn(fakeMailboxSession); + when(mockAuthenticationExtractor.authHeaders(request)) + .thenReturn(Stream.of(fakeAuthHeaderWithPrefix)); + + + assertThatThrownBy(() -> testee.createMailboxSession(request)) + .isExactlyInstanceOf(NoValidAuthHeaderException.class); + } + + @Test public void createMailboxSessionShouldReturnEmptyWhenAuthHeaderIsInvalid() throws Exception { when(mockAuthenticationExtractor.authHeaders(request)) .thenReturn(Stream.of("bad")); @@ -113,69 +132,4 @@ public class JWTAuthenticationStrategyTest { MailboxSession result = testee.createMailboxSession(request); assertThat(result).isEqualTo(fakeMailboxSession); } - - @Test - public void checkAuthorizationHeaderShouldReturnFalsewWhenAuthHeaderIsEmpty() { - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(Stream.empty()); - - assertThat(testee.checkAuthorizationHeader(request)).isFalse(); - } - - @Test - public void checkAuthorizationHeaderShouldReturnFalseWhenAuthHeaderIsInvalid() { - String wrongAuthHeader = "invalid"; - String fakeAuthHeaderWithPrefix = JWTAuthenticationStrategy.AUTHORIZATION_HEADER_PREFIX + wrongAuthHeader; - - when(stubTokenVerifier.verify(wrongAuthHeader)).thenReturn(false); - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(Stream.of(fakeAuthHeaderWithPrefix)); - - assertThat(testee.checkAuthorizationHeader(request)).isFalse(); - } - - @Test - public void checkAuthorizationHeaderShouldReturnFalseWhenAuthHeadersAreInvalid() { - String wrongAuthHeader = "invalid"; - String invalidAuthHeader = "INVALID"; - - when(stubTokenVerifier.verify(wrongAuthHeader)).thenReturn(false); - when(stubTokenVerifier.verify(invalidAuthHeader)).thenReturn(false); - - Stream<String> authHeadersStream = Stream.of(wrongAuthHeader, invalidAuthHeader) - .map(h -> JWTAuthenticationStrategy.AUTHORIZATION_HEADER_PREFIX + h); - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(authHeadersStream); - - assertThat(testee.checkAuthorizationHeader(request)).isFalse(); - } - - @Test - public void checkAuthorizationHeaderShouldReturnTrueWhenAuthHeaderIsValid() { - String validAuthHeader = "valid"; - String validAuthHeaderWithPrefix = JWTAuthenticationStrategy.AUTHORIZATION_HEADER_PREFIX + validAuthHeader; - - when(stubTokenVerifier.verify(validAuthHeader)).thenReturn(true); - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(Stream.of(validAuthHeaderWithPrefix)); - - assertThat(testee.checkAuthorizationHeader(request)).isTrue(); - } - - @Test - public void checkAuthorizationHeaderShouldReturnTrueWhenOneAuthHeaderIsValid() { - String dummyAuthHeader = "invalid"; - String validAuthHeader = "correct"; - - when(stubTokenVerifier.verify(dummyAuthHeader)).thenReturn(false); - when(stubTokenVerifier.verify(validAuthHeader)).thenReturn(true); - - Stream<String> authHeadersStream = Stream.of(dummyAuthHeader, validAuthHeader) - .map(h -> JWTAuthenticationStrategy.AUTHORIZATION_HEADER_PREFIX + h); - when(mockAuthenticationExtractor.authHeaders(request)) - .thenReturn(authHeadersStream); - - assertThat(testee.checkAuthorizationHeader(request)).isTrue(); - } - } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org