feat: Initial changes for x509 support #1482
Conversation
Signed-off-by: Rinkal Bhojani <[email protected]>
WalkthroughIntroduces end-to-end X.509 certificate management: new X509 microservice (NATS), API Gateway X509 module (HTTP endpoints), shared DTOs/interfaces/enums/constants, Prisma schema/migration, and agent-service RPC handlers for create/decode/import. Adds X509 NATS topics, URLs, response messages, and an environment variable for the X509 NKEY seed. Minor OIDC typing/enum adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant API as API Gateway (X509Controller)
participant XSvc as X509 Microservice
participant Repo as X509 Repository (Prisma)
participant Agent as Agent Service
participant Ext as External Agent API
rect rgb(235,245,255)
note over User,API: Create X.509 certificate
User->>API: HTTP POST /x509/:orgId
API->>XSvc: NATS create-x509-certificate {orgId, options, user}
XSvc->>Repo: hasDateCollision/checks
Repo-->>XSvc: Collision result
XSvc->>Agent: NATS agent-create-x509-certificate {url, orgId, options}
Agent->>Ext: HTTP POST /x509 (org API key)
Ext-->>Agent: certificateBase64
Agent-->>XSvc: certificateBase64
XSvc->>Repo: create(record)
Repo-->>XSvc: X509CertificateRecord
XSvc-->>API: X509CertificateRecord
API-->>User: 201 Created (payload)
end
sequenceDiagram
autonumber
actor User
participant API as API Gateway (X509Controller)
participant XSvc as X509 Microservice
participant Repo as X509 Repository
participant Agent as Agent Service
participant Ext as External Agent API
rect rgb(245,235,255)
note over User,API: Import X.509 certificate
User->>API: HTTP POST /x509/:orgId/import
API->>XSvc: NATS import-x509-certificate {orgId, options, user}
XSvc->>Agent: NATS agent-decode-x509-certificate {url, orgId, options.certificate}
Agent->>Ext: HTTP POST /x509/decode
Ext-->>Agent: decoded fields
Agent-->>XSvc: decoded info
XSvc->>Repo: hasDateCollision/checks
Repo-->>XSvc: Collision result
XSvc->>Agent: NATS agent-import-x509-certificate {url, orgId, options}
Agent->>Ext: HTTP POST /x509/import
Ext-->>Agent: wallet import result
Agent-->>XSvc: import ok
XSvc->>Repo: create(record isImported=true)
Repo-->>XSvc: X509CertificateRecord
XSvc-->>API: X509CertificateRecord
API-->>User: 201 Created (payload)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
|
1 similar comment
✅ 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: 19
🧹 Nitpick comments (19)
apps/agent-service/src/agent-service.controller.ts (1)
389-414: Consider more specific return types.All three X.509 methods return
Promise<object>, which is very generic. Consider defining specific response DTOs for better type safety and API documentation.Example for
createX509Certificate:interface IX509CertificateResponse { certificateId: string; status: string; certificate: string; // other relevant fields } async createX509Certificate(payload: { options: X509CreateCertificateOptions; url: string; orgId: string; }): Promise<IX509CertificateResponse> { return this.agentServiceService.createX509Certificate(payload.options, payload.url, payload.orgId); }apps/oid4vc-issuance/src/main.ts (1)
15-15: Consider aligning environment variable naming.The constant name changed to
OIDC4VC_ISSUANCE_SERVICEbut still usesprocess.env.ISSUANCE_NKEY_SEED. Consider either:
- Creating a new
OIDC4VC_ISSUANCE_NKEY_SEEDenvironment variable for clarity, or- Adding a comment explaining why the generic
ISSUANCE_NKEY_SEEDis used for the OIDC4VC issuance service.libs/prisma-service/prisma/schema.prisma (1)
627-644: Consider adding validation constraints and foreign key relationships.Additional recommendations for the
x509_certificatesschema:
Add foreign keys for audit fields:
createdByandlastChangedByare UUIDs but lack foreign key constraints to theusertable, unlike other tables in the schema.Add unique constraint: Consider adding a unique constraint on
certificateBase64or a hash of the certificate to prevent duplicate imports.Database-level validation: While application logic can handle this, consider adding a CHECK constraint to ensure
expiry > validFrom.libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql (1)
18-31: oid4vc_credentials: add referential integrity and audit defaults
- Add FKs for createdBy/lastChangedBy if a users table exists.
- Consider default for lastChangedDateTime (align with createDateTime).
- Index orgId for filtering.
Apply post-create additions:
-- CreateTable CREATE TABLE "oid4vc_credentials" ( "id" UUID NOT NULL, "orgId" UUID NOT NULL, "offerId" TEXT NOT NULL, "credentialOfferId" TEXT NOT NULL, "state" TEXT NOT NULL, "contextCorrelationId" TEXT NOT NULL, "createdBy" UUID NOT NULL, - "createDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, - "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "createDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, "lastChangedBy" UUID NOT NULL, CONSTRAINT "oid4vc_credentials_pkey" PRIMARY KEY ("id") ); + +-- Optional: add user FKs if table exists +-- ALTER TABLE "oid4vc_credentials" ADD CONSTRAINT "oid4vc_credentials_createdBy_fkey" FOREIGN KEY ("createdBy") REFERENCES "user"("id") ON DELETE RESTRICT ON UPDATE CASCADE; +-- ALTER TABLE "oid4vc_credentials" ADD CONSTRAINT "oid4vc_credentials_lastChangedBy_fkey" FOREIGN KEY ("lastChangedBy") REFERENCES "user"("id") ON DELETE RESTRICT ON UPDATE CASCADE; + +-- Helpful index for common filters +CREATE INDEX IF NOT EXISTS "oid4vc_credentials_orgId_idx" ON "oid4vc_credentials"("orgId");libs/common/src/response-messages/index.ts (1)
556-577: Polish x509 messages (punctuation/grammar only; keys unchanged)
- errorActivation has double period.
- collisionForActivatingX5c message grammar (active/Inactivate) is off.
- errorActivation: 'Failed to activate x509 certificate..', + errorActivation: 'Failed to activate x509 certificate.', @@ - collisionForActivatingX5c: - 'Certificate date range collides with existing certificates for this organization, In order to active this you need to Inactivate the previous one.', + collisionForActivatingX5c: + 'Certificate date range collides with existing certificates for this organization. To activate this, deactivate the previous one.',apps/x509/src/x509.controller.ts (1)
36-43: User in read payload is ignored; align contract or forward for auth/auditgetCertificateByOrgId and getCertificate accept user but don’t pass it to the service. Either:
- forward user to service for authorization/audit, or
- drop user from payload to avoid confusion.
Also consider DTOs with ValidationPipe/Zod for payload validation on message patterns.
Also applies to: 45-48
apps/api-gateway/src/x509/x509.controller.ts (3)
56-60: Remove unused dependencycommonService is injected but not used.
- constructor( - private readonly x509Service: X509Service, - private readonly commonService: CommonService - ) {} + constructor(private readonly x509Service: X509Service) {}
133-135: Fix wording in docs“Deactive” → “Deactivate”.
- summary: 'Deactive X509 certificate', - description: 'Deactive X509 certificate' + summary: 'Deactivate X509 certificate', + description: 'Deactivate X509 certificate'
186-207: Add validation/transform for query DTOEnsure X509SearchCriteriaDto is validated and query strings are transformed to enums/dates as needed.
Example:
- async getAllX509ByOrgId( - @Query() x509SearchCriteriaDto: X509SearchCriteriaDto, + async getAllX509ByOrgId( + @Query(new (require('@nestjs/common').ValidationPipe)({ transform: true, whitelist: true })) + x509SearchCriteriaDto: X509SearchCriteriaDto,apps/x509/src/interfaces/x509.interface.ts (1)
4-13: Align status to enum for type safetyUse x5cRecordStatus instead of string.
export interface CreateX509CertificateEntity { orgId: string; // We'll accept orgId and find orgAgent internally keyType: x5cKeyType; - status: string; + status: x5cRecordStatus; validFrom: Date; expiry: Date; certificateBase64: string; createdBy: string; lastChangedBy: string; }apps/x509/src/repositories/x509.repository.ts (1)
254-267: Use enum for status instead of string literalPrefer x5cRecordStatus.Active for consistency and type safety.
- status: 'Active' + status: x5cRecordStatus.Activeapps/api-gateway/src/x509/dtos/x509.dto.ts (2)
103-110: Ensure nested validation runs for arraysAdd
{ each: true }so each NameDto is validated.- @ValidateNested() + @ValidateNested({ each: true })
259-282: Tighten validation (optional): base64 and union handling
- If certificates/keys are base64, prefer
@IsBase64()over@IsString().issueraccepts string or object while using@ValidateNested(). Consider guarding with@ValidateIf((o) => typeof o.issuer === 'object')to avoid invalid nested validation on strings.Are certificate/privateKey inputs PEM or base64? Confirm expected format to choose correct validators.
Also applies to: 284-291, 234-247
libs/common/src/interfaces/x509.interface.ts (6)
69-79: DTO dates at JSON boundary: prefer string | DateWhen crossing service boundaries, JSON carries dates as strings. Make Validity fields accept string | Date to avoid confusion.
export interface Validity { /** * @example "2024-01-01T00:00:00.000Z" */ - notBefore?: Date; + notBefore?: string | Date; /** * @example "2025-01-01T00:00:00.000Z" */ - notAfter?: Date; + notAfter?: string | Date; }Also applies to: 199-201
236-242: Handle privateKey as sensitive secretDo not log it, redact in traces/errors, encrypt at rest, restrict exposure to the minimal scope, and consider immediate zeroization after use.
94-103: Nit: Fix EKU example values“MdlDs” looks invalid; prefer known EKUs matching X509ExtendedKeyUsage enum (e.g., ServerAuth, ClientAuth, OCSPSigning).
105-115: RenameNameList.name→names
Use a plural property for an array.libs/common/src/interfaces/x509.interface.ts @@ -109 +109 - name: Name[]; + names: Name[];
217-220: Rename IPaginationSortingdto → IPaginationSortingDto and narrow sortBy typeCasing nit and stricter typing for sort order.
-export interface IX509SearchCriteria extends IPaginationSortingdto { +export interface IX509SearchCriteria extends IPaginationSortingDto { keyType: string; status: string; } -export interface IPaginationSortingdto { +export interface IPaginationSortingDto { pageNumber: number; pageSize: number; sortField?: string; - sortBy?: string; + sortBy?: 'asc' | 'desc'; searchByText?: string; }
203-215: Use existing enums for status and keyType
Replacestatus: string→status: x5cRecordStatusandkeyType: string→keyType: x5cKeyType, importing both fromlibs/enum/src/enum.ts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.env.demo(1 hunks).env.sample(1 hunks)apps/agent-service/src/agent-service.controller.ts(2 hunks)apps/agent-service/src/agent-service.service.ts(2 hunks)apps/api-gateway/src/app.module.ts(2 hunks)apps/api-gateway/src/oid4vc-issuance/dtos/issuer-sessions.dto.ts(0 hunks)apps/api-gateway/src/oid4vc-issuance/oid4vc-issuance.service.ts(2 hunks)apps/api-gateway/src/x509/dtos/x509.dto.ts(1 hunks)apps/api-gateway/src/x509/x509.controller.ts(1 hunks)apps/api-gateway/src/x509/x509.module.ts(1 hunks)apps/api-gateway/src/x509/x509.service.ts(1 hunks)apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts(1 hunks)apps/oid4vc-issuance/src/main.ts(1 hunks)apps/x509/src/interfaces/x509.interface.ts(1 hunks)apps/x509/src/main.ts(1 hunks)apps/x509/src/repositories/x509.repository.ts(1 hunks)apps/x509/src/x509.controller.ts(1 hunks)apps/x509/src/x509.module.ts(1 hunks)apps/x509/src/x509.service.ts(1 hunks)apps/x509/tsconfig.app.json(1 hunks)libs/common/src/common.constant.ts(3 hunks)libs/common/src/common.utils.ts(1 hunks)libs/common/src/interfaces/x509.interface.ts(1 hunks)libs/common/src/response-messages/index.ts(1 hunks)libs/enum/src/enum.ts(2 hunks)libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql(1 hunks)libs/prisma-service/prisma/schema.prisma(2 hunks)nest-cli.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api-gateway/src/oid4vc-issuance/dtos/issuer-sessions.dto.ts
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.sample
[warning] 124-124: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 124-124: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🔇 Additional comments (16)
apps/x509/tsconfig.app.json (1)
1-9: LGTM!The TypeScript configuration follows standard NestJS conventions and is consistent with other applications in the monorepo.
nest-cli.json (1)
298-307: LGTM!The X.509 project configuration follows the established pattern for other microservices in the monorepo and includes all necessary properties.
.env.demo (1)
105-105: LGTM!The X509_NKEY_SEED variable is correctly positioned among other NKEY_SEED variables and follows the established naming convention.
apps/x509/src/x509.module.ts (2)
14-30: LGTM with minor observation.The module structure follows NestJS best practices and correctly configures NATS connectivity with the X509_NKEY_SEED.
Minor observation:
Loggeris both imported viaLoggerModule(line 17) and explicitly provided (line 29). Depending on the LoggerModule implementation, the explicit Logger provider might be redundant. Verify if this is intentional.
20-26: NATS_CLIENT injection confirmed:@Inject('NATS_CLIENT')is used in x509.service.ts (line 37) and no directClientProxyinjections without the token were found.apps/oid4vc-issuance/src/main.ts (1)
15-15: Constant name update verified:CommonConstants.OIDC4VC_ISSUANCE_SERVICEis defined and no leftover references toISSUANCE_SERVICEremain.libs/enum/src/enum.ts (1)
275-286: Safe to use concrete enum for OpenId4VcIssuanceSessionState
All imports reference it as a value export; no ambient or type-only imports detected.apps/agent-service/src/agent-service.controller.ts (1)
389-414: Agent service methods implemented and HTTP calls verified. The methodscreateX509Certificate,decodeX509Certificate, andimportX509Certificateexist inAgentServiceServiceand correctly invokecommonService.httpPost(url, options, { headers: { authorization: getApiKey } })with proper logging and error handling.apps/api-gateway/src/app.module.ts (1)
36-36: LGTM!The X509Module import and registration follow the established pattern in the codebase. The module is correctly added to the imports array alongside other feature modules.
Also applies to: 70-71
libs/common/src/common.utils.ts (1)
108-111: LGTM!The X509 URL mappings are correctly added and follow the established pattern in the
agentUrlMap. The trailing comma on line 108 properly enables the subsequent entries.libs/common/src/common.constant.ts (1)
129-132: LGTM!The X509-related constants are well-structured and follow the established naming conventions:
- URL constants use the
/x509prefix consistently- Microservice name follows the
*-servicepattern- Action constants use the
X509_*prefixAlso applies to: 382-382, 414-419
apps/api-gateway/src/oid4vc-issuance/oid4vc-issuance.service.ts (1)
102-108: LGTM!The method signature update improves type safety by using the explicit
CreateCredentialOfferD2ADtotype instead of a generic object. This follows the pattern used by other methods in the service.apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts (1)
5-5: No action needed:CredentialFormatexport is correct.
CredentialFormatis properly defined and exported from@credebl/enum/enum.apps/api-gateway/src/x509/x509.controller.ts (1)
210-214: No conflicting GET route detected
Confirmed absence of@Get('/:orgId/import'); current GET vs POST routes won’t collide.libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql (1)
9-16: Verified no remaining references; safe to drop columns/enum
Ran a global search—supported_protocolandCredentialExchangeProtocolonly appear in these migrations. Proceed, ensuring you’ve backed up or archived any existing column data if needed for analytics.apps/x509/src/x509.service.ts (1)
84-96: Verify agent response property names to avoid undefined DB writes
- createCertificate uses
certificate.response.publicCertificateBase64.- importCertificate uses
certificate.response.issuerCertficicate(note the spelling).If these keys differ (e.g.,
issuerCertificate),certificateBase64may persist as undefined.Please confirm the exact response shape from agent-service. If needed, I can adjust the code to map the correct fields and add guards.
Also applies to: 208-218
| SCHEMA_NKEY_SEED= xxxxxxxxxxxxx // Please provide Nkeys secret for schema service | ||
| UTILITIES_NKEY_SEED= xxxxxxxxxxxxx // Please provide Nkeys secret for utilities service | ||
| GEOLOCATION_NKEY_SEED= xxxxxxxxxxx // Please provide Nkeys secret for geo-location service | ||
| X509_NKEY_SEED= xxxxxxxxxxx // Please provide Nkeys secret for x509 service |
There was a problem hiding this comment.
Fix formatting issues.
The line has formatting issues flagged by static analysis:
- Extra space before the value
- Missing quotes around the placeholder value
Based on static analysis hints.
Apply this diff to fix the formatting:
-X509_NKEY_SEED= xxxxxxxxxxx // Please provide Nkeys secret for x509 service
+X509_NKEY_SEED=xxxxxxxxxxx # Please provide Nkeys secret for x509 serviceNote: Also changed // to # for consistency with shell-style comments used in other parts of the file (lines 29, 137-143).
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 124-124: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 124-124: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
In .env.sample around line 124 the X509_NKEY_SEED entry has an extra space
before the value, missing quotes around the placeholder, and uses '//' for the
comment; remove the leading space after the '=', wrap the placeholder value in
double quotes (e.g. "xxxxxxxxxxx"), and replace the '//' comment with a
shell-style '#' comment so the line follows the file's formatting conventions.
| async createX509Certificate(options: X509CreateCertificateOptions, url: string, orgId: string): Promise<object> { | ||
| try { | ||
| this.logger.log('Start creating X509 certificate'); | ||
| this.logger.debug('Creating X509 certificate with options', options); | ||
| const getApiKey = await this.getOrgAgentApiKey(orgId); | ||
| const x509Certificate = await this.commonService | ||
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | ||
| .then(async (response) => response); | ||
| return x509Certificate; | ||
| } catch (error) { | ||
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| async decodeX509Certificate(options: x509CertificateDecodeDto, url: string, orgId: string): Promise<object> { | ||
| try { | ||
| this.logger.log('Start decoding X509 certificate'); | ||
| this.logger.debug('Decoding X509 certificate with options', options); | ||
| const getApiKey = await this.getOrgAgentApiKey(orgId); | ||
| const x509Certificate = await this.commonService | ||
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | ||
| .then(async (response) => response); | ||
| return x509Certificate; | ||
| } catch (error) { | ||
| this.logger.error(`Error in decoding x509 certificate in agent service : ${JSON.stringify(error)}`); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| async importX509Certificate(options: IX509ImportCertificateOptionsDto, url: string, orgId: string): Promise<object> { | ||
| try { | ||
| this.logger.log('Start importing X509 certificate'); | ||
| this.logger.debug(`Importing X509 certificate with options`, options.certificate); | ||
| const getApiKey = await this.getOrgAgentApiKey(orgId); | ||
| const x509Certificate = await this.commonService | ||
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | ||
| .then(async (response) => response); | ||
| return x509Certificate; | ||
| } catch (error) { | ||
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Do not log certificate payloads; align error handling with RpcException
- Debug logs print entire options/certificates (sensitive). Redact.
- Wrap thrown errors as RpcException for consistency.
Apply this diff:
async createX509Certificate(options: X509CreateCertificateOptions, url: string, orgId: string): Promise<object> {
try {
this.logger.log('Start creating X509 certificate');
- this.logger.debug('Creating X509 certificate with options', options);
+ this.logger.debug('Creating X509 certificate (redacted)', {
+ keyType: options?.authorityKey?.keyType,
+ hasSubjectPublicKey: !!options?.subjectPublicKey
+ });
const getApiKey = await this.getOrgAgentApiKey(orgId);
const x509Certificate = await this.commonService
.httpPost(url, options, { headers: { authorization: getApiKey } })
.then(async (response) => response);
return x509Certificate;
} catch (error) {
this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`);
- throw error;
+ throw new RpcException(error.response ?? error);
}
}
async decodeX509Certificate(options: x509CertificateDecodeDto, url: string, orgId: string): Promise<object> {
try {
this.logger.log('Start decoding X509 certificate');
- this.logger.debug('Decoding X509 certificate with options', options);
+ this.logger.debug('Decoding X509 certificate (payload redacted)');
const getApiKey = await this.getOrgAgentApiKey(orgId);
const x509Certificate = await this.commonService
.httpPost(url, options, { headers: { authorization: getApiKey } })
.then(async (response) => response);
return x509Certificate;
} catch (error) {
this.logger.error(`Error in decoding x509 certificate in agent service : ${JSON.stringify(error)}`);
- throw error;
+ throw new RpcException(error.response ?? error);
}
}
async importX509Certificate(options: IX509ImportCertificateOptionsDto, url: string, orgId: string): Promise<object> {
try {
this.logger.log('Start importing X509 certificate');
- this.logger.debug(`Importing X509 certificate with options`, options.certificate);
+ this.logger.debug('Importing X509 certificate (payload redacted)');
const getApiKey = await this.getOrgAgentApiKey(orgId);
const x509Certificate = await this.commonService
.httpPost(url, options, { headers: { authorization: getApiKey } })
.then(async (response) => response);
return x509Certificate;
} catch (error) {
this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`);
- throw error;
+ throw new RpcException(error.response ?? error);
}
}I can add a small redact helper if you prefer.
📝 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.
| async createX509Certificate(options: X509CreateCertificateOptions, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start creating X509 certificate'); | |
| this.logger.debug('Creating X509 certificate with options', options); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw error; | |
| } | |
| } | |
| async decodeX509Certificate(options: x509CertificateDecodeDto, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start decoding X509 certificate'); | |
| this.logger.debug('Decoding X509 certificate with options', options); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in decoding x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw error; | |
| } | |
| } | |
| async importX509Certificate(options: IX509ImportCertificateOptionsDto, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start importing X509 certificate'); | |
| this.logger.debug(`Importing X509 certificate with options`, options.certificate); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw error; | |
| } | |
| } | |
| async createX509Certificate(options: X509CreateCertificateOptions, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start creating X509 certificate'); | |
| this.logger.debug('Creating X509 certificate (redacted)', { | |
| keyType: options?.authorityKey?.keyType, | |
| hasSubjectPublicKey: !!options?.subjectPublicKey | |
| }); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw new RpcException(error.response ?? error); | |
| } | |
| } | |
| async decodeX509Certificate(options: x509CertificateDecodeDto, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start decoding X509 certificate'); | |
| this.logger.debug('Decoding X509 certificate (payload redacted)'); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in decoding x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw new RpcException(error.response ?? error); | |
| } | |
| } | |
| async importX509Certificate(options: IX509ImportCertificateOptionsDto, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start importing X509 certificate'); | |
| this.logger.debug('Importing X509 certificate (payload redacted)'); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw new RpcException(error.response ?? error); | |
| } | |
| } |
| }) | ||
| ) | ||
| orgId: string, | ||
| @Param('id', ParseUUIDPipe) id: string, |
There was a problem hiding this comment.
🧩 Analysis chain
Critical: ID validation mismatches DB (TEXT vs UUID)
x509_certificates.id is TEXT in the migration, but these endpoints enforce UUID via ParseUUIDPipe. Non‑UUID IDs (e.g., fingerprints) will 400.
Fix by removing UUID enforcement on id:
- @Param('id', ParseUUIDPipe) id: string,
+ @Param('id', TrimStringParamPipe) id: string,Apply to activateX509 (Line 118), deActivateX509 (Line 150), and getX509Certificate (Line 229). If IDs are intended to be UUIDs, change DB column to UUID with default and keep ParseUUIDPipe.
Also applies to: 150-150, 229-229
🏁 Script executed:
#!/bin/bash
# Show all x509 ':id' params using ParseUUIDPipe
rg -nP -C2 "@Param\\(['\"]id['\"],\\s*ParseUUIDPipe\\)" apps/api-gateway/src/x509
# Confirm DB column type (TEXT) in migration
rg -nP -C2 'CREATE TABLE "x509_certificates"|^\\s*"id"\\s+TEXT' libs/prisma-service/prisma/migrationsLength of output: 1705
🏁 Script executed:
#!/bin/bash
rg -n "TrimStringParamPipe" -g "*.ts"Length of output: 3619
Replace ParseUUIDPipe with TrimStringParamPipe for X.509 ID params
DB x509_certificates.id is TEXT, so ParseUUIDPipe rejects valid non-UUID IDs. Update in activateX509 (L118), deActivateX509 (L150) and getX509Certificate (L229):
- @Param('id', ParseUUIDPipe) id: string,
+ @Param('id', TrimStringParamPipe) id: string,If IDs should be UUIDs, convert the DB column to UUID and retain ParseUUIDPipe instead.
📝 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.
| @Param('id', ParseUUIDPipe) id: string, | |
| @Param('id', TrimStringParamPipe) id: string, |
🤖 Prompt for AI Agents
In apps/api-gateway/src/x509/x509.controller.ts around lines 118, 150 and 229,
the controller currently uses ParseUUIDPipe for the 'id' route param which
rejects valid DB TEXT IDs; replace ParseUUIDPipe with TrimStringParamPipe for
the id parameter in activateX509 (L118), deActivateX509 (L150) and
getX509Certificate (L229) so the incoming id is trimmed and accepted as a
string; if ids are intended to be UUIDs instead, migrate the DB column to UUID
and keep ParseUUIDPipe.
| ClientsModule.register([ | ||
| { | ||
| name: 'NATS_CLIENT', | ||
| transport: Transport.NATS, | ||
| options: getNatsOptions(CommonConstants.X509_SERVICE, process.env.API_GATEWAY_NKEY_SEED) | ||
| }, | ||
| CommonModule | ||
| ]) |
There was a problem hiding this comment.
Fix the CommonModule placement.
CommonModule is incorrectly placed inside the ClientsModule.register() array. It should be in the root imports array of the module.
Apply this diff:
imports: [
HttpModule,
ConfigModule.forRoot(),
ClientsModule.register([
{
name: 'NATS_CLIENT',
transport: Transport.NATS,
options: getNatsOptions(CommonConstants.X509_SERVICE, process.env.API_GATEWAY_NKEY_SEED)
- },
- CommonModule
+ }
- ])
+ ]),
+ CommonModule
],📝 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.
| ClientsModule.register([ | |
| { | |
| name: 'NATS_CLIENT', | |
| transport: Transport.NATS, | |
| options: getNatsOptions(CommonConstants.X509_SERVICE, process.env.API_GATEWAY_NKEY_SEED) | |
| }, | |
| CommonModule | |
| ]) | |
| imports: [ | |
| HttpModule, | |
| ConfigModule.forRoot(), | |
| ClientsModule.register([ | |
| { | |
| name: 'NATS_CLIENT', | |
| transport: Transport.NATS, | |
| options: getNatsOptions(CommonConstants.X509_SERVICE, process.env.API_GATEWAY_NKEY_SEED) | |
| } | |
| ]), | |
| CommonModule | |
| ], |
🤖 Prompt for AI Agents
In apps/api-gateway/src/x509/x509.module.ts around lines 17 to 24, CommonModule
is incorrectly included inside the ClientsModule.register([...]) array; remove
CommonModule from that array so ClientsModule.register only contains client
config objects (the NATS client entry), and add CommonModule to the module's
top-level imports array in the @Module decorator (alongside
ClientsModule.register(...)) so CommonModule is imported properly by the module.
| this.logger.log(`Start creating x509 certficate`); | ||
| this.logger.debug(`payload : `, createDto, reqUser); | ||
| const payload = { options: createDto, user: reqUser, orgId }; | ||
| return this.natsClient.sendNatsMessage(this.serviceProxy, 'create-x509-certificate', payload); | ||
| } |
There was a problem hiding this comment.
Redact sensitive payloads in logs (certificate/user)
Debug logs include create/import payloads and user objects. Avoid logging certs/keys/PII.
Apply this diff:
- this.logger.log(`Start creating x509 certficate`);
- this.logger.debug(`payload : `, createDto, reqUser);
+ this.logger.log(`Start creating x509 certificate`);
+ this.logger.debug(`payload (redacted)`);
// activate
- this.logger.log(`Start activating x509 certficate`);
+ this.logger.log(`Start activating x509 certificate`);
// deactivate
- this.logger.log(`Start deactivating x509 certficate`);
+ this.logger.log(`Start deactivating x509 certificate`);
// list
- this.logger.log(`Start getting x509 certficate for org`);
- this.logger.debug(`Filters applied : `, x509SearchCriteriaDto);
+ this.logger.log(`Start getting x509 certificates for org`);
+ this.logger.debug(`Filters applied (redacted)`);
// get by id
- this.logger.log(`Start getting x509 certficate by id`);
+ this.logger.log(`Start getting x509 certificate by id`);
// import
- this.logger.log(`Start importing x509 certficate by id`);
- this.logger.debug(`certificate : `, importDto.certificate);
+ this.logger.log(`Start importing x509 certificate`);
+ this.logger.debug(`certificate (payload redacted)`);Also applies to: 41-46, 48-53, 60-64, 66-71, 83-87
🤖 Prompt for AI Agents
In apps/api-gateway/src/x509/x509.service.ts around lines 35-39 (and also apply
same change to 41-46, 48-53, 60-64, 66-71, 83-87): current debug logs print full
create/import payloads and user objects which may contain certificates, private
keys or PII; change each logger.debug call to log only a sanitized version of
the data by removing or replacing sensitive fields (e.g., certificate,
privateKey, passphrase, key, or any PII fields) or by logging just metadata
(operation type, orgId, user id) and not the full objects; ensure payloads sent
to NATS remain unchanged, only the log arguments are replaced with the sanitized
object or minimal identifiers.
| export enum x5cKeyType { | ||
| Ed25519 = 'ed25519', | ||
| P256 = 'p256' | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix naming inconsistency.
The enum x5cKeyType uses lowercase x while other X.509-related enums like X509KeyUsage and X509ExtendedKeyUsage use uppercase X. This creates inconsistency in naming conventions.
Apply this diff to standardize the naming:
-export enum x5cKeyType {
+export enum X5cKeyType {
Ed25519 = 'ed25519',
P256 = 'p256'
}Ensure all references to this enum are updated accordingly throughout the codebase.
📝 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.
| export enum x5cKeyType { | |
| Ed25519 = 'ed25519', | |
| P256 = 'p256' | |
| } | |
| export enum X5cKeyType { | |
| Ed25519 = 'ed25519', | |
| P256 = 'p256' | |
| } |
🤖 Prompt for AI Agents
In libs/enum/src/enum.ts around lines 288 to 291, the enum name x5cKeyType uses
a lowercase "x" which is inconsistent with other X.509-related enums (e.g.,
X509KeyUsage); rename the enum to X5cKeyType (or X5CKeyType if you prefer
consistent capitalization) and update its exported name accordingly, then find
and update all references across the codebase (imports, type annotations, tests,
and docs) to the new enum name to ensure compilation and tests pass.
| export enum x5cRecordStatus { | ||
| Active = 'Active', | ||
| PendingActivation = 'Pending activation', | ||
| InActive = 'In Active' | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix naming inconsistency.
The enum x5cRecordStatus uses lowercase x while other X.509-related enums use uppercase X. Additionally, consider renaming to X509RecordStatus for consistency with the X.509 domain.
Apply this diff:
-export enum x5cRecordStatus {
+export enum X509RecordStatus {
Active = 'Active',
PendingActivation = 'Pending activation',
InActive = 'In Active'
}📝 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.
| export enum x5cRecordStatus { | |
| Active = 'Active', | |
| PendingActivation = 'Pending activation', | |
| InActive = 'In Active' | |
| } | |
| export enum X509RecordStatus { | |
| Active = 'Active', | |
| PendingActivation = 'Pending activation', | |
| InActive = 'In Active' | |
| } |
🤖 Prompt for AI Agents
In libs/enum/src/enum.ts around lines 293 to 297, the enum name x5cRecordStatus
should be renamed to X509RecordStatus to match other X.509-related enums and
code style; update the enum declaration to use the new name and then find and
update all references/imports across the repo to the new X509RecordStatus
identifier (ensure exports/signatures remain consistent), preserving the
existing member keys and values (optionally fix member text like "In Active" ->
"Inactive" in a follow-up if desired).
| CREATE TABLE "x509_certificates" ( | ||
| "id" TEXT NOT NULL, | ||
| "orgAgentId" UUID NOT NULL, | ||
| "keyType" TEXT NOT NULL, | ||
| "status" TEXT NOT NULL, | ||
| "validFrom" TIMESTAMP(3) NOT NULL, | ||
| "expiry" TIMESTAMP(3) NOT NULL, | ||
| "certificateBase64" TEXT NOT NULL, | ||
| "isImported" BOOLEAN NOT NULL DEFAULT false, | ||
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "createdBy" UUID NOT NULL, | ||
| "lastChangedDateTime" TIMESTAMP(3) NOT NULL, | ||
| "lastChangedBy" UUID NOT NULL, | ||
|
|
||
| CONSTRAINT "x509_certificates_pkey" PRIMARY KEY ("id") | ||
| ); |
There was a problem hiding this comment.
x509_certificates: fix timezone types, add validity checks and helpful indexes
- validFrom/expiry/createdAt/lastChangedDateTime should be TIMESTAMPTZ to avoid TZ bugs.
- Add CHECK (validFrom < expiry).
- Add default for lastChangedDateTime.
- Index orgAgentId (and optionally composite for lookups).
- Consider a partial/exclusion constraint to prevent overlapping ACTIVE ranges for same orgAgentId/keyType.
Suggested adjustments:
CREATE TABLE "x509_certificates" (
- "id" TEXT NOT NULL,
+ "id" TEXT NOT NULL,
"orgAgentId" UUID NOT NULL,
"keyType" TEXT NOT NULL,
"status" TEXT NOT NULL,
- "validFrom" TIMESTAMP(3) NOT NULL,
- "expiry" TIMESTAMP(3) NOT NULL,
+ "validFrom" TIMESTAMPTZ(6) NOT NULL,
+ "expiry" TIMESTAMPTZ(6) NOT NULL,
"certificateBase64" TEXT NOT NULL,
"isImported" BOOLEAN NOT NULL DEFAULT false,
- "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
+ "createdAt" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP,
"createdBy" UUID NOT NULL,
- "lastChangedDateTime" TIMESTAMP(3) NOT NULL,
+ "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP,
"lastChangedBy" UUID NOT NULL,
CONSTRAINT "x509_certificates_pkey" PRIMARY KEY ("id")
);
+
+-- Validity window sanity check
+ALTER TABLE "x509_certificates"
+ ADD CONSTRAINT "x509_certificates_valid_range_chk" CHECK ("validFrom" < "expiry");
+
+-- Indexes to speed up lookups
+CREATE INDEX IF NOT EXISTS "x509_certificates_orgAgentId_idx" ON "x509_certificates"("orgAgentId");
+CREATE INDEX IF NOT EXISTS "x509_certificates_orgAgentId_keyType_status_idx" ON "x509_certificates"("orgAgentId","keyType","status");
+
+-- Optional: enforce no overlapping ACTIVE ranges per orgAgentId+keyType
+-- Requires btree_gist extension
+-- CREATE EXTENSION IF NOT EXISTS btree_gist;
+-- ALTER TABLE "x509_certificates"
+-- ADD CONSTRAINT x509_no_overlap_active
+-- EXCLUDE USING gist (
+-- "orgAgentId" WITH =,
+-- "keyType" WITH =,
+-- tstzrange("validFrom", "expiry", '[]') WITH &&
+-- ) WHERE ("status" = 'ACTIVE');Also consider constraining "status" to an enum or CHECK of allowed values to align with application enums.
🤖 Prompt for AI Agents
In
libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql
around lines 34-49, change the four TIMESTAMP(3) columns (validFrom, expiry,
createdAt, lastChangedDateTime) to TIMESTAMPTZ(3) to preserve timezones; add a
CHECK constraint CHECK (validFrom < expiry); set a DEFAULT CURRENT_TIMESTAMP for
lastChangedDateTime; add an index on orgAgentId and a composite index on
(orgAgentId, keyType) to speed lookups; and constrain status to allowed values
(either by using a proper enum type or a CHECK IN (...) list); optionally
consider adding a partial/exclusion constraint to prevent overlapping ACTIVE
validity ranges per (orgAgentId, keyType).
| ALTER TABLE "oid4vc_credentials" ADD CONSTRAINT "oid4vc_credentials_orgId_fkey" FOREIGN KEY ("orgId") REFERENCES "organisation"("id") ON DELETE RESTRICT ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "x509_certificates" ADD CONSTRAINT "x509_certificates_orgAgentId_fkey" FOREIGN KEY ("orgAgentId") REFERENCES "org_agents"("id") ON DELETE RESTRICT ON UPDATE CASCADE; |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm id type alignment across layers
DB uses TEXT for x509_certificates.id. Ensure API/controllers don’t enforce UUID for this id. See related controller comments.
Check for UUID validation on x509 id:
🏁 Script executed:
#!/bin/bash
# Find ParseUUIDPipe applied to ':id' in x509 controllers
rg -nP -C2 "@Param\\(['\"]id['\"],\\s*ParseUUIDPipe\\)" --type=ts
# Confirm DB column type
rg -nP -C2 'CREATE TABLE "x509_certificates"|^\\s*"id"\\s+TEXT' --libsLength of output: 1512
🏁 Script executed:
#!/bin/bash
# Locate table declaration
rg -nP 'CREATE TABLE "x509_certificates"' -n libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql
# Confirm id column type
rg -nP '"id"\s+TEXT' -n libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sqlLength of output: 64
Align x509_certificates.id type across DB and API
Controllers apply ParseUUIDPipe on :id (apps/api-gateway/src/x509/x509.controller.ts lines 118, 150 & 229) but migration defines "id" TEXT (migration.sql line 35). Remove or replace the UUID pipe or convert the column to UUID.
🤖 Prompt for AI Agents
In
libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql
around lines 35 and 58, the migration creates "id" as TEXT while API controllers
expect UUIDs (apps/api-gateway/src/x509/x509.controller.ts lines 118, 150 &
229). Change the column to UUID (not TEXT) so DB and API types align: alter the
migration to create "id" UUID (or ALTER TABLE ... ALTER COLUMN id TYPE UUID
USING id::uuid for existing data), add a default generator if required (eg
gen_random_uuid() or uuid_generate_v4()), and keep foreign key constraints
unchanged; alternatively, if you prefer to keep TEXT, remove the ParseUUIDPipe
usages in the controllers—but prefer converting the column to UUID for type
safety and update the migration accordingly.
| model x509_certificates { | ||
| id String @id @default(uuid()) | ||
| orgAgentId String @db.Uuid | ||
| org_agents org_agents @relation(fields: [orgAgentId], references: [id]) | ||
| keyType String // "p256", "ed25519" | ||
| status String //e.g "Active", "Pending activation", "InActive" | ||
| validFrom DateTime | ||
| expiry DateTime | ||
| certificateBase64 String | ||
| isImported Boolean @default(false) | ||
| createdAt DateTime @default(now()) | ||
| createdBy String @db.Uuid | ||
| lastChangedDateTime DateTime @updatedAt | ||
| lastChangedBy String @db.Uuid | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add index on orgAgentId and consider using enums for status/keyType.
The new x509_certificates table has several design considerations:
-
Missing index: Add an index on
orgAgentIdfor query performance, especially when fetching all certificates for an organization:@@index([orgAgentId]) -
String fields that should be enums:
statusandkeyTypeuseStringtype but likely have a fixed set of values. Consider creating enums for better type safety and data integrity:enum CertificateStatus { ACTIVE PENDING_ACTIVATION INACTIVE } enum X509KeyType { P256 ED25519 }
-
Consider soft delete: Other tables in the schema include
deletedAtfor soft deletion. Consider adding it here for audit purposes. -
Certificate storage:
certificateBase64could be very large. Verify that storing entire certificates in the database is appropriate, or consider external storage with references.
Apply this diff to add the index:
model x509_certificates {
id String @id @default(uuid())
orgAgentId String @db.Uuid
org_agents org_agents @relation(fields: [orgAgentId], references: [id])
keyType String // "p256", "ed25519"
status String //e.g "Active", "Pending activation", "InActive"
validFrom DateTime
expiry DateTime
certificateBase64 String
isImported Boolean @default(false)
createdAt DateTime @default(now())
createdBy String @db.Uuid
lastChangedDateTime DateTime @updatedAt
lastChangedBy String @db.Uuid
+
+ @@index([orgAgentId])
}🤖 Prompt for AI Agents
In libs/prisma-service/prisma/schema.prisma around lines 626-644, add an index
on orgAgentId, convert status and keyType from String to enum types (e.g.,
CertificateStatus and X509KeyType) and update the model fields to use those
enums, add an optional deletedAt DateTime field to support soft-delete
consistency with other tables, and consider changing certificateBase64 storage
to a larger/text/blob-backed column or moving certificates to external storage
and storing a reference; implement these changes by declaring the two enums in
the schema, replacing the String types in the model with the enum names, adding
@@index([orgAgentId]) to the model, and adding deletedAt? DateTime for soft
deletes (no runtime/data migration instructions included here).
There was a problem hiding this comment.
Actionable comments posted: 26
🧹 Nitpick comments (17)
apps/oid4vc-issuance/src/main.ts (1)
15-15: Consider renaming the environment variable for consistency.The environment variable
ISSUANCE_NKEY_SEEDdoesn't match the new service name patternOIDC4VC_ISSUANCE_SERVICE. Consider renaming it toOIDC4VC_ISSUANCE_NKEY_SEEDfor consistency, unless the current name is intentionally maintained for backward compatibility.If you decide to rename, apply this diff:
- options: getNatsOptions(CommonConstants.OIDC4VC_ISSUANCE_SERVICE, process.env.ISSUANCE_NKEY_SEED) + options: getNatsOptions(CommonConstants.OIDC4VC_ISSUANCE_SERVICE, process.env.OIDC4VC_ISSUANCE_NKEY_SEED)Remember to update deployment configurations and documentation to reflect the new environment variable name.
libs/enum/src/enum.ts (1)
288-297: Consider PascalCase naming convention for enum types.The enum names
x5cKeyTypeandx5cRecordStatusstart with a lowercase 'x', which is inconsistent with the TypeScript/JavaScript convention where type names (including enums) use PascalCase. Consider renaming toX5cKeyTypeandX5cRecordStatusfor consistency with other enums in this file and standard conventions.Apply this diff if you choose to follow the PascalCase convention:
-export enum x5cKeyType { +export enum X5cKeyType { Ed25519 = 'ed25519', P256 = 'p256' } -export enum x5cRecordStatus { +export enum X5cRecordStatus { Active = 'Active', PendingActivation = 'Pending activation', InActive = 'In Active' }apps/x509/src/main.ts (2)
16-16: Reconsider HttpExceptionFilter for NATS microservice.
HttpExceptionFilteris typically designed for HTTP-based exceptions (e.g., REST/GraphQL). For NATS microservices, consider using an RPC exception filter instead, or verify that HttpExceptionFilter properly handles NATS RPC exceptions.If the filter is intended for NATS messages, consider creating a dedicated RPC exception filter:
import { Catch, RpcExceptionFilter, ArgumentsHost } from '@nestjs/common'; import { Observable, throwError } from 'rxjs'; import { RpcException } from '@nestjs/microservices'; @Catch() export class NatsExceptionFilter implements RpcExceptionFilter<RpcException> { catch(exception: RpcException, host: ArgumentsHost): Observable<any> { return throwError(() => exception.getError()); } }Then update line 16:
-app.useGlobalFilters(new HttpExceptionFilter()); +app.useGlobalFilters(new NatsExceptionFilter());
19-19: Remove trailing space in log message.Minor formatting issue: log message has a trailing space.
-Logger.log('X509 Microservice is listening to NATS '); +Logger.log('X509 Microservice is listening to NATS');libs/common/src/response-messages/index.ts (2)
568-568: Fix double period in error message.Minor typo: error message has double periods at the end.
-errorActivation: 'Failed to activate x509 certificate..', +errorActivation: 'Failed to activate x509 certificate.',
562-562: Consider renaming for consistency with other messages.The message key
fetchAllcould befetchedfor consistency with other sections (e.g.,schema.success.fetch,verification.success.fetch). However, this is a minor naming preference.-fetchAll: 'x509 certificates fetched successfully', +fetched: 'x509 certificates fetched successfully',Note: Only apply this if you want to match the naming pattern in other sections, otherwise the current name is acceptable.
libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql (1)
1-58: Migration mixes OIDC and X509 concerns.This migration file includes both OIDC credential table creation and X509 certificate table creation. Consider splitting these into separate migrations for better separation of concerns and easier rollback if needed.
Create two separate migrations:
20251013125236_added_oid4vc_credentials_table.sqlfor OIDC changes20251013125237_added_x509_certificate_table.sqlfor X509 changesapps/x509/src/interfaces/x509.interface.ts (1)
29-43: Consider using Prisma-generated type for OrgAgent.The
OrgAgentinterface duplicates the structure of theorg_agentsPrisma model. Consider importing and using the Prisma-generated type instead to maintain type safety and reduce duplication.import { org_agents } from '@prisma/client'; // Then use org_agents type directly, or create a type alias if you need a subset export type OrgAgent = Pick<org_agents, | 'id' | 'createDateTime' | 'createdBy' | 'lastChangedDateTime' | 'lastChangedBy' | 'orgDid' | 'verkey' | 'agentEndPoint' | 'agentId' | 'isDidPublic' | 'ledgerId' | 'orgAgentTypeId' | 'tenantId' >;This approach ensures that if the
org_agentsmodel changes, the TypeScript types stay in sync.apps/x509/src/x509.service.ts (3)
245-258: Don’t log full NATS payloads (may include secrets)Log only operation and orgId; avoid dumping options in logs.
- this.logger.log(`Requesing agent service for create x509 certificate`); - this.logger.debug(`agent service payload - _createX509CertificateForOrg : `, payload); + this.logger.log(`Requesting agent service to create x509 certificate for orgId=${orgId}`);
302-331: Prefer firstValueFrom/lastValueFrom over deprecated toPromisetoPromise is deprecated in RxJS 7+. Use firstValueFrom for clarity.
- return this.x509ServiceProxy - .send<string>(pattern, payload) - .pipe( - map((response) => ({ - response - })) - ) - .toPromise() - .catch((error) => { + const obs$ = this.x509ServiceProxy + .send<string>(pattern, payload) + .pipe(map((response) => ({ response }))); + try { + const { firstValueFrom } = await import('rxjs'); + return await firstValueFrom(obs$); + } catch (error) { this.logger.error(`catch: ${JSON.stringify(error)}`); throw new HttpException( { status: error.statusCode, error: error.message }, error.error ); - }); + }
333-345: Minor: reuse x509 response messages for agent endpoint not foundCurrently uses ResponseMessages.issuance.*; consider aligning under x509 messages for consistency.
apps/api-gateway/src/x509/x509.service.ts (2)
48-53: Redact user from logs in deactivate flowSame as above.
- this.logger.debug(`certificate Id : `, id); + this.logger.debug(`certificateId=${id}, orgId=${orgId}, userId=${reqUser?.id}`);
66-71: Redact user from logs in get-by-id flowAvoid logging full objects.
- this.logger.debug(`certificate Id : `, id); + this.logger.debug(`certificateId=${id}, orgId=${orgId}, userId=${reqUser?.id}`);apps/api-gateway/src/x509/x509.controller.ts (2)
133-135: Fix typo in Swagger docs: “Deactive” → “Deactivate”Minor copy fix for professionalism and searchability.
- @ApiOperation({ - summary: 'Deactive X509 certificate', - description: 'Deactive X509 certificate' - }) + @ApiOperation({ + summary: 'Deactivate X509 certificate', + description: 'Deactivate X509 certificate' + })
75-97: Avoid using@Res()unless you need raw Express responseUsing
@Res()opts out of Nest’s standard response handling (interceptors/serializers). Consider returning the DTO directly and using@HttpCode().- @Res() res: Response, ... - const finalResponse: IResponse = { + const finalResponse: IResponse = { statusCode: HttpStatus.CREATED, message: ResponseMessages.x509.success.create, data: record }; - return res.status(HttpStatus.CREATED).json(finalResponse); + return finalResponse;You can apply similarly in other handlers, adding
@HttpCode(HttpStatus.XXX)as needed.apps/x509/src/repositories/x509.repository.ts (1)
262-263: Use enum for status instead of hardcoded stringAlign with
x5cRecordStatus.Activeto avoid drift.- status: 'Active' + status: x5cRecordStatus.Activeapps/api-gateway/src/x509/dtos/x509.dto.ts (1)
117-121:includeis required but documented as optionalUse
@ApiProperty()(not Optional) to match the non-optional boolean.- @ApiPropertyOptional() + @ApiProperty() @IsBoolean() include: boolean;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.env.demo(1 hunks).env.sample(1 hunks)apps/agent-service/src/agent-service.controller.ts(2 hunks)apps/agent-service/src/agent-service.service.ts(2 hunks)apps/api-gateway/src/app.module.ts(2 hunks)apps/api-gateway/src/oid4vc-issuance/dtos/issuer-sessions.dto.ts(0 hunks)apps/api-gateway/src/oid4vc-issuance/oid4vc-issuance.service.ts(2 hunks)apps/api-gateway/src/x509/dtos/x509.dto.ts(1 hunks)apps/api-gateway/src/x509/x509.controller.ts(1 hunks)apps/api-gateway/src/x509/x509.module.ts(1 hunks)apps/api-gateway/src/x509/x509.service.ts(1 hunks)apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts(1 hunks)apps/oid4vc-issuance/src/main.ts(1 hunks)apps/x509/src/interfaces/x509.interface.ts(1 hunks)apps/x509/src/main.ts(1 hunks)apps/x509/src/repositories/x509.repository.ts(1 hunks)apps/x509/src/x509.controller.ts(1 hunks)apps/x509/src/x509.module.ts(1 hunks)apps/x509/src/x509.service.ts(1 hunks)apps/x509/tsconfig.app.json(1 hunks)libs/common/src/common.constant.ts(3 hunks)libs/common/src/common.utils.ts(1 hunks)libs/common/src/interfaces/x509.interface.ts(1 hunks)libs/common/src/response-messages/index.ts(1 hunks)libs/enum/src/enum.ts(2 hunks)libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql(1 hunks)libs/prisma-service/prisma/schema.prisma(2 hunks)nest-cli.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api-gateway/src/oid4vc-issuance/dtos/issuer-sessions.dto.ts
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.sample
[warning] 124-124: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 124-124: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🔇 Additional comments (33)
apps/api-gateway/src/oid4vc-issuance/oid4vc-issuance.service.ts (1)
10-10: LGTM! Type safety improvement aligns with existing patterns.Adding the
CreateCredentialOfferD2ADtotype annotation strengthens compile-time validation and is consistent with other methods in this service (createOidcCredentialOffer,updateOidcCredentialOffer). The runtime behavior remains unchanged.Also applies to: 102-105
.env.sample (1)
124-124: Static analysis warnings are consistent with existing patterns.The static analysis tools flagged spacing around the equals sign and missing quotes. However, this formatting is consistent with all other NKEY_SEED entries in this file (lines 112-123), so maintaining this pattern is acceptable for consistency within the file.
apps/x509/tsconfig.app.json (1)
1-9: LGTM!The TypeScript configuration follows NestJS monorepo conventions and is consistent with other app configurations in the project. The settings are appropriate for a new microservice.
nest-cli.json (1)
298-307: LGTM!The X509 project configuration follows the established pattern for application projects in the monorepo. The structure and paths are correctly defined.
libs/common/src/common.constant.ts (3)
129-132: LGTM!The X509 URL constants follow the established naming pattern and RESTful conventions used throughout the codebase.
382-382: LGTM!The X509_SERVICE identifier follows the naming convention used for other microservices in the platform.
416-419: LGTM!The X509 operation signature constants follow the established pattern and provide clear semantic meaning for NATS message patterns.
libs/enum/src/enum.ts (4)
275-286: LGTM!Removing the
declarekeyword is correct. Thedeclaremodifier is used for ambient type declarations in.d.tsfiles, but for actual enum definitions in.tsfiles, it should be omitted.
299-309: LGTM!The X509KeyUsage enum correctly implements RFC 5280 key usage bit flags with appropriate numeric values for bitwise operations.
311-319: LGTM!The X509ExtendedKeyUsage enum correctly implements RFC 5280 extended key usage OIDs.
321-324: LGTM!The CredentialFormat enum is appropriately defined at the shared library level, supporting the refactor in credential-sessions.builder.ts that removes the local duplicate definition.
apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts (1)
5-5: LGTM!The refactor to import
CredentialFormatfrom the shared enum library eliminates code duplication and centralizes the type definition. This is a good practice for maintaining consistency across the codebase..env.demo (1)
105-105: LGTM!The X509_NKEY_SEED environment variable is correctly placed with other NKEY_SEED entries and follows the established pattern for demo configuration.
apps/api-gateway/src/app.module.ts (2)
36-36: LGTM!The X509Module import follows the established pattern for importing feature modules.
70-71: LGTM!The X509Module is correctly registered in the AppModule's imports array, following the established pattern for feature module integration.
libs/common/src/common.utils.ts (1)
108-111: LGTM! X509 URL mappings added correctly.The new X509 certificate operation mappings follow the existing pattern and are properly integrated into the
agentUrlMap. The trailing comma on line 108 is a good addition for maintainability.apps/x509/src/x509.module.ts (1)
14-31: LGTM! Module structure is well-organized.The X509Module is properly configured with all necessary imports, controllers, and providers. The NATS client registration follows NestJS best practices.
libs/prisma-service/prisma/schema.prisma (2)
626-644: LGTM! X509 certificates model is well-structured.The
x509_certificatesmodel is properly defined with all necessary fields and relationships. The use of@updatedAtonlastChangedDateTimeensures automatic timestamp updates.Note: The timestamp precision inconsistency flagged in the migration review should be addressed in both the migration SQL and the Prisma schema to ensure they stay in sync.
234-234: x509_certificates relation usage verified
Nestedwhere: { org_agents: { orgId } }filters inx509.repository.tsconfirm the relation is leveraged when querying certificates by organization agent. No further changes required.apps/x509/src/interfaces/x509.interface.ts (1)
4-13: LGTM! X509 entity interfaces are well-defined.The interface definitions are clear and comprehensive, with good documentation (e.g., the comment on line 5). The type constraints using enums (
x5cKeyType,x5cRecordStatus) ensure type safety.libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql (1)
8-15: No references tosupported_protocolorCredentialExchangeProtocoloutside migration files—safe to apply.apps/x509/src/main.ts (1)
13-13: X509_NKEY_SEED is documented in .env.sample and .env.demo.apps/agent-service/src/agent-service.controller.ts (4)
30-34: LGTM: X.509 DTO imports are correct and scopedTypes imported from common are consistent with new handlers.
389-396: LGTM: Handler wired to service correctlyCommand, payload shape, and delegation look consistent.
398-405: LGTM: Decode handler consistent with DTOPayload typing and service call align.
407-414: LGTM: Import handler consistent with DTOPayload typing and service call align.
apps/agent-service/src/agent-service.service.ts (1)
83-87: LGTM: X.509 type importsInterfaces are imported from the shared contract module.
apps/x509/src/x509.controller.ts (5)
17-24: LGTM: Create handler mirrors service contractMessage name and payload shape are correct.
26-34: LGTM: Activate/Deactivate handlersContracts align with service methods.
36-43: LGTM: Get-all handler returns data+totalReturn type matches repository contract.
45-48: LGTM: Get-by-id handlerContract aligns with service method.
50-57: LGTM: Import handlerPayload and message pattern match service flow.
apps/api-gateway/src/x509/x509.controller.ts (1)
34-34: Import casing is correct: the Prisma schema declaresmodel user, so importing{ user }matches the generated type.Likely an incorrect or invalid review comment.
| async createX509Certificate(options: X509CreateCertificateOptions, url: string, orgId: string): Promise<object> { | ||
| try { | ||
| this.logger.log('Start creating X509 certificate'); | ||
| this.logger.debug('Creating X509 certificate with options', options); | ||
| const getApiKey = await this.getOrgAgentApiKey(orgId); | ||
| const x509Certificate = await this.commonService | ||
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | ||
| .then(async (response) => response); | ||
| return x509Certificate; | ||
| } catch (error) { | ||
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Do not log certificate options; redact or remove debug
Logging entire options risks leaking private material. Also, Logger.debug’s second arg is a context string, not an object.
Apply to restrict logging:
async createX509Certificate(options: X509CreateCertificateOptions, url: string, orgId: string): Promise<object> {
try {
- this.logger.log('Start creating X509 certificate');
- this.logger.debug('Creating X509 certificate with options', options);
+ this.logger.log('Start creating X509 certificate');
+ this.logger.debug('Creating X509 certificate request'); // avoid logging payload
const getApiKey = await this.getOrgAgentApiKey(orgId);
const x509Certificate = await this.commonService
.httpPost(url, options, { headers: { authorization: getApiKey } })
.then(async (response) => response);
return x509Certificate;
} catch (error) {
this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`);
throw error;
}
}📝 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.
| async createX509Certificate(options: X509CreateCertificateOptions, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start creating X509 certificate'); | |
| this.logger.debug('Creating X509 certificate with options', options); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw error; | |
| } | |
| } | |
| async createX509Certificate(options: X509CreateCertificateOptions, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start creating X509 certificate'); | |
| this.logger.debug('Creating X509 certificate request'); // avoid logging payload | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/agent-service/src/agent-service.service.ts around lines 2229 to 2243,
remove the detailed logging of the certificate options (do not log the full
options object or stringify it) and replace it with a minimal, non-sensitive
message; if you must record something, log only a safe summary (e.g., orgId,
url, or a boolean/size indicator) after redacting any sensitive fields, and
ensure you call logger.debug with a proper string message (or the logger's
accepted signature) rather than passing the options object as the second
argument.
| async decodeX509Certificate(options: x509CertificateDecodeDto, url: string, orgId: string): Promise<object> { | ||
| try { | ||
| this.logger.log('Start decoding X509 certificate'); | ||
| this.logger.debug('Decoding X509 certificate with options', options); | ||
| const getApiKey = await this.getOrgAgentApiKey(orgId); | ||
| const x509Certificate = await this.commonService | ||
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | ||
| .then(async (response) => response); | ||
| return x509Certificate; | ||
| } catch (error) { | ||
| this.logger.error(`Error in decoding x509 certificate in agent service : ${JSON.stringify(error)}`); | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Redact decode options; Logger.debug misuse
Avoid logging full decode payload; keep message-only logs.
async decodeX509Certificate(options: x509CertificateDecodeDto, url: string, orgId: string): Promise<object> {
try {
- this.logger.log('Start decoding X509 certificate');
- this.logger.debug('Decoding X509 certificate with options', options);
+ this.logger.log('Start decoding X509 certificate');
+ this.logger.debug('Decoding X509 certificate request');
const getApiKey = await this.getOrgAgentApiKey(orgId);
const x509Certificate = await this.commonService
.httpPost(url, options, { headers: { authorization: getApiKey } })
.then(async (response) => response);
return x509Certificate;
} catch (error) {
this.logger.error(`Error in decoding x509 certificate in agent service : ${JSON.stringify(error)}`);
throw error;
}
}📝 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.
| async decodeX509Certificate(options: x509CertificateDecodeDto, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start decoding X509 certificate'); | |
| this.logger.debug('Decoding X509 certificate with options', options); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in decoding x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw error; | |
| } | |
| } | |
| async decodeX509Certificate(options: x509CertificateDecodeDto, url: string, orgId: string): Promise<object> { | |
| try { | |
| this.logger.log('Start decoding X509 certificate'); | |
| this.logger.debug('Decoding X509 certificate request'); | |
| const getApiKey = await this.getOrgAgentApiKey(orgId); | |
| const x509Certificate = await this.commonService | |
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | |
| .then(async (response) => response); | |
| return x509Certificate; | |
| } catch (error) { | |
| this.logger.error(`Error in decoding x509 certificate in agent service : ${JSON.stringify(error)}`); | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/agent-service/src/agent-service.service.ts around lines 2244 to 2257,
the debug log currently prints the full decode options object which may contain
sensitive data; change the debug call to only log a message (e.g., "Decoding
X509 certificate") and do not include or stringify the options payload. If you
need lightweight telemetry, log non-sensitive metadata only (like option keys or
payload size) or a redacted/hashed representation instead of the full options
object.
| async importX509Certificate(options: IX509ImportCertificateOptionsDto, url: string, orgId: string): Promise<object> { | ||
| try { | ||
| this.logger.log('Start importing X509 certificate'); | ||
| this.logger.debug(`Importing X509 certificate with options`, options.certificate); | ||
| const getApiKey = await this.getOrgAgentApiKey(orgId); | ||
| const x509Certificate = await this.commonService | ||
| .httpPost(url, options, { headers: { authorization: getApiKey } }) | ||
| .then(async (response) => response); | ||
| return x509Certificate; | ||
| } catch (error) { | ||
| this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`); | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Do not log base64 certificate material; fix error message text
Logging options.certificate leaks sensitive content. Error log says “creating” in import path.
async importX509Certificate(options: IX509ImportCertificateOptionsDto, url: string, orgId: string): Promise<object> {
try {
- this.logger.log('Start importing X509 certificate');
- this.logger.debug(`Importing X509 certificate with options`, options.certificate);
+ this.logger.log('Start importing X509 certificate');
+ this.logger.debug('Importing X509 certificate request'); // do not log cert
const getApiKey = await this.getOrgAgentApiKey(orgId);
const x509Certificate = await this.commonService
.httpPost(url, options, { headers: { authorization: getApiKey } })
.then(async (response) => response);
return x509Certificate;
} catch (error) {
- this.logger.error(`Error in creating x509 certificate in agent service : ${JSON.stringify(error)}`);
+ this.logger.error(`Error in importing x509 certificate in agent service : ${JSON.stringify(error)}`);
throw error;
}
}🤖 Prompt for AI Agents
In apps/agent-service/src/agent-service.service.ts around lines 2259-2272, the
code currently logs options.certificate (leaking base64 sensitive data) and uses
an incorrect error message saying "creating" instead of "importing"; remove any
logging of the certificate or other sensitive payloads, change the debug log to
omit or redact the certificate (e.g., log only metadata such as certificate
length or a redacted placeholder), and update the catch logger to say "Error
importing x509 certificate" while logging only non-sensitive error details
(error.message and optional sanitized stack) rather than JSON.stringify of the
entire error or request payload.
| import { ApiProperty, ApiPropertyOptional, getSchemaPath } from '@nestjs/swagger'; | ||
| import { | ||
| ArrayNotEmpty, | ||
| IsArray, | ||
| IsBoolean, | ||
| IsDate, | ||
| IsEnum, | ||
| IsNotEmpty, | ||
| IsNotEmptyObject, | ||
| IsNumber, | ||
| IsOptional, | ||
| IsString, | ||
| Max, | ||
| Min, | ||
| ValidateNested | ||
| } from 'class-validator'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add ValidateIf to support union validation below
Needed for issuer/subject union validation decorators.
-import {
+import {
ArrayNotEmpty,
IsArray,
IsBoolean,
IsDate,
IsEnum,
IsNotEmpty,
IsNotEmptyObject,
IsNumber,
IsOptional,
IsString,
Max,
Min,
- ValidateNested
+ ValidateNested,
+ ValidateIf
} from 'class-validator';📝 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.
| import { ApiProperty, ApiPropertyOptional, getSchemaPath } from '@nestjs/swagger'; | |
| import { | |
| ArrayNotEmpty, | |
| IsArray, | |
| IsBoolean, | |
| IsDate, | |
| IsEnum, | |
| IsNotEmpty, | |
| IsNotEmptyObject, | |
| IsNumber, | |
| IsOptional, | |
| IsString, | |
| Max, | |
| Min, | |
| ValidateNested | |
| } from 'class-validator'; | |
| import { ApiProperty, ApiPropertyOptional, getSchemaPath } from '@nestjs/swagger'; | |
| import { | |
| ArrayNotEmpty, | |
| IsArray, | |
| IsBoolean, | |
| IsDate, | |
| IsEnum, | |
| IsNotEmpty, | |
| IsNotEmptyObject, | |
| IsNumber, | |
| IsOptional, | |
| IsString, | |
| Max, | |
| Min, | |
| ValidateNested, | |
| ValidateIf | |
| } from 'class-validator'; |
🤖 Prompt for AI Agents
In apps/api-gateway/src/x509/dtos/x509.dto.ts around lines 1 to 16, the
class-validator import list is missing ValidateIf which is required for the
issuer/subject union validation decorators; update the import from
'class-validator' to include ValidateIf so the union conditional validators
compile and work correctly.
| export class NameListDto { | ||
| @ApiProperty({ type: [NameDto] }) | ||
| @ArrayNotEmpty() | ||
| @IsArray() | ||
| @ValidateNested() | ||
| @Type(() => NameDto) | ||
| name: NameDto[]; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate each element in NameListDto.name
Add { each: true } so nested NameDto items are validated.
- @ValidateNested()
+ @ValidateNested({ each: true })
@Type(() => NameDto)
name: NameDto[];📝 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.
| export class NameListDto { | |
| @ApiProperty({ type: [NameDto] }) | |
| @ArrayNotEmpty() | |
| @IsArray() | |
| @ValidateNested() | |
| @Type(() => NameDto) | |
| name: NameDto[]; | |
| export class NameListDto { | |
| @ApiProperty({ type: [NameDto] }) | |
| @ArrayNotEmpty() | |
| @IsArray() | |
| @ValidateNested({ each: true }) | |
| @Type(() => NameDto) | |
| name: NameDto[]; | |
| } |
🤖 Prompt for AI Agents
In apps/api-gateway/src/x509/dtos/x509.dto.ts around lines 103 to 110, the class
property name: NameDto[] does not validate each nested NameDto element; update
the decorators to validate nested items by changing ValidateNested() to
ValidateNested({ each: true }) (keep @Type(() => NameDto) and the other
decorators as-is) so each array element is properly validated.
| this.logger.log(`Decoded certificate`); | ||
| this.logger.debug(`certificate data:`, JSON.stringify(decodedResult)); | ||
|
|
||
| const { publicKey } = decodedResult.response; | ||
| const decodedCert = decodedResult.response.x509Certificate; | ||
|
|
||
| this.logger.log(`Start validating certificate`); | ||
| const isValidKeyType = Object.values(x5cKeyType).includes(publicKey.keyType as x5cKeyType); | ||
|
|
There was a problem hiding this comment.
Avoid logging decoded certificate content
decodedResult may contain cert/public key. Don’t log it; if needed, log a fingerprint/serial only.
- this.logger.log(`Decoded certificate`);
- this.logger.debug(`certificate data:`, JSON.stringify(decodedResult));
+ this.logger.log(`Decoded certificate`);
+ // Consider logging only serial/fingerprint if available, not full content.📝 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.
| this.logger.log(`Decoded certificate`); | |
| this.logger.debug(`certificate data:`, JSON.stringify(decodedResult)); | |
| const { publicKey } = decodedResult.response; | |
| const decodedCert = decodedResult.response.x509Certificate; | |
| this.logger.log(`Start validating certificate`); | |
| const isValidKeyType = Object.values(x5cKeyType).includes(publicKey.keyType as x5cKeyType); | |
| this.logger.log(`Decoded certificate`); | |
| // Consider logging only serial/fingerprint if available, not full content. | |
| const { publicKey } = decodedResult.response; | |
| const decodedCert = decodedResult.response.x509Certificate; | |
| this.logger.log(`Start validating certificate`); | |
| const isValidKeyType = Object.values(x5cKeyType).includes(publicKey.keyType as x5cKeyType); |
🤖 Prompt for AI Agents
In apps/x509/src/x509.service.ts around lines 161 to 169, avoid logging the full
decodedResult (which can contain certificates and public keys); remove the
this.logger.debug(`certificate data:`, JSON.stringify(decodedResult)) call and
instead log a safe identifier such as the certificate serial number or a
fingerprint (e.g., compute and log a SHA-256 hash of
decodedResult.response.x509Certificate or use
decodedResult.response.x509Certificate.serialNumber if available), and ensure no
raw publicKey or full certificate bytes are emitted to logs.
| if (!isValidKeyType) { | ||
| this.logger.error(`keyType is not valid for importing certificate`); | ||
| throw new InternalServerErrorException(ResponseMessages.x509.error.import); | ||
| } |
There was a problem hiding this comment.
Use a 4xx for invalid key type (client error), not 5xx
Invalid input should return UnprocessableEntity or BadRequest.
- throw new InternalServerErrorException(ResponseMessages.x509.error.import);
+ throw new UnprocessableEntityException(ResponseMessages.x509.error.import);🤖 Prompt for AI Agents
In apps/x509/src/x509.service.ts around lines 170 to 173, the code currently
throws an InternalServerErrorException for an invalid keyType; change this to a
4xx client error (e.g., throw new
UnprocessableEntityException(ResponseMessages.x509.error.import) or
BadRequestException) and adjust the log to reflect a client input error (use
logger.warn or keep logger.error but clarify the message), so invalid input
returns an appropriate 4xx response instead of a 5xx.
| const createDto: CreateX509CertificateEntity = { | ||
| orgId, | ||
| certificateBase64: certificate.response.issuerCertficicate, | ||
| keyType: publicKey.keyType, | ||
| status: certStatus, | ||
| validFrom, | ||
| expiry, | ||
| createdBy: user.id, | ||
| lastChangedBy: user.id | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Likely typo: response.issuerCertficicate
This property name looks misspelled and may be incorrect, causing bad persistence. Verify agent-service/import response shape and fix the field name.
Run this to search for the field name and related variants:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for possible response field names across repo..."
rg -n -C2 -iP 'issuerCertficicate|issuerCertificate|publicCertificateBase64|certificateBase64|x509Certificate' || true
echo "Search agent import/decoder handlers for response shapes..."
rg -n -C3 -iP 'import-x509|decode-x509|X509_IMPORT_CERTIFICATE|X509_DECODE_CERTIFICATE|create-x509' || trueLength of output: 53149
Fix typo in import response property Change certificate.response.issuerCertficicate to the correctly spelled field returned by the agent (e.g. issuerCertificate) in apps/x509/src/x509.service.ts.
🤖 Prompt for AI Agents
In apps/x509/src/x509.service.ts around lines 209 to 218, the code uses the
misspelled property certificate.response.issuerCertficicate; update this to the
correctly spelled property returned by the agent (e.g.
certificate.response.issuerCertificate) everywhere it is referenced in this
block so the CreateX509CertificateEntity.certificateBase64 receives the correct
field.
| export interface IX509ImportCertificateOptionsDto { | ||
| /* | ||
| X.509 certificate in base64 string format | ||
| */ | ||
| certificate: string; | ||
|
|
||
| /* | ||
| Private key in base64 string format | ||
| */ | ||
| privateKey?: string; | ||
|
|
||
| keyType: KeyType; | ||
| } | ||
|
|
There was a problem hiding this comment.
Undefined type KeyType used in public interface
KeyType isn’t imported/declared. This breaks compilation. Use x5cKeyType (already imported).
export interface IX509ImportCertificateOptionsDto {
@@
- keyType: KeyType;
+ keyType: x5cKeyType;
}📝 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.
| export interface IX509ImportCertificateOptionsDto { | |
| /* | |
| X.509 certificate in base64 string format | |
| */ | |
| certificate: string; | |
| /* | |
| Private key in base64 string format | |
| */ | |
| privateKey?: string; | |
| keyType: KeyType; | |
| } | |
| export interface IX509ImportCertificateOptionsDto { | |
| /* | |
| X.509 certificate in base64 string format | |
| */ | |
| certificate: string; | |
| /* | |
| Private key in base64 string format | |
| */ | |
| privateKey?: string; | |
| keyType: x5cKeyType; | |
| } |
🤖 Prompt for AI Agents
In libs/common/src/interfaces/x509.interface.ts around lines 230 to 243, the
interface uses an undefined type KeyType which breaks compilation; replace
KeyType with the existing imported x5cKeyType for the keyType property and
ensure the import for x5cKeyType remains (remove KeyType reference), updating
any related typings or exports to use x5cKeyType consistently.
| "validFrom" TIMESTAMP(3) NOT NULL, | ||
| "expiry" TIMESTAMP(3) NOT NULL, | ||
| "certificateBase64" TEXT NOT NULL, | ||
| "isImported" BOOLEAN NOT NULL DEFAULT false, | ||
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "createdBy" UUID NOT NULL, | ||
| "lastChangedDateTime" TIMESTAMP(3) NOT NULL, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Timestamp precision inconsistency between tables.
The x509_certificates table uses TIMESTAMP(3) (millisecond precision) for validFrom, expiry, createdAt, and lastChangedDateTime, while the oid4vc_credentials table uses TIMESTAMPTZ(6) (microsecond precision with timezone). This inconsistency may lead to issues when comparing timestamps across tables.
For consistency, consider using the same timestamp type and precision across all tables. Based on learnings, TIMESTAMPTZ(6) is the standard used elsewhere in the schema.
Apply this diff:
CREATE TABLE "x509_certificates" (
"id" TEXT NOT NULL,
"orgAgentId" UUID NOT NULL,
"keyType" TEXT NOT NULL,
"status" TEXT NOT NULL,
- "validFrom" TIMESTAMP(3) NOT NULL,
- "expiry" TIMESTAMP(3) NOT NULL,
+ "validFrom" TIMESTAMPTZ(6) NOT NULL,
+ "expiry" TIMESTAMPTZ(6) NOT NULL,
"certificateBase64" TEXT NOT NULL,
"isImported" BOOLEAN NOT NULL DEFAULT false,
- "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
+ "createdAt" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP,
"createdBy" UUID NOT NULL,
- "lastChangedDateTime" TIMESTAMP(3) NOT NULL,
+ "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL,
"lastChangedBy" UUID NOT NULL,
CONSTRAINT "x509_certificates_pkey" PRIMARY KEY ("id")
);📝 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.
| "validFrom" TIMESTAMP(3) NOT NULL, | |
| "expiry" TIMESTAMP(3) NOT NULL, | |
| "certificateBase64" TEXT NOT NULL, | |
| "isImported" BOOLEAN NOT NULL DEFAULT false, | |
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| "createdBy" UUID NOT NULL, | |
| "lastChangedDateTime" TIMESTAMP(3) NOT NULL, | |
| CREATE TABLE "x509_certificates" ( | |
| "id" TEXT NOT NULL, | |
| "orgAgentId" UUID NOT NULL, | |
| "keyType" TEXT NOT NULL, | |
| "status" TEXT NOT NULL, | |
| "validFrom" TIMESTAMPTZ(6) NOT NULL, | |
| "expiry" TIMESTAMPTZ(6) NOT NULL, | |
| "certificateBase64" TEXT NOT NULL, | |
| "isImported" BOOLEAN NOT NULL DEFAULT false, | |
| "createdAt" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| "createdBy" UUID NOT NULL, | |
| "lastChangedDateTime" TIMESTAMPTZ(6) NOT NULL, | |
| "lastChangedBy" UUID NOT NULL, | |
| CONSTRAINT "x509_certificates_pkey" PRIMARY KEY ("id") | |
| ); |
🤖 Prompt for AI Agents
In
libs/prisma-service/prisma/migrations/20251013125236_added_x509_certificate_table/migration.sql
around lines 39 to 45, change the four columns using TIMESTAMP(3) to use
TIMESTAMPTZ(6) to match the rest of the schema: replace "validFrom" TIMESTAMP(3)
NOT NULL, "expiry" TIMESTAMP(3) NOT NULL, "createdAt" TIMESTAMP(3) NOT NULL
DEFAULT CURRENT_TIMESTAMP, and "lastChangedDateTime" TIMESTAMP(3) NOT NULL with
TIMESTAMPTZ(6) equivalents (and update the createdAt default to
CURRENT_TIMESTAMP(6) or now() AT TIME ZONE 'UTC' with microsecond precision) so
all tables consistently use microsecond precision with timezone.
tipusinghaw
changed the title
Signed-off-by: Rinkal Bhojani <[email protected]>
Signed-off-by: Rinkal Bhojani <[email protected]>
Signed-off-by: Rinkal Bhojani <[email protected]>
Signed-off-by: Rinkal Bhojani <[email protected]>
Summary by CodeRabbit
New Features
Configuration
Refactor