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
This commit is contained in:
Anmol Singh Bhatia 2023-12-28 17:17:04 +05:30 committed by GitHub
parent 71bf049e89
commit 91e84aede1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 69 additions and 42 deletions

View File

@ -18,7 +18,8 @@ type Props = {
export const NotAuthorizedView: React.FC<Props> = ({ actionButton, type }) => { export const NotAuthorizedView: React.FC<Props> = ({ actionButton, type }) => {
const { user } = useUser(); const { user } = useUser();
const { asPath: currentPath } = useRouter(); const { query } = useRouter();
const { next_path } = query;
return ( return (
<DefaultLayout> <DefaultLayout>
@ -37,7 +38,7 @@ export const NotAuthorizedView: React.FC<Props> = ({ actionButton, type }) => {
{user ? ( {user ? (
<p> <p>
You have signed in as {user.email}. <br /> 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> <span className="font-medium text-custom-text-100">Sign in</span>
</Link>{" "} </Link>{" "}
with different account that has access to this page. with different account that has access to this page.
@ -45,7 +46,7 @@ export const NotAuthorizedView: React.FC<Props> = ({ actionButton, type }) => {
) : ( ) : (
<p> <p>
You need to{" "} You need to{" "}
<Link href={`/?next=${currentPath}`}> <Link href={`/?next_path=${next_path}`}>
<span className="font-medium text-custom-text-100">Sign in</span> <span className="font-medium text-custom-text-100">Sign in</span>
</Link>{" "} </Link>{" "}
with an account that has access to this page. with an account that has access to this page.

View File

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

View File

@ -27,7 +27,7 @@ export const CycleIssueQuickActions: React.FC<IQuickActionProps> = (props) => {
const { setToastAlert } = useToast(); const { setToastAlert } = useToast();
const handleCopyIssueLink = () => { const handleCopyIssueLink = () => {
copyUrlToClipboard(`/${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() => copyUrlToClipboard(`${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
setToastAlert({ setToastAlert({
type: "success", type: "success",
title: "Link copied", title: "Link copied",

View File

@ -27,7 +27,7 @@ export const ModuleIssueQuickActions: React.FC<IQuickActionProps> = (props) => {
const { setToastAlert } = useToast(); const { setToastAlert } = useToast();
const handleCopyIssueLink = () => { const handleCopyIssueLink = () => {
copyUrlToClipboard(`/${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() => copyUrlToClipboard(`${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
setToastAlert({ setToastAlert({
type: "success", type: "success",
title: "Link copied", title: "Link copied",

View File

@ -37,7 +37,7 @@ export const ProjectIssueQuickActions: React.FC<IQuickActionProps> = (props) =>
const { setToastAlert } = useToast(); const { setToastAlert } = useToast();
const handleCopyIssueLink = () => { const handleCopyIssueLink = () => {
copyUrlToClipboard(`/${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() => copyUrlToClipboard(`${workspaceSlug}/projects/${issue.project}/issues/${issue.id}`).then(() =>
setToastAlert({ setToastAlert({
type: "success", type: "success",
title: "Link copied", title: "Link copied",

View File

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

View File

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

View File

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

View File

@ -56,7 +56,7 @@ export const UserAuthWrapper: FC<IUserAuthWrapper> = observer((props) => {
if (currentUserError) { if (currentUserError) {
const redirectTo = router.asPath; const redirectTo = router.asPath;
router.push(`/?next=${redirectTo}`); router.push(`/?next_path=${redirectTo}`);
return null; return null;
} }

View File

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