Skip to content

fix: improve member identity resolve and create api#4117

Open
skwowet wants to merge 3 commits into
mainfrom
hotfix-public-member-identity-endpoints
Open

fix: improve member identity resolve and create api#4117
skwowet wants to merge 3 commits into
mainfrom
hotfix-public-member-identity-endpoints

Conversation

@skwowet
Copy link
Copy Markdown
Collaborator

@skwowet skwowet commented May 14, 2026

What

Fixes two member identity conflict cases in the public API:

  • Member resolution now only matches verified LFID and email identities.
  • Creating a member identity is now idempotent when the same identity already exists on the same member, and verifies existing same-value identities for that member.

Why

Previously, /v1/members/resolve could 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/identities returned 409 Identity already exists on this member when 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=true for LFID and email identities passed to POST /v1/members/resolve, reducing false conflicts from unverified matches.

Makes POST /v1/members/:memberId/identities idempotent for same-member duplicates by normalizing identity values (trim/lowercase), returning 200 when 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, and findMemberIdentitiesByValue now 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.

Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
@skwowet skwowet requested a review from joanagmaia May 14, 2026 17:09
@skwowet skwowet self-assigned this May 14, 2026
Copilot AI review requested due to automatic review settings May 14, 2026 17:09
@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

  • feat: add user authentication (CM-123)
  • feat: add user authentication (IN-123)

Projects:

  • CM: Community Data Platform
  • IN: Insights

Please add a Jira issue key to your PR title.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  • resolveMemberByIdentities now restricts LFID and email matching to verified identities.
  • createMemberIdentity becomes idempotent: it short-circuits when the exact (platform, value, type) already exists on the member and returns 200; otherwise it inserts and, if verified=true, also flips matching same-member identities to verified.
  • findMemberIdentitiesByValue widens its projection to SELECT * 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 by findMemberIdentitiesByValue to verified=true — not just the exact (platform, value, type) match. Since existing is filtered only by memberId, 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 on data.platform. Consider scoping the update to exactMatch only (or to identities whose platform matches data.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.all updates run outside the try/catch that handles uix_memberIdentities_platform_value_type_verified violations. If one of the existing identities being flipped to verified=true collides 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 409 ConflictError returned 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 exactMatch is found and data.verified is true but exactMatch.verified was 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 stored verifiedBy value (and won't reflect the new updatedAt from the DB). Consider only updating when a state change is actually needed, and either re-fetching the row or preserving the prior verifiedBy when 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.

Comment thread backend/src/api/public/v1/members/identities/createMemberIdentity.ts Outdated
Comment thread services/libs/data-access-layer/src/members/identities.ts
Comment thread backend/src/api/public/v1/members/identities/createMemberIdentity.ts Outdated
skwowet added 2 commits May 16, 2026 20:16
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ 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 *
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c0dfc54. Configure here.

}

throw error
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c0dfc54. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants