auth0 / auth0-react

Auth0 SDK for React Single Page Applications (SPA)
MIT License
871 stars 253 forks source link

withAuthenticationRequired does not consider errors #370

Closed danjunger closed 2 years ago

danjunger commented 2 years ago

Please do not report security vulnerabilities here. The Responsible Disclosure Program details the procedure for disclosing security issues.

Thank you in advance for helping us to improve this library! Please read through the template below and answer all relevant questions. Your additional work here is greatly appreciated and will help us respond as quickly as possible. For general support or usage questions, use the Auth0 Community or Auth0 Support. Finally, to avoid duplicates, please search existing Issues before submitting one here.

By submitting an Issue to this repository, you agree to the terms within the Auth0 Code of Conduct.

Describe the problem

Any time an error occurs within an Auth0 rule, an application which uses the withAuthenticationRequired higher order component to guard the app will be sent into an infinite loop of checking if the user is authenticated, finding that the user is not authenticated, and then calling loginWithRedirect again which starts the loop over again.

The code for this behavior is here:

[loginWithRedirect](https://github.com/auth0/auth0-react/blob/master/src/with-authentication-required.tsx#L97-L111)

This code has an opportunity to handle this error, but currently does not consider any error that could be returned from the call to useAuth0.

What was the expected behavior?

I would like to see:

  1. Some default error handling to catch the error from useAuth0 and display or throw the error instead of just calling loginWithRedirect again.
  2. Even better would be if we were allowed to supply our own error page to be rendered with the error, ie something like this:
export const withAuthenticationRequired = <P extends object>(
  Component: ComponentType<P>,
  options: WithAuthenticationRequiredOptions = {},
  ErrorPage: ComponentType<{ error: Error }> = ({ error }) => <div>Error: {error.message}</div>
): FC<P> => {
  return function WithAuthenticationRequired(props: P): JSX.Element {
    const { error, user, isAuthenticated, isLoading, loginWithRedirect } = useAuth0();
    const {
      returnTo = defaultReturnTo,
      onRedirecting = defaultOnRedirecting,
      claimCheck = (): boolean => true,
      loginOptions,
    } = options;

    /**
     * The route is authenticated if the user has valid auth and there are no
     * JWT claim mismatches.
     */
    const routeIsAuthenticated = isAuthenticated && claimCheck(user);

    useEffect(() => {
      if (isLoading || routeIsAuthenticated) {
        return;
      }
      const opts = {
        ...loginOptions,
        appState: {
          ...(loginOptions && loginOptions.appState),
          returnTo: typeof returnTo === 'function' ? returnTo() : returnTo,
        },
      };

      if (error) {
        // prevent the infinite loop
        return;
      }

      (async (): Promise<void> => {
        await loginWithRedirect(opts);
      })();
    }, [error, isLoading, routeIsAuthenticated, loginWithRedirect, loginOptions, returnTo]);

    if (error) {
      // gracefully allow the user of this library to handle the error
      return <ErrorPage error={error} />;
    }

    return routeIsAuthenticated ? <Component {...props} /> : onRedirecting();
  };
};

Reproduction

  1. Make an application component which is wrapped with withAuthenticationRequired
  2. Make an Auth0 rule which raises an exception
  3. Log in
  4. Infinite loginWithRedirect loop

Can the behavior be reproduced using the React SDK Playground?

If so, provide steps:

Where applicable, please include:

  • The smallest possible sample app that reproduces the undesirable behavior
  • Log files (redact/remove sensitive information)
  • Application settings (redact/remove sensitive information)
  • clientId and domain (these are publicly available)
  • Screenshots

Environment

Please provide the following:

adamjmcgrath commented 2 years ago

Hi @danjunger - thanks for raising this

withAuthenticationProvider needs to handle errors that a user can recover from by logging in again (login_required, consent_required, mfa_required, interaction_required etc)

For errors like configuration errors or errors from the handleRedirectCallback you need to handle them at the app level, before you start rendering routes that require authentication.

Closing as a Duplicate of https://github.com/auth0/auth0-react/issues/298#issuecomment-961949134

Kavan72 commented 6 months ago

@adamjmcgrath would be great if you can add one example which handle post-login action errors with withAuthenticationRequired