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..ee1bee0dc --- /dev/null +++ b/test/cypress/e2e/api/AccessListHostToggle.cy.js @@ -0,0 +1,210 @@ +/// + +// 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); + }); + + // 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}`, + }).then((data) => { + cy.validateSwaggerSchema('delete', 200, '/nginx/access-lists/{listID}', data); + expect(data).to.be.equal(true); + }); + }); + }); + +}); 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 }), }); }; 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;