This is an automated email from the ASF dual-hosted git repository.
pzampino pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new c758be4 KNOX-2228 - JWTFilter should handle unknown token exception
from token state service (#260)
c758be4 is described below
commit c758be4775e53fcb869bd70c711be12597a6a2b2
Author: Phil Zampino <[email protected]>
AuthorDate: Mon Feb 10 15:39:31 2020 -0500
KNOX-2228 - JWTFilter should handle unknown token exception from token
state service (#260)
---
.../provider/federation/jwt/JWTMessages.java | 5 +-
.../federation/jwt/filter/AbstractJWTFilter.java | 11 +++--
.../provider/federation/CommonJWTFilterTest.java | 56 ++++++++++++++++++++++
.../token/impl/AliasBasedTokenStateService.java | 19 ++++----
.../token/impl/DefaultTokenStateService.java | 26 ++++------
.../token/impl/TokenStateServiceMessages.java | 26 ++++++----
.../service/knoxtoken/TokenServiceMessages.java | 34 -------------
7 files changed, 103 insertions(+), 74 deletions(-)
diff --git
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
index e92d204..42baa30 100644
---
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
+++
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
@@ -41,7 +41,10 @@ public interface JWTMessages {
@Message( level = MessageLevel.INFO, text = "Unable to verify token: {0}" )
void unableToVerifyToken(@StackTrace( level = MessageLevel.ERROR) Exception
e);
- @Message( level = MessageLevel.ERROR, text = "Unable to verify token: {0}" )
+ @Message( level = MessageLevel.WARN, text = "Unable to verify token
expiration: {0}" )
+ void unableToVerifyExpiration(@StackTrace( level = MessageLevel.DEBUG)
Exception e);
+
+ @Message( level = MessageLevel.ERROR, text = "Unable to issue token: {0}" )
void unableToIssueToken(@StackTrace( level = MessageLevel.DEBUG) Exception
e);
@Message( level = MessageLevel.DEBUG, text = "Sending redirect to: {0}" )
diff --git
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
index 25813f4..5b885a3 100644
---
a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
+++
b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
@@ -170,7 +170,13 @@ public abstract class AbstractJWTFilter implements Filter {
protected boolean tokenIsStillValid(JWT jwtToken) {
Date expires;
if (tokenStateService != null) {
- expires = new
Date(tokenStateService.getTokenExpiration(jwtToken.toString()));
+ long timestamp = 0;
+ try {
+ timestamp = tokenStateService.getTokenExpiration(jwtToken.toString());
+ } catch (Exception e) {
+ log.unableToVerifyExpiration(e);
+ }
+ expires = new Date(timestamp);
} else {
// if there is no expiration date then the lifecycle is tied entirely to
// the cookie validity - otherwise ensure that the current time is before
@@ -328,8 +334,7 @@ public abstract class AbstractJWTFilter implements Filter {
handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
"Bad request: missing required token
audience");
}
- }
- else {
+ } else {
log.tokenHasExpired();
handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
"Bad request: token has expired");
diff --git
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/CommonJWTFilterTest.java
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/CommonJWTFilterTest.java
index 93bd51b..68fd502 100644
---
a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/CommonJWTFilterTest.java
+++
b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/CommonJWTFilterTest.java
@@ -19,6 +19,7 @@ package org.apache.knox.gateway.provider.federation;
import org.apache.knox.gateway.config.GatewayConfig;
import
org.apache.knox.gateway.provider.federation.jwt.filter.AbstractJWTFilter;
import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.security.token.impl.JWT;
import org.easymock.EasyMock;
import org.junit.After;
import org.junit.Before;
@@ -33,8 +34,10 @@ import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
+import java.lang.reflect.Field;
import java.lang.reflect.Method;
+import static org.easymock.EasyMock.anyObject;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -109,6 +112,59 @@ public class CommonJWTFilterTest {
return (Boolean) m.invoke(handler, fc);
}
+ @Test
+ public void testIsStillValid() throws Exception {
+ assertTrue("Expected the token to be valid because it has not yet
expired.",
+ doTestIsStillValid(System.currentTimeMillis() + 300000)); // 5
minutes later
+ }
+
+ @Test
+ public void testIsStillValidExpired() throws Exception {
+ assertFalse("Expected the token to be invalid because it has already
expired.",
+ doTestIsStillValid(System.currentTimeMillis() - 300000)); // 5
minutes ago
+ }
+
+ @Test
+ public void testIsStillValidUnknownToken() throws Exception {
+ TokenStateService tss = EasyMock.createNiceMock(TokenStateService.class);
+ EasyMock.expect(tss.getTokenExpiration(anyObject()))
+ .andThrow(new IllegalArgumentException("Unknown token"))
+ .anyTimes();
+ EasyMock.replay(tss);
+
+ assertFalse("Expected the token to be invalid because it in an unknown
token.",
+ doTestIsStillValid(tss));
+ }
+
+ private boolean doTestIsStillValid(final Long expiration) throws Exception {
+ TokenStateService tss = EasyMock.createNiceMock(TokenStateService.class);
+ EasyMock.expect(tss.getTokenExpiration(anyObject()))
+ .andReturn(expiration)
+ .anyTimes();
+ EasyMock.replay(tss);
+ return doTestIsStillValid(tss);
+ }
+
+ private boolean doTestIsStillValid(final TokenStateService tss) throws
Exception {
+ GatewayConfig gwConf = EasyMock.createNiceMock(GatewayConfig.class);
+
EasyMock.expect(gwConf.isServerManagedTokenStateEnabled()).andReturn(true).anyTimes();
+ EasyMock.replay(gwConf);
+
+ ServletContext sc = EasyMock.createNiceMock(ServletContext.class);
+
EasyMock.expect(sc.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(gwConf).anyTimes();
+ EasyMock.replay(sc);
+
+ JWT jwt = EasyMock.createNiceMock(JWT.class);
+ EasyMock.replay(jwt);
+
+ Field tokenStateServiceField =
AbstractJWTFilter.class.getDeclaredField("tokenStateService");
+ tokenStateServiceField.setAccessible(true);
+ tokenStateServiceField.set(handler, tss);
+
+ Method m = AbstractJWTFilter.class.getDeclaredMethod("tokenIsStillValid",
JWT.class);
+ m.setAccessible(true);
+ return (Boolean) m.invoke(handler, jwt);
+ }
static final class TestHandler extends AbstractJWTFilter {
@Override
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
index 6d29cae..1be3259 100644
---
a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
+++
b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
@@ -48,13 +48,12 @@ public class AliasBasedTokenStateService extends
DefaultTokenStateService {
long expiration,
long maxLifetimeDuration) {
isValidIdentifier(token);
-
try {
aliasService.addAliasForCluster(AliasService.NO_CLUSTER_NAME, token,
String.valueOf(expiration));
setMaxLifetime(token, issueTime, maxLifetimeDuration);
- log.addedToken(getTokenDisplayText(token));
+ log.addedToken(getTokenDisplayText(token),
getTimestampDisplay(expiration));
} catch (AliasServiceException e) {
- log.failedToSaveTokenState(e);
+ log.failedToSaveTokenState(getTokenDisplayText(token), e);
}
}
@@ -65,7 +64,7 @@ public class AliasBasedTokenStateService extends
DefaultTokenStateService {
token + "--max",
String.valueOf(issueTime +
maxLifetimeDuration));
} catch (AliasServiceException e) {
- log.failedToSaveTokenState(e);
+ log.failedToSaveTokenState(getTokenDisplayText(token), e);
}
}
@@ -79,7 +78,7 @@ public class AliasBasedTokenStateService extends
DefaultTokenStateService {
result = Long.parseLong(new String(maxLifetimeStr));
}
} catch (AliasServiceException e) {
- log.errorAccessingTokenState(e);
+ log.errorAccessingTokenState(getTokenDisplayText(token), e);
}
return result;
}
@@ -96,7 +95,7 @@ public class AliasBasedTokenStateService extends
DefaultTokenStateService {
expiration = Long.parseLong(new String(expStr));
}
} catch (Exception e) {
- log.errorAccessingTokenState(e);
+ log.errorAccessingTokenState(getTokenDisplayText(token), e);
}
return expiration;
@@ -115,7 +114,7 @@ public class AliasBasedTokenStateService extends
DefaultTokenStateService {
try {
isUnknown =
(aliasService.getPasswordFromAliasForCluster(AliasService.NO_CLUSTER_NAME,
token) == null);
} catch (AliasServiceException e) {
- log.errorAccessingTokenState(e);
+ log.errorAccessingTokenState(getTokenDisplayText(token), e);
}
return isUnknown;
}
@@ -127,10 +126,10 @@ public class AliasBasedTokenStateService extends
DefaultTokenStateService {
try {
aliasService.removeAliasForCluster(AliasService.NO_CLUSTER_NAME, token);
aliasService.removeAliasForCluster(AliasService.NO_CLUSTER_NAME,token +
"--max");
+ log.removedTokenState(getTokenDisplayText(token));
} catch (AliasServiceException e) {
- log.failedToUpdateTokenExpiration(e);
+ log.failedToRemoveTokenState(getTokenDisplayText(token), e);
}
-
}
@Override
@@ -144,7 +143,7 @@ public class AliasBasedTokenStateService extends
DefaultTokenStateService {
aliasService.removeAliasForCluster(AliasService.NO_CLUSTER_NAME, token);
aliasService.addAliasForCluster(AliasService.NO_CLUSTER_NAME, token,
String.valueOf(expiration));
} catch (AliasServiceException e) {
- log.failedToUpdateTokenExpiration(e);
+ log.failedToUpdateTokenExpiration(getTokenDisplayText(token), e);
}
}
}
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java
b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java
index e158154..3286c81 100644
---
a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java
+++
b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java
@@ -22,6 +22,7 @@ import
org.apache.knox.gateway.services.ServiceLifecycleException;
import org.apache.knox.gateway.services.security.token.TokenStateService;
import org.apache.knox.gateway.services.security.token.impl.JWTToken;
+import java.time.Instant;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
@@ -91,7 +92,7 @@ public class DefaultTokenStateService implements
TokenStateService {
tokenExpirations.put(token, expiration);
}
setMaxLifetime(token, issueTime, maxLifetimeDuration);
- log.addedToken(getTokenDisplayText(token));
+ log.addedToken(getTokenDisplayText(token),
getTimestampDisplay(expiration));
}
@Override
@@ -129,13 +130,13 @@ public class DefaultTokenStateService implements
TokenStateService {
public long renewToken(final String token, long renewInterval) {
long expiration;
- validateToken(token, true);
+ validateToken(token);
// Make sure the maximum lifetime has not been (and will not be) exceeded
if (hasRemainingRenewals(token, renewInterval)) {
expiration = System.currentTimeMillis() + renewInterval;
updateExpiration(token, expiration);
- log.renewedToken(getTokenDisplayText(token));
+ log.renewedToken(getTokenDisplayText(token),
getTimestampDisplay(expiration));
} else {
log.renewalLimitExceeded(token);
throw new IllegalArgumentException("The renewal limit for the token has
been exceeded");
@@ -205,11 +206,12 @@ public class DefaultTokenStateService implements
TokenStateService {
protected void removeToken(final String token) {
validateToken(token);
synchronized (tokenExpirations) {
- tokenExpirations.remove(token);
+ tokenExpirations.remove(token);
}
synchronized (maxTokenLifetimes) {
maxTokenLifetimes.remove(token);
}
+ log.removedTokenState(getTokenDisplayText(token));
}
protected boolean hasRemainingRenewals(final String token, long
renewInterval) {
@@ -237,18 +239,6 @@ public class DefaultTokenStateService implements
TokenStateService {
* @throws IllegalArgumentException if the specified token in invalid.
*/
protected void validateToken(final String token) throws
IllegalArgumentException {
- validateToken(token, false);
- }
-
- /**
- * Validate the specified token identifier.
- *
- * @param token The token identifier to validate.
- * @param includeRevocation true, if the revocation status of the specified
token should be considered in the validation.
- *
- * @throws IllegalArgumentException if the specified token in invalid.
- */
- protected void validateToken(final String token, boolean includeRevocation)
throws IllegalArgumentException {
if (!isValidIdentifier(token)) {
throw new IllegalArgumentException("Token data cannot be null.");
}
@@ -264,4 +254,8 @@ public class DefaultTokenStateService implements
TokenStateService {
return String.format(Locale.ROOT, "%s...%s", token.substring(0, 10),
token.substring(token.length() - 3));
}
+ protected String getTimestampDisplay(long timestamp) {
+ return Instant.ofEpochMilli(timestamp).toString();
+ }
+
}
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateServiceMessages.java
b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateServiceMessages.java
index 8b7d9b5..d03833c 100644
---
a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateServiceMessages.java
+++
b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateServiceMessages.java
@@ -24,28 +24,34 @@ import org.apache.knox.gateway.i18n.messages.StackTrace;
@Messages(logger = "org.apache.knox.gateway.services.token.state")
public interface TokenStateServiceMessages {
- @Message(level = MessageLevel.DEBUG, text = "Added token {0}")
- void addedToken(String tokenDisplayText);
+ @Message(level = MessageLevel.DEBUG, text = "Added token {0}, expiration
{1}")
+ void addedToken(String tokenDisplayText, String expiration);
- @Message(level = MessageLevel.DEBUG, text = "Renewed token {0}")
- void renewedToken(String tokenDisplayText);
+ @Message(level = MessageLevel.DEBUG, text = "Renewed token {0}, expiration
{1}")
+ void renewedToken(String tokenDisplayText, String expiration);
@Message(level = MessageLevel.DEBUG, text = "Revoked token {0}")
void revokedToken(String tokenDisplayText);
+ @Message(level = MessageLevel.DEBUG, text = "Removed state for token {0}")
+ void removedTokenState(String tokenDisplayText);
+
@Message(level = MessageLevel.DEBUG, text = "Unknown token {0}")
void unknownToken(String tokenDisplayText);
@Message(level = MessageLevel.ERROR, text = "The renewal limit for the token
({0}) has been exceeded.")
void renewalLimitExceeded(String tokenDisplayText);
- @Message(level = MessageLevel.ERROR, text = "Failed to save token state:
{0}")
- void failedToSaveTokenState(@StackTrace(level = MessageLevel.DEBUG)
Exception e);
+ @Message(level = MessageLevel.ERROR, text = "Failed to save state for token
{0} : {1}")
+ void failedToSaveTokenState(String tokenDisplayText, @StackTrace(level =
MessageLevel.DEBUG) Exception e);
+
+ @Message(level = MessageLevel.ERROR, text = "Error accessing state for token
{0} : {1}")
+ void errorAccessingTokenState(String tokenDisplayText, @StackTrace(level =
MessageLevel.DEBUG) Exception e);
- @Message(level = MessageLevel.ERROR, text = "Error accessing token state:
{0}")
- void errorAccessingTokenState(@StackTrace(level = MessageLevel.DEBUG)
Exception e);
+ @Message(level = MessageLevel.ERROR, text = "Failed to update expiration for
token {1} : {1}")
+ void failedToUpdateTokenExpiration(String tokenDisplayText,
@StackTrace(level = MessageLevel.DEBUG) Exception e);
- @Message(level = MessageLevel.ERROR, text = "Failed to update token
expiration: {0}")
- void failedToUpdateTokenExpiration(@StackTrace(level = MessageLevel.DEBUG)
Exception e);
+ @Message(level = MessageLevel.ERROR, text = "Failed to remove state for
token {0} : {1}")
+ void failedToRemoveTokenState(String tokenDisplayText, @StackTrace(level =
MessageLevel.DEBUG) Exception e);
}
diff --git
a/gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceMessages.java
b/gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceMessages.java
index c98c87f..df374ce 100644
---
a/gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceMessages.java
+++
b/gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceMessages.java
@@ -24,47 +24,13 @@ import org.apache.knox.gateway.i18n.messages.StackTrace;
@Messages(logger="org.apache.knox.gateway.service.knoxtoken")
public interface TokenServiceMessages {
- @Message( level = MessageLevel.INFO, text = "About to redirect to original
URL: {0}")
- void aboutToRedirectToOriginal(String original);
-
- @Message( level = MessageLevel.DEBUG, text = "Adding the following JWT token
as a cookie: {0}")
- void addingJWTCookie(String token);
-
- @Message( level = MessageLevel.INFO, text = "Unable to find cookie with
name: {0}")
- void cookieNotFound(String name);
-
- @Message( level = MessageLevel.ERROR, text = "Unable to properly send needed
HTTP status code: {0}, {1}")
- void unableToCloseOutputStream(String message, String string);
-
- @Message( level = MessageLevel.ERROR, text = "Unable to add cookie to
response. {0}: {1}")
- void unableAddCookieToResponse(String message, String stackTrace);
-
- @Message( level = MessageLevel.ERROR, text = "Original URL not found in
request.")
- void originalURLNotFound();
-
- @Message( level = MessageLevel.INFO, text = "JWT cookie successfully added.")
- void addedJWTCookie();
@Message( level = MessageLevel.ERROR, text = "Unable to issue token.")
void unableToIssueToken(@StackTrace( level = MessageLevel.DEBUG) Exception
e);
- @Message( level = MessageLevel.WARN,
- text = "The SSO cookie SecureOnly flag is set to FALSE and is
therefore insecure.")
- void cookieSecureOnly(boolean secureOnly);
-
- @Message( level = MessageLevel.WARN, text = "The SSO cookie max age
configuration is invalid: {0} - using default.")
- void invalidMaxAgeEncountered(String age);
-
@Message( level = MessageLevel.WARN, text = "The SSO token time to live -
ttl is invalid: {0} - using default.")
void invalidTokenTTLEncountered(String ttl);
- @Message( level = MessageLevel.INFO, text = "The cookie max age is being set
to: {0}.")
- void setMaxAge(String age);
-
- @Message( level = MessageLevel.ERROR, text = "The original URL: {0} for
redirecting back after authentication is " +
- "not valid according to the configured whitelist: {1}. See documentation
for KnoxSSO Whitelisting.")
- void whiteListMatchFail(String original, String whitelist);
-
@Message( level = MessageLevel.WARN,
text = "Unable to acquire cert for endpoint clients - assume trust
will be provisioned separately: {0}.")
void unableToAcquireCertForEndpointClients(@StackTrace( level =
MessageLevel.DEBUG ) Exception e);
