KNOX-3478 - Add support to forward auth tokens in KNOX-AUTH-SERVICE - #1425
Conversation
Test Results 4 files 4 suites 43s ⏱️ Results for commit 6b15c3c. ♻️ This comment has been updated with latest results. |
smolnar82
left a comment
There was a problem hiding this comment.
HadoopAuthPostFilter never captures the caller JWT; this feature silently dead for topologies using the HadoopAuth filter.
gateway-provider-security-hadoopauth/.../HadoopAuthPostFilter.java:91
doFilter() still calls the public single-arg createSubjectFromToken(String), which resolves to createSubjectFromToken(JWT, null). It was never updated to capture the caller's JWT as an AuthTokenCredential the way JWTFederationFilter.doFilter() now does.
Impact: On any topology using the HadoopAuth federation provider with support.jwt=true, no AuthTokenCredential is ever added to the Subject, so SubjectUtils.getAuthToken() returns null and the X-Knox-Auth-Token header is never emitted, even though the same caller gets it on a JWTProvider/SSOCookieProvider topology. The PR docs misattribute this to an inherent "innermost Subject.doAs wins" quirk, when it's actually a one-line oversight at this call site.
I approve this PR now and file a follow-up JIRA to fill this gap.
lmccay
left a comment
There was a problem hiding this comment.
I expected a bit more generic of an approach here. This is a rather large change for adding a single header to the AUTH-SERVICE API. When I saw the description I expected something more generic that would inform the API what things to pull from the security context. Since Principal name isn't likely enough since that is also the actual value of the Principal and they are more class type based. I would have expected an extension that you can interrogate with a getType() for a value that would match the param in the topology to add to a header. It would require a new convention of Principals and Credentials where this extension is used and maybe not be fully dynamic but maybe there is something more we can do to make it so.
You are right, I did not think about Looks like you created a JIRA for this https://issues.apache.org/jira/browse/KNOX-3483 |
I see, i misunderstood the intention. The intention for the feature was to have, say credential "types" and principal "types" that we can use getType() on to pull data from security context? example of such type include Credential (type, value), Principal(type, value) e.g. Credential("token", "ey......"), Principal("UserPrincipal", "Tom"), Principal("X500Principal", "Acme") etc. |
| * its own, so 6KB is a token size that still leaves room for the status line, the standard | ||
| * headers and this service's actor id header. | ||
| */ | ||
| private static final String DEFAULT_AUTH_TOKEN_SIZE_LIMIT = "6144"; |
There was a problem hiding this comment.
Do we expect tokens to be this large?
There was a problem hiding this comment.
We have had cases where JWT tokens were huge, this can be because they can include assertions, groups etc!
This is configurable.
KNOX-3478 - Add support to forward auth tokens in KNOX-AUTH-SERVICE
What changes were proposed in this pull request?
Downstream services increasingly need the caller's JWT bearer token itself, not just the
resolved username — e.g. to re-validate it, to extract scopes, or to call a third service on
the user's behalf. There is currently no way to get it: the token is consumed and discarded
by
JWTFederationFilter, and nothing carries it forward.This PR fixes this by setting the new
preauth.auth.header.auth.token.nameservice parameter onKNOX-AUTH-SERVICEand makingauth/api/v1/preandauth/api/v1/extauthzreturn the caller's token in the named response header, which the calling proxy then copies onto the downstream request.The header name that enables this feature is
preauth.auth.header.auth.token.name. By default this feature is OFF i.e. auth tokens are no propagated downstream by default.How was this patch tested?
This patch was tested locally.
Topology snippet
curl request output
Integration Tests
Added
.github/workflows/tests/test_knoxauth_token_forwarding.py(11 tests) plus three topology fixtures, all picked up by the existing defaulttestsservice with notests.ymlor compose change — these topologies coexist with the base gateway config, so they don't need a dedicated step.knoxauthtoken.xmlknoxauthtokenlimit.xmlsize.limit=16, below any real JWT ⇒ omitted, still 200knoxauthtokencollide.xmlknoxtoken.xml,knoxldap.xmlAll three new topologies configure
group.principal.mapping = guest=forwarded-token-group. That is deliberate, not decoration: it makesgroupsMappedtrue so every request goes through the identity-assertion re-wrap described above. That branch is the one a unit test can most easily pass while the real code path drops the token, sotest_forwarded_token_survives_the_identity_assertion_rewrapasserts the mapped group and the token in the same response — if the mapping ever stops running, the test fails loudly instead of passing vacuously.Coverage: byte-identical JWT with no
Bearerprefix;subnaming the original caller alongsideX-Knox-Actor-ID;Cache-Control: no-storepresent when a token is emitted and absent when it isn't; no header when the parameter is unset; none for a Basic-auth caller; oversized token omitted at 200; collision preserving the identity; 401 with no header for unauthenticated and for a tampered token.