fix: send signed-out visitors to login instead of a blank page (EN-157)

frontend/src/auth/AuthProvider.tsx

@@ -1,6 +1,6 @@
 import React, { useState, useEffect } from 'react';
-import { INIT_KRATOS_LOGIN_FLOW, User } from '@/types';
-import { AuthContext, fetchUser, handleLogout } from '@/auth/useAuth';
+import { User } from '@/types';
+import { AuthContext, checkSession, signInAgain } from '@/auth/useAuth';
 import { useNavigate } from 'react-router-dom';
 import { tabSessionManager } from '@/session/tabSession';
 import { analyticsDistinctId, identifyUser } from '@/lib/events';
@@ -11,34 +11,38 @@ export const AuthProvider: React.FC<{ children: React.ReactNode }> = ({
 }) => {
     const passReset = '/reset-password';
     const [user, setUser] = useState<User | undefined>();
-    const [loading, setLoading] = useState(true);
+    const [unavailable, setUnavailable] = useState(false);
     const navigate = useNavigate();
 
     useEffect(() => {
         let cancelled = false;
         const checkAuth = async () => {
-            const authUser = await fetchUser();
+            const session = await checkSession();
             if (cancelled) return;
-            if (authUser) {
-                const hasValidTabSession =
-                    await tabSessionManager.validateSession();
+            if (session === 'unavailable') {
+                setUnavailable(true);
+                return;
+            }
+            if (
+                session === 'signed-out' ||
+                !(await tabSessionManager.validateSession())
+            ) {
                 if (cancelled) return;
-                if (!hasValidTabSession) {
-                    await handleLogout();
-                    return;
-                }
-                // Before anything reads Learning Record storage: discard it if
-                // it belongs to a different resident. Facility workstations are
-                // shared, the keys are per-origin, and logging out clears none of
-                // them — so without this the next person to sign in inherits the
-                // previous one's unfinished achievement and their sitting.
-                await setDigitalTranscriptStorageOwner(
-                    analyticsDistinctId(authUser.id)
-                );
-                identifyUser(authUser);
-                setUser(authUser);
+                await signInAgain(window.location);
+                return;
             }
-            setLoading(false);
+            if (cancelled) return;
+            const authUser = session;
+            // Before anything reads Learning Record storage: discard it if
+            // it belongs to a different resident. Facility workstations are
+            // shared, the keys are per-origin, and logging out clears none of
+            // them — so without this the next person to sign in inherits the
+            // previous one's unfinished achievement and their sitting.
+            await setDigitalTranscriptStorageOwner(
+                analyticsDistinctId(authUser.id)
+            );
+            identifyUser(authUser);
+            setUser(authUser);
         };
         void checkAuth();
         return () => {
@@ -47,17 +51,17 @@ export const AuthProvider: React.FC<{ children: React.ReactNode }> = ({
     }, []);
 
     useEffect(() => {
-        if (loading) return;
-        if (!user) {
-            window.location.href = INIT_KRATOS_LOGIN_FLOW;
-            return;
-        }
-        if (user.password_reset && window.location.pathname !== passReset) {
+        if (user?.password_reset && window.location.pathname !== passReset) {
             navigate(passReset);
         }
-    }, [loading, user, navigate]);
+    }, [user, navigate]);
 
-    if (loading || !user) {
+    if (unavailable) {
+        // Hands it to the route's error page, so a resident sees "Something
+        // went wrong" and a way back instead of a blank screen.
+        throw new Error("Couldn't check the session: the server didn't answer");
+    }
+    if (!user) {
         return null;
     }
 

frontend/src/auth/RouteGuard.tsx

@@ -1,6 +1,21 @@
+import { useEffect } from 'react';
 import { Navigate, Outlet } from 'react-router-dom';
+import { toast } from 'sonner';
 import { AUTHCALLBACK, hasFeature, useAuth } from '@/auth/useAuth';
-import { FeatureAccess, UserRole } from '@/types';
+import { FeatureAccess, User, UserRole } from '@/types';
+
+const NO_ACCESS = "You don't have access to that page";
+
+function canOpen(
+    user: User,
+    allowedRoles?: UserRole[],
+    features?: FeatureAccess[]
+): boolean {
+    if (allowedRoles && !allowedRoles.includes(user.role)) {
+        return false;
+    }
+    return !features || hasFeature(user, ...features);
+}
 
 export default function RouteGuard({
     allowedRoles,
@@ -10,17 +25,22 @@ export default function RouteGuard({
     features?: FeatureAccess[];
 }) {
     const { user } = useAuth();
+    const denied = user !== undefined && !canOpen(user, allowedRoles, features);
+
+    useEffect(() => {
+        if (denied) {
+            // The id keeps a repeat (StrictMode, a second guard) to one toast.
+            toast.error(NO_ACCESS, { id: NO_ACCESS });
+        }
+    }, [denied]);
 
     // AuthProvider (mounted in AuthenticatedShell) guarantees a user before
     // children render, but guard defensively.
     if (!user) {
         return null;
     }
-    if (
-        (allowedRoles && !allowedRoles.includes(user.role)) ||
-        (features && !hasFeature(user, ...features))
-    ) {
-        return <Navigate to={AUTHCALLBACK} />;
+    if (denied) {
+        return <Navigate to={AUTHCALLBACK} replace />;
     }
     return <Outlet />;
 }

frontend/src/auth/useAuth.ts

@@ -62,16 +62,82 @@ export const isStudent = (user: User): boolean => {
 export const SESSION_URL = '/sessions/whoami';
 export const KRATOS_CHECK_FLOW_URL = '/self-service/login/flows?id=';
 export const AUTHCALLBACK = '/authcallback';
+const KRATOS_LOGOUT_URL = '/self-service/logout/browser';
+const RETURN_TO_KEY = 'return_to';
+// Kratos's login flow lifespan. A saved page older than this was left by
+// someone who walked away from login, so the next person to sign in in that
+// tab doesn't inherit it.
+const RETURN_TO_MAX_AGE_MS = 10 * 60 * 1000;
 
-export async function fetchUser(): Promise<User | undefined> {
+/**
+ * Who core says is signed in: the user, 'signed-out' when it rejects the
+ * session, or 'unavailable' when it couldn't answer (server down, network).
+ */
+export async function checkSession(): Promise<
+    User | 'signed-out' | 'unavailable'
+> {
     const response = await API.get<User>('auth');
-    const user = response.data as User;
-    return user;
+    if (response.success) {
+        return response.data as User;
+    }
+    return response.status === 401 || response.status === 403
+        ? 'signed-out'
+        : 'unavailable';
+}
+
+export async function fetchUser(): Promise<User | undefined> {
+    const session = await checkSession();
+    return typeof session === 'string' ? undefined : session;
+}
+
+type Page = Pick<URL, 'pathname' | 'search' | 'hash'>;
+
+/**
+ * Sends a signed-out visitor to login, and once they sign in, back to page.
+ * The page is remembered per tab, for RETURN_TO_MAX_AGE_MS, and the next
+ * sign-in in that tab uses it up.
+ */
+export async function signInAgain({
+    pathname,
+    search,
+    hash
+}: Page): Promise<void> {
+    if (pathname !== AUTHCALLBACK) {
+        setSessionItem(
+            RETURN_TO_KEY,
+            JSON.stringify({ path: pathname + search + hash, at: Date.now() })
+        );
+    }
+    await handleLogout();
+}
+
+function takeReturnTo(): string | null {
+    const saved = getSessionItem(RETURN_TO_KEY);
+    removeSessionItem(RETURN_TO_KEY);
+    try {
+        const { path, at } = JSON.parse(saved ?? '') as {
+            path?: unknown;
+            at?: unknown;
+        };
+        const fresh =
+            typeof at === 'number' && Date.now() - at < RETURN_TO_MAX_AGE_MS;
+        // Only a path on this site: browsers read "//host" and "/\host" as
+        // another site.
+        return fresh &&
+            typeof path === 'string' &&
+            path.startsWith('/') &&
+            !/^\/[/\\]/.test(path)
+            ? path
+            : null;
+    } catch {
+        return null;
+    }
 }
 
 export const checkRole: LoaderFunction = async () => {
     const user = await fetchUser();
-    return redirect(getDashboardLink(user));
+    const returnTo = user ? takeReturnTo() : null;
+    return redirect(returnTo ?? getDashboardLink(user));
 };
 
 export function useAuth(): AuthContextType {
@@ -219,27 +285,35 @@ export const checkExistingFlow: LoaderFunction = async ({ request }) => {
     return json<AuthFlow>(attributes);
 };
 
+/**
+ * Ends the Kratos session and lands on login, whatever fails along the way.
+ * It asks Kratos directly rather than core, because it also runs after core
+ * has rejected the session. Ending the Kratos session matters then too: a
+ * session Kratos still accepts (a deactivated user's) would send the login
+ * flow straight back into the app, and the app back to login.
+ */
 export async function handleLogout(): Promise<void> {
+    resetAnalytics();
+    tabSessionManager.onLogout();
+    window.dispatchEvent(new Event('logoutEvent'));
     try {
-        resetAnalytics();
-        tabSessionManager.onLogout();
-        const resp = await API.post<AuthResponse, object>('logout', {});
-        if (resp.success) {
-            const logout = await fetch(
-                (resp.data as AuthResponse).redirect_to,
-                { credentials: 'include' }
-            );
-            if (logout.status === 200) {
-                window.dispatchEvent(new Event('logoutEvent'));
-                const logoutResp = (await logout.json()) as AuthResponse;
-                window.location.replace(
-                    logoutResp.logout_url ?? INIT_KRATOS_LOGIN_FLOW
-                );
+        // Without the Accept header, Kratos answers "no session" by redirecting
+        // to its error page instead of a 401.
+        const resp = await fetch(KRATOS_LOGOUT_URL, {
+            credentials: 'include',
+            headers: { Accept: 'application/json' }
+        });
+        if (resp.ok) {
+            const { logout_url } = (await resp.json()) as AuthResponse;
+            if (logout_url) {
+                window.location.replace(logout_url);
+                return;
             }
         }
     } catch {
-        window.location.href = INIT_KRATOS_LOGIN_FLOW;
+        // Falls through to the login flow.
     }
+    window.location.replace(INIT_KRATOS_LOGIN_FLOW);
 }
 
 export const getDashboardLink = (user?: User) => {

frontend/src/layouts/AuthenticatedShell.tsx

@@ -11,13 +11,18 @@ import AuthenticatedLayout from '@/layouts/AuthenticatedLayout';
  */
 export default function AuthenticatedShell() {
     return (
-        <AuthProvider>
-            <PageTitleProvider>
-                <BreadcrumbProvider>
-                    <AuthenticatedLayout />
-                </BreadcrumbProvider>
-            </PageTitleProvider>
+        <>
+            {/* Outside AuthProvider so it's listening before any page renders:
+                sonner drops a toast fired before the Toaster mounts, such as
+                RouteGuard's on a page load. */}
             <Toaster position="bottom-right" />
-        </AuthProvider>
+            <AuthProvider>
+                <PageTitleProvider>
+                    <BreadcrumbProvider>
+                        <AuthenticatedLayout />
+                    </BreadcrumbProvider>
+                </PageTitleProvider>
+            </AuthProvider>
+        </>
     );
 }

frontend/src/loaders/routeLoaders.ts

@@ -18,7 +18,12 @@ import {
     BreadcrumbItem
 } from '@/types';
 import API from '@/api/api';
-import { fetchUser, hasFeature } from '@/auth/useAuth';
+import {
+    checkSession,
+    fetchUser,
+    hasFeature,
+    signInAgain
+} from '@/auth/useAuth';
 import { FeatureAccess } from '@/types';
 import { decodeHtmlEntities } from '@/lib/decodeHtmlEntities';
 
@@ -144,7 +149,7 @@ const getLibraryOptionsHelper = async ({ request }: { request: Request }) => {
     return libraryOptions;
 };
 
-export const getProgramData: LoaderFunction = async ({ params }) => {
+export const getProgramData: LoaderFunction = async ({ params, request }) => {
     const { program_id } = params;
     const [tagsResp] = await Promise.all([API.get(`tags`)]);
     const categories = tagsResp.data as Option[];
@@ -162,7 +167,7 @@ export const getProgramData: LoaderFunction = async ({ params }) => {
                 { label: program.name }
             ];
         } else {
-            return redirectOnError(resp);
+            return redirectOnError(resp, request);
         }
     }
     return json({
@@ -197,7 +202,7 @@ export const getProgramTitle: LoaderFunction = async ({
         if (resp.success) {
             programName = 'Program: ' + (resp.data as Program).name;
         } else {
-            return redirectOnError(resp);
+            return redirectOnError(resp, request);
         }
     }
     if (class_id && class_id != 'new') {
@@ -207,7 +212,7 @@ export const getProgramTitle: LoaderFunction = async ({
         if (classResp.success) {
             cls = classResp.data;
         } else {
-            return redirectOnError(classResp);
+            return redirectOnError(classResp, request);
         }
     }
 
@@ -293,16 +298,16 @@ export const getClassTitle: LoaderFunction = async ({
             breadcrumbs
         };
     } else {
-        return redirectOnError(classResp);
+        return redirectOnError(classResp, request);
     }
 };
 
-export const getProgram: LoaderFunction = async ({ params }) => {
+export const getProgram: LoaderFunction = async ({ params, request }) => {
     const resp = (await API.get(`programs/${params.id}`)) as ServerResponseOne<
         ProgramOverview | Response
     >;
     if (!resp.success) {
-        return redirectOnError(resp);
+        return redirectOnError(resp, request);
     }
     return json(resp.data);
 };
@@ -317,7 +322,15 @@ export function resolveTitle<T>(
         : handle.title;
 }
 
-function redirectOnError<T>(resp: ServerResponse<T>) {
+async function redirectOnError<T>(resp: ServerResponse<T>, request: Request) {
+    // Core also answers 401 to a signed-in user who may not open the page, so
+    // only a 401 for a session core rejects means signed out.
+    if (resp.status === 401 && (await checkSession()) === 'signed-out') {
+        await signInAgain(new URL(request.url));
+        // The browser is already on its way to login. Never settling keeps the
+        // router from showing anything before it leaves.
+        return new Promise<never>(() => undefined);
+    }
     if (resp.status === 401 || resp.status === 403) {
         return redirect('/unauthorized');
     }