Fix: clear the stored password when protection is turned off

CHANGELOG.md

@@ -41,6 +41,9 @@ 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.
+
 ### Security
 
 - A Connections tile that cannot reach a service now says so in plain words

api/src/routes/auth.js

@@ -171,9 +171,21 @@ on('POST', '/api/auth/toggle', async (req, res) => {
       return json(res, 400, { error: 'Set a password before turning authentication on.', kind: KIND.INVALID });
     }
     cfg.settings.auth.enabled = !!enabled;
+    /* Turning protection off discards the password with it. Keeping the hash
+       strands the install: setting a new password needs a session, no session
+       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. */
+    const cleared = !enabled && !!cfg.settings.auth.passwordHash;
+    if (!enabled) {
+      delete cfg.settings.auth.passwordHash;
+      delete cfg.settings.auth.secret;
+    }
     if (enabled && !cfg.settings.auth.secret) cfg.settings.auth.secret = newSessionSecret();
     saveConfig(cfg);
     log.audit('auth toggled', { enabled: !!enabled });
+    if (cleared) log.audit('password cleared', {});
     json(res, 200, { ok: true });
   } catch (e) {
     fail(res, e, { status: 400 });

api/test/auth-lockout.test.js

@@ -127,6 +127,65 @@ test('auth can always be switched off', async () => {
   assert.equal(loadConfig().settings.auth.enabled, false);
 });
 
+/* ── switching off discards the password ──────────────────────────────────── */
+
+/* A disabled password that stays on disk is its own dead end: set-password
+   refuses while a hash exists unless the caller holds a session, and no session
+   can be obtained with auth off, so a forgotten password could only be replaced
+   by hand-editing the config. */
+
+test('switching auth off clears the stored password and secret', async () => {
+  const cfg = loadConfig();
+  cfg.settings.auth = { enabled: true, secret: SECRET, passwordHash: await hashPassword('correct-horse') };
+  saveConfig(cfg);
+
+  assert.equal(
+    (await req('POST', '/api/auth/toggle', { enabled: false }, 'ds=' + makeToken('s1', SECRET))).status,
+    200,
+  );
+
+  const stored = loadConfig().settings.auth;
+  assert.equal(stored.passwordHash, undefined);
+  assert.equal(stored.secret, undefined);
+  assert.equal((await req('GET', '/api/auth/check')).body.passwordSet, false);
+});
+
+test('an orphaned hash left by an earlier version is cleared on the next switch off', async () => {
+  const cfg = loadConfig();
+  cfg.settings.auth = { enabled: false, secret: SECRET, passwordHash: await hashPassword('correct-horse') };
+  saveConfig(cfg);
+
+  assert.equal((await req('POST', '/api/auth/toggle', { enabled: true })).status, 200, 'the old password still works');
+  /* Auth is in force from here, so switching back off goes through the gate the
+     same way the admin would, with a session. The stored secret is untouched by
+     the enable, so a token signed with it is valid. */
+  const cookie = 'ds=' + makeToken('s1', SECRET);
+  assert.equal((await req('POST', '/api/auth/toggle', { enabled: false }, cookie)).status, 200);
+  assert.equal(loadConfig().settings.auth.passwordHash, undefined);
+
+  const r = await req('POST', '/api/auth/toggle', { enabled: true });
+  assert.equal(r.status, 400, 're-enabling now needs a new password rather than reviving the cleared one');
+  assert.match(r.body.error, /password/i);
+});
+
+test('a session token from before the switch off is not honoured again', async () => {
+  const cfg = loadConfig();
+  cfg.settings.auth = { enabled: true, secret: SECRET, passwordHash: await hashPassword('correct-horse') };
+  saveConfig(cfg);
+  const cookie = 'ds=' + makeToken('s1', SECRET);
+
+  await req('POST', '/api/auth/toggle', { enabled: false }, cookie);
+  await req('POST', '/api/auth/set-password', { password: 'a-brand-new-one' });
+
+  assert.equal((await req('GET', '/api/config', null, cookie)).status, 401);
+});
+
+test('switching off with no password stored is not an error', async () => {
+  const r = await req('POST', '/api/auth/toggle', { enabled: false });
+  assert.equal(r.status, 200);
+  assert.equal(loadConfig().settings.auth.enabled, false);
+});
+
 /* ── an install already locked recovers itself ────────────────────────────── */
 
 test('a locked install reports auth as off, matching how it behaves', async () => {

ui/i18n/de.json

@@ -164,6 +164,7 @@
   "confirm": {
     "deleteFolder": "Ordner „{name}“ löschen? Enthaltene Apps werden nicht gelöscht.",
     "revokeSessions": "Alle Geräte abmelden, auch dieses?",
+    "clearPassword": "Passwortschutz ausschalten? Das gespeicherte Passwort wird gelöscht, für ein erneutes Einschalten ist ein neues nötig.",
     "remove": "„{name}“ entfernen?"
   },
   "list": {

ui/i18n/en.json

@@ -164,6 +164,7 @@
   "confirm": {
     "deleteFolder": "Delete folder \"{name}\"? Apps inside will not be deleted.",
     "revokeSessions": "Sign out all devices, including this one?",
+    "clearPassword": "Turn off Password Protection? The stored password is deleted, so turning it back on will need a new one.",
     "remove": "Remove \"{name}\"?"
   },
   "list": {

ui/i18n/es.json

@@ -164,6 +164,7 @@
   "confirm": {
     "deleteFolder": "¿Eliminar la carpeta \"{name}\"? Las aplicaciones que contiene no se eliminarán.",
     "revokeSessions": "¿Cerrar sesión en todos los dispositivos, incluido este?",
+    "clearPassword": "¿Desactivar la protección con contraseña? Se eliminará la contraseña guardada, por lo que volver a activarla requerirá una nueva.",
     "remove": "¿Eliminar \"{name}\"?"
   },
   "list": {

ui/i18n/fa.json

@@ -164,6 +164,7 @@
   "confirm": {
     "deleteFolder": "پوشه «{name}» حذف شود؟ برنامه‌های داخل آن حذف نخواهند شد.",
     "revokeSessions": "از همه دستگاه‌ها، از جمله همین دستگاه، خارج شوم؟",
+    "clearPassword": "محافظت با رمز عبور خاموش شود؟ رمز عبور ذخیره‌شده حذف می‌شود، بنابراین روشن کردن دوباره به رمز تازه‌ای نیاز دارد.",
     "remove": "«{name}» حذف شود؟"
   },
   "list": {

ui/i18n/fr.json

@@ -164,6 +164,7 @@
   "confirm": {
     "deleteFolder": "Supprimer le dossier « {name} » ? Les applications qu'il contient ne seront pas supprimées.",
     "revokeSessions": "Déconnecter tous les appareils, y compris celui-ci ?",
+    "clearPassword": "Désactiver la protection par mot de passe ? Le mot de passe enregistré est supprimé, le réactiver en exigera un nouveau.",
     "remove": "Supprimer « {name} » ?"
   },
   "list": {

ui/i18n/zh-Hans.json

@@ -164,6 +164,7 @@
   "confirm": {
     "deleteFolder": "删除文件夹“{name}”？其中的应用不会被删除。",
     "revokeSessions": "退出所有设备，包括当前设备？",
+    "clearPassword": "关闭密码保护？已保存的密码将被删除，再次开启时需要设置新密码。",
     "remove": "删除“{name}”？"
   },
   "list": {

ui/js/admin-settings.js

@@ -303,10 +303,15 @@ async function saveServer() {
     c.settings.logLevel = inp('log-level')?.value || 'info';
     c.settings.language = inp('lang-sel')?.value || 'en';
     const langChanged = c.settings.language !== prevLang;
-    await ap('/api/config', c);
 
     const pw = inp('sec-pw')?.value || '';
     const enabled = inp('sec-en')?.checked || false;
+    /* Asked before the first request of the save, not next to the toggle: the
+       password survives until Save, so this is the moment it is really deleted,
+       and cancelling here must leave every other field unsaved too. */
+    if (!enabled && _passwordSet && !confirm(t('confirm.clearPassword'))) return;
+    await ap('/api/config', c);
+
     if (authEnableBlocked({ enabled, passwordSet: _passwordSet, newPassword: pw })) {
       toast(t('toast.authNeedsPassword'), 'err');
       return;
@@ -326,6 +331,15 @@ async function saveServer() {
       }
     }
     await ap('/api/auth/toggle', { enabled });
+    if (!enabled) {
+      /* The server drops the password with the toggle, so the page must stop
+         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';
+      const pwEl = inp('sec-pw');
+      if (pwEl) pwEl.placeholder = '';
+    }
     toast(t('toast.saved'));
     if (langChanged) location.reload();
   } catch (e) {