search

Rigidflame / firebase-passport-login

Use Express Passport libraries for Firebase Authentication
43 stars 10 forks source link

Any tentavtive dates for production ready version #1

Closed thandaanda closed 9 years ago

thandaanda commented 10 years ago

Hi @abeisgreat,

Is there any tentative dates for production ready version of this library.

Thanks.

abeisgoat commented 10 years ago

"Production Ready" is always a loosely defined term.

I've been using it for awhile and haven't had any outside reports of issues in weeks.

The only step I'd like to take before taking it out of beta is to have the security rules reviewed by an expert and if you're looking to launch a product, I'll be happy to push that out sooner rather than later.

thandaanda commented 9 years ago

I just came around one issue, in server.js we are generating JWT token of user data and storing in firebase. On the client side, we are listening when there is a value on that path and fetch it and set it to cookie. But cookie has limit of 4092 bytes and sometime token size is greater than 4092 and there it breaks. passportSession cookie is set to null. Any workarounds?

lefnire commented 9 years ago

The only fork I see is https://github.com/jessta/firebase-passport-login, which has quite a few commits. @jessta, can you speak to that fork? Are there bugfixes there which may push upstream closer to a prod release, in which case are you holding off on a PR until things are further along?

lefnire commented 9 years ago

also @jessta if you don't mind my asking, what are your thoughts on all that es6-promises added code? Could the same be achieved simpler with async.waterfall, or is it some firebase mechanisms that require that extra stuff?

abeisgoat commented 9 years ago

I'd love to speak with @jessta about his improvements, so we could potentially merge some back. In terms of the bug @thandaanda mentioned, I'm not sure. I'll look into it. The obvious solution is to use sessionStorage over cookies, which should have larger size restraints. The way Firebase proper seems to fix this is by not storing 3rd party data in the token itself, but using a limited subset, but still supplying a complete user object to the callback. This is reasonable as well.

abeisgoat commented 9 years ago

So I looked at @jessta's code and I don't think we'll be merging any in because I don't believe most of it conforms to the same programming ideologies I have. I have, however, started fixes for 1.0 which include fixes for the 4092 byte cookie mentioned above and will also add support for refresh tokens at the same time. I'll be closing this for now.