smolnar82 commented on code in PR #1405:
URL: https://github.com/apache/knox/pull/1405#discussion_r4034183129
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandler.java:
##########
@@ -316,7 +316,29 @@ ActionOutcome.SUCCESS, auditMessage(policyDecision,
actorIdentity, subjectToken,
// present so that KNOXTOKEN falls back to its resource query parameter
otherwise; when set,
// the body value takes precedence over the query parameter.
if (!requestedAudiences.isEmpty()) {
-
request.setAttribute(CommonTokenConstants.REQUESTED_AUDIENCES_REQUEST_ATTR,
requestedAudiences);
+ final boolean sameSubjectExchange = !hasActorToken &&
!requestedSubjectDiffersFromSubject;
Review Comment:
> nit: I hadn't computed the boolean delegationExchange above for the big
branch that handles delegation, because it was only used once, so I just
inlined the AND expression. It branches off for the delegation authorization
logic. But if we're using it again here, we can compute it up there and share
it.
Makes sense. I'll hoist `delegationExchange = hasActorToken ||
requestedSubjectDiffersFromSubject` up where it's first used and reuse it here.
> nit: Arguably this logic, or at least the enforcement logic could be moved
into the above conditional, where we already have an if delegation block. We
can add an else block to enforce the audiences are valid there, so all the
enforcement is done before the subject setting. That is, if we later add more
options for how to validate same-subject audience requests, we'd have a more
complex validation step there. Grouping it with setting the attribute is a bit
confusing.
Agreed. I'll move the same-subject audience enforcement into an else on the
delegation block so all authorization happens before the subject is set,
leaving the downstream conveyance as a plain attribute-set.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
