[
https://issues.apache.org/jira/browse/KNOX-3073?focusedWorklogId=942571&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-942571
]
ASF GitHub Bot logged work on KNOX-3073:
----------------------------------------
Author: ASF GitHub Bot
Created on: 07/Nov/24 21:37
Start Date: 07/Nov/24 21:37
Worklog Time Spent: 10m
Work Description: pzampino commented on code in PR #949:
URL: https://github.com/apache/knox/pull/949#discussion_r1833401823
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java:
##########
@@ -512,17 +520,22 @@ protected boolean verifyTokenSignature(final JWT token) {
// If it has not yet been verified, then perform the verification now
if (!verified) {
try {
+ boolean hasPem = false;
+ boolean hasJWKS = false;
+
if (publicKey != null) {
+ hasPem = true;
verified = authority.verifyToken(token, publicKey);
log.pemVerificationResultMessage(verified);
}
if (!verified && expectedJWKSUrls != null &&
!expectedJWKSUrls.isEmpty()) {
+ hasJWKS = true;
verified = authority.verifyToken(token, expectedJWKSUrls,
expectedSigAlg, allowedJwsTypes);
log.jwksVerificationResultMessage(verified);
}
- if(!verified) {
+ if(!verified && ((!hasPem && !hasJWKS) || isJwtInstanceKeyFallback)) {
Review Comment:
The booleans are intended to indicate which methods have been attempted, and
they avoid additional (albeit minimal) method overhead; They're effectively
caching the result of the evaluations you're proposing to repeat. I do see your
point about hasJWKS being a little misleading, but I'm not convinced it matters
since we only really care if they're BOTH (PEM, JWKS) missing. I could be
persuaded to rename them to something like attemptedPEMVerification and
attemptedJwksVerification, which more accurately reflect their respective
intentions.
What do you think?
Issue Time Tracking
-------------------
Worklog Id: (was: 942571)
Time Spent: 40m (was: 0.5h)
> Token verification fallback to Knox keys behavior should configurable
> ---------------------------------------------------------------------
>
> Key: KNOX-3073
> URL: https://issues.apache.org/jira/browse/KNOX-3073
> Project: Apache Knox
> Issue Type: Improvement
> Components: Server
> Reporter: Philip Zampino
> Assignee: Philip Zampino
> Priority: Major
> Time Spent: 40m
> Remaining Estimate: 0h
>
> KNOX-3040
> ntroduced support for multiple token verification mechanisms (i.e., PEM,
> jwks) for the same topology (provider instance), falling back to Knox's own
> signing and TLS keys if any of those configured should fail.
> This behavior may not be expected by some, and we should provide the ability
> to control the fallback to the Knox keys.
> To support deployments expecting the previous behavior, there should be a
> provider param for indicating that the new fall-back behavior is desired
> (e.g., instance-keys-fallback=true), which defaults to false.
> Default Behavior:
> * Neither PEM nor jwks URL(s) is configured, attempt verification using (in
> order)
> ** Knox's signing key
> ** Knox's TLS key
> * Only PEM is configured: Knox will attempt verification using ONLY the
> configured PEM
> * Only jwks URL(s) are configured: Knox will attempt verification using ONLY
> the configured jwks URL(s)
> * PEM AND jwks URL(s) are configured: Knox will attempt verification using
> ONLY (in order)
> ** The configured PEM
> ** The configured jwks URL(s).
> instance-keys-fallback=true Behavior:
> * Same as default behavior except that in the cases where PEM and/or jwks
> URL(s) are configured and fail to verify, Knox will subsequently attempt
> verification using (in order):
> ** Knox's signing key
> ** Knox's TLS key
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
