# Tenant ↔ Platform Permission Lockdown

**Severity:** Critical — privilege escalation across tenants and across the platform boundary.
**Status:** Fixed on branch `security-tenant-permission-lockdown`.
**Audit command:** `php artisan security:audit-tenant-permissions`

---

## 1. What was broken

Before this branch, **any tenant user holding `users.assign_roles`** (the typical
`business-owner` shape that every paying tenant has out of the box) could:

| # | Attack | Endpoint | Effect |
|---|---|---|---|
| 1 | Create a new tenant user with role `superadmin` | `POST /api/v1/users` | Instant cross-tenant + platform privilege for the new user. |
| 2 | Invite a user with any platform role | `POST /api/v1/users/invite` | Same as 1 once the invitee accepts. |
| 3 | Promote an existing tenant user to `superadmin` | `PUT /api/v1/users/{id}/roles` | Any teammate becomes a platform admin. |
| 4 | Self-escalate to `superadmin` | `PUT /api/v1/users/{me}/roles` | The attacker themselves becomes a platform admin. |
| 5 | Create a brand-new role packed with `platform.*` permissions, then assign it | `POST /api/v1/roles` → `PUT /api/v1/users/{id}/roles` | Re-creates 1–4 without touching role names. |
| 6 | Edit the existing `manager` role to add `platform.*` permissions | `PATCH /api/v1/roles/{id}` | **Every tenant's `manager` role gains the permission** — Spatie roles are GLOBAL, not company-scoped. |
| 7 | Mutate a user belonging to another company | `PUT /api/v1/users/{other}/roles` | Cross-tenant write. |

The escalations bypassed every existing safety rail:

- **Validation only checked `exists:roles,name`.** Any seeded role — including
  the platform ones — passed.
- **The `users.assign_roles` permission** is part of the `business-owner`
  default kit.
- **Step-up** (`stepup:users.assign_roles`) gated `PUT /users/{id}/roles` but
  was satisfied by a plain manager-PIN; it slowed the attack by one HTTP
  round-trip and did not block it.
- **Spatie roles are global.** Editing the `manager` row in one tenant
  silently changed every tenant's `manager`.

---

## 2. Root cause

Three code-level gaps stacked on top of each other:

1. **No "platform vs tenant" boundary at the controller layer.** Controllers
   passed user-supplied role / permission names straight to
   `User::syncRoles()` / `Role::syncPermissions()` after only an `exists`
   check.
2. **Spatie's role model is global.** A tenant who can mutate `manager`
   mutates every tenant's `manager`.
3. **The `business-owner` role was seeded with `$allPerms`** — including the
   full `platform.*` slice. The middleware `EnsurePlatformStaff` still
   blocked the platform routes because `is_platform_staff = false`, but the
   permissions sat there as a single-point-of-failure and as noise in
   audits.

---

## 3. The fix

### 3.1 Central guard: `App\Services\Security\PermissionBoundary`

One class, one source of truth, used by every controller that mutates roles,
permissions, or role-assignments:

```php
PermissionBoundary::assertCanAssignRoles($actor, $roleNames);         // FORBIDDEN_ROLE_ASSIGNMENT
PermissionBoundary::assertCanAssignPermissions($actor, $permNames);   // FORBIDDEN_PERMISSION_ASSIGNMENT
PermissionBoundary::assertCanModifyRole($actor, $role);               // FORBIDDEN_ROLE_ASSIGNMENT
PermissionBoundary::assertCanCreateRoleNamed($actor, $name);          // FORBIDDEN_ROLE_ASSIGNMENT
```

Rules:

- A "platform actor" = `User->is_platform_staff === true`. Only platform
  actors may grant platform roles or platform.* permissions.
- A role is "platform-only" iff its name is in
  `PermissionBoundary::PLATFORM_ROLES`:
  `['superadmin', 'finance-manager', 'finance-employee', 'support-agent', 'operations']`.
- A permission is "platform-only" iff it starts with `platform.`.
- Every assertion throws an `ApiException` (HTTP 422) **before** any
  Spatie call, so a denied request leaves zero side effects (no
  half-created user, no orphan role).

### 3.2 Controllers patched

- `UserController::store`     — `assertCanAssignRoles` before `User::create`.
- `UserController::invite`    — `assertCanAssignRoles` before `User::create`.
- `UserController::syncRoles` — `assertCanAssignRoles` before `syncRoles`.
- `RoleController::store`     — `assertCanCreateRoleNamed` + `assertCanAssignPermissions` before `Role::create`.
- `RoleController::update`    — `assertCanModifyRole` + `assertCanAssignPermissions` before `syncPermissions`.
- `RoleController::destroy`   — `assertCanModifyRole` before `delete`.

The platform routes (`/api/v1/platform/*`) already lived behind the
`platform` group middleware (`EnsurePlatformStaff` + per-route
`permission:platform.*`); no change needed there.

### 3.3 Seeder + remediation migration

- **Seeder change:** the `business-owner` role now syncs the tenant-safe
  slice of permissions (every permission minus `platform.*`). Fresh
  installs no longer carry the exposure.
- **Migration** `2026_05_11_040000_security_strip_platform_perms_from_tenant_roles`:
  detaches every `platform.*` permission from every Spatie role whose name
  is NOT in `PLATFORM_ROLES`, plus any direct user-permission rows for
  tenant users. Idempotent. `down()` is a deliberate no-op.

### 3.4 Frontend UX hardening

`app/src/modules/ops/rbac.hooks.js` now filters platform rows out of the
tenant role + permission dropdowns:

- `useRolesList` / `useRolesSummary` strip `PLATFORM_ROLES`.
- `usePermissionsList` strips any name starting with `platform.`.

This is purely a UX layer — the backend `PermissionBoundary` is still the
authoritative check.

---

## 4. Tests

`backend/tests/Feature/TenantPermissionLockdownTest.php` — 14 cases, all
green:

```
✓ tenant cannot create user with superadmin role
✓ tenant cannot invite user with platform role
✓ tenant cannot promote existing user to superadmin
✓ tenant cannot self escalate to superadmin
✓ tenant cannot create role with platform permissions
✓ tenant cannot edit existing role to add platform permissions
✓ tenant cannot create a role named like a platform role
✓ tenant cannot assign roles to user from another company
✓ tenant admin cannot access platform activation codes
✓ tenant admin cannot promote signups
✓ tenant admin cannot modify platform staff
✓ platform superadmin can still assign platform roles      ← positive
✓ tenant admin can still assign safe tenant roles          ← positive
✓ tenant admin can still edit tenant role with safe permissions ← positive
```

Regression suites verified green:

- `php artisan test --filter=DineCashSaleAccountingPostingTest` — 1/1 pass.
- `php artisan test --filter=DinePostingTest` — 16/16 pass.
- `php artisan test --filter=OpsPermissionGatingTest|StepUpTest|PlatformRolesTest` — 13/13 pass.
- `npx vitest run` — 180/180 pass.

---

## 5. Existing-data exposure audit

Run the audit any time you want a picture of the boundary:

```
php artisan security:audit-tenant-permissions
php artisan security:audit-tenant-permissions --json
```

It reports:

1. Tenant users holding a platform role.
2. Tenant users holding a `platform.*` permission directly.
3. Tenant-side roles whose permission set contains `platform.*`.
4. Cross-tenant `branch_user` pivot rows (defence-in-depth).
5. Platform staff who somehow lost their platform role.

Exit code 0 if clean, 2 if anything is found — wire into CI to catch
regressions.

On our current development database the audit was clean immediately after
the remediation migration ran. **Run it once on production right after the
migration deploys.**

---

## 6. Deployment runbook

1. Merge this branch to `main`.
2. Build + deploy the backend.
3. Run migrations: `php artisan migrate --force`.
   - The remediation migration `2026_05_11_040000_security_strip_platform_perms_from_tenant_roles`
     will detach existing platform.* rows from tenant roles + tenant users.
4. Run the audit immediately: `php artisan security:audit-tenant-permissions`.
   - Expect: `OK — tenant↔platform permission boundary is clean.`
   - If anything is reported, page security on-call. Do not auto-remediate.
5. Smoke-test the platform cockpit (your superadmin login should still
   reach `/api/v1/platform/*`).
6. Smoke-test a tenant: log in as `business-owner`, attempt to assign
   `superadmin` to another user — expect 422 with code
   `FORBIDDEN_ROLE_ASSIGNMENT`.

If anything goes wrong:

- The migration `down()` is a no-op by design (restoring platform.* to
  tenant roles would re-open the vulnerability). To roll back, revert the
  branch and redeploy — the guard service is gone, but the migration's
  effect remains (which is the safe state). Re-seeding `business-owner`
  with the full permission set would require an explicit operator
  decision and another migration.

---

## 7. Rules of engagement going forward

- **Platform vs tenant boundary is `is_platform_staff`.** Permission flags
  alone are not enough. Any new route that mutates roles / permissions
  MUST call `PermissionBoundary` before touching Spatie.
- **Spatie roles are global.** Editing a tenant role affects every
  tenant. New role-mutation endpoints must call
  `PermissionBoundary::assertCanModifyRole` before any `syncPermissions`.
- **`platform.*` is reserved.** Don't introduce new platform-only
  permissions outside the prefix; the audit + the migration both
  depend on it.
- **PLATFORM_ROLES is a closed allow-list.** Adding a new platform role
  is a code review in two places (`PermissionBoundary::PLATFORM_ROLES`
  and `PlatformStaffController::PLATFORM_ROLES`). Don't ship them out of
  sync.