-
Notifications
You must be signed in to change notification settings - Fork 15
Fix/issuer verifier session type issues #337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: fix/agent-setup-and-controllers
Are you sure you want to change the base?
Fix/issuer verifier session type issues #337
Conversation
|
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 📝 WalkthroughWalkthroughThis PR adds the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Holder as Client<br/>(Holder)
participant HolderCtrl as HolderController
participant HolderSvc as HolderService
participant Agent as Agent<br/>(openid4vc.holder)
Holder->>HolderCtrl: POST /request-credential<br/>(offerUri)
activate HolderCtrl
HolderCtrl->>HolderSvc: requestCredential(offerUri, ...)
activate HolderSvc
HolderSvc->>Agent: resolveCredentialOffer(offerUri)
activate Agent
Agent-->>HolderSvc: credentialOffer + metadata
deactivate Agent
Note over HolderSvc: Determine flow:<br/>PreAuthorized vs<br/>AuthorizationCode
alt PreAuthorized Flow
HolderSvc->>Agent: requestToken(preAuthCode)
activate Agent
Agent-->>HolderSvc: accessToken
deactivate Agent
else AuthorizationCode Flow
HolderSvc->>HolderSvc: initiateAuthorization()<br/>return OAuth2 redirect
end
HolderSvc->>Agent: requestCredentials(accessToken, proofPayloads)
activate Agent
Agent-->>HolderSvc: credentials[]
deactivate Agent
HolderSvc->>Agent: storeCredentials(credentials)
activate Agent
Agent-->>HolderSvc: stored
deactivate Agent
HolderSvc-->>HolderCtrl: credentialRecords
deactivate HolderSvc
HolderCtrl-->>Holder: 200 {credentials}
deactivate HolderCtrl
sequenceDiagram
autonumber
participant Verifier as Client<br/>(Verifier)
participant VerCtrl as VerificationSessionsController
participant VerSvc as VerificationSessionsService
participant Agent as Agent<br/>(openid4vc.verifier)
Verifier->>VerCtrl: POST /create-proof-request<br/>(presentationDefinition, ...)
activate VerCtrl
VerCtrl->>VerSvc: createProofRequest(authReqOptions)
activate VerSvc
Note over VerSvc: Resolve signer DID or<br/>X.509 certificate chain
VerSvc->>Agent: resolveDidDocument(did)
activate Agent
Agent-->>VerSvc: didDocument<br/>+ verificationMethod
deactivate Agent
VerSvc->>Agent: createAuthorizationRequest<br/>(presentationDef, signer)
activate Agent
Agent-->>VerSvc: authorizationRequest<br/>+ session
deactivate Agent
VerSvc-->>VerCtrl: {authorizationRequestUri, sessionId}
deactivate VerSvc
VerCtrl-->>Verifier: 201 {uri, sessionId}
deactivate VerCtrl
Note over Verifier: Verifier sends authorizationRequestUri<br/>to Holder for presentation submission
Verifier->>VerCtrl: GET /verification-session/{id}/response
activate VerCtrl
VerCtrl->>VerSvc: getVerifiedAuthorizationResponse(sessionId)
activate VerSvc
VerSvc->>Agent: getVerificationSessionById(sessionId)
activate Agent
Agent-->>VerSvc: session<br/>+ presentations[]
deactivate Agent
Note over VerSvc: Transform presentations:<br/>W3cJsonLd | W3cJwt<br/>| MdocDeviceResponse
VerSvc-->>VerCtrl: {presentations, dcql, submissions}
deactivate VerSvc
VerCtrl-->>Verifier: 200 {verified data}
deactivate VerCtrl
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 |
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.
I think this looks good, just one thing. I guess it would be good if we could test the controllers before merging. Coz, we have removed some patches that were added by tipu and rinkal, which were required
| openId4VcVerifier: new OpenId4VcVerifierModule({ | ||
| baseUrl: | ||
| process.env.NODE_ENV === 'PROD' | ||
| ? `https://${process.env.APP_URL}/oid4vp` | ||
| : `${process.env.AGENT_HTTP_URL}/oid4vp`, | ||
| app: openId4VpApp, | ||
| authorizationRequestExpirationInSeconds: Number(process.env.OID4VP_AUTH_REQUEST_PROOF_REQUEST_EXPIRY) || 3600, | ||
| }), | ||
| openId4VcIssuer: new OpenId4VcIssuerModule({ | ||
| baseUrl: | ||
| process.env.NODE_ENV === 'PROD' | ||
| ? `https://${process.env.APP_URL}/oid4vci` | ||
| : `${process.env.AGENT_HTTP_URL}/oid4vci`, | ||
| app: openId4VcApp, | ||
| statefulCredentialOfferExpirationInSeconds: Number(process.env.OID4VCI_CRED_OFFER_EXPIRY) || 3600, | ||
| accessTokenExpiresInSeconds: Number(process.env.OID4VCI_ACCESS_TOKEN_EXPIRY) || 3600, | ||
| authorizationCodeExpiresInSeconds: Number(process.env.OID4VCI_AUTH_CODE_EXPIRY) || 3600, | ||
| cNonceExpiresInSeconds: Number(process.env.OID4VCI_CNONCE_EXPIRY) || 3600, | ||
| dpopRequired: false, | ||
| credentialRequestToCredentialMapper: (...args) => getCredentialRequestToCredentialMapper()(...args), | ||
| }), | ||
| openId4VcHolderModule: new OpenId4VcHolderModule(), |
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.
I think we need to have this configuration when starting the agent as per the updated config types. Can we add this?
| // OpenId4VcHolderModule, | ||
| // OpenId4VcIssuerModule, |
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.
I think we can remove this commented code?
| // OpenId4VcHolderModule, | ||
| // OpenId4VcIssuerModule, | ||
| OpenId4VcModule, | ||
| // OpenId4VcVerifierModule, |
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.
I think we can remove this commented code?
| import { SignerMethod } from '../../../enums' | ||
| import { CreateAuthorizationRequest, OpenId4VcIssuerX5c, ResponseModeEnum } from '../types/verifier.types' | ||
|
|
||
| // import { CreateAuthorizationRequest } from '../types/verifier.types' |
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.
I think we can remove this commented code?
| // if ( | ||
| // presentation instanceof W3cV2JwtVerifiablePresentation || | ||
| // presentation instanceof W3cV2SdJwtVerifiablePresentation | ||
| // ) { | ||
| // throw new Error('W3C V2 presentations are not supported yet') | ||
| // } |
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.
I think we can remove this commented code, if not required?
| // const { credentialOffer, issuanceSession } = await agentReq.agent.modules.openId4VcIssuer.createCredentialOffer({ | ||
| // issuerId: publicIssuerId, | ||
| // issuanceMetadata: options.issuanceMetadata, | ||
| // offeredCredentials: credentials.map((c) => c.credentialSupportedId), | ||
| // preAuthorizedCodeFlowConfig: options.preAuthorizedCodeFlowConfig, | ||
| // authorizationCodeFlowConfig: options.authorizationCodeFlowConfig, | ||
| // }) |
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.
I think we can remove this commented code?
|
|
||
| // // import { AgentWithRootOrTenant } from '../../types/agent' | ||
| // import { OpenId4VcIssuanceSessionsCreateOffer } from '../types/issuer.types' | ||
| // import { AgentWithRootOrTenant } from '../../types/agent' |
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.
I think we can remove this commented code?
src/cliAgent.ts
Outdated
| // }, | ||
| // }), | ||
| // openId4VcVerifier: new OpenId4VcVerifierModule({ | ||
| // baseUrl: | ||
| // process.env.NODE_ENV === 'PROD' | ||
| // ? `https://${process.env.APP_URL}/oid4vp` | ||
| // : `${process.env.AGENT_HTTP_URL}/oid4vp`, | ||
| // app: openId4VpApp, | ||
| // authorizationRequestExpirationInSeconds: Number(process.env.OID4VP_AUTH_REQUEST_PROOF_REQUEST_EXPIRY) || 3600, | ||
| // }), | ||
| // openId4VcIssuer: new OpenId4VcIssuerModule({ | ||
| // baseUrl: | ||
| // process.env.NODE_ENV === 'PROD' | ||
| // ? `https://${process.env.APP_URL}/oid4vci` | ||
| // : `${process.env.AGENT_HTTP_URL}/oid4vci`, | ||
| // app: openId4VcApp, | ||
| // statefulCredentialOfferExpirationInSeconds: Number(process.env.OID4VCI_CRED_OFFER_EXPIRY) || 3600, | ||
| // accessTokenExpiresInSeconds: Number(process.env.OID4VCI_ACCESS_TOKEN_EXPIRY) || 3600, | ||
| // authorizationCodeExpiresInSeconds: Number(process.env.OID4VCI_AUTH_CODE_EXPIRY) || 3600, | ||
| // cNonceExpiresInSeconds: Number(process.env.OID4VCI_CNONCE_EXPIRY) || 3600, | ||
| // dpopRequired: false, | ||
| // credentialRequestToCredentialMapper: (...args) => getCredentialRequestToCredentialMapper()(...args), | ||
| // }), | ||
| // openId4VcHolderModule: new OpenId4VcHolderModule(), |
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.
I think we can remove this commented code?
|
@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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/oid4vc-agent.ts (1)
254-257: Use the extracted configuration instead of accessing the map directly.At line 224,
credentialConfigurationSupportedis correctly extracted ascredentialConfiguration[credentialConfigurationId]. However, at line 258 (and throughout the subsequent conditional blocks at lines 289–322), the code directly accesses properties oncredentialConfigurationinstead of using the extracted configuration. This is incorrect sincecredentialConfigurationis a map, not a single configuration object.Change
credentialConfiguration.vcttocredentialConfigurationSupported.vct. Also apply the same fix to other direct property accesses oncredentialConfiguration(e.g.,.format,.doctype) — they should all usecredentialConfigurationSupportedor access via the ID explicitly.The commented code at line 266 shows the intended pattern:
vct: credentialConfigurationSupported.vct,.
♻️ Duplicate comments (1)
src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts (1)
60-66: Remove commented-out code.This commented code block should be removed as it's no longer needed.
🧹 Nitpick comments (20)
src/controllers/openid4vc/holder/credentialBindingResolver.ts (2)
57-57: Replaceconsole.logwith proper logging.Debug statements using
console.logshould be removed or replaced with the project's logger (TsLogger) for production code.🔎 Proposed fix
- console.log('proofTypes', JSON.stringify(proofTypes)) + // Remove debug log or use proper logger if needed for debugging
43-46: Inconsistent optional chaining usage.The first format check uses optional chaining (
credentialConfiguration?.format), but the second does not (credentialConfiguration.format). IfcredentialConfigurationcould be nullish, both should use optional chaining for consistency and safety.🔎 Proposed fix
const shouldKeyBeHardwareBackedForSdJwtVc = - (credentialConfiguration?.format === 'vc+sd-jwt' || credentialConfiguration.format === 'dc+sd-jwt') && + (credentialConfiguration?.format === 'vc+sd-jwt' || credentialConfiguration?.format === 'dc+sd-jwt') && credentialConfiguration.vct && pidSchemes?.sdJwtVcVcts.includes(credentialConfiguration.vct)src/controllers/openid4vc/issuers/issuer.service.ts (2)
17-18: Replaceconsole.logwith proper logging.Production code should use the project's logger instead of
console.log.🔎 Proposed fix
- // eslint-disable-next-line no-console - console.log(`\nIssuer URL: ${issuerMetadata?.credentialIssuer.credential_issuer}`) + // Use logger if URL logging is needed for debugging
13-19: Handle undefined issuer module gracefully.If
modules.openid4vc.issueris undefined,issuerRecordwill be undefined, and passing an empty string togetIssuerMetadatamay cause unexpected behavior. Consider adding explicit error handling.🔎 Proposed fix
public async createIssuerAgent( agentReq: Req, createIssuerOptions: any, //TODO: Replace with OpenId4VciCreateIssuerOptions, ) { + const issuer = agentReq.agent.modules.openid4vc.issuer + if (!issuer) { + throw new Error('OpenID4VC issuer module is not configured') + } - const issuerRecord = await agentReq.agent.modules.openid4vc.issuer?.createIssuer(createIssuerOptions) - const issuerMetadata = await agentReq.agent.modules.openid4vc.issuer?.getIssuerMetadata( - issuerRecord?.issuerId ?? '', - ) + const issuerRecord = await issuer.createIssuer(createIssuerOptions) + const issuerMetadata = await issuer.getIssuerMetadata(issuerRecord.issuerId)src/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.ts (1)
96-106: Consider returning 204 No Content for successful deletion.The
deleteIssuanceSessionByIdmethod should return an appropriate HTTP status code (204 No Content) for successful deletions. Currently, it returns nothing which could be confusing for API consumers.🔎 Proposed fix
@Delete('/:issuanceSessionId') public async deleteIssuanceSessionById( @Request() request: Req, @Path('issuanceSessionId') issuanceSessionId: string, - ) { + ): Promise<void> { try { await issuanceSessionService.deleteById(request, issuanceSessionId) + this.setStatus(204) } catch (error) { throw ErrorHandlingService.handle(error) } }src/cliAgent.ts (1)
131-138: Module-levelexpressAppsingleton may cause testing issues.Creating
expressAppat module level means it's shared across all tests and cannot be easily mocked or reset. Consider lazy initialization or dependency injection.src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts (1)
85-88: Optional chaining result may be undefined.The optional chaining on
verifier?.createAuthorizationRequestmeans the result could beundefined, but the return type is cast toany. Consider adding explicit error handling when the verifier module is unavailable.🔎 Proposed fix
+ const verifier = agentReq.agent.modules.openid4vc.verifier + if (!verifier) { + throw new Error('OpenID4VC verifier module is not configured') + } - return (await agentReq.agent.modules.openid4vc.verifier?.createAuthorizationRequest(options)) as any + return await verifier.createAuthorizationRequest(options) } catch (error) { throw error }src/controllers/openid4vc/holder/holder.service.ts (6)
16-24: Remove unused imports.The following imports appear unused in this file:
DifPresentationExchangeService,DidKey,DidJwk,Mdoc,W3cJsonLdVerifiableCredential,W3cJwtVerifiableCredential. TheMdocclass is used indecodeMdocCredential, but the others are not referenced anywhere in the active code.🔎 Proposed fix
import { - DifPresentationExchangeService, - DidKey, - DidJwk, Mdoc, - W3cJsonLdVerifiableCredential, - W3cJwtVerifiableCredential, SdJwtVcRecord, } from '@credo-ts/core'
44-57: UnusedagentReqparameter indecodeMdocCredential.The
agentReqparameter is never used in this method. Consider removing it if it's not needed for future use.
121-167: Remove commented-out code blocks.These large commented-out code blocks reduce readability and maintainability. If this code is no longer needed, it should be removed. If it serves as a reference, consider documenting the approach elsewhere or using version control history.
177-223: Additional commented-out code should be removed.Same as above — this block of commented code clutters the file. Clean up by removing it.
315-317: Uselesstry-catchblock that only rethrows.The
catchblock catches an error and immediately rethrows it without any additional handling, logging, or transformation. This adds no value.🔎 Proposed fix
- let dcqlCredentials - try { - dcqlCredentials = await agentReq.agent.modules.openid4vc.holder.selectCredentialsForDcqlRequest( - resolved.dcql.queryResult, - ) - } catch (error) { - throw error - } + const dcqlCredentials = await agentReq.agent.modules.openid4vc.holder.selectCredentialsForDcqlRequest( + resolved.dcql.queryResult, + )
332-343: Incomplete implementation with TODO and placeholder return.The
getSelectedCredentialsForRequestmethod has a TODO comment and returns an empty object placeholder. This may cause unexpected behavior if called.Would you like me to help implement this method or open an issue to track this task?
src/controllers/openid4vc/holder/holder.Controller.ts (2)
53-56: Inconsistent route path format.The route
'resolve-credential-offer'lacks a leading slash, while other routes like'/sd-jwt-vcs'include it. While tsoa may normalize these, consistency improves readability.🔎 Proposed fix
- @Post('resolve-credential-offer') + @Post('/resolve-credential-offer')Apply similar fixes to other routes missing the leading slash:
'authorization-request','request-credential','resolve-proof-request','accept-proof-request','decode-sdjwt'.
93-96: Missing JSDoc comment.This endpoint lacks a JSDoc description unlike other methods in the controller.
🔎 Proposed fix
+ /** + * Decode an SD-JWT credential + */ @Post('decode-sdjwt') public async decodeSdJwt(@Request() request: Req, @Body() body: { jwt: string }) { return await holderService.decodeSdJwt(request, body) }src/controllers/openid4vc/verifier-sessions/verification-sessions.Controller.ts (2)
1-1: Remove unused import.
Agentis imported but never used in this file.🔎 Proposed fix
-import { Agent } from '@credo-ts/core'
73-76: Stale TODO comment.The TODO comment mentions "Uncomment when the method is implemented" and references an issue with
IDTokenPayloadtype, but the method below (lines 77-87) is already implemented. Either update the comment to reflect the current issue or remove it if no longer relevant.src/utils/oid4vc-agent.ts (3)
30-188: Remove large commented-out code block.This 150+ line commented-out function reduces readability. If
getCredentialRequestToCredentialMapperis deprecated in favor ofgetMixedCredentialRequestToCredentialMapper, remove it entirely or keep only a brief comment explaining the change.
413-419: Unused functionassertDidBasedHolderBinding.This assertion function is defined but never called in the active code. It was likely used by the commented-out JwtVcJson handling code.
Consider removing this function if it's no longer needed, or uncomment and use it if DID-based holder binding validation is required.
486-486: Removeconsole.logstatement from production code.The
console.log('Success:', data)statement should be removed or replaced with proper structured logging.🔎 Proposed fix
- console.log('Success:', data)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
package.jsonsrc/cliAgent.tssrc/controllers/openid4vc/holder/credentialBindingResolver.tssrc/controllers/openid4vc/holder/holder.Controller.tssrc/controllers/openid4vc/holder/holder.service.tssrc/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.tssrc/controllers/openid4vc/issuance-sessions/issuance-sessions.service.tssrc/controllers/openid4vc/issuers/issuer.Controller.tssrc/controllers/openid4vc/issuers/issuer.service.tssrc/controllers/openid4vc/types/issuer.types.tssrc/controllers/openid4vc/verifier-sessions/verification-sessions.Controller.tssrc/controllers/openid4vc/verifier-sessions/verification-sessions.service.tssrc/routes/routes.tssrc/routes/swagger.jsonsrc/utils/oid4vc-agent.ts
🧰 Additional context used
🧬 Code graph analysis (10)
src/controllers/openid4vc/holder/holder.service.ts (3)
src/controllers/openid4vc/types/holder.types.ts (4)
ResolveCredentialOfferBody(1-3)AuthorizeRequestCredentialOffer(13-16)RequestCredentialBody(5-11)ResolveProofRequest(18-20)src/controllers/openid4vc/holder/credentialBindingResolver.ts (1)
getCredentialBindingResolver(4-130)src/utils/logger.ts (1)
error(120-122)
src/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.ts (3)
src/controllers/openid4vc/types/issuer.types.ts (1)
OpenId4VcIssuanceSessionsCreateOffer(51-70)src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts (1)
issuanceSessionService(149-149)src/utils/logger.ts (1)
error(120-122)
src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts (1)
src/controllers/openid4vc/types/verifier.types.ts (3)
CreateAuthorizationRequest(74-83)OpenId4VcJwtIssuerDid(62-65)OpenId4VcIssuerX5c(67-72)
src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts (2)
src/controllers/openid4vc/types/issuer.types.ts (1)
OpenId4VcIssuanceSessionsCreateOffer(51-70)src/errors/errors.ts (2)
BadRequestError(102-102)NotFoundError(101-101)
src/controllers/openid4vc/verifier-sessions/verification-sessions.Controller.ts (2)
src/controllers/openid4vc/types/verifier.types.ts (1)
CreateAuthorizationRequest(74-83)src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts (1)
verificationSessionService(206-206)
src/controllers/openid4vc/holder/holder.Controller.ts (2)
src/controllers/openid4vc/holder/holder.service.ts (1)
holderService(345-345)src/controllers/openid4vc/types/holder.types.ts (4)
ResolveCredentialOfferBody(1-3)AuthorizeRequestCredentialOffer(13-16)RequestCredentialBody(5-11)ResolveProofRequest(18-20)
src/cliAgent.ts (1)
src/utils/oid4vc-agent.ts (1)
getMixedCredentialRequestToCredentialMapper(190-411)
src/routes/routes.ts (1)
src/utils/tsyringeTsoaIocContainer.ts (1)
iocContainer(5-9)
src/controllers/openid4vc/issuers/issuer.Controller.ts (2)
src/controllers/openid4vc/examples/issuer.examples.ts (1)
OpenId4VcUpdateIssuerRecordOptionsExample(1-111)src/controllers/openid4vc/types/issuer.types.ts (2)
CreateIssuerOptions(159-167)UpdateIssuerRecordOptions(169-174)
src/controllers/openid4vc/issuers/issuer.service.ts (1)
src/cliAgent.ts (1)
RestAgentModules(130-130)
🔇 Additional comments (8)
package.json (1)
46-46: LGTM!The
@credo-ts/didcommdependency is correctly added at version0.6.1, consistent with other@credo-tspackages in the project.src/cliAgent.ts (1)
251-274: LGTM!The OpenId4VcModule configuration with environment-based URL selection and fail-fast behavior via
requireEnvis well-structured. The configuration properly separates issuer and verifier settings.src/controllers/openid4vc/issuers/issuer.Controller.ts (1)
16-28: LGTM!The controller is well-structured with proper security decorators, error handling, and API documentation via the
@Exampledecorator.src/controllers/openid4vc/holder/holder.Controller.ts (1)
1-97: LGTM!The controller is well-structured with proper decorators for routing, security, and dependency injection. The delegation to
holderServicekeeps the controller thin and focused on HTTP concerns.src/controllers/openid4vc/verifier-sessions/verification-sessions.Controller.ts (1)
17-88: LGTM!The controller properly implements the verification session endpoints with consistent error handling via
ErrorHandlingService. The structure aligns well with the corresponding service layer.src/controllers/openid4vc/types/issuer.types.ts (1)
1-174: LGTM!The type definitions are well-structured and appropriately model the OpenID4VC issuer configuration. The new types for claims, metadata, and key attestation support the expanded issuance flows.
src/routes/routes.ts (2)
1-2553: Auto-generated file - no manual modifications needed.This file is auto-generated by tsoa and includes the new route registrations for
IssuanceSessionsControllerandVerificationSessionsController. The generated routes correctly wire up the new endpoints with appropriate authentication middleware.
740-746: Theundefinedproperty inSdJwtObjectis a generated artifact that cannot be manually fixed in routes.ts.This file is auto-generated by tsoa (marked with explicit "do not modify" warnings). The suspicious
undefinedproperty at line 743 appears in both routes.ts and swagger.json and cannot be corrected directly here.Since
SdJwtObjectis not defined in the local source code and the types are sourced from the@credo-ts/openid4vcexternal library, the property naming issue likely originates from how tsoa is parsing types from that dependency. Regenerating routes.ts viatsoa specandtsoa routeswill not resolve the issue unless the upstream type definitions in@credo-ts/openid4vcare corrected.
| // transport.app.use('/oid4vci', modules.openid4vc.issuer?.config.app.routes ?? express.Router()) | ||
| // transport.app.use('/oid4vp', modules.openid4vc.verifier?.config.app.routes ?? express.Router()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Check the current state of route mounting in cliAgent.ts around those lines
cat -n src/cliAgent.ts | sed -n '500,530p'Repository: credebl/credo-controller
Length of output: 1558
🏁 Script executed:
# Search for how OpenId4VcModule handles routes
rg -n "OpenId4VcModule|openid4vc" src/cliAgent.ts -A 2 -B 2Repository: credebl/credo-controller
Length of output: 1088
🏁 Script executed:
# Check if routes are being mounted elsewhere
rg -n "oid4vci|oid4vp" --type tsRepository: credebl/credo-controller
Length of output: 924
🏁 Script executed:
# Look for the module configuration and route setup
rg -n "config.app.routes" --type tsRepository: credebl/credo-controller
Length of output: 316
🏁 Script executed:
# Check the full OpenId4VcModule configuration
sed -n '251,290p' src/cliAgent.tsRepository: credebl/credo-controller
Length of output: 2071
🏁 Script executed:
# Search for any route registration or setup in the OpenId4VcModule configuration
rg -n "issuer.*config\|verifier.*config" src/cliAgent.ts -A 5 -B 1Repository: credebl/credo-controller
Length of output: 50
🏁 Script executed:
# Look for any initialization or setup that might be handling routes
rg -n "app.use\|app.get\|app.post" src/cliAgent.ts | head -20Repository: credebl/credo-controller
Length of output: 50
🏁 Script executed:
# Get the exact verifier configuration
sed -n '268,280p' src/cliAgent.tsRepository: credebl/credo-controller
Length of output: 642
🏁 Script executed:
# Search for any initialization logging or setup that might indicate routes being registered
rg -n "router|routes|register" src/cliAgent.ts | grep -v "node_modules"Repository: credebl/credo-controller
Length of output: 501
🏁 Script executed:
# Check if there are any other files that might configure OID4VC routing
fd -e ts -e js src/ | xargs grep -l "oid4v" | head -5Repository: credebl/credo-controller
Length of output: 389
Verifier routes will not be registered: missing app property in verifier config.
The issuer configuration includes app: expressApp at lines 261-262, enabling route registration for /oid4vci. However, the verifier configuration (lines 266-272) is missing the corresponding app property—the line app: openId4VpApp is commented out with no replacement. This will prevent the /oid4vp routes from being properly mounted on the express application. Add app: expressApp to the verifier configuration to ensure routes are registered alongside the issuer routes.
🤖 Prompt for AI Agents
In src/cliAgent.ts around lines 513-514, the verifier config is missing the app
property so its routes won't be mounted (the issuer has app: expressApp); add
app: expressApp to the verifier configuration object (or otherwise reference the
express app used for issuer) so transport.app.use('/oid4vp',
modules.openid4vc.verifier?.config.app.routes ?? express.Router()) can register
the verifier routes correctly.
| const signatureAlgorithm = proofTypes.jwt?.supportedSignatureAlgorithms[0] | ||
| const keys = await Promise.all( | ||
| new Array(batchSize).fill(0).map(() => | ||
| kms | ||
| .createKeyForSignatureAlgorithm({ | ||
| algorithm: signatureAlgorithm!, |
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.
Validate signatureAlgorithm before use.
If supportedSignatureAlgorithms is empty, signatureAlgorithm will be undefined, and the non-null assertion at line 68 will pass an undefined value, potentially causing runtime errors.
🔎 Proposed fix
const signatureAlgorithm = proofTypes.jwt?.supportedSignatureAlgorithms[0]
+ if (!signatureAlgorithm) {
+ throw new Error('No supported signature algorithm found in proof types')
+ }
const keys = await Promise.all(
new Array(batchSize).fill(0).map(() =>
kms
.createKeyForSignatureAlgorithm({
- algorithm: signatureAlgorithm!,
+ algorithm: signatureAlgorithm,
// FIXME: what should happen with already existing keys created in the secure environment?
backend: shouldKeyBeHardwareBacked ? 'secureEnvironment' : 'askar',
})📝 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.
| const signatureAlgorithm = proofTypes.jwt?.supportedSignatureAlgorithms[0] | |
| const keys = await Promise.all( | |
| new Array(batchSize).fill(0).map(() => | |
| kms | |
| .createKeyForSignatureAlgorithm({ | |
| algorithm: signatureAlgorithm!, | |
| const signatureAlgorithm = proofTypes.jwt?.supportedSignatureAlgorithms[0] | |
| if (!signatureAlgorithm) { | |
| throw new Error('No supported signature algorithm found in proof types') | |
| } | |
| const keys = await Promise.all( | |
| new Array(batchSize).fill(0).map(() => | |
| kms | |
| .createKeyForSignatureAlgorithm({ | |
| algorithm: signatureAlgorithm, |
🤖 Prompt for AI Agents
In src/controllers/openid4vc/holder/credentialBindingResolver.ts around lines 63
to 68, the code reads the first element of
proofTypes.jwt?.supportedSignatureAlgorithms without checking it and then uses a
non-null assertion when creating keys; validate that signatureAlgorithm is
present before use (e.g., if supportedSignatureAlgorithms is empty or undefined,
throw a clear error or choose a safe default), and only call
kms.createKeyForSignatureAlgorithm when signatureAlgorithm is defined, ensuring
you remove the non-null assertion and surface a meaningful error message so the
code cannot pass undefined into the KMS.
| const storedCredentials = await Promise.all( | ||
| credentialResponse.credentials.map(async (response) => { | ||
| // console.log('response', JSON.stringify(response)) | ||
| // const credential = response.credentials[0] | ||
| // if (credential instanceof W3cJwtVerifiableCredential || credential instanceof W3cJsonLdVerifiableCredential) { | ||
| // return await agentReq.agent.w3cCredentials.storeCredential({ credential }) | ||
| // } | ||
| // if (credential instanceof Mdoc) { | ||
| // return await agentReq.agent.mdoc.store({ record: credential }) | ||
| // } | ||
| const credentialRecord = response.record | ||
| if (credentialRecord instanceof SdJwtVcRecord) { | ||
| return await agentReq.agent.sdJwtVc.store({ | ||
| record: credentialRecord, | ||
| }) | ||
| } | ||
| }), | ||
| ) | ||
|
|
||
| // const grants = resolvedCredentialOffer.credentialOfferPayload.grants | ||
| // console.log('Grants:', grants) | ||
| return storedCredentials as any |
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.
storedCredentials array may contain undefined values.
The map callback only returns a value when credentialRecord instanceof SdJwtVcRecord. For other credential types (Mdoc, W3c), the callback implicitly returns undefined. This results in an array with potential undefined entries.
🔎 Proposed fix
const storedCredentials = await Promise.all(
credentialResponse.credentials.map(async (response) => {
- // console.log('response', JSON.stringify(response))
- // const credential = response.credentials[0]
- // if (credential instanceof W3cJwtVerifiableCredential || credential instanceof W3cJsonLdVerifiableCredential) {
- // return await agentReq.agent.w3cCredentials.storeCredential({ credential })
- // }
- // if (credential instanceof Mdoc) {
- // return await agentReq.agent.mdoc.store({ record: credential })
- // }
const credentialRecord = response.record
if (credentialRecord instanceof SdJwtVcRecord) {
return await agentReq.agent.sdJwtVc.store({
record: credentialRecord,
})
}
+ // TODO: Handle other credential types (Mdoc, W3c) or log unhandled types
+ return undefined
}),
)
- return storedCredentials as any
+ return storedCredentials.filter(Boolean) as any📝 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.
| const storedCredentials = await Promise.all( | |
| credentialResponse.credentials.map(async (response) => { | |
| // console.log('response', JSON.stringify(response)) | |
| // const credential = response.credentials[0] | |
| // if (credential instanceof W3cJwtVerifiableCredential || credential instanceof W3cJsonLdVerifiableCredential) { | |
| // return await agentReq.agent.w3cCredentials.storeCredential({ credential }) | |
| // } | |
| // if (credential instanceof Mdoc) { | |
| // return await agentReq.agent.mdoc.store({ record: credential }) | |
| // } | |
| const credentialRecord = response.record | |
| if (credentialRecord instanceof SdJwtVcRecord) { | |
| return await agentReq.agent.sdJwtVc.store({ | |
| record: credentialRecord, | |
| }) | |
| } | |
| }), | |
| ) | |
| // const grants = resolvedCredentialOffer.credentialOfferPayload.grants | |
| // console.log('Grants:', grants) | |
| return storedCredentials as any | |
| const storedCredentials = await Promise.all( | |
| credentialResponse.credentials.map(async (response) => { | |
| const credentialRecord = response.record | |
| if (credentialRecord instanceof SdJwtVcRecord) { | |
| return await agentReq.agent.sdJwtVc.store({ | |
| record: credentialRecord, | |
| }) | |
| } | |
| // TODO: Handle other credential types (Mdoc, W3c) or log unhandled types | |
| return undefined | |
| }), | |
| ) | |
| return storedCredentials.filter(Boolean) as any |
🤖 Prompt for AI Agents
In src/controllers/openid4vc/holder/holder.service.ts around lines 225-244, the
Promise.all mapping can produce undefined entries because the callback only
returns for SdJwtVcRecord; update the logic so the array passed to Promise.all
contains only valid promises: either (a) handle other credential types by
returning the appropriate store calls for W3C/Mdoc (uncomment/adapt existing
code) or (b) build the array with a conditional push/filter so non-matching
responses are excluded (e.g., map to a promise or null then filter out
null/undefined before Promise.all). Ensure the final returned storedCredentials
contains only resolved credential records (no undefined).
| const { credentialOffer, issuanceSession } = (await agentReq.agent.modules.openid4vc.issuer?.createCredentialOffer({ | ||
| issuerId: publicIssuerId, | ||
| issuanceMetadata: options.issuanceMetadata, | ||
| credentialConfigurationIds: credentials.map((c) => c.credentialSupportedId), | ||
| preAuthorizedCodeFlowConfig: options.preAuthorizedCodeFlowConfig, | ||
| authorizationCodeFlowConfig: options.authorizationCodeFlowConfig, | ||
| })) as { | ||
| credentialOffer: unknown | ||
| issuanceSession: unknown | ||
| } | ||
|
|
||
| return { credentialOffer, issuanceSession } |
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.
Unsafe destructuring with optional chaining.
If issuer?.createCredentialOffer returns undefined (when issuer is nullish), destructuring { credentialOffer, issuanceSession } will throw a runtime error. Add a guard before destructuring.
🔎 Proposed fix
- const { credentialOffer, issuanceSession } = (await agentReq.agent.modules.openid4vc.issuer?.createCredentialOffer({
+ const issuer = agentReq.agent.modules.openid4vc.issuer
+ if (!issuer) {
+ throw new Error('OpenID4VC issuer module is not configured')
+ }
+ const { credentialOffer, issuanceSession } = await issuer.createCredentialOffer({
issuerId: publicIssuerId,
issuanceMetadata: options.issuanceMetadata,
credentialConfigurationIds: credentials.map((c) => c.credentialSupportedId),
preAuthorizedCodeFlowConfig: options.preAuthorizedCodeFlowConfig,
authorizationCodeFlowConfig: options.authorizationCodeFlowConfig,
- })) as {
- credentialOffer: unknown
- issuanceSession: unknown
- }
+ })
return { credentialOffer, issuanceSession }📝 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.
| const { credentialOffer, issuanceSession } = (await agentReq.agent.modules.openid4vc.issuer?.createCredentialOffer({ | |
| issuerId: publicIssuerId, | |
| issuanceMetadata: options.issuanceMetadata, | |
| credentialConfigurationIds: credentials.map((c) => c.credentialSupportedId), | |
| preAuthorizedCodeFlowConfig: options.preAuthorizedCodeFlowConfig, | |
| authorizationCodeFlowConfig: options.authorizationCodeFlowConfig, | |
| })) as { | |
| credentialOffer: unknown | |
| issuanceSession: unknown | |
| } | |
| return { credentialOffer, issuanceSession } | |
| const issuer = agentReq.agent.modules.openid4vc.issuer | |
| if (!issuer) { | |
| throw new Error('OpenID4VC issuer module is not configured') | |
| } | |
| const { credentialOffer, issuanceSession } = await issuer.createCredentialOffer({ | |
| issuerId: publicIssuerId, | |
| issuanceMetadata: options.issuanceMetadata, | |
| credentialConfigurationIds: credentials.map((c) => c.credentialSupportedId), | |
| preAuthorizedCodeFlowConfig: options.preAuthorizedCodeFlowConfig, | |
| authorizationCodeFlowConfig: options.authorizationCodeFlowConfig, | |
| }) | |
| return { credentialOffer, issuanceSession } |
🤖 Prompt for AI Agents
In src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts
around lines 68 to 79, the code destructures the result of
agent.modules.openid4vc.issuer?.createCredentialOffer using optional chaining
which can yield undefined and cause a runtime error; before destructuring, check
that agentReq.agent.modules.openid4vc.issuer exists and that
createCredentialOffer returned a non-null value, then proceed to destructure; if
the issuer is missing or the call returns undefined, handle that case explicitly
(throw a clear error or return a controlled error response/log) so the function
never attempts to destructure undefined.
| public async getIssuersByQuery(agentReq: Req, publicIssuerId?: string) { | ||
| const result = publicIssuerId | ||
| ? (agentReq.agent as Agent<RestAgentModules>).openid4vc.issuer.getIssuerByIssuerId(publicIssuerId) // .dependencyManager.resolve(OpenId4VcIssuerRepository) | ||
| : (agentReq.agent as Agent<RestAgentModules>).openid4vc.issuer.getAllIssuers() | ||
| return result | ||
| } |
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.
Inconsistent API access pattern.
This method accesses openid4vc.issuer directly without optional chaining, while other methods (e.g., createIssuerAgent, getIssuer) use modules.openid4vc.issuer?.. This inconsistency could cause runtime errors if the issuer module is unavailable.
🔎 Proposed fix
public async getIssuersByQuery(agentReq: Req, publicIssuerId?: string) {
- const result = publicIssuerId
- ? (agentReq.agent as Agent<RestAgentModules>).openid4vc.issuer.getIssuerByIssuerId(publicIssuerId) // .dependencyManager.resolve(OpenId4VcIssuerRepository)
- : (agentReq.agent as Agent<RestAgentModules>).openid4vc.issuer.getAllIssuers()
- return result
+ const issuer = agentReq.agent.modules.openid4vc.issuer
+ return publicIssuerId
+ ? issuer?.getIssuerByIssuerId(publicIssuerId)
+ : issuer?.getAllIssuers()
}🤖 Prompt for AI Agents
In src/controllers/openid4vc/issuers/issuer.service.ts around lines 34 to 39,
the code directly accesses openid4vc.issuer which is inconsistent with other
methods and can throw if the module is missing; change the access to use the
same safe pattern (agentReq.agent as
Agent<RestAgentModules>).modules.openid4vc.issuer?.getIssuerByIssuerId(...) or
.getAllIssuers() and handle the case where issuer is undefined (return undefined
or throw a clear error) so the method won’t cause a runtime crash when the
module is absent.
| export interface CreateIssuerOptions { | ||
| issuerId?: string | ||
| accessTokenSignerKeyType?: string | ||
| accessTokenSignerKeyType?: any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
@credo-ts/openid4vc accessTokenSignerKeyType key types supported
💡 Result:
I can fetch the exact supported values — do you want me to search the @credo-ts/openid4vc package source/docs now to list the allowed accessTokenSignerKeyType values?
🏁 Script executed:
# Search for usage of accessTokenSignerKeyType in the codebase
rg "accessTokenSignerKeyType" -A 2 -B 2Repository: credebl/credo-controller
Length of output: 873
🏁 Script executed:
# Check what's imported from @credo-ts/openid4vc and related libraries
rg "@credo-ts" --type ts --type tsx | head -20Repository: credebl/credo-controller
Length of output: 95
🏁 Script executed:
# Look at the issuer.types.ts file to understand the context
cat -n src/controllers/openid4vc/types/issuer.types.ts | sed -n '150,170p'Repository: credebl/credo-controller
Length of output: 994
🏁 Script executed:
# Search for all references to accessTokenSignerKeyType
rg "accessTokenSignerKeyType" -i -B 3 -A 3Repository: credebl/credo-controller
Length of output: 1164
🏁 Script executed:
# Check if there are any type imports or references that hint at the expected type
rg "SignerKeyType|KeyType.*signer|signer.*key" -i --type ts | head -30Repository: credebl/credo-controller
Length of output: 497
🏁 Script executed:
# Look at the @credo-ts packages in package.json to understand the library better
cat package.json | grep -A 5 -B 5 "@credo-ts"Repository: credebl/credo-controller
Length of output: 1050
🏁 Script executed:
# Search for key algorithm constants or enums
rg "ed25519|EC256|RSA" --type ts | head -20Repository: credebl/credo-controller
Length of output: 1375
🏁 Script executed:
# Search for where accessTokenSignerKeyType is processed or validated
rg "accessTokenSignerKeyType" -B 5 -A 10 --type tsRepository: credebl/credo-controller
Length of output: 2655
🏁 Script executed:
# Look for OpenID4VC issuer service implementations
fd "issuer.*service" --type f -e ts | head -10Repository: credebl/credo-controller
Length of output: 119
🏁 Script executed:
# Check if there are any related type definitions or enums in the codebase
rg "KeyType|KeyAlgorithm|SigningAlgorithm" --type ts | grep -i enum -A 5Repository: credebl/credo-controller
Length of output: 50
🏁 Script executed:
# Read the issuer service to understand how accessTokenSignerKeyType is used
cat -n src/controllers/openid4vc/issuers/issuer.service.ts | head -150Repository: credebl/credo-controller
Length of output: 2740
🏁 Script executed:
# Search within the issuer service for accessTokenSignerKeyType handling
rg "accessTokenSignerKeyType" -B 10 -A 10 src/controllers/openid4vc/issuers/Repository: credebl/credo-controller
Length of output: 50
🏁 Script executed:
# Check the size of the issuer.service.ts file
wc -l src/controllers/openid4vc/issuers/issuer.service.tsRepository: credebl/credo-controller
Length of output: 122
🏁 Script executed:
# Check if node_modules or package definitions exist for credo-ts
fd "credo-ts" --type d | head -5Repository: credebl/credo-controller
Length of output: 50
🏁 Script executed:
# Search for any type definitions or documentation about key types
rg "KeyType|KeyAlgorithm" -i --type ts --type json | grep -i "credo\|export\|enum" | head -20Repository: credebl/credo-controller
Length of output: 178
🏁 Script executed:
# Look for common key type patterns in the codebase
rg "\"(ed25519|p256|p384|k256|rsa|x25519|bls12381)\"" --type ts | head -15Repository: credebl/credo-controller
Length of output: 410
Change any to string or specific key type union.
accessTokenSignerKeyType?: any removes type safety. Usage in examples shows string values like "ed25519". Consider typing as string or a union of supported key types (e.g., "ed25519" | "p256" | "p384" | "k256" | "x25519" | "bls12381g1" | "bls12381g2"), matching patterns used elsewhere in the codebase for algorithm selection.
| if (!requestSigner) { | ||
| } else if (requestSigner.method === 'did') { | ||
| } |
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.
Remove empty conditional block - dead code.
This if/else if block has empty bodies and serves no purpose. Either add the intended logic or remove the block entirely.
🔎 Proposed fix
- if (!requestSigner) {
- } else if (requestSigner.method === 'did') {
- }📝 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.
| if (!requestSigner) { | |
| } else if (requestSigner.method === 'did') { | |
| } |
🤖 Prompt for AI Agents
In src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts
around lines 66 to 68, there is an empty if/else-if block checking requestSigner
and requestSigner.method ('did') that contains no code; remove the dead
conditional entirely or replace it with the intended logic. If there was
supposed to be handling for missing requestSigner or a 'did' method, implement
the corresponding actions (e.g., throw a descriptive error or perform the needed
initialization/processing); otherwise delete the entire if/else-if lines to
eliminate dead code and run tests/linter to confirm no usages rely on it.
| if (dto.responseMode === ResponseModeEnum.DC_API || ResponseModeEnum.DC_API_JWT) { | ||
| options.expectedOrigins = dto.expectedOrigins | ||
| } |
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.
Logic error in response mode condition.
The condition dto.responseMode === ResponseModeEnum.DC_API || ResponseModeEnum.DC_API_JWT is incorrect. The second operand ResponseModeEnum.DC_API_JWT is always truthy, so expectedOrigins will always be set regardless of responseMode.
🔎 Proposed fix
- if (dto.responseMode === ResponseModeEnum.DC_API || ResponseModeEnum.DC_API_JWT) {
+ if (dto.responseMode === ResponseModeEnum.DC_API || dto.responseMode === ResponseModeEnum.DC_API_JWT) {
options.expectedOrigins = dto.expectedOrigins
}📝 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.
| if (dto.responseMode === ResponseModeEnum.DC_API || ResponseModeEnum.DC_API_JWT) { | |
| options.expectedOrigins = dto.expectedOrigins | |
| } | |
| if (dto.responseMode === ResponseModeEnum.DC_API || dto.responseMode === ResponseModeEnum.DC_API_JWT) { | |
| options.expectedOrigins = dto.expectedOrigins | |
| } |
🤖 Prompt for AI Agents
In src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts
around lines 74-76, the responseMode condition mistakenly uses `||
ResponseModeEnum.DC_API_JWT` which is always truthy; change the condition to
explicitly compare dto.responseMode against both enums (e.g., `dto.responseMode
=== ResponseModeEnum.DC_API || dto.responseMode ===
ResponseModeEnum.DC_API_JWT`) so options.expectedOrigins is set only when
responseMode matches one of those values.
| public async getVerifiedAuthorizationResponse(request: Req, verificationSessionId: string) { | ||
| const verificationSession = | ||
| await request.agent.modules.openid4vc.verifier?.getVerificationSessionById(verificationSessionId) | ||
| const verified = await request.agent.modules.openid4vc.verifier?.getVerifiedAuthorizationResponse( | ||
| verificationSession!.id, | ||
| ) |
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.
Handle undefined verificationSession before accessing properties.
The verificationSession could be undefined due to optional chaining, but line 116 uses non-null assertion verificationSession!.id. This could cause a runtime error.
🔎 Proposed fix
public async getVerifiedAuthorizationResponse(request: Req, verificationSessionId: string) {
const verificationSession =
await request.agent.modules.openid4vc.verifier?.getVerificationSessionById(verificationSessionId)
+ if (!verificationSession) {
+ throw new Error(`Verification session with id ${verificationSessionId} not found`)
+ }
const verified = await request.agent.modules.openid4vc.verifier?.getVerifiedAuthorizationResponse(
- verificationSession!.id,
+ verificationSession.id,
)🤖 Prompt for AI Agents
In src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts
around lines 112-117, the code uses optional chaining to retrieve
verificationSession but then dereferences it with a non-null assertion
(verificationSession!.id) which can throw at runtime; add an explicit guard
after fetching verificationSession (e.g., if (!verificationSession) throw a
descriptive error or return an appropriate error response/value) and then call
getVerifiedAuthorizationResponse with verificationSession.id (no non-null
assertion), ensuring any downstream callers handle the thrown/returned error.
| const credentialConfigurationSupported = credentialConfiguration[credentialConfigurationId] | ||
|
|
||
| const credential = credentialPayload[0] |
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.
Potential issues with credential configuration and payload access.
credentialConfigurationSupportedis assigned by indexingcredentialConfigurationwithcredentialConfigurationId, but this variable appears unused afterward.credentialPayload[0]is accessed without checking if the array is empty, which could cause runtime errors if no matching credentials are found.
🔎 Proposed fix
- const credentialConfigurationSupported = credentialConfiguration[credentialConfigurationId]
-
- const credential = credentialPayload[0]
+ if (credentialPayload.length === 0) {
+ throw new Error(`No credential payload found for credentialConfigurationId: ${credentialConfigurationId}`)
+ }
+ const credential = credentialPayload[0]📝 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.
| const credentialConfigurationSupported = credentialConfiguration[credentialConfigurationId] | |
| const credential = credentialPayload[0] | |
| if (credentialPayload.length === 0) { | |
| throw new Error(`No credential payload found for credentialConfigurationId: ${credentialConfigurationId}`) | |
| } | |
| const credential = credentialPayload[0] |
🤖 Prompt for AI Agents
In src/utils/oid4vc-agent.ts around lines 215 to 217,
credentialConfigurationSupported is assigned but never used and
credentialPayload[0] is accessed directly which can throw if the array is empty;
remove or use credentialConfigurationSupported as intended (or delete the unused
assignment) and guard access to credentialPayload by checking its length (or
existence) before indexing — if empty, handle the case explicitly (throw a clear
error or return a handled value) so you never access index 0 on an empty array.
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
b00803d to
bbb0e23
Compare
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
|



What?
Resolve type issues from issuer and verifier session
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.