Skip to content

SEP-1844: Hide per-app write controls from non-admin users so the UI matches what the API allows - #1387

Open
nachodd wants to merge 2 commits into
mainfrom
SEP-1844
Open

SEP-1844: Hide per-app write controls from non-admin users so the UI matches what the API allows#1387
nachodd wants to merge 2 commits into
mainfrom
SEP-1844

Conversation

@nachodd

@nachodd nachodd commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

What

A non-admin signed in to SEP saw create / execute / stop / retry / delete controls that answer 403 now that the server enforces per-route roles. This hides them, so the UI offers only what the session can actually do.

The gap was structural, not an oversight per control. useAuth lived in @sep/shell, and the workspace dependency direction is shell -> apps -> framework -> api — neither the app packages nor the framework could reach it. Exactly one app (snippets) was role-aware, and only because a shell-side wrapper read the flag and threaded it down as a prop.

How

The context moved down the graph rather than the boolean being threaded across it. @sep/api has zero @sep dependencies, already declares React as a peer and hosts the query hooks, and every consuming package already depends on it — so no new workspace package and no package.json changes. It now owns AuthContext + useAuth() (frontend/packages/api/src/auth-context.ts); @sep/shell keeps the provider and all session/token state, and re-exports useAuth so shell-local imports are unchanged.

A missing provider resolves to a signed-out, non-admin session instead of throwing, so the many test and Storybook renders that mount these components bare keep working. That trades a loud failure for a silent one, so a once-per-bundle dev-mode warning fires when it happens.

Gated controls read canMutate, never isAdmin. The derivation lives in one function, deriveCanMutate. isAdmin survives only where the question really is "is this an administrator": the shell's Settings and Admin Apps guards and their enabled: isAdmin query suppression. (The AtwApp registry wrapper still passes isAdmin as a prop — that one gates a link to the Settings page, an admin-identity concern, and was left alone.)

Gating the framework's shared components covers every schema-driven app at once. The bespoke apps are swept at their rendering call sites, not on the hooks, so a hidden control issues no request and the hooks stay usable unchanged on the admin path.

Layer Gated
framework AppCreatePage, AppTaskEditPage, SchemaDrivenApp's entity edit (whole-page guard); AppListPage create + row delete; AppDetailPage execute / edit / delete, and the whole action bar when nothing is left in it; ScheduledTasksPanel + ScheduledTaskRow; TaskHistoryTable stop; SnippetExecutionAccordion execute form; SchemaListView actions column
atw create / rename / delete / close / reopen, batch execute, send + re-send, and the execution selection feeding them
alerts push / restore / PagerDuty, and the template selection feeding push
report start PDF job, start upload job
inventory sync, connectivity probe, nested-list row delete, schedule attach / edit / clear / enable
dipper execute form
topology collect
tasks, dipper stop, via the shared TaskHistoryTable

Reads are untouched. The server gate keys on HTTP method, so every GET still succeeds for a non-admin and no new query suppression was needed. One query is now avoided: the snippet accordion no longer fetches a form schema it will not render.

snippets collapses onto the hook and the bespoke SnippetsApp wrapper in appRegistry.tsx is deleted.

Hiding a control is not enforcement. The backend gate stays the only boundary.

Acceptance criteria

  • AC1 — useAuth() and isAdmin importable by @sep/framework and every apps/* package with no dependency on @sep/shell; same module exports canMutate.
  • AC2 — outside a provider, resolves to UNAUTHENTICATED_SESSION instead of throwing.
  • AC3 — all eight framework surfaces render no create / execute / stop / retry / delete for a non-admin.
  • AC4 — all seven bespoke apps swept.
  • AC5 — admin behaviour unchanged; diffs are && canMutate on existing markup, so the admin branch is byte-identical.
  • AC6 — no new query for a non-admin, and no new suppression (one query removed).
  • AC7 — snippets reads the hook; registry wrapper deleted.
  • AC8 — both halves asserted for each gated surface.
  • AC9 — every gated control reads canMutate; a repo-wide check shows no isAdmin left in a mutation decision.

Notes for the reviewer

SEP-1850 has already merged (214e45f52), so the server resolves a minimum role per route and User.role is already on the wire. AC9 assumes it has not. I kept deriveCanMutate as isAdmin per the ticket: most unsafe routes still require admin, so keying on a lesser role here would put back the controls that answer 403. The one real consequence is that an editor — who the server would accept on Alert Templates' push and restore — is offered no button for them. Widening this is now a change to that one function plus a per-control minimum role, which is exactly the seam this PR builds. Called out in the derivation comment and the changelog fragment.

Two judgement calls beyond the ticket, both needed to keep the suite honest:

  • The 20 e2e specs stubbed isAdmin: false while clicking create / execute / delete. The flag did not previously affect app pages; now it does. Flipped to admin, which is what those specs were written to exercise.
  • Added an admin-session Storybook decorator, otherwise every story would render its read-only state.

Out of scope, per the ticket: reporting a refusal when a 403 still happens (SEP-1845), granular per-action permissions, redesigning the sparse read-only pages that remain, and server-driven per-app capability flags.

One tradeoff recorded in the module doc: the session and its derived capability share one context, so a silent-refresh token rotation re-renders capability consumers too. That is a handful of controls every few minutes; splitting the capability into its own context is the fix if it ever costs more than it saves.

Test plan

  • pnpm -r type-check — clean across all 17 packages.
  • pnpm -r test — 1426 passing. New both-halves coverage for every gated surface: framework (AppCreatePage, AppTaskEditPage, AppListPage, AppDetailPage task + entity, SchemaDrivenApp, ScheduledTasksPanel, TaskHistoryTable, SnippetExecutionAccordion, SchemaListView), apps (atw list / workspace / results / collect, alerts, report, inventory sync / connectivity / schedule, dipper, topology, snippets), plus the capability derivation and the missing-provider fallback.
  • pnpm test:e2e — 132 passing.
  • oxlint 0 errors, oxfmt --check clean.

Reviewed by two independent reviewers before opening. One returned no defects; the other found a real one and three design points, all addressed in this branch: the shared SchemaListView left a header-only actions column for non-admins (now dropped when no delete handler is wired, with a regression test); the page guards had no way back (back chrome kept); the read-only wording was forked between the guard and the snippet accordion (one component, new inline variant); and the capability derivation had no test in its own package (added).

@nachodd
SEP-1844: Hide per-app write controls from non-admin sessions
f20e939 
The React shell gated its own Settings and Admin Apps pages on
`useAuth().isAdmin`, but no per-app control did, so a non-admin was
offered create / execute / stop / retry / delete buttons that answer 403
now that the server enforces them. The gap was structural rather than an
oversight per control: `useAuth` lived in `@sep/shell`, and the workspace
dependency direction is shell -> apps -> framework -> api, so neither the
app packages nor the framework could reach it. Exactly one app was
role-aware, and only via a shell-side wrapper threading a boolean down as
a prop.

Move the context down the graph instead of threading the boolean across
it. `@sep/api` has no `@sep` dependencies, already carries React as a peer
and hosts the query hooks, and every consuming package already depends on
it, so it hosts the context and `useAuth` while `@sep/shell` keeps owning
the provider and all session/token state. A missing provider now resolves
to a signed-out, non-admin session instead of throwing, so the many test
and Storybook renders that mount these components bare keep working; a
once-per-bundle dev warning keeps that from failing silently.

Gated controls read a derived `canMutate`, never `isAdmin`. The server
already resolves a minimum role per route rather than one administrator
flag, so widening the UI to match is an edit to `deriveCanMutate` and to
no call site. `isAdmin` survives only where the question really is
"is this an administrator": the shell's two page guards and their
`enabled: isAdmin` query suppression.

Gating the framework's shared components covers every schema-driven app
at once; the bespoke apps' local mutations are swept at their rendering
call sites, so a hidden control issues no request and the hooks stay
usable unchanged on the admin path. Selection affordances are hidden
alongside the action they feed, so no bulk toolbar is left stranded. The
create and edit pages keep their back chrome behind the guard state,
since nothing links a read-only session there and anyone who arrives did
so by URL. `SchemaListView` now drops an `actions` column with no delete
handler rather than rendering a header over empty cells.

Reads are untouched: the server gate keys on HTTP method, so every GET
still succeeds for a non-admin, and no query suppression was added. This
is what the UI advertises, never a security control -- the backend gate
remains the only boundary.

Also collapses the snippets prop-threading onto the hook and deletes the
registry wrapper that fed it. The e2e harness mocked `isAdmin: false`
while driving write controls, which the flag did not previously affect;
those stubs now say admin, matching what the specs exercise.

Addressed review feedback: dropped the dead `actions` column in the
shared list view, kept back navigation on the page guards, moved the
read-only wording into one component so it cannot drift, and added direct
tests for the capability derivation and the missing-provider fallback.
@nachodd
nachodd requested review from a team, peter-o-addo and yyyyyyyan as code owners August 21, 2026 01:11
Copilot AI balanced review requested due to automatic review settings August 21, 2026 01:11
@github-actions github-actions Bot added frontend app:alert_troubleshooting PR touches the alert_troubleshooting app slice app:alerts PR touches the alerts app slice app:alters PR touches the alters app slice app:archives PR touches the archives app slice app:atw PR touches the atw app slice app:backup_mongo PR touches the backup_mongo app slice app:backup_pg PR touches the backup_pg app slice app:dipper PR touches the dipper app slice app:inventory PR touches the inventory app slice app:mysql_backups PR touches the mysql_backups app slice app:report PR touches the report app slice app:snippets PR touches the snippets app slice app:tasks PR touches the tasks app slice app:topology PR touches the topology app slice large-diff Over 1500 changed lines, generated files discounted labels Aug 21, 2026
Copilot AI reviewed Aug 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Centralizes frontend authorization capability state and hides mutation controls from non-admin users across shared framework components and bespoke apps.

Changes:

  • Moves AuthContext and useAuth() into @sep/api, deriving canMutate.
  • Gates create, execute, edit, delete, stop, scheduling, and integration controls.
  • Adds broad unit/E2E coverage and read-only messaging.

Reviewed changes

Copilot reviewed 77 out of 77 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
frontend/packages/shell/src/contexts/auth.tsx Uses shared API auth context.
frontend/packages/shell/src/contexts/auth.test.tsx Tests capability derivation.
frontend/packages/shell/src/appRegistry.tsx Removes snippets auth wrapper.
frontend/packages/framework/src/index.ts Exports read-only notice.
frontend/packages/framework/src/components/TaskHistoryTable/TaskHistoryTable.tsx Gates task stopping.
frontend/packages/framework/src/components/TaskHistoryTable/TaskHistoryTable.test.tsx Tests stop visibility.
frontend/packages/framework/src/components/SnippetExecutionAccordion/SnippetExecutionAccordion.tsx Gates snippet execution.
frontend/packages/framework/src/components/SnippetExecutionAccordion/SnippetExecutionAccordion.test.tsx Tests read-only execution behavior.
frontend/packages/framework/src/components/SchemaListView/SchemaListView.tsx Removes empty action columns.
frontend/packages/framework/src/components/SchemaListView/SchemaListView.test.tsx Tests action-column removal.
frontend/packages/framework/src/components/SchemaDrivenApp/SchemaDrivenApp.tsx Guards entity editing.
frontend/packages/framework/src/components/SchemaDrivenApp/SchemaDrivenApp.test.tsx Tests entity edit access.
frontend/packages/framework/src/components/SchemaDrivenApp/AppTaskEditPage.tsx Guards task editing.
frontend/packages/framework/src/components/SchemaDrivenApp/AppTaskEditPage.test.tsx Tests task edit access.
frontend/packages/framework/src/components/SchemaDrivenApp/AppListPage.tsx Gates creation and deletion.
frontend/packages/framework/src/components/SchemaDrivenApp/AppListPage.test.tsx Tests list write controls.
frontend/packages/framework/src/components/SchemaDrivenApp/AppDetailPage.tsx Gates detail-page actions.
frontend/packages/framework/src/components/SchemaDrivenApp/AppDetailPage.test.tsx Tests detail action visibility.
frontend/packages/framework/src/components/SchemaDrivenApp/AppCreatePage.tsx Guards creation pages.
frontend/packages/framework/src/components/SchemaDrivenApp/AppCreatePage.test.tsx Tests create-page access.
frontend/packages/framework/src/components/ScheduledTasksPanel/ScheduledTasksPanel.tsx Gates schedule mutations.
frontend/packages/framework/src/components/ScheduledTasksPanel/ScheduledTasksPanel.test.tsx Tests schedule controls.
frontend/packages/framework/src/components/ScheduledTasksPanel/ScheduledTaskRow.tsx Adds read-only schedule rows.
frontend/packages/framework/src/components/ReadOnlyNotice/ReadOnlyNotice.tsx Adds shared permission notice.
frontend/packages/framework/src/components/ReadOnlyNotice/index.ts Exports notice component.
frontend/packages/framework/.storybook/preview.tsx Supplies admin story context.
frontend/packages/e2e/tests/tasks.spec.ts Updates task test session.
frontend/packages/e2e/tests/snippets-list-filters.spec.ts Updates snippets test session.
frontend/packages/e2e/tests/snippets-download.spec.ts Updates download test session.
frontend/packages/e2e/tests/sidebar-navigation.spec.ts Updates navigation test session.
frontend/packages/e2e/tests/shell.spec.ts Updates shell test session.
frontend/packages/e2e/tests/schema-form-empty-state.spec.ts Updates form test session.
frontend/packages/e2e/tests/report.spec.ts Updates report test session.
frontend/packages/e2e/tests/mysql-backups.spec.ts Updates backup test session.
frontend/packages/e2e/tests/inventory.spec.ts Updates inventory test session.
frontend/packages/e2e/tests/inventory-sync.spec.ts Updates sync test session.
frontend/packages/e2e/tests/inventory-schedules.spec.ts Updates schedule test session.
frontend/packages/e2e/tests/backup_pg.spec.ts Updates PostgreSQL backup session.
frontend/packages/e2e/tests/backup_mongo.spec.ts Updates MongoDB backup session.
frontend/packages/e2e/tests/atw.spec.ts Updates ATW test session.
frontend/packages/e2e/tests/archives.spec.ts Updates archives test session.
frontend/packages/e2e/tests/app-disabled.spec.ts Updates disabled-app test session.
frontend/packages/e2e/tests/alters.spec.ts Updates alters test session.
frontend/packages/e2e/tests/alerts.spec.ts Updates alerts test session.
frontend/packages/e2e/tests/alert-troubleshooting.spec.ts Updates troubleshooting session.
frontend/packages/e2e/tests/_template.spec.ts Updates E2E template session.
frontend/packages/apps/topology/src/TopologyView.tsx Gates topology collection.
frontend/packages/apps/topology/src/TopologyView.test.tsx Tests topology access.
frontend/packages/apps/snippets/src/SnippetsListPage.tsx Uses shared mutation capability.
frontend/packages/apps/snippets/src/SnippetsListPage.test.tsx Tests snippets write controls.
frontend/packages/apps/snippets/src/SnippetsApp.tsx Removes auth prop threading.
frontend/packages/apps/report/tests/ReportResultPage.test.tsx Tests report actions.
frontend/packages/apps/report/src/ReportResultPage.tsx Gates report jobs.
frontend/packages/apps/inventory/src/SyncControl.tsx Gates inventory synchronization.
frontend/packages/apps/inventory/src/SyncControl.test.tsx Tests sync visibility.
frontend/packages/apps/inventory/src/InventorySchedulePage.tsx Gates inventory schedules.
frontend/packages/apps/inventory/src/InventorySchedulePage.test.tsx Tests schedule access.
frontend/packages/apps/inventory/src/InventoryAppNavigation.tsx Gates nested deletion.
frontend/packages/apps/inventory/src/ConnectivityControl.tsx Gates connectivity probes.
frontend/packages/apps/inventory/src/ConnectivityControl.test.tsx Tests probe visibility.
frontend/packages/apps/dipper/src/DipperApp.tsx Gates diagnostic execution.
frontend/packages/apps/dipper/src/DipperApp.test.tsx Tests Dipper access.
frontend/packages/apps/atw/tests/ResultsPane.test.tsx Tests send controls.
frontend/packages/apps/atw/tests/IncidentWorkspacePage.test.tsx Tests lifecycle controls.
frontend/packages/apps/atw/tests/IncidentListPage.test.tsx Tests incident mutations.
frontend/packages/apps/atw/tests/CollectPane.test.tsx Adds auth test setup.
frontend/packages/apps/atw/tests/CollectPane.search.test.tsx Tests batch execution access.
frontend/packages/apps/atw/src/ResultsPane.tsx Gates sending and selection.
frontend/packages/apps/atw/src/IncidentWorkspacePage.tsx Gates incident lifecycle actions.
frontend/packages/apps/atw/src/IncidentListPage.tsx Gates incident mutations.
frontend/packages/apps/atw/src/CollectPane.tsx Gates batch execution.
frontend/packages/apps/alerts/tests/AlertsListPage.test.tsx Tests alert write controls.
frontend/packages/apps/alerts/src/AlertsListPage.tsx Gates alert mutations.
frontend/packages/api/tests/auth-context.test.ts Tests shared auth fallback.
frontend/packages/api/src/index.ts Exports auth context API.
frontend/packages/api/src/auth-context.ts Defines shared auth capability state.
changelog.d/SEP-1844.changed.md Documents read-only UI behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread frontend/packages/e2e/tests/snippets-download.spec.ts Outdated
Comment thread frontend/packages/e2e/tests/sidebar-navigation.spec.ts Outdated
Comment thread frontend/packages/api/src/auth-context.ts
Comment thread frontend/packages/framework/src/components/ReadOnlyNotice/ReadOnlyNotice.tsx
Comment thread frontend/packages/shell/src/contexts/auth.tsx
Comment thread ...nd/packages/framework/src/components/SnippetExecutionAccordion/SnippetExecutionAccordion.tsx
@nachodd
SEP-1844: Gate the snippet execute form on the capability, not the qu…
dbe0a69 
…ery flag

Address review feedback.

The snippet accordion withheld its execute form from a read-only session
by disabling the schema query, but a disabled query still serves a cached
entry. The schema is held with `staleTime: Infinity` under a key carrying
no identity, and the shell never clears the query cache, so an admin's
fetch would render the form for a non-admin opening the same snippet
later in the same tab. The render now gates on `canMutate` as well; the
query flag stays as the request optimization. Covered by a test that
populates the cache as an admin and re-renders read-only, which fails
without the render gate.

Also restores two e2e session fixtures to non-admin. The snippet-download
spec exercises a GET that stays permitted, and the sidebar-navigation
specs only navigate and assert readable sentinels, so leaving both
non-admin keeps end-to-end coverage of the read-only experience this
change creates rather than broadening privileges the specs never use.
@nachodd nachodd mentioned this pull request Aug 21, 2026
Open
@nachodd nachodd added the qa passed Tests for this PR are completed and successful. label Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@peter-o-addo peter-o-addo Awaiting requested review from peter-o-addo peter-o-addo is a code owner
@yyyyyyyan yyyyyyyan Awaiting requested review from yyyyyyyan yyyyyyyan is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@nachodd nachodd

Labels

app:alert_troubleshooting PR touches the alert_troubleshooting app slice app:alerts PR touches the alerts app slice app:alters PR touches the alters app slice app:archives PR touches the archives app slice app:atw PR touches the atw app slice app:backup_mongo PR touches the backup_mongo app slice app:backup_pg PR touches the backup_pg app slice app:dipper PR touches the dipper app slice app:inventory PR touches the inventory app slice app:mysql_backups PR touches the mysql_backups app slice app:report PR touches the report app slice app:snippets PR touches the snippets app slice app:tasks PR touches the tasks app slice app:topology PR touches the topology app slice frontend large-diff Over 1500 changed lines, generated files discounted qa passed Tests for this PR are completed and successful.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nachodd