TenantAtlas/specs/067-rbac-troubleshooting/research.md

Research — RBAC Troubleshooting & Tenant UI Bugfix Pack v1

Date: 2026-01-31
Spec: spec.md

This document captures design decisions and supporting rationale. Where applicable, decisions are grounded in the current codebase and the TenantPilot Constitution.

Decision 1 — Archived tenants must remain resolvable in tenant-scoped routes

Decision: Make the tenant binding used by Filament tenancy include archived (soft-deleted) tenants so entitled members can view /admin/t/{tenant}/... even when archived.

Rationale:

• The current tenant lifecycle uses soft deletes + status='archived' on delete and restores back to active.
• If tenancy resolution excludes trashed records, the app will 404 before membership middleware can apply “deny-as-not-found for non-members”, causing false 404s for legitimate members.
• Constitution RBAC-UX requires:
  • non-members → 404
  • members but missing capability → 403
  • archived is a lifecycle state, not an isolation boundary.

Evidence:

• Tenant tenancy config (slug is external_id, route prefix t): Admin panel tenancy
• Archived behavior is implemented via soft deletes + status mutations: Tenant booted hooks

Alternatives considered:

• Stop using soft deletes for tenants and represent archived purely via a status field.
  • Rejected: larger behavioral change, wider surface area (queries, “activeQuery”, etc.), higher regression risk.
• Keep current 404 and treat “archived” as inaccessible.
  • Rejected: contradicts spec FR-004 and the operational need to restore.

Decision 2 — Standardize Tenant view header actions via UiEnforcement

Decision: Wrap Tenant view page header actions (Edit/Deactivate) in the existing UI enforcement helper so tenant members without the capability see disabled actions with tooltips, avoiding “normal click leads to 403 page”.

Rationale:

• Constitution RBAC-UX-004 requires visible-but-disabled for members lacking capability.
• Existing code already follows this pattern in multiple resources (e.g., tenant list actions, membership manager relation managers).
• The current ViewTenant header actions include an unconditional edit action and an archive action that executes without capability checks.

Evidence:

• Unconditional edit header action: ViewTenant header action group
• Deactivate executes without capability gating: ViewTenant archive action

Alternatives considered:

• Hide actions entirely for users without capability.
  • Rejected by default: RBAC-UX prefers visible-but-disabled.
• Leave the action visible and rely only on server-side 403.
  • Rejected: fails the UX requirement (normal click to 403).

Decision 3 — Restore icon consistency is a UI-only patch

Decision: Add an icon to the tenant row Restore action.

Rationale:

• Improves consistency and reduces “is this safe?” uncertainty.
• No behavioral/authorization changes; minimal regression risk.

Evidence:

• Restore action currently has no icon: TenantResource restore action

Alternatives considered:

• Leave it as-is.
  • Rejected: explicit FR-001.

Decision 4 — Diagnostics is DB-only and uses existing audit logging patterns

Decision: Implement tenant-scoped diagnostics as DB-only rendering and DB-only repairs with explicit confirmation + server-side authorization, recording audit logs.

Rationale:

• Constitution requires read/write separation and that monitoring/diagnostics pages do not trigger external calls during render.
• Repairs are security-relevant mutations and must be audited.

Alternatives considered:

• Attach diagnostics to Monitoring/Operations.
  • Rejected: these repairs are DB-only and do not need OperationRun by default; keep scope small.

Decision 5 — Membership invariants

Decision:

• Continue using server-side guards preventing last-owner removal/demotion.
• Add diagnostics checks for “missing owner” and provide a safe repair (promote chosen member to owner) gated by an existing management capability.

Rationale:

• Last-owner guard exists and is correct; the missing-owner case needs a UI recovery path.

Evidence:

• Last-owner guard behavior: TenantMembershipManager last-owner guards

Alternatives considered:

• DB constraint enforcing at least one owner.
  • Rejected: non-trivial, would complicate bulk edits and migrations; better handled at app layer with explicit repair tools.

Decision 6 — Duplicate memberships

Decision: Keep the DB-level uniqueness constraint as the primary protection, but still implement diagnostics that can detect historical duplicates and merge them safely (no-op when none exist).

Rationale:

• The uniqueness constraint exists today, but the product requirement includes a recovery/repair flow.

Evidence:

• Unique constraint in migration: tenant_memberships unique index

Alternatives considered:

• Remove diagnostics for duplicates since the DB constraint exists.
  • Rejected: conflicts with FR-006 and the “UI-only recovery” goal.

Decision 7 — GUID vs bigint guardrails

Decision: Explicitly distinguish:

• internal tenant primary key (tenants.id, bigint)
• external tenant identifiers (tenant_id GUID / external_id string)

and ensure code paths that need an internal FK use $tenant->getKey() (cast to int), not $tenant->tenant_id.

Rationale:

• Prevent PostgreSQL invalid input syntax for type bigint errors caused by passing the GUID into bigint tenant_id columns.

Alternatives considered:

• Rename columns.
  • Rejected (out of scope / migration-heavy).