feat: sd-jwt revocation flow#357
Conversation
…e sessions and integrate SdJwtVcModule. Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
…e sessions and integrate SdJwtVcModule. Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR introduces credential revocation via status lists for OpenID4VC issuance. It adds CLI configuration for status-list server endpoints, extends the issuance controller and service to create and manage status lists, implements a revocation endpoint, and provides utilities for remote status-list operations including JWT signing and concurrent-modification locking. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as IssuanceSessionsController
participant Service as IssuanceSessionsService
participant StatusListUtil as statusListService
participant Server as Status List Server
participant Agent
rect rgba(135, 206, 250, 0.5)
Note over Client,Agent: Credential Offer Creation with Status List
Client->>Controller: POST /issuance-sessions (with statusListDetails)
Controller->>Service: createCredentialOffer(options, agentReq)
Service->>Service: Map credentials async
loop For each credential
Service->>StatusListUtil: checkAndCreateStatusList(agent, listId, issuerDid)
StatusListUtil->>Server: GET /status-lists/{listId}
alt Status List Exists
Server-->>StatusListUtil: 200 OK
else Status List Not Found
Server-->>StatusListUtil: 404 Not Found
StatusListUtil->>Agent: Resolve issuer DID verification method
Agent-->>StatusListUtil: Verification method
StatusListUtil->>StatusListUtil: Sign statuslist+jwt
StatusListUtil->>Server: POST /status-lists (id, jwt)
Server-->>StatusListUtil: 201 Created
end
Service->>Service: Build status block with uri & idx
Service->>Service: Store credential in offerStatusInfo
end
Service->>Service: Set issuanceMetadata.StatusListInfo
Service-->>Controller: Offer with status list injection
Controller-->>Client: 200 OK (credential offer)
end
rect rgba(144, 238, 144, 0.5)
Note over Client,Agent: Credential Revocation
Client->>Controller: POST /issuance-sessions/{id}/revoke
Controller->>Service: revokeBySessionId(agentReq, sessionId)
Service->>Service: Load issuance session record
Service->>Service: Extract StatusListInfo from metadata
loop For each revocation entry
Service->>StatusListUtil: revokeCredentialInStatusList(agent, listId, idx, issuerDid)
StatusListUtil->>StatusListUtil: Acquire per-listId lock
StatusListUtil->>Server: GET /status-lists/{listId}
Server-->>StatusListUtil: Current JWT
StatusListUtil->>StatusListUtil: Parse JWT → StatusList
StatusListUtil->>StatusListUtil: Set index status to revoked (1)
StatusListUtil->>StatusListUtil: Sign updated statuslist+jwt
StatusListUtil->>Server: PATCH /status-lists/{listId} (new jwt)
Server-->>StatusListUtil: 200 OK
StatusListUtil->>StatusListUtil: Release per-listId lock
end
Service-->>Controller: success message
Controller-->>Client: 200 OK (revocation complete)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…e sessions and integrate SdJwtVcModule. Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
…e sessions and integrate SdJwtVcModule. Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
|
@coderabbitai Please review |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/controllers/x509/x509.types.ts (1)
103-113: Example values don't match enum type.The
@exampleshows string names (["digitalSignature", "keyEncipherment", "crlSign"]), butX509KeyUsageis a numeric enum (e.g.,DigitalSignature = 1,KeyEncipherment = 4). API consumers may be confused about whether to send enum names or numeric values.Update the example to reflect the actual expected format, or clarify how the API deserializes string names to enum values.
📝 Suggested example update
export interface KeyUsageDto { /** - * `@example` ["digitalSignature", "keyEncipherment", "crlSign"] + * `@example` [1, 4, 64] + * `@description` Array of X509KeyUsage values: DigitalSignature=1, NonRepudiation=2, KeyEncipherment=4, DataEncipherment=8, KeyAgreement=16, KeyCertSign=32, CrlSign=64, EncipherOnly=128, DecipherOnly=256 */ usages: X509KeyUsage[]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/x509/x509.types.ts` around lines 103 - 113, The example for KeyUsageDto is misleading because usages is typed as X509KeyUsage (a numeric enum); update the example to show numeric enum values or clarify conversion: change the usages example to use the numeric enum values (e.g., [1, 4, 128]) or add a note that string names are accepted and will be mapped to X509KeyUsage; modify the KeyUsageDto JSDoc for the usages property and/or add a brief remark next to markAsCritical to indicate optionality so consumers know the expected payload format for the usages field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`:
- Around line 194-197: The code treats a missing issuanceMetadata.StatusListInfo
as an internal error; instead, update IssuanceSessionsService to return a client
error (4xx) for this input/caller problem by replacing the thrown generic Error
when statusInfo is falsy/empty with a specific HTTP error (e.g., BadRequest or a
400 HttpError) that includes the sessionId and a clear message; locate the check
around statusInfo = record.issuanceMetadata?.StatusListInfo and change the throw
to the appropriate HttpError type used in the codebase so callers receive a 4xx
rather than a 500.
- Around line 53-88: The code currently always injects a top-level SD-JWT-style
status block and doesn't validate that revocability is actually supported for
the chosen signer/format; update the logic around effectiveIssuerDid,
effectiveStatusList, isRevocable, and the payload merge to (1) validate at offer
creation time that isRevocable is only allowed when a revocation mechanism will
actually be attached (e.g., DID signer + SD-JWT/status-list for SD-JWT formats,
or DID signer + W3C-style credentialStatus for W3C VC formats; reject
combinations like isRevocable true with x5c signer or mso_mdoc or when no
effectiveStatusList), (2) compute and attach format-specific status payloads:
for SD-JWT formats use status: { status_list: { uri, idx } }, for W3C VC formats
use credentialStatus with the equivalent uri/index structure, and for mso_mdoc
do not add a JSON status claim (status is handled in CBOR MSO), and (3) keep
using checkAndCreateStatusList and offerStatusInfo but only push into
offerStatusInfo when you actually attach a revocation mechanism; use the
existing symbols effectiveIssuerDid, effectiveStatusList,
checkAndCreateStatusList, offerStatusInfo, supported, statusBlock and replace
the unconditional payload injection with format-aware branches that either
attach the appropriate claim or throw/reject when revocability cannot be
supported.
In `@src/utils/statusListService.ts`:
- Around line 69-71: The fetch calls in statusListService (the call that uses
uri and getApiKeyHeaders) must be routed through the shared HTTP helper that
enforces an abort/timeout and retry policy; replace direct fetch(uri, { headers:
getApiKeyHeaders() }) usages with the helper so requests are bounded by a
timeout/AbortSignal and ensure retries are only applied to safe read operations
(e.g., the status-list GET path) while write paths (issuance/revocation updates)
use a single timed request without retries; update every occurrence (the fetch
at lines using uri/getApiKeyHeaders and the other fetch blocks around 88-92 and
125-149) to use the helper and propagate the AbortSignal or timeout error
handling back to the caller.
- Around line 68-99: The GET→404→POST path for creating status lists is racy for
concurrent callers: wrap the creation logic for a given listId in a per-listId
lock (e.g., acquire/release based on listId) so only one caller performs the
fetch/create flow, or at minimum handle POST conflicts by treating a 409 as
success and re-reading the list after POST; specifically guard the block that
fetches uri, constructs StatusList, resolves issuer DID via agent.dids.resolve,
signs with signStatusList (using keyId), and posts to
`${getServerUrl()}/status-lists` (i.e., the code referencing listId, listSize,
StatusList, signStatusList, postRes) so concurrent Promise.all callers do not
both try to create the same list and fail.
- Around line 37-63: signStatusList currently hardcodes alg: 'EdDSA' and relies
on a non-specific verification method; change it to select the signing key and
algorithm based on the verification relationship (e.g., assertionMethod) and the
actual key type or provided signerOptions rather than array position, and set
the protected header kid to the full DID URL of the selected verification
method. Concretely: update the logic that calls getKmsKeyIdForDid and the
JwsProtectedHeaderOptions (the header variable) to accept/respect a
signerOptions or derive alg from the resolved key type (Ed25519->EdDSA,
P-256->ES256, secp256k1->ES256K), lookup the correct verificationMethod by its
relationship (not index 0), populate protectedHeaderOptions.kid with the DID URL
of that method, and pass the resolved alg and kid into
jwsService.createJwsCompact; apply the same pattern to the other signing sites
flagged (the other blocks using JwsProtectedHeaderOptions and createJwsCompact).
In `@tsconfig.build.json`:
- Line 39: The file ends with a closing brace but lacks a trailing newline;
update the tsconfig.build.json so that after the final closing brace (`}`) there
is a single newline character at EOF (ensure the file ends with a newline to
satisfy linters and POSIX conventions).
- Around line 19-22: The tsconfig.build.json currently includes "DOM" in the
"lib" array which brings browser types into a Node backend and causes conflicts;
remove "DOM" from the "lib" array so it only contains "ESNext" (or replace with
the minimal required ES libs) and rely on the existing "types": ["node"] for
Node typings; update any project docs or a one-line comment if there's a
specific reason to keep DOM types, otherwise delete the "DOM" entry to resolve
the type conflicts.
---
Nitpick comments:
In `@src/controllers/x509/x509.types.ts`:
- Around line 103-113: The example for KeyUsageDto is misleading because usages
is typed as X509KeyUsage (a numeric enum); update the example to show numeric
enum values or clarify conversion: change the usages example to use the numeric
enum values (e.g., [1, 4, 128]) or add a note that string names are accepted and
will be mapped to X509KeyUsage; modify the KeyUsageDto JSDoc for the usages
property and/or add a brief remark next to markAsCritical to indicate
optionality so consumers know the expected payload format for the usages field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b154db66-7575-4f6c-917e-11e4e5ca9a08
📒 Files selected for processing (12)
samples/cliConfig.jsonsrc/cli.tssrc/cliAgent.tssrc/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.tssrc/controllers/openid4vc/issuance-sessions/issuance-sessions.service.tssrc/controllers/openid4vc/types/issuer.types.tssrc/controllers/types.tssrc/controllers/x509/x509.types.tssrc/routes/routes.tssrc/routes/swagger.jsonsrc/utils/statusListService.tstsconfig.build.json
👮 Files not reviewed due to content moderation or server errors (6)
- src/controllers/openid4vc/types/issuer.types.ts
- samples/cliConfig.json
- src/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.ts
- src/cliAgent.ts
- src/cli.ts
- src/routes/swagger.json
5e7263b to
1291984
Compare
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
…lection for status list operations Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
|



Feat/sd jwt revocation flow
Summary by CodeRabbit
New Features
Documentation