diff --git a/.gitignore b/.gitignore index fe88b85..a2fc0e8 100644 --- a/.gitignore +++ b/.gitignore @@ -42,4 +42,7 @@ Thumbs.db /tools/ #Claude folder -.claude/ \ No newline at end of file +.claude/ + +# Serena local workspace metadata +.serena/ diff --git a/CHANGELOG.md b/CHANGELOG.md index ccffac0..430d852 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ All notable changes to DevBrain are tracked in this file. Versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- Hardened the OAuth `refresh_token` grant for MCP clients that retry or restart while their local credential cache is catching up to token rotation. A successful refresh now leaves a five-minute replay marker for the old refresh token, so an immediate retry returns the same replacement refresh token instead of forcing a reconnect. Wrong-client refresh attempts are rejected without consuming the legitimate client's token, and successful refresh/replay calls slide the upstream token vault TTL forward with the local refresh window. + +### Changed +- Synced the release notes with merged Dependabot PR #19, which already moved `Microsoft.AspNetCore.DataProtection` and `System.Security.Cryptography.Xml` to 10.0.7. +- Patched the remaining Azure Data Protection helper packages to `Azure.Extensions.AspNetCore.DataProtection.Blobs` 1.5.2 and `Azure.Extensions.AspNetCore.DataProtection.Keys` 1.6.2, then replaced the stale 10.0.6 workaround comment in the project file. +- Added `.serena/` to `.gitignore` so local Serena workspace metadata stays out of the public repository. + +### Validation +- `dotnet list devbrain.slnx package --vulnerable --include-transitive` reports no vulnerable packages. +- `dotnet list devbrain.slnx package --outdated --highest-patch` reports no patch-level updates for direct package references. +- `dotnet list devbrain.slnx package --outdated --include-transitive` was checked; it still reports broader direct/transitive updates in Azure Functions, Application Insights, IdentityModel, Cosmos, and test tooling that are left for a separate dependency refresh. +- `dotnet list devbrain.slnx package --deprecated` still reports two known migration items left outside this auth fix: `Microsoft.ApplicationInsights.WorkerService` 2.22.0 and `xunit` 2.9.3. +- `dotnet test devbrain.slnx` passes with 141 tests. + ## [1.9.0] — 2026-04-15 A new `EditTags` tool lets callers adjust tag metadata on an existing document without re-emitting its content. Previously the only way to add or drop a tag was a full `UpsertDocument` round-trip that re-sent the entire body — wasteful for large documents whose content isn't changing. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cf68fcf..65876d9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,8 +34,15 @@ Thanks for your interest in contributing to DevBrain! 1. Fork the repository and create a feature branch from `main`. 2. Make your changes. Keep commits focused and atomic. 3. Ensure `dotnet build` completes with no warnings (warnings are treated as errors). -4. Open a pull request against `main` with a clear description of the change. -5. A maintainer will review and merge once CI passes. +4. For dependency changes, check the whole solution from the repository root before opening the PR: + ```bash + dotnet list devbrain.slnx package --vulnerable --include-transitive + dotnet list devbrain.slnx package --outdated --highest-patch + dotnet list devbrain.slnx package --outdated --include-transitive + dotnet list devbrain.slnx package --deprecated + ``` +5. Open a pull request against `main` with a clear description of the change. +6. A maintainer will review and merge once CI passes. ## Code Style diff --git a/README.md b/README.md index e8130c7..8ea7c3a 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,8 @@ OAuth completes automatically — no proxy, no function key, no manual headers. codex mcp add devbrain --transport http https:///runtime/webhooks/mcp ``` +DevBrain rotates OAuth refresh tokens on every refresh. To tolerate Codex Windows App/CLI retry or restart races while the local credential cache catches up, DevBrain keeps a short replay window for the just-rotated token and returns the same replacement refresh token during that window. + ### VS Code / GitHub Copilot ⚠️ **Known issue:** The VS Code MCP extension connects successfully and discovers all tools, but does not trigger the OAuth flow. See [Known Limitations](#vs-code--github-copilot-mcp-extension--oauth-not-triggered) below for the full explanation and fix paths. @@ -270,6 +272,14 @@ Keys use colon as the separator (e.g. `sprint:license-sync`). **Writes** (`Upser func start ``` +5. Optional dependency health checks from the repository root: + ```powershell + dotnet list devbrain.slnx package --vulnerable --include-transitive + dotnet list devbrain.slnx package --outdated --highest-patch + dotnet list devbrain.slnx package --outdated --include-transitive + dotnet list devbrain.slnx package --deprecated + ``` + ## Authentication DevBrain implements RFC 7591 Dynamic Client Registration (DCR) with an in-process OAuth proxy that brokers a single pre-registered Entra app. From the client's perspective, DevBrain *is* the authorization server. Internally it delegates to your tenant's Entra ID for user authentication. @@ -281,6 +291,10 @@ This solves two problems that previously blocked MCP OAuth: Every write operation records the authenticated user's Entra UPN in the `updatedBy` field. +### Refresh Token Rotation + +Access tokens are short-lived and DevBrain refresh tokens rotate on every refresh. The old refresh token becomes a five-minute replay marker that points at the replacement token, which makes immediate MCP client retries idempotent without reopening the OAuth flow. Replays outside that window still fail with `invalid_grant`, and every successful refresh or replay extends the upstream token vault record for the same local refresh window. + ## Known Limitations ### VS Code / GitHub Copilot MCP extension — OAuth not triggered diff --git a/src/DevBrain.Functions/Auth/DcrFacade/TokenHandler.cs b/src/DevBrain.Functions/Auth/DcrFacade/TokenHandler.cs index 0c3fddb..e97eb12 100644 --- a/src/DevBrain.Functions/Auth/DcrFacade/TokenHandler.cs +++ b/src/DevBrain.Functions/Auth/DcrFacade/TokenHandler.cs @@ -9,14 +9,14 @@ namespace DevBrain.Functions.Auth.DcrFacade; /// /// Service layer for POST /token. Handles both the authorization_code and /// refresh_token grant types. Atomicity of code redemption and refresh rotation lives in -/// / +/// / /// — this handler is responsible for the validation around those two atomic pivots. /// /// Acceptance gates covered here: /// /// #2 PKCE downgrade — verifier mismatch returns invalid_grant. /// #3 Code replay — second redemption returns invalid_grant via the atomic store. -/// #5 Refresh rotation — every refresh grant rotates; the old token is consumed atomically. +/// #5 Refresh rotation — every refresh grant rotates; the old token becomes a short-lived replay marker. /// /// public sealed class TokenHandler @@ -28,6 +28,14 @@ public sealed class TokenHandler // Refresh tokens: 30 days to match the sprint spec. Rotated on every use. private static readonly TimeSpan RefreshTokenLifetime = TimeSpan.FromDays(30); + // Short replay window for clients that retry or restart immediately after a rotation but still + // present the just-rotated token from local credential cache. + private static readonly TimeSpan RefreshReplayLifetime = TimeSpan.FromMinutes(5); + + // Keep the upstream vault alive for the full local refresh window. CallbackHandler creates the + // initial vault record; the refresh path slides it forward on every successful rotation/replay. + private static readonly TimeSpan UpstreamVaultTtl = TimeSpan.FromDays(30); + private readonly IOAuthStateStore _store; private readonly DevBrainJwtIssuer _jwtIssuer; private readonly TimeProvider _timeProvider; @@ -147,34 +155,33 @@ private async Task HandleRefreshAsync(TokenRequest request) return TokenResult.Error("invalid_request", "client_id is required."); } - var record = await _store.ConsumeRefreshAsync(request.RefreshToken); - if (record is null) - { - _logger?.LogWarning("TokenHandler/refresh: rejected — refresh_token invalid, expired, or already rotated"); - return TokenResult.Error("invalid_grant", "refresh_token is invalid, expired, or already rotated."); - } + var replacementRefresh = GenerateOpaqueToken(); + var rotation = await _store.RotateRefreshAsync( + request.RefreshToken, + request.ClientId, + replacementRefresh, + RefreshTokenLifetime, + RefreshReplayLifetime, + UpstreamVaultTtl); - if (!string.Equals(record.ClientId, request.ClientId, StringComparison.Ordinal)) + if (rotation is null) { - _logger?.LogWarning( - "TokenHandler/refresh: rejected — client binding mismatch tokenClientId={TokenClientId} requestClientId={RequestClientId}", - record.ClientId, request.ClientId); - return TokenResult.Error("invalid_grant", "refresh_token was issued to a different client."); + _logger?.LogWarning("TokenHandler/refresh: rejected — refresh_token invalid, expired, wrong client, or upstream session expired"); + return TokenResult.Error("invalid_grant", "refresh_token is invalid, expired, already rotated outside the replay window, or bound to a different client."); } - var upstreamJti = record.UpstreamJti; + var upstreamJti = rotation.UpstreamJti; var (jwt, _) = IssueJwtForUpstream(upstreamJti); - var newRefresh = await MintAndStoreRefreshAsync(record.ClientId, upstreamJti); _logger?.LogInformation( - "TokenHandler/refresh: rotated refresh clientId={ClientId} upstreamJti={Jti}", - record.ClientId, upstreamJti); + "TokenHandler/refresh: {RotationKind} refresh clientId={ClientId} upstreamJti={Jti}", + rotation.IsReplay ? "replayed" : "rotated", request.ClientId, upstreamJti); return TokenResult.Success(new TokenResponse( AccessToken: jwt, TokenType: "Bearer", ExpiresIn: (int)AccessTokenLifetime.TotalSeconds, - RefreshToken: newRefresh, + RefreshToken: rotation.RefreshToken, Scope: "documents.readwrite")); } diff --git a/src/DevBrain.Functions/Auth/Models/DevBrainRefreshRecord.cs b/src/DevBrain.Functions/Auth/Models/DevBrainRefreshRecord.cs index 0e308d5..6685907 100644 --- a/src/DevBrain.Functions/Auth/Models/DevBrainRefreshRecord.cs +++ b/src/DevBrain.Functions/Auth/Models/DevBrainRefreshRecord.cs @@ -4,9 +4,8 @@ namespace DevBrain.Functions.Auth.Models; /// /// A DevBrain refresh token. Rotated on every use (see -/// ) — FastMCP's rotation pattern, -/// adopted because a long-lived non-rotating refresh is a meaningfully worse stolen-token story. -/// Stored in Cosmos under key refresh:{RefreshToken}. +/// ) with a short replay marker for +/// idempotent client retries. Stored in Cosmos under key refresh:{RefreshToken}. /// public sealed class DevBrainRefreshRecord { @@ -32,6 +31,19 @@ public sealed class DevBrainRefreshRecord [JsonPropertyName("expiresAt")] public DateTimeOffset ExpiresAt { get; set; } + /// + /// When set, this record is no longer an active refresh token. It is a short-lived replay + /// marker pointing to the replacement token returned by the winning rotation request. + /// + [JsonPropertyName("rotatedToRefreshToken")] + public string? RotatedToRefreshToken { get; set; } + + [JsonPropertyName("rotatedAt")] + public DateTimeOffset? RotatedAt { get; set; } + [JsonPropertyName("ttl")] public int Ttl { get; set; } + + [JsonIgnore] + public bool IsReplayMarker => !string.IsNullOrEmpty(RotatedToRefreshToken); } diff --git a/src/DevBrain.Functions/Auth/Services/CosmosOAuthStateStore.cs b/src/DevBrain.Functions/Auth/Services/CosmosOAuthStateStore.cs index 0779332..24066c0 100644 --- a/src/DevBrain.Functions/Auth/Services/CosmosOAuthStateStore.cs +++ b/src/DevBrain.Functions/Auth/Services/CosmosOAuthStateStore.cs @@ -10,10 +10,9 @@ namespace DevBrain.Functions.Auth.Services; /// Cosmos-backed . Uses the dedicated oauth_state container /// (partition key /key) with native TTL enabled. /// -/// Atomic operations (, ) use the -/// read-then-ETag-conditional-delete pattern: one reader wins the delete, all concurrent readers -/// that lose get a PreconditionFailed and return null. Same optimistic-concurrency idea as -/// CosmosDocumentStore.AppendAsync. +/// Atomic operations use ETag checks. Auth-code redemption keeps the read-then-conditional-delete +/// pattern; refresh rotation conditionally replaces the old token with a short-lived replay marker +/// so parallel client retries can observe the winning replacement token. /// /// Expiry is checked defensively against an injected on every read — /// Cosmos native TTL is best-effort and must not be trusted for security decisions. @@ -195,6 +194,108 @@ public Task SaveRefreshAsync(DevBrainRefreshRecord refresh) return UpsertAsync(refresh, key); } + public async Task RotateRefreshAsync( + string refreshToken, + string clientId, + string replacementRefreshToken, + TimeSpan replacementLifetime, + TimeSpan replayLifetime, + TimeSpan upstreamVaultLifetime) + { + var key = RefreshKey(refreshToken); + var partition = new PartitionKey(key); + var now = _timeProvider.GetUtcNow(); + + DevBrainRefreshRecord record; + string etag; + try + { + var response = await _container.ReadItemAsync(key, partition); + record = response.Resource; + etag = response.ETag; + } + catch (CosmosException ex) when (ex.StatusCode == HttpStatusCode.NotFound) + { + return null; + } + + if (record.ExpiresAt <= now) + { + await TryDeleteAsync(key, partition, etag); + return null; + } + + if (!string.Equals(record.ClientId, clientId, StringComparison.Ordinal)) + { + return null; + } + + if (record.IsReplayMarker) + { + if (string.IsNullOrEmpty(record.RotatedToRefreshToken)) + { + return null; + } + + if (!await TouchUpstreamTokenAsync(record.UpstreamJti, upstreamVaultLifetime)) + { + return null; + } + + return new RefreshRotationResult(record.UpstreamJti, record.RotatedToRefreshToken, IsReplay: true); + } + + if (!await TouchUpstreamTokenAsync(record.UpstreamJti, upstreamVaultLifetime)) + { + return null; + } + + var replacement = new DevBrainRefreshRecord + { + RefreshToken = replacementRefreshToken, + ClientId = record.ClientId, + UpstreamJti = record.UpstreamJti, + CreatedAt = now, + ExpiresAt = now + replacementLifetime, + Ttl = (int)replacementLifetime.TotalSeconds, + }; + await SaveRefreshAsync(replacement); + + var marker = new DevBrainRefreshRecord + { + Id = key, + Key = key, + RefreshToken = refreshToken, + ClientId = record.ClientId, + UpstreamJti = record.UpstreamJti, + CreatedAt = record.CreatedAt, + ExpiresAt = now + replayLifetime, + RotatedAt = now, + RotatedToRefreshToken = replacementRefreshToken, + Ttl = (int)replayLifetime.TotalSeconds, + }; + + try + { + await _container.ReplaceItemAsync( + marker, + key, + partition, + new ItemRequestOptions { IfMatchEtag = etag }); + + return new RefreshRotationResult(record.UpstreamJti, replacementRefreshToken, IsReplay: false); + } + catch (CosmosException ex) + when (ex.StatusCode == HttpStatusCode.PreconditionFailed + || ex.StatusCode == HttpStatusCode.NotFound) + { + // Another request won the rotation. Re-read once and, if it left a replay marker, + // return that winning replacement instead of surfacing a spurious invalid_grant. + await DeleteAsync(RefreshKey(replacementRefreshToken)); + return await ReadRefreshReplayAsync(refreshToken, clientId, upstreamVaultLifetime); + } + } + public async Task ConsumeRefreshAsync(string refreshToken) { var key = RefreshKey(refreshToken); @@ -237,6 +338,89 @@ await _container.DeleteItemAsync( // ---------------- Internal helpers ---------------- + private async Task ReadRefreshReplayAsync( + string refreshToken, + string clientId, + TimeSpan upstreamVaultLifetime) + { + var key = RefreshKey(refreshToken); + try + { + var response = await _container.ReadItemAsync(key, new PartitionKey(key)); + var record = response.Resource; + if (record.ExpiresAt <= _timeProvider.GetUtcNow() + || !record.IsReplayMarker + || string.IsNullOrEmpty(record.RotatedToRefreshToken) + || !string.Equals(record.ClientId, clientId, StringComparison.Ordinal)) + { + return null; + } + + if (!await TouchUpstreamTokenAsync(record.UpstreamJti, upstreamVaultLifetime)) + { + return null; + } + + return new RefreshRotationResult(record.UpstreamJti, record.RotatedToRefreshToken, IsReplay: true); + } + catch (CosmosException ex) when (ex.StatusCode == HttpStatusCode.NotFound) + { + return null; + } + } + + private async Task TouchUpstreamTokenAsync(string jti, TimeSpan lifetime) + { + var key = UpstreamKey(jti); + var partition = new PartitionKey(key); + try + { + var response = await _container.ReadItemAsync(key, partition); + var dto = response.Resource; + var now = _timeProvider.GetUtcNow(); + if (dto.ExpiresAt <= now) + { + await TryDeleteAsync(key, partition, response.ETag); + return false; + } + + dto.ExpiresAt = now + lifetime; + dto.Ttl = (int)lifetime.TotalSeconds; + await _container.ReplaceItemAsync( + dto, + key, + partition, + new ItemRequestOptions { IfMatchEtag = response.ETag }); + return true; + } + catch (CosmosException ex) + when (ex.StatusCode == HttpStatusCode.NotFound) + { + return false; + } + catch (CosmosException ex) + when (ex.StatusCode == HttpStatusCode.PreconditionFailed) + { + // A concurrent refresh already slid the expiry. Treat that as success if the vault + // entry is still present and live, otherwise parallel refreshes can fail spuriously. + return await UpstreamTokenStillLiveAsync(jti); + } + } + + private async Task UpstreamTokenStillLiveAsync(string jti) + { + var key = UpstreamKey(jti); + try + { + var response = await _container.ReadItemAsync(key, new PartitionKey(key)); + return response.Resource.ExpiresAt > _timeProvider.GetUtcNow(); + } + catch (CosmosException ex) when (ex.StatusCode == HttpStatusCode.NotFound) + { + return false; + } + } + private async Task UpsertAsync(T item, string key) { await _container.UpsertItemAsync(item, new PartitionKey(key)); diff --git a/src/DevBrain.Functions/Auth/Services/IOAuthStateStore.cs b/src/DevBrain.Functions/Auth/Services/IOAuthStateStore.cs index 600c337..22060e9 100644 --- a/src/DevBrain.Functions/Auth/Services/IOAuthStateStore.cs +++ b/src/DevBrain.Functions/Auth/Services/IOAuthStateStore.cs @@ -14,10 +14,9 @@ namespace DevBrain.Functions.Auth.Services; /// the prefixed Cosmos key. /// Expiry is checked defensively against an injected on every /// read. Cosmos native TTL is best-effort and not trusted for security decisions. -/// Two operations are explicitly atomic: and -/// . Concurrent callers are guaranteed that exactly one -/// receives the record; all others receive null. Implemented via Cosmos ETag -/// conditional delete. +/// Auth-code redemption and legacy refresh consumption are single-take operations. +/// Refresh rotation uses so concurrent client retries can +/// replay the winning rotation for a short grace window. /// /// public interface IOAuthStateStore @@ -59,10 +58,27 @@ public interface IOAuthStateStore Task SaveRefreshAsync(DevBrainRefreshRecord refresh); + /// + /// Rotates a refresh token when it is still active and issued to . + /// The old refresh token becomes a short-lived replay marker pointing to + /// ; calls during that window return the same + /// replacement token. The referenced upstream vault record is defensively extended as part of + /// the rotation/replay path. + /// + Task RotateRefreshAsync( + string refreshToken, + string clientId, + string replacementRefreshToken, + TimeSpan replacementLifetime, + TimeSpan replayLifetime, + TimeSpan upstreamVaultLifetime); + /// /// Atomically consumes a refresh token. Returns the stored record on success. /// Returns null if the token does not exist, has expired, or has already been rotated. /// Callers are expected to mint a new refresh and it as part of the same operation. + /// Legacy primitive retained for low-level store tests; new token endpoint code should use + /// . /// Task ConsumeRefreshAsync(string refreshToken); } diff --git a/src/DevBrain.Functions/Auth/Services/RefreshRotationResult.cs b/src/DevBrain.Functions/Auth/Services/RefreshRotationResult.cs new file mode 100644 index 0000000..9b077e5 --- /dev/null +++ b/src/DevBrain.Functions/Auth/Services/RefreshRotationResult.cs @@ -0,0 +1,10 @@ +namespace DevBrain.Functions.Auth.Services; + +/// +/// Result of rotating a DevBrain refresh token. Immediate replays of the old token return the +/// same replacement refresh token so client retries remain idempotent. +/// +public sealed record RefreshRotationResult( + string UpstreamJti, + string RefreshToken, + bool IsReplay); diff --git a/src/DevBrain.Functions/DevBrain.Functions.csproj b/src/DevBrain.Functions/DevBrain.Functions.csproj index 1928129..a225448 100644 --- a/src/DevBrain.Functions/DevBrain.Functions.csproj +++ b/src/DevBrain.Functions/DevBrain.Functions.csproj @@ -19,16 +19,13 @@ - - + + - + @@ -44,4 +41,4 @@ - \ No newline at end of file + diff --git a/tests/DevBrain.Functions.Tests/Auth/DcrFacade/TokenHandlerTests.cs b/tests/DevBrain.Functions.Tests/Auth/DcrFacade/TokenHandlerTests.cs index 67a3c61..498a9b7 100644 --- a/tests/DevBrain.Functions.Tests/Auth/DcrFacade/TokenHandlerTests.cs +++ b/tests/DevBrain.Functions.Tests/Auth/DcrFacade/TokenHandlerTests.cs @@ -204,9 +204,9 @@ public async Task AuthorizationCode_Expired_Rejected() Assert.Equal("invalid_grant", result.ErrorCode); } - /// Gate #5: refresh token rotation — happy path and rotation invariant. + /// Gate #5: refresh token rotation with short idempotent replay. [Fact] - public async Task RefreshToken_RotatesOldAndIssuesNew() + public async Task RefreshToken_RotatesOldAndAllowsShortReplay() { var h = Create(); var (code, verifier, _) = await SeedAuthCodeAsync(h); @@ -229,11 +229,20 @@ public async Task RefreshToken_RotatesOldAndIssuesNew() Assert.NotEqual(firstRefresh, refreshed.Response!.RefreshToken); Assert.NotEmpty(refreshed.Response.AccessToken); - // Gate #5 invariant: the old refresh must now be unusable. + // Immediate retry/restart with the old token returns the same replacement refresh token. var replayed = await h.Handler.HandleAsync(new TokenRequest( "refresh_token", ClientId, null, null, null, firstRefresh)); - Assert.False(replayed.IsSuccess); - Assert.Equal("invalid_grant", replayed.ErrorCode); + Assert.True(replayed.IsSuccess); + Assert.Equal(refreshed.Response.RefreshToken, replayed.Response!.RefreshToken); + Assert.NotEmpty(replayed.Response.AccessToken); + + h.Clock.Advance(TimeSpan.FromMinutes(6)); + + // After the replay marker expires, the old refresh is rejected. + var lateReplay = await h.Handler.HandleAsync(new TokenRequest( + "refresh_token", ClientId, null, null, null, firstRefresh)); + Assert.False(lateReplay.IsSuccess); + Assert.Equal("invalid_grant", lateReplay.ErrorCode); // New refresh works. var third = await h.Handler.HandleAsync(new TokenRequest( @@ -255,6 +264,34 @@ public async Task RefreshToken_WrongClient_Rejected() Assert.False(attacker.IsSuccess); Assert.Equal("invalid_grant", attacker.ErrorCode); + + // A wrong-client attempt must not burn the legitimate client's refresh token. + var legitimate = await h.Handler.HandleAsync(new TokenRequest( + "refresh_token", ClientId, null, null, null, initial.Response!.RefreshToken)); + Assert.True(legitimate.IsSuccess); + } + + [Fact] + public async Task RefreshToken_ExtendsUpstreamVaultExpiry() + { + var h = Create(); + var (code, verifier, upstreamJti) = await SeedAuthCodeAsync(h); + + var initial = await h.Handler.HandleAsync(new TokenRequest( + "authorization_code", ClientId, code, verifier, ClientRedirect, null)); + + var before = await h.Store.GetUpstreamTokenAsync(upstreamJti); + Assert.NotNull(before); + Assert.Equal(Epoch.AddHours(1), before!.ExpiresAt); + + var refreshed = await h.Handler.HandleAsync(new TokenRequest( + "refresh_token", ClientId, null, null, null, initial.Response!.RefreshToken)); + + Assert.True(refreshed.IsSuccess); + + var after = await h.Store.GetUpstreamTokenAsync(upstreamJti); + Assert.NotNull(after); + Assert.Equal(Epoch.AddDays(30), after!.ExpiresAt); } [Fact] diff --git a/tests/DevBrain.Functions.Tests/Auth/Services/FakeOAuthStateStore.cs b/tests/DevBrain.Functions.Tests/Auth/Services/FakeOAuthStateStore.cs index ddd028d..5a7de8a 100644 --- a/tests/DevBrain.Functions.Tests/Auth/Services/FakeOAuthStateStore.cs +++ b/tests/DevBrain.Functions.Tests/Auth/Services/FakeOAuthStateStore.cs @@ -9,8 +9,9 @@ namespace DevBrain.Functions.Tests.Auth.Services; /// : /// /// Defensive expiry check on every read using the injected (typically a FakeTimeProvider). -/// and are single-take under a lock — -/// concurrent callers get the record at most once. +/// is single-take under a lock. +/// leaves a short-lived replay marker so immediate +/// retries see the winning replacement token. /// /// /// @@ -241,10 +242,95 @@ public Task SaveRefreshAsync(DevBrainRefreshRecord refresh) } } + public Task RotateRefreshAsync( + string refreshToken, + string clientId, + string replacementRefreshToken, + TimeSpan replacementLifetime, + TimeSpan replayLifetime, + TimeSpan upstreamVaultLifetime) + { + lock (_lock) + { + ReadCallCount++; + if (!_refreshes.TryGetValue(refreshToken, out var record)) + { + return Task.FromResult(null); + } + + if (IsExpired(record.ExpiresAt)) + { + _refreshes.Remove(refreshToken); + return Task.FromResult(null); + } + + if (!string.Equals(record.ClientId, clientId, StringComparison.Ordinal)) + { + return Task.FromResult(null); + } + + if (record.IsReplayMarker) + { + if (string.IsNullOrEmpty(record.RotatedToRefreshToken) + || !TouchUpstreamTokenCore(record.UpstreamJti, upstreamVaultLifetime)) + { + return Task.FromResult(null); + } + + return Task.FromResult( + new RefreshRotationResult(record.UpstreamJti, record.RotatedToRefreshToken, IsReplay: true)); + } + + if (!TouchUpstreamTokenCore(record.UpstreamJti, upstreamVaultLifetime)) + { + return Task.FromResult(null); + } + + var now = _timeProvider.GetUtcNow(); + _refreshes[replacementRefreshToken] = new DevBrainRefreshRecord + { + RefreshToken = replacementRefreshToken, + ClientId = record.ClientId, + UpstreamJti = record.UpstreamJti, + CreatedAt = now, + ExpiresAt = now + replacementLifetime, + Ttl = (int)replacementLifetime.TotalSeconds, + }; + + _refreshes[refreshToken] = new DevBrainRefreshRecord + { + RefreshToken = refreshToken, + ClientId = record.ClientId, + UpstreamJti = record.UpstreamJti, + CreatedAt = record.CreatedAt, + ExpiresAt = now + replayLifetime, + RotatedAt = now, + RotatedToRefreshToken = replacementRefreshToken, + Ttl = (int)replayLifetime.TotalSeconds, + }; + + return Task.FromResult( + new RefreshRotationResult(record.UpstreamJti, replacementRefreshToken, IsReplay: false)); + } + } + // ---------------- Helpers ---------------- private bool IsExpired(DateTimeOffset expiresAt) => expiresAt <= _timeProvider.GetUtcNow(); + private bool TouchUpstreamTokenCore(string jti, TimeSpan lifetime) + { + if (!_upstreams.TryGetValue(jti, out var dto) || IsExpired(dto.ExpiresAt)) + { + _upstreams.Remove(jti); + return false; + } + + dto.ExpiresAt = _timeProvider.GetUtcNow() + lifetime; + dto.Ttl = (int)lifetime.TotalSeconds; + return true; + } + // Cheap clone so the caller can't mutate the stored copy. Uses the same JSON round-trip as // Cosmos does on the wire — ensures test expectations match production serialization surface. private static T Clone(T record) where T : class diff --git a/tests/DevBrain.Functions.Tests/Auth/Services/FakeOAuthStateStoreTests.cs b/tests/DevBrain.Functions.Tests/Auth/Services/FakeOAuthStateStoreTests.cs index 5777373..53673aa 100644 --- a/tests/DevBrain.Functions.Tests/Auth/Services/FakeOAuthStateStoreTests.cs +++ b/tests/DevBrain.Functions.Tests/Auth/Services/FakeOAuthStateStoreTests.cs @@ -154,6 +154,134 @@ await store.SaveRefreshAsync(new DevBrainRefreshRecord Assert.Equal("jti-new", again.UpstreamJti); } + [Fact] + public async Task RefreshToken_RotateAllowsBoundReplayAndExpiresMarker() + { + var (store, clock) = CreateStore(); + await store.SaveUpstreamTokenAsync(new UpstreamTokenRecord + { + Jti = "jti-old", + Envelope = new UpstreamTokenEnvelope("at", "rt", 0), + ExpiresAt = Epoch.AddHours(1), + }); + await store.SaveRefreshAsync(new DevBrainRefreshRecord + { + RefreshToken = "refresh-old", + ClientId = "abc-123", + UpstreamJti = "jti-old", + ExpiresAt = Epoch.AddDays(30), + }); + + var rotated = await store.RotateRefreshAsync( + "refresh-old", + "abc-123", + "refresh-new", + TimeSpan.FromDays(30), + TimeSpan.FromMinutes(5), + TimeSpan.FromDays(30)); + + Assert.NotNull(rotated); + Assert.False(rotated.IsReplay); + Assert.Equal("refresh-new", rotated.RefreshToken); + + var replay = await store.RotateRefreshAsync( + "refresh-old", + "abc-123", + "ignored-candidate", + TimeSpan.FromDays(30), + TimeSpan.FromMinutes(5), + TimeSpan.FromDays(30)); + + Assert.NotNull(replay); + Assert.True(replay.IsReplay); + Assert.Equal("refresh-new", replay.RefreshToken); + + clock.Advance(TimeSpan.FromMinutes(6)); + + var expiredReplay = await store.RotateRefreshAsync( + "refresh-old", + "abc-123", + "ignored-candidate", + TimeSpan.FromDays(30), + TimeSpan.FromMinutes(5), + TimeSpan.FromDays(30)); + + Assert.Null(expiredReplay); + } + + [Fact] + public async Task RefreshToken_RotateWrongClientDoesNotConsumeToken() + { + var (store, _) = CreateStore(); + await store.SaveUpstreamTokenAsync(new UpstreamTokenRecord + { + Jti = "jti-old", + Envelope = new UpstreamTokenEnvelope("at", "rt", 0), + ExpiresAt = Epoch.AddHours(1), + }); + await store.SaveRefreshAsync(new DevBrainRefreshRecord + { + RefreshToken = "refresh-old", + ClientId = "abc-123", + UpstreamJti = "jti-old", + ExpiresAt = Epoch.AddDays(30), + }); + + var attacker = await store.RotateRefreshAsync( + "refresh-old", + "different-client", + "attacker-refresh", + TimeSpan.FromDays(30), + TimeSpan.FromMinutes(5), + TimeSpan.FromDays(30)); + + Assert.Null(attacker); + + var legitimate = await store.RotateRefreshAsync( + "refresh-old", + "abc-123", + "refresh-new", + TimeSpan.FromDays(30), + TimeSpan.FromMinutes(5), + TimeSpan.FromDays(30)); + + Assert.NotNull(legitimate); + Assert.Equal("refresh-new", legitimate.RefreshToken); + } + + [Fact] + public async Task RefreshToken_RotateExtendsUpstreamVaultExpiry() + { + var (store, _) = CreateStore(); + await store.SaveUpstreamTokenAsync(new UpstreamTokenRecord + { + Jti = "jti-old", + Envelope = new UpstreamTokenEnvelope("at", "rt", 0), + ExpiresAt = Epoch.AddHours(1), + }); + await store.SaveRefreshAsync(new DevBrainRefreshRecord + { + RefreshToken = "refresh-old", + ClientId = "abc-123", + UpstreamJti = "jti-old", + ExpiresAt = Epoch.AddDays(30), + }); + + var rotated = await store.RotateRefreshAsync( + "refresh-old", + "abc-123", + "refresh-new", + TimeSpan.FromDays(30), + TimeSpan.FromMinutes(5), + TimeSpan.FromDays(30)); + + Assert.NotNull(rotated); + + var upstream = await store.GetUpstreamTokenAsync("jti-old"); + Assert.NotNull(upstream); + Assert.Equal(Epoch.AddDays(30), upstream!.ExpiresAt); + } + [Fact] public async Task UpstreamToken_RoundTripPreservesEnvelopeAndClaims() {