fix(oauth): stop leaking access-token format across requests (#139)

docs/integrate/oauth.md

@@ -89,23 +89,36 @@ can't accept them cross-realm.
 ### AccessTokenTypeHandler
 
 OpenIddict has `UseReferenceAccessTokens()` globally. We want to choose
-between reference and JWT **per client**:
+between reference and JWT **per client**. The switch is a per-request
+`GenerateTokenContext` handler that, for JWT clients, flips the access
+token's per-request `IsReferenceToken` / `PersistTokenPayload` off:
 
 ```csharp
-public async ValueTask HandleAsync(ProcessSignInContext context)
+public async ValueTask HandleAsync(GenerateTokenContext context)
 {
+    // Access tokens only — refresh tokens keep the global setting.
+    if (context.TokenType != OpenIddictConstants.TokenTypeIdentifiers.AccessToken) return;
+
     var app = await _querySession.Query<OAuthApplicationState>()
-        .FirstOrDefaultAsync(a => a.ClientId == clientId && !a.IsDeleted);
+        .FirstOrDefaultAsync(a => a.ClientId == context.ClientId && !a.IsDeleted);
 
     if (app?.AccessTokenType == AccessTokenType.Jwt)
     {
-        // Disable reference token storage for this request.
-        // OpenIddict generates a self-contained JWT instead.
-        context.Options.UseReferenceAccessTokens = false;
+        // Emit a self-contained JWT for this request instead of an
+        // opaque, server-stored reference identifier.
+        context.IsReferenceToken = false;
+        context.PersistTokenPayload = false;
     }
 }
 ```
 
+It hooks `GenerateTokenContext` (a per-request context) and must **never**
+mutate `context.Options`: that object is the process-wide
+`OpenIddictServerOptions` singleton (`IOptionsMonitor.CurrentValue`), so
+writing `Options.UseReferenceAccessTokens` there leaks across requests — one
+JWT client's sign-in would flip the global default off for every later
+reference client — and races concurrent requests.
+
 Tokens default to reference (= server-side stored, opaque, revocable).
 Per client this can be switched to JWT when the round-trip profile is
 a problem.
@@ -159,8 +172,8 @@ For authorization-code exchange:
 1. POST /connect/token with grant_type=authorization_code + code + verifier
 2. OpenIddict validates the code (exists, not expired, not used)
 3. PKCE challenge is verified
-4. ProcessSignIn fires → AccessTokenTypeHandler decides
-   reference vs. JWT
+4. Token generation fires → AccessTokenTypeHandler decides
+   reference vs. JWT (per access token)
 5. Tokens are issued:
    - Reference: OpenIddictTokenDocument(s) created, reference IDs returned
    - JWT: signed JWTs returned, no DB entry

src/dotnet/Modgud.Api.Tests/Authorization/UserInfoPerAudienceTests.cs

@@ -1128,6 +1128,65 @@ await GrantAsync(user.Id, roleAppSlug: "rs-refapp", resourceType: "policy",
         Assert.Equal(HttpStatusCode.Unauthorized, (await rs.GetAsync("/policy/read", TestContext.Current.CancellationToken)).StatusCode);
     }
 
+    [Fact]
+    public async Task ReferenceClient_TokenFormat_IsNotLeaked_By_A_Prior_JwtClient()
+    {
+        // Regression for a cross-request state-leak in AccessTokenTypeHandler. It
+        // used to switch a JWT client onto self-contained tokens by mutating the
+        // SHARED OpenIddictServerOptions singleton
+        // (context.Options.UseReferenceAccessTokens = false) and never restoring
+        // it. Because IOptionsMonitor.CurrentValue hands every request the same
+        // options instance, the first JWT-client sign-in flipped the global
+        // reference default OFF process-wide — so every Reference-configured
+        // client that authenticated afterwards silently received a self-contained
+        // JWT instead of an opaque reference token, losing instant revocation.
+        //
+        // This drives a JWT client FIRST (to flip the flag under the old code)
+        // then a Reference client in the SAME process, and asserts the reference
+        // client's token is still opaque.
+        var app = await CreateAppAsync("leak-app", "Leak App",
+            permissions: [("policy", "read"), ("policy", "write")]);
+        const string audience = "https://leak-api.example.com";
+        await CreateOAuthApiAsync(audience, app.Id);
+        const string scopeName = "leak-api";
+        await CreateScopeAsync(scopeName, [audience], app.Id);
+
+        var user = await Factory.CreateTestUserWithIdentityAsync(
+            firstname: "Leak", lastname: "Guard", acronym: "lg",
+            email: "lg@test.com", password: "TestPass1234");
+        await GrantAsync(user.Id, roleAppSlug: "leak-app", resourceType: "policy",
+            actions: ["write"], groupBoundTo: ["leak-app"]);
+
+        // 1) JWT client authenticates first — under the bug this flips the shared
+        //    singleton's UseReferenceAccessTokens to false for the whole process.
+        var jwtSecret = "TestClientSecret_" + Guid.NewGuid().ToString("N");
+        var jwtClientId = "test-jwt-leaker-" + Guid.NewGuid().ToString("N");
+        await CreateOAuthClientAsync(
+            jwtClientId, jwtSecret, "http://localhost/jwt-callback", [app.Id],
+            ["openid", "roles", "permissions", scopeName], AccessTokenType.Jwt);
+        var jwtToken = await DriveAuthCodeFlowAsync(
+            username: "lg", password: "TestPass1234", clientId: jwtClientId, clientSecret: jwtSecret,
+            redirectUri: "http://localhost/jwt-callback",
+            scope: $"openid roles permissions {scopeName}", resources: [audience]);
+        Assert.Contains('.', jwtToken); // sanity: the JWT client really got a JWT.
+
+        // 2) Reference client authenticates AFTER — its token MUST stay opaque.
+        var refSecret = "TestClientSecret_" + Guid.NewGuid().ToString("N");
+        var refClientId = "test-ref-victim-" + Guid.NewGuid().ToString("N");
+        await CreateOAuthClientAsync(
+            refClientId, refSecret, "http://localhost/ref-callback", [app.Id],
+            ["openid", "roles", "permissions", scopeName], AccessTokenType.Reference);
+        var refToken = await DriveAuthCodeFlowAsync(
+            username: "lg", password: "TestPass1234", clientId: refClientId, clientSecret: refSecret,
+            redirectUri: "http://localhost/ref-callback",
+            scope: $"openid roles permissions {scopeName}", resources: [audience]);
+
+        // An OpenIddict reference access token is Base64Url(random 256 bytes) —
+        // opaque, no dots. A dot means the client was handed a self-contained JWT
+        // (the leak); the Base64Url alphabet (A–Z a–z 0–9 - _) never contains one.
+        Assert.DoesNotContain('.', refToken);
+    }
+
     private static async Task<HttpResponseMessage> SendWithTokenAsync(HttpClient client, string path, string token)
     {
         using var req = new HttpRequestMessage(HttpMethod.Get, path);

src/dotnet/Modgud.Infrastructure/OpenIddict/AccessTokenTypeHandler.cs

@@ -1,24 +1,47 @@
 using Modgud.Domain.OAuth.Applications;
 using Modgud.Domain.OAuth.Common;
 using Marten;
+using OpenIddict.Abstractions;
 using OpenIddict.Server;
+using static OpenIddict.Server.OpenIddictServerEvents;
+using static OpenIddict.Server.OpenIddictServerHandlers;
 
 namespace Modgud.Infrastructure.OpenIddict;
 
 /// <summary>
-/// OpenIddict server event handler that switches access tokens from reference to JWT
-/// based on the per-client <see cref="AccessTokenType"/> setting.
+/// OpenIddict token-generation handler that switches a client's ACCESS token
+/// from the global reference-token default to a self-contained JWT, based on the
+/// per-client <see cref="AccessTokenType"/> setting.
 ///
-/// <para>By default, <c>UseReferenceAccessTokens()</c> is enabled globally. For clients
-/// configured with <see cref="AccessTokenType.Jwt"/>, this handler disables reference
-/// token storage for that request so OpenIddict generates a self-contained JWT instead.</para>
+/// <para><c>UseReferenceAccessTokens()</c> is enabled globally. For a client
+/// configured with <see cref="AccessTokenType.Jwt"/>, this handler flips OFF —
+/// for the access token of this request only — the two per-request switches
+/// OpenIddict reads at generation time (<c>IsReferenceToken</c> and
+/// <c>PersistTokenPayload</c>), so a self-contained JWT is emitted instead of an
+/// opaque reference identifier.</para>
+///
+/// <para>It hooks <see cref="GenerateTokenContext"/> — a fresh, per-request
+/// context — and MUST NOT mutate <c>context.Options</c>. That options object is
+/// the process-wide <see cref="OpenIddictServerOptions"/> singleton
+/// (<c>IOptionsMonitor.CurrentValue</c> hands every request the same instance);
+/// writing <c>Options.UseReferenceAccessTokens</c> there leaks across requests
+/// and races concurrent ones — a single JWT client's sign-in would flip the
+/// global default off for every later reference client, silently downgrading
+/// their opaque tokens to self-contained JWTs and losing instant revocation.</para>
+///
+/// <para>Only access tokens are touched; refresh (and any other) token types keep
+/// the reference semantics they derive from the global options.</para>
 /// </summary>
-public sealed class AccessTokenTypeHandler : IOpenIddictServerHandler<OpenIddictServerEvents.ProcessSignInContext>
+public sealed class AccessTokenTypeHandler : IOpenIddictServerHandler<GenerateTokenContext>
 {
     public static OpenIddictServerHandlerDescriptor Descriptor { get; }
-        = OpenIddictServerHandlerDescriptor.CreateBuilder<OpenIddictServerEvents.ProcessSignInContext>()
+        = OpenIddictServerHandlerDescriptor.CreateBuilder<GenerateTokenContext>()
             .UseScopedHandler<AccessTokenTypeHandler>()
-            .SetOrder(OpenIddictServerHandlers.GenerateAccessToken.Descriptor.Order - 1)
+            // Run before OpenIddict persists/attaches the payload (AttachTokenPayload,
+            // which reads IsReferenceToken + PersistTokenPayload). CreateTokenEntry is
+            // the earliest stable anchor and ignores both flags, so slotting just
+            // ahead of it guarantees every downstream handler sees our override.
+            .SetOrder(Protection.CreateTokenEntry.Descriptor.Order - 500)
             .SetType(OpenIddictServerHandlerType.Custom)
             .Build();
 
@@ -31,13 +54,19 @@ public AccessTokenTypeHandler(IQuerySession querySession, Cimd.CimdClientResolve
         _cimdResolver = cimdResolver;
     }
 
-    public async ValueTask HandleAsync(OpenIddictServerEvents.ProcessSignInContext context)
+    public async ValueTask HandleAsync(GenerateTokenContext context)
     {
+        ArgumentNullException.ThrowIfNull(context);
+
+        // Access tokens only — leave refresh/authorization-code/… to the global
+        // reference settings.
+        if (context.TokenType != OpenIddictConstants.TokenTypeIdentifiers.AccessToken) return;
+
         var clientId = context.ClientId;
         if (string.IsNullOrEmpty(clientId)) return;
 
         var app = await _querySession.Query<OAuthApplicationState>()
-            .FirstOrDefaultAsync(a => a.ClientId == clientId && !a.IsDeleted);
+            .FirstOrDefaultAsync(a => a.ClientId == clientId && !a.IsDeleted, context.CancellationToken);
 
         // CIMD clients are non-persisted, so the direct query misses them —
         // fall back to the resolver (cache-warm after the store's resolve
@@ -46,7 +75,11 @@ public async ValueTask HandleAsync(OpenIddictServerEvents.ProcessSignInContext c
 
         if (app is null || app.AccessTokenType != AccessTokenType.Jwt) return;
 
-        // For JWT clients: disable reference token storage for this request.
-        context.Options.UseReferenceAccessTokens = false;
+        // JWT client: emit a self-contained token — don't persist the payload as a
+        // reference and don't hand back a reference identifier. The token ENTRY is
+        // still created for revocation tracking (governed by CreateTokenEntry /
+        // DisableTokenStorage, which we leave untouched).
+        context.IsReferenceToken = false;
+        context.PersistTokenPayload = false;
     }
 }