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

hasCodeInUrl checks both hashParams and searchParams even though the OIDC Auth is configured for just one of them.

Open john-adatree opened this issue 1 year ago • 4 comments

Hi

I am hoping you can help with this issue

Using "oidc-react": "^1.5.1",

Please note my Authentication callback is configured to use the query delimiter '?'

In my app, I have a non-authentication callback from a third party to a page which includes the hash parameter 'code' example 'myapp.com/mypage#code=12345'

This page is wrapped in the oidc-react AuthProvider component. And because of that, the following code is executed

  useEffect(() => {
    const getUser = async (): Promise<void> => {
      /**
       * Check if the user is returning back from OIDC.
       */
      if (hasCodeInUrl(location)) {
        const user = await userManager.signinCallback();
        setUserData(user);
        setIsLoading(false);
        onSignIn && onSignIn(user);
        return;
      }
      ...

https://github.com/bjerkio/oidc-react/blob/main/src/AuthContext.tsx#L109

The hasCodeInUrl function sees the hash code parameter in the URL and returns true.

The issue is userManager.signinCallback(); will call readSigninResponseState and it checks to see what delimiter to use and it correctly chooses '?' because this is what is configured but since this a non-authentication callback and it is not using the '?' delimiter, it will always throw an error Error: No state in response

Possible solution

hasCodeInUrl should not blindly look for hashParams 'code' OR searchParams 'code' but instead should check which delimiter has been configured, '?' or '#' and just check for parameters using that delimiter.

Thanks John

john-adatree avatar Jul 20 '22 21:07 john-adatree

Hello 👋

Thanks for reaching out! You are definitely right, I haven't thought to much about this. Do you have an idea to fix it?

I'm not sure we should introduce the callback component as many others have, since it produces more boilerplate. What do you think? Maybe add the option to use a component like that, and/or disable hadCodeInUrl function?

simenandre avatar Jul 20 '22 21:07 simenandre

Since we have access to the userManager.settings

I would just use the same check oidc-client-js uses https://github.com/IdentityModel/oidc-client-js/blob/dev/src/OidcClient.js#L103 and https://github.com/IdentityModel/oidc-client-js/blob/dev/src/SigninRequest.js#L96

 const isCode = (response_type) => {
    var result = response_type.split(/\s+/g).filter(function (item) {
      return item === 'code';
    });
    return !!result[0];
  };
  
  const getDelimiter = (settings: UserManagerSettings): string => {
    let useQuery = settings.response_mode === 'query' || (!settings.response_mode && isCode(settings.response_type));

    let delimiter = useQuery ? '?' : '#';
    return delimiter;
  };

  const isOidcCallback = (location: Location, settings: UserManagerSettings): boolean => {
    let delimiter = getDelimiter(settings);

    if (delimiter === '?') {
      const searchParams = new URLSearchParams(location.search);
      return Boolean(searchParams.get('code') || searchParams.get('id_token') || searchParams.get('session_state'));
    } else {
      const hashParams = new URLSearchParams(location.hash.replace('#', '?'));
      return Boolean(hashParams.get('code') || hashParams.get('id_token') || hashParams.get('session_state'));
    }
  };

john-adatree avatar Jul 21 '22 06:07 john-adatree

We also have the redirect_uri in the UserManagerSettings, is there a reason we do not check the path to confirm it is the correct OIDC callback location?

john-adatree avatar Jul 21 '22 07:07 john-adatree

We also have the redirect_uri in the UserManagerSettings, is there a reason we do not check the path to confirm it is the correct OIDC callback location?

Nice, I haven't thought about that. We can check against that! Do you want to open a pull request on this?

simenandre avatar Jul 25 '22 19:07 simenandre