You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/03/25 18:57:34 UTC
[GitHub] [knox] lmccay commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
lmccay commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601759902
##########
File path: gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
##########
@@ -342,7 +376,34 @@ protected boolean validateToken(HttpServletRequest request, HttpServletResponse
return false;
}
- protected boolean verifyTokenSignature(final JWT token) {
+ protected boolean validateToken(final HttpServletRequest request,
+ final HttpServletResponse response,
+ final FilterChain chain,
+ final String tokenId)
+ throws IOException, ServletException {
+
+ if (tokenStateService != null) {
+ try {
+// TokenMetadata metadata = tokenStateService.getTokenMetadata(tokenId);
+// if (expectedIssuer.equals(metadata.getIssuer())) { // TODO: PJZ: Additional metadata support for validation
Review comment:
remove commented out code?
##########
File path: gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
##########
@@ -174,11 +175,9 @@ protected void configureExpectedParameters(FilterConfig filterConfig) {
return audList;
}
- protected boolean tokenIsStillValid(JWT jwtToken) throws UnknownTokenException {
- Date expires;
- if (tokenStateService != null) {
- expires = new Date(tokenStateService.getTokenExpiration(jwtToken));
- } else {
+ protected boolean tokenIsStillValid(final JWT jwtToken) throws UnknownTokenException {
+ Date expires = getServerManagedStateExpiration(TokenUtils.getTokenId(jwtToken));
+ if (expires == null) {
Review comment:
Is it possible here that an unknown token will return null and be considered still valid?
##########
File path: gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java
##########
@@ -187,6 +186,22 @@ protected boolean tokenIsStillValid(JWT jwtToken) throws UnknownTokenException {
return expires == null || new Date().before(expires);
}
+ protected boolean tokenIsStillValid(final String tokenId) throws UnknownTokenException {
Review comment:
Is it possible here that an unknown token will return null and be considered still valid?
##########
File path: gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
##########
@@ -93,55 +99,74 @@ public void destroy() {
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
- final String wireToken = getWireToken(request);
+ final Pair<TokenType, String> wireToken = getWireToken(request);
if (wireToken != null) {
- try {
- JWT token = new JWTToken(wireToken);
- if (validateToken((HttpServletRequest)request, (HttpServletResponse)response, chain, token)) {
- Subject subject = createSubjectFromToken(token);
- continueWithEstablishedSecurityContext(subject, (HttpServletRequest)request, (HttpServletResponse)response, chain);
+ TokenType tokenType = wireToken.getLeft();
+ String tokenValue = wireToken.getRight();
+
+ if (TokenType.JWT.equals(tokenType)) {
+ try {
+ JWT token = new JWTToken(tokenValue);
+ if (validateToken((HttpServletRequest) request, (HttpServletResponse) response, chain, token)) {
+ Subject subject = createSubjectFromToken(token);
+ continueWithEstablishedSecurityContext(subject, (HttpServletRequest) request, (HttpServletResponse) response, chain);
+ }
+ } catch (ParseException ex) {
+ ((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ }
+ } else if (TokenType.Passcode.equals(tokenType)) {
+ // Validate the token based on the server-managed metadata
+ if (validateToken((HttpServletRequest) request, (HttpServletResponse) response, chain, tokenValue)) {
+ Subject subject = createSubjectFromTokenIdentifier(tokenValue);
+ continueWithEstablishedSecurityContext(subject, (HttpServletRequest) request, (HttpServletResponse) response, chain);
}
- } catch (ParseException ex) {
- ((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
}
- }
- else {
+ } else {
// no token provided in header
((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
}
}
- public String getWireToken(final ServletRequest request) {
+ public Pair<TokenType, String> getWireToken(final ServletRequest request) {
+ Pair<TokenType, String> parsed = null;
String token = null;
final String header = ((HttpServletRequest)request).getHeader("Authorization");
if (header != null) {
if (header.startsWith(BEARER)) {
// what follows the bearer designator should be the JWT token being used
- // to request or as an access token
+ // to request or as an access token
token = header.substring(BEARER.length());
- }
- else if (header.toLowerCase(Locale.ROOT).startsWith(BASIC.toLowerCase(Locale.ROOT))) {
- // what follows the Basic designator should be the JWT token being used
- // to request or as an access token
- token = this.parseFromHTTPBasicCredentials(token, header);
+ parsed = Pair.of(TokenType.JWT, token);
+ } else if (header.toLowerCase(Locale.ROOT).startsWith(BASIC.toLowerCase(Locale.ROOT))) {
+ // what follows the Basic designator should be the JWT token or the unique token ID being used
+ // to request or as an access token
+ parsed = parseFromHTTPBasicCredentials(header);
}
}
- if (token == null) {
+
+ if (parsed == null) {
token = request.getParameter(this.paramName);
+ if (token != null) {
+ parsed = Pair.of(TokenType.JWT, token);
+ }
}
- return token;
+
+ return parsed;
}
- private String parseFromHTTPBasicCredentials(String token, final String header) {
+ private Pair<TokenType, String> parseFromHTTPBasicCredentials(final String header) {
+ Pair<TokenType, String> parsed = null;
final String base64Credentials = header.substring(BASIC.length()).trim();
final byte[] credDecoded = Base64.getDecoder().decode(base64Credentials);
final String credentials = new String(credDecoded, StandardCharsets.UTF_8);
final String[] values = credentials.split(":", 2);
- if (values[0].equalsIgnoreCase(TOKEN)) {
- token = values[1];
+ String username = values[0];
+ if (TOKEN.equals(username) || PASSCODE.equals(username)) {
Review comment:
since this is user input, I originally had this as case insensitive. Do you feel strongly that it should be case sensitive? An in that case do we still want it to be first char upper?
##########
File path: gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
##########
@@ -38,14 +39,19 @@
public class JWTFederationFilter extends AbstractJWTFilter {
+ public enum TokenType {
+ JWT, Passcode;
+ }
+
public static final String KNOX_TOKEN_AUDIENCES = "knox.token.audiences";
public static final String TOKEN_VERIFICATION_PEM = "knox.token.verification.pem";
public static final String KNOX_TOKEN_QUERY_PARAM_NAME = "knox.token.query.param.name";
public static final String TOKEN_PRINCIPAL_CLAIM = "knox.token.principal.claim";
public static final String JWKS_URL = "knox.token.jwks.url";
- private static final String BEARER = "Bearer ";
- private static final String BASIC = "Basic";
- private static final String TOKEN = "Token";
+ public static final String BEARER = "Bearer ";
+ public static final String BASIC = "Basic";
+ public static final String TOKEN = "Token";
+ public static final String PASSCODE = "TokenPasscode";
Review comment:
s/TokenPasscode/Passcode/
--
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:
users@infra.apache.org