Remediate the 2026-09-03 feature-test findings

This commit is contained in:
2026-09-03 17:14:53 -04:00
parent ddf41c3e65
commit ff9ae693a6
112 changed files with 5182 additions and 857 deletions

View File

@@ -993,16 +993,30 @@ def kiosk_checkout():
'Badge not recognized - see IT to check this device out',
http_code=422)
# Claim the device FIRST, and let the status transition be the
# guard: the read-then-write above cannot stop two scans of the
# same drive that interleave, because both read 'available'. With
# the status in the WHERE clause the second UPDATE matches no row
# and there is no log entry to undo.
cur.execute(
'UPDATE devices SET status = %s '
'WHERE device_id = %s AND status <> %s',
(_STATUS_CHECKED_OUT, device_id, _STATUS_CHECKED_OUT),
)
if not cur.rowcount:
holder = _kiosk_holder_name(cur, device_id)
conn.rollback()
return error_response(
ErrorCodes.CONFLICT,
'That device is already checked out{}'.format(
' to {}'.format(holder) if holder else ''),
http_code=409)
cur.execute(
'INSERT INTO checkinoutlog '
'(badge_number, device_id, action, timestamp, locker_location) '
"VALUES (%s, %s, 'check-out', %s, %s)",
(badge, device_id, _now(), device.get('locker_location')),
)
cur.execute(
'UPDATE devices SET status = %s WHERE device_id = %s',
(_STATUS_CHECKED_OUT, device_id),
)
conn.commit()
finally:
conn.close()

View File

@@ -33,18 +33,56 @@ _FILTER = {'available': STATUS_CHECKED_IN, 'checkedout': STATUS_CHECKED_OUT, 're
def _resolve_name(sso):
"""Best-effort name from the self-hosted employee directory."""
"""Best-effort name from the self-hosted employee directory.
The badge goes through the shared shape rule first. This compared the raw
string, so the PayNo wrapper some scanners emit (0<digits>BZ) resolved to
nobody and the kiosk answered "Badge not recognized - see IT" - for a badge
the parts kiosk a few metres away accepted. It also used str.isdigit(),
true for Arabic-Indic digits, so a badge in those characters reached int()
and matched a real employee.
"""
from shopdb.api import BadgeShapeError, normalise_badge
try:
digits = normalise_badge(sso)
except BadgeShapeError:
return None
try:
from plugins.employees.models import DirectoryEmployee
if sso and str(sso).isdigit():
emp = db.session.get(DirectoryEmployee, int(sso))
if emp:
return f'{emp.firstname} {emp.lastname}'.strip()
emp = db.session.get(DirectoryEmployee, int(digits))
if emp:
return f'{emp.firstname} {emp.lastname}'.strip()
except Exception:
pass
return None
def _kiosk_badge_digits(raw):
"""Return (digits, errorresponse) for a scanned badge.
Every kiosk write stores the DIGITS rather than the string that arrived:
the PayNo wrapper (0<digits>BZ) and the bare number are the same person,
and recording one row under each spelling makes a custody history that
cannot be joined to itself.
An empty scan and an unreadable one get different answers, because they
are different problems for the person standing at the cabinet: one has not
scanned yet, the other has scanned something this site does not know.
"""
from shopdb.api import BadgeShapeError, normalise_badge
if not (raw or '').strip():
return None, error_response(ErrorCodes.VALIDATION_ERROR,
'Scan your badge')
try:
return normalise_badge(raw), None
except BadgeShapeError:
return None, error_response(
ErrorCodes.VALIDATION_ERROR,
'Badge not recognized - see IT', http_code=422)
def _status(device):
if not device.isactive:
return STATUS_RETIRED
@@ -307,9 +345,9 @@ def kiosk_device(device_id):
def kiosk_session(data):
"""See routes.kiosk_session for why the list costs a valid badge."""
badge = (data.get('badge') or '').strip()
if not badge:
return error_response(ErrorCodes.VALIDATION_ERROR, 'Scan your badge')
badge, badgeerror = _kiosk_badge_digits(data.get('badge'))
if badgeerror:
return badgeerror
name = _resolve_name(badge)
if not name:
return error_response(ErrorCodes.VALIDATION_ERROR,
@@ -335,9 +373,9 @@ def kiosk_session(data):
def kiosk_others(data):
badge = (data.get('badge') or '').strip()
if not badge:
return error_response(ErrorCodes.VALIDATION_ERROR, 'Scan your badge')
badge, badgeerror = _kiosk_badge_digits(data.get('badge'))
if badgeerror:
return badgeerror
if not _resolve_name(badge):
return error_response(ErrorCodes.VALIDATION_ERROR,
'Badge not recognized - see IT', http_code=422)
@@ -358,9 +396,9 @@ def kiosk_others(data):
def kiosk_checkout(data):
device_id = (data.get('device_id') or '').strip()
badge = (data.get('badge') or '').strip()
if not badge:
return error_response(ErrorCodes.VALIDATION_ERROR, 'Scan your badge')
badge, badgeerror = _kiosk_badge_digits(data.get('badge'))
if badgeerror:
return badgeerror
device = _find(device_id)
if not device:
@@ -387,12 +425,34 @@ def kiosk_checkout(data):
http_code=422)
now = datetime.now(timezone.utc).replace(tzinfo=None)
# The transition IS the guard. The ischeckedout test above reads, decides
# in Python and then writes, so two kiosk scans of the same drive that
# interleave both read "available" and both insert a custody row - one
# drive recorded out to two people, and the documented 409 never fires.
# Making the flag part of the WHERE clause moves the decision into the
# write, where the database settles it: the second UPDATE matches no row.
claimed = USBDevice.query.filter(
USBDevice.usbdeviceid == device.usbdeviceid,
USBDevice.ischeckedout == False).update(
{USBDevice.ischeckedout: True,
USBDevice.currentuserid: badge,
USBDevice.currentusername: name,
USBDevice.currentcheckoutdate: now},
synchronize_session=False)
if not claimed:
db.session.rollback()
# Re-read the row rather than refreshing the object this request is
# holding: the whole point of arriving here is that the object is
# stale, and the caller deserves the name of whoever actually has it.
holder = db.session.query(USBDevice.currentusername).filter(
USBDevice.usbdeviceid == device.usbdeviceid).scalar()
return error_response(
ErrorCodes.CONFLICT,
'That device is already checked out{}'.format(
' to {}'.format(holder) if holder else ''),
http_code=409)
db.session.add(USBCheckout(usbdeviceid=device.usbdeviceid, sso=badge,
checkoutname=name, checkouttime=now))
device.ischeckedout = True
device.currentuserid = badge
device.currentusername = name
device.currentcheckoutdate = now
db.session.commit()
return success_response({'device_id': device.serialnumber,
'holdername': name},
@@ -402,9 +462,9 @@ def kiosk_checkout(data):
def kiosk_checkin(data):
device_id = (data.get('device_id') or '').strip()
badge = (data.get('badge') or '').strip()
if not badge:
return error_response(ErrorCodes.VALIDATION_ERROR, 'Scan your badge')
badge, badgeerror = _kiosk_badge_digits(data.get('badge'))
if badgeerror:
return badgeerror
forsomeoneelse = bool(data.get('forsomeoneelse'))
device = _find(device_id)
@@ -439,15 +499,38 @@ def kiosk_checkin(data):
'holdername': device.currentusername or ''},
http_code=409)
open_checkout = (USBCheckout.query
.filter_by(usbdeviceid=device.usbdeviceid, checkintime=None)
.order_by(USBCheckout.checkouttime.desc()).first())
if open_checkout:
open_checkout.checkintime = datetime.now(timezone.utc).replace(tzinfo=None)
device.ischeckedout = False
device.currentuserid = None
device.currentusername = None
device.currentcheckoutdate = None
now = datetime.now(timezone.utc).replace(tzinfo=None)
# Guarded the same way as the checkout: only a device that is still out can
# be brought back, and the database decides that, not a Python read from
# earlier in this request.
released = USBDevice.query.filter(
USBDevice.usbdeviceid == device.usbdeviceid,
USBDevice.ischeckedout == True).update(
{USBDevice.ischeckedout: False,
USBDevice.currentuserid: None,
USBDevice.currentusername: None,
USBDevice.currentcheckoutdate: None},
synchronize_session=False)
if not released:
db.session.rollback()
return error_response(ErrorCodes.VALIDATION_ERROR,
'That device is not checked out')
# EVERY open row, not just the newest. One device can only be out once, so
# a second open row is corruption - closing only the newest one left the
# older ones open forever, and the device then reads as returned while its
# custody history says otherwise.
#
# checkinsso/checkinname record WHO HANDED IT BACK, which is not always the
# holder: returning for a colleague is a supported flow, and attributing
# the check-in to the holder made the audit trail name somebody who was not
# there.
(USBCheckout.query
.filter_by(usbdeviceid=device.usbdeviceid, checkintime=None)
.update({USBCheckout.checkintime: now,
USBCheckout.checkinsso: badge,
USBCheckout.checkinname: name},
synchronize_session=False))
db.session.commit()
return success_response({'device_id': device.serialnumber},
message='{} checked in'.format(device.serialnumber))

View File

@@ -0,0 +1,59 @@
"""Record who handed a drive back, not just when.
usbcheckouts held the checkout identity (sso, checkoutname) and, for the
return, only checkintime. Returning a drive for a colleague is a supported
kiosk flow - usb_kiosk_allow_return_for_others gates it and the kiosk asks for
confirmation - but the row it wrote attributed the check-in to the HOLDER. The
person who actually walked the drive back was recorded nowhere, so the custody
history and the audit report both named somebody who was not there.
Two columns rather than one: the badge is what the scanner emits and what joins
to a directory, and the name is what a report has to print when the directory
no longer resolves that badge.
Nullable, and not backfilled. A row closed before this column existed is one
where the returner was not recorded; copying the holder's badge into it would
manufacture evidence that the holder returned it, which is the exact claim this
migration exists to stop making.
Idempotent, so a re-run on an already-migrated database is a no-op.
Revision ID: usb0004checkinidentity
Revises: usb0003scanfields
"""
from alembic import op
import sqlalchemy as sa
revision = 'usb0004checkinidentity'
down_revision = 'usb0003scanfields'
branch_labels = None
depends_on = None
NEW_COLUMNS = (
('checkinsso', sa.String(length=20)),
('checkinname', sa.String(length=100)),
)
def upgrade():
bind = op.get_bind()
insp = sa.inspect(bind)
if 'usbcheckouts' not in insp.get_table_names():
return
cols = {c['name'] for c in insp.get_columns('usbcheckouts')}
for name, coltype in NEW_COLUMNS:
if name not in cols:
op.add_column('usbcheckouts',
sa.Column(name, coltype, nullable=True))
def downgrade():
bind = op.get_bind()
insp = sa.inspect(bind)
if 'usbcheckouts' not in insp.get_table_names():
return
cols = {c['name'] for c in insp.get_columns('usbcheckouts')}
for name, _ in NEW_COLUMNS:
if name in cols:
op.drop_column('usbcheckouts', name)

View File

@@ -128,6 +128,13 @@ class USBCheckout(BaseModel):
# Checkout details
checkouttime = db.Column(db.DateTime, nullable=False, default=_utcnow)
checkintime = db.Column(db.DateTime, nullable=True)
# WHO handed it back. Not always the holder: returning for a colleague is a
# supported flow, and recording the check-in against the holder named
# somebody who was not there. See migration usb0004checkinidentity.
checkinsso = db.Column(db.String(20), nullable=True,
comment='SSO of whoever returned it')
checkinname = db.Column(db.String(100), nullable=True,
comment='Name of whoever returned it')
# Metadata
checkoutreason = db.Column(db.Text, nullable=True, comment='Reason for checkout')