[
https://issues.apache.org/jira/browse/KNOX-3403?focusedWorklogId=1033113&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1033113
]
ASF GitHub Bot logged work on KNOX-3403:
----------------------------------------
Author: ASF GitHub Bot
Created on: 30/Jul/26 21:23
Start Date: 30/Jul/26 21:23
Worklog Time Spent: 10m
Work Description: smolnar82 opened a new pull request, #1336:
URL: https://github.com/apache/knox/pull/1336
[KNOX-3403](https://issues.apache.org/jira/browse/KNOX-3403) - Token
exchange issues the wrong `sub` on non-server-managed topologies
## What changes were proposed in this pull request?
When a token was issued under impersonation via an RFC 8693 token exchange
(`subject_token` + `actor_token`) on a topology that is **not** server-managed,
the issued JWT's `sub` was set to the **actor** (the primary principal /
authenticated caller) instead of the **subject** (the impersonated end user) -
so the token represented the acting party rather than the user, defeating the
on-behalf-of semantics. (The `act` claim was already correct; only `sub` was
wrong.)
**Root cause:** `TokenResource.buildUserContext(...)` only replaced the
default username (the primary principal, from `request.getUserPrincipal()`)
with the impersonated principal inside an `if (tokenStateService != null)`
block - i.e. only when the topology is server-managed. That gate predates token
exchange and was intended only to guard *metadata* storage
(`userName`/`createdBy` persisted to the token state service), not to control
the issued token's `sub`.
This PR moves the impersonation -> `userName` resolution out of that gate,
so the issued token's `sub` is the effective (impersonated) identity whenever
`SubjectUtils.isImpersonating(subject)` is true, independent of server-managed
state. `createdBy` continues to be persisted only for managed tokens
(`persistTokenDetails` already guards on `tokenStateService != null`, so
computing it otherwise is harmless). The stale comment was updated.
**Scope:** only the non-server-managed impersonation path changes.
Traditional `doAs` was already correct (the identity-assertion request wrapper
exposes the impersonated user via `getUserPrincipal()`), server-managed
topologies are unchanged, and non-impersonating requests are unchanged.
## How was this patch tested?
* Reproduced end-to-end on a live gateway: an RFC 8693 exchange on a
non-server-managed topology returned `sub = <actor>` (wrong); the same exchange
on a server-managed topology returned the correct `sub`, isolating the cause to
this gate.
* Added unit tests in `TokenServiceResourceTest`:
* `testImpersonatedTokenSubjectOnNonServerManagedTopology`:
non-server-managed + impersonation (actor `admin`, subject `bob`) asserts `sub
== bob` and `act == { sub: admin }`.
* `testNonImpersonatedTokenSubjectOnNonServerManagedTopology`:
non-server-managed, no impersonation asserts `sub == <primary>` and no `act`
claim (no regression).
* Confirmed both new tests **fail without the fix** (`expected:<bob> but
was:<alice>`) and pass with it; full `TokenServiceResourceTest` passes (73
tests) with 0 Checkstyle violations.
## Integration Tests
N/A
## UI changes
N/A
Issue Time Tracking
-------------------
Worklog Id: (was: 1033113)
Remaining Estimate: 0h
Time Spent: 10m
> RFC 8693 token exchange: issued token 'sub' is the actor instead of the
> subject on non-server-managed topologies
> ----------------------------------------------------------------------------------------------------------------
>
> Key: KNOX-3403
> URL: https://issues.apache.org/jira/browse/KNOX-3403
> Project: Apache Knox
> Issue Type: Bug
> Components: Server
> Affects Versions: 3.0.0
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Critical
> Fix For: 3.0.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h2. Summary
> When a token is issued via an RFC 8693 token exchange
> ({{{}grant_type=urn:ietf:params:oauth:grant-type:token-exchange{}}}, with a
> {{subject_token}} and an {{{}actor_token{}}}) on a topology whose
> {{KNOXTOKEN}} service is *not* server-managed, the issued JWT's {{sub}} is
> set to the *actor* (the authenticated caller / primary principal) instead of
> the *subject* (the impersonated end user). The impersonated identity is
> effectively dropped from {{{}sub{}}}, defeating the on-behalf-of semantics.
> h2. Impact
> * RFC 8693 OBO on a non-server-managed topology: the exchanged token
> represents the acting party (e.g. the service account) rather than the end
> user. The {{act}} claim is correct; only {{sub}} is wrong.
> * Traditional {{doAs}} is NOT affected - there the identity-assertion
> request wrapper already exposes the impersonated user via
> {{{}getUserPrincipal(){}}}, so the issued {{sub}} is correct.
> * Server-managed topologies are NOT affected - the impersonation-aware
> username resolution already runs.
> h2. Reproduction
> # Deploy a NON-server-managed topology with a JWTProvider (trusting both the
> subject and actor issuers) + KNOXTOKEN
> ({{{}knox.token.enable.delegated.auth=true{}}}) + identity-assertion Default.
> # Obtain a {{subject_token}} (an end user, e.g. admin) and an
> {{actor_token}} for a DIFFERENT principal (e.g. a k8s service account).
> # POST the exchange:
> {code:java}
> grant_type=urn:ietf:params:oauth:grant-type:token-exchange
> subject_token=<user JWT>
> subject_token_type=urn:ietf:params:oauth:token-type:jwt
> actor_token=<service-account JWT>
> actor_token_type=urn:ietf:params:oauth:token-type:jwt
> {code}
> # Decode the issued access_token:
> ** Actual: {{sub}} = the actor (e.g. system:serviceaccount:...), {{act}} =
> \{ sub: actor }
> ** Expected: {{sub}} = the subject (the end user), {{act}} = \{ sub: actor }
> # Running the same exchange on a server-managed topology yields the correct
> {{sub}} - isolating the cause to the non-server-managed code path.
> h2. Root cause
> {{TokenResource.buildUserContext(HttpServletRequest)}} initializes the token
> username from {{request.getUserPrincipal().getName()}} and only replaces it
> with the impersonated principal inside an {{if (tokenStateService != null)}}
> (i.e. server-managed) block:
> {code:java}
> String userName = request.getUserPrincipal().getName();
> String createdBy = null;
> if (tokenStateService != null) { // <-- only when
> server-managed
> final Subject subject = SubjectUtils.getCurrentSubject();
> if (subject != null && SubjectUtils.isImpersonating(subject)) {
> String primaryPrincipalName =
> SubjectUtils.getPrimaryPrincipalName(subject);
> String impersonatedPrincipalName =
> SubjectUtils.getImpersonatedPrincipalName(subject);
> if (!primaryPrincipalName.equals(impersonatedPrincipalName)) {
> createdBy = primaryPrincipalName;
> userName = impersonatedPrincipalName;
> }
> }
> }
> {code}
> In the RFC 8693 token-exchange flow the principal reaching TokenResource via
> {{getUserPrincipal()}} is the actor (primary principal), so the impersonated
> subject is only applied by the override above. Because that override is gated
> on {{{}tokenStateService != null{}}}, it is skipped on non-server-managed
> topologies and {{sub}} falls back to the actor.
> The gate predates token exchange and was intended only to guard metadata
> storage ({{{}userName{}}}/{{{}createdBy{}}} persisted to the token state
> service), not to control the issued token's {{{}sub{}}}.
> h2. Fix
> Move the impersonation -> {{userName}} resolution out of the
> {{tokenStateService != null}} block so the issued token's {{sub}} is the
> effective (impersonated) identity whenever
> {{SubjectUtils.isImpersonating(subject)}} is true, regardless of
> server-managed state. {{createdBy}} continues to be persisted only when
> server-managed ({{{}persistTokenDetails{}}} already guards on
> {{{}tokenStateService != null{}}}, so computing it otherwise is harmless).
> Remove the now-stale comment.
> h2. Tests
> Add coverage in {{TokenServiceResourceTest}} on a NON-server-managed topology:
> * An RFC 8693 / impersonated subject (PrimaryPrincipal = actor,
> ImpersonatedPrincipal = subject, getUserPrincipal() = actor) yields \{sub ==
> subject} and \{act == { sub: actor }}.
> * A non-impersonating request still yields \{sub == primary}(no regression).
> * Confirm server-managed behavior is unchanged.
> h2. Related
> Discovered while validating KNOX-3399 (RFC 8693 token exchange) on a
> non-server managed exchange topology. Independent of that routing fix.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
