Skip to content

Remove need to call passport.initialize() #875

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

Merged
merged 9 commits into from
Dec 15, 2021
Merged

Remove need to call passport.initialize() #875

merged 9 commits into from
Dec 15, 2021

Conversation

jaredhanson
Copy link
Owner

This PR continues the work started in #848, completing the refactor away from using req._passport.

Following the patterns of other middleware, the HTTP request is now extended when it passes through authenticate() middleware, rather than initialize(). This removes the need to use passport.initialize() entirely, which simplifies the app-level middleware stack. It should also make it easier to adapt Passport to non-Express web frameworks.

@jaredhanson jaredhanson added the enhancement New feature or request label Dec 15, 2021
@jaredhanson jaredhanson merged commit 932c1b8 into master Dec 15, 2021
@RealDrewKlayman
Copy link

This shouldn't really be marked as a patch.
It has breaking changes and will break anybody that has it listed in the package.json with ^0.5.0 and show this error:
passport.initialize() middleware not in use

@jaredhanson
Copy link
Owner Author

Can you provide details? You shouldn't be getting that error, so there must be some other unexpected condition occurring. I'm confident that there should be no breaking changes here.

@jaredhanson
Copy link
Owner Author

To add to my rationale: The error string you mention, "passport.initialize() middleware not in use", is only present in the passport package itself, prior to 0.5.1. If you upgrade to the latest release, the error string has been removed (since passport.initialize() is no longer required). If you are still seeing this error message, it suggests that you haven't updated to 0.5.1. Perhaps a package-lock.json is pinning you to a prior release?

@jaredhanson
Copy link
Owner Author

@RealDrewKlayman Can you provide a list of the strategies you use in your application? As discussed in #877, passport-azure-ad pulls in an older version of passport, which can trigger this issue. I'm working on a fix, but I'd like to assemble a list of strategies that require a compatibility layer. Thanks!

@sonofole
Copy link

I am working on the same project as @RealDrewKlayman. Our package.json had passport: "^0.5.0" (which resulted in passport@1.5.1 being used according to npm list passport on the server), as well as passport-azure-ad@4.3.1, which has a dependency of passport@0.4.1.

It's possible that some interaction between these caused the problem, but the problem was resolved by changing our package.json to passport: "0.5.0", which prevented passport@0.5.1 from being installed on the server.

The project also includes passport-headerapikey@1.2.2 and passport-local@1.0.0.

@sonofole
Copy link

@jaredhanson I just read through #877 and that does appear to be the same problem we're experiencing

@jaredhanson
Copy link
Owner Author

Thanks for confirming @sonofole . I've published passport@0.5.2 which should resolve the issue. Could you upgrade and confirm that works?

@sonofole
Copy link

@jaredhanson Yes it appears to be working with 0.5.2, thanks for the quick fix!

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

Successfully merging this pull request may close these issues.

None yet

3 participants