fix: improve member identity resolve and create api#4117
Conversation
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
There was a problem hiding this comment.
Pull request overview
Fixes two correctness issues in the public member identity APIs: /v1/members/resolve was treating unverified email identities as resolvable matches, and POST /v1/members/:memberId/identities was returning 409 when the same identity already existed on the same target member.
Changes:
resolveMemberByIdentitiesnow restricts LFID and email matching to verified identities.createMemberIdentitybecomes idempotent: it short-circuits when the exact(platform, value, type)already exists on the member and returns 200; otherwise it inserts and, ifverified=true, also flips matching same-member identities to verified.findMemberIdentitiesByValuewidens its projection toSELECT *to expose more columns to the new caller.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/src/api/public/v1/members/resolveMember.ts | Adds verified: true to both LFID and email identity tuples used for resolution. |
| backend/src/api/public/v1/members/identities/createMemberIdentity.ts | Adds idempotent same-member handling, bulk-verifies matching identities when verified=true, and returns 200 vs 201 accordingly. |
| services/libs/data-access-layer/src/members/identities.ts | Changes findMemberIdentitiesByValue projection from explicit columns to SELECT *. |
Comments suppressed due to low confidence (3)
backend/src/api/public/v1/members/identities/createMemberIdentity.ts:107
- The
if (data.verified && existing.length > 0)branch updates every identity returned byfindMemberIdentitiesByValueto verified=true — not just the exact(platform, value, type)match. Sinceexistingis filtered only bymemberId,value, and (optionally)type, any other-platform identity on the same member with the same value/type will be force-verified as a side effect of this API call, even though the caller only asked to create/verify an identity ondata.platform. Consider scoping the update toexactMatchonly (or to identities whoseplatformmatchesdata.platform).
if (data.verified && existing.length > 0) {
await Promise.all(
existing.map((i) =>
updateMemberIdentity(tx, memberId, i.id, {
verified: true,
verifiedBy: data.verifiedBy,
}),
),
)
backend/src/api/public/v1/members/identities/createMemberIdentity.ts:107
- The
Promise.allupdates run outside thetry/catchthat handlesuix_memberIdentities_platform_value_type_verifiedviolations. If one of the existing identities being flipped toverified=truecollides with another member who has the same(platform, value, type)already verified, the unique-index error will bubble up as a raw 500/InternalError instead of the clean 409ConflictErrorreturned by the insert path. Consider wrapping these updates in the same constraint-aware error handling.
if (data.verified && existing.length > 0) {
await Promise.all(
existing.map((i) =>
updateMemberIdentity(tx, memberId, i.id, {
verified: true,
verifiedBy: data.verifiedBy,
}),
),
)
backend/src/api/public/v1/members/identities/createMemberIdentity.ts:116
- When
exactMatchis found anddata.verifiedis true butexactMatch.verifiedwas already true, the code still issues an UPDATE for it (and possibly other matching rows). More importantly, the result is rebuilt as{ ...exactMatch, verified: true, verifiedBy: data.verifiedBy }, which overwrites the previously storedverifiedByvalue (and won't reflect the newupdatedAtfrom the DB). Consider only updating when a state change is actually needed, and either re-fetching the row or preserving the priorverifiedBywhen the identity was already verified.
if (data.verified && existing.length > 0) {
await Promise.all(
existing.map((i) =>
updateMemberIdentity(tx, memberId, i.id, {
verified: true,
verifiedBy: data.verifiedBy,
}),
),
)
if (alreadyExisted) {
result = {
...exactMatch,
verified: true,
verifiedBy: data.verifiedBy,
}
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c0dfc54. Configure here.
| return qx.select( | ||
| ` | ||
| SELECT id, platform, "sourceId", type, value, verified | ||
| SELECT * |
There was a problem hiding this comment.
Case-sensitive query misses legacy mixed-case identity rows
Medium Severity
The findMemberIdentitiesByValue query uses a case-sensitive comparison (WHERE value = $(value)) while the caller passes a lowercased normalizedValue. Identities created through the old version of this API (which stored data.value without lowercasing) will have mixed-case values that this query won't match. Since the unique index uix_memberIdentities_memberId_platform_value_type is also case-sensitive, the insert of the lowercased value succeeds without constraint violation, silently creating a semantic duplicate on the same member and breaking the idempotency guarantee. The query needs lower(value) = $(value) to detect all existing rows regardless of historical casing.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c0dfc54. Configure here.
| } | ||
|
|
||
| throw error | ||
| } |
There was a problem hiding this comment.
Missing constraint handler causes 500 on concurrent requests
Medium Severity
The throwIdentityConflict function only handles uix_memberIdentities_platform_value_type_verified, but the old code also handled uix_memberIdentities_memberId_platform_value_type. The pre-check with findMemberIdentitiesByValue doesn't fully prevent this constraint from firing under concurrent requests — two transactions can both pass the SELECT check then race on the INSERT. When this happens, the raw DB error is re-thrown, resulting in a 500 instead of the previous clean 409 response. For an endpoint designed for idempotent retries, concurrent duplicates are a realistic scenario.
Reviewed by Cursor Bugbot for commit c0dfc54. Configure here.


What
Fixes two member identity conflict cases in the public API:
Why
Previously,
/v1/members/resolvecould return a conflict when an unverified email identity matched the request, even if the verified LFID/email clearly pointed to one member.Also,
POST /v1/members/:memberId/identitiesreturned409 Identity already exists on this memberwhen the requested identity was already present on that same member. That made retry/update flows noisy even though there was no real cross-member conflict.Note
Medium Risk
Changes public API behavior for identity resolution and creation (verified-only matching, idempotent create, and cascading verification updates), which could affect existing client flows and data consistency if assumptions differ.
Overview
Tightens member resolution by requiring
verified=truefor LFID and email identities passed toPOST /v1/members/resolve, reducing false conflicts from unverified matches.Makes
POST /v1/members/:memberId/identitiesidempotent for same-member duplicates by normalizing identity values (trim/lowercase), returning200when the identity already exists, and propagating verification to other same-value identities on the member. Conflict handling is narrowed to the cross-member verified-uniqueness constraint, andfindMemberIdentitiesByValuenow returns full rows (SELECT *) to support the new update flow.Reviewed by Cursor Bugbot for commit c0dfc54. Bugbot is set up for automated code reviews on this repo. Configure here.