feat: credo v6.1 upgrade #1554
Open
feat: credo v6.1 upgrade #1554
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughRefactors credential metadata layout, renames credential format from "vc+sd-jwt" to "dc+sd-jwt", adds Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant IssuanceSvc as Issuance Service
participant X509Svc as X509 Service/Repo
participant DB as Database
participant VerificationSvc as Verification Service
Client->>IssuanceSvc: request create issuer/session (X5C)
IssuanceSvc->>X509Svc: fetch active certificate by key type (P-256 / Ed25519)
X509Svc->>DB: query x509_certificates (includes keyId)
DB-->>X509Svc: certificate + keyId
X509Svc-->>IssuanceSvc: certificateBase64 + keyId
IssuanceSvc->>IssuanceSvc: validate keyId present
IssuanceSvc-->>Client: session payload (requestSigner: { method:X5C, x5c:[...], keyId })
Client->>VerificationSvc: submit presentation request with requestSigner
VerificationSvc->>X509Svc: resolve certificate & keyId for verification
X509Svc-->>VerificationSvc: certificate + keyId
VerificationSvc-->>Client: verification result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/oid4vc-verification/src/oid4vc-verification.service.ts (1)
243-246:⚠️ Potential issue | 🟡 MinorFix typo in error messages: double closing braces.
The error messages contain
}}instead of}in the template literal, which will produce malformed output like"No active certificate(X509_P256}) found for issuer".🐛 Proposed fix
if (!activeCertificate) { throw new NotFoundException( - `No active certificate(${sessionRequest.requestSigner.method}}) found for issuer` + `No active certificate(${sessionRequest.requestSigner.method}) found for issuer` ); }Apply the same fix at line 264.
Also applies to: 262-266
🤖 Fix all issues with AI agents
In `@apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts`:
- Around line 136-139: The mapping function mapDbFormatToApiFormat is missing
legacy 'vc+sd-jwt' variants so stored templates using that token will mis-map;
update the includes check in mapDbFormatToApiFormat to accept 'vc+sd-jwt' (and
lower-cased variants like 'vc+sdjwt'/'vc+sd+jwt') alongside the existing
'sd-jwt'/'dc+sd-jwt' entries, and mirror the same acceptance in the other
mapping/fallback logic referenced around the credential session handling (the
fallback that currently masks missing format data) so both places recognize the
legacy token rather than falling back.
In `@apps/oid4vc-issuance/libs/helpers/issuer.metadata.ts`:
- Around line 341-359: The buildClaimsFromTemplate function's declared return
type unnecessarily includes Record<string, Claim> while both branches return
Claim[]; update the function signature to return Claim[] (not Record<string,
Claim> | Claim[]) and ensure callers (e.g., where credential_metadata.claims is
assigned) still accept Claim[]; you can keep the existing logic using
generateClaims and the MdocTemplate handling (namespaces loop) but narrow the
signature and remove any related casting/comments that assumed a Record return
type so types align with generateClaims, SdJwtTemplate, and MdocTemplate.
In `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts`:
- Around line 551-553: activeCertificate.keyId can be null and you must fail
fast before building the signer payload; add a guard where you construct the
signer options (the blocks that set x5c: [activeCertificate.certificateBase64],
keyId: activeCertificate.keyId) to check if activeCertificate.keyId is present
and if not throw a clear Error (or call the re-issue/backfill routine) with
context (e.g., certificate id/serial) so you never pass keyId: undefined to the
signer; apply the same guard to both the P256 and Ed25519 branches (also around
the other occurrence at the 568–570 region).
In `@apps/oid4vc-verification/src/oid4vc-verification.service.ts`:
- Around line 256-271: The X509_ED25519 branch handling in
oid4vc-verification.service.ts is missing a push to activeCertificateDetails,
causing inconsistency with the X509_P256 branch; after retrieving
activeCertificate via oid4vpRepository.getCurrentActiveCertificate(orgId,
x5cKeyType.Ed25519) and before creating the requestSigner object
(SignerMethodOption.X5C), add a push to activeCertificateDetails (the same
structure used in the X509_P256 branch) so activeCertificateDetails contains the
activeCertificate.certificateBase64 and keyId for downstream use.
In `@apps/x509/src/x509.service.ts`:
- Around line 176-178: The parsing assumes
decodedCert.asn.tbsCertificate.validity.notBefore.utcTime and .notAfter.utcTime
exist; update the logic in the function that builds CertificateDateCheckDto to
defensively read validity by checking both notBefore.utcTime and
notBefore.generalTime (and same for notAfter), using the present value as the
input to new Date(...), validate the resulting Date objects
(isFinite/date.getTime) and handle missing/invalid dates by returning/throwing a
clear error or populating CertificateDateCheckDto with an explicit error state
instead of creating an Invalid Date; reference decodedCert,
asn.tbsCertificate.validity.notBefore, notAfter, and CertificateDateCheckDto to
locate and apply the fix.
- Around line 165-170: The code reads nested, optional fields from
decodedResult.response (e.g., publicJwk.jwk.jwk.crv, issuerCertificate, keyId)
without guarding for undefined and casts crv to x5cKeyType; update the logic to
null-check these agent response fields before use: check decodedResult.response
and decodedResult.response.publicJwk exist, extract crv from publicJwk safely
and only compute isValidKeyType and assign certificateDateCheckDto.keyType when
crv is non-null and matches an x5cKeyType (avoid using crv as x5cKeyType on
possibly undefined values), and similarly guard any usage of
decodedResult.response.issuerCertificate and decodedResult.response.keyId so
they are only accessed/assigned when present.
🧹 Nitpick comments (3)
apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer.dto.ts (1)
169-181: Consider adding@Min(1)validation for consistency.
IssuerCreationDto.batchCredentialIssuanceSizenow has@Min(1)validation, butIssuerUpdationDto.batchCredentialIssuanceSizelacks it. This allows update operations to set invalid values that creation would reject.Proposed fix
+import { IsString, IsOptional, IsArray, ValidateNested, IsUrl, IsInt, Min } from 'class-validator'; + // ... in IssuerUpdationDto `@IsOptional`() `@IsInt`({ message: 'batchCredentialIssuanceSize must be an integer' }) + `@Min`(1, { message: 'batchCredentialIssuanceSize must be greater than 0' }) batchCredentialIssuanceSize?: number;apps/oid4vc-issuance/interfaces/oid4vc-issuance.interfaces.ts (1)
57-70: InconsistentaccessTokenSignerKeyTypetyping across interfaces.
IssuerCreation(line 57) andIssuerInitialConfig(line 70) now use the inline object type{ kty: string; crv: string }, butIssuerUpdation(line 95) still references theAccessTokenSignerKeyTypeenum. This inconsistency could cause type mismatches when updating an issuer.Consider aligning
IssuerUpdationto use the same object type, or extract a reusable type alias for consistency.♻️ Suggested refactor
+export interface KeyTypeDescriptor { + kty: string; + crv: string; +} export interface IssuerCreation { authorizationServerUrl: string; issuerId: string; - accessTokenSignerKeyType?: { kty: string; crv: string }; + accessTokenSignerKeyType?: KeyTypeDescriptor; display: Display[]; dpopSigningAlgValuesSupported?: string[]; authorizationServerConfigs: AuthorizationServerConfig; batchCredentialIssuanceSize: number; } export interface IssuerInitialConfig { issuerId: string; // eslint-disable-next-line `@typescript-eslint/ban-types` display: Display[] | {}; // eslint-disable-next-line `@typescript-eslint/ban-types` authorizationServerConfigs: AuthorizationServerConfig | {}; - accessTokenSignerKeyType: { kty: string; crv: string }; + accessTokenSignerKeyType: KeyTypeDescriptor; dpopSigningAlgValuesSupported: string[]; batchCredentialIssuance?: object; credentialConfigurationsSupported: object; } export interface IssuerUpdation { issuerId: string; - accessTokenSignerKeyType: AccessTokenSignerKeyType; + accessTokenSignerKeyType: KeyTypeDescriptor; display; batchCredentialIssuanceSize?: number; }Also applies to: 93-96
apps/oid4vc-verification/src/oid4vc-verification.service.ts (1)
237-271: Consider consolidating X509 signer branches to reduce duplication.The X509_P256 and X509_ED25519 branches share nearly identical logic—only the key type differs. Extracting a helper or collapsing the conditions would reduce maintenance burden and risk of divergence (as seen with the missing
activeCertificateDetails.push).♻️ Suggested refactor
- } else if (sessionRequest.requestSigner.method === SignerOption.X509_P256) { - this.logger.debug('X5C based request signer method selected'); - - const activeCertificate = await this.oid4vpRepository.getCurrentActiveCertificate(orgId, x5cKeyType.P256); - this.logger.debug(`activeCertificate=${JSON.stringify(activeCertificate)}`); - - if (!activeCertificate) { - throw new NotFoundException( - `No active certificate(${sessionRequest.requestSigner.method}) found for issuer` - ); - } - - requestSigner = { - method: SignerMethodOption.X5C, - x5c: [activeCertificate.certificateBase64], - keyId: activeCertificate.keyId - }; - - activeCertificateDetails.push(activeCertificate); - } else if (sessionRequest.requestSigner.method === SignerOption.X509_ED25519) { - this.logger.debug('X5C based request signer method selected'); - - const activeCertificate = await this.oid4vpRepository.getCurrentActiveCertificate(orgId, x5cKeyType.Ed25519); - this.logger.debug(`activeCertificate=${JSON.stringify(activeCertificate)}`); - - if (!activeCertificate) { - throw new NotFoundException( - `No active certificate(${sessionRequest.requestSigner.method}) found for issuer` - ); - } - requestSigner = { - method: SignerMethodOption.X5C, - x5c: [activeCertificate.certificateBase64], - keyId: activeCertificate.keyId - }; + } else if ( + sessionRequest.requestSigner.method === SignerOption.X509_P256 || + sessionRequest.requestSigner.method === SignerOption.X509_ED25519 + ) { + this.logger.debug('X5C based request signer method selected'); + const keyType = + sessionRequest.requestSigner.method === SignerOption.X509_P256 ? x5cKeyType.P256 : x5cKeyType.Ed25519; + + const activeCertificate = await this.oid4vpRepository.getCurrentActiveCertificate(orgId, keyType); + this.logger.debug(`activeCertificate=${JSON.stringify(activeCertificate)}`); + + if (!activeCertificate) { + throw new NotFoundException( + `No active certificate(${sessionRequest.requestSigner.method}) found for issuer` + ); + } + + requestSigner = { + method: SignerMethodOption.X5C, + x5c: [activeCertificate.certificateBase64], + keyId: activeCertificate.keyId + }; + + activeCertificateDetails.push(activeCertificate); } else {
Comment on lines
+165
to
+193
| const { publicJwk } = decodedResult.response; | ||
| const decodedCert = decodedResult.response.x509Certificate; | ||
|
|
||
| this.logger.log(`Start validating certificate`); | ||
| const isValidKeyType = Object.values(x5cKeyType).includes(publicKey.keyType as x5cKeyType); | ||
|
|
||
| const crv = publicJwk?.jwk?.jwk?.crv; | ||
| const isValidKeyType = Object.values(x5cKeyType).includes(crv as x5cKeyType); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the x509.service.ts file to see the context around the mentioned lines
cat -n apps/x509/src/x509.service.ts | sed -n '160,225p'Repository: credebl/platform
Length of output: 3392
🏁 Script executed:
# Let's also check the imports to understand what's being used
cat -n apps/x509/src/x509.service.ts | head -50Repository: credebl/platform
Length of output: 2119
🌐 Web query:
Credo 6.1 x509 decode certificate response fields structure publicJwk issuerCertificate keyId
💡 Result:
In Credo (@credo-ts/core) v0.6.1 (what you referred to as “6.1”) the X.509 “decode certificate” result is essentially a key material + identifier object, meant to let Credo verify signatures (COSE/JWS) using the public key carried in the leaf certificate. (jsdelivr.com)
A practical TypeScript shape for the response fields you listed is:
type X509DecodeCertificateResult = {
/** Public key extracted from the (leaf) X.509 certificate, expressed as a JWK */
publicJwk: Record<string, unknown> // JWK (EC/RSA/OKP), e.g. { kty, crv, x, y } or { kty, n, e }
/**
* Optional issuer/CA certificate used to validate the leaf certificate.
* (Depending on the call/site this may be a single issuer cert or part of a chain.)
*/
issuerCertificate?: string // typically PEM or base64 DER
/**
* Key identifier to correlate this key in headers (e.g., `kid`) and for lookup/caching.
* Commonly derived from the JWK / certificate thumbprint-like material.
*/
keyId: string
}Field meaning (how to think about them):
publicJwk: the public key inside the X.509 certificate converted to JWK form (this is the key used for signature verification). (connect2id.com)issuerCertificate: the issuer/CA certificate (or a chain element) that can be used to validate the leaf cert before trusting its embedded public key. (connect2id.com)keyId: a key identifier suitable for putting intokid(or mapping a receivedkidto the right key). Implementations commonly derive this from thumbprint-like data (JWK thumbprint / X.509 thumbprint / SKI). (codegenes.net)
If you paste the exact Credo API method you’re calling (e.g., the function name + the TypeScript type you see in IntelliSense), I can map this to the precise Credo type names and the exact encoding (PEM vs base64 DER) used for issuerCertificate.
Citations:
- 1: https://www.jsdelivr.com/package/npm/%40credo-ts/core?utm_source=openai
- 2: https://connect2id.com/products/nimbus-jose-jwt/examples/parse-x509-certificate-extract-keys?utm_source=openai
- 3: https://connect2id.com/products/nimbus-jose-jwt/examples/parse-x509-certificate-extract-keys?utm_source=openai
- 4: https://www.codegenes.net/blog/how-to-properly-consume-openid-connect-jwks-uri-metadata-in-c/?utm_source=openai
🏁 Script executed:
# Find the _importX509CertificateForOrg method
rg -A 15 "_importX509CertificateForOrg" apps/x509/src/x509.service.tsRepository: credebl/platform
Length of output: 1561
🏁 Script executed:
# Check the X509 interfaces to see the response type definitions
cat -n apps/x509/src/interfaces/x509.interface.tsRepository: credebl/platform
Length of output: 1663
🏁 Script executed:
# Also check common interfaces
fd -t f "x509.interface" --exec cat -n {} \;Repository: credebl/platform
Length of output: 7930
Add null/undefined checks for optional agent response fields.
Lines 165-170, 182-183, 212-213, and 218-220 access publicJwk.jwk.jwk.crv, issuerCertificate, and keyId from agent responses without explicit guards. Per Credo 0.6.1 documentation, issuerCertificate is optional and the response type is untyped. Additionally, line 170 uses a type assertion (crv as x5cKeyType) on a value that could be undefined from optional chaining, and line 182 assigns this potentially undefined crv to certificateDateCheckDto.keyType which expects x5cKeyType. Add explicit null checks before using these fields or declare the response type to ensure TypeScript validates field presence.
🤖 Prompt for AI Agents
In `@apps/x509/src/x509.service.ts` around lines 165 - 170, The code reads nested,
optional fields from decodedResult.response (e.g., publicJwk.jwk.jwk.crv,
issuerCertificate, keyId) without guarding for undefined and casts crv to
x5cKeyType; update the logic to null-check these agent response fields before
use: check decodedResult.response and decodedResult.response.publicJwk exist,
extract crv from publicJwk safely and only compute isValidKeyType and assign
certificateDateCheckDto.keyType when crv is non-null and matches an x5cKeyType
(avoid using crv as x5cKeyType on possibly undefined values), and similarly
guard any usage of decodedResult.response.issuerCertificate and
decodedResult.response.keyId so they are only accessed/assigned when present.
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>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
shitrerohit
force-pushed
the
feat/credo6.1-version-upgradation
branch
from
e6e69a3 to
2fec541
Compare
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/issuer.metadata.ts (1)
458-478:⚠️ Potential issue | 🟠 MajorGuard against null
appearanceJsonbefore accessing.display.If
appearanceis null/invalid, the current access can throw at runtime even though appearance is optional. A null-safe guard prevents issuance metadata building from crashing.🛠️ Suggested fix
- const displayConfigurations = - (appearanceJson as Appearance).display?.map((displayEntry) => ({ + const appearance = appearanceJson as Appearance | null; + const displayConfigurations = + appearance?.display?.map((displayEntry) => ({ name: displayEntry.name, description: displayEntry.description, locale: displayEntry.locale, logo: displayEntry.logo ? { uri: displayEntry.logo.uri, alt_text: displayEntry.logo.alt_text } : undefined - })) ?? []; + })) ?? [];
🤖 Fix all issues with AI agents
In `@apps/oid4vc-verification/src/oid4vc-verification.service.ts`:
- Around line 260-262: In the NotFoundException thrown in
oid4vc-verification.service.ts (inside the code path that constructs the "No
active certificate" message for sessionRequest.requestSigner.method), remove the
stray extra brace so the template string is valid; locate the throw new
NotFoundException(...) expression that currently contains
`sessionRequest.requestSigner.method}}` and correct it to use a single
interpolation placeholder for the method value.
🧹 Nitpick comments (2)
apps/oid4vc-issuance/constant/issuance.ts (1)
5-8: Prefer narrowingktyand avoiding theascast.The current cast allows any string for
kty, which weakens type safety. Consider narrowingktyto the literal'OKP'and using a typed constant instead of a cast.Suggested refactor
-export const accessTokenSignerKeyType = { kty: 'OKP', crv: 'Ed25519' } as { - kty: string; - crv: AccessTokenSignerKeyType; -}; +export const accessTokenSignerKeyType: { kty: 'OKP'; crv: AccessTokenSignerKeyType } = { + kty: 'OKP', + crv: 'Ed25519', +};apps/oid4vc-verification/src/oid4vc-verification.service.ts (1)
236-268: Consolidate X509_P256 and X509_ED25519 branches to reduce duplication.Both branches do identical work; selecting
x5cKeyTypeonce will prevent future drift.♻️ Suggested refactor
- } else if (sessionRequest.requestSigner.method === SignerOption.X509_P256) { - this.logger.debug('X5C based request signer method selected'); - - const activeCertificate = await this.oid4vpRepository.getCurrentActiveCertificate(orgId, x5cKeyType.P256); - this.logger.debug(`activeCertificate=${JSON.stringify(activeCertificate)}`); - - if (!activeCertificate) { - throw new NotFoundException( - `No active certificate(${sessionRequest.requestSigner.method}}) found for issuer` - ); - } - - requestSigner = { - method: SignerMethodOption.X5C, // "x5c" - x5c: [activeCertificate.certificateBase64], // array with PEM/DER base64 - keyId: activeCertificate.keyId - }; - } else if (sessionRequest.requestSigner.method === SignerOption.X509_ED25519) { - this.logger.debug('X5C based request signer method selected'); - - const activeCertificate = await this.oid4vpRepository.getCurrentActiveCertificate(orgId, x5cKeyType.Ed25519); - this.logger.debug(`activeCertificate=${JSON.stringify(activeCertificate)}`); - - if (!activeCertificate) { - throw new NotFoundException( - `No active certificate(${sessionRequest.requestSigner.method}}) found for issuer` - ); - } - requestSigner = { - method: SignerMethodOption.X5C, // "x5c" - x5c: [activeCertificate.certificateBase64], // array with PEM/DER base64 - keyId: activeCertificate.keyId - }; + } else if ( + sessionRequest.requestSigner.method === SignerOption.X509_P256 || + sessionRequest.requestSigner.method === SignerOption.X509_ED25519 + ) { + this.logger.debug('X5C based request signer method selected'); + const keyType = + sessionRequest.requestSigner.method === SignerOption.X509_P256 ? x5cKeyType.P256 : x5cKeyType.Ed25519; + const activeCertificate = await this.oid4vpRepository.getCurrentActiveCertificate(orgId, keyType); + this.logger.debug(`activeCertificate=${JSON.stringify(activeCertificate)}`); + + if (!activeCertificate) { + throw new NotFoundException( + `No active certificate(${sessionRequest.requestSigner.method}) found for issuer` + ); + } + requestSigner = { + method: SignerMethodOption.X5C, // "x5c" + x5c: [activeCertificate.certificateBase64], // array with PEM/DER base64 + keyId: activeCertificate.keyId + }; } else {
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/oid4vc-verification/src/oid4vc-verification.service.ts`:
- Around line 239-250: The activeCertificate returned from
oid4vpRepository.getCurrentActiveCertificate must be checked for a defined keyId
before building the X5C requestSigner: after retrieving activeCertificate verify
activeCertificate.keyId is present and if not throw a clear NotFoundException
(e.g., "No active certificate keyId found for issuer") rather than passing
undefined into requestSigner; update the same validation for the other X5C
construction sites (the other requestSigner blocks used in session creation and
intent-based flows) so all places that set requestSigner.method =
SignerMethodOption.X5C and x5c = [...] also guard and error if keyId is missing.
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
|
ajile-in
reviewed
Feb 2, 2026
| */ | ||
| function buildNestedClaims(attributes: CredentialAttribute[]): Record<string, Claim> { | ||
| const claims: Record<string, Claim> = {}; | ||
| // function buildNestedClaims(attributes: CredentialAttribute[]): Record<string, Claim> { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Changes required to support Credo version 6.1.
How ?
Updated APIs to support OID4VC issuance and verification in alignment with Credo version 6.1.
Verified OID4VC issuance and verification flows after the upgrade
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation