Skip to content
This repository has been archived by the owner on Aug 28, 2023. It is now read-only.

Readme example not working with master #141

Closed
yarandoo opened this issue Jul 7, 2016 · 5 comments
Closed

Readme example not working with master #141

yarandoo opened this issue Jul 7, 2016 · 5 comments

Comments

@yarandoo
Copy link

yarandoo commented Jul 7, 2016

Hi,

I am using master branch (As it has fixes #117 #112), and the readme sample: "OAuth2Bearer Strategy" is not working.

The callback function(Verify) receives only 2 params: (token, done)

new BearerStrategy(options, **function(token, done)** {

When using this code I found out that actually the verify cb function gets called with 3 params (req, token, done) and thus generating and error when trying to call done. Which in this case done is the token.

"Done is not a function"

Is this an error in the readme or myself not correctly using the library?

Using a 3 params callback function solves the issue.

@yarandoo
Copy link
Author

yarandoo commented Jul 7, 2016

Also it seems that options.passReqToCallback it is always set to true:

in bearerStrategy.js
BearerStrategy.call(this, util._extend(opts, { passReqToCallback: true }), jwtVerify);

Making the following conditional not necessary (BearerStrategy Line 177) ?

if (opts.passReqToCallback) {
        log.info('We did pass Req back to Callback');
        return verify(req, verifiedToken, done);
      }
      log.info('We did not pass Req back to Callback');
      return verify(verifiedToken, done);

@yarandoo
Copy link
Author

yarandoo commented Jul 7, 2016

Also documentation is wrong:

/**

  • Applications must supply a verify callback, for which the function
  • signature is:
    *
  • function(token, done) { ... }
    

I am happy to create a PR if you think all of the above issues are correct.

@lovemaths
Copy link
Contributor

@yarandoo I tried the sample using the master branch passport code, I didn't get the "Done is not a function" error. Is the 'passReqToCallback' set to 'false' in your server_config.js?

There are two 'passReqToCallback' parameters involved in this code, one for the Strategy, one for the BearerStrategy (which Strategy inherits) (Yes, that's confusing...)

In the following code:
line 188: BearerStrategy.call(this, util._extend(opts, { passReqToCallback: true }), jwtVerify);
the 'passReqToCallback' parameter is for the verify function of passport-http-bearer strategy, in other word, function jwtVerify(req, token, done), so it should be always 'true'. BTW, the jwtVerify function is defined at line 143.

In the following code:
line 79: function Strategy(options, verifyFn) { ..... }
The options.passReqToCallback is for the verifyFn, its value is based on whether verifyFn takes (req, token, done) or (token, done).

I suspect your verifyFn takes (token, done), but your options.passReqToCallback is 'true'. If this is not your setting, please let me know, thanks!

@polita
Copy link
Contributor

polita commented Jul 13, 2016

Please reopen if you believe this is still an issue, @yarandoo.

@polita polita closed this as completed Jul 13, 2016
@adaur
Copy link

adaur commented Aug 7, 2016

Hi there,

I've had the same problem, and I strongly believe this is still an issue.

My passReqToCallback from server_config.js was set to false, but obviously the

BearerStrategy.call(this, util._extend(opts, { passReqToCallback: true }), jwtVerify);

made it useless.

I got rid of the "Done is not a function" changing

var bearerStrategy = new BearerStrategy(options,
    function(token, done) {

to

var bearerStrategy = new BearerStrategy(options,
    function(req, token, done) {

after having applied the code provided in #112 and #117 to finally made the Bearer strategy work as expected.

It is very hard to figure out what to do to fix such issues, I had to dig into closed issues to see what was wrong, PLEASE update your samples and update the version available from npm.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants