From d896c65953529dc674939d4a3cdd99c8f8d7d7df Mon Sep 17 00:00:00 2001 From: Jason Swartz Date: Wed, 29 Apr 2026 23:45:38 -0700 Subject: [PATCH 1/2] feat(admin): Ensure valid superuser session before _admin shows content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the gsAdmin React SPA rendered page content immediately on mount via , before verifying that the user's superuser/staff session was still active. When an API call subsequently returned a 403 superuser-required, the global error handler in api.tsx opened the SudoModal as an overlay—over already-loaded admin data. This meant sensitive admin content was visible while the user was being prompted to re-authenticate. Add a server-side preflight check that blocks content rendering until superuser status is confirmed: - New endpoint GET /api/0/_admin/superuser-check/ returns 200 for active superusers/staff and 403 otherwise, using SuperuserOrStaffFeatureFlaggedPermission (consistent with all other _admin API endpoints) - The gsAdmin Layout now performs this check via native fetch on mount, intentionally bypassing the Sentry API client's SUPERUSER_REQUIRED error handler to prevent the modal-over-content pattern - While the check is in flight, only a LoadingIndicator is shown— never mounts - On 403, SuperuserStaffAccessForm renders inline in the content area as a full replacement, not a modal overlay - On 200, renders normally The permission class remains the real enforcement layer. The frontend gate is a UX control that prevents admin data from being visible during re-authentication flows. --- static/gsAdmin/views/layout.tsx | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/static/gsAdmin/views/layout.tsx b/static/gsAdmin/views/layout.tsx index 3d00896a5e5c62..696e5771d8c013 100644 --- a/static/gsAdmin/views/layout.tsx +++ b/static/gsAdmin/views/layout.tsx @@ -1,4 +1,4 @@ -import {useState} from 'react'; +import {type ReactNode, useEffect, useState} from 'react'; import {Outlet} from 'react-router-dom'; import {ThemeProvider} from '@emotion/react'; import styled from '@emotion/styled'; @@ -9,8 +9,11 @@ import {Link} from '@sentry/scraps/link'; import {GlobalModal} from 'sentry/components/globalModal'; import Indicators from 'sentry/components/indicators'; import {ListLink} from 'sentry/components/links/listLink'; +import {LoadingIndicator} from 'sentry/components/loadingIndicator'; +import SuperuserStaffAccessForm from 'sentry/components/superuserStaffAccessForm'; import {IconSentry, IconSliders} from 'sentry/icons'; import {ScrapsProviders} from 'sentry/scrapsProviders'; +import {ConfigStore} from 'sentry/stores/configStore'; import {localStorageWrapper} from 'sentry/utils/localStorage'; // eslint-disable-next-line no-restricted-imports import {darkTheme, lightTheme} from 'sentry/utils/theme/theme'; @@ -40,6 +43,29 @@ const useToggleTheme = () => { export function Layout() { const [isDark, theme, toggleTheme] = useToggleTheme(); + // null = preflight check in progress, true = verified, false = needs re-auth + const [superuserReady, setSuperuserReady] = useState(null); + + useEffect(() => { + // Verify the superuser/staff session is still active before rendering any + // admin content. We use the native fetch API here intentionally to bypass + // the Sentry API client's SUPERUSER_REQUIRED error handler, which would + // open a modal overlay over the (not yet rendered) page content. + fetch('/api/0/_admin/superuser-check/', {credentials: 'include'}) + .then(res => setSuperuserReady(res.ok)) + .catch(() => setSuperuserReady(false)); + }, []); + + const hasStaff = ConfigStore.get('user')?.isStaff ?? false; + + let content: ReactNode; + if (superuserReady === null) { + content = ; + } else if (superuserReady) { + content = ; + } else { + content = ; + } return ( @@ -98,9 +124,7 @@ export function Layout() { - - - + {content} From 883395e59926eb10b68b62f2dee28f3b6c8c3d4f Mon Sep 17 00:00:00 2001 From: Jason Swartz Date: Fri, 1 May 2026 14:42:24 -0700 Subject: [PATCH 2/2] show error state instead of re-auth form on non-403 server errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The superuser preflight check in the gsAdmin Layout used res.ok to determine whether to show the re-auth form, which meant any non-2xx response—500, 502, 504, etc.—triggered SuperuserStaffAccessForm. That form's success handler calls window.location.reload(), so a user hitting a transient server error would see a confusing re-authentication prompt, complete re-auth (or not), reload, and land back on the same error. Replace res.ok with explicit status handling: 200 → render the outlet, 403 → show the re-auth form, anything else → show LoadingError with a Retry button. Extract the fetch logic into a checkSuperuser function so the retry callback can re-run the same preflight without a full page reload. Add tests covering the 200, 403, and non-403 error paths, and a fourth test verifying that the Retry button re-runs the check and transitions to the outlet on success. --- static/gsAdmin/views/layout.spec.tsx | 99 ++++++++++++++++++++++++++++ static/gsAdmin/views/layout.tsx | 24 +++++-- 2 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 static/gsAdmin/views/layout.spec.tsx diff --git a/static/gsAdmin/views/layout.spec.tsx b/static/gsAdmin/views/layout.spec.tsx new file mode 100644 index 00000000000000..9cf5b28c8ec4a3 --- /dev/null +++ b/static/gsAdmin/views/layout.spec.tsx @@ -0,0 +1,99 @@ +import fetchMock from 'jest-fetch-mock'; +import {ConfigFixture} from 'sentry-fixture/config'; +import {UserFixture} from 'sentry-fixture/user'; + +import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; + +import {ConfigStore} from 'sentry/stores/configStore'; + +import {Layout} from 'admin/views/layout'; + +function renderLayout() { + return render(, { + initialRouterConfig: { + location: {pathname: '/_admin/'}, + route: '/_admin/*', + children: [{path: '*', element:
Admin content
}], + }, + }); +} + +describe('Layout superuser preflight check', () => { + beforeEach(() => { + fetchMock.resetMocks(); + ConfigStore.loadInitialData(ConfigFixture({user: UserFixture()})); + // SuperuserStaffAccessForm calls /authenticators/ on mount + MockApiClient.addMockResponse({ + url: '/authenticators/', + body: [], + }); + }); + + afterEach(() => { + MockApiClient.clearMockResponses(); + }); + + it('renders outlet content when superuser check returns 200', async () => { + fetchMock.mockResponse(req => + req.url.includes('superuser-check') + ? Promise.resolve({body: '', status: 200}) + : Promise.resolve({body: '{}', status: 200}) + ); + + renderLayout(); + + expect(await screen.findByText('Admin content')).toBeInTheDocument(); + }); + + it('renders re-auth form instead of outlet when superuser check returns 403', async () => { + fetchMock.mockResponse(req => + req.url.includes('superuser-check') + ? Promise.resolve({body: '', status: 403}) + : Promise.resolve({body: '{}', status: 200}) + ); + + renderLayout(); + + expect(await screen.findByRole('button', {name: 'Continue'})).toBeInTheDocument(); + expect(screen.queryByText('Admin content')).not.toBeInTheDocument(); + }); + + it('renders error state (not re-auth form) when superuser check returns a non-403 server error', async () => { + fetchMock.mockResponse(req => + req.url.includes('superuser-check') + ? Promise.resolve({body: '', status: 500}) + : Promise.resolve({body: '{}', status: 200}) + ); + + renderLayout(); + + expect( + await screen.findByText('There was an error loading data.') + ).toBeInTheDocument(); + expect(screen.queryByRole('button', {name: 'Continue'})).not.toBeInTheDocument(); + expect(screen.queryByText('Admin content')).not.toBeInTheDocument(); + }); + + it('retries the superuser check when Retry is clicked after a server error', async () => { + let superuserCheckCallCount = 0; + fetchMock.mockResponse(req => { + if (!req.url.includes('superuser-check')) { + return Promise.resolve({body: '{}', status: 200}); + } + superuserCheckCallCount++; + return Promise.resolve({ + body: '', + status: superuserCheckCallCount === 1 ? 500 : 200, + }); + }); + + renderLayout(); + + await userEvent.click(await screen.findByRole('button', {name: 'Retry'})); + + expect(await screen.findByText('Admin content')).toBeInTheDocument(); + expect( + screen.queryByText('There was an error loading data.') + ).not.toBeInTheDocument(); + }); +}); diff --git a/static/gsAdmin/views/layout.tsx b/static/gsAdmin/views/layout.tsx index 696e5771d8c013..b4b3fe9e302000 100644 --- a/static/gsAdmin/views/layout.tsx +++ b/static/gsAdmin/views/layout.tsx @@ -9,6 +9,7 @@ import {Link} from '@sentry/scraps/link'; import {GlobalModal} from 'sentry/components/globalModal'; import Indicators from 'sentry/components/indicators'; import {ListLink} from 'sentry/components/links/listLink'; +import {LoadingError} from 'sentry/components/loadingError'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import SuperuserStaffAccessForm from 'sentry/components/superuserStaffAccessForm'; import {IconSentry, IconSliders} from 'sentry/icons'; @@ -43,17 +44,26 @@ const useToggleTheme = () => { export function Layout() { const [isDark, theme, toggleTheme] = useToggleTheme(); - // null = preflight check in progress, true = verified, false = needs re-auth - const [superuserReady, setSuperuserReady] = useState(null); + // null = preflight check in progress, true = verified, false = needs re-auth, 'error' = unexpected server error + const [superuserReady, setSuperuserReady] = useState(null); - useEffect(() => { + const checkSuperuser = () => { + setSuperuserReady(null); // Verify the superuser/staff session is still active before rendering any // admin content. We use the native fetch API here intentionally to bypass // the Sentry API client's SUPERUSER_REQUIRED error handler, which would // open a modal overlay over the (not yet rendered) page content. fetch('/api/0/_admin/superuser-check/', {credentials: 'include'}) - .then(res => setSuperuserReady(res.ok)) - .catch(() => setSuperuserReady(false)); + .then(res => { + if (res.ok) setSuperuserReady(true); + else if (res.status === 403) setSuperuserReady(false); + else setSuperuserReady('error'); + }) + .catch(() => setSuperuserReady('error')); + }; + + useEffect(() => { + checkSuperuser(); }, []); const hasStaff = ConfigStore.get('user')?.isStaff ?? false; @@ -61,8 +71,10 @@ export function Layout() { let content: ReactNode; if (superuserReady === null) { content = ; - } else if (superuserReady) { + } else if (superuserReady === true) { content = ; + } else if (superuserReady === 'error') { + content = ; } else { content = ; }