Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(deps): bump passport from 0.4.0 to 0.6.0 #31

Merged
merged 2 commits into from Nov 22, 2022

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Nov 14, 2022

Bumps passport from 0.4.0 to 0.6.0.

Changelog

Sourced from passport's changelog.

[0.6.0] - 2022-05-20

Added

  • authenticate(), req#login, and req#logout accept a keepSessionInfo: true option to keep session information after regenerating the session.

Changed

  • req#login() and req#logout() regenerate the the session and clear session information by default.
  • req#logout() is now an asynchronous function and requires a callback function as the last argument.

Security

  • Improved robustness against session fixation attacks in cases where there is physical access to the same system or the application is susceptible to cross-site scripting (XSS).

[0.5.3] - 2022-05-16

Fixed

  • initialize() middleware extends request with login(), logIn(), logout(), logOut(), isAuthenticated(), and isUnauthenticated() functions again, reverting change from 0.5.1.

[0.5.2] - 2021-12-16

Fixed

  • Introduced a compatibility layer for strategies that depend directly on passport@0.4.x or earlier (such as passport-azure-ad), which were broken by the removal of private variables in passport@0.5.1.

[0.5.1] - 2021-12-15

Added

  • Informative error message in session strategy if session support is not available.

Changed

  • authenticate() middleware, rather than initialize() middleware, extends request with login(), logIn(), logout(), logOut(), isAuthenticated(), and isUnauthenticated() functions.

[0.5.0] - 2021-09-23

Changed

  • initialize() middleware extends request with login(), logIn(), logout(), logOut(), isAuthenticated(), and isUnauthenticated() functions.

... (truncated)

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
  • @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
  • @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
  • @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
  • @dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the Security Alerts page.

@dependabot dependabot bot added the dependencies Pull requests that update a dependency file label Nov 14, 2022
@DevWithTheHair
Copy link
Member

Getting a weird error in the logs with this one:

State parameter not found in store

@DevWithTheHair
Copy link
Member

Related, gets weirdly stuck in-browser with https://localhost:8080/login.html but does log out the tokens in the console.

Directly navigating to https://localhost:8080/me seems to work.

Not sure why the redirect isn't working here or what is wrong with the state.

Bumps [passport](https://github.com/jaredhanson/passport) from 0.4.0 to 0.6.0.
- [Release notes](https://github.com/jaredhanson/passport/releases)
- [Changelog](https://github.com/jaredhanson/passport/blob/master/CHANGELOG.md)
- [Commits](jaredhanson/passport@v0.4.0...v0.6.0)

---
updated-dependencies:
- dependency-name: passport
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/passport-0.6.0 branch from cecde1c to 35f60c7 Compare November 14, 2022 17:14
@DevWithTheHair
Copy link
Member

Note to self:

Upgrading from 0.4.0 all the way up to 0.5.3 works as expected.

Looks like it is specifically 0.6.0 where things break.

@DevWithTheHair
Copy link
Member

I'm closer to understanding what is happening based on using the debugging option from express-session:
image

Using that debugging option in the start script in package.json:

"scripts": {
    "start": "ENVIRONMENT='local' DEBUG=express-session node server.js"
  },

...I see these differences in how the session is handled between passport version 0.4.0 and 0.6.0.

0.4.0

[jaimelopez@C02YX7W4LVCG ~/Documents/Source/consumer-api-openid-connect-example (main !)] $ npm run start

> @banno/consumer-api-openid-connect-example@1.0.0 start /Users/jaimelopez/Documents/Source/consumer-api-openid-connect-example
> ENVIRONMENT='local' DEBUG=express-session node server.js

Environment: local
API_ENVIRONMENT: https://digital.garden-fi.com
Server listening on https://localhost:8080
  express-session fetching QKJFunyp6gZzwyHuRP9PXy_js5VSvgX7 +0ms
  express-session no session found +1ms
  express-session set-cookie connect.sid=s%3A85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk.mQPzqDzO6kJe%2BkYGt1B8x7qA%2FEDwRKluG9Pwefg3Cek; Path=/; HttpOnly; Secure; SameSite=None +7ms
  express-session saving 85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk +1ms
  express-session fetching 85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk +21ms
  express-session session found +0ms
  express-session touching +3ms
  express-session touched +0ms
  express-session fetching 85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk +953ms
  express-session session found +0ms
  express-session saving 85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk +2ms
  express-session fetching 85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk +140ms
  express-session session found +0ms
TokenSet {
  access_token: (token)',
  expires_at: 1669144785,
  id_token: '(token),
  refresh_token: '(token),
  scope: 'openid https://api.banno.com/consumer/auth/offline_access https://api.banno.com/consumer/auth/accounts.readonly https://api.banno.com/consumer/auth/transactions.detail.readonly',
  token_type: 'Bearer'
}
null {
  sub: '277020a3-fe0d-4e04-a6db-39d06ad13bd7',
  given_name: 'Jaime',
  family_name: 'Lopez',
  name: 'Jaime Lopez',
  address: {
    locality: 'Banno',
    postal_code: '657080000',
    region: 'MO',
    street_address: '123 Banno St.'
  },
  email: 'jaime.lopez@banno.com',
  'https://api.banno.com/consumer/claim/institution_id': '899f4398-106d-409a-9ed4-a72346778076',
  at_hash: 'hTcrMbeCsAq6tChNFLu4Tw',
  aud: '3f85ce95-00e7-4f3e-abbe-132d95f96d4e',
  exp: 1669147785,
  iat: 1669144185,
  iss: 'https://digital.garden-fi.com/a/consumer/api/v0/oidc'
} {}
  express-session saving 85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk +491ms
  express-session split response +0ms
  express-session fetching 85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk +4ms
  express-session session found +0ms
  express-session touching +1ms
  express-session split response +0ms
  express-session touched +0ms

The session is consistently 85sDdaJ3jqrbmWuLMYjvkAWqwglmj8dk.

0.6.0

[jaimelopez@C02YX7W4LVCG ~/Documents/Source/consumer-api-openid-connect-example (main !)] $ npm run start

> @banno/consumer-api-openid-connect-example@1.0.0 start /Users/jaimelopez/Documents/Source/consumer-api-openid-connect-example
> ENVIRONMENT='local' DEBUG=express-session node server.js

Environment: local
API_ENVIRONMENT: https://digital.garden-fi.com
Server listening on https://localhost:8080
  express-session fetching A7ELmwf-4BWhCrK6hmap6SVBw35q281H +0ms
  express-session no session found +1ms
  express-session set-cookie connect.sid=s%3Ab6aiykVUWqOPwwhNAd2cmAqpLSLz7yDZ.4lvjHai%2FkqTtGBjZ6HX%2FATsPefzvu32UfJ7seTvA%2Fa0; Path=/; HttpOnly; Secure; SameSite=None +5ms
  express-session saving b6aiykVUWqOPwwhNAd2cmAqpLSLz7yDZ +2ms
  express-session fetching b6aiykVUWqOPwwhNAd2cmAqpLSLz7yDZ +33ms
  express-session session found +1ms
  express-session touching +2ms
  express-session touched +0ms
  express-session fetching b6aiykVUWqOPwwhNAd2cmAqpLSLz7yDZ +2s
  express-session session found +0ms
  express-session saving b6aiykVUWqOPwwhNAd2cmAqpLSLz7yDZ +2ms
  express-session fetching b6aiykVUWqOPwwhNAd2cmAqpLSLz7yDZ +272ms
  express-session session found +0ms
TokenSet {
  access_token: '(token)',
  expires_at: 1669144633,
  id_token: '(token),
  refresh_token: '(token)',
  scope: 'openid https://api.banno.com/consumer/auth/offline_access https://api.banno.com/consumer/auth/accounts.readonly https://api.banno.com/consumer/auth/transactions.detail.readonly',
  token_type: 'Bearer'
}
null {
  sub: '277020a3-fe0d-4e04-a6db-39d06ad13bd7',
  given_name: 'Jaime',
  family_name: 'Lopez',
  name: 'Jaime Lopez',
  address: {
    locality: 'Banno',
    postal_code: '657080000',
    region: 'MO',
    street_address: '123 Banno St.'
  },
  email: 'jaime.lopez@banno.com',
  'https://api.banno.com/consumer/claim/institution_id': '899f4398-106d-409a-9ed4-a72346778076',
  at_hash: 'h6bd9RaV-1qM1nLCI_q-Mg',
  aud: '3f85ce95-00e7-4f3e-abbe-132d95f96d4e',
  exp: 1669147634,
  iat: 1669144034,
  iss: 'https://digital.garden-fi.com/a/consumer/api/v0/oidc'
} {}
State parameter not found in store
  express-session split response +454ms
  express-session set-cookie connect.sid=s%3AIhYaD2ckkKhu-E86RvC_gpa4MnxShNc6.zZ559VzoXZCDvS7TV4QN16%2FrFc59PRZvuhz0G6%2FlvF4; Path=/; HttpOnly; Secure; SameSite=None +0ms
  express-session fetching IhYaD2ckkKhu-E86RvC_gpa4MnxShNc6 +4ms
  express-session session found +1ms
  express-session touching +1ms
  express-session touched +0ms
  express-session fetching IhYaD2ckkKhu-E86RvC_gpa4MnxShNc6 +15ms
  express-session session found +0ms
  express-session touching +1ms
  express-session touched +0ms

In this case, the session switches from b6aiykVUWqOPwwhNAd2cmAqpLSLz7yDZ to IhYaD2ckkKhu-E86RvC_gpa4MnxShNc6, so it's not surprising that we have a mismatch of our 'store' (i.e. the session) and get the error message:

State parameter not found in store

...which will end up redirecting the user back to the login.html page, thus exhibiting the undesired behavior because of these lines in our server.js:

// If a state parameter is present and has a matching local state, lookup the value
if (req.query.state) {
  if (req.session.oAuthState && req.session.oAuthState[req.query.state]) {
    if (req.session.oAuthState[req.query.state].returnPath) {
      nextPath = req.session.oAuthState[req.query.state].returnPath;
    }

    delete req.session.oAuthState[req.query.state];
  } else {
    console.error('State parameter not found in store');
    return res.redirect('/login.html');
  }
}

@DevWithTheHair
Copy link
Member

Based on the passport changelog for 0.5.3 to 0.6.0, it looks like this is pretty much the only PR change:

The back-and-forth comments with angry users of passport ends up with this seemingly useful blog post on Medium entitled "Fixing Session Fixation".

The passport.js changes in `0.6.0` have breaking changes related to protecting against "Session Fixation".
- jaredhanson/passport#900
- https://medium.com/passportjs/fixing-session-fixation-b2b68619c51d

The assumption for the fix in this commit is that our example project here only has the session storage as its storage mechanism, so we're not quite vulnerable to the same thing since the storage goes away when the local project is stopped.
@DevWithTheHair
Copy link
Member

I've updated the dependabot branch with another commit that works in tandem with upgrading passport.js to 0.6.0: 984eddb

@DevWithTheHair
Copy link
Member

Seems like things are working again, so self-merging this one. 😬

@DevWithTheHair DevWithTheHair merged commit 7a2e5bd into main Nov 22, 2022
@DevWithTheHair DevWithTheHair deleted the dependabot/npm_and_yarn/passport-0.6.0 branch November 22, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant