[Harm-r]

Currently, simplesamlphp-module-authoauth2 only invalidates the OAuth state parameter on a successful linkback. This is a minimal pull request that makes the Oauth2Controller always delete the state on linkback, notably also on error.

Why?

It is recommended by RFC 9700: "Best Current Practice for OAuth 2.0 Security": "The state value SHOULD be invalidated by the client after its first use at the redirection endpoint.". This is to prevent the state parameter from leaking through e.g. a Referrer header on error.

[pradtke]

@Harm-r Thank you for your contribution. We'll review ASAP.

[pradtke]

Hi @Harm-r , We've done some review of your PR.

Our error handling for consent/access_denied errors (and potentially other areas) instructs the user to "click back" and then authorize the client. For that to work, the $state must remain in place for error conditions to allow the user to retry.

Reviewing RFC 9700, there are other mitigations available to webapps to reduce referrer header leakage.

During major SSP and PHP upgrades we evaluate more comprehensive changes in this module, and at that time we can see if there is way to allow user's to retry while also invalidating the state.

Thanks again for raising the issue.

[Harm-r]

Ah, I missed that use of $state in my testing, thank you for pointing that out. I agree with your evaluation that this can wait until a later update.