refactor(auth): injectable DB seam unlocks unit-level success-path tests (P5-M)#86
Merged
Merged
Conversation
…sts (P5-M) Architect audit P5-M. The last item on the audit issue (#73 GH / #50 Codeberg) closes here. Two changes to `app/middleware/auth.js`: 1. **Models instead of raw SQL.** Every DB hit in auth.js now goes through the Sequelize model layer (`ApiMaster.findOne`, `ApiKey.findOne`, `Customer.findByPk`, `PurchaseOrderVendor`, `PurchaseOrderHeader`, `Job` — the last two using `include`-based eager loading for the cascade lookups). The archive filter relies on P2-E's `defaultScope` instead of a repeated `<arch>: false` WHERE clause. Net effect: fewer hand-rolled SQL strings, identical semantics, and every code path is mockable from a test fixture. 2. **`_setDbForTesting(stub)` injection seam.** vitest's `vi.mock` does not reliably intercept this codebase's CJS `require()` chains — the model files use sequelize-cli's CJS conventions and `require('../config/db.config.js')` was capturing the real module before the mock could register. Late-binding via a `getDb()` accessor didn't fix it (vitest's mock layer still missed the late require). The smallest practical injection point is an explicit setter that overrides the lookup; tests call `auth._setDbForTesting({ ApiMaster: { findOne: vi.fn() }, ... })` to wire fixtures, and reset with `_setDbForTesting(null)` in afterEach. Production code MUST NOT call it — the underscore prefix flags the test-only contract. Tests: rewritten `tests/unit/auth.test.js` exercises 28 cases vs the previous 14. New coverage: - isMaster: success path "row found → returns true" + verifies the WHERE clause uses the hashed key (not the raw header value). - getCompanyId: success path "returns akCompanyId". - getCompanyIdByCustomerId / ByPovId / ByPohId / ByJobId: each helper has its own describe block covering both the success path and the "no row" / "broken FK" / "non-positive id" branches. - requireAuth: full strict-gate matrix (missing key / unknown key / master / scoped). Full suite: 420 pass / 4 skip (was 401/4 — +19 net new tests). Lint: 0 errors / 0 warnings. With P5-M shipping, every audit item from issue #73 / #50 is now landed. The audit tracker can be closed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the architect audit tracker (issue #73 — last item on the list).
Summary
Two changes in
app/middleware/auth.js:ApiMaster.findOne,ApiKey.findOne,Customer.findByPk,PurchaseOrderVendor.findByPk,PurchaseOrderHeader.findByPk+ eager-loaded vendor,Job.findByPk+ eager-loaded customer. P2-E defaultScope replaces the hand-rolled<arch>: falseWHERE clause._setDbForTesting(stub)seam. vitest'svi.mockdoes not reliably intercept this codebase's CJSrequire()chains. The setter is the smallest practical injection point. Tests callauth._setDbForTesting({ ApiMaster: { findOne: vi.fn() }, ... })to wire fixtures.Test plan
tests/unit/auth.test.jsrewritten to 28 cases (was 14). New coverage on the previously un-mockable success paths.npm run lint: 0 errors / 0 warnings.This code proudly made in Nebraska. GO BIG RED! 🌽 https://xkcd.com/2347/