fix: handle empty oauth url in login page

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,
+  };
+};

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);
 };

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 (
       <Navigate
-        to={`/login?redirect_uri=${encodeURIComponent(redirectUri || "")}`}
+        to={`/login${redirectUri ? `?redirect_uri=${encodeURIComponent(redirectUri)}` : ""}`}
         replace
       />
     );
   }
 
-  if (!isValidRedirectUri || !isAllowedRedirectProto) {
+  if (!valid || !allowedProto) {
     return <Navigate to="/logout" replace />;
   }
 
-  if (!isTrustedRedirectUri && !disableUiWarnings) {
+  if (!trusted && !disableUiWarnings) {
     return (
       <Card role="alert" aria-live="assertive" className="min-w-xs sm:min-w-sm">
         <CardHeader>
@@ -115,8 +113,8 @@ export const ContinuePage = () => {
         </CardHeader>
         <CardFooter className="flex flex-col items-stretch gap-2">
           <Button
-            onClick={handleRedirect}
-            loading={loading}
+            onClick={() => handleRedirect()}
+            loading={isLoading}
             variant="destructive"
           >
             {t("continueTitle")}
@@ -124,7 +122,7 @@ export const ContinuePage = () => {
           <Button
             onClick={() => navigate("/logout")}
             variant="outline"
-            disabled={loading}
+            disabled={isLoading}
           >
             {t("cancelTitle")}
           </Button>
@@ -133,7 +131,7 @@ export const ContinuePage = () => {
     );
   }
 
-  if (isHttpsDowngrade && !disableUiWarnings) {
+  if (httpsDowngrade && !disableUiWarnings) {
     return (
       <Card role="alert" aria-live="assertive" className="min-w-xs sm:min-w-sm">
         <CardHeader>
@@ -151,13 +149,17 @@ export const ContinuePage = () => {
           </CardDescription>
         </CardHeader>
         <CardFooter className="flex flex-col items-stretch gap-2">
-          <Button onClick={handleRedirect} loading={loading} variant="warning">
+          <Button
+            onClick={() => handleRedirect()}
+            loading={isLoading}
+            variant="warning"
+          >
             {t("continueTitle")}
           </Button>
           <Button
             onClick={() => navigate("/logout")}
             variant="outline"
-            disabled={loading}
+            disabled={isLoading}
           >
             {t("cancelTitle")}
           </Button>
@@ -176,7 +178,7 @@ export const ContinuePage = () => {
       </CardHeader>
       {showRedirectButton && (
         <CardFooter className="flex flex-col items-stretch">
-          <Button onClick={handleRedirect}>
+          <Button onClick={() => handleRedirect()}>
             {t("continueRedirectManually")}
           </Button>
         </CardFooter>

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<number | null>(null);
   const redirectButtonTimer = useRef<number | null>(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 <Navigate to={`/authorize?${compiledOIDCParams}`} replace />;
@@ -158,7 +175,7 @@ export const LoginPage = () => {
   if (isLoggedIn && props.redirect_uri !== "") {
     return (
       <Navigate
-        to={`/continue?redirect_uri=${encodeURIComponent(props.redirect_uri)}`}
+        to={`/continue${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`}
         replace
       />
     );
@@ -168,7 +185,7 @@ export const LoginPage = () => {
     return <Navigate to="/logout" replace />;
   }
 
-  if (oauthAutoRedirectHandover) {
+  if (isOauthAutoRedirect) {
     return (
       <Card className="min-w-xs sm:min-w-sm">
         <CardHeader>
@@ -183,7 +200,14 @@ export const LoginPage = () => {
           <CardFooter className="flex flex-col items-stretch">
             <Button
               onClick={() => {
-                window.location.replace(oauthMutation.data?.data.url);
+                if (oauthData?.data.url) {
+                  window.location.replace(oauthData.data.url);
+                } else {
+                  setIsOauthAutoRedirect(false);
+                  toast.error(t("loginOauthFailTitle"), {
+                    description: t("loginOauthFailSubtitle"),
+                  });
+                }
               }}
             >
               {t("loginOauthAutoRedirectButton")}
@@ -214,12 +238,9 @@ export const LoginPage = () => {
                 title={provider.name}
                 icon={iconMap[provider.id] ?? <OAuthIcon />}
                 className="w-full"
-                onClick={() => oauthMutation.mutate(provider.id)}
-                loading={
-                  oauthMutation.isPending &&
-                  oauthMutation.variables === provider.id
-                }
-                disabled={oauthMutation.isPending || loginMutation.isPending}
+                onClick={() => oauthMutate(provider.id)}
+                loading={oauthIsPending && oauthVariables === provider.id}
+                disabled={oauthIsPending || loginIsPending}
               />
             ))}
           </div>
@@ -229,8 +250,8 @@ export const LoginPage = () => {
         )}
         {userAuthConfigured && (
           <LoginForm
-            onSubmit={(values) => loginMutation.mutate(values)}
-            loading={loginMutation.isPending || oauthMutation.isPending}
+            onSubmit={(values) => loginMutate(values)}
+            loading={loginIsPending || oauthIsPending}
           />
         )}
         {providers.length == 0 && (

frontend/src/pages/logout-page.tsx

@@ -29,7 +29,7 @@ export const LogoutPage = () => {
       });
 
       redirectTimer.current = window.setTimeout(() => {
-        window.location.assign("/login");
+        window.location.replace("/login");
       }, 500);
     },
     onError: () => {
@@ -39,12 +39,13 @@ export const LogoutPage = () => {
     },
   });
 
-  useEffect(
-    () => () => {
-      if (redirectTimer.current) clearTimeout(redirectTimer.current);
-    },
-    [],
-  );
+  useEffect(() => {
+    return () => {
+      if (redirectTimer.current) {
+        clearTimeout(redirectTimer.current);
+      }
+    };
+  }, [redirectTimer]);
 
   if (!isLoggedIn) {
     return <Navigate to="/login" replace />;

frontend/src/pages/totp-page.tsx

@@ -45,11 +45,11 @@ export const TotpPage = () => {
         if (isOidc) {
           window.location.replace(`/authorize?${compiledOIDCParams}`);
           return;
-        } else {
-          window.location.replace(
-            `/continue?redirect_uri=${encodeURIComponent(props.redirect_uri)}`,
-          );
         }
+
+        window.location.replace(
+          `/continue${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
+        );
       }, 500);
     },
     onError: () => {
@@ -59,12 +59,13 @@ export const TotpPage = () => {
     },
   });
 
-  useEffect(
-    () => () => {
-      if (redirectTimer.current) clearTimeout(redirectTimer.current);
-    },
-    [],
-  );
+  useEffect(() => {
+    return () => {
+      if (redirectTimer.current) {
+        clearTimeout(redirectTimer.current);
+      }
+    };
+  }, [redirectTimer]);
 
   if (!totpPending) {
     return <Navigate to="/" replace />;