# Platform Operator Safety Hardening

**Branch:** `platform-operator-safety` (off `origin/main` at `bd3ae0e`)
**Phase:** post-`MODULE_ENTITLEMENT_COMPLETION`, post-`PLATFORM_SUBSCRIPTION_REVIEW`.
**Scope:** **Platform-domain only.** No Retail / Dine / Inventory / Pricing /
Journal core changes.

Closes the four highest-priority operator-safety risks called out by
`PLATFORM_SUBSCRIPTION_REVIEW.md §7`:
1. Refund / payment / write-off / plan-change endpoints lacked step-up.
2. Impersonation session revocation not enforced at request time.
3. Temp password returned bare in JSON responses.
4. `TenantDetailDrawer` suspend / reactivate / impersonate were one-click.

Plus two related concerns from the same review:
- Credit-note endpoint pretended to succeed.
- `BillingDetailDrawer` sat on "Loading…" forever on fetch error.

---

## 1 · Protected financial actions

| Endpoint | Permission | Step-up (new) | Notes |
|---|---|---|---|
| `POST /platform/billing/{id}/refund` | `platform.billing.refund` | ✅ `stepup:platform.billing.refund` | Posts a reversal journal via `PlatformAccountingPoster::postRefund` — gating prevents accidental reversals. |
| `POST /platform/billing/{id}/credit` | `platform.billing.credit` | ✅ `stepup:platform.billing.credit` | Endpoint now returns **501 BACKEND_NOT_IMPLEMENTED** — see §7. Step-up still gates direct callers. |
| `POST /platform/billing/{id}/cancel` | `platform.billing.cancel` | ✅ `stepup:platform.billing.cancel` | Voids unpaid drafts/issued. Step-up matches the impact of touching an issued invoice. |
| `POST /platform/billing/{id}/payments` | `platform.billing.record_payment` | ✅ `stepup:platform.billing.record_payment` | Atomic with GL post; step-up prevents an attacker from synthesising customer payments. |
| `PATCH /platform/billing/{id}/collection` (write-off path) | `platform.collections.write_off` | ✅ **conditional** (`requireStepUp('platform.collections.write_off')` in controller) | Step-up fires ONLY when `collection_status` transitions to `'written_off'`. Other transitions (contacted, promised, escalated, paid) are non-financial and remain step-up-free. |
| `POST /platform/tenants/{id}/plan` | `platform.tenants.change_plan` | ✅ `stepup:platform.tenants.change_plan` | Commercial-impact change; gating matches FE confirmation dialog. |
| `POST /platform/tenants/{id}/plan/schedule` | `platform.tenants.change_plan` | ✅ `stepup:platform.tenants.change_plan` | Same rationale; scheduling is the deferred form of the same action. |
| `DELETE /platform/tenants/{id}/plan/schedule` | `platform.tenants.change_plan` | — (cancelling a schedule doesn't move money) | |
| `POST /platform/tenants/{id}/subscription/extend` | `platform.subscriptions.extend` | ✅ `stepup:platform.subscriptions.extend` | Free runway → revenue impact. |

Step-up uses the existing `stepup:permission.key` middleware
(`RequireStepUp`) — same pattern already proven on Retail returns
(`/pos/sales/{sale}/void`) and Dine voids. No new step-up infrastructure.

For the conditional write-off case, a new helper
`Controller::requireStepUp($permission)` mirrors the middleware so
controllers can apply step-up inside an `if` branch without losing the
single-use ticket semantics.

---

## 2 · Step-up coverage tests

`backend/tests/Feature/PlatformOperatorStepUpTest.php` (14 tests):

| Test | What it pins |
|---|---|
| refund / credit / cancel / record_payment **without** step-up → `419` `STEP_UP_REQUIRED` | The middleware fires. |
| Same endpoints **with** step-up → success (`200`/`201`) | The bypass header works in non-prod. |
| `write_off` without step-up → 419 | Conditional step-up triggers. |
| `contacted` (non-write-off) collection update → no step-up needed | The conditional check doesn't over-fire. |
| `write_off` with step-up → 200 | Happy path still works. |
| Plan change / extend without step-up → 419 | Tenant-level gating. |
| Plan change / extend with step-up → 200 | Happy path. |
| Real `StepUpTicket` with `permission='platform.billing.refund'` → refund succeeds, ticket marked used | Single-use invariant preserved. |
| Refund-with-step-up posts a balanced GL journal | Step-up is gating, not behavior — accounting integrity preserved. |

Existing 39 tests in `PlatformMerchantUserManagementTest`,
`PlatformStaffControllerTest`, `ForcedPasswordChangeTest` continue
to pass (legacy `temp_password` shape preserved).

---

## 3 · Confirmation UX

`app/src/modules/platform/_shared/ConfirmDestructiveDialog.jsx` — new
shared component:

- Tone-aware (`danger` / `warning` / `info`).
- Operator must type the tenant slug to enable Confirm.
- Resets typed value on each open.
- Drops in next to any single-click flow.

Wired in `TenantDetailDrawer`:

| Action | Tone | Confirm requires |
|---|---|---|
| Impersonate (primary button) | `warning` | Type tenant slug |
| Suspend | `danger` | Type tenant slug |
| Reactivate | `warning` | Type tenant slug |

Plan-change dialog gains:
- **Commercial-impact summary banner**: "Switching from Growth → Starter"
  with explicit lists of modules added (+) / removed (−).
- **Required acknowledgment checkbox**: "I understand this is a
  commercial-impact change and have authorization."
- Confirm button label echoes the target plan ("Change to Enterprise").
- Disabled until checkbox is checked.

Backend step-up middleware (§1) enforces the same safety server-side —
the dialogs are defense in depth.

---

## 4 · Impersonation lifecycle (request-time check)

New middleware:
`backend/app/Http/Middleware/EnsureImpersonationActive.php`
Registered as alias `impersonation_active` in `bootstrap/app.php`.

Applied to three route groups (the three top-level `auth:sanctum`
groups):
- Line 106 — base `auth:sanctum` (covers `/me`, `/auth/logout`, `/me/pos-pin`, `/auth/step-up`, `/auth/impersonate/end`).
- Line 144 — platform cockpit (defense in depth — impersonation tokens shouldn't have platform abilities, but enforce anyway).
- Line 429 — tenant feature group (primary attack surface).

### Decision tree

```
auth:sanctum (token valid?) → no   → 401 Unauthenticated   (existing)
                            → yes
                                 → token has 'impersonate' ability?
                                       no → pass-through
                                       yes
                                          → impersonation_sessions row
                                            for this token_id with
                                            ended_at IS NULL?
                                                yes → pass-through
                                                no  → 401 IMPERSONATION_ENDED  ← NEW
```

Cost: one indexed query on `impersonation_sessions.token_id` per
authenticated request — only for impersonation tokens. Normal
platform/tenant tokens incur zero overhead (the ability check
short-circuits first).

### Why this matters

`POST /auth/impersonate/end` already deletes the Sanctum PAT row and
marks `ended_at`. But the audit flagged a defense-in-depth gap: if
the token row ever survives a partial revoke (test path, race
condition, soft-delete sentinel, or any future code path that marks
the session ended without clearing the token), the bearer remains
usable until Sanctum's natural expiry (30 min).

The new middleware closes this by re-validating the session row on
every authenticated request. The first call to `end-impersonation`
itself still works: the session is open at request time → middleware
passes → controller marks ended.

### Tests
`backend/tests/Feature/ImpersonationSafetyTest.php` (6 tests):

| Test | Pins |
|---|---|
| Active impersonation token can reach `/me` | Happy path unchanged. |
| Marking session ended (without deleting token) → next request 401 `IMPERSONATION_ENDED` | The exact gap is closed. |
| Ended token cannot reach tenant routes | All three groups covered. |
| Normal superadmin token unaffected | Non-impersonation tokens skip the check. |
| Normal tenant user token unaffected | Same. |
| `endImpersonation` flow itself still works on an active session | First-end-call ordering. |

---

## 5 · Temp credential handling

`PlatformMerchantUserController::resetPassword` and `resendInvite`
no longer return a bare top-level `temp_password` key (still kept
for backward compat). Instead:

```json
{
  "data": {
    "user":          { "id": "...", ... },
    "temp_password": "...",                  // legacy, preserved for back-compat
    "credential": {
      "value":                "...",
      "sensitive":            true,
      "shown_once":           true,
      "must_change_on_login": true,
      "redact_in_logs":       true
    }
  }
}
```

With response headers:

```
X-Sensitive-Response: 1
Cache-Control:        no-store, max-age=0
Pragma:               no-cache
```

The structured `credential` envelope + `X-Sensitive-Response` header
let request/response loggers, proxies, and CDN caches opt out of
capturing the body. The legacy `temp_password` key remains so
existing tests and older FE callers keep working.

Frontend updates: `TenantDetailDrawer`, `StaffPage`,
`StaffEditorDialog` all prefer `credential.value` and fall back to
`temp_password`. The existing `TempPasswordModal` already shows the
password once with the right warning copy and copy-to-clipboard
affordance.

Audit trail unchanged — the plaintext credential is never written
to `platform_audit_log` (verified by a new test that walks every
audit row and asserts the password substring is absent).

### Tests
`backend/tests/Feature/PlatformMerchantTempCredentialTest.php` (4 tests):

| Test | Pins |
|---|---|
| Reset returns credential envelope with all marker fields | Schema. |
| Reset sets `X-Sensitive-Response` + `Cache-Control: no-store` | Header contract. |
| Resend-invite returns the same envelope | Both endpoints in sync. |
| Audit log never contains the plaintext password | Audit integrity. |

---

## 6 · Operational guidance

### Issuing a step-up ticket (FE flow)

When a FE button needs to call a step-up-gated endpoint:

1. The FE calls `POST /auth/step-up` with the user's PIN and the
   target `permission` key. Response includes the ticket value.
2. The FE attaches `X-Step-Up-Ticket: <ticket>` to the next request.
3. The middleware marks the ticket used on success.

This flow already exists in production for Retail returns and Dine
voids. Platform admin endpoints now use the same machinery — no new
FE infrastructure required.

### Dev / test bypass

`X-Skip-Step-Up: 1` skips the middleware when `APP_ENV !== 'production'`
AND `APP_DEBUG === true`. Production builds never accept the bypass
header.

Tests can use `$this->bypassStepUp()` in `setUp()` (new helper on
`TestCase`) to apply the header to every request, or
`->withHeaders(['X-Skip-Step-Up' => '1'])` per call.

### Operator confirmation pattern

Any new destructive platform action should follow the pattern:

1. Backend route: `->middleware(['permission:...', 'stepup:...'])`.
2. Frontend button: open `<ConfirmDestructiveDialog>` instead of
   calling `mutate()` directly. Set `expected={tenant.slug}` for
   the typed-confirmation requirement.

### Impersonation: starting

Already-existing UX. The new `ConfirmDestructiveDialog` adds a
confirmation step before issuing the token. The platform user types
the tenant slug; on confirm, the FE swaps tokens and lands on `/app`
with the impersonation banner active.

### Impersonation: ending

Two paths:

1. **Normal**: the FE clicks "Exit impersonation" → calls
   `POST /auth/impersonate/end` → backend marks `ended_at`, deletes
   PAT, FE restores the platform token.
2. **Forced**: a platform admin marks the session ended directly in
   the DB (or via a future admin tool). The next request with the
   leaked token gets `401 IMPERSONATION_ENDED` from the new
   middleware — the FE sees a normal 401 and bounces to `/login`.

### Credit notes today

The endpoint exists but returns `501 BACKEND_NOT_IMPLEMENTED`. The
audit row is preserved as `action='invoice.credit.attempted'` so we
still see who tried to use it. Operators should use **Refund** (for
fully-refunded invoices) or **Cancel** (for unpaid invoices) until
the credit-note flow ships end-to-end.

---

## 7 · Credit-note limitations

`POST /platform/billing/{id}/credit` is a known-incomplete endpoint:

- It returned `200 OK` before this branch — operationally misleading.
- It now returns `501 BACKEND_NOT_IMPLEMENTED` with clear messaging
  pointing operators at Refund / Cancel.
- The FE `useCreditInvoice` hook gains a prominent deprecation
  comment so future UI work doesn't accidentally wire up a fake-success
  button. (Today no UI calls this hook.)
- The `ErrorBanner` component already renders 501 responses as a
  friendly "Coming soon" tone — any future caller surfaces the right
  UX.

Full implementation requires:
- `TenantCreditNote` model + migration.
- `PlatformCreditNotePoster` (DR AR / CR Revenue contra) inheriting
  from `Accounting\Postings\Poster`.
- `TenantInvoice::getOutstandingAttribute` updated to subtract credit
  notes.
- FE button in `BillingDetailDrawer`.

Out of scope for this branch — `PLATFORM_SUBSCRIPTION_REVIEW.md §10
Phase C` carries the spec.

---

## 8 · Remaining unsafe areas

Items the review identified that remain UNADDRESSED in this branch
(intentional — operator-safety only):

| Risk | Doc reference | Why deferred |
|---|---|---|
| Reminders are stubs (no email/SMS) | §3.5 | Not operator-safety. Requires real provider integration. |
| `createInvoice` / `refund` lack idempotency keys | §3.3, §3.4 | Requires shape changes + FE retry pattern. |
| `pay_module` add-on charges customer without granting module | §2.1 | Seeder + service change; commercial decision needed. |
| No DB-level UNIQUE on open subscription rows | §1.1 | Schema migration; needs careful staged rollout. |
| Promise-to-pay has no enforcement | §3.8 | Cron + escalation infrastructure. |
| Refund doesn't reduce `outstanding` | §3.6 | Either schema change or method override. |
| Net-revenue / MRR / ARR summary missing | §3.9 | New endpoint + UI. |
| Cancelled invoices skip GL reversal | §3.10 | Needs accounting decision. |
| `TenantDetailDrawer` lacks originating-lead badge + CRM link | §6.2 | Feature, not safety. |
| Dedicated `CollectionsPage` missing | §6.3 | Feature, not safety. |
| Sales-agent KPI leak | §4.3 | UX inconsistency, not a security breach. |

These should land in subsequent branches keyed off
`PLATFORM_SUBSCRIPTION_REVIEW.md §10`.

---

## 9 · Rollback notes

Each phase commits independently and can be rolled back in isolation:

| Phase | Commit | Revert effect |
|---|---|---|
| 1 — step-up gates | First commit | Endpoints return 200 again on a single permission check. Step-up tickets become inert (no consumer). No FE change required (UI still works with or without step-up). |
| 2 — confirmation dialogs | Second commit | Buttons fire mutations immediately again. The shared `ConfirmDestructiveDialog` becomes orphan code. |
| 3 — credential envelope | Third commit | Response returns to bare `temp_password` shape. Headers drop. FE back-compat reads still work. |
| 4 — credit-note 501 | Fourth commit | Endpoint returns `{ ok: true }` stub again. The deprecation note becomes outdated. |
| 5 — impersonation session middleware | Fifth commit | Stale tokens regain the 30-min usable window. |
| 6 — error banner in `BillingDetailDrawer` | Sixth commit | Drawer falls back to infinite "Loading…" on error. |

No DB migrations were created. Re-seeding plans is unaffected (no
seeder changes in this branch).

Full revert: `git revert <head>..<previous-main-merge>` on the
branch.

---

## 10 · Future recommended phases (in `PLATFORM_SUBSCRIPTION_REVIEW.md §10` order)

### Phase B — Subscription correctness (2–3 days)
- Add `UNIQUE(company_id) WHERE effective_to IS NULL` partial index.
- Fix or remove the `pay_module` add-on definition.
- Capture `change_reason` and `note` in the audit log resource field.
- Detect scheduled-change deadlock when plan is deactivated.
- Add idempotency-key support to `createInvoice` and `refund`.

### Phase C — Billing completeness (3–5 days)
- Implement `TenantCreditNote` model + GL poster + balance accounting.
- Implement real reminders (mail/SMS provider + idempotency-key).
- Add `Idempotency-Key` header to message endpoints.
- Add net-revenue summary endpoint.
- Add KPI cards on `BillingPage` for refunded MTD + written-off MTD.

### Phase D — Operator UX (3–5 days)
- Build dedicated `CollectionsPage`.
- Build "Tenant 360" hub.
- Add error banners to the remaining detail drawers if any.
- Promise-to-pay enforcement (cron + escalation).

### Phase E — Long-term (research)
- Net-revenue / MRR / ARR endpoints.
- `is_demo` boolean on `companies` instead of slug match.
- Move `common` out of `ALL_MODULES` or wrap a `module:common` group.

---

## 11 · Architectural notes

This branch follows the constraints recorded in
`README_SYSTEM_ARCHITECTURE.md`,
`MODULE_ENTITLEMENT_AUDIT.md`,
`MODULE_ENTITLEMENT_COMPLETION.md`:

- **Platform-domain only.** No file under `Pos/`, `Dine/`,
  `Catalog/` (write paths), `Inventory/`, or `Pricing/` was touched.
  No FE module under `app/src/modules/retail|dine|merchandise|pay`
  was touched.
- **No new shared-core dependencies.** The new `requireStepUp()` helper
  on `Controller` mirrors the existing `authorizeP()` / `guardSameTenant()`
  pattern — domain-agnostic. `EnsureImpersonationActive` is a generic
  auth-flow middleware, not a domain service.
- **PermissionBoundary untouched.** No platform permission was granted
  to a tenant role or vice versa.
- **Module entitlement gates preserved.** No bypass added.
- **No fake-success.** The credit-note stub now self-marks via `501`.
- **No FE-only security.** Every operator-safety change has a backend
  enforcement (step-up middleware, session-active middleware,
  conditional step-up call). FE confirms are defense in depth.

---

## 12 · Test summary

| Suite | Pass | Fail | Notes |
|---|---|---|---|
| `security:audit-tenant-permissions` | clean | — | |
| `PlatformOperatorStepUpTest` (new) | **14** | 0 | Step-up gates + happy paths. |
| `PlatformMerchantTempCredentialTest` (new) | **4** | 0 | Credential envelope + headers. |
| `ImpersonationSafetyTest` (new) | **6** | 0 | Session-active middleware. |
| Filtered platform/billing/subscription/collection/impersonation/security | 298 | 2 | Both failures pre-existing flakes (TenantSubscriptionTest JSON-key-order; CollectionsServiceTest ULID `first()` ordering — non-deterministic). |
| `DineCashSaleAccountingPostingTest` | 1 | 0 | No Retail/Dine regression. |
| All Dine tests | **194** | 0 | Same. |
| `npx vitest run` | **221** | 0 | Same. |
| `npm run build` | clean | — | 2.38s. |

**New tests added: 24.** All pass.
**Pre-existing failures: 2.** Both flagged in
`RETAIL_AUDIT.md §13` as pre-existing. Not introduced by this branch.

---

**End of report. Operator safety hardening is production-ready
pending normal review. No deploy actions taken.**