auth0 / express-openid-connect

An Express.js middleware to protect OpenID Connect web applications.
MIT License
460 stars 139 forks source link

absoluteDuration: false not working as expected #624

Open hwride opened 1 month ago

hwride commented 1 month ago

Checklist

Description

When you set rolling: true, rollingDuration: <some-value>, absoluteDuration: false I would expect that there would be no absolute timeout, and only idle/rolling timeout. What actually happens is expiry is set to be immediate, so the user is effectively logged out immediately.

Reproduction

  1. Setup express-openid-connect with the auth middleware with the following config:
    session: {
      rolling: true,
      rollingDuration: 24 * 60 * 60,
      absoluteDuration: false,
    },
  2. Login
  3. Make another request to see if the user is logged in.

Expected behaviour: req.oidc.isAuthenticated() returns true Actual behaviour: req.oidc.isAuthenticated() returns false. The appSession cookie is returned with an expiry of immediately, not an expiry of the rollingDuration.

Additional context

I believe the problem code is in appSession.ts calculateExp().

This code:

return Math.min(
  ...[uat + rollingDuration, iat + absoluteDuration].filter(Boolean)
);

When absoluteDuration is false, the code appears to try and filter out expiry time from the array using .filter(Boolean). But this does not work, for example if you have the following values:

[86400 + 120, 86400 + false]

This will not return a falsy value for 86400 + false, but it returns 86400.

There is a unit test for this here. But if you run the test, you'll find it has iat as 0. So you get 0 + false = 0, which is falsy and gets filtered out. So because the test has a value of 0 for iat it appears to show the filtering logic works, but it doesn't work when iat > 0, which happens in real apps.

express-openid-connect version

2.17.1

Express version

4.17.3

Node.js version

18.16.0

gyaneshgouraw-okta commented 1 month ago

Hey @hwride, thanks for bringing it up. It looks like a valid bug, we will get it prioritised and fixed in upcoming weeks.

hwride commented 1 month ago

Thanks! Will follow :)