Release: develop -> main

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_<id>`), 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.

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.
+  }
+};

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('<html><body>gateway error</body></html>');
+        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(

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(/^\/+/, '')}`;
   }

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

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.

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.