Skip to content

Commit 41ccd17

Browse files
authored
SEP-1844: Hide per-app write controls from non-admin sessions
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.
1 parent 95a7038 commit 41ccd17

77 files changed

Lines changed: 2019 additions & 470 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

changelog.d/SEP-1844.changed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Non-administrator sessions no longer see per-app create, execute, stop, retry or delete controls in the React UI: every such control is hidden rather than offered and answered with a 403. This is a UI-only change and never a security boundary — the server-side gate is unchanged and remains the only one. Reads are untouched, so a non-administrator keeps every list, detail, log and history view; the pages they are left with are read-only, which for some apps means a sparse view with no actions. Which sessions this affects depends on the auth provider: Casdoor defaults new users to non-administrator, and the Grafana provider grants administrator only from `isGrafanaAdmin` or an Admin org role. The UI still keys on the administrator flag alone, so a session holding a lesser role that the server would accept (an `editor` on Alert Templates' push and restore) is offered no control for it yet.
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/**
2+
* Copyright (C) 2026 Percona LLC
3+
*
4+
* This program is free software: you can redistribute it and/or modify
5+
* it under the terms of the GNU Affero General Public License as published by
6+
* the Free Software Foundation, either version 3 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU Affero General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU Affero General Public License
15+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
16+
*/
17+
18+
import { createContext, useContext, useMemo } from 'react';
19+
import type { User } from './types/api';
20+
21+
/**
22+
* Session state owned by the shell's ``AuthProvider``.
23+
*
24+
* The context lives here, at the root of the frontend dependency graph, so the
25+
* framework and every app package can read it — they all depend on ``@sep/api``
26+
* and none of them may depend on ``@sep/shell``. The provider itself, and all
27+
* token/session bookkeeping, stays in the shell.
28+
*
29+
* One context carries both the session and the capability derived from it, so a
30+
* silent-refresh token rotation re-renders every capability consumer too. That
31+
* is a handful of controls every few minutes; splitting the capability into its
32+
* own context is the fix if it ever costs more than it saves.
33+
*/
34+
export interface AuthSession {
35+
user: User | null;
36+
accessToken: string | null;
37+
isAuthenticated: boolean;
38+
/**
39+
* Administrator identity. Read this only for genuinely admin-only surfaces
40+
* (the shell's Settings / Admin Apps pages and their query suppression). Per-app
41+
* write controls gate on {@link AuthState.canMutate} instead.
42+
*/
43+
isAdmin: boolean;
44+
/** true during initial session bootstrap & during login */
45+
loading: boolean;
46+
/** true after the initial session check finishes (success or failure) */
47+
ready: boolean;
48+
login: (username: string, password: string) => Promise<void>;
49+
logout: () => Promise<void>;
50+
}
51+
52+
/** {@link AuthSession} plus the capabilities derived from it. */
53+
export interface AuthState extends AuthSession {
54+
/**
55+
* Whether this session may mutate: the gate every per-app create / execute /
56+
* stop / retry / delete control reads.
57+
*
58+
* Semantically distinct from {@link AuthSession.isAdmin} even though it is
59+
* exactly that today. The server already resolves a minimum role per route
60+
* rather than one administrator flag (SEP-1850), and ``User`` carries that
61+
* role, so widening the UI to match is an edit to
62+
* {@link deriveCanMutate} — per-control minimum roles — and to no call site.
63+
*/
64+
canMutate: boolean;
65+
}
66+
67+
/**
68+
* Single derivation of "may this session mutate?" from session state.
69+
*
70+
* Deliberately the administrator flag and nothing finer: most unsafe routes
71+
* require ``admin``, so keying on a lesser role here would put back the
72+
* controls that answer 403. Widening this to a per-control minimum role is the
73+
* follow-up that {@link AuthState.canMutate} describes.
74+
*/
75+
export function deriveCanMutate(session: AuthSession): boolean {
76+
return session.isAdmin;
77+
}
78+
79+
/**
80+
* Resolved state for a consumer rendered outside an ``AuthProvider``: signed
81+
* out, non-admin, and therefore unable to mutate. Tests and Storybook renders
82+
* mount framework/app components without the shell's provider, so a missing
83+
* provider must degrade to the least-privileged state rather than throw.
84+
*/
85+
export const UNAUTHENTICATED_SESSION: AuthSession = Object.freeze({
86+
user: null,
87+
accessToken: null,
88+
isAuthenticated: false,
89+
isAdmin: false,
90+
loading: false,
91+
ready: false,
92+
login: async () => {},
93+
logout: async () => {},
94+
});
95+
96+
export const AuthContext = createContext<AuthSession | null>(null);
97+
98+
let warnedMissingProvider = false;
99+
100+
/**
101+
* Warn once per bundle when the provider is missing. Hiding controls is a
102+
* quieter failure than the throw this replaced, so a stray consumer mounted
103+
* outside the provider would otherwise silently look like a non-admin session.
104+
*/
105+
function warnMissingProvider(): void {
106+
if (warnedMissingProvider || !import.meta.env?.DEV) {
107+
return;
108+
}
109+
warnedMissingProvider = true;
110+
// eslint-disable-next-line no-console -- surface a silently degraded session in dev
111+
console.warn(
112+
'useAuth() was called outside an AuthProvider — falling back to a signed-out, ' +
113+
'non-admin session. Mutation controls will be hidden.',
114+
);
115+
}
116+
117+
/**
118+
* Read the current session and its derived capabilities.
119+
*
120+
* Resolves to {@link UNAUTHENTICATED_SESSION} when no provider is mounted.
121+
*/
122+
export function useAuth(): AuthState {
123+
const session = useContext(AuthContext);
124+
if (!session) {
125+
warnMissingProvider();
126+
}
127+
const resolved = session ?? UNAUTHENTICATED_SESSION;
128+
return useMemo(() => ({ ...resolved, canMutate: deriveCanMutate(resolved) }), [resolved]);
129+
}

frontend/packages/api/src/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ export { createQueryClient, defaultQueryClientConfig } from './queryClient';
3737
export { ApiError, normalizeAxiosError, parseFieldErrors } from './errors';
3838
export type { ApiErrorDetails, ApiErrorKind, FieldValidationError } from './errors';
3939

40+
// Auth context (provider lives in @sep/shell; the context lives here so the
41+
// framework and app packages can read it without depending on the shell)
42+
export { AuthContext, UNAUTHENTICATED_SESSION, deriveCanMutate, useAuth } from './auth-context';
43+
export type { AuthSession, AuthState } from './auth-context';
44+
4045
// Auth
4146
export {
4247
postLogin,
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/**
2+
* Copyright (C) 2026 Percona LLC
3+
*
4+
* This program is free software: you can redistribute it and/or modify
5+
* it under the terms of the GNU Affero General Public License as published by
6+
* the Free Software Foundation, either version 3 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU Affero General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU Affero General Public License
15+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
16+
*/
17+
18+
import { describe, expect, it } from 'vitest';
19+
import { UNAUTHENTICATED_SESSION, deriveCanMutate, type AuthSession } from '../src/auth-context';
20+
21+
function session(overrides: Partial<AuthSession> = {}): AuthSession {
22+
return { ...UNAUTHENTICATED_SESSION, ...overrides };
23+
}
24+
25+
describe('deriveCanMutate', () => {
26+
it('grants mutation to an administrator', () => {
27+
expect(deriveCanMutate(session({ isAdmin: true, isAuthenticated: true }))).toBe(true);
28+
});
29+
30+
it('withholds mutation from an authenticated non-administrator', () => {
31+
expect(deriveCanMutate(session({ isAdmin: false, isAuthenticated: true }))).toBe(false);
32+
});
33+
34+
it('withholds mutation from a signed-out session', () => {
35+
expect(deriveCanMutate(UNAUTHENTICATED_SESSION)).toBe(false);
36+
});
37+
38+
it('keys on the administrator flag alone, not on authentication', () => {
39+
// The server resolves a minimum role per route; the UI deliberately keys on
40+
// the one flag, so widening it stays a single-function change.
41+
expect(deriveCanMutate(session({ isAdmin: true, isAuthenticated: false }))).toBe(true);
42+
});
43+
});
44+
45+
describe('UNAUTHENTICATED_SESSION', () => {
46+
it('is the least-privileged session a missing provider can resolve to', () => {
47+
expect(UNAUTHENTICATED_SESSION.isAdmin).toBe(false);
48+
expect(UNAUTHENTICATED_SESSION.isAuthenticated).toBe(false);
49+
expect(UNAUTHENTICATED_SESSION.user).toBeNull();
50+
expect(UNAUTHENTICATED_SESSION.accessToken).toBeNull();
51+
});
52+
53+
it('is frozen, so a consumer cannot escalate the shared fallback', () => {
54+
expect(Object.isFrozen(UNAUTHENTICATED_SESSION)).toBe(true);
55+
expect(() => {
56+
(UNAUTHENTICATED_SESSION as { isAdmin: boolean }).isAdmin = true;
57+
}).toThrow();
58+
expect(UNAUTHENTICATED_SESSION.isAdmin).toBe(false);
59+
});
60+
61+
it('resolves its no-op login and logout without throwing', async () => {
62+
await expect(UNAUTHENTICATED_SESSION.login('u', 'p')).resolves.toBeUndefined();
63+
await expect(UNAUTHENTICATED_SESSION.logout()).resolves.toBeUndefined();
64+
});
65+
});

frontend/packages/apps/alerts/src/AlertsListPage.tsx

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
import ExpandMoreIcon from '@mui/icons-material/ExpandMore';
3636
import NotificationsActiveIcon from '@mui/icons-material/NotificationsActive';
3737
import SettingsIcon from '@mui/icons-material/Settings';
38+
import { useAuth } from '@sep/api';
3839
import { useAlertBackups, useAlertsIndex } from './hooks';
3940
import { AlertsWizard } from './AlertsWizard';
4041
import type { AlertTemplate, WizardMode } from './types';
@@ -54,6 +55,7 @@ const SEVERITY_COLORS: Record<string, 'default' | 'info' | 'warning' | 'error'>
5455
* AlertsWizard dialog.
5556
*/
5657
export function AlertsListPage() {
58+
const { canMutate } = useAuth();
5759
const { data, isLoading, error } = useAlertsIndex();
5860

5961
const [selected, setSelected] = useState<Set<string>>(new Set());
@@ -133,30 +135,34 @@ export function AlertsListPage() {
133135
PMM not connected
134136
</Alert>
135137
)}
136-
<Button
137-
variant="contained"
138-
startIcon={<NotificationsActiveIcon />}
139-
disabled={selected.size === 0 || !data?.pmm_connected}
140-
onClick={() => openWizard('push')}
141-
>
142-
Push Selected ({selected.size})
143-
</Button>
144-
<Button
145-
variant="outlined"
146-
onClick={() => openWizard('restore')}
147-
disabled={(data?.recent_backups ?? []).length === 0}
148-
>
149-
Restore from Backup
150-
</Button>
151-
<Button
152-
variant="outlined"
153-
startIcon={<SettingsIcon />}
154-
onClick={() => openWizard('pagerduty')}
155-
disabled={!data?.pmm_connected}
156-
color={pagerdutyConfigured ? 'success' : 'primary'}
157-
>
158-
{pagerdutyConfigured ? 'PagerDuty Configured ✓' : 'Configure PagerDuty'}
159-
</Button>
138+
{canMutate && (
139+
<>
140+
<Button
141+
variant="contained"
142+
startIcon={<NotificationsActiveIcon />}
143+
disabled={selected.size === 0 || !data?.pmm_connected}
144+
onClick={() => openWizard('push')}
145+
>
146+
Push Selected ({selected.size})
147+
</Button>
148+
<Button
149+
variant="outlined"
150+
onClick={() => openWizard('restore')}
151+
disabled={(data?.recent_backups ?? []).length === 0}
152+
>
153+
Restore from Backup
154+
</Button>
155+
<Button
156+
variant="outlined"
157+
startIcon={<SettingsIcon />}
158+
onClick={() => openWizard('pagerduty')}
159+
disabled={!data?.pmm_connected}
160+
color={pagerdutyConfigured ? 'success' : 'primary'}
161+
>
162+
{pagerdutyConfigured ? 'PagerDuty Configured ✓' : 'Configure PagerDuty'}
163+
</Button>
164+
</>
165+
)}
160166
</Box>
161167
</Box>
162168

@@ -173,16 +179,19 @@ export function AlertsListPage() {
173179
<Accordion key={group.service_type} disableGutters sx={{ mb: 1 }}>
174180
<AccordionSummary expandIcon={<ExpandMoreIcon />}>
175181
<Box sx={{ display: 'flex', alignItems: 'center', gap: 1 }}>
176-
<Checkbox
177-
checked={allChecked}
178-
indeterminate={someChecked && !allChecked}
179-
onClick={(e) => {
180-
e.stopPropagation();
181-
toggleGroup(groupNames);
182-
}}
183-
size="small"
184-
inputProps={{ 'aria-label': `Select all ${group.label} templates` }}
185-
/>
182+
{/* Selection exists only to feed Push Selected, so both go together. */}
183+
{canMutate && (
184+
<Checkbox
185+
checked={allChecked}
186+
indeterminate={someChecked && !allChecked}
187+
onClick={(e) => {
188+
e.stopPropagation();
189+
toggleGroup(groupNames);
190+
}}
191+
size="small"
192+
inputProps={{ 'aria-label': `Select all ${group.label} templates` }}
193+
/>
194+
)}
186195
<Typography variant="subtitle1" fontWeight={500}>
187196
{group.label} ({group.templates.length})
188197
</Typography>
@@ -205,12 +214,14 @@ export function AlertsListPage() {
205214
py: 0.5,
206215
}}
207216
>
208-
<Checkbox
209-
checked={selected.has(template.name)}
210-
onChange={() => toggleTemplate(template.name)}
211-
size="small"
212-
inputProps={{ 'aria-label': template.name }}
213-
/>
217+
{canMutate && (
218+
<Checkbox
219+
checked={selected.has(template.name)}
220+
onChange={() => toggleTemplate(template.name)}
221+
size="small"
222+
inputProps={{ 'aria-label': template.name }}
223+
/>
224+
)}
214225
<Box sx={{ flex: 1, minWidth: 0 }}>
215226
<Box sx={{ display: 'flex', alignItems: 'center', gap: 1 }}>
216227
<Typography variant="body2" fontWeight={500}>

0 commit comments

Comments
 (0)