AxaFrance / oidc-client

Light, Secure, Pure Javascript OIDC (Open ID Connect) Client. We provide also a REACT wrapper (compatible NextJS, etc.).
MIT License
600 stars 161 forks source link

accessToken expiry not checked until after autoRenew is started when using localStorage #1025

Open joseffsmith opened 1 year ago

joseffsmith commented 1 year ago

Issue and Steps to Reproduce

On page load the function tryKeepExistingSessionAsync checks that tokens exist, if they do then the tokens inside storage are valid. This does not check the expiry so if the token is expired then it's not until after the first timeout (1 second) in autoRenewTokens that the token is refreshed.

you can see the initial events here image

and then one second later image

config, the key line is 'localStorage' I think, because if sessionStorage was used then there would be no token at all and the initial check would fail:

const IDENTITY_CONFIG: OidcConfiguration = {
  authority: rootPath, 
  client_id: clientId, 
  redirect_uri: rootPath + basePath + "/signin-oidc", 
  silent_redirect_uri: rootPath + basePath + "/silent-renew",
  scope: "openid offline_access", 
  storage: localStorage,
};

Versions

Screenshots

Expected

I would expect there to be an initial expiry check when kicking off the autoRenew

Actual

Additional Details

I am doing this to fix the issue

  const [renewed, setRenewed] = useState(false)
  const { renewTokens } = useOidc()
  const { tokens } = JSON.parse(localStorage.getItem('oidc.default'))

  const timeLeft = tokens ? Math.round(tokens.expiresAt - 1 - new Date().getTime() / 1000) : 1 // add a second buffer

  useEffect(() => {
    const handleExpiredToken = async () => {
      if (timeLeft < 0) {
        console.debug("Forcing token renew")
        await renewTokens()
        setRenewed(true)
      }
    }
    handleExpiredToken()
  })
  if (timeLeft < 0 && !renewed) {
    return <Loading />
  }
  return (
    <OidcSecure>
      {children}
    </OidcSecure>
  )
guillaume-chervet commented 1 year ago

hi @joseffsmith , thank you for your issue.

That a good enhancement to aad. whithout servieworker, I'am using https://github.com/AxaFrance/react-oidc/blob/master/packages/react/src/FetchUser.tsx that allow to make fetch always with valid token. to make token valid in tryKeepSession make sence.

joseffsmith commented 1 year ago

Hi @guillaume-chervet, thanks for your response.

So you are using the fetch to make sure the token is valid? That example requests userinfo which doesn't have the token but I suppose you can use any endpoint?

I suppose that is similar to what I am doing here, checking the token is valid and renewing if it is not. It would be nice if the validity check in the library checked expiry instead of token presence.

If you agree with my comment and want to proceed with a similar approach please let me know if I can help in any way, I am happy to contribute

guillaume-chervet commented 1 year ago

Hi @joseffsmith , yes it is very similar. It is because on some edge case. For example if your browser is sleeping and you come back to your website. You may have still old tokens and it requires few time to refresh it. Service worker do it internally as a proxy but whithout. Client js should wait for the synchronisation.