Expand test coverage for authentication routes (#565)#691
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the security-critical authentication surface by bringing the auth controller/middleware back under Jest coverage measurement and adding targeted unit + integration tests for local and EntraID login flows and JWT enforcement.
Changes:
- Jest: stop excluding
src/controllers/auth.tsandsrc/middleware/passport-auth.tsfrom coverage reporting. - Refactor for testability: export
checkTokenFitsInCookieand extract EntraID verify logic into an exportedentraIdVerify(openidConfig)factory. - Tests: add unit tests for
loginEntraIDandentraIdVerify, and extend integration coverage for/auth/localand additional JWT failure branches.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
jest.config.ts |
Removes auth controller/middleware from coverage ignore patterns so they’re measured. |
src/controllers/auth.ts |
Exports checkTokenFitsInCookie to enable direct unit testing. |
src/middleware/passport-auth.ts |
Extracts EntraID verify callback into exported entraIdVerify for unit testing. |
test/unit/controllers/auth.test.ts |
Adds unit coverage for cookie size helper + loginEntraID branches. |
test/unit/middleware/passport-auth.test.ts |
Adds unit coverage for entraIdVerify success/error branches. |
test/integration/routes/auth.test.ts |
Adds integration coverage for /auth/local and extra JWT rejection scenarios. |
Remove src/controllers/auth.ts and src/middleware/passport-auth.ts from coveragePathIgnorePatterns so the auth code is measured, and add the missing tests for the login flows and JWT strategy. - loginLocal: success sets a jwt cookie and redirects; missing/unknown user redirects with error=login - loginEntraID controller: provider error, no user, req.login failure, and success branches (passport.authenticate mocked) - entraIdVerify: extracted into an exported factory and unit-tested for match-by-provider-id, match-by-email, no-match, missing claims and lookup-error branches (openid-client and UserRepository mocked) - JWT middleware: expired token and permissions-changed both return 401 - checkTokenFitsInCookie: exported and unit-tested against the 4096-byte cap auth.ts reaches 100% and passport-auth.ts ~85% statement coverage.
- Move tokens.claims() and fetchUserInfo inside entraIdVerify's try/catch so a provider/network failure rejects the auth flow deterministically instead of escaping as an unhandled rejection (pre-existing on main). - Rename misleading passport-auth unit test and add a test for the fetchUserInfo rejection path. - Consolidate the JWT-strategy integration cases (missing/invalid/expired token, unknown user, changed permissions, 200) into auth.test.ts; keep a single 200 smoke test in healthcheck.test.ts.
b5e28fb to
dea734d
Compare
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
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.
Overview
Closes #565. Expands test coverage for the authentication routes, which were previously excluded from coverage measurement and largely untested.
Changes
Coverage config
src/controllers/auth.tsandsrc/middleware/passport-auth.tsfromcoveragePathIgnorePatternsinjest.config.tsso the auth code is actually measured.Source (behaviour-neutral refactors to enable testing)
checkTokenFitsInCookiefromauth.ts.passport-auth.tsinto an exportedentraIdVerify(openidConfig)factory;initEntraIdnow uses it. No behaviour change.Tests
test/unit/controllers/auth.test.ts(new): cookie size-cap helper (under/over 4096 bytes);loginEntraIDcontroller branches — provider error, no user,req.loginfailure, and success (setsjwtcookie + redirects).passport.authenticateis mocked.test/unit/middleware/passport-auth.test.ts(new):entraIdVerifybranches — missing sub/email, match by provider id, match by email, no match, and lookup error.openid-clientandUserRepositoryare mocked.test/integration/routes/auth.test.ts(extended):loginLocalviaGET /auth/local(success sets cookie + redirects; missing username and unknown user redirect with?error=login); JWT middleware gaps via/healthcheck/jwt(expired token → 401, permissions-changed-since-issue → 401).The existing no-token / invalid-token / unknown-user / valid JWT cases continue to live in
healthcheck.test.ts.Notes on EntraID testing
In the CI/test config only
AuthProvider.Localis enabled, so the/auth/entraid/callbackroute is not registered andinitEntraId()performs live OIDC discovery. EntraID is therefore covered by mocking (openid-client+passport) rather than over HTTP. The uncovered lines inpassport-auth.tsare the init/discovery paths that require a real provider.Coverage
src/controllers/auth.ts: 100% statementssrc/middleware/passport-auth.ts: ~85% statementsBoth above the issue's 80% target.
Verification
tscbuild, Prettier and ESLint all clean.