Unable to obtain access token whenever I pass custom state param

[somprabhsharma]

Whenever I pass custom state param, i get following error Unable to verify authorization request state. , If I remove state param then everything works fine.

[analog-nico]

If you don't pass a state parameter then Passport's default state management will be used. Usually, you don't need to set your own state parameter.

However, if you want to pass your own state parameter nonetheless, please refer to Passport's OAuth2Strategy since passport-pinterest only passes the parameter through.

[somprabhsharma]

@analog-nico but if I use any other passport strategy like passport-linkedin or passport-soundcloud with the same code then I am not getting any error while using state param. But I am getting error in passport-pinterest. So issue is not with Passport's OAuth2Strategy .

[analog-nico]

Sorry, buddy, I can't help you with this because I currently have no test setup. But anyhow it really is not caused by passport-pinterest itself because there is just no code other than setting state = true as the default.

The most likely causes are:

• The parameter value you pass as the state option
• The way the Pinterest API works

It is unlikely that the OAuth2Strategy contains a bug. But I am pointing you to this because the OAuth2Strategy is the one actually processing your state parameter value. What you basically need to find out is what happens under the hood with your state parameter value and how the Pinterest handles it.

[somprabhsharma]

@analog-nico I did some changes in passport-pinterest library in my local system. and It worked. I first analysed all other passport strategy and find out that no one is using state param in the strategy.js file. So I removed it and it worked. should I submit a PR ?

[analog-nico]

Yes, a PR would be great. Please also tell me which state parameter value you are using. Thanks!

[somprabhsharma]

@analog-nico I submitted a PR. please review.

[analog-nico]

Judging from PR #4 you want to pass for state either a falsy value or an empty string, right? What actual value do you want to pass?

[somprabhsharma]

@analog-nico As per the oauth2 documentation state param contains a custom string value to maintain state of the client. and after successfull authentication client gets the same state value. So I want to pass some custom string param in state param. e.g. I want to use the below code in /pinterest route.

passport.authenticate('pinterest', {
      state: 'dGVzdGVuY29kZWR2YWx1ZQ'
})(req, res)

[somprabhsharma]

@analog-nico Also below are link to popular passport-strategys. No one is using state in their code.
https://github.com/mjpearson/passport-wordpress/blob/master/lib/passport-wordpress/strategy.js#L48

https://github.com/jaredhanson/passport-tumblr/blob/master/lib/passport-tumblr/strategy.js#L50

https://github.com/jaredhanson/passport-facebook/blob/master/lib/strategy.js#L54

https://github.com/mstade/passport-google-oauth2/blob/master/lib/oauth2.js#L48

https://github.com/jaredhanson/passport-soundcloud/blob/master/lib/passport-soundcloud/strategy.js#L48

[analog-nico]

Sorry, I am not trying to reject your issue or anything.

But did you debug your code? At least by looking at the passport-pinterest code your state parameter should be forwarded to the OAuth2Strategy as expected. There is no need to remove the lines in PR #4 .

Btw, the line options.state = options.state || true makes sure that a state value is always passed to the Pinterest API even if the user doesn't set the state option. This is for added security that the other strategies just not employ.

[somprabhsharma]

@analog-nico you are doing it wrong. You are setting options.state = options.state||true while creating Pinterest Strategy. But Pinterest Strategy is a part of middleware and runs only at the time of starting the server. So at that time options.state is undefined so it sets its value to true and if passport-oauth2 sees state param's value as true then it handles the state by itself. Hence the state param I am sending explicitly is replaced by passport-oauth2 and is of no use. To use it you have to remove those lines from your code. Please test your code nicely. It does not work unless you remove those lines of code. It works only in one scenario when you do not pass state param explicitly.

[analog-nico]

Ok, I see. That means setting state to true as the default has to happen here.

Could you do me a favor and update your PR and test both scenarios - with and without setting the state option?

As I said I have to test set up and it would be important to not introduce breaking changes.

Thanks!

[somprabhsharma]

Yes, I will test all possible scenarios and update the PR.

[analog-nico]

Awesome, thank you very much!

[somprabhsharma]

@analog-nico : I have got clear understanding of how state works in passport. Let me explain it to you. There are two state params -

1. The state param that you specify in passport.authenticate e.g.

app.get('/auth/pinterest',
    passport.authenticate('pinterest', {
       state: 'asfds3UGVUSAFsad'
    })
);

Let's call this state as stateParam. It is a String value. this is state param which is used by all platforms to stop CSRF and is mentioned in Oauth2 specification.
2. The state param that you specify in Strategy Constructor. e.g.

passport.use(new PinterestStrategy({
        clientID: PINTEREST_APP_ID,
        clientSecret: PINTEREST_APP_SECRET,
        scope: ['read_public', 'read_relationships'],
        callbackURL: "https://localhost:3000/auth/pinterest/callback",
        state: true
    },
    function(accessToken, refreshToken, profile, done) {
       done();
    }
));

Let's call this state as strategyState. It is a boolean value (default value: false if user does not override it in constructer) used in passport-oauth2 to decide if user wants passport to handle stateParam or want to handle stateParam by himself.

Now based on this strategyState and stateParam there are Two Main cases.

1. If strategyState value is true. It means that user wants passport to handle the stateParam. In this case the value of stateParam will be generated by passport and verified by passport. User does not have to do anything. In this case user can not supply stateParam value explicitly. [The current scenario happening with your code as you are overriding state as true in constructor.]
2. If its value is false or not overridden. It means that user wants to handle stateParam by himself and does not want passport to interfere. (this is my case). Here can be two more cases -

a. If user add custom stateParam, then it is secure and does not allow CSRF to happen.
b. If user does not add any stateParam, then it is not secure and CSRF can happen.

Now if your code remains same then it works fine for case 1 and case 2.b but does not work for case 2.a as the strategyValue will always be true and if user add custom stateParam then it will not match with the stateParam generated by passport.

And now if you merge the PR submitted by me then it works fine for all 3 cases. But there is security risk in case 2.b that is totally fault of the user who is implementing. He can simple add strategyState as true and ignore the case2.b to make it secure. So my suggestions is to merge the PR.

I hope, I cleared everything to you. Please be wise and make decision.

[analog-nico]

Thanks for the details @somprabhsharma ! The main reason why our discussion turns out to be so complicated is that I am solely responding based on looking at the code. If I would do some testing it would be resolved easily. However, I am not using passport-pinterest myself anymore and thus have no test set up.

I was still thinking there was a way to keep the default behavior of state = true even with PR #4 merged. But after checking the passport-oauth2 code I realized there isn't.

Ok, I'll merge PR #4 and make a semver major version bump because it - as you point out - introduces a breaking change for case 2.b.

Thanks for working on this! Btw, would you like to take over the project and continue its maintenance?

[analog-nico]

I just published version 1.0.0. Thanks again!

[somprabhsharma]

@analog-nico Yes, I would love to be a part of this project and ready for its maintenance. Thank you for asking.

[analog-nico]

@somprabhsharma I am glad you wanna join in! Since you are actually using this lib this is a big help! You should just have received an invite for push rights. Is @somprabhsharma you handle on npm as well?

[somprabhsharma]

@analog-nico yes @somprabhsharma is my handle on npm as well.

[boyangwang]

@somprabhsharma Thank you so much. You explanation helped many who are struggling with oauth state