From f08d8593eac20075a2568d81720a7e075daed1a7 Mon Sep 17 00:00:00 2001 From: Stavros Date: Sat, 7 Feb 2026 12:01:19 +0200 Subject: [PATCH] refactor: rework frontend use effect calls (#630) * refactor: rework frontend use effect calls * fix: rabbit comments * fix: handle empty oauth url in login page --- frontend/src/lib/hooks/redirect-uri.ts | 64 +++++++++++++++++ frontend/src/lib/utils.ts | 9 --- frontend/src/pages/continue-page.tsx | 88 +++++++++++------------ frontend/src/pages/login-page.tsx | 97 ++++++++++++++++---------- frontend/src/pages/logout-page.tsx | 15 ++-- frontend/src/pages/totp-page.tsx | 21 +++--- 6 files changed, 187 insertions(+), 107 deletions(-) create mode 100644 frontend/src/lib/hooks/redirect-uri.ts diff --git a/frontend/src/lib/hooks/redirect-uri.ts b/frontend/src/lib/hooks/redirect-uri.ts new file mode 100644 index 0000000..bfd5d28 --- /dev/null +++ b/frontend/src/lib/hooks/redirect-uri.ts @@ -0,0 +1,64 @@ +type IuseRedirectUri = { + url?: URL; + valid: boolean; + trusted: boolean; + allowedProto: boolean; + httpsDowngrade: boolean; +}; + +export const useRedirectUri = ( + redirect_uri: string | null, + cookieDomain: string, +): IuseRedirectUri => { + let isValid = false; + let isTrusted = false; + let isAllowedProto = false; + let isHttpsDowngrade = false; + + if (!redirect_uri) { + return { + valid: false, + trusted: false, + allowedProto: false, + httpsDowngrade: false, + }; + } + + let url: URL; + + try { + url = new URL(redirect_uri); + } catch { + return { + valid: false, + trusted: false, + allowedProto: false, + httpsDowngrade: false, + }; + } + + isValid = true; + + if ( + url.hostname == cookieDomain || + url.hostname.endsWith(`.${cookieDomain}`) + ) { + isTrusted = true; + } + + if (url.protocol == "http:" || url.protocol == "https:") { + isAllowedProto = true; + } + + if (window.location.protocol == "https:" && url.protocol == "http:") { + isHttpsDowngrade = true; + } + + return { + url, + valid: isValid, + trusted: isTrusted, + allowedProto: isAllowedProto, + httpsDowngrade: isHttpsDowngrade, + }; +}; diff --git a/frontend/src/lib/utils.ts b/frontend/src/lib/utils.ts index a451ae6..7e685ac 100644 --- a/frontend/src/lib/utils.ts +++ b/frontend/src/lib/utils.ts @@ -5,15 +5,6 @@ export function cn(...inputs: ClassValue[]) { return twMerge(clsx(inputs)); } -export const isValidUrl = (url: string) => { - try { - new URL(url); - return true; - } catch { - return false; - } -}; - export const capitalize = (str: string) => { return str.charAt(0).toUpperCase() + str.slice(1); }; diff --git a/frontend/src/pages/continue-page.tsx b/frontend/src/pages/continue-page.tsx index 0505428..efcf220 100644 --- a/frontend/src/pages/continue-page.tsx +++ b/frontend/src/pages/continue-page.tsx @@ -8,10 +8,10 @@ import { } from "@/components/ui/card"; import { useAppContext } from "@/context/app-context"; import { useUserContext } from "@/context/user-context"; -import { isValidUrl } from "@/lib/utils"; import { Trans, useTranslation } from "react-i18next"; import { Navigate, useLocation, useNavigate } from "react-router"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { useRedirectUri } from "@/lib/hooks/redirect-uri"; export const ContinuePage = () => { const { cookieDomain, disableUiWarnings } = useAppContext(); @@ -20,48 +20,35 @@ export const ContinuePage = () => { const { t } = useTranslation(); const navigate = useNavigate(); - const [loading, setLoading] = useState(false); + const [isLoading, setIsLoading] = useState(false); const [showRedirectButton, setShowRedirectButton] = useState(false); + const hasRedirected = useRef(false); const searchParams = new URLSearchParams(search); const redirectUri = searchParams.get("redirect_uri"); - const isValidRedirectUri = - redirectUri !== null ? isValidUrl(redirectUri) : false; - const redirectUriObj = isValidRedirectUri - ? new URL(redirectUri as string) - : null; - const isTrustedRedirectUri = - redirectUriObj !== null - ? redirectUriObj.hostname === cookieDomain || - redirectUriObj.hostname.endsWith(`.${cookieDomain}`) - : false; - const isAllowedRedirectProto = - redirectUriObj !== null - ? redirectUriObj.protocol === "https:" || - redirectUriObj.protocol === "http:" - : false; - const isHttpsDowngrade = - redirectUriObj !== null - ? redirectUriObj.protocol === "http:" && - window.location.protocol === "https:" - : false; + const { url, valid, trusted, allowedProto, httpsDowngrade } = useRedirectUri( + redirectUri, + cookieDomain, + ); - const handleRedirect = () => { - setLoading(true); - window.location.assign(redirectUriObj!.toString()); - }; + const handleRedirect = useCallback(() => { + hasRedirected.current = true; + setIsLoading(true); + window.location.assign(url!); + }, [url]); useEffect(() => { if (!isLoggedIn) { return; } + if (hasRedirected.current) { + return; + } + if ( - (!isValidRedirectUri || - !isAllowedRedirectProto || - !isTrustedRedirectUri || - isHttpsDowngrade) && + (!valid || !allowedProto || !trusted || httpsDowngrade) && !disableUiWarnings ) { return; @@ -72,7 +59,7 @@ export const ContinuePage = () => { }, 100); const reveal = setTimeout(() => { - setLoading(false); + setIsLoading(false); setShowRedirectButton(true); }, 5000); @@ -80,22 +67,33 @@ export const ContinuePage = () => { clearTimeout(auto); clearTimeout(reveal); }; - }); + }, [ + isLoggedIn, + hasRedirected, + valid, + allowedProto, + trusted, + httpsDowngrade, + disableUiWarnings, + setIsLoading, + handleRedirect, + setShowRedirectButton, + ]); if (!isLoggedIn) { return ( ); } - if (!isValidRedirectUri || !isAllowedRedirectProto) { + if (!valid || !allowedProto) { return ; } - if (!isTrustedRedirectUri && !disableUiWarnings) { + if (!trusted && !disableUiWarnings) { return ( @@ -115,8 +113,8 @@ export const ContinuePage = () => { @@ -133,7 +131,7 @@ export const ContinuePage = () => { ); } - if (isHttpsDowngrade && !disableUiWarnings) { + if (httpsDowngrade && !disableUiWarnings) { return ( @@ -151,13 +149,17 @@ export const ContinuePage = () => { - @@ -176,7 +178,7 @@ export const ContinuePage = () => { {showRedirectButton && ( - diff --git a/frontend/src/pages/login-page.tsx b/frontend/src/pages/login-page.tsx index f8221c7..d8f0929 100644 --- a/frontend/src/pages/login-page.tsx +++ b/frontend/src/pages/login-page.tsx @@ -40,10 +40,11 @@ export const LoginPage = () => { const { providers, title, oauthAutoRedirect } = useAppContext(); const { search } = useLocation(); const { t } = useTranslation(); - const [oauthAutoRedirectHandover, setOauthAutoRedirectHandover] = - useState(false); + const [showRedirectButton, setShowRedirectButton] = useState(false); + const hasAutoRedirectedRef = useRef(false); + const redirectTimer = useRef(null); const redirectButtonTimer = useRef(null); @@ -54,6 +55,11 @@ export const LoginPage = () => { compiled: compiledOIDCParams, } = useOIDCParams(searchParams); + const [isOauthAutoRedirect, setIsOauthAutoRedirect] = useState( + providers.find((provider) => provider.id === oauthAutoRedirect) !== + undefined && props.redirect_uri, + ); + const oauthProviders = providers.filter( (provider) => provider.id !== "local" && provider.id !== "ldap", ); @@ -62,10 +68,15 @@ export const LoginPage = () => { (provider) => provider.id === "local" || provider.id === "ldap", ) !== undefined; - const oauthMutation = useMutation({ + const { + mutate: oauthMutate, + data: oauthData, + isPending: oauthIsPending, + variables: oauthVariables, + } = useMutation({ mutationFn: (provider: string) => axios.get( - `/api/oauth/url/${provider}?redirect_uri=${encodeURIComponent(props.redirect_uri)}`, + `/api/oauth/url/${provider}${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`, ), mutationKey: ["oauth"], onSuccess: (data) => { @@ -76,22 +87,28 @@ export const LoginPage = () => { redirectTimer.current = window.setTimeout(() => { window.location.replace(data.data.url); }, 500); + + if (isOauthAutoRedirect) { + redirectButtonTimer.current = window.setTimeout(() => { + setShowRedirectButton(true); + }, 5000); + } }, onError: () => { - setOauthAutoRedirectHandover(false); + setIsOauthAutoRedirect(false); toast.error(t("loginOauthFailTitle"), { description: t("loginOauthFailSubtitle"), }); }, }); - const loginMutation = useMutation({ + const { mutate: loginMutate, isPending: loginIsPending } = useMutation({ mutationFn: (values: LoginSchema) => axios.post("/api/user/login", values), mutationKey: ["login"], onSuccess: (data) => { if (data.data.totpPending) { window.location.replace( - `/totp?redirect_uri=${encodeURIComponent(props.redirect_uri)}`, + `/totp${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`, ); return; } @@ -106,7 +123,7 @@ export const LoginPage = () => { return; } window.location.replace( - `/continue?redirect_uri=${encodeURIComponent(props.redirect_uri)}`, + `/continue${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`, ); }, 500); }, @@ -122,34 +139,34 @@ export const LoginPage = () => { useEffect(() => { if ( - providers.find((provider) => provider.id === oauthAutoRedirect) && !isLoggedIn && - props.redirect_uri !== "" + isOauthAutoRedirect && + !hasAutoRedirectedRef.current && + props.redirect_uri ) { - // Not sure of a better way to do this - // eslint-disable-next-line react-hooks/set-state-in-effect - setOauthAutoRedirectHandover(true); - oauthMutation.mutate(oauthAutoRedirect); - redirectButtonTimer.current = window.setTimeout(() => { - setShowRedirectButton(true); - }, 5000); + hasAutoRedirectedRef.current = true; + oauthMutate(oauthAutoRedirect); } }, [ - providers, isLoggedIn, - props.redirect_uri, + oauthMutate, + hasAutoRedirectedRef, oauthAutoRedirect, - oauthMutation, + isOauthAutoRedirect, + props.redirect_uri, ]); - useEffect( - () => () => { - if (redirectTimer.current) clearTimeout(redirectTimer.current); - if (redirectButtonTimer.current) + useEffect(() => { + return () => { + if (redirectTimer.current) { + clearTimeout(redirectTimer.current); + } + + if (redirectButtonTimer.current) { clearTimeout(redirectButtonTimer.current); - }, - [], - ); + } + }; + }, [redirectTimer, redirectButtonTimer]); if (isLoggedIn && isOidc) { return ; @@ -158,7 +175,7 @@ export const LoginPage = () => { if (isLoggedIn && props.redirect_uri !== "") { return ( ); @@ -168,7 +185,7 @@ export const LoginPage = () => { return ; } - if (oauthAutoRedirectHandover) { + if (isOauthAutoRedirect) { return ( @@ -183,7 +200,14 @@ export const LoginPage = () => {