Skip to content

fix(rp): send the issuer as the only audience of the code exchange client assertion - #986

Merged
wim07101993 merged 4 commits into
zitadel:mainfrom
NK-IT-CLOUD:fix/rp-jwt-profile-audience
Sep 25, 2026
Merged

wim07101993 merged 4 commits into
zitadel:mainfrom
NK-IT-CLOUD:fix/rp-jwt-profile-audience

Conversation

@NK-IT-CLOUD

Copy link
Copy Markdown
Contributor

Which Problems Are Solved

  • CodeExchangeHandler signs the private_key_jwt client assertion with two audiences: the issuer and the token endpoint URL. Authorization servers that allow a single audience reject the code exchange:

How the Problems Are Solved

  • The assertion audience is []string{rp.Issuer()}, the value already used in device.go, tokenexchange.go and resource_server.go.
  • This matches draft-ietf-oauth-rfc7523bis: the issuer identifier is the sole audience value, the token endpoint URL must not be used.

Additional Changes

  • Test: runs CodeExchangeHandler against a stub token endpoint and asserts the audience of the received client assertion. Fails without the fix (two audiences), passes with it.

Additional Context

  • Closes [Bug]: Client Certificate authentication against Entra ID failes with AADSTS5002741 #887
  • Verified against Keycloak 26.6.3 with ES256 assertions differing only in aud: [issuer, token_endpoint] rejected, [issuer] accepted.
  • Compatibility: an authorization server that accepts only its token endpoint URL as audience would reject the assertion. The device authorization, token exchange and resource server flows of this library already send only the issuer.

…ient assertion

CodeExchangeHandler built the private_key_jwt client assertion with two
audiences, the issuer and the token endpoint URL. Keycloak 26 and
Microsoft Entra ID reject assertions with more than one audience. The
device authorization, token exchange and resource server flows already
send only the issuer.

Closes zitadel#887
@NK-IT-CLOUD

Copy link
Copy Markdown
Contributor Author

For Keycloak this change alone is not enough: Keycloak also requires a jti claim in the client assertion (OpenID Connect Core 1.0, section 9) and rejects it with "Token jti claim is required". Opened #987 for that. With both, the code exchange works against Keycloak 26.6.3 and 26.7.4.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused fix matches the cited specification and is covered by an appropriate regression test.

Review effort: Balanced
Findings: None

What changed in this PR

Aligns authorization-code client assertions with RFC 7523bis by using only the issuer as the audience.

Changes:

  • Removes the token endpoint from the assertion audience.
  • Adds an integration-style regression test.
File Description
pkg/​client/​rp/​relying_party.go Uses the issuer as the sole assertion audience.
pkg/​client/​rp/​relying_party_test.go Verifies the generated audience claim.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@wim07101993 wim07101993 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution. Just a small comment about the test.

Comment thread pkg/client/rp/relying_party_test.go Outdated
wim07101993 added a commit that referenced this pull request Sep 25, 2026
# Which Problems Are Solved

- `SignedJWTProfileAssertion` builds the private_key_jwt client
assertion without a `jti` claim. OpenID Connect Core 1.0, section 9,
requires `jti` for private_key_jwt. Keycloak rejects the code exchange
with `invalid_client` "Token jti claim is required" (26.6.3 and 26.7.4;
still required on Keycloak main).

# How the Problems Are Solved

- `JWTTokenRequest` gets `JWTID` (`jti`, `omitempty`), so parsing an
assertion without `jti` is unchanged.
- `SignedJWTProfileAssertion` sets a random UUID per assertion
(`uuid.NewRandom`; an error is returned to the caller).

# Additional Changes

- Test: two assertions carry a non-empty, distinct `jti`.

# Additional Context

- Related: #986 (issuer as the only audience), #887. Keycloak needs both
to accept the code exchange assertion.
- Verified against Keycloak 26.6.3 and 26.7.4 with an ES256
private_key_jwt client and a real login, relying party built on this
library:

| zitadel/oidc | code exchange |
|---|---|
| v3.51.3 | `invalid_client`: Multiple audiences not allowed |
| + #986 | `invalid_client`: Token jti claim is required |
| + #986 + this PR | OK |

Co-authored-by: Wim Van Laer <wim@wvl.app>
@wim07101993
wim07101993 merged commit 9db978b into zitadel:main Sep 25, 2026
6 checks passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 3.51.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

wim07101993 added a commit that referenced this pull request Sep 28, 2026
…tion when a signer is set (#989)

# Which Problems Are Solved

- `CodeExchangeHandler` signs a JWT profile client assertion when the
relying party has a signer (`WithJWTProfile`). `RefreshTokens` does not:
without an assertion from the caller it sends `client_id` and
`client_secret`, and the secret is empty for such a client. Keycloak
answers `invalid_client` "Parameter client_assertion_type is missing".
Every caller has to build and pass the assertion itself.
- `RevokeToken` has no way to send an assertion, so a relying party that
uses private_key_jwt cannot revoke tokens. RFC 7009 section 2.1 asks the
client to authenticate at the revocation endpoint as it does at the
token endpoint.

# How the Problems Are Solved

- `RefreshTokens`: when the caller passes no assertion and the relying
party has a signer, it signs one with the issuer as the only audience,
as `CodeExchangeHandler` does since #986, and sends it with
`oidc.ClientAssertionTypeJWTAssertion`.
- `RevokeToken`: with a signer, it sends `client_id` plus a signed
assertion instead of the secret.
- Both only apply when the client authenticates in the request body.
With `AuthStyleInHeader` nothing changes, since `RefreshTokens` rejects
an assertion in that mode.

# Additional Changes

- `client.RevokeRequest` gets `client_assertion` and
`client_assertion_type`, and `client_secret` becomes `omitempty`, so a
request with an assertion carries no empty `client_secret` next to it.
- Tests against a stub token and revocation endpoint: refresh and revoke
with a signer send the assertion (issuer as the only audience, a `jti`,
a new one per call, no `client_secret`, no basic auth). The tests for
the unchanged cases check that an assertion from the caller wins, that a
client without a signer still sends its secret and that
`AuthStyleInHeader` still uses basic auth. The two signer tests fail on
main, the others pass there too.

# Additional Context

- Follow-up for PR #986
- Reported in jkroepke/openvpn-auth-oauth2#1180, where the refresh fails
for this reason.
- Verified against Keycloak 26 with a private_key_jwt client (ES256):
code exchange, refresh without a caller assertion and revocation
succeed, Keycloak logs all three with `client_auth_method: client-jwt`,
and a refresh with the revoked token is refused. On main the same
refresh fails with `invalid_client` "Parameter client_assertion_type is
missing".

---------

Co-authored-by: Wim Van Laer <wim@wvl.app>
Mohil-Ahuja added a commit to Mohil-Ahuja/oidc that referenced this pull request Oct 5, 2026
…sToken does

Addresses review on the previous commit:

- A RequestAuthorization is not necessarily client authentication: it is
  also how callers add headers such as a tenant id, and a typed nil one is
  skipped by FormRequest. Treating either as authentication dropped the
  form credentials and sent the request unauthenticated. The method is now
  picked from AuthStyle and the signer alone, and with AuthStyleInHeader
  basic auth is combined with the caller's RequestAuthorization instead of
  replacing it. A FormAuthorization cannot be combined with a header, so
  with one the credentials stay in the form.
- The assertion is signed only when it is sent, so basic auth no longer
  pays for, or fails on, a signer it does not use, and
  withoutFormCredentials goes away.
- Device assertions use jwtProfileAudience, like the other flows since
  zitadel#986/zitadel#989, so an OAuth-only relying party without an issuer addresses
  them to the token endpoint instead of to "". This covers
  DeviceAccessToken as well.
- ClientCredentialsRequest.ClientSecret is omitempty, so a request
  authenticated another way carries no empty client_secret parameter.
wim07101993 pushed a commit that referenced this pull request Oct 6, 2026
…zation (#996)

# Which Problems Are Solved

#858 made `DeviceAccessToken` send exactly one client authentication
method, as [RFC 6749
§2.3](https://datatracker.ietf.org/doc/html/rfc6749#section-2.3)
requires, but `DeviceAuthorization` was not changed and can still send
two:

- **With a signer**, `newDeviceClientCredentialsRequest` sets
`client_assertion` *and* `client_secret`, so the form carries both.
- **`AuthStyleInHeader` is ignored**, so the secret always goes in the
form. A caller who needs `client_secret_basic` passes an `authFn` doing
basic auth, and the request then carries the `Authorization` header
*and* the form secret.

An OP that enforces §2.3 rejects either request. Authelia does, which is
the kind of failure #830 reports for the device flow.

The device flow also signed its assertions with `aud: [rp.Issuer()]`,
which is `[""]` for an OAuth-only RP created with
`NewRelyingPartyOAuth`; the other flows fall back to the token endpoint
since #986/#989.

# How the Problems Are Solved

`DeviceAuthorization` now picks one method first, with a `switch` in the
same order as `DeviceAccessToken`:

1. `AuthStyleInHeader`: basic auth, combined with the caller's
`RequestAuthorization` if there is one (so a tenant or tracing header is
kept);
2. a signer: a signed assertion, signed only on this path;
3. otherwise: the secret in the form.

The caller's `authFn` is never treated as authentication itself. A
`FormAuthorization` (for example the PKCE workaround in #784) can't be
combined with a header, so with one the credentials stay in the form, as
before. `client_id` stays in the form in every case.

Both device-flow assertions now use `jwtProfileAudience`, and
`ClientCredentialsRequest.ClientSecret` is `omitempty`, so a request
authenticated another way carries no empty `client_secret` parameter.

# Additional Changes

- `pkg/client/rp/device_test.go`: an `httptest` OP records the form and
headers and asserts one method per request across each auth style, with
and without a signer, and with header-only, typed-nil and form-only
`authFn`s; a failing signer must not break basic auth; and the assertion
audience is checked for both `DeviceAuthorization` and
`DeviceAccessToken`, with and without an issuer.
- Against `main`, the double-method, header-style, failing-signer and
issuer-less audience cases fail; the default, `AuthStyleInParams` and
form-only cases pass on both.
- `go test ./...` passes except in `pkg/op` (`TestRoutes`,
`TestServerRoutes`, `TestAuthResponseCode`), which fail identically on
`main` for me locally. `go vet` is clean.

# Additional Context

- Follow-up for PR #858
- Related to #830: the token polling part of that report was fixed by
#858 (released in v3.47.9); this covers the device authorization
request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Client Certificate authentication against Entra ID failes with AADSTS5002741

3 participants