react-oidc-context icon indicating copy to clipboard operation
react-oidc-context copied to clipboard

Why can't my React SPA react to `signin-silent?error=login_required` state?

Open berkerdemirer opened this issue 1 year ago • 5 comments

Hello,

I have a strange issue which I can't seem to solve. I have the following auth wrapper code and the config in my app. The problem is that the spa should log out and show the user login screen whenever I log out from my identity server. In the network tab of the spa I can see that the session monitor tried to do a document type GET call to sign in-silent and it fails but still returns 200 with an error in URL params such as:

react-oidc-context version is 2.2.2

https://myurl.com/signin-silent?error=login_required&state=2f1e294a48ec494b8f5ff707e18b58e5&session_state=9P2gJqv7QOEaYSdAfR93As4oMO9umhUdvTv-GqXLEuE.E8DD8DE3357CF3563ED3A75711E705DA

But nothing in my code reacts to this GET call and forwards the user to the login screen. Can someone guide me on what I am missing here?

Even after the page refresh, I am still in the app. Only if I open a new tab and go to the URL of the spa then I see the login screen.

//Auth wrapper


import { FC, useEffect, useState } from 'react';
import { useAuth } from 'react-oidc-context';
import { Loader } from '@fleet/shared';

interface AuthWrapperProps {
  appLoading: boolean;
}

interface ErrorResponse {
  error: string;
  error_description?: string;
  error_uri?: string;
  state?: string;
}

export function isErrorResponseType(error: unknown): error is ErrorResponse {
  if (typeof error === 'object' && error !== null) {
    const e = error as { error?: unknown };

    return 'error' in e && typeof e.error === 'string';
  }
  return false;
}

export const APP_REDIRECT_URI_KEY = 'REDIRECT_URI';

const AuthWrapper: FC<AuthWrapperProps> = ({ children, appLoading }) => {
  const auth = useAuth();
  const [loginAttempted, setLoginAttempted] = useState(false);

  // Attempt silent sign-in once if appropriate
  useEffect(() => {
    if (
      !loginAttempted &&
      !auth.isAuthenticated &&
      !auth.activeNavigator &&
      !auth.isLoading &&
      !auth.error
    ) {
      console.log(loginAttempted);
      setLoginAttempted(true);
      auth.signinSilent().catch((error) => {
        auth
          .signinRedirect()
          .catch(() => console.error('Sign-in redirect error'));
        console.error('Silent sign-in error:', error);
      });
    }
  }, [auth, loginAttempted]);

  useEffect(() => {
    if (!auth.error) {
      return;
    }

    if (
      isErrorResponseType(auth.error) &&
      [
        'interaction_required',
        'login_required',
        'account_selection_required',
        'consent_required',
      ].includes(auth.error.error)
    ) {
      console.error('Authentication error:', auth.error);
      return;
    }
  }, [auth.error]);

  const isLoading = auth.isLoading || auth.activeNavigator || appLoading;

  if (isLoading) {
    return <Loader active size="fullscreen" />;
  }

  if (!auth.isAuthenticated) {
    return null;
  }

  return <>{children}</>;
};

export default AuthWrapper;

//authConf.ts

const isProduction = process.env.NODE_ENV === 'production';
const configuration: AuthProviderProps = {
  checkSessionIntervalInSeconds: 2,
  accessTokenExpiringNotificationTimeInSeconds: 300,
  revokeTokensOnSignout: true,
  silentRequestTimeoutInSeconds: 20,
  authority: process.env.REACT_APP_AUTH_URL!,
  client_id: process.env.REACT_APP_IDENTITY_CLIENT_ID!,
  redirect_uri: process.env.REACT_APP_BASE_URL + '/signin-oidc',
  automaticSilentRenew: true,
  post_logout_redirect_uri: process.env.REACT_APP_BASE_URL + '/signout-oidc',
  silent_redirect_uri: process.env.REACT_APP_BASE_URL + '/signin-silent',
  response_type: 'code',
  scope: `openid profile ${process.env.REACT_APP_API_SCOPE}`,
  onSigninCallback: () => {
    const redirectUri = sessionStorage.getItem(APP_REDIRECT_URI_KEY) || '/';
    sessionStorage.removeItem(APP_REDIRECT_URI_KEY);
    window.location.replace(redirectUri);
  },
  monitorSession: isProduction,
};

export default configuration;

berkerdemirer avatar Mar 26 '24 11:03 berkerdemirer

actually, with 3.0.0 version it seems to be working

berkerdemirer avatar Mar 26 '24 12:03 berkerdemirer

actually, with 3.0.0 version it seems to be working

good to know

pamapa avatar Mar 26 '24 15:03 pamapa

I'm seeing the following in react-oidc-context

navigatorKeys.map((key) => [
          key,
          userManager[key] ? async (...args) => {
            dispatch({
              type: "NAVIGATOR_INIT",
              method: key
            });
            try {
              return await userManager[key](...args);
            } catch (error) {
              dispatch({ type: "ERROR", error });
              return null;
            } finally {
              dispatch({ type: "NAVIGATOR_CLOSE" });
            }
          } : unsupportedEnvironment(key)
        ])

Which silences the promise error, that comes from the OIDC silent signup for me when I do signinSilent (ErrorResponse, to be exact). It simply wraps every function, and catches all errors there through the internal event manager, looks like. That way the errors are no longer thrown in the original promises, i.e. signinSilent(...).catch() will never fire, but you will get .then(null) call.

That feels very counterintuitive considering oidc-client-ts original behavior, and I am not yet sure what to do about that. It happens on 2.2.5-2.4.0 at least. I assume the wrapper is needed to update the state and trigger reflow, but why it's not thrown back?

dantaeusb avatar Apr 20 '24 23:04 dantaeusb

I assume the wrapper is needed to update the state and trigger reflow, but why it's not thrown back?

In contrast to oidc-client-ts this library manages an error state, accessible with useAuth. Why can't you access error?

pamapa avatar Apr 22 '24 14:04 pamapa

@pamapa I can indeed, that's what I did, I am just confused by the decision to suppress that error and resolve with null, which does not have any information about the reason it was resolved with null. Moreover, it does not seem to be type safe (as in signinPopup(args?: SigninPopupArgs): Promise<User>;, which can be resolved to null). Looking at handling error state update, it might turn into a large map of every possible error handled by a single component, and the scenario "if this error happened when that flow failed here, then handle this way, but if flow failed in another place, handle another way" getting significantly more complex because of that.

I can understand the benefit of having that state manager in react wrapper for sure, just wasn't sure if that was intended behavior to just silence it and resolve promise. But from your answer, it looks like it is.

dantaeusb avatar Apr 22 '24 15:04 dantaeusb