fix: prevent infinite redirect oauth auth flow (coder#10430)

Emyrk · web-flow · commit 9d2b805fb7d0 · 2023-10-30T14:45:06.000-05:00
* fix: prevent infinite redirect oauth auth flow
diff --git a/coderd/externalauth.go b/coderd/externalauth.go
@@ -268,8 +268,9 @@ func (api *API) externalAuthCallback(externalAuthConfig *externalauth.Config) ht
 
 		redirect := state.Redirect
 		if redirect == "" {
-			// This is a nicely rendered screen on the frontend
-			redirect = fmt.Sprintf("/external-auth/%s", externalAuthConfig.ID)
+			// This is a nicely rendered screen on the frontend. Passing the query param lets the
+			// FE know not to enter the authentication loop again, and instead display an error.
+			redirect = fmt.Sprintf("/external-auth/%s?redirected=true", externalAuthConfig.ID)
 		}
 		http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect)
 	}
diff --git a/site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx b/site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx
@@ -6,16 +6,21 @@ import {
 } from "api/api";
 import { usePermissions } from "hooks";
 import { type FC } from "react";
-import { useParams } from "react-router-dom";
+import { useParams, useSearchParams } from "react-router-dom";
 import ExternalAuthPageView from "./ExternalAuthPageView";
 import { ApiErrorResponse } from "api/errors";
 import { isAxiosError } from "axios";
+import Box from "@mui/material/Box";
+import Button from "@mui/material/Button";
+import { SignInLayout } from "components/SignInLayout/SignInLayout";
+import { Welcome } from "components/Welcome/Welcome";
 
 const ExternalAuthPage: FC = () => {
   const { provider } = useParams();
   if (!provider) {
     throw new Error("provider must exist");
   }
+  const [searchParams] = useSearchParams();
   const permissions = usePermissions();
   const queryClient = useQueryClient();
   const getExternalAuthProviderQuery = useQuery({
@@ -72,6 +77,35 @@ const ExternalAuthPage: FC = () => {
     !getExternalAuthProviderQuery.data.authenticated &&
     !getExternalAuthProviderQuery.data.device
   ) {
+    const redirectedParam = searchParams?.get("redirected");
+    if (redirectedParam && redirectedParam.toLowerCase() === "true") {
+      // The auth flow redirected the user here. If we redirect back to the
+      // callback, that resets the flow and we'll end up in an infinite loop.
+      // So instead, show an error, as the user expects to be authenticated at
+      // this point.
+      // TODO: Unsure what to do about the device auth flow, should we also
+      // show an error there?
+      return (
+        <SignInLayout>
+          <Welcome message="Failed to validate oauth access token" />
+          <Box textAlign="center">
+            Attempted to validate the user&apos;s oauth access token from the
+            authentication flow. This situation may occur as a result of an
+            external authentication provider misconfiguration. Verify the
+            external authentication validation URL is accurately configured.
+          </Box>
+          <br />
+          <Button
+            onClick={() => {
+              // Redirect to the auth flow again. *crosses fingers*
+              window.location.href = `/external-auth/${provider}/callback`;
+            }}
+          >
+            Retry
+          </Button>
+        </SignInLayout>
+      );
+    }
     window.location.href = `/external-auth/${provider}/callback`;
     return null;
   }

Original file line number	Diff line number	Diff line change
`@@ -268,8 +268,9 @@ func (api API) externalAuthCallback(externalAuthConfig externalauth.Config) ht`
`268`	`268`
`269`	`269`	`redirect := state.Redirect`
`270`	`270`	`if redirect == "" {`
`271`		`- // This is a nicely rendered screen on the frontend`
`272`		`- redirect = fmt.Sprintf("/external-auth/%s", externalAuthConfig.ID)`
	`271`	`+ // This is a nicely rendered screen on the frontend. Passing the query param lets the`
	`272`	`+ // FE know not to enter the authentication loop again, and instead display an error.`
	`273`	`+ redirect = fmt.Sprintf("/external-auth/%s?redirected=true", externalAuthConfig.ID)`
`273`	`274`	`}`
`274`	`275`	`http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect)`
`275`	`276`	`}`