From 2afd723cd8ab4b0b3f7a45cb6230c6b42b09cb02 Mon Sep 17 00:00:00 2001 From: Jamie Curnow Date: Mon, 5 Oct 2026 21:59:46 +1000 Subject: [PATCH 1/2] Fix access list rules and SSL missing from re-rendered host configs Some paths rendered host configs without fully loading the relations the templates depend on, silently dropping access rules (and SSL): - Enabling a proxy host loaded the access list without its clients or auth items, so the host came back with no access restrictions. - Requesting a Let's Encrypt certificate re-rendered every host sharing its domains from bare rows, dropping access rules and SSL. generateConfig now loads any referenced certificate or access list that isn't fully loaded, so no caller can render a partial config. The two callers above also fetch the relations themselves, and a warning is logged when a host's access list is missing or has no rules. Adds integration tests covering access lists across disable/enable. Fixes #5919, #4286, #5710 Co-Authored-By: Claude Opus 5.5 --- backend/internal/host.js | 9 +- backend/internal/nginx.js | 52 ++++- backend/internal/proxy-host.js | 2 +- docker/docker-compose.ci.yml | 1 + docker/docker-compose.dev.yml | 1 + .../e2e/api/AccessListHostToggle.cy.js | 206 ++++++++++++++++++ test/docker/nginx/conf.d/website123.conf | 2 +- 7 files changed, 267 insertions(+), 6 deletions(-) create mode 100644 test/cypress/e2e/api/AccessListHostToggle.cy.js diff --git a/backend/internal/host.js b/backend/internal/host.js index 748716262..81efe160e 100644 --- a/backend/internal/host.js +++ b/backend/internal/host.js @@ -79,15 +79,18 @@ const internalHost = { redirection_hosts: [], }; - const proxyRes = await proxyHostModel.query().where("is_deleted", 0); + const proxyRes = await proxyHostModel + .query() + .where("is_deleted", 0) + .withGraphFetched("[certificate, access_list.[clients,items]]"); responseObject.proxy_hosts = internalHost._getHostsWithDomains(proxyRes, domainNames); responseObject.total_count += responseObject.proxy_hosts.length; - const redirRes = await redirectionHostModel.query().where("is_deleted", 0); + const redirRes = await redirectionHostModel.query().where("is_deleted", 0).withGraphFetched("[certificate]"); responseObject.redirection_hosts = internalHost._getHostsWithDomains(redirRes, domainNames); responseObject.total_count += responseObject.redirection_hosts.length; - const deadRes = await deadHostModel.query().where("is_deleted", 0); + const deadRes = await deadHostModel.query().where("is_deleted", 0).withGraphFetched("[certificate]"); responseObject.dead_hosts = internalHost._getHostsWithDomains(deadRes, domainNames); responseObject.total_count += responseObject.dead_hosts.length; diff --git a/backend/internal/nginx.js b/backend/internal/nginx.js index a50e0780d..378b53591 100644 --- a/backend/internal/nginx.js +++ b/backend/internal/nginx.js @@ -7,6 +7,7 @@ import errs from "../lib/error.js"; import utils from "../lib/utils.js"; import { debug, nginx as logger } from "../logger.js"; import accessListModel from "../models/access_list.js"; +import certificateModel from "../models/certificate.js"; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); @@ -209,11 +210,13 @@ const internalNginx = { * @param {Object} host * @returns {Promise} */ - generateConfig: (host_type, host_row) => { + generateConfig: async (host_type, host_row) => { // Prevent modifying the original object: const host = JSON.parse(JSON.stringify(host_row)); const nice_host_type = internalNginx.getFileFriendlyHostType(host_type); + await internalNginx.loadRenderRelations(nice_host_type, host); + debug(logger, `Generating ${nice_host_type} Config:`, JSON.stringify(host, null, 2)); const renderEngine = utils.getRenderEngine(); @@ -299,6 +302,53 @@ const internalNginx = { }); }, + /** + * The templates rely on the certificate and access list relations being loaded. + * Not every caller fetches them (or fetches them fully), and when they're missing + * the config silently renders without SSL or without any access rules (#5919). + * Load anything that is referenced but not loaded onto the given host object. + * + * @param {String} host_type + * @param {Object} host + * @returns {Promise} + */ + loadRenderRelations: async (host_type, host) => { + if (host.certificate_id > 0 && host.certificate?.id !== host.certificate_id) { + const certificate = await certificateModel + .query() + .where("is_deleted", 0) + .andWhere("id", host.certificate_id) + .first(); + host.certificate = certificate ? JSON.parse(JSON.stringify(certificate)) : null; + } + + if (host_type === "proxy_host" && host.access_list_id > 0) { + if ( + host.access_list?.id !== host.access_list_id || + !Array.isArray(host.access_list.clients) || + !Array.isArray(host.access_list.items) + ) { + const accessList = await accessListModel + .query() + .where("is_deleted", 0) + .andWhere("id", host.access_list_id) + .withGraphFetched("[clients,items]") + .first(); + host.access_list = accessList ? JSON.parse(JSON.stringify(accessList)) : null; + } + + if (!host.access_list) { + logger.warn( + `Proxy Host #${host.id} references Access List #${host.access_list_id} which does not exist`, + ); + } else if (!host.access_list.clients.length && !host.access_list.items.length) { + logger.warn( + `Proxy Host #${host.id} uses Access List #${host.access_list_id} which has no clients or authorizations, no access restrictions will apply`, + ); + } + } + }, + /** * This generates a temporary nginx config listening on port 80 for the domain names listed * in the certificate setup. It allows the letsencrypt acme challenge to be requested by letsencrypt diff --git a/backend/internal/proxy-host.js b/backend/internal/proxy-host.js index eac0c148a..f079dab02 100644 --- a/backend/internal/proxy-host.js +++ b/backend/internal/proxy-host.js @@ -323,7 +323,7 @@ const internalProxyHost = { .then(() => { return internalProxyHost.get(access, { id: data.id, - expand: ["certificate", "owner", "access_list"], + expand: ["certificate", "owner", "access_list.[clients,items]"], }); }) .then(async (row) => { diff --git a/docker/docker-compose.ci.yml b/docker/docker-compose.ci.yml index be6f517d3..2aef4e8ff 100644 --- a/docker/docker-compose.ci.yml +++ b/docker/docker-compose.ci.yml @@ -37,6 +37,7 @@ services: - website1.example.com - website2.example.com - website3.example.com + - website4.example.com examplesite: image: "${IMAGE}-examplesite:ci-${BUILD_NUMBER}" diff --git a/docker/docker-compose.dev.yml b/docker/docker-compose.dev.yml index d6b07ec01..20e01e6be 100644 --- a/docker/docker-compose.dev.yml +++ b/docker/docker-compose.dev.yml @@ -16,6 +16,7 @@ services: - website1.example.com - website2.example.com - website3.example.com + - website4.example.com environment: TZ: "${TZ:-Australia/Brisbane}" PUID: 1000 diff --git a/test/cypress/e2e/api/AccessListHostToggle.cy.js b/test/cypress/e2e/api/AccessListHostToggle.cy.js new file mode 100644 index 000000000..b566a3a98 --- /dev/null +++ b/test/cypress/e2e/api/AccessListHostToggle.cy.js @@ -0,0 +1,206 @@ +/// + +// Regression tests for #5919, #4286 and #5710: access list rules must survive +// every path that re-renders a proxy host config, not just the initial save. +describe('Access Lists across Proxy Host disable/enable', () => { + const domain = 'website4.example.com'; + + const creds = { + username: 'toggle-user', + password: 'toggle-pass', + }; + + let token; + let authListId; + let clientListId; + let hostId; + + /** + * Requests a path on the proxy host directly (bypassing squid) + * and yields the HTTP status code + * + * @param {object} [auth] + * @param {string} auth.username + * @param {string} auth.password + */ + const request = (auth) => { + const authArg = auth ? `-u '${auth.username}:${auth.password}'` : ''; + return cy.exec(`curl --noproxy '*' -s -o /dev/null -w '%{http_code}' ${authArg} http://${domain}/`) + .then((result) => { + expect(result.exitCode).to.eq(0); + return result.stdout.trim(); + }); + }; + + // nginx reloads are signalled and return immediately, so the old + // config can still be served for a moment after an API change. + // Retry until the expected status is seen. + const expectStatus = (auth, status) => { + let last = null; + cy.waitUntil(() => request(auth).then((res) => { + last = res; + return res === status; + }), { + timeout: 15000, + interval: 500, + errorMsg: () => `${domain} did not return ${status}, last status: ${last}`, + }); + }; + + const createAccessList = (data) => { + return cy.task('backendApiPost', { + token: token, + path: '/api/nginx/access-lists', + data: { + satisfy_any: false, + pass_auth: false, + items: [], + clients: [], + ...data, + } + }).then((res) => { + cy.validateSwaggerSchema('post', 201, '/nginx/access-lists', res); + expect(res).to.have.property('id'); + expect(res.id).to.be.greaterThan(0); + return cy.wrap(res.id); + }); + }; + + const setHostAccessList = (accessListId) => { + cy.task('backendApiPut', { + token: token, + path: `/api/nginx/proxy-hosts/${hostId}`, + data: { + access_list_id: accessListId, + } + }).then((data) => { + // No swagger validation here: with a host access list set, the expanded + // access_list in the response has no proxy_host_count, which the schema requires + expect(data).to.have.property('access_list_id', accessListId); + }); + }; + + const toggleHost = () => { + cy.task('backendApiPost', { + token: token, + path: `/api/nginx/proxy-hosts/${hostId}/disable`, + }).then((data) => { + cy.validateSwaggerSchema('post', 200, '/nginx/proxy-hosts/{hostID}/disable', data); + expect(data).to.be.equal(true); + }); + + cy.task('backendApiPost', { + token: token, + path: `/api/nginx/proxy-hosts/${hostId}/enable`, + }).then((data) => { + cy.validateSwaggerSchema('post', 200, '/nginx/proxy-hosts/{hostID}/enable', data); + expect(data).to.be.equal(true); + }); + }; + + before(() => { + cy.resetUsers(); + cy.getToken().then((tok) => { + token = tok; + }); + }); + + it('Should be able to create an auth access list and a client access list', () => { + createAccessList({ + name: 'Toggle Auth', + items: [creds], + }).then((id) => { + authListId = id; + }); + + // The test runner is never in this range, so it must always be denied + createAccessList({ + name: 'Toggle Clients', + clients: [ + { + directive: 'allow', + address: '203.0.113.0/24', + } + ], + }).then((id) => { + clientListId = id; + }); + }); + + it('Should be able to create a proxy host without an access list', () => { + cy.task('backendApiPost', { + token: token, + path: '/api/nginx/proxy-hosts', + data: { + domain_names: [domain], + forward_scheme: 'http', + forward_host: 'examplesite', + forward_port: 80, + access_list_id: 0, + certificate_id: 0, + meta: { + dns_challenge: false + }, + advanced_config: '', + locations: [], + block_exploits: false, + caching_enabled: false, + allow_websocket_upgrade: false, + http2_support: false, + hsts_enabled: false, + hsts_subdomains: false, + ssl_forced: false + } + }).then((data) => { + cy.validateSwaggerSchema('post', 201, '/nginx/proxy-hosts', data); + expect(data).to.have.property('id'); + expect(data.id).to.be.greaterThan(0); + hostId = data.id; + }); + + expectStatus(null, '200'); + }); + + it('Should enforce an auth access list applied to an existing host', () => { + setHostAccessList(authListId); + expectStatus(null, '401'); + expectStatus(creds, '200'); + }); + + it('Should still enforce the auth access list after the host is disabled and enabled', () => { + toggleHost(); + expectStatus(null, '401'); + expectStatus(creds, '200'); + }); + + it('Should enforce a client access list applied to an existing host', () => { + setHostAccessList(clientListId); + expectStatus(null, '403'); + }); + + it('Should still enforce the client access list after the host is disabled and enabled', () => { + toggleHost(); + expectStatus(null, '403'); + }); + + it('Should be able to delete the proxy host and access lists', () => { + cy.task('backendApiDelete', { + token: token, + path: `/api/nginx/proxy-hosts/${hostId}`, + }).then((data) => { + cy.validateSwaggerSchema('delete', 200, '/nginx/proxy-hosts/{hostID}', data); + expect(data).to.be.equal(true); + }); + + [authListId, clientListId].forEach((id) => { + cy.task('backendApiDelete', { + token: token, + path: `/api/nginx/access-lists/${id}`, + }).then((data) => { + cy.validateSwaggerSchema('delete', 200, '/nginx/access-lists/{listID}', data); + expect(data).to.be.equal(true); + }); + }); + }); + +}); diff --git a/test/docker/nginx/conf.d/website123.conf b/test/docker/nginx/conf.d/website123.conf index 656f3ade4..eefb233bc 100644 --- a/test/docker/nginx/conf.d/website123.conf +++ b/test/docker/nginx/conf.d/website123.conf @@ -1,6 +1,6 @@ server { listen 80; - server_name website1.example.com website2.example.com website3.example.com; + server_name website1.example.com website2.example.com website3.example.com website4.example.com; root /www; index index.html; From 717877db3feea710eb45b3dddc7da342b9d61d17 Mon Sep 17 00:00:00 2001 From: Jamie Curnow Date: Fri, 9 Oct 2026 21:21:13 +1000 Subject: [PATCH 2/2] Fix flakey cypress test --- test/cypress/e2e/api/AccessListHostToggle.cy.js | 4 ++++ test/cypress/plugins/backendApi/client.mjs | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/test/cypress/e2e/api/AccessListHostToggle.cy.js b/test/cypress/e2e/api/AccessListHostToggle.cy.js index b566a3a98..ee1bee0dc 100644 --- a/test/cypress/e2e/api/AccessListHostToggle.cy.js +++ b/test/cypress/e2e/api/AccessListHostToggle.cy.js @@ -192,7 +192,11 @@ describe('Access Lists across Proxy Host disable/enable', () => { expect(data).to.be.equal(true); }); + // Each delete signals an nginx reload and returns immediately. A request + // made while the workers are being swapped can be reset by the nginx + // serving the API (502 from squid), so let each reload settle first. [authListId, clientListId].forEach((id) => { + cy.wait(1500); cy.task('backendApiDelete', { token: token, path: `/api/nginx/access-lists/${id}`, diff --git a/test/cypress/plugins/backendApi/client.mjs b/test/cypress/plugins/backendApi/client.mjs index 78618abb2..5c0daa5c0 100644 --- a/test/cypress/plugins/backendApi/client.mjs +++ b/test/cypress/plugins/backendApi/client.mjs @@ -1,3 +1,4 @@ +import http from "node:http"; import axios from "axios"; import logger from "./logger.mjs"; @@ -8,6 +9,10 @@ const BackendApi = function (config, token) { this.axios = axios.create({ baseURL: config.baseUrl, timeout: 90000, + // Don't reuse sockets: a pooled keep-alive connection that squid or the + // backend has since closed fails with a 502 (ERR_READ_ERROR 104), which + // made tests flaky. + httpAgent: new http.Agent({ keepAlive: false }), }); };