-
Notifications
You must be signed in to change notification settings - Fork 79
Feat/sd jwt revocation flow #1594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3eef6d3
a9b4613
53be9b5
05e30f9
51d314b
fe14467
379fc76
7ba2bf7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,7 +33,7 @@ | |||||||||||||||||||
|
|
||||||||||||||||||||
| export interface DisclosureFrame { | ||||||||||||||||||||
| _sd?: string[]; | ||||||||||||||||||||
| [claim: string]: DisclosureFrame | string[] | undefined; | ||||||||||||||||||||
| [claim: string]: DisclosureFrame | DisclosureFrame[] | string[] | undefined; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export interface validityInfo { | ||||||||||||||||||||
|
|
@@ -58,6 +58,7 @@ | |||||||||||||||||||
| authorizationServerUrl: string; | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| publicIssuerId?: string; | ||||||||||||||||||||
| isRevocable?: boolean; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export interface ResolvedSignerOption { | ||||||||||||||||||||
|
|
@@ -84,12 +85,18 @@ | |||||||||||||||||||
| format: CredentialFormat; | ||||||||||||||||||||
| payload: Record<string, unknown>; | ||||||||||||||||||||
| disclosureFrame?: DisclosureFrame; | ||||||||||||||||||||
| statusListDetails?: { | ||||||||||||||||||||
| listId: string; | ||||||||||||||||||||
| index: number; | ||||||||||||||||||||
| listSize?: number; | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export interface BuiltCredentialOfferBase { | ||||||||||||||||||||
| signerOption?: ResolvedSignerOption; | ||||||||||||||||||||
| credentials: BuiltCredential[]; | ||||||||||||||||||||
| publicIssuerId?: string; | ||||||||||||||||||||
| isRevocable?: boolean; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export type CredentialOfferPayload = BuiltCredentialOfferBase & | ||||||||||||||||||||
|
|
@@ -223,36 +230,46 @@ | |||||||||||||||||||
| return { valid: 0 === errors.length, errors }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function buildDisclosureFrameFromTemplate(attributes: CredentialAttribute[]): DisclosureFrame { | ||||||||||||||||||||
| function buildDisclosureFrameFromTemplate( | ||||||||||||||||||||
| attributes: CredentialAttribute[], | ||||||||||||||||||||
| payload?: Record<string, any> | ||||||||||||||||||||
| ): DisclosureFrame { | ||||||||||||||||||||
| const frame: DisclosureFrame = {}; | ||||||||||||||||||||
| const sd: string[] = []; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| for (const attr of attributes) { | ||||||||||||||||||||
| const childFrame = | ||||||||||||||||||||
| attr.children && 0 < attr.children.length ? buildDisclosureFrameFromTemplate(attr.children) : undefined; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const hasChildDisclosure = | ||||||||||||||||||||
| childFrame && (childFrame._sd?.length || Object.keys(childFrame).some((k) => '_sd' !== k)); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Case 1: this attribute itself is disclosed | ||||||||||||||||||||
| if (attr.disclose) { | ||||||||||||||||||||
| // If it has children, children are handled separately | ||||||||||||||||||||
| if (!attr.children || 0 === attr.children.length) { | ||||||||||||||||||||
| sd.push(attr.key); | ||||||||||||||||||||
| continue; | ||||||||||||||||||||
| const hasChildren = attr.children && 0 < attr.children.length; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (hasChildren) { | ||||||||||||||||||||
| const payloadValue = payload?.[attr.key]; | ||||||||||||||||||||
| //todo: | ||||||||||||||||||||
|
Check warning on line 245 in apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts
|
||||||||||||||||||||
| //1) Need to handle the type validation here to ensure payloadValue is in expected format (object or array of objects) | ||||||||||||||||||||
| //2) Need to add add validation based on template definition (e.g. if template defines an array, payload must be an array, etc.) | ||||||||||||||||||||
|
Comment on lines
+245
to
+247
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Address the TODO comment before merging. The comment acknowledges missing type validation for payload values. Consider creating an issue to track this work if it cannot be completed in this PR. Do you want me to open a new issue to track this validation task? 🧰 Tools🪛 GitHub Check: SonarCloud Code Analysis[warning] 245-245: Complete the task associated to this "TODO" comment. 🤖 Prompt for AI Agents |
||||||||||||||||||||
| if (Array.isArray(payloadValue)) { | ||||||||||||||||||||
| // Array of objects → [{ _sd: [...] }, ...] | ||||||||||||||||||||
| const childFrame = buildDisclosureFrameFromTemplate(attr.children!); | ||||||||||||||||||||
|
Check warning on line 250 in apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts
|
||||||||||||||||||||
| frame[attr.key] = payloadValue.map(() => ({ ...childFrame })); | ||||||||||||||||||||
|
Comment on lines
+248
to
+251
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Array handling has two issues: inconsistent payload propagation and shallow copy.
🔧 Proposed fix if (Array.isArray(payloadValue)) {
- // Array of objects → [{ _sd: [...] }, ...]
- const childFrame = buildDisclosureFrameFromTemplate(attr.children!);
- frame[attr.key] = payloadValue.map(() => ({ ...childFrame }));
+ // Array of objects → [{ _sd: [...] }, ...] — build per-element frame
+ frame[attr.key] = payloadValue.map((element) =>
+ buildDisclosureFrameFromTemplate(attr.children, element as Record<string, any>)
+ );
} else {📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: SonarCloud Code Analysis[warning] 250-250: This assertion is unnecessary since it does not change the type of the expression. 🤖 Prompt for AI Agents |
||||||||||||||||||||
| } else { | ||||||||||||||||||||
| // Plain object → { _sd: [...] } | ||||||||||||||||||||
| const childFrame = buildDisclosureFrameFromTemplate(attr.children!, payloadValue as Record<string, any>); | ||||||||||||||||||||
|
Check warning on line 254 in apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts
|
||||||||||||||||||||
| const hasChildDisclosure = childFrame._sd?.length || Object.keys(childFrame).some((k) => '_sd' !== k); | ||||||||||||||||||||
| if (hasChildDisclosure) { | ||||||||||||||||||||
| frame[attr.key] = childFrame; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| continue; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Case 2: attribute has disclosed children | ||||||||||||||||||||
| if (hasChildDisclosure) { | ||||||||||||||||||||
| frame[attr.key] = childFrame!; | ||||||||||||||||||||
| // Root attribute — add to _sd if disclosed | ||||||||||||||||||||
| if (attr.disclose) { | ||||||||||||||||||||
| sd.push(attr.key); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (0 < sd.length) { | ||||||||||||||||||||
| frame._sd = sd; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // console.log('Built disclosure frame:', JSON.stringify(frame, null, 2)); | ||||||||||||||||||||
|
Check warning on line 272 in apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts
|
||||||||||||||||||||
| return frame; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -366,8 +383,11 @@ | |||||||||||||||||||
| const apiFormat = mapDbFormatToApiFormat(templateRecord.format); | ||||||||||||||||||||
| const idSuffix = formatSuffix(apiFormat); | ||||||||||||||||||||
| const credentialSupportedId = `${templateRecord.name}-${idSuffix}`; | ||||||||||||||||||||
| const disclosureFrame = buildDisclosureFrameFromTemplate(sdJwtTemplate.attributes); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const disclosureFrame = buildDisclosureFrameFromTemplate( | ||||||||||||||||||||
| sdJwtTemplate.attributes, | ||||||||||||||||||||
| payloadCopy as Record<string, any> | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| return { | ||||||||||||||||||||
| credentialSupportedId, | ||||||||||||||||||||
| signerOptions: templateSignerOption ? templateSignerOption : undefined, | ||||||||||||||||||||
|
|
@@ -477,7 +497,8 @@ | |||||||||||||||||||
|
|
||||||||||||||||||||
| const baseEnvelope: BuiltCredentialOfferBase = { | ||||||||||||||||||||
| credentials: builtCredentials, | ||||||||||||||||||||
| ...(finalPublicIssuerId ? { publicIssuerId: finalPublicIssuerId } : {}) | ||||||||||||||||||||
| ...(finalPublicIssuerId ? { publicIssuerId: finalPublicIssuerId } : {}), | ||||||||||||||||||||
| isRevocable: dto.isRevocable | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Determine which authorization flow to return: | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: credebl/platform
Length of output: 846
🏁 Script executed:
Repository: credebl/platform
Length of output: 811
🏁 Script executed:
Repository: credebl/platform
Length of output: 128
🏁 Script executed:
rg "isRevocable" --type ts -B 2 -A 2Repository: credebl/platform
Length of output: 4843
🏁 Script executed:
rg "if.*isRevocable|\.isRevocable" --type ts -B 1 -A 1Repository: credebl/platform
Length of output: 1701
🏁 Script executed:
Repository: credebl/platform
Length of output: 938
Add
@IsBoolean()validator toisRevocablefields in both DTOs.Without boolean validation, requests can send
"false"or"true"strings which pass validation. Since JavaScript treats non-empty strings as truthy, theif (dto.isRevocable)checks in the service layer will incorrectly enable revocation even when"false"is provided.Both occurrences in
CreateOidcCredentialOfferDto(lines 191-193) andCreateCredentialOfferD2ADto(lines 389-391) need the@IsBoolean()decorator added after@IsOptional(). Update the imports to includeIsBooleanfromclass-validator.🤖 Prompt for AI Agents