Forcibly reload app from server when API is redirected #3286

TimQuelch · 2024-08-18T08:15:28Z

This adds a wrapper around fetch so the application is reloaded when an API request is redirected. The motivation for this is as a workaround for the bug in #2793.

This issue occurs when using forward authentication with something like authentik with a proxy (e.g. caddy, traefik). The proxy checks with the auth service whether all requests to the host are valid, if they are they are forwarded onto the backend server (in this case actual server). If the request is invalid e.g. the cookie has expired or the user has not authenticated yet, then the proxy redirects the user to the authentication page. After authenticating they are redirected back to what they were originally requesting.

For a more standard application; after a authenticatoin cookie has expired the next time the page is reloaded it would redirect to the auth page in order for the user to re-authenticate. However actual uses a service worker pattern where the application code is cached and all requests are served by the worker. This means that reloading the page in the browser does not actually result in a http request to the backend server; which means that that request cannot be redirected to authenticate. This results in an invalid authentication session and API queries to the backend failling because they are redirecting to auth pages.

This PR adds a button in the advanced settings section that un-registers the service worker handling the routing and then reloads the page. This forces the page to be requeried from the backend server which is then able to be redirected to the authentication page.

I've tested this functionality on web, mobile browser, and also iphone PWA and it works as expected.

I've got a couple things I want to check on my implementation:

This functionality isn't relevant on electron. I have followed the pattern that the ResetSync follows by disabling the button if we're on electron. I considered also completely hiding the setting when on electron. Which would be preferable?
this page mentions that changes to global.Acutal are somewhat fragile. Basically what I've done is defined the new reload function only in the browser version and typed it as maybe undefined. We check if it is defined before trying to call it; this means that on electron it won't be called. Is this the correct way to go about this?

I couldn't think of a solution that was less of a workaround than this. Maybe this reloading could be triggered if an API query gets a 302 to a configurable path? e.g if /sync/sync gets redirected uselessly to auth.mydomain.com then we trigger reloading the whole app so that the user can re-auth? I feel I don't know the full consequences of this solution though. Does anyone have any thoughts on a more automatic solution that will work in a general and/or configurable way?

Edit: After feedback I re-implemented the solution to wrap the API fetch so that if any API call is redirected the application is reloaded.

netlify · 2024-08-18T08:15:44Z

✅ Deploy Preview for actualbudget ready!

Name	Link
🔨 Latest commit	`63f425f`
🔍 Latest deploy log	https://app.netlify.com/sites/actualbudget/deploys/66f078ab47dac400084db689
😎 Deploy Preview	https://deploy-preview-3286.demo.actualbudget.org
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions · 2024-08-18T08:16:38Z

Bundle Stats — desktop-client

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count	Total bundle size	% Changed
9	5.29 MB → 5.29 MB (+470 B)	+0.01%

Changeset

File	Δ	Size
`home/runner/work/actual/actual/packages/loot-core/src/client/actions/app.ts`	📈 +77 B (+13.73%)	561 B → 638 B
`src/browser-preload.browser.js`	📈 +321 B (+8.67%)	3.61 kB → 3.93 kB
`src/global-events.ts`	📈 +72 B (+2.34%)	3 kB → 3.07 kB

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
static/js/index.js	3.31 MB → 3.31 MB (+470 B)	+0.01%

Smaller

No assets were smaller

Unchanged

Asset	File Size	% Changed
static/js/resize-observer.js	18.37 kB	0%
static/js/indexeddb-main-thread-worker-e59fee74.js	13.5 kB	0%
static/js/BackgroundImage.js	122.29 kB	0%
static/js/usePreviewTransactions.js	1.59 kB	0%
static/js/AppliedFilters.js	20.97 kB	0%
static/js/narrow.js	81.02 kB	0%
static/js/wide.js	225.4 kB	0%
static/js/ReportRouter.js	1.5 MB	0%

github-actions · 2024-08-18T08:16:39Z

Bundle Stats — loot-core

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count	Total bundle size	% Changed
1	1.19 MB → 1.19 MB (+208 B)	+0.02%

Changeset

File	Δ	Size
`packages/loot-core/src/platform/server/fetch/index.web.ts`	📈 +629 B (+1612.82%)	39 B → 668 B

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
kcab.worker.js	1.19 MB → 1.19 MB (+208 B)	+0.02%

Smaller

No assets were smaller

Unchanged

No assets were unchanged

StefanFabian · 2024-08-21T19:07:33Z

I found this because I have the same issue with Authelia and Actual.
Why did you decide against just handling every 302 on API requests and executing the 302?
I would assume that the user is in control of the server hosting Actual, so all 302 requests will probably happen for a reason, and IMO, that would be a cleaner way of handling that than having to do it manually.

alexyao2015 · 2024-08-21T23:55:42Z

Could we do something like this? I run silverbullet as well and it seems like they were able to solve the issue by just setting location.href to the redirected url. This has the benefit of being able to automatically redirect the user rather than manually pressing a button.

https://github.com/silverbulletmd/silverbullet/blob/main/common/spaces/http_space_primitives.ts#L42-L65

TimQuelch · 2024-08-22T04:48:03Z

Reloading when an API query gets a 302 would work for my setup (and I'd guess many other people's), however there are probably many valid setups where a 302 is normal and the correct action is to have the API follow the redirect and return the redirected response normally.

Can we be confident that any time a 302 is returned that we need to reload? Maybe a configuration setting which has this automatic reloading as an optional feature?

TimQuelch · 2024-08-22T04:50:01Z

Maybe an option where you can configure hosts that if we get redirected to one of those hosts we reload?

In this case any time I get a redirect to auth.mydomain.com I want to reload

alexyao2015 · 2024-08-22T06:13:59Z

I don't see why any config would be necessary. If you make a request to a trusted destination and the request is calling for you to be redirected, you should redirect. You don't even need to handle reloading I think. As long as your proxy sends a redirect call, it should just directly redirect you without prompting. That's how normal webpages without service workers would react anyways.

alexyao2015 · 2024-08-22T06:25:32Z

Just glacing over the source I see this which appears to let you wrap the fetch command.

actual/packages/api/index.ts

Line 32 in 9c0e6a3

if (!globalThis.fetch) {

StefanFabian · 2024-08-22T08:05:59Z

Reloading when an API query gets a 302 would work for my setup (and I'd guess many other people's), however there are probably many valid setups where a 302 is normal and the correct action is to have the API follow the redirect and return the redirected response normally.

Can we be confident that any time a 302 is returned that we need to reload? Maybe a configuration setting which has this automatic reloading as an optional feature?

A whitelist would also be an option but as this is a common problem with reverse proxies + auth, I think having to first figure out what's wrong and then find the solution would be a bit annoying to users.

As I see it, there are two common behaviors that can happen when you get a 302:

Your browser sends an options request and the reply doesn't have the correct CORS headers

Should you redirect to the page in that case? I think this will be the default case unless someone already configured their auth server to return Access-Control-Allow-Headers: * since actual will send the following access control request Access-Control-Request-Headers: x-actual-token with the options request.

The options request passes and the API follows the 302 successfully

If it is just an API redirect, the data should still be valid JSON. For an auth server, the JSON parsing would fail and we could use that as an indication that we should redirect to the 302 page.
Maybe first verify that the returned data is HTML?

Another point to consider is that some auth solutions pass the return URL to the auth page.
If we just set the location to the redirect URL, after authentication, it would redirect the user to the result of an API call, which doesn't make sense.
Is there a way to fetch the current URL without the service worker catching the request and serving it from the cache?
In that case, we could use that after determining that authentication is required.

TimQuelch · 2024-08-22T11:45:13Z

I'm happy to implement doing this automatically when an API request gets a 302 if maintainers agree that is appropriate. My initial design was erring on the side of not breaking existing setups.

If you make a request to a trusted destination and the request is calling for you to be redirected, you should redirect. You don't even need to handle reloading I think. As long as your proxy sends a redirect call, it should just directly redirect you without prompting. That's how normal webpages without service workers would react anyways.

The problem is only api requests get redirected. GET requests for the page itself get served by the service worker with cached code and therefore do not get redirected.

Another point to consider is that some auth solutions pass the return URL to the auth page.
If we just set the location to the redirect URL, after authentication, it would redirect the user to the result of an API call, which doesn't make sense.

I agree, we do not want to follow the 302 that originates from the API request because after authentication finishes we'd be redirected back to the api call; not the app. Instead we'd want to force a uncached GET the current page and follow the resulting redirect.

Is there a way to fetch the current URL without the service worker catching the request and serving it from the cache?
In that case, we could use that after determining that authentication is required.

The way I've figured out how to do that is by unregistering the navigation service worker and reloading. This is what I do in my reload function now. I'm open to other suggestions on how to do this. My understanding of service workers is fairly limited.

jfdoming · 2024-08-22T12:39:57Z

I'm happy to implement doing this automatically when an API request gets a 302 if maintainers agree that is appropriate. My initial design was erring on the side of not breaking existing setups.

If you make a request to a trusted destination and the request is calling for you to be redirected, you should redirect. You don't even need to handle reloading I think. As long as your proxy sends a redirect call, it should just directly redirect you without prompting. That's how normal webpages without service workers would react anyways.

The problem is only api requests get redirected. GET requests for the page itself get served by the service worker with cached code and therefore do not get redirected.

Another point to consider is that some auth solutions pass the return URL to the auth page.
If we just set the location to the redirect URL, after authentication, it would redirect the user to the result of an API call, which doesn't make sense.

I agree, we do not want to follow the 302 that originates from the API request because after authentication finishes we'd be redirected back to the api call; not the app. Instead we'd want to force a uncached GET the current page and follow the resulting redirect.

Is there a way to fetch the current URL without the service worker catching the request and serving it from the cache?
In that case, we could use that after determining that authentication is required.

The way I've figured out how to do that is by unregistering the navigation service worker and reloading. This is what I do in my reload function now. I'm open to other suggestions on how to do this. My understanding of service workers is fairly limited.

I think the approach needed here would be to change the service worker logic to stale-while-revalidate, which causes it to still fetch in the background if there is a connection. That way we can handle the background fetch and redirect if it returns 302. I'm on board with just redirecting unconditionally for now; if someone does start using a setup where the redirect isn't an auth request, we can help them at that time.

Another thing worth noting is that IIRC one of the first things the app does when it loads is make an API request to check if the server has been initialized. Assuming my memory is correct, maybe handling only API requests is fine?

alexyao2015 · 2024-08-22T15:06:31Z

Your browser sends an options request and the reply doesn't have the correct CORS headers

In this case, nothing happens since the redirect url cannot be determined.

change the service worker logic to stale-while-revalidate

It's probably better to use network first. Stale while revalidate would work but only on the second request since it pulls from the cache first.

alexyao2015 · 2024-08-22T15:22:03Z

Looking at the config, it seems like no runtime caching is being performed right now. Presumably, all you would need to do is handle the redirect that the service worker returns. A slight disadvantage is the return url would likely be an API call. I suppose if you unregister the service worker and remake the request, that will direct the user to the right place. I'm just not sure of the implications of unregistering the service worker.

https://github.com/actualbudget/actual/blob/master/packages/desktop-client/vite.config.mts#L155

TimQuelch · 2024-08-23T17:49:57Z

I've added functionality to automatically reload when an API call gets redirected. Please let me know if there's anything to improve

alexyao2015 · 2024-08-24T03:21:59Z

Is the reload button on settings even needed still? Can that be removed?

TimQuelch · 2024-09-25T05:48:05Z

user could get stuck in a reload loop if the server sends a 302 to only API resources (if someone forgets to put the JS bundle behind a proxy).

I agree this would likely be quite a rare scenario, but not impossible.

Do you (or anyone else!) have any thoughts on mitigating cases like this?

What's the risk appetite for potential issues like this? Maybe it's best to wait to see if anyone actually has this issue before we try to fix it?

jfdoming · 2024-09-25T13:14:12Z

user could get stuck in a reload loop if the server sends a 302 to only API resources (if someone forgets to put the JS bundle behind a proxy).

I agree this would likely be quite a rare scenario, but not impossible.

Do you (or anyone else!) have any thoughts on mitigating cases like this?

What's the risk appetite for potential issues like this? Maybe it's best to wait to see if anyone actually has this issue before we try to fix it?

One option we've used in the past is an experimental feature that defaults to on. That way, if folks run into any problems, they can turn off the redirects, disable the feature, and then report the issue.

alexyao2015 · 2024-09-25T13:48:51Z

That doesn't seem like a bug worth fixing. Say a user encounters an infinite reload. They disable the automatic reload feature (which they're unlikely to disable without clearing browser cache because now the page is reloading constantly). Would we expect the server to function at all in this case? The user would still continue to experience redirects when attempting to reach the API. I think if someone did have this issue, having it loop like this would show them that something is wrong with the configuration.

youngcw · 2024-09-25T15:39:05Z

I agree its probably not worth worrying about an edge case that is unlikely to happen, especially since this will help a bunch of people as is. The issue of needing to reauthenticate trips people up all the time.

jfdoming · 2024-09-25T17:37:50Z

I agree its probably not worth worrying about an edge case that is unlikely to happen, especially since this will help a bunch of people as is. The issue of needing to reauthenticate trips people up all the time.

Alright, I'm fine to address it later if someone runs into it 👍

youngcw · 2024-10-28T19:59:11Z

This doesn't seem to be working for me anymore on edge. I wonder if something broke this.

StefanFabian · 2024-11-05T11:06:48Z

Same, I'm on 24.10.1 and it does not reload even though all the API requests are redirected to my auth page.

alexyao2015 · 2024-11-05T12:34:33Z

I realize we can change the service worker to not capture the sync API call to avoid this issue entirely.

xxxarmitagexxx · 2024-11-07T04:01:29Z

I am on 24.11.0 and am still experiencing a sync issue when going through Authentik after the auth token expires.

gcoremans · 2024-11-20T21:25:47Z

I recently installed Actual 24.11.0 and this is also not working for me.

xxxarmitagexxx · 2024-11-21T04:05:55Z

I am on 24.11.0 and am still experiencing a sync issue when going through Authentik after the auth token expires.

To clarify, the issue presents itself independently from the Authentik expiry. I have set it to 52 weeks last time I had the issue and had to delete all browser data to force a reload and had again the issue today, a week later.

A fix would be greatly appreciated as this makes Actual difficult to transition to in a family setting. I intend to get my wife to move off YNAB in favor of Actual and certainly don’t see her clearing her browser data on a frequent basis.

TimQuelch · 2024-11-21T08:41:04Z

What browser are you using? What errors are you seeing? What do the redirects look like?

I have set it to 52 weeks last time I had the issue and had to delete all browser data to force a reload and had again the issue today, a week later.

keep in mind session expiry and token expiry are distinct. Your token may have expired, but it may still need to redirect to your auth to get a refreshed token using your session which has not yet expired.

xxxarmitagexxx · 2024-11-21T09:04:17Z

I am using Firefox normally on Desktop but have the same issue with Chrome and also on Mobile (Samsung Internet, Chrome, Firefox).

When the issue happen, the sync would stop working, I would see the server as offline. If I close the file and delete it, I cannot see the server at all, it would say that actual-fqdn.com cannot be reached, please check SSL configuration.

I will check my redirections next time the problem occurs.
Under normal circumstances, the redirections looks like actual-fdqn.com --> actual-fdqn.com//outpost.goauthentik.io/start?rd=/ -> authentik-fdqn.com//if/flow/default-authentication-flow/....

I was not aware of the differences between session expiry and token expiry, thanks for pointing it out.
The token validity for the actual provider is set as weeks=52.
The session validity for the user login authentication stage is set as 0 (session expires when browser closes). Looking in the list of sessions against my user in Authentik, I can see that the most recent session created today has a 14 days expiry so I guess there is a maximum regardless of the browser being closed or not.

Considering those two parameters, I don’t see how the Actual token would expire before the session itself expires.

So I am not clear on what exactly is happening.
Any tips on what to look for/investigate, next time the problem reoccurs?

xxxarmitagexxx · 2024-11-21T09:15:11Z

I have the issue currently with my phone.
I have deleted all sessions in Authentik.

Going to actual-fdqn.com, it simply loads the offline file showing the server offline.
If I go to one other service being Authentik, I immediately get the Authentik sign in page before being redirected to that service.

Once logged in Authentik, if I try to reload actual-fdqn.com or open a new tab in the same browser exhibiting the issue, it loads immediately the offline file, with the server shown offline...

Is there a better way to force a redirection to Authentik? So far the only I found to get around the issue in any browser is to clear all data which gets old every time.

When the issue occurs, the browser does not even attempt to reach Authentik, the offline file is loaded. It is clear as day, as I can unmap the actual-fdqn.com in Nginx proxy manager and it would still resolve loading the offline file....

gcoremans · 2024-11-21T21:13:50Z

Here is the request sequence that's tripping it up for me, if it helps:

TimQuelch · 2024-11-21T23:34:42Z

Ah interesting. Ultimately the request fails because it is blocked by CORS. I doubt the current check is detecting this as a redirected request and instead only seeing a CORs error.

paumove · 2024-11-25T10:58:38Z

Same problem here with v24.11.0 and Authentik.

TimQuelch · 2024-11-25T11:37:00Z

There's a couple of fixes here

Configure your auth server and/or proxy to send the appropriate CORS headers (Access-Control-Allow-Origin, Access-Control-Allow-Methods, etc.) to allow responses from your auth server to be allowed in Actual. How this is done will be entirely dependent on your setup. (this is how my setup is configured which is why I wasn't aware this was an issue)
Update the check that was added in this PR to also detect if a CORs error was encountered. I think this is quite difficult, because as far as I can tell a CORS error is pretty much indistinguishable from any other network error. We may end up in refresh loops if there's some other network problem.
Change how service workers behave so that this check isn't necessary. I'm out of my depth on what those changes would need to be.

gcoremans · 2024-11-25T11:44:51Z

Telling people to set the necessary CORS headers seems like a sufficient solution, but perhaps the docs should be updated to indicate that this might be necessary?

alexyao2015 · 2024-11-29T17:25:38Z

The cors headers was definitely the fix! I think we should just notify people about this in the documentation. I'm running nginx ingress with authentik and this config did it for me. Working flawlessly now.

          nginx.ingress.kubernetes.io/configuration-snippet: |
            set $$allowed_origin "";
            if ($$http_origin = "https://actual.${SECRET_EXTERNAL_DOMAIN_NAME}") {
              set $$allowed_origin $$http_origin;
            }
            if ($request_method = OPTIONS) {
              add_header "Access-Control-Allow-Origin" $$allowed_origin;
              add_header "Access-Control-Allow-Headers" "*";
              add_header "Content-Type" "text/plain; charset=utf-8";
              add_header "Content-Length" "0";
              return 204;
            }
            if ($request_method = GET) {
              add_header "Access-Control-Allow-Origin" $$allowed_origin;
            }

xxxarmitagexxx · 2024-11-30T12:20:49Z

Thanks, it took me a while to figure out how to adapt it in Nginx proxy manager, but I got it working eventually.

As a note for those in the same case: edit the authentik proxy in NPM, create a custom location for /, click the cog wheel and paste the code from alexyao2015 from "set $allowed_origin"";.

actual-github-bot bot changed the title ~~Add advanced setting to reload app from server~~ [WIP] Add advanced setting to reload app from server Aug 18, 2024

actual-github-bot bot added the 🚧 WIP label Aug 18, 2024

TimQuelch mentioned this pull request Aug 18, 2024

[Bug]: Service Worker preventing correct execution of bootstrap when using NGINX reverse proxy #2793

Open

2 tasks

Add advanced setting to reload app from server

d7e5b6a

TimQuelch force-pushed the reload-app branch from a25afe9 to d7e5b6a Compare August 18, 2024 08:31

TimQuelch added 2 commits August 18, 2024 18:38

Add change notes

e77d22e

Merge branch 'master' into reload-app

132fc34

TimQuelch changed the title ~~[WIP] Add advanced setting to reload app from server~~ Add advanced setting to reload app from server Aug 18, 2024

actual-github-bot bot added 🔍 Ready for review and removed 🚧 WIP labels Aug 18, 2024

Automatically reload when API call is redirected

d2ef2c4

TimQuelch force-pushed the reload-app branch from 48695bf to d2ef2c4 Compare August 23, 2024 17:48

TimQuelch added 2 commits August 24, 2024 16:59

Error on redirect so we don't use the response

64f4f2a

Remove setting to reload

6d7808e

TimQuelch and others added 2 commits September 22, 2024 18:55

Merge remote-tracking branch 'origin/master' into reload-app

c0d5a55

Merge branch 'master' into reload-app

63f425f

jfdoming approved these changes Sep 25, 2024

View reviewed changes

actual-github-bot bot added ✅ Approved and removed 🔍 Ready for review labels Sep 25, 2024

youngcw merged commit fe17c6b into actualbudget:master Sep 25, 2024
20 checks passed

actual-github-bot bot added ✨ Merged and removed ✅ Approved labels Sep 25, 2024

coderabbitai bot mentioned this pull request Oct 20, 2024

Auto-reload on app updates #3693

Merged

Forcibly reload app from server when API is redirected #3286

Forcibly reload app from server when API is redirected #3286

Conversation

TimQuelch commented Aug 18, 2024 • edited Loading

netlify bot commented Aug 18, 2024 • edited Loading

✅ Deploy Preview for actualbudget ready!

github-actions bot commented Aug 18, 2024 • edited Loading

Bundle Stats — desktop-client

github-actions bot commented Aug 18, 2024 • edited Loading

Bundle Stats — loot-core

StefanFabian commented Aug 21, 2024

alexyao2015 commented Aug 21, 2024 • edited Loading

TimQuelch commented Aug 22, 2024 • edited Loading

TimQuelch commented Aug 22, 2024

alexyao2015 commented Aug 22, 2024 • edited Loading

alexyao2015 commented Aug 22, 2024

StefanFabian commented Aug 22, 2024 • edited Loading

TimQuelch commented Aug 22, 2024 • edited Loading

jfdoming commented Aug 22, 2024

alexyao2015 commented Aug 22, 2024 • edited Loading

alexyao2015 commented Aug 22, 2024

TimQuelch commented Aug 23, 2024

alexyao2015 commented Aug 24, 2024

TimQuelch commented Sep 25, 2024

jfdoming commented Sep 25, 2024

alexyao2015 commented Sep 25, 2024

youngcw commented Sep 25, 2024

jfdoming commented Sep 25, 2024

youngcw commented Oct 28, 2024

StefanFabian commented Nov 5, 2024

alexyao2015 commented Nov 5, 2024

xxxarmitagexxx commented Nov 7, 2024

gcoremans commented Nov 20, 2024

xxxarmitagexxx commented Nov 21, 2024

TimQuelch commented Nov 21, 2024 • edited Loading

xxxarmitagexxx commented Nov 21, 2024

xxxarmitagexxx commented Nov 21, 2024

gcoremans commented Nov 21, 2024

TimQuelch commented Nov 21, 2024

paumove commented Nov 25, 2024

TimQuelch commented Nov 25, 2024

gcoremans commented Nov 25, 2024

alexyao2015 commented Nov 29, 2024

xxxarmitagexxx commented Nov 30, 2024

TimQuelch commented Aug 18, 2024 •

edited

Loading

netlify bot commented Aug 18, 2024 •

edited

Loading

github-actions bot commented Aug 18, 2024 •

edited

Loading

github-actions bot commented Aug 18, 2024 •

edited

Loading

alexyao2015 commented Aug 21, 2024 •

edited

Loading

TimQuelch commented Aug 22, 2024 •

edited

Loading

alexyao2015 commented Aug 22, 2024 •

edited

Loading

StefanFabian commented Aug 22, 2024 •

edited

Loading

TimQuelch commented Aug 22, 2024 •

edited

Loading

alexyao2015 commented Aug 22, 2024 •

edited

Loading

TimQuelch commented Nov 21, 2024 •

edited

Loading