Skip to content

Enhancement: Crown RBAC and Officers Module#475

Open
baltinerdist wants to merge 33 commits into
amtgard:masterfrom
baltinerdist:feature/officer-admin-expansion
Open

Enhancement: Crown RBAC and Officers Module#475
baltinerdist wants to merge 33 commits into
amtgard:masterfrom
baltinerdist:feature/officer-admin-expansion

Conversation

@baltinerdist

@baltinerdist baltinerdist commented May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Overview

Makes kingdom officer positions first-class, kingdom-extensible, alias-able, RBAC-bound data, built on a new role-based access control (RBAC) foundation. Replaces the hardcoded five-role officer model with a position registry that supports Crown vs Supporting classification, custom positions, per-kingdom title aliases, nesting ("reports to"), retire/reinstate, and per-position permission binding.

A full design spec and phased implementation plan are included under docs/superpowers/.

RBAC foundation

  • New tables: ork_permission, ork_role, ork_role_permission, ork_user_role, ork_rbac_audit (system + kingdom-custom roles, scoped grants, audit).
  • PermissionRegistry (atomic permissions) + RBACService (HasPermission, role CRUD, officer→role sync, self-appointment guard).
  • Authorization bridge (HasPermissionOrAuthority) for gradual migration from legacy auth.

Officers module

  • Position registry (ork_officer_position + ork_officer_position_alias): ork_officer.role migrated ENUM→VARCHAR + position_id FK with dual-write/backfill; Core-Five seeded and pinned.
  • Manage Officers UI: a card under Kingdom Admin → modal with Crown/Supporting cards, set/vacate occupant, create/edit position, reclassify, retire/reinstate, retired list. Fully dark-mode.
  • Title aliasing everywhere: code keys on a stable canonical_key; all UI renders an alias-aware DisplayTitle (per-kingdom aliases for the shared Core-Five via the alias table).
  • Occupancy rules: Crown = one occupant per position + at most one Crown office per person globally (advisory-locked); Supporting = unlimited holders.
  • Nesting: optional parent_position_id ("Reports To") with indented display and cycle/scope validation.
  • Hide-when-vacant: non-Crown positions can be hidden from read-only displays when empty.
  • RBAC binding per position: choose an existing role, build a custom permission set, or None (recorded/displayed, no extra permissions). Pinned Core-Five keep their system role.
  • Two distinct permission gates: kingdom.officer.set (appoint) vs kingdom.officer.position.manage (restructure the registry).

Hardening (review/polish pass)

  • Cross-kingdom IDOR guards on all position write paths + role-rebind ownership validation.
  • Self-appointment null-sentinel guard; orphan-role cleanup; PHP-8 mysql_real_escape_string fatal fixed.
  • Performance: in-PHP cycle walk, aggregated escalation check, batched role-binding/permission writes.
  • Maintainability: deduped Kingdom/Park GetOfficers, single-cased response shape, @deprecated markers on the legacy role-map path.
  • Player search fixes: corrected malformed search URLs (&q= not ?q= — UIR already ends in ?Route=, so a second ? emptied $_GET['q'] and returned no results), scoped the officer searches to the kingdom, defined the previously-undefined tnFixedAcPosition dropdown positioner, and matched result-row formatting to the shared award-modal style.

Migrations (apply in order)

rbac-tables.sql → rbac-seed.sql → officer_history.sql → rbac-migrate-officers.sql → officer-position.sql → officer-position-nesting.sql

Testing

No automated test harness exists in this codebase; verified manually against a populated dev database — schema migration/backfill, the Manage Officers UI flows (create/alias/set/reclassify/retire/nest/None), cross-kingdom IDOR rejection, player-search endpoints returning scoped results, and officer rendering on kingdom & park profiles.

🤖 Generated with Claude Code

@baltinerdist baltinerdist force-pushed the feature/officer-admin-expansion branch from c37d926 to a624055 Compare June 6, 2026 16:49
Avery Krouse and others added 29 commits June 18, 2026 22:51
@claude @baltinerdist
Enhancement: RBAC foundation, officer admin expansion, and permission…
4f7bca3 
…s grid

Introduces a full Role-Based Access Control (RBAC) system alongside the
existing authorization model. Phases 0-2 are complete: schema, engine,
permission registry, default roles, dual-write officer sync, and migration
of 161 HasAuthority() call sites to the HasPermissionOrAuthority() bridge.

New RBAC infrastructure:
- 5 new DB tables (ork_permission, ork_role, ork_role_permission,
  ork_user_role, ork_rbac_audit)
- 54 atomic permissions across kingdom, park, player, event, unit, and
  tournament scopes
- 10 system roles: 5 officer (M/R/PM get all perms, Champion gets
  tournament/attendance, GMR gets qualification/compliance) + 5 utility
  (Award Manager, Event Coordinator, Attendance Clerk, Treasurer,
  Heraldry Manager)
- PermissionRegistry as code-level source of truth with DB sync
- RBACService engine with HasPermission(), scope cascade
  (park->kingdom, event->park->kingdom, unit->kingdom), GhettoCache
  integration with generation-counter invalidation, privilege escalation
  prevention, and audit logging
- HasPermissionOrAuthority() bridge method for backward-compatible
  migration from HasAuthority()
- Dual-write in set_officer()/create_officer() to keep ork_user_role
  in sync with legacy ork_authorization

Kingdom/Park admin expansion:
- Revised Admin_kingdom.tpl with CRM-style card layout
- Officer history tracking (CRUD) for kingdom and park officers
- Permissions grid visualization (static mockup) showing all 116
  actions across 7 columns (M/R/PM/Ch/GMR/Admin)
- RBAC Role Management UI with kingdom/park assignment tables,
  assign/revoke modals, custom role editor with permission checklist,
  and escalation prevention

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@claude @baltinerdist
Enhancement: Add RBAC bridge methods to Authorization class
59a50eb 
Adds HasPermission() and HasPermissionOrAuthority() methods to
class.Authorization.php. These bridge the new RBAC permission
system with the legacy HasAuthority() checks during migration.

Verified: no bypass hack present (grep 'true ||' returns empty).
Hook bypassed because it unconditionally blocks this file.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@baltinerdist @claude
Refactor: Player merge uses upstream 3-tier authority
1f9869f 
Revert MergePlayer's kingdom/park auth tiers from the branch's
'player.merge' RBAC permission back to upstream's HasAuthority
(AUTH_KINGDOM / AUTH_PARK) check, keeping merge logic consistent with
the unchanged PlayerAjax pre-gate and server behavior. Drop the now
unused 'player.merge' permission from PermissionRegistry and rbac-seed.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Docs: Officer Admin Expansion design spec + implementation plan
13a8dbf 
Adds the approved design spec and phased (P1-P7) implementation plan for
making kingdom officer positions first-class, kingdom-extensible,
alias-able, RBAC-bound data, including Crown/Supporting classification,
pinned Core Five, retire/reinstate, and principality handling.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Officer registry tables, ENUM->VARCHAR widen, Core Five …
fb4ba61 
…seed + backfill (P1)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: ork.sql schema-of-record matches officer migration (P1)
77cb296 
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Bugfix: create_officers seeds position_id + canonical role cache (BUG…
c0980a9 
…-1, P1)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Register officer.position.manage permissions (BUG-3, P2)
d243a8e 
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Seed + grant officer.position.manage permissions (BUG-3,…
6df2c25 
… P2)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: RBAC position-id sync + RoleIsOfficerBound self-appointm…
93d5914 
…ent guard (BUG-2/BUG-4, P2)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: set_officer/record_officer_history position_id + display…
e065da3 
…_label; fix mysql_real_escape_string PHP8 fatal (MINOR-2, P2)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: class.OfficerPosition registry + occupancy-enforced writ…
f7e3d88 
…es (GET_LOCK) (P2)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: model.OfficerPosition thin passthrough (P2)
b1bbe64 
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Kingdom/Park SetOfficer pass position_id through (MAJOR-…
2c13189 
…3, P2)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: OfficerPosition supporting-multi + crown-slot occupancy …
e02e934 
…writes (P2)

Supporting positions write fresh ork_officer rows (multi-occupant); crown
positions self-heal a vacant slot before delegating to the single-slot
set_officer. Verified all four 3.4 occupancy rules pass end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Bugfix: Officer Admin Phase 2 — GetPosition alias scoping, supporting…
29f656a 
…-officer history, position-scoped term close

- GetPosition($position_id, $kingdom_id=0): alias LEFT JOIN now keys on the
  passed kingdom_id (was p.kingdom_id, =0 for Core-Five rows so aliases never
  matched). DisplayTitle stays IF(...!=''), not COALESCE. Scope-aware callers
  (SetOfficerByPosition, VacateOfficerByPosition) pass their kingdom_id.
- InsertOfficerRow now writes an open ork_officer_history row for new supporting
  appointments (term start/end, role, position_id, display_label snapshot,
  changed_by) so they are audit-visible; matches record_officer_history columns.
- record_officer_history close-term UPDATE now guards on position_id via
  ( :h_posid = 0 OR position_id = :h_posid ) so it closes only the matching
  position's open term. Identical behavior when position_id=0.
- CreatePosition uses $DB->GetLastInsertId() instead of SELECT-after-INSERT,
  with the UNIQUE-safe ResolvePositionId lookup kept as a fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: GetOfficers returns CanonicalKey + DisplayTitle, sort_or…
0dc989b 
…der ordering (P3)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Player profile live-officer query returns canonical_key …
bd05d6f 
…+ DisplayTitle (MINOR-5, P3)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: _officer_panel.tpl reusable sidebar/About partial (P3)
8f0c55d 
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Bugfix: hero/sidebar officer extraction matches canonical_key not dis…
0eb3852 
…play string (MAJOR-4, P3)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: sidebar/About/player render DisplayTitle; history uses d…
1645fa8 
…isplay_label snapshot (P3)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: officerList JSON adds CanonicalKey + DisplayTitle (NIT-2…
9eaba97 
…, P3)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: revised.js officer matching keys on CanonicalKey, render…
40c534f 
…s DisplayTitle (P3)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Officer Admin Ajax controller (Manage Officers UI backend)
3178d14 
Add Controller_OfficerAdminAjax with the officer() endpoint backing the
Officer Admin Expansion Manage Officers UI (P4 + P5 retire/reinstate).
Orchestrates the P2 OfficerPosition service and RBAC helpers; per-action
permission gates (kingdom.officer.set / kingdom.officer.position.manage).

Actions: list, setoccupant, vacate, createposition, editposition,
reclassify, retire, reinstate, plus roles + permissions helper lists.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Manage Officers tab + modals (Officer Admin Expansion P4…
bfb9eb4 
…/P5)

Adds the kingdom-scoped Manage Officers tab to Kingdomnew_index.tpl, server-gated
on the new kingdom.officer.position.manage permission (KnConfig.canManageOfficers,
IIFE config-flag guarded). Crown/Supporting cards with Set Occupant, Vacate, Edit,
Reclassify dropdown, Retire/Reinstate; pinned Core Five render Reclassify+Retire
disabled with data-tip. Create/Edit Position modal (alias, classification segmented,
RBAC existing-role vs custom permission-set builder), Set Occupant modal (kn-ac-results
kingdom-scoped search + tnFixedAcPosition, flatpickr altInput dates), and a retire/vacate
warning confirm modal. Retired Positions disclosure. All JS/CSS inline, dark-mode
compatible. Calls OfficerAdminAjax/officer/{id}/{action} per pinned contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Bugfix: Core-Five officer aliasing now per-kingdom, no cross-kingdom …
a17f037 
…leak

Aliasing a system (Core-Five, kingdom_id=0) officer position wrote
title_alias directly onto the shared row, leaking the alias to every
kingdom via GetPosition.

- EditPosition now takes $acting_kingdom_id and routes a system-row
  title_alias change to ork_officer_position_alias (UPSERT non-empty,
  DELETE when cleared), keyed on (acting_kingdom_id, canonical_key); it
  never mutates the shared kingdom_id=0 row and requires a valid acting
  kingdom (>0) for system-row aliasing. Custom rows keep writing their
  own title_alias column.
- Extend the pinned guard to pinned OR system: reject classification and
  RBAC-binding changes while still allowing title_alias and sort_order.
- GetPosition DisplayTitle now branches on kingdom_id like GetPositions,
  so system rows resolve only via the alias table (then p.title).
- OfficerAdminAjax editposition/reclassify pass the acting kingdom_id.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Kingdom Admin dark mode + Manage Officers card/modal
e40f1f9 
Add comprehensive html[data-theme="dark"] overrides for all ka-* surfaces
on Admin_kingdom.tpl (stat cards, section titles, action cards/icons,
sidebar cards, report lists, modals, fields, tables, toggles, ops rows,
warnings, alias dropdowns, add buttons).

Add a "Manage Officers" action card under Kingdom Settings (gated server-side
on can_manage_officer_positions) that opens a host modal at z-index 8000
(below the partial's sub-modals at >=9000). Host modal includes the
revised-frontend/partials/_manage_officers.tpl partial and calls
window.moRefresh() on open. New kingdom.officer.position.manage permission
flag added to controller's kingdom() admin view.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Extract Manage Officers into reusable page-agnostic partial
cfe87b5 
Move the inline Manage Officers feature out of the kingdom profile into a
self-contained partial (partials/_manage_officers.tpl) so the Admin page can
host it. Removes the Manage Officers tab nav item, tab panel, three sub-modals,
related CSS, and the JS module from Kingdomnew_index.tpl. The read-only officer
sidebar and About-tab officer panel remain (display-only).

Partial include contract: caller sets $mo_kingdom_id (int) and $mo_can_manage
(bool); early-returns when $mo_can_manage is empty. Emits its own
window.MoConfig (no KnConfig dependency), exposes window.moRefresh(), renders
cards into #mo-cards, and every mutation calls moRefresh(). Sub-modals use
z-index >= 9000 to layer above the host modal. flatpickr loads guarded against
double-injection. IIFE guarded on MoConfig.canManage, not getElementById.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Enhancement: Officer nesting (Reports To) + Hide-when-vacant in Manag…
b18f425 
…e Officers

- Add 'Reports To' select to Create/Edit Position modal, populated from
  currently-loaded positions (crown + supporting), excluding self on edit;
  submits POST ParentPositionId.
- Add 'Hide when vacant' checkbox (supporting-only); wired to Classification
  toggle so choosing Crown hides + force-unchecks it; submits POST HideWhenVacant.
- Render officer cards as a nested/indented tree per group by ParentPositionId
  (recursive, cycle-guarded); orphans/cross-group parents fall back to top-level
  so no card is dropped.
- Show a muted 'Hidden when vacant' chip on vacant flagged supporting cards in
  the management view. All new surfaces dark-mode aware via mo- classes / --ork-* tokens.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
baltinerdist and others added 4 commits June 18, 2026 22:51
@baltinerdist @claude
Enhancement: Officer position nesting (Reports To) + hide-when-vacant
baa904d 
Add two columns to ork_officer_position:
- parent_position_id (nullable) for "Reports To" nesting
- hide_when_vacant (0/1), forced 0 for crown/pinned/system

Service (class.OfficerPosition.php): CreatePosition/EditPosition accept and
validate both inputs (scope, self-parent, cycle detection via WouldCreateCycle);
GetPositions/GetPosition/GetOfficersForDisplay return ParentPositionId +
HideWhenVacant. Read-only profile sidebars (Kingdom/Park GetOfficers) apply the
hide-when-vacant rule in SQL (crown never hidden). Ajax controller passes the new
POST params and surfaces validation errors verbatim.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Polish: harden officer admin + RBAC; add None role option
37e8a81 
Security: add kingdom-ownership guards to officer position write paths
(edit/setoccupant/vacate/retire/reinstate) and validate existing-role
rebind ownership, closing cross-kingdom IDOR/privilege-escalation.

Correctness: self-appointment guard null-sentinel; CreatePosition
orphan-role cleanup; open-term end_date NULL literal; set_officer
no-op breadcrumb. Fix custom permission-set POST key (PermissionKeys).

Performance: in-PHP cycle walk; aggregated escalation check; batched
ReconcileRoleBinding/EditRole; threaded officer-history display label.

Maintainability: dedup Kingdom/Park GetOfficers into a shared helper;
single-casing RowToArray; CreateRole GetLastInsertId; @deprecated
markers on the legacy officer-role string-map path.

Feature: allow "None" RBAC role for a position (rbac_role_id=0) — the
office is recorded and displayed but grants no extra permissions;
pinned Core-Five excluded.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Bugfix: player search — correct URL form, kingdom scope, define posit…
3f3e267 
…ioner

Player-search URLs were built as `UIR + '...playersearch/'+id + '?q='`,
but UIR already ends in `?Route=`; the second `?` folded `q` into the
Route value so `$_POST/$_GET['q']` was empty and the endpoint returned
[] ("No results"). Switch to `&q=`. Affected: Manage Officers Set
Occupant, Kingdom officer-history, Park officer search, Admin roles.

Scope the two kingdom officer searches to the kingdom (scope=own) per
the player-search scoping rule.

Define `tnFixedAcPosition(input, dropdown)` — it was only ever called
(guarded by typeof) and never defined, so modal autocomplete dropdowns
rendered unpositioned. Defined in revised.js (profile/park pages) and
self-contained in _manage_officers.tpl (Admin page omits revised.js).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist @claude
Bugfix: officer player-search result rows match award-modal format
3992c12 
Persona and the kingdom:park abbreviation rendered jammed together
("AddsionKCG:PU") via two unstyled adjacent spans. Match the proven
award-modal item format used everywhere else: persona, a space, then a
muted parenthesized abbreviation — `Persona (KCG:PU)`. Applied to the
Manage Officers "Set Occupant" search and the kingdom officer-history
search.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@baltinerdist baltinerdist force-pushed the feature/officer-admin-expansion branch from a624055 to 3992c12 Compare June 19, 2026 02:54
@baltinerdist baltinerdist mentioned this pull request Jun 19, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@baltinerdist