fix(website): stop club rank matrix from faking a permission save (WEB-W1)

The Ranks & Roles "Operational Permission Matrix" previously waited ~400ms and
showed a "Saved" state without persisting anything. No Firestore document, API
route, or security rule stores or enforces a per-rank capability matrix; club
authorization is role-based (Owner/Admin via getPortalPermissions). Persisting
the matrix would require frozen security-rule changes and would create an
unenforced permission model that misleads owners.

Make the control truthful instead of faking success:
- Render the matrix as a read-only reference of the standard governance model
  by default (persistenceAvailable=false): no Save/Reset controls, disabled
  toggles, a "View-only reference" indicator, and a notice stating that access
  is enforced by Owner/Admin roles and per-rank customization is planned.
- Remove the fake setTimeout save handler and the unconditional success state.
- Retain a forward-compatible editable path behind persistenceAvailable for
  when a backend contract ships.
- Document the precise backend contract required to enable persistence.

Tests: read-only matrix renders no Save control, reports no save, and does not
mutate permissions even for Owner/Admin; editable path still fires toggle/save
/reset when persistenceAvailable=true.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Developer 2026-10-14 11:20:29 -04:00
parent 493192078f
commit c014c2f04f
4 changed files with 225 additions and 46 deletions

View File

@ -34,7 +34,6 @@ export default function ClubFounderRanksRolesPage({ params }: PageProps) {
const [members, setMembers] = useState<ClubMember[]>([]); const [members, setMembers] = useState<ClubMember[]>([]);
const [permissions, setPermissions] = useState(DEFAULT_RANK_PERMISSIONS); const [permissions, setPermissions] = useState(DEFAULT_RANK_PERMISSIONS);
const [selectedRankId, setSelectedRankId] = useState<string | null>('president'); const [selectedRankId, setSelectedRankId] = useState<string | null>('president');
const [isSavingPermissions, setIsSavingPermissions] = useState(false);
const [loading, setLoading] = useState(true); const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(null); const [error, setError] = useState<string | null>(null);
const [unauthenticated, setUnauthenticated] = useState(false); const [unauthenticated, setUnauthenticated] = useState(false);
@ -104,16 +103,6 @@ export default function ClubFounderRanksRolesPage({ params }: PageProps) {
})); }));
} }
function handleResetDefaults() {
setPermissions(DEFAULT_RANK_PERMISSIONS);
}
async function handleSavePermissions() {
setIsSavingPermissions(true);
await new Promise((r) => setTimeout(r, 400));
setIsSavingPermissions(false);
}
if (loading) { if (loading) {
return ( return (
<div className="min-h-screen bg-brand-bg telemetry-grid-bg p-6"> <div className="min-h-screen bg-brand-bg telemetry-grid-bg p-6">
@ -220,16 +209,23 @@ export default function ClubFounderRanksRolesPage({ params }: PageProps) {
/> />
{/* Interactive Permission Toggle Switches Matrix */} {/* Interactive Permission Toggle Switches Matrix */}
{/*
Per-rank capability permissions have no server-side persistence contract yet.
The enforced authorization model is role-based (Owner / Admin) via
getPortalPermissions in lib/club-data.ts, not this rank matrix. Rendering the
matrix read-only (persistenceAvailable={false}) keeps it truthful: it shows the
standard governance conventions without claiming a save that does not occur.
See website/docs/backend-contracts/club-rank-permissions.md for the proposed
contract that would flip persistenceAvailable to true.
*/}
<ClubPermissionMatrix <ClubPermissionMatrix
ranks={DEFAULT_CLUB_RANKS} ranks={DEFAULT_CLUB_RANKS}
capabilities={DEFAULT_CLUB_CAPABILITIES} capabilities={DEFAULT_CLUB_CAPABILITIES}
permissions={permissions} permissions={permissions}
selectedRankId={selectedRankId} selectedRankId={selectedRankId}
onTogglePermission={handleTogglePermission} onTogglePermission={handleTogglePermission}
onResetDefaults={handleResetDefaults}
onSavePermissions={handleSavePermissions}
isSaving={isSavingPermissions}
canEdit={context.permissions.canManageSettings} canEdit={context.permissions.canManageSettings}
persistenceAvailable={false}
/> />
</div> </div>
</FounderPortalShell> </FounderPortalShell>

View File

@ -9,10 +9,22 @@ interface ClubPermissionMatrixProps {
permissions: Record<string, Record<ClubCapability, boolean>>; permissions: Record<string, Record<ClubCapability, boolean>>;
selectedRankId: string | null; selectedRankId: string | null;
onTogglePermission: (rankId: string, capability: ClubCapability) => void; onTogglePermission: (rankId: string, capability: ClubCapability) => void;
onResetDefaults: () => void; onResetDefaults?: () => void;
onSavePermissions: () => Promise<void>; onSavePermissions?: () => Promise<void>;
isSaving: boolean; isSaving?: boolean;
canEdit: boolean; canEdit: boolean;
/**
* Whether a server-side persistence contract for per-rank capability
* permissions actually exists. Defaults to false.
*
* The enforced club authorization model is role-based (Owner / Admin), not
* rank-based (see getPortalPermissions in lib/club-data.ts). No Firestore
* document or API persists this capability matrix today, so by default the
* matrix renders as a read-only reference of the standard governance model.
* The editable + save path is retained for when a backend contract ships
* (see website/docs/backend-contracts/club-rank-permissions.md).
*/
persistenceAvailable?: boolean;
} }
export function ClubPermissionMatrix({ export function ClubPermissionMatrix({
@ -23,12 +35,19 @@ export function ClubPermissionMatrix({
onTogglePermission, onTogglePermission,
onResetDefaults, onResetDefaults,
onSavePermissions, onSavePermissions,
isSaving, isSaving = false,
canEdit, canEdit,
persistenceAvailable = false,
}: ClubPermissionMatrixProps) { }: ClubPermissionMatrixProps) {
const [saveSuccess, setSaveSuccess] = useState(false); const [saveSuccess, setSaveSuccess] = useState(false);
// Edit + save controls are only live when the caller both grants edit rights
// AND a real persistence backend is wired. Without persistence the matrix is
// a truthful read-only reference: it never mutates and never reports a save.
const editable = canEdit && persistenceAvailable && typeof onSavePermissions === 'function';
async function handleSave() { async function handleSave() {
if (!editable || !onSavePermissions) return;
await onSavePermissions(); await onSavePermissions();
setSaveSuccess(true); setSaveSuccess(true);
setTimeout(() => setSaveSuccess(false), 2500); setTimeout(() => setSaveSuccess(false), 2500);
@ -48,12 +67,13 @@ export function ClubPermissionMatrix({
</h2> </h2>
</div> </div>
<p className="font-body text-xs sm:text-sm text-slate-400 mt-1"> <p className="font-body text-xs sm:text-sm text-slate-400 mt-1">
Configure delegated operational privileges per squad rank. Administrative roles enforce backend limits. Standard delegated operational privileges per squad rank. Administrative roles enforce backend limits.
</p> </p>
</div> </div>
{canEdit && ( {editable ? (
<div className="flex items-center gap-2.5 self-start sm:self-center"> <div className="flex items-center gap-2.5 self-start sm:self-center">
{onResetDefaults && (
<button <button
type="button" type="button"
onClick={onResetDefaults} onClick={onResetDefaults}
@ -61,6 +81,7 @@ export function ClubPermissionMatrix({
> >
Reset Defaults Reset Defaults
</button> </button>
)}
<button <button
type="button" type="button"
disabled={isSaving} disabled={isSaving}
@ -85,9 +106,33 @@ export function ClubPermissionMatrix({
)} )}
</button> </button>
</div> </div>
) : (
<span
className="inline-flex items-center gap-1.5 self-start sm:self-center px-3 py-1.5 rounded-xl text-[11px] font-mono text-slate-400 bg-surface-container border border-brand-border/60"
title="Per-rank permission customization is not yet available. Software access is enforced by Owner and Admin roles."
>
<span className="material-symbols-outlined text-[15px]">visibility</span>
View-only reference
</span>
)} )}
</div> </div>
{/* Truthful availability notice: shown whenever persistence is not wired. */}
{!editable && (
<div className="flex items-start gap-2.5 p-3 rounded-xl bg-amber-500/10 border border-amber-500/30 text-xs font-body text-amber-200/90">
<span className="material-symbols-outlined text-[16px] text-amber-400 shrink-0 mt-0.5">
info
</span>
<span>
This matrix shows MotorSignal&rsquo;s standard rank conventions for reference. Custom per-rank
permissions are not saved yet. Who can actually manage this club is enforced by the{' '}
<strong className="font-semibold text-amber-100">Owner</strong> and{' '}
<strong className="font-semibold text-amber-100">Admin</strong> roles in the member roster — see
Ranks &amp; Roles above. Per-rank customization is planned.
</span>
</div>
)}
{/* Desktop / Tablet Matrix Table */} {/* Desktop / Tablet Matrix Table */}
<div className="overflow-x-auto rounded-xl border border-brand-border/60 bg-surface-container/60"> <div className="overflow-x-auto rounded-xl border border-brand-border/60 bg-surface-container/60">
<table className="w-full text-left border-collapse min-w-[720px]"> <table className="w-full text-left border-collapse min-w-[720px]">
@ -142,17 +187,22 @@ export function ClubPermissionMatrix({
{capabilities.map((cap) => { {capabilities.map((cap) => {
const isGranted = rankPerms[cap.id] ?? false; const isGranted = rankPerms[cap.id] ?? false;
const isImmutableOwner = rank.id === 'president'; const isImmutableOwner = rank.id === 'president';
const toggleDisabled = !editable || isImmutableOwner;
return ( return (
<td key={cap.id} className="py-3.5 px-3 text-center"> <td key={cap.id} className="py-3.5 px-3 text-center">
<button <button
type="button" type="button"
disabled={!canEdit || isImmutableOwner} disabled={toggleDisabled}
onClick={() => onTogglePermission(rank.id, cap.id)} aria-readonly={!editable}
aria-label={`Toggle ${cap.label} for ${rank.title}`} onClick={() => {
className={`relative inline-flex h-5 w-9 shrink-0 cursor-pointer rounded-full border-2 border-transparent transition-colors duration-200 ease-in-out focus:outline-none disabled:opacity-75 disabled:cursor-not-allowed ${ if (toggleDisabled) return;
isGranted ? 'bg-brand-cyan' : 'bg-slate-700' onTogglePermission(rank.id, cap.id);
}`} }}
aria-label={`${editable ? 'Toggle' : 'Standard setting:'} ${cap.label} for ${rank.title}`}
className={`relative inline-flex h-5 w-9 shrink-0 rounded-full border-2 border-transparent transition-colors duration-200 ease-in-out focus:outline-none disabled:opacity-75 disabled:cursor-not-allowed ${
editable ? 'cursor-pointer' : 'cursor-default'
} ${isGranted ? 'bg-brand-cyan' : 'bg-slate-700'}`}
> >
<span <span
className={`pointer-events-none inline-block h-4 w-4 transform rounded-full bg-black shadow ring-0 transition duration-200 ease-in-out ${ className={`pointer-events-none inline-block h-4 w-4 transform rounded-full bg-black shadow ring-0 transition duration-200 ease-in-out ${

View File

@ -0,0 +1,98 @@
# Backend Contract Proposal — Club Rank Permission Matrix (WEB-W1)
Status: **Proposed / not implemented.** This documents the backend work required
before the "Operational Permission Matrix" on
`app/clubs/[clubId]/manage/ranks-roles/page.tsx` can be made editable. Until this
ships, the matrix is rendered read-only (`persistenceAvailable={false}`).
## Problem
`handleSavePermissions` previously waited ~400 ms and showed a "Saved" state
without writing anything (confirmed in source prior to this change). There is no
Firestore document, API route, or security rule that stores or enforces a
per-rank capability matrix. The enforced club authorization model is **role
based**, derived solely from the administrative role on the member document:
- `lib/club-data.ts` → `getPortalPermissions(role)` maps `owner`/`admin` →
`canManageSettings`, `canInviteMembers`, `canApproveApplications`,
`canPlanRides`; `member` → all false.
- Display rank (`President`, `Road Captain`, …) is cosmetic and explicitly does
**not** grant software authority.
Persisting the matrix without enforcement would mislead club owners into
believing a toggle restricts members when it does nothing. That is prohibited by
the project rules (no unenforced permission models, no false success).
## Why this was not implemented in the website task
- Enforcement requires **Firestore security rule** changes (out of scope / frozen).
- A new persistence model must not conflict with the existing Flutter club role
model (`owner`/`admin`/`member`). Rank → capability is a different axis and
needs a product decision on precedence.
## Proposed contract (for a future, authorized backend change)
### 1. Storage
Document: `clubs/{clubId}/settings/rankPermissions`
```jsonc
{
"version": 1,
"updatedAt": "<serverTimestamp>",
"updatedByUid": "<uid>",
"ranks": {
"road_captain": {
"planRides": true,
"manageApplications": false,
"postAnnouncements": true,
"moderateWall": true,
"manageRoster": false,
"editProfile": false
}
// ... one entry per rank id in DEFAULT_CLUB_RANKS except `president`
}
}
```
Invariants:
- `president` is immutable (always full authority); never written or editable.
- Unknown rank ids and unknown capability keys are rejected.
- Capability keys are constrained to `DEFAULT_CLUB_CAPABILITIES` ids.
### 2. Security rules (requires authorized change to the frozen rules)
- Read: any active club member.
- Write: only `clubs/{clubId}.ownerUid == request.auth.uid`, or an active member
whose `role == 'admin'`. Validate the written shape (version, allowed rank ids,
boolean capability values only). Reject writes that would alter `president`.
### 3. Enforcement precedence (product decision required)
Rank capabilities may only **narrow**, never widen, what the administrative role
already denies. Administrative `role` remains the hard security boundary enforced
in rules; rank capabilities are a soft, owner-configurable delegation layer that
the Flutter and web clients honor for UI affordances. The Flutter club model must
adopt the same precedence so web and mobile agree.
### 4. Website wiring once the above exists
1. Add `fetchRankPermissions(clubId)` and `saveRankPermissions(clubId, callerUid, matrix)`
to `lib/club-data.ts`, mirroring the authorization pattern of
`updateClubAccessSettings` (owner/admin check + `updateDoc`).
2. In `ranks-roles/page.tsx`: load persisted permissions in `loadData`, and
implement `handleSavePermissions` to call `saveRankPermissions`, then re-read
to confirm the write survived before showing success.
3. Flip `<ClubPermissionMatrix persistenceAvailable={true} />` and pass
`onSavePermissions` / `onResetDefaults` / `isSaving`.
4. Add tests: persistence round-trip (save → reload → values survive), owner/admin
authorization success, member/unauthenticated rejection, `president`
immutability, and rejection of unknown rank/capability keys.
## Current truthful behavior (shipped in this branch)
- Matrix renders as a read-only reference of the standard governance model.
- No Save/Reset controls; a "View-only reference" indicator and an availability
notice explain that access is enforced by Owner/Admin roles and that per-rank
customization is planned.
- No save is attempted and no success is reported.

View File

@ -119,10 +119,8 @@ describe('WEB-13: ClubHierarchyBuilder & ClubPermissionMatrix', () => {
expect(handleUpdateMemberRank).toHaveBeenCalledWith('rider-03', expect.any(String)); expect(handleUpdateMemberRank).toHaveBeenCalledWith('rider-03', expect.any(String));
}); });
it('renders permission matrix table with capabilities and allows toggling', () => { it('renders permission matrix capabilities for reference', () => {
const handleToggle = vi.fn(); const handleToggle = vi.fn();
const handleReset = vi.fn();
const handleSave = vi.fn().mockResolvedValue(undefined);
render( render(
<ClubPermissionMatrix <ClubPermissionMatrix
@ -131,9 +129,6 @@ describe('WEB-13: ClubHierarchyBuilder & ClubPermissionMatrix', () => {
permissions={DEFAULT_RANK_PERMISSIONS} permissions={DEFAULT_RANK_PERMISSIONS}
selectedRankId="road_captain" selectedRankId="road_captain"
onTogglePermission={handleToggle} onTogglePermission={handleToggle}
onResetDefaults={handleReset}
onSavePermissions={handleSave}
isSaving={false}
canEdit={true} canEdit={true}
/> />
); );
@ -143,15 +138,50 @@ describe('WEB-13: ClubHierarchyBuilder & ClubPermissionMatrix', () => {
expect(screen.getByText('Review Applications')).toBeDefined(); expect(screen.getByText('Review Applications')).toBeDefined();
expect(screen.getByText('Post Announcements')).toBeDefined(); expect(screen.getByText('Post Announcements')).toBeDefined();
expect(screen.getByText('Moderate Club Wall')).toBeDefined(); expect(screen.getByText('Moderate Club Wall')).toBeDefined();
// Find toggle button for road captain and click it
const toggleButton = screen.getByRole('button', { name: 'Toggle Plan Club Rides for Road Captain' });
fireEvent.click(toggleButton);
expect(handleToggle).toHaveBeenCalledWith('road_captain', 'planRides');
}); });
it('triggers save matrix and reset defaults', async () => { // WEB-W1 regression: without a persistence backend the matrix must be an honest
// read-only reference. It must not offer a Save control, must not report a save,
// and must not mutate permissions — even for an Owner/Admin (canEdit=true).
it('is read-only and never reports a false save when persistence is unavailable', () => {
const handleToggle = vi.fn();
const handleSave = vi.fn().mockResolvedValue(undefined);
render(
<ClubPermissionMatrix
ranks={DEFAULT_CLUB_RANKS}
capabilities={DEFAULT_CLUB_CAPABILITIES}
permissions={DEFAULT_RANK_PERMISSIONS}
selectedRankId="road_captain"
onTogglePermission={handleToggle}
onSavePermissions={handleSave}
canEdit={true}
// persistenceAvailable defaults to false
/>
);
// No save/reset affordances are rendered.
expect(screen.queryByRole('button', { name: /Save Matrix/i })).toBeNull();
expect(screen.queryByRole('button', { name: /Reset Defaults/i })).toBeNull();
expect(screen.queryByText(/Saved/)).toBeNull();
// A truthful view-only indicator and availability notice are present.
expect(screen.getByText(/View-only reference/i)).toBeDefined();
expect(screen.getByText(/not saved yet/i)).toBeDefined();
// Toggles are disabled and do not mutate permissions.
const toggleButton = screen.getByRole('button', {
name: /Standard setting:\s*Plan Club Rides for Road Captain/i,
});
expect((toggleButton as HTMLButtonElement).disabled).toBe(true);
fireEvent.click(toggleButton);
expect(handleToggle).not.toHaveBeenCalled();
expect(handleSave).not.toHaveBeenCalled();
});
// Forward-compatible editable path: when a persistence backend is wired AND the
// caller may edit, the matrix exposes real Save/Reset controls and toggles.
it('allows editing and saving only when persistence is available and canEdit', async () => {
const handleToggle = vi.fn(); const handleToggle = vi.fn();
const handleReset = vi.fn(); const handleReset = vi.fn();
const handleSave = vi.fn().mockResolvedValue(undefined); const handleSave = vi.fn().mockResolvedValue(undefined);
@ -167,9 +197,14 @@ describe('WEB-13: ClubHierarchyBuilder & ClubPermissionMatrix', () => {
onSavePermissions={handleSave} onSavePermissions={handleSave}
isSaving={false} isSaving={false}
canEdit={true} canEdit={true}
persistenceAvailable={true}
/> />
); );
const toggleButton = screen.getByRole('button', { name: 'Toggle Plan Club Rides for Road Captain' });
fireEvent.click(toggleButton);
expect(handleToggle).toHaveBeenCalledWith('road_captain', 'planRides');
const resetBtn = screen.getByRole('button', { name: /Reset Defaults/i }); const resetBtn = screen.getByRole('button', { name: /Reset Defaults/i });
fireEvent.click(resetBtn); fireEvent.click(resetBtn);
expect(handleReset).toHaveBeenCalled(); expect(handleReset).toHaveBeenCalled();