Fix: stop a password being stored and deleted in one save

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.

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;

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');
+  });
+}

ui/admin/index.html

@@ -285,6 +285,6 @@ <h1 class="sec-title" id="ev-title">General</h1>
   </div></div>
 </div>
 
-<script type="module" src="/js/admin.js?v=453c0383"></script>
+<script type="module" src="/js/admin.js?v=23b16aa6"></script>
 </body>
 </html>

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",

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",

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",

ui/i18n/fa.json

@@ -23,7 +23,9 @@
     "edit": "ویرایش",
     "color": "رنگ",
     "create": "ساختن",
-    "chooseFiles": "انتخاب فایل‌ها"
+    "chooseFiles": "انتخاب فایل‌ها",
+    "configured": "تنظیم‌شده",
+    "notSet": "تنظیم نشده"
   },
   "general": {
     "title": "عنوان",

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",

ui/i18n/zh-Hans.json

@@ -23,7 +23,9 @@
     "edit": "编辑",
     "color": "颜色",
     "create": "创建",
-    "chooseFiles": "选择文件"
+    "chooseFiles": "选择文件",
+    "configured": "已设置",
+    "notSet": "未设置"
   },
   "general": {
     "title": "标题",

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,

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();

ui/js/admin.js

@@ -1,15 +1,15 @@
 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';
 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. */

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);
+});