oauth2: CSRF cookie should be cleared after flow complete

[bplotnick]

Title: CSRF cookie should be cleared after flow complete

Description:
I think that the CSRF cookie should be cleared after the completion of the flow. Firstly, it isn't needed anymore after the flow, so it's extraneous. But more importantly, you can have a scenario where it fails validation but improperly causes a failed flow. Let's say you rotate the HMAC secret, the token hmac validation will fail, which will send you into redirectToOAuthServer(), but then the CSRF token will get validated here and also fail validation.

We should probably 1) clear the CSRF cooke after the callback validation and 2) clear and remove the cookie when during redirect rather than failing the flow.

Repro steps:

1. Configure Envoy with the oauth2 filter and a given HMAC secret
2. Complete the oauth2 flow - you will now have the token cookies as well as the OauthNonce (CSRF) cookie
3. Change the HMAC secret
4. Make a request - you'll get csrf token validation failed

CC: @zhaohuabing

[zhaohuabing]

@bplotnick Thanks for reporting this issue! Is it blocking your use case? I'll take care of it when I have some time.

[zhaohuabing]

/assign

[bplotnick]

@zhaohuabing yeah, it's blocking but i can write the fix (i've already written the regression test). just wanted to open the issue first to solicit input and make sure that the proposed solution is acceptable.

[zhaohuabing]

1. clear the CSRF cooke after the callback validation

The browser sometimes sends multiple requests before the first one gets authenticated, resulting in multiple callbacks from the OAuth2 server, which may not always arrive in the same order as the requests were sent. The same nonce is used for these requests to ensure successful nonce validation.

More discussion can be found here: #36276

2. clear and remove the cookie when during redirect rather than failing the flow.

I think this approach should work. we can remove the existing CSRF cookie and create a new one using the new HMAC.

[zhaohuabing]

/unassign

[bplotnick]

The browser sometimes sends multiple requests before the first one gets authenticated, resulting in multiple callbacks from the OAuth2 server, which may not always arrive in the same order as the requests were sent. The same nonce is used for these requests to ensure successful nonce validation.

This makes sense, thanks for the context

clear and remove the cookie when during redirect rather than failing the flow.
I think this approach should work. we can remove the existing CSRF cookie and create a new one using the new HMAC.

ok i'll go ahead and implement this