diff --git a/src/shared/auth/__tests__/role.guard.spec.ts b/src/shared/auth/__tests__/role.guard.spec.ts index 5f4637084b..d681634071 100644 --- a/src/shared/auth/__tests__/role.guard.spec.ts +++ b/src/shared/auth/__tests__/role.guard.spec.ts @@ -264,7 +264,7 @@ describe('RoleGuard (staff KYC gate on elevated endpoints)', () => { expect(thrown.getStatus()).toBe(HttpStatus.FORBIDDEN); expect(thrown.getResponse()).toEqual({ code: 'STAFF_KYC_REQUIRED', - message: expect.stringContaining('KYC level 50'), + message: expect.stringContaining('verified name'), }); }); diff --git a/src/shared/auth/exceptions/staff-kyc-required.exception.ts b/src/shared/auth/exceptions/staff-kyc-required.exception.ts index c93d96ae5a..30b9d3aaa3 100644 --- a/src/shared/auth/exceptions/staff-kyc-required.exception.ts +++ b/src/shared/auth/exceptions/staff-kyc-required.exception.ts @@ -3,13 +3,13 @@ import { ForbiddenException } from '@nestjs/common'; // Answer for a staff caller who holds the role but is not KYC-cleared (see `HasStaffKycClearance`). // A bare `false` from the guard would produce the generic "Forbidden resource", which is // indistinguishable from "your role was removed" — leaving the caller, and any tooling in front of -// it, with no way to tell that the fix is to complete an identification. The machine-readable `code` -// follows the existing `TFA_REQUIRED` pattern so clients can branch on it instead of matching text. +// it, with no way to tell that the fix is to have a verified name on the account. The machine-readable +// `code` follows the existing `TFA_REQUIRED` pattern so clients can branch on it instead of matching text. export class StaffKycRequiredException extends ForbiddenException { constructor() { super({ code: 'STAFF_KYC_REQUIRED', - message: 'Staff access requires a completed identification: KYC level 50 and a verified name on your account', + message: 'Staff access requires a verified name on your account', }); } } diff --git a/src/shared/auth/staff-kyc-clearance.ts b/src/shared/auth/staff-kyc-clearance.ts index 69275ff88c..6eb635c6fe 100644 --- a/src/shared/auth/staff-kyc-clearance.ts +++ b/src/shared/auth/staff-kyc-clearance.ts @@ -1,10 +1,11 @@ // 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: `kycLevel >= LEVEL_50` AND a -// non-empty `verifiedName`. `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 KYC takes effect on live tokens within one refresh interval — no -// re-login, no JWT-secret rotation. +// 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. +// `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. // // Fail-CLOSED, unlike the denylists: a not-yet-primed or empty Set denies every elevated endpoint. // That asymmetry is deliberate — a DB or cron outage must never silently re-open admin access — and diff --git a/src/shared/auth/user-role.enum.ts b/src/shared/auth/user-role.enum.ts index 3fb67687a2..23de152039 100644 --- a/src/shared/auth/user-role.enum.ts +++ b/src/shared/auth/user-role.enum.ts @@ -28,8 +28,8 @@ export const StaffRoles = [UserRole.COMPLIANCE, UserRole.SUPPORT, UserRole.REALU // 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 -// (`kycLevel >= LEVEL_50` AND 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 +// (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 diff --git a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts index c0e621cfa7..6fd243214d 100644 --- a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts +++ b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts @@ -208,7 +208,7 @@ describe('KycService getFileByUid protected-file access', () => { expect(error.getResponse()).toEqual({ code: 'STAFF_KYC_REQUIRED', - message: expect.stringContaining('KYC level 50'), + message: expect.stringContaining('verified name'), }); }); diff --git a/src/subdomains/generic/user/models/user/__tests__/staff-kyc-clearance.service.spec.ts b/src/subdomains/generic/user/models/user/__tests__/staff-kyc-clearance.service.spec.ts index 6b95fabb01..56685ef023 100644 --- a/src/subdomains/generic/user/models/user/__tests__/staff-kyc-clearance.service.spec.ts +++ b/src/subdomains/generic/user/models/user/__tests__/staff-kyc-clearance.service.spec.ts @@ -12,7 +12,7 @@ describe('StaffKycClearanceService', () => { jest.spyOn(userRepo, 'find').mockResolvedValue(users as never); } - // Rows as the DB returns them: the role / kycLevel / verifiedName filtering has already happened in + // Rows as the DB returns them: the role / verifiedName filtering has already happened in // SQL, so a returned row is by definition a cleared one. function staffUser(accountId: number): unknown { return { id: accountId * 10, userData: { id: accountId } }; @@ -83,7 +83,7 @@ describe('StaffKycClearanceService', () => { expect(settingService.setObj).toHaveBeenCalledWith('staffKycClearance', []); }); - it('queries only staff roles and kycLevel >= 50', async () => { + it('queries only staff roles and requires a verified name, with no KYC-level condition', async () => { setup([]); await service.syncStaffKycClearance(); @@ -91,7 +91,9 @@ describe('StaffKycClearanceService', () => { const where = (userRepo.find as jest.Mock).mock.calls[0][0].where; expect(where.role._value).toEqual(expect.arrayContaining(['Admin', 'SuperAdmin', 'Debug', 'RealUnit'])); expect(where.role._value).not.toContain('User'); - expect(where.userData.kycLevel._value).toBe(50); + // Clearance no longer depends on a KYC level — a verified name is the sole identification condition. + expect(where.userData.kycLevel).toBeUndefined(); + expect(where.userData.verifiedName.type).toBe('raw'); }); it('does not swallow a repository failure — a failed sync must keep the last known Set', async () => { 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 new file mode 100644 index 0000000000..2fd4e0c017 --- /dev/null +++ b/src/subdomains/generic/user/models/user/__tests__/user.service.spec.ts @@ -0,0 +1,69 @@ +import { ForbiddenException } from '@nestjs/common'; +import { UserRole } from 'src/shared/auth/user-role.enum'; +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. +describe('UserService — elevated role assignment guard', () => { + let service: UserService; + let userRepo: { findOne: jest.Mock; save: jest.Mock }; + + function userWith(verifiedName: string | null | undefined, loaded = true): any { + const userData = { id: 7, verifiedName }; + return { 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. + service = new UserService( + userRepo as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + ); + }); + + it('rejects a gated role when the account has no verified name', async () => { + await expect(service.updateUserInternal(userWith(null), { role: UserRole.DEBUG })).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( + ForbiddenException, + ); + expect(userRepo.save).not.toHaveBeenCalled(); + }); + + it('allows a gated role when a verified name is present', async () => { + await service.updateUserInternal(userWith('Jane Doe'), { role: UserRole.SUPPORT }); + expect(userRepo.save).toHaveBeenCalledTimes(1); + }); + + 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' } }); + + await service.updateUserInternal(userWith(undefined, false), { role: UserRole.DEBUG }); + + expect(userRepo.findOne).toHaveBeenCalledWith({ where: { id: 42 }, relations: { userData: true } }); + expect(userRepo.save).toHaveBeenCalledTimes(1); + }); + + it('does not gate a non-elevated role even without a verified name', async () => { + await service.updateUserInternal(userWith(null), { role: UserRole.USER }); + expect(userRepo.save).toHaveBeenCalledTimes(1); + }); +}); 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 2a3aa9feb6..c6a1c9ad7d 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 @@ -4,8 +4,7 @@ import { rolesSatisfying } from 'src/shared/auth/role.guard'; import { KycGatedRoles } from 'src/shared/auth/user-role.enum'; import { SettingService } from 'src/shared/models/setting/setting.service'; import { DfxCron } from 'src/shared/utils/cron'; -import { In, MoreThanOrEqual, Raw } from 'typeorm'; -import { KycLevel } from '../user-data/user-data.enum'; +import { In, Raw } from 'typeorm'; import { UserRepository } from './user.repository'; // Roles that can reach a KYC-gated endpoint: the gated entry roles plus their super-roles (e.g. @@ -39,8 +38,8 @@ export function nonBlankPredicate(alias: string): string { // // Mirrors JwtRevocationSyncService: the cron lives in the user domain (which owns User/UserData) to // keep the shared ProcessService/SettingService free of subdomain dependencies. Self-healing in both -// directions — losing kycLevel, losing verifiedName, or losing the staff role drops the account out of -// the query and thus out of the setting on the next run. +// directions — losing the verified name or losing the staff role drops the account out of the query +// and thus out of the setting on the next run. @Injectable() export class StaffKycClearanceService { constructor( @@ -57,7 +56,12 @@ export class StaffKycClearanceService { where: { role: In(ClearanceRelevantRoles), userData: { - kycLevel: MoreThanOrEqual(KycLevel.LEVEL_50), + // 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 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 // comparison yields NULL. See BlankChars for why the character set is explicit. diff --git a/src/subdomains/generic/user/models/user/user.service.ts b/src/subdomains/generic/user/models/user/user.service.ts index 9200a9a2c7..20bd975106 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 { UserRole } from 'src/shared/auth/user-role.enum'; +import { KycGatedRoles, 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'; @@ -471,6 +471,20 @@ 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)) { + const userData = + user.userData ?? + (await this.userRepo.findOne({ where: { id: user.id }, relations: { userData: true } }))?.userData; + if (!userData?.verifiedName?.trim()) + throw new ForbiddenException('Cannot assign an elevated role to an account without a verified name'); + } + if (update.status && update.status === UserStatus.ACTIVE && user.status === UserStatus.NA) await this.activateUser(user, user.userData);