search

simov / grant

OAuth Proxy
MIT License
4.09k stars 257 forks source link

Require state + state validation #226

Closed mshwery closed 3 years ago

mshwery commented 3 years ago

Is it possible for some providers to require state and ensure that the state gets validated?

It appears that in both the latest version and in previous versions we only validate state if it is present in both the authorize object and session object.

simov commented 3 years ago

The state parameter is strictly an OAuth 2.0 thing:

RECOMMENDED. An opaque value used by the client to maintain state between the request and callback. The authorization server includes this value when redirecting the user-agent back to the client. The parameter SHOULD be used for preventing cross-site request forgery as described in Section 10.12.

For all OAuth 2.0 providers in Grant if you specify state: true, then Grant will generate a random state string, store it in session, and validate it when returned back from the authorization server.

mshwery commented 3 years ago

Yeah that makes sense. I understand that state only applies sometimes, but even so, it looks like the code only validates the state if it's present. That means if someone messes with the url or removes the state from the callback querystring, it wont get validated at all.

It doesn't seem like setting state: true influences the validation step at all. Am I missing something? https://github.com/simov/grant/blob/d8dabdcf0de3e6a953eaeed4c2db4d1b8d2be0f9/lib/flow/oauth2.js#L68

simov commented 3 years ago

I think you are right. I traced back that commit https://github.com/simov/grant/commit/d7883342ea98c190916b3653623f61dee4f21ad6 and I think my goal was to make the state optional, but the condition should've been checking for the existence of state in session instead.

Good catch! I will prepare the necessary tests for this and I will release a patch by the end of the week.

simov commented 3 years ago

v5.4.10 was published with the fix :+1:

Here is the relevant change and test in case you are curious https://github.com/simov/grant/commit/3071b5bf38116aa4dc7e730766f74536fdb8add7