[
https://issues.apache.org/jira/browse/KNOX-3040?focusedWorklogId=925179&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-925179
]
ASF GitHub Bot logged work on KNOX-3040:
----------------------------------------
Author: ASF GitHub Bot
Created on: 10/Jul/24 05:04
Start Date: 10/Jul/24 05:04
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #924:
URL: https://github.com/apache/knox/pull/924#discussion_r1671617962
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenAuthorityService.java:
##########
@@ -83,6 +84,8 @@ public class DefaultTokenAuthorityService implements
JWTokenAuthority, Service {
// https://tools.ietf.org/html/rfc7518
private static final Set<String> SUPPORTED_PKI_SIG_ALGS = new
HashSet<>(Arrays.asList("RS256", "RS384", "RS512", "PS256", "PS384", "PS512"));
private static final Set<String> SUPPORTED_HMAC_SIG_ALGS = new
HashSet<>(Arrays.asList("HS256", "HS384", "HS512"));
+ /* cache JWKS endpoint for 2 hrs in case of outage */
+ private static final long OUTAGE_TTL = 2*60*60*1000;
Review Comment:
Having the default set to 2 hours is good, but, IMO, this should be
configurable on the topology level.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java:
##########
@@ -110,7 +114,7 @@ public void init( FilterConfig filterConfig ) throws
ServletException {
// JWKSUrl
String oidcjwksurl = filterConfig.getInitParameter(JWKS_URL);
Review Comment:
IIWU, I'd introduce the new `knox.token.jwks.urls` config name now and mark
the previous one (`knox.token.jwks.url`) deprecated. Have the code handle both
for now and remove the support for the old one in the next release.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java:
##########
@@ -115,7 +116,7 @@ public abstract class AbstractJWTFilter implements Filter {
private String expectedIssuer;
private String expectedSigAlg;
protected String expectedPrincipalClaim;
- protected String expectedJWKSUrl;
+ protected Set<URI> expectedJWKSUrl = new HashSet();
Review Comment:
The class member should be renamed (even if the config name does not change
in this release) to `expectedJWKSUrls`.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java:
##########
@@ -123,4 +123,7 @@ public interface JWTMessages {
@Message( level = MessageLevel.INFO, text = "Token verification result using
knox signing cert, verified: {0}" )
void signingKeyVerificationResultMessage(boolean verified);
+
+ @Message(level = MessageLevel.ERROR, text = "Invalid URL ignored. Not a
valid JWKS url {0}")
+ void notValidJwksUrl(String jwksUrl);
Review Comment:
I'd call this method `invalidJwksUrl`.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java:
##########
@@ -149,6 +153,30 @@ public void init( FilterConfig filterConfig ) throws
ServletException {
configureExpectedParameters(filterConfig);
}
+ /**
+ * Helper function to extract URLs from given string
+ * in the form of https://url:port/contxt/.wellknown,
https://url2:port/contxt/.wellknown
+ * into expectedJWKSUrl URL set.
+ * @param oidcjwksurl
+ */
+ private Set<URI> getJwksUrlsFromConfig(final String oidcjwksurl) {
Review Comment:
The parameter should be renamed to `oidcJWKSUrls`.
Issue Time Tracking
-------------------
Worklog Id: (was: 925179)
Time Spent: 1h (was: 50m)
> Support multiple ways to verify JWT tokens
> ------------------------------------------
>
> Key: KNOX-3040
> URL: https://issues.apache.org/jira/browse/KNOX-3040
> Project: Apache Knox
> Issue Type: Bug
> Reporter: Sandeep More
> Assignee: Sandeep More
> Priority: Major
> Fix For: 2.1.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Currently we can only have one way to validate JWT token either
> # Using JWKS endpoint
> # Using PEM
> # Using the signing-key
> We should be able to support multiple verifications together.
> This is an example configuration
> {code:java}
> <provider>
> <role>federation</role>
> <name>JWTProvider</name>
> <enabled>true</enabled>
> <param>
> <name>knox.token.use.cookie</name>
> <value>true</value>
> </param>
>
> <param>
> <name>knox.token.jwks.url</name>
> <value>https://my.idp.com/oauth/.wellknown</value>
> </param>
> <param>
> <name>knox.token.verification.pem</name>
>
> <value>MIIDaDCCAlCgAwIBAgIJAKFjn6W+gdAXMA0GCSqGSIb3DQEBBQUAMF8xCzAJBgNVBAYTAlVTMQ0wC...</value>
> </param>
> <param>
> <name>jwt.expected.issuer</name>
> <value>https://my.idp.com/</value>
> </param>
> <param>
> <name>knox.token.use.cookie</name>
> <value>true</value>
> </param>
> </provider>
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
