4

I have a web application that I've been developing for a little over a year and some change. The frontend is react w/ react-router-dom 5.2 to handle navigation, a service worker, to handle caching, installing, and webpush notifications, and then the backend is a Javalin application, which exists on top of Jetty.

I am using the context API to store some session details. When you navigate to my application, if you are not already logged in, then you won't have your information stored in that context yet, so you will be redirected to /login which will begin that process. The LoginLogout component simply redirects to an external authserver that handles the authentication workflow before redirecting back to another endpoint.

Here's the detail:

  1. There are no redirects to /login in the server code and the ProtectedRoute code is definitely to blame for this issue. Navigating to /login is causing either an infinite redirect or an infinite rerender.
  2. All redirects server side are performed with code 302 temporary. And again, none of them point to /login
  3. The issue, as I have tracked it down, I believe has something to do with the context itself. I have made modifications to the context and now I am experiencing different behavior from before, when I believed the service worker to be the culprit. The issue is still an infinite redirect or rerender and is hard to troubleshoot.
  4. I know the server is doing it's part and the /auth/check endpoint is providing exactly what it should at all times.

Here's my ProtectedRoute code

import { Redirect, Route, useLocation } from "react-router-dom";
import PropTypes from "prop-types";
import React, { useContext, useEffect, useState } from "react";
import { AuthContext } from "../Contexts/AuthProvider";
import LoadingComponent from "components/Loading/LoadingComponent";
import { server } from "variables/sitevars";

export const ProtectedRoute = ({ component: Component, ...rest }) => {
  const { session, setSession } = useContext(AuthContext);
  const [isLoading, setLoading] = useState(true);
  const [isError, setError] = useState(false);
  const cPath = useLocation().pathname;

  //Create a checkAgainTime
  const getCAT = (currTime, expireTime) => {
    return new Date(
      Date.now() + (new Date(expireTime) - new Date(currTime)) * 0.95
    );
  };

  //See if it's time to check with the server about our session
  const isCheckAgainTime = (checkTime) => {
    if (checkTime === undefined) {
      return true;
    } else {
      return Date.now() >= checkTime;
    }
  };

  useEffect(() => {
    let isMounted = true;
    let changed = false;
    if (isMounted) {
      (async () => {
        let sesh = session;
        try {
          //If first run, or previously not logged in, or past checkAgain
          if (!sesh.isLoggedIn || isCheckAgainTime(sesh.checkAgainTime)) {
            //Do fetch
            const response = await fetch(`${server}/auth/check`);
            if (response.ok) {
              const parsed = await response.json();
              //Set Login Status
              if (!sesh.isLoggedIn && parsed.isLoggedIn) {
                sesh.isLoggedIn = parsed.isLoggedIn;
                sesh.webuser = parsed.webuser;
                sesh.perms = parsed.perms;
                if (sesh.checkAgainTime === undefined) {
                  //Set checkAgainTime if none already set
                  sesh.checkAgainTime = getCAT(
                    parsed.currTime,
                    parsed.expireTime
                  );
                }
                changed = true;
              }
              if (sesh.isLoggedIn && !parsed.isLoggedIn) {
                sesh.isLoggedIn = false;
                sesh.checkAgainTime = undefined;
                sesh.webuser = undefined;
                sesh.perms = undefined;
                changed = true;
              }
            } else {
              setError(true);
            }
          }
          if (changed) {
            setSession(sesh);
          }
        } catch (error) {
          setError(true);
        }
        setLoading(false);
      })();
    }
    return function cleanup() {
      isMounted = false;
    };
  }, []);

  if (isLoading) {
    return <LoadingComponent isLoading={isLoading} />;
  }

  if (session.isLoggedIn && !isError) {
    return (
      <Route
        {...rest}
        render={(props) => {
          return <Component {...props} />;
        }}
      />
    );
  }

  if (!session.isLoggedIn && !isError) {
    return <Redirect to="/login" />;
  }

  if (isError) {
    return <Redirect to="/offline" />;
  }

  return null;    
};

ProtectedRoute.propTypes = {
  component: PropTypes.any.isRequired,
  exact: PropTypes.bool,
  path: PropTypes.string.isRequired,
};

Here's the use of the Authprovider. I also went ahead and give login/logout a different endpoint:

export default function App() {
  return (
    <BrowserRouter>
      <Switch>
        <Suspense fallback={<LoadingComponent />}>
          <Route path="/login" exact component={InOutRedirect} />
          <Route path="/logout" exact component={InOutRedirect} />
          <Route path="/auth/forbidden" component={AuthPage} />
          <Route path="/auth/error" component={ServerErrorPage} />
          <Route path="/offline" component={OfflinePage} />
          <AuthProvider>
            <ProtectedRoute path="/admin" component={AdminLayout} />
          </AuthProvider>
        </Suspense>
      </Switch>
    </BrowserRouter>
  );
}

And this is the AuthProvider itself:

import React, { createContext, useState } from "react";
import PropTypes from "prop-types";

export const AuthContext = createContext(null);

import { defaultProfilePic } from "../../views/Users/UserVarsAndFuncs/UserVarsAndFuncs";

const AuthProvider = (props) => {
  const [session, setSesh] = useState({
    isLoggedIn: undefined,
    checkAgainTime: undefined,
    webuser: {
      IDX: undefined,
      LastName: "",
      FirstName: "",
      EmailAddress: "",
      ProfilePic: defaultProfilePic,
    },
    perms: {
      IDX: undefined,
      Name: "",
      Descr: "",
    },
  });

  const setSession = (newSession) => {
    setSesh(newSession);
  };

  return (
    <AuthContext.Provider value={{ session, setSession }}>
      {props.children}
    </AuthContext.Provider>
  );
};

export default AuthProvider;

AuthProvider.propTypes = {
  children: PropTypes.any,
};

Update: Because it was asked for, here is my login/logout component, with the changes suggested (separated from the ProtectedRoute dependency)

import React, { useEffect, useState } from "react";
import { Redirect, useLocation } from "react-router-dom";

//Components
import LoadingComponent from "components/Loading/LoadingComponent";
import { server } from "variables/sitevars";

//Component Specific Vars

export default function InOutRedirect() {
  const rPath = useLocation().pathname;
  const [isError, setError] = useState(false);
  const [isLoading, setLoading] = useState(true);

  useEffect(() => {
    let isMounted = true;
    if (isMounted) {
      (async () => {
        try {
          //Do fetch
          const response = await fetch(`${server}/auth/server/data`);
          if (response.ok) {
            const parsed = await response.json();
            if (rPath === "/login") {
              window.location.assign(`${parsed.LoginURL}`);
            } else if (rPath === "/logout") {
              window.location.assign(`${parsed.LogoutURL}`);
            }
          }
        } catch (error) {
          setError(true);
        }
      })();
      setLoading(false);
    }
    return function cleanup() {
      isMounted = false;
    };
  }, []);

  if (isLoading) {
    return <LoadingComponent />;
  }

  if (isError) {
    return <Redirect to="/offline" />;
  }
}

How can I track down this issue?

UPDATE: I have done further troubleshooting and am now convinced that something is wrong with how I'm using context and that the service worker does not actually play a role in this issue. I've updated the post to reflect this.

UPDATE 2: I have done further simplification. The issue is assuredly that the context is not updating via setSession either prior to the page rendering the redirect component and redirecting back to login, or altogether.

UPDATE 3: I believe I found the issue, not positive but I think it's resolved. The bounty already being offered, if someone can explain why this happened, it's yours.

halfer
  • 19,824
  • 17
  • 99
  • 186
TheFunk
  • 981
  • 11
  • 39
  • 1
    Why do you need your login route to be protected? Seems to me like it should be a regular route which redirects to a 3rd party, which then redirects to your route. – Lior Pollak Aug 09 '21 at 19:54
  • @LiorPollak the login route needs to cause the same fetch to /auth/check to occur to fetch the address of the external fusionauth server. When /auth/check is hit, regardless of whether you are logged in or not, the context is updated with the external FusionAuth URL. That's why I check that the URL isn't login when redirecting to login. – TheFunk Aug 09 '21 at 19:58
  • 1
    please share [MWE](https://en.wikipedia.org/wiki/Minimal_working_example#:~:text=In%20computing%2C%20a%20minimal%20working,to%20be%20demonstrated%20and%20reproduced.&text=A%20minimal%20working%20example%20may,short%20self%2Dcontained%20correct%20example.) – Chandan Aug 10 '21 at 08:43
  • 1
    @LiorPollak I went ahead and separated that logic but the issue persists. The page is rendering the redirects prior to updating the stateful context. – TheFunk Aug 10 '21 at 19:22
  • @TheFunk I am not able to reproduce the problem maybe the problem is with `InOutRedirect` component can you please share that too – Chandan Aug 11 '21 at 14:50
  • @Chandan, I'll add that code to the post above, but I think I found the answer. That said, I offered the bounty so if someone can explain what was going wrong or why the fix I put in place worked, then I'm obligated to fork over the gold. – TheFunk Aug 11 '21 at 15:27
  • 1
    @TheFunk After checking the old version of your question i understand why it was continously reloading, but I don't see any issue if null is returned or not return from `ProtectedRoutes` component with recent version would not cause any issue, please check [here](https://stackblitz.com/edit/react-nvjpt6) and point me if anything that I am missing – Chandan Aug 11 '21 at 18:51
  • 1
    @Chandan that looks correct. Maybe my assumption was wrong or cache was playing tricks on me, but I've been religiously deleting cache. – TheFunk Aug 11 '21 at 19:36

4 Answers4

2

The issue seems to be with your unordered conditions. What if you have not logged in but has error? There'll be no default render for this and will cause the application halt. When user tries to login the state is touched and and upon error, this won't match any render. When you put return null, it will first render that and after a while it will match to the correct condition and return that. So, you could order your conditions like:

if (isLoading) {
  // return ...
}
if (isError) {
  // return ...
}
if (session.isLoggedIn) {
 // return ...
}
return <Redirect to="/login" />;

Here, we're first checking if there is any error, and if it is so, redirect to error page. Otherwise, route to the logged in component or redirect to the login page.

Bhojendra Rauniyar
  • 83,432
  • 35
  • 168
  • 231
2

I think your issue is that you're trying to redirect to a server-side route. I ran into the same issue before. React-router is picking up the path and trying to serve it to you on the client side where there is no route available and it's using the default route which is causing the loop.

To resolve, create a route to mimic the server-side route, then redirect on the client side with react-router once the server-side workflow is completed.

EDIT: The suspense tag should be outside your switch iirc. And I would include a default route pointing to a 404 or similar.

DadBot 9001
  • 286
  • 1
  • 4
  • I think you might be correct. Were you attempting an OAuth code grant? Looking at the return in the network tab on my application, it looks like react-router is trying to serve my API's callback URL. And now that I am thinking about it, the authentication server's redirect to the callback URL (which is supposed to be handled server side) would in fact hit the browser before the server. I'm going to change the auth server's redirect URL and create a new route in react-router to test this. – TheFunk Aug 14 '21 at 19:22
  • My issue was related to an OAuth code grant. The resolution your proposing appears to be the same methodology I employed to resolve my issue. – DadBot 9001 Aug 14 '21 at 20:26
  • 1
    THIS IS WHAT IT WAS! Accounting for server side routes and moving Suspense outside of the Switch together resolved the issue! So it seems like this was multiple issues in one. – TheFunk Aug 15 '21 at 23:53
2

Here I was naively believing that Switch only allows Route or Redirect as direct children, or at the very least components with path will be treated like routes. So I have no idea how any of your routing is working, to me it looks like Suspense will just always be loaded and after that everything will be considered as children of Suspense after which it will just load and render all the routes.

Other than that you really don't need to do stuff like let isMounted = true, the fact that you're using useEffect means that it's mounted, you don't have to ask or tell it or clean up or anything, it's mounted, you can just know that, whatever is inside useEffect will only be executed on component mount, while whatever you return from useEffect will be executed on component unmount.

Other than that you really shouldn't put the auth context inside routes, it's business logic and it belongs outside, way outside of your page routes. The login should also not 'redirect' the user somewhere, it should just access the auth context and update the state, which should automatically rerender the tree from your auth context, no manual redirection required. Put the auth context in something like App and then import the router as a child of it.

  • I'm just now seeing this. This was half the issue, but I saw DadBot's post first. Sorry. I would have awarded you each half of the bounty if I had seen this sooner. – TheFunk Aug 15 '21 at 23:58
  • 1
    That's unfortunate because the accepted answer here is not the correct answer, the reason you were struggling with most of your stuff was because of all your routes loading. He also edited that and answered that after I answered you, but such is life. –  Aug 16 '21 at 00:10
  • @Matriarx I wish I could share some of the reward with you! I wasn't paying attention when posting my update. So sorry! – DadBot 9001 Aug 16 '21 at 00:53
1

I am hesitant to call this resolved. And will not accept this answer until I am sure. But the issue appears to have been, that I had no default render path in my ProtectedRoute. I've updated the ProtectedRoute code to include:

return null;

This was missing from my original ProtectedRoute. Without a default render path, the /login render path (if user not signed in and no error) was the only one that would return. I am not sure why. I expect it has something to do with how react batches state updates and renders. With return null this code is working....knock on wood.

TheFunk
  • 981
  • 11
  • 39