Make sure Scratch API calls use renewed tokens#1441
Open
zetter-rpf wants to merge 5 commits intomainfrom
Open
Conversation
zetter-rpf
temporarily deployed
to
previews/1441/merge
GitHub Actions
Inactive
zetter-rpf
force-pushed
the
better-token-renewal
branch
from
e0107e5 to
25f91cd
Compare
zetter-rpf
temporarily deployed
to
previews/1441/merge
GitHub Actions
Inactive
We expect the access access token to change when it's periodically renewed. We should not reload the whole project when this happens as it will cause the loading state to flash and the project to be loaded again. This also has might loose any unsaved work. I'm not sure if it's needed, but i've made sure the effect still runs if the access token goes from unset to set or set to unset.
zetter-rpf
force-pushed
the
better-token-renewal
branch
from
25f91cd to
26aacd0
Compare
zetter-rpf
temporarily deployed
to
previews/1441/merge
GitHub Actions
Inactive
zetter-rpf
force-pushed
the
better-token-renewal
branch
from
26aacd0 to
6f956c1
Compare
zetter-rpf
temporarily deployed
to
previews/1441/merge
GitHub Actions
Inactive
zetter-rpf
changed the title
zetter-rpf
force-pushed
the
better-token-renewal
branch
from
6f956c1 to
77c2ff9
Compare
zetter-rpf
temporarily deployed
to
previews/1441/merge
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Pull request overview
This PR ensures long-lived editor sessions (especially Scratch-in-iframe) keep using the latest renewed access token, while avoiding unintended project reloads when tokens refresh.
Changes:
- Prevent project reloads on token refresh by making project-loading effects depend on “token present” rather than token value.
- Replace the localStorage user-sync middleware with explicit initial user load + a polling hook to keep Redux auth state updated.
- Add a new
scratch-gui-update-tokenmessage flow so the parent can push refreshed tokens into the Scratch iframe.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/web-component.js | Dispatches initial auth user into Redux from localStorage at mount time. |
| src/scratch.jsx | Centralizes/standardizes parent origin validation via allowedParentOrigin(). |
| src/redux/stores/WebComponentStore.js | Removes the prior localStorage user middleware from the store setup. |
| src/redux/middlewares/localStorageUserMiddleware.js | Removes middleware previously used to sync user from localStorage on editor actions. |
| src/redux/middlewares/localStorageUserMiddleware.test.js | Removes tests for the deleted middleware. |
| src/hooks/useSyncUserFromLocalStorage.js | Adds polling hook to keep Redux auth user in sync with localStorage changes. |
| src/hooks/useProject.js | Stops reloading project data on token refresh by depending on hasAccessToken instead of accessToken. |
| src/hooks/useProject.test.js | Adds tests for “load when token becomes available” and “don’t reload on token change.” |
| src/containers/WebComponentLoader.jsx | Switches to Redux-only user source + starts localStorage polling hook. |
| src/containers/WebComponentLoader.test.js | Updates tests to reflect new auth sync behavior. |
| src/components/ScratchEditor/events.js | Exposes allowedParentOrigin() and uses it in postScratchGuiEvent. |
| src/components/ScratchEditor/ScratchIntegrationHOC.jsx | Silences warnings for the new scratch-gui-update-token message type. |
| src/components/ScratchEditor/ScratchEditor.jsx | Listens for scratch-gui-update-token and updates Scratch fetch metadata. |
| src/components/ScratchEditor/ScratchEditor.test.jsx | Adds test asserting Scratch metadata updates on token update message. |
| src/components/Editor/Project/ScratchContainer.jsx | Posts scratch-gui-update-token to the iframe when auth token changes. |
| src/components/Editor/Project/ScratchContainer.test.js | Adds coverage for scratch-gui-update-token posting behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The user of the web component is responsible for managing the auth token. Currently we have use a custom middleware which runs before certain actions to sync the user data. The problem with this approach is that if there are no actions dispatched then the user data will become out of date and requests will fail. This is a problem for Scratch which runs in a separate iframe and does not trigger actions in the root window before saving. Instead, set syncing that runs every 45 seconds to check that the user data is in sync. I've chosen to key the changes off the access token as that's the most important object and I found that the whole user data object always failed comparisons. I've chosen 45 seconds as by default our oidc client renews tokens 60 seconds before they expire. As part of this I've moved the initial setUser call into web-component.js so we don't need to re-render the component the first time the user loads. Note more will need to be done in the next commit to pass this updated token to Scratch. An alternative to this would be to pass the user data as props. This would be a breaking change for users of the web component which I why I've preferred this approach.
In the next commit I'm going to introduce another message so make sure we're checking the correct ones
zetter-rpf
force-pushed
the
better-token-renewal
branch
from
77c2ff9 to
047ed58
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1197
We periodically renew tokens (every hour in production). We should make sure Scratch has access to the latest token otherwise someone who has kept the window open working on their project may loose their data.
This PR address a few related parts:
See commits for more.
A note on reproducing locally:
By default
oidc-client-tsrenews the token when it has 60s left, and our tokens last for an hour. To avoid waiting an hour, you can updateTTL_ACCESS_TOKENandTTL_REFRESH_TOKENin the profiledocker-compose.yml. I recommend setting them to90sand120swhich will cause a refresh every 30s.I also had to temporally comment out
"Cross-Origin-Embedder-Policy": "require-corp",from editor standalone'scraco.config.jsin order for token refresh to work locally.