[PM-44104] Save auth grant on change by alexyao2015 · Pull Request #8450 · bitwarden/server

alexyao2015 · 2026-09-24T23:11:47Z

📔 Objective

The authorization grants were not being updated if they exist. This means for any non MSSQL db, users will experience timeouts after a set period of time. In practice, a user's client will be forcibly kicked out and forced to perform a relogin even if they are actively using it.

The database entry was updated, but this change was not saved.

bitwarden-bot · 2026-09-24T23:12:44Z

Thank you for your contribution! We've added this to our internal tracking system for review.
ID: PM-44104
Link: https://bitwarden.atlassian.net/browse/PM-44104

Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process.

ike-kottlowski

The most elegant of changes are always single lines.

We would like for you to add integration tests please.

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.

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<Grant> cannot cast to ICollection<IGrant> because ICollection<T> is invariant. Materialize with ToList<IGrant>() 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).

codecov · 2026-09-30T18:47:57Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.83%. Comparing base (ecad9ae) to head (432a88b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8450      +/-   ##
==========================================
+ Coverage   74.76%   74.83%   +0.07%     
==========================================
  Files        2750     2750              
  Lines      117617   117618       +1     
  Branches    10781    10781              
==========================================
+ Hits        87937    88024      +87     
+ Misses      27022    26937      -85     
+ Partials     2658     2657       -1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

ike-kottlowski · 2026-10-02T15:57:19Z

I will update when QA has had a chance to take a look on our end.

aikido-pr-checks · 2026-11-04T22:03:14Z

            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();


The subjectId predicate combines || and && on one line, increasing the logical complexity of the filter.

Show fix

Suggested change

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();

var grants = dbContext.Grants.Where(g =>

(subjectId == null || g.SubjectId == subjectId) &&

(clientId == null || g.ClientId == clientId) &&

(sessionId == null || g.SessionId == sessionId) &&

(type == null || g.Type == type));

await grants.ExecuteDeleteAsync();

Details

✨ AI Reasoning
The line combines a null check and a field comparison with two different logical operators. Readers must track both the grouped alternatives and the outer conjunction.

_{Reply on this comment to give feedback or ignore the issue.}
_{More info}

CLAassistant · 2026-11-05T21:55:42Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ alexyao2015
✅ ike-kottlowski
❌ yashwalter12
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

ike-kottlowski · 2026-11-06T14:32:08Z

@alexyao2015 great news, the change passed QA! Thank you for finding this bug and iterating on the PR with me. 🚀 🚀

alexyao2015 requested a review from a team as a code owner September 24, 2026 23:11

alexyao2015 requested a review from rr-bw September 24, 2026 23:11

bitwarden-bot changed the title ~~Save auth grant on change~~ [PM-44104] Save auth grant on change Sep 24, 2026

bitwarden-bot added community-pr needs-qa labels Sep 24, 2026

Save auth grants on changes

b41a669

alexyao2015 force-pushed the fix/login-timeout branch from f6e6cc5 to b41a669 Compare September 24, 2026 23:14

ike-kottlowski requested changes Sep 25, 2026

View reviewed changes

Comment thread src/Infrastructure.EntityFramework/Auth/Repositories/GrantRepository.cs Resolved

alexyao2015 added 2 commits September 29, 2026 10:57

rr-bw removed their request for review September 30, 2026 12:02

ike-kottlowski previously approved these changes Sep 30, 2026

View reviewed changes

Fix GrantRepositoryTests file encoding

d6785dc

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.

alexyao2015 dismissed ike-kottlowski’s stale review via d6785dc September 30, 2026 22:03

ike-kottlowski added the t:bugfix Change Type - Bugfix label Oct 2, 2026

Merge branch 'main' into fix/login-timeout

4652d36

aikido-pr-checks Bot reviewed Nov 4, 2026

View reviewed changes

ike-kottlowski added the ci:signed-build Runs fork PR build workflows with secrets (including artifact signing) label Nov 4, 2026

github-actions Bot removed the ci:signed-build Runs fork PR build workflows with secrets (including artifact signing) label Nov 4, 2026

ike-kottlowski removed the needs-qa label Nov 6, 2026

ike-kottlowski approved these changes Nov 6, 2026

View reviewed changes

Merge branch 'main' into fix/login-timeout

432a88b

ike-kottlowski enabled auto-merge (squash) November 6, 2026 14:37

ike-kottlowski merged commit 9e5b3ec into bitwarden:main Nov 6, 2026
25 of 42 checks passed

Navigation Menu

Repository navigation

[PM-44104] Save auth grant on change - #8450

[PM-44104] Save auth grant on change#8450
ike-kottlowski merged 6 commits into
bitwarden:mainfrom
alexyao2015:fix/login-timeout

alexyao2015 commented Sep 24, 2026

Uh oh!

bitwarden-bot commented Sep 24, 2026

Uh oh!

ike-kottlowski left a comment

Uh oh!

Uh oh!

codecov Bot commented Sep 30, 2026 •

edited

Loading

Uh oh!

ike-kottlowski commented Oct 2, 2026

Uh oh!

aikido-pr-checks Bot Nov 4, 2026

Uh oh!

CLAassistant commented Nov 5, 2026 •

edited

Loading

Uh oh!

ike-kottlowski commented Nov 6, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

alexyao2015 commented Sep 24, 2026

📔 Objective

Uh oh!

bitwarden-bot commented Sep 24, 2026

Uh oh!

ike-kottlowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ike-kottlowski commented Oct 2, 2026

Uh oh!

aikido-pr-checks Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ike-kottlowski commented Nov 6, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Sep 30, 2026 •

edited

Loading

CLAassistant commented Nov 5, 2026 •

edited

Loading