From 1961ff030b9753876a7cab14097ec300daaf07b0 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 3 Jul 2026 01:38:15 +0200 Subject: [PATCH 01/10] feat(user): add additive RealUnit service-provider marker on UserData (#4045) --- ...82990000000-AddUserDataServiceProviders.js | 26 +++++++++ ...0000010-BackfillRealUnitServiceProvider.js | 55 +++++++++++++++++++ .../models/user-data/user-data.entity.spec.ts | 41 ++++++++++++++ .../user/models/user-data/user-data.entity.ts | 24 ++++++++ .../user/models/user-data/user-data.enum.ts | 6 ++ .../models/user-data/user-data.service.ts | 13 ++++- .../supporting/realunit/realunit.service.ts | 5 +- 7 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 migration/1782990000000-AddUserDataServiceProviders.js create mode 100644 migration/1782990000010-BackfillRealUnitServiceProvider.js diff --git a/migration/1782990000000-AddUserDataServiceProviders.js b/migration/1782990000000-AddUserDataServiceProviders.js new file mode 100644 index 0000000000..e8a2c009f3 --- /dev/null +++ b/migration/1782990000000-AddUserDataServiceProviders.js @@ -0,0 +1,26 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * @class + * @implements {MigrationInterface} + */ +module.exports = class AddUserDataServiceProviders1782990000000 { + name = 'AddUserDataServiceProviders1782990000000' + + /** + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + await queryRunner.query(`ALTER TABLE "user_data" ADD "serviceProviders" character varying(256)`); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + await queryRunner.query(`ALTER TABLE "user_data" DROP COLUMN "serviceProviders"`); + } +} diff --git a/migration/1782990000010-BackfillRealUnitServiceProvider.js b/migration/1782990000010-BackfillRealUnitServiceProvider.js new file mode 100644 index 0000000000..6f5af0685f --- /dev/null +++ b/migration/1782990000010-BackfillRealUnitServiceProvider.js @@ -0,0 +1,55 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * Backfills the additive RealUnit service-provider marker for existing customers. + * A userData is treated as a RealUnit customer if it has either + * (a) a user onboarded under the RealUnit wallet, or + * (b) a RealUnit (Aktionariat) registration KYC step. + * These are the durable, merge-surviving provenance signals; the RealUnit wallet name on the + * user is used only as historical backfill provenance, never as the runtime scoping anchor. + * + * @class + * @implements {MigrationInterface} + */ +module.exports = class BackfillRealUnitServiceProvider1782990000010 { + name = 'BackfillRealUnitServiceProvider1782990000010' + + /** + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + await queryRunner.query(` + UPDATE "user_data" SET "serviceProviders" = + CASE + WHEN "serviceProviders" IS NULL OR "serviceProviders" = '' THEN 'RealUnit' + WHEN ';' || "serviceProviders" || ';' LIKE '%;RealUnit;%' THEN "serviceProviders" + ELSE "serviceProviders" || ';RealUnit' + END + WHERE "id" IN ( + SELECT u."userDataId" + FROM "user" u + INNER JOIN "wallet" w ON w."id" = u."walletId" + WHERE w."name" = 'RealUnit' + UNION + SELECT ks."userDataId" + FROM "kyc_step" ks + WHERE ks."name" = 'RealUnitRegistration' + ) + `); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + await queryRunner.query(` + UPDATE "user_data" + SET "serviceProviders" = + NULLIF(array_to_string(array_remove(string_to_array("serviceProviders", ';'), 'RealUnit'), ';'), '') + WHERE ';' || "serviceProviders" || ';' LIKE '%;RealUnit;%' + `); + } +} diff --git a/src/subdomains/generic/user/models/user-data/user-data.entity.spec.ts b/src/subdomains/generic/user/models/user-data/user-data.entity.spec.ts index a3b9fd5b2f..c0b0abb64d 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.entity.spec.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.entity.spec.ts @@ -3,6 +3,8 @@ import { createCustomUser } from '../user/__mocks__/user.entity.mock'; import { User } from '../user/user.entity'; import { UserStatus } from '../user/user.enum'; import { createCustomUserData } from './__mocks__/user-data.entity.mock'; +import { UserData } from './user-data.entity'; +import { ServiceProvider } from './user-data.enum'; describe('UserData', () => { // getMailLoginUser resolves which user a mail login authenticates as for an elevated role. It is the @@ -114,4 +116,43 @@ describe('UserData', () => { expect(isStaff(undefined)).toBe(false); }); }); + + // serviceProviders is the additive RealUnit customer marker ("add-on on top" of the DFX core). It must + // never influence DFX core logic; only the RealUnit dashboards read it. These tests pin the additive, + // idempotent, merge-safe semantics the scope service and the merge union rely on. + describe('serviceProviders (RealUnit customer add-on)', () => { + const userData = (serviceProviders?: string): UserData => createCustomUserData({ serviceProviders }); + + it('serviceProviderList is empty when unset', () => { + expect(userData(undefined).serviceProviderList).toEqual([]); + }); + + it('isRealUnitCustomer is false for a plain DFX account', () => { + expect(userData(undefined).isRealUnitCustomer).toBe(false); + }); + + it('isRealUnitCustomer is true when the RealUnit marker is present', () => { + expect(userData('RealUnit').isRealUnitCustomer).toBe(true); + }); + + it('addServiceProvider sets the marker on an account that had none', () => { + const ud = userData(undefined); + ud.addServiceProvider(ServiceProvider.REALUNIT); + expect(ud.serviceProviders).toBe('RealUnit'); + expect(ud.isRealUnitCustomer).toBe(true); + }); + + it('addServiceProvider is idempotent — no duplicate token', () => { + const ud = userData('RealUnit'); + ud.addServiceProvider(ServiceProvider.REALUNIT); + expect(ud.serviceProviders).toBe('RealUnit'); + }); + + it('addServiceProvider returns an UpdateResult tuple [id, update]', () => { + const ud = createCustomUserData({ id: 42, serviceProviders: undefined }); + const [id, update] = ud.addServiceProvider(ServiceProvider.REALUNIT); + expect(id).toBe(42); + expect(update).toEqual({ serviceProviders: 'RealUnit' }); + }); + }); }); diff --git a/src/subdomains/generic/user/models/user-data/user-data.entity.ts b/src/subdomains/generic/user/models/user-data/user-data.entity.ts index b574387e85..d17121ab19 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.entity.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.entity.ts @@ -45,6 +45,7 @@ import { PhoneCallPreferredTime, PhoneCallStatus, RiskStatus, + ServiceProvider, SignatoryPower, UserDataStatus, } from './user-data.enum'; @@ -246,6 +247,9 @@ export class UserData extends IEntity { @Column({ length: 256, nullable: true }) kycClients?: string; // semicolon separated wallet id's + @Column({ length: 256, nullable: true }) + serviceProviders?: string; // semicolon separated ServiceProvider add-ons (e.g. RealUnit); DFX core never reads this, only the RealUnit dashboards + @Column({ type: 'timestamp', nullable: true }) phoneCallCheckDate?: Date; @@ -493,6 +497,18 @@ export class UserData extends IEntity { return [this.id, update]; } + addServiceProvider(provider: ServiceProvider): UpdateResult { + const update: Partial = { + serviceProviders: this.serviceProviderList.includes(provider) + ? this.serviceProviders + : [...this.serviceProviderList, provider].join(';'), + }; + + Object.assign(this, update); + + return [this.id, update]; + } + removeKycClient(walletId: number): UpdateResult { const update: Partial = { kycClients: this.kycClientList.filter((id) => id !== walletId).join(';'), @@ -583,6 +599,14 @@ export class UserData extends IEntity { return this.kycClients?.split(';')?.map(Number) ?? []; } + get serviceProviderList(): ServiceProvider[] { + return (this.serviceProviders?.split(';') as ServiceProvider[]) ?? []; + } + + get isRealUnitCustomer(): boolean { + return this.serviceProviderList.includes(ServiceProvider.REALUNIT); + } + get hasActiveUser(): boolean { return !!this.users.find((e) => e.status === UserStatus.ACTIVE); } diff --git a/src/subdomains/generic/user/models/user-data/user-data.enum.ts b/src/subdomains/generic/user/models/user-data/user-data.enum.ts index 8dd60eac4c..99c2afc472 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.enum.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.enum.ts @@ -63,6 +63,12 @@ export enum KycType { LOCK = 'LOCK', } +// Additive service-provider tenants an account is a customer of, on top of the DFX core (e.g. RealUnit, +// a regulated intermediary DFX serves as SaaS). Never consumed by DFX core logic — only the RealUnit dashboards. +export enum ServiceProvider { + REALUNIT = 'RealUnit', +} + export enum LegalEntity { AG = 'AG', GMBH = 'GmbH', diff --git a/src/subdomains/generic/user/models/user-data/user-data.service.ts b/src/subdomains/generic/user/models/user-data/user-data.service.ts index 6d1e5683eb..d203d0e78b 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.service.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.service.ts @@ -66,7 +66,7 @@ import { UpdateUserDataDto } from './dto/update-user-data.dto'; import { KycIdentificationType } from './kyc-identification-type.enum'; import { UserDataNotificationService } from './user-data-notification.service'; import { UserData } from './user-data.entity'; -import { KycLevel, PhoneCallStatus, TradeApprovalReason, UserDataStatus } from './user-data.enum'; +import { KycLevel, PhoneCallStatus, ServiceProvider, TradeApprovalReason, UserDataStatus } from './user-data.enum'; import { UserDataRepository } from './user-data.repository'; export const MergedPrefix = 'Merged into '; @@ -1062,6 +1062,14 @@ export class UserDataService { await this.userDataRepo.update(...userData.removeKycClient(walletId)); } + // --- SERVICE PROVIDERS --- // + + async addServiceProvider(userData: UserData, provider: ServiceProvider): Promise { + if (userData.serviceProviderList.includes(provider)) return; + + await this.userDataRepo.update(...userData.addServiceProvider(provider)); + } + // --- FEES --- // async addFee(userData: UserData, feeId: number): Promise { @@ -1298,6 +1306,9 @@ export class UserDataService { master.transactions = master.transactions.concat(slave.transactions); slave.individualFeeList?.forEach((fee) => !master.individualFeeList?.includes(fee) && master.addFee(fee)); slave.kycClientList.forEach((kc) => !master.kycClientList.includes(kc) && master.addKycClient(kc)); + slave.serviceProviderList.forEach( + (sp) => !master.serviceProviderList.includes(sp) && master.addServiceProvider(sp), + ); // copy all documents void this.documentService diff --git a/src/subdomains/supporting/realunit/realunit.service.ts b/src/subdomains/supporting/realunit/realunit.service.ts index b5754804bf..d1feac8c2a 100644 --- a/src/subdomains/supporting/realunit/realunit.service.ts +++ b/src/subdomains/supporting/realunit/realunit.service.ts @@ -49,7 +49,7 @@ import { KycService } from 'src/subdomains/generic/kyc/services/kyc.service'; import { AccountMergeService } from 'src/subdomains/generic/user/models/account-merge/account-merge.service'; import { AccountType } from 'src/subdomains/generic/user/models/user-data/account-type.enum'; import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; -import { KycLevel } from 'src/subdomains/generic/user/models/user-data/user-data.enum'; +import { KycLevel, ServiceProvider } from 'src/subdomains/generic/user/models/user-data/user-data.enum'; import { UserDataService } from 'src/subdomains/generic/user/models/user-data/user-data.service'; import { User } from 'src/subdomains/generic/user/models/user/user.entity'; import { UserService } from 'src/subdomains/generic/user/models/user/user.service'; @@ -663,6 +663,9 @@ export class RealUnitService { await this.kycService.initializeProcess(userData); } + // mark the account as a RealUnit customer (additive add-on; never read by DFX core logic) + await this.userDataService.addServiceProvider(userData, ServiceProvider.REALUNIT); + return RealUnitEmailRegistrationStatus.EMAIL_REGISTERED; } From b1580b36f089db39581c10e4fd002b1fa383c7ce Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 3 Jul 2026 02:23:24 +0200 Subject: [PATCH 02/10] fix(auth): harden staff 2FA factor and revoke addressless JWTs (#4028/#4029/#4032) (#4050) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(auth): harden staff 2FA factor against brute-force and self-enrollment - auth/2fa/verify and the code-header kyc/2fa/verify had no rate limit, and the transient tryCount never applies to an enrolled TOTP secret, leaving it brute-forceable (#4029). Add RateLimitGuard + throttle and a durable per-account lockout (totpFailedAttempts / totpBlockedUntil) that locks after 5 failures for 15 min and resets on success. - An un-enrolled staff account could establish its first TOTP factor from a mail-only or code-header session, seeding the second factor from the same inbox as the magic link (#4032). Require a trusted wallet-signature origin for initial staff enrollment in both setup and verify. * fix(auth): add account-id JWT revocation for addressless tokens Account/mail/staff JWTs minted by generateAccountToken carry no address, so the existing address denylist cannot revoke them, and JwtStrategy re-validates no status against the DB — a blocked staff token stays valid until expiry (#4028). Add an account-id-keyed JWT denylist (jwtAccountDenylist) resynced alongside the address one and checked in JwtStrategy, giving an operational revocation path for addressless tokens. * fix(auth): apply review findings - Bump the AddTotpLockout migration timestamp above the current develop max so it keeps strict ascending order after the rebase. - Require a positive wallet-address on the token (not just the absence of the tfaRequired marker) to treat a session as a trusted origin for initial staff TOTP enrollment, closing two mail-origin edge cases (staff enforcement off, or a wallet-less staff user) where an account-token fallback carries no marker. - Drop the redundant totpSecret check in the setup staff-enrollment guard (the preceding ConflictException already returns for any set secret). --- migration/1782990500000-AddTotpLockout.js | 28 +++++ .../auth/__tests__/jwt.strategy.spec.ts | 54 ++++++++ src/shared/auth/jwt.strategy.ts | 5 +- src/shared/models/setting/setting.service.ts | 4 + .../__tests__/process.service.spec.ts | 49 ++++++++ src/shared/services/process.service.ts | 25 +++- .../generic/kyc/controllers/kyc.controller.ts | 4 + .../services/__tests__/tfa.service.spec.ts | 119 +++++++++++++++++- .../generic/kyc/services/tfa.service.ts | 48 ++++++- .../user/models/auth/auth.controller.ts | 11 +- .../user/models/user-data/user-data.entity.ts | 6 + .../models/user-data/user-data.service.ts | 5 + 12 files changed, 346 insertions(+), 12 deletions(-) create mode 100644 migration/1782990500000-AddTotpLockout.js create mode 100644 src/shared/auth/__tests__/jwt.strategy.spec.ts create mode 100644 src/shared/services/__tests__/process.service.spec.ts diff --git a/migration/1782990500000-AddTotpLockout.js b/migration/1782990500000-AddTotpLockout.js new file mode 100644 index 0000000000..42b7649c87 --- /dev/null +++ b/migration/1782990500000-AddTotpLockout.js @@ -0,0 +1,28 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * @class + * @implements {MigrationInterface} + */ +module.exports = class AddTotpLockout1782990500000 { + name = 'AddTotpLockout1782990500000'; + + /** + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + await queryRunner.query(`ALTER TABLE "user_data" ADD "totpFailedAttempts" integer NOT NULL DEFAULT 0`); + await queryRunner.query(`ALTER TABLE "user_data" ADD "totpBlockedUntil" TIMESTAMP`); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + await queryRunner.query(`ALTER TABLE "user_data" DROP COLUMN "totpBlockedUntil"`); + await queryRunner.query(`ALTER TABLE "user_data" DROP COLUMN "totpFailedAttempts"`); + } +}; diff --git a/src/shared/auth/__tests__/jwt.strategy.spec.ts b/src/shared/auth/__tests__/jwt.strategy.spec.ts new file mode 100644 index 0000000000..073e6de3c2 --- /dev/null +++ b/src/shared/auth/__tests__/jwt.strategy.spec.ts @@ -0,0 +1,54 @@ +import { UnauthorizedException } from '@nestjs/common'; +import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; +import { JwtStrategy } from 'src/shared/auth/jwt.strategy'; +import { UserRole } from 'src/shared/auth/user-role.enum'; +import { SettingService } from 'src/shared/models/setting/setting.service'; +import { ProcessService } from 'src/shared/services/process.service'; + +describe('JwtStrategy account denylist', () => { + let settingService: jest.Mocked; + let processService: ProcessService; + + // validate() reads no instance state, so exercise it off the prototype to avoid the passport-jwt + // constructor (which requires a JWT secret from the environment). + const validate = (payload: JwtPayload): Promise => + JwtStrategy.prototype.validate.call({} as JwtStrategy, payload); + + const accountPayload = (account: number): JwtPayload => ({ + account, + role: UserRole.ACCOUNT, + ip: '1.1.1.1', + }); + + beforeEach(() => { + settingService = { + getDeniedJwtAddresses: jest.fn().mockResolvedValue([]), + getDeniedJwtAccounts: jest.fn().mockResolvedValue([]), + } as unknown as jest.Mocked; + + processService = new ProcessService(settingService); + }); + + afterEach(async () => { + // reset the shared module-level denylists so state does not leak between tests + settingService.getDeniedJwtAddresses.mockResolvedValue([]); + settingService.getDeniedJwtAccounts.mockResolvedValue([]); + await processService.resyncDeniedJwtAddresses(); + await processService.resyncDeniedJwtAccounts(); + }); + + it('rejects an addressless account token whose account id is denied', async () => { + settingService.getDeniedJwtAccounts.mockResolvedValue([42]); + await processService.resyncDeniedJwtAccounts(); + + await expect(validate(accountPayload(42))).rejects.toBeInstanceOf(UnauthorizedException); + }); + + it('accepts an account token whose account id is not denied', async () => { + settingService.getDeniedJwtAccounts.mockResolvedValue([42]); + await processService.resyncDeniedJwtAccounts(); + + const payload = accountPayload(7); + await expect(validate(payload)).resolves.toEqual(payload); + }); +}); diff --git a/src/shared/auth/jwt.strategy.ts b/src/shared/auth/jwt.strategy.ts index 11ebb0c99c..c8eb685fd7 100644 --- a/src/shared/auth/jwt.strategy.ts +++ b/src/shared/auth/jwt.strategy.ts @@ -2,7 +2,7 @@ import { UnauthorizedException } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { ExtractJwt, Strategy } from 'passport-jwt'; import { GetConfig } from 'src/config/config'; -import { IsJwtAddressDenied } from '../services/process.service'; +import { IsJwtAccountDenied, IsJwtAddressDenied } from '../services/process.service'; import { JwtPayload } from './jwt-payload.interface'; import { UserRole } from './user-role.enum'; @@ -33,6 +33,9 @@ export class JwtStrategy extends PassportStrategy(Strategy) { } if (IsJwtAddressDenied(address)) throw new UnauthorizedException(); + // Addressless-token counterpart: account/mail/staff tokens carry no `address`, so revoke them by + // account id (checked in-memory, primed by ProcessService) — see `IsJwtAccountDenied`. + if (IsJwtAccountDenied(account)) throw new UnauthorizedException(); return payload; } diff --git a/src/shared/models/setting/setting.service.ts b/src/shared/models/setting/setting.service.ts index 77582bd10b..ddf0c47879 100644 --- a/src/shared/models/setting/setting.service.ts +++ b/src/shared/models/setting/setting.service.ts @@ -108,6 +108,10 @@ export class SettingService { return this.getObj('jwtAddressDenylist', []); } + async getDeniedJwtAccounts(): Promise { + return this.getObj('jwtAccountDenylist', []).then((list) => list.map(Number)); + } + async getCustomBalanceSettings(): Promise<{ addresses: string[]; assets: string[] }> { const [addresses, assets] = await Promise.all([ this.getObjCached('customBalanceAddresses', []), diff --git a/src/shared/services/__tests__/process.service.spec.ts b/src/shared/services/__tests__/process.service.spec.ts new file mode 100644 index 0000000000..daf3feb509 --- /dev/null +++ b/src/shared/services/__tests__/process.service.spec.ts @@ -0,0 +1,49 @@ +import { SettingService } from 'src/shared/models/setting/setting.service'; +import { IsJwtAccountDenied, ProcessService } from 'src/shared/services/process.service'; + +describe('ProcessService JWT account denylist', () => { + let settingService: jest.Mocked; + let service: ProcessService; + + beforeEach(() => { + settingService = { + getDeniedJwtAccounts: jest.fn().mockResolvedValue([]), + } as unknown as jest.Mocked; + + service = new ProcessService(settingService); + }); + + afterEach(async () => { + // reset the shared module-level denylist so state does not leak between tests + settingService.getDeniedJwtAccounts.mockResolvedValue([]); + await service.resyncDeniedJwtAccounts(); + }); + + it('denies an account id present in the setting', async () => { + settingService.getDeniedJwtAccounts.mockResolvedValue([123, 456]); + await service.resyncDeniedJwtAccounts(); + + expect(IsJwtAccountDenied(123)).toBe(true); + expect(IsJwtAccountDenied(456)).toBe(true); + }); + + it('allows an account id not present in the setting', async () => { + settingService.getDeniedJwtAccounts.mockResolvedValue([123]); + await service.resyncDeniedJwtAccounts(); + + expect(IsJwtAccountDenied(999)).toBe(false); + }); + + it('allows an undefined account (addressless tokens are not implicitly denied)', async () => { + settingService.getDeniedJwtAccounts.mockResolvedValue([123]); + await service.resyncDeniedJwtAccounts(); + + expect(IsJwtAccountDenied(undefined)).toBe(false); + }); + + it('fails open on an empty denylist', async () => { + await service.resyncDeniedJwtAccounts(); + + expect(IsJwtAccountDenied(123)).toBe(false); + }); +}); diff --git a/src/shared/services/process.service.ts b/src/shared/services/process.service.ts index 75142abb42..0bf76e91bb 100644 --- a/src/shared/services/process.service.ts +++ b/src/shared/services/process.service.ts @@ -126,6 +126,19 @@ export function IsJwtAddressDenied(address: string | undefined): boolean { return !!address && DeniedJwtAddresses.has(address.toLowerCase()); } +// JWT account denylist — the addressless-token counterpart of `DeniedJwtAddresses`. Account/mail/staff +// JWTs minted by `generateAccountToken` carry no `address`, so the address denylist cannot revoke them +// at all. A compromised account (user data) id can be added to the `jwtAccountDenylist` setting (JSON +// number[]) and active addressless JWTs for that account are rejected within one refresh interval +// (~30s) — no JWT-secret rotation, no API redeploy. Same fail-open-on-empty-Set semantics as the +// address denylist: lookup is sync (in-memory Set) to keep JwtStrategy.validate off the DB hot path, +// refreshed by ProcessService alongside the address denylist. +let DeniedJwtAccounts: Set = new Set(); + +export function IsJwtAccountDenied(account: number | undefined): boolean { + return account != null && DeniedJwtAccounts.has(account); +} + @Injectable() export class ProcessService implements OnModuleInit { private safetyModeInactive = true; @@ -134,9 +147,11 @@ export class ProcessService implements OnModuleInit { async onModuleInit(): Promise { void this.resyncDisabledProcesses(); - // await so the JWT denylist is primed before HTTP starts — `IsJwtAddressDenied` defaults to - // false on an empty Set (fail-open), unlike DisabledProcess which is fail-closed by sentinel + // await so the JWT denylists are primed before HTTP starts — `IsJwtAddressDenied` / + // `IsJwtAccountDenied` default to false on an empty Set (fail-open), unlike DisabledProcess which + // is fail-closed by sentinel await this.resyncDeniedJwtAddresses(); + await this.resyncDeniedJwtAccounts(); } @DfxCron(CronExpression.EVERY_30_SECONDS, { timeout: 1800 }) @@ -155,6 +170,12 @@ export class ProcessService implements OnModuleInit { DeniedJwtAddresses = new Set(list.map((a) => a.toLowerCase())); } + @DfxCron(CronExpression.EVERY_30_SECONDS, { timeout: 1800 }) + async resyncDeniedJwtAccounts(): Promise { + const list = await this.settingService.getDeniedJwtAccounts(); + DeniedJwtAccounts = new Set(list); + } + public async setSafetyModeActive(active: boolean): Promise { this.safetyModeInactive = DisabledProcess(Process.SAFETY_MODE) ? true : !active; await this.resyncDisabledProcesses(); diff --git a/src/subdomains/generic/kyc/controllers/kyc.controller.ts b/src/subdomains/generic/kyc/controllers/kyc.controller.ts index d9c97a2e63..67acea4473 100644 --- a/src/subdomains/generic/kyc/controllers/kyc.controller.ts +++ b/src/subdomains/generic/kyc/controllers/kyc.controller.ts @@ -25,11 +25,13 @@ import { ApiTags, ApiUnauthorizedResponse, } from '@nestjs/swagger'; +import { Throttle } from '@nestjs/throttler'; import { Request } from 'express'; import { GetConfig } from 'src/config/config'; import { GetJwt } from 'src/shared/auth/get-jwt.decorator'; import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; import { OptionalJwtAuthGuard } from 'src/shared/auth/optional.guard'; +import { RateLimitGuard } from 'src/shared/auth/rate-limit.guard'; import { RealIP } from 'src/shared/auth/real-ip.decorator'; import { RoleGuard } from 'src/shared/auth/role.guard'; import { UserActiveGuard } from 'src/shared/auth/user-active.guard'; @@ -105,6 +107,8 @@ export class KycController { } @Post('2fa/verify') + @UseGuards(RateLimitGuard) + @Throttle(10, 60) @ApiCreatedResponse({ description: '2FA successful' }) @ApiUnauthorizedResponse(MergedResponse) @ApiForbiddenResponse({ description: 'Invalid or expired 2FA token' }) diff --git a/src/subdomains/generic/kyc/services/__tests__/tfa.service.spec.ts b/src/subdomains/generic/kyc/services/__tests__/tfa.service.spec.ts index 7dc6b5e2e7..30dc69bd63 100644 --- a/src/subdomains/generic/kyc/services/__tests__/tfa.service.spec.ts +++ b/src/subdomains/generic/kyc/services/__tests__/tfa.service.spec.ts @@ -1,5 +1,7 @@ +import { ForbiddenException } from '@nestjs/common'; import { mock } from 'jest-mock-extended'; import { UserRole } from 'src/shared/auth/user-role.enum'; +import { Util } from 'src/shared/utils/util'; import { NotificationService } from 'src/subdomains/supporting/notification/services/notification.service'; import { createCustomUserData } from '../../../user/models/user-data/__mocks__/user-data.entity.mock'; import { UserData } from '../../../user/models/user-data/user-data.entity'; @@ -39,18 +41,37 @@ describe('TfaService', () => { }); describe('setup', () => { - it('forces an app/TOTP factor for a staff account (never mail), even at STRICT with linked wallets', async () => { + it('forces an app/TOTP factor for a staff account (never mail) from a trusted session, even at STRICT', async () => { userDataService.getByKycHashOrThrow.mockResolvedValue( activeUserData({ users: [createCustomUser({ role: UserRole.SUPPORT })] }), ); - const result = await service.setup('hash', TfaLevel.STRICT); + const result = await service.setup('hash', TfaLevel.STRICT, true); expect(result.type).toBe(TfaType.APP); expect(result.secret).toBeDefined(); expect(notificationService.sendMail).not.toHaveBeenCalled(); }); + it('forbids a staff first-TOTP enrollment from an untrusted session (default allowStaffEnrollment=false)', async () => { + userDataService.getByKycHashOrThrow.mockResolvedValue( + activeUserData({ users: [createCustomUser({ role: UserRole.SUPPORT })] }), + ); + + await expect(service.setup('hash', TfaLevel.STRICT)).rejects.toBeInstanceOf(ForbiddenException); + }); + + it('allows a non-staff app enrollment without a trusted session', async () => { + userDataService.getByKycHashOrThrow.mockResolvedValue( + activeUserData({ mail: undefined, users: [createCustomUser({ role: UserRole.USER })] }), + ); + + const result = await service.setup('hash', TfaLevel.STRICT); + + expect(result.type).toBe(TfaType.APP); + expect(result.secret).toBeDefined(); + }); + it('keeps mail 2FA for a regular account with mail and linked wallets', async () => { userDataService.getByKycHashOrThrow.mockResolvedValue( activeUserData({ users: [createCustomUser({ role: UserRole.USER })] }), @@ -63,6 +84,100 @@ describe('TfaService', () => { }); }); + // TfaMaxTryCount = 5: five wrong TOTPs lock the enrolled account for 15 minutes. + describe('verify (durable TOTP lockout)', () => { + const enrolledUser = (overrides: Partial = {}): UserData => + activeUserData({ totpSecret: 'ENROLLED_SECRET', totpFailedAttempts: 0, ...overrides }); + + it('increments the durable failed-attempt counter on a wrong TOTP for an enrolled account', async () => { + const user = enrolledUser({ totpFailedAttempts: 0 }); + userDataService.getByKycHashOrThrow.mockResolvedValue(user); + jest.spyOn(service as any, 'verifyOrThrow').mockImplementation(() => { + throw new ForbiddenException('Invalid or expired 2FA token'); + }); + + await expect(service.verify('hash', '000000', ip)).rejects.toBeInstanceOf(ForbiddenException); + expect(userDataService.setTotpLockout).toHaveBeenCalledWith(user, 1, null); + }); + + it('locks the account for 15 minutes once the failed attempts reach the max', async () => { + const user = enrolledUser({ totpFailedAttempts: 4 }); + userDataService.getByKycHashOrThrow.mockResolvedValue(user); + jest.spyOn(service as any, 'verifyOrThrow').mockImplementation(() => { + throw new ForbiddenException('Invalid or expired 2FA token'); + }); + + await expect(service.verify('hash', '000000', ip)).rejects.toBeInstanceOf(ForbiddenException); + expect(userDataService.setTotpLockout).toHaveBeenCalledWith(user, 0, expect.any(Date)); + }); + + it('rejects immediately while locked, without verifying the token', async () => { + const user = enrolledUser({ totpBlockedUntil: Util.minutesAfter(5) }); + userDataService.getByKycHashOrThrow.mockResolvedValue(user); + const verifySpy = jest.spyOn(service as any, 'verifyOrThrow'); + + await expect(service.verify('hash', '123456', ip)).rejects.toBeInstanceOf(ForbiddenException); + expect(verifySpy).not.toHaveBeenCalled(); + expect(userDataService.setTotpLockout).not.toHaveBeenCalled(); + }); + + it('resets the durable counter on a successful TOTP verification', async () => { + const user = enrolledUser({ totpFailedAttempts: 3 }); + userDataService.getByKycHashOrThrow.mockResolvedValue(user); + jest.spyOn(service as any, 'verifyOrThrow').mockReturnValue(undefined); + + await service.verify('hash', '123456', ip); + + expect(userDataService.setTotpLockout).toHaveBeenCalledWith(user, 0, null); + expect(tfaRepo.save).toHaveBeenCalled(); + }); + }); + + describe('verify (staff self-enroll guard)', () => { + const staffUser = (overrides: Partial = {}): UserData => + activeUserData({ users: [createCustomUser({ role: UserRole.COMPLIANCE })], totpSecret: undefined, ...overrides }); + + const seedAppSecret = (user: UserData, secret = 'CACHED_SECRET'): void => + (service as any).secretCache.set(user.id, { + type: TfaType.APP, + secret, + expiryDate: Util.hoursAfter(1), + tryCount: 0, + }); + + it('forbids a staff first-TOTP verification from an untrusted session and does not persist the secret', async () => { + const user = staffUser(); + userDataService.getByKycHashOrThrow.mockResolvedValue(user); + seedAppSecret(user); + jest.spyOn(service as any, 'verifyOrThrow').mockReturnValue(undefined); + + await expect(service.verify('hash', '123456', ip, false)).rejects.toBeInstanceOf(ForbiddenException); + expect(userDataService.updateTotpSecret).not.toHaveBeenCalled(); + }); + + it('allows a staff first-TOTP verification from a trusted session and persists the secret', async () => { + const user = staffUser(); + userDataService.getByKycHashOrThrow.mockResolvedValue(user); + seedAppSecret(user, 'CACHED_SECRET'); + jest.spyOn(service as any, 'verifyOrThrow').mockReturnValue(undefined); + + await service.verify('hash', '123456', ip, true); + + expect(userDataService.updateTotpSecret).toHaveBeenCalledWith(user, 'CACHED_SECRET'); + }); + + it('lets an already-enrolled staff re-verify regardless of the enrollment flag', async () => { + const user = staffUser({ totpSecret: 'ENROLLED_SECRET' }); + userDataService.getByKycHashOrThrow.mockResolvedValue(user); + jest.spyOn(service as any, 'verifyOrThrow').mockReturnValue(undefined); + + await service.verify('hash', '123456', ip, false); + + expect(userDataService.updateTotpSecret).not.toHaveBeenCalled(); + expect(tfaRepo.save).toHaveBeenCalled(); + }); + }); + describe('checkVerification', () => { const user = { id: 1 } as UserData; const withLogs = (...comments: string[]) => diff --git a/src/subdomains/generic/kyc/services/tfa.service.ts b/src/subdomains/generic/kyc/services/tfa.service.ts index 489d799725..6870109209 100644 --- a/src/subdomains/generic/kyc/services/tfa.service.ts +++ b/src/subdomains/generic/kyc/services/tfa.service.ts @@ -61,7 +61,7 @@ export class TfaService { keysToBeDeleted.forEach((k) => this.secretCache.delete(k)); } - async setup(kycHash: string, level: TfaLevel): Promise { + async setup(kycHash: string, level: TfaLevel, allowStaffEnrollment = false): Promise { const user = await this.getUser(kycHash); if (user.isBlockedOrDeactivated) throw new ForbiddenException('Account is blocked/deactivated'); @@ -89,6 +89,11 @@ export class TfaService { // app 2FA if (user.totpSecret) throw new ConflictException('2FA already set up'); + // Initial staff enrollment must originate from a trusted (wallet-signature) session: a code-header or + // mail-elevated session shares the magic-link inbox and would not be an independent second factor. + if (user.isStaff && !allowStaffEnrollment) + throw new ForbiddenException('Staff 2FA must be enrolled from a wallet-authenticated session'); + const type = TfaType.APP; const { secret, uri } = generateSecret({ name: 'DFX.swiss', account: user.mail ?? '' }); @@ -103,7 +108,7 @@ export class TfaService { } } - async verify(kycHash: string, token: string, ip: string): Promise { + async verify(kycHash: string, token: string, ip: string, allowStaffEnrollment = false): Promise { const user = await this.getUser(kycHash); let level: TfaLevel; @@ -126,9 +131,21 @@ export class TfaService { const secret = user.totpSecret ?? cacheEntry?.secret; if (!secret) throw new NotFoundException('2FA not set up'); - this.verifyOrThrow(secret, token); + // Durable per-account lockout: an enrolled account (totpSecret set) has no secretCache entry, so the + // transient tryCount gate above never fires for it — this is the brute-force gap being closed here. + if (user.totpBlockedUntil && user.totpBlockedUntil > new Date()) + throw new ForbiddenException('Too many failed 2FA attempts, please try again later'); + + await this.verifyTotpOrLock(user, secret, token); - if (!user.totpSecret) await this.userDataService.updateTotpSecret(user, secret); + if (!user.totpSecret) { + // Initial staff enrollment must originate from a trusted (wallet-signature) session; a code-header or + // mail-elevated session shares the magic-link inbox and is not an independent factor. + if (user.isStaff && !allowStaffEnrollment) + throw new ForbiddenException('Staff 2FA must be enrolled from a wallet-authenticated session'); + + await this.userDataService.updateTotpSecret(user, secret); + } level = TfaLevel.STRICT; type = TfaType.APP; @@ -143,6 +160,29 @@ export class TfaService { await this.createTfaLog(user, ip, level, type); } + private async verifyTotpOrLock(user: UserData, secret: string, token: string): Promise { + try { + this.verifyOrThrow(secret, token); + } catch (e) { + const failedAttempts = (user.totpFailedAttempts ?? 0) + 1; + const isLocked = failedAttempts >= TfaMaxTryCount; + + if (isLocked) + this.logger.warn(`TOTP lockout triggered for account ${user.id} after ${failedAttempts} failed attempts`); + + await this.userDataService.setTotpLockout( + user, + isLocked ? 0 : failedAttempts, + isLocked ? Util.minutesAfter(15) : null, + ); + + throw e; + } + + // reset the durable counter for a legitimate user so failures never accumulate to a lockout over time + if (user.totpFailedAttempts || user.totpBlockedUntil) await this.userDataService.setTotpLockout(user, 0, null); + } + async check(userDataId: number, ip: string, level?: TfaLevel): Promise { const userData = await this.userDataService.getUserData(userDataId, { users: true }); if (!userData) throw new NotFoundException('User data not found'); diff --git a/src/subdomains/generic/user/models/auth/auth.controller.ts b/src/subdomains/generic/user/models/auth/auth.controller.ts index 531be61825..857bdf7510 100644 --- a/src/subdomains/generic/user/models/auth/auth.controller.ts +++ b/src/subdomains/generic/user/models/auth/auth.controller.ts @@ -103,16 +103,21 @@ export class AuthController { @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard()) async setup2fa(@GetJwt() jwt: JwtPayload, @Query() { level }: Start2faDto): Promise { const { kycHash } = await this.userDataService.getUserData(jwt.account); - return this.tfaService.setup(kycHash, level); + // A wallet-signature login has no tfaRequired marker (trusted); a mail-elevated staff token has it (not trusted). + // Trusted origin = a wallet-signature login: no tfaRequired marker AND a real wallet address on the token. + // The account-token fallback (staff enforcement off, or a wallet-less staff user) carries neither, so it + // cannot self-enroll a staff TOTP factor from a mail inbox. + return this.tfaService.setup(kycHash, level, !jwt.tfaRequired && !!jwt.address); } @Post('2fa/verify') @ApiBearerAuth() @ApiCreatedResponse({ description: '2FA successful' }) - @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard()) + @UseGuards(RateLimitGuard, AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard()) + @Throttle(10, 60) async verify2fa(@GetJwt() jwt: JwtPayload, @RealIP() ip: string, @Body() dto: Verify2faDto): Promise { const { kycHash } = await this.userDataService.getUserData(jwt.account); - return this.tfaService.verify(kycHash, dto.token, ip); + return this.tfaService.verify(kycHash, dto.token, ip, !jwt.tfaRequired && !!jwt.address); } @Get('mail/confirm') diff --git a/src/subdomains/generic/user/models/user-data/user-data.entity.ts b/src/subdomains/generic/user/models/user-data/user-data.entity.ts index d17121ab19..9eb03312d5 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.entity.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.entity.ts @@ -360,6 +360,12 @@ export class UserData extends IEntity { @Column({ nullable: true }) totpSecret?: string; + @Column({ type: 'integer', default: 0 }) + totpFailedAttempts: number; + + @Column({ type: 'timestamp', nullable: true }) + totpBlockedUntil?: Date; + // Point of Sale @Column({ default: false }) paymentLinksAllowed: boolean; diff --git a/src/subdomains/generic/user/models/user-data/user-data.service.ts b/src/subdomains/generic/user/models/user-data/user-data.service.ts index d203d0e78b..9211a47bb9 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.service.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.service.ts @@ -647,6 +647,11 @@ export class UserDataService { await this.userDataRepo.update(user.id, { totpSecret: secret }); } + async setTotpLockout(user: UserData, failedAttempts: number, blockedUntil: Date | null): Promise { + await this.userDataRepo.update(user.id, { totpFailedAttempts: failedAttempts, totpBlockedUntil: blockedUntil }); + Object.assign(user, { totpFailedAttempts: failedAttempts, totpBlockedUntil: blockedUntil }); + } + async updatePaymentLinksConfig(user: UserData, dto: Partial): Promise { const mergedConfig = { ...JSON.parse(user.paymentLinksConfig || '{}'), ...dto }; const customConfig = Util.removeDefaultFields(mergedConfig, DefaultPaymentLinkConfig); From 7f606478f47f30f98d0f0ba2a07f37dd348df6a4 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 3 Jul 2026 02:35:23 +0200 Subject: [PATCH 03/10] fix(exchange): persist Scrypt deposits event-driven from the balance transaction stream (#4044) * fix(exchange): persist Scrypt deposits event-driven from the balance transaction stream The finance log matches bank->Scrypt deposits only against exchange_tx rows. The EVERY_5_MINUTES sync reads the WebSocket map once at run start and then saves sequentially for minutes, so live deposit confirmations lagged by up to two cycles and totals were overstated after each bank->Scrypt deposit. Write deposit records straight from the WebSocket stream and guard the sync against overwriting rows the event path has already updated with fresher data. * fix(exchange): isolate per-record failures in the exchange tx sync loop A single failing record (unique-index race with the event-driven Scrypt path, or a pricing error) previously aborted the remaining run for all exchanges. Wrap the per-record body in try/catch: log and skip instead, so the record is retried on the next sync. --- .../__tests__/exchange-tx.service.spec.ts | 277 ++++++++++++++++++ .../exchange/services/exchange-tx.service.ts | 221 +++++++++----- .../exchange/services/scrypt.service.ts | 4 + 3 files changed, 430 insertions(+), 72 deletions(-) create mode 100644 src/integration/exchange/services/__tests__/exchange-tx.service.spec.ts diff --git a/src/integration/exchange/services/__tests__/exchange-tx.service.spec.ts b/src/integration/exchange/services/__tests__/exchange-tx.service.spec.ts new file mode 100644 index 0000000000..369478f920 --- /dev/null +++ b/src/integration/exchange/services/__tests__/exchange-tx.service.spec.ts @@ -0,0 +1,277 @@ +import { createMock } from '@golevelup/ts-jest'; +import { AssetService } from 'src/shared/models/asset/asset.service'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; +import * as processServiceModule from 'src/shared/services/process.service'; +import { Util } from 'src/shared/utils/util'; +import { PricingService } from 'src/subdomains/supporting/pricing/services/pricing.service'; +import { ScryptBalanceTransaction, ScryptTransactionStatus, ScryptTransactionType } from '../../dto/scrypt.dto'; +import { ExchangeTx, ExchangeTxType } from '../../entities/exchange-tx.entity'; +import { ExchangeName } from '../../enums/exchange.enum'; +import { ExchangeTxRepository } from '../../repositories/exchange-tx.repository'; +import { ExchangeRegistryService } from '../exchange-registry.service'; +import { ExchangeTxService } from '../exchange-tx.service'; +import { ScryptService } from '../scrypt.service'; + +function createDepositTx(overrides: Partial = {}): ScryptBalanceTransaction { + const now = new Date(); + return { + TransactionID: 'tx-1', + Currency: 'CHF', + TransactionType: ScryptTransactionType.DEPOSIT, + Status: ScryptTransactionStatus.COMPLETED, + Quantity: '100.5', + TxHash: 'hash-1', + Timestamp: now.toISOString(), + TransactTime: now.toISOString(), + ...overrides, + }; +} + +describe('ExchangeTxService', () => { + let service: ExchangeTxService; + + let exchangeTxRepo: ExchangeTxRepository; + let registryService: ExchangeRegistryService; + let assetService: AssetService; + let pricingService: PricingService; + let fiatService: FiatService; + + beforeEach(() => { + exchangeTxRepo = createMock(); + registryService = createMock(); + assetService = createMock(); + pricingService = createMock(); + fiatService = createMock(); + + jest.spyOn(exchangeTxRepo, 'create').mockImplementation((dto) => dto as ExchangeTx); + jest.spyOn(processServiceModule, 'DisabledProcess').mockReturnValue(false); + + service = new ExchangeTxService(exchangeTxRepo, registryService, assetService, pricingService, fiatService); + }); + + afterEach(() => jest.restoreAllMocks()); + + // helper: drain the serialized event queue + const flushQueue = () => service['scryptTxQueue']; + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + describe('event-driven Scrypt deposits', () => { + it('creates a new exchange_tx row for a COMPLETED deposit event', async () => { + jest.spyOn(exchangeTxRepo, 'findOneBy').mockResolvedValue(null); + + service['handleScryptTransactions']([createDepositTx()]); + await flushQueue(); + + expect(exchangeTxRepo.save).toHaveBeenCalledTimes(1); + expect(exchangeTxRepo.save).toHaveBeenCalledWith( + expect.objectContaining({ + exchange: ExchangeName.SCRYPT, + type: ExchangeTxType.DEPOSIT, + externalId: 'tx-1', + status: 'ok', + currency: 'CHF', + txId: 'hash-1', + amount: 100.5, + }), + ); + }); + + it('updates an existing pending row to ok', async () => { + const existing = { + id: 7, + exchange: ExchangeName.SCRYPT, + type: ExchangeTxType.DEPOSIT, + externalId: 'tx-1', + status: 'pending', + externalUpdated: new Date(Date.now() - 60_000), + amount: 100.5, + } as ExchangeTx; + jest.spyOn(exchangeTxRepo, 'findOneBy').mockResolvedValue(existing); + + service['handleScryptTransactions']([createDepositTx()]); + await flushQueue(); + + expect(exchangeTxRepo.create).not.toHaveBeenCalled(); + expect(exchangeTxRepo.save).toHaveBeenCalledTimes(1); + expect(exchangeTxRepo.save).toHaveBeenCalledWith(expect.objectContaining({ id: 7, status: 'ok' })); + }); + + it('does not save when the existing row is fresher than the event (stale guard)', async () => { + const existing = { id: 7, externalUpdated: new Date(), status: 'ok' } as ExchangeTx; + jest.spyOn(exchangeTxRepo, 'findOneBy').mockResolvedValue(existing); + + // two hours old: within the 1-day recency bound but older than the DB row + const olderTx = createDepositTx({ Timestamp: new Date(Date.now() - 2 * 60 * 60 * 1000).toISOString() }); + service['handleScryptTransactions']([olderTx]); + await flushQueue(); + + expect(exchangeTxRepo.save).not.toHaveBeenCalled(); + }); + + it('ignores withdrawal and unknown transaction types', async () => { + const withdrawal = createDepositTx({ TransactionType: ScryptTransactionType.WITHDRAWAL, TransactionID: 'w-1' }); + const unknown = createDepositTx({ TransactionType: 'Fee' as ScryptTransactionType, TransactionID: 'f-1' }); + + service['handleScryptTransactions']([withdrawal, unknown]); + await flushQueue(); + + expect(exchangeTxRepo.findOneBy).not.toHaveBeenCalled(); + expect(exchangeTxRepo.save).not.toHaveBeenCalled(); + }); + + it('ignores deposits older than one day', async () => { + const oldTx = createDepositTx({ Timestamp: Util.daysBefore(2).toISOString() }); + + service['handleScryptTransactions']([oldTx]); + await flushQueue(); + + expect(exchangeTxRepo.findOneBy).not.toHaveBeenCalled(); + expect(exchangeTxRepo.save).not.toHaveBeenCalled(); + }); + + it('catches a save rejection and continues with the rest of the batch', async () => { + jest.spyOn(exchangeTxRepo, 'findOneBy').mockResolvedValue(null); + jest + .spyOn(exchangeTxRepo, 'save') + .mockRejectedValueOnce(new Error('unique constraint')) + .mockResolvedValueOnce({} as ExchangeTx); + + const depA = createDepositTx({ TransactionID: 'a', TxHash: 'ha' }); + const depB = createDepositTx({ TransactionID: 'b', TxHash: 'hb' }); + + service['handleScryptTransactions']([depA, depB]); + await expect(flushQueue()).resolves.toBeUndefined(); + + expect(exchangeTxRepo.save).toHaveBeenCalledTimes(2); + }); + + it('skips event-driven writes while the sync process is disabled', async () => { + jest.spyOn(processServiceModule, 'DisabledProcess').mockReturnValue(true); + jest.spyOn(exchangeTxRepo, 'findOneBy').mockResolvedValue(null); + + service['handleScryptTransactions']([createDepositTx()]); + await flushQueue(); + + expect(exchangeTxRepo.findOneBy).not.toHaveBeenCalled(); + expect(exchangeTxRepo.save).not.toHaveBeenCalled(); + }); + }); + + describe('syncExchanges stale guard', () => { + let scryptService: ScryptService; + + // pass an explicit `from` so getSyncSinceDate (which reads global Config) is bypassed + const syncFrom = Util.daysBefore(1); + + beforeEach(() => { + scryptService = Object.create(ScryptService.prototype); + (scryptService as unknown as { getAllTransactions: jest.Mock }).getAllTransactions = jest.fn(); + (scryptService as unknown as { getTrades: jest.Mock }).getTrades = jest.fn().mockResolvedValue([]); + + jest.spyOn(registryService, 'getExchange').mockReturnValue(scryptService); + }); + + it('skips overwriting a row the event path already updated with fresher data', async () => { + const snapshotTs = new Date(Date.now() - 60_000).toISOString(); // run-start snapshot is 1 min stale + const snapshot = createDepositTx({ + TransactionID: 'tx-sync', + TxHash: 'hash-sync', + Timestamp: snapshotTs, + TransactTime: snapshotTs, + }); + (scryptService as unknown as { getAllTransactions: jest.Mock }).getAllTransactions.mockResolvedValue([snapshot]); + + const fresher = { + id: 9, + exchange: ExchangeName.SCRYPT, + type: ExchangeTxType.DEPOSIT, + externalId: 'tx-sync', + externalUpdated: new Date(), + amount: 100.5, + amountChf: 100.5, + status: 'ok', + } as ExchangeTx; + jest.spyOn(exchangeTxRepo, 'findOneBy').mockResolvedValue(fresher); + + await service.syncExchanges(syncFrom, ExchangeName.SCRYPT); + + expect(exchangeTxRepo.save).not.toHaveBeenCalled(); + }); + + it('still saves when the snapshot is not older than the row', async () => { + const ts = new Date(); + const snapshot = createDepositTx({ + TransactionID: 'tx-sync-2', + TxHash: 'hash-sync-2', + Timestamp: ts.toISOString(), + TransactTime: ts.toISOString(), + }); + (scryptService as unknown as { getAllTransactions: jest.Mock }).getAllTransactions.mockResolvedValue([snapshot]); + + const existing = { + id: 10, + exchange: ExchangeName.SCRYPT, + type: ExchangeTxType.DEPOSIT, + externalId: 'tx-sync-2', + externalUpdated: new Date(ts), // equal timestamp → guard does not skip + amount: 100.5, + amountChf: 100.5, // already priced → no pricing calls needed + status: 'ok', + } as ExchangeTx; + jest.spyOn(exchangeTxRepo, 'findOneBy').mockResolvedValue(existing); + + await service.syncExchanges(syncFrom, ExchangeName.SCRYPT); + + expect(exchangeTxRepo.save).toHaveBeenCalledTimes(1); + }); + + it('isolates a failing record (e.g. unique-index race) and continues with the rest of the run', async () => { + const ts = new Date(); + const snapshotA = createDepositTx({ + TransactionID: 'tx-race-a', + TxHash: 'hash-race-a', + Timestamp: ts.toISOString(), + TransactTime: ts.toISOString(), + }); + const snapshotB = createDepositTx({ + TransactionID: 'tx-race-b', + TxHash: 'hash-race-b', + Timestamp: ts.toISOString(), + TransactTime: ts.toISOString(), + }); + (scryptService as unknown as { getAllTransactions: jest.Mock }).getAllTransactions.mockResolvedValue([ + snapshotA, + snapshotB, + ]); + + const existingFor = (id: number, externalId: string) => + ({ + id, + exchange: ExchangeName.SCRYPT, + type: ExchangeTxType.DEPOSIT, + externalId, + externalUpdated: new Date(ts), // equal timestamp → guard does not skip + amount: 100.5, + amountChf: 100.5, // already priced → no pricing calls needed + status: 'ok', + }) as ExchangeTx; + jest + .spyOn(exchangeTxRepo, 'findOneBy') + .mockResolvedValueOnce(existingFor(11, 'tx-race-a')) + .mockResolvedValueOnce(existingFor(12, 'tx-race-b')); + + // first save loses the insert race (unique index), second one succeeds + jest + .spyOn(exchangeTxRepo, 'save') + .mockRejectedValueOnce(new Error('unique constraint')) + .mockResolvedValueOnce({} as ExchangeTx); + + await expect(service.syncExchanges(syncFrom, ExchangeName.SCRYPT)).resolves.toBeUndefined(); + + expect(exchangeTxRepo.save).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/src/integration/exchange/services/exchange-tx.service.ts b/src/integration/exchange/services/exchange-tx.service.ts index 84ca48897e..c3cb9d82bd 100644 --- a/src/integration/exchange/services/exchange-tx.service.ts +++ b/src/integration/exchange/services/exchange-tx.service.ts @@ -1,10 +1,10 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, OnModuleInit } from '@nestjs/common'; import { CronExpression } from '@nestjs/schedule'; import { Config } from 'src/config/config'; import { AssetService } from 'src/shared/models/asset/asset.service'; import { FiatService } from 'src/shared/models/fiat/fiat.service'; import { DfxLogger } from 'src/shared/services/dfx-logger'; -import { Process } from 'src/shared/services/process.service'; +import { DisabledProcess, Process } from 'src/shared/services/process.service'; import { DfxCron } from 'src/shared/utils/cron'; import { Util } from 'src/shared/utils/util'; import { @@ -14,6 +14,7 @@ import { } from 'src/subdomains/supporting/pricing/services/pricing.service'; import { FindOptionsRelations, In, MoreThan, MoreThanOrEqual } from 'typeorm'; import { ExchangeTxDto } from '../dto/exchange-tx.dto'; +import { ScryptBalanceTransaction, ScryptTransactionType } from '../dto/scrypt.dto'; import { ExchangeSync, ExchangeSyncs, ExchangeTx, ExchangeTxType } from '../entities/exchange-tx.entity'; import { ExchangeName } from '../enums/exchange.enum'; import { ExchangeTxMapper } from '../mappers/exchange-tx.mapper'; @@ -22,11 +23,14 @@ import { ExchangeRegistryService } from './exchange-registry.service'; import { ScryptService } from './scrypt.service'; @Injectable() -export class ExchangeTxService { +export class ExchangeTxService implements OnModuleInit { private readonly logger = new DfxLogger(ExchangeTxService); private readonly syncWarningsLogged = new Set(); + // serializes event-driven upserts so overlapping WS callbacks cannot interleave DB writes + private scryptTxQueue: Promise = Promise.resolve(); + constructor( private readonly exchangeTxRepo: ExchangeTxRepository, private readonly registryService: ExchangeRegistryService, @@ -35,6 +39,61 @@ export class ExchangeTxService { private readonly fiatService: FiatService, ) {} + onModuleInit() { + const scryptService = this.registryService.getExchange(ExchangeName.SCRYPT); + if (scryptService instanceof ScryptService && scryptService.isConfigured) { + scryptService.onBalanceTransactions((transactions) => this.handleScryptTransactions(transactions)); + } + } + + //*** EVENTS ***// + + private handleScryptTransactions(transactions: ScryptBalanceTransaction[]): void { + // the 5-minute sync also persists these; skip event-driven writes when it is disabled + if (DisabledProcess(Process.EXCHANGE_TX_SYNC)) return; + + // deposits only — withdrawals/trades keep their sync-only persistence timing, and the type + // filter on the raw records makes unknown transaction types harmless (never reach the mapper); + // the recency bound trims reconnect snapshot bursts while live pushes trivially pass + const deposits = transactions.filter( + (t) => + t.TransactionType === ScryptTransactionType.DEPOSIT && + t.Timestamp && + new Date(t.Timestamp) >= Util.daysBefore(1), + ); + if (!deposits.length) return; + + const dtos = ExchangeTxMapper.mapScryptTransactions(deposits, ExchangeName.SCRYPT); + + this.scryptTxQueue = this.scryptTxQueue + .then(() => this.upsertScryptTx(dtos)) + .catch((e) => this.logger.warn('Failed to persist Scrypt deposit transactions:', e)); + } + + private async upsertScryptTx(dtos: ExchangeTxDto[]): Promise { + for (const dto of dtos) { + try { + let entity = await this.exchangeTxRepo.findOneBy({ + exchange: dto.exchange, + externalId: dto.externalId, + type: dto.type, + }); + + // stale event: the row already carries fresher data (later push or sync) — keep it + if (entity?.externalUpdated && dto.externalUpdated && entity.externalUpdated > dto.externalUpdated) continue; + + // no pricing calls in the event path — amountChf/feeAmountChf stay untouched (they are not + // on the DTO, so Object.assign preserves them) and the sync backfills them on its next run + entity = entity ? Object.assign(entity, dto) : this.exchangeTxRepo.create(dto); + + await this.exchangeTxRepo.save(entity); + } catch (e) { + // unique-index race or transient DB error — the 5-minute sync backfills the record + this.logger.warn(`Failed to persist Scrypt deposit ${dto.externalId}:`, e); + } + } + } + //*** JOBS ***// @DfxCron(CronExpression.EVERY_5_MINUTES, { process: Process.EXCHANGE_TX_SYNC, timeout: 1800 }) @@ -51,82 +110,100 @@ export class ExchangeTxService { transactions.sort((a, b) => a.externalCreated.getTime() - b.externalCreated.getTime()); for (const transaction of transactions) { - let entity = await this.exchangeTxRepo.findOneBy({ - exchange: transaction.exchange, - externalId: transaction.externalId, - type: transaction.type, - }); - - // Preserve calculated spread fee for Scrypt (DTO's feeAmount=0 would overwrite it) - const preservedSpreadFee = - entity?.exchange === ExchangeName.SCRYPT && entity.feeAmountChf != null - ? { feeAmount: entity.feeAmount, feeCurrency: entity.feeCurrency, feeAmountChf: entity.feeAmountChf } - : undefined; - - entity = entity ? Object.assign(entity, transaction) : this.exchangeTxRepo.create(transaction); - - if (preservedSpreadFee) { - entity.feeAmount = preservedSpreadFee.feeAmount; - entity.feeCurrency = preservedSpreadFee.feeCurrency; - entity.feeAmountChf = preservedSpreadFee.feeAmountChf; - } - - // Calculate spread fee for new Scrypt trades - if ( - entity.exchange === ExchangeName.SCRYPT && - entity.type === ExchangeTxType.TRADE && - entity.price && - entity.amount && - entity.feeAmountChf == null - ) { - try { - await this.calculateSpreadFee(entity); - } catch (e) { - this.logger.warn(`Failed to calculate spread fee for Scrypt trade ${entity.externalId}:`, e); + try { + let entity = await this.exchangeTxRepo.findOneBy({ + exchange: transaction.exchange, + externalId: transaction.externalId, + type: transaction.type, + }); + + // the run reads the map once at start and saves for minutes — do not let that stale + // snapshot overwrite rows the event-driven path has already updated + if ( + entity?.externalUpdated && + transaction.externalUpdated && + entity.externalUpdated > transaction.externalUpdated + ) + continue; + + // Preserve calculated spread fee for Scrypt (DTO's feeAmount=0 would overwrite it) + const preservedSpreadFee = + entity?.exchange === ExchangeName.SCRYPT && entity.feeAmountChf != null + ? { feeAmount: entity.feeAmount, feeCurrency: entity.feeCurrency, feeAmountChf: entity.feeAmountChf } + : undefined; + + entity = entity ? Object.assign(entity, transaction) : this.exchangeTxRepo.create(transaction); + + if (preservedSpreadFee) { + entity.feeAmount = preservedSpreadFee.feeAmount; + entity.feeCurrency = preservedSpreadFee.feeCurrency; + entity.feeAmountChf = preservedSpreadFee.feeAmountChf; } - } - if (entity.feeAmount && !entity.feeAmountChf) { - if (entity.feeCurrency === 'CHF') { - entity.feeAmountChf = entity.feeAmount; - } else { - const feeAsset = - (await this.fiatService.getFiatByName(entity.feeCurrency)) ?? - (await this.assetService.getAssetByQuery({ - blockchain: undefined, - type: undefined, - name: entity.feeCurrency, - })); - if (!feeAsset) throw new Error(`Unknown fee currency ${entity.feeCurrency}`); - - const price = await this.pricingService.getPrice(feeAsset, PriceCurrency.CHF, PriceValidity.ANY); - - entity.feeAmountChf = price.convert(entity.feeAmount, Config.defaultVolumeDecimal); + // Calculate spread fee for new Scrypt trades + if ( + entity.exchange === ExchangeName.SCRYPT && + entity.type === ExchangeTxType.TRADE && + entity.price && + entity.amount && + entity.feeAmountChf == null + ) { + try { + await this.calculateSpreadFee(entity); + } catch (e) { + this.logger.warn(`Failed to calculate spread fee for Scrypt trade ${entity.externalId}:`, e); + } } - } - if (entity.amount && !entity.amountChf) { - const currencyName = entity.type === ExchangeTxType.TRADE ? entity.symbol.split('/')[0] : entity.currency; - - if (currencyName === 'CHF') { - entity.amountChf = entity.amount; - } else { - const currency = - (await this.fiatService.getFiatByName(currencyName)) ?? - (await this.assetService.getAssetByQuery({ - blockchain: undefined, - type: undefined, - name: currencyName, - })); - if (!currency) throw new Error(`Unknown currency ${currencyName}`); - - const priceChf = await this.pricingService.getPrice(currency, PriceCurrency.CHF, PriceValidity.ANY); + if (entity.feeAmount && !entity.feeAmountChf) { + if (entity.feeCurrency === 'CHF') { + entity.feeAmountChf = entity.feeAmount; + } else { + const feeAsset = + (await this.fiatService.getFiatByName(entity.feeCurrency)) ?? + (await this.assetService.getAssetByQuery({ + blockchain: undefined, + type: undefined, + name: entity.feeCurrency, + })); + if (!feeAsset) throw new Error(`Unknown fee currency ${entity.feeCurrency}`); + + const price = await this.pricingService.getPrice(feeAsset, PriceCurrency.CHF, PriceValidity.ANY); + + entity.feeAmountChf = price.convert(entity.feeAmount, Config.defaultVolumeDecimal); + } + } - entity.amountChf = priceChf.convert(entity.amount, Config.defaultVolumeDecimal); + if (entity.amount && !entity.amountChf) { + const currencyName = entity.type === ExchangeTxType.TRADE ? entity.symbol.split('/')[0] : entity.currency; + + if (currencyName === 'CHF') { + entity.amountChf = entity.amount; + } else { + const currency = + (await this.fiatService.getFiatByName(currencyName)) ?? + (await this.assetService.getAssetByQuery({ + blockchain: undefined, + type: undefined, + name: currencyName, + })); + if (!currency) throw new Error(`Unknown currency ${currencyName}`); + + const priceChf = await this.pricingService.getPrice(currency, PriceCurrency.CHF, PriceValidity.ANY); + + entity.amountChf = priceChf.convert(entity.amount, Config.defaultVolumeDecimal); + } } - } - await this.exchangeTxRepo.save(entity); + await this.exchangeTxRepo.save(entity); + } catch (e) { + // one failing record (e.g. unique-index race with the event-driven Scrypt path, or a pricing + // failure) must not abort the rest of the run — the record is retried on the next sync + this.logger.warn( + `Failed to sync exchange tx ${transaction.exchange}/${transaction.type}/${transaction.externalId}:`, + e, + ); + } } } diff --git a/src/integration/exchange/services/scrypt.service.ts b/src/integration/exchange/services/scrypt.service.ts index 637caac598..3a712a3ca4 100644 --- a/src/integration/exchange/services/scrypt.service.ts +++ b/src/integration/exchange/services/scrypt.service.ts @@ -212,6 +212,10 @@ export class ScryptService extends PricingProvider { // --- TRANSACTIONS --- // + onBalanceTransactions(callback: (transactions: ScryptBalanceTransaction[]) => void): void { + this.connection.subscribeToStream(ScryptMessageType.BALANCE_TRANSACTION, callback); + } + async getAllTransactions(since?: Date): Promise { const transactions = Array.from(this.balanceTransactions.values()); return transactions.filter((t) => !since || (t.TransactTime && new Date(t.TransactTime) >= since)); From ffad6881c82b9e21e500a20ceef10e6deb0ec206 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 3 Jul 2026 03:06:17 +0200 Subject: [PATCH 04/10] feat(realunit): add customer-scoped RealUnit support dashboard endpoints (#4054) * feat(realunit): add RealunitScopeService (tenant customer scope from serviceProvider marker) * feat(realunit): add customer-scoped RealUnit support dashboard endpoints * fix(realunit): align RealUnit casing and use exact token match for customer scope --- .../models/user-data/user-data.service.ts | 11 ++ .../__tests__/realunit-scope.service.spec.ts | 53 +++++++ .../realunit-support.controller.ts | 140 ++++++++++++++++++ .../realunit/realunit-scope.service.ts | 25 ++++ .../supporting/realunit/realunit.module.ts | 10 +- .../support-issue-dto.mapper.spec.ts | 44 ++++++ .../dto/support-issue-dto.mapper.ts | 8 +- .../services/support-issue.service.ts | 88 ++++++++--- 8 files changed, 355 insertions(+), 24 deletions(-) create mode 100644 src/subdomains/supporting/realunit/__tests__/realunit-scope.service.spec.ts create mode 100644 src/subdomains/supporting/realunit/controllers/realunit-support.controller.ts create mode 100644 src/subdomains/supporting/realunit/realunit-scope.service.ts create mode 100644 src/subdomains/supporting/support-issue/dto/__tests__/support-issue-dto.mapper.spec.ts diff --git a/src/subdomains/generic/user/models/user-data/user-data.service.ts b/src/subdomains/generic/user/models/user-data/user-data.service.ts index 9211a47bb9..b2301ebdf8 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.service.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.service.ts @@ -159,6 +159,17 @@ export class UserDataService { return this.userDataRepo.find({ where: { id: In(ids) } }); } + async getUserDataIdsByServiceProvider(provider: ServiceProvider): Promise { + // exact token match on the semicolon list (mirrors the backfill migration and the UserData.isRealUnitCustomer + // getter), so the scope source and the per-issue membership check share one definition and cannot diverge + return this.userDataRepo + .createQueryBuilder('userData') + .select('userData.id', 'id') + .where(`';' || userData.serviceProviders || ';' LIKE :pattern`, { pattern: `%;${provider};%` }) + .getRawMany<{ id: number }>() + .then((rows) => rows.map((r) => r.id)); + } + async getByKycHashOrThrow(kycHash: string, relations?: FindOptionsRelations): Promise { if (!Config.formats.kycHash.test(kycHash)) throw new UnauthorizedException('Invalid KYC hash'); diff --git a/src/subdomains/supporting/realunit/__tests__/realunit-scope.service.spec.ts b/src/subdomains/supporting/realunit/__tests__/realunit-scope.service.spec.ts new file mode 100644 index 0000000000..c6860e27af --- /dev/null +++ b/src/subdomains/supporting/realunit/__tests__/realunit-scope.service.spec.ts @@ -0,0 +1,53 @@ +import { NotFoundException } from '@nestjs/common'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; +import { ServiceProvider } from 'src/subdomains/generic/user/models/user-data/user-data.enum'; +import { UserDataService } from 'src/subdomains/generic/user/models/user-data/user-data.service'; +import { RealUnitScopeService } from 'src/subdomains/supporting/realunit/realunit-scope.service'; + +describe('RealUnitScopeService', () => { + let service: RealUnitScopeService; + let userDataService: DeepMocked; + + const realUnitCustomer = Object.assign(new UserData(), { id: 1, serviceProviders: ServiceProvider.REALUNIT }); + const dfxCustomer = Object.assign(new UserData(), { id: 2, serviceProviders: undefined }); + + beforeEach(() => { + userDataService = createMock(); + service = new RealUnitScopeService(userDataService); + }); + + describe('isCustomer', () => { + it('returns true for a RealUnit customer', async () => { + userDataService.getUserData.mockResolvedValue(realUnitCustomer); + await expect(service.isCustomer(1)).resolves.toBe(true); + }); + + it('returns false for a non-RealUnit account', async () => { + userDataService.getUserData.mockResolvedValue(dfxCustomer); + await expect(service.isCustomer(2)).resolves.toBe(false); + }); + + it('returns false (fail-closed) for an unknown account', async () => { + userDataService.getUserData.mockResolvedValue(null); + await expect(service.isCustomer(999)).resolves.toBe(false); + }); + }); + + describe('assertCustomer', () => { + it('resolves for a RealUnit customer', async () => { + userDataService.getUserData.mockResolvedValue(realUnitCustomer); + await expect(service.assertCustomer(1)).resolves.toBeUndefined(); + }); + + it('throws NotFound for a non-member (tenant isolation, no existence leak)', async () => { + userDataService.getUserData.mockResolvedValue(dfxCustomer); + await expect(service.assertCustomer(2)).rejects.toBeInstanceOf(NotFoundException); + }); + + it('throws NotFound for an unknown account', async () => { + userDataService.getUserData.mockResolvedValue(null); + await expect(service.assertCustomer(999)).rejects.toBeInstanceOf(NotFoundException); + }); + }); +}); diff --git a/src/subdomains/supporting/realunit/controllers/realunit-support.controller.ts b/src/subdomains/supporting/realunit/controllers/realunit-support.controller.ts new file mode 100644 index 0000000000..e438be3cff --- /dev/null +++ b/src/subdomains/supporting/realunit/controllers/realunit-support.controller.ts @@ -0,0 +1,140 @@ +import { Body, Controller, Get, Param, Post, Put, Query, UseGuards } from '@nestjs/common'; +import { AuthGuard } from '@nestjs/passport'; +import { ApiBearerAuth, ApiExcludeEndpoint, ApiTags } from '@nestjs/swagger'; +import { BlobContent } from 'src/integration/infrastructure/azure-storage.service'; +import { GetJwt } from 'src/shared/auth/get-jwt.decorator'; +import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; +import { RoleGuard } from 'src/shared/auth/role.guard'; +import { UserActiveGuard } from 'src/shared/auth/user-active.guard'; +import { UserRole } from 'src/shared/auth/user-role.enum'; +import { TfaGuard } from 'src/subdomains/generic/kyc/guards/tfa.guard'; +import { CreateSupportMessageDto } from 'src/subdomains/supporting/support-issue/dto/create-support-message.dto'; +import { GetSupportIssueListFilter } from 'src/subdomains/supporting/support-issue/dto/get-support-issue.dto'; +import { + SupportIssueInternalDataDto, + SupportIssueListDto, + SupportIssueStatisticsDto, + SupportMessageDto, +} from 'src/subdomains/supporting/support-issue/dto/support-issue.dto'; +import { UpdateSupportIssueDto } from 'src/subdomains/supporting/support-issue/dto/update-support-issue.dto'; +import { SupportIssue } from 'src/subdomains/supporting/support-issue/entities/support-issue.entity'; +import { SupportIssueInternalState } from 'src/subdomains/supporting/support-issue/enums/support-issue.enum'; +import { SupportIssueService } from 'src/subdomains/supporting/support-issue/services/support-issue.service'; +import { RealUnitScopeService } from '../realunit-scope.service'; + +// RealUnit tenant support dashboard: lets RealUnit staff (UserRole.REALUNIT) manage ONLY their own customers' +// support issues. Every endpoint is strictly customer-scoped and fail-closed; the DFX RoleGuard is never widened +// and no Department.REALUNIT exists. Aggregates are filtered by the RealUnit customer id set; single-issue routes +// enforce membership before any data or mutation (fail-closed 404 for a foreign or unknown id). +@ApiTags('Realunit') +@Controller('realunit/support') +export class RealUnitSupportController { + constructor( + private readonly supportIssueService: SupportIssueService, + private readonly scopeService: RealUnitScopeService, + ) {} + + @Get('list') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async getSupportIssueList( + @GetJwt() jwt: JwtPayload, + @Query() filter: GetSupportIssueListFilter, + ): Promise<{ data: SupportIssueListDto[]; total: number }> { + const customerIds = await this.scopeService.getCustomerIds(); + return this.supportIssueService.getSupportIssueList(filter, jwt.role, customerIds); + } + + @Get('counts') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async getSupportIssueCounts(@GetJwt() jwt: JwtPayload): Promise> { + const customerIds = await this.scopeService.getCustomerIds(); + return this.supportIssueService.getSupportIssueCounts(jwt.role, customerIds); + } + + @Get('statistics') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async getSupportIssueStatistics( + @GetJwt() jwt: JwtPayload, + @Query('days') days?: string, + ): Promise { + const customerIds = await this.scopeService.getCustomerIds(); + return this.supportIssueService.getSupportIssueStatistics(jwt.role, days ? +days : undefined, customerIds); + } + + @Get('activity') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async getSupportIssueActivity( + @GetJwt() jwt: JwtPayload, + @Query('since') since?: string, + ): Promise<{ count: number; latestAt?: Date }> { + const customerIds = await this.scopeService.getCustomerIds(); + return this.supportIssueService.getSupportIssueActivity(since ? new Date(since) : undefined, jwt.role, customerIds); + } + + @Get('clerks') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async getRealUnitSupportClerks(): Promise { + return this.supportIssueService.getRealUnitSupportClerks(); + } + + @Get(':id/data') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async getIssueData(@GetJwt() jwt: JwtPayload, @Param('id') id: string): Promise { + const customerIds = await this.scopeService.getCustomerIds(); + await this.assertIssueOwnership(+id); // membership before returning; getIssueData(customerIds) also enforces it + return this.supportIssueService.getIssueData(+id, jwt.role, customerIds); + } + + @Put(':id') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async updateSupportIssue(@Param('id') id: string, @Body() dto: UpdateSupportIssueDto): Promise { + await this.assertIssueOwnership(+id); + return this.supportIssueService.updateIssue(+id, dto); + } + + @Post(':id/message') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async createSupportMessage( + @Param('id') id: string, + @Body() dto: CreateSupportMessageDto, + ): Promise { + await this.assertIssueOwnership(+id); + return this.supportIssueService.createMessageSupport(+id, dto); + } + + @Get(':id/message/:messageId/file') + @ApiBearerAuth() + @ApiExcludeEndpoint() + @UseGuards(AuthGuard(), RoleGuard(UserRole.REALUNIT), UserActiveGuard(), TfaGuard) + async getFile(@Param('id') id: string, @Param('messageId') messageId: string): Promise { + const userDataId = await this.assertIssueOwnership(+id); + return this.supportIssueService.getIssueFile(id, +messageId, userDataId); + } + + // --- HELPER METHODS --- // + + // Enforces the RealUnit tenant boundary for a single issue: resolves the owning userData id and asserts it is a + // RealUnit customer, throwing NotFound (fail-closed 404, no existence leak) for a foreign or unknown issue. + private async assertIssueOwnership(issueId: number): Promise { + const userDataId = await this.supportIssueService.getIssueUserDataId(issueId); + await this.scopeService.assertCustomer(userDataId); + + return userDataId; + } +} diff --git a/src/subdomains/supporting/realunit/realunit-scope.service.ts b/src/subdomains/supporting/realunit/realunit-scope.service.ts new file mode 100644 index 0000000000..da5295428a --- /dev/null +++ b/src/subdomains/supporting/realunit/realunit-scope.service.ts @@ -0,0 +1,25 @@ +import { Injectable, NotFoundException } from '@nestjs/common'; +import { ServiceProvider } from 'src/subdomains/generic/user/models/user-data/user-data.enum'; +import { UserDataService } from 'src/subdomains/generic/user/models/user-data/user-data.service'; + +// Resolves the RealUnit tenant customer scope from the persisted UserData.serviceProviders marker (NOT the +// unreliable User.wallet). Everything is fail-closed: an empty scope yields an empty result, never unrestricted, +// and a non-member id is treated as a missing resource (404) so tenant boundaries never leak existence. +@Injectable() +export class RealUnitScopeService { + constructor(private readonly userDataService: UserDataService) {} + + async getCustomerIds(): Promise { + return this.userDataService.getUserDataIdsByServiceProvider(ServiceProvider.REALUNIT); + } + + async isCustomer(userDataId: number): Promise { + const userData = await this.userDataService.getUserData(userDataId); + if (!userData) return false; // fail-closed: an unknown account is not a RealUnit customer + return userData.isRealUnitCustomer; + } + + async assertCustomer(userDataId: number): Promise { + if (!(await this.isCustomer(userDataId))) throw new NotFoundException('Not found'); + } +} diff --git a/src/subdomains/supporting/realunit/realunit.module.ts b/src/subdomains/supporting/realunit/realunit.module.ts index 7ada921602..a6210b3e6d 100644 --- a/src/subdomains/supporting/realunit/realunit.module.ts +++ b/src/subdomains/supporting/realunit/realunit.module.ts @@ -9,15 +9,18 @@ import { FaucetRequestModule } from 'src/subdomains/core/faucet-request/faucet-r import { SellCryptoModule } from 'src/subdomains/core/sell-crypto/sell-crypto.module'; import { KycModule } from 'src/subdomains/generic/kyc/kyc.module'; import { UserModule } from 'src/subdomains/generic/user/user.module'; +import { SupportIssueModule } from 'src/subdomains/supporting/support-issue/support-issue.module'; import { BalanceModule } from '../balance/balance.module'; import { BankTxModule } from '../bank-tx/bank-tx.module'; import { BankModule } from '../bank/bank.module'; import { PaymentModule } from '../payment/payment.module'; import { TransactionModule } from '../payment/transaction.module'; import { PricingModule } from '../pricing/pricing.module'; +import { RealUnitSupportController } from './controllers/realunit-support.controller'; import { RealUnitController } from './controllers/realunit.controller'; import { RealUnitDevService } from './realunit-dev.service'; import { RealUnitJobService } from './realunit-job.service'; +import { RealUnitScopeService } from './realunit-scope.service'; import { RealUnitService } from './realunit.service'; @Module({ @@ -38,9 +41,10 @@ import { RealUnitService } from './realunit.service'; forwardRef(() => BuyCryptoModule), forwardRef(() => SellCryptoModule), FaucetRequestModule, + SupportIssueModule, ], - controllers: [RealUnitController], - providers: [RealUnitService, RealUnitDevService, RealUnitJobService], - exports: [RealUnitService], + controllers: [RealUnitController, RealUnitSupportController], + providers: [RealUnitService, RealUnitDevService, RealUnitJobService, RealUnitScopeService], + exports: [RealUnitService, RealUnitScopeService], }) export class RealUnitModule {} diff --git a/src/subdomains/supporting/support-issue/dto/__tests__/support-issue-dto.mapper.spec.ts b/src/subdomains/supporting/support-issue/dto/__tests__/support-issue-dto.mapper.spec.ts new file mode 100644 index 0000000000..7731459032 --- /dev/null +++ b/src/subdomains/supporting/support-issue/dto/__tests__/support-issue-dto.mapper.spec.ts @@ -0,0 +1,44 @@ +import { UserRole } from 'src/shared/auth/user-role.enum'; +import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; +import { LimitRequest } from 'src/subdomains/supporting/support-issue/entities/limit-request.entity'; +import { SupportIssue } from 'src/subdomains/supporting/support-issue/entities/support-issue.entity'; +import { Department } from 'src/subdomains/supporting/support-issue/enums/department.enum'; +import { + SupportIssueInternalState, + SupportIssueReason, + SupportIssueType, +} from 'src/subdomains/supporting/support-issue/enums/support-issue.enum'; +import { SupportIssueDtoMapper } from 'src/subdomains/supporting/support-issue/dto/support-issue-dto.mapper'; + +describe('SupportIssueDtoMapper.mapSupportIssueData limitRequest redaction', () => { + // mirrors the role -> flag rule in SupportIssueService.getIssueData: DFX Support staff and RealUnit tenant staff + // must not see the DFX AML-internal limit request; Compliance / Admin do. + const hideLimitRequest = (role: UserRole): boolean => [UserRole.SUPPORT, UserRole.REALUNIT].includes(role); + + function makeIssue(): SupportIssue { + return Object.assign(new SupportIssue(), { + id: 1, + created: new Date('2026-01-01T00:00:00.000Z'), + uid: 'Iabc', + type: SupportIssueType.GENERIC_ISSUE, + department: Department.COMPLIANCE, + reason: SupportIssueReason.OTHER, + state: SupportIssueInternalState.PENDING, + name: 'Help', + clerk: 'Alice', + userData: Object.assign(new UserData(), { id: 42 }), + limitRequest: Object.assign(new LimitRequest(), { id: 7, limit: 100000 }), + }); + } + + it.each([UserRole.SUPPORT, UserRole.REALUNIT])('hides the limit request for %s staff', (role) => { + const dto = SupportIssueDtoMapper.mapSupportIssueData(makeIssue(), hideLimitRequest(role)); + expect(dto.limitRequest).toBeUndefined(); + }); + + it.each([UserRole.COMPLIANCE, UserRole.ADMIN])('exposes the limit request for %s', (role) => { + const dto = SupportIssueDtoMapper.mapSupportIssueData(makeIssue(), hideLimitRequest(role)); + expect(dto.limitRequest).toBeDefined(); + expect(dto.limitRequest?.id).toBe(7); + }); +}); diff --git a/src/subdomains/supporting/support-issue/dto/support-issue-dto.mapper.ts b/src/subdomains/supporting/support-issue/dto/support-issue-dto.mapper.ts index 49e3225df7..0f5d7d148a 100644 --- a/src/subdomains/supporting/support-issue/dto/support-issue-dto.mapper.ts +++ b/src/subdomains/supporting/support-issue/dto/support-issue-dto.mapper.ts @@ -1,4 +1,3 @@ -import { UserRole } from 'src/shared/auth/user-role.enum'; import { CountryDtoMapper } from 'src/shared/models/country/dto/country-dto.mapper'; import { LanguageDtoMapper } from 'src/shared/models/language/dto/language-dto.mapper'; import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; @@ -37,7 +36,9 @@ export class SupportIssueDtoMapper { return Object.assign(new SupportIssueDto(), dto); } - static mapSupportIssueData(supportIssue: SupportIssue, role: UserRole): SupportIssueInternalDataDto { + // `hideLimitRequest` redacts the DFX AML-internal limit request for callers who must not see it + // (DFX Support staff and RealUnit tenant staff); Compliance/Admin pass false and keep it. + static mapSupportIssueData(supportIssue: SupportIssue, hideLimitRequest: boolean): SupportIssueInternalDataDto { const dto: SupportIssueInternalDataDto = { id: supportIssue.id, created: supportIssue.created, @@ -50,8 +51,7 @@ export class SupportIssueDtoMapper { clerk: supportIssue.clerk, account: SupportIssueDtoMapper.mapUserData(supportIssue.userData), transaction: SupportIssueDtoMapper.mapTransactionData(supportIssue.transaction), - limitRequest: - role === UserRole.SUPPORT ? undefined : SupportIssueDtoMapper.mapLimitRequestData(supportIssue.limitRequest), + limitRequest: hideLimitRequest ? undefined : SupportIssueDtoMapper.mapLimitRequestData(supportIssue.limitRequest), transactionMissing: SupportIssueDtoMapper.mapTransactionMissingData(supportIssue), }; diff --git a/src/subdomains/supporting/support-issue/services/support-issue.service.ts b/src/subdomains/supporting/support-issue/services/support-issue.service.ts index c5f39753ce..fd5c87d60c 100644 --- a/src/subdomains/supporting/support-issue/services/support-issue.service.ts +++ b/src/subdomains/supporting/support-issue/services/support-issue.service.ts @@ -71,6 +71,11 @@ export class SupportIssueService { return clerks.length > 0 ? clerks : ['Support']; } + async getRealUnitSupportClerks(): Promise { + const clerks = await this.settingService.getObj('realUnitSupportClerks', []); + return clerks.length > 0 ? clerks : ['Support']; + } + // Resolves the clerk name assigned to a support account via the `supportClerkAccounts` // setting ([{ account, name }]). Returns undefined if the account is unmapped. async getSupportIssueClerkForAccount(account: number): Promise { @@ -78,21 +83,27 @@ export class SupportIssueService { return clerks.find((c) => c.account === account)?.name; } - async getSupportIssueCounts(role: UserRole): Promise> { + async getSupportIssueCounts( + role: UserRole, + customerIds?: number[], + ): Promise> { const counts = Object.values(SupportIssueInternalState).reduce( (acc, state) => ({ ...acc, [state]: 0 }), {} as Record, ); const departments = getVisibleDepartments(role); - if (departments?.length === 0) return counts; // no department access + if (!customerIds && departments?.length === 0) return counts; // no department access + if (customerIds && !customerIds.length) return counts; // fail-closed: empty customer scope const qb = this.supportIssueRepo .createQueryBuilder('issue') .select('issue.state', 'state') .addSelect('COUNT(*)', 'count') .groupBy('issue.state'); - if (departments) qb.andWhere('issue.department IN (:...departments)', { departments }); + if (customerIds) + qb.innerJoin('issue.userData', 'scopeUd').andWhere('scopeUd.id IN (:...customerIds)', { customerIds }); + else if (departments) qb.andWhere('issue.department IN (:...departments)', { departments }); const raw: { state: SupportIssueInternalState; count: string }[] = await qb.getRawMany(); for (const row of raw) counts[row.state] = +row.count; @@ -100,9 +111,14 @@ export class SupportIssueService { return counts; } - async getSupportIssueActivity(since: Date | undefined, role: UserRole): Promise<{ count: number; latestAt?: Date }> { + async getSupportIssueActivity( + since: Date | undefined, + role: UserRole, + customerIds?: number[], + ): Promise<{ count: number; latestAt?: Date }> { const departments = getVisibleDepartments(role); - if (departments?.length === 0) return { count: 0, latestAt: undefined }; // no department access + if (!customerIds && departments?.length === 0) return { count: 0, latestAt: undefined }; // no department access + if (customerIds && !customerIds.length) return { count: 0, latestAt: undefined }; // fail-closed: empty customer scope const qb = this.messageRepo .createQueryBuilder('m') @@ -110,13 +126,18 @@ export class SupportIssueService { .select('COUNT(*)', 'count') .addSelect('MAX(m.created)', 'latestAt'); if (since) qb.andWhere('m.created > :since', { since: since.toISOString() }); - if (departments) qb.andWhere('i.department IN (:...departments)', { departments }); + if (customerIds) qb.innerJoin('i.userData', 'scopeUd').andWhere('scopeUd.id IN (:...customerIds)', { customerIds }); + else if (departments) qb.andWhere('i.department IN (:...departments)', { departments }); const raw = await qb.getRawOne<{ count: string | number; latestAt: Date | null }>(); return { count: +(raw?.count ?? 0), latestAt: raw?.latestAt ?? undefined }; } - async getSupportIssueStatistics(role: UserRole, periodDays = 365): Promise { + async getSupportIssueStatistics( + role: UserRole, + periodDays = 365, + customerIds?: number[], + ): Promise { const departments = getVisibleDepartments(role); // guard against a non-numeric ?days reaching the clamp as NaN (which would propagate to an Invalid Date) const days = Number.isFinite(periodDays) ? Math.min(Math.max(Math.round(periodDays), 1), 366) : 365; @@ -125,7 +146,8 @@ export class SupportIssueService { // no department access: return an empty statistic without querying. An empty `departments` list would // otherwise reach the queries below, where `if (departments)` is truthy and expands to a degenerate // `IN ()` clause (the same fail-closed contract the counts / activity / list queries already follow). - if (departments?.length === 0) + // A customer-scoped (RealUnit) caller bypasses the department gate but fail-closes on an empty scope. + if ((!customerIds && departments?.length === 0) || (customerIds && !customerIds.length)) return { periodDays: days, total: 0, @@ -146,7 +168,9 @@ export class SupportIssueService { .createQueryBuilder('issue') .select('COUNT(*)', 'count') .where('issue.created >= :from', { from }); - if (departments) totalQb.andWhere('issue.department IN (:...departments)', { departments }); + if (customerIds) + totalQb.innerJoin('issue.userData', 'scopeUd').andWhere('scopeUd.id IN (:...customerIds)', { customerIds }); + else if (departments) totalQb.andWhere('issue.department IN (:...departments)', { departments }); const total = +((await totalQb.getRawOne<{ count: string }>())?.count ?? 0); const msgQb = this.messageRepo @@ -154,7 +178,9 @@ export class SupportIssueService { .innerJoin('m.issue', 'issue') .select('COUNT(*)', 'count') .where('issue.created >= :from', { from }); - if (departments) msgQb.andWhere('issue.department IN (:...departments)', { departments }); + if (customerIds) + msgQb.innerJoin('issue.userData', 'scopeUd').andWhere('scopeUd.id IN (:...customerIds)', { customerIds }); + else if (departments) msgQb.andWhere('issue.department IN (:...departments)', { departments }); const messages = +((await msgQb.getRawOne<{ count: string }>())?.count ?? 0); // trend buckets: always group by day in SQL (CAST avoids Postgres-incompatible date-part functions and @@ -165,7 +191,9 @@ export class SupportIssueService { .addSelect('COUNT(*)', 'count') .where('issue.created >= :from', { from }) .groupBy('CAST(issue.created AS DATE)'); - if (departments) trendQb.andWhere('issue.department IN (:...departments)', { departments }); + if (customerIds) + trendQb.innerJoin('issue.userData', 'scopeUd').andWhere('scopeUd.id IN (:...customerIds)', { customerIds }); + else if (departments) trendQb.andWhere('issue.department IN (:...departments)', { departments }); const dayRows = await trendQb.getRawMany<{ d: Date | string; count: string }>(); // build keys from local date parts on both the rows and the bucket loop; the app and DB both run UTC, @@ -211,7 +239,9 @@ export class SupportIssueService { .addSelect('issue.updated', 'updated') .where('issue.state = :completed', { completed: SupportIssueInternalState.COMPLETED }) .andWhere('issue.updated >= :from', { from }); - if (departments) resolvedQb.andWhere('issue.department IN (:...departments)', { departments }); + if (customerIds) + resolvedQb.innerJoin('issue.userData', 'scopeUd').andWhere('scopeUd.id IN (:...customerIds)', { customerIds }); + else if (departments) resolvedQb.andWhere('issue.department IN (:...departments)', { departments }); const resolvedRows = await resolvedQb.getRawMany<{ type: string; created: Date; updated: Date }>(); const resolutionStats = new Map(); @@ -432,12 +462,14 @@ export class SupportIssueService { async getSupportIssueList( filter: GetSupportIssueListFilter, role: UserRole, + customerIds?: number[], ): Promise<{ data: SupportIssueListDto[]; total: number }> { const where: FindOptionsWhere = {}; // department filtering: the role defines the allowed departments, an explicit filter may narrow within them const allowedDepartments = getVisibleDepartments(role); - if (allowedDepartments?.length === 0) return { data: [], total: 0 }; // no department access + if (!customerIds && allowedDepartments?.length === 0) return { data: [], total: 0 }; // no department access + if (customerIds && !customerIds.length) return { data: [], total: 0 }; // fail-closed: empty customer scope const departments = filter.department && (!allowedDepartments || allowedDepartments.includes(filter.department)) @@ -454,9 +486,13 @@ export class SupportIssueService { .slice(0, 10); const qb = this.supportIssueRepo.createQueryBuilder('issue'); - if (terms.length > 0) qb.leftJoin('issue.userData', 'userData'); + // the search predicate and the RealUnit customer scope both need the userData join; share the single 'userData' alias + if (terms.length > 0 || customerIds) qb.leftJoin('issue.userData', 'userData'); - if (departments) qb.andWhere('issue.department IN (:...departments)', { departments }); + // customer scope (RealUnit) takes precedence over and replaces the department gate; the left join + IN filter + // fail-closes issues without a userData (NULL is never IN the scope list) + if (customerIds) qb.andWhere('"userData".id IN (:...customerIds)', { customerIds }); + else if (departments) qb.andWhere('issue.department IN (:...departments)', { departments }); if (filter.states?.length) qb.andWhere('issue.state IN (:...states)', { states: filter.states }); if (where.type) qb.andWhere('issue.type = :type', { type: where.type }); if (filter.clerk) qb.andWhere('issue.clerk = :clerk', { clerk: filter.clerk }); @@ -584,7 +620,7 @@ export class SupportIssueService { return SupportIssueDtoMapper.mapSupportIssue(issue); } - async getIssueData(id: number, role: UserRole): Promise { + async getIssueData(id: number, role: UserRole, customerIds?: number[]): Promise { const issue = await this.supportIssueRepo.findOne({ where: { id }, relations: { @@ -599,8 +635,13 @@ export class SupportIssueService { loadEagerRelations: false, }); if (!issue) throw new NotFoundException('Support issue not found'); + // customer scope (RealUnit): fail-closed 404 when the issue does not belong to a scoped customer (no existence leak) + if (customerIds && !customerIds.includes(issue.userData?.id)) + throw new NotFoundException('Support issue not found'); - return SupportIssueDtoMapper.mapSupportIssueData(issue, role); + // DFX Support and RealUnit tenant staff must not see the DFX AML-internal limit request + const hideLimitRequest = [UserRole.SUPPORT, UserRole.REALUNIT].includes(role); + return SupportIssueDtoMapper.mapSupportIssueData(issue, hideLimitRequest); } async getIssueFile(id: string, messageId: number, userDataId?: number): Promise { @@ -623,6 +664,19 @@ export class SupportIssueService { }; } + // Resolves the owning userData id of an issue by numeric id, so a caller can enforce a membership/tenant + // boundary before a mutating or data call. Throws NotFound (no existence leak) when the issue or its owner is missing. + async getIssueUserDataId(id: number): Promise { + const issue = await this.supportIssueRepo.findOne({ + where: { id }, + relations: { userData: true }, + loadEagerRelations: false, + }); + if (!issue?.userData) throw new NotFoundException('Support issue not found'); + + return issue.userData.id; + } + // --- HELPER METHODS --- // async createMessageInternal(issue: SupportIssue, dto: CreateSupportMessageDto): Promise { From ed055bc21c8dcc41ad2c3325cee1d9d8834e6f51 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 3 Jul 2026 03:32:20 +0200 Subject: [PATCH 05/10] feat(auth): centralize staff 2FA enforcement and auto-revoke blocked accounts (#4052/#4053) (#4055) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(auth): enforce staff 2FA centrally via a global interceptor Per-route TfaGuard only gates routes that opt in, so role-authorized routes without it — optional-auth branches and service-layer role checks — silently skipped 2FA (that was #4031/#4036). Add a global TfaEnforcementInterceptor that requires a valid STRICT app-2FA for any request carrying a mail-origin staff token (tfaRequired), except endpoints marked @AllowTfaPending (the 2FA-completion flow). It runs after guards so request.user is populated, fast-paths all non- tfaRequired traffic on a single property read, and resolves TfaService lazily via ModuleRef (mirrors TfaGuard) to avoid a module-import cycle. Per-route TfaGuard is now redundant and can be retired in a follow-up. Closes #4052 * feat(auth): auto-revoke JWTs of blocked or deactivated accounts The account-id JWT denylist (#4028/#4050) was only ever populated manually, so blocking or deactivating an account did not revoke its live tokens. Add a per-minute JwtRevocationSyncService in the user domain that derives blocked account ids from the DB (status BLOCKED/DEACTIVATED or risk BLOCKED/SUSPICIOUS) into a dedicated jwtAccountDenylistAuto setting; SettingService.getDeniedJwtAccounts now returns the deduped union of the manual override and this auto list. Every user token carries the userDataId as account, so this revokes all of a blocked user's tokens within ~1 min, from any block path, and self-heals on reactivation. The shared ProcessService/SettingService stay free of subdomain dependencies. Closes #4053 * fix(auth): apply review findings - Add the missing-account guard clause to TfaEnforcementInterceptor for parity with TfaGuard (fails closed regardless; tfaRequired is always minted with an account), plus a test covering it. - Fix Prettier formatting in the revocation-sync spec. --- src/app.module.ts | 8 +- .../tfa-enforcement.interceptor.spec.ts | 97 +++++++++++++++++++ .../auth/allow-tfa-pending.decorator.ts | 10 ++ .../auth/tfa-enforcement.interceptor.ts | 58 +++++++++++ .../setting/__tests__/setting.service.spec.ts | 58 +++++++++++ src/shared/models/setting/setting.service.ts | 8 +- .../user/models/auth/auth.controller.ts | 4 + .../jwt-revocation-sync.service.spec.ts | 66 +++++++++++++ .../user-data/jwt-revocation-sync.service.ts | 43 ++++++++ src/subdomains/generic/user/user.module.ts | 2 + 10 files changed, 351 insertions(+), 3 deletions(-) create mode 100644 src/shared/auth/__tests__/tfa-enforcement.interceptor.spec.ts create mode 100644 src/shared/auth/allow-tfa-pending.decorator.ts create mode 100644 src/shared/auth/tfa-enforcement.interceptor.ts create mode 100644 src/shared/models/setting/__tests__/setting.service.spec.ts create mode 100644 src/subdomains/generic/user/models/user-data/__tests__/jwt-revocation-sync.service.spec.ts create mode 100644 src/subdomains/generic/user/models/user-data/jwt-revocation-sync.service.ts diff --git a/src/app.module.ts b/src/app.module.ts index 984c5fa800..e762e5f825 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -1,15 +1,19 @@ import { Module } from '@nestjs/common'; +import { APP_INTERCEPTOR } from '@nestjs/core'; import { TypeOrmModule } from '@nestjs/typeorm'; import { AppController } from './app.controller'; -import { SharedModule } from './shared/shared.module'; import { GetConfig } from './config/config'; import { IntegrationModule } from './integration/integration.module'; +import { TfaEnforcementInterceptor } from './shared/auth/tfa-enforcement.interceptor'; +import { SharedModule } from './shared/shared.module'; import { SubdomainsModule } from './subdomains/subdomains.module'; @Module({ imports: [TypeOrmModule.forRoot(GetConfig().database), SharedModule, IntegrationModule, SubdomainsModule], controllers: [AppController], - providers: [], + // Global backstop enforcing STRICT app-2FA on every route for a mail-origin staff session (tfaRequired). + // Only needs ModuleRef + Reflector (both globally available), so no extra module imports are required. + providers: [{ provide: APP_INTERCEPTOR, useClass: TfaEnforcementInterceptor }], exports: [], }) export class AppModule {} diff --git a/src/shared/auth/__tests__/tfa-enforcement.interceptor.spec.ts b/src/shared/auth/__tests__/tfa-enforcement.interceptor.spec.ts new file mode 100644 index 0000000000..2ba80919e8 --- /dev/null +++ b/src/shared/auth/__tests__/tfa-enforcement.interceptor.spec.ts @@ -0,0 +1,97 @@ +import { createMock } from '@golevelup/ts-jest'; +import { CallHandler, ExecutionContext, ForbiddenException } from '@nestjs/common'; +import { ModuleRef, Reflector } from '@nestjs/core'; +import { of } from 'rxjs'; +import { TfaRequiredException } from 'src/subdomains/generic/kyc/exceptions/tfa-required.exception'; +import { TfaLevel, TfaService } from 'src/subdomains/generic/kyc/services/tfa.service'; +import { TfaEnforcementInterceptor } from '../tfa-enforcement.interceptor'; + +// tfa.service transitively imports the kyc entity graph, which is circular at import time and breaks when +// this isolated unit test is the first to load it. The interceptor only needs the service's shape and the +// TfaLevel enum, so the module is replaced at runtime; the real types are still used for type-checking. +jest.mock('src/subdomains/generic/kyc/services/tfa.service', () => ({ + TfaLevel: { BASIC: 'Basic', STRICT: 'Strict' }, + TfaService: class TfaService {}, +})); + +describe('TfaEnforcementInterceptor', () => { + let interceptor: TfaEnforcementInterceptor; + let tfaService: TfaService; + let moduleRef: ModuleRef; + let reflector: Reflector; + let next: CallHandler; + let handle: jest.Mock; + + const context = (request: any): ExecutionContext => + createMock({ switchToHttp: () => ({ getRequest: () => request }) as any }); + + beforeEach(() => { + tfaService = createMock(); + moduleRef = createMock(); + reflector = createMock(); + jest.spyOn(moduleRef, 'get').mockReturnValue(tfaService); + + handle = jest.fn().mockReturnValue(of('handled')); + next = { handle } as CallHandler; + + interceptor = new TfaEnforcementInterceptor(moduleRef, reflector); + }); + + it('enforces STRICT 2FA for a mail-origin staff token (tfaRequired, live request ip) and continues on success', async () => { + jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(false); + jest.spyOn(tfaService, 'check').mockResolvedValue(undefined); + + await interceptor.intercept(context({ user: { account: 7, tfaRequired: true }, realIp: '1.2.3.4' }), next); + + expect(moduleRef.get).toHaveBeenCalledWith(TfaService, { strict: false }); + expect(tfaService.check).toHaveBeenCalledWith(7, '1.2.3.4', TfaLevel.STRICT); + expect(handle).toHaveBeenCalled(); + }); + + it('propagates TfaRequiredException and does not continue when 2FA is missing', async () => { + jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(false); + jest.spyOn(tfaService, 'check').mockRejectedValue(new TfaRequiredException(TfaLevel.STRICT)); + + await expect( + interceptor.intercept(context({ user: { account: 7, tfaRequired: true }, realIp: '1.2.3.4' }), next), + ).rejects.toBeInstanceOf(TfaRequiredException); + expect(handle).not.toHaveBeenCalled(); + }); + + it('rejects a tfaRequired token with no account (fails closed) without calling the 2FA check', async () => { + jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(false); + + await expect( + interceptor.intercept(context({ user: { tfaRequired: true }, realIp: '1.2.3.4' }), next), + ).rejects.toBeInstanceOf(ForbiddenException); + expect(tfaService.check).not.toHaveBeenCalled(); + expect(handle).not.toHaveBeenCalled(); + }); + + it('skips enforcement on an @AllowTfaPending endpoint so a tfaRequired token can complete 2FA', async () => { + jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(true); + + await interceptor.intercept(context({ user: { account: 7, tfaRequired: true }, realIp: '1.2.3.4' }), next); + + expect(handle).toHaveBeenCalled(); + expect(tfaService.check).not.toHaveBeenCalled(); + expect(moduleRef.get).not.toHaveBeenCalled(); + }); + + it('fast-path: passes through without touching Reflector/TfaService when tfaRequired is absent', async () => { + await interceptor.intercept(context({ user: { account: 1, role: 'Support' } }), next); + + expect(handle).toHaveBeenCalled(); + expect(reflector.getAllAndOverride).not.toHaveBeenCalled(); + expect(moduleRef.get).not.toHaveBeenCalled(); + expect(tfaService.check).not.toHaveBeenCalled(); + }); + + it('fast-path: passes through a public route without request.user', async () => { + await interceptor.intercept(context({}), next); + + expect(handle).toHaveBeenCalled(); + expect(reflector.getAllAndOverride).not.toHaveBeenCalled(); + expect(moduleRef.get).not.toHaveBeenCalled(); + }); +}); diff --git a/src/shared/auth/allow-tfa-pending.decorator.ts b/src/shared/auth/allow-tfa-pending.decorator.ts new file mode 100644 index 0000000000..261fab3269 --- /dev/null +++ b/src/shared/auth/allow-tfa-pending.decorator.ts @@ -0,0 +1,10 @@ +import { CustomDecorator, SetMetadata } from '@nestjs/common'; + +// Metadata key marking an endpoint a mail-origin staff token (tfaRequired) may reach BEFORE completing 2FA. +// Shared between the decorator and TfaEnforcementInterceptor so the marker and its reader can never drift. +export const AllowTfaPendingKey = 'allowTfaPending'; + +// Marks the 2FA-completion endpoints (see AuthController) as reachable by a not-yet-verified mail-origin staff +// token. The global TfaEnforcementInterceptor skips its STRICT-2FA enforcement on any handler or controller +// carrying this marker; without it a tfaRequired token could never reach the flow that clears the marker. +export const AllowTfaPending = (): CustomDecorator => SetMetadata(AllowTfaPendingKey, true); diff --git a/src/shared/auth/tfa-enforcement.interceptor.ts b/src/shared/auth/tfa-enforcement.interceptor.ts new file mode 100644 index 0000000000..0a4f1f825c --- /dev/null +++ b/src/shared/auth/tfa-enforcement.interceptor.ts @@ -0,0 +1,58 @@ +import { CallHandler, ExecutionContext, ForbiddenException, Injectable, NestInterceptor } from '@nestjs/common'; +import { ModuleRef, Reflector } from '@nestjs/core'; +import { Observable } from 'rxjs'; +import { AllowTfaPendingKey } from 'src/shared/auth/allow-tfa-pending.decorator'; +import { TfaLevel, TfaService } from 'src/subdomains/generic/kyc/services/tfa.service'; + +// Global backstop for the "mail-origin staff ⇒ STRICT app-2FA" invariant. A staff role reached via the weaker +// mail-magic-link factor carries request.user.tfaRequired (stamped when the token is minted, see +// AuthService.generateUserToken); such a session must complete STRICT app/TOTP 2FA before it may touch ANY +// route. Enforcing this per-route via TfaGuard is easy to forget on optional-auth branches and service-layer +// role checks (that gap was #4031/#4036) — this interceptor closes it in ONE place so it cannot be forgotten. +// +// - Registered as an APP_INTERCEPTOR: interceptors run AFTER guards, so request.user is already populated here +// (a guard would run before AuthGuard and see no user). +// - FAST PATH: any request without a tfaRequired token — wallet-signature staff (never stamped) and all +// non-staff traffic — returns on a single property read, with zero Reflector/DB/ModuleRef overhead. This is +// critical: the interceptor is global and must be a no-op for essentially all traffic. +// - Endpoints marked @AllowTfaPending (the 2FA-completion endpoints) are skipped, otherwise a tfaRequired +// token could never reach the flow that clears its marker. +// - TfaService is resolved lazily via ModuleRef (mirrors TfaGuard) so AppModule need not import KycModule, +// which would create a module-import cycle. +// - With this global backstop in place the per-route TfaGuard decorators are now redundant and can be retired +// in a follow-up; they are kept here belt-and-suspenders to keep this change minimal. +@Injectable() +export class TfaEnforcementInterceptor implements NestInterceptor { + constructor( + private readonly moduleRef: ModuleRef, + private readonly reflector: Reflector, + ) {} + + async intercept(context: ExecutionContext, next: CallHandler): Promise> { + const request = context.switchToHttp().getRequest(); + + // FAST PATH: only mail-origin staff sessions (tfaRequired) are gated; this single property read + // short-circuits all other traffic before any Reflector/DB/ModuleRef work runs. + if (!request.user?.tfaRequired) return next.handle(); + + // The 2FA-completion endpoints must stay reachable so a tfaRequired token can actually clear its marker. + const allowPending = this.reflector.getAllAndOverride(AllowTfaPendingKey, [ + context.getHandler(), + context.getClass(), + ]); + if (allowPending) return next.handle(); + + // Mirror TfaGuard: a tfaRequired token is always minted with an account, but guard against a malformed one. + if (!request.user.account) throw new ForbiddenException('User not authenticated'); + + // live request IP (mirrors RealIP), so the check matches the IP the 2FA log was written with + const ip = request.realIp ?? request.socket?.remoteAddress ?? 'unknown'; + + // Throws TfaRequiredException (403, code TFA_REQUIRED) when no valid STRICT app-2FA log exists; the staff + // frontend already routes that code through the 2FA flow. request.user.account is the userDataId. + const tfaService = this.moduleRef.get(TfaService, { strict: false }); + await tfaService.check(request.user.account, ip, TfaLevel.STRICT); + + return next.handle(); + } +} diff --git a/src/shared/models/setting/__tests__/setting.service.spec.ts b/src/shared/models/setting/__tests__/setting.service.spec.ts new file mode 100644 index 0000000000..427114219d --- /dev/null +++ b/src/shared/models/setting/__tests__/setting.service.spec.ts @@ -0,0 +1,58 @@ +import { createMock } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { Setting } from '../setting.entity'; +import { SettingRepository } from '../setting.repository'; +import { SettingService } from '../setting.service'; + +describe('SettingService', () => { + let service: SettingService; + let settingRepo: jest.Mocked; + + function mockSettings(values: Record): void { + settingRepo.findOneBy.mockImplementation(async ({ key }: { key: string }) => + key in values ? Object.assign(new Setting(), { key, value: JSON.stringify(values[key]) }) : null, + ); + } + + beforeEach(async () => { + settingRepo = createMock(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [SettingService, { provide: SettingRepository, useValue: settingRepo }], + }).compile(); + + service = module.get(SettingService); + }); + + describe('getDeniedJwtAccounts', () => { + it('returns the deduped union of the manual and auto denylists as numbers', async () => { + mockSettings({ jwtAccountDenylist: [1], jwtAccountDenylistAuto: [2, 2, 3] }); + + await expect(service.getDeniedJwtAccounts()).resolves.toEqual([1, 2, 3]); + }); + + it('dedupes ids present in both the manual and auto denylists', async () => { + mockSettings({ jwtAccountDenylist: [1, 2], jwtAccountDenylistAuto: [2, 3] }); + + await expect(service.getDeniedJwtAccounts()).resolves.toEqual([1, 2, 3]); + }); + + it('coerces string ids to numbers', async () => { + mockSettings({ jwtAccountDenylist: ['1'], jwtAccountDenylistAuto: ['2'] }); + + await expect(service.getDeniedJwtAccounts()).resolves.toEqual([1, 2]); + }); + + it('returns an empty array when both settings are missing', async () => { + mockSettings({}); + + await expect(service.getDeniedJwtAccounts()).resolves.toEqual([]); + }); + + it('returns only the auto denylist when the manual override is unset', async () => { + mockSettings({ jwtAccountDenylistAuto: [5, 6] }); + + await expect(service.getDeniedJwtAccounts()).resolves.toEqual([5, 6]); + }); + }); +}); diff --git a/src/shared/models/setting/setting.service.ts b/src/shared/models/setting/setting.service.ts index ddf0c47879..39da8a4865 100644 --- a/src/shared/models/setting/setting.service.ts +++ b/src/shared/models/setting/setting.service.ts @@ -109,7 +109,13 @@ export class SettingService { } async getDeniedJwtAccounts(): Promise { - return this.getObj('jwtAccountDenylist', []).then((list) => list.map(Number)); + // Union of the manual override (`jwtAccountDenylist`) and the DB-derived auto denylist + // (`jwtAccountDenylistAuto`, maintained by JwtRevocationSyncService), deduped to numbers. + const [manual, auto] = await Promise.all([ + this.getObj<(string | number)[]>('jwtAccountDenylist', []), + this.getObj<(string | number)[]>('jwtAccountDenylistAuto', []), + ]); + return [...new Set([...manual, ...auto].map(Number))]; } async getCustomBalanceSettings(): Promise<{ addresses: string[]; assets: string[] }> { diff --git a/src/subdomains/generic/user/models/auth/auth.controller.ts b/src/subdomains/generic/user/models/auth/auth.controller.ts index 857bdf7510..dad1b9c19b 100644 --- a/src/subdomains/generic/user/models/auth/auth.controller.ts +++ b/src/subdomains/generic/user/models/auth/auth.controller.ts @@ -3,6 +3,7 @@ import { AuthGuard } from '@nestjs/passport'; import { ApiBearerAuth, ApiCreatedResponse, ApiExcludeEndpoint, ApiOkResponse, ApiTags } from '@nestjs/swagger'; import { Throttle } from '@nestjs/throttler'; import { Request, Response } from 'express'; +import { AllowTfaPending } from 'src/shared/auth/allow-tfa-pending.decorator'; import { RealIP } from 'src/shared/auth/real-ip.decorator'; import { GetJwt } from 'src/shared/auth/get-jwt.decorator'; import { IpCountryGuard } from 'src/shared/auth/ip-country.guard'; @@ -90,6 +91,7 @@ export class AuthController { // Lets a logged-in user (e.g. staff who reached a staff endpoint and got TFA_REQUIRED) set up and // verify 2FA via their session token, resolving the kycHash from jwt.account. Reuses TfaService. @Get('2fa') + @AllowTfaPending() @ApiBearerAuth() @ApiOkResponse({ description: '2FA active' }) @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard()) @@ -98,6 +100,7 @@ export class AuthController { } @Post('2fa') + @AllowTfaPending() @ApiBearerAuth() @ApiCreatedResponse({ type: Setup2faDto }) @UseGuards(AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard()) @@ -111,6 +114,7 @@ export class AuthController { } @Post('2fa/verify') + @AllowTfaPending() @ApiBearerAuth() @ApiCreatedResponse({ description: '2FA successful' }) @UseGuards(RateLimitGuard, AuthGuard(), RoleGuard(UserRole.ACCOUNT), UserActiveGuard()) diff --git a/src/subdomains/generic/user/models/user-data/__tests__/jwt-revocation-sync.service.spec.ts b/src/subdomains/generic/user/models/user-data/__tests__/jwt-revocation-sync.service.spec.ts new file mode 100644 index 0000000000..ba33d7eeb1 --- /dev/null +++ b/src/subdomains/generic/user/models/user-data/__tests__/jwt-revocation-sync.service.spec.ts @@ -0,0 +1,66 @@ +import { createMock } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { SettingService } from 'src/shared/models/setting/setting.service'; +import { FindOperator, In } from 'typeorm'; +import { JwtRevocationSyncService } from '../jwt-revocation-sync.service'; +import { UserData } from '../user-data.entity'; +import { RiskStatus, UserDataStatus } from '../user-data.enum'; +import { UserDataRepository } from '../user-data.repository'; + +describe('JwtRevocationSyncService', () => { + let service: JwtRevocationSyncService; + let userDataRepo: jest.Mocked; + let settingService: jest.Mocked; + + beforeEach(async () => { + userDataRepo = createMock(); + settingService = createMock(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + JwtRevocationSyncService, + { provide: UserDataRepository, useValue: userDataRepo }, + { provide: SettingService, useValue: settingService }, + ], + }).compile(); + + service = module.get(JwtRevocationSyncService); + }); + + describe('syncDeniedJwtAccounts', () => { + it('writes exactly the ids of blocked/deactivated/suspicious accounts to jwtAccountDenylistAuto', async () => { + userDataRepo.find.mockResolvedValue([ + Object.assign(new UserData(), { id: 3 }), + Object.assign(new UserData(), { id: 7 }), + ]); + + await service.syncDeniedJwtAccounts(); + + expect(settingService.setObj).toHaveBeenCalledWith('jwtAccountDenylistAuto', [3, 7]); + }); + + it('writes an empty array when no account is blocked', async () => { + userDataRepo.find.mockResolvedValue([]); + + await service.syncDeniedJwtAccounts(); + + expect(settingService.setObj).toHaveBeenCalledWith('jwtAccountDenylistAuto', []); + }); + + it('filters in SQL on blocking status OR risk status and selects only the id', async () => { + userDataRepo.find.mockResolvedValue([]); + + await service.syncDeniedJwtAccounts(); + + const options = userDataRepo.find.mock.calls[0][0]; + expect(options.select).toEqual({ id: true }); + + const where = options.where as [ + { status: FindOperator }, + { riskStatus: FindOperator }, + ]; + expect(where[0].status).toEqual(In([UserDataStatus.BLOCKED, UserDataStatus.DEACTIVATED])); + expect(where[1].riskStatus).toEqual(In([RiskStatus.BLOCKED, RiskStatus.SUSPICIOUS])); + }); + }); +}); diff --git a/src/subdomains/generic/user/models/user-data/jwt-revocation-sync.service.ts b/src/subdomains/generic/user/models/user-data/jwt-revocation-sync.service.ts new file mode 100644 index 0000000000..7a50409395 --- /dev/null +++ b/src/subdomains/generic/user/models/user-data/jwt-revocation-sync.service.ts @@ -0,0 +1,43 @@ +import { Injectable } from '@nestjs/common'; +import { CronExpression } from '@nestjs/schedule'; +import { SettingService } from 'src/shared/models/setting/setting.service'; +import { DfxCron } from 'src/shared/utils/cron'; +import { In } from 'typeorm'; +import { RiskStatus, UserDataStatus } from './user-data.enum'; +import { UserDataRepository } from './user-data.repository'; + +// Auto-populates the `jwtAccountDenylistAuto` setting from the DB so that blocking or deactivating an +// account revokes its live JWTs without a manual denylist edit. Every user token (wallet-signature and +// mail/account) carries the userDataId as `account`, so an account-id denylist revokes all of a blocked +// user's tokens. `ProcessService` primes the in-memory denied-account Set from +// `SettingService.getDeniedJwtAccounts()` (which unions this auto setting with the manual override), and +// `JwtStrategy.validate` rejects any token whose account is denied. Reactivation self-heals: a reactivated +// account drops out of the query and is removed from the setting on the next run. +// +// This cron lives in the user domain (owns UserData) to keep the shared `ProcessService`/`SettingService` +// free of subdomain dependencies. +@Injectable() +export class JwtRevocationSyncService { + constructor( + private readonly userDataRepo: UserDataRepository, + private readonly settingService: SettingService, + ) {} + + // Runs every minute: fast revocation of a blocked or compromised account is a security requirement that + // warrants the security-revocation exception to the "prefer 15min" cron guideline. + @DfxCron(CronExpression.EVERY_MINUTE, { timeout: 1800 }) + async syncDeniedJwtAccounts(): Promise { + const blockedAccounts = await this.userDataRepo.find({ + select: { id: true }, + where: [ + { status: In([UserDataStatus.BLOCKED, UserDataStatus.DEACTIVATED]) }, + { riskStatus: In([RiskStatus.BLOCKED, RiskStatus.SUSPICIOUS]) }, + ], + }); + + await this.settingService.setObj( + 'jwtAccountDenylistAuto', + blockedAccounts.map((account) => account.id), + ); + } +} diff --git a/src/subdomains/generic/user/user.module.ts b/src/subdomains/generic/user/user.module.ts index 7f734378d1..5fce80a978 100644 --- a/src/subdomains/generic/user/user.module.ts +++ b/src/subdomains/generic/user/user.module.ts @@ -44,6 +44,7 @@ import { RecommendationService } from './models/recommendation/recommendation.se import { UserDataRelationController } from './models/user-data-relation/user-data-relation.controller'; import { UserDataRelationRepository } from './models/user-data-relation/user-data-relation.repository'; import { UserDataRelationService } from './models/user-data-relation/user-data-relation.service'; +import { JwtRevocationSyncService } from './models/user-data/jwt-revocation-sync.service'; import { UserDataJobService } from './models/user-data/user-data-job.service'; import { UserDataNotificationService } from './models/user-data/user-data-notification.service'; import { UserData } from './models/user-data/user-data.entity'; @@ -124,6 +125,7 @@ import { WebhookService } from './services/webhook/webhook.service'; OrganizationService, OrganizationRepository, UserDataJobService, + JwtRevocationSyncService, UserJobService, RecommendationRepository, RecommendationService, From 54175b735f0b311fe0f3498c1690df65a3eb5ffd Mon Sep 17 00:00:00 2001 From: Danswar <48102227+Danswar@users.noreply.github.com> Date: Fri, 3 Jul 2026 05:25:47 -0300 Subject: [PATCH 06/10] fix(kyc): restore kycFileId assignment (NULLS-FIRST max bug) and make it race-safe (#4023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(kyc): compute kycFileId from the real max, not a NULLS-FIRST null getLastKycFileId() did findOne({ order: { kycFileId: 'DESC' } }). On PostgreSQL ORDER BY … DESC is NULLS FIRST, and most user_data rows have no kycFileId, so it returned a null row → the AML without-KYC path computed null + 1 = 1, and every such user collided with the legacy holder of kycFileId 1, throwing 'A user with this KYC file ID already exists' (~10/h on prod) while never assigning a real number (kycFileId assignment has been broken since the MSSQL→PostgreSQL migration; the real max is ~6076). Filter NULLs so the query returns the true max (next ≈ 6077), and guard the empty case with ?? 0. * fix(kyc): close the concurrent kycFileId assignment race getLastKycFileId() + updateUserDataInternal()'s uniqueness check was a read-then-write race with no DB backstop (kycFileId had no unique index), so two concurrent AML postProcessing calls (e.g. buy-crypto and buy-fiat pipelines running at the same time) could both compute and assign the same id. Add a partial unique index on kycFileId (mirrors the existing apiKeyCT / kyc_step patterns) as the real constraint, and replace the method with assignNextKycFileId(), which retries with a fresh max on conflict - mirroring the existing concurrent-create-race handling in getOrCreateStepInternal. * refactor(kyc): use generated index name, note sequence tradeoff Review feedback: drop the custom index name for the TypeORM-generated one (IDX_8dae6f6af0a6b5dc2ec16c333c, repo convention), and document why max+1 with retry was kept over a Postgres sequence (gapless ids; nextval burns numbers on rollback and doesn't follow manual/merge kycFileId writes). --- ...1659474-AddUserDataKycFileIdUniqueIndex.js | 29 ++++++++++ .../core/aml/services/aml.service.ts | 3 +- .../__tests__/user-data.service.spec.ts | 57 ++++++++++++++++++- .../user/models/user-data/user-data.entity.ts | 1 + .../models/user-data/user-data.service.ts | 21 ++++++- 5 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 migration/1782911659474-AddUserDataKycFileIdUniqueIndex.js diff --git a/migration/1782911659474-AddUserDataKycFileIdUniqueIndex.js b/migration/1782911659474-AddUserDataKycFileIdUniqueIndex.js new file mode 100644 index 0000000000..e51f2cf518 --- /dev/null +++ b/migration/1782911659474-AddUserDataKycFileIdUniqueIndex.js @@ -0,0 +1,29 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * Backs the app-level kycFileId uniqueness check (UserDataService.updateUserDataInternal) with a real + * constraint - that check is a read-then-write race between concurrent AML postProcessing calls. + * + * @class + * @implements {MigrationInterface} + */ +module.exports = class AddUserDataKycFileIdUniqueIndex1782911659474 { + name = 'AddUserDataKycFileIdUniqueIndex1782911659474'; + + /** + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + await queryRunner.query(`CREATE UNIQUE INDEX "IDX_8dae6f6af0a6b5dc2ec16c333c" ON "user_data" ("kycFileId") WHERE "kycFileId" IS NOT NULL`); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + await queryRunner.query(`DROP INDEX "public"."IDX_8dae6f6af0a6b5dc2ec16c333c"`); + } +}; diff --git a/src/subdomains/core/aml/services/aml.service.ts b/src/subdomains/core/aml/services/aml.service.ts index 2e14242ae2..37d6aa9122 100644 --- a/src/subdomains/core/aml/services/aml.service.ts +++ b/src/subdomains/core/aml/services/aml.service.ts @@ -103,8 +103,7 @@ export class AmlService { (!entity.cryptoInput || entity.cryptoInput.txType !== PayInType.PAYMENT) && last30dVolume > Config.tradingLimits.monthlyDefaultWoKyc ) { - const kycFileId = (await this.userDataService.getLastKycFileId()) + 1; - await this.userDataService.updateUserDataInternal(entity.userData, { kycFileId, amlListAddedDate: new Date() }); + entity.userData = await this.userDataService.assignNextKycFileId(entity.userData); } } } diff --git a/src/subdomains/generic/user/models/user-data/__tests__/user-data.service.spec.ts b/src/subdomains/generic/user/models/user-data/__tests__/user-data.service.spec.ts index 1618d3b153..91eba75647 100644 --- a/src/subdomains/generic/user/models/user-data/__tests__/user-data.service.spec.ts +++ b/src/subdomains/generic/user/models/user-data/__tests__/user-data.service.spec.ts @@ -1,7 +1,7 @@ import { createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { ConflictException } from '@nestjs/common'; -import { FindOperator } from 'typeorm'; +import { FindOperator, IsNull, Not } from 'typeorm'; import { RepositoryFactory } from 'src/shared/repositories/repository.factory'; import { CountryService } from 'src/shared/models/country/country.service'; import { FiatService } from 'src/shared/models/fiat/fiat.service'; @@ -105,6 +105,61 @@ describe('UserDataService', () => { }); }); + describe('assignNextKycFileId', () => { + it('starts at 1 when no user has a kycFileId yet', async () => { + const userData = Object.assign(new UserData(), { id: 1 }); + + userDataRepo.findOne.mockResolvedValue(null); // max lookup: table empty + userDataRepo.findOneBy.mockResolvedValue(null); // uniqueness check: no conflict + userDataRepo.update.mockResolvedValue(undefined); + + const result = await service.assignNextKycFileId(userData); + + expect(result.kycFileId).toBe(1); + }); + + it('assigns the real max + 1 (excluding nulls, not just the last-inserted row)', async () => { + const userData = Object.assign(new UserData(), { id: 1 }); + + userDataRepo.findOne.mockResolvedValue(Object.assign(new UserData(), { kycFileId: 6076 })); + userDataRepo.findOneBy.mockResolvedValue(null); + userDataRepo.update.mockResolvedValue(undefined); + + const result = await service.assignNextKycFileId(userData); + + expect(userDataRepo.findOne.mock.calls[0][0].where).toEqual({ kycFileId: Not(IsNull()) }); + expect(result.kycFileId).toBe(6077); + }); + + it('retries with a fresh max when it loses a concurrent assignment race', async () => { + const userData = Object.assign(new UserData(), { id: 1 }); + const winner = Object.assign(new UserData(), { id: 2, kycFileId: 6077 }); + + userDataRepo.findOne + .mockResolvedValueOnce(Object.assign(new UserData(), { kycFileId: 6076 })) // attempt 0: stale max + .mockResolvedValueOnce(Object.assign(new UserData(), { kycFileId: 6077 })); // attempt 1: winner's row now visible + userDataRepo.findOneBy + .mockResolvedValueOnce(winner) // attempt 0: 6077 already taken by the concurrent winner + .mockResolvedValueOnce(null); // attempt 1: 6078 is free + userDataRepo.update.mockResolvedValue(undefined); + + const result = await service.assignNextKycFileId(userData); + + expect(result.kycFileId).toBe(6078); + expect(userDataRepo.findOne).toHaveBeenCalledTimes(2); + }); + + it('gives up and rethrows after repeated concurrent conflicts', async () => { + const userData = Object.assign(new UserData(), { id: 1 }); + + userDataRepo.findOne.mockResolvedValue(Object.assign(new UserData(), { kycFileId: 6076 })); + userDataRepo.findOneBy.mockResolvedValue(Object.assign(new UserData(), { id: 2 })); // always conflicts + + await expect(service.assignNextKycFileId(userData)).rejects.toBeInstanceOf(ConflictException); + expect(userDataRepo.findOne).toHaveBeenCalledTimes(5); // initial attempt + 4 retries + }); + }); + describe('updateUserData', () => { it('does not pass kycSteps or users to save() to prevent stale-collection FK clobber', async () => { const fakeKycSteps = [{ id: 10 }] as UserData['kycSteps']; diff --git a/src/subdomains/generic/user/models/user-data/user-data.entity.ts b/src/subdomains/generic/user/models/user-data/user-data.entity.ts index 9eb03312d5..0e1971db80 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.entity.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.entity.ts @@ -204,6 +204,7 @@ export class UserData extends IEntity { @OneToMany(() => KycFile, (kycFile) => kycFile.userData) kycFiles?: KycFile[]; + @Index({ unique: true, where: '"kycFileId" IS NOT NULL' }) @Column({ type: 'integer', nullable: true }) kycFileId?: number; diff --git a/src/subdomains/generic/user/models/user-data/user-data.service.ts b/src/subdomains/generic/user/models/user-data/user-data.service.ts index b2301ebdf8..0f0e5ecb15 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.service.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.service.ts @@ -565,8 +565,25 @@ export class UserDataService { return userData; } - async getLastKycFileId(): Promise { - return this.userDataRepo.findOne({ where: {}, order: { kycFileId: 'DESC' } }).then((u) => u.kycFileId); + // Read-max-then-write is racy between concurrent AML postProcessing calls; the unique index on + // kycFileId is the actual backstop, this just retries so the loser gets the next free id. + // max+1 over a sequence (nextval) to keep ids gapless: a sequence burns a number on every + // rolled-back txn and doesn't follow manual/merge kycFileId writes. + async assignNextKycFileId(userData: UserData, attempt = 0): Promise { + // Postgres `ORDER BY … DESC` is NULLS FIRST, so exclude nulls to get the real max. + const last = await this.userDataRepo.findOne({ + where: { kycFileId: Not(IsNull()) }, + order: { kycFileId: 'DESC' }, + }); + const kycFileId = (last?.kycFileId ?? 0) + 1; + + try { + return await this.updateUserDataInternal(userData, { kycFileId, amlListAddedDate: new Date() }); + } catch (e) { + if (attempt >= 4 || !(e instanceof ConflictException || e.message?.includes('duplicate key'))) throw e; + + return this.assignNextKycFileId(userData, attempt + 1); + } } async updatePersonalData(userData: UserData, data: KycPersonalData): Promise { From ed76cee3bc592bc136be2d370b6b94e5e117ca95 Mon Sep 17 00:00:00 2001 From: Danswar <48102227+Danswar@users.noreply.github.com> Date: Fri, 3 Jul 2026 07:04:50 -0300 Subject: [PATCH 07/10] fix(kyc): assign merged kyc steps absolute sequence numbers (#4027) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(kyc): assign merged kyc steps absolute sequence numbers mergeUserData shifted each slave kyc_step by `sequenceNumber + (master.min - 100)` — additive. After migration 1779812989037 made the kyc_step(userDataId, name, type, sequenceNumber) unique index NULLS NOT DISTINCT (2026-05-26), a slave step reshifted onto an existing master tuple throws a duplicate-key error; the merge isn't transactional, so partial writes survive and every retry re-applies the -100, deepening a ladder that re-collides forever and never self-heals. Assign each reassigned slave step a fresh strictly-decreasing number below the master's minimum (min-1, min-2, ...) instead. Absolute, not additive, so it can't collide with any existing master step (even with debris present) and a re-run of a partial merge can't compound. Stuck merges self-heal on the next amlCheck cycle; no data migration required to unblock them. * fix(kyc): start merged sequence numbers below both sides' min The per-step sequenceNumber update runs before save(master) flips userDataId, so the unique index is checked against the SLAVE's own rows at that point. Starting at master.min-1 could collide with steps the slave absorbed while it was a master in an earlier merge (same name/type at small negative numbers) — and deterministically, since the assigned values don't change between retries. Take the min over master and slave steps so assigned values sit below every existing row on either side. * fix(kyc): keep absorbed steps' relative order when renumbering Assign the counter in descending old-sequenceNumber order, so the newest attempt of a step keeps the highest new number — same relative order the old block-shift produced. Matters where the latest attempt is picked by max sequenceNumber among steps that exist only as absorbed history. * test(kyc): cover merge kyc-step renumbering Five specs pinning the renumbering guarantees: numbers strictly below both sides' min, pairwise distinct, same-name attempt order preserved, no landing on a pre-existing (name, sequenceNumber) tuple of either account (the prod collision shape, slave debris at 0/-100…-400), and a re-run of a partially applied merge assigning fresh lower numbers. The collision specs fail against the previous additive-offset implementation. * refactor(kyc): seed merged sequence numbers 100 below the floor Review feedback: the gap makes merge batches recognizable from the sequenceNumber alone when debugging. Collision guarantees unchanged. --------- Co-authored-by: David May --- .../__tests__/user-data.service.spec.ts | 134 ++++++++++++++++++ .../models/user-data/user-data.service.ts | 14 +- 2 files changed, 144 insertions(+), 4 deletions(-) diff --git a/src/subdomains/generic/user/models/user-data/__tests__/user-data.service.spec.ts b/src/subdomains/generic/user/models/user-data/__tests__/user-data.service.spec.ts index 91eba75647..27a05a577e 100644 --- a/src/subdomains/generic/user/models/user-data/__tests__/user-data.service.spec.ts +++ b/src/subdomains/generic/user/models/user-data/__tests__/user-data.service.spec.ts @@ -23,7 +23,11 @@ import { SiftService } from 'src/integration/sift/services/sift.service'; import { OrganizationService } from 'src/subdomains/generic/user/models/organization/organization.service'; import { TfaService } from 'src/subdomains/generic/kyc/services/tfa.service'; import { CustodyService } from 'src/subdomains/core/custody/services/custody.service'; +import { KycStep } from 'src/subdomains/generic/kyc/entities/kyc-step.entity'; +import { KycStepName } from 'src/subdomains/generic/kyc/enums/kyc-step-name.enum'; +import { ReviewStatus } from 'src/subdomains/generic/kyc/enums/review-status.enum'; import { UserData } from '../user-data.entity'; +import { KycType, UserDataStatus } from '../user-data.enum'; import { UserDataRepository } from '../user-data.repository'; import { UserDataService } from '../user-data.service'; import { UserRepository } from '../../user/user.repository'; @@ -31,6 +35,11 @@ import { UserRepository } from '../../user/user.repository'; describe('UserDataService', () => { let service: UserDataService; let userDataRepo: jest.Mocked; + let userRepo: jest.Mocked; + let kycAdminService: jest.Mocked; + let transactionService: jest.Mocked; + let bankDataService: jest.Mocked; + let documentService: jest.Mocked; beforeEach(async () => { userDataRepo = createMock(); @@ -65,6 +74,11 @@ describe('UserDataService', () => { }).compile(); service = module.get(UserDataService); + userRepo = module.get(UserRepository); + kycAdminService = module.get(KycAdminService); + transactionService = module.get(TransactionService); + bankDataService = module.get(BankDataService); + documentService = module.get(KycDocumentService); }); describe('getUsersByMail', () => { @@ -105,6 +119,126 @@ describe('UserDataService', () => { }); }); + describe('mergeUserData kyc step renumbering', () => { + let stepId: number; + + const buildStep = (name: KycStepName, sequenceNumber: number, status = ReviewStatus.COMPLETED): KycStep => + Object.assign(new KycStep(), { id: ++stepId, name, type: null, status, sequenceNumber }); + + const buildAccount = (id: number, kycLevel: number): UserData => + Object.assign(new UserData(), { + id, + kycLevel, + kycType: KycType.DFX, + status: UserDataStatus.ACTIVE, + accountRelations: [], + relatedAccountRelations: [], + supportIssues: [], + }); + + const runMerge = async (masterSteps: KycStep[], slaveSteps: KycStep[]): Promise<[number, number][]> => { + const master = buildAccount(1000, 50); + const slave = buildAccount(2000, 20); + + userDataRepo.findOne.mockResolvedValueOnce(master).mockResolvedValueOnce(slave); + transactionService.getAllTransactionsForUserData.mockResolvedValue([]); + userRepo.find.mockResolvedValue([]); + bankDataService.getAllBankDatasForUser.mockResolvedValue([]); + kycAdminService.getKycSteps.mockResolvedValueOnce(masterSteps).mockResolvedValueOnce(slaveSteps); + documentService.copyFiles.mockResolvedValue(undefined); + jest.spyOn(service, 'updateVolumes').mockResolvedValue(undefined); + jest + .spyOn(service as unknown as { updateBankTxTime: () => Promise }, 'updateBankTxTime') + .mockResolvedValue(undefined); + + await service.mergeUserData(master.id, slave.id); + + // updateKycStepInternal receives KycStep.update()'s [id, partial]; extract [stepId, newSequenceNumber] + return kycAdminService.updateKycStepInternal.mock.calls.map((c) => { + const [id, update] = c[0] as unknown as [number, Partial]; + return [id, update.sequenceNumber]; + }); + }; + + beforeEach(() => { + stepId = 0; + }); + + // prod debris shape (userData 240169): repeated failed merges left same-name steps at 0, -100 … -400 + const debrisSlaveSteps = () => [ + buildStep(KycStepName.CONTACT_DATA, 0), + buildStep(KycStepName.CONTACT_DATA, -100), + buildStep(KycStepName.CONTACT_DATA, -200), + buildStep(KycStepName.CONTACT_DATA, -300), + buildStep(KycStepName.CONTACT_DATA, -400), + buildStep(KycStepName.PERSONAL_DATA, 0), + ]; + + it('assigns numbers strictly below the minimum of both sides', async () => { + const masterSteps = [buildStep(KycStepName.CONTACT_DATA, 0)]; + const slaveSteps = debrisSlaveSteps(); + + const assigned = await runMerge(masterSteps, slaveSteps); + + expect(assigned).toHaveLength(6); + for (const [, seq] of assigned) expect(seq).toBeLessThan(-400); + }); + + it('assigns pairwise-distinct numbers (no collision within the batch)', async () => { + const assigned = await runMerge([buildStep(KycStepName.CONTACT_DATA, 0)], debrisSlaveSteps()); + + const seqs = assigned.map(([, seq]) => seq); + expect(new Set(seqs).size).toBe(seqs.length); + }); + + it('preserves the relative order of same-name attempts (newest keeps the highest number)', async () => { + const slaveSteps = debrisSlaveSteps(); + // update() mutates the entities, so capture the old order before the merge runs + const idsByOldSeqDesc = slaveSteps + .filter((s) => s.name === KycStepName.CONTACT_DATA) + .sort((a, b) => b.sequenceNumber - a.sequenceNumber) + .map((s) => s.id); + + const assigned = new Map(await runMerge([buildStep(KycStepName.CONTACT_DATA, 0)], slaveSteps)); + + const newSeqs = idsByOldSeqDesc.map((id) => assigned.get(id)); + for (let i = 1; i < newSeqs.length; i++) expect(newSeqs[i - 1]).toBeGreaterThan(newSeqs[i]); + }); + + it('never lands on a pre-existing (name, sequenceNumber) tuple of either side — the prod collision', async () => { + const masterSteps = [buildStep(KycStepName.CONTACT_DATA, 0), buildStep(KycStepName.PERSONAL_DATA, -1)]; + const slaveSteps = debrisSlaveSteps(); + const preExisting = new Set([...masterSteps, ...slaveSteps].map((s) => `${s.name}|${s.sequenceNumber}`)); + + const assigned = new Map(await runMerge(masterSteps, slaveSteps)); + + for (const step of slaveSteps) { + expect(preExisting.has(`${step.name}|${assigned.get(step.id)}`)).toBe(false); + } + }); + + it('re-running a partially-applied merge assigns fresh lower numbers (no compounding, no collision)', async () => { + // first run seeded 100 below the -400 floor and assigned -500 … -505; simulate those writes committed + const committedSlaveSteps = [ + buildStep(KycStepName.CONTACT_DATA, -500), + buildStep(KycStepName.CONTACT_DATA, -502), + buildStep(KycStepName.CONTACT_DATA, -503), + buildStep(KycStepName.CONTACT_DATA, -504), + buildStep(KycStepName.CONTACT_DATA, -505), + buildStep(KycStepName.PERSONAL_DATA, -501), + ]; + const preExisting = new Set(committedSlaveSteps.map((s) => `${s.name}|${s.sequenceNumber}`)); + + const assigned = await runMerge([buildStep(KycStepName.CONTACT_DATA, 0)], committedSlaveSteps); + + for (const [id, seq] of assigned) { + expect(seq).toBeLessThan(-505); + expect(preExisting.has(`${committedSlaveSteps.find((s) => s.id === id).name}|${seq}`)).toBe(false); + } + expect(new Set(assigned.map(([, s]) => s)).size).toBe(assigned.length); + }); + }); + describe('assignNextKycFileId', () => { it('starts at 1 when no user has a kycFileId yet', async () => { const userData = Object.assign(new UserData(), { id: 1 }); diff --git a/src/subdomains/generic/user/models/user-data/user-data.service.ts b/src/subdomains/generic/user/models/user-data/user-data.service.ts index 0f0e5ecb15..49b09784fb 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.service.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.service.ts @@ -1285,10 +1285,16 @@ export class UserDataService { if (notifyUser && slave.mail && ![slave.mail, mail].includes(master.mail)) await this.userDataNotificationService.userDataChangedMailInfo(master, slave); - // Adapt slave kyc step sequenceNumber - const sequenceNumberOffset = master.kycSteps.length ? Util.minObjValue(master.kycSteps, 'sequenceNumber') - 100 : 0; + // Adapt slave kyc step sequenceNumber: absolute, strictly-decreasing numbers below BOTH sides' min, so a + // reassigned step can't collide on the userDataId+name+type+sequenceNumber unique index — neither against the + // master's rows (checked when userDataId flips on save) nor against the slave's own rows from earlier merges + // (checked at update time, before the flip) — and a re-run of a partially-applied merge can't compound. + const existingSteps = [...master.kycSteps, ...(slave.kycSteps ?? [])]; + // Seeded 100 below the floor: the gap marks each merge batch as such in the raw data. + let nextSequenceNumber = (existingSteps.length ? Util.minObjValue(existingSteps, 'sequenceNumber') : 0) - 100; const kycStepMerge = !!slave.kycSteps?.length; - for (const kycStep of slave.kycSteps) { + // Descending by old sequenceNumber, so the newest attempt keeps the highest new number (order-preserving). + for (const kycStep of [...slave.kycSteps].sort((a, b) => b.sequenceNumber - a.sequenceNumber)) { await this.kycAdminService.updateKycStepInternal( kycStep.update( [ @@ -1306,7 +1312,7 @@ export class UserDataService { : undefined, undefined, undefined, - kycStep.sequenceNumber + sequenceNumberOffset, + nextSequenceNumber--, ), ); } From 57e87121b861a3ed668cad91c6a32001a2c72076 Mon Sep 17 00:00:00 2001 From: Danswar <48102227+Danswar@users.noreply.github.com> Date: Fri, 3 Jul 2026 11:00:21 -0300 Subject: [PATCH 08/10] fix(exchange): skip Scrypt websocket setup when unconfigured (#4008) * fix(exchange): skip Scrypt websocket setup when unconfigured ScryptService's constructor eagerly opened the Scrypt websocket (subscriptions + fetchAll cache warm-up) regardless of whether Scrypt is configured. Where it is not (e.g. dev), the connection reconnect-loops on ETIMEDOUT (~14/h) and the warm-up fetches log errors. Gate the eager setup behind isConfigured and warn once (the constructor runs once per process). No change where Scrypt is configured. The balances path is already guarded upstream (#3870); the other consumers only touch Scrypt when it is explicitly targeted, which does not happen on an unconfigured env. * fix(exchange): guard Scrypt subscription readers against unconfigured state Make securities/balances explicitly optional (they are undefined when Scrypt isn't configured) and throw a clear error from the readers instead of a TypeError on undefined.values()/find(). --- .../exchange/services/scrypt.service.ts | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/integration/exchange/services/scrypt.service.ts b/src/integration/exchange/services/scrypt.service.ts index 3a712a3ca4..f8a96b5dda 100644 --- a/src/integration/exchange/services/scrypt.service.ts +++ b/src/integration/exchange/services/scrypt.service.ts @@ -33,9 +33,9 @@ export class ScryptService extends PricingProvider { private readonly logger = new DfxLogger(ScryptService); private readonly connection: ScryptWebSocketConnection; - // Subscriptions - private readonly securities: AsyncSubscription; - private readonly balances: AsyncSubscription>; + // Subscriptions (undefined when Scrypt is not configured, see constructor) + private readonly securities?: AsyncSubscription; + private readonly balances?: AsyncSubscription>; private readonly executionReports: Map = new Map(); private readonly balanceTransactions: Map = new Map(); @@ -52,6 +52,13 @@ export class ScryptService extends PricingProvider { const config = GetConfig().scrypt; this.connection = new ScryptWebSocketConnection(config.wsUrl, config.apiKey, config.apiSecret); + // Skip the eager websocket connect + cache warm-up where Scrypt is unconfigured + // (e.g. dev): the connection would otherwise reconnect-loop on ETIMEDOUT. + if (!this.isConfigured) { + this.logger.warn('Scrypt is not configured — skipping websocket subscriptions and cache warm-up'); + return; + } + // Securities subscription this.securities = new AsyncSubscription((cb) => { this.connection.subscribeToStream(ScryptMessageType.SECURITY, cb); @@ -105,7 +112,7 @@ export class ScryptService extends PricingProvider { // --- BALANCES --- // async getBalances(): Promise<{ total: Record; available: Record }> { - const balances = await this.balances; + const balances = await this.getBalancesSubscription(); const total: Record = {}; const available: Record = {}; @@ -122,7 +129,7 @@ export class ScryptService extends PricingProvider { } async getAvailableBalance(currency: string): Promise { - const balances = await this.balances; + const balances = await this.getBalancesSubscription(); const balance = balances.get(currency); if (!balance) return 0; @@ -131,6 +138,11 @@ export class ScryptService extends PricingProvider { return parseFloat(balance.AvailableAmount) || amount; } + private getBalancesSubscription(): AsyncSubscription> { + if (!this.balances) throw new Error(`${this.name} is not configured`); + return this.balances; + } + // --- WITHDRAWALS --- // async withdrawFunds( @@ -538,7 +550,7 @@ export class ScryptService extends PricingProvider { // --- MARKET DATA --- // async getTradePair(from: string, to: string): Promise<{ symbol: string; side: ScryptOrderSide }> { - const securities = await this.securities; + const securities = await this.getSecuritiesSubscription(); // Find matching pair: either from=base,to=quote (SELL base) or from=quote,to=base (BUY base) const security = securities.find( @@ -556,7 +568,7 @@ export class ScryptService extends PricingProvider { } private async getSecurity(symbol: string): Promise { - const securities = await this.securities; + const securities = await this.getSecuritiesSubscription(); const security = securities.find((s) => s.Symbol === symbol); if (!security) { @@ -566,6 +578,11 @@ export class ScryptService extends PricingProvider { return security; } + private getSecuritiesSubscription(): AsyncSubscription { + if (!this.securities) throw new Error(`${this.name} is not configured`); + return this.securities; + } + private async getOrderBookPrice(symbol: string, side: ScryptOrderSide): Promise { const orderBook = await this.fetchOrderBook(symbol); From e718056b1aac931d338ee3116f5c6def425cf5f2 Mon Sep 17 00:00:00 2001 From: Danswar <48102227+Danswar@users.noreply.github.com> Date: Fri, 3 Jul 2026 11:00:34 -0300 Subject: [PATCH 09/10] fix(realunit): redact data from the API trace middleware (#4007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(realunit): redact PII from the API trace middleware The RealUnit trace middleware (#3747) logged full request/response bodies and headers per realunit-app call, masking only credential fields, as a multi-line block whose JSON got split and mis-tagged as ERROR by the log pipeline. Keep the full trace (headers + request body + response body) but: - redact personal data — names, addresses, email, phone, IBAN etc. by key, and wallet addresses / emails / IPv4 by value (the client IP and cookies live in the headers); - emit it on a single INFO line (compact JSON) so the pipeline can't split it into fragments and mis-tag them. The dfx-api alloy stage pins detected_level from the NestJS LOG token, which wins over body keywords, so the trace stays INFO even when a response body carries an "error" field. Adds a spec asserting personal fields, the client IP, cookies and the wallet address are masked while the non-personal trace data is kept. * fix(realunit): address review — scope body capture, tighten redaction - Scope full headers/body/response capture to /v{n}/realunit/* paths; a realunit-app call to any other endpoint logs metadata-only, so generic KYC/ident DTOs (documentNumber/gender) are never traced through this RealUnit-scoped denylist. - Add bic / number / gender / document to the denylist. - Wallet value-mask matches exactly 40 hex via lookahead, so tx hashes and signatures stay intact instead of being partially masked. - Cap each serialized section and summarize Buffer bodies as , so an oversized line can't be re-split downstream. - Tests: res.send-only path, json->send chaining, non-realunit (no trace), header-only metadata-only, nested kycData recursion, oversized string, binary body. * fix(realunit): resolve CodeQL type-confusion alert + zip test coverage - format(): drop the typeof==='object' empty-object shortcut flagged by CodeQL js/type-confusion-through-parameter-tampering — req.body is attacker-controlled and could be an array, and the shortcut treated it as an object without an Array.isArray guard. redact() + JSON.stringify already handle every shape safely (redact guards arrays before the object branch), so an empty body now just serializes as {}. - spec: add a distinct nested addressPostalCode assertion. * fix(realunit): resolve remaining CodeQL type-confusion in redact() Move the Array.isArray check to the top of redact() so a tampered array parameter (a header / body value can be string|array) is resolved before any typeof / length / string comparison — clears the remaining js/type-confusion-through-parameter-tampering alerts. The key is passed through to array elements so values under a sensitive key stay masked. Adds a test for an array under a sensitive key. * fix(realunit): clear last CodeQL type-confusion in format() format() read value.length in its Buffer branch; CodeQL doesn't recognize Buffer.isBuffer() as a type guard (unlike Array.isArray), so it still treated the value as a possibly-array HTTP param. Drop the Buffer special-case from format() and let redact() (which excludes arrays first) summarize Buffers, so the raw value is no longer length/type-inspected in format(). * fix(realunit): harden trace middleware — bounded regex, compute budget, signature redaction --- src/main.ts | 8 +- .../__tests__/api-trace.middleware.spec.ts | 209 ++++++++++++++++++ .../__tests__/api-trace.redact-keys.spec.ts | 136 ++++++++++++ .../middlewares/api-trace.middleware.ts | 143 ++++++++---- 4 files changed, 456 insertions(+), 40 deletions(-) create mode 100644 src/shared/middlewares/__tests__/api-trace.middleware.spec.ts create mode 100644 src/shared/middlewares/__tests__/api-trace.redact-keys.spec.ts diff --git a/src/main.ts b/src/main.ts index cad4538a7e..48e445c257 100644 --- a/src/main.ts +++ b/src/main.ts @@ -11,7 +11,7 @@ import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger'; import { spawnSync } from 'child_process'; import { useContainer } from 'class-validator'; import cors from 'cors'; -import { json, raw, text } from 'express'; +import { json, raw, text, Request } from 'express'; import helmet from 'helmet'; import morgan from 'morgan'; import { join } from 'path'; @@ -19,7 +19,7 @@ import { getVerifiedIp } from './shared/utils/ip.util'; import { AppModule } from './app.module'; import { Config, Environment } from './config/config'; import { ApiExceptionFilter } from './shared/filters/exception.filter'; -import { apiTraceMiddleware } from './shared/middlewares/api-trace.middleware'; +import { apiTraceMiddleware, maskUrl } from './shared/middlewares/api-trace.middleware'; import { DfxLogger } from './shared/services/dfx-logger'; import { AccountChangedWebhookDto } from './subdomains/generic/user/services/webhook/dto/account-changed-webhook.dto'; import { @@ -51,6 +51,10 @@ async function bootstrap() { const app = await NestFactory.create(AppModule, { bodyParser: false }); + // morgan's default :url logs the raw query string and unmasked wallet + // addresses to the same stdout the log pipeline ships — mask it like the + // trace middleware does. + morgan.token('url', (req) => maskUrl((req as Request).originalUrl ?? req.url ?? '')); app.use(morgan('dev')); app.use(helmet()); app.use( diff --git a/src/shared/middlewares/__tests__/api-trace.middleware.spec.ts b/src/shared/middlewares/__tests__/api-trace.middleware.spec.ts new file mode 100644 index 0000000000..68f7106be5 --- /dev/null +++ b/src/shared/middlewares/__tests__/api-trace.middleware.spec.ts @@ -0,0 +1,209 @@ +import { apiTraceMiddleware } from 'src/shared/middlewares/api-trace.middleware'; +import { DfxLogger } from 'src/shared/services/dfx-logger'; + +type Emit = (res: any) => void; + +function runTrace(req: any, statusCode: number, emit?: Emit): { lines: string[]; nextCalled: boolean } { + const spy = jest.spyOn(DfxLogger.prototype, 'info').mockImplementation(() => undefined); + + let finish: () => void = () => undefined; + const res: any = { + statusCode, + // Model Express: res.json() delegates to res.send() (so the send override is exercised). + json(body: unknown) { + return this.send(body); + }, + send(body: unknown) { + return body; + }, + on(event: string, cb: () => void) { + if (event === 'finish') finish = cb; + }, + }; + + let nextCalled = false; + apiTraceMiddleware()(req, res, () => { + nextCalled = true; + }); + if (emit) emit(res); + finish(); + + const lines = spy.mock.calls.map((c) => c.join(' ')); + spy.mockRestore(); + return { lines, nextCalled }; +} + +const realunitReq = (body: unknown) => ({ + method: 'POST', + originalUrl: '/v1/realunit/buy/0x1234567890123456789012345678901234567890/confirm?ref=abc', + headers: { + 'x-client': 'realunit-app', + 'content-type': 'application/json', + 'x-forwarded-for': '192.0.2.1', + cookie: 'session=dummy-session-value', + authorization: 'Bearer dummy.jwt.value', + }, + body, +}); + +describe('apiTraceMiddleware', () => { + describe('realunit path — full redacted trace (via res.json → res.send)', () => { + let lines: string[]; + let line: string; + + beforeAll(() => { + const req = realunitReq({ + email: 'jane.doe@example.com', + name: 'Jane Doe', + phoneNumber: '+41790000000', + bic: 'TESTCHBEXXX', + kycData: { + firstName: 'Jane', + addressStreet: 'Teststrasse 1', + addressPostalCode: '8001ABC', + addressCity: 'Testtown', + documentNumber: 'X1234567', + }, + walletAddress: '0x1234567890123456789012345678901234567890', + txHash: '0xabcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789', + amount: 50, + }); + ({ lines } = runTrace(req, 201, (res) => res.json({ status: 'CONFIRMED', error: 'Not Found' }))); + line = lines.join('\n'); + }); + + it('logs exactly one line', () => { + expect(lines).toHaveLength(1); + expect(line).not.toContain('\n'); + }); + + it('keeps the non-personal data', () => { + expect(line).toContain('POST'); + expect(line).toContain('→ 201'); + expect(line).toContain('client=realunit-app'); + expect(line).toContain('"amount":50'); + expect(line).toContain('"status":"CONFIRMED"'); + }); + + it.each([ + ['email', 'jane.doe@example.com'], + ['name', 'Jane Doe'], + ['nested firstName', '"firstName":"Jane"'], + ['nested street', 'Teststrasse'], + ['nested postal code', '8001ABC'], + ['nested document number', 'X1234567'], + ['phone', '41790000000'], + ['bic', 'TESTCHBEXXX'], + ['client IP', '192.0.2.1'], + ['cookie value', 'dummy-session-value'], + ['auth token', 'dummy.jwt.value'], + ])('masks the %s', (_label, secret) => { + expect(line).not.toContain(secret); + }); + + it('masks the wallet address in path and body but keeps the tx hash intact', () => { + expect(line).toContain('/v1/realunit/buy/0x…/confirm'); + expect(line).toContain('"walletAddress":"***"'); + expect(line).toContain('0xabcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789'); + }); + + it('drops the query string', () => { + expect(line).not.toContain('ref=abc'); + }); + }); + + it('captures a response sent via res.send only', () => { + const { lines } = runTrace(realunitReq({ amount: 1 }), 200, (res) => + res.send({ secretMail: 'x@example.com', ok: true }), + ); + const line = lines.join('\n'); + expect(line).toContain('"ok":true'); + expect(line).not.toContain('x@example.com'); + }); + + it('masks arrays under a sensitive key (tampered array param)', () => { + const req = realunitReq({ mails: ['a@example.com', 'b@example.com'], tags: ['ok', 'fine'] }); + (req.headers as any)['x-forwarded-for'] = ['192.0.2.1', '192.0.2.2']; + const { lines } = runTrace(req, 200, (res) => res.json({})); + const line = lines.join('\n'); + expect(line).not.toContain('a@example.com'); + expect(line).not.toContain('192.0.2.2'); + expect(line).toContain('"tags":["ok","fine"]'); + }); + + it('masks sell-broadcast signature material (unsignedTx / r / s / v)', () => { + const { lines } = runTrace( + realunitReq({ + unsignedTx: '0x02f87261e08459682f008459682f0e82520894', + r: '0x1111111111111111111111111111111111111111111111111111111111111111', + s: '0x2222222222222222222222222222222222222222222222222222222222222222', + v: 27, + }), + 200, + (res) => res.json({}), + ); + const line = lines.join('\n'); + expect(line).not.toContain('0x02f872'); + expect(line).not.toContain('0x1111'); + expect(line).not.toContain('0x2222'); + expect(line).toContain('"v":"***"'); + }); + + it('masks an uppercase-hex wallet and an email in the path', () => { + const req = { + method: 'GET', + originalUrl: '/v1/realunit/account/0X12345678901234567890123456789012345678AB/jane@example.com', + headers: {}, + body: undefined, + }; + const { lines } = runTrace(req, 404, (res) => res.send('Not Found')); + const line = lines.join('\n'); + expect(line).toContain('/v1/realunit/account/0x…/***'); + expect(line).not.toContain('0X12345678'); + expect(line).not.toContain('jane@example.com'); + }); + + it('stops redaction at the compute budget instead of walking a huge body', () => { + const huge = { items: Array.from({ length: 50_000 }, (_, i) => `leaf-value-${i}-${'x'.repeat(400)}`) }; + const { lines } = runTrace(realunitReq(huge), 200, (res) => res.json({})); + const line = lines.join('\n'); + expect(lines).toHaveLength(1); + expect(line).not.toContain('leaf-value-49999'); + expect(line.length).toBeLessThan(3 * 4500); // each of the 3 sections stays within MAX_PART + // the reported serialized size proves the walk stopped at the budget + // instead of stringifying the whole ~20 MB body + const [, serializedSize] = line.match(/req\.body=.*…\((\d+) chars\)/) ?? []; + expect(Number(serializedSize)).toBeLessThan(10_000); + }); + + it('truncates oversized strings and summarizes binary bodies', () => { + const big = 'A'.repeat(600); + const { lines } = runTrace(realunitReq({ note: big, img: Buffer.from('PNGDATA') }), 200, (res) => res.json({})); + const line = lines.join('\n'); + expect(line).not.toContain(big); + expect(line).toContain('600 chars'); + expect(line).toContain(''); + }); + + it('logs metadata-only for a realunit-app call to a non-realunit path', () => { + const req = { + method: 'POST', + originalUrl: '/v1/kyc/data', + headers: { 'x-client': 'realunit-app' }, + body: { documentNumber: 'X1234567', gender: 'male' }, + }; + const { lines } = runTrace(req, 200, (res) => res.json({ documentNumber: 'X1234567' })); + const line = lines.join('\n'); + expect(lines).toHaveLength(1); + expect(line).toContain('POST /v1/kyc/data → 200'); + expect(line).not.toContain('req.body'); + expect(line).not.toContain('X1234567'); + }); + + it('does not trace a non-realunit request at all', () => { + const req = { method: 'GET', originalUrl: '/v1/transaction', headers: { 'x-client': 'dfx-app' }, body: {} }; + const { lines, nextCalled } = runTrace(req, 200, (res) => res.json({ ok: true })); + expect(lines).toHaveLength(0); + expect(nextCalled).toBe(true); + }); +}); diff --git a/src/shared/middlewares/__tests__/api-trace.redact-keys.spec.ts b/src/shared/middlewares/__tests__/api-trace.redact-keys.spec.ts new file mode 100644 index 0000000000..9ec3652c39 --- /dev/null +++ b/src/shared/middlewares/__tests__/api-trace.redact-keys.spec.ts @@ -0,0 +1,136 @@ +import 'reflect-metadata'; +import { KycAddress, KycPersonalData } from 'src/subdomains/generic/kyc/dto/input/kyc-data.dto'; +import * as realunitAdminDto from 'src/subdomains/supporting/realunit/dto/realunit-admin.dto'; +import * as realunitPdfDto from 'src/subdomains/supporting/realunit/dto/realunit-pdf.dto'; +import * as realunitRegistrationDto from 'src/subdomains/supporting/realunit/dto/realunit-registration.dto'; +import * as realunitSellDto from 'src/subdomains/supporting/realunit/dto/realunit-sell.dto'; +import * as realunitDto from 'src/subdomains/supporting/realunit/dto/realunit.dto'; +import { REDACT_KEY } from '../api-trace.middleware'; + +// Regression net for the REDACT_KEY denylist: every property of every DTO that +// can flow through the RealUnit trace must either match REDACT_KEY or be +// explicitly listed here as safe to log. A new personal field on a RealUnit DTO +// fails this spec instead of shipping to PRD logs unmasked. +const SAFE_KEYS = new Set([ + // ids / enums / status + 'id', + 'uid', + 'routeId', + 'chainId', + 'type', + 'status', + 'state', + 'eventType', + 'accountType', + 'isRegistered', + 'isValid', + 'error', + 'reference', + // amounts / rates / volumes + 'amount', + 'targetAmount', + 'estimatedAmount', + 'amountInChf', + 'amountWei', + 'minVolume', + 'maxVolume', + 'minVolumeTarget', + 'maxVolumeTarget', + 'exchangeRate', + 'rate', + 'fees', + 'balance', + 'ethBalance', + 'requiredGasEth', + 'value', + 'valueChf', + 'valueEur', + 'total', + 'totalCount', + 'totalShares', + 'totalSupply', + 'percentage', + 'chf', + 'eur', + 'usd', + // currencies / times / i18n + 'currency', + 'assets', + 'date', + 'created', + 'timestamp', + 'outputDate', + 'registrationDate', + 'lastUpdated', + 'timeFrame', + 'language', + 'lang', + // on-chain data (addresses in these values are masked by the WALLET_ADDRESS value regex) + 'txHash', + 'txHashes', + 'from', + 'to', + 'spender', + 'holders', + 'transfer', + 'approval', + 'tokensDeclaredInvalid', + 'userNonce', + 'domain', + 'types', + 'message', + 'priceSteps', + 'historicalBalances', + 'history', + // pagination + 'limit', + 'offset', + 'first', + 'before', + 'after', + 'pageInfo', + 'endCursor', + 'startCursor', + 'hasNextPage', + 'hasPreviousPage', + // nested containers (their own properties are checked individually) + 'kycData', + 'userData', + 'eip7702', + 'beneficiary', + 'addressTypeUpdate', + // DFX's own recipient payment data + 'paymentRequest', + 'remittanceInfo', +]); + +const DTO_CLASSES: Array object> = [ + ...[realunitDto, realunitSellDto, realunitRegistrationDto, realunitAdminDto, realunitPdfDto].flatMap((module) => + Object.values(module).filter( + (value): value is new () => object => typeof value === 'function' && !!value.prototype, + ), + ), + KycPersonalData, + KycAddress, +]; + +function propertiesOf(cls: new () => object): string[] { + const props: string[] = Reflect.getMetadata('swagger/apiModelPropertiesArray', cls.prototype) ?? []; + return props.map((p) => p.slice(1)); +} + +describe('api-trace REDACT_KEY coverage of RealUnit DTOs', () => { + it('reflects a meaningful number of DTO classes (guard against silent metadata changes)', () => { + const withProps = DTO_CLASSES.filter((cls) => propertiesOf(cls).length > 0); + expect(withProps.length).toBeGreaterThanOrEqual(25); + }); + + it('every DTO property is either redacted by key or explicitly known-safe', () => { + const unaccounted = DTO_CLASSES.flatMap((cls) => + propertiesOf(cls) + .filter((prop) => !REDACT_KEY.test(prop) && !SAFE_KEYS.has(prop)) + .map((prop) => `${cls.name}.${prop}`), + ); + expect(unaccounted).toEqual([]); + }); +}); diff --git a/src/shared/middlewares/api-trace.middleware.ts b/src/shared/middlewares/api-trace.middleware.ts index c007d8b6d9..f2165c40dc 100644 --- a/src/shared/middlewares/api-trace.middleware.ts +++ b/src/shared/middlewares/api-trace.middleware.ts @@ -7,77 +7,144 @@ const CLIENT_HEADER = 'x-client'; const REALUNIT_CLIENT = /realunit-app/i; const REALUNIT_PATH = /^\/v\d+\/realunit\//i; -// Keys whose values are masked: auth header, JWT/access tokens, signatures, credentials. -// Anchored so public fields like `tokenInfo` / `tokenAddress` are NOT redacted. -const SECRET_KEY = /(^authorization$|token$|signature$|password|secret|mnemonic|privatekey)/i; -const MAX_STRING = 512; +// Object keys whose value is fully replaced with `***`: credentials, personal +// data, and the client-IP / cookie headers. Body capture is scoped to the +// `/v{n}/realunit/*` paths (below), so this only has to cover the RealUnit DTOs +// — but kept deliberately broad: over-masking a harmless field is fine, leaking +// a personal one is not. +export const REDACT_KEY = + /(^authorization$|^cookie$|^set-cookie$|^forwarded$|^r$|^s$|^v$|^unsignedtx$|token$|signature$|password|secret|mnemonic|privatekey|name$|firstname|surname|mail|phone|street|address|city|zip|postalcode|housenumber|^number$|country|nationality|gender|document|birth|iban|bic|tin|tax|x-forwarded-for|x-real-ip|cf-connecting-ip|true-client-ip|x-client-ip)/i; + +// Value patterns masked wherever they appear, even under a key we didn't list. +// The wallet match is exactly 40 hex — the lookahead leaves longer hex runs (tx +// hashes) intact, since those are on-chain identifiers, not PII. The EMAIL +// quantifiers are bounded so an adversarial body can't trigger super-linear +// backtracking on the request path. +const WALLET_ADDRESS = /0x[0-9a-f]{40}(?![0-9a-f])/gi; +const EMAIL = /[^\s"@/]{1,64}@[^\s"@/]{1,255}\.[^\s"@/.]{1,24}/g; +const IPV4 = /\b\d{1,3}(?:\.\d{1,3}){3}\b/g; + +const MAX_STRING = 512; // per string leaf +const MAX_PART = 4000; // per serialized section (headers / req body / res body) +const REDACT_BUDGET = 2 * MAX_PART; // per section: bounds the compute, not just the output const REDACTED = '***'; +const TRUNCATED = '<…truncated…>'; + +function maskValue(s: string): string { + return s.replace(WALLET_ADDRESS, '0x…').replace(EMAIL, REDACTED).replace(IPV4, REDACTED); +} -function redact(value: unknown, key?: string): unknown { - if (key && SECRET_KEY.test(key) && value != null && value !== '') return REDACTED; +export function maskUrl(url: string): string { + return maskValue(url.split('?')[0]); +} + +// `budget` bounds the total work per section: each processed node deducts from +// it and the walk stops once it is spent, so a 20 MB body can't burn seconds of +// synchronous CPU (regexes + stringify) just to emit a 4000-char log line. +function redact(value: unknown, key: string | undefined, budget: { left: number }): unknown { + if (budget.left <= 0) return TRUNCATED; + // Resolve the array case first: a tampered HTTP parameter (header / body) can + // be an array, so this runs before any typeof / length / string comparison + // (CodeQL js/type-confusion-through-parameter-tampering). The key is passed + // through so array elements under a sensitive key are still masked. + if (Array.isArray(value)) { + const out: unknown[] = []; + for (const entry of value) { + if (budget.left <= 0) { + out.push(TRUNCATED); + break; + } + out.push(redact(entry, key, budget)); + } + return out; + } + budget.left -= (key?.length ?? 0) + 8; + if (key && REDACT_KEY.test(key) && value != null && value !== '') return REDACTED; + if (Buffer.isBuffer(value)) return ``; if (typeof value === 'string') { - return value.length > MAX_STRING ? `<… ${value.length} chars …>` : value; + budget.left -= Math.min(value.length, MAX_STRING); + return value.length > MAX_STRING ? `<… ${value.length} chars …>` : maskValue(value); } - if (Array.isArray(value)) return value.map((entry) => redact(entry)); if (value && typeof value === 'object') { - return Object.fromEntries(Object.entries(value as Record).map(([k, v]) => [k, redact(v, k)])); + const out: Record = {}; + for (const [k, v] of Object.entries(value as Record)) { + if (budget.left <= 0) { + out['…'] = TRUNCATED; + break; + } + out[k] = redact(v, k, budget); + } + return out; } return value; } function format(value: unknown): string { if (value === undefined || value === null) return '(empty)'; - if (typeof value === 'object' && Object.keys(value as object).length === 0) return '(empty)'; + let s: string; try { - return JSON.stringify(redact(value), null, 2); + // redact() handles Buffer + the array case (Array.isArray first), so the + // raw value is never length/type-inspected here. + s = JSON.stringify(redact(value, undefined, { left: REDACT_BUDGET })); } catch { return '(unserializable)'; } + return s.length > MAX_PART ? `${s.slice(0, MAX_PART)}…(${s.length} chars)` : s; } /** - * Full request/response tracer for the RealUnit internal test phase. - * Enabled on DEV and PRD — see the environment gate in `main.ts`. + * Request/response tracer for the RealUnit internal test phase (DEV + PRD — see + * the environment gate in `main.ts`). Emits one INFO line per call from the + * realunit-app (`X-Client: realunit-app`) or a `/v{n}/realunit/*` path. + * + * Headers + request body + response body are captured only for `/v{n}/realunit/*` + * paths — the DTOs the redaction is tuned for. A realunit-app call to any other + * endpoint is logged metadata-only (method/path/status/duration/client), so + * generic KYC/ident bodies are never traced through this RealUnit-scoped denylist. * - * Emits one log block per call originating from the realunit-app — detected - * either via the `X-Client: realunit-app` header or a `/v{n}/realunit/*` path - * (the latter also covers app builds shipped before the header existed). - * Secrets are masked; KYC/PII bodies are kept intact by design — on PRD this - * means real customer data is written to the container logs. + * Personal data is redacted — credentials, names, addresses, email, phone, + * document/IBAN/BIC etc. by key, plus wallet addresses / emails / IPv4 by value. + * Each section is size-capped and binary bodies are summarized, and the whole + * trace is a single INFO line, so the pipeline can't split it into fragments and + * mis-tag them as ERROR. */ export function apiTraceMiddleware(): RequestHandler { return (req: Request, res: Response, next: NextFunction) => { const client = req.headers[CLIENT_HEADER]; const clientStr = Array.isArray(client) ? client[0] : (client ?? ''); - const isRealUnit = REALUNIT_CLIENT.test(clientStr) || REALUNIT_PATH.test(req.originalUrl); - if (!isRealUnit) return next(); + const isRealUnitPath = REALUNIT_PATH.test(req.originalUrl); + if (!REALUNIT_CLIENT.test(clientStr) && !isRealUnitPath) return next(); const start = Date.now(); let responseBody: unknown; - const originalJson = res.json.bind(res); - res.json = (body: any) => { - responseBody = body; - return originalJson(body); - }; + // Only capture bodies for the RealUnit paths the redaction is built for. + if (isRealUnitPath) { + const originalJson = res.json.bind(res); + res.json = (body: any) => { + responseBody = body; + return originalJson(body); + }; - const originalSend = res.send.bind(res); - res.send = (body: any) => { - if (responseBody === undefined) responseBody = body; - return originalSend(body); - }; + const originalSend = res.send.bind(res); + res.send = (body: any) => { + if (responseBody === undefined) responseBody = body; + return originalSend(body); + }; + } res.on('finish', () => { const durationMs = Date.now() - start; - const block = [ - `${req.method} ${req.originalUrl} → ${res.statusCode} (${durationMs}ms) ` + - `client=${clientStr || '(none)'} ip=${req.realIp}`, - ` req.headers: ${format(req.headers)}`, - ` req.body: ${format(req.body)}`, - ` res.body: ${format(responseBody)}`, - ].join('\n'); - logger.info(block); + const path = maskUrl(req.originalUrl); + const meta = `${req.method} ${path} → ${res.statusCode} (${durationMs}ms) client=${clientStr || '(none)'}`; + if (isRealUnitPath) { + logger.info( + `${meta} req.headers=${format(req.headers)} req.body=${format(req.body)} res.body=${format(responseBody)}`, + ); + } else { + logger.info(meta); + } }); next(); From b4cf5ea8bd337756b6a924650e97a52d379dcb2e Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 3 Jul 2026 16:44:40 +0200 Subject: [PATCH 10/10] feat(support): brand support mail by the app the ticket was opened from (X-Client) (#3937) * fix(support): brand support-message mail by user source instead of account history Support replies (newSupportMessage) sent the mail without an explicit input.wallet, so resolveMailWallet's account-history override forced RealUnit branding onto any account ever linked to a preferred (RealUnit) wallet - including DFX support answers, which arrived RealUnit-branded. Resolve the user's origin wallet explicitly and pass it, so branding follows the user source (DFX -> DFX, RealUnit -> RealUnit) and never falls back to the account-history override. The wallet is resolved via the user (cached) rather than the entity relation, because not every caller eager-loads it (e.g. the hourly auto-responder job) - reading the unloaded relation would otherwise mis-brand RealUnit users as DFX. Falls back to the default (DFX) wallet when no origin wallet is recorded. Adds unit tests covering origin branding, the default fallback and the no-mail case. Mirrors the login-mail fix #3846 for support-message mails. * feat(support): brand support mail by the wallet the ticket was opened from Builds on the previous commit (brand by account origin) to follow the actual ticket source: a user can have both a DFX and a RealUnit identity on one account, so account origin alone could not tell which app a ticket was opened from. - SupportIssue gains a nullable 'wallet' relation (the wallet/app the issue was opened from) + migration (column, index, FK). - Capture the source on creation: customer issues from the originating login user (jwt.user) -> its wallet; transaction-request issues from the request's user wallet; support-created issues have no source. - newSupportMessage brands by issue.wallet first, then the account origin wallet (legacy/support-created issues), then the default (DFX) wallet. Extends the unit tests: ticket source wins over account origin, origin fallback, default fallback. * fix(support): correct migration index/FK names; lock in resolveMailWallet safeguard Review fixes (PR #3937): - Migration used hand-written index/FK names that did not match TypeORM's DefaultNamingStrategy, which would make the next schema comparison emit a spurious drop+recreate. Use the generated names (verified: sha1 of 'support_issue_walletId' -> IDX_f5224979beab23e21df3066a60 / FK_...a60a, and the same method reproduces the existing user_data.walletId names exactly). - Add a NotificationService spec exercising the real resolveMailWallet: an explicitly set input.wallet skips the account-history override (the safeguard our support-mail fix relies on), and absent it falls back to the account wallet. Previously every test mocked sendMail, so that line was never covered. * fix(support): brand support mail strictly by issue source, fail closed on unknown Per review (PR #3937): the priority chain (issue.wallet ?? account-origin ?? DFX-default) was a heuristic that silently mis-brands when the source is unknown - violating the hard requirement to attribute every ticket to its exact originating app or not brand at all. newSupportMessage now brands only from issue.wallet and fails closed (skips the mail, logs a warning) when it is null, instead of guessing. Drops the origin/default fallback and the UserDataService/WalletService deps. Customer and transaction-request tickets already carry the exact source (jwt.user / request user wallet). Support-created tickets and legacy rows have no exact source yet and therefore fail closed - exact attribution there (agent-specified product, NOT NULL after backfill) is a follow-up needing an FE/API contract and a backfill decision. Tests updated: exact branding by issue source (RealUnit/DFX), fail-closed when absent, no-mail case. * feat(support): attribute ticket source via trusted X-Client header, DFX default Replaces the wallet-derived source (forbidden per review: user.wallet / userData.wallet are set once at signup and don't identify the calling app) with the per-request, server-read X-Client signal. - Centralize client detection in shared/utils/request-client (single source for the api-trace middleware and the support flow). - SupportIssue.wallet is set from the X-Client header at creation: RealUnit app -> RealUnit wallet; every other client -> null (= DFX default). Product decision: DFX is the default brand, only RealUnit-app tickets are RealUnit. - newSupportMessage brands by issue.wallet, falling back to the default (DFX) wallet; passing it explicitly still bypasses the account-history override. - Migrate the sibling limit-request mail to the same issue source instead of userData.wallet origin. - Fix createCustomWallet mock to carry name, so branding assertions are real. - Drop the jwt.user/UserService path and the origin/fail-closed heuristics. Tests: request-client detection (6), branding by issue source vs DFX default, no-mail; full suite 1068 passed. Prereqs (flagged on PR): RealUnit app must send 'X-Client: realunit-app' on support requests; REALUNIT_MAIL_USER must be set for the RealUnit template. * fix(support): make DFX-default branding observable and dedup-safe Hardens the X-Client source attribution (no behavioural reversal of the trusted-signal model, no persisted-wallet heuristics): - newSupportMessage logs when an issue has no attributed source and falls back to the DFX house brand, so the default path is observable instead of silent; and warns when a ticket is RealUnit-attributed but REALUNIT_MAIL_USER is unset (factory would silently render DFX otherwise). - Dedup now upgrades a not-yet-attributed (legacy/DFX-defaulted) issue to a positively resolved source on a follow-up message, but never clobbers an existing source (no RealUnit -> DFX downgrade). - Document the deliberate decisions: support-created tickets are DFX, and walletId is nullable by design (null = DFX; X-Client is RealUnit-only today, so there is no positive DFX signal to backfill against). - Tests assert the observability log and the REALUNIT_MAIL_USER guard. Note: X-Client propagation verified end-to-end - the RealUnit app sends X-Client: realunit-app on /v1/support/issue (api_client.dart, test-contract); the DFX ecosystem sends no header and correctly falls to the DFX default. * fix(support): harden X-Client attribution per Big Brother review - request-client: anchor the RealUnit client regex (^realunit-app$) so substrings like 'realunit-app-proxy' no longer match; trim the header value. Clarify in the comment that X-Client is client-supplied and NOT authenticated - it only selects mail branding, never authorization. - support-issue.service: split the support-tool path into a dedicated createIssueBySupport (no client param) so the 'support-created = DFX' invariant cannot be broken by a future caller forwarding a client header. - support-issue.service: warn when a RealUnit ticket is requested but the RealUnit wallet row is missing (was a silent DFX fallback). - Tests: anchored-regex cases for request-client. * style(support): prettier-format support-message branding block * fix(support): enforce exact fail-closed ticket source attribution - resolveSourceWallet: unknown/missing X-Client is rejected (400), never guessed into a brand; dfx-services is now a positively attributed DFX source; missing RealUnit wallet fails closed (503) instead of mis-branding - support-staff tickets are exactly DFX-attributed (support tool is part of the DFX services app) - SupportIssue.wallet NOT NULL; migration backfills legacy rows to the DFX default wallet as an explicit one-time decision - mail time: no fallback chain - unattributed or unconfigured-RealUnit issues suppress the mail with an error log instead of guessing - Config.mail.wallet gets an explicit DFX entry (rendering-identical), so DFX is a first-class brand mapping, not the absence of RealUnit - specs updated: fail-closed paths asserted, resolveClientSource covered Deploy order: DFXswiss/services must send 'X-Client: dfx-services' on support endpoints before this ships, otherwise web-app ticket creation fails closed by design. --- .../1781862303000-AddWalletToSupportIssue.js | 37 ++++++ src/config/config.ts | 6 + .../middlewares/api-trace.middleware.ts | 9 +- .../utils/__tests__/request-client.spec.ts | 66 +++++++++++ src/shared/utils/request-client.ts | 42 +++++++ .../wallet/__mocks__/wallet.entity.mock.ts | 3 +- .../__tests__/notification.service.spec.ts | 79 +++++++++++++ .../entities/support-issue.entity.ts | 8 ++ ...support-issue-notification.service.spec.ts | 108 ++++++++++++++++++ .../__tests__/support-issue.service.spec.ts | 5 + .../limit-request-notification.service.ts | 5 +- .../support-issue-notification.service.ts | 56 ++++++--- .../services/support-issue.service.ts | 76 +++++++++++- .../support-issue/support-issue.controller.ts | 13 ++- 14 files changed, 479 insertions(+), 34 deletions(-) create mode 100644 migration/1781862303000-AddWalletToSupportIssue.js create mode 100644 src/shared/utils/__tests__/request-client.spec.ts create mode 100644 src/shared/utils/request-client.ts create mode 100644 src/subdomains/supporting/notification/services/__tests__/notification.service.spec.ts create mode 100644 src/subdomains/supporting/support-issue/services/__tests__/support-issue-notification.service.spec.ts diff --git a/migration/1781862303000-AddWalletToSupportIssue.js b/migration/1781862303000-AddWalletToSupportIssue.js new file mode 100644 index 0000000000..72b9f374ee --- /dev/null +++ b/migration/1781862303000-AddWalletToSupportIssue.js @@ -0,0 +1,37 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * @class + * @implements {MigrationInterface} + */ +module.exports = class AddWalletToSupportIssue1781862303000 { + name = 'AddWalletToSupportIssue1781862303000' + + /** + * Adds the source wallet (app the ticket was opened from) to support_issue. + * The column is NOT NULL: every creation path resolves an exact source (X-Client) or fails, so an + * unattributed ticket cannot exist. Legacy rows predate source attribution and carry no exact signal; + * they are backfilled to the DFX default wallet (Config.defaultWalletId = 1) as an explicit one-time + * legacy decision - NOT as a runtime fallback. + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + await queryRunner.query(`ALTER TABLE "support_issue" ADD "walletId" integer`); + await queryRunner.query(`UPDATE "support_issue" SET "walletId" = 1 WHERE "walletId" IS NULL`); + await queryRunner.query(`ALTER TABLE "support_issue" ALTER COLUMN "walletId" SET NOT NULL`); + await queryRunner.query(`CREATE INDEX "IDX_f5224979beab23e21df3066a60" ON "support_issue" ("walletId")`); + await queryRunner.query(`ALTER TABLE "support_issue" ADD CONSTRAINT "FK_f5224979beab23e21df3066a60a" FOREIGN KEY ("walletId") REFERENCES "wallet"("id") ON DELETE NO ACTION ON UPDATE NO ACTION`); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + await queryRunner.query(`ALTER TABLE "support_issue" DROP CONSTRAINT "FK_f5224979beab23e21df3066a60a"`); + await queryRunner.query(`DROP INDEX "IDX_f5224979beab23e21df3066a60"`); + await queryRunner.query(`ALTER TABLE "support_issue" DROP COLUMN "walletId"`); + } +} diff --git a/src/config/config.ts b/src/config/config.ts index 6d6d6dcfc2..f54c547122 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -635,6 +635,12 @@ export class Configuration { noReplyMail: process.env.NOREPLY_MAIL || 'noreply@dfx.swiss', }, wallet: { + // Explicit entry for the DFX house brand (name of the default wallet, Config.defaultWalletId), so a + // positively DFX-attributed mail is a first-class mapping and not the absence of every other brand. + // Values mirror the previous implicit defaults exactly (default transport, user-v2 template). + DFX: { + template: 'user-v2', + }, onchainlabs: { template: 'onChainLabs', }, diff --git a/src/shared/middlewares/api-trace.middleware.ts b/src/shared/middlewares/api-trace.middleware.ts index f2165c40dc..edf08ae5bf 100644 --- a/src/shared/middlewares/api-trace.middleware.ts +++ b/src/shared/middlewares/api-trace.middleware.ts @@ -1,10 +1,9 @@ import { NextFunction, Request, RequestHandler, Response } from 'express'; import { DfxLogger } from '../services/dfx-logger'; +import { getClient, isRealUnitRequest } from '../utils/request-client'; const logger = new DfxLogger('RealUnitTrace'); -const CLIENT_HEADER = 'x-client'; -const REALUNIT_CLIENT = /realunit-app/i; const REALUNIT_PATH = /^\/v\d+\/realunit\//i; // Object keys whose value is fully replaced with `***`: credentials, personal @@ -110,11 +109,11 @@ function format(value: unknown): string { */ export function apiTraceMiddleware(): RequestHandler { return (req: Request, res: Response, next: NextFunction) => { - const client = req.headers[CLIENT_HEADER]; - const clientStr = Array.isArray(client) ? client[0] : (client ?? ''); + const clientStr = getClient(req); + // Same gate as develop's inline client/path test, via the shared (anchored) helpers. const isRealUnitPath = REALUNIT_PATH.test(req.originalUrl); - if (!REALUNIT_CLIENT.test(clientStr) && !isRealUnitPath) return next(); + if (!isRealUnitRequest(req)) return next(); const start = Date.now(); let responseBody: unknown; diff --git a/src/shared/utils/__tests__/request-client.spec.ts b/src/shared/utils/__tests__/request-client.spec.ts new file mode 100644 index 0000000000..1e637effa9 --- /dev/null +++ b/src/shared/utils/__tests__/request-client.spec.ts @@ -0,0 +1,66 @@ +import { Request } from 'express'; +import { getClient, isRealUnitClient, isRealUnitRequest, resolveClientSource } from '../request-client'; + +function req(headers: Record, originalUrl = '/v1/support'): Request { + return { headers, originalUrl } as unknown as Request; +} + +describe('request-client', () => { + describe('isRealUnitClient', () => { + it('matches the realunit-app client (case-insensitive)', () => { + expect(isRealUnitClient('realunit-app')).toBe(true); + expect(isRealUnitClient('RealUnit-App')).toBe(true); + }); + + it('does not match other or missing clients', () => { + expect(isRealUnitClient('dfx-app')).toBe(false); + expect(isRealUnitClient('')).toBe(false); + expect(isRealUnitClient(undefined)).toBe(false); + }); + + it('is anchored - substrings of the client id do not match', () => { + expect(isRealUnitClient('realunit-app-proxy')).toBe(false); + expect(isRealUnitClient('x-realunit-app')).toBe(false); + expect(isRealUnitClient(' realunit-app ')).toBe(true); // trimmed + }); + }); + + describe('getClient', () => { + it('reads the x-client header, taking the first value of an array', () => { + expect(getClient(req({ 'x-client': 'realunit-app' }))).toBe('realunit-app'); + expect(getClient(req({ 'x-client': ['realunit-app', 'x'] }))).toBe('realunit-app'); + expect(getClient(req({}))).toBe(''); + }); + }); + + describe('resolveClientSource', () => { + it('resolves the exact known clients (case-insensitive, trimmed)', () => { + expect(resolveClientSource('realunit-app')).toBe('RealUnit'); + expect(resolveClientSource('RealUnit-App')).toBe('RealUnit'); + expect(resolveClientSource('dfx-services')).toBe('DFX'); + expect(resolveClientSource(' DFX-Services ')).toBe('DFX'); + }); + + it('returns undefined for unknown, empty or missing clients - never a brand', () => { + expect(resolveClientSource('dfx-services-proxy')).toBeUndefined(); + expect(resolveClientSource('realunit-app-proxy')).toBeUndefined(); + expect(resolveClientSource('dfx')).toBeUndefined(); + expect(resolveClientSource('')).toBeUndefined(); + expect(resolveClientSource(undefined)).toBeUndefined(); + }); + }); + + describe('isRealUnitRequest', () => { + it('detects RealUnit via the x-client header', () => { + expect(isRealUnitRequest(req({ 'x-client': 'realunit-app' }))).toBe(true); + }); + + it('detects RealUnit via the /v{n}/realunit/ path (legacy app builds)', () => { + expect(isRealUnitRequest(req({}, '/v1/realunit/register'))).toBe(true); + }); + + it('is false for a plain DFX request (no header, non-realunit path)', () => { + expect(isRealUnitRequest(req({}, '/v1/support'))).toBe(false); + }); + }); +}); diff --git a/src/shared/utils/request-client.ts b/src/shared/utils/request-client.ts new file mode 100644 index 0000000000..fed5f9f790 --- /dev/null +++ b/src/shared/utils/request-client.ts @@ -0,0 +1,42 @@ +import { Request } from 'express'; + +// Identifies the client application a request originates from, via the `X-Client` header (and, for +// legacy app builds, the `/v{n}/realunit/*` path). This is a per-request, server-read signal of the +// calling app - distinct from the user's persisted wallet, which is set once at signup and is not a +// reliable indicator of which app a given request came from. +// +// NOTE: X-Client is client-supplied and NOT cryptographically authenticated. It is only used to pick +// mail branding (DFX vs. RealUnit visuals); it grants no access and carries no privilege, so a spoofed +// value at worst mislabels a support mail. Never gate authorization or data access on it. + +export const CLIENT_HEADER = 'x-client'; + +// Anchored: only the exact `realunit-app` client matches (not substrings like `realunit-app-proxy`). +const REALUNIT_CLIENT = /^realunit-app$/i; +const REALUNIT_PATH = /^\/v\d+\/realunit\//i; +const DFX_CLIENT = /^dfx-services$/i; + +// The application a request exactly identifies itself as. `undefined` means the request carried no (or +// an unrecognized) X-Client value - callers that need exact attribution must treat that as unresolvable +// and fail closed, never map it to a brand. +export type ClientSource = 'DFX' | 'RealUnit'; + +export function getClient(req: Request): string { + const client = req.headers[CLIENT_HEADER]; + return ((Array.isArray(client) ? client[0] : client) ?? '').trim(); +} + +export function isRealUnitClient(client: string | undefined): boolean { + return REALUNIT_CLIENT.test(client?.trim() ?? ''); +} + +export function resolveClientSource(client: string | undefined): ClientSource | undefined { + const value = client?.trim() ?? ''; + if (REALUNIT_CLIENT.test(value)) return 'RealUnit'; + if (DFX_CLIENT.test(value)) return 'DFX'; + return undefined; +} + +export function isRealUnitRequest(req: Request): boolean { + return isRealUnitClient(getClient(req)) || REALUNIT_PATH.test(req.originalUrl); +} diff --git a/src/subdomains/generic/user/models/wallet/__mocks__/wallet.entity.mock.ts b/src/subdomains/generic/user/models/wallet/__mocks__/wallet.entity.mock.ts index 9264b08fa3..784fd58247 100644 --- a/src/subdomains/generic/user/models/wallet/__mocks__/wallet.entity.mock.ts +++ b/src/subdomains/generic/user/models/wallet/__mocks__/wallet.entity.mock.ts @@ -5,12 +5,13 @@ export function createDefaultWallet(): Wallet { } export function createCustomWallet(customValues: Partial): Wallet { - const { address } = customValues; + const { address, name } = customValues; const keys = Object.keys(customValues); const entity = new Wallet(); entity.address = keys.includes('address') ? address : 'x0ZZZYYY'; + if (keys.includes('name')) entity.name = name; return entity; } diff --git a/src/subdomains/supporting/notification/services/__tests__/notification.service.spec.ts b/src/subdomains/supporting/notification/services/__tests__/notification.service.spec.ts new file mode 100644 index 0000000000..c043912523 --- /dev/null +++ b/src/subdomains/supporting/notification/services/__tests__/notification.service.spec.ts @@ -0,0 +1,79 @@ +import { createMock } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { TestSharedModule } from 'src/shared/utils/test.shared.module'; +import { TestUtil } from 'src/shared/utils/test.util'; +import { createCustomUserData } from 'src/subdomains/generic/user/models/user-data/__mocks__/user-data.entity.mock'; +import { createCustomWallet } from 'src/subdomains/generic/user/models/wallet/__mocks__/wallet.entity.mock'; +import { DataSource } from 'typeorm'; +import { MailContext, MailType } from '../../enums'; +import { MailFactory } from '../../factories/mail.factory'; +import { MailRequest } from '../../interfaces'; +import { NotificationRepository } from '../../repositories/notification.repository'; +import { MailService } from '../mail.service'; +import { NotificationService } from '../notification.service'; + +describe('NotificationService', () => { + let service: NotificationService; + + let mailFactory: MailFactory; + let mailService: MailService; + let notificationRepo: NotificationRepository; + let dataSource: DataSource; + let userRepo: { findOne: jest.Mock }; + + beforeEach(async () => { + mailFactory = createMock(); + mailService = createMock(); + notificationRepo = createMock(); + dataSource = createMock(); + userRepo = { findOne: jest.fn() }; + + jest.spyOn(dataSource, 'getRepository').mockReturnValue(userRepo as any); + // short-circuit sendMail right after resolveMailWallet so only the wallet resolution runs + jest.spyOn(mailFactory, 'createMail').mockReturnValue(undefined); + + const module: TestingModule = await Test.createTestingModule({ + imports: [TestSharedModule], + providers: [ + NotificationService, + { provide: MailFactory, useValue: mailFactory }, + { provide: MailService, useValue: mailService }, + { provide: NotificationRepository, useValue: notificationRepo }, + { provide: DataSource, useValue: dataSource }, + TestUtil.provideConfig(), + ], + }).compile(); + + service = module.get(NotificationService); + }); + + function userMailRequest(input: Record): MailRequest { + return { type: MailType.USER_V2, context: MailContext.SUPPORT_MESSAGE, input } as unknown as MailRequest; + } + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + // locks in the safeguard our support-mail fix relies on (resolveMailWallet: `if (input.wallet) return`) + it('keeps an explicitly set wallet and skips the account-history override', async () => { + const realUnitWallet = createCustomWallet({ name: 'RealUnit' }); + const request = userMailRequest({ userData: createCustomUserData({ id: 7 }), wallet: realUnitWallet }); + + await service.sendMail(request); + + expect((request.input as any).wallet).toBe(realUnitWallet); + expect(dataSource.getRepository).not.toHaveBeenCalled(); + expect(userRepo.findOne).not.toHaveBeenCalled(); + }); + + it('falls back to the account wallet when none is set and no preferred wallet is configured', async () => { + const accountWallet = createCustomWallet({ name: 'DFX' }); + const request = userMailRequest({ userData: createCustomUserData({ id: 7, wallet: accountWallet }) }); + + await service.sendMail(request); + + expect((request.input as any).wallet).toBe(accountWallet); + expect(userRepo.findOne).not.toHaveBeenCalled(); + }); +}); diff --git a/src/subdomains/supporting/support-issue/entities/support-issue.entity.ts b/src/subdomains/supporting/support-issue/entities/support-issue.entity.ts index 43d516dade..42826f0c1b 100644 --- a/src/subdomains/supporting/support-issue/entities/support-issue.entity.ts +++ b/src/subdomains/supporting/support-issue/entities/support-issue.entity.ts @@ -1,6 +1,7 @@ import { Config } from 'src/config/config'; import { IEntity, UpdateResult } from 'src/shared/models/entity'; import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; +import { Wallet } from 'src/subdomains/generic/user/models/wallet/wallet.entity'; import { LimitRequest } from 'src/subdomains/supporting/support-issue/entities/limit-request.entity'; import { Column, Entity, Index, JoinColumn, ManyToOne, OneToMany, OneToOne } from 'typeorm'; import { TransactionRequest } from '../../payment/entities/transaction-request.entity'; @@ -51,6 +52,13 @@ export class SupportIssue extends IEntity { @ManyToOne(() => UserData, { nullable: false, eager: true }) userData: UserData; + // Wallet/app the issue was opened from, resolved exactly from the inbound X-Client header at creation + // (NOT the user's persisted wallet) - drives mail branding. NOT NULL: every creation path must attribute + // an exact source or fail; legacy rows were backfilled to the DFX default wallet by the migration. + @Index() + @ManyToOne(() => Wallet, { nullable: false, eager: true }) + wallet: Wallet; + @OneToOne(() => LimitRequest, { nullable: true }) @JoinColumn() limitRequest?: LimitRequest; diff --git a/src/subdomains/supporting/support-issue/services/__tests__/support-issue-notification.service.spec.ts b/src/subdomains/supporting/support-issue/services/__tests__/support-issue-notification.service.spec.ts new file mode 100644 index 0000000000..8d881d5732 --- /dev/null +++ b/src/subdomains/supporting/support-issue/services/__tests__/support-issue-notification.service.spec.ts @@ -0,0 +1,108 @@ +import { createMock } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { Config } from 'src/config/config'; +import * as processServiceModule from 'src/shared/services/process.service'; +import { TestSharedModule } from 'src/shared/utils/test.shared.module'; +import { TestUtil } from 'src/shared/utils/test.util'; +import { createCustomUserData } from 'src/subdomains/generic/user/models/user-data/__mocks__/user-data.entity.mock'; +import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; +import { createCustomWallet } from 'src/subdomains/generic/user/models/wallet/__mocks__/wallet.entity.mock'; +import { Wallet } from 'src/subdomains/generic/user/models/wallet/wallet.entity'; +import { MailRequest } from '../../../notification/interfaces'; +import { NotificationService } from '../../../notification/services/notification.service'; +import { SupportIssue } from '../../entities/support-issue.entity'; +import { SupportMessage } from '../../entities/support-message.entity'; +import { SupportIssueNotificationService } from '../support-issue-notification.service'; + +describe('SupportIssueNotificationService', () => { + let service: SupportIssueNotificationService; + + let notificationService: NotificationService; + + const dfxWallet = createCustomWallet({ name: 'DFX' }); + const realUnitWallet = createCustomWallet({ name: 'RealUnit' }); + + beforeEach(async () => { + notificationService = createMock(); + + jest.spyOn(processServiceModule, 'DisabledProcess').mockReturnValue(false); + + const module: TestingModule = await Test.createTestingModule({ + imports: [TestSharedModule], + providers: [ + SupportIssueNotificationService, + { provide: NotificationService, useValue: notificationService }, + TestUtil.provideConfig(), + ], + }).compile(); + + service = module.get(SupportIssueNotificationService); + }); + + afterEach(() => { + delete Config.mail.wallet.RealUnit; + }); + + function createSupportMessage(userData: UserData, issueWallet?: Wallet): SupportMessage { + const issue = Object.assign(new SupportIssue(), { id: 1, uid: 'I-1', userData, wallet: issueWallet }); + return Object.assign(new SupportMessage(), { id: 1, author: 'Support', issue }); + } + + async function sendForWallet(issueWallet?: Wallet): Promise { + const sendMail = jest.spyOn(notificationService, 'sendMail').mockResolvedValue(undefined); + const userData = createCustomUserData({ id: 7, mail: 'user@test.com' }); + await service.newSupportMessage(createSupportMessage(userData, issueWallet)); + return sendMail; + } + + async function sentMailInput(issueWallet?: Wallet): Promise { + const sendMail = await sendForWallet(issueWallet); + expect(sendMail).toHaveBeenCalledTimes(1); + return sendMail.mock.calls[0][0].input; + } + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + it('brands the mail RealUnit when the ticket was opened from the RealUnit app', async () => { + Config.mail.wallet.RealUnit = { template: 'realunit' }; + + const input = await sentMailInput(realUnitWallet); + + expect('wallet' in input && (input.wallet as Wallet)?.name).toBe('RealUnit'); + }); + + it('brands the mail DFX when the ticket was exactly DFX-attributed', async () => { + const input = await sentMailInput(dfxWallet); + + expect('wallet' in input && (input.wallet as Wallet)?.name).toBe('DFX'); + }); + + it('fails closed (no mail, error log) when the issue carries no attributed source', async () => { + const error = jest.spyOn(service['logger'], 'error'); + + const sendMail = await sendForWallet(undefined); + + expect(sendMail).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith(expect.stringContaining('no attributed source')); + }); + + it('fails closed instead of mis-branding when RealUnit is attributed but REALUNIT_MAIL_USER is unset', async () => { + const error = jest.spyOn(service['logger'], 'error'); + + // test config has no REALUNIT_MAIL_USER -> Config.mail.wallet.RealUnit is absent + const sendMail = await sendForWallet(realUnitWallet); + + expect(sendMail).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith(expect.stringContaining('REALUNIT_MAIL_USER is unset')); + }); + + it('does not send a mail when the user has no mail address', async () => { + const sendMail = jest.spyOn(notificationService, 'sendMail').mockResolvedValue(undefined); + + await service.newSupportMessage(createSupportMessage(createCustomUserData({ id: 7, mail: undefined }), dfxWallet)); + + expect(sendMail).not.toHaveBeenCalled(); + }); +}); diff --git a/src/subdomains/supporting/support-issue/services/__tests__/support-issue.service.spec.ts b/src/subdomains/supporting/support-issue/services/__tests__/support-issue.service.spec.ts index ce91d2fc20..11928b8768 100644 --- a/src/subdomains/supporting/support-issue/services/__tests__/support-issue.service.spec.ts +++ b/src/subdomains/supporting/support-issue/services/__tests__/support-issue.service.spec.ts @@ -5,6 +5,7 @@ import { validate } from 'class-validator'; import * as ConfigModule from 'src/config/config'; import { UserRole } from 'src/shared/auth/user-role.enum'; import { SettingService } from 'src/shared/models/setting/setting.service'; +import { WalletService } from 'src/subdomains/generic/user/models/wallet/wallet.service'; import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; import { BankDataService } from 'src/subdomains/generic/user/models/bank-data/bank-data.service'; import { UserDataService } from 'src/subdomains/generic/user/models/user-data/user-data.service'; @@ -76,6 +77,7 @@ describe('SupportIssueService.getSupportIssueList', () => { createMock(), createMock(), createMock(), + createMock(), ); }); @@ -275,6 +277,7 @@ describe('SupportIssueService.closeIssue', () => { supportLogService, createMock(), createMock(), + createMock(), ); }); @@ -385,6 +388,7 @@ describe('SupportIssueService.getSupportIssueStatistics', () => { createMock(), createMock(), createMock(), + createMock(), ); }); @@ -527,6 +531,7 @@ describe('SupportIssueService no-department-access guards', () => { createMock(), createMock(), createMock(), + createMock(), ); }); diff --git a/src/subdomains/supporting/support-issue/services/limit-request-notification.service.ts b/src/subdomains/supporting/support-issue/services/limit-request-notification.service.ts index 47f4e42235..6653c61dab 100644 --- a/src/subdomains/supporting/support-issue/services/limit-request-notification.service.ts +++ b/src/subdomains/supporting/support-issue/services/limit-request-notification.service.ts @@ -42,8 +42,9 @@ export class LimitRequestNotificationService { for (const entity of entities) { try { - // RealUnit: limit-request approval is handled by phone, not by email - if (entity.userData.wallet?.name === REALUNIT_WALLET_NAME) { + // RealUnit: limit-request approval is handled by phone, not by email. Brand by the app the + // ticket was opened from (issue source, eager-loaded), not the user's persisted origin wallet. + if (entity.supportIssue.wallet?.name === REALUNIT_WALLET_NAME) { await this.limitRequestRepo.update(...entity.skipMail()); continue; } diff --git a/src/subdomains/supporting/support-issue/services/support-issue-notification.service.ts b/src/subdomains/supporting/support-issue/services/support-issue-notification.service.ts index 7a93279d9e..dbf8d1557a 100644 --- a/src/subdomains/supporting/support-issue/services/support-issue-notification.service.ts +++ b/src/subdomains/supporting/support-issue/services/support-issue-notification.service.ts @@ -1,8 +1,10 @@ import { Injectable } from '@nestjs/common'; +import { Config } from 'src/config/config'; import { DfxLogger } from 'src/shared/services/dfx-logger'; import { DisabledProcess, Process } from 'src/shared/services/process.service'; import { MailContext, MailType } from '../../notification/enums'; import { MailTranslationKey } from '../../notification/factories/mail.factory'; +import { REALUNIT_WALLET_NAME } from '../../notification/realunit-mail-rules'; import { NotificationService } from '../../notification/services/notification.service'; import { SupportMessage } from '../entities/support-message.entity'; @@ -14,22 +16,44 @@ export class SupportIssueNotificationService { async newSupportMessage(entity: SupportMessage): Promise { try { - if (entity.userData.mail && !DisabledProcess(Process.SUPPORT_MESSAGE_MAIL)) - await this.notificationService.sendMail({ - type: MailType.USER_V2, - context: MailContext.SUPPORT_MESSAGE, - input: { - userData: entity.userData, - title: `${MailTranslationKey.SUPPORT_MESSAGE}.title`, - salutation: { key: `${MailTranslationKey.SUPPORT_MESSAGE}.salutation` }, - texts: [ - { - key: `${MailTranslationKey.SUPPORT_MESSAGE}.message`, - params: { url: entity.issue.url, urlText: entity.issue.url }, - }, - ], - }, - }); + if (!entity.userData.mail || DisabledProcess(Process.SUPPORT_MESSAGE_MAIL)) return; + + // Mail branding follows the app the ticket was opened from, attributed EXACTLY at creation from the + // X-Client signal (NOT the user's persisted wallet) and persisted NOT NULL. No fallback chain here: + // an unattributed issue (only possible for rows predating the backfill migration) or a positively + // RealUnit-attributed issue without its mail config means the correct brand cannot be rendered - + // fail closed (no mail, error log) instead of guessing a brand. + // Passing the wallet explicitly also bypasses resolveMailWallet's account-history override. + const wallet = entity.issue.wallet; + if (!wallet) { + this.logger.error( + `Support message mail for issue ${entity.issue.id} suppressed: issue has no attributed source wallet`, + ); + return; + } + if (wallet.name === REALUNIT_WALLET_NAME && !Config.mail.wallet[REALUNIT_WALLET_NAME]) { + this.logger.error( + `Support message mail for issue ${entity.issue.id} suppressed: RealUnit-attributed but REALUNIT_MAIL_USER is unset (would mis-brand as DFX)`, + ); + return; + } + + await this.notificationService.sendMail({ + type: MailType.USER_V2, + context: MailContext.SUPPORT_MESSAGE, + input: { + userData: entity.userData, + wallet, + title: `${MailTranslationKey.SUPPORT_MESSAGE}.title`, + salutation: { key: `${MailTranslationKey.SUPPORT_MESSAGE}.salutation` }, + texts: [ + { + key: `${MailTranslationKey.SUPPORT_MESSAGE}.message`, + params: { url: entity.issue.url, urlText: entity.issue.url }, + }, + ], + }, + }); } catch (e) { this.logger.error(`Failed to send support message mail for message (${entity.id}):`, e); } diff --git a/src/subdomains/supporting/support-issue/services/support-issue.service.ts b/src/subdomains/supporting/support-issue/services/support-issue.service.ts index fd5c87d60c..89b565efb2 100644 --- a/src/subdomains/supporting/support-issue/services/support-issue.service.ts +++ b/src/subdomains/supporting/support-issue/services/support-issue.service.ts @@ -3,19 +3,25 @@ import { ForbiddenException, Injectable, NotFoundException, + ServiceUnavailableException, UnauthorizedException, } from '@nestjs/common'; import { Config } from 'src/config/config'; import { BlobContent } from 'src/integration/infrastructure/azure-storage.service'; import { UserRole } from 'src/shared/auth/user-role.enum'; +import { DfxLogger } from 'src/shared/services/dfx-logger'; import { SupportClerkAccountDto } from 'src/shared/models/setting/dto/support-clerk-account.dto'; import { SettingService } from 'src/shared/models/setting/setting.service'; +import { CLIENT_HEADER, resolveClientSource } from 'src/shared/utils/request-client'; import { Util } from 'src/shared/utils/util'; +import { REALUNIT_WALLET_NAME } from 'src/subdomains/supporting/notification/realunit-mail-rules'; import { ContentType } from 'src/subdomains/generic/kyc/enums/content-type.enum'; import { BankDataService } from 'src/subdomains/generic/user/models/bank-data/bank-data.service'; import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; import { PhoneCallStatus } from 'src/subdomains/generic/user/models/user-data/user-data.enum'; import { UserDataService } from 'src/subdomains/generic/user/models/user-data/user-data.service'; +import { Wallet } from 'src/subdomains/generic/user/models/wallet/wallet.entity'; +import { WalletService } from 'src/subdomains/generic/user/models/wallet/wallet.service'; import { FindOptionsWhere, In, IsNull, MoreThan, Not } from 'typeorm'; import { TransactionRequestType } from '../../payment/entities/transaction-request.entity'; import { TransactionSourceType } from '../../payment/entities/transaction.entity'; @@ -52,6 +58,8 @@ import { SupportLogService } from './support-log.service'; @Injectable() export class SupportIssueService { + private readonly logger = new DfxLogger(SupportIssueService); + constructor( private readonly supportIssueRepo: SupportIssueRepository, private readonly transactionService: TransactionService, @@ -64,6 +72,7 @@ export class SupportIssueService { private readonly supportLogService: SupportLogService, private readonly bankDataService: BankDataService, private readonly settingService: SettingService, + private readonly walletService: WalletService, ) {} async getSupportIssueClerks(): Promise { @@ -272,7 +281,7 @@ export class SupportIssueService { }; } - async createTransactionRequestIssue(dto: CreateSupportIssueBaseDto): Promise { + async createTransactionRequestIssue(dto: CreateSupportIssueBaseDto, client?: string): Promise { if (!dto?.transaction?.orderUid) throw new BadRequestException('JWT Token or quoteUid missing'); const transactionRequest = await this.transactionRequestService.getTransactionRequestByUid( dto.transaction.orderUid, @@ -280,21 +289,66 @@ export class SupportIssueService { ); if (!transactionRequest) throw new NotFoundException('TransactionRequest not found'); - return this.createIssueInternal(transactionRequest.userData, dto); + return this.createIssueInternal(transactionRequest.userData, dto, await this.resolveSourceWallet(client)); + } + + // User-opened ticket: source app comes from the per-request X-Client header. + async createIssue(userDataId: number, dto: CreateSupportIssueDto, client?: string): Promise { + return this.createForUserData(userDataId, dto, await this.resolveSourceWallet(client)); + } + + // Support-tool-created ticket: the support tool is part of the DFX services app, so these tickets are + // deterministically DFX-attributed (an exact property of the creating application, not a guess). A + // dedicated method - rather than an omitted optional arg on createIssue - so the invariant cannot be + // broken by a future caller forwarding a customer client header into the support path. + async createIssueBySupport(userDataId: number, dto: CreateSupportIssueDto): Promise { + return this.createForUserData(userDataId, dto, await this.walletService.getDefault()); } - async createIssue(userDataId: number, dto: CreateSupportIssueDto): Promise { + private async createForUserData( + userDataId: number, + dto: CreateSupportIssueDto, + sourceWallet: Wallet, + ): Promise { const userData = await this.userDataService.getUserData(userDataId, { wallet: true }); if (!userData) throw new NotFoundException('UserData not found'); - return this.createIssueInternal(userData, dto); + return this.createIssueInternal(userData, dto, sourceWallet); } - async createIssueInternal(userData: UserData, dto: CreateSupportIssueDto): Promise { + // App the ticket is opened from, resolved EXACTLY from the per-request X-Client header (never from the + // user's persisted wallet). Fail closed: an unknown or missing client is rejected instead of being + // guessed into a brand - every ticket-creating app must identify itself (realunit-app, dfx-services). + private async resolveSourceWallet(client?: string): Promise { + const source = resolveClientSource(client); + if (!source) + throw new BadRequestException( + `Support ticket source could not be resolved: missing or unknown '${CLIENT_HEADER}' header`, + ); + + if (source === 'RealUnit') { + const wallet = await this.walletService.getByIdOrName(undefined, REALUNIT_WALLET_NAME); + // Fail closed: without the RealUnit wallet the ticket cannot be attributed exactly, and rendering + // it as DFX would be a wrong brand, not a fallback. + if (!wallet) + throw new ServiceUnavailableException( + `RealUnit ticket source resolved but the '${REALUNIT_WALLET_NAME}' wallet is missing`, + ); + return wallet; + } + + return this.walletService.getDefault(); + } + + async createIssueInternal( + userData: UserData, + dto: CreateSupportIssueDto, + sourceWallet: Wallet, + ): Promise { // mail is required if (!userData.mail) throw new BadRequestException('Mail is missing'); - const newIssue = this.supportIssueRepo.create({ userData, ...dto }); + const newIssue = this.supportIssueRepo.create({ userData, wallet: sourceWallet, ...dto }); const existingWhere: FindOptionsWhere = { userData: { id: userData.id }, @@ -384,6 +438,16 @@ export class SupportIssueService { } const entity = existingIssue ?? (await this.supportIssueRepo.save(newIssue)); + + // Dedup keeps the existing issue's attribution: the source is a property of the app the ticket was + // originally opened from (NOT NULL since the backfill migration), so a follow-up message from another + // app must not rebrand it. Legacy rows created before attribution existed were backfilled to DFX; if + // one ever surfaces unattributed, upgrade it with the now-known exact source instead of guessing. + if (existingIssue && !existingIssue.wallet) { + existingIssue.wallet = sourceWallet; + await this.supportIssueRepo.update(existingIssue.id, { wallet: sourceWallet }); + } + const supportMessage = await this.createMessageInternal(entity, dto); const issue = SupportIssueDtoMapper.mapSupportIssue(entity); diff --git a/src/subdomains/supporting/support-issue/support-issue.controller.ts b/src/subdomains/supporting/support-issue/support-issue.controller.ts index bb7d70a2ec..2f8e68ec16 100644 --- a/src/subdomains/supporting/support-issue/support-issue.controller.ts +++ b/src/subdomains/supporting/support-issue/support-issue.controller.ts @@ -1,4 +1,4 @@ -import { Body, Controller, Get, Param, Post, Put, Query, UseGuards } from '@nestjs/common'; +import { Body, Controller, Get, Headers, Param, Post, Put, Query, UseGuards } from '@nestjs/common'; import { ModuleRef } from '@nestjs/core'; import { AuthGuard } from '@nestjs/passport'; import { ApiBadRequestResponse, ApiBearerAuth, ApiExcludeEndpoint, ApiTags } from '@nestjs/swagger'; @@ -10,6 +10,7 @@ import { RealIP } from 'src/shared/auth/real-ip.decorator'; import { hasRoleAccess, RoleGuard } from 'src/shared/auth/role.guard'; import { isUserActive, UserActiveGuard } from 'src/shared/auth/user-active.guard'; import { UserRole } from 'src/shared/auth/user-role.enum'; +import { CLIENT_HEADER } from 'src/shared/utils/request-client'; import { TfaGuard } from 'src/subdomains/generic/kyc/guards/tfa.guard'; import { TfaLevel, TfaService } from 'src/subdomains/generic/kyc/services/tfa.service'; import { BindEscalationChatDto } from './dto/bind-escalation-chat.dto'; @@ -52,6 +53,7 @@ export class SupportIssueController { async createIssue( @GetJwt() jwt: JwtPayload | undefined, @Body() dto: CreateSupportIssueDto, + @Headers(CLIENT_HEADER) client?: string, ): Promise { const input: CreateSupportIssueDto = { ...dto, @@ -62,8 +64,8 @@ export class SupportIssueController { : Department.SUPPORT, }; return jwt?.account - ? this.supportIssueService.createIssue(jwt.account, input) - : this.supportIssueService.createTransactionRequestIssue(input); + ? this.supportIssueService.createIssue(jwt.account, input, client) + : this.supportIssueService.createTransactionRequestIssue(input, client); } @Post('support') @@ -79,7 +81,10 @@ export class SupportIssueController { ...dto, department: jwt.role === UserRole.COMPLIANCE ? Department.COMPLIANCE : Department.SUPPORT, }; - return this.supportIssueService.createIssue(+userDataId, input); + // Support-created tickets originate from the DFX support tool (part of the DFX services app) and are + // therefore exactly DFX-attributed. The dedicated service method encodes that invariant (no client + // param to forward), so a customer client header can never rebrand a staff-created ticket. + return this.supportIssueService.createIssueBySupport(+userDataId, input); } @Get()