feat(oidc,client): Adds support for OpenID Key Binding to client - #943
feat(oidc,client): Adds support for OpenID Key Binding to client#943EthanHeilman wants to merge 8 commits into
Conversation
|
@wim07101993 How much interest is there in merging this? Do you want me to move this to v4 rather v3? |
There was a problem hiding this comment.
Pull request overview
Adds Relying Party (client) support for OpenID Connect Key Binding 1.0 by allowing an RP to request key-bound ID Tokens (via bound_key + dpop_jkt), attach DPoP proofs to token endpoint calls, and verify returned ID tokens are actually bound to the configured key.
Changes:
- Introduces OIDC-level DPoP/key-binding helpers (thumbprints, proof claims, key strength checks, code hash).
- Adds RP option
WithKeyBinding(...)plus HTTP client transport logic to sign/pin DPoP proofs for token endpoint requests and verify key-bound ID Tokens (typ+cnf.jwk). - Extends authorization/device/refresh flows to include
dpop_jkt/bound_keybehavior and adds unit tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/oidc/dpop.go | New helpers/constants/types for key binding (DPoP proof claims, thumbprints, key validation, code hash). |
| pkg/oidc/dpop_test.go | Unit tests for new OIDC-level key-binding helpers. |
| pkg/oidc/device_authorization.go | Adds dpop_jkt field to device authorization request model. |
| pkg/oidc/authorization.go | Adds bound_key scope constant. |
| pkg/client/rp/relying_party.go | Hooks key binding into AuthURL, code exchange, refresh token flow, and ID token verification. |
| pkg/client/rp/key_binding.go | Implements WithKeyBinding, DPoP proof signing transport, and key-bound ID token verification. |
| pkg/client/rp/key_binding_test.go | Tests key-binding option behavior, proof signing, token verification, and auth URL parameterization. |
| pkg/client/rp/device.go | Adds device-flow support (device authorization + polling with DPoP proof + bound ID token verification). |
| pkg/client/client.go | Adds a bound-key variant for device authorization endpoint calls (adds dpop_jkt). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
pkg/oidc/dpop.go:59
- DPoPProofClaims.UnmarshalJSON checks for a null
iatwithbytes.Equal(raw, []byte("null")), butjson.RawMessagemay include leading/trailing whitespace (e.g."iat": null). In that case the explicit null check won’t trigger and the error type/field details become inconsistent. Trimming whitespace before comparison makes the null handling reliable.
if raw, ok := fields["iat"]; ok {
var issuedAt int64
if bytes.Equal(raw, []byte("null")) {
return &json.UnmarshalTypeError{Value: "null", Type: reflect.TypeOf(issuedAt), Field: "iat"}
}
|
@EthanHeilman it looks like we're not handling DPoP server provided nonces: https://www.rfc-editor.org/rfc/rfc9449.html#section-8 - not sure if this deliberate for now, but I think auth/resource servers sending them will fail if they're not received from the client. |
|
@kipz Thanks for the review! We don't use a DPoP
The DPoP code in this PR is only for issuance. |
|
thanks @EthanHeilman , yeah, I see the key binding spec uses one thing though: "optional" in rfc 9449 §8 reads to me as optional for the AS, not the client. If an AS does require a nonce it answers with Guess it's more of a potential interop issue for later maybe :) |
|
RFC 9449 and OpenID Key Binding intentionally differ in this regard and they are different protocols. Key binding token issuance only requires RFC 9449 Section 5 validation. When doing key binding, When using an key bound ID Token, nonce headers should respected by the client. When issuing a key bound ID Token, a nonce should not be required by the AS. It might be worth clarifying this point in the spec. To rant for a second because it is a topic close to my heart =) The use of a DPoP From an implementers point of view, key binding is nice because it does not add any additional requests, it just augments the existing requests. If we added nonce's into the flow, we now have a "opps wrong nonce" response from the AS and a client "retrying with nonce" request. Even with all that, DPoP nonces in this context don't even solve the problems we want to solve, replay protection and session binding. This is because the standard does not require that they are single use and unique to a session. We already have something in that is single use and unique to the session the authorization code. In practice, as far as I can tell, no one uses nonces in access token issuance due to the above issues. Key binding provides a better pattern here. |
Which Problems Are Solved
This PR adds client support for OpenID Key Binding 1.0 a draft standard for Proof-of-Possession ID Tokens. Basically ID Tokens that have a public key enabling the user to sign under their identity.
OpenPubkey supports key binding using zitadel/oidc as the library. However because zitadel/oidc does not natively support key binding, OpenPubkey has to use a fragile rounder-tripper work around for the RP.
This PR allows key binding support for OP and RP:
This PR is only for client/RP. It does not add key binding to the OP code. I would like to add that at a future point, but client/RP support the main blocker to using plain zitadel/oidc with key binding.
I've run manual tests for code flow, refresh and device flow using the key binding with the Hello OP and this PR in OpenPubkey that runs against this branch: openpubkey/openpubkey#395
How the Problems Are Solved
Adds a
WithKeyBindingoption to RP so that the RP can request key bound ID Tokens from the OP and aKeyBindingRelyingPartyto perform the additional steps used by key binding.Additional Changes
No additional changes
Additional Context
Do you want me to port this to v4 as well as v3?
Relevant PRs