[PM-40336] Access Rules: API by Hinton · Pull Request #7983 · bitwarden/server

Hinton · 2026-07-14T17:34:43Z

🎟️ Tracking

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

📔 Objective

Add the commercial CRUD API for org-scoped PAM Access Rules, consuming the
domain and persistence layer from #7981. This PR is stacked on #7981 — review
and merge that first; GitHub will retarget this PR to main once it lands.

All endpoints are gated behind the pm-37044-pam-v-0 (FeatureFlagKeys.Pam)
feature flag, so nothing is reachable in production until PAM v0 ships. Access-rule
data (name, description, conditions) is organization configuration metadata, not
Vault Data — no zero-knowledge surface is touched.

What's included

API — bitwarden_license/src/Services/Pam

CRUD endpoints on the AccessRule group: GET (list), GET {id}, POST,
PUT {id}, DELETE {id}, all org-scoped and feature-flag gated.
Request/response models (AccessRuleRequestModel, AccessRuleResponseModel)
and the condition models: human_approval and ip_allowlist.
Create/Update/Delete commands behind interfaces; AccessRuleValidator
validating the conditions JSON (max 10 conditions, per-type rules; empty list is
vacuously valid and still routes access through PAM for audit).
Command and validator unit tests.

📸 Screenshots

N/A — backend only.

github-actions · 2026-07-14T17:35:31Z

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the commercial CRUD API for org-scoped PAM Access Rules — the endpoint handler, request/response models, condition models, create/update/delete commands, and the shared write validator, plus their unit tests. Security-sensitive paths are sound: every endpoint is gated behind Policies.Application, the FeatureFlagKeys.Pam feature flag, and per-request org membership/admin checks, with rules scoped to orgId and 404s used to avoid leaking existence. Input validation (conditions JSON shape, condition count cap, CIDR parsing, name uniqueness, collection ownership/conflict) is thorough and well covered by the command and validator tests. Access-rule data is organization configuration metadata, not Vault Data, so no zero-knowledge surface is affected.

Code Review Details

No blocking findings.

Notes considered and not flagged:

The handler's EnsureMemberAsync/EnsureAdminAsync authorization is already discussed in an open thread where the author proposes lifting it into RequireAuthorization; not duplicating here.
DI registration uses AddScoped/AddSingleton rather than TryAdd*, matching the existing convention in ServiceCollectionExtensions.
packages.lock.json changes reflect the internal Pam.Domain/Data project references added to Pam.csproj (transitive lock regeneration) — no new external dependency, no AppSec approval needed.

codecov · 2026-07-14T18:43:13Z

Codecov Report

❌ Patch coverage is 58.86525% with 116 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.79%. Comparing base (2c74d20) to head (e13bb46).

Files with missing lines	Patch %	Lines
...i/Endpoints/Handlers/AccessRuleEndpointsHandler.cs	0.00%	48 Missing ⚠️
...Pam/Api/Models/Response/AccessRuleResponseModel.cs	0.00%	39 Missing ⚠️
...s/Pam/Api/Models/Request/AccessRuleRequestModel.cs	0.00%	18 Missing ⚠️
...e/src/Services/Pam/Services/AccessRuleValidator.cs	85.41%	4 Missing and 3 partials ⚠️
...s/Pam/Api/Models/Response/PamDateTimeExtensions.cs	0.00%	2 Missing ⚠️
.../Services/Pam/Services/AccessRuleWriteValidator.cs	95.65%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##           pam/access-rule    #7983      +/-   ##
===================================================
- Coverage            67.29%   62.79%   -4.51%     
===================================================
  Files                 2306     2314       +8     
  Lines               100426   100689     +263     
  Branches              9028     9052      +24     
===================================================
- Hits                 67582    63223    -4359     
- Misses               30562    35277    +4715     
+ Partials              2282     2189      -93

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

Hinton · 2026-07-27T09:30:30Z

+    private async Task EnsureMemberAsync(Guid orgId)
+    {
+        if (!await currentContext.OrganizationUser(orgId))
+        {
+            throw new NotFoundException();
+        }
+    }
+
+    private async Task EnsureAdminAsync(Guid orgId)
+    {
+        if (!await currentContext.OrganizationAdmin(orgId) && !await currentContext.OrganizationOwner(orgId))
+        {
+            throw new NotFoundException();
+        }
+    }


We should move this into RequireAuthorization (the minimal API variant of Authorize attribute). However since MemberRequirement is in Api we can't use it in PAM.

@eliykat Any interest in lifting this out into it's own server library?

eliykat

AC was requested for review but it doesn't look like it's needed anymore. Let me know if I've misunderstood.

Agreed that we'll extract the authorization logic in #8095.

…models

Remove the time_of_day condition kind along with the AccessWeekday enum and its JSON converter, which existed only to type the weekday tokens inside a time window. Access rules now expose human_approval and ip_allowlist; a document that still carries a time_of_day entry is rejected as an unknown kind.

The create and update commands each carried their own copy of the same checks: name required, a positive maximum when extensions are allowed, the conditions document, name uniqueness within the organization, and the collection lookup that confirms every requested collection exists, belongs to the organization, and is not already governed by another rule. Move all of it into AccessRuleWriteValidator, which takes the id of the rule being updated, or null when creating. Both places where the two paths differ reduce to a comparison against that id: an update excludes itself from the uniqueness check and may keep the collections it already governs, and with a null id the same expressions give a create its stricter behaviour, where any governed collection conflicts. The validator returns the deduplicated collection ids so callers do not normalize them a second time. The commands keep only what is theirs. Neither needs ICollectionRepository any more, and update keeps its existence guard and the plain AccessRule it maps for persistence. Update now resolves the rule before judging the payload, so editing a rule that does not exist, or belongs to another organization, is a 404 rather than a 400 from a field check. The command tests assert the validator is never reached in those cases to hold that order in place, and the shared rules are covered once in AccessRuleWriteValidatorTests, including the create and update asymmetries.

….csproj Pam.csproj now references Pam.Domain directly for the AccessRule command and validator implementations, so consumers of the Pam service need the reference recorded in their own lock files too. Force-evaluated a full-solution restore to bring every lock file back in sync.

The access-rule commands now take ICollectionRepository for the collection association write, matching the method's new home after review feedback on #7981.

Hinton requested a review from a team as a code owner July 14, 2026 17:34

Hinton mentioned this pull request Jul 14, 2026

[PM-40336] Access Rules: domain, persistence & schema #7981

Open

Hinton marked this pull request as draft July 14, 2026 17:36

Hinton force-pushed the pam/access-rule-api branch 2 times, most recently from 0351a28 to 37a5507 Compare July 14, 2026 18:06

Hinton force-pushed the pam/access-rule-api branch from 37a5507 to a85b172 Compare July 14, 2026 18:14

Hinton force-pushed the pam/access-rule-api branch from a85b172 to 9c09687 Compare July 14, 2026 18:16

Hinton force-pushed the pam/access-rule-api branch 2 times, most recently from 56c6cac to 44b46da Compare July 16, 2026 10:04

github-code-quality Bot found potential problems Jul 17, 2026

View reviewed changes

Comment thread bitwarden_license/src/Services/Pam/Services/AccessRuleValidator.cs Dismissed

Comment thread src/Infrastructure.Dapper/Pam/Repositories/AccessRuleRepository.cs Dismissed

Comment thread src/Infrastructure.EntityFramework/Pam/Models/AccessRule.cs Dismissed

Hinton force-pushed the pam/access-rule-api branch 2 times, most recently from c17c469 to ccf6444 Compare July 17, 2026 13:45

Hinton added the t:feature Change Type - Feature Development label Jul 27, 2026

Hinton commented Jul 27, 2026

View reviewed changes

Hinton marked this pull request as ready for review July 27, 2026 12:08

Hinton force-pushed the pam/access-rule-api branch from e5f25d5 to 801a80f Compare July 27, 2026 13:38

Hinton requested review from a team as code owners July 27, 2026 15:22

Hinton requested a review from BTreston July 27, 2026 15:22

Hinton force-pushed the pam/access-rule-api branch from 801a80f to ca963d2 Compare July 27, 2026 15:22

Hinton force-pushed the pam/access-rule-api branch from ca963d2 to 77d93e9 Compare July 27, 2026 15:53

Hinton force-pushed the pam/access-rule-api branch from 77d93e9 to 9a6165c Compare July 27, 2026 16:32

Hinton mentioned this pull request Jul 30, 2026

[PM-41272] Extract organization authorization into a new library #8095

Open

Hinton force-pushed the pam/access-rule-api branch from 9a6165c to d6deeaa Compare July 30, 2026 12:03

eliykat reviewed Jul 31, 2026

View reviewed changes

Hinton added 7 commits July 31, 2026 16:57

Implement AccessRule endpoints handler, commands, validator, and API …

a671c15

…models

Add AccessRule command and validator tests

b46482f

Update comments

d8cf693

Follow the SetAccessRuleAssociations move onto ICollectionRepository

e13bb46

The access-rule commands now take ICollectionRepository for the collection association write, matching the method's new home after review feedback on #7981.

Hinton force-pushed the pam/access-rule-api branch from d6deeaa to e13bb46 Compare July 31, 2026 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PM-40336] Access Rules: API - #7983

[PM-40336] Access Rules: API#7983
Hinton wants to merge 7 commits into
pam/access-rulefrom
pam/access-rule-api

Hinton commented Jul 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 14, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hinton Jul 27, 2026 •

edited

Loading

Uh oh!

eliykat left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Hinton commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

What's included

📸 Screenshots

Uh oh!

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

Uh oh!

🤖 Bitwarden Claude Code Review

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hinton Jul 27, 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!

eliykat left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Hinton commented Jul 14, 2026 •

edited

Loading

github-actions Bot commented Jul 14, 2026 •

edited

Loading

codecov Bot commented Jul 14, 2026 •

edited

Loading

Hinton Jul 27, 2026 •

edited

Loading