ge-aerospace/shopdb-flask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/wj' into wilm

Browse Source
This commit is contained in:
benjamin.baker
2026-09-18 17:25:24 -04:00
parent 2ff7b7e906 f16b463736
commit e00d86827b
64 changed files with 35788 additions and 20614 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
shopdb/__init__.py
View File

@@ -139,7 +139,17 @@ from .plugins import plugin_manager
# bump can catch, and it is the kind of thing that gets copied. Exposing the
# model closes the gap instead of leaving the workaround as the worked example.
# Additive name on the surface, minor bump.
__contract_version__ = '0.34.0'
# 0.35.0 adds ip_in_cidrs and request_ip_allowlisted to shopdb.api: network
# position as an admission check for routes that run without a login. The two
# kiosks needed the identical gate - a device token suits an unattended screen
# provisioned once, and suits nothing about somebody opening the same page on
# their own PC - and a CIDR parser copied into two plugins is exactly where a
# fail-open creeps in later. Both fail closed on an empty setting, an
# unparseable caller address and a malformed entry. Caller trustworthiness is a
# deployment property: they read remote_addr, never raw X-Forwarded-For, so the
# IIS rewrite rule is what makes them meaningful. Additive names on the surface,
# minor bump.
__contract_version__ = '0.35.0'
# Product release version (see ADR-007). The product version and the
# plugin-contract version above are distinct series with independent

7
shopdb/api/__init__.py
View File

@@ -187,6 +187,11 @@ from shopdb.utils.uploads import send_upload, harden_upload_response
# nothing. See shopdb/utils/clientip.py.
from shopdb.utils.clientip import client_ip, strip_port
# Network-position allowlists for the unauthenticated kiosk routes. On the
# contract surface because two plugins need the identical gate and a
# copy-pasted CIDR parser is exactly where a fail-open creeps in.
from shopdb.utils.ipallowlist import ip_in_cidrs, request_ip_allowlisted
# Strict date parsing. On the contract surface because every plugin that stores
# a date needs it and the two that rolled their own both swallowed the error.
from shopdb.utils.queryargs import QueryArgError, intarg, intargs
@@ -477,6 +482,8 @@ __all__ = [
'harden_upload_response',
'client_ip',
'strip_port',
'ip_in_cidrs',
'request_ip_allowlisted',
# Import-mode helpers
'apply_import_timestamps',
'import_mode_active',

98
shopdb/core/api/collector.py
View File

@@ -27,6 +27,44 @@ COLLECTOR_SCOPE = 'collector.ingest'
# declaring acceptsagentcredential.
AGENT_CREDENTIAL_HEADER = 'X-Endpoint-Credential'
# How a collector request authorized, recorded on g so a route can tell a
# managed service token from the shared env key after the check has passed.
AUTH_MANAGED_TOKEN = 'token'
AUTH_SHARED_KEY = 'sharedkey'
# The four legacy routes below (/pc, /apps, /heartbeat, /bulk) predate both the
# generic collector contract and per-PC credentials. They take a hostname from
# the body and rewrite that PC's logged-in user, boot time, serial number and
# installed-app inventory, so the shared key is an any-host write key on them.
# plugin.endpoints.sharedkeyallowed does not reach here - it gates the endpoints
# collector, not core - so turning it off leaves these open. This setting is the
# core counterpart. Default false = today's behaviour, unchanged.
LEGACY_SHARED_KEY_SETTING = 'collector.legacysharedkeyrefused'
# Declared here so the wording stays with the code that enforces it.
# build_default_settings (shopdb/core/api/settings.py) has to include this dict
# for the row to be seeded; until it does, the key is created on demand through
# the settings API and the absent-row default below is what applies.
LEGACY_SHARED_KEY_SETTING_DEFAULT = {
'key': LEGACY_SHARED_KEY_SETTING,
'value': 'false',
'valuetype': 'boolean',
'category': 'collector',
'description': 'Refuse the shared collector key on the legacy core routes '
'POST /api/collector/pc, /apps, /heartbeat and /bulk. Those '
'routes name their target PC in the body, so any holder of '
'the shared key can rewrite logged-in user, last boot time, '
'serial number and installed apps for ANY hostname. Turning '
'this on closes them to the shared key; a collector-scoped '
'managed token still works. Turn it on only after checking '
'the "Legacy collector route used with the shared key" '
'warnings in the log for a while: whatever still calls them '
'stops working the moment it is on. The two GE-Enforce '
'reporters post to /api/collector/computers and '
'/api/collector/printers and are NOT affected. This should '
'follow the move to per-PC credentials, not precede it.',
}
def _computer_models():
"""Lazily import the computers plugin models.
@@ -131,13 +169,17 @@ def _check_collector_auth(expected_key, pluginname=None):
api_key = request.headers.get('X-API-Key')
g.collectorauth = None
token = _collector_managed_token(api_key)
if token is not None and _token_is_collector(token, pluginname):
touch_apitoken_lastused(token)
g.collectorauth = AUTH_MANAGED_TOKEN
return None
if expected_key and api_key == expected_key \
and not _env_fallback_disabled(pluginname):
g.collectorauth = AUTH_SHARED_KEY
return None
# Fail-closed only on pure server misconfiguration: no env key AND the
@@ -164,6 +206,54 @@ def require_api_key(f):
return decorated
def _legacy_shared_key_refused():
"""True when the site has closed the legacy routes to the shared key.
Absent setting = false = the routes stay open, which is what every site
does today. Read tolerantly because the row can be created by hand through
the settings API, where it may land with valuetype string.
"""
from shopdb.core.models.setting import Setting
value = Setting.get(LEGACY_SHARED_KEY_SETTING, default=False)
if isinstance(value, str):
return value.strip().lower() in ('true', '1', 'yes')
return bool(value)
def require_api_key_legacy(f):
"""require_api_key plus the legacy-route shared-key gate.
These routes take their target hostname from the body, so the shared key is
an any-host write key on them and no per-PC credential narrows it. A site
that has finished the move to per-PC credentials can shut it off here
without waiting for the routes to be retired.
Every shared-key use is logged whether the gate is on or off, so a site can
read the warnings and learn what still depends on the key BEFORE flipping
it - otherwise turning it on is a guess.
"""
from shopdb.utils.clientip import client_ip
@wraps(f)
def decorated(*args, **kwargs):
error = _check_collector_auth(current_app.config.get('COLLECTOR_API_KEY'))
if error is not None:
return error
if getattr(g, 'collectorauth', None) == AUTH_SHARED_KEY:
if _legacy_shared_key_refused():
return error_response(
'SHARED_KEY_REFUSED',
'The shared key is not accepted on the legacy collector '
'routes; use a collector-scoped token',
http_code=401)
current_app.logger.warning(
'Legacy collector route used with the shared key: %s from %s',
request.path, client_ip(request))
return f(*args, **kwargs)
return decorated
def _find_pc(hostname):
"""Find a computer by hostname, falling back to its asset number.
@@ -347,7 +437,7 @@ def collector_schemas():
@collector_bp.route('/pc', methods=['POST'])
@require_api_key
@require_api_key_legacy
def update_pc_info():
"""Update one PC from the collection script (matched by hostname)."""
data = request.get_json()
@@ -385,7 +475,7 @@ def update_pc_info():
@collector_bp.route('/apps', methods=['POST'])
@require_api_key
@require_api_key_legacy
def update_installed_apps():
"""Update installed applications for a PC (matched by hostname)."""
data = request.get_json()
@@ -457,7 +547,7 @@ def update_installed_apps():
@collector_bp.route('/heartbeat', methods=['POST'])
@require_api_key
@require_api_key_legacy
def pc_heartbeat():
"""Record PC online status / heartbeat (single or batch)."""
data = request.get_json()
@@ -492,7 +582,7 @@ def pc_heartbeat():
@collector_bp.route('/bulk', methods=['POST'])
@require_api_key
@require_api_key_legacy
def bulk_update():
"""Bulk update multiple PCs at once."""
data = request.get_json()

69
shopdb/core/api/settings.py
View File

@@ -161,14 +161,50 @@ def _declared_default(key: str) -> dict:
return _plugin_declared_settings().get(key, {})
def _is_secret(key: str) -> bool:
return 'password' in key or 'token' in key or 'secret' in key
def _is_secret(key: str, valuetype=None) -> bool:
"""True when a key's value must never leave the API in plaintext.
The test is the key NAME, which is a heuristic, so it is narrowed by the
stored type when the caller knows it. A boolean, integer or float setting
cannot hold a secret, and masking one breaks the control that renders it:
plugin.endpoints.bootstrapquerytoken is a true/false switch whose name
contains 'token', and the settings page read back '********' instead of
its state, so the checkbox could not show whether it was on.
valuetype omitted means "judge by name alone", which is the safe direction:
an unknown type is treated as a string.
"""
named = 'password' in key or 'token' in key or 'secret' in key
if not named:
return False
return valuetype is None or str(valuetype).lower() == 'string'
def _inferred_valuetype(value) -> str:
"""Valuetype for a key nobody declares, from the JSON type sent.
Only reached on the upsert path in update_setting. JSON already carries the
distinction the row has to keep, so honour it: true/false is a boolean row,
a number is integer or float, everything else is a string. Guessing from
the TEXT (e.g. the string 'true') is deliberately not done - a genuine
string setting whose value happens to read 'true' would then be typed
boolean and coerced out from under its reader.
bool is checked before int because bool subclasses int in Python.
"""
if isinstance(value, bool):
return 'boolean'
if isinstance(value, int):
return 'integer'
if isinstance(value, float):
return 'float'
return 'string'
def _serialize_setting(setting):
"""Serialize a setting, masking secret values so they never leave the API."""
data = setting.to_dict()
if _is_secret(setting.key):
if _is_secret(setting.key, setting.valuetype):
data['value'] = SECRET_MASK if setting.value else ''
return data
@@ -795,7 +831,14 @@ def update_setting(key: str):
setting = Setting(
key=key,
value='',
valuetype=declared.get('valuetype', 'string'),
# An undeclared key used to be born valuetype='string'
# unconditionally, and that was a trap: a caller PUTting JSON true
# got a string row, get_typed_value then handed every reader the
# text 'true', and a reader doing bool(...) or `is True` silently
# disagreed with the UI. Infer from the JSON type the client sent
# instead, so a boolean key is born boolean.
valuetype=declared.get('valuetype') or _inferred_valuetype(
data.get('value')),
category=declared.get('category', 'plugin'),
description=declared.get('description'),
)
@@ -808,7 +851,7 @@ def update_setting(key: str):
# A secret submitted as the mask placeholder means "leave unchanged" - the
# client only ever received the mask, so don't overwrite the real secret.
if _is_secret(key) and value == SECRET_MASK:
if _is_secret(key, setting.valuetype) and value == SECRET_MASK:
return success_response(_serialize_setting(setting), message='Setting unchanged')
# The declared valuetype is a promise about what every reader will get, and
@@ -846,7 +889,7 @@ def update_setting(key: str):
setting.value = str(value) if value is not None else None
# Audit log (mask sensitive values)
is_sensitive = _is_secret(key)
is_sensitive = _is_secret(key, setting.valuetype)
AuditLog.log('updated', 'Setting', entityname=key, changes={
'value': {
'old': '***' if is_sensitive else old_value,
@@ -915,10 +958,13 @@ def create_setting():
else:
value_str = str(value) if value is not None else None
# Same reasoning as the upsert in update_setting: a caller that omits
# valuetype and sends JSON true must not get a string row that every
# boolean reader then misreads.
setting = Setting(
key=data['key'],
value=value_str,
valuetype=data.get('valuetype', 'string'),
valuetype=data.get('valuetype') or _inferred_valuetype(value),
category=data.get('category', 'general'),
description=data.get('description')
)
@@ -1268,7 +1314,16 @@ def build_default_settings():
# Collector pc-type -> ComputerType mapping is computers-plugin domain;
# the plugin seeds pctypemap_<pxetype> settings on install.
# Collector authentication. The wording lives with the code that enforces
# it (shopdb/core/api/collector.py); seeding it here is what makes the row
# exist, and the row is what the settings UI can render a control for.
# Imported inside the function because core.api.__init__ imports collector
# and settings in the same package - a module-level import would couple
# their import order for no gain.
from shopdb.core.api.collector import LEGACY_SHARED_KEY_SETTING_DEFAULT
defaults = sitedefaults + brandingdefaults + printingdefaults + identifierdefaults + searchdefaults + mapdefaults + [
LEGACY_SHARED_KEY_SETTING_DEFAULT,
# ServiceNow ticket links. Each instance points these at its own
# ServiceNow tenant; blank/disabled renders tickets as plain text.
#

64
shopdb/utils/ipallowlist.py Normal file
View File

@@ -0,0 +1,64 @@
"""Network-position allowlists for unauthenticated kiosk routes.
A kiosk route writes without a login. The device token (see the kiosk views)
covers unattended screens an administrator provisions once, but it does not fit
the other caller: somebody opening the kiosk page on their own PC, who never
logs in and whose browser nobody provisioned. Per-browser provisioning would
mean touching every PC on the floor and losing it again on the next reimage.
What those callers have in common is where they are on the network.
Fails closed, in every direction that matters:
- an empty or absent setting matches nothing (the feature is off),
- an unparseable caller address matches nothing,
- a malformed entry in the list is skipped rather than widening the match.
SPOOF RESISTANCE IS A DEPLOYMENT PROPERTY, NOT A PROPERTY OF THIS CODE.
The caller address comes from client_ip(), which reads remote_addr and never
the raw X-Forwarded-For header - reading that header directly would let any
caller send 'X-Forwarded-For: <allowlisted-ip>' and walk straight through.
remote_addr is only the real client because of what sits in front of the app:
behind IIS the URL-Rewrite rule overwrites X-Forwarded-For with the real TCP
peer and ProxyFix resolves it into remote_addr (TRUSTED_PROXY_COUNT). Remove
that rule and IIS forwards whatever the caller sent; waitress trusts it because
it arrives from 127.0.0.1, remote_addr becomes attacker-controlled, and every
allowlist built on this becomes bypassable from anywhere on the network. It
does NOT fail closed in that case. The rewrite rule is the control. The
GE-Enforce client allowlist carries the same dependency.
Callers store plain CIDR strings rather than references to subnet records: a
site that does not run the network plugin still has to be able to configure
this, and a stored allowlist must not break when an inventory row is deleted.
"""
import ipaddress
from shopdb.utils.clientip import client_ip
def ip_in_cidrs(address, rawcidrs):
"""True when `address` falls inside any CIDR in the comma-separated list.
Pure and side-effect free so it can be tested without a request context.
"""
raw = (rawcidrs or '').strip()
if not raw:
return False
try:
caller = ipaddress.ip_address((address or '').strip())
except ValueError:
return False
for part in raw.split(','):
part = part.strip()
if not part:
continue
try:
if caller in ipaddress.ip_network(part, strict=False):
return True
except ValueError:
continue
return False
def request_ip_allowlisted(request_obj, rawcidrs):
"""True when the request's client address is inside the allowlist."""
return ip_in_cidrs(client_ip(request_obj), rawcidrs)