[PM-22450] Bump Collection.RevisionDate on edits and access changes by r-tome · Pull Request #7380 · bitwarden/server

r-tome · 2026-04-02T15:40:31Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-22450

📔 Objective

Collection.RevisionDate was not updated when a collection was edited or when its access changed. Clients rely on this date to know when to re-sync. This PR bumps it across all code paths: collection edits via UpdateCollectionCommand, user/group assignment changes, and bulk access operations -- in both stored procedures and EF repositories.

…d update corresponding tests. Adjust tests to verify correct RevisionDate assignment during collection updates.

codecov · 2026-04-02T15:48:46Z

Codecov Report

❌ Patch coverage is 93.25843% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.92%. Comparing base (ebbf6dd) to head (d339457).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
...Console/Repositories/OrganizationUserRepository.cs	50.00%	2 Missing and 1 partial ⚠️
...Console/Repositories/OrganizationUserRepository.cs	92.10%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7380      +/-   ##
==========================================
+ Coverage   58.56%   62.92%   +4.36%     
==========================================
  Files        2063     2063              
  Lines       91198    91279      +81     
  Branches     8125     8132       +7     
==========================================
+ Hits        53406    57439    +4033     
+ Misses      35881    31841    -4040     
- Partials     1911     1999      +88

☔ View full report in Codecov by Sentry.
📢 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-04-02T15:51:58Z

Checkmarx One – Scan Summary & Details – 0979bb56-2b75-49f3-a18d-8c03c5bcd5ae

New Issues (2)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1558	details Method at line 1558 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector
2		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1385	details Method at line 1385 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector

Fixed Issues (2)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	~~CSRF~~	/src/Api/AdminConsole/Public/Controllers/MembersController.cs: 269
	~~CSRF~~	/src/Api/Vault/Controllers/CiphersController.cs: 287

eliykat

The ticket says we need to bump the revision date in these scenarios:

Whenever a collection has its name updated (including when it’s nested)
Whenever a collection has its user/group assignment updated
Whenever a user has their access to a collection updated
Whenever a group has their access to a collection updated

I'm guessing this covers 1 and 2. What about 3 and 4? (That said - this PR is still good to merge.)

…cess updates. Update ICollectionRepository and its implementations to accept revision date parameter. Modify stored procedure to update collection revision dates accordingly. Add tests to verify correct behavior of access creation and revision date updates.

… affected collections during group creation and updates. Enhance integration tests to verify that collection revision dates are correctly updated when groups are created or modified.

…ionUserRepository and related stored procedures. Add integration tests to ensure revision dates are correctly bumped during organization user creation and updates.

github-actions · 2026-04-03T15:04:18Z

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR bumps Collection.RevisionDate across all code paths where collection access is modified — collection edits, user/group assignment changes, and bulk access operations. The implementation is consistent between Dapper/SQL stored procedures and EF repositories. Test coverage is thorough with both unit tests (using FakeTimeProvider) and integration tests for create, update, and bulk operations.

Code Review Details

🎨 : internal set → set on Group.RevisionDate and OrganizationUser.RevisionDate may be unnecessary — test assemblies already have InternalsVisibleTo and no production code outside Core sets these properties
- src/Core/AdminConsole/Entities/Group.cs:19, src/Core/AdminConsole/Entities/OrganizationUser.cs:68

r-tome · 2026-04-03T15:09:29Z

The ticket says we need to bump the revision date in these scenarios:

Whenever a collection has its name updated (including when it’s nested)

Whenever a collection has its user/group assignment updated

Whenever a user has their access to a collection updated

Whenever a group has their access to a collection updated

I'm guessing this covers 1 and 2. What about 3 and 4? (That said - this PR is still good to merge.)

Great callout! I went even further and also updated when user is invited with collection access.

eliykat

Looking good.

src/Sql/dbo/Stored Procedures/Group_UpdateWithCollections.sql

src/Sql/dbo/Stored Procedures/OrganizationUser_CreateManyWithCollectionsGroups.sql

src/Sql/dbo/Stored Procedures/OrganizationUser_UpdateWithCollections.sql

src/Infrastructure.EntityFramework/AdminConsole/Repositories/GroupRepository.cs

src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs

....IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs

...tionTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserCreateTests.cs

…updated

… updating RevisionDate of affected collections. This change improves readability and maintainability by consolidating the logic for identifying affected collections in Group_UpdateWithCollections and OrganizationUser_UpdateWithCollections procedures.

…ocedure to accept RevisionDate parameter for updating affected collections. Update OrganizationUserRepository to utilize the provided RevisionDate when available, ensuring accurate revision date management during organization user operations.

…on script to utilize temporary table for CollectionUser data insertion. This change improves performance and maintains consistency in updating RevisionDate for affected collections.

…from created OrganizationUsers when updating affected collections. This change enhances the accuracy of revision date management across the repository.

…roup and Collection repositories. Update assertions to compare RevisionDate directly, improving accuracy in revision date management during tests.

…eWithCollections

…ndling of RevisionDate. Updated collection filtering logic to use HashSet for efficiency and ensured that affected collections are filtered by OrganizationId, enhancing accuracy in revision date management.

The Dapper repositories use a System.Text.Json serialize/deserialize round-trip to build *WithCollections objects. System.Text.Json silently skips properties with non-public setters, so RevisionDate was reverting to DateTime.UtcNow instead of preserving the value set in C#.

src/Sql/dbo/Stored Procedures/OrganizationUser_CreateManyWithCollectionsGroups.sql

…on script to improve the logic for updating RevisionDate. The update now uses INNER JOINs to ensure accurate filtering of collections based on OrganizationId and CollectionUser data, enhancing the precision of revision date management.

r-tome · 2026-04-06T14:47:02Z

src/Core/AdminConsole/Entities/Group.cs

    [MaxLength(300)]
    public string? ExternalId { get; set; }
    public DateTime CreationDate { get; internal set; } = DateTime.UtcNow;
-    public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow;


The repository serializes/deserializes the entity for the sproc call, and the internal set prevented RevisionDate from being set, causing the value to be lost. Making it public fixes that.

I also saw that only some entities have an internal set. I'm not sure why we have that inconsistency.

mkincaid-bw

Mainly formatting nit-picks and a comment on joins

mkincaid-bw · 2026-04-07T00:19:50Z

src/Sql/dbo/Stored Procedures/Collection_CreateOrUpdateAccessForMany.sql

+    IF @RevisionDate IS NOT NULL
+    BEGIN
+		-- Bump the revision date on all affected collections
+        UPDATE C


⛏️ UPDATE statements should be broken out on separate lines per the style guide

mkincaid-bw · 2026-04-07T00:19:56Z

src/Sql/dbo/Stored Procedures/Group_CreateWithCollections.sql

        [Id] IN (SELECT [Id] FROM [AvailableCollectionsCTE])

+    -- Bump RevisionDate on all affected collections
+    UPDATE C


⛏️ UPDATE statements should be broken out on separate lines per the style guide

mkincaid-bw · 2026-04-07T00:20:11Z

src/Sql/dbo/Stored Procedures/Group_UpdateWithCollections.sql


+    -- Bump RevisionDate on all affected collections
+    ;WITH [AffectedCollectionsCTE] AS (
+        SELECT [Id] FROM @Collections


⛏️ SELECT statements should be broken out on separate lines per the style guide

mkincaid-bw · 2026-04-07T00:24:50Z

src/Sql/dbo/Stored Procedures/OrganizationUser_CreateManyWithCollectionsGroups.sql

+    -- Bump RevisionDate on all affected collections
+    IF @RevisionDate IS NOT NULL
+    BEGIN
+        UPDATE C


⛏️ UPDATE statements should be broken out on separate lines per the style guide

Also, the join to the OrganizationUser table can cause a performance bottleneck depending on how SQL server decides to execute the query. Since the list of CollectionIds are in the #CollectionUserData table, I think the join is unnecessary, and we can probably just use that.

UPDATE C SET C.[RevisionDate] = @RevisionDate FROM [dbo].[Collection] C INNER JOIN #CollectionUserData CUD ON CUD.[CollectionId] = C.[Id]

mkincaid-bw · 2026-04-07T00:25:11Z

src/Sql/dbo/Stored Procedures/OrganizationUser_CreateWithCollections.sql

        [Id] IN (SELECT [Id] FROM [AvailableCollectionsCTE])
+
+    -- Bump RevisionDate on all affected collections
+    UPDATE C


⛏️ UPDATE statements should be broken out on separate lines per the style guide

mkincaid-bw · 2026-04-07T00:25:29Z

src/Sql/dbo/Stored Procedures/OrganizationUser_UpdateWithCollections.sql

+
+    -- Bump RevisionDate on all affected collections
+    ;WITH [AffectedCollectionsCTE] AS (
+        SELECT [Id] FROM @Collections


⛏️ SELECT statements should be broken out on separate lines per the style guide

mkincaid-bw · 2026-04-07T00:25:37Z

src/Sql/dbo/Stored Procedures/OrganizationUser_UpdateWithCollections.sql

+        FROM [dbo].[CollectionUser] CU
+        WHERE CU.[OrganizationUserId] = @Id
+    )
+    UPDATE C


⛏️ UPDATE statements should be broken out on separate lines per the style guide

eliykat

Very minor things only.

src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs

src/Infrastructure.EntityFramework/AdminConsole/Repositories/GroupRepository.cs

src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs

....IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs

…updated

…he supplied parameter is empty

r-tome · 2026-04-07T12:30:23Z

@mkincaid-bw Thanks for catching all the style guide violations! All fixed now.

…updated

claude · 2026-04-08T09:26:15Z

src/Core/AdminConsole/Entities/Group.cs

    public string? ExternalId { get; set; }
    public DateTime CreationDate { get; internal set; } = DateTime.UtcNow;
-    public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow;
+    public DateTime RevisionDate { get; set; } = DateTime.UtcNow;


🎨 SUGGESTED: The internal set → set change here (and on OrganizationUser.RevisionDate) widens the access modifier. Since Core.csproj already declares InternalsVisibleTo for Core.Test, Infrastructure.IntegrationTest, and Identity.IntegrationTest, the test assemblies can already use internal set. The production callers in this PR (EF/Dapper repos) only read Group.RevisionDate and OrganizationUser.RevisionDate — they don't set them.

Consider keeping internal set to preserve the encapsulation that prevents accidental modification from assemblies outside Core. CreationDate on both entities still uses internal set, so this change also introduces an inconsistency between the two date properties.

That is wrong, Claude. I had to make the set public because otherwise on

server/src/Infrastructure.Dapper/AdminConsole/Repositories/GroupRepository.cs

Line 145 in 13eecfd

var objWithCollections = JsonSerializer.Deserialize<GroupWithCollections>(JsonSerializer.Serialize(obj))!;

it fails to populate RevisionDate with the given value and defaults to DateTime.UtcNow. I noticed this because the tests would fail.

mkincaid-bw

LGTM

…updated

sonarqubecloud · 2026-04-09T09:56:24Z

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
61.9% Duplication on New Code

See analysis details on SonarQube Cloud

Fix UpdateCollectionCommand to set RevisionDate using TimeProvider an…

8d67715

…d update corresponding tests. Adjust tests to verify correct RevisionDate assignment during collection updates.

r-tome added needs-qa ai-review-vnext Request a Claude code review using the vNext workflow labels Apr 2, 2026

r-tome marked this pull request as ready for review April 2, 2026 15:43

r-tome requested a review from a team as a code owner April 2, 2026 15:43

r-tome requested a review from eliykat April 2, 2026 15:43

eliykat previously approved these changes Apr 3, 2026

View reviewed changes

r-tome added 6 commits April 3, 2026 15:31

Update GroupRepository and stored procedures to bump RevisionDate for…

59a90c6

… affected collections during group creation and updates. Enhance integration tests to verify that collection revision dates are correctly updated when groups are created or modified.

Implement revision date updates for affected collections in Organizat…

4ee23dd

…ionUserRepository and related stored procedures. Add integration tests to ensure revision dates are correctly bumped during organization user creation and updates.

Update database migration script

0a7c65f

Update migration script summary

e878ae2

Refactor OrganizationUserReplaceTests to create collection first

48d9e7c

r-tome dismissed eliykat’s stale review via 48d9e7c April 3, 2026 15:03

r-tome requested a review from a team as a code owner April 3, 2026 15:03

r-tome changed the title ~~[PM-22450] Fix Collection.RevisionDate not updating when a collection is edited~~ [PM-22450] Bump Collection.RevisionDate on edits and access changes Apr 3, 2026

r-tome requested a review from eliykat April 3, 2026 19:39

eliykat requested changes Apr 3, 2026

View reviewed changes

Merge branch 'main' into ac/pm-22450/collection-revision-date-is-not-…

2cc7800

…updated

r-tome marked this pull request as draft April 6, 2026 10:12

r-tome added 6 commits April 6, 2026 11:13

Refactor OrganizationUser_CreateManyWithCollectionsGroups and migrati…

4d80849

…on script to utilize temporary table for CollectionUser data insertion. This change improves performance and maintains consistency in updating RevisionDate for affected collections.

Refactor OrganizationUserRepository to consistently use RevisionDate …

7206101

…from created OrganizationUsers when updating affected collections. This change enhances the accuracy of revision date management across the repository.

Refactor tests to ensure consistent handling of RevisionDate across G…

1f9ae7b

…roup and Collection repositories. Update assertions to compare RevisionDate directly, improving accuracy in revision date management during tests.

Restore BOM in Group_UpdateWithCollections and OrganizationUser_Updat…

0e81b49

…eWithCollections

r-tome added 3 commits April 6, 2026 12:38

Refactor GroupRepository and OrganizationUserRepository to improve ha…

1bd19cb

…ndling of RevisionDate. Updated collection filtering logic to use HashSet for efficiency and ensured that affected collections are filtered by OrganizationId, enhancing accuracy in revision date management.

Bump migration script date

dabb672

claude bot reviewed Apr 6, 2026

View reviewed changes

src/Sql/dbo/Stored Procedures/OrganizationUser_CreateManyWithCollectionsGroups.sql Show resolved Hide resolved

r-tome commented Apr 6, 2026

View reviewed changes

r-tome marked this pull request as ready for review April 6, 2026 15:05

r-tome requested a review from eliykat April 6, 2026 15:05

mkincaid-bw requested changes Apr 7, 2026

View reviewed changes

eliykat requested changes Apr 7, 2026

View reviewed changes

r-tome added 3 commits April 7, 2026 10:47

Merge branch 'main' into ac/pm-22450/collection-revision-date-is-not-…

7b39459

…updated

Fix sprocs styling

c3cf999

Added early return to OrganizationUserRepository.CreateManyAsync if t…

7de27a9

…he supplied parameter is empty

r-tome requested review from eliykat and mkincaid-bw April 7, 2026 12:02

eliykat approved these changes Apr 7, 2026

View reviewed changes

Merge branch 'main' into ac/pm-22450/collection-revision-date-is-not-…

a308ef2

…updated

claude bot reviewed Apr 8, 2026

View reviewed changes

mkincaid-bw approved these changes Apr 8, 2026

View reviewed changes

r-tome removed the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 9, 2026

Merge branch 'main' into ac/pm-22450/collection-revision-date-is-not-…

d339457

…updated

r-tome removed the needs-qa label Apr 10, 2026

r-tome merged commit 3dd72f6 into main Apr 10, 2026
70 of 72 checks passed

r-tome deleted the ac/pm-22450/collection-revision-date-is-not-updated branch April 10, 2026 06:27

Conversation

r-tome commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

Uh oh!

codecov bot commented Apr 2, 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 Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eliykat left a comment • 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!

github-actions bot commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 Bitwarden Claude Code Review

Uh oh!

r-tome commented Apr 3, 2026

Uh oh!

eliykat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eliykat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

r-tome commented Apr 7, 2026

Uh oh!

claude bot Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkincaid-bw left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 9, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

r-tome commented Apr 2, 2026 •

edited

Loading

codecov bot commented Apr 2, 2026 •

edited

Loading

github-actions bot commented Apr 2, 2026 •

edited

Loading

eliykat left a comment •

edited

Loading

github-actions bot commented Apr 3, 2026 •

edited

Loading