feat(admin): Ensure valid superuser session before _admin shows content#114407
Open
swartzrock wants to merge 4 commits into
Open
feat(admin): Ensure valid superuser session before _admin shows content#114407swartzrock wants to merge 4 commits into
swartzrock wants to merge 4 commits into
Conversation
Previously, the gsAdmin React SPA rendered page content immediately on mount via <Outlet />, 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—<Outlet /> never mounts - On 403, SuperuserStaffAccessForm renders inline in the content area as a full replacement, not a modal overlay - On 200, <Outlet /> 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.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e72ca26. Configure here.
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.
Contributor
📊 Type Coverage Diff✅ No new type safety issues introduced. Coverage: 93.40% |
Contributor
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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:
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.
Fixes https://linear.app/getsentry/issue/REVENG-39/
Uses an endpoint added in https://github.com/getsentry/getsentry/pull/20170