diff --git a/src/app/accounts/assign-npub.ts b/src/app/accounts/assign-npub.ts new file mode 100644 index 000000000..9a030c1ef --- /dev/null +++ b/src/app/accounts/assign-npub.ts @@ -0,0 +1,75 @@ +import { checkedToAccountId } from "@domain/accounts" +import { DuplicateKeyForPersistError } from "@domain/errors" +import { checkedToNpub, NpubNotAvailableError } from "@domain/nostr" +import { baseLogger } from "@services/logger" +import { AccountsRepository } from "@services/mongoose" + +/** + * The other half of `releaseNpub`'s reassignment, reachable on its own. + * + * `releaseNpub` is two writes and no transaction: the unset lands, then the + * claim. When the claim fails — mongo hiccups, or the target linked a key in + * the window — the key is unclaimed and the account it was taken off no longer + * holds it, so `accountReleaseNpub` cannot be re-run against that account: it + * returns `NoNpubToReleaseError`. Before this existed the only remaining npub + * write was `userUpdateNpub`, which is self-service and subject to the very + * race the reassignment exists to win — the squatter is the party polling + * `isFlashNpub`, so the desk's only advice was to pit a human against a script. + * This makes the second write retryable by an admin. + * + * Safety comes from the same two mechanisms the reassignment already relies on, + * not from new ones: the unique partial index refuses a key another account + * holds (surfaced as `NpubNotAvailableError`), and `claimNpub`'s write-time + * `$not: { $type: "string" }` filter refuses a target that already holds a key + * (`AccountAlreadyHasNpubError`) rather than silently overwriting it. So this + * can only ever fill a hole; it cannot take a key from anyone. + * + * `assignedByUserId` is the attribution trail, for the reason spelled out in + * `release-npub.ts`: the admin server never assigns `req.gqlContext`, so these + * structured lines are the only record of who moved a key. Refusals are logged + * too — a stolen admin token sweeping ids should not be able to probe silently. + */ +export const assignNpub = async ({ + id, + npub, + assignedByUserId, +}: { + id: string + npub: string + assignedByUserId: UserId +}): Promise => { + const idChecked = checkedToAccountId(id) + if (idChecked instanceof Error) return idChecked + + const npubChecked = checkedToNpub(npub) + if (npubChecked instanceof Error) return npubChecked + + const accountsRepo = AccountsRepository() + + const claimed = await accountsRepo.claimNpub(idChecked, npubChecked) + if (claimed instanceof Error) { + baseLogger.error( + { + accountId: idChecked, + npub: npubChecked, + assignedByUserId, + reason: claimed.name, + error: claimed.message, + }, + "admin npub assignment refused", + ) + // Same mapping the reassignment path uses, so one cause reads identically + // wherever it surfaces: the key belongs to someone else. + if (claimed instanceof DuplicateKeyForPersistError) { + return new NpubNotAvailableError(npubChecked) + } + return claimed + } + + baseLogger.info( + { accountId: idChecked, npub: npubChecked, assignedByUserId }, + "admin assigned an npub claim", + ) + + return claimed +} diff --git a/src/app/accounts/find-by-npub.ts b/src/app/accounts/find-by-npub.ts new file mode 100644 index 000000000..92364c1b5 --- /dev/null +++ b/src/app/accounts/find-by-npub.ts @@ -0,0 +1,21 @@ +import { checkedToNpub } from "@domain/nostr" +import { AccountsRepository } from "@services/mongoose" + +/** + * Also re-exported as `Admin.getAccountByNpub` — one implementation, two + * barrels, so fixes cannot drift between them. Normalised because the + * repository query is a plain `$eq` with no collation, so a caller that is not + * the GraphQL boundary (a script, a backfill, a REST shim) passing a mixed-case + * npub gets a silent not-found on a real user — which the public + * `isFlashNpub` query reports as `isFlashNpub: false`. + * + * Lives outside `index.ts` so it can be unit tested against a mocked + * repository — importing the accounts barrel drags in the notification stack. + */ +export const findByNpub = async (npub: Npub): Promise => { + const npubChecked = checkedToNpub(npub) + if (npubChecked instanceof Error) return npubChecked + + const accounts = AccountsRepository() + return accounts.findByNpub(npubChecked) +} diff --git a/src/app/accounts/index.ts b/src/app/accounts/index.ts index f29ff4e21..145b32609 100644 --- a/src/app/accounts/index.ts +++ b/src/app/accounts/index.ts @@ -33,6 +33,9 @@ export * from "./enable-notification-category" export * from "./enable-notification-channel" export * from "./disable-notification-channel" export * from "./set-npub" +export * from "./release-npub" +export * from "./assign-npub" +export * from "./find-by-npub" export * from "./update-external-wallet" const accounts = AccountsRepository() @@ -43,10 +46,6 @@ export const getAccount = async ( return accounts.findById(accountId) } -export const findByNpub = async (npub: Npub): Promise => { - return accounts.findByNpub(npub) -} - export const getAccountFromUserId = async ( kratosUserId: UserId, ): Promise => { diff --git a/src/app/accounts/release-npub.ts b/src/app/accounts/release-npub.ts new file mode 100644 index 000000000..ab3c1ca5c --- /dev/null +++ b/src/app/accounts/release-npub.ts @@ -0,0 +1,199 @@ +import { checkedToAccountId } from "@domain/accounts" +import { + CouldNotFindAccountFromIdError, + CouldNotFindError, + DuplicateKeyForPersistError, +} from "@domain/errors" +import { AccountAlreadyHasNpubError, NpubNotAvailableError } from "@domain/nostr" +import { baseLogger } from "@services/logger" +import { AccountsRepository } from "@services/mongoose" + +export type NpubRelease = { + account: Account + previousNpub: Npub + reassignedTo?: Account + // Set when the release landed and the claim meant to follow it did not. This + // is not an alternative to the release: the key is off the holder either way, + // and re-running the mutation now fails with `NoNpubToReleaseError`. + reassignmentError?: ApplicationError +} + +/** + * The escape hatch for `setNpub`'s refusal. `userUpdateNpub` takes a bare npub + * from any authenticated account with no proof of key control, so anyone can + * read a victim's npub off a public relay and claim it first. The unique index + * then makes that permanent: the rightful owner gets `NpubNotAvailableError` + * forever, and the support desk resolves their DMs to the squatter's contact + * card. Support needs to be able to free the key from the admin panel rather + * than hand-writing an `$unset` against prod mongo. + * + * A bare release loses the race it exists to win: the squatter is the party + * polling `isFlashNpub`, so telling the victim to go re-link pits a human + * against a script. `reassignToAccountId` closes that by handing the key + * straight to the rightful owner. This repository has no MongoDB sessions + * anywhere, so the two writes are not a transaction: the key is unclaimed for + * the round-trip between them, and a claim that lands in that window makes the + * reassignment fail with `NpubNotAvailableError`. The release still stands, so + * that failure comes back as `reassignmentError` on an otherwise populated + * `NpubRelease` rather than as a bare error — a bare error reads as "nothing + * happened", and the operator would neither know the key is now unclaimed nor + * that recovering it means finding its current holder with + * `accountDetailsByNpub` and releasing it from there. The target is read and + * checked before the release so that everything knowable up front fails before + * the key is freed; the unique partial index guarantees the reassignment + * cannot collide with a concurrent claim of the same key, and `claimNpub`'s + * own write-time filter refuses a target that linked a different key after the + * pre-release check — surfaced as `AccountAlreadyHasNpubError` in + * `reassignmentError`, never a silent overwrite of the target's key. + * + * `releasedByUserId` is the whole attribution trail. Neither the account + * document nor the payload retains the npub that was removed, and the admin + * server never assigns `req.gqlContext`, so the Pino request log records the + * actor as undefined — the structured log lines below are the only record that + * a given admin took a given key off a given account. Refusals are logged for + * the same reason: a stolen admin token sweeping account ids leaves one line + * for the release that worked and, without them, nothing at all for the probes + * that did not. + */ +export const releaseNpub = async ({ + id, + releasedByUserId, + reassignToAccountId, +}: { + id: string + releasedByUserId: UserId + reassignToAccountId?: string +}): Promise => { + const accountsRepo = AccountsRepository() + + const refuse = (reason: string, error: E): E => { + baseLogger.warn( + { accountId: id, releasedByUserId, reassignToAccountId, reason }, + "admin npub release refused", + ) + return error + } + + const idChecked = checkedToAccountId(id) + if (idChecked instanceof Error) return refuse("malformed account id", idChecked) + + const targetIdChecked = + reassignToAccountId === undefined + ? undefined + : checkedToAccountId(reassignToAccountId) + if (targetIdChecked instanceof Error) { + return refuse("malformed reassignment target id", targetIdChecked) + } + + let target: Account | undefined + if (targetIdChecked !== undefined) { + const found = await accountsRepo.findById(targetIdChecked) + if (found instanceof CouldNotFindError) { + return refuse( + "unknown reassignment target", + new CouldNotFindAccountFromIdError(targetIdChecked), + ) + } + if (found instanceof Error) return found + // Also catches `reassignToAccountId === id`, where the target is the holder + // and there is nothing to move. + // + // `typeof` rather than `!== undefined`: the field is `Npub | null` on the + // record and the migration deliberately leaves pre-existing `npub: null` + // documents alone, so an account that has never linked a key can arrive + // here holding an explicit null. That is not a claim, and treating it as + // one would make such an account permanently ineligible to receive one. + if (typeof found.npub === "string") { + return refuse( + "reassignment target already holds an npub", + new AccountAlreadyHasNpubError(targetIdChecked), + ) + } + target = found + } + + // The holder's existence and its npub are both established by `unsetNpub` + // off the pre-update document, so this branch carries the refusals that a + // separate holder read used to make here — plus genuine write failures. + const released = await accountsRepo.unsetNpub(idChecked) + if (released instanceof Error) return refuse(released.name, released) + + const { account, previousNpub } = released + + baseLogger.info( + { + accountId: idChecked, + previousNpub, + releasedByUserId, + // Intent, not outcome. The claim has not been attempted yet and can still + // lose to a concurrent one; an investigator reading this line must not + // conclude the key reached the target. + reassignToAccountId: targetIdChecked, + }, + "admin released an npub claim", + ) + + if (target === undefined) return { account, previousNpub } + + const reassigned = await accountsRepo.claimNpub(target.id, previousNpub) + if (reassigned instanceof Error) { + baseLogger.error( + { + accountId: idChecked, + previousNpub, + reassignToAccountId: targetIdChecked, + releasedByUserId, + // Which failure, not just that one happened. Three distinct causes + // reach this branch and each has a different recovery, so a line that + // does not name the cause hands the operator all three procedures at + // once. + // + // What is true for ALL of them: the release already landed, so this + // account no longer holds the key and `accountReleaseNpub` cannot be + // re-run against it — it answers `NoNpubToReleaseError`. Recovery is + // `accountAssignNpub(accountId, npub)`, which is the second write on + // its own and carries the same two guards (unique index, write-time + // refusal of a target that already holds a key). + // + // `DuplicateKeyForPersistError` (surfaced to the caller as + // `NpubNotAvailableError`): someone else claimed the key in the window + // between the two writes — it is gone, so assignment will refuse too; + // find the new holder with `accountDetailsByNpub` and release it from + // there first. + // `AccountAlreadyHasNpubError`: the target linked a different key + // after the pre-release check — the released key is still unclaimed, + // so assign it to the right account before anyone polling + // `isFlashNpub` takes it. + // Anything else (e.g. `UnknownRepositoryError`): the write failed and + // the key is unclaimed — assign it to the intended target. + // + // Taken off the raw repository error, before the + // `DuplicateKeyForPersistError` → `NpubNotAvailableError` mapping + // below, so the line names the failure that actually happened. + reason: reassigned.name, + reassignmentError: reassigned.message, + }, + "npub released but reassignment failed", + ) + return { + account, + previousNpub, + reassignmentError: + reassigned instanceof DuplicateKeyForPersistError + ? new NpubNotAvailableError(previousNpub) + : reassigned, + } + } + + baseLogger.info( + { + accountId: idChecked, + previousNpub, + releasedByUserId, + reassignedToAccountId: target.id, + }, + "admin reassigned a released npub", + ) + + return { account, previousNpub, reassignedTo: reassigned } +} diff --git a/src/app/accounts/set-npub.ts b/src/app/accounts/set-npub.ts index e3b0a0203..68cadc3f5 100644 --- a/src/app/accounts/set-npub.ts +++ b/src/app/accounts/set-npub.ts @@ -1,7 +1,6 @@ +import { checkedToNpub, NpubNotAvailableError } from "@domain/nostr" +import { CouldNotFindError, DuplicateKeyForPersistError } from "@domain/errors" import { AccountsRepository } from "@services/mongoose" -import { checkValidNpub } from "@domain/nostr" - -import { ValidationError } from "ajv" export const setNpub = async ({ id, @@ -11,10 +10,35 @@ export const setNpub = async ({ npub: Npub }): Promise => { const accountsRepo = AccountsRepository() - if (!checkValidNpub(npub)) - throw new ValidationError([{ message: "Invalid npub format" }]) + + const npubChecked = checkedToNpub(npub) + if (npubChecked instanceof Error) return npubChecked + + // An npub is an identity, and since this PR it is also a support-desk + // lookup key: `findByNpub` is a `findOne`, so two accounts claiming the same + // npub would resolve nondeterministically and could paint one customer's + // phone/email/level onto another's contact card. Refuse the claim here, and + // let the unique index on `accounts.npub` catch the concurrent-write race. + const existing = await accountsRepo.findByNpub(npubChecked) + if (!(existing instanceof Error)) { + if (existing.id !== id) return new NpubNotAvailableError(npubChecked) + return existing + } + if (!(existing instanceof CouldNotFindError)) return existing + const account = await accountsRepo.findById(id) if (account instanceof Error) return account - account.npub = npub - return accountsRepo.update(account) + account.npub = npubChecked + + const updated = await accountsRepo.update(account) + // The probe above and this write are not atomic, so two concurrent claims on + // the same npub can both pass it. The loser's write trips the unique index, + // which `parseRepositoryError` surfaces as `DuplicateKeyForPersistError` — + // an error `error-map` buckets into `UnexpectedClientError` ("please contact + // support"). It is not unexpected: it is the same refusal as the probe, and + // the caller deserves the same answer. + if (updated instanceof DuplicateKeyForPersistError) { + return new NpubNotAvailableError(npubChecked) + } + return updated } diff --git a/src/app/admin/get-account-by-npub.ts b/src/app/admin/get-account-by-npub.ts new file mode 100644 index 000000000..8961d7fac --- /dev/null +++ b/src/app/admin/get-account-by-npub.ts @@ -0,0 +1,8 @@ +/** + * `Admin.getAccountByNpub` is `Accounts.findByNpub` under the admin barrel's + * name — same validation, same normalisation rationale, same repository call. + * A re-export rather than a copy so a fix (e.g. a bech32 validation upgrade) + * cannot land in one and not the other. The admin unit spec stays pointed at + * this module so registration coverage of the re-export survives. + */ +export { findByNpub as getAccountByNpub } from "@app/accounts/find-by-npub" diff --git a/src/app/admin/index.ts b/src/app/admin/index.ts index b01b57848..fdfc0d75f 100644 --- a/src/app/admin/index.ts +++ b/src/app/admin/index.ts @@ -4,6 +4,7 @@ export * from "./update-user-phone" export * from "./send-cashout-notification" export * from "./send-user-notification" export * from "./invite" +export * from "./get-account-by-npub" // Re-export query functions from invite module for admin GraphQL compatibility export { getInviteById, listInvites } from "../invite/queries" diff --git a/src/app/errors.ts b/src/app/errors.ts index 687b49fe7..1fabd9921 100644 --- a/src/app/errors.ts +++ b/src/app/errors.ts @@ -19,6 +19,7 @@ import * as PubSubErrors from "@domain/pubsub/errors" import * as CaptchaErrors from "@domain/captcha/errors" import * as AuthenticationErrors from "@domain/authentication/errors" import * as UserErrors from "@domain/users/errors" +import * as NostrErrors from "@domain/nostr/errors" import * as CashWalletCutoverErrors from "@app/cash-wallet-cutover/errors" import * as LedgerFacadeErrors from "@services/ledger/domain/errors" @@ -54,6 +55,7 @@ export const ApplicationErrors = { ...CaptchaErrors, ...AuthenticationErrors, ...UserErrors, + ...NostrErrors, ...CashWalletCutoverErrors, ...KratosErrors, diff --git a/src/domain/accounts/index.types.d.ts b/src/domain/accounts/index.types.d.ts index 0017d6e19..6de63af7f 100644 --- a/src/domain/accounts/index.types.d.ts +++ b/src/domain/accounts/index.types.d.ts @@ -80,7 +80,9 @@ type Account = { readonly uuid: AccountUuid readonly createdAt: Date username: Username - npub: Npub + // Optional: an account holds an npub only once it links one, and `releaseNpub` + // takes it back off. + npub?: Npub defaultWalletId: WalletId withdrawFee: Satoshis // TODO: make it optional. only save when not default value from yaml level: AccountLevel @@ -185,6 +187,15 @@ type AccountValidator = { validateWalletForAccount(wallet: Wallet): true | ValidationError } +// `unsetNpub` reads the pre-update document to decide whether anything was +// actually freed, so it is the only place the removed npub still exists — the +// updated document no longer carries it. Handing it back saves the caller a +// second read that could disagree with what the `$unset` removed. +type NpubUnset = { + account: Account + previousNpub: Npub +} + interface IAccountsRepository { listUnlockedAccounts(): AsyncGenerator | RepositoryError findById(accountId: AccountId): Promise @@ -196,6 +207,15 @@ interface IAccountsRepository { findByUsername(username: Username): Promise // listBusinessesForMap(): Promise findByNpub(npub: Npub): Promise + unsetNpub(accountId: AccountId): Promise + // `AccountAlreadyHasNpubError` in the union is the write-time guard: the + // caller's "target holds no npub" check is a read from before the release, + // and a key the target claims in that window must refuse the reassignment + // rather than be silently overwritten. + claimNpub( + accountId: AccountId, + npub: Npub, + ): Promise update(account: Account): Promise transitionBridgeKycStatus( diff --git a/src/domain/errors.ts b/src/domain/errors.ts index c350acf5c..b0257d407 100644 --- a/src/domain/errors.ts +++ b/src/domain/errors.ts @@ -61,8 +61,14 @@ export class CouldNotFindLnPaymentFromHashError extends CouldNotFindError { level = ErrorLevel.Critical } +export class CouldNotFindAccountFromIdError extends CouldNotFindError {} export class CouldNotFindAccountFromUuidError extends CouldNotFindError {} export class CouldNotFindAccountFromUsernameError extends CouldNotFindError {} +export class CouldNotFindAccountFromNpubError extends CouldNotFindError {} +// An account that holds no npub has nothing to release. Distinct from +// "account not found" because `$unset` on a document without the field is a +// no-op that still matches, so the write alone cannot tell the two apart. +export class NoNpubToReleaseError extends CouldNotFindError {} export class CouldNotFindAccountFromPhoneError extends CouldNotFindError {} export class CouldNotFindMerchantFromUsernameError extends CouldNotFindError {} export class CouldNotFindMerchantFromIdError extends CouldNotFindError {} diff --git a/src/domain/nostr/errors.ts b/src/domain/nostr/errors.ts new file mode 100644 index 000000000..33521c912 --- /dev/null +++ b/src/domain/nostr/errors.ts @@ -0,0 +1,9 @@ +import { DomainError, ValidationError } from "@domain/shared" + +export class NostrError extends DomainError {} + +export class InvalidNpubError extends ValidationError {} + +export class NpubNotAvailableError extends NostrError {} + +export class AccountAlreadyHasNpubError extends NostrError {} diff --git a/src/domain/nostr/index.ts b/src/domain/nostr/index.ts index 3433b79b7..1f503be58 100644 --- a/src/domain/nostr/index.ts +++ b/src/domain/nostr/index.ts @@ -1,3 +1,16 @@ +import { InvalidNpubError } from "./errors" + +export * from "./errors" + export const checkValidNpub = (npub: string): boolean => { return npub.startsWith("npub1") && npub.length === 63 } + +// Mirrors `checkedToUsername` / `checkedToAccountUuid`: the branded `Npub` type +// only means something if there is a single place that mints it. Callers that +// are not the GraphQL boundary (scripts, backfills, REST shims) get a +// validation error instead of a silent not-found on a malformed value. +export const checkedToNpub = (npub: string): Npub | ValidationError => { + if (!checkValidNpub(npub)) return new InvalidNpubError(npub) + return npub.toLowerCase() as Npub +} diff --git a/src/domain/nostr/index.types.d.ts b/src/domain/nostr/index.types.d.ts index cbc34a598..4ca131adc 100644 --- a/src/domain/nostr/index.types.d.ts +++ b/src/domain/nostr/index.types.d.ts @@ -1 +1,3 @@ type Npub = `npub1${string}` + +type AccountAlreadyHasNpubError = import("./errors").AccountAlreadyHasNpubError diff --git a/src/graphql/admin/mutations.ts b/src/graphql/admin/mutations.ts index e20b3a4c8..e963753a1 100644 --- a/src/graphql/admin/mutations.ts +++ b/src/graphql/admin/mutations.ts @@ -2,6 +2,8 @@ import { GT } from "@graphql/index" import AccountUpdateLevelMutation from "@graphql/admin/root/mutation/account-update-level" import AccountUpdateStatusMutation from "@graphql/admin/root/mutation/account-update-status" +import AccountAssignNpubMutation from "@graphql/admin/root/mutation/account-assign-npub" +import AccountReleaseNpubMutation from "@graphql/admin/root/mutation/account-release-npub" import BusinessUpdateMapInfoMutation from "@graphql/admin/root/mutation/business-update-map-info" import CashWalletCutoverUpdateMutation from "@graphql/admin/root/mutation/cash-wallet-cutover-update" import CashWalletCutoverRollbackMutation from "@graphql/admin/root/mutation/cash-wallet-cutover-rollback" @@ -21,6 +23,8 @@ export const mutationFields = { userUpdatePhone: UserUpdatePhoneMutation, accountUpdateLevel: AccountUpdateLevelMutation, accountUpdateStatus: AccountUpdateStatusMutation, + accountReleaseNpub: AccountReleaseNpubMutation, + accountAssignNpub: AccountAssignNpubMutation, merchantMapValidate: MerchantMapValidateMutation, merchantMapDelete: MerchantMapDeleteMutation, businessUpdateMapInfo: BusinessUpdateMapInfoMutation, diff --git a/src/graphql/admin/queries.ts b/src/graphql/admin/queries.ts index 89d215e72..c26a897ba 100644 --- a/src/graphql/admin/queries.ts +++ b/src/graphql/admin/queries.ts @@ -13,6 +13,7 @@ import AccountDetailsByUserEmailQuery from "./root/query/account-details-by-emai import ListWalletIdsQuery from "./root/query/all-walletids" import WalletQuery from "./root/query/wallet" import AccountDetailsByAccountId from "./root/query/account-details-by-account-id" +import AccountDetailsByNpubQuery from "./root/query/account-details-by-npub" import MerchantsPendingApprovalQuery from "./root/query/merchants-pending-approval-listing" import IdDocumentReadUrlQuery from "./root/query/id-document-read-url" import NotificationTopicsQuery from "./root/query/notification-topics" @@ -29,6 +30,7 @@ export const queryFields = { accountDetailsByUsername: AccountDetailsByUsernameQuery, accountDetailsByEmail: AccountDetailsByUserEmailQuery, accountDetailsByAccountId: AccountDetailsByAccountId, + accountDetailsByNpub: AccountDetailsByNpubQuery, transactionById: TransactionByIdQuery, transactionDetailsById: TransactionDetailsByIdQuery, transactionsByHash: TransactionsByHashQuery, diff --git a/src/graphql/admin/root/mutation/account-assign-npub.ts b/src/graphql/admin/root/mutation/account-assign-npub.ts new file mode 100644 index 000000000..e3959344e --- /dev/null +++ b/src/graphql/admin/root/mutation/account-assign-npub.ts @@ -0,0 +1,56 @@ +import { GT } from "@graphql/index" + +import AccountAssignNpubPayload from "@graphql/admin/types/payload/account-assign-npub" +import { Accounts } from "@app" +import { mapAndParseErrorForGqlResponse } from "@graphql/error-map" +import Npub from "@graphql/shared/types/scalar/npub" + +const AccountAssignNpubInput = GT.Input({ + name: "AccountAssignNpubInput", + fields: () => ({ + accountId: { + type: GT.NonNullID, + }, + // The `npub` scalar, not String: it validates and normalises at the + // boundary, the same way `accountDetailsByNpub` does. A mutation that + // mints a permanent identity claim should not accept a looser input than + // the query that merely reads one. + npub: { + type: GT.NonNull(Npub), + }, + }), +}) + +const AccountAssignNpubMutation = GT.Field< + null, + GraphQLAdminContext, + { input: { accountId: string | Error; npub: Npub | ValidationError } } +>({ + extensions: { + complexity: 120, + }, + type: GT.NonNull(AccountAssignNpubPayload), + args: { + input: { type: GT.NonNull(AccountAssignNpubInput) }, + }, + resolve: async (_, args, ctx) => { + const { accountId, npub } = args.input + const supportUser = ctx.user.id + + if (accountId instanceof Error) return { errors: [{ message: accountId.message }] } + if (npub instanceof Error) return { errors: [{ message: npub.message }] } + + const assigned = await Accounts.assignNpub({ + id: accountId, + npub, + assignedByUserId: supportUser, + }) + if (assigned instanceof Error) { + return { errors: [mapAndParseErrorForGqlResponse(assigned)] } + } + + return { errors: [], accountDetails: assigned } + }, +}) + +export default AccountAssignNpubMutation diff --git a/src/graphql/admin/root/mutation/account-release-npub.ts b/src/graphql/admin/root/mutation/account-release-npub.ts new file mode 100644 index 000000000..c1913e50b --- /dev/null +++ b/src/graphql/admin/root/mutation/account-release-npub.ts @@ -0,0 +1,72 @@ +import { GT } from "@graphql/index" + +import AccountReleaseNpubPayload from "@graphql/admin/types/payload/account-release-npub" +import { Accounts } from "@app" +import { mapAndParseErrorForGqlResponse } from "@graphql/error-map" + +const AccountReleaseNpubInput = GT.Input({ + name: "AccountReleaseNpubInput", + fields: () => ({ + accountId: { + type: GT.NonNullID, + }, + // Hands the freed key straight to the rightful owner. Omitted, the key goes + // back to unclaimed and whoever polls for it first gets it. + reassignToAccountId: { + type: GT.ID, + }, + }), +}) + +const AccountReleaseNpubMutation = GT.Field< + null, + GraphQLAdminContext, + { + input: { accountId: string | Error; reassignToAccountId?: string | Error } + } +>({ + extensions: { + complexity: 120, + }, + type: GT.NonNull(AccountReleaseNpubPayload), + args: { + input: { type: GT.NonNull(AccountReleaseNpubInput) }, + }, + resolve: async (_, args, ctx) => { + const { accountId, reassignToAccountId } = args.input + const supportUser = ctx.user.id + + if (accountId instanceof Error) return { errors: [{ message: accountId.message }] } + if (reassignToAccountId instanceof Error) { + return { errors: [{ message: reassignToAccountId.message }] } + } + + const released = await Accounts.releaseNpub({ + id: accountId, + releasedByUserId: supportUser, + reassignToAccountId, + }) + if (released instanceof Error) { + return { errors: [mapAndParseErrorForGqlResponse(released)] } + } + + // A post-release reassignment failure is reported twice on purpose: in + // `errors` because something did fail, and in `reassignmentError` because + // `errors` alone is indistinguishable from a mutation that changed nothing + // — and here the key has already left the holder. + const reassignmentError = + released.reassignmentError === undefined + ? undefined + : mapAndParseErrorForGqlResponse(released.reassignmentError) + + return { + errors: reassignmentError === undefined ? [] : [reassignmentError], + accountDetails: released.account, + previousNpub: released.previousNpub, + reassignedTo: released.reassignedTo, + reassignmentError, + } + }, +}) + +export default AccountReleaseNpubMutation diff --git a/src/graphql/admin/root/query/account-details-by-npub.ts b/src/graphql/admin/root/query/account-details-by-npub.ts new file mode 100644 index 000000000..b62dc293f --- /dev/null +++ b/src/graphql/admin/root/query/account-details-by-npub.ts @@ -0,0 +1,35 @@ +import { GT } from "@graphql/index" + +import GraphQLAccount from "@graphql/admin/types/object/account" +import Npub from "@graphql/shared/types/scalar/npub" +import { mapError } from "@graphql/error-map" + +import { Admin } from "@app" + +const AccountDetailsByNpubQuery = GT.Field< + null, + GraphQLAdminContext, + { + // FIXME: doesn't respect the input: {} pattern + npub: Npub | ValidationError + } +>({ + type: GT.NonNull(GraphQLAccount), + args: { + npub: { type: GT.NonNull(Npub) }, + }, + resolve: async (parent, { npub }) => { + if (npub instanceof Error) { + throw npub + } + + const account = await Admin.getAccountByNpub(npub) + if (account instanceof Error) { + throw mapError(account) + } + + return account + }, +}) + +export default AccountDetailsByNpubQuery diff --git a/src/graphql/admin/schema.graphql b/src/graphql/admin/schema.graphql index c4579b2b2..b40c21224 100644 --- a/src/graphql/admin/schema.graphql +++ b/src/graphql/admin/schema.graphql @@ -1,3 +1,13 @@ +input AccountAssignNpubInput { + accountId: ID! + npub: npub! +} + +type AccountAssignNpubPayload { + accountDetails: AuditedAccount + errors: [Error] +} + type AccountCapabilities { """An approved bank account is on file for payouts.""" bankPayout: Boolean! @@ -26,6 +36,19 @@ enum AccountLevel { ZERO } +input AccountReleaseNpubInput { + accountId: ID! + reassignToAccountId: ID +} + +type AccountReleaseNpubPayload { + accountDetails: AuditedAccount + errors: [Error] + previousNpub: String + reassignedTo: AuditedAccount + reassignmentError: Error +} + enum AccountStatus { ACTIVE CLOSED @@ -429,6 +452,8 @@ type MerchantPayload { } type Mutation { + accountAssignNpub(input: AccountAssignNpubInput!): AccountAssignNpubPayload! + accountReleaseNpub(input: AccountReleaseNpubInput!): AccountReleaseNpubPayload! accountUpdateLevel(input: AccountUpdateLevelInput!): AccountDetailPayload! accountUpdateStatus(input: AccountUpdateStatusInput!): AccountDetailPayload! businessDeleteMapInfo(input: BusinessDeleteMapInfoInput!): AccountDetailPayload! @@ -489,6 +514,7 @@ type PriceOfOneSettlementMinorUnitInDisplayMinorUnit implements PriceInterface { type Query { accountDetailsByAccountId(accountId: ID!): AuditedAccount! accountDetailsByEmail(email: EmailAddress!): AuditedAccount! + accountDetailsByNpub(npub: npub!): AuditedAccount! accountDetailsByUserPhone(phone: Phone!): AuditedAccount! accountDetailsByUsername(username: Username!): AuditedAccount! allLevels: [AccountLevel!]! @@ -871,4 +897,7 @@ enum WalletCurrency { } """Unique identifier of a wallet""" -scalar WalletId \ No newline at end of file +scalar WalletId + +"""Nostr Identity public key""" +scalar npub \ No newline at end of file diff --git a/src/graphql/admin/types/payload/account-assign-npub.ts b/src/graphql/admin/types/payload/account-assign-npub.ts new file mode 100644 index 000000000..63977f1f2 --- /dev/null +++ b/src/graphql/admin/types/payload/account-assign-npub.ts @@ -0,0 +1,21 @@ +import { GT } from "@graphql/index" +import IError from "@graphql/shared/types/abstract/error" + +import GraphQLAccount from "../object/account" + +// `accountDetails` carries the npub this time (unlike the release payload, +// where the key has just been taken off), so the operator can confirm from the +// response alone that the key landed on the intended account. +const AccountAssignNpubPayload = GT.Object({ + name: "AccountAssignNpubPayload", + fields: () => ({ + errors: { + type: GT.List(IError), + }, + accountDetails: { + type: GraphQLAccount, + }, + }), +}) + +export default AccountAssignNpubPayload diff --git a/src/graphql/admin/types/payload/account-release-npub.ts b/src/graphql/admin/types/payload/account-release-npub.ts new file mode 100644 index 000000000..95cb4a7a9 --- /dev/null +++ b/src/graphql/admin/types/payload/account-release-npub.ts @@ -0,0 +1,36 @@ +import { GT } from "@graphql/index" +import IError from "@graphql/shared/types/abstract/error" + +import GraphQLAccount from "../object/account" + +// Not `AccountDetailPayload`: `accountDetails` is the account the key was taken +// off, so it no longer carries the npub, and nothing else in the response says +// which key was freed or where it went. +const AccountReleaseNpubPayload = GT.Object({ + name: "AccountReleaseNpubPayload", + fields: () => ({ + errors: { + type: GT.List(IError), + }, + accountDetails: { + type: GraphQLAccount, + }, + previousNpub: { + type: GT.String, + }, + reassignedTo: { + type: GraphQLAccount, + }, + // The release and the reassignment are two writes, not a transaction, so + // the second can fail with the first already applied. That cannot be + // reported by discarding the payload: `accountDetails` and `previousNpub` + // are what tell the operator the key is now unclaimed, that re-running the + // mutation will refuse with `NoNpubToReleaseError`, and which key to hunt + // down with `accountDetailsByNpub`. + reassignmentError: { + type: IError, + }, + }), +}) + +export default AccountReleaseNpubPayload diff --git a/src/graphql/error-map.ts b/src/graphql/error-map.ts index 8c291ef1e..691ae8937 100644 --- a/src/graphql/error-map.ts +++ b/src/graphql/error-map.ts @@ -104,6 +104,10 @@ export const mapError = (error: ApplicationError): CustomApolloError => { message = `User does not exist for id ${error.message}` return new NotFoundError({ message, logger: baseLogger }) + case "CouldNotFindAccountFromIdError": + message = `Account does not exist for id ${error.message}` + return new NotFoundError({ message, logger: baseLogger }) + case "CouldNotFindAccountFromUuidError": message = `Account does not exist for uuid ${error.message}` return new NotFoundError({ message, logger: baseLogger }) @@ -112,6 +116,14 @@ export const mapError = (error: ApplicationError): CustomApolloError => { message = `Account does not exist for username ${error.message}` return new NotFoundError({ message, logger: baseLogger }) + case "CouldNotFindAccountFromNpubError": + message = `Account does not exist for npub ${error.message}` + return new NotFoundError({ message, logger: baseLogger }) + + case "NoNpubToReleaseError": + message = `Account ${error.message} has no npub linked` + return new NotFoundError({ message, logger: baseLogger }) + case "CouldNotFindMerchantFromUsernameError": message = `Merchant does not exist for username ${error.message}` return new NotFoundError({ message, logger: baseLogger }) @@ -381,6 +393,18 @@ export const mapError = (error: ApplicationError): CustomApolloError => { message = "username is immutable" return new UsernameError({ message, logger: baseLogger }) + case "NpubNotAvailableError": + message = "npub is already linked to another account" + return new ValidationInternalError({ message, logger: baseLogger }) + + case "AccountAlreadyHasNpubError": + message = "the receiving account already has an npub linked" + return new ValidationInternalError({ message, logger: baseLogger }) + + case "InvalidAccountIdError": + message = `Invalid account id ${error.message}` + return new ValidationInternalError({ message, logger: baseLogger }) + case "InvalidWalletId": message = "Invalid walletId for account." return new ValidationInternalError({ message, logger: baseLogger }) @@ -766,6 +790,7 @@ export const mapError = (error: ApplicationError): CustomApolloError => { case "LnRouteValidationError": case "BadAmountForRouteError": case "InvalidUsername": + case "InvalidNpubError": case "InvalidDeviceId": case "InvalidIdentityPassword": case "InvalidIdentityUsername": @@ -888,7 +913,6 @@ export const mapError = (error: ApplicationError): CustomApolloError => { case "CaptchaError": case "InvalidNonHodlInvoiceError": case "InvalidAccountError": - case "InvalidAccountIdError": case "InvalidMinutesError": case "InvalidWalletForAccountError": case "AuthenticationError": diff --git a/src/graphql/shared/types/scalar/npub.ts b/src/graphql/shared/types/scalar/npub.ts index fb3a6d48c..f4ea1b41c 100644 --- a/src/graphql/shared/types/scalar/npub.ts +++ b/src/graphql/shared/types/scalar/npub.ts @@ -5,19 +5,25 @@ import { GT } from "@graphql/index" const Npub = GT.Scalar({ name: "npub", description: "Nostr Identity public key", + // Both coercion paths must normalise identically: bech32 npubs are a + // lowercase-only charset, so lowercasing here is what lets the repository + // query be a plain, index-backed `$eq` with no collation. parseValue(value) { if (typeof value !== "string") { return new InputValidationError({ message: "Invalid type for Npub" }) } else if (!checkValidNpub(value)) return new InputValidationError({ message: "Invalid value for Npub" }) - return value.toString() + return value.toLowerCase() }, parseLiteral(ast) { if (ast.kind !== GT.Kind.STRING) return new InputValidationError({ message: "Invalid type for Npub" }) else if (!checkValidNpub(ast.value)) return new InputValidationError({ message: "Invalid value for Npub" }) - else ast.value.toLowerCase() + // NOTE: the `return` is load-bearing. graphql-js reads `undefined` from + // parseLiteral as failed coercion, so dropping it rejects every valid + // inline-literal npub at validation time. + else return ast.value.toLowerCase() }, }) diff --git a/src/migrations/20260824120000-accounts-unique-npub.ts b/src/migrations/20260824120000-accounts-unique-npub.ts new file mode 100644 index 000000000..48ea465bd --- /dev/null +++ b/src/migrations/20260824120000-accounts-unique-npub.ts @@ -0,0 +1,222 @@ +/* eslint-disable @typescript-eslint/ban-ts-comment */ +// @ts-nocheck +/* eslint @typescript-eslint/no-var-requires: "off" */ + +/** + * Migration: unique index on accounts.npub + * + * Background + * ---------- + * `npub` had neither an index nor a uniqueness constraint, and `setNpub` did no + * duplicate checking — it wrote whatever the caller sent. `findByNpub` is a + * `findOne`, so two accounts holding the same npub resolved to whichever + * document Mongo scanned first. That became support-facing the moment + * `accountDetailsByNpub` shipped: a support agent opening a DM from an npub + * would see username / level / phone / email off an arbitrary one of the + * colliding accounts. + * + * What this migration does + * ------------------------ + * 1. Lowercases any non-lowercase npub. The old `findByNpub` carried a + * case-insensitive collation which has been dropped (bech32 is a + * lowercase-only charset, and the collation blocked index use), so stored + * values must be normalised or they stop being findable. + * 2. Audits for duplicate npubs. When a group is found, npub is UNSET on EVERY + * account in it — including the oldest. Accounts are never deleted or + * merged here. Guessing an owner by created_at is worse than releasing the + * key: the oldest account is often an abandoned one left behind by a phone + * reset, while the live handset actually holding the nostr secret key is + * the newer account. Once the unique index exists, whoever re-links first + * wins, and the index makes that race safe. Every unset is logged with + * account id + npub so support can tell the owners to re-link from the app. + * 3. Creates the unique partial index. + * + * `partialFilterExpression: { npub: { $type: "string" } }` rather than + * `sparse: true`: a sparse index still indexes documents holding an explicit + * `npub: null`, and the second such document would collide. + * + * Rollout + * ------- + * THIS MIGRATION MUST COMPLETE BEFORE ANY POD RUNNING THE NEW IMAGE BOOTS. + * It is the first unique index on a field that holds duplicates in prod, and + * `schema.ts` now declares that index on `AccountSchema` too. + * `graphql-main-server.ts` boots with `setupMongoConnection(true)`, which runs + * `syncIndexes()` across every model. Against un-deduped data `createIndex` + * rejects with E11000, `setupMongoConnection` rethrows + * (`src/services/mongodb/index.ts:107`), and the `.catch` in + * `graphql-main-server.ts` logs a single "server error" line and returns: + * `bootstrap()` and both Apollo servers never start, the process stays alive + * on mongoose's open handles, and the pod reports Ready with no listener. One + * log line is the only clue. + * + * The chart already enforces the ordering: every galoy workload — api, + * websocket, trigger, exporter, the ibex/bridge/fygaro webhooks and the + * cronjobs — carries a `wait-for-mongodb-migrate` initContainer + * (`groundnuty/k8s-wait-for` `job-wr`) that blocks on the per-revision + * `-mongodb-migrate-` Job and fails if that Job fails, so + * the app container cannot start ahead of the migration + * (`charts/flash/templates/api-deployment.yaml`, + * `charts/flash/templates/galoy-migration-job.yaml`). + * + * Two ways to lose that guarantee, both of which produce the dead-pod failure + * above: bumping `galoy.images.app.digest` without bumping + * `galoy.images.mongodbMigrate.digest` in the same release (the migrate Job + * then runs an older image that does not contain this file, succeeds, and lets + * the new app through against un-migrated data), and force-rolling or + * recreating a Deployment out of band so the initContainer's Job is not the + * one carrying this migration. If a pod is up with no listener, check that the + * revision's migrate Job ran this migration before touching anything else. + * + * Rollback (down) + * --------------- + * Drops the unique index. The lowercasing and the unsets are NOT reverted — + * they are data repairs, and restoring known-ambiguous npubs would reintroduce + * the identity collision. + * + * Recovery + * -------- + * `setNpub` refuses an npub already held by another account + * (`NpubNotAvailableError`), so once the index exists a wrong or squatted claim + * locks the rightful owner out. Support frees the key from the admin panel with + * the `accountReleaseNpub` mutation, which unsets it on the holding account; the + * owner then re-links from the app. No hand-written write against prod mongo. + */ + +const COLLECTION = "accounts" +const INDEX_NAME = "npub_1" + +module.exports = { + async up(db) { + const col = db.collection(COLLECTION) + + const exists = await db.listCollections({ name: COLLECTION }).toArray() + if (exists.length === 0) { + // Fresh database (this is what CI's clean-migration run sees). + // createIndex creates the collection, and there is nothing to repair. + await col.createIndex( + { npub: 1 }, + { + unique: true, + name: INDEX_NAME, + partialFilterExpression: { npub: { $type: "string" } }, + }, + ) + console.log(`[migration] ${COLLECTION} did not exist; created "${INDEX_NAME}".`) + return + } + + // ── Step 1: normalise case ─────────────────────────────────────────────── + // Server-side, so this does not stream the collection through the migration + // process. The ids are listed first purely so the repair is auditable. + const mixedCase = await col + .aggregate( + [ + { $match: { npub: { $type: "string" } } }, + { $match: { $expr: { $ne: ["$npub", { $toLower: "$npub" }] } } }, + { $project: { _id: 1, id: 1, npub: 1 } }, + ], + { allowDiskUse: true }, + ) + .toArray() + + for (const doc of mixedCase) { + console.log(`[migration] accountId=${doc.id} npub will be lowercased: ${doc.npub}`) + } + + if (mixedCase.length > 0) { + // Scoped to the ids just collected. Filtering on `{ npub: { $type: + // "string" } }` instead would rewrite every npub-bearing account — + // oplog churn and index re-touching during the deploy window for zero + // additional repairs. + await col.updateMany({ _id: { $in: mixedCase.map((d) => d._id) } }, [ + { $set: { npub: { $toLower: "$npub" } } }, + ]) + } + console.log(`[migration] Normalised ${mixedCase.length} npub value(s) to lowercase.`) + + // ── Step 2: find and release duplicate npub groups ─────────────────────── + // Only the fields needed for the audit log are pushed — `$$ROOT` would risk + // the 16MB per-group limit on a large accounts collection. + const duplicates = await col + .aggregate( + [ + { $match: { npub: { $type: "string" } } }, + { + $group: { + _id: "$npub", + count: { $sum: 1 }, + docs: { $push: { _id: "$_id", id: "$id", created_at: "$created_at" } }, + }, + }, + { $match: { count: { $gt: 1 } } }, + ], + { allowDiskUse: true }, + ) + .toArray() + + if (duplicates.length > 0) { + console.log( + `[migration] Found ${duplicates.length} npub(s) claimed by more than one account. Releasing...`, + ) + + for (const group of duplicates) { + // Every account in the group loses the npub, including the oldest. + // There is no way to tell from mongo which account still holds the + // nostr secret key, and picking wrong locks the real owner out: + // `setNpub` refuses an already-claimed npub, so they would need a + // support-desk release before they could re-link at all. Releasing + // every claim here lets the real owner re-link straight from the app; + // the unique index makes the re-link race safe. + const ids = group.docs.map((d) => d._id) + + console.log( + `[migration] npub=${group._id} — releasing from ${ids.length} account(s): ${group.docs + .map((d) => `${d.id} (_id=${d._id}, created_at=${d.created_at})`) + .join(", ")}`, + ) + + await col.updateMany({ _id: { $in: ids } }, { $unset: { npub: "" } }) + } + + console.log( + "[migration] Duplicate npub release complete. Affected owners must re-link from the app.", + ) + } else { + console.log("[migration] No duplicate npub values found. Proceeding.") + } + + // ── Step 3: drop any stale index, then create the unique partial index ─── + const existingIndexes = await col.indexes() + const stale = existingIndexes.find((idx) => idx.name === INDEX_NAME && !idx.unique) + if (stale) { + await col.dropIndex(INDEX_NAME) + console.log(`[migration] Dropped existing non-unique index "${INDEX_NAME}".`) + } + + await col.createIndex( + { npub: 1 }, + { + unique: true, + name: INDEX_NAME, + partialFilterExpression: { npub: { $type: "string" } }, + }, + ) + console.log(`[migration] Created unique index "${INDEX_NAME}" on ${COLLECTION}.`) + }, + + async down(db) { + const col = db.collection(COLLECTION) + + const exists = await db.listCollections({ name: COLLECTION }).toArray() + if (exists.length === 0) return + + const existingIndexes = await col.indexes() + const hasUniqueIndex = existingIndexes.some( + (idx) => idx.name === INDEX_NAME && idx.unique, + ) + if (hasUniqueIndex) { + await col.dropIndex(INDEX_NAME) + console.log(`[migration] Dropped unique index "${INDEX_NAME}".`) + } + }, +} diff --git a/src/services/mongoose/accounts.ts b/src/services/mongoose/accounts.ts index 779545639..086eb564c 100644 --- a/src/services/mongoose/accounts.ts +++ b/src/services/mongoose/accounts.ts @@ -3,12 +3,16 @@ import { OnboardingEarn } from "@config" import { AccountStatus } from "@domain/accounts" import { CouldNotFindAccountError, + CouldNotFindAccountFromIdError, CouldNotFindAccountFromKratosIdError, + CouldNotFindAccountFromNpubError, CouldNotFindAccountFromUsernameError, CouldNotFindAccountFromUuidError, + NoNpubToReleaseError, RepositoryError, } from "@domain/errors" import { UsdDisplayCurrency } from "@domain/fiat" +import { AccountAlreadyHasNpubError } from "@domain/nostr" import { Account } from "@services/mongoose/schema" @@ -77,14 +81,107 @@ export const AccountsRepository = (): IAccountsRepository => { } } + // No `.collation(...)` here on purpose. bech32 npubs are a lowercase-only + // charset and both scalar coercion paths lowercase, so case-insensitivity + // buys nothing — and a non-simple collation would stop the query from using + // the unique `{ npub: 1 }` index, leaving every support-desk lookup a + // collection scan. + // + // The redundant-looking `$type: "string"` is what actually gets the index + // used. The index is partial on `{ npub: { $type: "string" } }`, and mongo + // only picks a partial index when the query provably matches a subset of the + // partial filter. It cannot derive that from an equality against a string + // literal — `$type` is not one of the predicates its implication check + // understands — so `{ npub: { $eq: npub } }` alone plans as a COLLSCAN + // (verified on 6.0 in test/flash/integration/accounts/npub.spec.ts). + // Restating the type predicate makes it an IXSCAN. It cannot change the + // result set: `npub` is always a string here, so any document that could + // match the equality is a string-typed one. const findByNpub = async (npub: Npub): Promise => { try { - const result = await Account.findOne({ npub: { $eq: npub } }).collation({ - locale: "en", - strength: 2, - }) + const result = await Account.findOne({ npub: { $eq: npub, $type: "string" } }) + if (!result) { + return new CouldNotFindAccountFromNpubError(npub) + } + return translateToAccount(result) + } catch (err) { + return parseRepositoryError(err) + } + } + + // An npub claim is permanently unique and carries no proof of key control, so + // support needs a way to hand one back. `update` cannot do it: mongoose strips + // undefined keys from an update doc, so clearing the field takes an explicit + // $unset — and the partial index only covers string npubs, so the released key + // is immediately re-claimable by whoever actually holds the secret key. + // + // `new: false` is load-bearing. `$unset` against a document that never held an + // npub is a no-op that still matches on `_id`, so the post-update document is + // identical in both cases and the operator would be told a release happened + // when nothing was freed — then send the rightful owner off to re-link, where + // `setNpub` refuses them because the squatter still holds the key. The + // pre-update document is the only thing that can tell the two apart, which + // also makes it the only read that cannot disagree with the key this `$unset` + // removed — hence `previousNpub` comes back with the account rather than the + // caller re-reading it. + // + // `typeof` rather than a null check: the field is `Npub | null` and legacy + // documents holding an explicit null predate the partial index, which only + // covers strings. A null is not a claim and there is nothing to release. + const unsetNpub = async ( + accountId: AccountId, + ): Promise => { + try { + const before = await Account.findOneAndUpdate( + { _id: toObjectId(accountId) }, + { $unset: { npub: "" } }, + { new: false }, + ) + if (!before) return new CouldNotFindAccountFromIdError(accountId) + if (typeof before.npub !== "string") return new NoNpubToReleaseError(accountId) + return { + account: { ...translateToAccount(before), npub: undefined }, + previousNpub: before.npub, + } + } catch (err) { + return parseRepositoryError(err) + } + } + + // The reassignment half of a release. A targeted `$set` rather than a + // read-modify-write through `update`, so the unique partial index is the only + // thing that decides whether the claim lands on the key side: a concurrent + // claimant of the same npub trips it and `parseRepositoryError` surfaces + // `DuplicateKeyForPersistError`. + // + // The filter guards the target side. The caller checks the target holds no + // npub before releasing, but that check is a read from before the release + // round-trip: if the target links a different key via `userUpdateNpub` in + // that window, an unguarded `$set` would silently overwrite the just-claimed + // key — which becomes unclaimed with no log line saying so. The unique index + // cannot catch this: it prevents duplicates, not overwrites. `$not: + // { $type: "string" }` rather than `$exists: false` because legacy documents + // predating the partial index hold an explicit `npub: null`, which is not a + // claim and must not block a reassignment. + // + // A no-match is ambiguous between "no such account" and "account claimed a + // key since the caller checked", so one follow-up read disambiguates. The + // claim did not land in either case, so a stale answer from that read still + // reports a refusal — the conservative outcome. + const claimNpub = async ( + accountId: AccountId, + npub: Npub, + ): Promise => { + try { + const result = await Account.findOneAndUpdate( + { _id: toObjectId(accountId), npub: { $not: { $type: "string" } } }, + { $set: { npub } }, + { new: true }, + ) if (!result) { - return new CouldNotFindAccountFromUsernameError(npub) + const existing = await Account.findOne({ _id: toObjectId(accountId) }) + if (!existing) return new CouldNotFindAccountFromIdError(accountId) + return new AccountAlreadyHasNpubError(accountId) } return translateToAccount(result) } catch (err) { @@ -293,6 +390,8 @@ export const AccountsRepository = (): IAccountsRepository => { findByUuid, findByUsername, findByNpub, + unsetNpub, + claimNpub, update, transitionBridgeKycStatus, updateBridgeFields, diff --git a/src/services/mongoose/schema.ts b/src/services/mongoose/schema.ts index a23a2ad37..766c729e3 100644 --- a/src/services/mongoose/schema.ts +++ b/src/services/mongoose/schema.ts @@ -390,6 +390,18 @@ AccountSchema.index({ AccountSchema.index({ bridgeEthereumAddress: 1 }, { sparse: true }) +// An npub identifies a customer to the admin/support lookup, so it must resolve +// to exactly one account. `partialFilterExpression` rather than `sparse` — a +// sparse index still indexes documents that hold an explicit `npub: null`, and +// the second such document would collide. Mirrors the `username` index above. +AccountSchema.index( + { npub: 1 }, + { + unique: true, + partialFilterExpression: { npub: { $type: "string" } }, + }, +) + export const Account = mongoose.model("Account", AccountSchema) const QuizSchema = new Schema({ diff --git a/test/flash/integration/accounts/npub.spec.ts b/test/flash/integration/accounts/npub.spec.ts new file mode 100644 index 000000000..b9ba6e51e --- /dev/null +++ b/test/flash/integration/accounts/npub.spec.ts @@ -0,0 +1,351 @@ +/** + * The npub repository layer added by this PR is almost entirely index- and + * filter-shaped, and a mocked mongoose model cannot report on any of it: the + * unit specs assert that `claimNpub` passes `{ npub: { $not: { $type: + * "string" } } }` and that the schema declares a unique partial index, but + * nothing there executes either against a database. If the filter were wrong, + * the guard would be silently inert — a reassignment onto a legacy `npub: + * null` account would come back `AccountAlreadyHasNpubError` forever — and the + * unit suite would stay green. + * + * These run against the real collection, so they can return "no" about the + * three claims the comments make: + * + * - `$not: { $type: "string" }` rather than `$exists: false`, because legacy + * documents predating the partial index hold an explicit `npub: null` and + * that is not a claim (`accounts.ts` `claimNpub`, `release-npub.ts`). + * - the unique partial index is what makes a concurrent claim of the same key + * fail rather than duplicate (`schema.ts`, migration + * 20260824120000-accounts-unique-npub). + * - `findByNpub` reaches that index instead of scanning the collection + * (`accounts.ts`). This one already came back "no" once: dropping the + * collation was not sufficient, because mongo will not use an index whose + * partial filter is `$type`-shaped for a bare equality. + */ +import mongoose from "mongoose" + +import { releaseNpub } from "@app/accounts/release-npub" +import { + CouldNotFindAccountFromNpubError, + DuplicateKeyForPersistError, + NoNpubToReleaseError, +} from "@domain/errors" +import { AccountAlreadyHasNpubError, checkedToNpub } from "@domain/nostr" +import { AccountsRepository } from "@services/mongoose" +import { Account as AccountModel } from "@services/mongoose/schema" +import { toObjectId } from "@services/mongoose/utils" + +import { createUser } from "test/galoy/helpers" + +const SUPPORT_USER_ID = "support-user-id" as UserId + +// bech32's charset — "1", "b", "i" and "o" are excluded. Values only have to be +// unique and well-formed; nothing here decodes them. +const CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l" +const RUN = BigInt(Math.floor(Math.random() * 2 ** 40)) +let minted = 0n + +const uniqueNpub = (): Npub => { + let remaining = RUN * 1_000_000n + minted++ + let body = "" + while (remaining > 0n) { + body = CHARSET[Number(remaining % 32n)] + body + remaining /= 32n + } + // Minted through the same checker the GraphQL boundary uses, so a value that + // would not survive validation cannot make these tests pass. + const npub = checkedToNpub(`npub1${body.padStart(58, CHARSET[0])}`) + if (npub instanceof Error) throw npub + return npub +} + +const newAccountId = async (): Promise => { + const user = await createUser() + return user.account.id +} + +// Legacy shape: written before the partial index existed, so it can hold an +// explicit null. `setNpub` never produces this — only direct mongo does. +const setLegacyNullNpub = async (accountId: AccountId) => { + await AccountModel.collection.updateOne( + { _id: toObjectId(accountId) }, + { $set: { npub: null } }, + ) +} + +const npubOf = async (accountId: AccountId): Promise => { + const doc = await AccountModel.collection.findOne({ + _id: toObjectId(accountId), + }) + return doc?.npub +} + +const claim = async (accountId: AccountId, npub: Npub) => { + const claimed = await AccountsRepository().claimNpub(accountId, npub) + if (claimed instanceof Error) throw claimed + return claimed +} + +// The filter the repository actually handed mongo, rather than one restated in +// the test. Mongoose's debug hook is the only seam that reports it without +// re-opening the connection with `monitorCommands`. +const captureFilter = async ( + run: () => Promise, +): Promise> => { + const sent: Record[] = [] + mongoose.set( + "debug", + (collectionName: string, methodName: string, ...args: unknown[]) => { + if ( + collectionName === AccountModel.collection.collectionName && + methodName === "findOne" + ) { + sent.push(args[0] as Record) + } + }, + ) + try { + await run() + } finally { + mongoose.set("debug", false) + } + if (sent.length !== 1) { + throw new Error(`expected exactly one findOne, saw ${sent.length}`) + } + return sent[0] +} + +describe("accounts npub persistence", () => { + it("has the unique partial index on the live collection", async () => { + // `schema.ts` declaring it is not the same as mongo holding it, and every + // other assertion below leans on the real index existing. + const indexes = (await AccountModel.collection.indexes()) as Record[] + const npubIndex = indexes.find( + (idx) => JSON.stringify(idx.key) === JSON.stringify({ npub: 1 }), + ) + + expect(npubIndex).toBeDefined() + expect(npubIndex?.unique).toBe(true) + expect(npubIndex?.partialFilterExpression).toEqual({ npub: { $type: "string" } }) + }) + + describe("claimNpub", () => { + it("lands on an account with no npub field", async () => { + const accountId = await newAccountId() + const npub = uniqueNpub() + + const result = await AccountsRepository().claimNpub(accountId, npub) + + expect(result).not.toBeInstanceOf(Error) + expect((result as Account).npub).toBe(npub) + expect(await npubOf(accountId)).toBe(npub) + }) + + it("lands on an account holding a legacy explicit null", async () => { + // The case `$not: { $type: "string" }` exists for. `$exists: false` would + // refuse here, and the account could never receive a key again. + const accountId = await newAccountId() + await setLegacyNullNpub(accountId) + const npub = uniqueNpub() + + const result = await AccountsRepository().claimNpub(accountId, npub) + + expect(result).not.toBeInstanceOf(Error) + expect(await npubOf(accountId)).toBe(npub) + }) + + it("refuses an account already holding a key, and leaves that key alone", async () => { + const accountId = await newAccountId() + const held = uniqueNpub() + await claim(accountId, held) + + const result = await AccountsRepository().claimNpub(accountId, uniqueNpub()) + + expect(result).toBeInstanceOf(AccountAlreadyHasNpubError) + // The unique index cannot catch an overwrite — only the filter can. If + // the guard were inert this would be the new key, and the old one would + // be unclaimed with nothing logging it. + expect(await npubOf(accountId)).toBe(held) + }) + + it("reports a duplicate when a second account claims the same key", async () => { + const first = await newAccountId() + const second = await newAccountId() + const npub = uniqueNpub() + + await claim(first, npub) + + const result = await AccountsRepository().claimNpub(second, npub) + + expect(result).toBeInstanceOf(DuplicateKeyForPersistError) + expect(await npubOf(second)).toBeUndefined() + }) + + it("lets two accounts hold a legacy null without colliding", async () => { + // `partialFilterExpression` rather than `sparse`: a sparse index would + // index both nulls and reject the second one. + const first = await newAccountId() + const second = await newAccountId() + + await setLegacyNullNpub(first) + await setLegacyNullNpub(second) + + expect(await npubOf(first)).toBeNull() + expect(await npubOf(second)).toBeNull() + }) + }) + + describe("findByNpub", () => { + it("resolves the claiming account", async () => { + const accountId = await newAccountId() + const npub = uniqueNpub() + await claim(accountId, npub) + + const found = await AccountsRepository().findByNpub(npub) + + expect(found).not.toBeInstanceOf(Error) + expect((found as Account).id).toBe(accountId) + }) + + it("reports an unclaimed key rather than an arbitrary account", async () => { + expect(await AccountsRepository().findByNpub(uniqueNpub())).toBeInstanceOf( + CouldNotFindAccountFromNpubError, + ) + }) + + it("uses the index rather than scanning the collection", async () => { + // Dropping the case-insensitive collation was supposed to make this + // lookup an index hit — it is the support desk's identity resolver and + // runs on every inbound nostr DM, plus once per `setNpub` as the + // duplicate probe. The collation was only half of it: the index is + // partial on `{ npub: { $type: "string" } }`, and mongo will not pick a + // partial index unless the query provably matches a subset of that + // filter, which it cannot derive from a bare equality. `{ npub: { $eq } + // }` alone plans as a COLLSCAN on 6.0. + // + // The plan is taken from the filter the repository actually sent, not + // one restated here — restating it would pass no matter what + // `findByNpub` does. + const probe = uniqueNpub() + const sent = await captureFilter(() => AccountsRepository().findByNpub(probe)) + + const plan = (await AccountModel.collection + .find(sent) + .explain("queryPlanner")) as unknown as { + queryPlanner: { winningPlan: unknown } + } + + expect(JSON.stringify(plan.queryPlanner.winningPlan)).toContain("IXSCAN") + }) + }) + + describe("unsetNpub", () => { + it("removes the field and reports the key it freed", async () => { + const accountId = await newAccountId() + const npub = uniqueNpub() + await claim(accountId, npub) + + const released = await AccountsRepository().unsetNpub(accountId) + + expect(released).not.toBeInstanceOf(Error) + expect((released as NpubUnset).previousNpub).toBe(npub) + // Removed, not blanked: the partial index only covers strings, so a null + // left behind would sit unindexed and keep failing lookups. + expect(await npubOf(accountId)).toBeUndefined() + expect(await AccountsRepository().findByNpub(npub)).toBeInstanceOf( + CouldNotFindAccountFromNpubError, + ) + }) + + it("frees the key for whoever actually holds the secret key", async () => { + const squatter = await newAccountId() + const owner = await newAccountId() + const npub = uniqueNpub() + await claim(squatter, npub) + + const released = await AccountsRepository().unsetNpub(squatter) + if (released instanceof Error) throw released + + expect(await AccountsRepository().claimNpub(owner, npub)).not.toBeInstanceOf(Error) + }) + + it("refuses an account holding a legacy explicit null", async () => { + const accountId = await newAccountId() + await setLegacyNullNpub(accountId) + + expect(await AccountsRepository().unsetNpub(accountId)).toBeInstanceOf( + NoNpubToReleaseError, + ) + }) + + it("refuses an account that held nothing", async () => { + const accountId = await newAccountId() + + expect(await AccountsRepository().unsetNpub(accountId)).toBeInstanceOf( + NoNpubToReleaseError, + ) + }) + }) + + describe("releaseNpub with reassignment", () => { + it("moves the key to a target that never held one", async () => { + const squatter = await newAccountId() + const owner = await newAccountId() + const npub = uniqueNpub() + await claim(squatter, npub) + + const result = await releaseNpub({ + id: squatter, + releasedByUserId: SUPPORT_USER_ID, + reassignToAccountId: owner, + }) + + expect(result).not.toBeInstanceOf(Error) + expect(result).toMatchObject({ previousNpub: npub, reassignedTo: { id: owner } }) + expect(await npubOf(squatter)).toBeUndefined() + expect(await npubOf(owner)).toBe(npub) + }) + + it("moves the key to a target holding a legacy explicit null", async () => { + // The end-to-end shape of the bug the `$not: { $type: "string" }` filter + // and the `typeof` pre-check exist to avoid: an owner whose account + // predates the partial index would otherwise be told + // `AccountAlreadyHasNpubError` forever, with the key already released. + const squatter = await newAccountId() + const owner = await newAccountId() + await setLegacyNullNpub(owner) + const npub = uniqueNpub() + await claim(squatter, npub) + + const result = await releaseNpub({ + id: squatter, + releasedByUserId: SUPPORT_USER_ID, + reassignToAccountId: owner, + }) + + expect(result).not.toBeInstanceOf(Error) + expect(result).not.toMatchObject({ reassignmentError: expect.anything() }) + expect(await npubOf(owner)).toBe(npub) + }) + + it("refuses a target already holding a key, before freeing anything", async () => { + const squatter = await newAccountId() + const owner = await newAccountId() + const npub = uniqueNpub() + const ownerNpub = uniqueNpub() + await claim(squatter, npub) + await claim(owner, ownerNpub) + + const result = await releaseNpub({ + id: squatter, + releasedByUserId: SUPPORT_USER_ID, + reassignToAccountId: owner, + }) + + expect(result).toBeInstanceOf(AccountAlreadyHasNpubError) + // Nothing was freed — re-running with a different target must still work. + expect(await npubOf(squatter)).toBe(npub) + expect(await npubOf(owner)).toBe(ownerNpub) + }) + }) +}) diff --git a/test/flash/unit/app/accounts/assign-npub.spec.ts b/test/flash/unit/app/accounts/assign-npub.spec.ts new file mode 100644 index 000000000..d02d22953 --- /dev/null +++ b/test/flash/unit/app/accounts/assign-npub.spec.ts @@ -0,0 +1,132 @@ +/** + * `releaseNpub` is two writes and no transaction. When the second one fails the + * key is unclaimed and the account it came off no longer holds it, so + * `accountReleaseNpub` cannot be re-run against it. `assignNpub` is that second + * write on its own — the only admin path that can finish a half-applied + * reassignment before the squatter's poller re-takes the key. + */ +const claimNpub = jest.fn() +const info = jest.fn() +const error = jest.fn() + +jest.mock("@services/mongoose", () => ({ + AccountsRepository: () => ({ claimNpub }), +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { + info: (...args: unknown[]) => info(...args), + warn: jest.fn(), + error: (...args: unknown[]) => error(...args), + }, +})) + +import { InvalidAccountIdError } from "@domain/accounts" +import { + CouldNotFindAccountFromIdError, + DuplicateKeyForPersistError, + UnknownRepositoryError, +} from "@domain/errors" +import { + AccountAlreadyHasNpubError, + InvalidNpubError, + NpubNotAvailableError, +} from "@domain/nostr" +import { assignNpub } from "@app/accounts/assign-npub" + +const ACCOUNT_ID = "5f4c9a2b1e7d3f8a6b0c4d2e" as AccountId +const SUPPORT_USER_ID = "support-user-id" as UserId +const NPUB = `npub1${"q".repeat(58)}` + +const assign = (overrides: Record = {}) => + assignNpub({ + id: ACCOUNT_ID, + npub: NPUB, + assignedByUserId: SUPPORT_USER_ID, + ...overrides, + }) + +describe("assignNpub", () => { + beforeEach(() => jest.clearAllMocks()) + + it("claims the key for the account and returns it", async () => { + const account = { id: ACCOUNT_ID, npub: NPUB } + claimNpub.mockResolvedValue(account) + + const result = await assign() + + expect(claimNpub).toHaveBeenCalledWith(ACCOUNT_ID, NPUB) + expect(result).toBe(account) + }) + + it("records who assigned the key", async () => { + claimNpub.mockResolvedValue({ id: ACCOUNT_ID, npub: NPUB }) + + await assign() + + // The admin server never assigns req.gqlContext, so this line is the only + // record that an admin moved a key. + expect(info).toHaveBeenCalledWith( + expect.objectContaining({ + accountId: ACCOUNT_ID, + npub: NPUB, + assignedByUserId: SUPPORT_USER_ID, + }), + expect.any(String), + ) + }) + + it("rejects a malformed account id before touching the repository", async () => { + const result = await assign({ id: "not-an-object-id" }) + + expect(result).toBeInstanceOf(InvalidAccountIdError) + expect(claimNpub).not.toHaveBeenCalled() + }) + + it("rejects a malformed npub before touching the repository", async () => { + const result = await assign({ npub: "not-an-npub" }) + + expect(result).toBeInstanceOf(InvalidNpubError) + expect(claimNpub).not.toHaveBeenCalled() + }) + + it("reports a key held by someone else as unavailable, not as a raw duplicate", async () => { + // Same mapping the reassignment path uses, so one cause reads identically + // wherever it surfaces. + claimNpub.mockResolvedValue(new DuplicateKeyForPersistError()) + + const result = await assign() + + expect(result).toBeInstanceOf(NpubNotAvailableError) + }) + + it("refuses rather than overwriting when the target already holds a key", async () => { + claimNpub.mockResolvedValue(new AccountAlreadyHasNpubError(ACCOUNT_ID)) + + const result = await assign() + + expect(result).toBeInstanceOf(AccountAlreadyHasNpubError) + }) + + it("passes through a missing account", async () => { + claimNpub.mockResolvedValue(new CouldNotFindAccountFromIdError(ACCOUNT_ID)) + + const result = await assign() + + expect(result).toBeInstanceOf(CouldNotFindAccountFromIdError) + }) + + it("logs every refusal, so a token sweeping account ids cannot probe silently", async () => { + claimNpub.mockResolvedValue(new UnknownRepositoryError()) + + await assign() + + expect(error).toHaveBeenCalledWith( + expect.objectContaining({ + accountId: ACCOUNT_ID, + assignedByUserId: SUPPORT_USER_ID, + }), + expect.any(String), + ) + }) +}) diff --git a/test/flash/unit/app/accounts/find-by-npub.spec.ts b/test/flash/unit/app/accounts/find-by-npub.spec.ts new file mode 100644 index 000000000..80eb4569f --- /dev/null +++ b/test/flash/unit/app/accounts/find-by-npub.spec.ts @@ -0,0 +1,48 @@ +/** + * `Accounts.findByNpub` is the one implementation behind both this name and + * `Admin.getAccountByNpub`, which re-exports it. It never had the normalisation + * the admin path did; once the case-insensitive collation came off the + * repository query, normalising here stopped being optional. + */ +const findByNpub = jest.fn() + +jest.mock("@services/mongoose", () => ({ + AccountsRepository: () => ({ findByNpub }), +})) + +import { InvalidNpubError } from "@domain/nostr" +import { findByNpub as findAccountByNpub } from "@app/accounts/find-by-npub" + +const NPUB = ("npub1" + "q".repeat(58)) as Npub + +describe("Accounts.findByNpub", () => { + beforeEach(() => { + findByNpub.mockReset() + }) + + it("hands a valid npub to the repository", async () => { + const account = { id: "account-id", npub: NPUB } + findByNpub.mockResolvedValue(account) + + expect(await findAccountByNpub(NPUB)).toBe(account) + expect(findByNpub).toHaveBeenCalledWith(NPUB) + }) + + it("normalises case before querying", async () => { + // The repository query is a plain `$eq` with no collation. Unnormalised, + // a mixed-case npub from a script or a backfill is a silent not-found on a + // real user — which the public `isFlashNpub` query reports as false. + findByNpub.mockResolvedValue({ id: "account-id" }) + + await findAccountByNpub(("npub1" + "Q".repeat(58)) as Npub) + + expect(findByNpub).toHaveBeenCalledWith(NPUB) + }) + + it("rejects a malformed npub instead of querying with it", async () => { + const result = await findAccountByNpub("not-an-npub" as Npub) + + expect(result).toBeInstanceOf(InvalidNpubError) + expect(findByNpub).not.toHaveBeenCalled() + }) +}) diff --git a/test/flash/unit/app/accounts/release-npub.spec.ts b/test/flash/unit/app/accounts/release-npub.spec.ts new file mode 100644 index 000000000..138d30d71 --- /dev/null +++ b/test/flash/unit/app/accounts/release-npub.spec.ts @@ -0,0 +1,434 @@ +/** + * An npub claim carries no proof of key control and, since the unique index, + * is permanent: `setNpub` refuses an npub already held by another account, so + * whoever claims a key first keeps it — including someone who read it off a + * public relay. `releaseNpub` is the revocation path. Before it, freeing a + * squatted key meant a hand-written `$unset` against prod mongo. + */ +const findById = jest.fn() +const unsetNpub = jest.fn() +const claimNpub = jest.fn() +const info = jest.fn() +const warn = jest.fn() +const error = jest.fn() + +jest.mock("@services/mongoose", () => ({ + AccountsRepository: () => ({ findById, unsetNpub, claimNpub }), +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { + info: (...args: unknown[]) => info(...args), + warn: (...args: unknown[]) => warn(...args), + error: (...args: unknown[]) => error(...args), + }, +})) + +import { InvalidAccountIdError } from "@domain/accounts" +import { + CouldNotFindAccountError, + CouldNotFindAccountFromIdError, + DuplicateKeyForPersistError, + NoNpubToReleaseError, + UnknownRepositoryError, +} from "@domain/errors" +import { AccountAlreadyHasNpubError, NpubNotAvailableError } from "@domain/nostr" +import { releaseNpub } from "@app/accounts/release-npub" + +const ACCOUNT_ID = "5f4c9a2b1e7d3f8a6b0c4d2e" as AccountId +const TARGET_ACCOUNT_ID = "6a1b2c3d4e5f60718293a4b5" as AccountId +const SUPPORT_USER_ID = "support-user-id" as UserId +const NPUB = `npub1${"q".repeat(58)}` as Npub + +const release = (overrides: Record = {}) => + releaseNpub({ id: ACCOUNT_ID, releasedByUserId: SUPPORT_USER_ID, ...overrides }) + +describe("Accounts.releaseNpub", () => { + beforeEach(() => { + findById.mockReset() + unsetNpub.mockReset().mockResolvedValue({ + account: { id: ACCOUNT_ID, username: "jaceth2009" }, + previousNpub: NPUB, + }) + claimNpub.mockReset() + info.mockReset() + warn.mockReset() + error.mockReset() + }) + + it("clears the npub on the holding account and reports which key was freed", async () => { + const result = await release() + + expect(unsetNpub).toHaveBeenCalledWith(ACCOUNT_ID) + expect(result).not.toBeInstanceOf(Error) + expect(result).toMatchObject({ previousNpub: NPUB }) + }) + + it("takes the freed key from the write itself rather than re-reading the holder", async () => { + // A second read is a later answer: if the holder re-links between the two, + // the log and the reassignment would carry a key the `$unset` never + // removed. `unsetNpub` reads the pre-update document and is the only + // reader that cannot disagree with itself. + const result = await release() + + expect(findById).not.toHaveBeenCalled() + expect(result).toMatchObject({ previousNpub: NPUB }) + }) + + it("logs the actor, the account and the key that was removed", async () => { + // The only attribution that exists: the account document keeps no trace of + // a removed npub, and the admin server's Pino line logs `gqlContext.user` + // as undefined because its Apollo context never assigns `req.gqlContext`. + await release() + + expect(info).toHaveBeenCalledWith( + expect.objectContaining({ + accountId: ACCOUNT_ID, + previousNpub: NPUB, + releasedByUserId: SUPPORT_USER_ID, + }), + expect.any(String), + ) + }) + + it("reports an unknown account instead of reporting a release", async () => { + unsetNpub.mockResolvedValue(new CouldNotFindAccountFromIdError(ACCOUNT_ID)) + + const result = await release() + + expect(result).toBeInstanceOf(CouldNotFindAccountFromIdError) + expect(claimNpub).not.toHaveBeenCalled() + }) + + it("refuses an account that holds no npub", async () => { + // `$unset` on a document without the field is a no-op that still matches, + // so without this the operator is told a release happened and sends the + // owner off to re-link — where the squatter still holds the key. + unsetNpub.mockResolvedValue(new NoNpubToReleaseError(ACCOUNT_ID)) + + const result = await release() + + expect(result).toBeInstanceOf(NoNpubToReleaseError) + expect(claimNpub).not.toHaveBeenCalled() + }) + + it("rejects a malformed account id without writing", async () => { + const result = await releaseNpub({ + id: "not-an-account-id", + releasedByUserId: SUPPORT_USER_ID, + }) + + expect(result).toBeInstanceOf(InvalidAccountIdError) + expect(findById).not.toHaveBeenCalled() + expect(unsetNpub).not.toHaveBeenCalled() + }) + + it("passes through a repository failure", async () => { + // A failed write is not a release: the operator must not be told the key is + // free and send the owner off to re-link. + unsetNpub.mockResolvedValue(new UnknownRepositoryError("mongo down")) + + expect(await release()).toBeInstanceOf(UnknownRepositoryError) + }) + + describe("refusal logging", () => { + /** + * A stolen admin token enumerating account ids produces one line for the id + * that happened to hold a key and nothing at all for the rest, which is the + * shape an investigator needs to see the sweep. The admin server's Pino + * request line cannot supply it: it carries neither the actor nor the body. + */ + const refusals: [string, () => Promise, string][] = [ + [ + "a malformed account id", + () => releaseNpub({ id: "not-an-account-id", releasedByUserId: SUPPORT_USER_ID }), + // The id as given, not a checked one — there is no checked one here, + // and the string the caller sent is what an investigator matches on. + "not-an-account-id", + ], + [ + "a malformed reassignment target id", + () => release({ reassignToAccountId: "not-an-account-id" }), + ACCOUNT_ID, + ], + [ + "an unknown holder", + async () => { + unsetNpub.mockResolvedValue(new CouldNotFindAccountFromIdError(ACCOUNT_ID)) + return release() + }, + ACCOUNT_ID, + ], + [ + "a holder with no npub", + async () => { + unsetNpub.mockResolvedValue(new NoNpubToReleaseError(ACCOUNT_ID)) + return release() + }, + ACCOUNT_ID, + ], + [ + "an unknown reassignment target", + async () => { + findById.mockResolvedValue(new CouldNotFindAccountError()) + return release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + }, + ACCOUNT_ID, + ], + [ + "a reassignment target that already holds a key", + async () => { + findById.mockResolvedValue({ id: TARGET_ACCOUNT_ID, npub: NPUB }) + return release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + }, + ACCOUNT_ID, + ], + ] + + it.each(refusals)( + "logs a refused release for %s", + async (_label, run, expectedAccountId) => { + await run() + + expect(warn).toHaveBeenCalledWith( + expect.objectContaining({ + accountId: expectedAccountId, + releasedByUserId: SUPPORT_USER_ID, + reason: expect.any(String), + }), + expect.any(String), + ) + expect(info).not.toHaveBeenCalled() + }, + ) + + it("does not log a refusal when the release succeeds", async () => { + await release() + + expect(warn).not.toHaveBeenCalled() + }) + }) + + describe("reassignment", () => { + beforeEach(() => { + findById.mockResolvedValue({ id: TARGET_ACCOUNT_ID, username: "rightful-owner" }) + claimNpub.mockResolvedValue({ + id: TARGET_ACCOUNT_ID, + username: "rightful-owner", + npub: NPUB, + }) + }) + + it("hands the freed key to the target", async () => { + const result = await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(unsetNpub).toHaveBeenCalledWith(ACCOUNT_ID) + expect(claimNpub).toHaveBeenCalledWith(TARGET_ACCOUNT_ID, NPUB) + expect(result).toMatchObject({ + previousNpub: NPUB, + reassignedTo: { npub: NPUB }, + }) + }) + + it("accepts a target carrying a legacy null npub", async () => { + // The partial index only covers string npubs and the migration leaves + // `npub: null` documents alone, so an account that never linked a key can + // still hold an explicit null. Reading that as a claim would make the + // account permanently unable to receive a reassignment. + findById.mockResolvedValue({ id: TARGET_ACCOUNT_ID, npub: null }) + + const result = await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(result).not.toBeInstanceOf(Error) + expect(claimNpub).toHaveBeenCalledWith(TARGET_ACCOUNT_ID, NPUB) + }) + + it("refuses a target that already holds an npub, before freeing anything", async () => { + findById.mockResolvedValue({ + id: TARGET_ACCOUNT_ID, + username: "rightful-owner", + npub: "npub1other", + }) + + const result = await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(result).toBeInstanceOf(AccountAlreadyHasNpubError) + expect(unsetNpub).not.toHaveBeenCalled() + }) + + it("refuses an unknown target, before freeing anything", async () => { + findById.mockResolvedValue(new CouldNotFindAccountError()) + + const result = await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(result).toBeInstanceOf(CouldNotFindAccountFromIdError) + expect(unsetNpub).not.toHaveBeenCalled() + }) + + it("rejects a malformed target id without writing", async () => { + const result = await release({ reassignToAccountId: "not-an-account-id" }) + + expect(result).toBeInstanceOf(InvalidAccountIdError) + expect(unsetNpub).not.toHaveBeenCalled() + }) + + it("records the target as intent before the claim, and as outcome after", async () => { + // The claim can still lose to a concurrent one, so the release line must + // not assert where the key went — an investigator reading it would put + // the key on an account that never received it. + await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(info).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ reassignToAccountId: TARGET_ACCOUNT_ID }), + expect.any(String), + ) + expect(info.mock.calls[0][0]).not.toHaveProperty("reassignedToAccountId") + expect(info).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + reassignedToAccountId: TARGET_ACCOUNT_ID, + previousNpub: NPUB, + releasedByUserId: SUPPORT_USER_ID, + }), + expect.any(String), + ) + }) + + describe("when the claim fails after the release landed", () => { + beforeEach(() => { + claimNpub.mockResolvedValue(new DuplicateKeyForPersistError()) + }) + + it("reports a claim lost in the window between the two writes", async () => { + // The release and the claim are not one transaction — this repository + // has no mongo sessions — so a squatter can take the key back in + // between. The unique index catches it; the operator must be told the + // reassignment did not land. + const result = await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(result).toMatchObject({ + reassignmentError: expect.any(NpubNotAvailableError), + }) + }) + + it("still reports the release that did land", async () => { + // Returning a bare error would discard both, leaving the operator + // unaware that the key is now unclaimed and that re-running the + // mutation refuses with `NoNpubToReleaseError`. `previousNpub` is what + // they feed to `accountDetailsByNpub` to find the new holder. + const result = await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(result).not.toBeInstanceOf(Error) + expect(result).toMatchObject({ + account: { id: ACCOUNT_ID }, + previousNpub: NPUB, + }) + expect(result).not.toMatchObject({ reassignedTo: expect.anything() }) + }) + + it("logs the partial application rather than a reassignment", async () => { + await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(error).toHaveBeenCalledWith( + expect.objectContaining({ + accountId: ACCOUNT_ID, + previousNpub: NPUB, + reassignToAccountId: TARGET_ACCOUNT_ID, + releasedByUserId: SUPPORT_USER_ID, + }), + expect.any(String), + ) + expect(info).toHaveBeenCalledTimes(1) + }) + + it("passes a non-collision claim failure through the same way", async () => { + claimNpub.mockResolvedValue(new UnknownRepositoryError("mongo down")) + + const result = await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(result).toMatchObject({ + previousNpub: NPUB, + reassignmentError: expect.any(UnknownRepositoryError), + }) + }) + + it("reports a target that claimed a different key after the pre-release check", async () => { + // The "target holds no npub" check above is a read from before the + // release round-trip. If the target links a different key via + // `userUpdateNpub` in that window, `claimNpub`'s write-time filter + // refuses rather than silently overwriting the just-claimed key — and + // the refusal must reach the operator as a reassignment failure on an + // otherwise-landed release, not vanish. + claimNpub.mockResolvedValue(new AccountAlreadyHasNpubError(TARGET_ACCOUNT_ID)) + + const result = await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(result).not.toBeInstanceOf(Error) + expect(result).toMatchObject({ + previousNpub: NPUB, + reassignmentError: expect.any(AccountAlreadyHasNpubError), + }) + expect(result).not.toMatchObject({ reassignedTo: expect.anything() }) + }) + + describe("naming which failure it was", () => { + /** + * Three different causes reach the one "npub released but reassignment + * failed" line, and each has its own recovery: the key is gone and must + * be hunted with `accountDetailsByNpub`; the key is unclaimed and the + * call should be re-run against another target; or the write failed and + * the same call should be retried. Without a discriminator the operator + * gets all three procedures at 3am behind one identical line — and the + * admin server never assigns `req.gqlContext`, so there is no request + * log to fall back on. + */ + const causes: [string, ApplicationError, string][] = [ + // The raw repository error, not the `NpubNotAvailableError` the + // caller is handed: logging the mapped name would hide that this was + // the unique index firing. + [ + "a key claimed by someone else in the window", + new DuplicateKeyForPersistError(), + "DuplicateKeyForPersistError", + ], + [ + "a target that self-claimed a different key", + new AccountAlreadyHasNpubError(TARGET_ACCOUNT_ID), + "AccountAlreadyHasNpubError", + ], + [ + "a write that failed outright", + new UnknownRepositoryError("mongo down"), + "UnknownRepositoryError", + ], + ] + + it.each(causes)("names %s", async (_label, failure, expectedReason) => { + claimNpub.mockResolvedValue(failure) + + await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + + expect(error).toHaveBeenCalledWith( + expect.objectContaining({ reason: expectedReason }), + expect.any(String), + ) + }) + + it("gives every cause a distinct reason", async () => { + // A shared or empty `name` would satisfy the assertions above while + // still collapsing the three recoveries into one line. + const reasons: unknown[] = [] + for (const [, failure] of causes) { + error.mockReset() + claimNpub.mockResolvedValue(failure) + await release({ reassignToAccountId: TARGET_ACCOUNT_ID }) + reasons.push(error.mock.calls[0][0].reason) + } + + expect(new Set(reasons).size).toBe(causes.length) + }) + }) + }) + }) +}) diff --git a/test/flash/unit/app/accounts/set-npub.spec.ts b/test/flash/unit/app/accounts/set-npub.spec.ts new file mode 100644 index 000000000..2736367f1 --- /dev/null +++ b/test/flash/unit/app/accounts/set-npub.spec.ts @@ -0,0 +1,105 @@ +/** + * `setNpub` used to write whatever the caller sent, with no duplicate check. + * Combined with `findByNpub` being a `findOne`, two accounts claiming the same + * npub resolved nondeterministically — and since `accountDetailsByNpub` shipped, + * that resolution paints a customer's phone/email/level onto a support agent's + * screen. These tests pin the refusal. + */ +const findByNpub = jest.fn() +const findById = jest.fn() +const update = jest.fn() + +jest.mock("@services/mongoose", () => ({ + AccountsRepository: () => ({ findByNpub, findById, update }), +})) + +import { + CouldNotFindAccountFromNpubError, + DuplicateKeyForPersistError, + UnknownRepositoryError, +} from "@domain/errors" +import { InvalidNpubError, NpubNotAvailableError } from "@domain/nostr" +import { setNpub } from "@app/accounts/set-npub" + +const NPUB = ("npub1" + "q".repeat(58)) as Npub +const ACCOUNT_ID = "account-id" as AccountId +const OTHER_ACCOUNT_ID = "other-account-id" as AccountId + +describe("Accounts.setNpub", () => { + beforeEach(() => { + findByNpub.mockReset().mockResolvedValue(new CouldNotFindAccountFromNpubError(NPUB)) + findById.mockReset().mockResolvedValue({ id: ACCOUNT_ID }) + update.mockReset().mockImplementation(async (account) => account) + }) + + it("links an unclaimed npub", async () => { + const result = await setNpub({ id: ACCOUNT_ID, npub: NPUB }) + + expect(result).toEqual({ id: ACCOUNT_ID, npub: NPUB }) + expect(update).toHaveBeenCalledWith({ id: ACCOUNT_ID, npub: NPUB }) + }) + + it("refuses an npub already claimed by another account", async () => { + // THE IDENTITY COLLISION. Restoring a nostr key on a second account — or + // deliberately pasting a victim's npub — used to just overwrite. + findByNpub.mockResolvedValue({ id: OTHER_ACCOUNT_ID, npub: NPUB }) + + const result = await setNpub({ id: ACCOUNT_ID, npub: NPUB }) + + expect(result).toBeInstanceOf(NpubNotAvailableError) + expect(update).not.toHaveBeenCalled() + }) + + it("is idempotent when the account already owns the npub", async () => { + const owned = { id: ACCOUNT_ID, npub: NPUB } + findByNpub.mockResolvedValue(owned) + + expect(await setNpub({ id: ACCOUNT_ID, npub: NPUB })).toBe(owned) + expect(update).not.toHaveBeenCalled() + }) + + it("rejects a malformed npub without touching the repository", async () => { + const result = await setNpub({ id: ACCOUNT_ID, npub: "nope" as Npub }) + + expect(result).toBeInstanceOf(InvalidNpubError) + expect(findByNpub).not.toHaveBeenCalled() + expect(update).not.toHaveBeenCalled() + }) + + it("normalises case before claiming", async () => { + await setNpub({ id: ACCOUNT_ID, npub: ("npub1" + "Q".repeat(58)) as Npub }) + + expect(findByNpub).toHaveBeenCalledWith(NPUB) + expect(update).toHaveBeenCalledWith({ id: ACCOUNT_ID, npub: NPUB }) + }) + + it("reports the lost concurrent-write race as NpubNotAvailable, not an unexpected error", async () => { + // Both callers pass the probe; the loser's write trips the unique index. + // `DuplicateKeyForPersistError` maps to UnexpectedClientError ("contact + // support") at the GraphQL edge — the wrong answer for a benign race. + update.mockResolvedValue(new DuplicateKeyForPersistError()) + + const result = await setNpub({ id: ACCOUNT_ID, npub: NPUB }) + + expect(result).toBeInstanceOf(NpubNotAvailableError) + expect(result).not.toBeInstanceOf(DuplicateKeyForPersistError) + }) + + it("passes through other repository failures from the write", async () => { + update.mockResolvedValue(new UnknownRepositoryError("mongo down")) + + const result = await setNpub({ id: ACCOUNT_ID, npub: NPUB }) + + expect(result).toBeInstanceOf(UnknownRepositoryError) + }) + + it("does not claim the npub when the uniqueness probe itself fails", async () => { + // A repository failure is not evidence that the npub is free. + findByNpub.mockResolvedValue(new UnknownRepositoryError("mongo down")) + + const result = await setNpub({ id: ACCOUNT_ID, npub: NPUB }) + + expect(result).toBeInstanceOf(UnknownRepositoryError) + expect(update).not.toHaveBeenCalled() + }) +}) diff --git a/test/flash/unit/app/admin/get-account-by-npub.spec.ts b/test/flash/unit/app/admin/get-account-by-npub.spec.ts new file mode 100644 index 000000000..eda9fefe4 --- /dev/null +++ b/test/flash/unit/app/admin/get-account-by-npub.spec.ts @@ -0,0 +1,55 @@ +/** + * `Admin.getAccountByNpub` is the admin barrel's name for `Accounts.findByNpub` + * — a re-export, not a second implementation, so a fix cannot land in one and + * not the other. The import path here is deliberately the admin module rather + * than the accounts one: it is what proves the re-export is actually wired, so + * the behaviour the admin GraphQL resolver depends on stays covered even though + * the code lives elsewhere. The repository is the mock and the app function is + * real. + */ +const findByNpub = jest.fn() + +jest.mock("@services/mongoose", () => ({ + AccountsRepository: () => ({ findByNpub }), + UsersRepository: () => ({}), +})) + +import { InvalidNpubError } from "@domain/nostr" +import { getAccountByNpub } from "@app/admin/get-account-by-npub" + +const NPUB = ("npub1" + "q".repeat(58)) as Npub + +describe("Admin.getAccountByNpub", () => { + beforeEach(() => { + findByNpub.mockReset() + }) + + it("hands a valid npub to the repository", async () => { + const account = { id: "account-id", npub: NPUB } + findByNpub.mockResolvedValue(account) + + expect(await getAccountByNpub(NPUB)).toBe(account) + expect(findByNpub).toHaveBeenCalledWith(NPUB) + }) + + it("normalises case before querying", async () => { + // The repository query is a plain `$eq` with no collation, so normalisation + // has to happen before it or a mixed-case npub is a false not-found. + findByNpub.mockResolvedValue({ id: "account-id" }) + + await getAccountByNpub(("npub1" + "Q".repeat(58)) as Npub) + + expect(findByNpub).toHaveBeenCalledWith(NPUB) + }) + + it("rejects a malformed npub instead of querying with it", async () => { + // Defence in depth for callers that are NOT the GraphQL boundary — a + // script, a backfill, a REST shim. Before the fix the parameter was a raw + // `string` laundered with `as Npub`, so those callers got a silent + // not-found rather than a validation error. + const result = await getAccountByNpub("not-an-npub" as Npub) + + expect(result).toBeInstanceOf(InvalidNpubError) + expect(findByNpub).not.toHaveBeenCalled() + }) +}) diff --git a/test/flash/unit/graphql/admin/account-details-by-npub.spec.ts b/test/flash/unit/graphql/admin/account-details-by-npub.spec.ts new file mode 100644 index 000000000..d7a8aef41 --- /dev/null +++ b/test/flash/unit/graphql/admin/account-details-by-npub.spec.ts @@ -0,0 +1,234 @@ +/** + * Schema-execution spec for the admin `accountDetailsByNpub` query. + * + * Deliberately NOT a direct `.resolve(...)` call. Calling the resolver by hand + * covers only the eight lines of boilerplate and leaves the two things that + * actually broke untested: the `Npub` scalar's coercion (its `parseLiteral` + * shipped without a `return`, rejecting every inline-literal npub) and the + * query's registration on the admin schema (deleting that line was invisible + * to the old spec). + * + * So: real field definition, real `Npub` scalar, real resolver, real error map, + * and the committed SDL for registration. Only the app layer is mocked — it is + * the boundary this file is not responsible for. + */ +const mockGetAccountByNpub = jest.fn() +const mockGetUser = jest.fn() + +jest.mock("@app", () => ({ + Admin: { + getAccountByNpub: (...args: unknown[]) => mockGetAccountByNpub(...args), + }, + Users: { + getUser: (...args: unknown[]) => mockGetUser(...args), + }, + Accounts: { getAccountCapabilities: jest.fn() }, + Wallets: { listWalletsByAccountId: jest.fn() }, + Merchants: { getMerchantsByUsername: jest.fn() }, +})) + +import fs from "fs" +import path from "path" + +import { + graphql, + parse, + validate, + GraphQLSchema, + GraphQLObjectType, + GraphQLFieldConfig, +} from "graphql" + +import { CouldNotFindAccountFromNpubError } from "@domain/errors" +import AccountDetailsByNpubQuery from "@graphql/admin/root/query/account-details-by-npub" + +const VALID_NPUB = "npub1" + "q".repeat(58) +const UPPERCASE_NPUB = "npub1" + "Q".repeat(58) +const MALFORMED_NPUB = "npub1toshort" + +/** + * The consumer's actual document — frappe-flash-admin + * `admin_panel/api/support_lookup.py` paints these fields onto a Chatwoot + * contact card. + */ +const SUPPORT_LOOKUP_QUERY = ` + query accountDetailsByNpub($npub: npub!) { + accountDetailsByNpub(npub: $npub) { + npub + username + level + owner { + phone + } + } + } +` + +const literalQuery = (npub: string) => ` + query { + accountDetailsByNpub(npub: "${npub}") { + npub + username + level + owner { + phone + } + } + } +` + +// One-field schema over the REAL field definition, so the scalar and the +// resolver both run. Two things this deliberately is not. It is not +// `buildSchema` over the checked-in SDL: that yields a default resolver and a +// stub scalar, and would pass with the feature deleted. And it is not +// `@graphql/admin/queries`: importing that barrel drags in the cash-wallet +// cutover query, which constructs Redis clients at import time and hangs the +// runner — registration is asserted against the committed SDL instead. +const adminSchema = () => + new GraphQLSchema({ + query: new GraphQLObjectType({ + name: "Query", + fields: { + accountDetailsByNpub: AccountDetailsByNpubQuery as unknown as GraphQLFieldConfig< + unknown, + unknown + >, + }, + }), + }) + +const account = { + id: "account-id", + uuid: "5a9f6f45-0a3a-4b0a-9f3e-1e0f9b1b1b1b", + username: "jaceth2009", + npub: VALID_NPUB, + level: 1, + kratosUserId: "kratos-user-id", +} + +const owner = { + id: "kratos-user-id", + phone: "+18765550100", + language: "en", + createdAt: new Date(1787340000 * 1000), +} + +describe("admin accountDetailsByNpub", () => { + beforeEach(() => { + jest.clearAllMocks() + mockGetUser.mockResolvedValue(owner) + }) + + it("is exposed on the published admin schema", () => { + // Guards the registration in src/graphql/admin/queries.ts. Unwire it and + // every resolver-level test still passes green, so assert the field the + // consumer actually calls exists on the committed SDL — which `yarn + // check:sdl` keeps in lockstep with the code. + const sdl = fs.readFileSync( + path.join(__dirname, "../../../../../src/graphql/admin/schema.graphql"), + "utf8", + ) + expect(sdl).toContain("accountDetailsByNpub(npub: npub!): AuditedAccount!") + expect(sdl).toContain("scalar npub") + }) + + it("validates the consumer's document against the real field", () => { + expect(validate(adminSchema(), parse(SUPPORT_LOOKUP_QUERY))).toHaveLength(0) + }) + + it("resolves the support-lookup document through a variable", async () => { + mockGetAccountByNpub.mockResolvedValue(account) + + const result = await graphql({ + schema: adminSchema(), + source: SUPPORT_LOOKUP_QUERY, + variableValues: { npub: VALID_NPUB }, + }) + + expect(result.errors).toBeUndefined() + expect(mockGetAccountByNpub).toHaveBeenCalledWith(VALID_NPUB) + expect(result.data?.accountDetailsByNpub).toEqual({ + npub: VALID_NPUB, + username: "jaceth2009", + level: "ONE", + owner: { phone: "+18765550100" }, + }) + }) + + it("resolves the same npub written as an inline literal", async () => { + // THE REGRESSION. `parseLiteral` returned undefined on its success branch, + // which graphql-js reads as failed coercion: every inline-literal npub was + // rejected at validation time with "Expected value of type npub!" — a + // perfectly valid key reported as malformed to anyone in GraphiQL. + mockGetAccountByNpub.mockResolvedValue(account) + + const source = literalQuery(VALID_NPUB) + expect(validate(adminSchema(), parse(source))).toHaveLength(0) + + const result = await graphql({ schema: adminSchema(), source }) + + expect(result.errors).toBeUndefined() + expect(mockGetAccountByNpub).toHaveBeenCalledWith(VALID_NPUB) + expect(result.data?.accountDetailsByNpub).toMatchObject({ npub: VALID_NPUB }) + }) + + it("normalises case identically on both coercion paths", async () => { + // The repository query is a plain `$eq` with no collation, so the scalar is + // the only thing standing between a mixed-case npub and a false not-found. + mockGetAccountByNpub.mockResolvedValue(account) + + await graphql({ + schema: adminSchema(), + source: SUPPORT_LOOKUP_QUERY, + variableValues: { npub: UPPERCASE_NPUB }, + }) + await graphql({ schema: adminSchema(), source: literalQuery(UPPERCASE_NPUB) }) + + expect(mockGetAccountByNpub).toHaveBeenNthCalledWith(1, VALID_NPUB) + expect(mockGetAccountByNpub).toHaveBeenNthCalledWith(2, VALID_NPUB) + }) + + it("rejects a malformed npub variable before it reaches the app layer", async () => { + const result = await graphql({ + schema: adminSchema(), + source: SUPPORT_LOOKUP_QUERY, + variableValues: { npub: MALFORMED_NPUB }, + }) + + expect(result.errors?.length).toBeGreaterThan(0) + expect(mockGetAccountByNpub).not.toHaveBeenCalled() + }) + + it("rejects a malformed npub literal before it reaches the app layer", async () => { + const result = await graphql({ + schema: adminSchema(), + source: literalQuery(MALFORMED_NPUB), + }) + + expect(result.errors?.length).toBeGreaterThan(0) + expect(mockGetAccountByNpub).not.toHaveBeenCalled() + }) + + it("reports a miss as an npub miss, not a username miss", async () => { + // The not-found path of a support-desk lookup lands in logs and in front of + // humans. Before the fix the repository returned + // CouldNotFindAccountFromUsernameError, so the operator was told + // "Account does not exist for username npub1…". + mockGetAccountByNpub.mockResolvedValue( + new CouldNotFindAccountFromNpubError(VALID_NPUB), + ) + + const result = await graphql({ + schema: adminSchema(), + source: SUPPORT_LOOKUP_QUERY, + variableValues: { npub: VALID_NPUB }, + }) + + expect(result.errors?.[0].message).toBe( + `Account does not exist for npub ${VALID_NPUB}`, + ) + expect(result.errors?.[0].message).not.toContain("username") + // The consumer keys off the NOT_FOUND code, so the 404 path is unchanged. + expect(result.errors?.[0].extensions?.code).toBe("NOT_FOUND") + }) +}) diff --git a/test/flash/unit/graphql/admin/account-release-npub.spec.ts b/test/flash/unit/graphql/admin/account-release-npub.spec.ts new file mode 100644 index 000000000..95a66818f --- /dev/null +++ b/test/flash/unit/graphql/admin/account-release-npub.spec.ts @@ -0,0 +1,239 @@ +/** + * `accountReleaseNpub` is the admin-side half of the npub squat remedy: without + * it the admin surface is read-only and a wrongly-claimed npub can only be + * freed by hand-editing prod mongo. These tests pin the resolver's contract and + * its registration on the admin schema. + */ +const mockReleaseNpub = jest.fn() + +// The admin mutation barrel is imported below for the registration guard, and +// it drags in siblings whose service graph opens a redis connection at import +// time — which never resolves in a unit run. +jest.mock("@services/redis/connection", () => ({ + redis: { on: jest.fn() }, + redisPubSub: { publish: jest.fn(), asyncIterator: jest.fn() }, + redisCache: { cache: jest.fn(), invalidate: jest.fn() }, + disconnectAll: jest.fn(), +})) + +jest.mock("@app", () => ({ + Accounts: { + releaseNpub: (...args: unknown[]) => mockReleaseNpub(...args), + getAccountCapabilities: jest.fn(), + }, + Admin: { getAccountByNpub: jest.fn() }, + Users: { getUser: jest.fn() }, + Wallets: { listWalletsByAccountId: jest.fn() }, + Merchants: { getMerchantsByUsername: jest.fn() }, +})) + +import { InvalidAccountIdError } from "@domain/accounts" +import { CouldNotFindAccountFromIdError, NoNpubToReleaseError } from "@domain/errors" +import { AccountAlreadyHasNpubError, NpubNotAvailableError } from "@domain/nostr" +import { mutationFields } from "@graphql/admin/mutations" +import AccountReleaseNpubMutation from "@graphql/admin/root/mutation/account-release-npub" + +const ACCOUNT_ID = "5f4c9a2b1e7d3f8a6b0c4d2e" +const TARGET_ACCOUNT_ID = "6a1b2c3d4e5f60718293a4b5" +const NPUB = `npub1${"q".repeat(58)}` + +type Result = { + errors: { message: string; code?: string }[] + accountDetails?: { id: string; npub?: string } + previousNpub?: string + reassignedTo?: { id: string; npub?: string } + reassignmentError?: { message: string; code?: string } +} + +// Mirrors what graphql-admin-server's Apollo `context` fn builds from the +// decoded admin JWT. +const adminContext = () => ({ + logger: { error: jest.fn() }, + user: { id: "support-user-id", roles: ["support"], ip: "127.0.0.1" }, +}) + +const resolveMutation = async (input: Record): Promise => { + const resolve = AccountReleaseNpubMutation.resolve as unknown as ( + source: null, + args: { input: Record }, + ctx: Record, + ) => Promise + + return resolve(null, { input }, adminContext()) +} + +describe("accountReleaseNpub", () => { + beforeEach(() => { + jest.clearAllMocks() + mockReleaseNpub.mockResolvedValue({ + account: { id: ACCOUNT_ID, username: "jaceth2009" }, + previousNpub: NPUB, + }) + }) + + it("is registered as an authed admin mutation", () => { + // The source of truth, not the checked-in SDL: `MutationType` spreads + // `unauthed` and `authed` into identical SDL, so `check:sdl` stays green + // when the field is moved out of the graphql-shield-guarded bucket and + // becomes callable by any holder of a role-less ERPNext JWT. + expect(mutationFields.authed).toHaveProperty( + "accountReleaseNpub", + AccountReleaseNpubMutation, + ) + expect(mutationFields.unauthed).not.toHaveProperty("accountReleaseNpub") + }) + + it("releases the npub and reports which key was freed", async () => { + const result = await resolveMutation({ accountId: ACCOUNT_ID }) + + expect(mockReleaseNpub).toHaveBeenCalledWith({ + id: ACCOUNT_ID, + releasedByUserId: "support-user-id", + reassignToAccountId: undefined, + }) + expect(result.errors).toEqual([]) + expect(result.previousNpub).toBe(NPUB) + }) + + it("attributes the release to the calling admin", async () => { + // The account document keeps no trace of a removed npub, and the admin + // server never assigns `req.gqlContext`, so its Pino line logs the actor as + // undefined. Dropping `ctx` here would leave no record of who freed a key. + await resolveMutation({ accountId: ACCOUNT_ID }) + + expect(mockReleaseNpub.mock.calls[0][0]).toMatchObject({ + releasedByUserId: "support-user-id", + }) + }) + + it("passes the reassignment target through and returns the new holder", async () => { + mockReleaseNpub.mockResolvedValue({ + account: { id: ACCOUNT_ID, username: "jaceth2009" }, + previousNpub: NPUB, + reassignedTo: { id: TARGET_ACCOUNT_ID, npub: NPUB }, + }) + + const result = await resolveMutation({ + accountId: ACCOUNT_ID, + reassignToAccountId: TARGET_ACCOUNT_ID, + }) + + expect(mockReleaseNpub).toHaveBeenCalledWith({ + id: ACCOUNT_ID, + releasedByUserId: "support-user-id", + reassignToAccountId: TARGET_ACCOUNT_ID, + }) + expect(result.errors).toEqual([]) + expect(result.reassignedTo?.npub).toBe(NPUB) + }) + + it("reports an unknown account as a not-found, not an internal code", async () => { + // A support agent told the key is free would send the rightful owner off to + // re-link, and `setNpub` would refuse them again. Both realistic operator + // mistakes — wrong id, and the uuid pasted where the ObjectId goes — used + // to surface as a leaked class name or "contact support". + mockReleaseNpub.mockResolvedValue(new CouldNotFindAccountFromIdError(ACCOUNT_ID)) + + const result = await resolveMutation({ accountId: ACCOUNT_ID }) + + expect(result.accountDetails).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0].code).toBe("NOT_FOUND") + expect(result.errors[0].message).toContain(ACCOUNT_ID) + expect(result.errors[0].message).not.toContain("contact support") + expect(result.errors[0].message).not.toContain("CouldNotFind") + }) + + it("reports a malformed account id as a validation failure", async () => { + mockReleaseNpub.mockResolvedValue(new InvalidAccountIdError("not-an-account-id")) + + const result = await resolveMutation({ accountId: "not-an-account-id" }) + + expect(result.accountDetails).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0].code).toBe("INVALID_INPUT") + expect(result.errors[0].message).not.toContain("contact support") + expect(result.errors[0].message).not.toContain("InvalidAccountIdError") + }) + + it("reports an account that holds no npub as a not-found", async () => { + mockReleaseNpub.mockResolvedValue(new NoNpubToReleaseError(ACCOUNT_ID)) + + const result = await resolveMutation({ accountId: ACCOUNT_ID }) + + expect(result.errors[0].code).toBe("NOT_FOUND") + expect(result.previousNpub).toBeUndefined() + }) + + it("reports a reassignment target that already holds a key", async () => { + mockReleaseNpub.mockResolvedValue(new AccountAlreadyHasNpubError(TARGET_ACCOUNT_ID)) + + const result = await resolveMutation({ + accountId: ACCOUNT_ID, + reassignToAccountId: TARGET_ACCOUNT_ID, + }) + + expect(result.errors[0].code).toBe("INVALID_INPUT") + expect(result.previousNpub).toBeUndefined() + }) + + describe("a reassignment that failed after the release landed", () => { + // The two writes are not a transaction, so the app layer reports this as a + // populated release carrying `reassignmentError` rather than as an error. + // Collapsing it back to `{ errors }` here would throw away the only signal + // that the key has already left the holder. + beforeEach(() => { + mockReleaseNpub.mockResolvedValue({ + account: { id: ACCOUNT_ID, username: "jaceth2009" }, + previousNpub: NPUB, + reassignmentError: new NpubNotAvailableError(NPUB), + }) + }) + + it("still returns the account the key was taken off, and the key", async () => { + const result = await resolveMutation({ + accountId: ACCOUNT_ID, + reassignToAccountId: TARGET_ACCOUNT_ID, + }) + + expect(result.accountDetails).toMatchObject({ id: ACCOUNT_ID }) + expect(result.previousNpub).toBe(NPUB) + expect(result.reassignedTo).toBeUndefined() + }) + + it("names the failure as the reassignment's, not the release's", async () => { + const result = await resolveMutation({ + accountId: ACCOUNT_ID, + reassignToAccountId: TARGET_ACCOUNT_ID, + }) + + expect(result.reassignmentError?.message).not.toContain("NpubNotAvailable") + expect(result.errors).toHaveLength(1) + expect(result.errors[0]).toEqual(result.reassignmentError) + }) + }) + + it("leaves the reassignment error unset on a clean release", async () => { + const result = await resolveMutation({ accountId: ACCOUNT_ID }) + + expect(result.reassignmentError).toBeUndefined() + expect(result.errors).toEqual([]) + }) + + it("surfaces an input coercion failure without calling the app layer", async () => { + const result = await resolveMutation({ accountId: new Error("Invalid value for ID") }) + + expect(result.errors).toEqual([{ message: "Invalid value for ID" }]) + expect(mockReleaseNpub).not.toHaveBeenCalled() + }) + + it("surfaces a coercion failure on the reassignment target too", async () => { + const result = await resolveMutation({ + accountId: ACCOUNT_ID, + reassignToAccountId: new Error("Invalid value for ID"), + }) + + expect(result.errors).toEqual([{ message: "Invalid value for ID" }]) + expect(mockReleaseNpub).not.toHaveBeenCalled() + }) +}) diff --git a/test/flash/unit/migrations/accounts-unique-npub.spec.ts b/test/flash/unit/migrations/accounts-unique-npub.spec.ts new file mode 100644 index 000000000..721182893 --- /dev/null +++ b/test/flash/unit/migrations/accounts-unique-npub.spec.ts @@ -0,0 +1,254 @@ +/* eslint @typescript-eslint/no-var-requires: "off" */ + +/** + * The two destructive branches of the `accounts.npub` migration — case + * normalisation and duplicate release — are unreachable in CI: `make + * test-migrate` runs against a clean database, so the collection is empty and + * both branches short-circuit. Without this file, the first execution of + * `$unset: { npub: "" }` would be against real customer identities. + * + * The stub below is a miniature mongo: `updateMany` actually mutates the + * documents, so the pipeline's ordering (lowercase, then group) is exercised + * rather than asserted on canned return values. + */ + +type Doc = { + _id: string + id: string + npub?: string + created_at?: string +} + +type IndexSpec = { name: string; unique?: boolean; key: Record } + +const isDuplicatePipeline = (pipeline: Record[]) => + pipeline.some((stage) => "$group" in stage) + +const makeDb = ({ docs, indexes }: { docs: Doc[]; indexes?: IndexSpec[] }) => { + const state = { docs, indexes: indexes ?? ([] as IndexSpec[]) } + + const aggregate = jest.fn((pipeline: Record[]) => ({ + toArray: async () => { + const withNpub = state.docs.filter((d) => typeof d.npub === "string") + + if (!isDuplicatePipeline(pipeline)) { + return withNpub + .filter((d) => d.npub !== (d.npub as string).toLowerCase()) + .map((d) => ({ _id: d._id, id: d.id, npub: d.npub })) + } + + const groups = new Map() + for (const d of withNpub) { + const key = d.npub as string + groups.set(key, [...(groups.get(key) ?? []), d]) + } + return [...groups.entries()] + .filter(([, members]) => members.length > 1) + .map(([npub, members]) => ({ + _id: npub, + count: members.length, + docs: members.map((d) => ({ + _id: d._id, + id: d.id, + created_at: d.created_at, + })), + })) + }, + })) + + const updateMany = jest.fn( + async ( + filter: { _id: { $in: string[] } }, + update: Record | Record[], + ) => { + const targeted = filter._id.$in + state.docs = state.docs.map((doc) => { + if (!targeted.includes(doc._id)) return doc + if (Array.isArray(update)) return { ...doc, npub: doc.npub?.toLowerCase() } + const released = { ...doc } + delete released.npub + return released + }) + return { modifiedCount: targeted.length } + }, + ) + + const createIndex = jest.fn( + async (key: Record, opts: { name: string; unique?: boolean }) => { + state.indexes = [...state.indexes, { key, ...opts }] + return opts.name + }, + ) + + const dropIndex = jest.fn(async (name: string) => { + state.indexes = state.indexes.filter((idx) => idx.name !== name) + }) + + const collection = jest.fn(() => ({ + aggregate, + updateMany, + createIndex, + dropIndex, + indexes: async () => state.indexes, + })) + + const db = { + collection, + listCollections: () => ({ toArray: async () => [{ name: "accounts" }] }), + } + + return { db, state, aggregate, updateMany, createIndex, dropIndex } +} + +const migration = require("../../../../src/migrations/20260824120000-accounts-unique-npub") + +const LOWER = "npub1" + "q".repeat(58) +const MIXED = "npub1" + "Q".repeat(58) + +describe("migration: accounts unique npub", () => { + beforeEach(() => { + jest.spyOn(console, "log").mockImplementation(() => undefined) + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + it("lowercases only the offending documents", async () => { + const { db, state, updateMany } = makeDb({ + docs: [ + { _id: "1", id: "acct-1", npub: MIXED, created_at: "2024-01-01" }, + { + _id: "2", + id: "acct-2", + npub: "npub1" + "z".repeat(58), + created_at: "2024-01-01", + }, + ], + }) + + await migration.up(db) + + expect(state.docs.find((d) => d._id === "1")?.npub).toEqual(LOWER) + // The write is scoped to the ids the audit scan found — not to every + // npub-bearing account, which would churn the oplog for zero repairs. + expect(updateMany).toHaveBeenCalledWith({ _id: { $in: ["1"] } }, [ + { $set: { npub: { $toLower: "$npub" } } }, + ]) + }) + + it("does not write at all when every npub is already lowercase", async () => { + const { db, updateMany } = makeDb({ + docs: [{ _id: "1", id: "acct-1", npub: LOWER, created_at: "2024-01-01" }], + }) + + await migration.up(db) + + expect(updateMany).not.toHaveBeenCalled() + }) + + it("lowercases before the duplicate scan, so case-variant collisions are caught", async () => { + const { db, state } = makeDb({ + docs: [ + { _id: "1", id: "acct-1", npub: MIXED, created_at: "2024-01-01" }, + { _id: "2", id: "acct-2", npub: LOWER, created_at: "2025-01-01" }, + ], + }) + + await migration.up(db) + + // Same key once normalised — both must be released. + expect(state.docs.every((d) => d.npub === undefined)).toBe(true) + }) + + it("releases the npub from EVERY account in a duplicate group, including the oldest", async () => { + // Picking a winner by created_at is unrecoverable when it picks wrong: + // `setNpub` refuses an already-claimed npub and no admin mutation can + // release one, so the loser could never re-link. + const { db, state, updateMany } = makeDb({ + docs: [ + { _id: "old", id: "acct-old", npub: LOWER, created_at: "2024-01-01" }, + { _id: "new", id: "acct-new", npub: LOWER, created_at: "2025-01-01" }, + ], + }) + + await migration.up(db) + + expect(state.docs.map((d) => d.npub)).toEqual([undefined, undefined]) + expect(updateMany).toHaveBeenCalledWith( + { _id: { $in: ["old", "new"] } }, + { $unset: { npub: "" } }, + ) + }) + + it("leaves uncontested npubs alone", async () => { + const { db, state } = makeDb({ + docs: [ + { _id: "1", id: "acct-1", npub: LOWER, created_at: "2024-01-01" }, + { + _id: "2", + id: "acct-2", + npub: "npub1" + "z".repeat(58), + created_at: "2024-01-01", + }, + { _id: "3", id: "acct-3", created_at: "2024-01-01" }, + ], + }) + + await migration.up(db) + + expect(state.docs.map((d) => d.npub)).toEqual([ + LOWER, + "npub1" + "z".repeat(58), + undefined, + ]) + }) + + it("creates the unique index with the string-typed partial filter", async () => { + const { db, createIndex } = makeDb({ docs: [] }) + + await migration.up(db) + + // `sparse: true` would still index documents holding an explicit + // `npub: null`, and the second such document would collide. + expect(createIndex).toHaveBeenCalledWith( + { npub: 1 }, + { + unique: true, + name: "npub_1", + partialFilterExpression: { npub: { $type: "string" } }, + }, + ) + }) + + it("drops a stale non-unique npub_1 before creating the unique one", async () => { + const { db, dropIndex, state } = makeDb({ + docs: [], + indexes: [{ name: "npub_1", key: { npub: 1 } }], + }) + + await migration.up(db) + + expect(dropIndex).toHaveBeenCalledWith("npub_1") + expect(state.indexes.filter((i) => i.name === "npub_1")).toEqual([ + { + key: { npub: 1 }, + unique: true, + name: "npub_1", + partialFilterExpression: { npub: { $type: "string" } }, + }, + ]) + }) + + it("down drops the unique index and leaves the data repairs in place", async () => { + const { db, state, updateMany } = makeDb({ + docs: [{ _id: "1", id: "acct-1", created_at: "2024-01-01" }], + indexes: [{ name: "npub_1", key: { npub: 1 }, unique: true }], + }) + + await migration.down(db) + + expect(state.indexes).toEqual([]) + expect(updateMany).not.toHaveBeenCalled() + }) +}) diff --git a/test/flash/unit/services/mongoose/account-npub-index.spec.ts b/test/flash/unit/services/mongoose/account-npub-index.spec.ts new file mode 100644 index 000000000..e2fcd2c3c --- /dev/null +++ b/test/flash/unit/services/mongoose/account-npub-index.spec.ts @@ -0,0 +1,23 @@ +import { Account } from "@services/mongoose/schema" + +describe("accounts.npub index", () => { + it("is unique, so an npub can only resolve to one account", () => { + // Without this, `setNpub`'s duplicate check is racy and `findByNpub` — now + // a support-desk identity resolver — can return either of two colliding + // accounts. The migration 20260824120000-accounts-unique-npub builds it in + // prod after deduping. + const npubIndex = Account.schema + .indexes() + .find(([fields]) => Object.keys(fields).join(",") === "npub") + + expect(npubIndex).toBeDefined() + + const [, options] = npubIndex as [Record, Record] + expect(options.unique).toBe(true) + // `partialFilterExpression`, not `sparse`: a sparse index still indexes + // documents holding an explicit `npub: null`, and the second such document + // would collide. + expect(options.partialFilterExpression).toEqual({ npub: { $type: "string" } }) + expect(options.sparse).toBeUndefined() + }) +}) diff --git a/test/flash/unit/services/mongoose/accounts-find-by-npub.spec.ts b/test/flash/unit/services/mongoose/accounts-find-by-npub.spec.ts new file mode 100644 index 000000000..ddb7362f2 --- /dev/null +++ b/test/flash/unit/services/mongoose/accounts-find-by-npub.spec.ts @@ -0,0 +1,68 @@ +import { CouldNotFindAccountFromNpubError } from "@domain/errors" +import { AccountsRepository } from "@services/mongoose/accounts" + +const findOne = jest.fn() + +jest.mock("@services/mongoose/schema", () => ({ + Account: { findOne: (...args: unknown[]) => findOne(...args) }, +})) + +jest.mock("@services/mongoose/utils", () => ({ + toObjectId: jest.fn((id) => id), + fromObjectId: jest.fn((id) => id), + parseRepositoryError: jest.fn((err) => err), +})) + +const NPUB = ("npub1" + "q".repeat(58)) as Npub + +const accountRecord = { + _id: "account-id", + id: "5a9f6f45-0a3a-4b0a-9f3e-1e0f9b1b1b1b", + created_at: new Date(), + npub: NPUB, + username: "jaceth2009", + level: 1, + statusHistory: [{ status: "active" }], + contacts: [], + earn: [], +} + +describe("AccountsRepository.findByNpub", () => { + beforeEach(() => { + findOne.mockReset() + }) + + it("queries npub with a plain $eq and no collation", async () => { + // `findOne` here resolves directly — it exposes no `.collation()`. That is + // the assertion: a non-simple collation cannot use the unique `{ npub: 1 }` + // index, so every support-desk lookup would be a collection scan. The old + // implementation chained `.collation({ locale: "en", strength: 2 })` and + // would blow up on this mock. + // + // The `$type` alongside the `$eq` is not decoration: the index is partial + // on `{ npub: { $type: "string" } }`, and mongo will not use a partial + // index unless the query provably matches a subset of its filter — which + // it cannot derive from an equality against a string literal. Without the + // restated predicate the lookup plans as a COLLSCAN. This mock cannot see + // that; `test/flash/integration/accounts/npub.spec.ts` explains the real + // plan and is what actually caught it. + findOne.mockResolvedValue(accountRecord) + + const result = await AccountsRepository().findByNpub(NPUB) + + expect(findOne).toHaveBeenCalledWith({ npub: { $eq: NPUB, $type: "string" } }) + expect(result).not.toBeInstanceOf(Error) + expect((result as Account).npub).toBe(NPUB) + }) + + it("reports a miss as an npub miss, not a username miss", async () => { + // The wrong error class renders as "Account does not exist for username + // npub1…" in logs and in front of a support agent. + findOne.mockResolvedValue(null) + + const result = await AccountsRepository().findByNpub(NPUB) + + expect(result).toBeInstanceOf(CouldNotFindAccountFromNpubError) + expect((result as Error).message).toBe(NPUB) + }) +}) diff --git a/test/flash/unit/services/mongoose/accounts-unset-npub.spec.ts b/test/flash/unit/services/mongoose/accounts-unset-npub.spec.ts new file mode 100644 index 000000000..6a92b7834 --- /dev/null +++ b/test/flash/unit/services/mongoose/accounts-unset-npub.spec.ts @@ -0,0 +1,139 @@ +import { CouldNotFindAccountFromIdError, NoNpubToReleaseError } from "@domain/errors" +import { AccountAlreadyHasNpubError } from "@domain/nostr" +import { AccountsRepository } from "@services/mongoose/accounts" + +const findOneAndUpdate = jest.fn() +const findOne = jest.fn() + +jest.mock("@services/mongoose/schema", () => ({ + Account: { + findOneAndUpdate: (...args: unknown[]) => findOneAndUpdate(...args), + findOne: (...args: unknown[]) => findOne(...args), + }, +})) + +jest.mock("@services/mongoose/utils", () => ({ + toObjectId: jest.fn((id) => id), + fromObjectId: jest.fn((id) => id), + parseRepositoryError: jest.fn((err) => err), +})) + +const ACCOUNT_ID = "5f4c9a2b1e7d3f8a6b0c4d2e" as AccountId +const NPUB = `npub1${"q".repeat(58)}` as Npub + +const accountRecord = { + _id: ACCOUNT_ID, + id: "5a9f6f45-0a3a-4b0a-9f3e-1e0f9b1b1b1b", + created_at: new Date(), + username: "jaceth2009", + level: 1, + statusHistory: [{ status: "active" }], + contacts: [], + earn: [], +} + +describe("AccountsRepository.unsetNpub", () => { + beforeEach(() => { + findOneAndUpdate.mockReset() + }) + + it("removes the field rather than blanking it", async () => { + // `$unset`, not `npub: undefined` (mongoose strips undefined keys from an + // update doc, so that is a silent no-op) and not `npub: null` (the partial + // index excludes non-strings, so a null would sit there unindexed and keep + // failing lookups). Removed means the key is genuinely unclaimed again. + findOneAndUpdate.mockResolvedValue({ ...accountRecord, npub: NPUB }) + + const result = await AccountsRepository().unsetNpub(ACCOUNT_ID) + + expect(findOneAndUpdate).toHaveBeenCalledWith( + { _id: ACCOUNT_ID }, + { $unset: { npub: "" } }, + { new: false }, + ) + expect(result).not.toBeInstanceOf(Error) + // The document read back is the pre-update one, which still carries the + // npub; the account handed to the caller must not. + expect((result as NpubUnset).account.npub).toBeUndefined() + // Nothing else can report it. The updated document no longer holds it, and + // a second read of the account is a later, potentially different, answer. + expect((result as NpubUnset).previousNpub).toBe(NPUB) + }) + + it("treats a legacy null npub as nothing to release", async () => { + // The partial index only covers strings, so documents predating it can + // still hold an explicit `npub: null`. `$unset` on one frees nothing. + findOneAndUpdate.mockResolvedValue({ ...accountRecord, npub: null }) + + expect(await AccountsRepository().unsetNpub(ACCOUNT_ID)).toBeInstanceOf( + NoNpubToReleaseError, + ) + }) + + it("reports an unknown account", async () => { + findOneAndUpdate.mockResolvedValue(null) + + expect(await AccountsRepository().unsetNpub(ACCOUNT_ID)).toBeInstanceOf( + CouldNotFindAccountFromIdError, + ) + }) + + it("refuses an account that held no npub", async () => { + // `$unset` on a document without the field is a no-op that still matches on + // `_id`, so the write itself reports success. Support pastes the wrong + // account id, is told the key is free, and sends the customer off to + // re-link — where `setNpub` refuses them because the squatter still has it. + findOneAndUpdate.mockResolvedValue(accountRecord) + + expect(await AccountsRepository().unsetNpub(ACCOUNT_ID)).toBeInstanceOf( + NoNpubToReleaseError, + ) + }) +}) + +describe("AccountsRepository.claimNpub", () => { + beforeEach(() => { + findOneAndUpdate.mockReset() + findOne.mockReset() + }) + + it("sets the key only on an account that holds none", async () => { + findOneAndUpdate.mockResolvedValue({ ...accountRecord, npub: NPUB }) + + const result = await AccountsRepository().claimNpub(ACCOUNT_ID, NPUB) + + // The npub guard in the filter is the write-time re-check: the caller's + // "target holds no npub" read happens before the release round-trip, so a + // key the target claims in that window must fail the match rather than be + // silently overwritten. `$not: { $type: "string" }` and not + // `$exists: false`, because legacy documents hold `npub: null`, which is + // not a claim. + expect(findOneAndUpdate).toHaveBeenCalledWith( + { _id: ACCOUNT_ID, npub: { $not: { $type: "string" } } }, + { $set: { npub: NPUB } }, + { new: true }, + ) + expect((result as Account).npub).toBe(NPUB) + }) + + it("reports an unknown account", async () => { + findOneAndUpdate.mockResolvedValue(null) + findOne.mockResolvedValue(null) + + expect(await AccountsRepository().claimNpub(ACCOUNT_ID, NPUB)).toBeInstanceOf( + CouldNotFindAccountFromIdError, + ) + }) + + it("refuses to overwrite a key the account claimed concurrently", async () => { + // The unique index cannot catch this case — it prevents duplicates, not + // overwrites. Without the filter guard, the $set would land, the target's + // just-claimed key would become unclaimed, and nothing would log it. + findOneAndUpdate.mockResolvedValue(null) + findOne.mockResolvedValue({ ...accountRecord, npub: `npub1${"z".repeat(58)}` }) + + expect(await AccountsRepository().claimNpub(ACCOUNT_ID, NPUB)).toBeInstanceOf( + AccountAlreadyHasNpubError, + ) + }) +})