cleanup: trim oversized comments across recent SaaS fixes

Reduce multi-paragraph comment blocks to short two-line notes and drop
history-style references; no behaviour changes.
This commit is contained in:
Anthony Stirling
2026-06-10 14:30:29 +01:00
parent bf18af4708
commit 5b412c0fed
11 changed files with 30 additions and 95 deletions
@@ -39,12 +39,8 @@ public class EnhancedJwtAuthenticationToken extends JwtAuthenticationToken {
} }
/** /**
* Shared proprietary/core code authorizes with {@code principal instanceof User} (e.g. {@code * Returns the resolved local {@link User} when available so shared {@code principal instanceof
* FileStorageService.requireAuthenticatedUser}, {@code FolderService}), which the default * User} authorization works under JWT auth; falls back to the decoded Jwt.
* {@link JwtAuthenticationToken} principal — the decoded {@link Jwt} — can never satisfy, so
* every such endpoint 401'd valid Supabase sessions. When the filter resolved a local {@link
* User}, expose it as the principal so those checks behave exactly as under form login; fall
* back to the Jwt when no User was attached (anonymous sessions, converter-built tokens).
*/ */
@Override @Override
public Object getPrincipal() { public Object getPrincipal() {
@@ -155,10 +155,8 @@ public class SupabaseAuthenticationFilter extends OncePerRequestFilter {
User user = getOrCreateUser(jwt); User user = getOrCreateUser(jwt);
// Attach the resolved User as the token principal for full accounts so shared // Full accounts carry the resolved User as principal for shared
// `principal instanceof User` authorization (storage services, account data) // instanceof-User authorization; anonymous sessions keep the raw Jwt.
// works under JWT auth. Anonymous sessions keep the raw Jwt principal and stay
// locked out of those User-gated APIs, matching pre-existing behaviour.
EnhancedJwtAuthenticationToken authToken = EnhancedJwtAuthenticationToken authToken =
new EnhancedJwtAuthenticationToken( new EnhancedJwtAuthenticationToken(
jwt, jwt,
@@ -365,11 +365,8 @@ export function FolderProvider({ children }: FolderProviderProps) {
// guaranteed-to-403 round-trip per session. // guaranteed-to-403 round-trip per session.
const { config: appConfig } = useAppConfig(); const { config: appConfig } = useAppConfig();
const storageBackedByServer = appConfig?.storageEnabled === true; const storageBackedByServer = appConfig?.storageEnabled === true;
// Don't hit the authenticated storage API on auth routes (/login, /signup, // Skip server pulls on auth routes: FolderProvider is mounted globally and
// /auth/*, ...). The global FolderProvider is mounted everywhere, including // /login has no session yet, so the pull would be a guaranteed 401.
// the login screen where the user has no session yet, so an unguarded pull
// fires a guaranteed-401 GET /api/v1/storage/folders before sign-in. Mirror
// the auth-route skip used by LicenseContext / AppConfigContext.
const location = useLocation(); const location = useLocation();
const onAuthRoute = isAuthRoute(location.pathname); const onAuthRoute = isAuthRoute(location.pathname);
useEffect(() => { useEffect(() => {
@@ -128,10 +128,8 @@ export function useMultipleEndpointsEnabled(endpoints: string[]): {
"[useEndpointConfig] Fetching all endpoint statuses from server", "[useEndpointConfig] Fetching all endpoint statuses from server",
); );
// Fetch all endpoints at once - no query params needed. // Fetch all endpoints at once; auto-fires on app load, so a 401 must
// Fires automatically on app load; a 401 here (e.g. backend rejecting // fail silently instead of triggering the global login redirect.
// an otherwise-valid session) must fail silently rather than reach the
// global handler, which hard-redirects to /login and can loop.
const response = await apiClient.get< const response = await apiClient.get<
Record<string, EndpointAvailabilityDetails> Record<string, EndpointAvailabilityDetails>
>(`/api/v1/config/endpoints-availability`, { >(`/api/v1/config/endpoints-availability`, {
@@ -28,9 +28,8 @@ export const accountService = {
* This is a public endpoint - doesn't require authentication * This is a public endpoint - doesn't require authentication
*/ */
async getLoginPageData(): Promise<LoginPageData> { async getLoginPageData(): Promise<LoginPageData> {
// Public endpoint, but also auto-called by the onboarding orchestrator // Also auto-called by onboarding when a stale stirling_jwt exists; a 401
// when a stale stirling_jwt is in localStorage — a 401 must never trigger // must never trigger the global login redirect.
// the global login redirect (it would loop on every page load).
const response = await apiClient.get<LoginPageData>( const response = await apiClient.get<LoginPageData>(
"/api/v1/proprietary/ui-data/login", "/api/v1/proprietary/ui-data/login",
{ suppressErrorToast: true, skipAuthRedirect: true }, { suppressErrorToast: true, skipAuthRedirect: true },
@@ -62,15 +62,8 @@ function toFolderRecord(dto: ServerFolder): FolderRecord {
export const folderSyncService = { export const folderSyncService = {
async list(): Promise<FolderRecord[]> { async list(): Promise<FolderRecord[]> {
// Background sync fired automatically by the global FolderProvider on // Auto-fired by FolderProvider on every load; a persistent 401 must fail
// every (non-auth-route) load. If the backend rejects the request with a // silently here or the global handler redirects to /login and loops.
// 401 - e.g. the storage API doesn't accept this deployment's session
// token - the error must NOT reach the global 401 handler: that handler
// hard-redirects to /login, the login page sees a valid session and
// bounces back to /, FolderProvider pulls again, and the app loops
// login->/->login forever. FolderContext.pullFromServer already handles
// 4xx locally (flips serverReachable, no banner), so fail silently here -
// mirroring fileSyncService's /api/v1/storage/files pull.
const response = await apiClient.get<ServerFolder[]>( const response = await apiClient.get<ServerFolder[]>(
"/api/v1/storage/folders", "/api/v1/storage/folders",
{ {
@@ -46,12 +46,8 @@ function stashPostLoginRedirect(path: string): void {
} }
} }
// Loop breaker for the 401 hard-redirect below. If the backend persistently // Loop breaker: a second 401 redirect within this window means the login page
// 401s an automatic call while the auth session is actually valid, the // bounced us back with a live session — redirecting again would loop forever.
// redirect lands on /login, the login page sees the valid session and bounces
// back, the call 401s again and we redirect again — forever. sessionStorage
// survives the full-page navigations of that cycle (per tab), so a second
// redirect within the window means we're looping, not expiring.
const LOGIN_REDIRECT_THROTTLE_KEY = "stirling_last_401_redirect"; const LOGIN_REDIRECT_THROTTLE_KEY = "stirling_last_401_redirect";
const LOGIN_REDIRECT_THROTTLE_MS = 10_000; const LOGIN_REDIRECT_THROTTLE_MS = 10_000;
+3 -6
View File
@@ -162,10 +162,8 @@ export function AuthProvider({ children }: { children: ReactNode }) {
"[Auth Debug] Fetching credits for user:", "[Auth Debug] Fetching credits for user:",
currentSession.user.id, currentSession.user.id,
); );
// Fired automatically on session init and TOKEN_REFRESHED. If the // Auto-fires on session init and TOKEN_REFRESHED; a backend 401 must
// backend rejects it (deploy skew, authz bug) the failure must stay // stay local rather than trigger the global login redirect.
// local — the global 401 handler would hard-redirect to /login and,
// with a valid session, loop login -> / -> login forever.
const response = await apiClient.get<ApiCredits>("/api/v1/credits", { const response = await apiClient.get<ApiCredits>("/api/v1/credits", {
suppressErrorToast: true, suppressErrorToast: true,
skipAuthRedirect: true, skipAuthRedirect: true,
@@ -636,8 +634,7 @@ export function AuthProvider({ children }: { children: ReactNode }) {
fetchTrialStatus(newSession), fetchTrialStatus(newSession),
fetchProfilePictureMetadata(newSession), fetchProfilePictureMetadata(newSession),
]).then(() => { ]).then(() => {
// Fetch the picture once the avatar sync settles rather than // Fetch the picture once the avatar sync settles.
// after a fixed 500ms the first-ever sync usually loses.
avatarSync.then(() => { avatarSync.then(() => {
fetchProfilePicture(newSession).finally(() => { fetchProfilePicture(newSession).finally(() => {
console.debug( console.debug(
+8 -30
View File
@@ -116,23 +116,12 @@ const publicEndpoints = [
"/api/v1/config/app-config", "/api/v1/config/app-config",
"/api/v1/info/status", "/api/v1/info/status",
"/api/v1/config/public-config", "/api/v1/config/public-config",
// Both real endpoint-config routes (an earlier entry here said
// "endpoints-enabled", which matches neither and silently guarded nothing).
"/api/v1/config/endpoints-availability", "/api/v1/config/endpoints-availability",
"/api/v1/config/endpoint-enabled", "/api/v1/config/endpoint-enabled",
]; ];
// De-duplicate concurrent token refreshes. // Share one in-flight refresh: Supabase rotates the refresh token on first
// // use, so concurrent refreshSession() calls fail with "Already Used".
// On a cold load with an expired access token, many bootstrap requests
// (config, credits, footer-info, storage, ...) 401 at roughly the same
// instant. If each one calls supabase.auth.refreshSession() independently,
// Supabase rotates the refresh token on first use and the remaining
// concurrent refreshes fail with "Invalid Refresh Token: Already Used".
// That spurious failure then bounced the whole app to /login even though the
// session was perfectly recoverable. Sharing a single in-flight refresh
// promise makes the first 401 do the network refresh and everyone else awaits
// the same result.
let inFlightRefresh: ReturnType<typeof supabase.auth.refreshSession> | null = let inFlightRefresh: ReturnType<typeof supabase.auth.refreshSession> | null =
null; null;
function refreshSessionOnce(): ReturnType<typeof supabase.auth.refreshSession> { function refreshSessionOnce(): ReturnType<typeof supabase.auth.refreshSession> {
@@ -185,12 +174,8 @@ apiClient.interceptors.response.use(
originalRequest.url?.includes(endpoint), originalRequest.url?.includes(endpoint),
); );
// On a 401 (that we haven't already retried), try to recover the session // On a first 401, refresh and retry — public endpoints included, since an
// before giving up. We attempt this for BOTH protected and public // expired Bearer token is rejected on any route during cold load.
// endpoints: the backend rejects any expired Bearer token regardless of
// route, so a "public" bootstrap call (e.g. /api/v1/config/app-config) can
// 401 on cold load too. Refreshing + retrying lets those succeed instead of
// tripping the global login redirect.
if (status === 401 && !originalRequest._retry) { if (status === 401 && !originalRequest._retry) {
originalRequest._retry = true; originalRequest._retry = true;
@@ -245,11 +230,8 @@ apiClient.interceptors.response.use(
} }
} }
// A 401 on a public endpoint must never trigger the global login redirect. // Public-endpoint 401s must never trigger the global login redirect
// If we reach here it means refresh/retry didn't resolve it (e.g. no // (e.g. transient 401s while Supabase is still restoring the session).
// session yet, or it 401'd again); suppress the redirect in the shared
// error handler so a transient bootstrap 401 can't bounce the app to
// /login while Supabase is still restoring the session.
if (status === 401 && isPublicEndpoint) { if (status === 401 && isPublicEndpoint) {
console.debug( console.debug(
"[API Client] 401 on public endpoint, continuing without auth:", "[API Client] 401 on public endpoint, continuing without auth:",
@@ -258,12 +240,8 @@ apiClient.interceptors.response.use(
originalRequest.skipAuthRedirect = true; originalRequest.skipAuthRedirect = true;
} }
// If a request was already retried with a freshly refreshed token and the // A 401 that survived refresh-and-retry means the backend rejected a
// backend STILL returned 401, the session is not the problem — the backend // valid token; redirecting to /login would only bounce back and loop.
// is rejecting a valid token (authorization bug, wrong API origin, etc.).
// Redirecting to /login cannot fix that: the login page sees the valid
// Supabase session and bounces straight back, producing an infinite
// login -> / -> login loop. Keep the error toast, skip the redirect.
if (status === 401 && originalRequest._retry && !isPublicEndpoint) { if (status === 401 && originalRequest._retry && !isPublicEndpoint) {
console.warn( console.warn(
"[API Client] 401 persisted after token refresh; backend rejected a valid session — not redirecting to login:", "[API Client] 401 persisted after token refresh; backend rejected a valid session — not redirecting to login:",
@@ -1,22 +1,8 @@
// SaaS-layer override for `@app/services/supabaseClient`. // SaaS override of `@app/services/supabaseClient`: re-export the auth
// // singleton so exactly one GoTrueClient (one refresh timer) exists per tab.
// There must be exactly ONE Supabase client (one GoTrueClient) per browser
// context. The :proprietary version of this module calls createClient() a
// second time with the same auth-token storage key as :saas's
// `@app/auth/supabase`. In the SaaS bundle, billing/licensing/user-management
// code (CheckoutContext, licenseService, AdminPlanSection, userManagementService)
// imports `@app/services/supabaseClient` and would otherwise pull in that second
// client. Two clients => two independent autoRefreshToken timers racing on the
// same refresh token => "Multiple GoTrueClient instances detected", rotated /
// "Already Used" refresh tokens, and spurious 401s (e.g. /api/v1/storage/*).
//
// Re-exporting the singleton from `@app/auth/supabase` keeps every
// `@app/services/supabaseClient` consumer pointed at the same instance, so the
// :proprietary module's createClient() is never bundled in the SaaS build.
import type { SupabaseClient } from "@supabase/supabase-js"; import type { SupabaseClient } from "@supabase/supabase-js";
import { supabase as supabaseSingleton } from "@app/auth/supabase"; import { supabase as supabaseSingleton } from "@app/auth/supabase";
// `@app/auth/supabase` throws on missing config, so in the SaaS build the // `@app/auth/supabase` throws on missing config, so the client always exists.
// client is always present and Supabase is always configured.
export const supabase: SupabaseClient | null = supabaseSingleton; export const supabase: SupabaseClient | null = supabaseSingleton;
export const isSupabaseConfigured = true; export const isSupabaseConfigured = true;
+2 -5
View File
@@ -243,11 +243,8 @@ export default defineConfig(async ({ mode }) => {
// SPA fallback returns index.html as text/html and React never mounts. // SPA fallback returns index.html as text/html and React never mounts.
// VITE_BUILD_FOR_PREVIEW=1 (set by the CI playwright steps) overrides to // VITE_BUILD_FOR_PREVIEW=1 (set by the CI playwright steps) overrides to
// an absolute base so deep-route asset paths resolve to /assets/... // an absolute base so deep-route asset paths resolve to /assets/...
// Trailing slash is required: it becomes `<base href>` in index.html, and // Trailing slash required: it becomes `<base href>`, and browsers resolve
// the browser resolves relative links (manifest.json, modern-logo/favicon.ico) // relative URLs (manifest.json, favicon) against the base's *directory*.
// against the base's *directory*. Without it, `/bpp` resolves relatives to
// the domain root → `/manifest.json` 404 instead of `/bpp/manifest.json`.
// getBasePath() strips the trailing slash, so BASE_PATH/routing are unchanged.
base: env.RUN_SUBPATH base: env.RUN_SUBPATH
? `/${env.RUN_SUBPATH}/` ? `/${env.RUN_SUBPATH}/`
: process.env.VITE_BUILD_FOR_PREVIEW === "1" : process.env.VITE_BUILD_FOR_PREVIEW === "1"