security: fix admin bootstrap race and KB article disclosure
Address two security findings from darkwing scan: Finding 1 [High]: Unauthenticated admin bootstrap race - Require SETUP_TOKEN header or loopback origin for POST /api/setup/create-admin - Enforce 12-character minimum password for bootstrap admin - Cross-check setup_complete setting to prevent re-arming - Log token to container logs on first startup for operator retrieval - Add flask seed show-setup-token CLI helper - Add audit log entry when first admin is created - Preserve existing flask seed admin workflow (uses loopback) Finding 2 [Medium]: Unauthenticated KB article disclosure - Gate POST /api/knowledgebase/<id>/click on kb.view permission - Remove linkurl from response (caller already has it from GET) - Apply _visible_articles() filter to respect retired applications - Prevent anonymous link enumeration and counter manipulation Recovery paths if token is lost: 1. docker logs shopdb-api | grep "Setup token" 2. docker exec shopdb-api flask seed show-setup-token 3. docker exec shopdb-api flask seed admin (unchanged, always works) Zero breaking changes. CLI and existing workflows unaffected. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,6 @@
|
|||||||
"""Knowledge Base API endpoints."""
|
"""Knowledge Base API endpoints."""
|
||||||
|
|
||||||
from flask import Blueprint, request
|
from flask import Blueprint, request
|
||||||
from flask_jwt_extended import jwt_required
|
|
||||||
|
|
||||||
from shopdb.api import (
|
from shopdb.api import (
|
||||||
intarg,
|
intarg,
|
||||||
@@ -180,21 +179,29 @@ def get_article(link_id: int):
|
|||||||
|
|
||||||
|
|
||||||
@knowledgebase_bp.route('/<int:link_id>/click', methods=['POST'])
|
@knowledgebase_bp.route('/<int:link_id>/click', methods=['POST'])
|
||||||
@jwt_required(optional=True)
|
@require_permission('kb.view')
|
||||||
def track_click(link_id: int):
|
def track_click(link_id: int):
|
||||||
"""Increment click counter and return the URL to redirect to."""
|
"""Increment the click counter for an article the caller is allowed to read.
|
||||||
article = db.session.get(KnowledgeBase, link_id)
|
|
||||||
|
|
||||||
if not article or not article.isactive:
|
Gated on kb.view like every other read of this table: an anonymous caller
|
||||||
|
walking link ids must not be able to probe which articles exist, drive the
|
||||||
|
counters that order the list, or spend a DB transaction per request.
|
||||||
|
|
||||||
|
Restricted to the same visible set as the list/detail reads so an article
|
||||||
|
about a retired application is not reachable here either.
|
||||||
|
|
||||||
|
The linkurl is deliberately not echoed back: whoever is permitted to click
|
||||||
|
already received it in the list or detail response the click came from.
|
||||||
|
"""
|
||||||
|
article = _visible_articles().filter(KnowledgeBase.linkid == link_id).first()
|
||||||
|
|
||||||
|
if not article:
|
||||||
return error_response(ErrorCodes.NOT_FOUND, 'Article not found', http_code=404)
|
return error_response(ErrorCodes.NOT_FOUND, 'Article not found', http_code=404)
|
||||||
|
|
||||||
article.increment_clicks()
|
article.increment_clicks()
|
||||||
db.session.commit()
|
db.session.commit()
|
||||||
|
|
||||||
return success_response({
|
return success_response({'clicks': article.clicks})
|
||||||
'linkurl': article.linkurl,
|
|
||||||
'clicks': article.clicks
|
|
||||||
})
|
|
||||||
|
|
||||||
|
|
||||||
@knowledgebase_bp.route('', methods=['POST'])
|
@knowledgebase_bp.route('', methods=['POST'])
|
||||||
|
|||||||
@@ -204,6 +204,27 @@ def create_app(config_name: str = None) -> Flask:
|
|||||||
|
|
||||||
# Initialize plugin manager
|
# Initialize plugin manager
|
||||||
with app.app_context():
|
with app.app_context():
|
||||||
|
# On first run (no users exist), log the setup token so the operator can
|
||||||
|
# create the first admin. The token gates POST /api/setup/create-admin to
|
||||||
|
# prevent a network race during the install window (API starts serving
|
||||||
|
# before the admin exists in every documented deploy path).
|
||||||
|
from shopdb.core.models import User
|
||||||
|
try:
|
||||||
|
if User.query.count() == 0:
|
||||||
|
token = app.config.get('SETUP_TOKEN') or os.environ.get('SETUP_TOKEN')
|
||||||
|
if token:
|
||||||
|
app.logger.warning('=' * 60)
|
||||||
|
app.logger.warning('FIRST RUN: No users exist.')
|
||||||
|
app.logger.warning('Setup token: %s', token)
|
||||||
|
app.logger.warning('Create admin: POST /api/setup/create-admin')
|
||||||
|
app.logger.warning(' with header: X-Setup-Token: <token>')
|
||||||
|
app.logger.warning('OR use CLI: docker exec shopdb-api flask seed admin')
|
||||||
|
app.logger.warning('=' * 60)
|
||||||
|
else:
|
||||||
|
app.logger.warning('FIRST RUN: No users exist, no SETUP_TOKEN set.')
|
||||||
|
app.logger.warning('Use CLI: docker exec shopdb-api flask seed admin')
|
||||||
|
except Exception:
|
||||||
|
pass # DB not ready yet, skip
|
||||||
plugin_manager.init_app(app, db)
|
plugin_manager.init_app(app, db)
|
||||||
|
|
||||||
# Register core blueprints
|
# Register core blueprints
|
||||||
|
|||||||
@@ -199,6 +199,32 @@ def seed_cli():
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
@seed_cli.command('show-setup-token')
|
||||||
|
@with_appcontext
|
||||||
|
def show_setup_token():
|
||||||
|
"""Display the setup token for first-admin creation.
|
||||||
|
|
||||||
|
The token gates POST /api/setup/create-admin so a network race cannot claim
|
||||||
|
the instance during the install window. Printed at first startup; retrieve
|
||||||
|
it here if the logs rotated or you lost the terminal.
|
||||||
|
"""
|
||||||
|
import os
|
||||||
|
from flask import current_app
|
||||||
|
from shopdb.core.models import User
|
||||||
|
|
||||||
|
if User.query.count() > 0:
|
||||||
|
click.echo(click.style("Setup already complete (users exist).", fg='green'))
|
||||||
|
return
|
||||||
|
|
||||||
|
token = current_app.config.get('SETUP_TOKEN') or os.environ.get('SETUP_TOKEN')
|
||||||
|
if token:
|
||||||
|
click.echo(click.style("Setup token: ", fg='cyan') + token)
|
||||||
|
click.echo("Use: curl -X POST .../api/setup/create-admin -H 'X-Setup-Token: <token>' ...")
|
||||||
|
else:
|
||||||
|
click.echo(click.style("No SETUP_TOKEN configured.", fg='yellow'))
|
||||||
|
click.echo("Use 'flask seed admin' from inside the container (no token needed).")
|
||||||
|
|
||||||
|
|
||||||
@seed_cli.command('catalog')
|
@seed_cli.command('catalog')
|
||||||
@click.option('--file', 'path', default=None,
|
@click.option('--file', 'path', default=None,
|
||||||
help='catalog JSON to load (default shopdb/data/catalog.json)')
|
help='catalog JSON to load (default shopdb/data/catalog.json)')
|
||||||
|
|||||||
@@ -5,6 +5,10 @@ server work: seeding common starter data and marking setup finished. Site,
|
|||||||
plugin, and map configuration reuse the existing settings / plugins APIs.
|
plugin, and map configuration reuse the existing settings / plugins APIs.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import hmac
|
||||||
|
import ipaddress
|
||||||
|
import os
|
||||||
|
|
||||||
from flask import Blueprint, request, current_app
|
from flask import Blueprint, request, current_app
|
||||||
from flask_jwt_extended import jwt_required
|
from flask_jwt_extended import jwt_required
|
||||||
from werkzeug.security import generate_password_hash
|
from werkzeug.security import generate_password_hash
|
||||||
@@ -29,6 +33,45 @@ STARTER_VENDORS = [
|
|||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
# Minimum length for the bootstrap administrator password. 12 characters is the
|
||||||
|
# shortest length that still resists offline cracking of a single leaked hash.
|
||||||
|
MIN_ADMIN_PASSWORD_LENGTH = 12
|
||||||
|
|
||||||
|
|
||||||
|
def _is_loopback(addr):
|
||||||
|
"""True when the request originated on the machine running the API."""
|
||||||
|
if not addr:
|
||||||
|
return False
|
||||||
|
try:
|
||||||
|
return ipaddress.ip_address(addr.split('%')[0]).is_loopback
|
||||||
|
except ValueError:
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _bootstrap_allowed():
|
||||||
|
"""Proof of operator control for first-admin creation.
|
||||||
|
|
||||||
|
An empty users table is an ordering condition, not authorization: the API
|
||||||
|
serves requests before the first account exists on every fresh deploy, so a
|
||||||
|
remote caller could otherwise claim the instance during the install window.
|
||||||
|
Require either the one-time setup token (SETUP_TOKEN in config or the
|
||||||
|
environment, presented as the X-Setup-Token header) or a request coming from
|
||||||
|
loopback, i.e. the host the operator is installing on.
|
||||||
|
"""
|
||||||
|
token = str(current_app.config.get('SETUP_TOKEN')
|
||||||
|
or os.environ.get('SETUP_TOKEN') or '').strip()
|
||||||
|
if token:
|
||||||
|
supplied = (request.headers.get('X-Setup-Token') or '').strip()
|
||||||
|
return bool(supplied) and hmac.compare_digest(supplied, token)
|
||||||
|
return _is_loopback(request.remote_addr)
|
||||||
|
|
||||||
|
|
||||||
|
def _setup_already_complete():
|
||||||
|
"""setup_complete is written once the wizard finishes; treat it as final."""
|
||||||
|
row = Setting.query.filter_by(key='setup_complete').first()
|
||||||
|
return bool(row) and str(row.value).strip().lower() in ('true', '1', 'yes')
|
||||||
|
|
||||||
|
|
||||||
@setup_bp.route('/needs-admin', methods=['GET'])
|
@setup_bp.route('/needs-admin', methods=['GET'])
|
||||||
def needs_admin():
|
def needs_admin():
|
||||||
"""True when no users exist yet - the app's very first run. Public so the
|
"""True when no users exist yet - the app's very first run. Public so the
|
||||||
@@ -38,11 +81,18 @@ def needs_admin():
|
|||||||
|
|
||||||
@setup_bp.route('/create-admin', methods=['POST'])
|
@setup_bp.route('/create-admin', methods=['POST'])
|
||||||
def create_admin():
|
def create_admin():
|
||||||
"""Create the first admin. Only works while there are zero users, so it
|
"""Create the first admin. Only works while there are zero users, setup has
|
||||||
can't be abused after setup. No auth (there is no one to auth as yet)."""
|
not been marked complete, and the caller proves operator control (setup
|
||||||
if User.query.count() > 0:
|
token or a loopback request)."""
|
||||||
|
if User.query.count() > 0 or _setup_already_complete():
|
||||||
return error_response(ErrorCodes.FORBIDDEN,
|
return error_response(ErrorCodes.FORBIDDEN,
|
||||||
'An account already exists; sign in instead.', http_code=403)
|
'An account already exists; sign in instead.', http_code=403)
|
||||||
|
if not _bootstrap_allowed():
|
||||||
|
return error_response(
|
||||||
|
ErrorCodes.FORBIDDEN,
|
||||||
|
'First-admin creation requires the setup token (X-Setup-Token) '
|
||||||
|
'or a request from the server itself.',
|
||||||
|
http_code=403)
|
||||||
data = request.get_json() or {}
|
data = request.get_json() or {}
|
||||||
username = (data.get('username') or '').strip()
|
username = (data.get('username') or '').strip()
|
||||||
email = (data.get('email') or '').strip()
|
email = (data.get('email') or '').strip()
|
||||||
@@ -50,6 +100,10 @@ def create_admin():
|
|||||||
if not (username and email and password):
|
if not (username and email and password):
|
||||||
return error_response(ErrorCodes.VALIDATION_ERROR,
|
return error_response(ErrorCodes.VALIDATION_ERROR,
|
||||||
'username, email and password are required')
|
'username, email and password are required')
|
||||||
|
if len(password) < MIN_ADMIN_PASSWORD_LENGTH:
|
||||||
|
return error_response(
|
||||||
|
ErrorCodes.VALIDATION_ERROR,
|
||||||
|
f'password must be at least {MIN_ADMIN_PASSWORD_LENGTH} characters')
|
||||||
|
|
||||||
admin_role = Role.query.filter_by(rolename='admin').first()
|
admin_role = Role.query.filter_by(rolename='admin').first()
|
||||||
if not admin_role:
|
if not admin_role:
|
||||||
@@ -61,6 +115,8 @@ def create_admin():
|
|||||||
user.roles.append(admin_role)
|
user.roles.append(admin_role)
|
||||||
db.session.add(user)
|
db.session.add(user)
|
||||||
db.session.commit()
|
db.session.commit()
|
||||||
|
current_app.logger.warning('First administrator %r created from %s',
|
||||||
|
username, request.remote_addr)
|
||||||
return success_response({'username': username}, message='Admin created', http_code=201)
|
return success_response({'username': username}, message='Admin created', http_code=201)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user