[PM-40673] Two Factor Remember Token Updates (Part 1) by rr-bw · Pull Request #8427 · bitwarden/server

rr-bw · 2026-09-23T01:25:27Z

🎟️ Tracking

PM-40673

PR Stack

auth/pm-40673/part-1 - Entity, Repository, and Command/Query updates
auth/pm-40673/part-2 - Add token issuance to login flow (and migrates from old-to-new tokens)
auth/pm-40673/part-3 - Updates call sites where we want to revoke 2FA remember tokens (admin 2FA reset, recovery code usage, removal of last 2FA method)

📔 Objective

Commit 1 - 239ad75

Adds a new C# entity (TwoFactorRememberToken) with a rotatable Stamp.
Adds a new repository interface (ITwoFactorRememberTokenRepository), defining methods that act on TwoFactorRememberToken row(s):
- Point lookup by (UserId, DeviceId)
- Upsert
- Bulk Stamp rotation
- Bulk delete

Commit 2 - 6d5f8ee

Lays the MSSQL foundation:
- src/Sql/ Table schema and View
- Migrator script
- Stored procedures (ReadByUserIdDeviceId, Save, DeleteExpired, UpdateManyStampsByUserId)
- Dapper repository implementation and DI wiring

Commit 3 - d463a2f

Adds Entity Framework infrastructure
- TwoFactorRememberTokenEntityTypeConfiguration.cs
- EF repository implementation
- Generated migration files

Commit 4 - 939a391

Adds TwoFactorRememberTokenable.cs

Commit 5 - 48aec51

Adds commands for issuing and revoking tokens, and a query for validating tokens

Commit 6 - 9130de8

Adds a job for deleting expired tokens.

📸 Screenshots

codecov · 2026-09-23T01:52:52Z

Codecov Report

❌ Patch coverage is 94.82759% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.55%. Comparing base (e6b10b5) to head (523502c).

Files with missing lines	Patch %	Lines
...h/Repositories/TwoFactorRememberTokenRepository.cs	88.31%	6 Missing and 3 partials ⚠️
src/Admin/Jobs/JobsHostedService.cs	14.28%	6 Missing ⚠️
...h/Repositories/TwoFactorRememberTokenRepository.cs	96.51%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8427      +/-   ##
==========================================
+ Coverage   67.12%   73.55%   +6.42%     
==========================================
  Files        2674     2685      +11     
  Lines      114478   114826     +348     
  Branches    10390    10420      +30     
==========================================
+ Hits        76847    84458    +7611     
+ Misses      35122    27742    -7380     
- Partials     2509     2626     +117

☔ 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.

github-actions · 2026-09-28T16:37:36Z

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Re-reviewed at 523502cbf. Since the last review, the DeleteExpiredTwoFactorRememberTokensJob trigger moved to Saturday 04:30 (Eastern, or local time when self-hosted), which no longer collides with DatabaseExpiredGrantsJob or DeleteUnverifiedOrganizationDomainsJob. ValidateTwoFactorRememberTokenQuery no longer checks whether 2FA is enabled or takes an organization parameter; it now validates only the token, the user binding, the security stamp, the device identifier, the row stamp and the row expiry, and the interface remarks say the 2FA-required decision belongs to the caller. The rest of the change set (entity, Dapper and EF repositories with their duplicate-key handling, the SSDT schema and the 2026-10-17_00 migration, the EF migrations and snapshots, the tokenable and its factory, the issue and revoke commands, and the DI wiring) was checked again, and no new issues were found.

mkincaid-bw

Overall a good PR from the DB side, with a couple minor changes requested.

mkincaid-bw · 2026-09-28T19:28:49Z

@@ -0,0 +1,57 @@
+CREATE PROCEDURE [dbo].[TwoFactorRememberToken_Save]


This type of Upsert is not Bitwarden's preferred methodology of performing an Insert/Update. This should be broken into a TwoFactorRememberToken_Create proc and a TwoFactorRememberToken_Update proc. See docs.

mkincaid-bw · 2026-09-28T19:36:00Z

@@ -0,0 +1,22 @@
+CREATE PROCEDURE [dbo].[TwoFactorRememberToken_DeleteExpired]


⛏️ This should probably be named TwoFactorRememberToken_DeleteManyExpired

mkincaid-bw · 2026-09-28T19:42:59Z

+    -- Deleted in batches so each statement holds few enough locks to avoid escalating to a table
+    -- lock, which would block logins while the sweep runs. The loop ends when a batch deletes
+    -- nothing.
+    DECLARE @BatchSize INT = 100


A batch size of 100 is the norm throughout the codebase. However, if you expect this proc to thousands of rows (or more) during each run, then @BatchSize should be raised to 1000.

mkincaid-bw

LGTM

enmande

Awesome! Thank you ✨
Two questions below: the migration date ordering, and the job trigger timing.
Just LMK on those, everything else looks great.

…kenQueryTests

…insert

…tion

…e as param

…pers.GenerateComb()

… is enabled for user

…Job to run at a new time to avoid multiple jobs running at same time

… parameter from ValidateTwoFactorRememberTokenQuery

aikido-pr-checks · 2026-10-21T19:49:05Z

+    SET NOCOUNT ON
+
+    SELECT
+        *


The TwoFactorRememberToken_ReadByUserIdDeviceId procedure selects *, making its returned columns fragile to view schema changes.

Show fix

Suggested change

*

[Id],

[UserId],

[DeviceId],

[Stamp],

[CreationDate],

[RevisionDate],

[ExpirationDate]

Details

✨ AI Reasoning
The query uses an explicit wildcard, so its returned columns can change when the view schema changes and its data dependencies are obscured.

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

aikido-pr-checks · 2026-10-21T19:49:06Z

+CREATE VIEW [dbo].[TwoFactorRememberTokenView]
+AS
+SELECT
+    *


The TwoFactorRememberTokenView selects *, so its exposed columns change implicitly when the underlying table schema changes.

Show fix

Suggested change

*

[Id],

[UserId],

[DeviceId],

[Stamp],

[CreationDate],

[RevisionDate],

[ExpirationDate]

Details

✨ AI Reasoning
The view exposes every table column through an explicit wildcard, coupling its output to future table schema changes.

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

aikido-pr-checks · 2026-10-21T19:49:08Z

+CREATE OR ALTER VIEW [dbo].[TwoFactorRememberTokenView]
+AS
+SELECT
+    *


The migration's TwoFactorRememberTokenView selects *, so its exposed columns change implicitly when the underlying table schema changes.

Show fix

Suggested change

*

[Id],

[UserId],

[DeviceId],

[Stamp],

[CreationDate],

[RevisionDate],

[ExpirationDate]

Details

✨ AI Reasoning
The migration defines a view with an explicit wildcard, making the view's exposed columns depend implicitly on the table schema.

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

aikido-pr-checks · 2026-10-21T19:49:15Z

+        Microsoft.Data.Sqlite.SqliteException lite => lite.SqliteErrorCode == 19
+            && lite.SqliteExtendedErrorCode is 1555 or 2067,


The lite exception filter mixes && with an or pattern on one line, making the duplicate-key condition harder to parse.

Show fix

Suggested change

Microsoft.Data.Sqlite.SqliteException lite => lite.SqliteErrorCode == 19

&& lite.SqliteExtendedErrorCode is 1555 or 2067,

Microsoft.Data.Sqlite.SqliteException lite =>

lite.SqliteErrorCode == 19 &&

lite.SqliteExtendedErrorCode is 1555 or 2067,

Details

✨ AI Reasoning
This condition combines a SQLite error-code check with an alternative pattern match. The mixed operators make it harder to scan the exact conditions under which the exception is treated as a duplicate-key violation.

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

github-code-quality Bot found potential problems Sep 23, 2026

View reviewed changes

rr-bw added this pull request to stack #8466 September 28, 2026 15:59

This was referenced Sep 28, 2026

[PM-40673] Two Factor Remember Token Updates (Part 2) #8428

Open

[PM-40673] Two Factor Remember Token Updates (Part 3) #8429

Open

rr-bw marked this pull request as ready for review September 28, 2026 16:36

rr-bw requested review from a team as code owners September 28, 2026 16:36

rr-bw requested a review from enmande September 28, 2026 16:36

claude Bot reviewed Sep 28, 2026

View reviewed changes

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

claude Bot reviewed Sep 28, 2026

View reviewed changes

Comment thread src/Core/Auth/Models/Business/Tokenables/TwoFactorRememberTokenable.cs Resolved

mkincaid-bw requested changes Sep 28, 2026

View reviewed changes

enmande reviewed Sep 28, 2026

View reviewed changes

rr-bw added ai-review Request a Claude code review t:feature Change Type - Feature Development labels Oct 2, 2026

rr-bw requested review from enmande and mkincaid-bw October 5, 2026 15:31

rr-bw removed the ai-review Request a Claude code review label Oct 6, 2026

mkincaid-bw previously approved these changes Oct 6, 2026

View reviewed changes

rr-bw added the ai-review Request a Claude code review label Oct 9, 2026

claude Bot reviewed Oct 9, 2026

View reviewed changes

Comment thread util/Migrator/DbScripts/2026-10-17_00_CreateTwoFactorRememberToken.sql Resolved

rr-bw dismissed mkincaid-bw’s stale review via 2a46609 October 9, 2026 16:49

mkincaid-bw previously approved these changes Oct 12, 2026

View reviewed changes

enmande reviewed Oct 14, 2026

View reviewed changes

rr-bw dismissed mkincaid-bw’s stale review via 627bdac October 17, 2026 11:02

rr-bw force-pushed the auth/pm-40673/part-1 branch from 627bdac to fb9f2c2 Compare October 17, 2026 11:04

claude Bot reviewed Oct 17, 2026

View reviewed changes

Comment thread util/MySqlMigrations/Migrations/20261017122324_CreateTwoFactorRememberToken.cs Resolved

rr-bw removed the ai-review Request a Claude code review label Oct 17, 2026

rr-bw force-pushed the auth/pm-40673/part-1 branch from 6a7c424 to 6d27724 Compare October 19, 2026 14:12

rr-bw added 22 commits October 21, 2026 14:48

[PM-40673 Part 1] add DeleteExpiredTwoFactorRememberTokensJob.cs

7dda6bf

[PM-40673 Part 1] link TODO comment to specific Jira ticket

d56f90b

[PM-40673 Part 1] remove prefixes from test comments

99e9ddc

[PM-40673 Part 1] rename stored procedure to _DeleteManyExpired

5614074

[PM-40673 Part 1] split upsert (_Save) into _Create and _Update

39edfbc

[PM-40673 Part 1] update expiration time comparison check

220186e

[PM-40673 Part 1] use FakeTimeProvider in ValidateTwoFactorRememberTo…

c601599

…kenQueryTests

[PM-40673 Part 1] add a TwoFactorRememberTokenableFactory

161cd50

[PM-40673 Part 1] update UpsertAsync to prevent concurrent duplicate …

afec27c

…insert

[PM-40673 Part 1] increase DeleteManyExpired batch size to 1000

89181ff

[PM-40673 Part 1] restore snapshots and re-generate migrations

edda633

[PM-40673 Part 1] update trigger to be unique for remember token dele…

4859929

…tion

[PM-40673 Part 1] remove unused row variable

1c99585

[PM-40673 Part 1] update rotate stamp methods to pass in revision tim…

e87bf08

…e as param

[PM-40673 Part 1] use CombGuid.Generate() instead of obselete CoreHel…

2717b9c

…pers.GenerateComb()

[PM-40673 Part 1] run dotnet format

500696c

[PM-40673 Part 1] restore snapshots and re-generate migrations

f9de219

[PM-40673 Part 1] add check for org-required-2FA when checking if 2FA…

b1cd61d

… is enabled for user

[PM-40673 Part 1] remove comment, update comment

e4c1ea9

[PM-40673 Part 1] move scheduled DeleteExpiredTwoFactorRememberTokens…

1983d85

…Job to run at a new time to avoid multiple jobs running at same time

[PM-40673 Part 1] remove redundant 2FA-enabled check and organization…

a27f3cd

… parameter from ValidateTwoFactorRememberTokenQuery

[PM-40673 Part 1] bump migrations

523502c

rr-bw force-pushed the auth/pm-40673/part-1 branch from 6d27724 to 523502c Compare October 21, 2026 19:48

aikido-pr-checks Bot reviewed Oct 21, 2026

View reviewed changes

rr-bw merged commit ae47d2b into main Oct 21, 2026
77 checks passed

rr-bw deleted the auth/pm-40673/part-1 branch October 21, 2026 20:43

quexten mentioned this pull request Nov 2, 2026

Use the factory clock in token expiry checks #8793

Merged

		@@ -0,0 +1,57 @@
		CREATE PROCEDURE [dbo].[TwoFactorRememberToken_Save]

		@@ -0,0 +1,22 @@
		CREATE PROCEDURE [dbo].[TwoFactorRememberToken_DeleteExpired]

-        *
+        [Id],
+        [UserId],
+        [DeviceId],
+        [Stamp],
+        [CreationDate],
+        [RevisionDate],
+        [ExpirationDate]

		Microsoft.Data.Sqlite.SqliteException lite => lite.SqliteErrorCode == 19
		&& lite.SqliteExtendedErrorCode is 1555 or 2067,

Conversation

rr-bw commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

📸 Screenshots

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 Bitwarden Claude Code Review

Uh oh!

Uh oh!

Uh oh!

mkincaid-bw left a comment

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

rr-bw Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

rr-bw Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

rr-bw Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkincaid-bw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

enmande left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aikido-pr-checks Bot Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

rr-bw commented Sep 23, 2026 •

edited

Loading

codecov Bot commented Sep 23, 2026 •

edited

Loading

github-actions Bot commented Sep 28, 2026 •

edited

Loading

aikido-pr-checks Bot Oct 21, 2026 •

edited

Loading

aikido-pr-checks Bot Oct 21, 2026 •

edited

Loading

aikido-pr-checks Bot Oct 21, 2026 •

edited

Loading