-
Notifications
You must be signed in to change notification settings - Fork 71
feat: verification webhook #1507
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
Conversation
Signed-off-by: Tipu_Singh <[email protected]>
Signed-off-by: Tipu_Singh <[email protected]>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughUpdates implement OID4VP presentation webhook handling across verification and issuance services, introduce a new database table and Prisma model for presentations, add stricter validation to credential templates with conditional signer options, update NATS configuration with new service constants, and expand response message structures. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant GW as API Gateway
participant NATS as NATS Bus
participant Service as Verification Service
participant Repo as Repository
participant DB as Database
Client->>GW: POST /wh/:id/openid4vc-verification
GW->>GW: Validate Oid4vpPresentationWhDto
GW->>Service: oid4vpPresentationWebhook(dto, id)
Service->>NATS: Publish webhook-oid4vp-presentation
NATS->>Service: webhook-oid4vp-presentation message
Service->>Service: Resolve organization from contextCorrelationId
Service->>Repo: storeOid4vpPresentationDetails(dto, orgId)
Repo->>DB: Upsert into oid4vp_presentations
alt Update existing
DB->>DB: Set lastChangedBy, update state
else Create new
DB->>DB: Initialize with orgId, createdBy, presentationId
end
Repo-->>Service: Return upsert result
Service-->>GW: Return stored details
GW-->>Client: 201 CREATED
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (1)
103-109: Consider simplifying the conditional check.The condition
batchCredentialIssuanceSize && 0 < batchCredentialIssuanceSizeis redundant. Since0 < batchCredentialIssuanceSizealready returnsfalseforundefined,null,0, orNaN, the truthy check is unnecessary.Optionally apply this simplification:
- ...(batchCredentialIssuanceSize && 0 < batchCredentialIssuanceSize + ...(0 < batchCredentialIssuanceSize ? { batchCredentialIssuance: { batchSize: batchCredentialIssuanceSize } } : {})Additionally, the empty object in the false branch is unnecessary:
- ...(0 < batchCredentialIssuanceSize - ? { - batchCredentialIssuance: { - batchSize: batchCredentialIssuanceSize - } - } - : {}) + ...(0 < batchCredentialIssuanceSize && { + batchCredentialIssuance: { + batchSize: batchCredentialIssuanceSize + } + })apps/api-gateway/src/oid4vc-issuance/dtos/oid4vp-presentation-wh.dto.ts (1)
1-27: Consider adding more comprehensive validation.The DTO currently only validates that fields are strings. Consider adding additional validation decorators for better input validation and API documentation:
@IsNotEmpty()for required fields@IsUUID()for theidfield if it's a UUID@IsDateString()or@IsISO8601()forcreatedAtandupdatedAtfields- Example values in
@ApiProperty({ example: '...' })for better Swagger documentationApply this diff to enhance validation:
import { ApiProperty } from '@nestjs/swagger'; -import { IsString } from 'class-validator'; +import { IsString, IsNotEmpty, IsUUID, IsDateString } from 'class-validator'; export class Oid4vpPresentationWhDto { - @ApiProperty() + @ApiProperty({ example: '550e8400-e29b-41d4-a716-446655440000' }) + @IsNotEmpty() + @IsUUID() - @IsString() id!: string; - @ApiProperty() + @ApiProperty({ example: 'completed' }) + @IsNotEmpty() @IsString() state!: string; - @ApiProperty() + @ApiProperty() + @IsNotEmpty() @IsString() authorizationRequestId!: string; - @ApiProperty() + @ApiProperty({ example: '2025-11-06T08:37:15Z' }) + @IsNotEmpty() + @IsDateString() - @IsString() createdAt!: string; - @ApiProperty() + @ApiProperty({ example: '2025-11-06T08:37:15Z' }) + @IsNotEmpty() + @IsDateString() - @IsString() updatedAt!: string; - @ApiProperty() + @ApiProperty() + @IsNotEmpty() @IsString() contextCorrelationId!: string; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts(2 hunks)apps/api-gateway/src/oid4vc-issuance/dtos/oid4vp-presentation-wh.dto.ts(1 hunks)apps/api-gateway/src/oid4vc-verification/oid4vc-verification.controller.ts(4 hunks)apps/api-gateway/src/oid4vc-verification/oid4vc-verification.module.ts(1 hunks)apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts(2 hunks)apps/oid4vc-issuance/interfaces/oid4vc-issuance.interfaces.ts(1 hunks)apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts(1 hunks)apps/oid4vc-issuance/src/main.ts(1 hunks)apps/oid4vc-issuance/src/oid4vc-issuance.service.ts(1 hunks)apps/oid4vc-verification/interfaces/oid4vp-verification-sessions.interfaces.ts(1 hunks)apps/oid4vc-verification/src/oid4vc-verification.controller.ts(2 hunks)apps/oid4vc-verification/src/oid4vc-verification.repository.ts(2 hunks)apps/oid4vc-verification/src/oid4vc-verification.service.ts(3 hunks)libs/common/src/common.constant.ts(1 hunks)libs/common/src/response-messages/index.ts(1 hunks)libs/prisma-service/prisma/migrations/20251105180717_created_table_oid4vp_presentation/migration.sql(1 hunks)libs/prisma-service/prisma/schema.prisma(4 hunks)
🔇 Additional comments (19)
apps/oid4vc-issuance/src/main.ts (1)
15-15: The environment variable change is correctly scoped and requires no action.The update from
ISSUANCE_NKEY_SEEDtoOIDC4VC_ISSUANCE_NKEY_SEEDin the oid4vc-issuance app is a localized naming improvement that aligns with the service constant. Each service (webhook, issuance, oid4vc-issuance) maintains its own environment variable naming convention—the old variable remains appropriately used in other services. The constant exists and both entry points in the oid4vc-issuance app are consistent.Likely an incorrect or invalid review comment.
apps/oid4vc-issuance/interfaces/oid4vc-issuance.interfaces.ts (1)
72-72: LGTM! Interface change aligns with conditional logic.Making
batchCredentialIssuanceoptional correctly reflects the conditional usage in the service layer, where it's only included whenbatchCredentialIssuanceSizeis valid.apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts (1)
221-226: LGTM! Validation properly constrains signerOption for Mdoc credentials.The conditional validation correctly enforces that:
signerOptionmust be defined when format is MdocsignerOptioncannot be DID when format is MdocThis aligns well with the X.509 certificate requirements for mdoc credentials and complements the runtime validation needed in the builder.
apps/api-gateway/src/oid4vc-verification/oid4vc-verification.module.ts (1)
19-19: LGTM! Correct NATS service configuration.This fix properly routes the verification module to
OIDC4VC_VERIFICATION_SERVICEinstead of the incorrectISSUANCE_SERVICEconstant. This aligns the module with its verification-specific functionality.apps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.ts (1)
86-94: LGTM! Webhook method properly integrated.The new
oid4vpPresentationWebhookmethod follows the established service pattern for NATS message handling and correctly forwards presentation webhook data to the verification service.apps/oid4vc-verification/src/oid4vc-verification.repository.ts (1)
193-228: LGTM! Repository method properly implements presentation storage.The upsert operation correctly handles webhook idempotency using
verificationSessionIdas the unique key. The field mapping fromauthorizationRequestIdtopresentationIdis appropriate, and error handling is consistent with other repository methods.libs/prisma-service/prisma/migrations/20251105180717_created_table_oid4vp_presentation/migration.sql (1)
1-21: LGTM! Migration properly creates presentation tracking table.The table schema is well-designed with:
- Appropriate UUID primary key
- Unique constraint on
verificationSessionIdsupporting idempotent upsert operations- Foreign key with
ON DELETE RESTRICTpreventing orphaned presentations- NOT NULL constraints ensuring data integrity
- Timestamp defaults for audit tracking
apps/oid4vc-verification/interfaces/oid4vp-verification-sessions.interfaces.ts (1)
1-8: LGTM! Interface clearly defines webhook payload structure.The
Oid4vpPresentationWhinterface provides proper typing for the verification presentation webhook payload, with descriptive property names that align with the database schema and service layer usage.apps/oid4vc-verification/src/oid4vc-verification.controller.ts (2)
7-7: LGTM!The import correctly brings in the
Oid4vpPresentationWhinterface for use in the new webhook handler.
96-102: LGTM!The webhook handler is properly implemented as a NATS MessagePattern listener that accepts the presentation webhook payload and delegates processing to the service layer.
libs/prisma-service/prisma/schema.prisma (3)
155-155: LGTM!The new relation correctly links the organisation model to the oid4vp_presentations table.
610-622: LGTM!The new
oid4vp_presentationsmodel is well-structured with:
- Proper UUID primary key
- Unique constraint on
verificationSessionIdfor efficient lookups- Complete audit trail fields (createdBy, createDateTime, lastChangedDateTime, lastChangedBy)
- Foreign key relationship to the organisation table
624-628: LGTM!The addition of
X509_ED25519to theSignerOptionenum aligns with the X.509 certificate support mentioned in the PR objectives.apps/api-gateway/src/oid4vc-verification/oid4vc-verification.controller.ts (4)
30-31: LGTM!The additional Swagger imports support the new webhook endpoint's exclusion from API documentation.
38-38: LGTM!Adding
IResponseTypeto support the webhook endpoint's response structure.
50-50: LGTM!Correctly imports the DTO for the webhook payload validation.
301-301: LGTM!Correctly updated to use the session-specific response message key.
apps/oid4vc-verification/src/oid4vc-verification.service.ts (2)
30-30: LGTM!Correctly imports the interface for the webhook payload.
286-290: LGTM!The URL construction correctly uses the
OIDC_VERIFIER_SESSION_RESPONSE_GET_BY_IDconstant for building the verification session response endpoint.
apps/api-gateway/src/oid4vc-verification/oid4vc-verification.controller.ts
Show resolved
Hide resolved
apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts
Outdated
Show resolved
Hide resolved
| oid4vpSession: { | ||
| success: { | ||
| create: 'OID4VP session verifier created successfully.', | ||
| update: 'OID4V session verifier updated successfully.', | ||
| delete: 'OID4VP session verifier deleted successfully.', | ||
| fetch: 'OID4VP session verifier(s) fetched successfully.', | ||
| getById: 'OID4VP session verifier details fetched successfully.' | ||
| }, | ||
| error: { | ||
| notFound: 'OID4VP session verifier not found.', | ||
| invalidId: 'Invalid OID4VP session verifier ID.', | ||
| createFailed: 'Failed to create OID4VP session verifier.', | ||
| updateFailed: 'Failed to update OID4VP session verifier.', | ||
| deleteFailed: 'Failed to delete OID4VP session verifier.', | ||
| notFoundIssuerDisplay: 'Issuer display not found.', | ||
| notFoundIssuerDetails: 'Issuer details not found.', | ||
| verifierIdAlreadyExists: 'Verifier ID already exists for this verifier.', | ||
| deleteTemplate: 'Error while deleting template.' | ||
| } | ||
| }, |
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.
Fix typo in success message.
Line 582 contains a typo: "OID4V session" should be "OID4VP session" (missing 'P').
Apply this diff to fix the typo:
- update: 'OID4V session verifier updated successfully.',
+ update: 'OID4VP session verifier updated successfully.',📝 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.
| oid4vpSession: { | |
| success: { | |
| create: 'OID4VP session verifier created successfully.', | |
| update: 'OID4V session verifier updated successfully.', | |
| delete: 'OID4VP session verifier deleted successfully.', | |
| fetch: 'OID4VP session verifier(s) fetched successfully.', | |
| getById: 'OID4VP session verifier details fetched successfully.' | |
| }, | |
| error: { | |
| notFound: 'OID4VP session verifier not found.', | |
| invalidId: 'Invalid OID4VP session verifier ID.', | |
| createFailed: 'Failed to create OID4VP session verifier.', | |
| updateFailed: 'Failed to update OID4VP session verifier.', | |
| deleteFailed: 'Failed to delete OID4VP session verifier.', | |
| notFoundIssuerDisplay: 'Issuer display not found.', | |
| notFoundIssuerDetails: 'Issuer details not found.', | |
| verifierIdAlreadyExists: 'Verifier ID already exists for this verifier.', | |
| deleteTemplate: 'Error while deleting template.' | |
| } | |
| }, | |
| oid4vpSession: { | |
| success: { | |
| create: 'OID4VP session verifier created successfully.', | |
| update: 'OID4VP session verifier updated successfully.', | |
| delete: 'OID4VP session verifier deleted successfully.', | |
| fetch: 'OID4VP session verifier(s) fetched successfully.', | |
| getById: 'OID4VP session verifier details fetched successfully.' | |
| }, | |
| error: { | |
| notFound: 'OID4VP session verifier not found.', | |
| invalidId: 'Invalid OID4VP session verifier ID.', | |
| createFailed: 'Failed to create OID4VP session verifier.', | |
| updateFailed: 'Failed to update OID4VP session verifier.', | |
| deleteFailed: 'Failed to delete OID4VP session verifier.', | |
| notFoundIssuerDisplay: 'Issuer display not found.', | |
| notFoundIssuerDetails: 'Issuer details not found.', | |
| verifierIdAlreadyExists: 'Verifier ID already exists for this verifier.', | |
| deleteTemplate: 'Error while deleting template.' | |
| } | |
| }, |
🤖 Prompt for AI Agents
In libs/common/src/response-messages/index.ts around lines 579 to 598, the
success.create message has a typo ("OID4V session" missing the 'P'); update that
string to "OID4VP session verifier created successfully." so it consistently
matches the other OID4VP messages.
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.
@tipusinghaw I think we missed this
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
@tipusinghaw
|
Signed-off-by: Tipu_Singh <[email protected]>
|
That's great :-) |
GHkrishna
left a comment
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.
Looks good just that one spelling mistake needs to be corrected
| oid4vpSession: { | ||
| success: { | ||
| create: 'OID4VP session verifier created successfully.', | ||
| update: 'OID4V session verifier updated successfully.', | ||
| delete: 'OID4VP session verifier deleted successfully.', | ||
| fetch: 'OID4VP session verifier(s) fetched successfully.', | ||
| getById: 'OID4VP session verifier details fetched successfully.' | ||
| }, | ||
| error: { | ||
| notFound: 'OID4VP session verifier not found.', | ||
| invalidId: 'Invalid OID4VP session verifier ID.', | ||
| createFailed: 'Failed to create OID4VP session verifier.', | ||
| updateFailed: 'Failed to update OID4VP session verifier.', | ||
| deleteFailed: 'Failed to delete OID4VP session verifier.', | ||
| notFoundIssuerDisplay: 'Issuer display not found.', | ||
| notFoundIssuerDetails: 'Issuer details not found.', | ||
| verifierIdAlreadyExists: 'Verifier ID already exists for this verifier.', | ||
| deleteTemplate: 'Error while deleting template.' | ||
| } | ||
| }, |
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.
@tipusinghaw I think we missed this
* feat: verification webhook implementation Signed-off-by: Tipu_Singh <[email protected]> * fix: batch size and nats config Signed-off-by: Tipu_Singh <[email protected]> * fix: review comment Signed-off-by: Tipu_Singh <[email protected]> --------- Signed-off-by: Tipu_Singh <[email protected]>
* feat: verification webhook implementation Signed-off-by: Tipu_Singh <[email protected]> * fix: batch size and nats config Signed-off-by: Tipu_Singh <[email protected]> * fix: review comment Signed-off-by: Tipu_Singh <[email protected]> --------- Signed-off-by: Tipu_Singh <[email protected]>
* feat: verification webhook implementation Signed-off-by: Tipu_Singh <[email protected]> * fix: batch size and nats config Signed-off-by: Tipu_Singh <[email protected]> * fix: review comment Signed-off-by: Tipu_Singh <[email protected]> --------- Signed-off-by: Tipu_Singh <[email protected]>



What
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores