feat: noAuth support for oid4vc issuance - #1606
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new "noAuth" authentication option across the OID4VC issuance types and builder: enum, DTO validation/type, and credential-offer builder logic now recognize and handle ChangesNo-Auth Authorization Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 46 minutes and 38 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts (1)
521-540:⚠️ Potential issue | 🟡 Minor
dto.authorizationTypeis only honored fornoAuth; theauthorizationCodeFlowselection can be silently overridden.After this change the builder partially trusts
dto.authorizationType:
'noAuth'→ empty pre-auth config (priority 1, new).'preAuthorizedCodeFlow'/'authorizationCodeFlow'→ not looked at; priority 2 always produces a pre-authorized flow wheneverissuerDetails.authorizationServerUrlis set.So if a caller sends
authorizationType: 'authorizationCodeFlow'but the issuer record has anauthorizationServerUrl, they silently get a pre-authorized flow withDEFAULT_TXCODE. Consider branching priority 2 ondto.authorizationTypetoo, e.g.:♻️ Suggested branching
- const overrideAuthorizationServerUrl = issuerDetails?.authorizationServerUrl; - if (overrideAuthorizationServerUrl) { - if ('string' !== typeof overrideAuthorizationServerUrl || '' === overrideAuthorizationServerUrl.trim()) { - throw new BadRequestException('issuerDetails.authorizationServerUrl must be a non-empty string when provided'); - } - return { - ...baseEnvelope, - preAuthorizedCodeFlowConfig: { - txCode: DEFAULT_TXCODE, - authorizationServerUrl: overrideAuthorizationServerUrl - } - }; - } + const overrideAuthorizationServerUrl = issuerDetails?.authorizationServerUrl; + if (overrideAuthorizationServerUrl) { + if ('string' !== typeof overrideAuthorizationServerUrl || '' === overrideAuthorizationServerUrl.trim()) { + throw new BadRequestException('issuerDetails.authorizationServerUrl must be a non-empty string when provided'); + } + if (dto.authorizationType === 'authorizationCodeFlow') { + return { + ...baseEnvelope, + authorizationCodeFlowConfig: { authorizationServerUrl: overrideAuthorizationServerUrl } + }; + } + return { + ...baseEnvelope, + preAuthorizedCodeFlowConfig: { + txCode: DEFAULT_TXCODE, + authorizationServerUrl: overrideAuthorizationServerUrl + } + }; + }This may be pre-existing behaviour, but since this PR starts using
dto.authorizationTypein this function, the asymmetry is now user-visible and worth aligning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts` around lines 521 - 540, The code currently forces a pre-authorized flow whenever issuerDetails.authorizationServerUrl is set, ignoring dto.authorizationType; change the branch that handles overrideAuthorizationServerUrl so it respects dto.authorizationType: if dto.authorizationType === 'noAuth' return the empty/no-auth envelope as before; if dto.authorizationType === 'preAuthorizedCodeFlow' return baseEnvelope with preAuthorizedCodeFlowConfig using DEFAULT_TXCODE and authorizationServerUrl; if dto.authorizationType === 'authorizationCodeFlow' do not force a pre-authorized config and fall through to the existing XOR validation of dto.preAuthorizedCodeFlowConfig vs dto.authorizationCodeFlowConfig; update the logic around overrideAuthorizationServerUrl, dto.authorizationType, preAuthorizedCodeFlowConfig, authorizationCodeFlowConfig, baseEnvelope and DEFAULT_TXCODE accordingly.
🧹 Nitpick comments (1)
apps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.ts (1)
23-27: Inconsistent enum value casing / semantics.The pre-existing members are OAuth spec grant-type strings (
'pre-authorized_code','authorization_code'), whileNO_AUTH = 'noAuth'is camelCase and matches the gateway DTO selector ('preAuthorizedCodeFlow' | 'authorizationCodeFlow' | 'noAuth') rather than any spec value. As a result, onlyNO_AUTH's value actually equals what the DTO sends; the other two enum values would never matchdto.authorizationTypeby equality. Consider either:
- renaming to a consistent style (e.g.,
NO_AUTH = 'no_auth'/'none') if this is meant to be a grant type, or- reusing the same literal set as the DTO union and retiring the spec-style values here to prevent a future bug where someone writes
dto.authorizationType === AuthenticationType.PRE_AUTHORIZED_CODEand it silently never matches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.ts` around lines 23 - 27, AuthenticationType's string values are inconsistent with the gateway DTO: PRE_AUTHORIZED_CODE and AUTHORIZATION_CODE use OAuth spec literals while NO_AUTH uses the DTO literal ('noAuth'), causing equality checks against dto.authorizationType to fail; update the AuthenticationType enum (the enum and its members PRE_AUTHORIZED_CODE, AUTHORIZATION_CODE, NO_AUTH) so all member values match the DTO union literals ('preAuthorizedCodeFlow' | 'authorizationCodeFlow' | 'noAuth') (or alternatively make all values spec-style and update the DTO usage) and ensure any comparisons like dto.authorizationType === AuthenticationType.PRE_AUTHORIZED_CODE now succeed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-gateway/src/oid4vc-issuance/dtos/issuer-sessions.dto.ts`:
- Around line 182-189: The DTO's authorizationType literals
('preAuthorizedCodeFlow', 'authorizationCodeFlow', 'noAuth') don't match the
AuthenticationType enum ('pre-authorized_code', 'authorization_code', 'noAuth'),
causing equality checks in credential-sessions.builder.ts to fail; fix by either
updating issuer-sessions.dto's authorizationType enum and IsIn values to use the
exact AuthenticationType members (e.g., 'pre-authorized_code' and
'authorization_code') and ApiProperty example, or add a single deterministic
mapping function (e.g., mapAuthorizationType(dto.authorizationType) used in
credential-sessions.builder.ts) that converts the DTO string values to
AuthenticationType enum values before comparisons, and update any uses of
authorizationType to use the mapped value.
---
Outside diff comments:
In `@apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts`:
- Around line 521-540: The code currently forces a pre-authorized flow whenever
issuerDetails.authorizationServerUrl is set, ignoring dto.authorizationType;
change the branch that handles overrideAuthorizationServerUrl so it respects
dto.authorizationType: if dto.authorizationType === 'noAuth' return the
empty/no-auth envelope as before; if dto.authorizationType ===
'preAuthorizedCodeFlow' return baseEnvelope with preAuthorizedCodeFlowConfig
using DEFAULT_TXCODE and authorizationServerUrl; if dto.authorizationType ===
'authorizationCodeFlow' do not force a pre-authorized config and fall through to
the existing XOR validation of dto.preAuthorizedCodeFlowConfig vs
dto.authorizationCodeFlowConfig; update the logic around
overrideAuthorizationServerUrl, dto.authorizationType,
preAuthorizedCodeFlowConfig, authorizationCodeFlowConfig, baseEnvelope and
DEFAULT_TXCODE accordingly.
---
Nitpick comments:
In `@apps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.ts`:
- Around line 23-27: AuthenticationType's string values are inconsistent with
the gateway DTO: PRE_AUTHORIZED_CODE and AUTHORIZATION_CODE use OAuth spec
literals while NO_AUTH uses the DTO literal ('noAuth'), causing equality checks
against dto.authorizationType to fail; update the AuthenticationType enum (the
enum and its members PRE_AUTHORIZED_CODE, AUTHORIZATION_CODE, NO_AUTH) so all
member values match the DTO union literals ('preAuthorizedCodeFlow' |
'authorizationCodeFlow' | 'noAuth') (or alternatively make all values spec-style
and update the DTO usage) and ensure any comparisons like dto.authorizationType
=== AuthenticationType.PRE_AUTHORIZED_CODE now succeed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f41adfbf-419f-4610-ab52-75d4d4cfd048
📒 Files selected for processing (3)
apps/api-gateway/src/oid4vc-issuance/dtos/issuer-sessions.dto.tsapps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.tsapps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
d806db7 to
a586512
Compare
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
|
2 participants
What
Summary by CodeRabbit