okta-auth-js icon indicating copy to clipboard operation
okta-auth-js copied to clipboard

updateAuthState incorrectly updates state (indefinitely pending)

Open andyrichardson opened this issue 3 years ago • 11 comments

About

Hey there! I'm having an issue where a token expires, okta-auth-js (@okta/[email protected]) doesn't detect it, and following a call to updateAuthState, the auth state is incorrectly and indefinitely set as isPending: true.

Reproduction

  • Set a token in okta-auth-js
  • Wait for token to expire (or manually set expiry value in local storage)
  • call updateAuthState

Expected result

On auth state update and an expired token is found, the expired token is renewed and the okta auth state manager has the following state

{
  isPending: true,
  isAuthenticated: false
}

Once the token has been refreshed, the following state is updated

{
  isPending: false,
  isAuthenticated: true
}

Actual result

On auth state update and an expired token is found, the expired token is not renewed and the okta auth state manager has the following state

{
  isPending: true,
  isAuthenticated: false
}

Despite isPending having the value true, no token update or retrieval is pending and the state never updates.

Additional info

Example usage in React
export const AuthProvider: FC = ({ children }) => {
  const [user, setUser] = useState<UserClaims | undefined | 'ERROR'>();
  const [authState, setAuthState] = useState({ isPending: true, isAuthenticated: false });

  const authClient = useMemo(
    () =>
      new OktaAuth({
        clientId: process.env.REACT_APP_OKTA_CLIENT_ID,
        issuer: process.env.REACT_APP_ISSUER,
        redirectUri: `${window.location.origin}/implicit/callback`,
        scopes: ['openid', 'profile', 'email'],
        pkce: false,
        tokenManager: {
          autoRenew: true,
        },
      }),
    [],
  );

  useEffect(() => {
    // Subscribe to auth state on mount
    authClient.authStateManager.subscribe(setAuthState);

    // Update for initial render
    authClient.authStateManager.updateAuthState();

    // Check for changes to local storage every second
    const interval = window.setInterval(() => authClient.authStateManager.updateAuthState(), 1000);
    return () => {
      clearInterval(interval);
      authClient.authStateManager.unsubscribe(setAuthState);
    };
  }, [authClient, setAuthState]);

  // Handle token retrieval
  useEffect(() => {
    // Ignore if pending or authenticated
    if (authState.isPending || authState.isAuthenticated) {
      return;
    }

    // Store tokens if sourced from redirect
    if (authClient.token.isLoginRedirect()) {
      authClient.storeTokensFromRedirect();
      return;
    }

    authClient.token.getWithRedirect();
  }, [authState, authClient]);

  // Handle user object
  useEffect(() => {
    if (user || authState.isPending || !authState.isAuthenticated) {
      return;
    }

    authClient
      .getUser()
      .then(setUser)
      .catch((err) => {
        console.error(err); // eslint-disable-line
        setUser('ERROR');
      });
  }, [user, authState, authClient]);

  // TODO: remove once we understand spinner issue
  // eslint-disable-next-line no-console
  console.log('Auth state', authState);

  // Pending auth check / redirecting to login / fetching user
  if (authState.isPending || !authState.isAuthenticated || user === undefined) {
    return (
      <PageSpinnerContainer>
        <Spinner />
      </PageSpinnerContainer>
    );
  }

  if (user === 'ERROR') {
    return (
      <>
        Error fetching user
        <a href="/">Reload</a>
      </>
    );
  }

  return <AuthContext.Provider value={{ authClient, user }}>{children}</AuthContext.Provider>;
};

andyrichardson avatar Feb 01 '21 17:02 andyrichardson

@andyrichardson - Thanks for the input.

A quick note: manually changing the expiry time in localStorage will not trigger an attempt to replace an expired token, as that is set when the token is originally stored. Since you say this problem also happens if the token actually expires and you shouldn't end up in a perpetual isPending state regardless, we'll continue with diagnosing the core problem, but I wanted to highlight that difference.

Can you clarify a few things about the reproduction steps?

  • Are you using autoRenew? (it will default to true, and attempt to replace the token prior to expiration)
  • Are you using a refresh token? (i.e. are you passing offline_access into your scopes, assuming your org is configured to allow refresh tokens)

In addition to the repro, what is your goal with calling updateAuthState()? The auth state should update automatically when tokens expire, you should only call it manually if you've done something to alter the auth state that the AuthManager wouldn't be aware of. It should still work, so this isn't about the bug, but more about making sure we're serving your business need.

Thanks!

swiftone avatar Feb 01 '21 18:02 swiftone

Hey @swiftone, thanks for the response 🙏

A quick note: manually changing the expiry time in localStorage will not trigger an attempt to replace an expired token, as that is set when the token is originally stored

Totally with you on that - I wouldn't expect indirect changes to local storage to be detected by Okta. I would expect Okta to update the state appropriately following a call to updateAuthState.

Are you using autoRenew? (it will default to true, and attempt to replace the token prior to expiration)

Yes (see line 5 of the React example above)

Are you using a refresh token? (i.e. are you passing offline_access into your scopes, assuming your org is configured to allow refresh tokens)

Our org is configured to allow refresh tokens :+1:

Looks like we aren't currently passing offline_access (see line 11 of the React example above).

We're passing the following values to okta-auth-js - ['openid', 'profile', 'email']

The auth state should update automatically when tokens expire

Understood! We weren't calling updateAuthState, but tokens weren't being refreshed and the state wasn't being updated upon token expiry. Now the state is updating, although not correctly 🤔

andyrichardson avatar Feb 02 '21 15:02 andyrichardson

@andyrichardson Thank you for the detailed information. I have an opened an internal issue (ref: OKTA-367047) to track this. We are investigating this issue carefully and will report back here as soon as we have updates regarding a fix or workaround.

aarongranick-okta avatar Feb 02 '21 18:02 aarongranick-okta

Hey @aarongranick-okta @swiftone, quick update - I've managed to get into this same infinite isPending state when toggling the network state

Reproduction

  • Copy above example
  • Load page and turn off internet connection
  • Wait for token expiry

Expected result

While the request is being made, state manager has the following state

{
  isPending: true,
  isAuthenticated: false
}

Because the device is offline, the request will fail and the pending state will change to false

{
  isPending: false,
  isAuthenticated: false
}

Actual result

While the request is being made, state manager has the following state

{
  isPending: true,
  isAuthenticated: false
}

When the request fails, the state manager state does not change

andyrichardson avatar Feb 04 '21 13:02 andyrichardson

We've run into this bug while trying to update our code to use okta-auth-js. It seems that any failure of the refresh request (in our case, a 400 due to the refresh token itself becoming expired) will lead to a bug where the Okta state never properly resolves the isPending state.

Is there a way that we can ourselves detect for failures, and manually trigger an auth redirect?

Is there a way for us to view the status of the internal issue and see when a resolution is available? We're actively blocked on feature release because of this bug, and need to know when we should rollback our changes and approach other solutions.

UlyssesInvictus avatar Feb 08 '21 19:02 UlyssesInvictus

@UlyssesInvictus You should be able to detect errors by subscribing to the error event from tokenManager:

oktaAuth.tokenManager.on('error', e => {
  console.error('error from token manager: ', e);
});

aarongranick-okta avatar Feb 08 '21 19:02 aarongranick-okta

Hi! Thanks for your help.

This unfortunately isn't working, as apparently the network request 400ing is (for some reason) not actually triggering an error in the token manager.

I'm currently implementing a solution where we are attempting to manually remove the refresh token if we've detected that it's expired.

That doesn't solve the problem in the Okta implementation, only avoids the problem case being triggered in the first place, but that should work for our needs.

UlyssesInvictus avatar Feb 08 '21 19:02 UlyssesInvictus

@UlyssesInvictus We are currently developing a fix for this issue. We believe it is isolated to the refresh token and does not affect "normal" token refresh (using iframe). Is it possible for your app to avoid using the refresh token at this time? (do not include "offline_access" in the scopes)

aarongranick-okta avatar Feb 08 '21 23:02 aarongranick-okta

Thanks for the explainer.

We do need to have refresh support at this time, but we've developed our own workaround according to the solution above in my previous comment.

We're looking forward to the fix, though, and will happily remove our workaround once the library is updated!

On Mon, Feb 8, 2021, 6:01 PM Aaron Granick [email protected] wrote:

@UlyssesInvictus https://github.com/UlyssesInvictus We are currently developing a fix for this issue. We believe it is isolated to the refresh token and does not affect "normal" token refresh (using iframe). Is it possible for your app to avoid using the refresh token at this time? (do not include "offline_access" in the scopes)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/okta/okta-auth-js/issues/612#issuecomment-775520806, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCTISV5DKFAQWCZVBIKXRLS6BUM5ANCNFSM4W5GPHWQ .

UlyssesInvictus avatar Feb 08 '21 23:02 UlyssesInvictus

Hi,

I was wondering if there was any update on the fix for this issue. We weren't able to detect errors by subscribing to the error event either, and had to do a workaround for this behaviour.

mathieu-valade avatar Apr 26 '21 08:04 mathieu-valade

We are running into this issue (9 months later). Is it going to be fixed soon?

jluncford avatar Nov 09 '21 21:11 jluncford