From b41a6699d78e77c60c8dbaafe07be1964799b2b7 Mon Sep 17 00:00:00 2001 From: alexyao2015 Date: Thu, 24 Sep 2026 19:08:15 -0400 Subject: [PATCH 1/4] Save auth grants on changes --- .../Auth/Repositories/GrantRepository.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs index dd958e0c6e8c..85f401440f21 100644 --- a/src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs @@ -84,6 +84,7 @@ public async Task SaveAsync(IGrant obj) { gObj.Id = existingGrant.Id; dbContext.Entry(existingGrant).CurrentValues.SetValues(gObj); + await dbContext.SaveChangesAsync(); } else { From a67fb1f95ba81f354021c4eebcd07489947c5322 Mon Sep 17 00:00:00 2001 From: alexyao2015 Date: Sat, 26 Sep 2026 15:51:03 -0400 Subject: [PATCH 2/4] Add GrantRepository SaveAsync integration tests Cover all three SaveAsync paths against every configured database provider: insert, update, and the non-Grant type guard. The update test is the regression test for the missing SaveChangesAsync on the update branch: IdentityServer stores refresh tokens with ReUse + Sliding semantics, so every token refresh re-saves the grant under the same key with a reset creation time and a new expiration. When that update is dropped, the row is frozen at issuance and continuously active clients are forced into a full re- authentication once the original window lapses. --- .../Auth/Repositories/GrantRepositoryTests.cs | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs diff --git a/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs b/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs new file mode 100644 index 000000000000..48981e31bd24 --- /dev/null +++ b/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs @@ -0,0 +1,115 @@ +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.Repositories; +using Xunit; + +namespace Bit.Infrastructure.IntegrationTest.Auth.Repositories; + +public class GrantRepositoryTests +{ + // ------------------------------------------------------------------------------------------- + // Test helpers + // ------------------------------------------------------------------------------------------- + + private const string RefreshTokenType = "refresh_token"; + + /// + /// Truncates a to second-level precision (preserving ). + /// Round-tripped values can lose sub-second precision on some providers (e.g. Dapper binds + /// DateTime parameters as legacy datetime with ~3.33ms granularity), so all + /// values written by these tests are truncated to whole seconds to keep equality asserts exact. + /// + private static DateTime TruncateToSecond(DateTime value) => + new(value.Ticks - value.Ticks % TimeSpan.TicksPerSecond, value.Kind); + + private static Grant CreateTestGrant( + string key = null, + string subjectId = null, + string clientId = "mobile", + DateTime? creationDate = null, + DateTime? expirationDate = null, + string data = null) + { + return new Grant + { + Key = key ?? Guid.NewGuid().ToString(), + Type = RefreshTokenType, + SubjectId = subjectId ?? Guid.NewGuid().ToString(), + ClientId = clientId, + CreationDate = creationDate ?? TruncateToSecond(DateTime.UtcNow), + ExpirationDate = expirationDate, + Data = data ?? "{}", + }; + } + + // ------------------------------------------------------------------------------------------- + // SaveAsync + // ------------------------------------------------------------------------------------------- + + [DatabaseTheory, DatabaseData] + public async Task SaveAsync_InputIsNotTypeGrant_ThrowsArgumentException( + IGrantRepository sutRepository) + { + // GrantItem is an IGrant (the Cosmos shape) but not the Grant the SQL repositories persist + var invalidGrant = new GrantItem { Key = Guid.NewGuid().ToString() }; + + var exception = await Assert.ThrowsAsync( + () => sutRepository.SaveAsync(invalidGrant)); + + Assert.Equal("obj", exception.ParamName); + } + + [DatabaseTheory, DatabaseData] + public async Task SaveAsync_GrantIsNull_GrantCreated( + IGrantRepository sutRepository) + { + var issuedAt = TruncateToSecond(DateTime.UtcNow); + var grant = CreateTestGrant( + creationDate: issuedAt, + expirationDate: issuedAt.AddDays(30)); + + await sutRepository.SaveAsync(grant); + + var stored = await sutRepository.GetByKeyAsync(grant.Key); + Assert.NotNull(stored); + Assert.Equal(grant.Key, stored!.Key); + } + + /// + /// Saving a grant whose key already exists must update the stored row. This is the + /// refresh-token slide: IdentityServer stores refresh tokens with + /// RefreshTokenUsage = ReUse, so every refresh re-saves the grant under the same key + /// with a reset creation time and a new expiration. A repository that drops that update + /// freezes the row at issuance and forces clients into a full re-authentication once the + /// original window lapses, no matter how active they are. + /// + [DatabaseTheory, DatabaseData] + public async Task SaveAsync_GrantIsNotNull_GrantExpiryUpdated( + IGrantRepository sutRepository) + { + // Arrange — grant issued with a 30-day window + var issuedAt = TruncateToSecond(DateTime.UtcNow); + var subjectId = Guid.NewGuid().ToString(); + var grant = CreateTestGrant( + subjectId: subjectId, + creationDate: issuedAt, + expirationDate: issuedAt.AddDays(30)); + await sutRepository.SaveAsync(grant); + + // Act — 29 days later the client refreshes: same key, slid window, re-serialized data + var refreshedAt = issuedAt.AddDays(29); + await sutRepository.SaveAsync(CreateTestGrant( + key: grant.Key, + subjectId: subjectId, + creationDate: refreshedAt, + expirationDate: refreshedAt.AddDays(30), + data: "{\"CreationTime\":\"refreshed\"}")); + + // Assert — the stored row carries the slid dates and data, not the issuance values + var stored = await sutRepository.GetByKeyAsync(grant.Key); + Assert.NotNull(stored); + Assert.Equal(refreshedAt, stored!.CreationDate); + Assert.Equal(refreshedAt.AddDays(30), stored.ExpirationDate); + Assert.Equal("{\"CreationTime\":\"refreshed\"}", stored.Data); + } +} From fc0084378accb06dd63b64c3797b77e27f492703 Mon Sep 17 00:00:00 2001 From: alexyao2015 Date: Sat, 26 Sep 2026 18:25:44 -0400 Subject: [PATCH 3/4] Fix EF GrantRepository.GetManyAsync and DeleteManyAsync Three defects on the EF grant repository (PostgreSQL, MySQL, SQLite self-hosted), all dormant because nothing in Bitwarden's IdentityServer wiring exercises these paths today: - GetManyAsync threw InvalidCastException on every call: List cannot cast to ICollection because ICollection is invariant. Materialize with ToList() like the Dapper implementation. - GetManyAsync and DeleteManyAsync treated null filter arguments as "the column must be NULL" (EF's null-semantics translation of plain equality), while the Grant_Read and Grant_Delete stored procedures treat NULL parameters as wildcards. For Bitwarden data, where SubjectId/ClientId/Type are always populated, a partial filter such as subject+type with a null clientId matched nothing on EF providers but everything on MSSQL. Align the LINQ predicates to the stored procedures' wildcard semantics, which is also what Duende's PersistedGrantFilter produces. Integration tests cover SaveAsync (type guard, insert, update), GetManyAsync (non-null filtering with distractors and empty results, null filters as wildcards), and DeleteManyAsync (null client filter deletes across clients while other types and subjects survive). --- .../Auth/Repositories/GrantRepository.cs | 18 +-- .../Auth/Repositories/GrantRepositoryTests.cs | 112 +++++++++++++++++- 2 files changed, 120 insertions(+), 10 deletions(-) diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs index 85f401440f21..457c75a153f6 100644 --- a/src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs @@ -31,10 +31,10 @@ public async Task DeleteManyAsync(string subjectId, string sessionId, string cli { var dbContext = GetDatabaseContext(scope); await dbContext.Grants.Where(g => - g.SubjectId == subjectId && - g.ClientId == clientId && - g.SessionId == sessionId && - g.Type == type).ExecuteDeleteAsync(); + (subjectId == null || g.SubjectId == subjectId) && + (clientId == null || g.ClientId == clientId) && + (sessionId == null || g.SessionId == sessionId) && + (type == null || g.Type == type)).ExecuteDeleteAsync(); } } @@ -57,13 +57,13 @@ public async Task> GetManyAsync(string subjectId, string ses { var dbContext = GetDatabaseContext(scope); var query = from g in dbContext.Grants - where g.SubjectId == subjectId && - g.ClientId == clientId && - g.SessionId == sessionId && - g.Type == type + where (subjectId == null || g.SubjectId == subjectId) && + (clientId == null || g.ClientId == clientId) && + (sessionId == null || g.SessionId == sessionId) && + (type == null || g.Type == type) select g; var grants = await query.ToListAsync(); - return (ICollection)grants; + return grants.ToList(); } } diff --git a/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs b/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs index 48981e31bd24..d5cef88df8d8 100644 --- a/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs @@ -12,6 +12,7 @@ public class GrantRepositoryTests // ------------------------------------------------------------------------------------------- private const string RefreshTokenType = "refresh_token"; + private const string AuthorizationCodeType = "authorization_code"; /// /// Truncates a to second-level precision (preserving ). @@ -25,7 +26,9 @@ private static DateTime TruncateToSecond(DateTime value) => private static Grant CreateTestGrant( string key = null, string subjectId = null, + string sessionId = null, string clientId = "mobile", + string type = RefreshTokenType, DateTime? creationDate = null, DateTime? expirationDate = null, string data = null) @@ -33,8 +36,9 @@ private static Grant CreateTestGrant( return new Grant { Key = key ?? Guid.NewGuid().ToString(), - Type = RefreshTokenType, + Type = type, SubjectId = subjectId ?? Guid.NewGuid().ToString(), + SessionId = sessionId, ClientId = clientId, CreationDate = creationDate ?? TruncateToSecond(DateTime.UtcNow), ExpirationDate = expirationDate, @@ -112,4 +116,110 @@ await sutRepository.SaveAsync(CreateTestGrant( Assert.Equal(refreshedAt.AddDays(30), stored.ExpirationDate); Assert.Equal("{\"CreationTime\":\"refreshed\"}", stored.Data); } + + // ------------------------------------------------------------------------------------------- + // GetManyAsync + // ------------------------------------------------------------------------------------------- + + /// + /// Non-null filters apply on every column and the results come back as an + /// ICollection<IGrant>: two matching rows are returned, distractors differing in + /// client, subject, or type are excluded, and a query with no matches returns an empty + /// collection rather than throwing. + /// + [DatabaseTheory, DatabaseData] + public async Task GetManyAsync_WithNonNullFilters_ReturnsOnlyMatchingGrants( + IGrantRepository sutRepository) + { + // Arrange — two matching grants plus distractors differing in client, subject, and type + var subjectId = Guid.NewGuid().ToString(); + var sessionId = Guid.NewGuid().ToString(); + await sutRepository.SaveAsync(CreateTestGrant(subjectId: subjectId, sessionId: sessionId, clientId: "mobile")); + await sutRepository.SaveAsync(CreateTestGrant(subjectId: subjectId, sessionId: sessionId, clientId: "mobile")); + await sutRepository.SaveAsync(CreateTestGrant(subjectId: subjectId, sessionId: sessionId, clientId: "desktop")); + await sutRepository.SaveAsync(CreateTestGrant(sessionId: sessionId, clientId: "mobile")); + await sutRepository.SaveAsync(CreateTestGrant( + subjectId: subjectId, sessionId: sessionId, clientId: "mobile", type: AuthorizationCodeType)); + + // Act + var grants = await sutRepository.GetManyAsync(subjectId, sessionId, "mobile", RefreshTokenType); + + // Assert — exactly the two matching rows + Assert.Equal(2, grants.Count); + Assert.All(grants, g => Assert.Equal(subjectId, g.SubjectId)); + Assert.All(grants, g => Assert.Equal("mobile", g.ClientId)); + Assert.All(grants, g => Assert.Equal(RefreshTokenType, g.Type)); + + // A query matching nothing returns an empty collection + Assert.Empty(await sutRepository.GetManyAsync( + Guid.NewGuid().ToString(), sessionId, "mobile", RefreshTokenType)); + } + + /// + /// A null filter argument means "do not filter on this field", matching the Grant_Read + /// stored procedure's @param IS NULL OR column = @param wildcard pattern and the + /// partial filters Duende's PersistedGrantFilter produces. The previous EF predicate + /// translated nulls to "the column must be NULL", which matched nothing for Bitwarden data + /// (SubjectId/ClientId/Type are always populated) — this covers the null-client and + /// null-session wildcards in a single arrange/act/assert so a regression on either fails loudly. + /// + [DatabaseTheory, DatabaseData] + public async Task GetManyAsync_WithNullFilters_TreatsNullAsWildcard( + IGrantRepository sutRepository) + { + // Arrange — the subject's grants span two clients and three session-id states + // (one session, another session, none), plus another subject's grant + var subjectId = Guid.NewGuid().ToString(); + var sessionId = Guid.NewGuid().ToString(); + await sutRepository.SaveAsync(CreateTestGrant(subjectId: subjectId, sessionId: sessionId, clientId: "mobile")); + await sutRepository.SaveAsync(CreateTestGrant( + subjectId: subjectId, sessionId: Guid.NewGuid().ToString(), clientId: "mobile")); + await sutRepository.SaveAsync(CreateTestGrant(subjectId: subjectId, clientId: "desktop")); + await sutRepository.SaveAsync(CreateTestGrant(sessionId: sessionId, clientId: "mobile")); + + // Act — no client or session filter + var grants = await sutRepository.GetManyAsync(subjectId, null, null, RefreshTokenType); + + // Assert — all three of the subject's grants return regardless of client or session id + Assert.Equal(3, grants.Count); + Assert.All(grants, g => Assert.Equal(subjectId, g.SubjectId)); + } + + // ------------------------------------------------------------------------------------------- + // DeleteManyAsync + // ------------------------------------------------------------------------------------------- + + /// + /// Null filters are wildcards and non-null filters still apply, matching the Grant_Delete + /// stored procedure: revoking a subject's refresh tokens with no client filter deletes them + /// across all clients, while the subject's other grant types and other subjects' grants survive. + /// + [DatabaseTheory, DatabaseData] + public async Task DeleteManyAsync_WithNullClientFilter_DeletesAcrossClientsButOnlyMatchingTypeAndSubject( + IGrantRepository sutRepository) + { + // Arrange — the subject's refresh tokens on two clients, an authorization code, and + // another subject's grant + var subjectId = Guid.NewGuid().ToString(); + var sessionId = Guid.NewGuid().ToString(); + var mobileGrant = CreateTestGrant(subjectId: subjectId, sessionId: sessionId, clientId: "mobile"); + var desktopGrant = CreateTestGrant(subjectId: subjectId, sessionId: sessionId, clientId: "desktop"); + var authorizationCode = CreateTestGrant( + subjectId: subjectId, sessionId: sessionId, type: AuthorizationCodeType); + var otherSubjectGrant = CreateTestGrant(sessionId: sessionId, clientId: "mobile"); + await sutRepository.SaveAsync(mobileGrant); + await sutRepository.SaveAsync(desktopGrant); + await sutRepository.SaveAsync(authorizationCode); + await sutRepository.SaveAsync(otherSubjectGrant); + + // Act — revoke the subject's refresh tokens, no client filter + await sutRepository.DeleteManyAsync(subjectId, null, null, RefreshTokenType); + + // Assert — both refresh tokens deleted across clients; the authorization code and the + // other subject's grant survive + Assert.Null(await sutRepository.GetByKeyAsync(mobileGrant.Key)); + Assert.Null(await sutRepository.GetByKeyAsync(desktopGrant.Key)); + Assert.NotNull(await sutRepository.GetByKeyAsync(authorizationCode.Key)); + Assert.NotNull(await sutRepository.GetByKeyAsync(otherSubjectGrant.Key)); + } } From d6785dca3b7623fad463ebbaf5138e775b5836d5 Mon Sep 17 00:00:00 2001 From: alexyao2015 Date: Wed, 30 Sep 2026 17:39:01 -0400 Subject: [PATCH 4/4] Fix GrantRepositoryTests file encoding dotnet format's CHARSET check requires C# files to be UTF-8 with BOM (.editorconfig); the test file was written without the BOM and failed dotnet format --verify-no-changes in CI. --- .../Auth/Repositories/GrantRepositoryTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs b/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs index d5cef88df8d8..d2ffabe8f893 100644 --- a/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/Auth/Repositories/GrantRepositoryTests.cs @@ -1,4 +1,4 @@ -using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Repositories; using Xunit;