smolnar82 commented on code in PR #1005:
URL: https://github.com/apache/knox/pull/1005#discussion_r2001616809
##########
gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTFederationFilterTest.java:
##########
@@ -43,6 +43,10 @@
@SuppressWarnings("PMD.TestClassWithoutTestCases")
public class JWTFederationFilterTest extends AbstractJWTFilterTest {
+
+ private static final String BASIC_ = "Basic ";
+ private static final String BEARER_ = "Bearer ";
Review Comment:
Is there any reason why `JWTFederationFilter.BEARER` and
`JWTFederationFilter.BASIC` cannot be reused? I see we have an extra space
after "Basic", but the "Bearer " seems to be the same.
Out of the scope of this PR, but we should consider introduce common
constant interfaces (the `BEARER` constant is listed in at least 4 classes as
`private`).
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java:
##########
@@ -279,7 +279,14 @@ public Pair<TokenType, String> getWireToken(final
ServletRequest request) throws
// what follows the bearer designator should be the JWT token
being used
// to request or as an access token
token = header.substring(BEARER.length());
- parsed = Pair.of(TokenType.JWT, token);
+
+ // if this appears to be a JWT token then attempt to use it as
such
+ // otherwise assume it is a passcode token
+ if (isJWT(token)) {
Review Comment:
Please ignore my comment above, I realized it would require way more work
than I anticipated in the first place and overcomplicate this simple check.
Your approach should work fine.
--
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.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
