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

Shut the parts kiosk behind a device token, and stop the health probe leaking

Browse Source 
Both from the second Darkwing scan. Every claim in it was verified against the
code before acting.

The kiosk was the product's only unauthenticated write, justified by a decision
record saying it was "physically rate-limited: it serves a touch screen on the
shop floor". Nothing in code enforced the physical half, and an HTTP route is
reachable from wherever the app is. What that left open:

  - _kiosk_find_item falls back to matching bare digits against the numeric
    tail of every active item, so 1, 2, 3 walk the catalogue without knowing a
    barcode;
  - a badge is a short numeric SSO, and the route answered 422 for an unknown
    one and 200 for a real one - an oracle for sorting guesses into employees;
  - the write then decremented stock and stamped the ledger, which IS the audit
    trail, with whatever SSO the caller supplied;
  - quantity was bounded only by what was on hand, so one request emptied a bin.

A badge is attribution, never authorization. Both kiosk routes now need the
site's kiosk flag AND a per-device token, and 404 rather than 403 without them
so a caller does not learn the door is there. The READ is gated too: gating only
the write would have left the enumeration oracle running against a kiosk a site
had switched off.

The token is per-DEVICE, not per-page, because a kiosk is a browser on a touch
screen and a token served inside the page is readable by anyone who can load it.
An admin opens the screen once as /parts-kiosk?token=..., the page moves it into
that browser's localStorage and strips it from the address bar.

Also: quantity is capped per take, JSON true no longer passes as 1
(isinstance(True, int) is True), and every badge failure on the anonymous route
returns one message. The gated restock and adjust handlers keep the specific
message - there the person is known and the detail is help, not an oracle. I
changed the wrong one of those three handlers first; caught by the test.

/api/dashboard/health formatted str(e) straight into the body of a route
PUBLIC_ENDPOINTS declares open because "monitoring has no account". A pymysql
OperationalError names the database account, the app host's address, the driver
and the failing statement. It is logged server-side now and the body carries a
coarse status. The version goes with it - it told an anonymous caller which
release to look up. Status stays 200 either way: a monitor keyed on the body's
status field is the contract here, and turning a degraded database into a
transport failure is a separate decision from not leaking the exception.

Nine tests. The three gate tests were confirmed to fail with the gate stubbed
out, rather than assumed to cover it. Suite 2325, frontend 316, build clean.

Scope: product
This commit is contained in:
cproudlock
2026-09-02 17:42:54 -04:00
parent 852aec5b7f
commit 744bc4341a
8 changed files with 295 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
tests/test_core/test_authz.py
View File

@@ -46,10 +46,14 @@ EXEMPT_BLUEPRINTS = {'auth', 'collector', 'setup'}
# the same blueprint are JWT+permission gated and ARE swept.
EXEMPT_ENDPOINTS = {'knowledgebase.track_click', 'users.update_user',
'geenforce.post_report',
# Deliberately open kiosk write: decrement-only,
# badge-attributed server-side. Decision record in
# docs/proposals/printedparts-plugin.md; justification in
# the plugin README.
# Kiosk write. No JWT - the people at the bin have no
# account - but no longer open: it needs the site's kiosk
# flag AND the per-device token (X-Kiosk-Token), and 404s
# without them. The badge only attributes the ledger row.
# The old "physically rate-limited by being a touch screen"
# justification did not survive a scanner: bare digits
# enumerated the catalogue and the 422/200 split was an
# oracle for real employee numbers.
'printedparts.kiosk_take',
# Deliberately open kiosk writes. NOT the same bar as the
# parts kiosk, and the difference is the point: these are

3
tests/test_core/test_public_read_surface.py
View File

@@ -60,6 +60,9 @@ PUBLIC_ENDPOINTS = {
'slides.feed', 'slides.serve_image',
# Parts kiosk: one item by its code, and that item's picture.
# kiosk_item takes no JWT but is NOT open: same kiosk flag + device token
# as the write. Gating only the write would have left the enumeration
# oracle running against a kiosk a site had switched off.
'printedparts.kiosk_item', 'printedparts.serve_item_image',
# USB kiosk.

148
tests/test_plugins/test_printedparts_ledger.py
View File

@@ -121,13 +121,35 @@ def test_anonymous_cannot_mutate(client, item):
json={'quantity': 1, 'badge': '1'}).status_code == 401
KIOSK_TOKEN = 'kiosk-device-token-for-tests'
# The header a provisioned screen sends. A module constant rather than the
# fixture's return value, so a test does not have to name the fixture just to
# reach the header.
KIOSK_HEADERS = {'X-Kiosk-Token': KIOSK_TOKEN}
@pytest.fixture(autouse=True)
def kiosk(app, db):
"""Provision the kiosk the way an administrator would, and hand back the
header a provisioned screen sends. Without this every kiosk route 404s,
which is the point: the routes are the only unauthenticated write in the
product, so they are off until a site opts in."""
with app.app_context():
Setting.set('printedparts_kiosk_enabled', 'true')
Setting.set('printedparts_kiosk_token', KIOSK_TOKEN)
db.session.commit()
return KIOSK_HEADERS
def test_catalog_reads_require_view_permission(client, member_headers, item):
"""Browsing the catalog is printedparts.view-gated; the kiosk stays open."""
"""Browsing the catalog is printedparts.view-gated; a PROVISIONED kiosk
reads without an account."""
assert client.get('/api/printedparts/items').status_code == 401
assert client.get(f'/api/printedparts/items/{item}').status_code == 401
assert client.get('/api/printedparts/items',
headers=member_headers).status_code == 403
assert client.get('/api/printedparts/kiosk/item/3DP-9001').status_code == 200
assert client.get('/api/printedparts/kiosk/item/3DP-9001',
headers=KIOSK_HEADERS).status_code == 200
def test_member_without_permission_gets_403(client, member_headers, item):
@@ -139,29 +161,32 @@ def test_member_without_permission_gets_403(client, member_headers, item):
headers=member_headers).status_code == 403
def test_kiosk_take_is_open_decrement_only(client, auth_headers, app, item,
directory_employee):
"""The kiosk endpoint needs no auth but only ever decrements stock."""
def test_kiosk_take_is_decrement_only(client, auth_headers, app, item,
directory_employee, kiosk):
"""A provisioned kiosk needs no account but only ever decrements stock."""
itemcode = '3DP-9001'
stocked = client.post(f'/api/printedparts/items/{item}/restock',
json={'quantity': 5, 'badge': directory_employee},
headers=auth_headers)
assert stocked.status_code == 200
lookup = client.get(f'/api/printedparts/kiosk/item/{itemcode}', headers=auth_headers)
lookup = client.get(f'/api/printedparts/kiosk/item/{itemcode}', headers={**auth_headers, **KIOSK_HEADERS})
assert lookup.status_code == 200
take = client.post('/api/printedparts/kiosk/take', json={
'itemcode': itemcode, 'badge': directory_employee, 'quantity': 2}, headers=auth_headers)
'itemcode': itemcode, 'badge': directory_employee, 'quantity': 2},
headers={**auth_headers, **KIOSK_HEADERS})
assert take.status_code == 200, take.get_json()
assert take.get_json()['data']['quantityonhand'] == 3
too_many = client.post('/api/printedparts/kiosk/take', json={
'itemcode': itemcode, 'badge': directory_employee, 'quantity': 99}, headers=auth_headers)
'itemcode': itemcode, 'badge': directory_employee, 'quantity': 99},
headers={**auth_headers, **KIOSK_HEADERS})
assert too_many.status_code == 400
unknown = client.post('/api/printedparts/kiosk/take', json={
'itemcode': itemcode, 'badge': '111111111', 'quantity': 1}, headers=auth_headers)
'itemcode': itemcode, 'badge': '111111111', 'quantity': 1},
headers={**auth_headers, **KIOSK_HEADERS})
assert unknown.status_code == 422
with app.app_context():
@@ -197,7 +222,7 @@ def test_lowstock_alert_fires_on_crossing_only(client, auth_headers, app, item,
return client.post('/api/printedparts/kiosk/take',
json={'itemcode': '3DP-9001',
'badge': directory_employee,
'quantity': quantity}, headers=auth_headers)
'quantity': quantity}, headers={**auth_headers, **KIOSK_HEADERS})
restock(10) # 10 on hand, threshold 5
assert take(3).status_code == 200 # 7: above threshold, no alert
@@ -245,7 +270,7 @@ def test_alert_recipients_merge_users_and_freetext(client, auth_headers, app,
headers=auth_headers)
take = client.post('/api/printedparts/kiosk/take',
json={'itemcode': '3DP-9001',
'badge': directory_employee, 'quantity': 6}, headers=auth_headers)
'badge': directory_employee, 'quantity': 6}, headers={**auth_headers, **KIOSK_HEADERS})
assert take.status_code == 200 # 4 on hand: crossed threshold 5
assert captured['to'] == ['lead@site.test', 'extra@site.test']
@@ -260,7 +285,7 @@ def test_retire_hides_and_restore_returns(client, auth_headers, item):
listed = client.get('/api/printedparts/items',
headers=auth_headers).get_json()['data']
assert all(row['printeditemid'] != item for row in listed)
kiosk = client.get('/api/printedparts/kiosk/item/3DP-9001', headers=auth_headers)
kiosk = client.get('/api/printedparts/kiosk/item/3DP-9001', headers={**auth_headers, **KIOSK_HEADERS})
assert kiosk.status_code == 404
including = client.get('/api/printedparts/items?active=false',
@@ -270,7 +295,7 @@ def test_retire_hides_and_restore_returns(client, auth_headers, item):
assert client.post(f'/api/printedparts/items/{item}/restore',
headers=auth_headers).status_code == 200
assert client.get('/api/printedparts/kiosk/item/3DP-9001', headers=auth_headers).status_code == 200
assert client.get('/api/printedparts/kiosk/item/3DP-9001', headers={**auth_headers, **KIOSK_HEADERS}).status_code == 200
def test_file_revisions_append_and_download(client, auth_headers, item, tmp_path):
@@ -335,7 +360,7 @@ def test_alert_role_members_receive(client, auth_headers, app, item,
headers=auth_headers)
take = client.post('/api/printedparts/kiosk/take',
json={'itemcode': '3DP-9001',
'badge': directory_employee, 'quantity': 6}, headers=auth_headers)
'badge': directory_employee, 'quantity': 6}, headers={**auth_headers, **KIOSK_HEADERS})
assert take.status_code == 200
assert captured['to'] == ['crewone@site.test']
@@ -362,9 +387,9 @@ def test_gagelabtag_assigned_searched_and_kiosk_resolved(client, auth_headers):
headers=auth_headers).get_json()['data']
assert len(searched) == 1
by_tag = client.get('/api/printedparts/kiosk/item/WJRP0117', headers=auth_headers)
by_tag = client.get('/api/printedparts/kiosk/item/WJRP0117', headers={**auth_headers, **KIOSK_HEADERS})
assert by_tag.status_code == 200
by_digits = client.get('/api/printedparts/kiosk/item/117', headers=auth_headers)
by_digits = client.get('/api/printedparts/kiosk/item/117', headers={**auth_headers, **KIOSK_HEADERS})
assert by_digits.status_code == 200
assert by_digits.get_json()['data']['gagelabtag'] == 'WJRP0117'
@@ -380,7 +405,7 @@ def test_kiosk_take_records_revision_from_qr(client, app, db, directory_employee
iid = row.printeditemid
take = client.post('/api/printedparts/kiosk/take', json={
'itemcode': 'WJRP7000|3', 'badge': directory_employee, 'quantity': 1}, headers=auth_headers)
'itemcode': 'WJRP7000|3', 'badge': directory_employee, 'quantity': 1}, headers={**auth_headers, **KIOSK_HEADERS})
assert take.status_code == 200, take.get_json()
with app.app_context():
@@ -391,7 +416,7 @@ def test_kiosk_take_records_revision_from_qr(client, app, db, directory_employee
# Manual entry (no |rev) records a null revision, still resolves.
take2 = client.post('/api/printedparts/kiosk/take', json={
'itemcode': 'WJRP7000', 'badge': directory_employee, 'quantity': 1}, headers=auth_headers)
'itemcode': 'WJRP7000', 'badge': directory_employee, 'quantity': 1}, headers={**auth_headers, **KIOSK_HEADERS})
assert take2.status_code == 200
with app.app_context():
last = (PrintedItemTransaction.query
@@ -418,7 +443,92 @@ def test_lowstock_routes_to_support_team_webhook(client, app, db, directory_empl
with patch('shopdb.api.send_webhook') as mock_send:
# take 2 -> 6 crosses to 4 (<= threshold 5) -> alert fires
resp = client.post('/api/printedparts/kiosk/take', json={
'itemcode': 'WJRP7100', 'badge': directory_employee, 'quantity': 2}, headers=auth_headers)
'itemcode': 'WJRP7100', 'badge': directory_employee, 'quantity': 2}, headers={**auth_headers, **KIOSK_HEADERS})
assert resp.status_code == 200, resp.get_json()
assert mock_send.called
assert mock_send.call_args.kwargs['url'] == 'https://teams.example/3dprint'
# --- the kiosk is off until a site opts in -----------------------------------
#
# These are the cases the Darkwing scan turned into an exploit path. Every one
# of them returned 200 and wrote to the ledger before the gate existed.
def _clear_kiosk(app, db):
with app.app_context():
Setting.set('printedparts_kiosk_enabled', 'false')
Setting.set('printedparts_kiosk_token', '')
db.session.commit()
def test_kiosk_is_shut_when_the_site_has_not_enabled_it(client, app, db, item):
_clear_kiosk(app, db)
assert client.get('/api/printedparts/kiosk/item/3DP-9001').status_code == 404
assert client.post('/api/printedparts/kiosk/take',
json={'itemcode': '3DP-9001', 'badge': '1',
'quantity': 1}).status_code == 404
def test_kiosk_is_shut_without_the_device_token(client, item):
"""Enabled is not enough: an anonymous caller has no token."""
assert client.get('/api/printedparts/kiosk/item/3DP-9001').status_code == 404
assert client.post('/api/printedparts/kiosk/take',
json={'itemcode': '3DP-9001', 'badge': '1',
'quantity': 1}).status_code == 404
def test_kiosk_is_shut_with_the_wrong_device_token(client, item):
wrong = {'X-Kiosk-Token': KIOSK_TOKEN + 'x'}
assert client.get('/api/printedparts/kiosk/item/3DP-9001',
headers=wrong).status_code == 404
assert client.post('/api/printedparts/kiosk/take', headers=wrong,
json={'itemcode': '3DP-9001', 'badge': '1',
'quantity': 1}).status_code == 404
def test_a_single_take_cannot_drain_a_bin(client, app, item, auth_headers,
directory_employee):
"""quantity was bounded only by what was on hand, so one request emptied
the bin. It is capped per transaction now."""
from plugins.printedparts.api.routes import KIOSK_MAX_TAKE_QUANTITY
client.post(f'/api/printedparts/items/{item}/restock',
json={'quantity': KIOSK_MAX_TAKE_QUANTITY + 10,
'badge': directory_employee}, headers=auth_headers)
over = client.post('/api/printedparts/kiosk/take', headers=KIOSK_HEADERS,
json={'itemcode': '3DP-9001',
'badge': directory_employee,
'quantity': KIOSK_MAX_TAKE_QUANTITY + 1})
assert over.status_code == 400
assert str(KIOSK_MAX_TAKE_QUANTITY) in over.get_json()['data']['error']['message']
def test_true_is_not_a_quantity(client, item, directory_employee):
"""isinstance(True, int) is True in Python, so a JSON true used to sail
through the type check and take one."""
resp = client.post('/api/printedparts/kiosk/take', headers=KIOSK_HEADERS,
json={'itemcode': '3DP-9001',
'badge': directory_employee, 'quantity': True})
assert resp.status_code == 400
def test_the_badge_route_is_not_an_enumeration_oracle(client, item, auth_headers,
directory_employee):
"""An unknown badge and a malformed one must be indistinguishable, or the
route tells an anonymous caller which numbers are real employees.
Stock first: the on-hand check runs BEFORE the badge is resolved, so an
empty bin would answer 400 for both and the test would pass without ever
reaching the code it is about."""
client.post(f'/api/printedparts/items/{item}/restock',
json={'quantity': 5, 'badge': directory_employee},
headers=auth_headers)
unknown = client.post('/api/printedparts/kiosk/take', headers=KIOSK_HEADERS,
json={'itemcode': '3DP-9001', 'badge': '999999999',
'quantity': 1})
malformed = client.post('/api/printedparts/kiosk/take', headers=KIOSK_HEADERS,
json={'itemcode': '3DP-9001', 'badge': 'not-a-badge',
'quantity': 1})
assert unknown.status_code == malformed.status_code == 422
assert (unknown.get_json()['data']['error']['message']
== malformed.get_json()['data']['error']['message'])