kanboard / plugin-oauth2

Generic OAuth2 authentication plugin
MIT License
27 stars 33 forks source link

state parameter should be different for each authentication request #39

Closed nagylzs closed 1 year ago

nagylzs commented 1 year ago

Actual behaviour

When you enable this plugin, then it shows a login link to /oauth/callback. That URL returns a HTTP 302 redirect response to the authorization url of the oauth2 authorization server. The response DOES NOT CONTAIN cache-control/no-cache header, nor max-age=0 header and it DOES CONTAIN the state parameter of the authorization request.

If anybody uses kanboard behind a proxy (probably most of the users), then this behaviour results in caching the state parameter of the authorization request.

Also, if the user aborts the login process by pressing the "back" button of the browser and goes back to the kanboard login page again, then it will basically make it impossible to login, because the HTTP 302 redirect will always be cached (by possibly both the browser and the proxy), and the authorization server will (or at least should) refuse any further authorization requests with the same state value.

Expected behaviour

The redirect response MUST contain cache-control no-cache and max-age 0 headers. The redirect SHOULD use HTTP 301 instead of HTTP 302, in order to prevent caching on the browser side.

Steps to reproduce

There are two ways to test this. One is to put kanboard behind a caching proxy, make sure that the redirect is cached for an unfinished/aborted authorization request. Then try to login again, and it will fail. (Or it might not fail, but the same state parameter will be used for all login requests that go through the same proxy, which is even worse.)

nagylzs commented 1 year ago

The other way to test it (without any proxy) is to start the oauth2 login flow, but abort it and go back directly to the kanboard login page before the authorization flow is completed. Then try to start the authorization flow again - it will most probably start with an authorization url that was cached by the browser, e.g. it will use the same state parameter. Although I did not test it.

nagylzs commented 1 year ago

After all there is a cache-control header, it was a mistake