0.3.0 is not working #400
Comments
|
Can you provide any more information? LocalStrategy is working fine with 0.3.0 for me. Sent from my iPhone
|
|
I get this issue as well However, authentication was successful, the user was returned.. |
|
|
|
Same error, however I only get it when in Chrome Private Browsing mode. |
|
Getting this error as well when using https://github.com/sahat/hackathon-starter with passport 0.3.0 and passport-local. |
|
@jaredhanson ,
so, I reverted to previous release and change my And its working correctly, Hope that above information is enough to repro the issue. |
|
Has this been addressed yet, just beat my head off the wall wondering why this wasn't working, sure works now that I've reverted to 0.2.2 |
|
Yeah, I'm seeing this too using passport-local. |
|
Are you guys using any other strategies besides passport-local? Any strategies that depend on old versions of passport core will cause this problem. They should be updated to depend on passport-strategy, rather than passport itself. |
✅ SolutionUpgrading passport-github and passport-instagram to 1.0.0 has fixed the issue for me. |
|
@jaredhanson we're only using your passport local package. Does this need to be updated? |
|
@julianlam Can't hurt. What version are you using? |
|
Thanks for your attention on the matter! https://github.com/NodeBB/NodeBB/blob/master/package.json 0.3.x of passport, and 1.0.0 of passport local |
|
Is the problem reproducible with a clean install? If not, I suspect the npm cache for updates might be giving old versions Sent from my iPhone
|
|
The problem only occurs to users on new sessions. For example, my main browser has an existing session from passport 0.2.x, so req.session.passport exists, but on 0.3.x, req.session.passport doesn't exist, and so we get an error when the user object is saved or serialised into that object (which is now undefined). Not sure if that's completely off the mark. @barisusakli, can you confirm we have passport local v1.x on production? |
|
Sorry, to clarify, my old session has req.session.passport whereas brand new sessions do not. I only have it because it is not erased on logout. |
|
Yeah passport-local@1.0.0 |
|
The main change in passport@0.3.0 is that This rationale for this is discussed in #320. If you have any dependencies (or dependencies of dependencies) that pull in older versions of Passport, the HttpRequest#login method will get overwritten, causing this conflict. @julianlam Can you verify that the new code (linked to above) is what is getting executed on login? A full stack trace of any error will help diagnose the issue. |
|
Thanks for the write up! Unfortunately I'm only seeing this on our production build and not my dev environment (isn't that how it always is?). So I'm going to have to do-it-live:tm: and try to repro there. So fingers crossed it is an npm cache issue we both won't have to do anything. |
|
@jaredhanson Identified the issue, submitting a PR against passport-totp shortly. (Assuming I can figure out how to use passport-strategy.....) |
|
This PR should fix the problem. As you suggested, it was an issue with a dependency of a dependency using passport itself (in this case, passport-totp): |
|
I've published It should now be possible to use any strategy with |
* Also trying out *chalk* dep to see if the VPS pro logs can handle this without too much visual interference... nicer in development and debug modes... if not will remove. Applies to Code Migrations, OpenUserJS#430, and jaredhanson/passport#400 (comment) Still getting `username is taken` with *passport*@0.3.2 and *passport-github*@1.0.0 but **not** *passport*@0.2.2 with *passport-github*@0.1.5
|
@jaredhanson
I am not entirely sure if our usage is considered a "strategy" or not since we appear to be doing some unusual things to passport starting here... as this is my least familiar portion of our code (not to mention authentication in general with passport). I am currently at a loss on what to do. |
|
@jaredhanson Cc: @sizzlemctwizzle I sure would like to know what changed so I can learn what was happening. Only thing I can vaguely conclude is GH literally just changed something in the last half hour with their OAuth session generation... I currently have no reference to their repository with this and https://github.com/github shows no activity in the last 30 minutes. :\ |
|
@jaredhanson Cc: @sizzlemctwizzle Another followup note here... when I did this most recent testing I upped just the passport@0.3.2, logged into our dev environment... that worked okay as expected... then I updated the passport-github@1.0.0 ... it failed a few times then magically, out of nowhere, it worked. So going backwards in testing... I did a complete reset of our repo (locally) and removed all deps and reinstalled them from scratch, e.g. passport@0.2.2 with passport-github@0.1.5 ... now with those dep versions it won't let me log in anymore... this could indicate that passport/passport-github and/or GH itself migrated my account silently in the background. In summary this could prove to be a very difficult migration with 0.3.2 since not all of our users may log in for months... so they'll be locked out after our production goes to 0.3.2 and 1.0.0 respectively. I'm now locked out of our dev environment for the time being... I can recreate my account there manually but we have a few thousand users and I'd rather not let them have this user experience in our production. Any ideas? |
|
@Martii: There's nothing in This issue is discussing a particular issue where sessions were lazily initialized in I honestly don't follow what you are describing, but from the sound of it seems like some issue with higher-level application logic. If you can isolate an issue with passport specifically, please file an issue. |
|
@jaredhanson I'll create an issue if I don't find anything but most likely it will be titled similarly. I just revoked my authorized applications too at https://github.com/settings/applications and no such luck there. This bug doesn't really follow any discernible pattern yet other than matching a handful of comments in this issue here. Thanks. |
|
0.3.0 may not work. 0.3.2 introduces the fix. If you upgrade to passport@0.3.2, you shouldn't see any issues with any strategies, GitHub included. |
|
Unfortunately I am experiencing it with 0.3.2 as denoted up here. I even recorded my sh-session logs and forwarded them off to @sizzlemctwizzle as to the magic working of 0.3.2 ... but now downgrading doesn't work now... this would indicate there's going to be a huge problem with upgrading passport and passport-github since our users don't log in daily... could be months or years down the line. I'll try clearing our stored sessions as alluded to in #400 (comment) ... perhaps the session id is messing with this. |
This is what I don't follow. Passport itself hasn't changed how sessions are stored from version to version. The only change is that an authentication session is not initialized until necessary, rather than ahead of time. There's no issue with upgrading passport or its strategies, and session behavior, as far as a user is concerned, is not impacted. It doesn't matter what version of a strategy you use yesterday, and what version you use a year from now. Authentication itself will proceed the same way. |
|
@Martii I notice in the code above, you are using custom callbacks with Passport. Does that callback get invoked. If it gets invoked, is the failure afterwards? |
|
Re: #400 (comment) For some reason, beyond my current understanding at this time, we store the session ids here in our model MongoDB. This is what I have to manually clear out and see if those are interfering with upgrading and downgrading. Here's a gist of what I just did with downgrading in our dev environment... you can see:
|
|
I have no idea why a user model would have a session ID on it, but that's not something Passport does. This error you encounter is clearly in application-specific code, here: https://github.com/OpenUserJs/OpenUserJS.org/blob/aa1529a6d3b3e098257b0eb98b316fae75efbe45/libs/passportVerify.js#L77 At this point, I don't see any evidence that Passport itself has regressed, at least so far in its publicly exposed API. If you have overridden internals at a higher layer, that I can't control for. If you can isolate those details, please post here. |
Both the I appreciate your patience with me, especially since I'm not fully versed with passport yet.
That is because the verify callback is returning not logged in in the parameter... otherwise we wouldn't be capturing the verify error of the strategy. |
|
From the looks of it, you have overridden Passport internals here: I don't know why that was done or why it was necessary. It appears to me that is the cause of your issue. Passport 0.3.x may have exacerbated your issue, but since internals shouldn't have been hijacked in the first place, there's not much guidance I can give you at this point. I'm closing this issue now, because I consider it fixed. If you want to discuss the behavior you are experiencing (which I'm confident is caused by problems other than those in this issue), please open a new issue. |
|
No problem... I am guessing that passport-github, or other strategy, currently has weak encryption support which is why the prior maintainers overloaded the "private" method. I'll do some more tinkering and see if I can work around this commented deficiency in the strategy and see if I can validate it under todays standards with passport and affected strategies. Thanks again. |
|
Here is the breaking change for the issue we are having... GH usually sends a phew Thank you again for your repsonses... at least I can fix our DB with a migration (or un-type-cast it) and we can work past this "correction" from that package. Hope this helps someone else out who may have this issue but hasn't spoken up yet. :) |
Hey people,
Just for your information, your latest version
(0.3.3)is not working for me.I am using its simple
LocalStrategyand getting user is undefined.so I reverted to previous release
(0.2.2)which is working for me.The text was updated successfully, but these errors were encountered: