okta / okta-oidc-js

okta-oidc-js
https://github.com/okta/okta-oidc-js
Other
395 stars 232 forks source link

accessToken refresh error using default setup #892

Open CristianoYL opened 4 years ago

CristianoYL commented 4 years ago

I'm submitting this issue for the package(s):

I'm submitting a:

Current behavior

I'm following the react okta tutorial to integrate Okta with React. What I'm trying to achieve is very basic:

  1. render the currently logged-in user in the app bar.
  2. fetch a list of data for that user when navigated to a certain page.

Both use cases are covered by the tutorial (retrieving user information + providing access token).

And everything seems to be working at a glance. Then I noticed some funky behavior when I left the browser open for a while. At first, the app bar stopped showing the user info. Then sometimes the app bar is showing the user name, but the user data cannot be fetched.

I went through quite a few previous issue discussions, the most related/helpful of which are: https://github.com/okta/okta-oidc-js/issues/460 https://github.com/okta/okta-oidc-js/issues/721 https://github.com/okta/okta-oidc-js/issues/804

I tried to dig into the problem and noticed that the error seems to be related to token expiration. So I used the Okta admin console to configure the access token to expire every 5 minutes and I was able to reproduce the error: image

Several issues threads have deemed concurrency/race condition to be the cause, but it seems to be still around with the current version. I don't have a conclusive theory on what exactly causes the error, but looks like something went wrong when the library tries to silently renew one of the tokens. There are occasions where the token refreshing works, but then something else would break it. Sometimes a logout-and-login, sometimes just a simple browser refresh:

image

It's a bit frustrating when even the error is not consistently reproducible and I'm a bit out of clues. Something that might be relevant:

But they don't seem to be relevant because I still experience the issue after clearing the application storage. Another possible relevant issue is that I noticed data gets stored in LocalStorage rather than SessionStorage (as I noticed here https://github.com/okta/okta-oidc-js/issues/804#issuecomment-664578561).

More specifically, LocalStorage has 3 keys:

SessionStorage only has okta-pkce-storage. Where in both storage, okta-pkce-storage is an empty object.

I end up solving the issue by overriding the AuthService.isAuthenticated() mehotd and requires both idToken and accessToken to then return true (as shown here https://github.com/okta/okta-oidc-js/tree/master/packages/okta-react#security). But I feel it's more of a workaround than a fix since the auto-renewal for tokens should work independently.

Expected behavior

I understand the rationale here https://github.com/okta/okta-oidc-js/issues/721#issuecomment-611212502 and would expect the accessToken/idToken to be able to silently refresh independently without causing any issue.

Minimal reproduction of the problem with instructions

  1. set the access token lifetime to 5 minutes.
  2. open the app and be redirect to Okta login page.
  3. login and redirect back to app view and wait for 5 minutes.

I've attached my most relevant pieces of code down here but feel free to ask for more details if needed:

// App.js

const App = () => {
  const config = {
    clientId: process.env.REACT_APP_OKTA_CLIENT_ID,
    issuer: process.env.REACT_APP_OKTA_ISSUER,
    redirectUri: process.env.REACT_APP_OKTA_REDIRECT_URL,
    scopes: process.env.REACT_APP_OKTA_SCOPES.split(','),
    pkce: true
  };

  return (
    <BrowserRouter>
      <Security {...config}>
        <AppBar />
        <Switch>
          <Route path="/implicit/callback" component={LoginCallback} />
          <SecureRoute path="/foo" exact component={FooListing} />
          <SecureRoute path="/" exact component={Home} />
          <Redirect to="/" />
        </Switch>
      </Security>
    </BrowserRouter>
  )
};
// AppBar.js

    const [userInfo, setUserInfo] = useState(null);
    const { authState, authService } = useOktaAuth();

    useEffect(() => {
        if (!authState.isAuthenticated) {
            // When user isn't authenticated, forget any user info
            setUserInfo(null);
        } else {
            authService.getUser().then((info) => {
                setUserInfo(info);
            });
        }
    }, [authState, authService]); // Update if authState changes

    return (
        ...
       {userInfo ? userInfo.name : null}
    );
// FooListing.js

    useEffect(() => {
        if (!authState.isAuthenticated) {
            setAccessToken(null);
        } else {
            setAccessToken(authState.accessToken);
        }
    }, [authState, authService]);

    useEffect(() => {
        if (accessToken) {
            console.log('token', accessToken);
            const onStart = () => {...};
            const onSuccess = (res) => {...};
            const onError = (error) => {...};
            API.getData(accessToken, onStart, onSuccess, onError);
        } else {
            console.log('no token', accessToken);
        }
    }, [accessToken, setUserMetadataState]);

Environment

P.S.: It's a bit verbose, I appreciate your help.

shuowu commented 4 years ago

@CristianoYL The SDK by default thinks the app isAuthenticated as long as one of the tokens (accessToken or idToken) is valid. In your app, it seems like your accessToken has expired and failed to be renewed due to the session also expired, but the idToken is still valid in the storage.

You can use the sample code for the isAuthenticated callback to evaluate the authState when both tokens are available, to make sure you always has valid accessToken in app. https://github.com/okta/okta-oidc-js/tree/master/packages/okta-react#configuration-options

CristianoYL commented 4 years ago

Hi @shuowu, thanks for your reply. I understand the logic behind determining what isAuthenticated means.

My main concern is that following the tutorial, the new user would encounter the same issue as I did and it is not ideal.

There seems to be an underlying issue that fails to refresh the access token/id token using this default setup, regardless of session timeout. As I can reproduce the error within minutes of logging the user in, and the session should still be fresh (please correct me if I'm wrong). And I can circumvent the issue if I simply override the isAuthenticated callback, presumably within the same session.

shuowu commented 4 years ago

@CristianoYL That's definitely good suggestions! They are actually on our roadmap to change the default isAuthenticated behavior and update the tutorial accordingly. It just takes time to get there.

chrismllr commented 4 years ago

What causes a session to "expire" has eluded me, I cannot quite nail that down, even after reading documentation pertaining to Session Cookies and Session tokens.

From what it looks like, a session should not expire unless user quits their browser? Although, I've tried leaving my browser open overnight only to realize the renewal process fails with the above error.

We have all of our Prompt for Reauthentication settings set at 7 days, but still see this pretty intermittently.

Is there somewhere to find documentation for this? It would be great to know all of the individual behaviors which cause session expiry.

CristianoYL commented 4 years ago

@chrismllr That's what I've been struggling too. As mentioned in my temporary solution, customizing the isAuthenticated can kind of avoid the error. But it really isn't solving the problem. The tokens are not auto-refreshing as it's supposed to, the session is somewhat a mystery.

askouras commented 4 years ago

@chrismllr In order to log into your OIDC application, a user needs to log into Okta, which will result in a session cookie being set in their browser. When you have an active Okta session, you can request tokens for an OIDC application, which includes the ability to renew tokens, throughout the life of the Okta session.

The scenario described implies that both the user's tokens and their Okta session have expired. By default, a user's (idle) session lifetime is 2 hours and if they do not interact with Okta again during this time period, their session will not be extended and they will be logged out of Okta. When the application attempts to fetch new tokens, it throws a login_required error because there is no longer an Okta session cookie in the browser. To continue using the application, the user will need to re-authenticate with Okta.

chrismllr commented 4 years ago

@askouras That link is helpful, I had not seen this rule prior. We will see if this is something we can configure, and will report back.

chrismllr commented 4 years ago

@askouras Great, so this looks to be a valid configuration to get the desired results.

Unfortunately, our Okta instance supports many applications, and there is a global policy set to two hours, which we are unable to scope to only our applications.

Does that sound correct? Should we be able to assign a Sign On policy to a specific authorization server or application?

Alternatively, is there a way to increase access token TTL, and detach the JS SDK from the session token? It seems that even a higher TTL on the token, the session expiry would still be reached and take precedent.