IdentityModel / oidc-client-js

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
Apache License 2.0
2.43k stars 840 forks source link

How to detect error on token refresh or pause token refresh #1158

Closed anied closed 3 years ago

anied commented 4 years ago

Per the documentation, we can detect and respond to a token refresh error with userManager.events.addSilentRenewError(handler) and stop the automatic silent renew with userManager.stopSilentRenew(). How do we do the same thing(s) for the refresh of a refresh token?

brockallen commented 4 years ago

The refresh implementation is part of the set of silent API calls. They originated as the iframe silent, but evolved to support refresh tokens, so I kept the same API.

anied commented 4 years ago

Hi @brockallen , thanks for your assistance on this question.

They originated as the iframe silent, but evolved to support refresh tokens, so I kept the same API.

When you say you "kept the same API", does that mean that a handler passed to addSilentRenewError is expected to be called when an error occurs for refresh of a refresh token? I have not seen that to be true thus far, but perhaps I am doing something wrong.

brockallen commented 4 years ago

Looking at the code, if the token POST fails or gets back a 400 then it raises an error. If this is initiated by the automatic silent renew, then that should raise the same error event.

anied commented 4 years ago

So, I am failing to see the same error event raised, but perhaps that could be due to a misconfiguration on my part. Let me share my specific case.

We are using the react wrapped version of the libary: react-oidc. Our IdP does not support the session management protocol, so we have no ability to renew our access token via iframe. However, we have set automaticSilentRenew, and see in the Chrome dev tools network pane that the library is making calls to our /token endpoint in the background to fetch a fresh access token when the current one is timing out.

I have pulled the underlying user manager from the library and am able to set a callback for any time a token fetch fails:

const userManager = getUserManager();
userManager.events.addSilentRenewError(() => {
  Logger.warn('Request to refresh users access token failed; surfacing warning message.');
  dispatchToastNotification(AuthenticationErrorToast);
});

If I set the Network panel to emulate "offline" right before the library attempts to fetch a fresh token, the call will fail and this callback will be executed. This occurs 60 seconds before the token is due to expire. If I wait another 60 seconds, the library will make a second call to /token when the token actually expires. I assume that this is a call requesting a new refresh token (although I'm not certain of that). However, if I force that call to fail (by again emulating "offline" mode w/ the Network panel) the callback will not be executed a second time.

So, I'm not certain if this is because:

  1. This second call is a different action that requires a different event handler to be configured
  2. We have somehow misconfigured something in code or in the library configuration that is causing this behavior
  3. Something else I'm completely unaware of

Thanks for you assistance-- this library and the React wrapped version have greatly reduced friction in getting OIDC setup. Please let me know if there is anything I can provide to help this issue.

anied commented 4 years ago

Actually, I may have misunderstood and misfiled this issue. I've been digging into it a bit more today, and discovered that the second call to /token is not prompted from oidc-client-js, but from react-oidc here:

export const onAccessTokenExpiredEvent = (oidcLog: Logger, unloadUserInternal: Function, userManager: UserManager) => async () => {
  oidcLog.info('AccessToken Expired');
  unloadUserInternal();
  await userManager.signinSilent();
};

So that second call to /token is actually a signinSilent-initiated request. Unfortunately, should this call fail, I still have no hooks that I see exposed by either library in which to handle the failure.

anied commented 4 years ago

OK, so I've created a POC that resolves my issue-- I added a signinSilentInternal that is used internal to the library, and added a silentSigninError that is raised only when an unhandled error is encountered when called from the public-facing signinSilent.

My question now is how to proceed. I would like to get this reviewed, and, if amenable to the library maintainers, I can update tests and documentation accordingly and have the change merged. However, the issue being solved is somewhat different from what was initially described in this Github issue. Would it be best to open a new issue and attach the PR to that? Basically, what is the best way to get this code to the library maintainers for review and possible merge?

You can see a comparison of my changes against dev here. For what it's worth, there are no failing tests, although I imagine new tests would need to be written for the new behavior.

brockallen commented 3 years ago

I also forgot there are stopSilentRenew and startSilentRenew APIs. Anyway, are you all sorted on this issue or is there something else?