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/24 21:01:02 UTC
[GitHub] [knox] pzampino opened a new pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
pzampino opened a new pull request #424:
URL: https://github.com/apache/knox/pull/424
## What changes were proposed in this pull request?
Added JWTProvider support for specifying the unique identifier associated with Knox JWTs as a HTTP Basic password (username=TokenPasscode, password=UUID) when server-managed token state is enabled.
Example:
curl -ivku TokenPasscode:$PASSCODE "https://localhost:8443/gateway/proxy-token/webhdfs/v1/tmp?op=LISTSTATUS"
## How was this patch tested?
I've added org.apache.knox.gateway.provider.federation.TokenIDAsHTTPBasicCredsFederationFilterTest and executed all the exising Knox tests (mvn -Ppackage,release clean package).
I've manually tested by:
- Enabling server-managed token state in the homepage deployment of KNOXTOKEN
- Defining a provider configuration with the JWTProvider (with server-managed token state enabled)
- Defining a descriptor that references that provider config
- Generated a token using the homepage facility
- curl -ivku TokenPasscode:$PASSCODE "https://localhost:8443/gateway/proxy-token/webhdfs/v1/tmp?op=LISTSTATUS"
I've also done some manual testing with server-managed token state DISABLED to verify the behavior.
I could not find any existing tests for org.apache.knox.gateway.hadoopauth.filter.HadoopAuthPostFilter, so I haven't yet added any tests to verify the associated changes.
I may add more negative test cases to org.apache.knox.gateway.provider.federation.TokenIDAsHTTPBasicCredsFederationFilterTest
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601772280
##########
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:
I'm pretty sure TokenIDAsHTTPBasicCredsFederationFilterTest#testInvalidJWTForPasscode() verifies this.
--
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
[GitHub] [knox] lmccay commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601766672
##########
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:
I see - future metadata...
I don't know that it makes sense to validate the issuer unless it has come in via JWT.
If it is a known token then we are obviously the issuer.
Audience will make sense though.
--
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
[GitHub] [knox] moresandeep commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601758744
##########
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);
Review comment:
nit: can loose the comments here if you revisit this.
##########
File path: gateway-provider-security-hadoopauth/src/main/java/org/apache/knox/gateway/hadoopauth/filter/HadoopAuthPostFilter.java
##########
@@ -78,7 +79,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
Subject subject = null;
if (shouldUseJwtFilter(jwtFilter, (HttpServletRequest) request)) {
try {
- subject = jwtFilter.createSubjectFromToken(jwtFilter.getWireToken(request));
+ Pair<JWTFederationFilter.TokenType, String> wireToken = jwtFilter.getWireToken(request);
+ JWTFederationFilter.TokenType tokenType = wireToken.getLeft();
+ String token = wireToken.getRight();
+ if (JWTFederationFilter.TokenType.JWT.equals(tokenType)) {
+ subject = jwtFilter.createSubjectFromToken(token);
+ } else if (JWTFederationFilter.TokenType.Passcode.equals(tokenType)) {
Review comment:
nit: this could be just `JWTFederationFilter.TokenType.PASSCODE.equals(tokenType)` for readability.
--
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
[GitHub] [knox] lmccay commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [knox] smolnar82 commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601610685
##########
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 {
+ Date expires = getExpiration(tokenId);
+ return expires == null || (new Date().before(expires));
+ }
+
+ private Date getExpiration(final String tokenId) throws UnknownTokenException {
Review comment:
We may indicate - in the method name - that expiration is checked in token state service.
##########
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:
I think it is redundant to have both; 'TokenPasscode' is handled at the same location as 'Token' and one should be enough. Since 'Token' is shorter I'd vote for that one.
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601765801
##########
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:
I don't believe so; The UnknownTokenException should work its way back to the validateToken method where it is handled.
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601763257
##########
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);
Review comment:
I've created KNOX-2563 to provide the necessary metadata.
--
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
[GitHub] [knox] smolnar82 commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601723500
##########
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:
Oh...I see now. I'm fine with the proposed names then!
--
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
[GitHub] [knox] lmccay commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601767452
##########
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:
Make sure we have a test for bogus tokens to be sure.
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601766712
##########
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:
That's a good point, since usernames are typically case-insensitive. I'll change the check and add some tests.
--
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
[GitHub] [knox] pzampino merged pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino merged pull request #424:
URL: https://github.com/apache/knox/pull/424
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601765194
##########
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:
+1
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601768075
##########
File path: gateway-provider-security-hadoopauth/src/main/java/org/apache/knox/gateway/hadoopauth/filter/HadoopAuthPostFilter.java
##########
@@ -78,7 +79,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
Subject subject = null;
if (shouldUseJwtFilter(jwtFilter, (HttpServletRequest) request)) {
try {
- subject = jwtFilter.createSubjectFromToken(jwtFilter.getWireToken(request));
+ Pair<JWTFederationFilter.TokenType, String> wireToken = jwtFilter.getWireToken(request);
+ JWTFederationFilter.TokenType tokenType = wireToken.getLeft();
+ String token = wireToken.getRight();
+ if (JWTFederationFilter.TokenType.JWT.equals(tokenType)) {
+ subject = jwtFilter.createSubjectFromToken(token);
+ } else if (JWTFederationFilter.TokenType.Passcode.equals(tokenType)) {
Review comment:
I see, you mean `JWTFederationFilter.PASSCODE.equals(tokenType)` ; This would require changing the parsing logic and resulting Pair contents.
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601766169
##########
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:
I don't believe so; The UnknownTokenException should work its way back to the validateToken method where it is handled.
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601769326
##########
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:
Yes, I've created KNOX-2563 to provide the necessary metadata, so the comments can go.
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601693342
##########
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 {
+ Date expires = getExpiration(tokenId);
+ return expires == null || (new Date().before(expires));
+ }
+
+ private Date getExpiration(final String tokenId) throws UnknownTokenException {
Review comment:
Changed to getServerManagedStateExpiration(String)
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601618049
##########
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:
We need to distinguish between the two: Token is for JWT password values while TokenPasscode is for UUID password values.
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601618773
##########
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:
We could of course debate the actual names, but I believe two distinct names are necessary.
--
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
[GitHub] [knox] lmccay commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601762684
##########
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? And in that case do we still want it to be first char upper?
--
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
[GitHub] [knox] pzampino commented on a change in pull request #424: KNOX-2556 - Enhance JWTProvider to accept knox.id as Passcode Token
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #424:
URL: https://github.com/apache/knox/pull/424#discussion_r601762106
##########
File path: gateway-provider-security-hadoopauth/src/main/java/org/apache/knox/gateway/hadoopauth/filter/HadoopAuthPostFilter.java
##########
@@ -78,7 +79,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
Subject subject = null;
if (shouldUseJwtFilter(jwtFilter, (HttpServletRequest) request)) {
try {
- subject = jwtFilter.createSubjectFromToken(jwtFilter.getWireToken(request));
+ Pair<JWTFederationFilter.TokenType, String> wireToken = jwtFilter.getWireToken(request);
+ JWTFederationFilter.TokenType tokenType = wireToken.getLeft();
+ String token = wireToken.getRight();
+ if (JWTFederationFilter.TokenType.JWT.equals(tokenType)) {
+ subject = jwtFilter.createSubjectFromToken(token);
+ } else if (JWTFederationFilter.TokenType.Passcode.equals(tokenType)) {
Review comment:
I don't understand what you're suggesting here. Can you elaborate for me?
--
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