acteng / update-your-capital-schemes

Update your capital schemes service.
https://update-your-capital-schemes.activetravelengland.gov.uk/
MIT License
4 stars 0 forks source link

Parallel log ins across multiple browser tabs leads to mismatched state error #125

Closed Sparrow0hawk closed 2 months ago

Sparrow0hawk commented 2 months ago

Summary

If a user starts a login journey in one browser tab, is interrupted and starts a new login journey in another tab it is possible for the user to experience an error after completing a successul login journey but returning to the other tab. This is caused by a mismatched state error due to the way Authlib stores the state parameter in the Flask session to implement CSRF protection.

Example error

[2024-07-10 15:00:42,242] INFO in bearer: User 'user@example.com' successfully signed in
https://update-your-capital-schemes.activetravelengland.gov.uk/auth?code=2KKcVFhe1Ke-cF1s1Wqj6G67QCs30O6l2Jy0n87W1pU&state=Fg9LHAlk1och3ATDttoHOqTZR2HAoj
[2024-07-10 15:00:42,698] ERROR in app: Exception on /auth [GET]
Traceback (most recent call last):   File "/usr/local/lib/python3.12/site-packages/flask/app.py", line 1473, in wsgi_app     response = self.full_dispatch_request()
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/flask/app.py", line 882, in full_dispatch_request
    rv = self.handle_user_exception(e)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/flask/app.py", line 880, in full_dispatch_request
    rv = self.dispatch_request()
         ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/flask/app.py", line 865, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)  # type: ignore[no-any-return]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/inject/__init__.py", line 405, in injection_wrapper
    return sync_func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/app/schemes/views/auth/bearer.py", line 30, in callback
    token = oauth.govuk.authorize_access_token(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/authlib/integrations/flask_client/apps.py", line 100, in authorize_access_token
    params = self._format_state_params(state_data, params)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/authlib/integrations/base_client/sync_app.py", line 234, in _format_state_params
    raise MismatchingStateError()
authlib.integrations.base_client.errors.MismatchingStateError: mismatching_state: CSRF Warning! State not equal in request and response.

Suggested fix

As described in https://github.com/lepture/authlib/issues/285 we could check for a logged in user within the callback method.

markhobson commented 2 months ago

Avoid reauthenticating user if they are already authenticated.