This still needs to be debugged, I am getting this error now after these changes: (implying trustedProxies is coming through as true instead of the default array)

ValidationError: The Express 'trust proxy' setting is true, which allows anyone to trivially bypass IP-based rate limiting. See https://express-rate-limit.github.io/ERR_ERL_PERMISSIVE_TRUST_PROXY/ for more information.
    at _Validations.<anonymous> (file:///app/node_modules/express-rate-limit/dist/index.mjs:134:15)
    at _Validations.wrap (file:///app/node_modules/express-rate-limit/dist/index.mjs:287:18)
    at _Validations.trustProxy (file:///app/node_modules/express-rate-limit/dist/index.mjs:132:10)
    at Object.keyGenerator (file:///app/node_modules/express-rate-limit/dist/index.mjs:515:19)
    at file:///app/node_modules/express-rate-limit/dist/index.mjs:569:32
    at async file:///app/node_modules/express-rate-limit/dist/index.mjs:550:5 {
  code: 'ERR_ERL_PERMISSIVE_TRUST_PROXY',
  help: 'https://express-rate-limit.github.io/ERR_ERL_PERMISSIVE_TRUST_PROXY/'
}

@djm2k we probably shouldn't have called the old one trusted proxies, as it's trying to do a different thing than what setting express's (or nginx's) trusted proxies is doing.

Setting express's trusted proxies is a method of excluding proxies that are within your network, leaving what's left as the untrusted client IPs, which you can then rate limit and use to log as the from IP.

That's a good setting to have, and if you need it to fix the rate limiting error, then we should add it. But we still want a seperate config item that identifies which IP subnet we trust header auth requests from.

Imagine you have your hosted actual and you want to be forced to login when away from your home network, but be auto-logged in when at home, regardless of the device. You need your express trusted proxies to basically include nothing but localhost the two proxy IPs (the proxy that you are using to expose this to the world, and the one for inside your network) In this scenario it's because this is in your home network, and you need to identify any private ip in your network as a client rather than trusted for rate limiting and other client ip middleware).

But you want your tusted auth IP to match the proxy that you are using inside your network to do header auth. (which you are not exposing outside your network), and not the public exposed proxy.

@twk3 I am very confused by the different terms. What you're describing is known to me as "Trusted IP Ranges" or "IP Authentication". Handling this at the express middleware level instead of in the reverse proxy config seems like a mistake which could lead to future admins misconfiguring their setups.

My previous experience with header authentication has been to set a specific header to a secret value, and ensure that header was cleared from user's requests by the reverse proxy before being set.

This could be handled by validateAuthHeader where it would check the header is present and it's value matches the secret key set in the config.
Nevermind, looking at #312 again, this is implemented with x-actual-password

I'd like to setup an E2E test which covers your use-case as well. For me- the whole app is wrapped in Authelia no matter if i am accessing it locally or externally.

Let me know what you think

@twk3 I am very confused by the different terms. What you're describing is known to me as "Trusted IP Ranges" or "IP Authentication". Handling this at the express middleware level instead of in the reverse proxy config seems like a mistake which could lead to future admins misconfiguring their setups.

My previous experience with header authentication has been to set a specific header to a secret value, and ensure that header was cleared from user's requests by the reverse proxy before being set.

This could be handled by validateAuthHeader where it would check the header is present and it's value matches the secret key set in the config. Nevermind, looking at #312 again, this is implemented with x-actual-password

I'd like to setup an E2E test which covers your use-case as well. For me- the whole app is wrapped in Authelia no matter if i am accessing it locally or externally.

Let me know what you think

Yeah, sorry. That was my fault for using trusted proxy as the name. Should have been allowedAuthCIDRs or something like that. And it SHOULD only need to be set if someone wants to lock header auth to a specific endpoint.

To fix the naming situation, we probably need to rename the current impl to allowedAuthCIDRs, but populate it by default with whatever folks have set in their trustedProxies (to maintain support for the config syntax that we released). But primarily start using trustedProxies for what you have here. (we also need to fix the current implementation so it's not needed to be set if folks just want header auth, without restricting it)

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

Still WIP https://github.com/djm2k/actual-server/commits/add-e2e-test/

👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review.

This PR was closed because it has been stalled for 5 days with no activity.