Skip to content

Commit

Permalink
Prod auth bug fix (#1351)
Browse files Browse the repository at this point in the history
* Fix issue where useEffect running twice as causing exception and breaking prod auth flow. Add a Public Route component that prevents logged in users from going to log in page

* Add ProtectedRoute component. Fix log-out component

* Fix effect logic

* Add sentry logging when initAuth method fails. Add comments and improve logic in auth interstitial page
  • Loading branch information
schroerbrian authored May 18, 2024
1 parent c3f2f07 commit e0d09f8
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 17 deletions.
35 changes: 25 additions & 10 deletions app/Router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { PopupMessageProp } from "components/ui";
import React from "react";
import { Switch, Route, Redirect } from "react-router-dom";
import { whiteLabel, useAppContext } from "utils";
import { ProtectedRoute } from "components/utils";
import { ProtectedRoute, PublicRoute } from "components/utils";

import { AuthInterstitial } from "pages/AuthInterstitial";
import { HomePage } from "pages/HomePage";
Expand Down Expand Up @@ -53,12 +53,6 @@ export const Router = ({
<Route exact path="/about" component={AboutPage} />
<Route exact path="/auth" component={AuthInterstitial} />
<Route exact path="/demo/listing" component={ListingDebugPage} />
<ProtectedRoute
exact
isAuthenticated={!!authState}
path="/navigator-dashboard"
component={NavigatorDashboard}
/>
{/* NB: /organizations/new must be listed before /organizations/:id or else the /new
step will be interpreted as an ID and will thus break the OrganizationEditPage */}
<Route
Expand Down Expand Up @@ -107,9 +101,30 @@ export const Router = ({
path="/breaking-news/edit"
component={EditBreakingNewsPage}
/>
<Route exact path="/log-in" component={LoginPage} />
<Route exact path="/sign-up" component={SignUpPage} />
<Route exact path="/log-out" component={LogoutPage} />
<ProtectedRoute
exact
isAuthenticated={!!authState}
path="/navigator-dashboard"
component={NavigatorDashboard}
/>
<PublicRoute
exact
isAuthenticated={!!authState}
path="/log-in"
component={LoginPage}
/>
<PublicRoute
exact
isAuthenticated={!!authState}
path="/sign-up"
component={SignUpPage}
/>
<ProtectedRoute
exact
isAuthenticated={!!authState}
path="/log-out"
component={LogoutPage}
/>

{/* UCSF white label paths */}
<Route
Expand Down
36 changes: 36 additions & 0 deletions app/components/utils/PublicRoute.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import React from "react";
import {
Redirect,
Route,
RouteComponentProps,
RouteProps,
} from "react-router-dom";

interface PublicRouteProps extends RouteProps {
// The type here is copied from the react-router component typing in react-router/index.d.ts
component:
| React.ComponentType<RouteComponentProps<any>>
| React.ComponentType<any>;
isAuthenticated: boolean;
redirectTo?: string;
}

export const PublicRoute = ({
component: Component,
isAuthenticated,
redirectTo = "/navigator-dashboard",
...rest
}: PublicRouteProps) => {
return (
<Route
{...rest}
render={(props) =>
!isAuthenticated ? (
<Component {...props} />
) : (
<Redirect to={{ pathname: redirectTo }} />
)
}
/>
);
};
1 change: 1 addition & 0 deletions app/components/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from "./ProtectedRoute";
export * from "./PublicRoute";
35 changes: 28 additions & 7 deletions app/pages/AuthInterstitial.tsx
Original file line number Diff line number Diff line change
@@ -1,35 +1,56 @@
/**
* This is the interstial redirect page after a user is authenticated via Auth0.
* The useEffect hook contains logic that checks the url for a hash, which contains an encoded
* access token. The code then intializes the user session using by decoding the token and
* passing the user props to the AppContext authState
* access token. The code then intializes the user session by decoding the token and passing
* the user props to the AppContext authState.
*
*/

import React, { useEffect } from "react";
import { useHistory } from "react-router-dom";
import * as Sentry from "@sentry/browser";

import * as AuthService from "utils/AuthService";
import { useAppContext } from "utils";
import { Loader } from "components/ui";

export const AuthInterstitial = () => {
const { authClient, setAuthState } = useAppContext();
const { authClient, authState, setAuthState } = useAppContext();
const history = useHistory();

useEffect(() => {
const { hash } = window.location;
// Check if the authState already exists and if so, redirect the user to the nav dashboard. We also
// want to prevent the initUserSession method from being called twice as that can break the auth flow
// and/or lead to other weird consequences
if (authState) {
history.push("navigator-dashboard");
return;
}

if (!hash || !hash.includes("access_token")) {
// If auth state does NOT exist BUT there is no hash or the hash has no access token, something went off somehow.
// Just default to redirecting the user to the log-in page until we can craft a more sophisticated way to handle this
history.push("log-in");
return;
}

AuthService.initializeUserSession(
window.location.hash,
authClient,
setAuthState
).then(() => {
history.push("navigator-dashboard");
});
}, [history, setAuthState, authClient]);
)
.then(() => {
history.push("navigator-dashboard");
})
.catch((err) => {
// Something went awry. Log the error and redirect the user back to the log-in page until we can better handle this case.
// Better luck next time (for now)!
// TODO: handle this case
Sentry.captureException(err);
history.push("log-in");
});
}, [authClient, authState, history, setAuthState]);

return (
<div>
Expand Down

0 comments on commit e0d09f8

Please sign in to comment.