diff --git a/CHANGELOG.md b/CHANGELOG.md index cd75a9c2..4428c521 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,11 +46,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Badges no longer lose their rounded shape in Safari after a long session. -- Turning Password Protection off now deletes the stored password, which - previously stayed behind and could leave an install unable to set a new one. +- Turning Password Protection off now deletes the stored password. + While protection is off, anyone who can reach the dashboard can set a new one. + +- A password typed in the same save that turns Password Protection off is no + longer stored and then immediately deleted, which signed out every other + device for nothing. + +- The sign-out row no longer stays on screen after Password Protection is + turned off, where pressing it returned an error. + +- The password and Unsplash key status labels are translated instead of always + reading in English. ### Security +- The Password Protection switch refuses a request that does not plainly say on + or off, instead of reading it as off and deleting the stored password. + - A Connections tile that cannot reach a service now says so in plain words instead of printing the address it tried, which put an internal host and port on the dashboard. diff --git a/api/src/routes/auth.js b/api/src/routes/auth.js index 839b2cff..9ea03f28 100644 --- a/api/src/routes/auth.js +++ b/api/src/routes/auth.js @@ -160,6 +160,13 @@ on('POST', '/api/auth/toggle', async (req, res) => { if (!checkOrigin(req, res)) return; try { const { enabled } = JSON.parse(await readBody(req)); + /* Only a real true or false. Turning protection off deletes the password + and cannot be undone from the UI, so a body that does not say so plainly, + from an old cached page or a script, must change nothing rather than + being read as "off". */ + if (typeof enabled !== 'boolean') { + return json(res, 400, { error: 'enabled must be true or false', kind: KIND.INVALID }); + } const cfg = loadConfig(); cfg.settings = cfg.settings || {}; cfg.settings.auth = cfg.settings.auth || {}; @@ -176,7 +183,14 @@ on('POST', '/api/auth/toggle', async (req, res) => { can be obtained while auth is off, and login is a bypass that issues no cookie, so a forgotten password could only be replaced by editing the config on disk. The secret goes too, so tokens signed under it cannot be - honoured if protection is turned back on later. */ + honoured if protection is turned back on later. + + The trade this makes: with no hash stored, set-password no longer demands + a session, so anyone who can reach the dashboard while protection is off + can set one and lock the owner out. That is accepted rather than guarded, + because protection being off already means anyone who can reach the + dashboard can read and rewrite the whole config; a guard here would only + look like security. It is why the toggle asks before clearing. */ const cleared = !enabled && !!cfg.settings.auth.passwordHash; if (!enabled) { delete cfg.settings.auth.passwordHash; diff --git a/api/test/auth-lockout.test.js b/api/test/auth-lockout.test.js index fb997856..d3000c05 100644 --- a/api/test/auth-lockout.test.js +++ b/api/test/auth-lockout.test.js @@ -239,3 +239,33 @@ test('login still refuses a wrong password when one is set', async () => { assert.equal((await req('POST', '/api/auth/login', { password: 'nope' })).status, 401); assert.equal((await req('POST', '/api/auth/login', { password: 'correct-horse' })).status, 200); }); + +/* ── the switch only answers to a real true or false ──────────────────────── */ + +/* Switching off deletes the password and the secret, and nothing in the UI can + undo it. A body that does not plainly say so, from a cached older page or a + script, used to be read as "off" and destroyed both. */ + +for (const [label, body] of [ + ['an empty body', {}], + ['a null flag', { enabled: null }], + ['a zero', { enabled: 0 }], + ['a string', { enabled: 'false' }], + ['the string "true"', { enabled: 'true' }], + ['a missing key with other fields', { enable: false }], +]) { + test(`${label} changes nothing rather than clearing the password`, async () => { + const cfg = loadConfig(); + const hash = await hashPassword('correct-horse'); + cfg.settings.auth = { enabled: true, secret: SECRET, passwordHash: hash }; + saveConfig(cfg); + + const r = await req('POST', '/api/auth/toggle', body, 'ds=' + makeToken('s1', SECRET)); + assert.equal(r.status, 400, 'refused'); + + const stored = loadConfig().settings.auth; + assert.equal(stored.passwordHash, hash, 'the password survives'); + assert.equal(stored.secret, SECRET, 'the secret survives'); + assert.equal(stored.enabled, true, 'the flag is untouched'); + }); +} diff --git a/ui/admin/index.html b/ui/admin/index.html index 262bdf8b..7b9b0f15 100644 --- a/ui/admin/index.html +++ b/ui/admin/index.html @@ -285,6 +285,6 @@

General

- + diff --git a/ui/i18n/de.json b/ui/i18n/de.json index 88e885bc..1dde1d58 100644 --- a/ui/i18n/de.json +++ b/ui/i18n/de.json @@ -23,7 +23,9 @@ "edit": "Bearbeiten", "color": "Farbe", "create": "Erstellen", - "chooseFiles": "Dateien wählen" + "chooseFiles": "Dateien wählen", + "configured": "Eingerichtet", + "notSet": "Nicht gesetzt" }, "general": { "title": "Titel", diff --git a/ui/i18n/en.json b/ui/i18n/en.json index 3ff9b8b6..0564bd99 100644 --- a/ui/i18n/en.json +++ b/ui/i18n/en.json @@ -23,7 +23,9 @@ "edit": "Edit", "color": "Color", "create": "Create", - "chooseFiles": "Choose files" + "chooseFiles": "Choose files", + "configured": "Configured", + "notSet": "Not set" }, "general": { "title": "Title", diff --git a/ui/i18n/es.json b/ui/i18n/es.json index cc8388ae..da10b4bc 100644 --- a/ui/i18n/es.json +++ b/ui/i18n/es.json @@ -23,7 +23,9 @@ "edit": "Editar", "color": "Color", "create": "Crear", - "chooseFiles": "Elegir archivos" + "chooseFiles": "Elegir archivos", + "configured": "Configurado", + "notSet": "Sin definir" }, "general": { "title": "Título", diff --git a/ui/i18n/fa.json b/ui/i18n/fa.json index 630ea8ca..d9427fac 100644 --- a/ui/i18n/fa.json +++ b/ui/i18n/fa.json @@ -23,7 +23,9 @@ "edit": "ویرایش", "color": "رنگ", "create": "ساختن", - "chooseFiles": "انتخاب فایل‌ها" + "chooseFiles": "انتخاب فایل‌ها", + "configured": "تنظیم‌شده", + "notSet": "تنظیم نشده" }, "general": { "title": "عنوان", diff --git a/ui/i18n/fr.json b/ui/i18n/fr.json index 20493492..74571b0d 100644 --- a/ui/i18n/fr.json +++ b/ui/i18n/fr.json @@ -23,7 +23,9 @@ "edit": "Modifier", "color": "Couleur", "create": "Créer", - "chooseFiles": "Choisir des fichiers" + "chooseFiles": "Choisir des fichiers", + "configured": "Définie", + "notSet": "Non définie" }, "general": { "title": "Titre", diff --git a/ui/i18n/zh-Hans.json b/ui/i18n/zh-Hans.json index 4d6a2e14..bc1bcdec 100644 --- a/ui/i18n/zh-Hans.json +++ b/ui/i18n/zh-Hans.json @@ -23,7 +23,9 @@ "edit": "编辑", "color": "颜色", "create": "创建", - "chooseFiles": "选择文件" + "chooseFiles": "选择文件", + "configured": "已设置", + "notSet": "未设置" }, "general": { "title": "标题", diff --git a/ui/js/admin-logic.js b/ui/js/admin-logic.js index 64f827da..549154a2 100644 --- a/ui/js/admin-logic.js +++ b/ui/js/admin-logic.js @@ -118,6 +118,14 @@ export function authEnableBlocked({ enabled, passwordSet, newPassword }) { return !!enabled && !passwordSet && !(newPassword || '').length; } +/* A password typed in the same save that switches protection off is not stored. + Writing it would rotate the session secret and sign every other device out, + and the toggle in the same save would delete it again a moment later, so the + round trip costs the user their other sessions and leaves nothing behind. */ +export function shouldWritePassword({ enabled, newPassword }) { + return !!enabled && !!(newPassword || '').length; +} + /* 'registry' renders the manifest's fields, 'custom' the URL editor, and 'unavailable' a registry widget whose manifest is not loaded. The last must not fall through to the custom editor: the server withholds that widget's config, diff --git a/ui/js/admin-settings.js b/ui/js/admin-settings.js index 91ffaccd..53a2ed67 100644 --- a/ui/js/admin-settings.js +++ b/ui/js/admin-settings.js @@ -1,15 +1,29 @@ /* Admin UI: the General, Appearance and Security settings sections. */ import { toast, ag, ap } from '/js/admin-shared.js?v=182410cc'; -import { wirePasswordStrength } from '/js/admin-auth.js?v=dd849d4c'; import { pwStrength } from '/js/password-strength.js?v=dab9978e'; import { t } from '/js/i18n.js?v=133a7aac'; -import { authEnableBlocked } from '/js/admin-logic.js?v=056a11e9'; +import { authEnableBlocked, shouldWritePassword } from '/js/admin-logic.js?v=3c60f7b0'; import { el, inp, q, qa, setUserText } from '/js/utils.js?v=17424946'; /* Mirrors the server's rule: auth cannot be switched on with no password behind it. */ let _passwordSet = false; +/* Both controls only mean anything while auth is on; the revoke one also needs + a password to exist, since that is what makes a session possible. + + Module scope rather than a closure, because the three moments that change + what it reflects are in three different places: the check on load, the + toggle, and the save that clears a stored password. */ +function syncSessionRows() { + const on = !!inp('sec-en')?.checked; + el('sec-logout')?.classList.toggle('d-none', !on); + const canRevoke = on && _passwordSet; + el('sec-revoke-row')?.classList.toggle('d-none', !canRevoke); + const revokeTip = el('revoke-tip'); + if (revokeTip) revokeTip.style.display = canRevoke ? '' : 'none'; +} + export function loadSettings(c) { const s = c.settings || {}; const ld = inp('set-lbl-d'); @@ -73,9 +87,9 @@ export function loadSettings(c) { const vEl = el('ie-apikey-v'); if (!d.configured) { apiEl.placeholder = 'Paste your Unsplash API key'; - if (vEl) vEl.textContent = 'Not set'; + if (vEl) vEl.textContent = t('common.notSet'); } else { - if (vEl) vEl.textContent = 'Configured'; + if (vEl) vEl.textContent = t('common.configured'); } }) .catch(() => {}); @@ -127,7 +141,7 @@ export function loadSettings(c) { ); _sv('ie-ip-v', s.server?.hostIp, '192.168.1.100'); _sv('ie-socket-v', s.server?.socketProxyUrl, 'http://socket-proxy:2375'); - _sv('ie-pw-v', '', 'Not set'); /* set below after auth check */ + _sv('ie-pw-v', '', t('common.notSet')); /* set below after auth check */ const _si = (id, v) => { const node = inp(id); if (node && v != null) node.value = v; @@ -166,31 +180,8 @@ export function loadSettings(c) { el('srv-save').addEventListener('click', saveServer); const secEnEl = inp('sec-en'); - const secSubEl = el('sec-sub'); - const secPwEl = el('sec-pw'); - let pwStrengthWired = false; - function openSecSub() { - secSubEl.classList.add('open'); - /* Wire strength meter on first open, avoids Safari input event bug - where listeners on password fields in hidden containers don't fire */ - if (!pwStrengthWired && secPwEl) { - pwStrengthWired = true; - wirePasswordStrength('sec-pw', 'sec-pw-bars', 'sec-pw-hint'); - } - } const secLogout = el('sec-logout'); const secRevoke = inp('sec-revoke'); - const secRevokeRow = el('sec-revoke-row'); - const revokeTip = el('revoke-tip'); - /* Both controls only mean anything while auth is on; the revoke one also needs - a password to exist, since that is what makes a session possible. */ - const syncLogout = () => { - const on = !!secEnEl?.checked; - if (secLogout) secLogout.classList.toggle('d-none', !on); - const canRevoke = on && _passwordSet; - if (secRevokeRow) secRevokeRow.classList.toggle('d-none', !canRevoke); - if (revokeTip) revokeTip.style.display = canRevoke ? '' : 'none'; - }; secLogout?.addEventListener('click', async () => { await ap('/api/auth/logout', {}).catch(() => {}); location.reload(); @@ -209,13 +200,7 @@ export function loadSettings(c) { secRevoke.disabled = false; } }); - if (secEnEl && secSubEl) { - secEnEl.addEventListener('change', () => { - if (secEnEl.checked) openSecSub(); - else secSubEl.classList.remove('open'); - syncLogout(); - }); - } + secEnEl?.addEventListener('change', syncSessionRows); ag('/api/auth/check') .then(d => { @@ -230,8 +215,8 @@ export function loadSettings(c) { if (pwHint) pwHint.style.display = d.enabled ? '' : 'none'; } const pwValEl = el('ie-pw-v'); - if (pwValEl) pwValEl.textContent = d.passwordSet ? 'Configured' : 'Not set'; - syncLogout(); + if (pwValEl) pwValEl.textContent = d.passwordSet ? t('common.configured') : t('common.notSet'); + syncSessionRows(); }) .catch(() => {}); } @@ -316,7 +301,7 @@ async function saveServer() { toast(t('toast.authNeedsPassword'), 'err'); return; } - if (pw) { + if (shouldWritePassword({ enabled, newPassword: pw })) { const { ok, labelKey } = pwStrength(pw); if (!ok) { toast(t('toast.pwWeak', { label: t(labelKey) }), 'err'); @@ -336,9 +321,17 @@ async function saveServer() { claiming one is stored: re-enabling in this session needs a new one. */ _passwordSet = false; const pwValEl = el('ie-pw-v'); - if (pwValEl) pwValEl.textContent = 'Not set'; + if (pwValEl) pwValEl.textContent = t('common.notSet'); const pwEl = inp('sec-pw'); - if (pwEl) pwEl.placeholder = ''; + if (pwEl) { + pwEl.placeholder = ''; + /* Whatever was typed was not stored, so leaving it in the box would + read as a password this dashboard now has. */ + pwEl.value = ''; + } + /* The revoke row offers to end sessions that no longer exist, and the + server answers it with an error. */ + syncSessionRows(); } toast(t('toast.saved')); if (langChanged) location.reload(); diff --git a/ui/js/admin.js b/ui/js/admin.js index 97f25bf3..53d9ffc9 100644 --- a/ui/js/admin.js +++ b/ui/js/admin.js @@ -1,7 +1,7 @@ import { loadLocalIcons, resolveIcon, iconChain } from '/js/icons.js?v=a0ea3e4b'; import { clr as rc, sanitizeCssUrl, el, inp, q, qa, tgt, setUserText } from '/js/utils.js?v=17424946'; import { html, raw, setHtml } from '/js/html.js?v=ccec347c'; -import { reorderItems, resolveAdminSection } from '/js/admin-logic.js?v=056a11e9'; +import { reorderItems, resolveAdminSection } from '/js/admin-logic.js?v=3c60f7b0'; import { newItemId, buildAppItem, upsertItem, claimFolderChildren } from '/js/admin-save-logic.js?v=77cac1d1'; import { API, toast, ag, ap, initInlineEdit } from '/js/admin-shared.js?v=182410cc'; import { checkAuth, wirePasswordStrength } from '/js/admin-auth.js?v=dd849d4c'; @@ -9,7 +9,7 @@ import { state } from '/js/admin-state.js?v=3f9ad806'; import { buildWidgetForm } from '/js/admin-widget-form.js?v=653071f0'; import { buildAppForm, buildFolderForm, serializeKvRows } from '/js/admin-app-form.js?v=b46e5b19'; import { LANGUAGES, initI18n, t } from '/js/i18n.js?v=133a7aac'; -import { loadSettings, showBgFields } from '/js/admin-settings.js?v=c2d82934'; +import { loadSettings, showBgFields } from '/js/admin-settings.js?v=536793f6'; import { canJoinFolder, applyDrop } from '/js/admin-drag-logic.js?v=53aeaa55'; /* openModal is the item editor in this file, so the shared dialog comes in under a different name. */ diff --git a/ui/test/admin-logic.test.mjs b/ui/test/admin-logic.test.mjs index b712b0f5..ee04beae 100644 --- a/ui/test/admin-logic.test.mjs +++ b/ui/test/admin-logic.test.mjs @@ -8,6 +8,7 @@ import { visibleFieldKeys, clearsStoredSecret, authEnableBlocked, + shouldWritePassword, widgetConfigMode, resolveAdminSection, rejectionLines, @@ -370,3 +371,25 @@ test('the notice key follows the count', () => { assert.equal(refusedNoticeKey(2), 'widgetCfg.refusedPlural'); assert.equal(refusedNoticeKey(0), 'widgetCfg.refusedPlural'); }); + +/* ── shouldWritePassword ──────────────────────────────────────────────────── */ + +/* Typing a password and switching protection off in the same save is a + contradiction. Storing it first signed every other device out, because + set-password rotates the session secret, and the toggle in the same save then + deleted the password again. The round trip cost the user their sessions and + left nothing behind, and the save reported success. */ + +test('a password typed while switching protection off is not written', () => { + assert.equal(shouldWritePassword({ enabled: false, newPassword: 'correct-horse' }), false); +}); + +test('a password typed while switching protection on is written', () => { + assert.equal(shouldWritePassword({ enabled: true, newPassword: 'correct-horse' }), true); +}); + +test('nothing is written when no password was typed', () => { + assert.equal(shouldWritePassword({ enabled: true, newPassword: '' }), false); + assert.equal(shouldWritePassword({ enabled: true, newPassword: undefined }), false); + assert.equal(shouldWritePassword({ enabled: false, newPassword: '' }), false); +});