Open Yonneh0 opened 2 years ago
This is the intended behavior.
There is no need for this. The state is there to prevent CSRF attacks. Here the client is the good guy and the attacker is trying to mislead the user-agent of your users, you're not trying to protect your server from your users/clients here.
What you want in this case is to ensure that you're accepting only the requests that the user-agent/client of your users initiated themselves.
You can read more about this in the OAuth2 spec here.
Once authorization has been obtained from the end-user, the authorization server redirects the end-user's user-agent back to the client with the required binding value contained in the "state" parameter. The binding value enables the client to verify the validity of the request by matching the binding value to the user-agent's authenticated state. The binding value used for CSRF protection MUST contain a non-guessable value (as described in Section 10.10), and the user-agent's authenticated state (e.g., session cookie, HTML5 local storage) MUST be kept in a location accessible only to the client and the user-agent (i.e., protected by same-origin policy).
I realize this is old code but I felt like I should give a heads up, incase anyone else comes across this.
The whole cookie portion accomplishes nothing- it just verifies the cookie from the client, against the state, that also comes from the client. It may add some obfuscation, but I don't believe it is achieving the intended result.
my solution, was:
map[string]struct{exp time.Time,state string}
, with the key being the client's IP addresshowever, after some quick googling, a far better version of my solution exists- https://github.com/letsencrypt/boulder/blob/main/nonce/nonce.go
https://github.com/douglasmakey/oauth2-example/blob/b19e764d1950c0d5cba1f56d185cdcd17171d950/handlers/oauth_google.go#L45