knox-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pzamp...@apache.org
Subject [knox] branch master updated: KNOX-2228 - JWTFilter should handle unknown token exception from token state service (#260)
Date Mon, 10 Feb 2020 20:39:40 GMT
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 <pzampino@apache.org>
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);


Mime
View raw message