Security closeout: settings public allowlist, audit.view gating, test-user guard
Settings exposure (review medium): GET /api/settings and /api/settings/<key> 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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/<entitytype>/<int:entityid>', 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
|
||||
|
||||
@@ -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))
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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'
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user