[2068] Service worker prevents auth redirect when dashboard is behind an authentication proxy - #2213
Conversation
|
Hey @colinl - could you test this one on your end please? |
…le assets during local development
|
I will give it a go. |
|
No problems specific to this PR on android PWA so far. I have noticed one thing, that sometimes when returning to the app it hangs forever showing the loading page, but I think that was happening on main. It recovers on swiping down to force a refresh. I need to go back to the current release I think and see if that did it too. It may not be new, it may just be that I am doing a lot of testing with a short expiry time on the cloudflare token and am seeing a rare event. It will take some time to test properly as it is only occurring once or twice or even no times a day. I want to stick with the PR for a few days though to make sure that I don't see lockup I saw on #2069 (#2069 (comment)). |
|
I took this for a spin locally to verify both the bug and the fix. Setup: built What checked out:
One real problem though, and it's subtle: passing The good news is the clean fix also simplifies the code: registerRoute(new NavigationRoute(new NetworkOnly({
networkTimeoutSeconds: 5,
plugins: [{
handlerDidError: async () => (await matchPrecache('index.html')) || Response.error()
}]
})))I patched the branch to this form, rebuilt, and re-ran every scenario: the strict proxy now sees A few smaller things worth a look while you're in here:
Nice work on the manual test plan by the way, it was accurate for everything it covered; the header-checking proxy behavior is just hard to catch with a stub that doesn't emulate it. |
@cstns, I don't fully understand the implications of that. What symptoms might that cause on a device running Android? |
|
With the current setup:
The symptom chain:
To a user it looks like the app got replaced by a broken browser page. It self-heals once the server is reachable again and the user refreshes, since pull-to-refresh works on that error page and Express serves |
|
The fix is tiny: build the URL with the trailing slash (or better, derive it from the current location instead of hardcoding), and the reload becomes an in-scope navigation that this PR's fallback can catch. |
OK, thanks. So not the symptom I see very occasionally (which is not specific to this PR) where it hangs on the Loading screen until refreshed. |
|
Yep! |
|
Hey @cstns ty for review and testing. Fixed:
|
|
All looking good here, apart from the occasional hang I get when I go back to the app on Android if the session has expired. For completeness, the symptom I see is that the node red icon shows for a couple of seconds, then it switches to the loading screen, with the cycling dots, and just hangs there apparently for ever. As I said, though, I think that was there before this PR was applied. |
Description
Lets a PWA behind an auth proxy follow the proxy's login redirect when the session expires, instead of getting stuck on a cached page. Page loads now go to the network first so the redirect comes through, and fall back to the cached copy only when the network is down, so the app still loads offline and on a cold start.
Builds on @pakerfeldt's #2069: that PR removed the
NavigationRoute, which fixed the redirect but left the PWA with no offline fallback, so it hangs on launch when the network isn't up yet, @colinl hit this on Android. Network-first keeps both.Test plan
Verified manually on Chrome + iOS Safari PWA
All run against a production build (
npm run build) served by Node-RED, the service worker only precaches in prod builds. The offline cases just turn the network off (DevTools Offline on desktop, airplane mode on iOS). The auth cases sit the dashboard behind a small stub proxy that redirects to a login page once the session is marked expired, standing in for Cloudflare Access; on iOS that runs over an HTTPS tunnel so the phone can reach it.Desktop Chrome
iOS Safari PWA (over HTTPS tunnel)
Related Issue(s)
Resolves #2068
Checklist
flowforge.yml?FlowFuse/helmto update ConfigMap TemplateFlowFuse/CloudProjectto update values for Staging/ProductionLabels
area:migrationlabel