refactor: create oid4vc issuer and update oid4vc issuer API endpoints#1607
Merged
Conversation
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
…y issuer Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
pranalidhanavade
requested review from
RinkalBhojani,
shitrerohit and
tipusinghaw
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds organisation ownership and single-primary-issuer support: DTOs and interfaces gain Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Service as OID4VC Service
participant Repo as Repository
participant DB as Database
Client->>Service: Create Issuer (orgAgentId, metadata)
Service->>Repo: hasPrimaryIssuer(orgAgentId)
Repo->>DB: SELECT COUNT(*) WHERE orgId = orgAgentId AND isPrimary = true
DB-->>Repo: count
Repo-->>Service: boolean (hasPrimary)
Note over Service: Set isPrimary = !hasPrimary
Service->>Repo: addOidcIssuerDetails(metadata + orgId + isPrimary)
Repo->>DB: INSERT oidc_issuer (orgId, isPrimary, metadata...)
DB-->>Repo: created row
Repo-->>Service: issuer record
Service-->>Client: Created issuer response
Client->>Service: Update Issuer (issuerId, orgId, isPrimary)
Service->>Repo: updateOidcIssuerDetails(createdById, updatePayload)
Repo->>DB: BEGIN TRANSACTION
Repo->>DB: SELECT issuer WHERE id = issuerId AND orgId = orgId
DB-->>Repo: issuer or not found
alt set isPrimary = true
Repo->>DB: UPDATE oidc_issuer SET isPrimary = false WHERE orgId = orgId AND id != issuerId
else set isPrimary = false
Repo->>DB: SELECT COUNT(*) WHERE orgId = orgId AND isPrimary = true AND id != issuerId
DB-->>Repo: count
alt count == 0
Repo-->>Service: throw BadRequest (would leave no primary)
Repo->>DB: ROLLBACK
end
end
Repo->>DB: UPDATE target issuer with new metadata, isPrimary...
DB-->>Repo: updated row
Repo->>DB: COMMIT
Repo-->>Service: updated issuer
Service-->>Client: Updated issuer response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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 (2)
apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (1)
98-160:⚠️ Potential issue | 🟠 MajorRace condition:
hasPrimaryIssuer+addOidcIssuerDetailsis not atomic.
hasPrimaryIssueris read outside any transaction, then a separate insert is performed. Two concurrentoidcIssuerCreatecalls for the same org can both observehasPrimary === falseand both insertisPrimary: true, leaving the org with two primary issuers — the exact invariant this PR is trying to enforce.Options, in order of robustness:
- Add a Postgres partial unique index
CREATE UNIQUE INDEX ... ON "oidc_issuer"("orgId") WHERE "isPrimary" = true;so concurrent races fail at the DB. (This also protects the update path.)- Wrap the
hasPrimaryIssuerread and theoidc_issuer.createin a singleprisma.$transaction, same wayupdateOidcIssuerDetailsdoes.Given the update path already uses a transaction,
#1is the cleanest single source of truth and also protects against update/create races.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts` around lines 98 - 160, The read-then-insert using hasPrimaryIssuer and addOidcIssuerDetails can race and allow multiple isPrimary=true rows; add a DB-level guard: create a Postgres partial unique index on oidc_issuer(orgId) WHERE isPrimary = true to enforce uniqueness atomically, and keep the existing code path but handle DB unique-constraint failures by mapping the DB error to the existing ConflictException logic (the same place that checks for 409 in the _createOIDCIssuer error handling); alternatively, if you prefer a code-only fix instead of the index, wrap the hasPrimaryIssuer read and the addOidcIssuerDetails insert in a single prisma.$transaction (matching the pattern used in updateOidcIssuerDetails) so the check-and-insert is atomic.apps/oid4vc-issuance/interfaces/oid4vc-issuance.interfaces.ts (1)
95-102:⚠️ Potential issue | 🔴 CriticalCritical:
orgIdis not merged intoIssuerUpdationbefore the repository call, causing cross-tenant data corruption.The service method
oidcIssuerUpdatereceivesorgIdas a separate parameter but passesissuerUpdationConfigto the repository without merging it in. When the repository destructuresorgIdfrom the config, it receivesundefined.If
isPrimary: trueis set, theupdateManyquery becomes:where: { orgId: undefined, isPrimary: true }Prisma omits
undefinedfilters, so this effectively resetsisPrimarytofalseon all primary issuers across every organization—a cross-tenant data corruption bug.Fix: In the service (line 190), either:
- Merge orgId before calling the repository:
const updateConfig = { ...issuerUpdationConfig, orgId }; await this.oid4vcIssuanceRepository.updateOidcIssuerDetails(userDetails.id, updateConfig);Or add a guard in the repository to prevent undefined orgId:
if (!orgId) { throw new Error('updateOidcIssuerDetails: orgId is required'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/interfaces/oid4vc-issuance.interfaces.ts` around lines 95 - 102, The IssuerUpdation payload lacks orgId when passed to the repository causing undefined orgId and cross-tenant updates; update the service method oidcIssuerUpdate to merge the orgId into issuerUpdationConfig before calling updateOidcIssuerDetails (e.g. create updateConfig = { ...issuerUpdationConfig, orgId } and pass that), and/or add a defensive check in the repository method updateOidcIssuerDetails to throw if orgId is missing to prevent accidental global updates.
🧹 Nitpick comments (5)
apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (1)
98-101: Scope the "primary" check byorgId, notorgAgentId, for consistency.
updateOidcIssuerDetailsscopes primary-reset byorgId, buthasPrimaryIssuerhere is called withorgAgentId. Todayorg_agents.orgIdis@uniqueso these happen to coincide, but if the system ever supports multiple agents per org (or the uniqueness is relaxed), the create and update paths will disagree on what "primary" means. PassingorgIdtohasPrimaryIssuerand queryingoidc_issuer.orgIdkeeps both paths aligned with the declared invariant "one primary issuer per org."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts` around lines 98 - 101, The primary-check is currently scoped by orgAgentId causing inconsistency with updateOidcIssuerDetails which scopes by orgId; update the call to oid4vcIssuanceRepository.hasPrimaryIssuer so it receives orgId (not orgAgentId) and ensure the repository/query logic for hasPrimaryIssuer checks oidc_issuer.orgId; update any callers or parameter names around hasPrimaryIssuer and the local variable (e.g., where isPrimary is computed) to use orgId so create and update paths consistently enforce "one primary issuer per org."apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts (3)
191-220:addOidcIssuerDetailscorrectly persists new fields — minor defensive check worth adding.The new fields
isPrimaryandorgIdare wired in correctly. One small suggestion: guard against an empty/undefinedorgIdreaching the repository, mirroring what the schema expects (NOT NULL). A missingorgIdhere produces a Prisma runtime error that's harder to trace than a clear application-level validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts` around lines 191 - 220, In addOidcIssuerDetails, validate issuerMetadata.orgId before calling prisma to avoid a Prisma NOT NULL runtime error: check that orgId is defined/non-empty (from the issuerMetadata destructured in addOidcIssuerDetails) and if missing log a clear message with this.logger.error and throw a descriptive application-level error (e.g., "Missing orgId in issuerMetadata") so callers get a clear validation failure instead of a Prisma exception.
222-230: AlignhasPrimaryIssuerwith the rest of the primary-issuer logic by keying onorgId.Everywhere else, "primary" is scoped by organization (
updateOidcIssuerDetailsusesorgId, the PR description says "only one primary issuer per organization"). Here the lookup is keyed onorgAgentId. Today these coincide becauseorg_agents.orgIdis@unique, but it's a subtle coupling that will silently break if that ever changes. TakingorgIdas the parameter makes the invariant explicit and consistent with the update path.♻️ Proposed refactor
- async hasPrimaryIssuer(orgAgentId: string): Promise<boolean> { - const count = await this.prisma.oidc_issuer.count({ - where: { - orgAgentId, - isPrimary: true - } - }); - return 0 < count; - } + async hasPrimaryIssuer(orgId: string): Promise<boolean> { + const count = await this.prisma.oidc_issuer.count({ + where: { orgId, isPrimary: true } + }); + return 0 < count; + }…and update the call site in
oidc-issuance.service.tsaccordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts` around lines 222 - 230, Change hasPrimaryIssuer to key on orgId instead of orgAgentId: update the method signature (hasPrimaryIssuer(orgId: string): Promise<boolean>), change the prisma query in oid4vc-issuance.repository (prisma.oidc_issuer.count where { orgId, isPrimary: true }), and then update its call site(s) in oidc-issuance.service.ts to pass the organization id (orgId) rather than orgAgentId so the "one primary per organization" invariant matches updateOidcIssuerDetails.
250-260: UnnecessarycreatedByoverwrite on update.
createdBy: createdByIdis written on every update, which means the field no longer reflects the actual creator of the row — it becomes "last updater." This is probably a pre-existing behavior but it's worth fixing while you're in the method: drop it or rename the target column tolastChangedBy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts` around lines 250 - 260, The update currently overwrites the original creator by setting createdBy: createdById in tx.oidc_issuer.update; remove the createdBy assignment from the data payload so updates no longer mutate the creator field (i.e., delete the createdBy: createdById entry in the object passed to tx.oidc_issuer.update). If you need to track the user who last modified the row, add/assign a proper lastChangedBy (or lastUpdatedBy) column instead and set that (e.g., lastChangedBy: lastChangedById) after adding the column to the schema—do not reuse createdBy for this purpose.libs/prisma-service/prisma/schema.prisma (1)
599-603: Consider a partial unique index to enforce one primary issuer per org at the DB level.The transactional logic in
updateOidcIssuerDetails/oidcIssuerCreateenforces "only one primary per org" in the application, but a concurrent create + update (or two concurrent updates) can race and briefly leave two primary rows — or permanently, if the transaction isolation level allows. Postgres supports a partial unique index that makes this an invariant:CREATE UNIQUE INDEX oidc_issuer_one_primary_per_org ON "oidc_issuer" ("orgId") WHERE "isPrimary" = true;In Prisma 5.22, you can declare this via
@@uniquewith awhere, or add it as a raw-SQL migration. Happy to draft either.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/prisma-service/prisma/schema.prisma` around lines 599 - 603, The DB schema allows multiple rows with isPrimary=true for the same org under concurrent operations; enforce the invariant at the DB level by adding a partial unique index on orgId when isPrimary is true. Update the Prisma schema for the oidc_issuer model (fields orgId and isPrimary) to declare a unique constraint with a where clause (or add a raw SQL migration) so Postgres will create a unique index like ON "oidc_issuer" ("orgId") WHERE "isPrimary" = true; this complements the application logic in updateOidcIssuerDetails and oidcIssuerCreate and prevents race conditions from producing multiple primary issuers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer.dto.ts`:
- Around line 182-188: The DTO currently allows setting isPrimary to false which
can leave an org with no primary issuer; update the behavior so either (a) the
DTO/validation for Oid4vcIssuerDto only permits clients to set isPrimary when
true (e.g., make isPrimary optional but only accept true from clients), or (b)
more robustly enforce in the service method updateOidcIssuerDetails that a
request setting isPrimary === false on the current primary is rejected unless
another issuer is simultaneously promoted — implement a guard in
updateOidcIssuerDetails to check current primary state and either auto-promote
another issuer or return a validation error to the caller.
In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts`:
- Around line 232-268: updateOidcIssuerDetails allows cross-tenant updates
because the final tx.oidc_issuer.update only filters by id; restrict the update
to the caller's tenant by changing the update "where" to include both id and
orgId (i.e., where: { id: issuerId, orgId }) so Prisma will fail if the issuer
doesn't belong to that org; keep the existing updateMany reset logic
(tx.oidc_issuer.updateMany) but ensure both updateMany and the subsequent
tx.oidc_issuer.update run in the same transaction (as they already do) to
maintain atomicity and prevent silent cross-org promotion.
In `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts`:
- Around line 99-102: Remove the two raw console.log debug statements that print
orgAgentId and isPrimary in oid4vc-issuance.service.ts; instead either delete
them or replace with structured logging via the service Logger (use
this.logger.debug('orgAgentId: %s', orgAgentId) and this.logger.debug('isPrimary
computed: %s', isPrimary)) where orgAgentId, isPrimary and hasPrimary are used
so you preserve context without bypassing log levels.
In
`@libs/prisma-service/prisma/migrations/20260424035744_add_orgid_to_oid4vc_issuer/migration.sql`:
- Line 17: Change the migration to add "orgId" in three steps instead of a NOT
NULL add: 1) ALTER TABLE "oidc_issuer" ADD COLUMN "orgId" UUID NULL; 2) backfill
values with an UPDATE using the existing relation: UPDATE "oidc_issuer" SET
"orgId" = oa."orgId" FROM "org_agents" oa WHERE "oidc_issuer"."orgAgentId" =
oa.id; 3) ALTER TABLE "oidc_issuer" ALTER COLUMN "orgId" SET NOT NULL; and also
update libs/prisma-service/prisma/schema.prisma to add the corresponding
`@relation` so Prisma will emit a stable foreign key constraint for
oidc_issuer."orgId" -> org_agents.id.
- Around line 7-23: This migration mixes unrelated FK rework with the intended
change; revert the DROP/ALTER/ADD statements for the oid4vc_credentials and
issued_oid4vc_credentials FK changes in this migration and keep only the
addition of the "orgId" column on oidc_issuer and any directly related
statements; move the FK modifications (DROP CONSTRAINT
"oid4vc_credentials_orgId_fkey", ALTER TABLE "oid4vc_credentials" ALTER COLUMN
"orgId" DROP NOT NULL, ADD CONSTRAINT "oid4vc_credentials_orgId_fkey", and the
analogous issued_oid4vc_credentials constraint changes) into a separate
migration with a clear commit message explaining the ON DELETE behavior change
(references: tables oidc_issuer, oid4vc_credentials, issued_oid4vc_credentials
and constraint names "oid4vc_credentials_orgId_fkey" and
"issued_oid4vc_credentials_orgId_fkey").
In `@libs/prisma-service/prisma/schema.prisma`:
- Around line 599-601: The oidc_issuer model's orgId field is missing a Prisma
foreign-key relation to organisation; update the oidc_issuer model to change
orgId from a bare String to a relation pair (e.g., orgId String `@db.Uuid` and org
Organisation `@relation`(fields: [orgId], references: [id])) and on the
organisation model add the back-relation field oidc_issuer oidc_issuer[]; then
create a migration that adds the FK constraint for oidc_issuer.orgId ->
organisation.id so deletes/cascades and referential integrity behave
consistently with the other models.
---
Outside diff comments:
In `@apps/oid4vc-issuance/interfaces/oid4vc-issuance.interfaces.ts`:
- Around line 95-102: The IssuerUpdation payload lacks orgId when passed to the
repository causing undefined orgId and cross-tenant updates; update the service
method oidcIssuerUpdate to merge the orgId into issuerUpdationConfig before
calling updateOidcIssuerDetails (e.g. create updateConfig = {
...issuerUpdationConfig, orgId } and pass that), and/or add a defensive check in
the repository method updateOidcIssuerDetails to throw if orgId is missing to
prevent accidental global updates.
In `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts`:
- Around line 98-160: The read-then-insert using hasPrimaryIssuer and
addOidcIssuerDetails can race and allow multiple isPrimary=true rows; add a
DB-level guard: create a Postgres partial unique index on oidc_issuer(orgId)
WHERE isPrimary = true to enforce uniqueness atomically, and keep the existing
code path but handle DB unique-constraint failures by mapping the DB error to
the existing ConflictException logic (the same place that checks for 409 in the
_createOIDCIssuer error handling); alternatively, if you prefer a code-only fix
instead of the index, wrap the hasPrimaryIssuer read and the
addOidcIssuerDetails insert in a single prisma.$transaction (matching the
pattern used in updateOidcIssuerDetails) so the check-and-insert is atomic.
---
Nitpick comments:
In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts`:
- Around line 191-220: In addOidcIssuerDetails, validate issuerMetadata.orgId
before calling prisma to avoid a Prisma NOT NULL runtime error: check that orgId
is defined/non-empty (from the issuerMetadata destructured in
addOidcIssuerDetails) and if missing log a clear message with this.logger.error
and throw a descriptive application-level error (e.g., "Missing orgId in
issuerMetadata") so callers get a clear validation failure instead of a Prisma
exception.
- Around line 222-230: Change hasPrimaryIssuer to key on orgId instead of
orgAgentId: update the method signature (hasPrimaryIssuer(orgId: string):
Promise<boolean>), change the prisma query in oid4vc-issuance.repository
(prisma.oidc_issuer.count where { orgId, isPrimary: true }), and then update its
call site(s) in oidc-issuance.service.ts to pass the organization id (orgId)
rather than orgAgentId so the "one primary per organization" invariant matches
updateOidcIssuerDetails.
- Around line 250-260: The update currently overwrites the original creator by
setting createdBy: createdById in tx.oidc_issuer.update; remove the createdBy
assignment from the data payload so updates no longer mutate the creator field
(i.e., delete the createdBy: createdById entry in the object passed to
tx.oidc_issuer.update). If you need to track the user who last modified the row,
add/assign a proper lastChangedBy (or lastUpdatedBy) column instead and set that
(e.g., lastChangedBy: lastChangedById) after adding the column to the schema—do
not reuse createdBy for this purpose.
In `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts`:
- Around line 98-101: The primary-check is currently scoped by orgAgentId
causing inconsistency with updateOidcIssuerDetails which scopes by orgId; update
the call to oid4vcIssuanceRepository.hasPrimaryIssuer so it receives orgId (not
orgAgentId) and ensure the repository/query logic for hasPrimaryIssuer checks
oidc_issuer.orgId; update any callers or parameter names around hasPrimaryIssuer
and the local variable (e.g., where isPrimary is computed) to use orgId so
create and update paths consistently enforce "one primary issuer per org."
In `@libs/prisma-service/prisma/schema.prisma`:
- Around line 599-603: The DB schema allows multiple rows with isPrimary=true
for the same org under concurrent operations; enforce the invariant at the DB
level by adding a partial unique index on orgId when isPrimary is true. Update
the Prisma schema for the oidc_issuer model (fields orgId and isPrimary) to
declare a unique constraint with a where clause (or add a raw SQL migration) so
Postgres will create a unique index like ON "oidc_issuer" ("orgId") WHERE
"isPrimary" = true; this complements the application logic in
updateOidcIssuerDetails and oidcIssuerCreate and prevents race conditions from
producing multiple primary issuers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 123110d4-9937-4fe2-a857-c4b003552fca
📒 Files selected for processing (7)
apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer.dto.tsapps/oid4vc-issuance/interfaces/oid4vc-issuance.interfaces.tsapps/oid4vc-issuance/src/oid4vc-issuance.repository.tsapps/oid4vc-issuance/src/oid4vc-issuance.service.tslibs/prisma-service/prisma/migrations/20260422115126_add_isprimary_flag_into_oid4vc_issuer_table/migration.sqllibs/prisma-service/prisma/migrations/20260424035744_add_orgid_to_oid4vc_issuer/migration.sqllibs/prisma-service/prisma/schema.prisma
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts (2)
260-270: Defense-in-depth: scope the finalupdateby bothidandorgId.Ownership is already validated via
findFirst({ id, orgId })earlier in the transaction, so this is not a current bug. However, scoping the update by both keys is essentially free and prevents a future refactor (e.g., moving the existence check elsewhere or skipping it on a "fast path") from silently re-introducing the cross-tenant write the previous review flagged.♻️ Proposed change
const updatedIssuer = await tx.oidc_issuer.update({ - where: { id: issuerId }, + where: { id: issuerId, orgId }, data: {Note: this requires
@@unique([id, orgId])(or similar) onoidc_issuerfor Prisma to accept a compositewhere; alternatively keepwhere: { id: issuerId }but stick with the explicit pre-check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts` around lines 260 - 270, The update to tx.oidc_issuer.update is currently only scoped by id (issuerId) which could allow cross-tenant writes if checks move; change the update's where to include both id: issuerId and orgId (orgId) so the final write is constrained to the tenant you validated earlier (after the findFirst that checked id and orgId). If your Prisma schema supports a composite unique ([id, orgId]) use a composite where ({ id_orgId: { id: issuerId, orgId }}) or equivalent; if not, keep the explicit pre-check (the existing findFirst) and ensure it cannot be removed in future refactors so the update remains tenant-scoped.
248-258: Enforce the single-primary invariant at the DB level.The transaction prevents intra-request races, but two concurrent
updateOidcIssuerDetailscalls (oraddOidcIssuerDetails+ update) for different issuers in the sameorgIdcan both pass theupdateMany/insert step and commit, leaving the org with twoisPrimary: truerows. Under PostgreSQL's default Read Committed isolation, Prisma$transactionwon't detect this conflict.Add a partial unique index in the Prisma schema so the database rejects a second primary atomically:
// libs/prisma-service/prisma/schema.prisma (oidc_issuer model) @@index([orgId]) @@unique([orgId], map: "oidc_issuer_org_primary_unique", where: "\"isPrimary\" = true")Note: Prisma doesn't natively support partial unique indexes via attributes for all targets; if so, declare it via a raw SQL migration:
CREATE UNIQUE INDEX oidc_issuer_org_primary_unique ON oidc_issuer ("orgId") WHERE "isPrimary" = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts` around lines 248 - 258, The DB must enforce that each org has only one primary issuer: add a partial unique index on oidc_issuer for (orgId) where isPrimary = true so concurrent addOidcIssuerDetails/updateOidcIssuerDetails calls cannot create two primaries; update the Prisma model (oidc_issuer) to add a unique constraint for orgId with a WHERE isPrimary = true if your generator supports it, otherwise create a migration containing the CREATE UNIQUE INDEX oidc_issuer_org_primary_unique ON oidc_issuer ("orgId") WHERE "isPrimary" = true; keep the existing tx.oidc_issuer.updateMany logic but handle unique-constraint errors from the DB in updateOidcIssuerDetails/addOidcIssuerDetails to return a clear failure to the caller.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts`:
- Around line 244-246: Replace the generic throw new Error('Issuer not found')
with a NotFoundException using the project's standardized response message;
specifically, throw new
NotFoundException(ResponseMessages.oidcIssuer.error.notFound) in the same block
where issuer is checked (the issuer variable check in
oid4vc-issuance.repository.ts) so the error becomes a 404 and uses the shared
ResponseMessages format (similar to the getAgentEndPoint usage).
- Around line 222-230: Change hasPrimaryIssuer to accept orgId (not orgAgentId)
and mirror the error-handling pattern used in updateOidcIssuerDetails: wrap the
prisma count call in try/catch, log errors via the same logger, and return false
on error; inside the method use this.prisma.oidc_issuer.count({ where: { orgId,
isPrimary: true } }). Then update the call site in oid4vc-issuance.service
(where hasPrimaryIssuer(agentDetails.id) is invoked) to pass agentDetails.orgId
instead of agentDetails.id so detection and enforcement consistently operate at
the org level.
---
Nitpick comments:
In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts`:
- Around line 260-270: The update to tx.oidc_issuer.update is currently only
scoped by id (issuerId) which could allow cross-tenant writes if checks move;
change the update's where to include both id: issuerId and orgId (orgId) so the
final write is constrained to the tenant you validated earlier (after the
findFirst that checked id and orgId). If your Prisma schema supports a composite
unique ([id, orgId]) use a composite where ({ id_orgId: { id: issuerId, orgId
}}) or equivalent; if not, keep the explicit pre-check (the existing findFirst)
and ensure it cannot be removed in future refactors so the update remains
tenant-scoped.
- Around line 248-258: The DB must enforce that each org has only one primary
issuer: add a partial unique index on oidc_issuer for (orgId) where isPrimary =
true so concurrent addOidcIssuerDetails/updateOidcIssuerDetails calls cannot
create two primaries; update the Prisma model (oidc_issuer) to add a unique
constraint for orgId with a WHERE isPrimary = true if your generator supports
it, otherwise create a migration containing the CREATE UNIQUE INDEX
oidc_issuer_org_primary_unique ON oidc_issuer ("orgId") WHERE "isPrimary" =
true; keep the existing tx.oidc_issuer.updateMany logic but handle
unique-constraint errors from the DB in
updateOidcIssuerDetails/addOidcIssuerDetails to return a clear failure to the
caller.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bb983fa-5ff6-474e-93f3-a5bd7835bdaa
📒 Files selected for processing (1)
apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@libs/prisma-service/prisma/migrations/20260427080006_added_org_id_and_is_primary_in_oidc_issuer/migration.sql`:
- Around line 10-15: Migration adds nullable oidc_issuer.orgId but doesn't
backfill existing rows, causing org-scoped queries (updateOidcIssuerDetails) and
isPrimary enforcement to ignore legacy issuers; fix by adding an UPDATE in this
migration that sets oidc_issuer.orgId = org_agents."orgId" for rows where
oidc_issuer."orgAgentId" = org_agents."id" (use a JOIN on oidc_issuer.orgAgentId
-> org_agents.id), run that update before adding the FOREIGN KEY/SET NOT NULL
step, and optionally include a comment or follow-up migration to SET NOT NULL
once data is verified; refer to tables/columns oidc_issuer.orgAgentId,
org_agents.id, org_agents.orgId, and constraint oidc_issuer_orgId_fkey to locate
the correct place in the migration.
In `@libs/prisma-service/prisma/schema.prisma`:
- Around line 600-605: The schema lacks a DB-level invariant preventing multiple
rows with isPrimary=true per org and also needs an index on orgId for lookups;
update the Prisma model oidc_issuer to include an @@index([orgId]) and then add
a SQL migration that creates a partial unique index enforcing one primary per
org, e.g. run a migration that executes: CREATE UNIQUE INDEX
"oidc_issuer_one_primary_per_org" ON "oidc_issuer" ("orgId") WHERE "isPrimary" =
true; this ensures updateOidcIssuerDetails and findOidcIssuerByOrgId queries are
safe under concurrent writes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29c2547b-3291-4820-b75c-b2cf42f120be
📒 Files selected for processing (3)
apps/oid4vc-issuance/src/oid4vc-issuance.service.tslibs/prisma-service/prisma/migrations/20260427080006_added_org_id_and_is_primary_in_oidc_issuer/migration.sqllibs/prisma-service/prisma/schema.prisma
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/oid4vc-issuance/src/oid4vc-issuance.service.ts
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts`:
- Around line 273-279: Replace the hard-coded BadRequestException message with a
standardized ResponseMessages key: add a new error key (e.g., atLeastOnePrimary:
'At least one primary issuer must exist for the organization') under
ResponseMessages.oidcIssuer.error in the response-messages index, then change
the throw to use ResponseMessages.oidcIssuer.error.atLeastOnePrimary (the code
around isPrimary / primaryCount / issuer.isPrimary should be updated to
reference this new key).
- Around line 237-299: The DB lacks a constraint guaranteeing at most one
primary issuer per org, so concurrent calls to addOidcIssuerDetails or
updateOidcIssuerDetails can create two records with isPrimary=true; add a
partial unique index on the oidc_issuer table (e.g.,
oidc_issuer_one_primary_per_org) that enforces uniqueness of orgId where
isPrimary = true, and deploy this migration so the database will reject any
insert/update that would create a second primary for the same org regardless of
application-side checks.
- Around line 222-235: The call to hasPrimaryIssuer(orgId: string) is being
passed the agent's ID (orgAgentId / agentDetails.id) instead of the organization
ID, causing incorrect results; update the caller in oid4vc-issuance.service.ts
(where hasPrimaryIssuer(...) is invoked) to pass agentDetails.orgId (or
destructure const { id, orgId } = agentDetails and call hasPrimaryIssuer(orgId))
so the DB count queries the orgId column correctly and prevents multiple primary
issuers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adcedeaf-da27-4916-af58-46051508f8ef
📒 Files selected for processing (2)
apps/oid4vc-issuance/src/oid4vc-issuance.repository.tsapps/oid4vc-issuance/src/oid4vc-issuance.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/oid4vc-issuance/src/oid4vc-issuance.service.ts
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts (2)
275-276:⚠️ Potential issue | 🟡 MinorUse
ResponseMessagesfor this 400 too.Line 276 is still hard-coded, so this one response can drift from the rest of the API contract.
🛠️ Proposed fix
- throw new BadRequestException('Cannot unset primary. Please assign another issuer as primary first.'); + throw new BadRequestException(ResponseMessages.oidcIssuer.error.atLeastOnePrimaryRequired);If the key does not exist yet, add it under
ResponseMessages.oidcIssuer.error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts` around lines 275 - 276, Replace the hard-coded 400 message thrown in the branch that checks otherPrimaryExists (the if (0 === otherPrimaryExists) block that currently throws new BadRequestException('Cannot unset primary...')) with a call into the shared ResponseMessages object: throw new BadRequestException(ResponseMessages.oidcIssuer.error.<key>). If the specific key does not yet exist, add a new entry under ResponseMessages.oidcIssuer.error (e.g. cannotUnsetPrimary or similar) containing the same user-facing message so the API uses the canonical ResponseMessages value.
241-292:⚠️ Potential issue | 🟠 MajorBack the single-primary invariant with a DB constraint.
This transaction helps, but it still races under concurrent create/update requests: two writers can each observe a promotable state and commit different rows with
isPrimary = true. If this PR's migration does not already add a filtered unique index onorgIdfor primary issuers, please add one and handle the resulting Prisma error cleanly.🛠️ Suggested DB guard
+CREATE UNIQUE INDEX oidc_issuer_one_primary_per_org + ON oidc_issuer ("orgId") + WHERE "isPrimary" = true;Verify by checking the schema/migration for a partial unique index on
oidc_issuer("orgId")filtered byisPrimary = true. Expected result: one matching index definition.#!/bin/bash set -euo pipefail echo "== oidc_issuer model ==" sed -n '/^model oidc_issuer {/,/^}/p' libs/prisma-service/prisma/schema.prisma echo echo "== candidate migration entries ==" fd migration.sql libs/prisma-service/prisma/migrations -x rg -n \ 'oidc_issuer_one_primary_per_org|CREATE UNIQUE INDEX|oidc_issuer.*orgId|WHERE.*isPrimary.*true' {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts` around lines 241 - 292, Add a DB-level guard and handle its error: create a migration that adds a partial unique index on oidc_issuer(orgId) WHERE isPrimary = true (since Prisma schema lacks partial-index syntax, add raw SQL in the migration to CREATE UNIQUE INDEX ... ON oidc_issuer(orgId) WHERE isPrimary = true), then update the code in apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts (the transaction that reads/updates oidc_issuer and sets isPrimary) to catch Prisma client constraint violations (catch Prisma.PrismaClientKnownRequestError and check error.code === 'P2002' and/or the index name) and translate them into a clean HTTP error (e.g., BadRequestException with a clear message about concurrent primary assignment), leaving the existing transaction logic in place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts`:
- Around line 275-276: Replace the hard-coded 400 message thrown in the branch
that checks otherPrimaryExists (the if (0 === otherPrimaryExists) block that
currently throws new BadRequestException('Cannot unset primary...')) with a call
into the shared ResponseMessages object: throw new
BadRequestException(ResponseMessages.oidcIssuer.error.<key>). If the specific
key does not yet exist, add a new entry under ResponseMessages.oidcIssuer.error
(e.g. cannotUnsetPrimary or similar) containing the same user-facing message so
the API uses the canonical ResponseMessages value.
- Around line 241-292: Add a DB-level guard and handle its error: create a
migration that adds a partial unique index on oidc_issuer(orgId) WHERE isPrimary
= true (since Prisma schema lacks partial-index syntax, add raw SQL in the
migration to CREATE UNIQUE INDEX ... ON oidc_issuer(orgId) WHERE isPrimary =
true), then update the code in
apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts (the transaction that
reads/updates oidc_issuer and sets isPrimary) to catch Prisma client constraint
violations (catch Prisma.PrismaClientKnownRequestError and check error.code ===
'P2002' and/or the index name) and translate them into a clean HTTP error (e.g.,
BadRequestException with a clear message about concurrent primary assignment),
leaving the existing transaction logic in place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 122fbeb2-b4fd-47e6-a507-f2390951cd25
📒 Files selected for processing (1)
apps/oid4vc-issuance/src/oid4vc-issuance.repository.ts
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
shitrerohit
approved these changes
Apr 28, 2026
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
|
This was referenced May 4, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Summary by CodeRabbit
New Features
Bug Fixes
API