[PM-42318] Add the collection reads behind PAM's access rules

[Hinton]

🎟️ Tracking

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

📔 Objective

Two independent reads, one per commit.

1. Who manages a collection

Bitwarden can already answer which collections may this user manage: callers pull GetManyByUserIdAsync and filter on Manage, then fold in the members who manage everything by role rather than by assignment. BulkCollectionAuthorizationHandler.CanManageCollectionsAsync and CollectionAuthorizationHandler both work that way.

Nothing answers the inverse — given a collection, who manages it. Anything that has to reach the people responsible for a collection, rather than check one person's access to one collection, has no read to call. Composing the existing one means loading every member of the organization and asking the per-user question once each: the wrong shape for the question, and it scales with the organization instead of with the collection's access list.

ICollectionRepository.GetManagingUserIdsAsync returns the distinct confirmed members who can Manage, by any of the four routes:

Route to Manage	Resolved from
Direct assignment	CollectionUser.Manage
Through a group	CollectionGroup.Manage + GroupUser
By role	Owners and Admins, when the organization sets AllowAdminAccessToAllCollectionItems
By permission	Custom members holding EditAnyCollection

MSSQL resolves it in Collection_ReadManagingUserIds as a union of those sources; EF reads the Permissions column in memory, because the JSON accessors the procedure uses are not portable across providers. The procedure is deliberately shaped like its neighbour CollectionCipher_ReadUserIdsByCollectionIds, which unions the same three assignment sources to resolve access rather than management.

2. Which collections an access rule governs

The collection browser renders a Controlled access column, but a collection row has nothing to put in it. No collection response model carried the association, so a client could only learn that a collection is governed by reading the organization's access rules itself.

A member can do that. A provider cannot: GET organizations/{orgId}/access-rules requires MemberRequirement, deliberately — providers manage an organization's billing and configuration, but access rules gate who can lease credentials out of it, which is not theirs to read. Providers do browse the client organization's collection list, so their column renders and every cell stays empty however the client is written.

HasEnabledAccessRule surfaces the fact on the collection instead, on the responses that feed sync and the organization listing.

It is derived, not the stored association. Collection.AccessRuleId records which rule governs a collection, but a rule that is switched off gates nothing — returning the association alone would start badging collections whose rule is disabled. Each read path joins AccessRule and reports whether it is enabled: MSSQL in Collection_ReadByUserId, Collection_ReadByIdWithPermissions and Collection_ReadSharedCollectionsByOrganizationIdWithPermissions; EF through an EXISTS subquery in UserCollectionDetailsQuery and CollectionAdminDetailsQuery, since Collection has no AccessRule navigation to traverse.

A boolean rather than the rule id, deliberately: the id is useless to a client, and it avoids handing providers the rule identity the access-rules endpoint withholds from them. Nothing writes the column and no request model gains a field — SetAccessRuleAssociationsAsync remains the single writer of the association.

Testing

CollectionRepositoryGetManagingUserIdsTests and CollectionRepositoryHasEnabledAccessRuleTests, plus every other ~CollectionRepository integration test: 153 passing on SQL Server, SQLite and Postgres (204 with the MySQL/MariaDB rows skipped — no local instance). Api.Test 187 and Core.Test 219 on ~Collection.

All three providers earn their place here:

• SQL Server is the only one running the stored procedures; the others take the EF paths.
• Postgres is the only one that groups by the EXISTS subquery in translated SQL — SQLite takes the client-side .GroupBy branch instead.
• Postgres also caught a defect the other two hid. CreateTestUserAsync names a user {identifier}-{guid}, so an identifier over 13 characters overflows User.Name. SQL Server passes the name to a procedure parameter declared NVARCHAR(50) and truncates silently on assignment rather than raising, and SQLite does not enforce declared lengths at all, so the test passed on both while inserting a name it had quietly cut short.

Reviewer notes

• Neither read has a consumer on main yet. The first is called by a notifier on pam/uat that pushes to a collection's approvers; the second is paired with a clients change. Both are the PAM-free halves, split out so the collection layer reviews separately from PAM. The same situation as the AccessRule tables and procedures already on main.
• The manage predicate is now stated a fourth time, alongside BulkCollectionAuthorizationHandler.CanManageCollectionsAsync, CollectionAuthorizationHandler, and (on pam/uat) ApproverCollectionAccessQuery. Centralizing it is worth doing, but not here: all three existing statements are user-first and cannot answer the collection-first question without the per-member loop above.
• The two bare-Collection CollectionAccessDetailsResponseModel constructors — create/update responses and the provider fallbacks — cannot compute HasEnabledAccessRule and leave it false. Documented on the property; no vault list reads those responses.
• Collection_ReadByUserId needed its columns qualified: joining AccessRule makes Id, OrganizationId and Name ambiguous against the UserCollectionDetails table function.
• Collection_ReadManagingUserIds's migration adds a CREATE OR ALTER PROCEDURE and no table or column DDL, so it carries no existence guard. Neither migration changes the EF model, so there is no snapshot diff.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.91%. Comparing base (ac309aa) to head (2f88669).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8243      +/-   ##
==========================================
+ Coverage   63.29%   68.91%   +5.61%     
==========================================
  Files        2401     2410       +9     
  Lines      104043   104516     +473     
  Branches     9426     9457      +31     
==========================================
+ Hits        65857    72027    +6170     
+ Misses      35930    30112    -5818     
- Partials     2256     2377     +121     

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

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed two additive collection reads: a new ICollectionRepository.GetManagingUserIdsAsync (MSSQL Collection_ReadManagingUserIds plus an EF equivalent) and a derived HasEnabledAccessRule flag threaded through three collection read paths and their API response models. Verified dual-ORM parity between the stored procedures and the EF/LINQ reimplementations, that the manage predicate matches BulkCollectionAuthorizationHandler.CanUpdateCollectionAsync (EditAnyCollection unconditional, Owner/Admin gated on AllowAdminAccessToAllCollectionItems), that JSON_VALUE(..., '$.editAnyCollection') matches the camelCase serialization used by CoreHelpers.ClassToJsonData, and that the new LEFT JOIN [dbo].[AccessRule] is on a primary key so it cannot fan out the existing GROUP BY results. The migration is procedure-only (CREATE OR ALTER), so it is idempotent without existence guards, needs no EF snapshot change, and the added result column is backwards compatible in both rolling-deployment directions since Dapper ignores unmapped columns.

Code Review Details

No findings. Notes from validation that did not rise to findings:

• The unguarded CoreHelpers.LoadClassFromJsonData<Permissions>(ou.Permissions) in the EF path diverges from the procedure's ISJSON(...) = 1 guard, but it matches the established pattern in CurrentContextOrganization and OrganizationUserUserDetails.
• The bare-Collection CollectionAccessDetailsResponseModel constructors leave HasEnabledAccessRule false on create/update responses; the provider collection listing goes through GetManySharedByOrganizationIdWithPermissionsAsync → CollectionAdminDetails, so the targeted provider path is populated. Already documented on the property.

[r-tome]

The EF query needs optimization. If an organization has for example 10k confirmed users then it has to load all of them in memory to filter them and would even have data it doesn't need -- like the Permissions column that is a NVARCHAR(MAX).

[r-tome]

This is loading all confirmed members from the database and then filters everything in memory. This could be more efficient we filtered it all in the database. We can use IQueryable to build the queries. For example:

var confirmed = dbContext.OrganizationUsers
  .Where(ou => ou.OrganizationId == organizationId
      && ou.Status == OrganizationUserStatusType.Confirmed
      && ou.UserId != null);
var userIdsWithDirectManage = confirmed
      .Where(ou => dbContext.CollectionUsers.Any(cu =>
          cu.CollectionId == collectionId && cu.Manage && cu.OrganizationUserId == ou.Id))
      .Select(ou => ou.UserId!.Value);
  var userIdsWithGroupManage = confirmed
      .Where(ou => dbContext.GroupUsers.Any(gu => gu.OrganizationUserId == ou.Id
          && dbContext.CollectionGroups.Any(cg =>
              cg.CollectionId == collectionId && cg.Manage && cg.GroupId == gu.GroupId)))
      .Select(ou => ou.UserId!.Value);
var managerUserIds = await userIdsWithDirectManage.Union(userIdsWithGroupManage).ToListAsync();

[r-tome]

Permissions are being selected for all users but only Custom users need them.
Using the IQueryable example:

var customUsers = await confirmed
    .Where(ou => ou.Type == OrganizationUserType.Custom)
    .Select(ou => new { ou.UserId, ou.Permissions })
    .ToListAsync();

[Hinton]

Seems EF 10 gives us json querying capability directly in the db layer. But I guess until then we're stuck with looping over all custom.

[r-tome]

Looks good! Nice work. Thanks for making the changes I requested.