fix(payments): complete PR #365 Redsys remediation

.env.example

@@ -87,6 +87,12 @@ STRIPE_MODE=mock
 # REDSYS_TERMINAL=001
 # REDSYS_SECRET_KEY=sq7HjrUOBfKmC576ILgskD5srU870gJ7
 # REDSYS_ENV=test
+# Exact allowed origins for the pages embedding the Redsys booking widget.
+# Required for hosted booking checkout; no wildcard. HTTPS in production.
+# BOOKING_RETURN_ORIGINS=https://hotel.example,https://www.hotel.example
+# Local development can explicitly allow http://localhost:5174.
+# Public API base shared by Redsys notifications and the compact browser-return relay.
+# Use the externally reachable HTTPS base in production; provider return URLs must fit 250 characters.
 # PUBLIC_API_BASE_URL=http://localhost:3000
 
 # Guest SMS / messaging (see docs/integrations/messaging-infobip-vonage-telegram.md)
@@ -131,7 +137,8 @@ STORAGE_DRIVER=local
 # S3_FORCE_PATH_STYLE=true                    # true for MinIO; false for AWS S3
 # S3_PUBLIC_BASE_URL=                         # optional CDN/public base for object URLs
 
-# Migration source-PMS credential vault (AES-256-GCM at rest)
+# Protected credential key ring: source-PMS migration credentials and per-property
+# Redsys signing keys (AES-256-GCM at rest). Required before saving Redsys credentials.
 # Generate: openssl rand -hex 32
 # MIGRATION_CREDENTIAL_ENCRYPTION_KEY=
 # MIGRATION_CREDENTIAL_ENCRYPTION_KEY_ID=default

.github/workflows/ci.yml

@@ -85,6 +85,7 @@ jobs:
         run: node scripts/sync-test-count.mjs --check
         env:
           DATABASE_URL: postgresql://haip:haip@localhost:5432/haip_test
+          REDSYS_TEST_DATABASE_URL: postgresql://haip:haip@localhost:5432/haip_test
           REDIS_URL: redis://localhost:6379
           FORCE_COLOR: '0'
           CI: 'true'

.github/workflows/release.yml

@@ -71,6 +71,7 @@ jobs:
         run: pnpm test
         env:
           DATABASE_URL: postgresql://haip:haip@localhost:5432/haip_test
+          REDSYS_TEST_DATABASE_URL: postgresql://haip:haip@localhost:5432/haip_test
           REDIS_URL: redis://localhost:6379
 
   ci-booking-requests:

README.md

@@ -14,7 +14,7 @@
   <img src="https://img.shields.io/badge/NestJS-framework-E0234E?logo=nestjs&logoColor=white" alt="NestJS" />
   <img src="https://img.shields.io/badge/PostgreSQL-database-4169E1?logo=postgresql&logoColor=white" alt="PostgreSQL" />
   <img src="https://img.shields.io/badge/License-Apache%202.0-blue" alt="Apache 2.0 License" />
-  <img src="https://img.shields.io/badge/Tests-2261%20passing-brightgreen" alt="2261 Tests Passing" />  <img src="https://img.shields.io/badge/AI%20Agents-12%20built--in-blueviolet" alt="12 AI Agents" />
+  <img src="https://img.shields.io/badge/Tests-2392%20passing-brightgreen" alt="2392 Tests Passing" />  <img src="https://img.shields.io/badge/AI%20Agents-12%20built--in-blueviolet" alt="12 AI Agents" />
 </p>
 
 <p align="center">
@@ -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 (2261 passing tests across 273 files with passing tests) | Unit and integration tests |
+| Testing | Vitest (2392 passing tests across 280 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: 2261 test cases across 273 files (skipped excluded)
+# Passing-test count: 2392 test cases across 280 files (skipped excluded)
 
 # API tests only
 pnpm --filter @telivityhaip/api test
@@ -1197,12 +1197,61 @@ 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 (2261 passing, 273 files with passes; skipped excluded)
+pnpm test             # Run all tests (2392 passing, 280 files with passes; skipped excluded)
 pnpm lint             # ESLint
 ```
 
 ---
 
+## Hosted booking payment returns
+
+For Redsys booking checkout, configure `BOOKING_RETURN_ORIGINS` with the exact
+origins of the pages embedding the booking widget (comma-separated, no wildcard).
+For example: `https://hotel.example,https://www.hotel.example`. Production requires
+HTTPS. Local development can explicitly allow `http://localhost:5174`. Configure
+the corresponding CORS origins for cross-origin widgets as usual.
+
+Run `pnpm db:migrate` before deploying the API changes, including
+`0024_payment_authorization_finalization.sql`,
+`0025_payment_booking_return_reference.sql`, and
+`0026_payment_booking_return_destination.sql`. Follow the
+[Redsys credential migration](docs/integrations/payments-redsys.md#existing-installations)
+before accepting payments with previously stored signing keys.
+The widget sends its full embedding-page URL as
+`returnUrl` to `POST /api/v1/booking-engine/book`. The API validates the origin
+before creating a guest or reservation, preserves the host path/query/fragment,
+and binds both provider outcomes to the same randomly generated return reference.
+The booking endpoint no longer accepts separate browser success/failure URLs.
+
+Both provider URLs use the compact API relay
+`/api/v1/booking-return/:reference?propertyId=:propertyId`, including for short
+hotel URLs. It shares the `PUBLIC_API_BASE_URL` configuration used by Redsys
+notifications (with the existing `API_BASE_URL` fallback). The server checks the
+provider's 250-character URL limit before creating booking records. Full hotel
+URLs are stored without the capability and are never truncated. The relay looks
+up the capability hash together with the supplied tenant scope, checks expiry and
+the destination allowlist again, and issues a non-cacheable 303 to that saved
+page with the original reference appended. It accepts no redirect destination or
+payment outcome from the browser.
+
+On return, the widget boots directly into its payment-status view without saved
+router state or browser storage. It polls
+`GET /api/v1/booking-engine/payment-return-status`, using the normal
+`x-booking-key` and an `x-payment-return-reference` header. The response contains
+only `status`: `processing`, `succeeded`, `failed`, `cancelled`, or `unavailable`.
+Only server payment state determines the result; browser flags are ignored.
+`succeeded` reports payment authorization, not unconditional booking confirmation.
+The reference expires seven days after payment creation and cannot retrieve guest
+details, retrieve a confirmation credential, or cancel a booking.
+
+Keep return references out of application, proxy, and analytics logs; the relay
+path reference and the host page's `haip_payment_return` query parameter are
+limited bearer capabilities. The relay sets `Referrer-Policy: no-referrer`.
+Failed, cancelled, or unverified returns direct guests to contact the hotel before
+trying again, because an earlier booking or payment may already exist.
+
+---
+
 ## License
 
 Licensed under the [Apache License, Version 2.0](LICENSE).

apps/api/src/modules/booking-engine/booking-engine.controller.ts

@@ -7,8 +7,10 @@ import {
   Param,
   Req,
   UseGuards,
+  Header,
+  Headers,
 } from '@nestjs/common';
-import { ApiTags, ApiOperation, ApiResponse, ApiSecurity } from '@nestjs/swagger';
+import { ApiTags, ApiOperation, ApiResponse, ApiSecurity, ApiHeader } from '@nestjs/swagger';
 import { Public } from '../auth/public.decorator';
 import { BookingKeyGuard } from '../auth/booking-key.guard';
 import { BookingEngineScopeGuard } from '../auth/booking-engine-scope.guard';
@@ -79,26 +81,26 @@ export class BookingEngineController {
     return this.service.book(this.propertyId(req), dto);
   }
 
-
-  @Get('checkouts/:checkoutToken')
-  @ApiOperation({
-    summary:
-      'Recover booking/payment state after a Redsys hosted-checkout browser return',
-  })
-  async getCheckout(
-    @Param('checkoutToken') checkoutToken: string,
-    @Req() req: any,
-  ) {
-    return this.service.getCheckout(this.propertyId(req), checkoutToken);
-  }
-
   @Get('bookings/:confirmationNumber')
   @ApiOperation({ summary: 'Retrieve a booking by confirmation number (guest self-service)' })
   async getBooking(@Param('confirmationNumber') confirmationNumber: string) {
     // Ownership is verified by BookingEngineScopeGuard before this runs.
     return this.service.verify(confirmationNumber);
   }
 
+  @Get('payment-return-status')
+  @Header('Cache-Control', 'no-store')
+  @ApiHeader({ name: 'x-payment-return-reference', required: true })
+  @ApiOperation({ summary: 'Read payment state using a limited, expiring return reference' })
+  @ApiResponse({ status: 200, schema: { type: 'object', required: ['status'], properties: {
+    status: { type: 'string', enum: ['processing', 'succeeded', 'failed', 'cancelled', 'unavailable'] },
+  } } })
+  @ApiResponse({ status: 404, description: 'Unknown, expired, or out-of-scope return reference' })
+  async paymentReturnStatus(@Headers('x-payment-return-reference') reference: string, @Req() req: any) {
+    // Tenant comes from the booking-key credential, not from the return reference.
+    return this.service.paymentReturnStatus(this.propertyId(req), reference);
+  }
+
   @Delete('bookings/:confirmationNumber')
   @ApiOperation({ summary: 'Cancel a booking by confirmation number' })
   async cancel(

apps/api/src/modules/booking-engine/booking-engine.module.ts

@@ -1,6 +1,7 @@
 import { Module } from '@nestjs/common';
 import { BookingEngineController } from './booking-engine.controller';
 import { BookingEngineAdminController } from './booking-engine-admin.controller';
+import { BookingReturnController } from './booking-return.controller';
 import { BookingEngineService } from './booking-engine.service';
 import { BookingEngineConfigService } from './booking-engine-config.service';
 import { BookingThrottleGuard } from './booking-throttle.guard';
@@ -32,7 +33,7 @@ import { PolicyModule } from '../policy/policy.module';
     AncillaryModule,
     PolicyModule,
   ],
-  controllers: [BookingEngineController, BookingEngineAdminController],
+  controllers: [BookingEngineController, BookingEngineAdminController, BookingReturnController],
   providers: [
     BookingEngineService,
     BookingEngineConfigService,

apps/api/src/modules/booking-engine/booking-engine.service.spec.ts

@@ -40,7 +40,10 @@ function makeService(overrides: Partial<Record<string, any>> = {}) {
     cancel: vi.fn(),
   };
   const folio = { createAutoFolio: vi.fn().mockResolvedValue({ id: 'folio-1' }) };
-  const payment = { authorizePayment: vi.fn().mockResolvedValue({ id: 'pay-1' }) };
+  const payment = {
+    assertAuthorizationAvailable: vi.fn().mockResolvedValue(undefined),
+    authorizePayment: vi.fn().mockResolvedValue({ id: 'pay-1', status: 'authorized' }),
+  };
   const deposit = { recordDeposit: vi.fn().mockResolvedValue({ id: 'dep-1', status: 'held' }) };
   const search = { search: vi.fn() };
   const bookingSvc = { verify: vi.fn() };
@@ -111,6 +114,52 @@ const bookDto = {
 };
 
 describe('BookingEngineService.quote', () => {
+  it('rejects unavailable Redsys credentials before any provisional booking writes', async () => {
+    const { svc, runtimeConfig, payment, guest, reservation, folio, deposit } = makeService();
+    runtimeConfig.get.mockImplementation((key: string) => ({ PAYMENT_GATEWAY: 'redsys', BOOKING_RETURN_ORIGINS: 'https://hotel.example' })[key]);
+    payment.assertAuthorizationAvailable.mockRejectedValue(new BadRequestException('Redsys credentials are not configured'));
+    await expect(svc.book(PROP, { ...bookDto, returnUrl: 'https://hotel.example/book' } as any)).rejects.toThrow(/credentials/);
+    expect(guest.create).not.toHaveBeenCalled();
+    expect(reservation.create).not.toHaveBeenCalled();
+    expect(folio.createAutoFolio).not.toHaveBeenCalled();
+    expect(deposit.recordDeposit).not.toHaveBeenCalled();
+  });
+  it('rejects an untrusted Redsys return before creating a guest or reservation', async () => {
+    const { svc, runtimeConfig, guest, reservation } = makeService();
+    runtimeConfig.get.mockImplementation((key: string) => key === 'PAYMENT_GATEWAY' ? 'redsys' : undefined);
+    await expect(svc.book(PROP, { ...bookDto, returnUrl: 'https://attacker.example/book' } as any)).rejects.toThrow(/return/i);
+    expect(guest.create).not.toHaveBeenCalled();
+    expect(reservation.create).not.toHaveBeenCalled();
+  });
+
+  it('binds the hosted return reference before signing the redirect and persists only its hash', async () => {
+    const { svc, runtimeConfig, payment } = makeService();
+    runtimeConfig.get.mockImplementation((key: string) => ({ PAYMENT_GATEWAY: 'redsys', BOOKING_RETURN_ORIGINS: 'https://hotel.example', PUBLIC_API_BASE_URL: 'https://api.example' })[key]);
+    const destination = `https://hotel.example/booking?lang=es&context=${'a'.repeat(500)}`;
+    await svc.book(PROP, { ...bookDto, returnUrl: destination } as any);
+    const [dto, , options] = payment.authorizePayment.mock.calls[0] as any[];
+    expect(dto.redirectUrlOk).toBe(dto.redirectUrlKo);
+    const url = new URL(dto.redirectUrlOk);
+    expect(url.origin).toBe('https://api.example');
+    expect(url.href.length).toBeLessThanOrEqual(250);
+    expect(url.pathname).toMatch(/^\/api\/v1\/booking-return\/[A-Za-z0-9_-]{43}$/);
+    expect(url.searchParams.get('propertyId')).toBe(PROP);
+    expect(options.returnReferenceHash).toMatch(/^[a-f0-9]{64}$/);
+    expect(options.returnDestination).toBe(destination);
+  });
+
+  it('rejects an oversized relay configuration before creating booking records', async () => {
+    const { svc, runtimeConfig, guest, reservation, payment } = makeService();
+    runtimeConfig.get.mockImplementation((key: string) => ({
+      PAYMENT_GATEWAY: 'redsys', BOOKING_RETURN_ORIGINS: 'https://hotel.example',
+      PUBLIC_API_BASE_URL: `https://api.example/${'a'.repeat(250)}`,
+    })[key]);
+    await expect(svc.book(PROP, { ...bookDto, returnUrl: 'https://hotel.example/book' } as any)).rejects.toThrow(/250/);
+    expect(guest.create).not.toHaveBeenCalled();
+    expect(reservation.create).not.toHaveBeenCalled();
+    expect(payment.authorizePayment).not.toHaveBeenCalled();
+  });
+
   it('prices server-side with the real tax engine and computes the deposit', async () => {
     const { svc } = makeService();
     const q = await svc.quote(PROP, { roomTypeId: RT, ratePlanId: RP, checkIn: '2026-07-01', checkOut: '2026-07-03', adults: 2 });
@@ -244,6 +293,34 @@ describe('BookingEngineService.quote', () => {
 });
 
 describe('BookingEngineService.book', () => {
+  it('does not classify an unauthorised payment as held when no next action is provided', async () => {
+    const { svc, config, payment, deposit, reservation } = makeService();
+    config.getConfig.mockResolvedValue({ autoConfirm: true });
+    payment.authorizePayment.mockResolvedValue({ id: 'pay-1', status: 'pending' });
+    const result = await svc.book(PROP, bookDto as any);
+    expect(deposit.recordDeposit).not.toHaveBeenCalled();
+    expect(reservation.confirm).not.toHaveBeenCalled();
+    expect(result.deposit?.status).toBe('pending');
+  });
+
+  it('keeps a redirect payment pending without recording a deposit or confirming', async () => {
+    const { svc, config, payment, deposit, reservation } = makeService();
+    config.getConfig.mockResolvedValue({ autoConfirm: true });
+    payment.authorizePayment.mockResolvedValue({
+      id: 'pay-1', status: 'pending', nextAction: { type: 'redirect' },
+    });
+
+    const result = await svc.book(PROP, bookDto as any);
+
+    expect(deposit.recordDeposit).not.toHaveBeenCalled();
+    expect(reservation.confirm).not.toHaveBeenCalled();
+    expect(result.status).toBe('pending');
+    expect(result.deposit).toMatchObject({ status: 'pending_redirect' });
+    expect(payment.authorizePayment).toHaveBeenCalledWith(expect.anything(), {
+      deposit: { reservationId: 'res-1', isRefundable: true, autoConfirm: true },
+    }, undefined);
+  });
+
   it('classifies the payment as a held deposit', async () => {
     const { svc, deposit, payment } = makeService();
     const res = await svc.book(PROP, bookDto as any);
@@ -313,32 +390,23 @@ describe('BookingEngineService.book', () => {
     await expect(svc.book(PROP, bookDto as any)).rejects.toBeInstanceOf(ForbiddenException);
   });
 
-
-  it('defers deposit + auto-confirm while Redsys redirect is pending', async () => {
-    const { svc, deposit, payment, reservation, config } = makeService();
-    config.getConfig.mockResolvedValue({ autoConfirm: true });
-    payment.authorizePayment.mockResolvedValue({
-      id: 'pay-1',
-      gatewayTransactionId: '1234ABCDEF',
-      nextAction: {
-        type: 'redirect',
-        url: 'https://sis-t.redsys.es/realizarPago',
-        method: 'POST',
-        formFields: { Ds_SignatureVersion: 'HMAC_SHA512_V2' },
-      },
+  it('rejects request mode before creating a guest, reservation, folio, or payment', async () => {
+    const { svc, config, guest, reservation, folio, payment } = makeService();
+    config.getPublicConfig.mockResolvedValue({
+      isEnabled: true,
+      bookingMode: 'request',
+      paymentMethodCollection: 'disabled',
+      formQuestions: [],
+      sellableRoomTypeIds: [RT],
+      sellableRatePlanIds: [RP],
+      depositPolicy: { type: 'first_night', refundable: true },
     });
 
-    const res = await svc.book(PROP, bookDto as any);
-
-    expect(deposit.recordDeposit).not.toHaveBeenCalled();
-    expect(reservation.confirm).not.toHaveBeenCalled();
-    expect(res.deposit).toMatchObject({
-      paymentId: 'pay-1',
-      status: 'pending_redirect',
-      checkoutToken: '1234ABCDEF',
-    });
-    expect(res.deposit?.nextAction).toMatchObject({ type: 'redirect' });
-    expect(res.status).toBe('pending');
+    await expect(svc.book(PROP, bookDto as any)).rejects.toBeInstanceOf(ForbiddenException);
+    expect(guest.create).not.toHaveBeenCalled();
+    expect(reservation.create).not.toHaveBeenCalled();
+    expect(folio.createAutoFolio).not.toHaveBeenCalled();
+    expect(payment.authorizePayment).not.toHaveBeenCalled();
   });
 
   it('requires a payment token when a deposit is due', async () => {