From ef39d0189feb1d09e55ae2fbfb7cd8dfdf06086f Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:39:24 +0200 Subject: [PATCH 1/3] Cover super-roles in the staff-clearance write guard, document the recovery path (#4575) * Cover super-roles in the elevated-role assignment guard The write-side check that forbids assigning a gated role to an account without a verified name compared against KycGatedRoles directly. A super-role such as SuperAdmin satisfies every gate through the role hierarchy without appearing in that list, so the highest privilege could still be assigned to an account with no identification signal. Match the clearance query and compare against rolesSatisfying(KycGatedRoles) instead. Cover the super-role case in the test, and construct the service under test with typed mocks so a future constructor change fails to compile rather than wiring dependencies into the wrong slots. * Document how staff regain clearance via a migration PR Explain the staff clearance gate and, most importantly, how an affected staff member regains access: by submitting a migration PR that sets a verified name on the account. Records that a real name is PII and must be sourced from the deployment config rather than hard-coded in this public repo, and that production runs the migration on deploy so no database shell access is involved. * Share the clearance role set and correct the propagation timing Export the clearance role set (ClearanceRelevantRoles) and reuse it in the write-side guard instead of recomputing rolesSatisfying(KycGatedRoles), so the read and write sides share one definition. Correct the documentation: clearance propagates through two crons (the EVERY_MINUTE DB scan and the EVERY_30_SECONDS in-memory prime), so a change takes effect within about 90 seconds, not one minute. --- docs/staff-kyc-clearance.md | 51 ++++++++++++++ .../user/__tests__/user.service.spec.ts | 70 +++++++++++++------ .../user/staff-kyc-clearance.service.ts | 4 +- .../generic/user/models/user/user.service.ts | 17 +++-- 4 files changed, 112 insertions(+), 30 deletions(-) create mode 100644 docs/staff-kyc-clearance.md diff --git a/docs/staff-kyc-clearance.md b/docs/staff-kyc-clearance.md new file mode 100644 index 0000000000..cb437b9a52 --- /dev/null +++ b/docs/staff-kyc-clearance.md @@ -0,0 +1,51 @@ +# Staff KYC clearance + +Elevated endpoints — every `RoleGuard` whose entry roles are all gated (`KycGatedRoles`: `Admin`, +`Debug`, `Compliance`, `Support`, `RealUnit`, plus the super-roles that satisfy them, e.g. +`SuperAdmin`) — require two things from the caller: the role itself **and** that an identified +person is behind the account, expressed as a non-empty `verifiedName`. + +There is **no KYC-level requirement**. `verifiedName` is only ever written by an identity-verified +path (bank-data verification) or a reviewed migration — never by customer self-service — so the +presence of the name is the authoritative identification signal on its own. + +Clearance flows through two cron steps: `StaffKycClearanceService.syncStaffKycClearance` +(`@DfxCron(EVERY_MINUTE)`) derives the cleared account ids from the DB — staff users whose +`user_data.verifiedName` is non-blank — and writes them to the `staffKycClearance` setting; +`ProcessService.resyncStaffKycClearance` (`@DfxCron(EVERY_30_SECONDS)`) then primes the in-memory set +that `RoleGuard` actually reads. A database change therefore takes effect within about 90 seconds +(up to one minute for the DB scan plus up to 30 seconds for the prime) — no re-login and no deploy +needed once the name is in the database. + +## Why an account is refused + +A staff account with a role but no `verifiedName` is denied every elevated endpoint. The refusal is +deliberate and fail-closed; the response carries the machine-readable code `STAFF_KYC_REQUIRED`. +This is the expected state for a newly-granted staff role, and for any account whose identification +was never recorded. + +## Regaining access — submit a migration PR + +**An affected staff member regains access by having a `verifiedName` set on their account, and the +way to do that is a migration PR.** A one-off manual database edit is not the path: the change must +be reviewed and reproducible like any other schema/data change. + +1. Open a PR against the api repo that adds a new migration under `migration/` (never modify an + existing one — `api-migration-check.yaml` blocks that). The migration runs a guarded, idempotent + `UPDATE user_data SET "verifiedName" = … WHERE id = … AND "verifiedName" IS NULL`. +2. **A real person's name is PII and must not be hard-coded in this public repo.** The migration + reads the value from a deployment secret (e.g. `process.env.STAFF_VERIFIED_NAME_`), set in the + production config, and applies it only when present. A non-personal service designation (for a + machine account that cannot complete a personal identification) is not PII and may appear inline. + The concrete name↔account mapping is recorded in the private operations repo, not here. +3. Merge the PR through the normal review, then release `develop → main`. The production deploy runs + pending migrations automatically when `SQL_MIGRATE=true` (see `migrationsRun` in + `src/config/config.ts`) — no shell access to the database is involved. +4. Within about 90 seconds of the deploy (the two cron steps above) the clearance set is re-derived + and the account's elevated endpoints answer normally again. + +## Revoking access + +Removing a staff member's `verifiedName` (or their role) drops them out of the clearance query on +the next cron run, within about 90 seconds — again with no deploy or token rotation. Clearance is never +auto-revoked by the migration's `down()`; removal is always a deliberate action. diff --git a/src/subdomains/generic/user/models/user/__tests__/user.service.spec.ts b/src/subdomains/generic/user/models/user/__tests__/user.service.spec.ts index 2fd4e0c017..0a498e7966 100644 --- a/src/subdomains/generic/user/models/user/__tests__/user.service.spec.ts +++ b/src/subdomains/generic/user/models/user/__tests__/user.service.spec.ts @@ -1,36 +1,48 @@ +import { createMock } from '@golevelup/ts-jest'; import { ForbiddenException } from '@nestjs/common'; +import { GeoLocationService } from 'src/integration/geolocation/geo-location.service'; +import { SiftService } from 'src/integration/sift/services/sift.service'; import { UserRole } from 'src/shared/auth/user-role.enum'; +import { AssetService } from 'src/shared/models/asset/asset.service'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; +import { LanguageService } from 'src/shared/models/language/language.service'; +import { UserDataRepository } from 'src/subdomains/generic/user/models/user-data/user-data.repository'; +import { UserDataService } from 'src/subdomains/generic/user/models/user-data/user-data.service'; +import { WalletService } from 'src/subdomains/generic/user/models/wallet/wallet.service'; +import { FeeService } from 'src/subdomains/supporting/payment/services/fee.service'; +import { UserData } from '../../user-data/user-data.entity'; +import { User } from '../user.entity'; +import { UserRepository } from '../user.repository'; import { UserService } from '../user.service'; // Focused on the write-side clearance invariant in `updateUserInternal`: an account may only be given a -// gated role when a verified name is already behind it. The guard reads only `userRepo`, so the service -// is constructed with a mock repository and inert stubs for the other collaborators. +// gated role when a verified name is already behind it. Only `userRepo` participates in this path; the +// remaining collaborators are typed mocks so a future constructor reorder fails the compiler instead of +// silently wiring the service with dependencies in the wrong slots. describe('UserService — elevated role assignment guard', () => { let service: UserService; - let userRepo: { findOne: jest.Mock; save: jest.Mock }; + let userRepo: jest.Mocked; - function userWith(verifiedName: string | null | undefined, loaded = true): any { - const userData = { id: 7, verifiedName }; - return { id: 42, userData: loaded ? userData : undefined }; + function userWith(verifiedName: string | null | undefined, loaded = true): User { + const userData = createMock({ id: 7, verifiedName: verifiedName ?? undefined }); + return createMock({ id: 42, userData: loaded ? userData : undefined }); } beforeEach(() => { - userRepo = { - findOne: jest.fn(), - save: jest.fn().mockImplementation((u) => Promise.resolve(u)), - }; - // Only userRepo participates in this path; the remaining collaborators are never reached. + userRepo = createMock(); + userRepo.save.mockImplementation((entity) => Promise.resolve(entity as User)); + service = new UserService( - userRepo as any, - {} as any, - {} as any, - {} as any, - {} as any, - {} as any, - {} as any, - {} as any, - {} as any, - {} as any, + userRepo, + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), ); }); @@ -41,8 +53,18 @@ describe('UserService — elevated role assignment guard', () => { expect(userRepo.save).not.toHaveBeenCalled(); }); + // SUPER_ADMIN is not itself in KycGatedRoles but satisfies every gate through the role hierarchy, so the + // guard must cover it exactly as the clearance query does — otherwise the highest privilege of all could + // be assigned to a faceless account. + it('rejects a super-role (SUPER_ADMIN) when the account has no verified name', async () => { + await expect(service.updateUserInternal(userWith(null), { role: UserRole.SUPER_ADMIN })).rejects.toBeInstanceOf( + ForbiddenException, + ); + expect(userRepo.save).not.toHaveBeenCalled(); + }); + it('rejects a gated role when the verified name is blank whitespace', async () => { - await expect(service.updateUserInternal(userWith('  \t'), { role: UserRole.ADMIN })).rejects.toBeInstanceOf( + await expect(service.updateUserInternal(userWith(' \t'), { role: UserRole.ADMIN })).rejects.toBeInstanceOf( ForbiddenException, ); expect(userRepo.save).not.toHaveBeenCalled(); @@ -54,7 +76,9 @@ describe('UserService — elevated role assignment guard', () => { }); it('reloads userData when the relation was not hydrated, then allows on a present name', async () => { - userRepo.findOne.mockResolvedValue({ userData: { id: 7, verifiedName: 'Jane Doe' } }); + userRepo.findOne.mockResolvedValue( + createMock({ userData: createMock({ verifiedName: 'Jane Doe' }) }), + ); await service.updateUserInternal(userWith(undefined, false), { role: UserRole.DEBUG }); diff --git a/src/subdomains/generic/user/models/user/staff-kyc-clearance.service.ts b/src/subdomains/generic/user/models/user/staff-kyc-clearance.service.ts index c6a1c9ad7d..53a3929d09 100644 --- a/src/subdomains/generic/user/models/user/staff-kyc-clearance.service.ts +++ b/src/subdomains/generic/user/models/user/staff-kyc-clearance.service.ts @@ -11,7 +11,9 @@ import { UserRepository } from './user.repository'; // SUPER_ADMIN, which satisfies every gate but is not itself listed in KycGatedRoles). Derived, not // hand-written — a role added to the hierarchy must not silently fall out of the clearance sync and // lose access. -const ClearanceRelevantRoles = rolesSatisfying(KycGatedRoles); +// Exported so the write-side guard in UserService gates the exact same set this service clears — +// a single source keeps the read and write sides from drifting. +export const ClearanceRelevantRoles = rolesSatisfying(KycGatedRoles); // Every character `String.prototype.trim()` strips (ECMAScript WhiteSpace + LineTerminator). Postgres' // bare `TRIM(x)` removes ASCII space ONLY, so a name of a single tab or a non-breaking space would pass diff --git a/src/subdomains/generic/user/models/user/user.service.ts b/src/subdomains/generic/user/models/user/user.service.ts index 20bd975106..b5d4d3d72e 100644 --- a/src/subdomains/generic/user/models/user/user.service.ts +++ b/src/subdomains/generic/user/models/user/user.service.ts @@ -12,7 +12,7 @@ import { Config } from 'src/config/config'; import { CryptoService } from 'src/integration/blockchain/shared/services/crypto.service'; import { GeoLocationService } from 'src/integration/geolocation/geo-location.service'; import { SiftService } from 'src/integration/sift/services/sift.service'; -import { KycGatedRoles, UserRole } from 'src/shared/auth/user-role.enum'; +import { UserRole } from 'src/shared/auth/user-role.enum'; import { Active } from 'src/shared/models/active'; import { AssetService } from 'src/shared/models/asset/asset.service'; import { FiatService } from 'src/shared/models/fiat/fiat.service'; @@ -54,6 +54,7 @@ import { ReferralDto, UpdateRefDto, UserV2Dto } from './dto/user-v2.dto'; import { UserDetailDto, UserDetails } from './dto/user.dto'; import { UpdateMailStatus } from './dto/verify-mail.dto'; import { VolumeQuery } from './dto/volume-query.dto'; +import { ClearanceRelevantRoles } from './staff-kyc-clearance.service'; import { User } from './user.entity'; import { RefPayoutFrequency, UserAddressType, UserStatus } from './user.enum'; import { UserRepository } from './user.repository'; @@ -473,11 +474,15 @@ export class UserService { async updateUserInternal(user: User, update: UpdateUserInternalDto): Promise { // Write-side counterpart to the staff KYC clearance: an account may only be given a gated role if a // verified name is already behind it, so a faceless staff account with no identification signal can - // never be created. Blankness matches the clearance definition — `verifiedName.trim()` strips exactly - // the characters `BlankChars` lists (see staff-kyc-clearance.service.ts), so this stays in step with - // the DB predicate that decides clearance. userData is reloaded when absent so a caller that did not - // hydrate the relation cannot slip an elevated role past the check. - if (update.role && KycGatedRoles.includes(update.role)) { + // never be created. It gates the same `ClearanceRelevantRoles` set the clearance query clears — + // that set is the gated roles plus their super-roles (e.g. SUPER_ADMIN, which satisfies every gate + // without being listed in KycGatedRoles), so sharing the one constant keeps write and read in step + // and stops the highest privilege from being granted without an identification signal. Blankness + // matches the clearance definition — `verifiedName.trim()` strips exactly the characters `BlankChars` + // lists (see staff-kyc-clearance.service.ts), so this stays in step with the DB predicate that decides + // clearance. userData is reloaded when absent so a caller that did not hydrate the relation cannot + // slip an elevated role past the check. + if (update.role && ClearanceRelevantRoles.includes(update.role)) { const userData = user.userData ?? (await this.userRepo.findOne({ where: { id: user.id }, relations: { userData: true } }))?.userData; From 4bda6a0e5615cb940d8f3fd66b42ef592c667d2b Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:40:57 +0200 Subject: [PATCH 2/3] Name the status and the faulty header when a signed response fails verification (#4565) * Pass the HTTP status of a signed response to its verifier A responseVerifier that rejects a response throws an error which replaces the original axios error, so the status those bytes arrived with is lost before any caller can log it. An unsigned 2xx and an unsigned 5xx then read identically, although one points at our own key or configuration and the other at an upstream outage. Hand the status to the verifier on both the success and the error path. The parameter is optional, so existing verifiers keep working unchanged. * Name the status and the faulty header when a Bank Frick response fails verification A failed signature check currently logs one of two fixed strings and nothing else, so an operator cannot tell an unsigned error page from an unsigned success, nor which of the two headers was at fault. During an upstream incident that is the difference between chasing our own key material and waiting for the bank. Append a bounded diagnostic tail: the HTTP status when it is a usable code, plus a per-header classification (missing, present, ok, unsupported). Header values are still never echoed - they are attacker-influenced bytes, and these messages reach logs, alert mail and persisted intent error fields. The existing message prefixes are unchanged, so log filters and alert rules that match on them keep matching. * Cover the unsigned-response case end to end for both a 5xx and a 2xx The two halves of the incident case were covered separately - the transport layer handing over a status, and the verifier turning it into a message - but never composed. Cover the actual failure through the real HttpService: an unsigned error page with no signature headers at all, asserted for a 5xx and for a 2xx, which produce identical bytes and headers and differ only in the status now carried into the message. --- .../services/__tests__/frick.service.spec.ts | 77 +++++++++++++++++++ .../bank/services/frick.service.ts | 39 ++++++++-- .../services/__tests__/http.service.spec.ts | 29 +++++-- src/shared/services/http.service.ts | 19 +++-- 4 files changed, 148 insertions(+), 16 deletions(-) diff --git a/src/integration/bank/services/__tests__/frick.service.spec.ts b/src/integration/bank/services/__tests__/frick.service.spec.ts index e20dbe5cb5..24864eec9a 100644 --- a/src/integration/bank/services/__tests__/frick.service.spec.ts +++ b/src/integration/bank/services/__tests__/frick.service.spec.ts @@ -213,6 +213,53 @@ describe('BankFrickService', () => { expect(() => service['verifyResponse'](Buffer.from('{"synthetic":true}'), headers as never)).toThrow(expectedError); }); + it.each([ + [{}, 500, 'Invalid Bank Frick response signature headers (HTTP 500, signature missing, algorithm missing)'], + [{}, 200, 'Invalid Bank Frick response signature headers (HTTP 200, signature missing, algorithm missing)'], + [ + { algorithm: 'rsa-sha512' }, + 502, + 'Invalid Bank Frick response signature headers (HTTP 502, signature missing, algorithm ok)', + ], + [ + { signature: 'irrelevant', algorithm: 'rsa-pss-sha512' }, + 200, + 'Invalid Bank Frick response signature headers (HTTP 200, signature present, algorithm unsupported)', + ], + [ + { signature: 'not-a-signature', algorithm: 'rsa-sha512' }, + 200, + 'Invalid Bank Frick response signature (HTTP 200)', + ], + ])('names the status and which signature header was at fault', (headers, status, expectedError) => { + // Without the status, an unsigned upstream failure and an unsigned success read identically in + // the logs, although one is the bank's outage and the other is our own key/config problem. + expect(() => service['verifyResponse'](Buffer.from('{"synthetic":true}'), headers as never, status)).toThrow( + expectedError, + ); + }); + + it.each([undefined, 0, 99, 600, Number.NaN])('omits an unusable status instead of printing it (%s)', (status) => { + expect(() => service['verifyResponse'](Buffer.from('{"synthetic":true}'), {} as never, status as never)).toThrow( + 'Invalid Bank Frick response signature headers (signature missing, algorithm missing)', + ); + }); + + it('never echoes header values into the error message', () => { + const secretish = 'AKIAIOSFODNN7EXAMPLE-should-never-reach-a-log-line'; + + expect(() => + service['verifyResponse'](Buffer.from('{"synthetic":true}'), { algorithm: secretish } as never, 500), + ).toThrow('Invalid Bank Frick response signature headers (HTTP 500, signature missing, algorithm unsupported)'); + + try { + service['verifyResponse'](Buffer.from('{"synthetic":true}'), { signature: secretish } as never, 500); + fail('expected a signature verification error'); + } catch (error) { + expect(error.message).not.toContain(secretish); + } + }); + it('fails closed when a payment is attempted without the explicit payout flag', async () => { await expect(service.createPaymentOrder(paymentInput())).rejects.toThrow('payout is not explicitly enabled'); await expect(service.approvePaymentWithoutTan(paymentOrder())).rejects.toThrow('payout is not explicitly enabled'); @@ -1733,6 +1780,36 @@ describe('BankFrickService', () => { ); await expect(rejected).rejects.not.toBeInstanceOf(FrickVibanNotCreatedError); }); + + it.each([ + [503, 'HTTP 503'], + [200, 'HTTP 200'], + ])( + 'carries the status of an unsigned %s response all the way into the reported message', + async (status, expectedStatus) => { + // The upstream-outage case, end to end through the real HttpService: an error page carries no + // signature headers at all. Body and headers are identical whether the gateway answered 503 or + // the API answered 200, so only the status separates "their outage" from "our key/config" - + // and it is exactly the value the verifier's error would otherwise discard. + const unsignedBody = Buffer.from('gateway error'); + const transportResponse = { data: unsignedBody, headers: {}, status }; + + nestHttp.request.mockReturnValueOnce(of(signedAuthorizeTransportResponse())).mockReturnValueOnce( + status >= 400 + ? throwError(() => + Object.assign(new Error(`Request failed with status code ${status}`), { + response: transportResponse, + isAxiosError: true, + }), + ) + : of(transportResponse), + ); + + await expect(frickWithRealHttp.createViban(debtorIban, 'dfx-viban-e2e-real-http')).rejects.toThrow( + `Bank Frick response signature verification failed (POST virtual-ibans): Invalid Bank Frick response signature headers (${expectedStatus}, signature missing, algorithm missing)`, + ); + }, + ); }); function virtualIbanResponse( diff --git a/src/integration/bank/services/frick.service.ts b/src/integration/bank/services/frick.service.ts index 3daf4d12b9..0001ffb4c0 100644 --- a/src/integration/bank/services/frick.service.ts +++ b/src/integration/bank/services/frick.service.ts @@ -808,7 +808,7 @@ export class BankFrickService { Signature: signature, algorithm: 'rsa-sha512', }, - responseVerifier: (rawBody, headers) => this.verifyResponse(rawBody, headers), + responseVerifier: (rawBody, headers, status) => this.verifyResponse(rawBody, headers, status), }); } catch (error) { if (error instanceof FrickSignatureVerificationError) @@ -919,7 +919,7 @@ export class BankFrickService { Signature: this.sign(bodyString), algorithm: 'rsa-sha512', }, - responseVerifier: (rawBody, headers) => this.verifyResponse(rawBody, headers), + responseVerifier: (rawBody, headers, status) => this.verifyResponse(rawBody, headers, status), }); } catch (error) { if (error instanceof FrickSignatureVerificationError) @@ -963,22 +963,49 @@ export class BankFrickService { } } - private verifyResponse(rawBody: Buffer, headers: AxiosResponse['headers']): void { + private verifyResponse(rawBody: Buffer, headers: AxiosResponse['headers'], status?: number): void { const signature = headers?.signature ?? headers?.Signature; const algorithm = String(headers?.algorithm ?? headers?.Algorithm ?? '').toLowerCase(); const algorithms = { 'rsa-sha512': 'sha512', 'rsa-sha384': 'sha384', 'rsa-sha256': 'sha256' } as const; const hashAlgorithm = algorithms[algorithm as keyof typeof algorithms]; - if (typeof signature !== 'string' || !signature || !hashAlgorithm) - throw new FrickSignatureVerificationError('Invalid Bank Frick response signature headers'); + const signaturePresent = typeof signature === 'string' && !!signature; + if (!signaturePresent || !hashAlgorithm) + throw new FrickSignatureVerificationError( + `Invalid Bank Frick response signature headers${this.describeVerificationContext(status, [ + `signature ${signaturePresent ? 'present' : 'missing'}`, + `algorithm ${!algorithm ? 'missing' : hashAlgorithm ? 'ok' : 'unsupported'}`, + ])}`, + ); try { if (!Util.verifySign(rawBody, Config.bank.frick.serverPublicKey, signature, hashAlgorithm, 'base64')) throw new Error('signature mismatch'); } catch { - throw new FrickSignatureVerificationError('Invalid Bank Frick response signature'); + throw new FrickSignatureVerificationError( + `Invalid Bank Frick response signature${this.describeVerificationContext(status)}`, + ); } } + /** + * Builds the parenthesised diagnostic tail of a signature-verification error. + * + * A rejected response is the one case where the caller never gets to see the underlying axios + * error, so without this the log line cannot tell an unsigned success from an unsigned upstream + * failure - the two demand opposite responses (our key/config vs. their outage). Everything here + * is drawn from a closed vocabulary: a bounded status integer and fixed classification words. + * Header *values* are deliberately never echoed - they are attacker-influenced bytes, and this + * string reaches server logs, alert mail and persisted intent/event error fields. + */ + private describeVerificationContext(status?: number, details: string[] = []): string { + const parts = [ + ...(Number.isInteger(status) && status >= 100 && status <= 599 ? [`HTTP ${status}`] : []), + ...details, + ]; + + return parts.length ? ` (${parts.join(', ')})` : ''; + } + private createUrl(path: string): string { return `${Config.bank.frick.baseUrl.replace(/\/+$/, '')}/${path.replace(/^\/+/, '')}`; } diff --git a/src/shared/services/__tests__/http.service.spec.ts b/src/shared/services/__tests__/http.service.spec.ts index f3996b65e7..383328fb72 100644 --- a/src/shared/services/__tests__/http.service.spec.ts +++ b/src/shared/services/__tests__/http.service.spec.ts @@ -22,7 +22,7 @@ describe('HttpService signed responses', () => { it('verifies exact raw JSON bytes before parsing them', async () => { const rawBody = '{ "answer": 42 }'; const headers = { signature: 'synthetic-signature', algorithm: 'rsa-sha512' }; - nestHttp.request.mockReturnValue(of({ data: Buffer.from(rawBody), headers })); + nestHttp.request.mockReturnValue(of({ data: Buffer.from(rawBody), headers, status: 200 })); const responseVerifier = jest.fn(); await expect( @@ -33,7 +33,7 @@ describe('HttpService signed responses', () => { }), ).resolves.toEqual({ answer: 42 }); - expect(responseVerifier).toHaveBeenCalledWith(Buffer.from(rawBody), headers); + expect(responseVerifier).toHaveBeenCalledWith(Buffer.from(rawBody), headers, 200); const transportConfig = nestHttp.request.mock.calls[0][0]; expect(transportConfig.responseType).toBe('arraybuffer'); expect(transportConfig.transformResponse[0](rawBody)).toBe(rawBody); @@ -50,12 +50,12 @@ describe('HttpService signed responses', () => { Buffer.from([0xff]), ]); const headers = { signature: 'synthetic-signature', algorithm: 'rsa-sha512' }; - nestHttp.request.mockReturnValue(of({ data: rawBytes, headers })); + nestHttp.request.mockReturnValue(of({ data: rawBytes, headers, status: 200 })); const responseVerifier = jest.fn(); await service.request({ url: 'https://synthetic.example/signed', responseType: 'text', responseVerifier }); - expect(responseVerifier).toHaveBeenCalledWith(rawBytes, headers); + expect(responseVerifier).toHaveBeenCalledWith(rawBytes, headers, 200); expect(Buffer.isBuffer(responseVerifier.mock.calls[0][0])).toBe(true); }); @@ -117,10 +117,29 @@ describe('HttpService signed responses', () => { originalError, ); - expect(responseVerifier).toHaveBeenCalledWith(errorBody, headers); + expect(responseVerifier).toHaveBeenCalledWith(errorBody, headers, 401); expect(originalError.response.status).toBe(401); }); + it('hands the error-response status to the verifier so a rejected error body can still name it', async () => { + // The verifier's error replaces the axios error, so the status it carried is lost unless the + // verifier is told what it was. This is the outage case: an upstream 5xx whose error body is + // unsigned must not be reported as if it were an unsigned 200. + const errorBody = Buffer.from('gateway error'); + const originalError = Object.assign(new Error('Request failed with status code 502'), { + response: { status: 502, data: errorBody, headers: {} }, + isAxiosError: true, + }); + nestHttp.request.mockReturnValue(throwError(() => originalError)); + const responseVerifier = jest.fn((_body: Buffer, _headers: unknown, status?: number) => { + throw new Error(`unsigned response (HTTP ${status})`); + }); + + await expect(service.request({ url: 'https://synthetic.example/signed', responseVerifier })).rejects.toThrow( + 'unsigned response (HTTP 502)', + ); + }); + it('propagates the verifier error instead of the original axios error when the error-response signature is invalid', async () => { const errorBody = Buffer.from('{"error":"unauthorized"}'); const headers = { signature: 'bad', algorithm: 'rsa-sha512' }; diff --git a/src/shared/services/http.service.ts b/src/shared/services/http.service.ts index 80a1dcb51d..c8cceee001 100644 --- a/src/shared/services/http.service.ts +++ b/src/shared/services/http.service.ts @@ -20,7 +20,10 @@ export type HttpRequestConfig = AxiosRequestConfig & { retryDelay?: number; // Raw response bytes, exactly as received - never decoded/transcoded before the caller verifies // them, so a legitimately signed response can never fail verification due to axios' own text decoding. - responseVerifier?: (rawBody: Buffer, headers: AxiosResponse['headers']) => void; + // `status` is the HTTP status the bytes arrived with. It is passed on both the success and the + // error path so a verifier that rejects a response can say which one it rejected: an unsigned 2xx + // and an unsigned 5xx are indistinguishable in the body alone, but mean very different things. + responseVerifier?: (rawBody: Buffer, headers: AxiosResponse['headers'], status?: number) => void; }; type MockResponseFactory = (url: string, config?: HttpRequestConfig) => unknown; @@ -182,7 +185,7 @@ export class HttpService { if (!responseVerifier) return response.data; if (!Buffer.isBuffer(response.data)) throw new Error('Signed HTTP response body is not a raw byte buffer'); - responseVerifier(response.data, response.headers); + responseVerifier(response.data, response.headers, response.status); const decoded = response.data.toString('utf8'); if (requestedResponseType === 'text') return decoded as T; return JSON.parse(decoded) as T; @@ -197,7 +200,11 @@ export class HttpService { error: unknown, responseVerifier: NonNullable, ): void { - const httpResponse = (error as { response?: { data?: unknown; headers?: AxiosResponse['headers'] } })?.response; + const httpResponse = ( + error as { + response?: { data?: unknown; headers?: AxiosResponse['headers']; status?: number }; + } + )?.response; if (!httpResponse) return; if (!Buffer.isBuffer(httpResponse.data)) { @@ -205,8 +212,10 @@ export class HttpService { } // Verifier throws on bad/missing signature → that error must replace the original axios error. - // On success, the caller re-throws the original so status classification stays intact. - responseVerifier(httpResponse.data, httpResponse.headers); + // On success, the caller re-throws the original so status classification stays intact. The status + // is handed over because the replacing error is the only one the caller will ever see: without it, + // a rejected error response loses the status that the original axios error carried. + responseVerifier(httpResponse.data, httpResponse.headers, httpResponse.status); } async downloadFile(fileUrl: string, filePath: string) { From fa2820d36fc45254b7524c70df0dce2076b5becc Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:44:12 +0200 Subject: [PATCH 3/3] 019fbcf7 - Backfill staff verified names on PRD (#4574) * Backfill staff verified names on PRD * Harden staff name backfill guarantees * Satisfy PostgreSQL migration syntax gate * Describe staff name as deployment variable --- ...785584840000-BackfillStaffVerifiedNames.js | 82 ++++++ src/shared/auth/role.guard.ts | 9 +- src/shared/auth/staff-kyc-clearance.ts | 7 +- src/shared/auth/user-role.enum.ts | 11 +- ...ill-staff-verified-names.migration.spec.ts | 275 ++++++++++++++++++ .../user/staff-kyc-clearance.service.ts | 7 +- 6 files changed, 376 insertions(+), 15 deletions(-) create mode 100644 migration/1785584840000-BackfillStaffVerifiedNames.js create mode 100644 src/subdomains/generic/user/models/user/__tests__/backfill-staff-verified-names.migration.spec.ts diff --git a/migration/1785584840000-BackfillStaffVerifiedNames.js b/migration/1785584840000-BackfillStaffVerifiedNames.js new file mode 100644 index 0000000000..3e741ab120 --- /dev/null +++ b/migration/1785584840000-BackfillStaffVerifiedNames.js @@ -0,0 +1,82 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * PRD-only backfill: sets `verifiedName` on the staff/service accounts that were gated out when the + * staff-clearance rule stopped requiring a KYC level (api#4395 → #4572). No plaintext personal name + * lives in this file: the human account's verified name is read from the PRD deployment variable + * STAFF_VERIFIED_NAME_375162; the service account carries the non-personal designation 'GSheet'. + * The deployment variable is mandatory on PRD so TypeORM cannot record a partial/no-op migration when + * it is missing. The update is idempotent (only touches a still-null verifiedName) and coupled to a + * durable before/after audit entry. Guarded to prd; a no-op elsewhere. + * @class @implements {MigrationInterface} + */ +module.exports = class BackfillStaffVerifiedNames1785584840000 { + name = 'BackfillStaffVerifiedNames1785584840000'; + + async up(queryRunner) { + if (process.env.ENVIRONMENT !== 'prd') return; + + const humanName = process.env.STAFF_VERIFIED_NAME_375162?.trim(); + if (!humanName) throw new Error('STAFF_VERIFIED_NAME_375162 is required for the PRD staff-name backfill'); + // Array.of avoids looking like MSSQL bracket quoting to the repository's migration syntax guard. + const queryParameters = Array.of(humanName); + + await queryRunner.query( + `WITH "targets" AS ( + SELECT 375162 AS "id", $1::varchar AS "nextVerifiedName" + UNION ALL + SELECT "userDataId", 'GSheet'::varchar + FROM "user" + WHERE address = '0x791D0AeC86EE6a86d260543ECD57d7932A7fec2D' + ), + "affected" AS ( + SELECT ud."id", ud."verifiedName" AS "previousVerifiedName", t."nextVerifiedName" + FROM "user_data" ud + JOIN "targets" t ON t."id" = ud."id" + WHERE ud."verifiedName" IS NULL + FOR UPDATE OF ud + ), + "audit" AS ( + INSERT INTO "log" ("created", "updated", "system", "subsystem", "severity", "message") + SELECT now(), now(), 'User', 'StaffVerifiedNameBackfill', 'Info', + json_agg(json_build_object( + 'userDataId', "id", + 'previousVerifiedName', "previousVerifiedName", + 'nextVerifiedName', "nextVerifiedName" + ) ORDER BY "id")::text + FROM "affected" + HAVING count(*) > 0 + RETURNING 1 + ) + UPDATE "user_data" ud + SET "verifiedName" = a."nextVerifiedName", "updated" = now() + FROM "affected" a + WHERE ud."id" = a."id" AND EXISTS (SELECT 1 FROM "audit")`, + queryParameters, + ); + + const [{ humanCount, serviceCount }] = await queryRunner.query( + `SELECT + (SELECT count(*)::int FROM "user_data" + WHERE "id" = 375162 AND "verifiedName" = $1) AS "humanCount", + (SELECT count(*)::int + FROM "user" u + JOIN "user_data" ud ON ud."id" = u."userDataId" + WHERE u."address" = '0x791D0AeC86EE6a86d260543ECD57d7932A7fec2D' + AND ud."verifiedName" = 'GSheet') AS "serviceCount"`, + queryParameters, + ); + + if (Number(humanCount) !== 1 || Number(serviceCount) !== 1) { + throw new Error('PRD staff-name backfill did not reach the required state for both target accounts'); + } + } + + async down() { + // No-op: a granted clearance is not auto-revoked here; removal requires a separate reviewed, + // audited revocation so an unrelated rollback cannot silently erase an identity/operator grant. + } +}; diff --git a/src/shared/auth/role.guard.ts b/src/shared/auth/role.guard.ts index cd8dfe0ce4..cc77a9e99c 100644 --- a/src/shared/auth/role.guard.ts +++ b/src/shared/auth/role.guard.ts @@ -91,10 +91,11 @@ class RoleGuardClass implements CanActivate { const user = context.switchToHttp().getRequest().user; if (!this.entryRoles.some((entryRole) => hasRoleAccess(entryRole, user?.role))) return false; - // Elevated endpoint: an identified natural person must be behind the account (see KycGatedRoles). - // The gate is a property of the ENDPOINT, not of the caller, so it applies only when EVERY entry - // role is gated — a gate that also admits e.g. UserRole.USER is an ordinary endpoint that an admin - // happens to reach through the role hierarchy, and must not start demanding staff KYC. + // Elevated endpoint: the account must carry an identity-verified name or an operator-reviewed + // service designation (see KycGatedRoles). The gate is a property of the ENDPOINT, not of the + // caller, so it applies only when EVERY entry role is gated — a gate that also admits e.g. + // UserRole.USER is an ordinary endpoint that an admin happens to reach through the role hierarchy, + // and must not start demanding staff KYC. // // Throws rather than returning false: a bare false becomes the generic "Forbidden resource", which // a caller cannot tell apart from a removed role, so neither staff nor tooling would learn that the diff --git a/src/shared/auth/staff-kyc-clearance.ts b/src/shared/auth/staff-kyc-clearance.ts index 6eb635c6fe..7f7bcf7028 100644 --- a/src/shared/auth/staff-kyc-clearance.ts +++ b/src/shared/auth/staff-kyc-clearance.ts @@ -1,8 +1,9 @@ // Staff KYC clearance ALLOWlist — the inverse of the JWT denylists in ProcessService. Elevated // endpoints (every `RoleGuard` whose entry roles are all in `KycGatedRoles`) require, on top of the -// role, that an identified natural person is behind the calling account: a non-empty `verifiedName`. -// That name is only ever set by an identity-verified path or a reviewed migration, never self-service, -// so it is the authoritative identification signal on its own — no KYC level is required. +// role, a non-empty `verifiedName`. For personal accounts this is an identity-verified natural-person +// name; an operator-reviewed service account may instead carry a non-personal designation. The value +// is only ever set by an identity-verified path or a reviewed migration, never self-service, so it is +// the authoritative clearance signal on its own — no KYC level is required. // `StaffKycClearanceService` derives the cleared account (user data) ids from the DB into the // `staffKycClearance` setting; `ProcessService` primes this Set from it, so revoking a staff member's // clearance takes effect on live tokens within one refresh interval — no re-login, no JWT-secret rotation. diff --git a/src/shared/auth/user-role.enum.ts b/src/shared/auth/user-role.enum.ts index 23de152039..23e1569ead 100644 --- a/src/shared/auth/user-role.enum.ts +++ b/src/shared/auth/user-role.enum.ts @@ -26,11 +26,12 @@ export enum UserRole { // Priority-ordered (highest privilege first) for mail-login role resolution. export const StaffRoles = [UserRole.COMPLIANCE, UserRole.SUPPORT, UserRole.REALUNIT]; -// Entry roles that mark an endpoint as elevated: reaching it requires an identified natural person -// behind the account, on top of the role itself. `RoleGuard` therefore demands staff KYC clearance -// (a non-empty `verifiedName`, see `HasStaffKycClearance`) whenever every entry role of a gate is -// listed here. Distinct from `StaffRoles` above, which is about mail-login -// role resolution — this list is about endpoint sensitivity and also covers ADMIN and DEBUG. +// Entry roles that mark an endpoint as elevated: reaching it requires an identity-verified personal +// name or an operator-reviewed service designation on the account, on top of the role itself. +// `RoleGuard` therefore demands staff KYC clearance (a non-empty `verifiedName`, see +// `HasStaffKycClearance`) whenever every entry role of a gate is listed here. Distinct from +// `StaffRoles` above, which is about mail-login role resolution — this list is about endpoint +// sensitivity and also covers ADMIN and DEBUG. // // Not listed, deliberately: BANKING_BOT and CUSTODY are non-staff entry roles and stay ungated, so a // cleared-role holder reaching those endpoints via the `additionalRoles` hierarchy is not KYC-gated. diff --git a/src/subdomains/generic/user/models/user/__tests__/backfill-staff-verified-names.migration.spec.ts b/src/subdomains/generic/user/models/user/__tests__/backfill-staff-verified-names.migration.spec.ts new file mode 100644 index 0000000000..2f77e7e6e4 --- /dev/null +++ b/src/subdomains/generic/user/models/user/__tests__/backfill-staff-verified-names.migration.spec.ts @@ -0,0 +1,275 @@ +import { DataSource, QueryRunner } from 'typeorm'; + +const PG_URL = process.env.MIGRATION_TEST_PG; +const describeDb = PG_URL ? describe : describe.skip; +const SCHEMA = 'backfill_staff_verified_names_spec'; +const STAFF_NAME_ENV = 'STAFF_VERIFIED_NAME_375162'; +const GSHEET_ADDRESS = '0x791D0AeC86EE6a86d260543ECD57d7932A7fec2D'; + +let BackfillStaffVerifiedNames: new () => { + up(queryRunner: QueryRunner): Promise; + down(): Promise; +}; + +function setEnv(name: string, value: string | undefined): void { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; +} + +describe('BackfillStaffVerifiedNames migration (SQL content)', () => { + const originalEnvironment = process.env.ENVIRONMENT; + const originalStaffName = process.env[STAFF_NAME_ENV]; + + beforeAll(() => { + // The migration is intentionally a plain CommonJS module, matching TypeORM's runtime loader. + // eslint-disable-next-line @typescript-eslint/no-require-imports + BackfillStaffVerifiedNames = require('../../../../../../../migration/1785584840000-BackfillStaffVerifiedNames'); + }); + + afterEach(() => { + setEnv('ENVIRONMENT', originalEnvironment); + setEnv(STAFF_NAME_ENV, originalStaffName); + }); + + it.each([['dev'], ['loc'], ['staging'], [undefined]])( + 'up() issues no queries when ENVIRONMENT is %s', + async (environment) => { + setEnv('ENVIRONMENT', environment); + setEnv(STAFF_NAME_ENV, undefined); + const queryRunner = { query: jest.fn(async (_sql: string) => []) }; + + await new BackfillStaffVerifiedNames().up(queryRunner as unknown as QueryRunner); + + expect(queryRunner.query).not.toHaveBeenCalled(); + }, + ); + + it.each([[undefined], [''], [' ']])( + 'fails before issuing SQL when the PRD deployment variable is %p', + async (staffName) => { + process.env.ENVIRONMENT = 'prd'; + setEnv(STAFF_NAME_ENV, staffName); + const queryRunner = { query: jest.fn(async (_sql: string) => []) }; + + await expect(new BackfillStaffVerifiedNames().up(queryRunner as unknown as QueryRunner)).rejects.toThrow( + `${STAFF_NAME_ENV} is required`, + ); + expect(queryRunner.query).not.toHaveBeenCalled(); + }, + ); + + it('issues one parameterized, audited update on PRD', async () => { + process.env.ENVIRONMENT = 'prd'; + process.env[STAFF_NAME_ENV] = ' Test Staff Name '; + const queryRunner = { + query: jest + .fn() + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ humanCount: 1, serviceCount: 1 }]), + }; + + await new BackfillStaffVerifiedNames().up(queryRunner as unknown as QueryRunner); + + expect(queryRunner.query).toHaveBeenCalledTimes(2); + const [sql, parameters] = queryRunner.query.mock.calls[0]; + expect(parameters).toEqual(['Test Staff Name']); + expect(sql).toContain('INSERT INTO "log"'); + expect(sql).toContain("'StaffVerifiedNameBackfill'"); + expect(sql).toContain("'previousVerifiedName'"); + expect(sql).toContain("'nextVerifiedName'"); + expect(sql).toContain('FOR UPDATE OF ud'); + expect(sql).toContain('EXISTS (SELECT 1 FROM "audit")'); + expect(sql).toContain('SET "verifiedName" = a."nextVerifiedName", "updated" = now()'); + expect(sql).toContain(GSHEET_ADDRESS); + expect(sql).not.toContain('Test Staff Name'); + + const [postconditionSql, postconditionParameters] = queryRunner.query.mock.calls[1]; + expect(postconditionParameters).toEqual(['Test Staff Name']); + expect(postconditionSql).toContain('AS "humanCount"'); + expect(postconditionSql).toContain('AS "serviceCount"'); + expect(parameters).toBe(postconditionParameters); + }); + + it('rejects when either target does not reach the exact required state', async () => { + process.env.ENVIRONMENT = 'prd'; + process.env[STAFF_NAME_ENV] = 'Test Staff Name'; + const queryRunner = { + query: jest + .fn() + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ humanCount: 1, serviceCount: 0 }]), + }; + + await expect(new BackfillStaffVerifiedNames().up(queryRunner as unknown as QueryRunner)).rejects.toThrow( + 'did not reach the required state for both target accounts', + ); + }); + + it('down() deliberately performs no rollback', async () => { + const migration = new BackfillStaffVerifiedNames(); + + expect(migration.down).toHaveLength(0); + await expect(migration.down()).resolves.toBeUndefined(); + }); +}); + +describeDb('BackfillStaffVerifiedNames migration (real Postgres)', () => { + const originalEnvironment = process.env.ENVIRONMENT; + const originalStaffName = process.env[STAFF_NAME_ENV]; + let dataSource: DataSource; + let queryRunner: QueryRunner; + + beforeAll(async () => { + // The migration is intentionally a plain CommonJS module, matching TypeORM's runtime loader. + // eslint-disable-next-line @typescript-eslint/no-require-imports + BackfillStaffVerifiedNames = require('../../../../../../../migration/1785584840000-BackfillStaffVerifiedNames'); + dataSource = new DataSource({ type: 'postgres', url: PG_URL }); + await dataSource.initialize(); + }); + + beforeEach(async () => { + process.env.ENVIRONMENT = 'prd'; + process.env[STAFF_NAME_ENV] = 'Test Staff Name'; + queryRunner = dataSource.createQueryRunner(); + await queryRunner.connect(); + await queryRunner.query(`DROP SCHEMA IF EXISTS "${SCHEMA}" CASCADE`); + await queryRunner.query(`CREATE SCHEMA "${SCHEMA}"`); + await queryRunner.query(`SET search_path TO "${SCHEMA}"`); + await queryRunner.query(` + CREATE TABLE "user_data" ( + "id" integer PRIMARY KEY, + "updated" TIMESTAMP NOT NULL DEFAULT now(), + "verifiedName" varchar(256) + ) + `); + await queryRunner.query(` + CREATE TABLE "user" ( + "id" SERIAL PRIMARY KEY, + "address" varchar(256), + "userDataId" integer REFERENCES "user_data"("id") + ) + `); + await queryRunner.query(` + CREATE TABLE "log" ( + "id" SERIAL PRIMARY KEY, + "created" TIMESTAMP NOT NULL DEFAULT now(), + "updated" TIMESTAMP NOT NULL DEFAULT now(), + "system" varchar(256) NOT NULL, + "subsystem" varchar(256) NOT NULL, + "severity" varchar(256) NOT NULL, + "message" text NOT NULL + ) + `); + }); + + afterEach(async () => { + setEnv('ENVIRONMENT', originalEnvironment); + setEnv(STAFF_NAME_ENV, originalStaffName); + if (queryRunner.isTransactionActive) await queryRunner.rollbackTransaction(); + await queryRunner.query(`SET search_path TO public`); + await queryRunner.query(`DROP SCHEMA IF EXISTS "${SCHEMA}" CASCADE`); + await queryRunner.release(); + }); + + afterAll(async () => { + if (dataSource?.isInitialized) await dataSource.destroy(); + }); + + async function insertTargets(humanName: string | null = null, serviceName: string | null = null): Promise { + await queryRunner.query( + `INSERT INTO "user_data" ("id", "updated", "verifiedName") + VALUES (375162, TIMESTAMP '2000-01-01', $1), (318765, TIMESTAMP '2000-01-01', $2)`, + [humanName, serviceName], + ); + await queryRunner.query(`INSERT INTO "user" ("address", "userDataId") VALUES ($1, 318765)`, [GSHEET_ADDRESS]); + } + + it('backfills both targets, updates their timestamps, and records one before/after audit row', async () => { + await insertTargets(); + + await new BackfillStaffVerifiedNames().up(queryRunner); + + const users = (await queryRunner.query( + `SELECT "id", "verifiedName", "updated" > TIMESTAMP '2000-01-01' AS "wasUpdated" + FROM "user_data" ORDER BY "id"`, + )) as { id: number; verifiedName: string | null; wasUpdated: boolean }[]; + expect(users).toEqual([ + { id: 318765, verifiedName: 'GSheet', wasUpdated: true }, + { id: 375162, verifiedName: 'Test Staff Name', wasUpdated: true }, + ]); + + const logs = (await queryRunner.query( + `SELECT "message" FROM "log" WHERE "system" = 'User' AND "subsystem" = 'StaffVerifiedNameBackfill'`, + )) as { message: string }[]; + expect(logs).toHaveLength(1); + expect(JSON.parse(logs[0].message)).toEqual([ + { userDataId: 318765, previousVerifiedName: null, nextVerifiedName: 'GSheet' }, + { userDataId: 375162, previousVerifiedName: null, nextVerifiedName: 'Test Staff Name' }, + ]); + }); + + it('is idempotent and does not append another audit row on a second run', async () => { + await insertTargets(); + const migration = new BackfillStaffVerifiedNames(); + + await migration.up(queryRunner); + await migration.up(queryRunner); + + const logCount = (await queryRunner.query(`SELECT count(*)::int AS "count" FROM "log"`)) as { + count: number; + }[]; + expect(logCount[0].count).toBe(1); + }); + + it('rejects an unexpected existing name and relies on the migration transaction to roll back', async () => { + await insertTargets('Existing Staff Name'); + await queryRunner.startTransaction(); + + await expect(new BackfillStaffVerifiedNames().up(queryRunner)).rejects.toThrow( + 'did not reach the required state for both target accounts', + ); + await queryRunner.rollbackTransaction(); + + const users = (await queryRunner.query(`SELECT "id", "verifiedName" FROM "user_data" ORDER BY "id"`)) as { + id: number; + verifiedName: string | null; + }[]; + expect(users).toEqual([ + { id: 318765, verifiedName: null }, + { id: 375162, verifiedName: 'Existing Staff Name' }, + ]); + + const logs = (await queryRunner.query(`SELECT "message" FROM "log"`)) as { message: string }[]; + expect(logs).toHaveLength(0); + }); + + it('changes nothing when a trigger suppresses the audit insert', async () => { + await insertTargets(); + await queryRunner.query(` + CREATE FUNCTION suppress_log_insert() RETURNS trigger AS $fn$ + BEGIN + RETURN NULL; + END; + $fn$ LANGUAGE plpgsql + `); + await queryRunner.query(` + CREATE TRIGGER suppress_log_insert_trigger + BEFORE INSERT ON "log" + FOR EACH ROW + EXECUTE FUNCTION suppress_log_insert() + `); + + await expect(new BackfillStaffVerifiedNames().up(queryRunner)).rejects.toThrow( + 'did not reach the required state for both target accounts', + ); + + const users = (await queryRunner.query(`SELECT "verifiedName" FROM "user_data" ORDER BY "id"`)) as { + verifiedName: string | null; + }[]; + expect(users).toEqual([{ verifiedName: null }, { verifiedName: null }]); + const logCount = (await queryRunner.query(`SELECT count(*)::int AS "count" FROM "log"`)) as { + count: number; + }[]; + expect(logCount[0].count).toBe(0); + }); +}); diff --git a/src/subdomains/generic/user/models/user/staff-kyc-clearance.service.ts b/src/subdomains/generic/user/models/user/staff-kyc-clearance.service.ts index 53a3929d09..6aec42ed42 100644 --- a/src/subdomains/generic/user/models/user/staff-kyc-clearance.service.ts +++ b/src/subdomains/generic/user/models/user/staff-kyc-clearance.service.ts @@ -60,9 +60,10 @@ export class StaffKycClearanceService { userData: { // A non-empty verified name is the sole clearance condition: it is only ever set by an // identity-verified path or a reviewed migration, never self-service (see the write paths of - // `verifiedName`), so it is the authoritative identification signal on its own. A KYC level is - // deliberately NOT required — it is unreachable for the DEBUG role and impossible for the - // service accounts that legitimately hold a gated role. + // `verifiedName`). Personal accounts carry an identity-verified name; operator-reviewed service + // accounts carry a non-personal designation. A KYC level is deliberately NOT required — it is + // unreachable for the DEBUG role and impossible for service accounts that legitimately hold a + // gated role. // // `verifiedName IS NOT NULL` is the stated rule, but an empty or blank name carries no // identification either — the predicate covers both, and NULL drops out on its own because the