fix: onboarding redirection loop and bug fixes (#3250)

* chore: try and catch added in handleSignInRedirection * chore: remove unnecessary hooks * fix: handleCopyIssueLink url updated * chore: swap next_url with next_path and validate redirection logic for next_path url
makeplane · Jan 22, 2024 · 9a02886 · 9a02886
1 parent 9665244
commit 9a02886
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 42 deletions.
diff --git a/web/components/auth-screens/not-authorized-view.tsx b/web/components/auth-screens/not-authorized-view.tsx
@@ -18,7 +18,8 @@ type Props = {
 
 export const NotAuthorizedView: React.FC<Props> = ({ actionButton, type }) => {
   const { user } = useUser();
-  const { asPath: currentPath } = useRouter();
+  const { query } = useRouter();
+  const { next_path } = query;
 
   return (
     <DefaultLayout>
@@ -37,15 +38,15 @@ export const NotAuthorizedView: React.FC<Props> = ({ actionButton, type }) => {
           {user ? (
             <p>
               You have signed in as {user.email}. <br />
-              <Link href={`/?next=${currentPath}`}>
+              <Link href={`/?next_path=${next_path}`}>
                 <span className="font-medium text-custom-text-100">Sign in</span>
               </Link>{" "}
               with different account that has access to this page.
             </p>
           ) : (
             <p>
               You need to{" "}
-              <Link href={`/?next=${currentPath}`}>
+              <Link href={`/?next_path=${next_path}`}>
                 <span className="font-medium text-custom-text-100">Sign in</span>
               </Link>{" "}
               with an account that has access to this page.

diff --git a/web/components/issues/issue-layouts/quick-action-dropdowns/archived-issue.tsx b/web/components/issues/issue-layouts/quick-action-dropdowns/archived-issue.tsx
@@ -23,7 +23,7 @@ export const ArchivedIssueQuickActions: React.FC<IQuickActionProps> = (props) =>
   const { setToastAlert } = useToast();
 
   const handleCopyIssueLink = () => {
-    copyUrlToClipboard(`/${workspaceSlug}/projects/${issue.project}/archived-issues/${issue.id}`).then(() =>
+    copyUrlToClipboard(`${workspaceSlug}/projects/${issue.project}/archived-issues/${issue.id}`).then(() =>
       setToastAlert({
         type: "success",
         title: "Link copied",

diff --git a/web/components/issues/issue-layouts/quick-action-dropdowns/cycle-issue.tsx b/web/components/issues/issue-layouts/quick-action-dropdowns/cycle-issue.tsx
@@ -27,7 +27,7 @@ export const CycleIssueQuickActions: React.FC<IQuickActionProps> = (props) => {
   const { setToastAlert } = useToast();
 
   const handleCopyIssueLink = () => {
-    copyUrlToClipboard(`/${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
+    copyUrlToClipboard(`${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
       setToastAlert({
         type: "success",
         title: "Link copied",

diff --git a/web/components/issues/issue-layouts/quick-action-dropdowns/module-issue.tsx b/web/components/issues/issue-layouts/quick-action-dropdowns/module-issue.tsx
@@ -27,7 +27,7 @@ export const ModuleIssueQuickActions: React.FC<IQuickActionProps> = (props) => {
   const { setToastAlert } = useToast();
 
   const handleCopyIssueLink = () => {
-    copyUrlToClipboard(`/${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
+    copyUrlToClipboard(`${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
       setToastAlert({
         type: "success",
         title: "Link copied",

diff --git a/web/components/issues/issue-layouts/quick-action-dropdowns/project-issue.tsx b/web/components/issues/issue-layouts/quick-action-dropdowns/project-issue.tsx
@@ -37,7 +37,7 @@ export const ProjectIssueQuickActions: React.FC<IQuickActionProps> = (props) =>
   const { setToastAlert } = useToast();
 
   const handleCopyIssueLink = () => {
-    copyUrlToClipboard(`/${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
+    copyUrlToClipboard(`${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
       setToastAlert({
         type: "success",
         title: "Link copied",

diff --git a/web/hooks/use-sign-in-redirection.ts b/web/hooks/use-sign-in-redirection.ts
@@ -17,36 +17,54 @@ const useSignInRedirection = (): UseSignInRedirectionProps => {
   const [error, setError] = useState<any | null>(null);
   // router
   const router = useRouter();
-  const { next_url } = router.query;
+  const { next_path } = router.query;
   // mobx store
   const {
     user: { fetchCurrentUser, fetchCurrentUserSettings },
   } = useMobxStore();
 
+  const isValidURL = (url: string): boolean => {
+    const disallowedSchemes = /^(https?|ftp):\/\//i;
+    return !disallowedSchemes.test(url);
+  };
+
+  console.log("next_path", next_path);
+
   const handleSignInRedirection = useCallback(
     async (user: IUser) => {
-      // if the user is not onboarded, redirect them to the onboarding page
-      if (!user.is_onboarded) {
-        router.push("/onboarding");
-        return;
-      }
-      // if next_url is provided, redirect the user to that url
-      if (next_url) {
-        router.push(next_url.toString());
-        return;
-      }
+      try {
+        // if the user is not onboarded, redirect them to the onboarding page
+        if (!user.is_onboarded) {
+          router.push("/onboarding");
+          return;
+        }
+        // if next_path is provided, redirect the user to that url
+        if (next_path) {
+          if (isValidURL(next_path.toString())) {
+            router.push(next_path.toString());
+            return;
+          } else {
+            router.push("/");
+            return;
+          }
+        }
+
+        // Fetch the current user settings
+        const userSettings: IUserSettings = await fetchCurrentUserSettings();
 
-      // if the user is onboarded, fetch their last workspace details
-      await fetchCurrentUserSettings()
-        .then((userSettings: IUserSettings) => {
-          const workspaceSlug =
-            userSettings?.workspace?.last_workspace_slug || userSettings?.workspace?.fallback_workspace_slug;
-          if (workspaceSlug) router.push(`/${workspaceSlug}`);
-          else router.push("/profile");
-        })
-        .catch((err) => setError(err));
+        // Extract workspace details
+        const workspaceSlug =
+          userSettings?.workspace?.last_workspace_slug || userSettings?.workspace?.fallback_workspace_slug;
+
+        // Redirect based on workspace details or to profile if not available
+        if (workspaceSlug) router.push(`/${workspaceSlug}`);
+        else router.push("/profile");
+      } catch (error) {
+        console.error("Error in handleSignInRedirection:", error);
+        setError(error);
+      }
     },
-    [fetchCurrentUserSettings, router, next_url]
+    [fetchCurrentUserSettings, router, next_path]
   );
 
   const updateUserInfo = useCallback(async () => {

diff --git a/web/hooks/use-user-auth.tsx b/web/hooks/use-user-auth.tsx
@@ -12,7 +12,7 @@ const workspaceService = new WorkspaceService();
 
 const useUserAuth = (routeAuth: "sign-in" | "onboarding" | "admin" | null = "admin") => {
   const router = useRouter();
-  const { next_url } = router.query;
+  const { next_path } = router.query;
 
   const [isRouteAccess, setIsRouteAccess] = useState(true);
   const {
@@ -29,6 +29,11 @@ const useUserAuth = (routeAuth: "sign-in" | "onboarding" | "admin" | null = "adm
     shouldRetryOnError: false,
   });
 
+  const isValidURL = (url: string): boolean => {
+    const disallowedSchemes = /^(https?|ftp):\/\//i;
+    return !disallowedSchemes.test(url);
+  };
+
   useEffect(() => {
     const handleWorkSpaceRedirection = async () => {
       workspaceService.userWorkspaces().then(async (userWorkspaces) => {
@@ -84,8 +89,15 @@ const useUserAuth = (routeAuth: "sign-in" | "onboarding" | "admin" | null = "adm
       if (!isLoading) {
         setIsRouteAccess(() => true);
         if (user) {
-          if (next_url) router.push(next_url.toString());
-          else handleUserRouteAuthentication();
+          if (next_path) {
+            if (isValidURL(next_path.toString())) {
+              router.push(next_path.toString());
+              return;
+            } else {
+              router.push("/");
+              return;
+            }
+          } else handleUserRouteAuthentication();
         } else {
           if (routeAuth === "sign-in") {
             setIsRouteAccess(() => false);
@@ -97,7 +109,7 @@ const useUserAuth = (routeAuth: "sign-in" | "onboarding" | "admin" | null = "adm
         }
       }
     }
-  }, [user, isLoading, routeAuth, router, next_url]);
+  }, [user, isLoading, routeAuth, router, next_path]);
 
   return {
     isLoading: isRouteAccess,

diff --git a/web/hooks/use-user.tsx b/web/hooks/use-user.tsx
@@ -31,7 +31,7 @@ export default function useUser({ redirectTo = "", redirectIfFound = false, opti
     ) {
       router.push(redirectTo);
       return;
-      // const nextLocation = router.asPath.split("?next=")[1];
+      // const nextLocation = router.asPath.split("?next_path=")[1];
       // if (nextLocation) {
       //   router.push(nextLocation as string);
       //   return;

diff --git a/web/layouts/auth-layout/user-wrapper.tsx b/web/layouts/auth-layout/user-wrapper.tsx
@@ -56,7 +56,7 @@ export const UserAuthWrapper: FC<IUserAuthWrapper> = observer((props) => {
 
   if (currentUserError) {
     const redirectTo = router.asPath;
-    router.push(`/?next=${redirectTo}`);
+    router.push(`/?next_path=${redirectTo}`);
     return null;
   }
 

diff --git a/web/pages/onboarding/index.tsx b/web/pages/onboarding/index.tsx
@@ -11,8 +11,6 @@ import { Controller, useForm } from "react-hook-form";
 import { useMobxStore } from "lib/mobx/store-provider";
 // services
 import { WorkspaceService } from "services/workspace.service";
-// hooks
-import useUserAuth from "hooks/use-user-auth";
 // layouts
 import DefaultLayout from "layouts/default-layout";
 import { UserAuthWrapper } from "layouts/auth-layout";
@@ -45,8 +43,6 @@ const OnboardingPage: NextPageWithLayout = observer(() => {
 
   const { setTheme } = useTheme();
 
-  const {} = useUserAuth("onboarding");
-
   const { control, setValue } = useForm<{ full_name: string }>({
     defaultValues: {
       full_name: "",
@@ -158,8 +154,8 @@ const OnboardingPage: NextPageWithLayout = observer(() => {
                             currentUser?.first_name
                               ? `${currentUser?.first_name} ${currentUser?.last_name ?? ""}`
                               : value.length > 0
-                                ? value
-                                : currentUser?.email
+                              ? value
+                              : currentUser?.email
                           }
                           src={currentUser?.avatar}
                           size={35}
@@ -174,8 +170,8 @@ const OnboardingPage: NextPageWithLayout = observer(() => {
                             {currentUser?.first_name
                               ? `${currentUser?.first_name} ${currentUser?.last_name ?? ""}`
                               : value.length > 0
-                                ? value
-                                : null}
+                              ? value
+                              : null}
                           </p>
                         )}