Feat/oid4vc verification template #1534
Conversation
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
* feat: oidc issunace Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * feat:create seperate microservice for oid4vc Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * feat:removed logs Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor: removed duplicate code Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * feat: create credential offer API Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix: missing credential details repository logic Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * feat: added docker file for oid4vc-issuance Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> --------- Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
…typo in verification service Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
WalkthroughAdds intent-template and verification-template CRUD plus intent-based verification flows across API Gateway, services, repositories, and DB schema; introduces signer option typing (ISignerOption), centralized ErrorHandler, new DTOs/interfaces, Prisma models/migrations, and NATS message subjects for template and intent operations. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant ApiGateway
participant Oid4vcVerificationService
participant UtilitiesService
participant Oid4vpMicroservice
participant PrismaDB as Database
Note over Client,PrismaDB: Intent-based verification presentation
Client->>ApiGateway: POST /oid4vc-verification/{org}/{verifier}/intent-based<br/>{intent,responseMode,requestSigner}
ApiGateway->>Oid4vcVerificationService: createIntentBasedVerificationPresentation(...)
Oid4vcVerificationService->>UtilitiesService: getIntentTemplateByIntentAndOrg(intent, org)
UtilitiesService->>PrismaDB: SELECT intent_templates JOIN intents/verification_templates
PrismaDB-->>UtilitiesService: IntentTemplate
UtilitiesService-->>Oid4vcVerificationService: IntentTemplate
alt signer = DID
Oid4vcVerificationService->>Oid4vcVerificationService: resolve DID signer
else signer = X509
Oid4vcVerificationService->>PrismaDB: SELECT active x509 certs
PrismaDB-->>Oid4vcVerificationService: Certificates
Oid4vcVerificationService->>Oid4vcVerificationService: validate x5c matches template
end
Oid4vcVerificationService->>Oid4vpMicroservice: Publish NATS oid4vp-intent-based-verification-presentation
Oid4vpMicroservice->>PrismaDB: create/update verification session
Oid4vpMicroservice-->>Oid4vcVerificationService: ack / sessionId
Oid4vcVerificationService-->>ApiGateway: session response
ApiGateway-->>Client: 200 {sessionId,...}
sequenceDiagram
actor Client
participant ApiGateway
participant UtilitiesController
participant UtilitiesService
participant UtilitiesRepository
participant PrismaDB as Database
Note over Client,PrismaDB: Intent-template CRUD & pagination
Client->>ApiGateway: POST /utilities/intent-templates {orgId?, intentId, templateId}
ApiGateway->>UtilitiesController: createIntentTemplate(dto,user)
UtilitiesController->>UtilitiesService: createIntentTemplate(...)
UtilitiesService->>UtilitiesRepository: createIntentTemplate(...)
UtilitiesRepository->>PrismaDB: INSERT (with duplicate check)
PrismaDB-->>UtilitiesRepository: Created record
UtilitiesRepository-->>UtilitiesService: record
UtilitiesService-->>UtilitiesController: record
UtilitiesController-->>ApiGateway: 201 Created
ApiGateway-->>Client: 201 Created
Client->>ApiGateway: GET /utilities/intent-templates?page...
ApiGateway->>UtilitiesController: getAllIntentTemplates(criteria)
UtilitiesController->>UtilitiesService: getAllIntentTemplatesByQuery(...)
UtilitiesService->>UtilitiesRepository: getAllIntentTemplateByQuery(...)
UtilitiesRepository->>PrismaDB: SELECT COUNT + SELECT with filters/limit/offset
PrismaDB-->>UtilitiesRepository: paged list, total
UtilitiesRepository-->>UtilitiesService: paged result
UtilitiesService-->>UtilitiesController: paged DTO
UtilitiesController-->>ApiGateway: 200 OK {pagination, data[]}
ApiGateway-->>Client: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts (1)
446-461: UseCredentialTemplateRecordinstead ofcredential_templatestype at line 446.The
templateRecordvariable at line 446 should be typed asCredentialTemplateRecordrather thancredential_templates(Prisma type). While the broader Prisma type works due to structural typing, the functionsbuildSdJwtCredentialandbuildMdocCredential(lines 456, 459) explicitly expectCredentialTemplateRecord. Using the precise interface type improves type safety and clarifies intent.
🧹 Nitpick comments (29)
libs/prisma-service/prisma/migrations/20250814141522_add_supported_protocol/migration.sql (1)
80-94: Redundant function redefinition — definecheck_unique_protocols()once.The function
check_unique_protocols()is defined twice with identical logic. The secondCREATE OR REPLACEat line 81 overwrites the first. While this works because both bodies are identical, it's redundant and fragile—if someone later modifies one definition expecting it to affect only one table, both triggers would be impacted.Define the function once before creating both triggers, or use distinct function names per table.
🔎 Proposed refactor: define function once
CREATE TRIGGER enforce_unique_protocols BEFORE INSERT OR UPDATE ON organisation FOR EACH ROW EXECUTE FUNCTION check_unique_protocols(); --- contraints, triggers and migrations for ledger table +-- constraints, triggers and migrations for ledger table ALTER TABLE "ledgers" ADD COLUMN "supported_protocol" "CredentialExchangeProtocol"[]; -- Set default for new rows ALTER TABLE "ledgers" ALTER COLUMN "supported_protocol" SET DEFAULT ARRAY['DIDCOMM']::"CredentialExchangeProtocol"[]; -- Clean existing duplicates before constraint -- UPDATE "ledgers" -- SET "supported_protocol" = ARRAY( -- SELECT DISTINCT unnest("supported_protocol") -- ) -- WHERE "supported_protocol" IS NOT NULL; -- Backfill missing/empty with DIDCOMM UPDATE "ledgers" SET "supported_protocol" = ARRAY['DIDCOMM']::"CredentialExchangeProtocol"[] WHERE "supported_protocol" IS NULL OR cardinality("supported_protocol") = 0; -- Add no-duplicates constraint -- ALTER TABLE "ledgers" -- ADD CONSTRAINT supported_protocol_unique -- CHECK ( -- cardinality(supported_protocol) = cardinality( -- ARRAY(SELECT DISTINCT unnest(supported_protocol)) -- ) -- ); --- trigger function to check unique values -CREATE OR REPLACE FUNCTION check_unique_protocols() -RETURNS trigger AS $$ -BEGIN - IF (SELECT COUNT(*) FROM unnest(NEW.supported_protocol) v - GROUP BY v HAVING COUNT(*) > 1 LIMIT 1) IS NOT NULL THEN - RAISE EXCEPTION 'supported_protocol contains duplicates'; - END IF; - RETURN NEW; -END; -$$ LANGUAGE plpgsql; - +-- Reuse existing trigger function check_unique_protocols() CREATE TRIGGER enforce_unique_protocols BEFORE INSERT OR UPDATE ON ledgers FOR EACH ROW EXECUTE FUNCTION check_unique_protocols();apps/agent-service/src/agent-service.controller.ts (1)
382-384: Consider renaming the function to match the message pattern.The function is named
oidcGetAllCredentialOffersbut the message pattern is nowagent-service-oid4vc-get-credential-offers(without "all"). This creates a minor inconsistency between the handler name and its routing command.🔎 Proposed refactor
- async oidcGetAllCredentialOffers(payload: { url: string; orgId: string }): Promise<any> { + async oidcGetCredentialOffers(payload: { url: string; orgId: string }): Promise<any> { return this.agentServiceService.oidcGetAllCredentialOffers(payload.url, payload.orgId); }Note: This would also require updating the corresponding service method name for full consistency.
apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts (3)
66-72: Consider narrowing theattributestype.Using
anyfor theattributesfield reduces type safety. Since the comment indicates it's parsed at runtime asSdJwtTemplate | MdocTemplate, consider using a union type or a discriminated union based on the format field.🔎 Suggested type improvement
export interface CredentialTemplateRecord { id: string; name: string; format: string; signerOption: SignerOption; - attributes: any; // Stored as JSON in DB; parsed at runtime as SdJwtTemplate | MdocTemplate + attributes: SdJwtTemplate | MdocTemplate; // Stored as JSON in DB; parsed at runtime }
362-368: Simplify redundant ternary expression.On line 364,
templateSignerOptionis guaranteed to be defined at this point (line 309-313 throws if undefined), making the ternary check redundant.🔎 Suggested simplification
return { credentialSupportedId, - signerOptions: templateSignerOption ? templateSignerOption : undefined, + signerOptions: templateSignerOption, format: apiFormat, payload: payloadCopy, ...(disclosureFrame ? { disclosureFrame } : {}) };
374-423: Consider aligning signer validation withbuildSdJwtCredential.Unlike
buildSdJwtCredential(lines 290-313), this function doesn't validate thattemplateRecord.signerOptionmatches the expectedX5Cmethod before proceeding. While mdoc credentials may always require x5c, explicitly validating the template's signer option would ensure consistency and catch misconfigured templates early.🔎 Suggested validation addition
function buildMdocCredential( credentialRequest: CredentialRequestDtoLike, templateRecord: CredentialTemplateRecord, signerOptions: ISignerOption[], activeCertificateDetails: X509CertificateRecord[] ): BuiltCredential { let incomingPayload = { ...(credentialRequest.payload as Record<string, unknown>) }; if ( !credentialRequest.validityInfo || !credentialRequest.validityInfo.validFrom || !credentialRequest.validityInfo.validUntil ) { throw new UnprocessableEntityException(`${ResponseMessages.oidcIssuerSession.error.missingValidityInfo}`); } + // Validate template expects x5c signer for mdoc + if ( + templateRecord.signerOption !== SignerOption.X509_P256 && + templateRecord.signerOption !== SignerOption.X509_ED25519 + ) { + throw new UnprocessableEntityException( + `Mdoc credentials require an X.509 signer option, but template ${templateRecord.id} has "${templateRecord.signerOption}"` + ); + } + if (!signerOptions?.length || !signerOptions[0].x5c?.length) {libs/common/src/interfaces/interface.ts (1)
66-72: Clarify the distinction betweensortFieldandsortBy.Both
sortFieldandsortByare optional string properties, but their semantic difference isn't clear from the interface. Typically, sorting requires:
- A field/column name to sort by
- A direction (ascending/descending)
Consider renaming for clarity, e.g.,
sortField+sortDirection(orsortOrder), or consolidate if they serve the same purpose.🔎 Suggested clarification
export interface IPaginationSortingDto { pageNumber: number; pageSize: number; sortField?: string; - sortBy?: string; + sortDirection?: 'asc' | 'desc'; searchByText?: string; }libs/prisma-service/prisma/schema.prisma (1)
714-730: Consider adding a unique constraint to prevent duplicate intent-template mappings.The
intent_templatesmodel allows multiple records with the same(orgId, intentId, templateId)combination. If each organization should only map a specific intent to a specific template once, add a unique constraint.🔎 Suggested constraint
model intent_templates { id String @id @default(uuid()) @db.Uuid orgId String? @db.Uuid intentId String @db.Uuid templateId String @db.Uuid createDateTime DateTime @default(now()) @db.Timestamptz(6) createdBy String @db.Uuid lastChangedDateTime DateTime @default(now()) @db.Timestamptz(6) lastChangedBy String @db.Uuid organisation organisation? @relation(fields: [orgId], references: [id]) intent intents @relation(fields: [intentId], references: [id]) template verification_templates @relation(fields: [templateId], references: [id]) @@index([orgId]) @@index([intentId]) @@index([templateId]) + @@unique([orgId, intentId, templateId]) }libs/prisma-service/prisma/migrations/20251218061025_added_table_for_verification_templates/migration.sql (1)
2-14: Consider adding a unique constraint on (orgId, name).The
verification_templatestable allows multiple templates with the same name within an organization. While this may be intentional, it could lead to ambiguity when selecting templates by name. Consider adding a unique constraint to prevent duplicate template names per organization.🔎 Proposed constraint addition
CREATE TABLE "verification_templates" ( "id" UUID NOT NULL, "name" VARCHAR(500) NOT NULL, "templateJson" JSONB NOT NULL, "orgId" UUID NOT NULL, "createDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, "createdBy" UUID NOT NULL, "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, "lastChangedBy" UUID NOT NULL, "signerOption" "SignerOption" NOT NULL, CONSTRAINT "verification_templates_pkey" PRIMARY KEY ("id") ); +-- CreateIndex +CREATE UNIQUE INDEX "verification_templates_orgId_name_key" ON "verification_templates"("orgId", "name");apps/api-gateway/src/oid4vc-verification/dtos/verification-template.dto.ts (1)
14-42: Consider adding JSON schema validation for templateJson.The
templateJsonfield currently uses@IsObjectwhich only validates that the value is an object, not its internal structure. Given the complexity of the DCQL query format shown in the example, consider adding a custom validator or JSON schema validation to ensure the template structure is valid before persisting.This would prevent invalid templates from being stored and provide better error messages to users at creation time rather than at verification time.
apps/api-gateway/src/utilities/dtos/get-intent-template-by-intent-and-org.dto.ts (1)
10-13: Consider adding UUID validation for verifierOrgId.The
verifierOrgIdfield expects a UUID (based on the example), but only validates it as a string. Consider adding@IsUUID()validation to catch malformed UUIDs early.🔎 Proposed validation enhancement
+import { IsNotEmpty, IsString, IsUUID } from 'class-validator'; export class GetIntentTemplateByIntentAndOrgDto { @ApiProperty({ example: 'WEB_CHECKIN', description: 'Intent name' }) @IsNotEmpty() @IsString() intentName: string; @ApiProperty({ example: '123e4567-e89b-12d3-a456-426614174000', description: 'Verifier Organization ID' }) @IsNotEmpty() + @IsUUID() - @IsString() verifierOrgId: string; }apps/issuance/src/issuance.controller.ts (1)
31-31: Injected service appears unused.The
oidcIssuanceServiceis injected into the constructor but is not used in any of the message handler methods shown. If this service is intended for future use, consider adding a TODO comment. Otherwise, remove the unused dependency.libs/common/src/interfaces/oid4vp-verification.ts (1)
38-40: Consider using a union type or enum for the method field.The
methodfield is typed asstringbut the comment indicates it should be one of specific enum values ('DID', 'X509_P256', 'X509_ED25519'). Consider using a string union type or importing the appropriate enum to provide type safety and better IDE support.🔎 Proposed type improvement
+export type SignerMethod = 'DID' | 'X509_P256' | 'X509_ED25519'; + /** * Request signer configuration for OID4VP verification presentations */ export interface IRequestSigner { - method: string; // SignerOption enum value: 'DID', 'X509_P256', 'X509_ED25519' + method: SignerMethod; }Or, if a
SignerOptionenum is already defined elsewhere, import and use it directly.apps/api-gateway/src/utilities/dtos/intent-template.dto.ts (1)
4-19: Consider making orgId handling consistent between Create and Update DTOs.In
CreateIntentTemplateDto,orgIdis optional (Line 6-8), but inUpdateIntentTemplateDto(Line 22-25), it's required. This inconsistency could lead to confusion about when the organization context is needed. If the organization context is always known (e.g., from the route parameter), consider making it consistently optional or consistently required across both DTOs.apps/api-gateway/src/utilities/dtos/get-all-intent-templates.dto.ts (1)
20-24: Consider removing redundant@Type(() => String)decorator.The
@Transform(({ value }) => trim(value))already coerces the value to a string through thetrimfunction. The subsequent@Type(() => String)on Line 23 may be redundant since class-transformer applies@Transformbefore@Type. Consider testing if the@Typedecorator is necessary here.libs/common/src/utils/error-handler.util.ts (1)
44-48: Consider improving type safety for error response extraction.The type assertions using
anyon Lines 48 and 67 bypass TypeScript's type checking when extracting error messages. While functional, this reduces type safety.🔎 Proposed refactor for safer type handling
// Handle HttpException and subclasses (BadRequestException, ConflictException, etc.) if (error instanceof HttpException) { const status = error.getStatus(); const response = error.getResponse(); - const message = - 'string' === typeof response - ? response - : // eslint-disable-next-line @typescript-eslint/no-explicit-any - (response as any).message || 'HTTP Exception'; + const message = + 'string' === typeof response + ? response + : ('object' === typeof response && response && 'message' in response) + ? String(response.message) + : 'HTTP Exception'; return { message, statusCode: status }; }Similar pattern for Lines 65-67.
Also applies to: 65-67
apps/api-gateway/src/utilities/utilities.controller.ts (2)
126-132: Consider using centralized response messages for consistency.The existing codebase uses
ResponseMessagesconstants for success/error messages (see lines 63, 98). The new intent template endpoints use hardcoded strings like'Intent template created successfully'.For maintainability and consistency, consider adding these messages to the
ResponseMessagesconstant and referencing them here.Also applies to: 154-159, 185-190, 206-211, 230-235, 254-259, 281-286, 302-307
204-212: Add UUID validation for path parameters.The
id,intentId, andorgIdpath parameters should be validated as UUIDs to fail fast on malformed input. Other endpoints in the codebase typically useParseUUIDPipefor this purpose.🔎 Proposed fix
- async getIntentTemplateById(@Param('id') id: string, @Res() res: Response): Promise<Response> { + async getIntentTemplateById(@Param('id', ParseUUIDPipe) id: string, @Res() res: Response): Promise<Response> {- async getIntentTemplatesByIntentId(@Param('intentId') intentId: string, @Res() res: Response): Promise<Response> { + async getIntentTemplatesByIntentId(@Param('intentId', ParseUUIDPipe) intentId: string, @Res() res: Response): Promise<Response> {- async getIntentTemplatesByOrgId(@Param('orgId') orgId: string, @Res() res: Response): Promise<Response> { + async getIntentTemplatesByOrgId(@Param('orgId', ParseUUIDPipe) orgId: string, @Res() res: Response): Promise<Response> {Import
ParseUUIDPipefrom@nestjs/common(already imported partially).Also applies to: 228-236, 252-260
apps/api-gateway/src/utilities/utilities.service.ts (2)
143-153: Consider wrapping primitive payloads in objects for consistency.Methods
getIntentTemplateById,getIntentTemplatesByIntentId, andgetIntentTemplatesByOrgIdsend raw string values as payloads. While this may work, wrapping them in objects (e.g.,{ id }) provides consistency with other methods and makes it easier to extend the payload in the future without breaking changes.🔎 Proposed fix
async getIntentTemplateById(id: string): Promise<object> { - return this.natsClient.sendNatsMessage(this.serviceProxy, 'get-intent-template-by-id', id); + return this.natsClient.sendNatsMessage(this.serviceProxy, 'get-intent-template-by-id', { id }); } async getIntentTemplatesByIntentId(intentId: string): Promise<object[]> { - return this.natsClient.sendNatsMessage(this.serviceProxy, 'get-intent-templates-by-intent-id', intentId); + return this.natsClient.sendNatsMessage(this.serviceProxy, 'get-intent-templates-by-intent-id', { intentId }); } async getIntentTemplatesByOrgId(orgId: string): Promise<object[]> { - return this.natsClient.sendNatsMessage(this.serviceProxy, 'get-intent-templates-by-org-id', orgId); + return this.natsClient.sendNatsMessage(this.serviceProxy, 'get-intent-templates-by-org-id', { orgId }); }Note: This would require updating the corresponding microservice handlers to destructure from an object.
178-180: Consistency issue: deleteIntentTemplate sends raw string.Similar to the getter methods,
deleteIntentTemplatesends a raw stringidinstead of an object payload. This should be consistent with the pattern chosen for other methods.apps/oid4vc-verification/src/oid4vc-verification.repository.ts (2)
252-254: Logging object directly may produce uninformative output.
createTemplateDto=${createTemplateDto}will log[object Object]. UseJSON.stringify()for meaningful output.🔎 Proposed fix
this.logger.debug( - `[createVerificationTemplate] called for orgId=${orgId}, userId=${userId}, createTemplateDto=${createTemplateDto}` + `[createVerificationTemplate] called for orgId=${orgId}, userId=${userId}, createTemplateDto=${JSON.stringify(createTemplateDto)}` );
341-356: Same consideration applies to delete operation.The
deleteoperation will throw if the record doesn't exist. The service layer handles this by checking existence first, but this introduces a race condition. Consider using a try-catch to handle theP2025error gracefully.apps/utility/src/utilities.repository.ts (3)
174-176: Redundant eslint-disable comments.There are two consecutive
eslint-disablecomments (lines 174-176) with line 175 being empty. Clean up the redundancy.🔎 Proposed fix
// eslint-disable-next-line camelcase - - // eslint-disable-next-line camelcase async updateIntentTemplate(
240-244: Remove commented-out code.The commented-out
includeblock should be removed as it clutters the codebase. If needed for reference, it can be found in version control history.🔎 Proposed fix
organisation: { select: { name: true } } }, - // include: { - // organisation: true, - // intent: true, - // template: true - // }, orderBy: [{ orgId: { sort: 'desc', nulls: 'last' } }] // org-specific first, null last
326-337: Duplicate field mapping:templateNameandtemplateboth contain the same value.The mapping includes both
templateName: it.template?.nameandtemplate: it.template?.namewhich seems redundant. Verify if both are intentional or if one should be removed.🔎 Proposed fix (if `template` is not needed)
const data = intentTemplates.map((it) => ({ id: it.id, createDateTime: it.createDateTime, createdBy: it.createdBy, intentId: it.intentId, templateId: it.templateId, intent: it.intent?.name, templateName: it.template?.name, - template: it.template?.name, assignedToOrg: it.organisation?.name, templateCreatedByOrg: it.template?.organisation?.name }));apps/utility/src/utilities.service.ts (1)
127-142: Inconsistent null handling between similar methods.
getIntentTemplateByIdthrows an error when no template is found (line 131-132), butgetIntentTemplateByIntentAndOrgreturnsnull(line 196). This inconsistency may confuse API consumers. Consider documenting this behavior difference clearly or aligning the approaches.Also applies to: 189-207
apps/oid4vc-verification/src/oid4vc-verification.service.ts (4)
273-273: Unnecessaryawaiton synchronous function.
getAgentUrlis a synchronous function that returns a string (as seen in the relevant code snippet fromlibs/common/src/common.utils.ts). Theawaitis unnecessary here and on line 373.🔎 Proposed fix
- const url = await getAgentUrl(agentEndPoint, CommonConstants.OID4VP_VERIFICATION_SESSION); + const url = getAgentUrl(agentEndPoint, CommonConstants.OID4VP_VERIFICATION_SESSION);
351-357: Simplify x5cKeyType mapping using a lookup or direct enum value.The ternary expression can be simplified. Also, ensure this mapping aligns with how
SignerOptionenums correspond tox5cKeyTypevalues.🔎 Alternative approach using direct mapping
} else if (signerOption === SignerOption.X509_P256 || signerOption === SignerOption.X509_ED25519) { this.logger.debug('[createIntentBasedVerificationPresentation] X5C based request signer method selected'); + const keyType = signerOption === SignerOption.X509_P256 ? x5cKeyType.P256 : x5cKeyType.Ed25519; const activeCertificate = await this.oid4vpRepository.getCurrentActiveCertificate( orgId, - signerOption === SignerOption.X509_P256 ? x5cKeyType.P256 : x5cKeyType.Ed25519 + keyType );
600-604: Remove commented-out code.The commented-out error logging code should be removed to keep the codebase clean.
🔎 Proposed fix
} catch (error) { - // this.logger.error( - // `[createVerificationTemplate] - error: ${error?.response?.message ?? JSON.stringify(error?.response ?? error)}` - // ); this.logger.error(`[createVerificationTemplate] - error: ${error}`); throw new RpcException(error?.response ?? error); }
590-591: Logging object directly produces uninformative output.Same issue as in the repository -
createTemplateDto=${createTemplateDto}will log[object Object].🔎 Proposed fix
this.logger.debug( - `[createVerificationTemplate] called for orgId=${orgId}, user=${userDetails?.id}, createTemplateDto=${createTemplateDto}` + `[createVerificationTemplate] called for orgId=${orgId}, user=${userDetails?.id}, createTemplateDto=${JSON.stringify(createTemplateDto)}` );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
Dockerfiles/Dockerfile.oid4vc-issuanceapps/agent-service/src/agent-service.controller.tsapps/api-gateway/src/oid4vc-verification/dtos/create-intent-based-verification.dto.tsapps/api-gateway/src/oid4vc-verification/dtos/oid4vc-verifier-presentation.dto.tsapps/api-gateway/src/oid4vc-verification/dtos/verification-template.dto.tsapps/api-gateway/src/oid4vc-verification/oid4vc-verification.controller.tsapps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.tsapps/api-gateway/src/utilities/dtos/get-all-intent-templates-response.dto.tsapps/api-gateway/src/utilities/dtos/get-all-intent-templates.dto.tsapps/api-gateway/src/utilities/dtos/get-intent-template-by-intent-and-org.dto.tsapps/api-gateway/src/utilities/dtos/intent-template-item.dto.tsapps/api-gateway/src/utilities/dtos/intent-template.dto.tsapps/api-gateway/src/utilities/utilities.controller.tsapps/api-gateway/src/utilities/utilities.service.tsapps/issuance/src/issuance.controller.tsapps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.tsapps/oid4vc-issuance/libs/helpers/credential-sessions.builder.tsapps/oid4vc-issuance/src/oid4vc-issuance.service.tsapps/oid4vc-verification/interfaces/verification-template.interfaces.tsapps/oid4vc-verification/src/oid4vc-verification.controller.tsapps/oid4vc-verification/src/oid4vc-verification.repository.tsapps/oid4vc-verification/src/oid4vc-verification.service.tsapps/utility/src/utilities.controller.tsapps/utility/src/utilities.repository.tsapps/utility/src/utilities.service.tslibs/common/src/dtos/pagination.dto.tslibs/common/src/index.tslibs/common/src/interfaces/intents-template.interface.tslibs/common/src/interfaces/interface.tslibs/common/src/interfaces/oid4vp-verification.tslibs/common/src/interfaces/x509.interface.tslibs/common/src/nats.config.tslibs/common/src/response-messages/index.tslibs/common/src/utils/error-handler.util.tslibs/prisma-service/prisma/migrations/20250814141522_add_supported_protocol/migration.sqllibs/prisma-service/prisma/migrations/20251218061025_added_table_for_verification_templates/migration.sqllibs/prisma-service/prisma/schema.prisma
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-11T04:24:30.878Z
Learnt from: GHkrishna
Repo: credebl/platform PR: 1509
File: apps/oid4vc-verification/src/oid4vc-verification.service.ts:368-372
Timestamp: 2025-11-11T04:24:30.878Z
Learning: In the NATSClient class (libs/common/src/NATSClient.ts), the sendNatsMessage method accepts a command as a string parameter and internally converts it to the object pattern format { cmd: string } before sending via NATS, so callers should pass the command as a plain string (e.g., 'agent-create-oid4vp-verifier') rather than an object.
Applied to files:
apps/agent-service/src/agent-service.controller.ts
🧬 Code graph analysis (15)
apps/api-gateway/src/utilities/dtos/get-all-intent-templates.dto.ts (1)
libs/common/src/cast.helper.ts (2)
toNumber(58-76)trim(39-43)
libs/common/src/interfaces/intents-template.interface.ts (1)
libs/common/src/interfaces/interface.ts (1)
IPaginationSortingDto(66-72)
apps/api-gateway/src/utilities/dtos/get-all-intent-templates-response.dto.ts (1)
apps/api-gateway/src/utilities/dtos/intent-template-item.dto.ts (1)
IntentTemplateItemDto(3-33)
libs/common/src/utils/error-handler.util.ts (1)
apps/api-gateway/src/oid4vc-verification/dtos/oid4vc-verifier-presentation.dto.ts (1)
defaultMessage(297-300)
apps/api-gateway/src/utilities/utilities.service.ts (4)
apps/api-gateway/src/utilities/dtos/intent-template.dto.ts (2)
CreateIntentTemplateDto(4-19)UpdateIntentTemplateDto(21-36)libs/user-request/src/user-request.interface.ts (1)
IUserRequest(1-13)apps/api-gateway/src/utilities/dtos/get-all-intent-templates.dto.ts (1)
GetAllIntentTemplatesDto(7-65)libs/common/src/interfaces/intents-template.interface.ts (1)
IIntentTemplateList(24-32)
apps/utility/src/utilities.controller.ts (1)
libs/common/src/interfaces/intents-template.interface.ts (2)
IIntentTemplateSearchCriteria(3-10)IIntentTemplateList(24-32)
apps/oid4vc-verification/src/oid4vc-verification.repository.ts (1)
apps/oid4vc-verification/interfaces/verification-template.interfaces.ts (2)
CreateVerificationTemplate(3-7)UpdateCredentialTemplate(9-9)
libs/common/src/interfaces/x509.interface.ts (1)
libs/common/src/interfaces/interface.ts (1)
IPaginationSortingDto(66-72)
apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts (1)
apps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.ts (1)
ISignerOption(16-20)
apps/api-gateway/src/utilities/utilities.controller.ts (5)
apps/api-gateway/src/utilities/dtos/intent-template.dto.ts (2)
CreateIntentTemplateDto(4-19)UpdateIntentTemplateDto(21-36)libs/common/src/interfaces/response.interface.ts (1)
IResponse(8-13)apps/api-gateway/src/utilities/dtos/get-all-intent-templates-response.dto.ts (1)
GetAllIntentTemplatesResponseDto(4-25)apps/api-gateway/src/utilities/dtos/get-all-intent-templates.dto.ts (1)
GetAllIntentTemplatesDto(7-65)libs/common/src/interfaces/intents-template.interface.ts (1)
IIntentTemplateList(24-32)
apps/oid4vc-verification/src/oid4vc-verification.controller.ts (3)
libs/common/src/interfaces/oid4vp-verification.ts (1)
IPresentationRequest(45-53)apps/oid4vc-verification/interfaces/oid4vp-verification-sessions.interfaces.ts (1)
Oid4vpPresentationWh(3-11)apps/oid4vc-verification/interfaces/verification-template.interfaces.ts (2)
CreateVerificationTemplate(3-7)UpdateCredentialTemplate(9-9)
apps/oid4vc-verification/src/oid4vc-verification.service.ts (3)
libs/common/src/common.utils.ts (1)
getAgentUrl(71-141)apps/oid4vc-verification/interfaces/oid4vp-verification-sessions.interfaces.ts (1)
RequestSigner(23-23)apps/oid4vc-verification/interfaces/verification-template.interfaces.ts (2)
CreateVerificationTemplate(3-7)UpdateCredentialTemplate(9-9)
apps/utility/src/utilities.repository.ts (1)
libs/common/src/interfaces/intents-template.interface.ts (2)
IIntentTemplateSearchCriteria(3-10)IIntentTemplateList(24-32)
apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (3)
apps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.ts (1)
ISignerOption(16-20)libs/common/src/utils/error-handler.util.ts (1)
ErrorHandler(16-142)libs/logger/src/winstonLogger.ts (1)
error(123-125)
apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts (3)
apps/api-gateway/src/oid4vc-verification/dtos/create-intent-based-verification.dto.ts (1)
CreateIntentBasedVerificationDto(14-30)libs/common/src/interfaces/oid4vp-verification.ts (1)
IPresentationRequest(45-53)apps/api-gateway/src/oid4vc-verification/dtos/verification-template.dto.ts (2)
CreateVerificationTemplateDto(5-47)UpdateVerificationTemplateDto(49-49)
🔇 Additional comments (50)
libs/prisma-service/prisma/migrations/20250814141522_add_supported_protocol/migration.sql (4)
1-2: LGTM!The ENUM type definition is appropriate for the protocol distinction.
4-23: LGTM!Column addition with default and backfill logic for the
organisationtable is correctly structured. The backfill handles both NULL and empty array cases.
34-47: LGTM!The trigger function logic correctly detects duplicates in the array using
unnestwithGROUP BYandHAVING COUNT(*) > 1.
52-71: LGTM!Column addition and backfill for the
ledgerstable mirrors theorganisationtable correctly.apps/agent-service/src/agent-service.controller.ts (1)
380-384: Breaking change successfully executed: All callers have been updated to the new message pattern.The old pattern
agent-service-oid4vc-get-all-credential-offershas been completely removed from the codebase. The new patternagent-service-oid4vc-get-credential-offersis being used consistently across both the handler (agent-service.controller.ts:380) and its caller (oid4vc-issuance.service.ts). No orphaned references exist that would result in unrouted NATS messages.Minor naming inconsistency: The function is named
oidcGetAllCredentialOffers(includes "All") while the message pattern omits "all". This is maintainable as-is but could be addressed as an optional refactor for consistency.libs/common/src/nats.config.ts (1)
17-18: Remove commented queue property or document the architectural change.Commenting out
queue: serviceNameat line 18 removes load-balanced message distribution across service instances, causing all instances to receive and process duplicate messages if multiple replicas are deployed. Either restore the queue configuration for proper load balancing, or document why broadcast behavior is desired for this architecture.If this is temporary debug code, remove it. If intentional, ensure all message handlers are idempotent and verify this aligns with the deployment strategy.
libs/common/src/dtos/pagination.dto.ts (1)
5-25: LGTM!The
PaginationDtoclass is well-structured with appropriate validation decorators and sensible defaults. The constraints (minimum page number of 1, page size between 1-100) are reasonable for pagination.Dockerfiles/Dockerfile.oid4vc-issuance (1)
42-42: LGTM!The CMD instruction correctly sequences Prisma migrations and application startup. The multi-stage build pattern is well-structured.
libs/common/src/interfaces/x509.interface.ts (1)
2-2: LGTM!Good refactor to extend the shared
IPaginationSortingDtointerface, promoting consistency across search criteria interfaces in the codebase.Also applies to: 218-221
libs/prisma-service/prisma/schema.prisma (2)
687-701: Verify cascading delete behavior for verification templates.The
verification_templatesmodel has a requiredorgIdwith no explicitonDeletebehavior specified. By default, Prisma usesRESTRICT, which will prevent deleting an organization if it has associated verification templates.If organizations should be deletable with their templates, consider adding
onDelete: Cascade. If the current behavior is intentional (protect templates from accidental deletion), this is fine as-is.
716-716: Clarify whetherorgIdshould be optional onintent_templates.In
verification_templates,orgIdis required, but inintent_templates, it's optional (String?). This asymmetry may be intentional (allowing global/shared intent mappings), but could also lead to orphaned records if not handled properly in application logic.If every intent-template mapping should belong to an organization, consider making
orgIdrequired for consistency withverification_templates.libs/prisma-service/prisma/migrations/20251218061025_added_table_for_verification_templates/migration.sql (1)
55-56: Verify the ON DELETE RESTRICT strategy for organization deletion.The foreign key constraint
verification_templates.orgId → organisation.idusesON DELETE RESTRICT, which prevents deletion of organizations that have verification templates. This means administrators must manually delete all templates before deleting an organization.Consider whether:
ON DELETE CASCADEwould be more appropriate to automatically clean up templates when an organization is deleted, or- A soft-delete pattern (e.g.,
deletedAttimestamp) would better preserve audit history.Would you like me to generate a script to check if there are existing organization deletion flows that might be affected by this constraint?
apps/api-gateway/src/oid4vc-verification/dtos/verification-template.dto.ts (1)
49-49: LGTM!Using
PartialTypefor the update DTO is the correct NestJS pattern, allowing partial updates of verification templates.libs/common/src/index.ts (1)
4-4: LGTM!The addition of the error handler utility export makes centralized error handling available across modules, improving consistency in error responses.
apps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.ts (1)
16-20: Ensure runtime validation of conditional fields.The
ISignerOptioninterface correctly models the signer configuration, but the optionaldidandx5cfields should be conditionally required based onmethod:
- When
method === SignerMethodOption.DID,didshould be required- When
method === SignerMethodOption.X5C,x5cshould be requiredEnsure validation logic enforces these constraints at runtime (e.g., in service layer or using custom validators).
Would you like me to generate a custom class-validator decorator to enforce these conditional requirements?
libs/common/src/interfaces/oid4vp-verification.ts (1)
45-53: LGTM with caveat on 'any' types.The
IPresentationRequestinterface appropriately models the presentation request structure. The use ofanyforpresentationExchangeanddcqlis pragmatic given they can have complex, varied structures, though consider creating more specific types as these structures stabilize.apps/oid4vc-verification/interfaces/verification-template.interfaces.ts (1)
3-7: LGTM!The
CreateVerificationTemplateinterface correctly models the verification template creation payload with appropriate types.apps/api-gateway/src/oid4vc-verification/dtos/create-intent-based-verification.dto.ts (1)
1-30: LGTM!The DTO structure is well-designed with proper validation decorators. The use of
@IsDefined()ensures strict validation,@ValidateNested()with@Type()correctly handles nested object validation, and the enum constraints provide type safety formethodandresponseModefields.apps/api-gateway/src/oid4vc-verification/oid4vc-verification.controller.ts (2)
310-365: LGTM!The intent-based verification presentation endpoint is well-structured with proper authentication, authorization (OrgRoles.OWNER), UUID validation, and error handling. The endpoint correctly delegates to the service layer and returns appropriate HTTP status codes.
521-720: LGTM!The verification template CRUD endpoints are consistently implemented with:
- Proper authentication and authorization guards
- UUID validation with informative error messages
- Appropriate HTTP verbs and status codes
- Structured logging for debugging
- Clean separation of concerns (controller delegates to service)
apps/api-gateway/src/utilities/dtos/get-all-intent-templates-response.dto.ts (1)
1-25: LGTM!The response DTO follows standard pagination patterns with clear property types and proper Swagger documentation. The use of
IntentTemplateItemDtoarray for the data field ensures type safety.libs/common/src/interfaces/intents-template.interface.ts (1)
1-37: LGTM!The interface definitions are well-structured and properly typed. The use of nullable types (
| null) for optional fields and the extension ofIPaginationSortingDtodemonstrates good TypeScript practices. These interfaces provide clear contracts for intent template data structures across the application.apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (3)
610-616: LGTM! Improved error handling.The adoption of
ErrorHandler.categorize()andErrorHandler.format()provides consistent, structured error responses across the microservice. This makes error tracking and debugging easier compared to the previous error handling approach.
299-314: LGTM! Proper signerOption propagation.The
signerOptionfield is correctly propagated through template creation and update flows. The conditional inclusion in the update payload (Lines 393-394) follows the established pattern for optional fields and ensures only defined values are passed to the repository.Also applies to: 383-395
531-531: Remove unusedResolvedSignerOptionimport.
ResolvedSignerOptionis imported on line 60 but never used in the service file. The active type isISignerOption[]which correctly aligns with the interface definition and downstream population logic (lines 534-572).apps/api-gateway/src/utilities/utilities.controller.ts (1)
103-132: LGTM for create endpoint structure.The create endpoint correctly applies JWT authentication, uses the
@User()decorator to capture user context, and follows the existing response pattern. The DTO validation will handle input validation.apps/oid4vc-verification/src/oid4vc-verification.controller.ts (4)
78-97: LGTM for improved typing.The
sessionRequestparameter is now properly typed asIPresentationRequestinstead ofany, improving type safety and code clarity.
99-120: LGTM for intent-based verification handler.The new handler correctly extracts payload fields, logs the operation, and delegates to the service. The destructuring and logging are consistent with existing patterns in this controller.
130-150: LGTM for template CRUD handlers.The create and get template handlers follow the established patterns with proper logging and delegation to the service layer.
171-178: LGTM for delete template handler.The delete handler correctly extracts
orgIdandtemplateIdfrom the payload with appropriate logging.apps/api-gateway/src/utilities/utilities.service.ts (2)
136-141: LGTM for createIntentTemplate.The method correctly spreads the DTO and includes user context in the payload, following established patterns.
155-167: LGTM for query and lookup methods.The
getAllIntentTemplatesByQueryandgetIntentTemplateByIntentAndOrgmethods correctly structure their payloads as objects.apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts (3)
22-35: LGTM for intent-based verification method.The method correctly extracts the signer option from the nested DTO structure and constructs the payload appropriately.
89-100: LGTM for improved session request typing.The
sessionRequestparameter is now properly typed asIPresentationRequest, enhancing type safety.
141-151: LGTM for template CRUD operations structure.The create, get, and delete template methods follow the established service patterns with appropriate logging and payload construction.
Also applies to: 153-157, 172-176
apps/utility/src/utilities.controller.ts (4)
53-61: LGTM for createIntentTemplate handler.The handler correctly receives a structured payload with all required fields and delegates to the service.
78-91: LGTM for query and lookup handlers.The
getAllIntentTemplateByQueryandgetIntentTemplateByIntentAndOrghandlers properly destructure their payloads and delegate to the service.
93-107: LGTM for updateIntentTemplate handler.The handler correctly extracts all fields from the payload and restructures them for the service call.
109-112: Verify deleteIntentTemplate payload structure.Similar to the getter methods, this handler expects a raw string
id. Confirm this matches what the API gateway service sends.apps/api-gateway/src/utilities/dtos/intent-template-item.dto.ts (2)
1-33: DTO structure looks good for API documentation.The DTO is well-structured with appropriate Swagger annotations. Optional fields are correctly marked with
required: falseandnullable: true. Consider adding a brief JSDoc comment to describe the DTO's purpose for better documentation.
25-26: Remove thestatefield from the DTO.The
statefield is defined in this DTO but does not exist in theintent_templatesPrisma schema. Since it's not available in the database model, it cannot be populated. Either remove this field from the DTO or add it to the schema and populate it in thegetAllIntentTemplateByQueryrepository method mapping.apps/oid4vc-verification/src/oid4vc-verification.repository.ts (3)
311-339: Prisma update with non-existent record throws an error rather than returning null.If the template doesn't exist for the given
idandorgId, Prisma'supdatewill throw aP2025error (Record not found). The service layer should handle this case, or you could useupdateManyand checkcount, or fetch-then-update. The current service does check existence first, which is fine but creates a TOCTOU window.
247-272: LGTM - Create operation is well implemented.The create operation follows the established pattern in this repository with proper logging and error handling. The audit fields (
createdBy,lastChangedBy) are correctly populated.
274-292: LGTM - Get operations are correctly implemented.The query methods properly filter by
orgIdfor tenant isolation and include sensible ordering bycreateDateTime desc.apps/utility/src/utilities.repository.ts (2)
265-322: Well-implemented paginated query with dynamic filtering.The
getAllIntentTemplateByQuerymethod is well-structured with proper pagination calculation, dynamic where clause building, and text search support. The use ofmode: 'insensitive'for search is a good practice.
217-263: Good org-specific vs global template resolution logic.The
orderBywithnulls: 'last'correctly prioritizes org-specific templates over global templates when querying by intent name.apps/utility/src/utilities.service.ts (2)
94-125: Good input validation and error handling pattern.The
createIntentTemplatemethod demonstrates good practices with explicit input validation, proper error categorization usingErrorHandler, and structured logging. The destructuring ofuserfrom data before passing to repository is clean.
172-187: Clean implementation of paginated query service method.The method correctly extracts the search criteria from the payload and delegates to the repository.
apps/oid4vc-verification/src/oid4vc-verification.service.ts (2)
292-391: Well-structured intent-based verification flow.The
createIntentBasedVerificationPresentationmethod is well-implemented with:
- Proper agent and verifier validation
- NATS-based template fetching
- Correct handling of DID vs X509 signer options
- Comprehensive logging throughout
The separation of concerns between fetching template data and building the session request is clean.
585-607: Template CRUD operations follow established patterns.The verification template CRUD methods (
createVerificationTemplate,updateVerificationTemplate,deleteVerificationTemplate) properly check for existence before update/delete operations and use consistent error handling patterns withRpcException.Also applies to: 628-661, 663-683
| this.logger.debug( | ||
| `[createVerificationTemplate] Called with orgId=${orgId}, user=${userDetails?.id}, createTemplateDto=${createTemplateDto}` | ||
| ); |
There was a problem hiding this comment.
Log statement will not serialize the DTO properly.
The log at line 148 uses template literal interpolation for createTemplateDto, which will result in [object Object] in the logs. Use JSON.stringify() for meaningful output, or omit the DTO from the log entirely (as done in other methods).
🔎 Proposed fix
this.logger.debug(
- `[createVerificationTemplate] Called with orgId=${orgId}, user=${userDetails?.id}, createTemplateDto=${createTemplateDto}`
+ `[createVerificationTemplate] Called with orgId=${orgId}, user=${userDetails?.id}, templateName=${createTemplateDto?.name}`
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.logger.debug( | |
| `[createVerificationTemplate] Called with orgId=${orgId}, user=${userDetails?.id}, createTemplateDto=${createTemplateDto}` | |
| ); | |
| this.logger.debug( | |
| `[createVerificationTemplate] Called with orgId=${orgId}, user=${userDetails?.id}, templateName=${createTemplateDto?.name}` | |
| ); |
🤖 Prompt for AI Agents
In apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts
around lines 147 to 149, the debug log interpolates createTemplateDto directly
which will serialize to [object Object]; update the log to either remove the DTO
(to match other methods) or call JSON.stringify(createTemplateDto) with a safe
replacer/try-catch to avoid throwing on circular refs, and include the resulting
string in the logger call so the DTO is readable in logs.
apps/oid4vc-verification/interfaces/verification-template.interfaces.ts
Outdated
Show resolved
Hide resolved
| private static getStatusCodeByMessage(message: string): number { | ||
| const lowerMessage = message.toLowerCase(); | ||
|
|
||
| // Conflict/Duplicate errors → 409 | ||
| if ( | ||
| lowerMessage.includes('already exists') || | ||
| lowerMessage.includes('already assigned') || | ||
| lowerMessage.includes('duplicate') || | ||
| lowerMessage.includes('conflict') | ||
| ) { | ||
| return HttpStatus.CONFLICT; | ||
| } | ||
|
|
||
| // Validation/Bad Request errors → 400 | ||
| if ( | ||
| lowerMessage.includes('invalid') || | ||
| lowerMessage.includes('required') || | ||
| lowerMessage.includes('validation') || | ||
| lowerMessage.includes('bad request') | ||
| ) { | ||
| return HttpStatus.BAD_REQUEST; | ||
| } | ||
|
|
||
| // Not Found errors → 404 | ||
| if (lowerMessage.includes('not found') || lowerMessage.includes('does not exist')) { | ||
| return HttpStatus.NOT_FOUND; | ||
| } | ||
|
|
||
| // Unauthorized/Forbidden errors → 401/403 | ||
| if (lowerMessage.includes('unauthorized') || lowerMessage.includes('not authorized')) { | ||
| return HttpStatus.UNAUTHORIZED; | ||
| } | ||
| if (lowerMessage.includes('forbidden') || lowerMessage.includes('permission denied')) { | ||
| return HttpStatus.FORBIDDEN; | ||
| } | ||
|
|
||
| // Default to 500 Internal Server Error | ||
| return HttpStatus.INTERNAL_SERVER_ERROR; | ||
| } |
There was a problem hiding this comment.
Keyword-based status inference may be fragile.
The getStatusCodeByMessage method infers HTTP status codes by searching for keywords in error messages (e.g., "already exists" → 409, "not found" → 404). This approach is brittle because:
- Message changes break inference: If error messages are refactored or localized, status codes could become incorrect
- Ambiguous keywords: Messages like "The requested item was not found, please validate your input" match both 404 (Line 102) and 400 (Line 92-96) patterns
- Case sensitivity: While using
.toLowerCase()helps, non-English error messages won't match
Consider enhancing error handling by:
- Having error classes explicitly declare their status codes
- Using error codes/types instead of message parsing
- Documenting that error messages should include these keywords
🤖 Prompt for AI Agents
libs/common/src/utils/error-handler.util.ts lines 78-116: the current
keyword-based getStatusCodeByMessage is brittle and ambiguous; instead modify
error handling to prefer explicit status indicators: update the caller to pass
an Error-like object with a numeric status/statusCode or use specific custom
error classes (e.g., ValidationError, NotFoundError, ConflictError) and have
getStatusCodeByMessage (or a new resolver) first check for
error.status/statusCode or instanceof specific error classes and return that
value, only falling back to keyword parsing as a last resort; ensure the
resolution order treats validation/bad-request checks before not-found when
falling back, and add a short comment/docstring that callers should supply
explicit status or use the custom error classes rather than relying on message
parsing.
|
|
||
|
|
||
|
|
||
| -- contraints, triggers and migrations for ledger table |
There was a problem hiding this comment.
Typo in comment.
Minor: "contraints" should be "constraints".
🔎 Proposed fix
--- constraints, triggers and migrations for ledger table
+-- constraints, triggers and migrations for ledger tableCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
libs/prisma-service/prisma/migrations/20250814141522_add_supported_protocol/migration.sql
around line 51: the comment contains a typo "contraints"; update the comment to
read "constraints" to correct the spelling while preserving the rest of the
comment text and file formatting.
| CREATE TABLE "intents" ( | ||
| "id" UUID NOT NULL, | ||
| "name" VARCHAR(500) NOT NULL, | ||
| "description" TEXT, | ||
| "createDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "createdBy" UUID NOT NULL, | ||
| "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "lastChangedBy" UUID NOT NULL, | ||
|
|
||
| CONSTRAINT "intents_pkey" PRIMARY KEY ("id") | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add a unique constraint on the intent name.
The intents table appears to store globally-defined intent types (e.g., 'WEB_CHECKIN'), but lacks a unique constraint on the name column. This allows duplicate intent names, which will cause ambiguity when querying by intent name (as seen in GetIntentTemplateByIntentAndOrgDto.intentName).
🔎 Proposed constraint addition
CREATE TABLE "intents" (
"id" UUID NOT NULL,
"name" VARCHAR(500) NOT NULL,
"description" TEXT,
"createDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP,
"createdBy" UUID NOT NULL,
"lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP,
"lastChangedBy" UUID NOT NULL,
CONSTRAINT "intents_pkey" PRIMARY KEY ("id")
);
+-- CreateIndex
+CREATE UNIQUE INDEX "intents_name_key" ON "intents"("name");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CREATE TABLE "intents" ( | |
| "id" UUID NOT NULL, | |
| "name" VARCHAR(500) NOT NULL, | |
| "description" TEXT, | |
| "createDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| "createdBy" UUID NOT NULL, | |
| "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| "lastChangedBy" UUID NOT NULL, | |
| CONSTRAINT "intents_pkey" PRIMARY KEY ("id") | |
| ); | |
| CREATE TABLE "intents" ( | |
| "id" UUID NOT NULL, | |
| "name" VARCHAR(500) NOT NULL, | |
| "description" TEXT, | |
| "createDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| "createdBy" UUID NOT NULL, | |
| "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| "lastChangedBy" UUID NOT NULL, | |
| CONSTRAINT "intents_pkey" PRIMARY KEY ("id") | |
| ); | |
| -- CreateIndex | |
| CREATE UNIQUE INDEX "intents_name_key" ON "intents"("name"); |
🤖 Prompt for AI Agents
In
libs/prisma-service/prisma/migrations/20251218061025_added_table_for_verification_templates/migration.sql
around lines 17 to 27, the intents table lacks a uniqueness constraint on the
name column allowing duplicate intent names; add a unique constraint for the
name column by altering the CREATE TABLE to include a UNIQUE constraint on
"name" (or add an explicit UNIQUE index/ALTER TABLE ... ADD CONSTRAINT ...
UNIQUE on "name") so intent names are enforced as unique across the table.
...ervice/prisma/migrations/20251218061025_added_table_for_verification_templates/migration.sql
Show resolved
Hide resolved
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/utility/src/utilities.service.ts (1)
87-145: Alert handling implementations look good, but validate emailFrom.The implementations for
handleLedgerAlertandsendWithRetryaddress the past review concerns about empty methods. The throttling, concurrency control, and retry logic are well-implemented.However, Line 107 assigns
emailDto.emailFrom = platformConfigData?.emailFromwithout validating thatemailFromis defined. If the platform config doesn't have anemailFromvalue, the email service may fail.🔎 Proposed fix to validate emailFrom
const platformConfigData = await this.utilitiesRepository.getPlatformConfigDetails(); if (!platformConfigData) { throw new NotFoundException(ResponseMessages.issuance.error.platformConfigNotFound); } +if (!platformConfigData.emailFrom) { + this.logger.error('[handleLedgerAlert] - emailFrom not configured in platform config'); + return; +} + emailDto.emailFrom = platformConfigData.emailFrom;apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (2)
299-313: Consider validatingsignerOptionbefore persistence.The
signerOptionis extracted from the input and stored directly in the database metadata without validation. While the downstream usage (lines 534-571) safely handles invalid values by checking against specificSignerOptionenum values, adding validation here would prevent invalid data from being persisted.🔎 Suggested validation approach
const { name, description, format, canBeRevoked, appearance, signerOption } = credentialTemplate; + +// Validate signerOption if provided +if (signerOption !== undefined && !Object.values(SignerOption).includes(signerOption)) { + throw new BadRequestException(`Invalid signerOption: ${signerOption}`); +} const checkNameExist = await this.oid4vcIssuanceRepository.getTemplateByNameForIssuer(name, issuerId);
383-394: Consider validatingsignerOptionbefore persistence.Similar to
createTemplate, thesignerOptioninupdateTemplateis stored without validation. Adding validation would ensure data integrity.🔎 Suggested validation approach
const { name, description, format, canBeRevoked, appearance, signerOption } = normalized; const attributes = instanceToPlain(normalized.template); + +// Validate signerOption if provided +if (signerOption !== undefined && !Object.values(SignerOption).includes(signerOption)) { + throw new BadRequestException(`Invalid signerOption: ${signerOption}`); +} const payload = {
♻️ Duplicate comments (2)
apps/utility/src/utilities.service.ts (2)
148-179: Potential TypeError ifuseris undefined.Line 159 checks
!user.idbut ifuseritself is undefined/null, accessinguser.idwill throw a TypeError before the validation logic runs. This was flagged in past reviews and remains unaddressed.🔎 Proposed fix
// Validate input -if (!templateData.intentId || !templateData.templateId || !user.id) { +if (!templateData.intentId || !templateData.templateId || !user?.id) { throw new Error('Invalid data: intentId, templateId, and user are required'); }
263-282: Validate user parameter before destructuring.Line 268 destructures
userfromdata, then Line 271 accessesuser.id. If the caller passes an object whereuseris undefined or null, this will fail. Add validation or use optional chaining.🔎 Proposed fix
async updateIntentTemplate( id: string, data: { orgId: string; intentId: string; templateId: string; user: { id: string } } ): Promise<object> { try { const { user, ...templateData } = data; + + if (!user?.id) { + throw new Error('Invalid data: user is required'); + } + const intentTemplate = await this.utilitiesRepository.updateIntentTemplate(id, { ...templateData, lastChangedBy: user.id });
🧹 Nitpick comments (1)
apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (1)
610-615: Consider applyingErrorHandlerconsistently across all methods.The centralized error handling using
ErrorHandler.categorizeandErrorHandler.formatis a good improvement, but it's currently only applied tocreateOidcCredentialOffer. Other methods (e.g.,createTemplate,updateTemplate,oidcIssuerCreate) still use the older pattern of directly throwingRpcException. Applying this pattern consistently would provide uniform error structure and better error categorization across the service.Example methods that could benefit from the same error handling pattern:
createTemplate(line 355)updateTemplate(line 453)oidcIssuerCreate(line 150-151)oidcIssuerUpdate(line 193-194)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/oid4vc-issuance/src/oid4vc-issuance.service.tsapps/utility/src/utilities.service.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/utility/src/utilities.service.ts (2)
libs/common/src/utils/error-handler.util.ts (1)
ErrorHandler(16-142)libs/common/src/interfaces/intents-template.interface.ts (2)
IIntentTemplateSearchCriteria(3-10)IIntentTemplateList(24-32)
🔇 Additional comments (5)
apps/utility/src/utilities.service.ts (3)
181-241: Retrieval methods look good with consistent error handling.The retrieval methods follow a consistent pattern using
ErrorHandler.categorizefor error handling and appropriate behavior (throwing for single record retrieval, returning empty arrays for list queries). The implementation is clean and maintainable.
243-261: Good null handling for template lookup.The method properly returns
nullwhen no template is found and logs this case for debugging. The error handling is consistent with other methods.
284-296: Delete method looks good with consistent error handling.The method follows the same error handling pattern as other CRUD operations. If validation of template existence before deletion is needed, it should be handled at the repository layer.
apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (2)
52-52: LGTM!The new imports for
ISignerOptionandErrorHandlerare properly added and used appropriately in the file.Also applies to: 66-66
531-531: No changes needed—type change is correct.The function
buildCredentialOfferPayloadalready expectsISignerOption[], and the declared type at line 531 matches this requirement. No type compatibility issues exist.
| issuerDetails.publicIssuerId | ||
| ); | ||
|
|
||
| this.logger.debug(`Creating OIDC Credential Offer for :`, buildOidcCredentialOffer); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find files containing buildCredentialOfferPayload or CredentialOfferPayload
fd -t f -e ts -e tsx | head -20 | xargs rg -l "buildCredentialOfferPayload|CredentialOfferPayload" 2>/dev/null | head -10Repository: credebl/platform
Length of output: 42
🏁 Script executed:
# Also search in the specific file mentioned in the review
head -n 610 apps/oid4vc-issuance/src/oid4vc-issuance.service.ts | tail -n 20Repository: credebl/platform
Length of output: 1091
🏁 Script executed:
# Search for the type definition of buildOidcCredentialOffer or CredentialOfferPayload
rg -n "type CredentialOfferPayload|interface CredentialOfferPayload|const buildCredentialOfferPayload|function buildCredentialOfferPayload" -A 15 --type tsRepository: credebl/platform
Length of output: 3360
🏁 Script executed:
# Find the BuiltCredentialOfferBase type definition
rg -n "type BuiltCredentialOfferBase|interface BuiltCredentialOfferBase" -A 20 --type tsRepository: credebl/platform
Length of output: 2124
🏁 Script executed:
# View more of the buildCredentialOfferPayload function to see what data is assembled
rg -n "function buildCredentialOfferPayload" -A 50 --type tsRepository: credebl/platform
Length of output: 5869
🏁 Script executed:
# Continue viewing the buildCredentialOfferPayload function
rg -n "function buildCredentialOfferPayload" -A 100 --type ts | tail -60Repository: credebl/platform
Length of output: 6787
🏁 Script executed:
# Also check what BuiltCredential type contains
rg -n "type BuiltCredential|interface BuiltCredential" -A 30 --type tsRepository: credebl/platform
Length of output: 3895
Avoid logging sensitive credential data in debug statements.
The debug log at line 602 logs the entire buildOidcCredentialOffer object, which contains credential payloads with potentially sensitive user information (names, addresses, email addresses, etc.). This poses privacy and compliance risks even at debug level.
Recommend either:
- Removing this log statement
- Sanitizing the logged data to exclude the
payloadfields - Using targeted logging that captures only non-sensitive metadata (e.g., credential IDs, issuer info, flow type)
🤖 Prompt for AI Agents
In apps/oid4vc-issuance/src/oid4vc-issuance.service.ts around line 602, the
debug log prints the full buildOidcCredentialOffer object which can include
sensitive credential payloads; remove or replace this statement so sensitive
fields are not logged — either delete the log, log a sanitized copy excluding
any payload/PII fields, or log only non-sensitive metadata (credential ID,
issuer, flow type), and ensure any helper used to sanitize explicitly omits
payload and related personal data before logging.
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
libs/common/src/utils/error-handler.util.ts (1)
82-120: Keyword-based status inference remains fragile (previously flagged).As noted in the previous review, the keyword-based approach in
getStatusCodeByMessageis brittle because message changes, ambiguous keywords, and non-English text can lead to incorrect status codes. The past review suggested preferring explicit status codes or custom error classes with declared status codes, falling back to keyword parsing only as a last resort.apps/api-gateway/src/utilities/utilities.controller.ts (1)
195-241: Route ordering issue persists (previously flagged).Although the
ParseUUIDPipeon the:idroute (lines 255-262) now rejects non-UUID values like "intent", the routes are still ordered incorrectly. Requests to/intent-templates/intent/xyzwill match the generic:idroute first and fail validation, rather than matching the more specific/intent/:intentIdroute.The recommended fix remains the same: reorder the routes to place specific paths before generic ones:
/intent-templates/by-intent-and-org/intent-templates/intent/:intentId/intent-templates/org/:orgId/intent-templates/:idapps/oid4vc-issuance/src/oid4vc-issuance.service.ts (1)
601-601: Debug log may expose sensitive credential data (previously flagged).As noted in the previous review, the debug log at line 601 logs the entire
buildOidcCredentialOfferobject, which may contain sensitive credential payloads with PII (names, addresses, emails, etc.). The recommendation remains: remove this log, sanitize the data to exclude payload fields, or log only non-sensitive metadata.apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts (1)
141-151: Log statement will not serialize the DTO properly.Line 148 uses template literal interpolation for
createTemplateDto, which will result in[object Object]in the logs. Either remove the DTO from the log (consistent with other methods in this file) or useJSON.stringify()with error handling.🔎 Proposed fix
this.logger.debug( - `[createVerificationTemplate] Called with orgId=${orgId}, user=${userDetails?.id}, createTemplateDto=${createTemplateDto}` + `[createVerificationTemplate] Called with orgId=${orgId}, user=${userDetails?.id}` );apps/oid4vc-verification/src/oid4vc-verification.service.ts (1)
287-386: Potential null reference error: add null safety for nested template properties.Line 333 accesses
templateData?.template?.templateJson.dcqlwithout optional chaining ontemplateJson, which will throw a TypeError iftemplateJsonis null or undefined. Add explicit validation or use full optional chaining.🔎 Proposed fix
// Build session request using fetched template + if (!templateData?.template?.templateJson?.dcql) { + throw new BadRequestException( + `Template for intent '${intent}' does not contain valid DCQL configuration` + ); + } + const sessionRequest = { verifierId: verifier.publicVerifierId, - dcql: templateData?.template?.templateJson.dcql, + dcql: templateData.template.templateJson.dcql, responseMode, requestSigner: null };
🧹 Nitpick comments (4)
apps/api-gateway/src/utilities/utilities.controller.ts (1)
282-321: Add ParseUUIDPipe to PUT and DELETE endpoints for consistency.The
updateIntentTemplate(line 288) anddeleteIntentTemplate(line 313) endpoints lack UUID validation on the:idparameter, unlike the GET endpoint at line 255 which includesParseUUIDPipe. For consistency and early rejection of invalid IDs, consider adding the same validation:@Param('id', new ParseUUIDPipe({ exceptionFactory: (): Error => { throw new BadRequestException(ResponseMessages.oid4vpIntentToTemplate.error.invalidId); } }))apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (1)
358-454: Consider including signerOption in rollback payload.The rollback logic (lines 422-429) doesn't include
signerOption, which means if a template update changes thesignerOptionand the agent call fails, the rollback won't restore the original signer option value. This could leave the template in an inconsistent state.🔎 Suggested fix
const rollbackPayload = { name: template.name, description: template.description, format: template.format, canBeRevoked: template.canBeRevoked, attributes: template.attributes, appearance: template.appearance, - issuerId: template.issuerId + issuerId: template.issuerId, + signerOption: template.signerOption };apps/oid4vc-verification/src/oid4vc-verification.repository.ts (1)
309-337: Parameter naming inconsistency: useupdateTemplateDtofor clarity.The parameter is named
updateCredentialTemplate(line 311), which is inconsistent with the verification domain and doesn't match naming conventions used elsewhere. Consider renaming toupdateTemplateDtoorupdateVerificationTemplatefor consistency.🔎 Proposed fix
async updateVerificationTemplate( templateId: string, - updateCredentialTemplate: UpdateVerificationTemplate, + updateTemplateDto: UpdateVerificationTemplate, orgId: string, userId: string ): Promise<object> { this.logger.debug( `[updateVerificationTemplate] called with templateId=${templateId}, orgId=${orgId}, userId=${userId}` ); try { const updated = await this.prisma.verification_templates.update({ where: { id: templateId, orgId }, data: { - name: updateCredentialTemplate.name, - templateJson: updateCredentialTemplate.templateJson, - signerOption: updateCredentialTemplate.signerOption, + name: updateTemplateDto.name, + templateJson: updateTemplateDto.templateJson, + signerOption: updateTemplateDto.signerOption, lastChangedBy: userId } });apps/oid4vc-verification/src/oid4vc-verification.controller.ts (1)
152-169: Consider renamingupdateCredentialTemplatetoupdateTemplateDtofor domain consistency.While the payload field name now matches between the API Gateway (line 165 in
apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts) and this controller, the nameupdateCredentialTemplateis inconsistent with the verification domain. Consider renaming toupdateTemplateDtoorupdateVerificationTemplatefor clarity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.tsapps/api-gateway/src/utilities/utilities.controller.tsapps/oid4vc-issuance/libs/helpers/credential-sessions.builder.tsapps/oid4vc-issuance/src/oid4vc-issuance.service.tsapps/oid4vc-verification/interfaces/verification-template.interfaces.tsapps/oid4vc-verification/src/oid4vc-verification.controller.tsapps/oid4vc-verification/src/oid4vc-verification.repository.tsapps/oid4vc-verification/src/oid4vc-verification.service.tslibs/common/src/response-messages/index.tslibs/common/src/utils/error-handler.util.tslibs/prisma-service/prisma/migrations/20251223061418_add_intent_templates_unique_indexes/migration.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/common/src/response-messages/index.ts
🧰 Additional context used
🧬 Code graph analysis (7)
apps/oid4vc-verification/src/oid4vc-verification.service.ts (3)
apps/oid4vc-verification/interfaces/oid4vp-verification-sessions.interfaces.ts (1)
RequestSigner(23-23)libs/common/src/common.utils.ts (1)
getAgentUrl(71-141)apps/oid4vc-verification/interfaces/verification-template.interfaces.ts (2)
CreateVerificationTemplate(3-7)UpdateVerificationTemplate(9-9)
apps/oid4vc-verification/src/oid4vc-verification.repository.ts (1)
apps/oid4vc-verification/interfaces/verification-template.interfaces.ts (2)
CreateVerificationTemplate(3-7)UpdateVerificationTemplate(9-9)
apps/api-gateway/src/utilities/utilities.controller.ts (7)
apps/api-gateway/src/utilities/dtos/intent-template.dto.ts (2)
CreateIntentTemplateDto(4-19)UpdateIntentTemplateDto(21-36)libs/user-request/src/user-request.interface.ts (1)
IUserRequest(1-13)libs/common/src/interfaces/response.interface.ts (1)
IResponse(8-13)apps/api-gateway/src/utilities/dtos/get-all-intent-templates-response.dto.ts (1)
GetAllIntentTemplatesResponseDto(4-25)apps/api-gateway/src/utilities/dtos/get-all-intent-templates.dto.ts (1)
GetAllIntentTemplatesDto(7-65)libs/common/src/interfaces/intents-template.interface.ts (1)
IIntentTemplateList(24-32)apps/api-gateway/src/utilities/dtos/get-intent-template-by-intent-and-org.dto.ts (1)
GetIntentTemplateByIntentAndOrgDto(4-14)
apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts (3)
apps/api-gateway/src/oid4vc-verification/dtos/create-intent-based-verification.dto.ts (1)
CreateIntentBasedVerificationDto(14-30)libs/common/src/interfaces/oid4vp-verification.ts (1)
IPresentationRequest(45-53)apps/api-gateway/src/oid4vc-verification/dtos/verification-template.dto.ts (2)
CreateVerificationTemplateDto(5-47)UpdateVerificationTemplateDto(49-49)
apps/oid4vc-verification/src/oid4vc-verification.controller.ts (3)
libs/common/src/interfaces/oid4vp-verification.ts (1)
IPresentationRequest(45-53)apps/oid4vc-verification/interfaces/oid4vp-verification-sessions.interfaces.ts (1)
Oid4vpPresentationWh(3-11)apps/oid4vc-verification/interfaces/verification-template.interfaces.ts (2)
CreateVerificationTemplate(3-7)UpdateVerificationTemplate(9-9)
apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (2)
apps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.ts (1)
ISignerOption(16-20)libs/common/src/utils/error-handler.util.ts (1)
ErrorHandler(17-175)
apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts (2)
apps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.ts (1)
ISignerOption(16-20)apps/oid4vc-issuance/interfaces/oid4vc-template.interfaces.ts (1)
SdJwtTemplate(3-6)
🔇 Additional comments (39)
libs/prisma-service/prisma/migrations/20251223061418_add_intent_templates_unique_indexes/migration.sql (1)
1-7: The migration is correctly designed for the current schema.The partial unique indexes properly enforce uniqueness for organization-scoped and global intent-template mappings using the
WHERE orgId IS NOT NULLandWHERE orgId IS NULLpattern. Theintent_templatesmodel does not have adeletedAtfield, so no soft delete filtering is required. Although other tables in the schema use soft deletes, this table is not configured for soft deletion.If existing duplicate rows violate these constraints, the migration will fail with a clear error. Run a pre-check only if duplicates are suspected.
Likely an incorrect or invalid review comment.
libs/common/src/utils/error-handler.util.ts (4)
1-27: LGTM!The error handling entry point is well-structured. The
handlemethod correctly uses theneverreturn type and provides a clean interface for converting errors to RpcException.
29-75: Well-structured error categorization with appropriate fallbacks.The categorization method implements a sensible priority chain (RpcException → HttpException → Error → unknown) and safely defaults to 500 for unrecognized error shapes.
122-155: LGTM!The
formatmethod provides robust error formatting with multiple fallbacks. The check for empty JSON ({}or[]) before falling back toutil.inspectis a good touch for producing richer debug output.
157-175: LGTM!The
getSafeReplacercorrectly handles circular references with a WeakSet and serializes bigint values, ensuring robust JSON stringification.apps/api-gateway/src/utilities/utilities.controller.ts (3)
106-134: LGTM!The
createIntentTemplateendpoint is properly secured with JWT authentication, correctly passes user context, and uses appropriate HTTP 201 status for resource creation.
136-162: LGTM!The
getAllIntentTemplatesendpoint correctly implements query-based pagination with proper response typing.
164-193: LGTM!The
getIntentTemplateByIntentAndOrgendpoint correctly handles the lookup by intent name and organization ID, with appropriate conditional messaging.apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (4)
52-52: LGTM!The new imports (
ISignerOption,CredentialOfferPayload,ErrorHandler) are appropriate and correctly sourced for the enhanced signer option handling and error management.Also applies to: 59-59, 65-65
291-356: LGTM!The
createTemplatemethod correctly extracts and persists thesignerOptionin template metadata, with appropriate rollback logic on agent call failures.
530-530: LGTM!The type change to
ISignerOption[]correctly aligns with the updated signer option interface and provides better type safety.
609-614: LGTM!The new error handling using
ErrorHandler.categorizeprovides consistent, structured error responses and improves logging with detailed error formatting.apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts (4)
1-72: LGTM!The new
CredentialTemplateRecordinterface provides good type safety for template records, with clear documentation that attributes are parsed at runtime. The use of Prisma'sSignerOptiontype is appropriate.
278-369: LGTM!The
buildSdJwtCredentialfunction correctly maps Prisma signer options to builder types and implements thorough certificate validation, including:
- Signer option mapping (DID/X509 → did/x5c)
- Certificate matching verification
- Credential validity window validation
Error messages appropriately include template IDs for debugging.
374-423: LGTM!The
buildMdocCredentialfunction correctly enforces mdoc-specific requirements (validityInfo, x5c signer) and implements consistent certificate validation matching the SdJwt flow.
425-462: LGTM!The
buildCredentialOfferPayloadfunction correctly integrates the updated signer option handling, passingISignerOption[]and certificate details to the credential building functions.apps/oid4vc-verification/interfaces/verification-template.interfaces.ts (2)
1-7: LGTM!The
CreateVerificationTemplateinterface is well-structured with appropriate types. UsingSignerOptionfrom@prisma/clientensures type safety for the signer configuration, and the optionalsignerOptionfield provides flexibility.
9-9: LGTM!The
UpdateVerificationTemplateinterface correctly extendsPartial<CreateVerificationTemplate>, following the standard TypeScript pattern for update operations. The naming is consistent with the verification domain.apps/oid4vc-verification/src/oid4vc-verification.repository.ts (4)
11-11: LGTM!The import statement correctly brings in the new interface types needed for verification template operations.
272-290: LGTM!The method correctly implements flexible template retrieval with optional filtering by templateId. The conditional where clause pattern is clean, and error handling is appropriate.
292-307: LGTM!The method correctly retrieves a verification template by ID with organization-level isolation. The use of
findFirstwith bothidandorgIdensures proper access control.
339-354: LGTM!The method correctly implements template deletion with proper organization-level isolation. Prisma will automatically throw an error if the template doesn't exist, which is appropriate behavior.
apps/oid4vc-verification/src/oid4vc-verification.controller.ts (6)
3-8: LGTM!The import statements correctly bring in the necessary types for the new verification template and intent-based verification functionality.
78-97: LGTM!The type change from
anytoIPresentationRequestfor thesessionRequestparameter improves type safety and makes the API contract more explicit.
99-120: LGTM!The intent-based verification presentation endpoint is well-structured with appropriate logging and follows the established controller pattern.
130-141: LGTM!The template creation endpoint correctly accepts and passes the DTO to the service layer with appropriate logging.
143-150: LGTM!The template retrieval endpoint correctly handles optional templateId filtering with clear logging.
171-178: LGTM!The template deletion endpoint is correctly structured with appropriate logging.
apps/oid4vc-verification/src/oid4vc-verification.service.ts (5)
32-33: LGTM!The import statements correctly bring in the necessary types and enums for verification template operations and X.509 certificate handling.
232-263: LGTM!The corrected field access
sessionRequest.requestSigner.method(line 245) properly aligns with theIPresentationRequesttype structure. The X.509 certificate retrieval logic correctly mapsSignerOptionenum values tox5cKeyTypeenum values.
580-597: LGTM!The template creation method correctly delegates to the repository with appropriate error handling and logging.
599-616: LGTM!The template retrieval method correctly handles both single and multiple template queries with appropriate error handling for not-found cases.
618-673: LGTM!Both update and delete methods correctly implement existence checks before performing operations, with appropriate error handling and logging.
apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts (6)
8-11: LGTM!The import statements correctly bring in the necessary DTOs and interfaces for the new verification template and intent-based verification endpoints.
22-35: LGTM!The intent-based verification method correctly extracts parameters from the DTO and forwards them via NATS with appropriate logging.
89-100: LGTM!The type change from
anytoIPresentationRequestimproves type safety for the verification session creation endpoint.
153-157: LGTM!The template retrieval method correctly forwards the request to the microservice with appropriate logging.
159-170: LGTM!The field name
updateCredentialTemplate(line 165) correctly matches the receiver's expectation in the controller. While the naming could be more consistent with the verification domain, the functionality is correct.
172-176: LGTM!The template deletion method correctly forwards the request to the microservice with appropriate logging.
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/oid4vc-verification/src/oid4vc-verification.repository.ts (1)
291-306: Consider usingfindUniqueif a unique constraint exists.The method correctly retrieves a template scoped to the organization. If the
idfield has a unique constraint in the Prisma schema, consider usingfindUniqueinstead offindFirstfor better semantics and potential performance benefits.🔎 Optional refactor
- const result = await this.prisma.verification_templates.findFirst({ + const result = await this.prisma.verification_templates.findUnique({ where: { - id: templateId, - orgId + id: templateId + }, + where: { + orgId } });Note: Only apply this if
idis the primary key. IffindUniquerequires a composite unique constraint, the currentfindFirstapproach is appropriate.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/oid4vc-verification/src/oid4vc-verification.repository.ts
🔇 Additional comments (4)
apps/oid4vc-verification/src/oid4vc-verification.repository.ts (4)
271-289: LGTM! Clean implementation with flexible filtering.The conditional spread operator for the optional
templateIdfilter is elegant and maintains type safety. The descending order bycreateDateTimeensures the most recent templates appear first.
308-336: LGTM! Secure update operation with proper scoping.The method correctly scopes the update to the organization using both
idandorgIdin the where clause, preventing cross-organization template modifications. ThelastChangedBytracking is properly maintained.
247-269: LGTM! Implementation correctly follows the established patterns. All methods properly scope operations toorgIdfor multi-tenancy, include appropriate logging and error handling, and reference valid schema fields. Prisma schema verification confirms theverification_templatestable exists with all required fields (name,templateJson,signerOption,orgId,createdBy,lastChangedBy).
338-353: Secure deletion with proper scoping, but verify cascade behavior.The method correctly scopes deletion to the organization, preventing cross-organization deletions. However, the
verification_templatesmodel has relatedintent_templatesrecords via foreign key withoutonDelete: Cascadeconfigured. This means deletion will fail if dependent records exist. Verify that referential actions like cascading deletes are properly defined at the Prisma schema level, or ensure callers handle cleanup of dependent records before deletion.
What?
Added intent to template mapping APIs and using it in creating proof request.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.