RaspberryPiFoundation / editor-ui

Code Editor web component
https://editor-static.raspberrypi.org
Apache License 2.0
37 stars 8 forks source link

Quick fix for expired access token problem in web component #1046

Closed floehopper closed 2 weeks ago

floehopper commented 2 weeks ago

This is a quick fix for https://github.com/RaspberryPiFoundation/editor-ui/issues/1044.

We've been seeing quite a few Faraday::Unauthorized exceptions reported by Sentry in Editor API. These errors are being caused by expired access tokens being sent in the requests from the web component.

This can happen if a user is logged in and is viewing a non-public project, but they close their browser and return to the project some time (1 hour?) later when their access token has expired. In this scenario the project will not load, because of the exception and the user will see the "Loading" text instead of the editor.

This commit prevents the web component from using an expired access token in WebComponentLoader and should thus avoid the exeptions we've been seeing.

However, it's not clear this is actually the correct place to fix the problem, because this ignoring/removal of expired access tokens must already be happening (maybe in oidc-client, redux-oidc, or associated code?). It might be better if the WebComponentLoader only reads the user from the redux state (and not also from local storage) and then rely on the component re-rendering when the user becomes available in the state.

We know that the WebComponentLoader component renders (and therefore makes a request to editor-api with no Authorization header) before the user is available in state. This is fine for public projects (i.e. those where user_id is null) but fails for user projects (i.e. those where user_id is set). When the user becomes available in state we'd expect the component to re-render and make a request to editor-api with the Authorization header set correctly. However, this second request doesn't currently happen because of the condition in this line of syncProject in EditorSlice, so we'd need to make a change there too.

The main downside of not making the proper fix is that there may be times when the logged-in "state" of different bits of the UI (e.g. the "Save" button, the global nav, etc) might be inconsistent in some circumstances. However, since the proper fix is likely to be a more significant bit of work, it seems sensible to ship this change to prevent the exceptions now and tackle the proper fix separately.

github-actions[bot] commented 2 weeks ago
github-actions[bot] commented 2 weeks ago
github-actions[bot] commented 2 weeks ago
github-actions[bot] commented 2 weeks ago