feat:API for verify the authorization response#344
feat:API for verify the authorization response#344shitrerohit merged 4 commits intofeat/oidc-main-syncfrom
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 Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR extends OpenID4VC holder and verifier functionality to support DCQL proof request verification. It propagates authorization options through the resolution flow, introduces a new verification endpoint for handling DCQL authorization responses with session tracking, and updates result handling to store complete submission results. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
|
![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
🤖 Fix all issues with AI agents
In `@src/controllers/openid4vc/holder/holder.service.ts`:
- Around line 231-233: Don't mutate submissionResult.serverResponse in place or
overwrite its body; instead create and return a new object that copies needed
HTTP metadata (e.g., status, headers) from submissionResult.serverResponse (or
uses defaults) and preserves its original body, and attach the full submission
payload under a new key (e.g., submission or submissionResult). Concretely, stop
assigning submissionResult.serverResponse to result and then setting
result['body'] = submissionResult; instead build a fresh object that merges
metadata from serverResponse and sets a separate field for the submission
payload so serverResponse.body is not discarded and no library-owned object is
mutated.
In
`@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`:
- Around line 203-210: The parameter name verifydcqlProofRquest in method
verifyDcqlProofRequest is misspelled and inconsistently cased; rename it to
verifyDCQLProofRequest (or verifyDcqlProofRequest -> verifyDCQLProofRequest)
throughout the service method signature and all call sites (including the
corresponding controller) and update the spread usage in
verifier.verifyAuthorizationResponse({ ...verifyDCQLProofRequest }) to use the
corrected identifier so types (OpenId4VCDCQLVerificationSessionRecord) and
naming are consistent.
- Line 208: Replace the incorrect call to
verifier.verifyAuthorizationResponse(...verifydcqlProofRquest) with
verifier.getVerifiedAuthorizationResponse(verificationSessionId); specifically,
stop spreading the verifydcqlProofRquest object and instead pass only the
verificationSessionId (the same id used earlier on line 118 usage), update the
surrounding code to accept the returned verified response type (remove the as
any cast if possible) and rename the local variable if needed to reflect that it
contains the verified authorization response.
🧹 Nitpick comments (3)
src/controllers/openid4vc/holder/holder.service.ts (1)
217-223: No-op try-catch — remove or add meaningful handling.The
catch (error) { throw error }block does nothing; it re-throws the same error. Either remove the try-catch entirely or add logging/wrapping before re-throwing.♻️ 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, + )src/controllers/openid4vc/verifier-sessions/verification-sessions.Controller.ts (2)
90-103: Endpoint looks consistent with existing patterns; fix the parameter typo here too.The new endpoint follows the established try/catch +
ErrorHandlingService.handlepattern. Apply the same parameter rename (verifydcqlProofRquest→verifyDcqlProofRequest) as noted in the service file.✏️ Proposed rename
public async verifyDcqlProofRequest( `@Request`() request: Req, - `@Body`() verifydcqlProofRquest: OpenId4VCDCQLVerificationSessionRecord, + `@Body`() verifyDcqlProofRequest: OpenId4VCDCQLVerificationSessionRecord, ) { try { - return await this.verificationSessionService.verifyDcqlProofRequest(request, verifydcqlProofRquest) + return await this.verificationSessionService.verifyDcqlProofRequest(request, verifyDcqlProofRequest) } catch (error) { throw ErrorHandlingService.handle(error) } }
90-92: Consider adding a JSDoc note about expectedresponseModevalues.The PR description mentions this endpoint handles
dc_apianddc_api.jwtresponse modes, but the doc comment doesn't clarify this constraint. A brief note would help API consumers understand when to use this endpoint versus the existing flow.
src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts
Outdated
Show resolved
Hide resolved
src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts
Show resolved
Hide resolved
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: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
shitrerohit
force-pushed
the
feat/oid4vc-verify-authorization-response
branch
from
6748c94 to
b130bee
Compare
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts (1)
14-23:⚠️ Potential issue | 🔴 CriticalDuplicate imports will cause compilation error.
ClientIdPrefixandOpenId4VcIssuerX5cOptionsare imported twice in the same import block (lines 15/20 and 17/21 respectively). This will fail TypeScript compilation.🔧 Proposed fix to remove duplicates
import { ClientIdPrefix, CreateAuthorizationRequest, OpenId4VcIssuerX5cOptions, OpenId4VCDCQLVerificationSessionRecord, OpenId4VcIssuerX5c, - ClientIdPrefix, - OpenId4VcIssuerX5cOptions, ResponseModeEnum, } from '../types/verifier.types'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts` around lines 14 - 23, The import block in verification-sessions.service.ts contains duplicate symbols (ClientIdPrefix and OpenId4VcIssuerX5cOptions) which will break TypeScript compilation; edit the import list where ClientIdPrefix and OpenId4VcIssuerX5cOptions are listed and remove the duplicate occurrences so each symbol is imported only once (keep one entry for ClientIdPrefix and one for OpenId4VcIssuerX5cOptions alongside the other imports such as CreateAuthorizationRequest, OpenId4VCDCQLVerificationSessionRecord, OpenId4VcIssuerX5c, and ResponseModeEnum).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`:
- Around line 14-23: The import block in verification-sessions.service.ts
contains duplicate symbols (ClientIdPrefix and OpenId4VcIssuerX5cOptions) which
will break TypeScript compilation; edit the import list where ClientIdPrefix and
OpenId4VcIssuerX5cOptions are listed and remove the duplicate occurrences so
each symbol is imported only once (keep one entry for ClientIdPrefix and one for
OpenId4VcIssuerX5cOptions alongside the other imports such as
CreateAuthorizationRequest, OpenId4VCDCQLVerificationSessionRecord,
OpenId4VcIssuerX5c, and ResponseModeEnum).
---
Duplicate comments:
In
`@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`:
- Line 216: The code incorrectly calls verifier.verifyAuthorizationResponse(...)
which doesn't exist; replace this by waiting until the verification session
reaches the ResponseVerified state and then call
verifier.getVerifiedAuthorizationResponse(verificationSessionId) (the same
pattern used elsewhere in this file), passing the verificationSessionId and
handling/typing the returned result appropriately; remove the invalid
verifyAuthorizationResponse call and adapt any downstream logic to use the
object returned by getVerifiedAuthorizationResponse instead.
- Line 210: Rename the misspelled parameter verifydcqlProofRquest to
verifyDcqlProofRequest (camelCase) in the function/method declaration that
accepts OpenId4VCDCQLVerificationSessionRecord, and update every use site inside
the function (references, destructuring, JSDoc, and any callers) to the new
identifier; ensure exports, type annotations and tests that reference
verifydcqlProofRquest are updated to the new name so compilation and imports
stay consistent.
2 participants
What?
API for verify authorization response
How?
Created a new route to verify the authorization response received from a proof request when the response mode is
dc_apiordc_api.jwt.Summary by CodeRabbit
New Features
Bug Fixes