Experiment: pam-poc#7797
Draft
abergs wants to merge 42 commits into
Draft
Conversation
Adds LeasingEnabled (bool) and LeasingPolicy (JSON) columns to Collection, gated by the FeatureFlagKeys.Pam feature flag. Includes: - Schema: SSDT table + sprocs update, dated MSSQL migration with COL_LENGTH guards and CREATE OR ALTER for the affected Collection_* sprocs, and EF migrations for Postgres, MySQL, and SQLite. - Domain: new src/Core/PrivilegedAccessManagement/ namespace with policy DTOs (human_approval, ip_allowlist, time_of_day, all_of) and the LeasingPolicyValidator service. - Commands: CreateCollectionCommand and UpdateCollectionCommand validate the policy when the flag is on and clear the leasing fields when it is off. - API: request/response models surface the two new fields; leasing flows through the existing collection create/update endpoints (the dedicated /leasing endpoint from the design doc is intentionally not introduced). - Tests: LeasingPolicyValidatorTests cover the four policy kinds and failure modes; command tests cover the flag on/off and validation paths.
Promotes leasing policy from inline Collection columns to a first-class, org-scoped, reusable entity that collections (and eventually Secrets Manager entities) reference by FK. - Schema: drop Collection.LeasingEnabled and Collection.LeasingPolicy; add Collection.LeasingPolicyId UNIQUEIDENTIFIER NULL with FK to the new LeasingPolicy(Id) ON DELETE SET NULL. - New LeasingPolicy table (org-scoped, unique Name per org) with CRUD stored procedures under src/Sql/dbo/PrivilegedAccessManagement/. - EF: new LeasingPolicy entity + DbSet, explicit OnModelCreating configuration for SET NULL behavior and the (OrganizationId, Name) unique index. Generated migrations for Postgres, MySQL, and SQLite. - MSSQL migration 2026-05-21_00_AddLeasingPolicy.sql (renamed from the previous AddCollectionLeasing) creates the table + sprocs, drops the prior inline columns, adds the FK column, and CREATE OR ALTERs the affected Collection_* sprocs to forward @LeasingPolicyId. - Collection request/response models now expose Guid? LeasingPolicyId. - Create/UpdateCollectionCommand reverted to pre-leasing shape; policy validation moves to upcoming LeasingPolicy CRUD commands. The existing LeasingPolicy DTOs and LeasingPolicyValidator survive for the new policy CRUD pipeline (commands, controller, DI wiring to follow).
Wires the HTTP surface for managing reusable PAM leasing policies — the
backing schema, entities, and migrations from the prior commit are now
reachable via:
- GET /organizations/{orgId}/leasing-policies (org member)
- GET /organizations/{orgId}/leasing-policies/{id} (org member)
- POST /organizations/{orgId}/leasing-policies (Owner/Admin)
- PUT /organizations/{orgId}/leasing-policies/{id} (Owner/Admin)
- DELETE /organizations/{orgId}/leasing-policies/{id} (Owner/Admin)
All endpoints are gated by [RequireFeature(FeatureFlagKeys.Pam)] and
return 404 on auth failure (matches PoliciesController convention so
resource existence isn't leaked).
- Repository: ILeasingPolicyRepository extends IRepository<T, Guid>;
base picks up the LeasingPolicy_Create/_Update/_ReadById/_DeleteById
sprocs from the migration, custom GetManyByOrganizationIdAsync added
for both Dapper and EF.
- Commands: CreateLeasingPolicyCommand and UpdateLeasingPolicyCommand
validate the JSON via ILeasingPolicyValidator, set/refresh
CreationDate/RevisionDate, and check name uniqueness within the org.
DeleteLeasingPolicyCommand performs the cross-org check then calls
DeleteAsync (FK ON DELETE SET NULL handles detaching referencing
collections at the DB level).
- API: LeasingPoliciesController plus request/response models under
src/Api/PrivilegedAccessManagement/. Request accepts `policy` as
structured JSON; response surfaces it as a parsed JsonElement.
- DI: AddLeasingPolicyCommands() in OrganizationServiceCollectionExtensions
registers the validator (singleton) and three commands (scoped). The
repository is registered alongside other singletons in the Dapper and
EF service-collection extensions.
- Tests: 11 new command tests (4 Create / 4 Update / 3 Delete) covering
happy paths, cross-org NotFound, validator failures, and duplicate
names.
Renames the PAM leasing-policy concept to access rule across the codebase
so the persisted entity and the JSON DSL have distinct names:
- Entity LeasingPolicy → AccessRule (table, Collection.AccessRuleId FK,
repos, commands, controller, request/response models). Controller route
is now /organizations/{orgId}/access-rules.
- JSON DSL base LeasingPolicy → Rule, subclasses *Policy → *Rule, namespace
Models/Policies → Models/Rules.
- Validator LeasingPolicyValidator → AccessRuleValidator and validates the
AccessRule.Rule column (renamed from Policy).
- DB column [Policy] → [Rule] on AccessRule; sproc params @Policy → @rule.
- MSSQL migration renamed to 2026-05-21_00_AddAccessRule.sql; EF migrations
regenerated as AddAccessRule. The legacy LeasingEnabled/LeasingPolicy
inline-column cleanup is preserved for dev DBs that ran the earlier
iteration.
Complete the server side of the PAM Approver Inbox: the four HTTP
endpoints the web client calls plus the RefreshApproverInbox push.
- GET /leasing/inbox/requests — pending queue for collections the
caller can Manage
- GET /leasing/inbox/history — resolved queue (90-day window)
- POST /leasing/requests/{id}/decision — approve/deny a pending request
- POST /leasing/leases/{id}/revoke — revoke an active lease
Inbox queries and the decision/revoke commands authorize via a reusable
Manage-on-collection predicate (IApproverCollectionAccessQuery); list
endpoints filter by the manageable set, decision/revoke check a single
collection. The decision endpoint records a human LeaseDecision and the
revoke endpoint persists its reason as a LeaseDecision (no Lease schema
change). State conflicts return 409; self-approval is blocked as 400
(Bitwarden clients treat 403 as a forced logout).
Each inbox-affecting change (new pending request, decision, revoke)
pushes RefreshApproverInbox (PushType 28) to every user who can Manage
the collection, resolved via a new ICollectionRepository
.GetManagingUserIdsAsync (Dapper + EF parity). Lease repositories
remain Dapper/MSSQL-only.
Adds unit tests (commands, queries, status mapper, predicate service,
notifier, controller) and SqlServer integration tests for the new
repository methods.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
AccessRule (Conditions) -> AccessRequest -> AccessDecision (Verdict) -> AccessLease, with the AccessCondition tree replacing the polymorphic Rule model and the engine renamed to AccessRuleEngine returning AccessEvaluation. Requester/approver replace grantee/resolver; ticket/redeem/policy vocabulary removed. Tables, sprocs, and the branch migrations are renamed in place, so dev databases must be recreated. Routes and controller names are unchanged.
abergs
commented
Jun 10, 2026
| services.AddScoped<IListMyAccessRequestsQuery, ListMyAccessRequestsQuery>(); | ||
| services.AddScoped<IListMyActiveLeasesQuery, ListMyActiveLeasesQuery>(); | ||
| services.AddScoped<IListMyActiveAccessLeasesQuery, ListMyActiveAccessLeasesQuery>(); | ||
| } |
Member
Author
There was a problem hiding this comment.
I think we should bundle these services in a single helper method
Approving an access request now records only the verdict; the lease that
authorizes access is minted when the requester activates the approved
request (POST leasing/requests/{id}/activate), within its window. The
automatic path still mints instantly at submit. Activation is idempotent
while the produced lease is live, race-safe via a guarded insert plus a
unique index on AccessLease(AccessRequestId), and the state endpoint now
surfaces the approved-but-not-started request. The caller-scoped list
endpoints also wrap in ListResponseModel so clients can parse them.
The inbox read procedures returned only the produced lease's id, never its status, so a lease that had ended (revoked/expired) still looked active to the client and offered a Revoke the server then rejected with 409. Select [Status] alongside [Id] in the pending/history inbox procedures and carry it through AccessRequestDetails to the response as ProducedLeaseStatus (active|expired|revoked).
Add SingleActiveLease to AccessRule (entity, API request/response, AccessRule_Create/_Update, migration) and enforce it when a lease is minted. Scope is per-cipher with union/OR gating: the singleton binds only when every collection a member reaches the cipher through is governed by a single-active-lease rule; any ungated or non-singleton path is an escape that leaves them unconstrained. The mint procs take @EnforceSingleActiveLease and serialize same-cipher activations under a UPDLOCK/HOLDLOCK range lock inside an explicit transaction; the activate and auto-approve paths surface contention as a retryable conflict.
The automatic (no-approval) path minted an active lease at submit, so the server granted access the moment the client posted the request on retrieval — never an explicit user choice. Approval was already deferred for the human path; this brings the automatic path in line. Auto-approval now records only the already-Approved AccessRequest and its automatic AccessDecision (no lease) via the new AccessRequest_CreateAutoApproved proc; the requester explicitly activates the approved request to mint the lease (ActivateAccessRequestCommand), exactly like the human path after approval, and the state endpoint surfaces it as the startable ApprovedRequest. The per-cipher single-active-lease guard now runs only at activation, the one remaining mint site, so it drops from the submit path. AccessLease_CreateAutoApproved is dropped; the submit result carries the approved request rather than a lease.
abergs
force-pushed
the
pam/collection-leasing-config
branch
from
bc745c5 to
89e16bb
Compare
The access-rule dialog sends defaultLeaseDurationSeconds and maxLeaseDurationSeconds, and the client reads them back from the rule response, but the server had no fields to receive them: AccessRuleRequestModel ignored them, the AccessRule entity had no columns, and the response never returned them. The values were silently dropped on every save, so reopening a rule reset both to defaults. Add DefaultLeaseDurationSeconds and MaxLeaseDurationSeconds (nullable int seconds) to AccessRule end to end: entity, API request/response, AccessRule_Create/_Update procs, table, the field-by-field rebuild in UpdateAccessRuleCommand, and AccessRuleDetails.From, plus a migration. Null default duration means the backend default; null max means no per-rule cap. Matches the SingleActiveLease footprint; EF migrations stay deferred for the POC.
abergs
force-pushed
the
pam/collection-leasing-config
branch
from
89e16bb to
2482b37
Compare
AccessRule gains AllowsExtensions + MaxExtensions, threaded through the entity, SSDT table/procs, API request/response models, and create/update validation. Extensions are always auto-approved (never routed to an approver): a new POST /leasing/requests/extension runs RequestLeaseExtensionCommand, which validates the active lease, the rule opt-in, the duration, a required justification, and the per-lease cap, then atomically records an approved extension request and pushes the parent lease's NotAfter out in place (AccessRequest_CreateApprovedExtension, under a per-lease lock) without minting a new lease. Also surface ExtensionsAllowed/ExtensionsRemaining on the cipher access-state snapshot, and exclude extension requests from the startable-approved read so an applied extension never shows as activatable.
Per updated product direction, a lease may now be extended exactly once, and the admin caps the length of that extension instead of a count. The AccessRule setting MaxExtensions (a count) becomes MaxExtensionDurationSeconds (the longest a single extension may run); the member picks any duration up to that maximum. RequestLeaseExtensionCommand caps the requested duration at the rule's MaxExtensionDurationSeconds and rejects a second extension (the outcome MaxExtensionsReached becomes AlreadyExtended; AccessRequest_CreateApprovedExtension drops its @MaxExtensions parameter and guards on EXISTS instead of a count). The cipher access-state snapshot now reports ExtensionsAllowed (the rule opts in and the lease has not been extended yet) plus MaxExtensionDurationSeconds so the client can cap its duration picker. Migration 2026-06-15_00 drops MaxExtensions, adds MaxExtensionDurationSeconds, and recreates the affected procs, leaving 2026-06-12_01/02 intact so existing dev databases roll forward without a reset.
A rule with no conditions is a deliberate way to route a collection's access through the PAM flow purely for audit logging. The engine and governing-rule resolver already treat an empty all_of as vacuously satisfied (auto-allow, no human approval), but AccessRuleValidator rejected it outright, blocking the create/update path. Permit an empty all_of in the validator (depth and max-children bounds are unchanged); the resolver's fail-closed-on-malformed behavior is untouched. Documents the semantics on AllOfCondition and adds validator, engine, and resolver test coverage for the conditionless case.
AccessRule.Conditions was a polymorphic AccessCondition tree rooted at an all_of node. Replace it with a flat, implicitly-ANDed array of leaf conditions, stored and serialized as a bare JSON array. Remove AllOfCondition; the engine, validator, resolver, and GoverningRule now operate over IReadOnlyList<AccessCondition>. An empty list is vacuously allowed. Unshipped PoC, so a clean break with no data migration. A future grouping condition kind can be reintroduced within the array, so a bare array does not preclude trees later.
abergs
force-pushed
the
pam/collection-leasing-config
branch
from
1b11dae to
d90ead2
Compare
abergs
force-pushed
the
pam/collection-leasing-config
branch
from
d90ead2 to
a7c3903
Compare
… into one fall-through case
Add the UsePam organization ability across the stack, modeled on UseInviteLinks/UseMyItems: - Entity + OrganizationAbility + profile/provider detail models - Licensing: claim emission and claims-based VerifyData using the conditional HasClaim check (PM-33980) so pre-existing license files still validate - API response models and an Admin portal toggle - MSSQL schema + migration; EF migrations for Postgres/MySQL/SQLite Defaults off for all organizations. Plan/pricing wiring is deliberately deferred until a PAM plan tier exists, so the Admin toggle is currently the only way to enable it.
| .GetOrganizationAbilityAsync(organizationId) | ||
| .Returns(new OrganizationAbility { Id = organizationId }); | ||
|
|
||
| var result = await sutProvider.Sut.GetCipher(id); |
| .GetLeasedCipherAsync(user.Id, id) | ||
| .ReturnsNull(); | ||
|
|
||
| await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.GetCipher(id)); |
|
|
||
| var userService = sutProvider.GetDependency<IUserService>(); | ||
| userService.GetUserByPrincipalAsync(Arg.Any<ClaimsPrincipal>()).ReturnsForAnyArgs(user); | ||
| userService.HasPremiumFromOrganization(user).Returns(false); |
Comment on lines
+35
to
+41
| foreach (var cidr in condition.Cidrs) | ||
| { | ||
| if (IPNetwork.TryParse(cidr, out var network) && network.Contains(signals.IpAddress)) | ||
| { | ||
| return AccessEvaluation.Allow; | ||
| } | ||
| } |
Comment on lines
+58
to
+64
| foreach (var window in condition.Windows) | ||
| { | ||
| if (WindowContains(window, day, time)) | ||
| { | ||
| return AccessEvaluation.Allow; | ||
| } | ||
| } |
Comment on lines
+87
to
+93
| foreach (var cidr in condition.Cidrs) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(cidr) || !IPNetwork.TryParse(cidr, out _)) | ||
| { | ||
| return AccessRuleValidationResult.Invalid($"Invalid CIDR: '{cidr}'."); | ||
| } | ||
| } |
Comment on lines
+66
to
+72
| foreach (var rule in rules) | ||
| { | ||
| if (collectionIdsByRule.TryGetValue(rule.Id, out var collectionIds)) | ||
| { | ||
| rule.CollectionIds = collectionIds; | ||
| } | ||
| } |
Comment on lines
+56
to
+63
| foreach (var condition in conditions) | ||
| { | ||
| var result = ValidateCondition(condition); | ||
| if (!result.IsValid) | ||
| { | ||
| return result; | ||
| } | ||
| } |
|
|
||
| namespace Bit.Infrastructure.EntityFramework.Pam.Models; | ||
|
|
||
| public class AccessRule : Core.Pam.Entities.AccessRule |
Comment on lines
+289
to
+295
| catch (Exception ex) | ||
| { | ||
| // Best effort: the request is already persisted and the inbox push already fired. An email failure | ||
| // must never fail the submission, so log and move on. | ||
| _logger.LogError(ex, | ||
| "Failed to send PAM approver emails for access request {AccessRequestId}.", request.Id); | ||
| } |
GoverningRuleResolver returned the first human-approval rule it found
("most restrictive wins") — the inverse of pam.allium, which resolves the
governing rule by union/OR: an automatic grant is favoured over one that
needs human approval, which is favoured over a denial. A member with a
valid auto-grant path was needlessly routed to an approver.
Resolve by evaluating each candidate rule through AccessRuleEngine and
returning the best (lowest) AccessEvaluationOutcome (Allow < RequiresApproval
< Deny), reusing the same engine the downstream decision uses. This also
honours the spec's rule that a failing automatic rule must not pre-empt a
path that needs approval but would grant.
ResolveAsync now takes AccessSignals (caller IP + timestamp); the five
callers build and pass them (three gained ICurrentContext, via the new
AccessSignals.From factory). As a side effect AccessPreCheckQuery is now
condition-aware instead of keying only on RequiresHumanApproval.
Adds multi-collection precedence tests to GoverningRuleResolverTests.
|
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
3 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
📔 Objective
📸 Screenshots