[
https://issues.apache.org/jira/browse/KNOX-2566?focusedWorklogId=575523&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-575523
]
ASF GitHub Bot logged work on KNOX-2566:
----------------------------------------
Author: ASF GitHub Bot
Created on: 01/Apr/21 14:10
Start Date: 01/Apr/21 14:10
Worklog Time Spent: 10m
Work Description: lmccay commented on a change in pull request #427:
URL: https://github.com/apache/knox/pull/427#discussion_r605678987
##########
File path:
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
##########
@@ -69,4 +69,8 @@
text = "The configuration value ({0}) for maximum token
verification cache is invalid; Using the default value." )
void invalidVerificationCacheMaxConfiguration(String value);
+ @Message( level = MessageLevel.ERROR,
Review comment:
Should this really be considered an error?
We actually have support for 3rd party tokens which relying on this
KnoxToken specific claim breaks backward compatibility for. I think this is a
warning at best and we can extend the message to indicate that it is assumed to
be a 3rd party token.
##########
File path:
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
##########
@@ -385,12 +387,18 @@ protected boolean validateToken(final HttpServletRequest
request,
if (tokenStateService != null) {
try {
- if (tokenIsStillValid(tokenId)) {
- return true;
+ if (tokenId != null) {
+ if (tokenIsStillValid(tokenId)) {
+ return true;
+ } else {
+ log.tokenHasExpired(Tokens.getTokenIDDisplayText(tokenId));
+ handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
+ "Bad request: token has expired");
+ }
} else {
- log.tokenHasExpired(Tokens.getTokenIDDisplayText(tokenId));
+ log.missingTokenPasscode();
handleValidationError(request, response,
HttpServletResponse.SC_BAD_REQUEST,
- "Bad request: token has expired");
Review comment:
It seems has though we are assuming that if token state server is
enabled for a given KnoxToken service that it cannot also accept 3rd party
tokens. Is this a necessary requirement? Would falling back to only using the
token expiry be acceptable? It is sort of unclear how we would verify the
signature given today's approach to signing keys but if we were to add support
for jwks for getting the public key for verification, it seems that we could
fully support 3rd party tokens as well as KnoxTokens in the same provider
deployment. If we waited for the jwks support to make that work it may be
harder to find and fix these.
##########
File path:
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
##########
@@ -407,7 +415,7 @@ protected boolean verifyTokenSignature(final JWT token) {
String tokenId = TokenUtils.getTokenId(token);
// Check if the token has already been verified
- verified = hasSignatureBeenVerified(tokenId);
+ verified = (tokenId != null) && hasSignatureBeenVerified(tokenId);
Review comment:
if we do decide to support 3rd party tokens, we would likely want to
make sure that such tokens also benefit from the caching. Which seems to imply
that we would either need to derive a tokenId from the 3rd party token somehow
or use the actual token as the id. The latter would have uniqueness
implications across jobs but may be okay for this optimization. If the token is
exactly the same then it is the same user and created at exactly the same time
with the exact same expiry?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 575523)
Time Spent: 20m (was: 10m)
> JWT Token Signature Verification Caching NPE
> --------------------------------------------
>
> Key: KNOX-2566
> URL: https://issues.apache.org/jira/browse/KNOX-2566
> Project: Apache Knox
> Issue Type: Bug
> Components: Server
> Affects Versions: 1.6.0
> Reporter: Philip Zampino
> Assignee: Philip Zampino
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> For JWT tokens that have not been issued by Knox, but which Knox can verify,
> the signature verification caching enhancement in the JWT providers
> (KNOX-2544) throws a NPE because it's assuming that all JWTs have been issued
> by Knox and have a Knox-token-specific claim.
> The providers should be able to handle these cases without throwing an
> exception.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
