Auth/Innovation/PM-4517 - Device Management - Add Last Activity Date#7302
Merged
JaredSnider-Bitwarden merged 46 commits intomainfrom Apr 16, 2026
Merged
Conversation
…d response models Adds the LastActivityDate nullable DateTime property to the Device entity, IDeviceRepository interface (BumpLastActivityDateByIdAsync and BumpLastActivityDateByIdentifierAsync), DeviceAuthDetails DTO, DeviceResponseModel, DeviceAuthRequestResponseModel, and the DevicesLastActivityDate feature flag key in Constants.
…e guard Adds IBumpDeviceLastActivityDateCommand and IDeviceLastActivityCacheService interfaces with their implementations. The cache service uses the persistent keyed IDistributedCache (Cosmos DB in cloud, SQL Server in self-hosted) with a 48h TTL to guard against redundant DB writes within the same calendar day. Moves device DI registration into a consolidated AddDeviceServices() extension.
…QL migration Adds LastActivityDate DATETIME2 column to the Device table. Updates Device_Create and Device_Update stored procedures. Adds Device_BumpLastActivityDateById and Device_BumpLastActivityDateByIdentifier stored procedures with a CAST AS DATE guard as a fallback against redundant writes when the application-layer cache is unavailable.
…ions Implements BumpLastActivityDateByIdAsync and BumpLastActivityDateByIdentifierAsync in both Dapper (via stored procedures) and EF (via ExecuteUpdateAsync with a date-level guard). Adds EF migrations for Postgres, SQLite, and MySQL.
Wires IBumpDeviceLastActivityDateCommand into BaseRequestValidator (login path, keyed on device.Id) and CustomTokenRequestValidator (refresh token path, keyed on device identifier from subject claims). Both call sites are feature-flagged behind DevicesLastActivityDate.
JaredSnider-Bitwarden
added
the
ai-review-vnext
…ceService Device services are not user features — co-locating them with IDeviceService in AddBaseServices is more cohesive than nesting them inside AddUserServices.
Contributor
|
New Issues (4)Checkmarx found the following issues in this Pull Request
Fixed Issues (24)Great job! The following issues were fixed in this Pull Request
|
Contributor
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed 42 changed files adding a Code Review DetailsNo issues found. The implementation is clean and well-documented across all layers — stored procedures, EF repositories, cache service, command, and validator integrations are consistent and correct. Error handling appropriately swallows bump failures with warning logs to avoid impacting login/refresh flows. The 48h cache TTL with date-string comparison correctly handles the edge case of bumps near midnight. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7302 +/- ##
==========================================
+ Coverage 58.75% 63.27% +4.51%
==========================================
Files 2071 2074 +3
Lines 91248 91391 +143
Branches 8129 8140 +11
==========================================
+ Hits 53613 57825 +4212
+ Misses 35721 31565 -4156
- Partials 1914 2001 +87 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
… match LastActivityDate column position
…nc guard conditions
…ce, and happy-path cases
…eature flag sites
…disabled test for refresh path
…o remove duplicate field in derived class
…ross-user collisions
The Device table's unique constraint is (UserId, Identifier), not Identifier alone,
so two users can share the same device identifier (e.g. account switching in a browser).
Scoping the cache key to device:last-activity:{userId}:{identifier} ensures that a cache
hit for one user never suppresses a DB write for another.
Also adds userId to BumpByIdAsync signature and reorders params to be consistent with
BumpByIdentifierAsync(string identifier, Guid userId).
… and add happy-path test Renames BumpDeviceLastActivityForRefreshAsync to TryBumpDeviceLastActivityForRefreshAsync to signal the swallow-on-error intent. Moves the try-catch to wrap the entire method body, including GetSubjectId() which can throw InvalidOperationException, so no exception can escape and disrupt token refresh. Also moves the XML doc comment to RecordActivityForInstallation where it belongs, and adds a happy-path test verifying BumpByIdentifierAsync is called with the correct identifier and userId.
…onsistent timestamp Avoids a minor inconsistency where the WHERE filter and SET clause could evaluate DateTime.UtcNow at slightly different moments, aligning behavior with the SQL stored procedures which use a single @RevisionDate parameter.
…vent regressions Device_Update previously overwrote LastActivityDate unconditionally, meaning any unrelated device update (push token rotation, trust changes, deactivation) could silently regress a recently-bumped value. COALESCE preserves the existing DB value when NULL is passed, while still allowing callers to set it in the same write by passing a non-NULL value. The EF ReplaceAsync override applies the same semantics via IsModified = false. Integration test added to cover the preserve-on-null behaviour across all DB providers.
JaredSnider-Bitwarden
removed
the
ai-review-vnext
enmande
previously approved these changes
Apr 1, 2026
addisonbeck
previously approved these changes
Apr 3, 2026
…add-last-activity-date + DeviceAuthDetails merge conflict fix
JaredSnider-Bitwarden
dismissed stale reviews from addisonbeck, mkincaid-bw, and enmande
via
66960ee
JaredSnider-Bitwarden
requested review from
addisonbeck,
enmande and
mkincaid-bw
|
enmande
approved these changes
Apr 16, 2026
JaredSnider-Bitwarden
deleted the
auth/pm-4517/devices-add-last-activity-date
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
Epic: https://bitwarden.atlassian.net/browse/PM-4516
Individual Tasks:
Clients PR: bitwarden/clients#19784
📔 Objective
We want to add a new last activity date column on our client's device management page, and we want to bump that date on login and successful refresh token usage. However, due to the volume of requests, we have to use a cache and SQL to guard against excessive writes to the DB.
File Changes by Layer
Database
src/Sql/dbo/Tables/Device.sql— AddedLastActivityDate DATETIME2(7) NULLcolumnsrc/Sql/dbo/Stored Procedures/Device_Create.sql— Added@LastActivityDateparam (defaultNULLfor backwards compatibility)src/Sql/dbo/Stored Procedures/Device_Update.sql— Added@LastActivityDateparam; uses aCASEexpression to ensureLastActivityDateonly moves forward — guards against bothNULLpassthrough and stale non-null overwrites from concurrent requestssrc/Sql/dbo/Stored Procedures/Device_UpdateLastActivityDateById.sql— New SP — updatesLastActivityDateby device ID, at most once per calendar daysrc/Sql/dbo/Stored Procedures/Device_UpdateLastActivityDateByIdentifierUserId.sql— New SP — updatesLastActivityDateby(UserId, Identifier), at most once per calendar daysrc/Sql/dbo/Auth/Stored Procedures/Device_ReadActiveWithPendingAuthRequestsByUserId.sql— Updated to selectLastActivityDatein result setutil/Migrator/DbScripts/2026-04-14_00_AddDeviceLastActivityDate.sql— SQL Server migration (idempotent)util/{MySql,Postgres,Sqlite}Migrations/— EF migrations for MySQL, PostgreSQL, SQLiteEntity & Repository
src/Core/Entities/Device.cs— AddedLastActivityDatepropertysrc/Core/Repositories/IDeviceRepository.cs— AddedBumpLastActivityDateByIdAsyncandBumpLastActivityDateByIdentifierAndUserIdAsyncsrc/Infrastructure.Dapper/Repositories/DeviceRepository.cs— Implemented both bump methods (Dapper/SQL Server)src/Infrastructure.EntityFramework/Repositories/DeviceRepository.cs— Implemented both bump methods (EF); addedReplaceAsyncoverride to preserveLastActivityDatewhennulltest/Infrastructure.IntegrationTest/.../DeviceRepositoryTests.cs— Integration tests:ReplaceAsyncnull-preservation,BumpLastActivityDateByIdentifierAndUserIdAsynchappy-path and userId scopingCommands & Services
src/Core/Auth/UserFeatures/Devices/Interfaces/IBumpDeviceLastActivityDateCommand.cs— New interface —BumpAsync(login path) andBumpByIdentifierAndUserIdAsync(refresh token path)src/Core/Auth/UserFeatures/Devices/Interfaces/IDeviceLastActivityCacheService.cs— New interface — distributed cache check/write to limit bump writes to once per device per daysrc/Core/Auth/UserFeatures/Devices/BumpDeviceLastActivityDateCommand.cs— Implementation — checks cache before writing to DBsrc/Core/Auth/UserFeatures/Devices/DeviceLastActivityCacheService.cs— Implementation — composite(userId, identifier)cache key scoped to the persistent distributed cachesrc/Core/Auth/UserFeatures/Devices/DeviceServiceCollectionExtensions.cs— DI registration for new servicessrc/SharedWeb/Utilities/ServiceCollectionExtensions.cs— WiresAddDeviceServices()into base service registrationsrc/Core/Constants.cs— AddedFeatureFlagKeys.DevicesLastActivityDatetest/Core.Test/.../BumpDeviceLastActivityDateCommandTests.cs— Unit tests for command (cache hit/miss, correct args)test/Core.Test/.../DeviceLastActivityCacheServiceTests.cs— Unit tests for cache service (key scoping, date comparison)Auth / Request Validators
src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs— CallsBumpAsyncon successful loginsrc/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs— CallsTryBumpDeviceLastActivityForRefreshAsync(swallows errors) on token refreshsrc/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs— InjectsIBumpDeviceLastActivityDateCommandsrc/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs— InjectsIBumpDeviceLastActivityDateCommandtest/Identity.Test/.../BaseRequestValidatorTests.cs— Updated for newBumpAsyncsignature; added happy-path assertiontest/Identity.Test/.../CustomTokenRequestValidatorTests.cs— Tests forTryBumpDeviceLastActivityForRefreshAsyncincluding happy-path and error-swallowingtest/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs— Updated to injectIBumpDeviceLastActivityDateCommandResponse Models
src/Api/Models/Response/DeviceResponseModel.cs— ExposesLastActivityDatesrc/Core/Auth/Models/Api/Response/DeviceAuthRequestResponseModel.cs— ExposesLastActivityDatesrc/Core/Auth/Models/Data/DeviceAuthDetails.cs— IncludesLastActivityDatein device auth details query