From 5c6688773217d5545fda94eb3c746c21cdc6c60d Mon Sep 17 00:00:00 2001 From: tigattack <10629864+tigattack@users.noreply.github.com> Date: Thu, 25 Sep 2025 19:39:52 +0100 Subject: [PATCH] refactor(frontend): optimise auth process - Stops frontend trying to make calls that require auth before auth has occured - Stops frontend making calls that aren't necessary before auth has occured - Implements state machine to better handle auth phases --- frontend/src/App.jsx | 10 +++-- frontend/src/components/Layout.jsx | 9 ++++ frontend/src/constants/authPhases.js | 29 ++++++++++++ frontend/src/contexts/AuthContext.jsx | 45 ++++++++++++------- .../contexts/UpdateNotificationContext.jsx | 23 +++++++--- 5 files changed, 90 insertions(+), 26 deletions(-) create mode 100644 frontend/src/constants/authPhases.js diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 4f015f6..e6a3efd 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -2,6 +2,7 @@ import { Route, Routes } from "react-router-dom"; import FirstTimeAdminSetup from "./components/FirstTimeAdminSetup"; import Layout from "./components/Layout"; import ProtectedRoute from "./components/ProtectedRoute"; +import { isAuthPhase } from "./constants/authPhases"; import { AuthProvider, useAuth } from "./contexts/AuthContext"; import { ThemeProvider } from "./contexts/ThemeContext"; import { UpdateNotificationProvider } from "./contexts/UpdateNotificationContext"; @@ -20,11 +21,14 @@ import Settings from "./pages/Settings"; import Users from "./pages/Users"; function AppRoutes() { - const { needsFirstTimeSetup, checkingSetup, isAuthenticated } = useAuth(); + const { needsFirstTimeSetup, authPhase, isAuthenticated } = useAuth(); const isAuth = isAuthenticated(); // Call the function to get boolean value - // Show loading while checking if setup is needed - if (checkingSetup) { + // Show loading while checking setup or initialising + if ( + isAuthPhase.initialising(authPhase) || + isAuthPhase.checkingSetup(authPhase) + ) { return (
diff --git a/frontend/src/components/Layout.jsx b/frontend/src/components/Layout.jsx index fcda7d2..c5952e6 100644 --- a/frontend/src/components/Layout.jsx +++ b/frontend/src/components/Layout.jsx @@ -241,6 +241,14 @@ const Layout = ({ children }) => { // Fetch GitHub stars count const fetchGitHubStars = useCallback(async () => { + // Skip if already fetched recently + const lastFetch = localStorage.getItem("githubStarsFetchTime"); + const now = Date.now(); + if (lastFetch && now - parseInt(lastFetch, 15) < 600000) { + // 15 minute cache + return; + } + try { const response = await fetch( "https://api.github.com/repos/9technologygroup/patchmon.net", @@ -248,6 +256,7 @@ const Layout = ({ children }) => { if (response.ok) { const data = await response.json(); setGithubStars(data.stargazers_count); + localStorage.setItem("githubStarsFetchTime", now.toString()); } } catch (error) { console.error("Failed to fetch GitHub stars:", error); diff --git a/frontend/src/constants/authPhases.js b/frontend/src/constants/authPhases.js new file mode 100644 index 0000000..e5acecf --- /dev/null +++ b/frontend/src/constants/authPhases.js @@ -0,0 +1,29 @@ +/** + * Authentication phases for the centralized auth state machine + * + * Flow: INITIALISING → CHECKING_SETUP → READY + */ +export const AUTH_PHASES = { + INITIALISING: "INITIALISING", + CHECKING_SETUP: "CHECKING_SETUP", + READY: "READY", +}; + +/** + * Helper functions for auth phase management + */ +export const isAuthPhase = { + initialising: (phase) => phase === AUTH_PHASES.INITIALISING, + checkingSetup: (phase) => phase === AUTH_PHASES.CHECKING_SETUP, + ready: (phase) => phase === AUTH_PHASES.READY, +}; + +/** + * Check if authentication is fully initialised and ready + * @param {string} phase - Current auth phase + * @param {boolean} isAuthenticated - Whether user is authenticated + * @returns {boolean} - True if auth is ready for other contexts to use + */ +export const isAuthReady = (phase, isAuthenticated) => { + return isAuthPhase.ready(phase) && isAuthenticated; +}; diff --git a/frontend/src/contexts/AuthContext.jsx b/frontend/src/contexts/AuthContext.jsx index 17357e8..bf2cf87 100644 --- a/frontend/src/contexts/AuthContext.jsx +++ b/frontend/src/contexts/AuthContext.jsx @@ -5,6 +5,7 @@ import { useEffect, useState, } from "react"; +import { AUTH_PHASES, isAuthPhase } from "../constants/authPhases"; const AuthContext = createContext(); @@ -20,11 +21,11 @@ export const AuthProvider = ({ children }) => { const [user, setUser] = useState(null); const [token, setToken] = useState(null); const [permissions, setPermissions] = useState(null); - const [isLoading, setIsLoading] = useState(true); - const [permissionsLoading, setPermissionsLoading] = useState(false); const [needsFirstTimeSetup, setNeedsFirstTimeSetup] = useState(false); - const [checkingSetup, setCheckingSetup] = useState(true); + // Authentication state machine phases + const [authPhase, setAuthPhase] = useState(AUTH_PHASES.INITIALISING); + const [permissionsLoading, setPermissionsLoading] = useState(false); // Define functions first const fetchPermissions = useCallback(async (authToken) => { @@ -77,14 +78,20 @@ export const AuthProvider = ({ children }) => { // Use the proper fetchPermissions function fetchPermissions(storedToken); } + // User is authenticated, skip setup check + setAuthPhase(AUTH_PHASES.READY); } catch (error) { console.error("Error parsing stored user data:", error); localStorage.removeItem("token"); localStorage.removeItem("user"); localStorage.removeItem("permissions"); + // Move to setup check phase + setAuthPhase(AUTH_PHASES.CHECKING_SETUP); } + } else { + // No stored auth, check if setup is needed + setAuthPhase(AUTH_PHASES.CHECKING_SETUP); } - setIsLoading(false); }, [fetchPermissions]); // Refresh permissions when user logs in (no automatic refresh) @@ -202,10 +209,6 @@ export const AuthProvider = ({ children }) => { } }; - const isAuthenticated = () => { - return !!(token && user); - }; - const isAdmin = () => { return user?.role === "admin"; }; @@ -243,42 +246,50 @@ export const AuthProvider = ({ children }) => { if (response.ok) { const data = await response.json(); setNeedsFirstTimeSetup(!data.hasAdminUsers); + setAuthPhase(AUTH_PHASES.READY); // Setup check complete, move to ready phase } else { // If endpoint doesn't exist or fails, assume setup is needed setNeedsFirstTimeSetup(true); + setAuthPhase(AUTH_PHASES.READY); } } catch (error) { console.error("Error checking admin users:", error); // If there's an error, assume setup is needed setNeedsFirstTimeSetup(true); - } finally { - setCheckingSetup(false); + setAuthPhase(AUTH_PHASES.READY); } }, []); - // Check for admin users on initial load + // Check for admin users ONLY when in CHECKING_SETUP phase useEffect(() => { - if (!token && !user) { + if (isAuthPhase.checkingSetup(authPhase)) { checkAdminUsersExist(); - } else { - setCheckingSetup(false); } - }, [token, user, checkAdminUsersExist]); + }, [authPhase, checkAdminUsersExist]); const setAuthState = (authToken, authUser) => { setToken(authToken); setUser(authUser); localStorage.setItem("token", authToken); localStorage.setItem("user", JSON.stringify(authUser)); + setAuthPhase(AUTH_PHASES.READY); // Authentication complete, move to ready phase + }; + + // Computed loading state based on phase and permissions state + const isLoading = !isAuthPhase.ready(authPhase) || permissionsLoading; + + // Function to check authentication status (maintains API compatibility) + const isAuthenticated = () => { + return !!(user && token && isAuthPhase.ready(authPhase)); }; const value = { user, token, permissions, - isLoading: isLoading || permissionsLoading || checkingSetup, + isLoading, needsFirstTimeSetup, - checkingSetup, + authPhase, login, logout, updateProfile, diff --git a/frontend/src/contexts/UpdateNotificationContext.jsx b/frontend/src/contexts/UpdateNotificationContext.jsx index 84d2fe0..cb57f5d 100644 --- a/frontend/src/contexts/UpdateNotificationContext.jsx +++ b/frontend/src/contexts/UpdateNotificationContext.jsx @@ -1,5 +1,6 @@ import { useQuery } from "@tanstack/react-query"; -import { createContext, useContext, useState } from "react"; +import { createContext, useContext, useMemo, useState } from "react"; +import { isAuthReady } from "../constants/authPhases"; import { settingsAPI, versionAPI } from "../utils/api"; import { useAuth } from "./AuthContext"; @@ -17,16 +18,26 @@ export const useUpdateNotification = () => { export const UpdateNotificationProvider = ({ children }) => { const [dismissed, setDismissed] = useState(false); - const { user, token } = useAuth(); + const { authPhase, isAuthenticated } = useAuth(); - // Ensure settings are loaded + // Ensure settings are loaded - but only after auth is fully ready const { data: settings, isLoading: settingsLoading } = useQuery({ queryKey: ["settings"], queryFn: () => settingsAPI.get().then((res) => res.data), - enabled: !!(user && token), - retry: 1, + staleTime: 5 * 60 * 1000, // Settings stay fresh for 5 minutes + refetchOnWindowFocus: false, + enabled: isAuthReady(authPhase, isAuthenticated()), }); + // Memoize the enabled condition to prevent unnecessary re-evaluations + const isQueryEnabled = useMemo(() => { + return ( + isAuthReady(authPhase, isAuthenticated()) && + !!settings && + !settingsLoading + ); + }, [authPhase, isAuthenticated, settings, settingsLoading]); + // Query for update information const { data: updateData, @@ -38,7 +49,7 @@ export const UpdateNotificationProvider = ({ children }) => { staleTime: 10 * 60 * 1000, // Data stays fresh for 10 minutes refetchOnWindowFocus: false, // Don't refetch when window regains focus retry: 1, - enabled: !!(user && token && settings && !settingsLoading), // Only run when authenticated and settings are loaded + enabled: isQueryEnabled, }); const updateAvailable = updateData?.isUpdateAvailable && !dismissed;