Skip to content

Commit fa700b9

Browse files
josephdecockjosephdecock
committed
New factory to create client assertions on demand
This allows for DPoP retries to obtain a fresh client assertion, which is required when the AS uses nonces and rejects reused assertions.
1 parent be75f3c commit fa700b9

File tree

24 files changed

+1182
-244
lines changed

24 files changed

+1182
-244
lines changed

‎fix.md‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
## Summary
2+
3+
When a token endpoint returns a DPoP nonce (401 with `DPoP-Nonce` header), `ProofTokenMessageHandler` retries the request using the **same request object**, which carries the **same `ClientAssertion`** (same `jti`, `iat`, etc.). Servers that enforce client assertion uniqueness (e.g. HelseID) reject the retry as a replay.
4+
5+
Reported in: https://github.com/DuendeSoftware/foss/issues/323
6+
7+
## Root Cause
8+
9+
The retry path in `ProofTokenMessageHandler.SendAsync()` reuses the original `HttpRequestMessage`. The `ClientAssertion` is set once at request creation time (e.g. in `ResponseProcessor.RedeemCodeAsync()`) and never regenerated for the retry.
10+
11+
Additionally, `ProtocolRequest.Clone<T>()` copies `ClientAssertion` **by reference** (shallow copy), so even cloned requests share the same assertion object — though the real issue is the JWT string itself being identical.
12+
13+
### Call chain
14+
15+
```
16+
ResponseProcessor.RedeemCodeAsync()
17+
→ AuthorizationCodeTokenRequest created with ClientAssertion (fetched once)
18+
→ HttpClientTokenRequestExtensions clones via ProtocolRequest.Clone<T>()
19+
→ Clone() shallow-copies ClientAssertion
20+
→ ProofTokenMessageHandler.SendAsync() retries on DPoP nonce
21+
→ Same ClientAssertion reused on retry ← BUG
22+
```
23+
24+
## Affected Code
25+
26+
| File | Package | Role |
27+
|------|---------|------|
28+
| `ProofTokenMessageHandler.cs` (L42-44) | oidc-client-extensions | Retries request with same object on DPoP nonce |
29+
| `ProtocolRequest.cs` (L103) | identity-model | `Clone()` shallow-copies `ClientAssertion` |
30+
| `ResponseProcessor.cs` (L184) | oidc-client | Sets client assertion once at request creation |
31+
| `HttpClientTokenRequestExtensions.cs` | identity-model | Calls `Clone()` + `Prepare()` on all token requests |
32+
| `AuthorizationServerDPoPHandler.cs` | access-token-management | Same retry pattern, same bug |
33+
34+
## Scope
35+
36+
Affects **all token request types** using client assertions + DPoP:
37+
- Authorization code exchange
38+
- Refresh token requests
39+
- Client credentials requests
40+
- PAR requests
41+
- Device authorization
42+
43+
## Expected Behavior
44+
45+
Each token request attempt (including DPoP nonce retries) should use a **unique client assertion** with a fresh `jti` and `iat`, per RFC 7521 §4.2.
46+
47+
---
48+
49+
## Proposed Fix: Factory on `HttpRequestMessage.Options` (Strategy C)
50+
51+
### Approach
52+
53+
Add a `ClientAssertionFactory` (`Func<Task<ClientAssertion>>?`) property to `ProtocolRequest` that flows through `Clone()` → `Prepare()` → `HttpRequestMessage.Options` → handler chain. On DPoP nonce retry, `ProofTokenMessageHandler` checks for the factory, calls it to get a fresh assertion, and rebuilds the form body.
54+
55+
### Why this strategy
56+
57+
- **Backward compatible** — additive only, defaults to null, no behavioral change when unset
58+
- **Minimal coupling** — the handler only needs to know about `client_assertion` / `client_assertion_type` form fields
59+
- **Uses standard .NET patterns** — `HttpRequestMessage.Options` is the idiomatic way to pass data through handler chains
60+
- **Works at the right level** — the factory is set by callers who have access to key material, and consumed by the handler that performs the retry
61+
62+
### Alternatives considered and rejected
63+
64+
| Strategy | Why rejected |
65+
|----------|-------------|
66+
| A: Pass factory directly to handler constructor | Handler is a `DelegatingHandler` — can't easily access per-request assertion factories |
67+
| B: Call `Prepare()` again on retry | `Prepare()` is not idempotent (double-adds parameters) and is synchronous |
68+
| D: Move retry to callers | Major architectural change, duplicates retry logic across all callers |
69+
70+
### Implementation tasks
71+
72+
#### identity-model
73+
74+
- [ ] Add `Func<Task<ClientAssertion>>? ClientAssertionFactory` property to `ProtocolRequest`
75+
- [ ] Copy `ClientAssertionFactory` in `Clone<T>()`
76+
- [ ] Define a public `HttpRequestOptionsKey` for the factory (e.g. `ProtocolRequestOptions.ClientAssertionFactory`)
77+
- [ ] Store the factory on `HttpRequestMessage.Options` in `RequestTokenAsync()` after `Prepare()`
78+
- [ ] Also store in `PushAuthorizationAsync()` for PAR flow
79+
- [ ] Update public API verification snapshot
80+
81+
#### identity-model-oidc-client
82+
83+
- [ ] Update `ProofTokenMessageHandler.SendAsync()` retry path to:
84+
1. Check for factory in `request.Options`
85+
2. Call factory to get fresh `ClientAssertion`
86+
3. Parse existing form body as `IEnumerable<KeyValuePair<string, string>>` (preserving duplicate keys)
87+
4. Replace `client_assertion` and `client_assertion_type` values
88+
5. Rebuild `FormUrlEncodedContent`
89+
- [ ] Wire `ClientAssertionFactory` in `ResponseProcessor.RedeemCodeAsync()`
90+
- [ ] Wire `ClientAssertionFactory` in `OidcClient.RefreshTokenAsync()`
91+
- [ ] Wire `ClientAssertionFactory` in `AuthorizeClient.PushAuthorizationRequestAsync()`
92+
93+
#### access-token-management
94+
95+
- [ ] Fix `AuthorizationServerDPoPHandler` retry path (same pattern as `ProofTokenMessageHandler`)
96+
- [ ] Evaluate `ClientCredentialsTokenClient` and `OpenIdConnectUserTokenEndpoint` retry paths — they reuse the same `request.ClientAssertion` value across retries
97+
98+
#### Tests
99+
100+
- [ ] Unit test: `ProofTokenMessageHandler` uses fresh assertion on DPoP nonce retry
101+
- [ ] Unit test: `Clone<T>()` preserves `ClientAssertionFactory`
102+
- [ ] Unit test: backward compatibility — no factory set, behavior unchanged
103+
- [ ] Integration test: end-to-end DPoP + client assertion with nonce enforcement
104+
105+
### Implementation notes
106+
107+
- Form body parsing on retry **must** use `IEnumerable<KeyValuePair<string, string>>`, not a dictionary — dictionaries collapse duplicate keys like `resource` (identified during security review)
108+
- Both `client_assertion` and `client_assertion_type` should be replaced from the factory result to guard against type mismatches
109+
- `FormUrlEncodedContent` buffers internally, so `ReadAsStringAsync()` works after the first send

‎foss.slnx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
<File Path="identity-model/README.md" />
5151
</Folder>
5252
<Folder Name="/identity-model/samples/">
53+
<Project Path="identity-model/samples/ClientAssertions/ClientAssertions.csproj" />
5354
<Project Path="identity-model/samples/HttpClientFactory/HttpClientFactory.csproj" />
5455
</Folder>
5556
<Folder Name="/identity-model/src/">

‎identity-model-oidc-client/clients/ConsoleClientWithBrowser/Program.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
using Serilog;
1313
using Serilog.Sinks.SystemConsole.Themes;
1414

15-
namespace ConsoleClientWithBrowser;
15+
namespace NetCoreConsoleClient;
1616

1717
public class Program
1818
{

‎identity-model-oidc-client/clients/ConsoleClientWithBrowser/SystemBrowser.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
using Microsoft.AspNetCore.Hosting;
1111
using Microsoft.AspNetCore.Http;
1212

13-
namespace ConsoleClientWithBrowser;
13+
namespace NetCoreConsoleClient;
1414

1515
public class SystemBrowser : IBrowser
1616
{

‎identity-model-oidc-client/samples/NetCoreConsoleClient/src/NetCoreConsoleClient/ClientAssertionService.cs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) Duende Software. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
using NetCoreConsoleClient;
5+
using Duende.IdentityModel;
6+
using Duende.IdentityModel.Client;
7+
using Microsoft.IdentityModel.JsonWebTokens;
8+
using Microsoft.IdentityModel.Tokens;
9+
10+
public class ClientAssertionService
11+
{
12+
private static string RsaKey =
13+
"""
14+
{
15+
"d":"GmiaucNIzdvsEzGjZjd43SDToy1pz-Ph-shsOUXXh-dsYNGftITGerp8bO1iryXh_zUEo8oDK3r1y4klTonQ6bLsWw4ogjLPmL3yiqsoSjJa1G2Ymh_RY_sFZLLXAcrmpbzdWIAkgkHSZTaliL6g57vA7gxvd8L4s82wgGer_JmURI0ECbaCg98JVS0Srtf9GeTRHoX4foLWKc1Vq6NHthzqRMLZe-aRBNU9IMvXNd7kCcIbHCM3GTD_8cFj135nBPP2HOgC_ZXI1txsEf-djqJj8W5vaM7ViKU28IDv1gZGH3CatoysYx6jv1XJVvb2PH8RbFKbJmeyUm3Wvo-rgQ",
16+
"dp":"YNjVBTCIwZD65WCht5ve06vnBLP_Po1NtL_4lkholmPzJ5jbLYBU8f5foNp8DVJBdFQW7wcLmx85-NC5Pl1ZeyA-Ecbw4fDraa5Z4wUKlF0LT6VV79rfOF19y8kwf6MigyrDqMLcH_CRnRGg5NfDsijlZXffINGuxg6wWzhiqqE",
17+
"dq":"LfMDQbvTFNngkZjKkN2CBh5_MBG6Yrmfy4kWA8IC2HQqID5FtreiY2MTAwoDcoINfh3S5CItpuq94tlB2t-VUv8wunhbngHiB5xUprwGAAnwJ3DL39D2m43i_3YP-UO1TgZQUAOh7Jrd4foatpatTvBtY3F1DrCrUKE5Kkn770M",
18+
"e":"AQAB",
19+
"kid":"ZzAjSnraU3bkWGnnAqLapYGpTyNfLbjbzgAPbbW2GEA",
20+
"kty":"RSA",
21+
"n":"wWwQFtSzeRjjerpEM5Rmqz_DsNaZ9S1Bw6UbZkDLowuuTCjBWUax0vBMMxdy6XjEEK4Oq9lKMvx9JzjmeJf1knoqSNrox3Ka0rnxXpNAz6sATvme8p9mTXyp0cX4lF4U2J54xa2_S9NF5QWvpXvBeC4GAJx7QaSw4zrUkrc6XyaAiFnLhQEwKJCwUw4NOqIuYvYp_IXhw-5Ti_icDlZS-282PcccnBeOcX7vc21pozibIdmZJKqXNsL1Ibx5Nkx1F1jLnekJAmdaACDjYRLL_6n3W4wUp19UvzB1lGtXcJKLLkqB6YDiZNu16OSiSprfmrRXvYmvD8m6Fnl5aetgKw",
22+
"p":"7enorp9Pm9XSHaCvQyENcvdU99WCPbnp8vc0KnY_0g9UdX4ZDH07JwKu6DQEwfmUA1qspC-e_KFWTl3x0-I2eJRnHjLOoLrTjrVSBRhBMGEH5PvtZTTThnIY2LReH-6EhceGvcsJ_MhNDUEZLykiH1OnKhmRuvSdhi8oiETqtPE",
23+
"q":"0CBLGi_kRPLqI8yfVkpBbA9zkCAshgrWWn9hsq6a7Zl2LcLaLBRUxH0q1jWnXgeJh9o5v8sYGXwhbrmuypw7kJ0uA3OgEzSsNvX5Ay3R9sNel-3Mqm8Me5OfWWvmTEBOci8RwHstdR-7b9ZT13jk-dsZI7OlV_uBja1ny9Nz9ts",
24+
"qi":"pG6J4dcUDrDndMxa-ee1yG4KjZqqyCQcmPAfqklI2LmnpRIjcK78scclvpboI3JQyg6RCEKVMwAhVtQM6cBcIO3JrHgqeYDblp5wXHjto70HVW6Z8kBruNx1AH9E8LzNvSRL-JVTFzBkJuNgzKQfD0G77tQRgJ-Ri7qu3_9o1M4"
25+
}
26+
""";
27+
28+
private static SigningCredentials Credential = new(new JsonWebKey(RsaKey), "RS256");
29+
30+
public static Task<ClientAssertion> Create(CancellationToken ct = default)
31+
{
32+
var descriptor = new SecurityTokenDescriptor
33+
{
34+
Issuer = Configuration.ClientId,
35+
36+
// Don't use the TokenEndpoint here. Use the Authority as the audience.
37+
// You may expose yourself to a vulnerability, as described in the document below:
38+
// https://openid.net/wp-content/uploads/2025/01/OIDF-Responsible-Disclosure-Notice-on-Security-Vulnerability-for-private_key_jwt.pdf
39+
Audience = Configuration.Authority,
40+
Expires = DateTime.UtcNow.AddMinutes(1),
41+
SigningCredentials = Credential,
42+
43+
Claims = new Dictionary<string, object>
44+
{
45+
{ JwtClaimTypes.JwtId, Guid.NewGuid().ToString() },
46+
{ JwtClaimTypes.Subject, Configuration.ClientId },
47+
{ JwtClaimTypes.IssuedAt, DateTimeOffset.UtcNow.ToUnixTimeSeconds() }
48+
},
49+
50+
AdditionalHeaderClaims = new Dictionary<string, object>
51+
{
52+
{ JwtClaimTypes.TokenType, "client-authentication+jwt" }
53+
}
54+
};
55+
56+
var handler = new JsonWebTokenHandler();
57+
var jwt = handler.CreateToken(descriptor);
58+
59+
return Task.FromResult(new ClientAssertion
60+
{
61+
Type = OidcConstants.ClientAssertionTypes.JwtBearer,
62+
Value = jwt
63+
});
64+
}
65+
}

‎identity-model-oidc-client/samples/NetCoreConsoleClient/src/NetCoreConsoleClient/NetCoreConsoleClient.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
1212
<PackageReference Include="Serilog.Extensions.Logging" Version="8.0.0" />
1313
<PackageReference Include="Serilog.Sinks.Console" Version="5.0.1" />
14-
<PackageReference Include="Duende.IdentityModel.OidcClient" Version="6.0.0" />
14+
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" Version="8.16.0" />
15+
<ProjectReference Include="../../../../src/IdentityModel.OidcClient.Extensions/IdentityModel.OidcClient.Extensions.csproj" />
1516
</ItemGroup>
1617

1718
</Project>

0 commit comments

Comments
 (0)