Add secret/key provider function

gpoole commented 7 years ago

Adds a function called secretOrKeyProvider as discussed in #94. The existing secretOrKey option is supported by creating a provider that just returns that secret or key immediately.

Apologies for the random whitespace removals, Atom does it automatically when I save files.

mikenicholson commented 7 years ago

Thanks for the contribution. I'm working on getting this merged. I have a branch in this repo (gpoole-secret-key-provider) where I was touching up the unit tests.

Was there any reason you avoided passing the request to the secretOrKeyProvider? I'm leaning towards adding that in my branch before I merge since users my need information like the requested URL in order to look up the appropriate secretOrKey in multi-tenant applications.

gpoole commented 7 years ago

No there's no real reason, I just didn't have that requirement. I've been working with JWTs that have "kid" headers so it wasn't necessary for me but I can see how it might be for some cases.

gpoole commented 7 years ago

@themikenicholson how's this looking? Are you happy to go ahead with the merge? Let me know if you need any changes to get the tests sorted and I'll be happy to help.

alexframe commented 7 years ago

Is there any update on getting this merged @themikenicholson?

It would be a very handy addition 🎉

mikenicholson commented 7 years ago

@gpoole @alexframe Sorry for the delay. Life has thrown some curve balls that have prevented me from touching side projects.

@gpoole Only holdup is adding the request to the secretorkeyprovider signature. I'm finishing it up today or tomorrow.

NullPtrEx commented 7 years ago

Would really like to see this merged. Can we do the addition of the request parameter as a separate PR at some point if that's the hold up?

oboloka commented 7 years ago

Any updates? It would be useful to have request object in secretOrKeyProvider.

lukaszblacha commented 7 years ago

Upvote, any progress here?

mikenicholson commented 7 years ago

@gpoole I'm ready to merge this after I made a few changes. Check out the version in my branch pr108-secret-key-provider. I made a few changes on top of yours:

the signature of secretOrKeyProvider is (request, rawJwtToken, done)
moved logic out of verify_jwt.js back into the strategy for calling secretOrKeyProvider.
corrected the secretOrKeyProvider function constructed when the secretOrKey arg is provided ( the implementation in your PR was passing the secret or key in the error arg's position).
updated the docs to make it clear that the token passed to the secretOrKeyProvider is a raw, undecoded jwt.

I considered decoding the jwt before passing it to the secretOrKey provider but I didn't do this for 2 reasons:

I don't want to decode the token twice (once there and once in the verification) if the implementer only cared about the request to get the key
I want the implementer to be very aware that this is an unverified jwt they are dealign with at this point. Forcing them to handle the decode (if they need it) should help make this clear.

Can you take a look at that branch and tell me if I missed anything? I'd like to get the new version released tomorrow or Saturday.

Huge apologies for taking so long to get to this. Those curve balls life was throwing didn't stop when I had thought they did.

mikenicholson commented 7 years ago

@gpoole any comment on the latest in master? I'm going to cut a 3.0.0 release soon so I'd like to hear if the changes look ok for your use case before that.

mikenicholson commented 7 years ago

Closing as this is merged into master. Updating docs for the 3.0 release.

carlosscheffer commented 6 years ago

Is this mixed up?

mikenicholson commented 6 years ago

@carlosscheffer What do you mean?

mikenicholson / passport-jwt

Add secret/key provider function #108