[PM-41259] Scaffold PAM cipher-lease endpoints - #8105
Conversation
Adds the /leases/ciphers/{id} Minimal API group (pre-check, state, submit)
with an intentionally unimplemented handler, following the access-rule,
access-request, and lease scaffolds. Contract tests lock the routes, names,
methods, and return types the generated spec is built from.
No coupling to unlanded PAM domain code to avoid: it's a standalone byte enum. Land it directly instead of duplicating a wire-only copy under Api/Models like the other scaffold enums.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8105 +/- ##
==========================================
- Coverage 62.83% 62.81% -0.03%
==========================================
Files 2299 2306 +7
Lines 100138 100191 +53
Branches 9012 9012
==========================================
+ Hits 62923 62935 +12
- Misses 35030 35071 +41
Partials 2185 2185 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the new Code Review Details
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /// An access request as its requester sees it right after submit — the shape <see cref="AccessRequestResultResponseModel"/> | ||
| /// wraps for the cipher-lease submit response. | ||
| /// </summary> | ||
| public class AccessRequestResponseModel : ResponseModel |
There was a problem hiding this comment.
❓ QUESTION: Is the second access-request wire shape intentional, rather than reusing AccessRequestDetailsResponseModel?
Details
All nine members here (Id, CipherId, CollectionId, OrganizationId, Status, LeaseNotBefore, LeaseNotAfter, Reason, SubmittedAt) are name-, type-, and doc-identical to AccessRequestDetailsResponseModel, so this is a strict subset with a different object discriminator (accessRequest vs accessRequestDetails).
The practical effect on the generated spec: within this same group, GET leases/ciphers/{id}/state hands back AccessRequestDetailsResponseModel (via PendingRequest/ApprovedRequest) while POST leases/ciphers/{id} hands back this one, so clients get two generated types for the same entity and can't pass one where the other is expected. Since these DTOs are what client bindings are generated from, collapsing them later is a breaking change.
If the narrower shape is deliberate (e.g. deliberately withholding decision history at submit), a one-line note in the class doc would keep the next person from "fixing" it. Otherwise AccessRequestResultResponseModel.Request could point at AccessRequestDetailsResponseModel and this type would go away.
1 participant
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-41259
📔 Objective
Adds the
/leases/ciphers/{id}Minimal API group (pre-check, state, submit) with an intentionally unimplemented handler, following the access-rule, access-request, and lease scaffolds.The four new response/request DTOs (
AccessPreCheckResponseModel,CipherAccessStateResponseModel,AccessRequestResponseModel,AccessRequestResultResponseModel,AccessRequestCreateRequestModel) and the wire-onlyAccessApprovalModeenum carry the wire contract without coupling to the (not-yet-landed) PAM domain. Contract tests lock the routes, names, methods, and return types the generated spec is built from.