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

Logging out after the HTTP session has expired results an error #164

Closed markhobson closed 1 month ago

markhobson commented 1 month ago

The following has been observed:

  1. A user signs in
  2. Sometime after the HTTP session expires (currently >1hr) the user signs out
  3. They receive a 500 error page

The error in the logs is:

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 59, in logout
    id_token = session["id_token"]
               ~~~~~~~^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/flask_session/base.py", line 80, in __getitem__
    return super().__getitem__(key)
           ^^^^^^^^^^^^^^^^^^^^^^^^
markhobson commented 1 month ago

Logout currently assumes that the HTTP session contains the id_token in order to log out of GOV.UK One Login. This could be fixed in two ways:

  1. We add @bearer_auth to /logout to force reauthentication to provide us with an id_token in the HTTP session
  2. We short-circuit /logout if there is no HTTP session

An issue with (2) is that we won't have an id_token to sign the user out of GOV.UK One Login. This can be mitigated by also not supplying post_logout_redirect_uri but the user won't end up on our service start page.

markhobson commented 1 month ago

Went for (2) since we don't currently redirect to original request when reauthenticating.