feat: changes to the OID4VC issuance workflow required for Credo-0.6.x #1548
Conversation
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
|
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 pull request introduces keyId support throughout the X.509 certificate and OID4VC issuance stack, updates credential format references from "vc+sd-jwt" to "dc+sd-jwt", refactors credential claim handling from nested objects to a generator-based approach, and canonicalizes key type enum values across the platform. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
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)
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: 4
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/interfaces/oid4vc-issuance.interfaces.ts (1)
93-98: Remove unusedaccessTokenSignerKeyTypefield fromIssuerUpdation.The
accessTokenSignerKeyTypeproperty inIssuerUpdationis declared but never used in the update flow—it's absent fromIssuerUpdationDto, not extracted in the service, and not persisted in the repository. This creates dead code. If this field should support updates to the token signer configuration, align it withIssuerCreationby making it optional with the object type{ kty: string; crv: string }. Otherwise, remove it entirely to eliminate confusion.Note: Line 96 also has an incomplete type annotation on
display;that should be corrected.
🤖 Fix all issues with AI agents
In `@apps/oid4vc-issuance/constant/issuance.ts`:
- Around line 5-8: The accessTokenSignerKeyType constant uses { kty: 'OKP', crv:
'Ed25519' } but other modules expect 'ed25519' and there are three conflicting
AccessTokenSignerKeyType enums; pick a single canonical enum value (either
'Ed25519' or 'ed25519'), update all enum definitions (including in
oid4vc-issuance/interfaces/oid4vc-issuance.interfaces.ts and api-gateway dto),
and change the constant accessTokenSignerKeyType to use the exact string/format
that matches that canonical enum (or replace the duplicate enums with a single
exported enum shared across modules) so the cast to AccessTokenSignerKeyType is
type-correct and all modules share the same value and casing.
In `@apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts`:
- Around line 136-140: The function mapDbFormatToApiFormat is dropping legacy
"vc+sd-jwt" representations causing DB rows or missing formats to throw; update
mapDbFormatToApiFormat to recognize and normalize legacy variants (e.g.,
"vc+sd-jwt") to CredentialFormat.SdJwtVc or, alternatively, change the default
mapping site that falls back to "vc+sd-jwt" to a supported value — specifically,
add "vc+sd-jwt" to the normalized checks in mapDbFormatToApiFormat and ensure
the code path that currently defaults to "vc+sd-jwt" (the default mapping usage)
is updated to use CredentialFormat.SdJwtVc so existing rows continue to resolve
without migration.
In `@libs/common/src/interfaces/x509.interface.ts`:
- Line 216: The property keyId on the X.509 interface is declared as a required
string but must be optional to match the Prisma schema; update the interface
property declaration from keyId: string to keyId?: string (in the interface that
declares the X.509 certificate fields), then fix any places that assume it is
always present (add null/undefined guards or optional chaining where the code
accesses keyId) and run type checks/tests to ensure no other type errors remain.
In `@libs/enum/src/enum.ts`:
- Around line 288-291: The x5cKeyType enum was changed from legacy literal
strings which will break persisted data and clients; restore backward
compatibility by adding legacy aliases or a compatibility mapping (e.g., accept
old strings in parsing/serialization code that handles x5cKeyType and map them
to the new enum values), implement a one-time data migration script to update
persisted records to the new values, update any (de)serializer functions and
validation logic to handle both old and new strings, and add tests and docs
noting the deprecation and the mapping; search for x5cKeyType (and the similar
enum referenced at lines ~321-323) to apply the same compatibility changes.
🧹 Nitpick comments (5)
libs/prisma-service/prisma/migrations/20260119085820_added_key_id_in_x509_certificates_table/migration.sql (1)
1-2: Consider indexing/uniqueness forkeyIdif it’s used for lookups.
IfkeyIdbecomes a lookup key or unique identifier, adding an index (or unique constraint) will prevent duplicate data and speed queries.apps/x509/src/x509.service.ts (1)
176-177: Consider handlinggeneralizedTimein addition toutcTime.X.509 certificates use
utcTimefor dates before 2050 andgeneralizedTimefor dates from 2050 onwards. Accessing only.utcTimemay fail for certificates with validity extending beyond 2050.Proposed defensive approach
- const validFrom = new Date(decodedCert.asn.tbsCertificate.validity.notBefore.utcTime); - const expiry = new Date(decodedCert.asn.tbsCertificate.validity.notAfter.utcTime); + const notBefore = decodedCert.asn.tbsCertificate.validity.notBefore; + const notAfter = decodedCert.asn.tbsCertificate.validity.notAfter; + const validFrom = new Date(notBefore.utcTime ?? notBefore.generalizedTime); + const expiry = new Date(notAfter.utcTime ?? notAfter.generalizedTime);apps/oid4vc-issuance/libs/helpers/issuer.metadata.ts (3)
247-304: Remove commented-out code.This large block of commented-out code (
buildNestedClaims,generateObjects) should be removed rather than left in the codebase. Dead code clutters the file and can cause confusion for future maintainers. If needed for reference, Git history preserves the old implementation.
305-334: Consider adding a type forattributesparameter.The
any[]type forattributesreduces type safety. Consider defining or importing a proper type (e.g.,CredentialAttribute[]) to improve maintainability and catch errors at compile time.♻️ Suggested improvement
-// eslint-disable-next-line `@typescript-eslint/no-explicit-any` -function generateClaims(attributes: any[], namespace?: string, parentPath: string[] = []): Claim[] { +import { CredentialAttribute } from 'apps/oid4vc-issuance/interfaces/oid4vc-template.interfaces'; + +function generateClaims(attributes: CredentialAttribute[], namespace?: string, parentPath: string[] = []): Claim[] {
341-359: Simplify return type toClaim[].The return type
Record<string, Claim> | Claim[]is misleading since both code paths (MDOC and SD-JWT) now returnClaim[]. TheRecord<string, Claim>appears to be a remnant from the removedbuildNestedClaimsimplementation.♻️ Suggested fix
-function buildClaimsFromTemplate(template: SdJwtTemplate | MdocTemplate): Record<string, Claim> | Claim[] { +function buildClaimsFromTemplate(template: SdJwtTemplate | MdocTemplate): Claim[] {
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
GHkrishna
changed the title
GHkrishna
changed the title
| */ | ||
| function buildNestedClaims(attributes: CredentialAttribute[]): Record<string, Claim> { | ||
| const claims: Record<string, Claim> = {}; | ||
| // function buildNestedClaims(attributes: CredentialAttribute[]): Record<string, Claim> { |
There was a problem hiding this comment.
Can we remove this commented code?
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
feat: update codebase for Credo 6.1 compatibility
|
shitrerohit
merged commit e6e69a3
into
feat/credo6.1-version-upgradation
What?
Made changes to the OID4VC issuance workflow required for Credo 6.1 version support.
How?
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.