feat: oid4vc issuance #1455
feat: oid4vc issuance #1455
Conversation
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 WalkthroughIntroduces full OIDC credential-offer management (create/update/get-by-id/get-all/delete) across API Gateway, Issuance, and Agent Service via new NATS patterns and HTTP integrations. Renames issuer listing topics. Updates DTOs/interfaces, narrows template format options, adds URL mappings/constants/messages, and removes NATS queue option. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant G as API Gateway (OIDC REST)
participant I as Issuance MS (NATS)
participant A as Agent Service (NATS)
participant X as External Agent API (HTTP)
rect rgb(242,248,255)
note over C,G: Update/Get/List/Delete Credential Offer
C->>G: REST: update/get/list/delete
G->>I: NATS: oidc-credential-offer-*
I->>A: NATS: agent-service-oidc-*
A->>X: HTTP: PUT/GET/GET/DELETE
X-->>A: HTTP response
A-->>I: NATS response
I-->>G: NATS response
G-->>C: HTTP 200/204 with body
end
sequenceDiagram
autonumber
participant C as Client
participant G as API Gateway
participant I as Issuance MS
participant A as Agent Service
participant X as External Agent API
rect rgb(245,255,245)
note over G,I: Issuers retrieval topic rename
C->>G: REST: get issuers
G->>I: NATS: oidc-get-issuers-issuance
I->>A: NATS: oidc-get-issuers-agent-service
A->>X: HTTP: GET issuers
X-->>A: issuers list
A-->>I: issuers list
I-->>G: issuers list (validated)
G-->>C: HTTP 200
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
|
review @coderabbitai |
|
@tipusinghaw I'll review the changes in PR #1455 that adds OIDC issuance functionality. This looks like a substantial feature addition with issuer creation, template creation, and offer-creation capabilities. ✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/api-gateway/src/issuance/oidc.controller.ts (3)
436-439: Re-enable auth on create-offer endpoint.Currently unauthenticated creation of credential offers is possible; restore guards/roles.
- // @ApiBearerAuth() - // @Roles(OrgRoles.OWNER) - // @UseGuards(AuthGuard('jwt'), OrgRolesGuard) + @ApiBearerAuth() + @Roles(OrgRoles.OWNER) + @UseGuards(AuthGuard('jwt'), OrgRolesGuard)
421-427: Critical: forward issuerId in deleteTemplate call (gateway → microservice)Gateway currently omits issuerId while the issuance microservice's deleteTemplate(templateId, orgId, userDetails, issuerId) requires it — either forward issuerId from the gateway or remove the microservice requirement.
- apps/api-gateway/src/issuance/oidc.controller.ts:421 — pass issuerId into issueCredentialService.deleteTemplate(...).
- apps/api-gateway/src/issuance/issuance.service.ts:314-316 — extend method signature to accept issuerId and set payload = { templateId, orgId, userDetails, issuerId } (so NATS message includes issuerId).
- apps/issuance/src/issuance.controller.ts:225-227 and apps/issuance/src/oidc-issuance.service.ts:399-406 — these expect issuerId; keep as-is if you forward issuerId, or update them if you intentionally drop issuerId.
326-334: Fix service signature mismatch: include issuerId in gateway payload or make issuerId optional.API gateway calls findByIdTemplate(user, orgId, templateId) and sends payload { templateId, orgId, userDetails } (no issuerId), but the issuance service expects findByIdTemplate(templateId, orgId, userDetails, issuerId). Reconcile the contract.
- apps/api-gateway/src/issuance/oidc.controller.ts:326 — calls this.issueCredentialService.findByIdTemplate(user, orgId, templateId)
- apps/api-gateway/src/issuance/issuance.service.ts:330–332 — builds payload { templateId, orgId, userDetails } and sends 'oidc-template-find-id'
- apps/issuance/src/issuance.controller.ts:238–240 — destructures { templateId, orgId, userDetails, issuerId } and forwards issuerId
- apps/issuance/src/oidc-issuance.service.ts:434 — signature async findByIdTemplate(templateId: string, orgId: string, userDetails: user, issuerId: string)
Either add issuerId to the gateway payload (preserve parameter ordering) or update the issuance service/controller to accept the (userDetails, orgId, templateId) shape or treat issuerId as optional and handle undefined.
apps/issuance/src/oidc-issuance.service.ts (1)
189-195: Fix null-check logic to avoid property access on undefined.The condition dereferences getIssuerDetails when it may be null.
- if (!getIssuerDetails && getIssuerDetails.publicIssuerId) { + if (!getIssuerDetails || !getIssuerDetails.publicIssuerId) { throw new NotFoundException(ResponseMessages.oidcIssuer.error.notFound); }
🧹 Nitpick comments (22)
apps/api-gateway/src/issuance/dtos/oidc-issuer.dto.ts (1)
251-255: Swagger vs validation mismatch (required vs optional).
authorizationServerConfigsis optional in validation but marked required in Swagger via@ApiProperty.Apply this diff:
- @ApiProperty({ + @ApiPropertyOptional({ description: 'Configuration of the authorization server', type: AuthorizationServerConfigDto }) @IsOptional() @ValidateNested() @Type(() => AuthorizationServerConfigDto) - authorizationServerConfigs?: AuthorizationServerConfigDto; + authorizationServerConfigs?: AuthorizationServerConfigDto;apps/issuance/interfaces/oidc-issuer-sessions.interfaces.ts (1)
63-67: Tighten type for issuerMetadata.Prefer a safer structural type.
Apply this diff:
-export interface UpdateCredentialRequest { +export interface UpdateCredentialRequest { issuerId: string; credentialOfferId: string; - issuerMetadata: object; + issuerMetadata: Record<string, unknown>; }apps/api-gateway/src/issuance/issuance.service.ts (3)
34-38: Import statement can be optimized.Consider destructuring all the DTOs from a single import statement to improve readability.
-import { - CreateOidcCredentialOfferDto, - GetAllCredentialOfferDto, - UpdateCredentialRequestDto -} from './dtos/issuer-sessions.dto'; +import { CreateOidcCredentialOfferDto, GetAllCredentialOfferDto, UpdateCredentialRequestDto } from './dtos/issuer-sessions.dto';
360-367: Consider adding user authentication context.The
updateOidcCredentialOffermethod doesn't include user details in the payload, unlike other methods likecreateOidcCredentialOffer. This could be a security concern if user context is needed for audit trails or authorization.async updateOidcCredentialOffer( oidcUpdateCredentialPayload: UpdateCredentialRequestDto, + userDetails: user, orgId: string, issuerId: string ) { - const payload = { oidcUpdateCredentialPayload, orgId, issuerId }; + const payload = { oidcUpdateCredentialPayload, orgId, issuerId, userDetails }; return this.natsClient.sendNatsMessage(this.issuanceProxy, 'oidc-update-credential-offer', payload); }
373-376: Method parameter order inconsistency.The
getAllCredentialOffersmethod hasorgIdas the first parameter, while most other methods in this service have it as a later parameter. Consider maintaining consistent parameter ordering across methods.- async getAllCredentialOffers(orgId: string, getAllCredentialOffer: GetAllCredentialOfferDto) { + async getAllCredentialOffers(getAllCredentialOffer: GetAllCredentialOfferDto, orgId: string) { const payload = { orgId, getAllCredentialOffer }; return this.natsClient.sendNatsMessage(this.issuanceProxy, 'oidc-credential-offer-get-all', payload); }apps/issuance/src/issuance.controller.ts (1)
262-272: Consider adding validation for update payload.The
updateOidcCredentialOfferhandler doesn't validate theoidcUpdateCredentialPayloadbefore passing it to the service. Consider adding validation to ensure data integrity.@MessagePattern({ cmd: 'oidc-update-credential-offer' }) // eslint-disable-next-line @typescript-eslint/no-explicit-any async updateOidcCredentialOffer(payload: { oidcUpdateCredentialPayload: UpdateCredentialRequest; orgId: string; issuerId: string; // eslint-disable-next-line @typescript-eslint/no-explicit-any }): Promise<any> { const { oidcUpdateCredentialPayload, orgId, issuerId } = payload; + if (!oidcUpdateCredentialPayload || !orgId || !issuerId) { + throw new BadRequestException('Missing required fields for credential offer update'); + } return this.oidcIssuanceService.updateOidcCredentialOffer(oidcUpdateCredentialPayload, orgId, issuerId); }apps/issuance/libs/helpers/credential-sessions.builder.ts (3)
277-321: Consider extracting common validation logic.The
buildUpdateCredentialOfferPayloadfunction duplicates validation logic frombuildCredentialOfferPayload. Consider extracting common validation into a shared helper function.+ // Add a shared validation helper + function validateTemplates( + credentials: CredentialRequestDtoLike[], + templates: credential_templates[] + ): Map<string, credential_templates> { + const byId = new Map(templates.map((t) => [t.id, t])); + const unknown = credentials.map((c) => c.templateId).filter((id) => !byId.has(id)); + if (unknown.length) { + throw new Error(`Unknown template ids: ${unknown.join(', ')}`); + } + return byId; + } export function buildUpdateCredentialOfferPayload( dto: CreateOidcCredentialOfferDtoLike, templates: credential_templates[] ): { credentials: BuiltCredential[] } { - // Index templates by id - const byId = new Map(templates.map((t) => [t.id, t])); - - // Validate all templateIds exist - const unknown = dto.credentials.map((c) => c.templateId).filter((id) => !byId.has(id)); - if (unknown.length) { - throw new Error(`Unknown template ids: ${unknown.join(', ')}`); - } + const byId = validateTemplates(dto.credentials, templates); // ... rest of the function }
296-300: Validation logic could be more descriptive.The error message for invalid attributes could be more helpful by indicating which template the invalid keys belong to.
const payloadKeys = Object.keys(cred.payload); const invalidKeys = payloadKeys.filter((k) => !attrs[k]); if (invalidKeys.length) { - throw new Error(`Invalid attributes for template "${cred.templateId}": ${invalidKeys.join(', ')}`); + throw new Error(`Invalid attributes for template "${template.name}" (ID: ${cred.templateId}): ${invalidKeys.join(', ')}`); }
323-348: Consider URL encoding edge cases.The
buildCredentialOfferUrlfunction should handle null/undefined values more explicitly and consider edge cases where values might contain special characters that need additional encoding.export function buildCredentialOfferUrl(baseUrl: string, getAllCredentialOffer: GetAllCredentialOffer): string { const criteriaParams: string[] = []; + + // Helper to safely encode and add parameters + const addParam = (key: string, value: unknown): void => { + if (value != null && value !== '') { + criteriaParams.push(`${key}=${encodeURIComponent(String(value))}`); + } + }; - if (getAllCredentialOffer.publicIssuerId) { - criteriaParams.push(`publicIssuerId=${encodeURIComponent(getAllCredentialOffer.publicIssuerId)}`); - } + addParam('publicIssuerId', getAllCredentialOffer.publicIssuerId); + addParam('preAuthorizedCode', getAllCredentialOffer.preAuthorizedCode); + addParam('state', getAllCredentialOffer.state); + addParam('credentialOfferUri', getAllCredentialOffer.credentialOfferUri); + addParam('authorizationCode', getAllCredentialOffer.authorizationCode); - if (getAllCredentialOffer.preAuthorizedCode) { - criteriaParams.push(`preAuthorizedCode=${encodeURIComponent(getAllCredentialOffer.preAuthorizedCode)}`); - } - // ... similar for other params // Append query string if any params exist return 0 < criteriaParams.length ? `${baseUrl}?${criteriaParams.join('&')}` : baseUrl; }apps/agent-service/src/agent-service.service.ts (2)
1478-1489: Error handling consistency.The new
oidcUpdateCredentialOffermethod follows the established error handling pattern correctly, but the error message uses an underscore prefix_oidcUpdateCredentialOfferwhile the method doesn't. Consider removing the underscore for consistency.} catch (error) { - this.logger.error(`Error in _oidcUpdateCredentialOffer in agent service : ${JSON.stringify(error)}`); + this.logger.error(`Error in oidcUpdateCredentialOffer in agent service : ${JSON.stringify(error)}`); throw error; }
1491-1502: Consistent error message formatting.All new OIDC credential offer methods have underscore prefixes in their error messages. Remove them for consistency with the actual method names.
// For oidcGetCredentialOfferById - this.logger.error(`Error in _oidcGetCredentialOfferById in agent service : ${JSON.stringify(error)}`); + this.logger.error(`Error in oidcGetCredentialOfferById in agent service : ${JSON.stringify(error)}`); // For oidcGetAllCredentialOffers - this.logger.error(`Error in _oidcGetAllCredentialOffers in agent service : ${JSON.stringify(error)}`); + this.logger.error(`Error in oidcGetAllCredentialOffers in agent service : ${JSON.stringify(error)}`); // For oidcDeleteCredentialOffer - this.logger.error(`Error in _oidcDeleteCredentialOffer in agent service : ${JSON.stringify(error)}`); + this.logger.error(`Error in oidcDeleteCredentialOffer in agent service : ${JSON.stringify(error)}`);Also applies to: 1504-1515, 1517-1528
apps/api-gateway/src/issuance/dtos/issuer-sessions.dto.ts (4)
32-40: Make disclosureFrame validator and TS type agree (support deep nesting or narrow the contract).Right now the validator allows only one nested level via isDisclosureFrameValue, but the error message implies arbitrary nesting. Either clarify the message or make it truly recursive and update the type.
-function isDisclosureFrameValue(v: unknown): boolean { - if ('boolean' === typeof v) { - return true; - } - if (v && 'object' === typeof v && !Array.isArray(v)) { - return Object.values(v as Record<string, unknown>).every((x) => 'boolean' === typeof x); - } - return false; -} +type DisclosureFrame = Record<string, boolean | DisclosureFrame>; +function isDisclosureFrameValue(v: unknown): boolean { + if (typeof v === 'boolean') return true; + if (v && typeof v === 'object' && !Array.isArray(v)) { + return Object.values(v as Record<string, unknown>).every(isDisclosureFrameValue); + } + return false; +} @@ - disclosureFrame?: Record<string, boolean | Record<string, boolean>>; + disclosureFrame?: DisclosureFrame;Also applies to: 145-153
166-185: Hide/internalize issuerId on CreateOidcCredentialOfferDto.issuerId is set from path params in the controller; don’t expose it in the public API schema. Also validate its type to avoid it being stripped by ValidationPipe with whitelist.
authorizationCodeFlowConfig?: AuthorizationCodeFlowConfigDto; - issuerId?: string; + @ApiHideProperty() + @IsOptional() + @IsString() + issuerId?: string;
187-212: Add string validators; avoid default '' on optional query fields.Defaulting to '' makes the properties always present and can skew filtering; prefer undefined + @IsString() for type safety.
export class GetAllCredentialOfferDto { - @ApiProperty({ required: false, example: 'credebl university' }) - @IsOptional() - publicIssuerId: string = ''; + @ApiProperty({ required: false, example: 'issuer-public-id-123' }) + @IsOptional() + @IsString() + publicIssuerId?: string; @@ - @ApiProperty({ required: false, example: '568345' }) - @IsOptional() - preAuthorizedCode: string = ''; + @ApiProperty({ required: false, example: '568345' }) + @IsOptional() + @IsString() + preAuthorizedCode?: string; @@ - @ApiProperty({ required: false, example: 'openid-credential-offer://?credential_offer_uri=http%3A%2F%2.....' }) - @IsOptional() - credentialOfferUri: string = ''; + @ApiProperty({ required: false, example: 'openid-credential-offer://?credential_offer_uri=...' }) + @IsOptional() + @IsString() + credentialOfferUri?: string; @@ - @ApiProperty({ required: false, example: 'Bob' }) - @IsOptional() - authorizationCode: string = ''; + @ApiProperty({ required: false, example: 'abc123' }) + @IsOptional() + @IsString() + authorizationCode?: string; }
214-227: Validate and hide server-populated identifiers on UpdateCredentialRequestDto.issuerId and credentialOfferId are server-set; hide in Swagger and validate type.
export class UpdateCredentialRequestDto { @@ issuerMetadata?: Record<string, unknown>; - issuerId?: string; + @ApiHideProperty() + @IsOptional() + @IsString() + issuerId?: string; - credentialOfferId?: string; + @ApiHideProperty() + @IsOptional() + @IsString() + credentialOfferId?: string; }apps/api-gateway/src/issuance/oidc.controller.ts (4)
239-241: Remove console logging from controllers.Use Nest Logger or structured logs; console.log leaks payloads.
- console.log('THis is dto', JSON.stringify(CredentialTemplate, null, 2)); + // use a logger if needed; avoid logging full DTOs with PII
151-157: Return 200 OK for GET by id (not 201 Created).Fix statusCode and HTTP status to OK.
- const finalResponse: IResponse = { - statusCode: HttpStatus.CREATED, + const finalResponse: IResponse = { + statusCode: HttpStatus.OK, message: ResponseMessages.oidcIssuer.success.fetch, data: oidcIssuer }; - return res.status(HttpStatus.CREATED).json(finalResponse); + return res.status(HttpStatus.OK).json(finalResponse);
449-455: Remove console logging of request payloads.Avoid logging raw offer payloads; use Logger with redaction if needed.
- console.log('This is dto', JSON.stringify(oidcCredentialPayload, null, 2)); + // consider Logger.debug with redaction if necessary
555-560: 204 No Content must not include a response body; align decorator and response.Either return 204 with empty body or 200 with JSON. Suggest 204 + empty body and fix @apiresponse.
- @ApiResponse({ status: HttpStatus.OK, description: 'Success', type: ApiResponseDto }) + @ApiResponse({ status: HttpStatus.NO_CONTENT, description: 'Deleted' }) @@ - const finalResponse: IResponse = { - statusCode: HttpStatus.NO_CONTENT, - message: ResponseMessages.oidcIssuerSession.success.delete, - data: deletedofferDetails - }; - return res.status(HttpStatus.NO_CONTENT).json(finalResponse); + return res.status(HttpStatus.NO_CONTENT).send();Also applies to: 543-544
apps/issuance/src/oidc-issuance.service.ts (3)
464-473: Guard against missing agent details before using orgDid.Add a null-check to prevent runtime errors when agent is not configured.
- const agentDetails = await this.issuanceRepository.getAgentEndPoint(orgId); + const agentDetails = await this.issuanceRepository.getAgentEndPoint(orgId); + if (!agentDetails?.orgDid) { + throw new NotFoundException(ResponseMessages.issuance.error.agentEndPointNotFound); + }
494-495: Normalize response shape (parse when JSON string).Other methods parse string responses; do the same for create for consistency.
- return createCredentialOfferOnAgent.response; + const resp = createCredentialOfferOnAgent.response; + return typeof resp === 'string' ? JSON.parse(resp) : resp;
549-555: Remove console.log; optionally parse response.- console.log('This is the updateCredentialOfferOnAgent:', JSON.stringify(updateCredentialOfferOnAgent)); @@ - return updateCredentialOfferOnAgent.response; + const resp = updateCredentialOfferOnAgent.response; + return typeof resp === 'string' ? JSON.parse(resp) : resp;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/agent-service/src/agent-service.controller.ts(2 hunks)apps/agent-service/src/agent-service.service.ts(1 hunks)apps/api-gateway/src/issuance/dtos/issuer-sessions.dto.ts(7 hunks)apps/api-gateway/src/issuance/dtos/oidc-issuer-template.dto.ts(1 hunks)apps/api-gateway/src/issuance/dtos/oidc-issuer.dto.ts(1 hunks)apps/api-gateway/src/issuance/issuance.service.ts(3 hunks)apps/api-gateway/src/issuance/oidc.controller.ts(9 hunks)apps/issuance/interfaces/oidc-issuer-sessions.interfaces.ts(3 hunks)apps/issuance/libs/helpers/credential-sessions.builder.ts(6 hunks)apps/issuance/src/issuance.controller.ts(3 hunks)apps/issuance/src/oidc-issuance.service.ts(7 hunks)libs/common/src/common.constant.ts(2 hunks)libs/common/src/common.utils.ts(1 hunks)libs/common/src/nats.config.ts(1 hunks)libs/common/src/response-messages/index.ts(1 hunks)libs/enum/src/enum.ts(1 hunks)
🔇 Additional comments (22)
libs/common/src/common.constant.ts (2)
126-128: LGTM: OIDC issuer-session URL constants added sensibly.Endpoints for by-id vs collection are distinct and consistent with
getAgentUrlusage.
403-408: LGTM: Action tokens cover CRUD for credential offers.Names are consistent with the new endpoints.
libs/common/src/response-messages/index.ts (2)
538-543: LGTM: Complete success surface for OIDC credential offers.Create/getById/getAll/update/delete messages added consistently.
545-548: Verify downstream references after key rename (errroCreateOffer → errorCreateOffer).
libs/common/src/response-messages/index.ts contains errorCreateOffer / errorUpdateOffer / deleteFailed (rename applied). Sandbox ripgrep could not scan the repo ("No files were searched"), so I cannot conclusively confirm there are no remaining references — run locally to verify:
rg -n --hidden --no-ignore 'errroCreateOffer|oidcIssuerSession.error.(invalidId|createFailed|updateFailed)' || trueapps/issuance/interfaces/oidc-issuer-sessions.interfaces.ts (2)
1-1: LGTM: Shared session-state type imported.Keeps the state filter aligned across services.
41-46: credentialSupportedId optionality confirmed — builders derive it.buildOneCredential / buildUpdateCredentialOfferPayload compute credentialSupportedId =
${template.name}-${formatSuffix(...)}(apps/issuance/libs/helpers/credential-sessions.builder.ts), and BuiltCredential requires it, so making CredentialRequest.credentialSupportedId optional is safe; ensure templates include valid name/format (unknown templateIds are already rejected).libs/common/src/nats.config.ts (1)
17-19: No internal usages of getNatsOptions().queue — confirm external consumersRepo-wide search found no references to
.queue(dot or bracket access) and getNatsOptions(...) is only passed into Nest microservice creation across apps, so removing the field will not break internal callers. If this function is part of a public API consumed by other repos, restore thequeue: serviceNamefield and mark it DEPRECATED (apply your suggested diff); otherwise removal is safe.libs/enum/src/enum.ts (1)
275-286: No runtime usages found — keep the type-onlyexport declare enumSearch found only type/commented references (declaration: libs/enum/src/enum.ts; type use: apps/issuance/interfaces/oidc-issuer-sessions.interfaces.ts; commented examples: apps/api-gateway/src/issuance/dtos/issuer-sessions.dto.ts). No Object.values/keys/entries or runtime property accesses detected — no change required.
apps/issuance/src/issuance.controller.ts (3)
27-31: LGTM!The new imports for OIDC credential offer interfaces are properly structured and follow the established pattern.
180-184: NATS command renamed - matches the service layer change.The command pattern
'oidc-get-issuers-issuance'correctly matches the updated service call, maintaining consistency across layers.
274-283: LGTM! Well-structured CRUD operations.The new handlers for OIDC credential offers (get-by-id, get-all, delete) follow consistent patterns and properly delegate to the service layer.
apps/issuance/libs/helpers/credential-sessions.builder.ts (3)
4-4: Import statement is correctly added.The
GetAllCredentialOfferimport is properly placed and necessary for the new URL building functionality.
115-116: Additional format variant properly handled.The mapping for
'sd+jwt-vc'toSdJwtVcis correctly added to handle another variant of the SD-JWT format string.
229-230: Good refactoring - signerOption is now optional.Making
signerOptionoptional improves flexibility and allows the function to be used in scenarios where signing configuration might not be immediately available.Also applies to: 249-252
apps/agent-service/src/agent-service.service.ts (1)
1478-1528: LGTM! Well-structured OIDC credential offer methods.The four new methods for OIDC credential offer management (update, get-by-id, get-all, delete) follow consistent patterns:
- Proper API key retrieval
- Correct HTTP verb usage (PUT, GET, DELETE)
- Consistent error handling and logging
- Proper use of authorization headers
apps/agent-service/src/agent-service.controller.ts (3)
349-353: Command pattern naming is consistent.The renamed command
'oidc-get-issuers-agent-service'properly differentiates this handler from the issuance service's handler.
361-383: LGTM! Consistent handler implementation.The new OIDC credential offer handlers follow the established patterns in the controller and properly delegate to their corresponding service methods.
369-370: Incorrect — offerId is redundant; the caller already embeds it in the URLIssuance builds the URL with offerId and calls _oidcGetCredentialOfferById(url, orgId) (apps/issuance/src/oidc-issuance.service.ts -> getCredentialOfferDetailsById); the controller handler (apps/agent-service/src/agent-service.controller.ts:369–370) accepts but never uses offerId. Remove the unused parameter from the handler signature or validate the URL contains the ID.
Likely an incorrect or invalid review comment.
apps/api-gateway/src/issuance/issuance.service.ts (1)
306-307: NATS topic renamed — repo uses service-specific topics; confirm external consumersOld 'oidc-get-issuers' no longer present. Repo usages found:
- 'oidc-get-issuers-issuance' — apps/api-gateway/src/issuance/issuance.service.ts:306, apps/issuance/src/issuance.controller.ts:180
- 'oidc-get-issuers-agent-service' — apps/issuance/src/oidc-issuance.service.ts:693, apps/agent-service/src/agent-service.controller.ts:349
Breaking change for any external consumers — ensure they are updated.
apps/api-gateway/src/issuance/oidc.controller.ts (1)
179-184: LGTM: corrected list-issuers status to 200.apps/issuance/src/oidc-issuance.service.ts (2)
222-244: LGTM: hardened issuer list retrieval and parsing.
715-726: LGTM: added update-offer transport wrapper.
| @ApiProperty({ enum: ['mso_mdoc', 'vc+sd-jwt'], description: 'Credential format type' }) | ||
| @IsEnum(['mso_mdoc', 'vc+sd-jwt']) | ||
| format: 'mso_mdoc' | 'vc+sd-jwt'; |
There was a problem hiding this comment.
Validation bug: IsEnum used with array — requests will be rejected.
@IsEnum(['mso_mdoc', 'vc+sd-jwt']) doesn’t work; use @IsIn or a real enum.
Apply this diff:
- @ApiProperty({ enum: ['mso_mdoc', 'vc+sd-jwt'], description: 'Credential format type' })
- @IsEnum(['mso_mdoc', 'vc+sd-jwt'])
+ @ApiProperty({ enum: ['mso_mdoc', 'vc+sd-jwt'], description: 'Credential format type' })
+ @IsIn(['mso_mdoc', 'vc+sd-jwt'])
format: 'mso_mdoc' | 'vc+sd-jwt';Add missing import:
-import {
+import {
IsString,
IsBoolean,
IsOptional,
- IsEnum,
+ IsEnum,
+ IsIn,
ValidateNested,
IsObject,
IsNotEmpty,
IsArray
} from 'class-validator';📝 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.
| @ApiProperty({ enum: ['mso_mdoc', 'vc+sd-jwt'], description: 'Credential format type' }) | |
| @IsEnum(['mso_mdoc', 'vc+sd-jwt']) | |
| format: 'mso_mdoc' | 'vc+sd-jwt'; | |
| import { | |
| IsString, | |
| IsBoolean, | |
| IsOptional, | |
| IsEnum, | |
| IsIn, | |
| ValidateNested, | |
| IsObject, | |
| IsNotEmpty, | |
| IsArray | |
| } from 'class-validator'; | |
| @ApiProperty({ enum: ['mso_mdoc', 'vc+sd-jwt'], description: 'Credential format type' }) | |
| @IsIn(['mso_mdoc', 'vc+sd-jwt']) | |
| format: 'mso_mdoc' | 'vc+sd-jwt'; |
🤖 Prompt for AI Agents
In apps/api-gateway/src/issuance/dtos/oidc-issuer-template.dto.ts around lines
128 to 130, the validation uses @IsEnum with an array which will reject
requests; replace @IsEnum(['mso_mdoc', 'vc+sd-jwt']) with @IsIn(['mso_mdoc',
'vc+sd-jwt']) (or define a proper TypeScript enum and use @IsEnum with that),
and add the missing import: import { IsIn } from 'class-validator'; keep the
ApiProperty as-is or change its enum to reference the TS enum if you create one.
| async getCredentialOffers(orgId: string, getAllCredentialOffer: GetAllCredentialOffer): Promise<any> { | ||
| try { | ||
| const url = await getAgentUrl(await this.getAgentEndpoint(orgId), CommonConstants.OIDC_ISSUER_SESSIONS); | ||
| const credentialOfferUrl = buildCredentialOfferUrl(url, getAllCredentialOffer); | ||
| console.log('This is credentialOfferUrl', credentialOfferUrl); | ||
| const offers = await this._oidcGetCredentialOfferById(credentialOfferUrl, orgId); | ||
| console.log('This is offer', JSON.stringify(offers, null, 2)); | ||
| if ('string' === typeof offers.response) { | ||
| offers.response = JSON.parse(offers.response); | ||
| } | ||
| return offers.response; | ||
| } catch (error) { | ||
| this.logger.error(`[getCredentialOffers] - error: ${JSON.stringify(error)}`); | ||
| throw new RpcException(error.response ?? error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Use the dedicated “get-all” NATS pattern.
Method getCredentialOffers builds a collection URL but calls the “by-id” transport. Switch to _oidcGetCredentialOffers.
- const offers = await this._oidcGetCredentialOfferById(credentialOfferUrl, orgId);
+ const offers = await this._oidcGetCredentialOffers(credentialOfferUrl, orgId);Also applies to: 741-752
🤖 Prompt for AI Agents
In apps/issuance/src/oidc-issuance.service.ts around lines 579-594 (and
similarly at lines 741-752), the method builds a collection credential-offer URL
but incorrectly calls the "by-id" transport _oidcGetCredentialOfferById; replace
that call with the dedicated collection NATS pattern method
_oidcGetCredentialOffers, passing the same credentialOfferUrl and orgId (and
adjust any returned shape handling if needed), remove or update the debug logs
accordingly, and ensure the catch still logs and throws the RpcException as
before.
| [String(CommonConstants.OIDC_ISSUER_SESSIONS_UPDATE_OFFER), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET)], | ||
| [String(CommonConstants.OIDC_ISSUER_SESSIONS_BY_ID), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET)], | ||
| [String(CommonConstants.OIDC_ISSUER_SESSIONS), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET_ALL)], | ||
| [String(CommonConstants.OIDC_DELETE_CREDENTIAL_OFFER), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET_ALL)] | ||
| ]); |
There was a problem hiding this comment.
Bug: delete mapping won’t inject id — will call collection endpoint.
OIDC_DELETE_CREDENTIAL_OFFER maps to URL_OIDC_ISSUER_SESSIONS_GET_ALL (no #), so paramId is ignored. It should use the by‑id path.
Apply this diff:
- [String(CommonConstants.OIDC_DELETE_CREDENTIAL_OFFER), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET_ALL)]
+ [String(CommonConstants.OIDC_DELETE_CREDENTIAL_OFFER), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET)]The other new mappings (update/byId/getAll) look correct.
📝 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.
| [String(CommonConstants.OIDC_ISSUER_SESSIONS_UPDATE_OFFER), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET)], | |
| [String(CommonConstants.OIDC_ISSUER_SESSIONS_BY_ID), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET)], | |
| [String(CommonConstants.OIDC_ISSUER_SESSIONS), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET_ALL)], | |
| [String(CommonConstants.OIDC_DELETE_CREDENTIAL_OFFER), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET_ALL)] | |
| ]); | |
| [String(CommonConstants.OIDC_ISSUER_SESSIONS_UPDATE_OFFER), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET)], | |
| [String(CommonConstants.OIDC_ISSUER_SESSIONS_BY_ID), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET)], | |
| [String(CommonConstants.OIDC_ISSUER_SESSIONS), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET_ALL)], | |
| [String(CommonConstants.OIDC_DELETE_CREDENTIAL_OFFER), String(CommonConstants.URL_OIDC_ISSUER_SESSIONS_GET)] | |
| ]); |
🤖 Prompt for AI Agents
In libs/common/src/common.utils.ts around lines 106 to 110, the mapping for
CommonConstants.OIDC_DELETE_CREDENTIAL_OFFER incorrectly points to the "get all"
endpoint so the id param is never injected; change that mapping to use the
by‑id/get path used for OIDC_ISSUER_SESSIONS_BY_ID (i.e. replace the
URL_OIDC_ISSUER_SESSIONS_GET_ALL entry with URL_OIDC_ISSUER_SESSIONS_GET) so the
delete route receives the id parameter.
Signed-off-by: Tipu_Singh <[email protected]>
Signed-off-by: Tipu_Singh <[email protected]>
Signed-off-by: Tipu_Singh <[email protected]>
Signed-off-by: Tipu_Singh <[email protected]>
| @@ -0,0 +1,29 @@ | |||
| export enum SortFields { | |||
| CREATED_DATE_TIME = 'createDateTime', | |||
| SCHEMA_ID = 'schemaId', | |||
There was a problem hiding this comment.
remove schemaId and connectionId
|
|
||
| let schemaId = ''; | ||
|
|
||
| if ( |
There was a problem hiding this comment.
Why this code is removed?
Signed-off-by: Tipu_Singh <[email protected]>
Signed-off-by: Tipu_Singh <[email protected]>
|
* feat: oidc issunace Signed-off-by: Tipu_Singh <[email protected]> * feat:create seperate microservice for oid4vc Signed-off-by: Tipu_Singh <[email protected]> * feat:removed logs Signed-off-by: Tipu_Singh <[email protected]> * refactor: removed duplicate code Signed-off-by: Tipu_Singh <[email protected]> * feat: create credential offer API Signed-off-by: Tipu_Singh <[email protected]> * fix: missing credential details repository logic Signed-off-by: Tipu_Singh <[email protected]> * feat: added docker file for oid4vc-issuance Signed-off-by: Tipu_Singh <[email protected]> --------- Signed-off-by: Tipu_Singh <[email protected]>
What
Summary by CodeRabbit
New Features
Changes