search

jaredhanson / passport-strategy

An abstract class implementing Passport's strategy API.
MIT License
114 stars 30 forks source link

"Strategy API" is undocumented. #7

Open mrluc opened 8 years ago

mrluc commented 8 years ago

Neither http://passportjs.org/ or this passport-strategy repo appear to describe the Strategy API. The first place I found it described was in the chai testing strategy README, where it says:

The callbacks correspond to Passport's strategy API: success(), fail(), redirect(), pass(), and error().

Huh! That makes sense, I've seen people use those methods in their custom strategies, and they all inherit from the abstract class provided by the passport-strategy module. But then I come here and I find only one method: authenticate.

So, per the chai-lib there's an API, but it seems to be undocumented.

Where can one find more detail on what's being patched into the strategy at runtime, what methods authors can rely on being available? I looked in the passportjs site, the wiki, issues, and this repo, and I have decent google-fu, but I honestly couldn't find anything other than the actual implementation:

https://github.com/jaredhanson/passport/blob/5334ee0fa52ab5f5225ddee695697432e0565679/lib/middleware/authenticate.js#L178

I think the API for strategy authors should be documented somewhere, as per comments in above file any authenticate() method can't be successful without using one of those 'augmented actions'. If it's not documented in this repo it should probably be linked to from here.

Apologies if it's documented and I missed it! I think having this called out could benefit Strategy authors -- as one example, in a multi-strategy situation, a strategy that can never call .pass() could be useless, but many authors are basing their strategies on 'passport-local' where .pass() isn't used, and may be unaware of the api.

srhoulam commented 8 years ago

I took a crack at documenting these augmented methods in #5. I did not know about the redirect and pass methods, though, so those are not covered. Thanks for making them known to me.

peteruithoven commented 7 years ago

Would be great if someone could also create a link in the Passport website and readme.

Ostefanini commented 6 years ago

agreed, I heard about it because of this stackoverflow post. Nothing on the official website...

max-applauz commented 1 year ago

These augmentations can be found here: https://github.com/jaredhanson/passport/blob/cfdbd4a762b51e339ebfea931d65bccbbde53282/lib/middleware/authenticate.js#L199