From d49baeb5fa63c3cbd7c0f1d4438eca2bbbffebb4 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Mon, 13 Jul 2026 08:36:19 -0400 Subject: [PATCH] Security closeout: settings public allowlist, audit.view gating, test-user guard Settings exposure (review medium): GET /api/settings and /api/settings/ now return the full table only to an authenticated principal. Unauthenticated callers (kiosk dashboards, print pages, login screen, setup router) get just a public allowlist - categories branding + map plus a named set (site_base_url, facility_name, printer_hostname_template, contact_email_domain, servicenow_enabled, setup_complete). A non-public single-key GET returns 404 so existence is not confirmed. Secrets stay masked in both cases. Closes the unauthenticated enumeration of smtp_host / employee_db_host / zabbix_url / servicenow URLs. Allowlist mirrors the keys siteSettings.js + mapConfig.js + setupState.js read before login. audit.view (review low): the three audit-read routes (list, entity-history, stats) were jwt_required only despite a defined-but-unwired audit.view permission; now gated by it (seeded to admin), so a role-less member or unscoped PAT can no longer read the cross-user audit trail. flask seed test-user (review low): refuses outside DEBUG/TESTING - it creates the well-known admin/admin123; production sites use `flask seed admin`. Tests: unauthenticated allowlist + authed-full-masked + private-key-404, and member-403 / admin-200 on audit routes. 336 authz tests pass; naming green. Co-Authored-By: Claude Opus 4.8 --- shopdb/cli/__init__.py | 10 ++++- shopdb/core/api/auditlogs.py | 4 ++ shopdb/core/api/settings.py | 29 +++++++++++- tests/test_core/test_authz.py | 54 +++++++++++++++++++++++ tests/test_core/test_settings_branding.py | 16 ++++--- 5 files changed, 104 insertions(+), 9 deletions(-) diff --git a/shopdb/cli/__init__.py b/shopdb/cli/__init__.py index 8ca1bbf..7d2c0d9 100644 --- a/shopdb/cli/__init__.py +++ b/shopdb/cli/__init__.py @@ -280,11 +280,19 @@ def seed_reference_data(): @seed_cli.command('test-user') @with_appcontext def seed_test_user(): - """Create a test admin user.""" + """Create a test admin user (admin / admin123). DEV ONLY.""" + from flask import current_app from shopdb.extensions import db from shopdb.core.models import User, Role from werkzeug.security import generate_password_hash + # Refuse in production: this seeds a well-known credential. Sites bootstrap + # a real admin with `flask seed admin` (generated password) or the wizard. + if not (current_app.config.get('DEBUG') or current_app.config.get('TESTING')): + raise click.ClickException( + 'seed test-user is dev-only (creates admin/admin123). ' + 'Use `flask seed admin` to create a production admin.') + # Create admin role if not exists admin_role = Role.query.filter_by(rolename='admin').first() if not admin_role: diff --git a/shopdb/core/api/auditlogs.py b/shopdb/core/api/auditlogs.py index bd3d9cf..fca6ec0 100644 --- a/shopdb/core/api/auditlogs.py +++ b/shopdb/core/api/auditlogs.py @@ -5,6 +5,7 @@ from flask_jwt_extended import jwt_required from shopdb.core.models import AuditLog from shopdb.utils.responses import success_response +from shopdb.utils.authz import require_permission auditlogs_bp = Blueprint('auditlogs', __name__) @@ -51,6 +52,7 @@ def _resolve_full_names(ssos): @auditlogs_bp.route('', methods=['GET']) @jwt_required() +@require_permission('audit.view') def list_auditlogs(): """ List audit logs with filtering and pagination. @@ -134,6 +136,7 @@ def list_auditlogs(): @auditlogs_bp.route('/entity//', methods=['GET']) @jwt_required() +@require_permission('audit.view') def get_entity_history(entitytype: str, entityid: int): """Get audit history for a specific entity.""" logs = AuditLog.query.filter_by( @@ -146,6 +149,7 @@ def get_entity_history(entitytype: str, entityid: int): @auditlogs_bp.route('/stats', methods=['GET']) @jwt_required() +@require_permission('audit.view') def get_stats(): """Get audit log statistics.""" from sqlalchemy import func diff --git a/shopdb/core/api/settings.py b/shopdb/core/api/settings.py index 0c7a0e9..5f93cd3 100644 --- a/shopdb/core/api/settings.py +++ b/shopdb/core/api/settings.py @@ -3,7 +3,7 @@ import os from flask import Blueprint, request, current_app, send_from_directory -from flask_jwt_extended import jwt_required +from flask_jwt_extended import jwt_required, get_jwt_identity from werkzeug.utils import secure_filename from shopdb.extensions import db, cache @@ -45,6 +45,24 @@ SETTINGS_CACHE_TTL = 300 # 5 minutes # exposed in plaintext. Sending it back on update is treated as "unchanged". SECRET_MASK = '********' +# Public-settings allowlist. An UNAUTHENTICATED caller (kiosk dashboards, print +# pages, the login screen, the setup router) may read only these; everything +# else (smtp_host, employee_db_host, zabbix_url, servicenow URLs, ...) needs a +# valid token. Secrets stay masked in both cases. Keep in sync with the keys +# frontend/src/utils/siteSettings.js + mapConfig.js + setupState.js read before +# login. Whole categories that are purely presentation are allowed wholesale; +# the rest are named keys so a new integration key does not leak by default. +PUBLIC_SETTING_CATEGORIES = {'branding', 'map'} +PUBLIC_SETTING_KEYS = { + 'site_base_url', 'facility_name', 'printer_hostname_template', + 'contact_email_domain', 'servicenow_enabled', 'setup_complete', +} + + +def _is_public_setting(setting) -> bool: + return (setting.category in PUBLIC_SETTING_CATEGORIES + or setting.key in PUBLIC_SETTING_KEYS) + # Optional asset identifiers and the asset types they can be toggled on. # Drives per-type seed keys and the Settings matrix UI. The asset type names # match the AssetType.assettype values seeded by each plugin. @@ -200,6 +218,10 @@ def list_settings(): query = query.filter_by(category=category) settings = query.order_by(Setting.category, Setting.key).all() + # Unauthenticated callers see only the public allowlist (branding + a few + # bootstrap keys); an authed principal sees everything (secrets masked). + if get_jwt_identity() is None: + settings = [s for s in settings if _is_public_setting(s)] return success_response([_serialize_setting(s) for s in settings]) @@ -212,6 +234,11 @@ def get_setting(key: str): if not setting: return error_response(ErrorCodes.NOT_FOUND, f'Setting {key} not found', http_code=404) + # A non-public key is invisible to an unauthenticated caller (404, not 403, + # so its existence is not confirmed either). + if get_jwt_identity() is None and not _is_public_setting(setting): + return error_response(ErrorCodes.NOT_FOUND, f'Setting {key} not found', http_code=404) + return success_response(_serialize_setting(setting)) diff --git a/tests/test_core/test_authz.py b/tests/test_core/test_authz.py index d836054..39875f0 100644 --- a/tests/test_core/test_authz.py +++ b/tests/test_core/test_authz.py @@ -267,3 +267,57 @@ def test_mutation_admin_passes_authz(client, db, auth_headers, f'{method} {endpoint} ({url}) returned {response.status_code} for ' f'admin - the admin role should bypass every permission check.' ) + + +# --- Settings exposure allowlist (unauthenticated recon guard) --------------- + +def _seed_settings(db): + from shopdb.core.models import Setting + db.session.add_all([ + Setting(key='facility_name', value='WJ', valuetype='string', category='site'), + Setting(key='brand_primary_color', value='#123', valuetype='string', category='branding'), + Setting(key='smtp_host', value='mail.internal', valuetype='string', category='communication'), + Setting(key='employee_db_host', value='hr.internal', valuetype='string', category='integrations'), + Setting(key='zabbix_token', value='supersecret', valuetype='string', category='integrations'), + ]) + db.session.commit() + + +def test_unauthenticated_settings_list_is_allowlisted(client, db): + """No token: only the public allowlist (branding + bootstrap keys) is + visible; internal infra config is not enumerable.""" + _seed_settings(db) + keys = {s['key'] for s in client.get('/api/settings').get_json()['data']} + assert 'facility_name' in keys and 'brand_primary_color' in keys + assert 'smtp_host' not in keys + assert 'employee_db_host' not in keys + assert 'zabbix_token' not in keys + + +def test_authenticated_settings_list_full_but_masked(client, db, member_headers): + """Any authed principal sees the full table; secret values stay masked.""" + _seed_settings(db) + rows = {s['key']: s['value'] + for s in client.get('/api/settings', headers=member_headers).get_json()['data']} + assert 'smtp_host' in rows and 'employee_db_host' in rows + assert rows['zabbix_token'] == '********' # secret masked even when visible + + +def test_unauthenticated_private_setting_get_is_404(client, db): + """A single private key is not confirmable without auth; a public one is.""" + _seed_settings(db) + assert client.get('/api/settings/smtp_host').status_code == 404 + assert client.get('/api/settings/facility_name').status_code == 200 + + +# --- audit.view gating ------------------------------------------------------- + +def test_member_cannot_read_auditlogs(client, db, member_headers): + """Audit-read routes now require audit.view; a role-less member is 403.""" + for url in ('/api/auditlogs', '/api/auditlogs/entity/Setting/1', '/api/auditlogs/stats'): + assert client.get(url, headers=member_headers).status_code == 403, url + + +def test_admin_can_read_auditlogs(client, db, auth_headers): + """Admin holds audit.view and reads the audit trail.""" + assert client.get('/api/auditlogs', headers=auth_headers).status_code == 200 diff --git a/tests/test_core/test_settings_branding.py b/tests/test_core/test_settings_branding.py index d987a3a..274f1f1 100644 --- a/tests/test_core/test_settings_branding.py +++ b/tests/test_core/test_settings_branding.py @@ -18,27 +18,29 @@ def _defaults_by_key(): return {d['key']: d for d in build_default_settings()} -def test_anon_settings_list_masks_secrets(client, db): - """Anonymous GET /api/settings never returns password/token/secret values.""" +def test_anon_settings_list_excludes_nonpublic(client, db): + """Anonymous GET /api/settings returns only the public allowlist - secret + AND non-secret internal keys are absent, so plaintext can never leak.""" from shopdb.core.models import Setting Setting.set('smtp_password', 'supersecret', valuetype='string', category='email') Setting.set('zabbix_token', 'tok-abc-123', valuetype='string', category='integrations') Setting.set('warranty_dell_clientsecret', 'shh', valuetype='string', category='integrations') - # A non-secret key stays visible for contrast. + # A public key stays visible for contrast. Setting.set('facility_name', 'Test Plant', valuetype='string', category='site') resp = client.get('/api/settings') assert resp.status_code == 200, resp.get_json() by_key = {s['key']: s['value'] for s in resp.get_json()['data']} - assert by_key['smtp_password'] == '********' - assert by_key['zabbix_token'] == '********' - assert by_key['warranty_dell_clientsecret'] == '********' + # Non-public keys are not enumerable at all (stronger than masking). + assert 'smtp_password' not in by_key + assert 'zabbix_token' not in by_key + assert 'warranty_dell_clientsecret' not in by_key # Plaintext secrets must not leak anywhere in the response body. assert 'supersecret' not in resp.get_data(as_text=True) assert 'tok-abc-123' not in resp.get_data(as_text=True) - # Non-secret value passes through untouched. + # A public key passes through untouched. assert by_key['facility_name'] == 'Test Plant'