[PM-39798] Scaffold PAM access-request endpoints#7893
Open
Hinton wants to merge 11 commits into
Open
Conversation
Extract the PAM-free swagger foundation from the PAM POC so it can land on main independently. - Add Bit.HttpExtensions.StandaloneEndpointDataSource and the AddOpenApiEndpointDataSource registration helper. The offline OpenAPI generator (dotnet swagger tofile) never runs the Configure pipeline where Minimal API endpoints are mapped via UseEndpoints, so it omits them. A single DI-registered EndpointDataSource composes every feature's mapping delegate and is gated to swagger generation only, so it never replaces the runtime composite EndpointDataSource that routing and link generation depend on. - Teach SwaggerGenOptionsExt.BuildOperationId and ActionNameOperationFilter to derive the operation ID and action name from the endpoint name set via .WithName(...) when there are no controller/action route values, as is the case for Minimal API endpoints. - Guarantee BuildOperationId never returns a null/empty operation ID: a Minimal API endpoint mapped without .WithName() now falls back to a deterministic id derived from the HTTP method and route template. Swashbuckle writes the selector's value straight onto operation.OperationId, so a null/empty id would collapse distinct endpoints together and abort spec generation via CheckDuplicateOperationIdsDocumentFilter. - Add unit tests for BuildOperationId (including the route fallback), the ActionNameOperationFilter endpoint-name branch, StandaloneEndpointDataSource composition, and AddOpenApiEndpointDataSource gating/registration.
…ation Add a minimal, self-contained slice of the PAM access-rule surface so client teams can start generating server-side bindings from the OpenAPI spec, ahead of the full PAM implementation landing. - Add Commercial.Pam project (references Core + HttpExtensions only) and register it in the solution and Api.csproj (#if !OSS) - Add AccessRuleRequestModel and AccessRuleResponseModel as standalone DTOs (no domain coupling), with docs on Name, Description and Conditions - Map the five access-rule endpoints (GET all, GET, POST, PUT, DELETE) with the shared PAM endpoint chain; handler bodies are stubs that throw NotImplementedException, so only the wire contract is defined - Wire AddEndpointsApiExplorer + AddCommercialPamServices + MapPamEndpoints into Startup - Add Commercial.Pam.Test covering the endpoint contract (routes, names, methods, group, error responses, return types) Depends on #7874 (minimal-API OpenAPI support).
Emit Commercial.Pam XML docs and auto-discover every assembly doc file in AddSwaggerGen (replacing the hardcoded per-assembly list), and add field-level descriptions to AccessRuleResponseModel mirroring the request model, so the generated spec and client bindings carry the rule's contract.
…neration Follow up the access-rule scaffold with the access-request slice, so client teams can generate bindings for the requester/approver surface from the OpenAPI spec ahead of the full PAM implementation landing. - Add AccessRequestDetailsResponseModel, AccessLeaseResponseModel, AccessRequestDecisionResponseModel and AccessDecisionRequestModel as standalone DTOs (no PAM-domain coupling), plus a standalone AccessDecisionVerdict so the wire contract carries the 0=deny/1=approve enum - Map the seven access-request endpoints (inbox, history, mine, GET details, decision, activate, revoke) through the shared PAM endpoint chain; handler bodies are stubs that throw NotImplementedException, so only the contract is defined - Register the handler in AddCommercialPamServices and wire the group into MapPamEndpoints - Add Commercial.Pam.Test coverage for the contract (routes, names, methods, group, error responses, return types) and update the access-rule tests to filter by tag now that MapPamEndpoints serves more than one group Follows #<scaffold-access-rules PR>.
Contributor
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the PAM access-request OpenAPI scaffold: seven Minimal API endpoints, four standalone DTOs, and the No security, correctness, or breaking-change concerns found. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7893 +/- ##
==========================================
- Coverage 61.34% 61.33% -0.02%
==========================================
Files 2236 2242 +6
Lines 98517 98630 +113
Branches 8907 8911 +4
==========================================
+ Hits 60437 60491 +54
- Misses 35943 36003 +60
+ Partials 2137 2136 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add field-level descriptions to the remaining access-request request/response properties — the ids and timestamps that were previously undocumented — so the generated spec and client bindings carry a description for every field. - AccessDecisionRequestModel: document Verdict and Comment - AccessRequestDecisionResponseModel: document Name, Email, Comment, DecidedAt - AccessRequestDetailsResponseModel: document the ids, RequestedNotAfter, Reason, SubmittedAt, ResolvedAt, and the denormalized requester identity - AccessLeaseResponseModel: document the ids and the lease window/revocation timestamps - AccessDecisionVerdict: document the Deny/Approve members The only undescribed property left is the `object` discriminator from the shared ResponseModel base, which is undocumented across all response models.
|
Hinton
changed the title
main relocated the PAM project from bitwarden_license/src/Commercial.Pam to bitwarden_license/src/Services/Pam per ADR-0032 (assembly Commercial.Pam -> Pam, namespace Bit.Commercial.Pam -> Bit.Services.Pam, DI AddCommercialPamServices -> AddPamServices). This branch's access-request scaffold was authored at the old location, so the merge: - Ported the access-request slice (7 DTO/endpoint/handler source files + the AccessRequestEndpointsTests) into Services/Pam with the Bit.Services.Pam namespace, and wired MapAccessRequestEndpoints + the AccessRequestEndpointsHandler registration into the new-location PamEndpointsExtensions/ServiceCollectionExtensions. - Rebuilt the access-request test on main's public-host materialization pattern (WebApplication.CreateSlimBuilder) since main keeps StandaloneEndpointDataSource internal; updated AccessRuleEndpointsTests to filter by the AccessRules tag now that MapPamEndpoints serves both groups. - Removed the orphaned Commercial.Pam / Commercial.Pam.Test projects. - Took main's version of the shared references (solution, Api.csproj, Startup.cs, CODEOWNERS, packages.lock.json) which already point at Services/Pam.
abergs
reviewed
Jul 2, 2026
abergs
reviewed
Jul 2, 2026
patriksvensson
left a comment
patriksvensson
left a comment
Member
There was a problem hiding this comment.
In general, it looks good to me, but I think we should have a discussion about whether we should be using enums for statuses and similar fields.
Replace the magic-string status/kind fields on the access-request DTOs with dedicated byte enums (AccessLeaseStatus, AccessRequestStatus, DeciderKind), addressing PR feedback. They serialize as integers, matching AccessDecisionVerdict and the rest of the API. AccessLeaseStatus is shared by the lease Status and the nullable ProducedLeaseStatus so the two cannot drift.
patriksvensson
approved these changes
Jul 4, 2026
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
https://bitwarden.atlassian.net/browse/PM-39798
📔 Objective
Extends the PAM OpenAPI scaffold to the access-request resource so client teams can generate bindings for the requester/approver surface ahead of the full PAM implementation, mirroring the access-rule slice in #7878.
AccessRequestDetailsResponseModel,AccessLeaseResponseModel,AccessRequestDecisionResponseModel,AccessDecisionRequestModel, plus a standaloneAccessDecisionVerdictenum so the wire contract carries the0 = deny/1 = approvevalues. These reproduce the field-level shape and docs of the eventual domain-backed models without referencingBit.Pam.*, keeping the project's Core + HttpExtensions-only references intact.WithPamDefaults):GET inbox,GET history,GET mine,GET {id},POST {id}/decision,POST {id}/activate,POST {id}/revoke. Handler bodies are stubs that throwNotImplementedException— only the wire contract (routes, names, methods, status codes, return types) is defined; behavior lands with the rest of PAM.AccessRequestEndpointsHandlerinAddCommercialPamServicesand add the/access-requestsgroup toMapPamEndpoints.AccessRequestEndpointsTestslocks the contract (routes/names/methods, internal doc group,ErrorResponseModel400/404, handler return types). The existingAccessRuleEndpointsTestsnow filters by tag, sinceMapPamEndpointsserves more than one group.Verified locally:
Commercial.Pam.Test(28 passing),dotnet format --verify-no-changes, anddotnet swagger tofile … internal— the generated spec emits all sevenPam_AccessRequests_*paths, the four DTOs, andAccessDecisionVerdictas an integer enum (Deny/Approve).