Login mode: enforce "every profile has a password" at every write-point (no gaps)
Invariant: while security.require_login is on, every profile must have a login password or it's locked out. Previously only the admin's own anti-lockout existed, so members could be stranded (created without a password, or login flipped on while passwordless members existed). Closed all the write-points: core/security/login_provisioning.py (pure policy, single source of truth): - members_without_password(profiles) — non-admin profiles that can't sign in - create_needs_password(require_login) / removing_password_strands(require_login) Wired into web_server: - create_profile: while login is on, a new member must be given a password (400 otherwise) and it's set on creation. - enable-login (settings save): refuses to turn login on while any member lacks a password — lists them — same shape as the existing admin anti-lockout. - set-password: refuses to CLEAR a password while login is on (would strand them). UI: Create Profile form gains a login-password field (alongside the optional PIN); the Manage Profiles per-member password button (prior commit) covers existing members + changes. Tests: pure policy seam + endpoint enforcement (create blocked w/o password when on, allowed w/ password, no friction when off, clear blocked when on). 442 profile/settings/auth tests green; ruff clean.
This commit is contained in:
parent
e046a2add4
commit
5b52d579c5
5 changed files with 176 additions and 0 deletions
45
core/security/login_provisioning.py
Normal file
45
core/security/login_provisioning.py
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
"""Login-mode password provisioning policy.
|
||||
|
||||
Invariant: while ``security.require_login`` is on, every profile must have a login
|
||||
password — otherwise it's fail-closed locked out (usable only after the admin
|
||||
provisions one). That's not a security hole (no-password = can't get in), but it's
|
||||
a usability gap, and the point here is to make it impossible to OPEN one from any
|
||||
write-point: creating a profile, clearing a password, or flipping login mode on.
|
||||
|
||||
These are pure decisions so they're the single source of truth + unit-testable;
|
||||
web_server wires them into the create / set-password / enable-login endpoints, and
|
||||
the UI mirrors them.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
|
||||
def members_without_password(profiles: Optional[List[Dict[str, Any]]]) -> List[Dict[str, Any]]:
|
||||
"""Non-admin profiles with no login password — they can't sign in once login
|
||||
mode is on. The admin is covered separately by its own anti-lockout, so it's
|
||||
excluded here. Returns ``[{'id', 'name'}, …]`` (empty = no gap)."""
|
||||
out: List[Dict[str, Any]] = []
|
||||
for p in (profiles or []):
|
||||
if not p.get('is_admin') and not p.get('has_password'):
|
||||
out.append({'id': p.get('id'), 'name': p.get('name')})
|
||||
return out
|
||||
|
||||
|
||||
def create_needs_password(require_login: bool, is_admin: bool = False) -> bool:
|
||||
"""A non-admin profile created while login mode is on must carry a password,
|
||||
or it's born unable to sign in."""
|
||||
return bool(require_login) and not is_admin
|
||||
|
||||
|
||||
def removing_password_strands(require_login: bool) -> bool:
|
||||
"""Clearing a profile's password while login mode is on would lock it out."""
|
||||
return bool(require_login)
|
||||
|
||||
|
||||
__all__ = [
|
||||
"members_without_password",
|
||||
"create_needs_password",
|
||||
"removing_password_strands",
|
||||
]
|
||||
97
tests/test_login_provisioning.py
Normal file
97
tests/test_login_provisioning.py
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
"""No-gaps invariant: while login mode is on, every profile must have a login
|
||||
password. Pure policy seam + endpoint enforcement at every write-point (create,
|
||||
clear, enable-login)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
import pytest
|
||||
|
||||
from core.security.login_provisioning import (
|
||||
members_without_password, create_needs_password, removing_password_strands)
|
||||
|
||||
|
||||
# ── pure policy ─────────────────────────────────────────────────────────────
|
||||
def test_members_without_password_flags_only_passwordless_nonadmins():
|
||||
profiles = [
|
||||
{'id': 1, 'name': 'Admin', 'is_admin': True, 'has_password': False}, # admin: own anti-lockout
|
||||
{'id': 2, 'name': 'HasPw', 'is_admin': False, 'has_password': True}, # fine
|
||||
{'id': 3, 'name': 'NoPw', 'is_admin': False, 'has_password': False}, # stranded
|
||||
]
|
||||
out = members_without_password(profiles)
|
||||
assert out == [{'id': 3, 'name': 'NoPw'}]
|
||||
|
||||
|
||||
def test_members_without_password_empty_when_all_set():
|
||||
assert members_without_password([{'id': 2, 'is_admin': False, 'has_password': True}]) == []
|
||||
assert members_without_password(None) == []
|
||||
|
||||
|
||||
def test_create_needs_password_only_when_login_on_and_nonadmin():
|
||||
assert create_needs_password(True) is True
|
||||
assert create_needs_password(False) is False
|
||||
assert create_needs_password(True, is_admin=True) is False
|
||||
|
||||
|
||||
def test_removing_password_strands_only_when_login_on():
|
||||
assert removing_password_strands(True) is True
|
||||
assert removing_password_strands(False) is False
|
||||
|
||||
|
||||
# ── endpoint enforcement ────────────────────────────────────────────────────
|
||||
_TMP = tempfile.mkdtemp(prefix='soulsync-testdb-prov-')
|
||||
os.environ['DATABASE_PATH'] = os.path.join(_TMP, 'p.db')
|
||||
os.environ['SOULSYNC_TEST_DB_READY'] = '1'
|
||||
web_server = pytest.importorskip('web_server')
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
return web_server.app.test_client()
|
||||
|
||||
|
||||
def _login_on(monkeypatch, on=True):
|
||||
real = web_server.config_manager.get
|
||||
monkeypatch.setattr(web_server.config_manager, 'get',
|
||||
lambda k, d=None: on if k == 'security.require_login' else real(k, d))
|
||||
|
||||
|
||||
def _auth(c):
|
||||
# Turning login mode on activates the HTTP gate — authenticate the session as
|
||||
# admin so the request reaches the endpoint (we're testing the endpoint logic).
|
||||
with c.session_transaction() as sess:
|
||||
sess['login_authenticated'] = True
|
||||
sess['profile_id'] = 1
|
||||
|
||||
|
||||
def test_create_without_password_blocked_when_login_on(monkeypatch, client):
|
||||
_login_on(monkeypatch, True); _auth(client)
|
||||
r = client.post('/api/profiles', json={'name': 'NoPwMember'})
|
||||
assert r.status_code == 400
|
||||
assert 'login' in r.get_json()['error'].lower()
|
||||
|
||||
|
||||
def test_create_with_password_succeeds_when_login_on(monkeypatch, client):
|
||||
_login_on(monkeypatch, True); _auth(client)
|
||||
r = client.post('/api/profiles', json={'name': 'PwMember', 'password': 'secret9'})
|
||||
assert r.status_code == 200 and r.get_json()['success'] is True
|
||||
pid = r.get_json()['profile_id']
|
||||
assert web_server.get_database().verify_profile_password(pid, 'secret9') is True
|
||||
|
||||
|
||||
def test_create_without_password_fine_when_login_off(monkeypatch, client):
|
||||
_login_on(monkeypatch, False)
|
||||
r = client.post('/api/profiles', json={'name': 'PinOnlyMember'})
|
||||
assert r.status_code == 200 and r.get_json()['success'] is True # no friction when off
|
||||
|
||||
|
||||
def test_clear_password_blocked_when_login_on(monkeypatch, client):
|
||||
db = web_server.get_database()
|
||||
r = client.post('/api/profiles', json={'name': 'Clearable', 'password': 'x12345'})
|
||||
pid = r.get_json()['profile_id']
|
||||
_login_on(monkeypatch, True); _auth(client)
|
||||
r2 = client.post(f'/api/profiles/{pid}/set-password', json={'password': ''})
|
||||
assert r2.status_code == 400 and 'login mode' in r2.get_json()['error'].lower()
|
||||
assert db.verify_profile_password(pid, 'x12345') is True # still set
|
||||
|
|
@ -3120,6 +3120,17 @@ def handle_settings():
|
|||
if not get_database().profile_has_password(1):
|
||||
return jsonify({"success": False,
|
||||
"error": "Set an admin password before enabling login mode."}), 400
|
||||
# No-gaps: every member must have a password too, or they'd be
|
||||
# locked out the moment login turns on.
|
||||
from core.security.login_provisioning import members_without_password
|
||||
_stranded = members_without_password(get_database().get_all_profiles())
|
||||
if _stranded:
|
||||
_names = ', '.join(str(m.get('name') or '?') for m in _stranded)
|
||||
return jsonify({"success": False,
|
||||
"error": f"These members have no login password and "
|
||||
f"couldn't sign in: {_names}. Set their passwords "
|
||||
f"in Manage Profiles first.",
|
||||
"members_without_password": _stranded}), 400
|
||||
|
||||
if 'active_media_server' in new_settings:
|
||||
config_manager.set_active_media_server(new_settings['active_media_server'])
|
||||
|
|
@ -25562,6 +25573,15 @@ def create_profile():
|
|||
from werkzeug.security import generate_password_hash
|
||||
pin_hash = generate_password_hash(pin, method='pbkdf2:sha256')
|
||||
|
||||
# No-gaps: while login mode is on, a new member must be born with a login
|
||||
# password or they could never sign in.
|
||||
password = (data.get('password') or '').strip()
|
||||
from core.security.login_provisioning import create_needs_password
|
||||
if create_needs_password(_require_login_enabled()) and not password:
|
||||
return jsonify({'success': False,
|
||||
'error': 'Login mode is on — give this profile a login '
|
||||
'password so they can sign in.'}), 400
|
||||
|
||||
# Profile settings: home_page, allowed_pages, can_download
|
||||
home_page = data.get('home_page') or None
|
||||
allowed_pages = data.get('allowed_pages') # list or None
|
||||
|
|
@ -25586,6 +25606,9 @@ def create_profile():
|
|||
if profile_id is None:
|
||||
return jsonify({'success': False, 'error': 'Profile name already exists'}), 409
|
||||
|
||||
if password:
|
||||
database.set_profile_password(profile_id, password)
|
||||
|
||||
return jsonify({'success': True, 'profile_id': profile_id})
|
||||
except Exception as e:
|
||||
return jsonify({'success': False, 'error': str(e)}), 500
|
||||
|
|
@ -26000,6 +26023,13 @@ def set_profile_password_endpoint(profile_id):
|
|||
return jsonify({'success': False, 'error': 'Unauthorized'}), 403
|
||||
data = request.json or {}
|
||||
password = data.get('password', '')
|
||||
# No-gaps: clearing a password while login mode is on would lock that
|
||||
# profile out — refuse it (delete the profile instead if that's intended).
|
||||
from core.security.login_provisioning import removing_password_strands
|
||||
if not (password or '').strip() and removing_password_strands(_require_login_enabled()):
|
||||
return jsonify({'success': False,
|
||||
'error': "Can't remove this password while login mode is on — "
|
||||
"that profile couldn't sign in."}), 400
|
||||
ok = database.set_profile_password(profile_id, password)
|
||||
return jsonify({'success': bool(ok), 'has_password': database.profile_has_password(profile_id)})
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -131,6 +131,7 @@
|
|||
<span class="profile-color-swatch" data-color="#14b8a6" style="background:#14b8a6" title="Teal"></span>
|
||||
</div>
|
||||
<input type="password" id="new-profile-pin" class="profile-input" placeholder="PIN (optional)" maxlength="6">
|
||||
<input type="password" id="new-profile-password" class="profile-input" placeholder="Login password (required when login mode is on)" autocomplete="new-password">
|
||||
<div class="profile-settings-section">
|
||||
<label class="profile-settings-label">Home Page</label>
|
||||
<select id="new-profile-home-page" class="profile-input">
|
||||
|
|
|
|||
|
|
@ -1794,6 +1794,7 @@ function initProfileManagement() {
|
|||
const name = document.getElementById('new-profile-name').value.trim();
|
||||
const avatarUrl = document.getElementById('new-profile-avatar-url').value.trim();
|
||||
const pin = document.getElementById('new-profile-pin').value;
|
||||
const loginPassword = (document.getElementById('new-profile-password') || {}).value || '';
|
||||
if (!name) return;
|
||||
|
||||
// Collect profile settings
|
||||
|
|
@ -1810,6 +1811,7 @@ function initProfileManagement() {
|
|||
name, avatar_color: selectedColor,
|
||||
avatar_url: avatarUrl || undefined,
|
||||
pin: pin || undefined,
|
||||
password: loginPassword || undefined,
|
||||
home_page: homePage,
|
||||
allowed_pages: allowedPages,
|
||||
can_download: canDl
|
||||
|
|
@ -1820,6 +1822,7 @@ function initProfileManagement() {
|
|||
document.getElementById('new-profile-name').value = '';
|
||||
document.getElementById('new-profile-avatar-url').value = '';
|
||||
document.getElementById('new-profile-pin').value = '';
|
||||
if (document.getElementById('new-profile-password')) document.getElementById('new-profile-password').value = '';
|
||||
document.getElementById('new-profile-home-page').value = '';
|
||||
pageCheckboxes.forEach(cb => cb.checked = true);
|
||||
document.getElementById('new-profile-can-download').checked = true;
|
||||
|
|
|
|||
Loading…
Reference in a new issue