Second-pass review fixes: kill last hardcoded creds, wire get_services, dedup
Verification audit (re-run of the 6 skill lenses) confirmed the prior fixes hold and surfaced a few misses: Security (HIGH): - search.py _check_smart_redirect still opened a raw pymysql connection with root/rootpassword (reachable on any 9-digit SSO query). Now uses the shared env-backed employee_connection helper. - Deleted dead shopdb/core/services/employee_service.py (zero importers; carried another root/rootpassword literal). No hardcoded credentials remain in app logic; config.py dev defaults stay gated by ProductionConfig.validate. Dead hook: - get_services was implemented by the printers plugin but had no consumer (docs claimed otherwise). Added PluginManager.get_service(name) that resolves a service from enabled plugins; updated PLUGIN-HOOKS.md. Tests: - search disabled-plugin exclusion (the high-value gap): enabled plugin's hostname appears, disabled plugin's hostname drops out (searched by a hostname distinct from assetnumber so only the gated domain can match). - get_service consumer test (unknown name -> None). Simplify: - Extract the triplicated GE_LOGO_SVG + loadLogo + drawLogoOverlay into shared frontend/src/views/print/qrLogo.js (renderQrDataUrl); both QR views use it. - applications.py: lift the misplaced pagination import to the top; drop unused Computer unpacking in the 3 endpoints that only touch ComputerInstalledApp. 154 tests pass, naming/style green, app boots, QR render verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
43
tests/test_core/test_search_disabled.py
Normal file
43
tests/test_core/test_search_disabled.py
Normal file
@@ -0,0 +1,43 @@
|
||||
"""Global search must exclude rows from disabled plugins (search.py _require_enabled).
|
||||
|
||||
Pins the data-exposure property: disabling a plugin removes its plugin-specific
|
||||
matches (e.g. hostname) from /api/search, mirroring the plugin being absent.
|
||||
|
||||
Search by HOSTNAME (distinct from assetnumber) so only the gated hostname domain
|
||||
can match - the core asset-number domain is intentionally always-on.
|
||||
"""
|
||||
|
||||
|
||||
def _seed_computer(client, db, auth_headers, assetnumber, hostname):
|
||||
from shopdb.core.models import AssetType
|
||||
if not AssetType.query.filter_by(assettype='computer').first():
|
||||
db.session.add(AssetType(assettype='computer', pluginname='computer',
|
||||
tablename='computer', description='c'))
|
||||
db.session.commit()
|
||||
resp = client.post('/api/computers',
|
||||
json={'assetnumber': assetnumber, 'hostname': hostname},
|
||||
headers=auth_headers)
|
||||
assert resp.status_code == 201, resp.get_json()
|
||||
|
||||
|
||||
def _computer_hits(client, auth_headers, term):
|
||||
resp = client.get(f'/api/search?q={term}', headers=auth_headers)
|
||||
assert resp.status_code == 200, resp.get_json()
|
||||
results = resp.get_json()['data']['results']
|
||||
return [r for r in results if r.get('type') == 'computer']
|
||||
|
||||
|
||||
def test_search_includes_enabled_plugin(app, client, db, auth_headers, monkeypatch):
|
||||
"""An enabled plugin's hostname matches appear in search."""
|
||||
_seed_computer(client, db, auth_headers, 'AST-SD01', 'hostsearch01')
|
||||
pm = app.extensions['plugin_manager']
|
||||
monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True)
|
||||
assert _computer_hits(client, auth_headers, 'hostsearch01')
|
||||
|
||||
|
||||
def test_search_excludes_disabled_plugin(app, client, db, auth_headers, monkeypatch):
|
||||
"""A disabled plugin's hostname matches drop out of search results."""
|
||||
_seed_computer(client, db, auth_headers, 'AST-SD02', 'hostsearch02')
|
||||
pm = app.extensions['plugin_manager']
|
||||
monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: name != 'computers')
|
||||
assert _computer_hits(client, auth_headers, 'hostsearch02') == []
|
||||
@@ -137,6 +137,15 @@ def test_baseplugin_has_dashboard_widgets_hook():
|
||||
assert hasattr(BasePlugin, 'get_dashboard_widgets')
|
||||
|
||||
|
||||
def test_get_services_hook_has_consumer(app):
|
||||
"""get_services is consumed by plugin_manager.get_service (no dead hook)."""
|
||||
with app.app_context():
|
||||
pm = app.extensions['plugin_manager']
|
||||
assert hasattr(pm, 'get_service')
|
||||
# Unknown service name resolves to None, not an error.
|
||||
assert pm.get_service('definitely-not-a-real-service') is None
|
||||
|
||||
|
||||
def test_baseplugin_does_not_have_event_handlers_hook():
|
||||
"""The removed get_event_handlers hook must not be on BasePlugin."""
|
||||
assert not hasattr(BasePlugin, 'get_event_handlers'), (
|
||||
|
||||
Reference in New Issue
Block a user