From 5b412c0fed8fb88508028c70a4b269cef03290f0 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+frooodle@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:30:29 +0100 Subject: [PATCH] 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. --- .../EnhancedJwtAuthenticationToken.java | 8 +--- .../SupabaseAuthenticationFilter.java | 6 +-- .../src/core/contexts/FolderContext.tsx | 7 +--- .../src/core/hooks/useEndpointConfig.ts | 6 +-- .../src/core/services/accountService.ts | 5 +-- .../src/core/services/folderSyncService.ts | 11 +----- .../src/core/services/httpErrorHandler.ts | 8 +--- frontend/editor/src/saas/auth/UseSession.tsx | 9 ++--- .../editor/src/saas/services/apiClient.ts | 38 ++++--------------- .../src/saas/services/supabaseClient.ts | 20 ++-------- frontend/editor/vite.config.ts | 7 +--- 11 files changed, 30 insertions(+), 95 deletions(-) diff --git a/app/saas/src/main/java/stirling/software/saas/security/EnhancedJwtAuthenticationToken.java b/app/saas/src/main/java/stirling/software/saas/security/EnhancedJwtAuthenticationToken.java index 679e285f0..6991c2bfa 100644 --- a/app/saas/src/main/java/stirling/software/saas/security/EnhancedJwtAuthenticationToken.java +++ b/app/saas/src/main/java/stirling/software/saas/security/EnhancedJwtAuthenticationToken.java @@ -39,12 +39,8 @@ public class EnhancedJwtAuthenticationToken extends JwtAuthenticationToken { } /** - * Shared proprietary/core code authorizes with {@code principal instanceof User} (e.g. {@code - * FileStorageService.requireAuthenticatedUser}, {@code FolderService}), which the default - * {@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). + * Returns the resolved local {@link User} when available so shared {@code principal instanceof + * User} authorization works under JWT auth; falls back to the decoded Jwt. */ @Override public Object getPrincipal() { diff --git a/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java b/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java index 888a5773f..5c97bd179 100644 --- a/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java +++ b/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java @@ -155,10 +155,8 @@ public class SupabaseAuthenticationFilter extends OncePerRequestFilter { User user = getOrCreateUser(jwt); - // Attach the resolved User as the token principal for full accounts so shared - // `principal instanceof User` authorization (storage services, account data) - // works under JWT auth. Anonymous sessions keep the raw Jwt principal and stay - // locked out of those User-gated APIs, matching pre-existing behaviour. + // Full accounts carry the resolved User as principal for shared + // instanceof-User authorization; anonymous sessions keep the raw Jwt. EnhancedJwtAuthenticationToken authToken = new EnhancedJwtAuthenticationToken( jwt, diff --git a/frontend/editor/src/core/contexts/FolderContext.tsx b/frontend/editor/src/core/contexts/FolderContext.tsx index 65b746bd6..81aba6a2b 100644 --- a/frontend/editor/src/core/contexts/FolderContext.tsx +++ b/frontend/editor/src/core/contexts/FolderContext.tsx @@ -365,11 +365,8 @@ export function FolderProvider({ children }: FolderProviderProps) { // guaranteed-to-403 round-trip per session. const { config: appConfig } = useAppConfig(); const storageBackedByServer = appConfig?.storageEnabled === true; - // Don't hit the authenticated storage API on auth routes (/login, /signup, - // /auth/*, ...). The global FolderProvider is mounted everywhere, including - // 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. + // Skip server pulls on auth routes: FolderProvider is mounted globally and + // /login has no session yet, so the pull would be a guaranteed 401. const location = useLocation(); const onAuthRoute = isAuthRoute(location.pathname); useEffect(() => { diff --git a/frontend/editor/src/core/hooks/useEndpointConfig.ts b/frontend/editor/src/core/hooks/useEndpointConfig.ts index 307b4a460..461516478 100644 --- a/frontend/editor/src/core/hooks/useEndpointConfig.ts +++ b/frontend/editor/src/core/hooks/useEndpointConfig.ts @@ -128,10 +128,8 @@ export function useMultipleEndpointsEnabled(endpoints: string[]): { "[useEndpointConfig] Fetching all endpoint statuses from server", ); - // Fetch all endpoints at once - no query params needed. - // Fires automatically on app load; a 401 here (e.g. backend rejecting - // an otherwise-valid session) must fail silently rather than reach the - // global handler, which hard-redirects to /login and can loop. + // Fetch all endpoints at once; auto-fires on app load, so a 401 must + // fail silently instead of triggering the global login redirect. const response = await apiClient.get< Record >(`/api/v1/config/endpoints-availability`, { diff --git a/frontend/editor/src/core/services/accountService.ts b/frontend/editor/src/core/services/accountService.ts index 97d4f4cad..bc43cdd2d 100644 --- a/frontend/editor/src/core/services/accountService.ts +++ b/frontend/editor/src/core/services/accountService.ts @@ -28,9 +28,8 @@ export const accountService = { * This is a public endpoint - doesn't require authentication */ async getLoginPageData(): Promise { - // Public endpoint, but also auto-called by the onboarding orchestrator - // when a stale stirling_jwt is in localStorage — a 401 must never trigger - // the global login redirect (it would loop on every page load). + // Also auto-called by onboarding when a stale stirling_jwt exists; a 401 + // must never trigger the global login redirect. const response = await apiClient.get( "/api/v1/proprietary/ui-data/login", { suppressErrorToast: true, skipAuthRedirect: true }, diff --git a/frontend/editor/src/core/services/folderSyncService.ts b/frontend/editor/src/core/services/folderSyncService.ts index 7d863f0c2..53e1d7e65 100644 --- a/frontend/editor/src/core/services/folderSyncService.ts +++ b/frontend/editor/src/core/services/folderSyncService.ts @@ -62,15 +62,8 @@ function toFolderRecord(dto: ServerFolder): FolderRecord { export const folderSyncService = { async list(): Promise { - // Background sync fired automatically by the global FolderProvider on - // every (non-auth-route) load. If the backend rejects the request with a - // 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. + // Auto-fired by FolderProvider on every load; a persistent 401 must fail + // silently here or the global handler redirects to /login and loops. const response = await apiClient.get( "/api/v1/storage/folders", { diff --git a/frontend/editor/src/core/services/httpErrorHandler.ts b/frontend/editor/src/core/services/httpErrorHandler.ts index b831f1581..69d4c6e06 100644 --- a/frontend/editor/src/core/services/httpErrorHandler.ts +++ b/frontend/editor/src/core/services/httpErrorHandler.ts @@ -46,12 +46,8 @@ function stashPostLoginRedirect(path: string): void { } } -// Loop breaker for the 401 hard-redirect below. If the backend persistently -// 401s an automatic call while the auth session is actually valid, the -// 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. +// Loop breaker: a second 401 redirect within this window means the login page +// bounced us back with a live session — redirecting again would loop forever. const LOGIN_REDIRECT_THROTTLE_KEY = "stirling_last_401_redirect"; const LOGIN_REDIRECT_THROTTLE_MS = 10_000; diff --git a/frontend/editor/src/saas/auth/UseSession.tsx b/frontend/editor/src/saas/auth/UseSession.tsx index 0fd02bb5f..94c1ae824 100644 --- a/frontend/editor/src/saas/auth/UseSession.tsx +++ b/frontend/editor/src/saas/auth/UseSession.tsx @@ -162,10 +162,8 @@ export function AuthProvider({ children }: { children: ReactNode }) { "[Auth Debug] Fetching credits for user:", currentSession.user.id, ); - // Fired automatically on session init and TOKEN_REFRESHED. If the - // backend rejects it (deploy skew, authz bug) the failure must stay - // local — the global 401 handler would hard-redirect to /login and, - // with a valid session, loop login -> / -> login forever. + // Auto-fires on session init and TOKEN_REFRESHED; a backend 401 must + // stay local rather than trigger the global login redirect. const response = await apiClient.get("/api/v1/credits", { suppressErrorToast: true, skipAuthRedirect: true, @@ -636,8 +634,7 @@ export function AuthProvider({ children }: { children: ReactNode }) { fetchTrialStatus(newSession), fetchProfilePictureMetadata(newSession), ]).then(() => { - // Fetch the picture once the avatar sync settles rather than - // after a fixed 500ms the first-ever sync usually loses. + // Fetch the picture once the avatar sync settles. avatarSync.then(() => { fetchProfilePicture(newSession).finally(() => { console.debug( diff --git a/frontend/editor/src/saas/services/apiClient.ts b/frontend/editor/src/saas/services/apiClient.ts index c806d5217..893bda76f 100644 --- a/frontend/editor/src/saas/services/apiClient.ts +++ b/frontend/editor/src/saas/services/apiClient.ts @@ -116,23 +116,12 @@ const publicEndpoints = [ "/api/v1/config/app-config", "/api/v1/info/status", "/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/endpoint-enabled", ]; -// De-duplicate concurrent token refreshes. -// -// 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. +// Share one in-flight refresh: Supabase rotates the refresh token on first +// use, so concurrent refreshSession() calls fail with "Already Used". let inFlightRefresh: ReturnType | null = null; function refreshSessionOnce(): ReturnType { @@ -185,12 +174,8 @@ apiClient.interceptors.response.use( originalRequest.url?.includes(endpoint), ); - // On a 401 (that we haven't already retried), try to recover the session - // before giving up. We attempt this for BOTH protected and public - // 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. + // On a first 401, refresh and retry — public endpoints included, since an + // expired Bearer token is rejected on any route during cold load. if (status === 401 && !originalRequest._retry) { originalRequest._retry = true; @@ -245,11 +230,8 @@ apiClient.interceptors.response.use( } } - // A 401 on a public endpoint must never trigger the global login redirect. - // If we reach here it means refresh/retry didn't resolve it (e.g. no - // 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. + // Public-endpoint 401s must never trigger the global login redirect + // (e.g. transient 401s while Supabase is still restoring the session). if (status === 401 && isPublicEndpoint) { console.debug( "[API Client] 401 on public endpoint, continuing without auth:", @@ -258,12 +240,8 @@ apiClient.interceptors.response.use( originalRequest.skipAuthRedirect = true; } - // If a request was already retried with a freshly refreshed token and the - // backend STILL returned 401, the session is not the problem — the backend - // 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. + // A 401 that survived refresh-and-retry means the backend rejected a + // valid token; redirecting to /login would only bounce back and loop. if (status === 401 && originalRequest._retry && !isPublicEndpoint) { console.warn( "[API Client] 401 persisted after token refresh; backend rejected a valid session — not redirecting to login:", diff --git a/frontend/editor/src/saas/services/supabaseClient.ts b/frontend/editor/src/saas/services/supabaseClient.ts index e3a91558f..3dd248c5a 100644 --- a/frontend/editor/src/saas/services/supabaseClient.ts +++ b/frontend/editor/src/saas/services/supabaseClient.ts @@ -1,22 +1,8 @@ -// SaaS-layer override for `@app/services/supabaseClient`. -// -// 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. +// SaaS override of `@app/services/supabaseClient`: re-export the auth +// singleton so exactly one GoTrueClient (one refresh timer) exists per tab. import type { SupabaseClient } from "@supabase/supabase-js"; import { supabase as supabaseSingleton } from "@app/auth/supabase"; -// `@app/auth/supabase` throws on missing config, so in the SaaS build the -// client is always present and Supabase is always configured. +// `@app/auth/supabase` throws on missing config, so the client always exists. export const supabase: SupabaseClient | null = supabaseSingleton; export const isSupabaseConfigured = true; diff --git a/frontend/editor/vite.config.ts b/frontend/editor/vite.config.ts index 0df5fbee6..76416d57c 100644 --- a/frontend/editor/vite.config.ts +++ b/frontend/editor/vite.config.ts @@ -243,11 +243,8 @@ export default defineConfig(async ({ mode }) => { // 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 // an absolute base so deep-route asset paths resolve to /assets/... - // Trailing slash is required: it becomes `` in index.html, and - // the browser resolves relative links (manifest.json, modern-logo/favicon.ico) - // 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. + // Trailing slash required: it becomes ``, and browsers resolve + // relative URLs (manifest.json, favicon) against the base's *directory*. base: env.RUN_SUBPATH ? `/${env.RUN_SUBPATH}/` : process.env.VITE_BUILD_FOR_PREVIEW === "1"