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

Fix #904, Required regenerate and save API (req.session.regenerate is not a function since upgrade to 0.6.0) #947

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

joeyguerra
Copy link

Background RE: #904

Using passport@v0.6.0, passport-oauth2@v1.6.1 with the cookie-session@v2.0.0 module to login with Github, the application throws the following error message:

TypeError: req.session.regenerate is not a function at SessionManager.logIn

Feature

Utilize the Delegate Pattern and default to an empty implementation so that the code does not throw an error.

Checklist

  • [ x ] I have read the CONTRIBUTING guidelines.
  • [ x ] I have added test cases which verify the correct operation of this feature or patch.
  • [ x ] I have added documentation pertaining to this feature or patch.
  • [ x ] The automated test suite ($ make test) executes successfully.
  • [ x ] The automated code linting ($ make lint) executes successfully. (NOTE: Lint errors are with code not touched by this PR)

YasharF added a commit to sahat/hackathon-starter that referenced this pull request Jul 13, 2023
patch-package allows seamless patching of npm modules (dependencies) without having to fork them.  You can modify the files in node_modules and use patch-package to capture the changes and reapply them each time the module is installed.
passport.js 0.6.0 currently has a bug that breaks the logout function.  The maintainer hasn't had bandwidth to fix it, but there is a pull request in github that addresses the issue.  The patch here applies the changes from the pull request to 0.6.0.
Passport bug: jaredhanson/passport#904
Passport Fix/Pull Request: jaredhanson/passport#947
Copy link

@raulrene raulrene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaredhanson Any chance you can take a look over this?

lib/sessionmanager.js Outdated Show resolved Hide resolved
@maicongodinho
Copy link

maicongodinho commented Feb 13, 2024

Is there a possibility of this being merged in the near future? @jaredhanson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants