You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/06/14 01:59:33 UTC
[james-project] 01/04: [PERFORMANCE] Avoid doing JWT parsing twice
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 f2542273fc176c617baa30df8c1b84cf0fc1f94f
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Jun 4 21:23:17 2021 +0700
[PERFORMANCE] Avoid doing JWT parsing twice
---
.../james/jmap/http/JWTAuthenticationStrategyTest.java | 11 +++++------
.../apache/james/jmap/jwt/JWTAuthenticationStrategy.java | 8 +++-----
.../main/java/org/apache/james/jwt/JwtTokenVerifier.java | 10 ++++++----
.../java/org/apache/james/jwt/JwtTokenVerifierTest.java | 16 ++++++++--------
.../apache/james/webadmin/authentication/JwtFilter.java | 10 ++++------
.../james/webadmin/authentication/JwtFilterTest.java | 8 +++++---
6 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JWTAuthenticationStrategyTest.java b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JWTAuthenticationStrategyTest.java
index 78b57ad..cf4ea0e 100644
--- a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JWTAuthenticationStrategyTest.java
+++ b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JWTAuthenticationStrategyTest.java
@@ -24,6 +24,8 @@ import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import java.util.Optional;
+
import org.apache.james.core.Username;
import org.apache.james.domainlist.api.DomainList;
import org.apache.james.jmap.exceptions.UnauthorizedException;
@@ -81,8 +83,7 @@ public class JWTAuthenticationStrategyTest {
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(stubTokenVerifier.verifyAndExtractLogin(validAuthHeader)).thenReturn(Optional.empty());
when(mockedMailboxManager.createSystemSession(eq(Username.of(username))))
.thenReturn(fakeMailboxSession);
@@ -121,8 +122,7 @@ public class JWTAuthenticationStrategyTest {
String fakeAuthHeaderWithPrefix = JWTAuthenticationStrategy.AUTHORIZATION_HEADER_PREFIX + validAuthHeader;
MailboxSession fakeMailboxSession = mock(MailboxSession.class);
- when(stubTokenVerifier.verify(validAuthHeader)).thenReturn(true);
- when(stubTokenVerifier.extractLogin(validAuthHeader)).thenReturn(username);
+ when(stubTokenVerifier.verifyAndExtractLogin(validAuthHeader)).thenReturn(Optional.of(username));
when(mockedMailboxManager.createSystemSession(eq(Username.of(username))))
.thenReturn(fakeMailboxSession);
when(mockedHeaders.get(AUTHORIZATION_HEADERS))
@@ -139,8 +139,7 @@ public class JWTAuthenticationStrategyTest {
String fakeAuthHeaderWithPrefix = JWTAuthenticationStrategy.AUTHORIZATION_HEADER_PREFIX + validAuthHeader;
MailboxSession fakeMailboxSession = mock(MailboxSession.class);
- when(stubTokenVerifier.verify(validAuthHeader)).thenReturn(true);
- when(stubTokenVerifier.extractLogin(validAuthHeader)).thenReturn(username);
+ when(stubTokenVerifier.verifyAndExtractLogin(validAuthHeader)).thenReturn(Optional.of(username));
when(mockedMailboxManager.createSystemSession(eq(Username.of(username))))
.thenReturn(fakeMailboxSession);
when(mockedHeaders.get(AUTHORIZATION_HEADERS))
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/jwt/JWTAuthenticationStrategy.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/jwt/JWTAuthenticationStrategy.java
index 55352af..06d8c80 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/jwt/JWTAuthenticationStrategy.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/jwt/JWTAuthenticationStrategy.java
@@ -63,11 +63,9 @@ public class JWTAuthenticationStrategy implements AuthenticationStrategy {
.filter(header -> header.startsWith(AUTHORIZATION_HEADER_PREFIX))
.map(header -> header.substring(AUTHORIZATION_HEADER_PREFIX.length()))
.flatMap(userJWTToken -> Mono.fromCallable(() -> {
- if (!tokenManager.verify(userJWTToken)) {
- throw new UnauthorizedException("Failed Jwt verification");
- }
-
- Username username = Username.of(tokenManager.extractLogin(userJWTToken));
+ Username username = tokenManager.verifyAndExtractLogin(userJWTToken)
+ .map(Username::of)
+ .orElseThrow(() -> new UnauthorizedException("Failed Jwt verification"));
try {
usersRepository.assertValid(username);
} catch (UsersRepositoryException e) {
diff --git a/server/protocols/jwt/src/main/java/org/apache/james/jwt/JwtTokenVerifier.java b/server/protocols/jwt/src/main/java/org/apache/james/jwt/JwtTokenVerifier.java
index f7f833c..6eb71ec 100644
--- a/server/protocols/jwt/src/main/java/org/apache/james/jwt/JwtTokenVerifier.java
+++ b/server/protocols/jwt/src/main/java/org/apache/james/jwt/JwtTokenVerifier.java
@@ -18,6 +18,8 @@
****************************************************************/
package org.apache.james.jwt;
+import java.util.Optional;
+
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -47,20 +49,20 @@ public class JwtTokenVerifier {
this.pubKeyProvider = pubKeyProvider;
}
- public boolean verify(String token) {
+ public Optional<String> verifyAndExtractLogin(String token) {
try {
String subject = extractLogin(token);
if (Strings.isNullOrEmpty(subject)) {
throw new MalformedJwtException("'subject' field in token is mandatory");
}
- return true;
+ return Optional.of(subject);
} catch (JwtException e) {
LOGGER.info("Failed Jwt verification", e);
- return false;
+ return Optional.empty();
}
}
- public String extractLogin(String token) throws JwtException {
+ private String extractLogin(String token) throws JwtException {
Jws<Claims> jws = parseToken(token);
return jws
.getBody()
diff --git a/server/protocols/jwt/src/test/java/org/apache/james/jwt/JwtTokenVerifierTest.java b/server/protocols/jwt/src/test/java/org/apache/james/jwt/JwtTokenVerifierTest.java
index 75b638a..08c854b 100644
--- a/server/protocols/jwt/src/test/java/org/apache/james/jwt/JwtTokenVerifierTest.java
+++ b/server/protocols/jwt/src/test/java/org/apache/james/jwt/JwtTokenVerifierTest.java
@@ -80,7 +80,7 @@ class JwtTokenVerifierTest {
@Test
void shouldReturnTrueOnValidSignature() {
- assertThat(sut.verify(VALID_TOKEN_WITHOUT_ADMIN)).isTrue();
+ assertThat(sut.verifyAndExtractLogin(VALID_TOKEN_WITHOUT_ADMIN)).isPresent();
}
@Test
@@ -89,7 +89,7 @@ class JwtTokenVerifierTest {
"tPL3EZdkeYxw_DV2KimE1U2FvuLHmfR_mimJ5US3JFU4J2Gd94O7rwpSTGN1B9h-_lsTebo4ua4xHsTtmczZ9xa8a_kWKaSkqFjNFa" +
"Fp6zcoD6ivCu03SlRqsQzSRHXo6TKbnqOt9D6Y2rNa3C4igSwoS0jUE4BgpXbc0";
- assertThat(sut.verify(invalidToken)).isFalse();
+ assertThat(sut.verifyAndExtractLogin(invalidToken)).isEmpty();
}
@Test
@@ -100,7 +100,7 @@ class JwtTokenVerifierTest {
"VPY3q15vWzZH9O9IJzB2KdHRMPxl2luRjzDbh4DLp56NhZuLX_2a9UAlmbV8MQX4Z_04ybhAYrcBfxR3MgJyr0jlxSibqSbXrkXuo-" +
"PyybfZCIhK_qXUlO5OS6sO7AQhKZO9p0MQ";
- assertThat(sut.verify(tokenWithNullSubject)).isFalse();
+ assertThat(sut.verifyAndExtractLogin(tokenWithNullSubject)).isEmpty();
}
@Test
@@ -112,22 +112,22 @@ class JwtTokenVerifierTest {
"sUSbogmgObA_BimNtUq_Q1p0SGtIYBXmQ";
- assertThat(sut.verify(tokenWithEmptySubject)).isFalse();
+ assertThat(sut.verifyAndExtractLogin(tokenWithEmptySubject)).isEmpty();
}
@Test
void verifyShouldNotAcceptNoneAlgorithm() {
- assertThat(sut.verify(TOKEN_NONE_ALGORITHM)).isFalse();
+ assertThat(sut.verifyAndExtractLogin(TOKEN_NONE_ALGORITHM)).isEmpty();
}
@Test
void verifyShouldNotAcceptNoneAlgorithmWithoutSignature() {
- assertThat(sut.verify(TOKEN_NONE_ALGORITHM_NO_SIGNATURE)).isFalse();
+ assertThat(sut.verifyAndExtractLogin(TOKEN_NONE_ALGORITHM_NO_SIGNATURE)).isEmpty();
}
@Test
void shouldReturnUserLoginFromValidToken() {
- assertThat(sut.extractLogin(VALID_TOKEN_WITHOUT_ADMIN)).isEqualTo("1234567890");
+ assertThat(sut.verifyAndExtractLogin(VALID_TOKEN_WITHOUT_ADMIN)).contains("1234567890");
}
@Test
@@ -153,7 +153,7 @@ class JwtTokenVerifierTest {
@Test
void extractLoginShouldWorkWithAdminClaim() {
- assertThat(sut.extractLogin(VALID_TOKEN_ADMIN_TRUE)).isEqualTo("admin@open-paas.org");
+ assertThat(sut.verifyAndExtractLogin(VALID_TOKEN_ADMIN_TRUE)).contains("admin@open-paas.org");
}
@Test
diff --git a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/authentication/JwtFilter.java b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/authentication/JwtFilter.java
index 972569b..ab61c60 100644
--- a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/authentication/JwtFilter.java
+++ b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/authentication/JwtFilter.java
@@ -52,10 +52,9 @@ public class JwtFilter implements AuthenticationFilter {
.map(value -> value.substring(AUTHORIZATION_HEADER_PREFIX.length()));
checkHeaderPresent(bearer);
- checkValidSignature(bearer);
+ String login = retrieveUser(bearer);
checkIsAdmin(bearer);
- String login = jwtTokenVerifier.extractLogin(bearer.get());
request.attribute(LOGIN, login);
}
}
@@ -66,10 +65,9 @@ public class JwtFilter implements AuthenticationFilter {
}
}
- private void checkValidSignature(Optional<String> bearer) {
- if (!jwtTokenVerifier.verify(bearer.get())) {
- halt(HttpStatus.UNAUTHORIZED_401, "Invalid Bearer header.");
- }
+ private String retrieveUser(Optional<String> bearer) {
+ return jwtTokenVerifier.verifyAndExtractLogin(bearer.get())
+ .orElseThrow(() -> halt(HttpStatus.UNAUTHORIZED_401, "Invalid Bearer header."));
}
private void checkIsAdmin(Optional<String> bearer) {
diff --git a/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/authentication/JwtFilterTest.java b/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/authentication/JwtFilterTest.java
index 6931493..765de36 100644
--- a/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/authentication/JwtFilterTest.java
+++ b/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/authentication/JwtFilterTest.java
@@ -24,6 +24,8 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
+import java.util.Optional;
+
import org.apache.james.jwt.JwtTokenVerifier;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -86,7 +88,7 @@ class JwtFilterTest {
Request request = mock(Request.class);
when(request.requestMethod()).thenReturn("GET");
when(request.headers(JwtFilter.AUTHORIZATION_HEADER_NAME)).thenReturn("Bearer value");
- when(jwtTokenVerifier.verify("value")).thenReturn(false);
+ when(jwtTokenVerifier.verifyAndExtractLogin("value")).thenReturn(Optional.empty());
assertThatThrownBy(() -> jwtFilter.handle(request, mock(Response.class)))
@@ -100,7 +102,7 @@ class JwtFilterTest {
Request request = mock(Request.class);
when(request.requestMethod()).thenReturn("GET");
when(request.headers(JwtFilter.AUTHORIZATION_HEADER_NAME)).thenReturn("Bearer value");
- when(jwtTokenVerifier.verify("value")).thenReturn(true);
+ when(jwtTokenVerifier.verifyAndExtractLogin("value")).thenReturn(Optional.of("value"));
when(jwtTokenVerifier.hasAttribute("admin", true, "value")).thenReturn(false);
assertThatThrownBy(() -> jwtFilter.handle(request, mock(Response.class)))
@@ -114,7 +116,7 @@ class JwtFilterTest {
Request request = mock(Request.class);
when(request.requestMethod()).thenReturn("GET");
when(request.headers(JwtFilter.AUTHORIZATION_HEADER_NAME)).thenReturn("Bearer value");
- when(jwtTokenVerifier.verify("value")).thenReturn(true);
+ when(jwtTokenVerifier.verifyAndExtractLogin("value")).thenReturn(Optional.of("value"));
when(jwtTokenVerifier.hasAttribute("admin", true, "value")).thenReturn(true);
jwtFilter.handle(request, mock(Response.class));
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org