Skip to content

fix(microbiology): secure analyzer event ingress - #4116

Open
pmanko wants to merge 9 commits into
feat/782-ogc-782-microbiology-r9-whonet-export-filtersfrom
feat/782-ogc-782-microbiology-r10-analyzer-ingress-security
Open

fix(microbiology): secure analyzer event ingress#4116
pmanko wants to merge 9 commits into
feat/782-ogc-782-microbiology-r9-whonet-export-filtersfrom
feat/782-ogc-782-microbiology-r10-analyzer-ingress-security

Conversation

@pmanko

@pmanko pmanko commented Aug 22, 2026

Copy link
Copy Markdown
Member

Stack

Behavior

  • Restricts normalized AST and culture analyzer-event writes to stateless HTTP Basic authentication and the existing Analyser Import role.
  • Denies browser-session authorization, missing or invalid credentials, and authenticated accounts without the role.
  • Records the authenticated machine account as the audit actor without creating an HTTP session.
  • Preserves the existing operator QC review and stuck-event reconciliation paths.
  • Uses the same authenticated-user resolver for order-entry test-section lookup, so request-scoped machine identity and an existing interactive session follow one path without creating a session.

The Bridge already supports configured outbound Basic credentials. Deployments must configure an OpenELIS account with the existing Analyser Import role; this PR does not add a role, hardcode an account, or claim to reroute existing raw ASTM/HL7 traffic through the normalized microbiology endpoints.

Development tooling repaired

  • Makes fresh-worktree submodule initialization deterministic and refuses destructive repair of populated checkouts.
  • Pins analyzer-mock-server#41, which makes per-analyzer Docker networks internal so they cannot replace the mock API container's default route on Docker Desktop.

Validation

  • Focused backend regression set: 19 tests pass across analyzer hold behavior, authenticated order-entry lookup, QA controller setup, and fixture-sequence synchronization.
  • Full frontend Vitest: 167 files; 1,094 tests pass and 13 are intentionally skipped.
  • Full backend Maven run: 5,242 tests passed and exposed one unrelated timezone-dependent alert test; the test now derives its cutoff from the persisted timestamp and its complete 9-test class passes.
  • The shared DbUnit loader now synchronizes observation_history_seq after explicit fixture IDs; a focused regression proves the next generated ID follows the loaded rows. No analyzer-specific sequence reset was added.
  • Registered deployed Playwright passes the analyzer AST QC review and unmatched-event reconciliation journeys without arbitrary waits or forced interactions.
  • python3 -m unittest scripts.tests.test_dev_stack
  • scripts/run-java21 mvn spotless:check
  • cd frontend && npm run check-format
  • Focused Playwright lint for the touched helper and journey.
  • OpenAPI contract parsed and verified for both secured endpoints.

CI repair

  • The test-only analyzer probe controller was being discovered by the full application component scan and colliding with the production import-issues route. It is now an explicitly registered test handler with no component stereotype.
  • Controller tests now model request-scoped authenticated identity instead of stubbing session creation.
  • The no-growth component test activates the Carbon action semantically; keyboard behavior remains covered by the dedicated microbiology accessibility Playwright suite.
  • The fixture loader, not an individual analyzer test, owns standalone-sequence synchronization.

Data and compatibility

  • No database migration.
  • No new role or authorization vocabulary.
  • No SQL-seeded feature fixtures, fixed feature primary keys, or DAO-bypassing feature setup.
  • Existing analyzer endpoints and interactive reconciliation access remain on their prior security chain.

Review evidence

  • specs/782-ogc-782-microbiology-mvp-spec/evidence/code-qa-r10-2026-08-22.md
  • Live Grist/overlay story: AMR-S31 (R10 analyzer ingress security)
  • Exact deployed implementation SHA: 819a60fb919d31bbc6ceb025b2be18ec50035d96
  • Deployment: 20260822T175834Z-819a60fb919d, state ready; health and smoke verification passed.
  • Deployed route checks: / and /Microbiology/worklist return 200; application container is healthy.
  • Deployed Playwright: Analyzer AST results expose QC evidence, resolve explicitly, and become reviewable passes, including browser-session rejection with 401, machine submission, QC evidence, explicit override, acceptance, and reviewed state.
  • Deployed Playwright: Unmatched analyzer AST results remain visible for admin reconciliation passes through the configured Admin -> Stuck analyzer events path.

This PR contains the analyzer-ingress security behavior, shared fixture-loader repair, Playwright helper contract, and focused regression coverage described above; it is not a tasks-only update. The deployment evidence remains pinned to the exact implementation SHA above until these review fixes are deployed. Human UAT remains separate, and R10 is not marked [✓].

Known separate issue

During startup, the Horiba Pentra 60 analyzer CSV initializer attempts to create a duplicate LYM#(Whole Blood) test description and aborts that configuration transaction. The application still becomes healthy and both R10 journeys pass. This is not introduced by R10, but the analyzer configuration import should be made idempotent in separate work.

Known analyzer-navigation warnings (dynamic analyzer names used as translation identifiers and duplicate React menu keys) are documented in the evidence and are also not introduced by this slice.

@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 0d4e0b3 to 350dc56 Compare August 24, 2026 08:36
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 350dc56 to 1fd94e9 Compare August 24, 2026 13:25
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 1fd94e9 to b86972f Compare August 24, 2026 15:25
Copilot AI lite review requested due to automatic review settings August 24, 2026 15:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Secures normalized microbiology analyzer-event ingress by enforcing stateless HTTP Basic auth + existing “Analyser Import” authority, while updating tests/tooling and documenting the contract and roadmap status.

Changes:

  • Refactors backend user/audit resolution to prefer request-scoped identity without creating HTTP sessions; updates controller tests accordingly.
  • Updates Playwright microbiology seeding to use a storage-free Basic-auth client and adds coverage for browser-session rejection.
  • Repairs dev-stack submodule initialization determinism; updates OpenAPI/spec documentation and pins analyzer-mock-server tooling.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tools/analyzer-mock-server Pins mock-server submodule to updated revision.
src/test/java/org/openelisglobal/sampletyperequest/controller/SampleTypeRequestRestControllerTest.java Updates tests to model request-scoped user identity (no session).
src/test/java/org/openelisglobal/qachecklist/controller/SampleQaChecklistRestControllerTest.java Removes session mocking from QA checklist tests.
src/test/java/org/openelisglobal/microbiology/MicrobiologyArchitectureTest.java Broadens Liquibase-related exclusions in fixture SQL-guard architecture test.
src/test/java/org/openelisglobal/eqa/controller/rest/EQADistributionRestControllerTest.java Updates EQA tests to use request attributes instead of sessions.
src/test/java/org/openelisglobal/eqa/controller/EQAProgramRestControllerTest.java Updates EQA tests to use request attributes instead of sessions.
src/test/java/org/openelisglobal/eqa/controller/EQAMyProgramsRestControllerTest.java Updates EQA tests to use request attributes instead of sessions.
src/test/java/org/openelisglobal/eqa/controller/EQAEnrollmentRestControllerTest.java Updates EQA tests to use request attributes instead of sessions.
src/test/java/org/openelisglobal/common/util/ControllerUtillsTest.java Adds regression tests ensuring audit actor lookup does not create sessions.
src/test/java/org/openelisglobal/common/rest/provider/SampleEntryTestsForTypeProviderRestControllerTest.java Updates provider controller test to use request-scoped user identity.
src/test/java/org/openelisglobal/alert/service/AlertNotificationServiceTest.java Fixes timezone-dependent cutoff logic in alert test.
src/test/java/org/openelisglobal/FixtureLoaderSequenceSynchronizationTest.java Adds regression test for DbUnit fixture sequence synchronization.
src/main/java/org/openelisglobal/common/util/ControllerUtills.java Resolves sys user id from request attribute first, then existing session only.
src/main/java/org/openelisglobal/common/rest/provider/SampleEntryTestsForTypeProviderRestController.java Uses shared sys-user resolver rather than session-only lookup.
specs/782-ogc-782-microbiology-mvp-spec/tasks.md Marks analyzer ingress security slice as complete; updates iteration text.
specs/782-ogc-782-microbiology-mvp-spec/plan.md Documents Basic-auth + role approach for normalized ingress.
specs/782-ogc-782-microbiology-mvp-spec/evidence/code-qa-r10-2026-08-22.md Adds Code QA evidence writeup for the slice.
specs/782-ogc-782-microbiology-mvp-spec/contracts/microbiology-openapi.yaml Adds Basic security scheme + 401/403 responses for ingress endpoints.
scripts/tests/test_dev_stack.py Adds coverage for safe/forceful submodule initialization behavior.
scripts/dev-stack Implements safer deterministic per-submodule initialization with guarded force.
frontend/src/components/microbiology/tests/MicrobiologyCaseView.test.jsx Switches interaction from keyboard Enter to click for Carbon action.
frontend/playwright/tests/foundational/core/microbiology-worklist-grains.spec.ts Adds explicit 401 assertion for browser ingress attempt; updates helper calls.
frontend/playwright/helpers/seed-microbiology-data.ts Introduces storage-free Basic-auth request context for analyzer ingress seeding.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread frontend/playwright/helpers/seed-microbiology-data.ts
Comment thread src/test/java/org/openelisglobal/microbiology/MicrobiologyArchitectureTest.java Outdated
Comment thread specs/782-ogc-782-microbiology-mvp-spec/tasks.md
Comment thread specs/782-ogc-782-microbiology-mvp-spec/tasks.md
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from b86972f to b08cfa1 Compare August 24, 2026 17:06
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from b08cfa1 to 5209096 Compare August 24, 2026 17:26
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 5209096 to eb313e1 Compare August 24, 2026 18:57
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from eb313e1 to 6ef691e Compare August 24, 2026 19:40
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 6ef691e to c79cdb3 Compare August 24, 2026 21:47
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from c79cdb3 to 5513108 Compare August 24, 2026 22:01
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 5513108 to acc330d Compare August 24, 2026 22:15
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from acc330d to 3dec7b6 Compare August 24, 2026 22:41
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 8c7fc23 to b6eb880 Compare August 25, 2026 18:02
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from b6eb880 to 7de81e5 Compare August 25, 2026 18:10
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 7de81e5 to 28b99c0 Compare August 25, 2026 20:17
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 28b99c0 to dbb7c82 Compare August 25, 2026 22:16
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from dbb7c82 to 7ef3756 Compare August 25, 2026 23:34
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 7ef3756 to c705b8d Compare August 25, 2026 23:48
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from c705b8d to c0cf94a Compare August 25, 2026 23:56
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from c0cf94a to ef286dc Compare August 26, 2026 00:44
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from ef286dc to 3740325 Compare August 26, 2026 02:03
@pmanko
pmanko force-pushed the feat/782-ogc-782-microbiology-r10-analyzer-ingress-security branch from 3740325 to 3a0bc3c Compare August 26, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants