bjerkio/oidc-react

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

john-adatree opened this issue · 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

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?

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'));
    }
  };

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?

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?