diff --git a/api/src/host-check.js b/api/src/host-check.js index 89d2f480..ec84a57f 100644 --- a/api/src/host-check.js +++ b/api/src/host-check.js @@ -1,39 +1,21 @@ -const { loadConfig, loadConfigForUpdate, saveConfig } = require('./config'); +const { loadConfig } = require('./config'); const { authActive } = require('./auth'); const { IS_DEMO } = require('./demo'); -const log = require('./log'); const { hostnameOf, isLocalAddress } = require('../../ui/js/host-names.js'); /* While no password is set, answer only addresses a page on another site - cannot point at this server (DNS rebinding). The first page load sets the - list when none is stored. An IP address sets an empty one. */ + cannot point at this server (DNS rebinding). Never add a name to the list + from a request: a rebound page can send that request first. */ /** @returns {string | null} the refused host name, or null when allowed */ -function refusedHost(req, pathname) { +function refusedHost(req) { if (IS_DEMO) return null; const cfg = loadConfig(); if (authActive(cfg)) return null; const host = hostnameOf(req.headers.host); if (!host) return String(req.headers.host || ''); const list = cfg.settings?.server?.allowedHosts; - const local = isLocalAddress(host); - if (!Array.isArray(list) && req.method === 'GET' && pathname === '/api/auth/check') { - trustFirst(local ? [] : [host], host); - return null; - } - return local || (Array.isArray(list) && list.includes(host)) ? null : host; -} - -function trustFirst(list, host) { - try { - const cfg = loadConfigForUpdate(); - if (Array.isArray(cfg.settings?.server?.allowedHosts)) return; - cfg.settings.server = { ...cfg.settings.server, allowedHosts: list }; - saveConfig(cfg); - log.audit('allowed addresses set from the first visit', { host, allowed: list.join(',') || 'local only' }); - } catch (e) { - log.warn('could not store the allowed addresses', { error: e.message }); - } + return isLocalAddress(host) || (Array.isArray(list) && list.includes(host)) ? null : host; } module.exports = { refusedHost }; diff --git a/api/src/router.js b/api/src/router.js index c71f34ff..e0d15b6d 100644 --- a/api/src/router.js +++ b/api/src/router.js @@ -52,7 +52,7 @@ function route(req, res) { } if (u.pathname !== '/health') { - const host = refusedHost(req, u.pathname); + const host = refusedHost(req); if (host !== null) { return json(res, 403, { error: 'Forbidden: this address is not allowed while no password is set', diff --git a/api/test/host-check.test.js b/api/test/host-check.test.js index 52ae5e51..56bc397a 100644 --- a/api/test/host-check.test.js +++ b/api/test/host-check.test.js @@ -86,20 +86,28 @@ test('a rebound page cannot read or replace the config', async () => { assert.equal(loadConfig().items.length, 1); }); -test('the first page load by name trusts that name and no other', async () => { - const first = await request('GET', '/api/auth/check', 'Dash.Example.com:8700'); - assert.equal(first.status, 200); - assert.deepEqual(stored(), ['dash.example.com']); +test('a rebound page that loads first is refused and stores nothing', async () => { + const first = await request('GET', '/api/auth/check', 'evil.example:8700'); + assert.equal(first.status, 403); + assert.equal(first.body.code, 'blocked.host'); + assert.equal(fs.existsSync(CFG), false); expireCache(); - assert.equal((await request('GET', '/api/config', 'dash.example.com')).status, 200); assert.equal((await request('GET', '/api/auth/check', 'evil.example')).status, 403); }); -test('the first page load by IP address trusts no name', async () => { +test('a page load by IP address trusts no name, and a name added there is answered', async () => { assert.equal((await request('GET', '/api/auth/check', '192.168.1.10:8700')).status, 200); - assert.deepEqual(stored(), []); + assert.equal(fs.existsSync(CFG), false); expireCache(); assert.equal((await request('GET', '/api/auth/check', 'dash.example.com')).status, 403); + const add = await request('POST', '/api/config', '192.168.1.10', { + items: [], + settings: { server: { allowedHosts: ['dash.example.com'] } }, + }); + assert.equal(add.status, 200); + expireCache(); + assert.equal((await request('GET', '/api/auth/check', 'dash.example.com')).status, 200); + assert.equal((await request('GET', '/api/auth/check', 'evil.example')).status, 403); }); test('local addresses are always answered', async () => { diff --git a/ui/index.html b/ui/index.html index e756e45d..fdc7beea 100644 --- a/ui/index.html +++ b/ui/index.html @@ -56,5 +56,5 @@ - + diff --git a/ui/js/dashboard.js b/ui/js/dashboard.js index 22ef102c..75eaa80f 100644 --- a/ui/js/dashboard.js +++ b/ui/js/dashboard.js @@ -746,18 +746,16 @@ async function boot() { v => ({ v }), e => ({ e }), ); - const loadConfig = () => - settled(fetch('/api/config', { cache: 'no-store', signal: AbortSignal.timeout(BOOT_TIMEOUT_MS) })); - let configReq = loadConfig(); + const configReq = settled(fetch('/api/config', { cache: 'no-store', signal: AbortSignal.timeout(BOOT_TIMEOUT_MS) })); const loadWidgets = () => settled( fetch('/api/widgets', { cache: 'no-store' }).then(r => { - if (!r.ok) throw Object.assign(new Error(`HTTP ${r.status}`), { status: r.status }); + if (!r.ok) throw new Error(`HTTP ${r.status}`); return r.json(); }), ); let widgetsReq = loadWidgets(); - let iconsReq = loadLocalIcons(); + const iconsReq = loadLocalIcons(); let authData = null; try { @@ -779,11 +777,6 @@ async function boot() { window.location.href = '/admin/'; return; } - /* Only the sign-in check stores the first address. Requests sent before it - on a first visit by host name are refused. */ - if ((await configReq).v?.status === 403) configReq = loadConfig(); - if ((await widgetsReq).e?.status === 403) widgetsReq = loadWidgets(); - if ((await iconsReq) === 403) iconsReq = loadLocalIcons(); } catch { /* API down, handled below */ } diff --git a/ui/test/boot-requests.test.mjs b/ui/test/boot-requests.test.mjs index 51560c24..62e06e4f 100644 --- a/ui/test/boot-requests.test.mjs +++ b/ui/test/boot-requests.test.mjs @@ -59,69 +59,6 @@ test('a widget list refused while the first-run password was set is requested ag assert.match(prompt.slice(0, 200), /if \(\(await widgetsReq\)\.e\) widgetsReq = loadWidgets\(\);/); }); -const firstVisitOrders = [ - ['/api/config', '/api/widgets', '/api/icons/local'], - ['/api/widgets'], - ['/api/icons/local'], - ['/api/config'], -]; - -for (const early of firstVisitOrders) { - test(`a first visit by host name asks again for what was refused: ${early.join(', ')}`, async () => { - const start = dashboard.indexOf('async function boot()'); - const src = dashboard.slice(start, dashboard.indexOf('\n}\n', start) + 2); - let trusted = false; - const sent = []; - const reply = (status, body) => ({ ok: status < 300, status, json: async () => body }); - const fetch = async url => { - sent.push(url); - const wait = url === '/api/auth/check' ? 5 : early.includes(url) ? 0 : 10; - await new Promise(r => setTimeout(r, wait)); - if (url === '/api/auth/check') { - trusted = true; - return reply(200, { enabled: false, passwordSet: false, setupPrompted: false }); - } - if (!trusted) return reply(403, { kind: 'blocked', code: 'blocked.host' }); - return reply(200, url === '/api/config' ? { items: [], settings: {}, _rev: 1 } : { widgets: [], files: [] }); - }; - const shown = []; - const stop = new Error('reached the first-run prompt'); - const boot = new Function( - 'fetch, loadLocalIcons, document, initI18n, sanitizeItemLinks, showSetupPrompt, setHtml, html, t, BOOT_TIMEOUT_MS, blockingScreenFor, showBlockingScreen, console', - `let items, S, _rev, widgetReg; ${src} return boot;`, - )( - fetch, - async () => (await fetch('/api/icons/local')).status, - { - body: { appendChild: el => shown.push(el.className), classList: { add() {} } }, - createElement: () => ({ querySelector: () => null }), - }, - async () => {}, - x => x, - async () => { - throw stop; - }, - () => {}, - () => '', - k => k, - 1000, - () => null, - async () => {}, - { error() {} }, - ); - await assert.rejects(boot(), stop); - assert.deepEqual(shown, [], 'the API-down screen was shown'); - for (const url of ['/api/config', '/api/widgets', '/api/icons/local']) { - const times = early.includes(url) ? 2 : 1; - assert.equal( - sent.filter(u => u === url).length, - times, - times === 2 ? `${url} not sent again after a refusal` : `${url} sent again without a refusal`, - ); - } - }); -} - for (const failures of [1, 2]) { test(`a widget list that fails ${failures === 1 ? 'once is asked for again' : 'twice shows the API-down screen'}`, async () => { const start = dashboard.indexOf('async function boot()');