Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 76 additions & 37 deletions backend/src/api/public/v1/members/identities/createMemberIdentity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import { ConflictError, NotFoundError } from '@crowd/common'
import {
MemberField,
findMemberById,
findMemberIdentitiesByValue,
createMemberIdentity as insertMemberIdentity,
optionsQx,
touchMemberUpdatedAt,
updateMemberIdentity,
} from '@crowd/data-access-layer'
import { IMemberIdentity, MemberIdentityType } from '@crowd/types'

import { created } from '@/utils/api'
import { created, ok } from '@/utils/api'
import { validateOrThrow } from '@/utils/validation'

const paramsSchema = z.object({
Expand All @@ -33,72 +35,103 @@ const bodySchema = z
path: ['verifiedBy'],
})

type DbConstraintError = Error & {
constraint?: string
original?: { constraint?: string }
parent?: { constraint?: string }
}

function throwIdentityConflict(
error: DbConstraintError,
data: { platform: string; value: string; type: MemberIdentityType },
): never {
const constraint = error.constraint ?? error.original?.constraint ?? error.parent?.constraint

if (constraint === 'uix_memberIdentities_platform_value_type_verified') {
throw new ConflictError('Identity already verified on another member', data)
}

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The old constraint handler for uix_memberIdentities_memberId_platform_value_type was the bug — it was throwing a 409 when the same identity already existed on the same member. The new approach is intentionally idempotent: we look up existing same-value identities first and verify them rather than throwing a conflict, per the product requirement.


export async function createMemberIdentity(req: Request, res: Response): Promise<void> {
const { memberId } = validateOrThrow(paramsSchema, req.params)
const data = validateOrThrow(bodySchema, req.body)

const qx = optionsQx(req)

const member = await findMemberById(qx, memberId, [MemberField.ID])
if (!member) {
throw new NotFoundError('Member not found')
}

// The data-sink writes identity values as trimmed lowercase, so normalize here
// to keep idempotency checks reliable against existing rows.
const normalizedValue = data.value.trim().toLowerCase()
const conflictContext = { platform: data.platform, value: normalizedValue, type: data.type }

let result!: IMemberIdentity
let alreadyExisted = false

await captureApiChange(
req,
memberEditIdentitiesAction(memberId, async (captureOldState, captureNewState) => {
captureOldState({})

await qx.tx(async (tx) => {
const existing = await findMemberIdentitiesByValue(tx, memberId, normalizedValue, {
type: data.type,
})
const exactMatch = existing.find((i) => i.platform === data.platform)

try {
result = await insertMemberIdentity(
tx,
{
memberId,
platform: data.platform,
value: data.value,
type: data.type,
source: data.source,
verified: data.verified,
verifiedBy: data.verifiedBy,
},
true,
true,
)
} catch (error) {
const constraint =
error.constraint ?? error.original?.constraint ?? error.parent?.constraint

if (constraint === 'uix_memberIdentities_memberId_platform_value_type') {
throw new ConflictError('Identity already exists on this member', {
platform: data.platform,
value: data.value,
type: data.type,
})
if (exactMatch) {
alreadyExisted = true
result = exactMatch
} else {
result = await insertMemberIdentity(
tx,
{
memberId,
platform: data.platform,
value: normalizedValue,
type: data.type,
source: data.source,
verified: data.verified,
verifiedBy: data.verifiedBy,
},
true,
true,
)
}

if (constraint === 'uix_memberIdentities_platform_value_type_verified') {
throw new ConflictError('Identity already verified on another member', {
platform: data.platform,
value: data.value,
type: data.type,
})
// A verified identity confirms the same value for this member, so keep same-value
// identities in sync instead of leaving stale unverified duplicates behind.
if (data.verified && existing.length > 0) {
const updatedResults: IMemberIdentity[] = []
for (const identity of existing) {
const updated = await updateMemberIdentity(tx, memberId, identity.id, {
verified: true,
verifiedBy: data.verifiedBy,
})
if (updated) updatedResults.push(updated)
}

if (alreadyExisted) {
result = updatedResults.find((r) => r.id === exactMatch.id) ?? result
}
}
Comment thread
cursor[bot] marked this conversation as resolved.

throw error
} catch (error) {
throwIdentityConflict(error as DbConstraintError, conflictContext)
}

// touch member updated at to trigger merge suggestion
await touchMemberUpdatedAt(tx, memberId)
})

captureNewState(result)
}),
)

created(res, {
const response = {
id: result.id,
value: result.value,
platform: result.platform,
Expand All @@ -108,5 +141,11 @@ export async function createMemberIdentity(req: Request, res: Response): Promise
source: result.source ?? null,
createdAt: result.createdAt,
updatedAt: result.updatedAt,
})
}

if (alreadyExisted) {
ok(res, response)
} else {
created(res, response)
}
}
7 changes: 6 additions & 1 deletion backend/src/api/public/v1/members/resolveMember.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@ export async function resolveMemberByIdentities(req: Request, res: Response): Pr
platform: PlatformType.LFID,
type: MemberIdentityType.USERNAME,
value: lfid,
verified: true,
})),
...(emails?.map((email) => ({ type: MemberIdentityType.EMAIL, value: email })) ?? []),
...(emails?.map((email) => ({
type: MemberIdentityType.EMAIL,
value: email,
verified: true,
})) ?? []),
]

const memberIds = await findMemberIdsByIdentities(qx, identities)
Expand Down
2 changes: 1 addition & 1 deletion services/libs/data-access-layer/src/members/identities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export async function findMemberIdentitiesByValue(
): Promise<IMemberIdentity[]> {
return qx.select(
`
SELECT id, platform, "sourceId", type, value, verified
SELECT *
Comment thread
skwowet marked this conversation as resolved.
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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not a concern in practice — the data-sink has always written identity values as trimmed lowercase, so mixed-case rows from this API don't exist. The normalization added in this PR covers new writes going forward.

FROM "memberIdentities"
WHERE value = $(value)
AND "memberId" = $(memberId)
Expand Down
Loading