diff --git a/README.md b/README.md index a485b78a..03425087 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ NestJS PostgreSQL Apache 2.0 License - 2222 Tests Passing 12 AI Agents + 2237 Tests Passing 12 AI Agents

@@ -510,7 +510,7 @@ Operator notes for activating existing adapters, metasearch landings on the dire | OTA Channels | Booking.com + Expedia (EQC) + SiteMinder + DerbySoft | Direct + aggregated OTA connectivity (ARI + content) | | XML Processing | fast-xml-parser | Booking.com OTA XML protocol | | Package Manager | pnpm workspaces | Monorepo management | -| Testing | Vitest (2222 passing tests across 266 files with passing tests) | Unit and integration tests | +| Testing | Vitest (2237 passing tests across 267 files with passing tests) | Unit and integration tests | | Build | tsup (packages) + Vite (dashboard) + nest build (API) | Fast builds | | Containers | Docker + docker-compose | Local dev and production deployment | | CI/CD | GitHub Actions | Automated testing, builds, and releases | @@ -648,7 +648,7 @@ Before going live, verify the items in [`docs/deployment.md`](./docs/deployment. ### Run tests ```bash -# Passing-test count: 2222 test cases across 266 files (skipped excluded) +# Passing-test count: 2237 test cases across 267 files (skipped excluded) # API tests only pnpm --filter @telivityhaip/api test @@ -1197,7 +1197,7 @@ HAIP is built in public and contributions are welcome. pnpm install # Install dependencies pnpm build # Build all workspace packages pnpm dev # Start API in dev mode (hot reload) -pnpm test # Run all tests (2222 passing, 266 files with passes; skipped excluded) +pnpm test # Run all tests (2237 passing, 267 files with passes; skipped excluded) pnpm lint # ESLint ``` diff --git a/apps/api/src/modules/accounting/accounting.controller.ts b/apps/api/src/modules/accounting/accounting.controller.ts index c629cd07..65023ab1 100644 --- a/apps/api/src/modules/accounting/accounting.controller.ts +++ b/apps/api/src/modules/accounting/accounting.controller.ts @@ -9,7 +9,7 @@ import { ParseUUIDPipe, } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse, ApiQuery } from '@nestjs/swagger'; -import { Roles } from '../auth/roles.decorator'; +import { RequirePermissions } from '../auth/permissions.decorator'; import { DepositService } from './deposit.service'; import { ArService } from './ar.service'; import { AccountingCodeService } from './accounting-code.service'; @@ -44,7 +44,7 @@ export class AccountingController { // --- Deposit Ledger (KB 10) --- @Post('deposits') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Record an advance deposit (held liability)' }) @ApiResponse({ status: 201, description: 'Deposit recorded' }) recordDeposit(@Body() dto: RecordDepositDto) { @@ -71,7 +71,7 @@ export class AccountingController { } @Post('deposits/:id/apply') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Apply a held deposit to a folio (KB 10.3)' }) @ApiResponse({ status: 200, description: 'Deposit applied' }) applyDeposit( @@ -82,7 +82,7 @@ export class AccountingController { } @Post('deposits/:id/refund') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Refund a held refundable deposit (KB 10.4)' }) @ApiResponse({ status: 200, description: 'Deposit refunded' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -94,7 +94,7 @@ export class AccountingController { } @Post('deposits/:id/forfeit') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Forfeit a held deposit as earned revenue (KB 10.4)' }) @ApiResponse({ status: 200, description: 'Deposit forfeited' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -108,7 +108,7 @@ export class AccountingController { // --- Accounts Receivable (KB 11) --- @Post('ar/ledgers') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Create an A/R ledger' }) @ApiResponse({ status: 201, description: 'A/R ledger created' }) createArLedger(@Body() dto: CreateArLedgerDto) { @@ -135,7 +135,7 @@ export class AccountingController { } @Patch('ar/ledgers/:id') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Update an A/R ledger' }) @ApiResponse({ status: 200, description: 'A/R ledger updated' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -148,7 +148,7 @@ export class AccountingController { } @Post('ar/ledgers/:id/close') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Close an A/R ledger (KB 11.2)' }) @ApiResponse({ status: 200, description: 'A/R ledger closed' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -171,7 +171,7 @@ export class AccountingController { } @Post('ar/transfer') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Transfer an outstanding folio balance into an A/R ledger (KB 11.3)' }) @ApiResponse({ status: 201, description: 'Balance transferred to A/R' }) transferToAr(@Body() dto: TransferToArDto) { @@ -179,7 +179,7 @@ export class AccountingController { } @Post('ar/transactions/:id/reverse') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Reverse an A/R transfer (KB 11.4)' }) @ApiResponse({ status: 200, description: 'Transfer reversed' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -191,7 +191,7 @@ export class AccountingController { } @Post('ar/ledgers/:id/payments') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Record a payment against an A/R ledger (KB 11.5)' }) @ApiResponse({ status: 201, description: 'A/R payment recorded' }) recordArPayment( @@ -223,7 +223,7 @@ export class AccountingController { // --- Custom Accounting / GL Codes (KB 5) --- @Post('accounting/codes') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Create a custom accounting / GL code' }) @ApiResponse({ status: 201, description: 'Accounting code created' }) createAccountingCode(@Body() dto: CreateAccountingCodeDto) { @@ -250,7 +250,7 @@ export class AccountingController { } @Patch('accounting/codes/:id') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Update an accounting code' }) @ApiResponse({ status: 200, description: 'Accounting code updated' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -263,7 +263,7 @@ export class AccountingController { } @Post('accounting/codes/:id/archive') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('accounting.manage') @ApiOperation({ summary: 'Archive an accounting code (soft delete)' }) @ApiResponse({ status: 200, description: 'Accounting code archived' }) @ApiQuery({ name: 'propertyId', type: String }) diff --git a/apps/api/src/modules/auth/billing-role-controllers-migration.spec.ts b/apps/api/src/modules/auth/billing-role-controllers-migration.spec.ts new file mode 100644 index 00000000..4b37fbc9 --- /dev/null +++ b/apps/api/src/modules/auth/billing-role-controllers-migration.spec.ts @@ -0,0 +1,63 @@ +import { describe, it, expect } from 'vitest'; +import { Reflector } from '@nestjs/core'; +import { ROLES_KEY } from './roles.decorator'; +import { PERMISSIONS_KEY } from './permissions.decorator'; +import { PaymentController } from '../payment/payment.controller'; +import { FolioController } from '../folio/folio.controller'; +import { NotificationsController } from '../notifications/notifications.controller'; +import { HouseAccountController } from '../house-account/house-account.controller'; +import { CashierController } from '../cashier/cashier.controller'; +import { AccountingController } from '../accounting/accounting.controller'; + +/** + * The six billing-adjacent controllers left behind by #340, which migrated + * eighteen others off the legacy @Roles() decorator. These were deferred + * because they gated every route -- read and write alike -- with one identical + * realm-role list, so no existing permission key matched it without either + * widening or narrowing who could record a payment or post a charge. + * + * The rule applied here is to PRESERVE the access the realm-role gate was + * granting in practice: reservations, night_auditor and accounting keep the + * billing-write ability the old list already gave them, and the matching + * ROLE_DEFAULT_PERMISSIONS entries plus migration + * 0023_billing_write_access_grants.sql backfill the keys for properties that + * already have RBAC rows. + * + * One side effect worth disclosing rather than burying: communications.manage + * (notifications.controller.ts) is already held by revenue_manager for an + * unrelated existing route (groups.controller.ts), so reusing that key here + * also lets revenue_manager send guest SMS/WhatsApp/Telegram messages, which + * the old gate did not. Reusing the existing key seemed better than inventing + * a near-duplicate one, but say the word and it can be split. + */ +const reflector = new Reflector(); + +// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type +function rolesOf(target: Function) { + return reflector.get(ROLES_KEY, target); +} +// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type +function permsOf(target: Function) { + return reflector.get(PERMISSIONS_KEY, target); +} + +describe('billing-write routes migrated off @Roles() onto @RequirePermissions', () => { + it.each([ + [PaymentController.prototype.recordPayment, 'folios.manage'], + [PaymentController.prototype.voidPayment, 'folios.manage'], + [FolioController.prototype.createFolio, 'folios.manage'], + [FolioController.prototype.settleFolio, 'folios.manage'], + [FolioController.prototype.postCharge, 'folios.manage'], + [NotificationsController.prototype.sendSms, 'communications.manage'], + [HouseAccountController.prototype.openHouseAccount, 'houseaccounts.manage'], + [HouseAccountController.prototype.addPayment, 'houseaccounts.manage'], + [CashierController.prototype.createDrawer, 'cashier.access'], + [CashierController.prototype.closeSession, 'cashier.access'], + [AccountingController.prototype.recordDeposit, 'accounting.manage'], + [AccountingController.prototype.createArLedger, 'accounting.manage'], + [AccountingController.prototype.createAccountingCode, 'accounting.manage'], + ])('%s: no leftover @Roles, gated on @RequirePermissions(%s)', (method, expectedKey) => { + expect(rolesOf(method)).toBeUndefined(); + expect(permsOf(method)).toEqual([expectedKey]); + }); +}); diff --git a/apps/api/src/modules/auth/permissions.catalog.spec.ts b/apps/api/src/modules/auth/permissions.catalog.spec.ts index 5a9fddcc..751abbbb 100644 --- a/apps/api/src/modules/auth/permissions.catalog.spec.ts +++ b/apps/api/src/modules/auth/permissions.catalog.spec.ts @@ -55,11 +55,34 @@ describe('permissions catalog', () => { expect(ROLE_DEFAULT_PERMISSIONS.general_manager).toContain('revenue.manage'); }); - it('reservations can book but not post folios or run cashier', () => { + // Was 'reservations can book but not post folios or run cashier' -- that + // split was never real. The legacy @Roles() gate on every billing controller + // (folio / payment / cashier / house-account / accounting) already included + // 'reservations'; only the newer @RequirePermissions catalog enforced the + // split, and only by omission. Migrating those controllers preserves the + // access the realm-role gate was granting rather than narrowing it silently. + it('reservations can book and post folios/cashier -- matches what the old realm-role gate already granted', () => { expect(ROLE_DEFAULT_PERMISSIONS.reservations).toContain('reservations.write'); expect(ROLE_DEFAULT_PERMISSIONS.reservations).toContain('folios.read'); - expect(ROLE_DEFAULT_PERMISSIONS.reservations).not.toContain('folios.manage'); - expect(ROLE_DEFAULT_PERMISSIONS.reservations).not.toContain('cashier.access'); + expect(ROLE_DEFAULT_PERMISSIONS.reservations).toContain('folios.manage'); + expect(ROLE_DEFAULT_PERMISSIONS.reservations).toContain('cashier.access'); + expect(ROLE_DEFAULT_PERMISSIONS.reservations).toContain('houseaccounts.manage'); + expect(ROLE_DEFAULT_PERMISSIONS.reservations).toContain('accounting.manage'); + }); + + it('night_auditor and accounting also gained the billing-write access the old realm-role gate granted', () => { + for (const role of ['night_auditor', 'accounting']) { + expect(ROLE_DEFAULT_PERMISSIONS[role], role).toContain('folios.manage'); + expect(ROLE_DEFAULT_PERMISSIONS[role], role).toContain('houseaccounts.manage'); + expect(ROLE_DEFAULT_PERMISSIONS[role], role).toContain('accounting.manage'); + expect(ROLE_DEFAULT_PERMISSIONS[role], role).toContain('communications.manage'); + } + expect(ROLE_DEFAULT_PERMISSIONS.night_auditor).toContain('cashier.access'); + }); + + it('front_desk gained cashier.access and accounting.manage', () => { + expect(ROLE_DEFAULT_PERMISSIONS.front_desk).toContain('cashier.access'); + expect(ROLE_DEFAULT_PERMISSIONS.front_desk).toContain('accounting.manage'); }); it('revenue_manager cannot view folios (nav and list API gated)', () => { diff --git a/apps/api/src/modules/auth/permissions.catalog.ts b/apps/api/src/modules/auth/permissions.catalog.ts index fbb315d4..33897161 100644 --- a/apps/api/src/modules/auth/permissions.catalog.ts +++ b/apps/api/src/modules/auth/permissions.catalog.ts @@ -38,6 +38,7 @@ export const PERMISSIONS: readonly PermissionDef[] = [ { key: 'houseaccounts.read', label: 'View house accounts', group: 'House Accounts', navKey: '/house-accounts' }, { key: 'houseaccounts.manage', label: 'Manage house accounts', group: 'House Accounts' }, { key: 'accounting.view', label: 'Accounting & deposits', group: 'Accounting', navKey: '/accounting' }, + { key: 'accounting.manage', label: 'Post deposits, A/R ledgers & accounting codes', group: 'Accounting' }, { key: 'tax.manage', label: 'Tax profiles & rules', group: 'Tax', navKey: '/tax' }, { key: 'rateplans.read', label: 'View rate plans', group: 'Rate Plans', navKey: '/rate-plans' }, { key: 'rateplans.manage', label: 'Manage rate plans', group: 'Rate Plans' }, @@ -97,6 +98,8 @@ export const ROLE_DEFAULT_PERMISSIONS: Record = { 'reviews.manage', 'ops.read', 'ops.manage', + 'cashier.access', + 'accounting.manage', ], housekeeping: ['dashboard.view', 'rooms.read', 'housekeeping.read', 'ops.read'], housekeeping_manager: [ @@ -112,11 +115,15 @@ export const ROLE_DEFAULT_PERMISSIONS: Record = { 'dashboard.view', 'reservations.read', 'folios.read', + 'folios.manage', 'nightaudit.run', 'reports.view', 'cashier.access', 'houseaccounts.read', + 'houseaccounts.manage', 'accounting.view', + 'accounting.manage', + 'communications.manage', 'commercial.read', ], readonly: [ @@ -162,12 +169,21 @@ export const ROLE_DEFAULT_PERMISSIONS: Record = { 'houseaccounts.manage', 'cashier.access', 'accounting.view', + 'accounting.manage', 'tax.manage', 'nightaudit.run', + 'communications.manage', 'reports.view', 'commercial.read', ], - /** Booking desk — reservations without cashier / folio posting. */ + /** + * Booking desk. The doc comment used to read "reservations without cashier / + * folio posting", but the legacy @Roles() gate never enforced that split: + * `reservations` was already in the allow-list of every billing controller + * below. These keys record the access the realm-role gate was granting in + * practice, so migrating to permissions preserves behaviour rather than + * silently narrowing it. + */ reservations: [ 'dashboard.view', 'frontdesk.access', @@ -178,6 +194,10 @@ export const ROLE_DEFAULT_PERMISSIONS: Record = { 'rooms.read', 'media.manage', 'folios.read', + 'folios.manage', + 'houseaccounts.manage', + 'cashier.access', + 'accounting.manage', 'groups.read', 'groups.manage', 'commercial.read', diff --git a/apps/api/src/modules/cashier/cashier.controller.ts b/apps/api/src/modules/cashier/cashier.controller.ts index 7e14092d..81556736 100644 --- a/apps/api/src/modules/cashier/cashier.controller.ts +++ b/apps/api/src/modules/cashier/cashier.controller.ts @@ -8,7 +8,7 @@ import { ParseUUIDPipe, } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse, ApiQuery } from '@nestjs/swagger'; -import { Roles } from '../auth/roles.decorator'; +import { RequirePermissions } from '../auth/permissions.decorator'; import { CashierService } from './cashier.service'; import { CreateDrawerDto } from './dto/create-drawer.dto'; import { OpenSessionDto } from './dto/open-session.dto'; @@ -28,7 +28,7 @@ export class CashierController { } @Post('drawers') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('cashier.access') @ApiOperation({ summary: 'Create a cash drawer (KB 12.1)' }) @ApiResponse({ status: 201, description: 'Cash drawer created' }) createDrawer(@Body() dto: CreateDrawerDto) { @@ -61,7 +61,7 @@ export class CashierController { } @Post('sessions') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('cashier.access') @ApiOperation({ summary: 'Open a cash drawer session/shift (KB 12.2)' }) @ApiResponse({ status: 201, description: 'Session opened' }) openSession(@Body() dto: OpenSessionDto) { @@ -98,7 +98,7 @@ export class CashierController { } @Post('sessions/:id/movements') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('cashier.access') @ApiOperation({ summary: 'Record a cash movement (KB 12.3)' }) @ApiResponse({ status: 201, description: 'Movement recorded' }) recordMovement( @@ -109,7 +109,7 @@ export class CashierController { } @Post('sessions/:id/close') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('cashier.access') @ApiOperation({ summary: 'Close a session and compute variance (KB 12.4)' }) @ApiResponse({ status: 200, description: 'Session closed' }) closeSession( diff --git a/apps/api/src/modules/folio/folio.controller.ts b/apps/api/src/modules/folio/folio.controller.ts index 976759a0..0b225c6e 100644 --- a/apps/api/src/modules/folio/folio.controller.ts +++ b/apps/api/src/modules/folio/folio.controller.ts @@ -9,7 +9,6 @@ import { ParseUUIDPipe, } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse, ApiQuery } from '@nestjs/swagger'; -import { Roles } from '../auth/roles.decorator'; import { RequirePermissions } from '../auth/permissions.decorator'; import { FolioService } from './folio.service'; import { FolioRoutingService } from './folio-routing.service'; @@ -43,7 +42,7 @@ export class FolioController { // captured by the ':id' parameter. @Post('routing-rules') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Create a split-folio routing rule (KB 14.2)' }) @ApiResponse({ status: 201, description: 'Routing rule created' }) createRoutingRule(@Body() dto: CreateRoutingRuleDto) { @@ -63,7 +62,7 @@ export class FolioController { } @Post() - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Create a folio' }) @ApiResponse({ status: 201, description: 'Folio created' }) createFolio(@Body() dto: CreateFolioDto) { @@ -72,8 +71,10 @@ export class FolioController { @Get() // Local permission, not a Keycloak realm-role list: a property-scoped - // custom role has no realm role at all, so @Roles() can never grant it - // "view folios" no matter what it was granted locally. + // custom role (e.g. Winter Manager) has no realm role at all, so @Roles() + // could never grant it "view folios" no matter what it was granted + // locally. Every ROLE_DEFAULT_PERMISSIONS entry with folios.read, + // including readonly, is meant to reach this route now. @RequirePermissions('folios.read') @ApiOperation({ summary: 'List folios with filters' }) @ApiResponse({ status: 200, description: 'Paginated list of folios' }) @@ -94,7 +95,7 @@ export class FolioController { } @Patch(':id') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Update folio' }) @ApiResponse({ status: 200, description: 'Folio updated' }) @ApiResponse({ status: 404, description: 'Folio not found' }) @@ -108,7 +109,7 @@ export class FolioController { } @Patch(':id/settle') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Settle folio (balance must be zero)' }) @ApiResponse({ status: 200, description: 'Folio settled' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -120,7 +121,7 @@ export class FolioController { } @Patch(':id/close') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Close folio (must be settled first)' }) @ApiResponse({ status: 200, description: 'Folio closed' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -132,7 +133,7 @@ export class FolioController { } @Post(':id/charges') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Post charge to folio' }) @ApiResponse({ status: 201, description: 'Charge posted' }) postCharge( @@ -153,7 +154,7 @@ export class FolioController { } @Post(':id/charges/:chargeId/reverse') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Reverse a charge' }) @ApiResponse({ status: 200, description: 'Charge reversed' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -166,7 +167,7 @@ export class FolioController { } @Post(':id/charges/lock') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Lock charges up to audit date' }) @ApiResponse({ status: 200, description: 'Charges locked' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -179,7 +180,7 @@ export class FolioController { } @Post(':id/transfer-charge') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Transfer charge to another folio' }) @ApiResponse({ status: 200, description: 'Charge transferred' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -192,7 +193,7 @@ export class FolioController { } @Post(':id/move-transactions') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Move transactions to another folio (KB 14.2)' }) @ApiResponse({ status: 200, description: 'Transactions moved' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -210,7 +211,7 @@ export class FolioController { // --- Fiscal documents (regional tax integrations, invoice.* events) --- @Post(':id/fiscal-documents') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Request a fiscal document (invoice/tax note) for a folio — emits invoice.requested for external issuing integrations', @@ -235,7 +236,7 @@ export class FolioController { } @Post(':id/fiscal-documents/:documentId/issue') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Record the issued document reference (called by the issuing integration) — emits invoice.issued', @@ -250,7 +251,7 @@ export class FolioController { } @Post(':id/fiscal-documents/:documentId/void') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Void a fiscal document (or cancel a pending request) — emits invoice.voided' }) @ApiResponse({ status: 200, description: 'Fiscal document voided' }) voidFiscalDocument( @@ -262,7 +263,7 @@ export class FolioController { } @Post(':id/transfer-to-city-ledger') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Transfer outstanding balance to city ledger' }) @ApiResponse({ status: 200, description: 'Balance transferred to city ledger' }) @ApiQuery({ name: 'propertyId', type: String }) diff --git a/apps/api/src/modules/house-account/house-account.controller.ts b/apps/api/src/modules/house-account/house-account.controller.ts index e0732344..f98ea886 100644 --- a/apps/api/src/modules/house-account/house-account.controller.ts +++ b/apps/api/src/modules/house-account/house-account.controller.ts @@ -9,7 +9,7 @@ import { ParseUUIDPipe, } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse, ApiQuery } from '@nestjs/swagger'; -import { Roles } from '../auth/roles.decorator'; +import { RequirePermissions } from '../auth/permissions.decorator'; import { HouseAccountService } from './house-account.service'; import { OpenHouseAccountDto } from './dto/open-house-account.dto'; import { ListHouseAccountsDto } from './dto/list-house-accounts.dto'; @@ -29,7 +29,7 @@ export class HouseAccountController { // Declared before :id house-account routes so '/products' is not shadowed. @Post('products') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('houseaccounts.manage') @ApiOperation({ summary: 'Create a retail product (catalog item)' }) @ApiResponse({ status: 201, description: 'Product created' }) createProduct(@Body() dto: CreateProductDto) { @@ -56,7 +56,7 @@ export class HouseAccountController { } @Patch('products/:id') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('houseaccounts.manage') @ApiOperation({ summary: 'Update a product' }) @ApiResponse({ status: 200, description: 'Product updated' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -71,7 +71,7 @@ export class HouseAccountController { // --- House accounts (KB 13) --- @Post('house-accounts') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('houseaccounts.manage') @ApiOperation({ summary: 'Open a house account (KB 13.2)' }) @ApiResponse({ status: 201, description: 'House account opened' }) openHouseAccount(@Body() dto: OpenHouseAccountDto) { @@ -98,7 +98,7 @@ export class HouseAccountController { } @Post('house-accounts/:id/close') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('houseaccounts.manage') @ApiOperation({ summary: 'Close a house account (read-only after, KB 13.2)' }) @ApiResponse({ status: 200, description: 'House account closed' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -110,7 +110,7 @@ export class HouseAccountController { } @Post('house-accounts/:id/charges') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('houseaccounts.manage') @ApiOperation({ summary: 'Post a charge to a house account' }) @ApiResponse({ status: 201, description: 'Charge posted' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -123,7 +123,7 @@ export class HouseAccountController { } @Post('house-accounts/:id/payments') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('houseaccounts.manage') @ApiOperation({ summary: 'Record a payment on a house account' }) @ApiResponse({ status: 201, description: 'Payment recorded' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -136,7 +136,7 @@ export class HouseAccountController { } @Post('house-accounts/:id/sell') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('houseaccounts.manage') @ApiOperation({ summary: 'Sell a catalog product to a house account (KB 13.3)' }) @ApiResponse({ status: 201, description: 'Product sold; charge (and optional payment) posted' }) @ApiQuery({ name: 'propertyId', type: String }) diff --git a/apps/api/src/modules/notifications/notifications.controller.ts b/apps/api/src/modules/notifications/notifications.controller.ts index 30d08afe..bd8f947e 100644 --- a/apps/api/src/modules/notifications/notifications.controller.ts +++ b/apps/api/src/modules/notifications/notifications.controller.ts @@ -1,6 +1,6 @@ import { Controller, Post, Body } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse } from '@nestjs/swagger'; -import { Roles } from '../auth/roles.decorator'; +import { RequirePermissions } from '../auth/permissions.decorator'; import { NotificationService } from './notification.service'; import { SendSmsDto } from './dto/send-sms.dto'; import { SendWhatsAppDto } from './dto/send-whatsapp.dto'; @@ -12,7 +12,7 @@ export class NotificationsController { constructor(private readonly notificationService: NotificationService) {} @Post('sms') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('communications.manage') @ApiOperation({ summary: 'Send an SMS to a guest via the configured provider' }) @ApiResponse({ status: 201, description: 'Dispatch result (sent flag + provider + messageId/error)' }) sendSms(@Body() dto: SendSmsDto) { @@ -20,7 +20,7 @@ export class NotificationsController { } @Post('whatsapp') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('communications.manage') @ApiOperation({ summary: 'Send a WhatsApp template/body via the configured provider' }) @ApiResponse({ status: 201, description: 'Dispatch result (sent flag + provider + messageId/error)' }) sendWhatsApp(@Body() dto: SendWhatsAppDto) { @@ -40,7 +40,7 @@ export class NotificationsController { } @Post('telegram') - @Roles('admin', 'general_manager', 'front_desk', 'reservations', 'night_auditor', 'accounting') + @RequirePermissions('communications.manage') @ApiOperation({ summary: 'Send a Telegram message to a guest via the configured bot' }) @ApiResponse({ status: 201, description: 'Dispatch result (sent flag + provider + messageId/error)' }) sendTelegram(@Body() dto: SendTelegramDto) { diff --git a/apps/api/src/modules/payment/payment-legacy-seam.spec.ts b/apps/api/src/modules/payment/payment-legacy-seam.spec.ts index 7e8d70d3..877c98e7 100644 --- a/apps/api/src/modules/payment/payment-legacy-seam.spec.ts +++ b/apps/api/src/modules/payment/payment-legacy-seam.spec.ts @@ -1,6 +1,6 @@ import { Reflector } from '@nestjs/core'; import { describe, expect, it, vi } from 'vitest'; -import { ROLES_KEY } from '../auth/roles.decorator'; +import { PERMISSIONS_KEY } from '../auth/permissions.decorator'; import { PaymentController } from './payment.controller'; import { PaymentService } from './payment.service'; @@ -72,7 +72,11 @@ function serviceWith(db: ReturnType) { } describe('legacy payment HTTP seam', () => { - it('uses role guards for generic payment mutations', () => { + it('requires folios.manage for generic payment mutations', () => { + // Updated in the same change that migrates PaymentController's mutation + // routes off @Roles() onto @RequirePermissions('folios.manage'). The + // route set and who can reach it are unchanged -- only the mechanism the + // guard reads is. const reflector = new Reflector(); for (const method of [ 'recordPayment', @@ -83,9 +87,9 @@ describe('legacy payment HTTP seam', () => { 'correctPayment', ] as const) { expect(reflector.get( - ROLES_KEY, + PERMISSIONS_KEY, PaymentController.prototype[method], - )).toEqual(['admin', 'general_manager', 'front_desk', 'reservations']); + )).toEqual(['folios.manage']); } }); diff --git a/apps/api/src/modules/payment/payment.controller.ts b/apps/api/src/modules/payment/payment.controller.ts index f967e0ef..7fcf6462 100644 --- a/apps/api/src/modules/payment/payment.controller.ts +++ b/apps/api/src/modules/payment/payment.controller.ts @@ -8,7 +8,7 @@ import { ParseUUIDPipe, } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse, ApiQuery } from '@nestjs/swagger'; -import { Roles } from '../auth/roles.decorator'; +import { RequirePermissions } from '../auth/permissions.decorator'; import { PaymentService } from './payment.service'; import { CreatePaymentDto } from './dto/create-payment.dto'; import { AuthorizePaymentDto } from './dto/authorize-payment.dto'; @@ -21,7 +21,7 @@ export class PaymentController { constructor(private readonly paymentService: PaymentService) {} @Post() - @Roles('admin', 'general_manager', 'front_desk', 'reservations') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Record payment (cash, bank transfer, etc.)' }) @ApiResponse({ status: 201, description: 'Payment recorded' }) recordPayment(@Body() dto: CreatePaymentDto) { @@ -29,7 +29,7 @@ export class PaymentController { } @Post('authorize') - @Roles('admin', 'general_manager', 'front_desk', 'reservations') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Authorize card payment (pre-auth)' }) @ApiResponse({ status: 201, description: 'Payment authorized' }) authorizePayment(@Body() dto: AuthorizePaymentDto) { @@ -56,7 +56,7 @@ export class PaymentController { } @Post(':id/capture') - @Roles('admin', 'general_manager', 'front_desk', 'reservations') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Capture authorized payment' }) @ApiResponse({ status: 200, description: 'Payment captured' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -68,7 +68,7 @@ export class PaymentController { } @Post(':id/void') - @Roles('admin', 'general_manager', 'front_desk', 'reservations') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Void authorized payment' }) @ApiResponse({ status: 200, description: 'Payment voided' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -80,7 +80,7 @@ export class PaymentController { } @Post(':id/refund') - @Roles('admin', 'general_manager', 'front_desk', 'reservations') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Refund captured payment' }) @ApiResponse({ status: 200, description: 'Payment refunded' }) @ApiQuery({ name: 'propertyId', type: String }) @@ -93,7 +93,7 @@ export class PaymentController { } @Post(':id/correct') - @Roles('admin', 'general_manager', 'front_desk', 'reservations') + @RequirePermissions('folios.manage') @ApiOperation({ summary: 'Correct a payment via the void/refund/adjust matrix (KB 14.1)' }) @ApiResponse({ status: 200, description: 'Payment corrected' }) correctPayment( diff --git a/docs/test-stats.json b/docs/test-stats.json index 50e12aef..0cc555ba 100644 --- a/docs/test-stats.json +++ b/docs/test-stats.json @@ -1,7 +1,7 @@ { - "tests": 2222, - "files": 266, + "tests": 2237, + "files": 267, "scope": "all workspace packages with a test script", "semantics": "passed test cases and files containing at least one passed test; skipped test cases and skipped-only files are excluded", - "updatedAt": "2026-08-27T23:21:35.046Z" + "updatedAt": "2026-09-07T21:42:19.752Z" } diff --git a/packages/database/src/migrations/0023_billing_write_access_grants.sql b/packages/database/src/migrations/0023_billing_write_access_grants.sql new file mode 100644 index 00000000..09c9396b --- /dev/null +++ b/packages/database/src/migrations/0023_billing_write_access_grants.sql @@ -0,0 +1,55 @@ +-- The 6 billing-adjacent controllers (folio's charge/settle/close routes, +-- payment, notifications, house-account, cashier, accounting) used to gate +-- every route -- read and write alike -- with one Keycloak realm-role list: +-- admin, general_manager, front_desk, reservations, night_auditor, +-- accounting. Moving those routes onto local @RequirePermissions() (see the +-- RBAC migration continuing #340) surfaced that four of those six roles were missing +-- permission keys the old realm-role gate had been granting them all along. +-- The rule applied is to preserve what the realm-role gate was granting in +-- practice rather than narrow it silently: reservations-desk staff keep the +-- ability to record payments and post charges, and night_auditor/accounting +-- get the keys the old list already gave them. +-- +-- admin and general_manager need no grant here: admin holds every key via +-- ALL_PERMISSIONS, and general_manager holds every key except the two +-- admin.* ones, so a brand-new key like accounting.manage is automatic for +-- both the moment it exists in the code catalog -- see permissions.catalog.ts. + +WITH role_ids AS ( + SELECT key, id AS role_id + FROM roles + WHERE property_id IS NULL + AND is_system = true + AND key IN ('front_desk', 'night_auditor', 'accounting', 'reservations') +), +grants(role_key, permission_key) AS ( + VALUES + ('front_desk', 'cashier.access'), + ('front_desk', 'accounting.manage'), + ('night_auditor', 'folios.manage'), + ('night_auditor', 'houseaccounts.manage'), + ('night_auditor', 'communications.manage'), + ('night_auditor', 'accounting.manage'), + ('accounting', 'communications.manage'), + ('accounting', 'accounting.manage'), + ('reservations', 'folios.manage'), + ('reservations', 'houseaccounts.manage'), + ('reservations', 'cashier.access'), + ('reservations', 'accounting.manage') +), +resolved AS ( + SELECT r.role_id, g.permission_key + FROM grants g + JOIN role_ids r ON r.key = g.role_key +) +-- Same "every property that already has RBAC grants" cross-join as 0015 -- +-- a property with zero role_permissions rows gets nothing here either, by +-- design (it has no RBAC set up at all yet, so there is nothing to widen). +INSERT INTO role_permissions (id, property_id, role_id, permission_key) +SELECT gen_random_uuid(), prop.id, res.role_id, res.permission_key +FROM ( + SELECT DISTINCT property_id AS id + FROM role_permissions +) prop +CROSS JOIN resolved res +ON CONFLICT (property_id, role_id, permission_key) DO NOTHING;