diff --git a/docs/DUPLICATE-CODE-CLEANUP.md b/docs/DUPLICATE-CODE-CLEANUP.md new file mode 100644 index 000000000..fc5964125 --- /dev/null +++ b/docs/DUPLICATE-CODE-CLEANUP.md @@ -0,0 +1,234 @@ +# Duplicate code cleanup — safety-tiered review + +> **This is a research/documentation deliverable only.** No code has been +> changed as a result of this investigation. Every item below needs +> explicit sign-off before anyone touches it — the entire point of this +> document is to separate "safe to clean up" from "do not touch" *before* +> any refactor is attempted, so that closing SonarQube's duplication metric +> never comes at the cost of breaking a working production feature. + +## Headline numbers (SonarQube, at time of writing) + +| Metric | Value | +|---|---| +| Overall duplicated lines density | **26.8%** | +| Total duplicated lines | 12,389 | +| Total duplicated blocks | 6,058 | +| Total lines of code (ncloc) | 41,288 | +| Files with any duplication | 62 | + +Investigation method: pulled every file's duplication data from SonarQube's +`/api/duplications/show` endpoint (exact matched line ranges, not just the +density percentage), then read the actual source on both sides of every +match to determine whether the "duplicate" text is truly interchangeable +or only superficially similar. This mattered — several clusters that Sonar +reports as byte-identical duplicates turned out, on inspection, to already +contain real behavioral divergence (see Level 3 findings below). Sonar's +duplicate detector matches token sequences; it cannot tell that two +similar-looking blocks encode different business rules, so its raw +percentage is not a safe proxy for "how much of this could be deleted." + +--- + +## Special case #1: `src/utils/whitelistApis.ts` — NOT a cleanup candidate at all + +This single file accounts for **1,574 of the 12,389 duplicated lines +(12.7% of all duplication in the repo)**, at an internal duplication +density of **81.6%** (5,309 duplicate blocks inside one 1,928-line file). +It is the single largest contributor to the 26.8% figure by a wide margin. + +It is **not** copy-pasted logic — it is a large authorization table: +`API_LIST.URL` maps ~150+ literal request paths to +`{ checksNeeded: [...], ROLE_CHECK: [...] }` objects, consumed directly by +`src/utils/apiWhiteList.ts` via `_.get(API_LIST.URL, REQ_URL)` on **every +incoming request** — this is the live security gate for the whole app. + +The repeated *shape* (`{ checksNeeded: [CHECK.ROLE], ROLE_CHECK: [...] }`) +across hundreds of different URL keys is what Sonar is matching — but +each key is a **different actual security boundary**. There is no way to +"deduplicate" this that doesn't risk either deleting a security check for +one URL or silently applying one URL's role list to a different URL. + +**Verdict: do not include this file in any cleanup effort.** The +effort-to-risk ratio is about as bad as it gets in this codebase — the +only theoretically "safe" restructuring (grouping URLs by shared +role-config, e.g. `{ roles: [...], urls: [...] }`) would still require +changing `apiWhiteList.ts`'s lookup logic itself, which gates every +protected endpoint in the app. Recommend closing this out of scope +entirely rather than assigning it a tier. + +## Special case #2: dead code found during this investigation + +Three files turned out to be **unreferenced by the running application** — +discovered as a side effect of investigating their "duplication" with a +live sibling file. Deleting genuinely dead code is a different (and +generally much safer) kind of cleanup than de-duplicating live code, but +it still needs explicit confirmation that nothing external (docs, mobile +app clients, Postman collections, another service) targets these paths +before removal — that confirmation is a sign-off question, not something +resolvable by reading this repo alone. + +| File | Status | Evidence | +|---|---|---| +| `src/protectedApi_v8/socialv2.ts` | Not mounted anywhere | Only reference in `src/` is its own test file; `protectedApiV8.ts` only mounts `social.ts`'s `socialApi` | +| `src/protectedApi_v8/connections.ts` | Not mounted anywhere | Its import **and** its `.use()` call are both commented out in `protectedApiV8.ts` (lines 17, 90); only `connections_v2.ts` is live | +| `src/publicApi_v8/userDataMigration.ts` | Not mounted anywhere | Only `forgotPassword.ts` is `.use()`-d at `/forgot-password/`; `userDataMigration.ts` has no mount call. **Also contains its own bug** (see below) | + +**Important:** `userDataMigration.ts` isn't just dead — its `/verifyOtp` +route **skips real OTP verification entirely** and resets the user's +password unconditionally once the account is found, unlike the live +`forgotPassword.ts`, which correctly checks +`verifyOtpResponse.data.result.response === 'SUCCESS'` first. This is a +real OTP-bypass bug sitting in unreachable code today. If this file is +ever re-mounted for any reason, that bug must be fixed first. Recorded +here and cross-referenced in `docs/PROD-VERIFICATION.md`. + +`socialv2.ts` is also not a full superset/subset of `social.ts` — it's +missing ~10 routes `social.ts` has (moderator/admin/forum endpoints), so +even setting aside the dead-code question, it was never a drop-in v2 +replacement. + +--- + +## Level 1 — genuinely safe, zero prod impact + +Criteria: the duplicated block is provably identical in structure *and* +error handling on every side of the match, the only variance is a plain +string/constant (a URL, a log label, a field name) that can be passed as +a parameter, and no documented bug or security-sensitive logic overlaps +the block. + +| # | Files | What's duplicated | What varies | Proposed extraction | +|---|---|---|---|---| +| L1-1 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts`, `signupWithAutoLoginOrgForm.ts` | Postgres pool bootstrap (`new Pool({...})` + error/connect/remove handlers) | Nothing — byte-identical | Shared `pgPool.ts` module | +| L1-2 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | `API_END_POINTS`, `msg91Headers`, `indianCountryCode`, `registrationSource`, `standardDob`, `userSuccessRegistrationMessage` | Nothing — byte-identical | Shared constants module | +| L1-3 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | Joi validators for `phone`, `firstName`, `lastName`, `district`, `email` | Nothing — identical text incl. error messages | Shared Joi fragments (`requiredPhoneValidator`, etc.) | +| L1-4 | `signupWithAutoLogin.ts`, `signupWithAutoLoginV2.ts`, `appSignUpWithAutoLogin.ts` | `API_END_POINTS`/`msg91Headers`/`indianCountryCode` (3-file-identical subset only — `emailOrMobileLoginSignIn.ts`'s map differs, see caveat) | Nothing among these 3 | Shared constants module; leave `emailOrMobileLoginSignIn.ts`'s map separate | +| L1-5 | `signupWithAutoLogin.ts`, `signupWithAutoLoginV2.ts`, `appSignUpWithAutoLogin.ts` | `createAccount()` helper (POST `user/v3/create`) | Nothing | Shared helper, parameterized on `profileData` | +| L1-6 | `signupWithAutoLogin.ts`, `signupWithAutoLoginV2.ts`, `appSignUpWithAutoLogin.ts` | `profileUpdate()` helper | Nothing | Shared helper | +| L1-7 | `signupWithAutoLogin.ts`, `signupWithAutoLoginV2.ts`, `appSignUpWithAutoLogin.ts`, `emailOrMobileLoginSignIn.ts` | `fetchUserBymobileorEmail()` (GET `user/v1/exists/...`) | Nothing — byte-identical in all 4 | Shared helper, parameterized on `(searchValue, searchType)`. **Scope any extraction to just these 4 files** — the same function also appears in `maternityFoundationAuth.ts`/`tnnmcAuthV2.ts`/`tnnmcAuth.ts`/`sashaktAuth.ts`/`tnaiAuth.ts`, which are Level 3 (see below); don't chase the full 9-file blast radius in one change | +| L1-8 | `social.ts`, `socialv2.ts`, `connections.ts`, `connections_v2.ts` | Generic catch-block error boilerplate + auth-header-object construction | Log-tag string only | Shared `sendErrorResponse`/`buildAuthHeaders` helpers (independent of the dead-code question in Special Case #2) | +| L1-9 | `myAnalytics.ts` (self-duplication) | Generic catch-block tail (~24 occurrences) | Nothing | Shared error handler | +| L1-10 | `leaderboard.ts` (self-duplication) | 10 routes' proxy-POST boilerplate (`/fetchLeaderBoardDetails`, `/leaderboardActivities`, etc.) + `updateApprovedPoints`/`updateConfiguration` pair | Only the endpoint constant (and one hardcoded ID override) | `proxyPost(res, endpoint, userIdOverride?, req)` helper | +| L1-11 | `publicSearch.ts` ↔ `ratingsSearch.ts` | The "primary query" object literal + the ~100-line "query" branch (ES search → Postgres competency lookup → secondary search → merge) | Nothing — line-by-line identical incl. variable names | Shared `handleQuerySearch(...)` function | +| L1-12 | `content.ts` (self-duplication, ~15 routes) | Org/rootOrg 400-guard + generic catch block | Optional log-label string only | `requireOrgHeaders(req,res)` + `handleApiError(res,err,label?)` | +| L1-13 | `content.ts` ↔ `home.ts` ↔ `publicContent.ts` | `searchAutoComplete` handler, `searchV6` handler, response-shaping tail | Only the `uuid` source (`extractUserIdFromRequest` vs constant) — **see Level 2 note**, cross-boundary extraction of the full `searchAutoComplete` block itself is L2, but the smaller response-shaping tail alone is L1 | Split: shape-tail as its own small helper now; leave the full search handlers for the L2 workstream | +| L1-14 | `content.ts` (self-duplication) | `hierarchy/update` vs `kb/:updateType` handlers | Endpoint-builder function/param only | Mechanical merge | +| L1-15 | `goals.ts` ↔ `playlist.ts` | `PATCH /:goalId` vs `PATCH /:playlistId` (update-content-then-hierarchy flow) | Nothing meaningful — same imported functions, same hardcoded URLs | `patchContentViaHierarchyUpdate(id, request, auth)` shared helper | +| L1-16 | `goals.ts` (self-duplication, ~12 routes) | Org-guard + axios call + catch, across `/share`, `/action/...`, `/common`, `/track/...`, etc. | Verb, endpoint, optional response-transform function | `withOrgValidation(req, res, coreFn)` wrapper | +| L1-17 | `rdbms.ts` (self-duplication, ~9 routes) | GET-proxy and POST-proxy boilerplate — the cleanest file found in this whole investigation | URL suffix + log label only | `proxyGet`/`proxyPost` helper pair; would deduplicate nearly the entire file | +| L1-18 | `discussionHub/writeApi.ts` (self-duplication, excl. the `/users` route — see Level 3) | `getRootOrg`/`extractUserIdFromRequest`/catch boilerplate across 8 routes; `topics` create vs reply body-build | Log label only | Shared boilerplate helper — **exclude the `/users` route and its neighborhood, which touches the documented never-invoked-closure bug** | +| L1-19 | `discussionHub/users.ts` (self-duplication, 7 of 9 routes) | `/bookmarks`, `/downvoted`, `/groups`, `/info`, `/posts`, `/upvoted`, `/watched`, `/about` — identical proxy skeleton | Endpoint function + log label only | `proxySlugResource(endpointFn, label, req, res)` — **do not fold in `/me` or `/email/:email`**, the latter touches the documented closure bug | +| L1-20 | `follow.ts` (self-duplication, ~6 routes) | Org-guard + POST + generic catch, across follow/unfollow/followers/following | Endpoint constant + body-builder only | `postWithOrgGuard(endpoint, buildBody, req, res)` | +| L1-21 | `roleActivity.ts` — `getAllRoles()` | Repeated hardcoded role/activity object literals (static seed data, not request logic) | Literal string/id values only | Convert to a data array (`ROLE_SEED_DATA`) — zero runtime behavior change since it's static data either way | +| L1-22 | `recommendation.ts` (self-duplication) | Org/header-extraction guard; "check array → map to processContent → shuffle" idiom; generic catch — each repeated 2-3× across `/`, `/interestBased`, `/:recommendationType` | Nothing | `extractOrgHeaders`, `mapAndShuffleContent`, `handleRecommendationError` helpers | +| L1-23 | `feedbackV2.ts` — clusters 1 & 2 only (NOT cluster 3, see Level 3) | `/platform` vs `sendSentimentNeutralFeedback` middleware body-build; generic catch block | One extra field (`sentiment`, harmless if undefined) | `submitFeedback(req,res,extraFields)`, `handleFeedbackError(err,res)` | +| L1-24 | `scoring.ts` (self-duplication) | Auth-header-object construction + generic catch, across `/calculate`, `/fetch`, `getTemplate` | Endpoint constant + param name only | `scoringAuthHeaders(...)`, `handleScoringError(...)` | + +**Total: 24 clusters spanning roughly 20 files.** This is the safe, +immediately-actionable subset — pure boilerplate (error handling, header +construction, generic proxy-and-forward shapes, or literal static data) +with no documented bug and no verified behavioral divergence anywhere in +the matched text. + +--- + +## Level 2 — real impact, but plannable with care + +Criteria: the blocks are functionally equivalent *today*, but +consolidating them is a genuine refactor (parameterizing on more than a +plain constant, merging resource pools, or touching a live auth/business +path) rather than a mechanical no-op — it needs a behavior-preserving +design and regression testing before merging, not just a find/replace. + +| # | Files | What's duplicated | Why it's not Level 1 | What a safe plan looks like | +|---|---|---|---|---| +| L2-1 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | Joi `.when('role', ...)` conditional validators for `instituteName`/`instituteType`/`facultyType`/`courseSelection` | Text is identical today only because all 3 orgs happen to use the literal role names `'Student'`/`'Faculty'` — each file separately owns its full `role` enum elsewhere; a rename in one org would silently break a shared fragment | Extract only after confirming role-name stability across all 3 orgs' product roadmaps; add explicit tests per org before merging | +| L2-2 | `signupWithAutoLogin.ts` vs `signupWithAutoLoginV2.ts`/`appSignUpWithAutoLogin.ts`/`emailOrMobileLoginSignIn.ts` | `updateRoles()` helper | `signupWithAutoLogin.ts` uses `axiosRequestConfig` (default timeout); the other three use `axiosRequestConfigLong` — a real timeout-behavior difference, not cosmetic | Decide which timeout is correct for `signupWithAutoLogin.ts` before merging, don't silently homogenize | +| L2-3 | `emailOrMobileLoginSignIn.ts` (self-duplication, `/auth` vs `/authv2/*`) | Token-exchange-and-session-establishment tail (POST → jwt_decode → session/kauth wiring → `getCurrentUserRoles` → response) | Same file, same author, verified identical — but still a live auth path exercised by two different grant types (password vs authorization_code) | `exchangeTokenAndEstablishSession(transformedData, req, res)` helper taking the pre-built grant payload; add regression tests for both `/auth` and `/authv2` before merging | +| L2-4 | `connections.ts` ↔ `connections_v2.ts` (5 of the simpler routes: requested/received/established/established-by-id/add/update) | Byte-identical route bodies | `connections.ts` is dead today (Special Case #2), so there's no *live* second caller to preserve — flagged L2 rather than L1 only because re-enabling the commented-out v1 mount would create a route collision on the same sub-path | If keeping v1 alive is ever reconsidered, resolve the routing collision explicitly; otherwise this converges to "just delete the dead file" | +| L2-5 | `myAnalytics.ts` (self-duplication, ~24 of 28 routes) | Generic axios-forward-and-respond shape | 3-4 real outliers: `/myskills` uses a different userId-resolution fallback; `/assessments` & `/certification` reshape the response before sending; 2 routes use a middleware-chain pattern instead of inline axios | `forwardMyAnalytics(req,res,method,urlSuffix,queryParams?,body?,transform?)` — outliers get the optional `transform`/override params or are excluded entirely | +| L2-6 | `workallocation.ts` (self-duplication, 14 routes) | Validation-tail + header-object + response-forward + catch | Auth mechanism genuinely differs v1 (`extractAuthorizationFromRequest`, no token header) vs v2 (`SB_API_KEY` + `x-authenticated-user-token`) routes; validation target/presence differs per route; **a pre-existing buggy `logError(Error + err)` call must be preserved as-is, not silently "fixed"** during extraction | `forwardWorkAllocation(...,{authMode:'v1'|'v2', validate?})`; keep the buggy log call verbatim unless a separate, explicitly-approved bugfix is done first | +| L2-7 | `workflow-handler.ts` (self-duplication, 9 routes) | Header-object + response-forward + catch | POST routes validate org headers; GET routes don't — systematic, not random. `/workflowProcess` uniquely omits the `org` header entirely | `forwardWorkflow(...,{requireOrgValidation, includeOrgHeader?, includeWid?})` | +| L2-8 | `leaderboard.ts` — `badgeWon`/`badgeYetToWin` | Same request-building as the Level-1 leaderboard cluster | Extra response-processing branch (`processBadgeArray`/`processAllBadges`) and a different success-path response call (`res.send` vs `res.status().send()`) | Preserve the processing branch explicitly in the shared helper, don't drop it | +| L2-9 | `publicSearch.ts` ↔ `ratingsSearch.ts` | `postgresConnectionDetails`/`pool = new Pool(...)` config object | Data is identical, but merging means combining two separate live `pg.Pool` connection-pool instances into one — an operational change, not just a text edit | Confirm connection-pool sizing/behavior is equivalent before merging pools; test under load | +| L2-10 | `goals.ts` vs `playlist.ts` — create-goal vs create-playlist | Two-step "create content, then patch hierarchy" scaffolding | Request-builder functions genuinely differ (`formGoalRequestObj` vs `formPlaylistRequestObj`), and goals.ts's catch reshapes the error body via `transformGoalUpsertResponse` while playlist.ts's doesn't | Extract the scaffolding only, keep the builder functions and the extra error-transform as explicit parameters | +| L2-11 | `discussionHub/writeApi.ts` — `bookmark` vs `vote`, and `follow` vs `tags` | POST-body-build + axios + catch | `bookmark` discards the client body (`{_uid}` only) while `vote` forwards it (`{...req.body,_uid}`); `follow` calls `getUserUID()` (with a `// TODO` marker) while `tags` skips that step entirely | Preserve both differences as explicit parameters; do not assume they're accidental without asking the route owner | +| L2-12 | `home.ts` ↔ `content.ts` — `/searchAutoComplete` and `/searchV6` | ~80 and ~24 lines of live Elasticsearch query-building/ranking logic, effectively line-for-line identical | Only variance is how the acting "uuid" is resolved (hardcoded admin constant on the public/unauthenticated side vs `extractUserIdFromRequest` on the protected side) — but this crosses the public/protected trust boundary and the block is large, live search-ranking logic | `buildSearchAutoCompleteHandler(resolveUuid)` factory, instantiated once per file; treat as its own workstream, not a quick hoist | + +**Total: 12 clusters.** These are real, worthwhile dedup targets, but each +needs a short design note and test plan before touching — none of them +are "safe to just do." + +--- + +## Level 3 — do not touch without deep functional review + +Criteria: actual verified discrepancies were found between the "duplicate" +copies (different validation, different response contracts, different +error-handling behavior), or the block overlaps a documented bug, or it's +part of a family already known to have diverged (auth-provider token +exchanges, v1/v2 signup pairs) where trusting surface-level similarity has +previously led to missed bugs in this exact codebase. + +| # | Files | What looks duplicated | The actual discrepancy / risk found | +|---|---|---|---| +| L3-1 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | `accessDeniedMessage`, `getUserDesignationFromRole` | Different contact emails per org; entirely different role→designation maps per org (core business data) | +| L3-2 | `upsmfUser.ts`, `mpNHMUser.ts` | `UserDetails` interface tail, `ERHMS_CODE_KEY`/`GOV_KEY` | Surrounding interface fields differ per org; the "shared" constants don't even exist in `bnrcUser.ts` | +| L3-3 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | `role` Joi validator + nested conditional chains | Different `role.valid(...)` lists per org, different messages, extra branch in upsmf not present in mpNHM — core per-org validation logic | +| L3-4 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | `/createUser` post-existing-user branch | References already-divergent `accessDeniedMessage`; catch-block log text differs; control-flow shape matches but content doesn't | +| L3-5 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | `createUser()`/`assignRoleToUser()` helpers | **Bug found:** all three reuse `CONSTANTS.BNRC_USER_DEFAULT_PASSWORD` regardless of org — needs a naming/security decision. mpNHM alone adds a `timeout: 60000` the others lack | +| L3-6 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | `userProfileUpdate()` | Different professionalDetails field sets per org (`ERHMS_CODE_KEY`/`hrmsId` vs `bnrcRegistrationNumber`/`nin`), different state names, different designations per role per org — the most org-specific logic in the file set | +| L3-7 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | `updateUserStatusInDatabase()` | **Bug found:** `bnrcUser.ts`'s version `break`s out of its retry loop and unconditionally `return true`s — a fully-failed DB audit insert is reported as success, unlike upsmf/mpNHM which correctly return `false`. Also: different table names/column counts per org, and mpNHM alone converts `dob` via a leftover `cassandra-driver` type despite Cassandra not being used elsewhere in the file | +| L3-8 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | `migrateUserToX()` | **Bug found:** `mpNHMUser.ts` builds `` `India, , ${district}` `` — the state name is literally blank — vs upsmf's `"Uttar Pradesh"` and bnrc's `"Bihar"` | +| L3-9 | `upsmfUser.ts`, `mpNHMUser.ts`, `bnrcUser.ts` | OTP send/resend/validate handlers | **Bug found:** `mpNHMUser.ts`'s catch blocks use `logInfo` instead of `logError` for OTP failures — these won't surface in error-level monitoring/alerting the way upsmf/bnrc's do. Also copy-paste leftover log text referencing "BNRC" in the other two files' OTP handlers | +| L3-10 | `signupWithAutoLogin.ts`, `signupWithAutoLoginV2.ts`, `appSignUpWithAutoLogin.ts` | OTP-send-after-signup block | `appSignUpWithAutoLogin.ts` returns an extra `userUUId` field the other two don't — a real response-contract difference for any client parsing it | +| L3-11 | `signupWithAutoLogin.ts`, `signupWithAutoLoginV2.ts`, `appSignUpWithAutoLogin.ts` | Mobile-OTP-verify-via-MSG91 block | Sits inside `validateOtpWithLogin`, already documented (PROD-VERIFICATION.md changes Q/R) as having independently-occurring double-send bugs in 2 of these 3 files | +| L3-12 | `signupWithAutoLogin.ts`, `signupWithAutoLoginV2.ts`, `appSignUpWithAutoLogin.ts` | Email-OTP-verify + outer control flow of `validateOtpWithLogin` | Three genuinely different request-body contracts: different field names for phone (`phone` vs none vs `mobileNumber`) and for the user id (`userUUId`/`userUUID` vs `userId` vs `userId`/`userUUID`) | +| L3-13 | `signupWithAutoLogin.ts`, `signupWithAutoLoginV2.ts` | Keycloak token-exchange + session-establishment block | **Highest-risk cluster in the signup family.** Different `client_id`/`client_secret`/grant type (`portal`+password vs `aastrika-sso-login`+`offline_access`, no password); `appSignUpWithAutoLogin.ts`'s equivalent skips session/kauth entirely (stateless token passthrough) — three fundamentally different auth strategies masquerading as "the same block" | +| L3-14 | `publicSearch.ts` ↔ `ratingsSearch.ts` | "No query" search branch | publicSearch forces `contentType: ['Course','CourseUnit']` unconditionally (ratingsSearch doesn't), uses `limit:200` vs `limit:20`, and ratingsSearch alone calls `getCombinedRatingsResult` to enrich with ratings — real functional divergence | +| L3-15 | `discussionHub/writeApi.ts` — `/users` route neighborhood | Boilerplate adjacent to `createDiscussionHubUser` | Overlaps the documented `return async () => {...}` never-invoked-closure bug — the route always sends `undefined` and never actually creates the NodeBB user. A generic helper here would either mask or interact unpredictably with the existing bug | +| L3-16 | `feedbackV2.ts` — cluster 3 only | Boilerplate bracketing `GET /:feedbackId` and `GET /categories` | Directly brackets the two routes in the documented route-shadowing bug (`/categories` is unreachable, shadowed by `/:feedbackId`) — touching this risks silently "fixing" the shadowing as an unplanned refactor side effect | +| L3-17 | `network.ts` (self-duplication, 9 routes) | Org/user validation + header-object + response-forward | **Security-relevant divergence found:** `/connections/established/:id` derives the target `userId` from the **path parameter**, not from the authenticated caller — i.e. it looks up a *different* user's established connections by id, unlike every other route in the file. Separately, `/connections/recommended` and its `userDepartment` variant omit `Authorization`/`x-authenticated-user-token` entirely from the outbound call. Needs an explicit access-control review before any merge — cross-reference `docs/PROD-VERIFICATION.md` | +| L3-18 | `connections_v2.ts` — `suggests`/`recommended`/`recommended/userDepartment` | Same URL, "looks" like `connections.ts`'s dead v1 equivalent | Verified divergence: v1 used `extractUserIdFromRequest` (session-based fallback), v2 uses `extractUserId` (Keycloak-JWT-`sub`-based fallback) — two different user-identity resolution mechanisms. `userDepartment` additionally uses a completely different upstream API, request shape, and error guard between the two | +| L3-19 | `publicCertifcateFlinkv2.ts` ↔ `mobileAppApi.ts` | userid/courseid/secretKey extraction + Cassandra query + the critical secret-key check | **The documented CRITICAL auth-bypass bug (change AR) is inside this exact duplicated block**, copied verbatim into `mobileAppApi.ts`. Any fix must be applied and re-verified in both places, or the bypass persists via the second route | +| L3-20 | `tnaiAuth.ts`, `tnnmcAuth.ts`, `sashaktAuth.ts`, `maternityFoundationAuth.ts` (+ `tnnmcAuthV2.ts`, out of cluster scope) | The entire auth-provider-token-exchange family: createUser/userRoles/profileUpdate skeleton, JWT-decode+session block, qs.stringify+generateToken block | **Confirmed real divergence, not assumed:** on Keycloak-exchange failure, tnai/sashakt/tnnmc respond `302` but `maternityFoundationAuth.ts` responds `400` — inside a block Sonar reports as byte-identical. Also: HTTP verb differs (GET for sashakt, POST for the rest), token-transport-to-provider differs completely per provider (JSON body vs Bearer header vs APIM subscription key vs HMAC-signed custom headers), and each has unique side effects (Cassandra audit insert, name-splitting, designation-mapping, phone-normalization). `tnnmcAuth.ts` additionally has its own known-divergent sibling `tnnmcAuthV2.ts` from earlier in this campaign — first-party evidence this family regresses silently | +| L3-21 | `userDataMigration.ts` ↔ `forgotPassword.ts` | Nearly the entire file | Not a dedup candidate — it's a dead-code decision. The dead copy also **skips OTP verification entirely** (see Special Case #2) | + +**Total: 21 clusters.** Every one of these either has a confirmed, +verified discrepancy, overlaps a documented bug, or belongs to a family +this codebase has already seen regress silently. None should be touched +as part of a "reduce duplication" pass — several of them are, on their +own, legitimate bug-fix or security-review items independent of the +duplication question. + +--- + +## Recommended sequencing, if this work is approved + +1. **Do not touch `whitelistApis.ts`.** Not a cleanup target — see Special + Case #1. +2. **Resolve the dead-code question first** (Special Case #2) — confirming + whether `socialv2.ts`, `connections.ts`, and `userDataMigration.ts` can + be deleted removes ~2,000+ duplicated lines with the least behavioral + risk of anything in this document, since by definition nothing live + calls them today. This needs a sign-off confirming no external + consumer depends on those paths, not a code change. +3. **Level 1 (24 clusters)** is the safe next batch — pure boilerplate + extraction, zero behavior change, verified by direct line-by-line + reading, not just Sonar's density number. +4. **Level 2 (12 clusters)** should each get a short design note + test + plan before touching; do them one at a time, not as a batch. +5. **Level 3 (21 clusters)** should not be scheduled as "cleanup" at all. + Several items inside it (L3-5, L3-7, L3-8, L3-9, L3-17, L3-19) are + real bugs independent of duplication and belong in a bug-fix triage + process — cross-referenced into `docs/PROD-VERIFICATION.md` — with + their own sign-off, separate from any dedup initiative. + +**On the metric itself:** even completing every Level 1 and Level 2 item +above would not move SonarQube's 26.8% figure dramatically, because +`whitelistApis.ts` alone (which must stay untouched) accounts for 12.7% +of all duplication on its own. Recommend treating "reduce duplication +density" as a secondary benefit of a genuine code-quality pass, not the +primary goal — chasing the number itself would create pressure to touch +Level 3 code, which is exactly the risk this document exists to prevent. diff --git a/docs/PROD-VERIFICATION.md b/docs/PROD-VERIFICATION.md index 157ef18ef..32ed38885 100644 --- a/docs/PROD-VERIFICATION.md +++ b/docs/PROD-VERIFICATION.md @@ -2163,7 +2163,7 @@ worker or throw outside the test's control. --- -### BE. `extractUserIdFromRequest` can throw before any try/catch runs, in at least two route handlers (`activity.ts`, `network-hub.ts`) +### BE. `extractUserIdFromRequest` can throw before any try/catch runs, in at least three route handlers (`activity.ts`, `network-hub.ts`, `validate.ts`) ```ts // src/utils/requestExtract.ts @@ -2197,12 +2197,23 @@ object) for every request that reaches these protected routes — this would only manifest if session middleware were ever misconfigured, absent, or the session expired in a way that clears the object entirely, which is an infrastructure-level concern rather than a per-route bug. It is recorded -here because it was independently surfaced twice this campaign and the -helper itself has no defensive guard, so a future change to the session +here because it was independently surfaced multiple times this campaign and +the helper itself has no defensive guard, so a future change to the session setup could make it reachable. Not reproduced live (this is exactly the kind of "logic outside try/catch, throws synchronously in an async handler" hazard this campaign avoids reproducing). +A third, related instance: `user/validate.ts`'s `GET /` handler calls +`extractUserEmailFromRequest`, `extractUserNameFromRequest`, and +`extractUserIdFromRequest` with **no try/catch anywhere in the handler at +all** (not even one placed after these calls). The first two helpers +(`requestExtract.ts`) guard `req.kauth` truthiness but then dereference +`req.kauth.grant.access_token.content.name`/`.email` unconditionally — if +`req.kauth` is present but its nested shape is ever malformed, that throws +the same way. Currently unreachable in practice for the same reason as +above (the real Keycloak middleware always populates this shape +consistently), so not reproduced live. + **MUST VERIFY IN PROD:** - [ ] Confirm session middleware is always attached ahead of these routes in `server.ts`'s actual middleware order, guaranteeing `req.session` @@ -2319,6 +2330,436 @@ reproduced live — doing so would either hang or crash the Jest worker. --- +### BI. `bnrcUser.ts` — `updateUserStatusInDatabase()` reports success even when the audit-log DB insert fails after all retries + +*Found while investigating SonarQube code duplication between `bnrcUser.ts`, +`upsmfUser.ts`, and `mpNHMUser.ts` — see `docs/DUPLICATE-CODE-CLEANUP.md` +change L3-7.* + +```ts +try { + const maxRetries = 2 + let retryCount = 0 + while (retryCount < maxRetries) { + try { + await pgPool.query(pgQuery, pgParams) + break // success + } catch (queryError) { + retryCount++ + if (retryCount >= maxRetries) { + logError('PostgreSQL insert failed after max retries', ...) + break // <-- exhausted retries, but still `break`s + } + await new Promise((resolve) => setTimeout(resolve, waitTime)) + } + } +} catch (pgError) { + logError('Unexpected error in PostgreSQL insert', ...) +} +return true // <-- reached on BOTH success AND exhausted retries +``` + +Both the success path (`break` after a successful `pgPool.query`) and the +exhausted-retries path (`break` after the second failed attempt) fall +through to the same unconditional `return true` at the end of the +function. A fully-failed audit-log insert — i.e. the registration +succeeded upstream but the PostgreSQL audit row was never written after +both retries failed — is reported as `true` (success) to every caller. + +For comparison, the sibling files `upsmfUser.ts` and `mpNHMUser.ts` both +correctly `return false` inside the `if (retryCount >= maxRetries)` +branch of their equivalent function — `bnrcUser.ts` is the only one of +the three with this defect, confirmed by direct line comparison. + +Safe to state as fact (not speculative) since this was found by reading +the actual code, not by exercising it live. This is a data-integrity/ +observability issue, not a hang/crash/security bug, so it wasn't run +through the live-reproduction safety process used elsewhere in this +document — it's a straightforward logic bug. + +**MUST VERIFY IN PROD:** +- [ ] Check whether any BNRC registration audit rows are missing in + `bnrc_registration_data_prod` for registrations that otherwise + completed successfully — this function's `true` return means + callers have no way to detect that gap today. +- [ ] Confirm whether any monitoring/alerting depends on this function's + return value to detect audit-log write failures (currently it + cannot, for BNRC specifically). + +--- + +### BJ. `mpNHMUser.ts` — migrated users get a postal address with a blank state name + +*Found during the same duplication investigation — change L3-8.* + +```ts +// migrateUserToMp() — the migration path for existing aastrika/staging users +userProfileDetails.profileReq.personalDetails.postalAddress = + `India, , ${userFormDetails.district}` +``` + +The state segment of the address is empty. For comparison, the *new-user* +path in the same file (`userProfileUpdate()`) correctly builds +`` `India, Madhya Pradesh, ${user.district}` ``, and the equivalent +migration functions in `upsmfUser.ts`/`bnrcUser.ts` correctly say +`"Uttar Pradesh"`/`"Bihar"` respectively. Only the MP-NHM *migration* +function (used when an existing aastrika/staging-org user is being moved +into MP-NHM) has this blank-state defect — new MP-NHM signups are +unaffected. + +**MUST VERIFY IN PROD:** +- [ ] Check whether any migrated (not newly-signed-up) MP-NHM user + profiles have a postal address missing the state name, and whether + any downstream consumer (reporting, mailing, compliance) depends on + that field being populated. + +--- + +### BK. `mpNHMUser.ts` — OTP send/resend failures are logged at info level, not error level + +*Found during the same duplication investigation — change L3-9.* + +`sendOtp`'s and `resendOtp`'s catch blocks both call +`logInfo('Error in sending/resending user OTP' + error)` instead of +`logError(...)`. The file's own `validateOtp` route, and the equivalent +OTP handlers in `upsmfUser.ts`/`bnrcUser.ts`, all correctly use +`logError` for their failure paths. This means MP-NHM's OTP send/resend +failures don't surface the same way in any error-level log +monitoring/alerting that the other two orgs' failures do — the requests +still get a normal error HTTP response, this is purely an observability +gap. + +**MUST VERIFY IN PROD:** +- [ ] Confirm whether error-level alerting exists on this log stream, and + if so, add MP-NHM OTP send/resend failures to it (currently + invisible to anything filtering on `logError`). + +--- + +### BL. `upsmfUser.ts` / `mpNHMUser.ts` / `bnrcUser.ts` — all three orgs' default password uses a constant named for BNRC + +*Found during the same duplication investigation — change L3-5.* + +All three files' `createUser()` helper sets +`password: CONSTANTS.BNRC_USER_DEFAULT_PASSWORD` — including UPSMF and +MP-NHM, which are not BNRC. This is either an intentional shared default +across all three org-signup flows (in which case the constant is just +misleadingly named), or a copy-paste leftover where UPSMF/MP-NHM were +supposed to get their own distinct default password constants and never +did. Either way this deserves a deliberate decision rather than being +left as an artifact of copy-paste — not flagging this as a security +"bug" outright since it may be intentional, but the naming alone is +evidence it wasn't a deliberate, reviewed choice. + +**MUST VERIFY IN PROD:** +- [ ] Confirm with product/security whether UPSMF, MP-NHM, and BNRC are + intended to share one literal default password, and if so, rename + the constant to something org-neutral; if not, split it into three + distinct constants. + +--- + +### BM. `network.ts` — `GET /connections/established/:id` looks up connections for an arbitrary user id from the URL, not the caller + +*Found during the same duplication investigation — change L3-17.* + +```ts +networkConnectionApi.get('/connections/established/:id', async (req, res) => { + const rootOrg = req.headers.rootorg + const userId = req.params.id // <-- from the URL path, not the authenticated caller + ... + const response = await axios.get(apiEndpoints.getConnectionEstablishedData, { + ...axiosRequestConfig, + headers: { Authorization: CONSTANTS.SB_API_KEY, rootOrg, userId, 'x-authenticated-user-token': extractUserToken(req) }, + }) +``` + +Every other route in this file derives `userId` from the authenticated +caller via `extractUserIdFromRequest(req)`. This one route instead takes +it directly from the URL's `:id` path parameter — meaning any +authenticated caller can request **any other user's** established +connections list simply by changing the id in the URL, provided they +know or can guess a valid user id. The real `Authorization`/ +`x-authenticated-user-token` headers are still forwarded upstream, so +whether this is actually exploitable depends entirely on whether the +downstream Network Hub service (`NETWORK_HUB_SERVICE_BACKEND`) +independently re-validates that the token's identity matches the `userId` +header, or trusts this service's headers at face value. This repo cannot +resolve that from static reading alone — it's a genuine +"MUST VERIFY IN PROD" rather than a confirmed bypass. + +**MUST VERIFY IN PROD — urgent, potential IDOR:** +- [ ] Confirm whether the downstream Network Hub service validates that + the `x-authenticated-user-token`'s identity matches the `userId` + header on this specific endpoint, or trusts it unconditionally. +- [ ] If unconditional trust, this is a real cross-user data exposure — + the fix is deriving `userId` from `extractUserIdFromRequest(req)` + like every other route in this file, not from `req.params.id`. +- [ ] Determine whether `:id` was intentionally designed as an + admin/lookup feature (in which case it needs its own permission + check, currently absent) or is simply a copy-paste inconsistency + from the other routes in this file. + +--- + +### BN. `network.ts` — `/connections/recommended` and `/connections/recommended/userDepartment` omit auth headers that every other route in the file sends + +*Found during the same duplication investigation — change L3-17.* + +Every other route in `network.ts` forwards +`Authorization: CONSTANTS.SB_API_KEY` and +`'x-authenticated-user-token': extractUserToken(req)` to the upstream +Network Hub service. `/connections/recommended` and +`/connections/recommended/userDepartment` send only `{ rootOrg, userId }` +— no `Authorization`, no token. This may be intentional (a different +downstream contract for the recommendation lookup specifically), but +given every sibling route on the same backend does send these headers, +it reads as an inconsistency rather than a deliberate design choice. + +**MUST VERIFY IN PROD:** +- [ ] Confirm with the Network Hub service owner whether these two + routes are intentionally unauthenticated calls, or whether the + auth headers were simply dropped by accident when these routes + were added. + +--- + +### BO. `content.ts` — `POST /getWebModuleManifest` missing-`return` double-send on an empty `url` + +```ts +contentApi.post('/getWebModuleManifest', async (req, res) => { + try { + if (!req.body.url || !req.body.url.length) { + res.status(400).send() + } // <-- no return + const url = req.body.url // undefined if the check above fired + const response = await axios.get(`${url}`, axiosRequestConfig) + res.json(response.data) // second send once this resolves + } catch (err) { ... } +}) +``` + +Same missing-`return` family as changes Q/R/S/.../BG/BO: the empty-`url` +check sends a 400 with no `return`, so execution falls through to a real +`axios.get('undefined', ...)` call and sends a second response once it +resolves or rejects — a double-send crash. Found while extending this +file's test coverage from 71.56% to 94.24%; not reproduced live for the +reason above. + +**MUST VERIFY IN PROD:** +- [ ] Check application logs for `ERR_HTTP_HEADERS_SENT` (or equivalent) + originating from `POST .../content/getWebModuleManifest`. + +--- + +### BP. `emailOrMobileLoginSignIn.ts` — `/registerUserWithMobile` also has a missing-`return` double-send on a missing `phone`, separate from change E's hang + +Found while extending this file's test coverage (88.57% → 95.51%). This is +a *different* defect on the *same* route as change E above — E documents a +zero-response hang when the upstream create-user call fails; this is the +missing-`return` double-send family (same shape as changes Q/R/S/.../BO) +triggered by a missing `phone` field instead. `if (!req.body.phone) { +res.status(400)... }` has no `return`, so execution falls through into the +real create-user flow and can send a second response once that resolves. +Not reproduced live for the same reason as every other entry in this +family. Recording alongside change E since they share a route but are +independent bugs. + +**MUST VERIFY IN PROD:** +- [ ] Check application logs for `ERR_HTTP_HEADERS_SENT` (or equivalent) + originating from `POST .../registerUserWithMobile` with a missing + `phone` field specifically (distinct from change E's upstream-failure + hang scenario). + +--- + +### BQ. `profile-registry.ts` — `GET /getProfilePageMeta` returns function references instead of master-data lists; one of the five helpers also reads the wrong JSON key + +```ts +async function govtOrgMeta() { + return async () => { + const data = await fs.promises.readFile(...) + return JSON.parse(data.toString()) + } +} +// ... industreisMeta, degreesMeta, statesMeta, designationMeta — same shape + +profileRegistryApi.get('/getProfilePageMeta', async (req, res) => { + try { + const govtOrg = await govtOrgMeta().catch(...) // resolves to the inner + const industries = await industreisMeta().catch(...) // arrow fn itself, + ... // never invoked + res.json({ govtOrg, industries, degrees, states, designations }) + } catch (err) { ... } +}) +``` + +Each of the five meta helpers is `async function () { return async () => {...} }` +— the outer function returns the inner arrow function without ever calling +it. So `govtOrgMeta()` resolves to a *function value*, not the parsed JSON +list, and the five `.catch(...)` blocks plus the outer `catch` are genuinely +unreachable (a promise that only ever resolves can't reject). The practical +effect: `/getProfilePageMeta`'s response body ships non-serializable +function references in place of the `govtOrg`/`industries`/`degrees`/ +`states`/`designations` lists any consumer expects — this endpoint's actual +payload has likely never matched its intended shape. + +Separately, `statesMeta` (independent of the above, but moot while the +function is dead code) reads `obj.industries` from `states.json` instead of +`obj.states` — looks like a copy-paste bug from `industreisMeta`. + +Found while extending this file's test coverage from 51.79% to 96.41%; not +changed, since fixing the invocation would change the response shape/behavior +of a live endpoint. + +**MUST VERIFY IN PROD:** +- [ ] Call `GET .../user/getProfilePageMeta` against a real environment and + inspect the actual JSON response for `govtOrg`/`industries`/`degrees`/ + `states`/`designations` — confirm whether consumers already tolerate + (or silently ignore) non-list values, before touching this code. + +--- + +### BR. `apiWhiteList.ts` / `whitelistApis.ts` — `SCOPE_CHECK` is defined but never wired into any route, leaving a documented `MDO_ADMIN` restriction inert + +`whitelistApis.ts` declares `SCOPE_CHECK: [MDO_ADMIN]` on +`/protected/v8/workallocation/getWorkOrderById/:workOrderId`, and +`apiWhiteList.ts`'s `isAllowed()` only runs a check function when its +`CHECK` constant appears in that route's `checksNeeded` array — but across +the entire 1928-line whitelist config, every `checksNeeded` array is either +`[CHECK.ROLE]` or `[]`; `CHECK.SCOPE` never appears in any of them. So this +route is effectively protected by `ROLE_CHECK: [PUBLIC]` alone — the +org-scoped `MDO_ADMIN` restriction its own data implies was intended is +silently never enforced. Not a bug in `SCOPE_CHECK`'s own logic (it would +correctly reject a scope mismatch if it ran) — it's a wiring gap between +`whitelistApis.ts`'s data and how `checksNeeded` is populated. + +Found while extending `apiWhiteList.test.ts`'s coverage (81.96% → 83.6%; +most of the remaining gap is genuinely unreachable dead code, this being the +one live-relevant exception). Not changed — wiring in `CHECK.SCOPE` would be +a behavior change to a live authorization route. + +**MUST VERIFY IN PROD:** +- [ ] Confirm with whoever owns `getWorkOrderById/:workOrderId` whether the + `MDO_ADMIN`-scoped restriction was actually intended to be enforced, + and whether any non-`MDO_ADMIN` caller has been relying on (or is + currently exploiting) its absence. + +--- + +### BS. `sashaktAuth.ts` — a second, independent missing-`return` double-send when `userDetails[0]` is falsy-but-non-throwing + +```ts +if (!sashaktData) { + res.status(400).json({ msg: 'User not present in sashakt', ... }) + logInfo('User details not present in e shashakt') +} // <-- no return +// ... falls through all the way to: +res.status(200).json(...) // unconditional, outside the try/catch +``` + +Same missing-`return` family as changes Q/R/S/.../BO/BP: if +`userDetails[0]` is a falsy-but-non-throwing primitive (`0`, `''`, `false` +— as opposed to `undefined`/`null`, which throw one line earlier at +`sashaktData.email` and land safely in the outer `catch`), the 400 response +has no `return`, so execution falls through to the unconditional +`res.status(200).json(...)` at the end of the handler and sends a second +response — `ERR_HTTP_HEADERS_SENT`. This is a *different* code path from the +already-documented double-send on this same file at the +`authTokenResponse.data` falsy branch (→ 302 then 200) — recording as a +separate, independent instance since they're triggered by different +conditions. Found while extending this file's test coverage (90.72% → +96.90%); not reproduced live for the reason above. + +**MUST VERIFY IN PROD:** +- [ ] Check application logs for `ERR_HTTP_HEADERS_SENT` (or equivalent) + originating from the sashakt auth route when the upstream sashakt + lookup returns a falsy-but-defined `userDetails[0]` (e.g. `0`, `''`, + `false`), distinct from the already-documented 302→200 double-send. + +--- + +### BT. `signupWithAutoLoginV2.ts` — `POST /register` is missing a `return` after its empty-email-and-phone validation response, same bug class as other missing-return double-sends but not currently exploitable + +```ts +if (!req.body.email && !req.body.phone) { + res.status(400).json({ ... }) +} // <-- no return +// ... falls through into createAccount/updateRoles/profileUpdate, then: +if (resultEmail || resultPhone) { res.status(200).json(...) } // guarded +``` + +Same missing-`return` shape as changes Q/R/S/.../BP/BS, but unlike those, +this one is **not exploitable today**: `userEmail`/`userPhone` stay `''` +(falsy) for the rest of the function when the initial check fires, so +`resultEmail || resultPhone` at the later guard stays falsy too and the +second `res.*` call never actually executes — no live double-send occurs +under the current implementation. Recording it anyway because (a) it does +unnecessary downstream work (calls `createAccount`/`updateRoles`/ +`profileUpdate` for a request that was already rejected), and (b) it is +fragile — any future change to how `resultEmail`/`resultPhone` are computed +could silently turn this into a live double-send, the same failure mode +already confirmed elsewhere in this file. Found while extending this file's +test coverage (89.92% → 97.84%). Low priority relative to the URGENT/ +CRITICAL findings elsewhere in this doc. + +**MUST VERIFY IN PROD:** none required — not currently reachable. Listed +for awareness if this function is touched again. + +--- + +### BU. `env.ts` — `POST_ASSESSMENT_BASE`'s fallback default is a real, publicly-registered external domain, not the loopback host + +```ts +POST_ASSESSMENT_BASE: env.POST_ASSESSMENT_BASE || 'http://localhost.com', +``` + +Found while investigating SonarCloud's 17 `http://`-related security hotspots +in this file (see `docs/DUPLICATE-CODE-CLEANUP.md`-adjacent review, prompted +by a report from another user's Sonar run). Every other fallback in this file +follows the pattern `http://localhost:` or an internal service name — +both fail safely (connection refused) if ever reached unconfigured in a real +deployment, since nothing in that environment listens there. `localhost.com` +is different: it is **not** the loopback address — it is a real, live, +third-party-registered domain on the public internet. If +`POST_ASSESSMENT_API_BASE` is ever unset in a deployed environment, this +fallback would silently send real network requests to that external domain +instead of failing loudly, which is a materially different risk profile from +every sibling default in this file. Not changed — this is a source-file edit +outside this campaign's scope without explicit sign-off. + +**MUST VERIFY IN PROD:** +- [ ] Confirm `POST_ASSESSMENT_API_BASE` is set in every real deployment + (it should already be, given the assessment-submission flow is live), + and consider whether the fallback should instead be an obviously-inert + value (e.g. `http://localhost:0`) so a missing env var fails fast + rather than silently reaching an external host. + +--- + +### BV. `env.ts` — `NETWORK_SERVICE_BACKEND`'s fallback default is a malformed URL (missing `//`) + +```ts +NETWORK_SERVICE_BACKEND: env.NETWOR_SERVICE_API_BASE || 'http:localhost:7001', +``` + +Found in the same review as change BU. `'http:localhost:7001'` is missing +the `//` after the scheme, so it is not a well-formed URL — if this fallback +is ever actually used (i.e. `NETWOR_SERVICE_API_BASE` — itself apparently a +typo'd env var name, missing the `K` in `NETWORK` — is unset), any URL +parser or HTTP client consuming it would either throw or misinterpret it, +unlike every sibling `http://localhost:` default in this file. Not +changed — outside this campaign's scope without explicit sign-off. + +**MUST VERIFY IN PROD:** +- [ ] Confirm whether the deployed env var is actually named + `NETWOR_SERVICE_API_BASE` (matching the typo in code) or + `NETWORK_SERVICE_API_BASE` (the presumably-intended name) — if the + latter, this fallback has silently never been reachable by the + intended env var name in any environment that set it correctly. + +--- + ## Pre-existing issues NOT changed Found during review, deliberately left alone — each would be a behavioural diff --git a/docs/sonarqube.md b/docs/sonarqube.md index 41fe1b112..a8c41a374 100644 --- a/docs/sonarqube.md +++ b/docs/sonarqube.md @@ -32,6 +32,15 @@ ratchets upward as code is touched. > than our 60% target. `npm run sonar:gate` creates a separate gate with the > correct value — do not just assign "Sonar way". +**Overall coverage floor: ≥ 80%.** Added once the Phase 1/2 Jest coverage +campaign pushed the whole-repo figure to 81%. This is the one condition in +the gate that is **not** new-code-scoped — Clean-as-You-Code alone can't +prevent the absolute number from drifting down again (a PR that only +touches already-covered lines could still let the overall percentage slip +if untested code is added elsewhere without being flagged as "new" in the +sense Sonar tracks). `npm run sonar:gate` keeps this condition in sync; +re-run it any time to correct drift. + --- ## Known flakiness: rare spurious failure in the full suite diff --git a/scripts/sonar-gate.mjs b/scripts/sonar-gate.mjs index 8f99bf5aa..dfaab9620 100644 --- a/scripts/sonar-gate.mjs +++ b/scripts/sonar-gate.mjs @@ -20,8 +20,9 @@ import { getConfig, sonarRequest } from './sonar-api.mjs' const GATE_NAME = process.env.SONAR_GATE_NAME || 'Aastrika Way' -// All conditions are on NEW code (Clean as You Code). `op` is the FAILING -// comparison: e.g. new_coverage LT 60 means "fail when coverage is below 60". +// All conditions except the last are on NEW code (Clean as You Code). `op` is +// the FAILING comparison: e.g. new_coverage LT 60 means "fail when coverage +// is below 60". const CONDITIONS = [ { metric: 'new_coverage', op: 'LT', error: '60', label: 'Coverage >= 60%' }, { metric: 'new_duplicated_lines_density', op: 'GT', error: '3', label: 'Duplicated lines <= 3%' }, @@ -29,6 +30,13 @@ const CONDITIONS = [ { metric: 'new_reliability_rating', op: 'GT', error: '1', label: 'Reliability rating = A' }, { metric: 'new_maintainability_rating', op: 'GT', error: '1', label: 'Maintainability rating = A' }, { metric: 'new_security_hotspots_reviewed', op: 'LT', error: '100', label: 'Hotspots 100% reviewed' }, + + // OVERALL (whole-repo) coverage, not new-code. Added once the Phase 1/2 + // Jest coverage campaign pushed the absolute figure to 81% — this condition + // is a floor so that number can't silently regress on a later PR that adds + // untested code elsewhere in the repo (Clean-as-You-Code alone wouldn't + // catch that, since it only judges lines actually touched by a change). + { metric: 'coverage', op: 'LT', error: '80', label: 'Overall coverage >= 80%' }, ] /** diff --git a/src/authoring/apis/editor/index.test.ts b/src/authoring/apis/editor/index.test.ts new file mode 100644 index 000000000..d3cc92f6b --- /dev/null +++ b/src/authoring/apis/editor/index.test.ts @@ -0,0 +1,16 @@ +import { mountRouter } from '../../../test-support/mountRouter' +import { editorApi } from './index' + +const agent = () => mountRouter(editorApi) + +/** + * @description Verifies editorApi's single passthrough middleware calls + * next() and does not itself handle any route, so an unmatched request + * falls through to a 404. + */ +describe('editorApi', () => { + it('should fall through to a 404 for any request, since no route is registered', async () => { + const response = await agent().get('/getCompleteDetails/123') + expect(response.status).toBe(404) + }) +}) diff --git a/src/authoring/apis/index.test.ts b/src/authoring/apis/index.test.ts new file mode 100644 index 000000000..c402847c0 --- /dev/null +++ b/src/authoring/apis/index.test.ts @@ -0,0 +1,31 @@ +const mockEditorApi = jest.fn((_req: unknown, res: any) => res.status(200).send({ mounted: 'editorApi' })) +jest.mock('./editor', () => ({ + editorApi: (req: unknown, res: unknown) => mockEditorApi(req, res), +})) + +import { mountRouter } from '../../test-support/mountRouter' +import { api } from './index' + +const agent = () => mountRouter(api) + +/** + * @description Verifies api mounts editorApi under /editor, so a request to + * that sub-path is actually dispatched to editorApi rather than falling + * through unmatched. + */ +describe('api', () => { + it('should mount editorApi under /editor', async () => { + const response = await agent().get('/editor/getCompleteDetails/123') + + expect(mockEditorApi).toHaveBeenCalledTimes(1) + expect(response.status).toBe(200) + expect(response.body).toEqual({ mounted: 'editorApi' }) + }) + + it('should not dispatch to editorApi for a path outside /editor', async () => { + const response = await agent().get('/somewhere-else') + + expect(mockEditorApi).not.toHaveBeenCalled() + expect(response.status).toBe(404) + }) +}) diff --git a/src/authoring/authContent.test.ts b/src/authoring/authContent.test.ts index dcb7ddcc7..12919ca35 100644 --- a/src/authoring/authContent.test.ts +++ b/src/authoring/authContent.test.ts @@ -147,3 +147,96 @@ describe('documented bug: proxy error/unhandledRejection listeners accumulate pe expect(mockProxy.on).toHaveBeenCalledTimes(4) }) }) + +describe('GET — position === -1 (no http/https prefix at all)', () => { + it('falls back to the private-content-service host for a bare /content-store/ path', async () => { + mockProxy.web.mockImplementation((_req, res) => res.end()) + await agent().get('/content-store/abc/def.json') + expect(mockProxy.web).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ target: 'https://content.test' }) + ) + }) +}) + +describe('GET — malformed single-slash scheme (http:/ instead of http://)', () => { + it('repairs the URL before proxying', async () => { + mockProxy.web.mockImplementation((_req, res) => res.end()) + await agent().get('/http:/cdn.test/content-store/abc/def.json') + expect(mockProxy.web).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ target: 'https://content.test' }) + ) + }) +}) + +describe('GET — URLs already rewritten to the contentv3/download path', () => { + it('proxies to the content API without re-matching /content-store/ or /content/', async () => { + mockProxy.web.mockImplementation((_req, res) => res.end()) + await agent().get('/http://cdn.test/contentv3/download/abc/def.json') + expect(mockProxy.web).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ target: 'https://content.test' }) + ) + }) +}) + +describe('proxyCreator listener callback bodies (invoked directly, no live HTTP)', () => { + // The 'error'/'unhandledRejection' handlers are only ever *registered* by a + // live request (mockProxy.on is a jest.fn() no-op), so their bodies never + // execute unless we invoke the captured callback ourselves. Calling the + // exported Router directly with a stub req/res (bypassing supertest/real + // HTTP entirely) lets us do that without touching a real, already-completed + // response object — avoiding any double-send on a real socket. + // tslint:disable-next-line: no-any + const fakeRes = () => { + // tslint:disable-next-line: no-any + const res: any = {} + res.set = jest.fn(() => res) + res.status = jest.fn(() => res) + res.send = jest.fn(() => res) + res.writeHead = jest.fn(() => res) + res.end = jest.fn(() => res) + return res + } + + it('sends a 500 response when the registered "error" listener fires with a truthy error', () => { + mockProxy.web.mockImplementation(() => undefined) + // tslint:disable-next-line: no-any + const req: any = { method: 'GET', url: '/https://cdn.test/content-store/abc/def.json' } + const res = fakeRes(); + + // Router instances are callable middleware functions: router(req, res, next). + // tslint:disable-next-line: no-any + (authContent as any)(req, res, jest.fn()) + + const errorCall = mockProxy.on.mock.calls.find((call) => call[0] === 'error') + expect(errorCall).toBeDefined() + const errorHandler = errorCall![1] + errorHandler(new Error('boom')) + + expect(res.writeHead).toHaveBeenCalledWith(500) + expect(res.end).toHaveBeenCalledWith({ error: 'Failed due to unknown reason' }) + }) + + it('sends a 500 response when the registered "unhandledRejection" listener fires', () => { + mockProxy.web.mockImplementation(() => undefined) + // tslint:disable-next-line: no-any + const req: any = { method: 'GET', url: '/https://cdn.test/content-store/abc/def.json' } + const res = fakeRes(); + + // tslint:disable-next-line: no-any + (authContent as any)(req, res, jest.fn()) + + const rejectionCall = mockProxy.on.mock.calls.find((call) => call[0] === 'unhandledRejection') + expect(rejectionCall).toBeDefined() + const rejectionHandler = rejectionCall![1] + rejectionHandler() + + expect(res.writeHead).toHaveBeenCalledWith(500) + expect(res.end).toHaveBeenCalledWith('Some error occured') + }) +}) diff --git a/src/authoring/utils/decode.test.ts b/src/authoring/utils/decode.test.ts new file mode 100644 index 000000000..963b998ff --- /dev/null +++ b/src/authoring/utils/decode.test.ts @@ -0,0 +1,120 @@ +/** + * decode.ts is NOT an Express-route file — it exports a single plain, + * synchronous, standalone transform function `decoder` (no axios, no + * res.send, no try/catch). So this test file calls the exported function + * directly and asserts on return values / thrown errors, per the "plain + * functions" style used in ./cdn-url-replacer.test.ts. + * + * `decoder` expects `data` to be a base64 string that, once base64-decoded + * to raw bytes and reinterpreted as a UTF-16LE character stream (via a + * `Uint16Array` view over the same underlying buffer), yields a valid JSON + * string. To build fixtures, `toDecoderInput` below performs the inverse of + * that pipeline: it takes a raw string, writes each character as a 2-byte + * UTF-16LE code unit into a Buffer (mirroring how `decoder` reads pairs of + * bytes back out via the Uint16Array view), then base64-encodes that + * buffer. `encodeValue` layers `JSON.stringify` on top so callers can pass + * plain JS values and get back valid `decoder` input. + * + * No hang/crash/security-bypass patterns apply here: there is no Router, no + * res object, and no try/catch to route around — the only branch is + * "JSON.parse succeeds" vs. "JSON.parse throws", and both are safe to + * exercise live. + */ + +import { decoder } from './decode' + +/** Encodes a raw string as base64-of-UTF16LE-bytes, the inverse of decoder's internal pipeline. */ +function toDecoderInput(raw: string): string { + const buf = Buffer.alloc(raw.length * 2) + for (let i = 0; i < raw.length; i += 1) { + buf.writeUInt16LE(raw.charCodeAt(i), i * 2) + } + return buf.toString('base64') +} + +/** Encodes an arbitrary JSON-serializable value into valid decoder() input. */ +function encodeValue(value: unknown): string { + return toDecoderInput(JSON.stringify(value)) +} + +/** + * @description Verifies decoder correctly reverses the UTF-16LE/base64 + * encoding pipeline for a range of JSON value shapes, and propagates a + * SyntaxError rather than swallowing it when the decoded bytes are not + * valid JSON. + */ +describe('decoder', () => { + /** + * @description Verifies decoder returns the original value for various + * JSON-serializable inputs once round-tripped through the encoding helper. + */ + describe('when given a base64 string that decodes to valid JSON', () => { + it('should decode a simple flat object', () => { + const input = { a: 1, b: 'two' } + expect(decoder(encodeValue(input))).toEqual(input) + }) + + it('should decode an array', () => { + const input = [1, 2, 3, 'four'] + expect(decoder(encodeValue(input))).toEqual(input) + }) + + it('should decode a JSON string value', () => { + const input = 'hello world' + expect(decoder(encodeValue(input))).toEqual(input) + }) + + it('should decode a nested object with arrays and objects', () => { + const input = { + meta: { count: 2, tags: ['x', 'y'] }, + name: 'thumbnail', + nested: { deeper: { value: true } }, + } + expect(decoder(encodeValue(input))).toEqual(input) + }) + + it('should decode a numeric value', () => { + expect(decoder(encodeValue(42))).toEqual(42) + }) + + it('should decode a boolean value', () => { + expect(decoder(encodeValue(true))).toEqual(true) + }) + + it('should decode a null value', () => { + expect(decoder(encodeValue(null))).toBeNull() + }) + + it('should decode an empty object', () => { + expect(decoder(encodeValue({}))).toEqual({}) + }) + + it('should decode a string containing unicode characters', () => { + const input = { greeting: 'héllo wörld 😀' } + expect(decoder(encodeValue(input))).toEqual(input) + }) + }) + + /** + * @description Verifies decoder throws (rather than swallowing) a + * SyntaxError when the base64-decoded, UTF-16-reinterpreted bytes do not + * form valid JSON — the source has no try/catch, so the error must + * propagate synchronously to the caller. + */ + describe('when the decoded bytes are not valid JSON', () => { + it('should throw a SyntaxError for a non-JSON plain string', () => { + const invalidInput = toDecoderInput('not valid json') + expect(() => decoder(invalidInput)).toThrow(SyntaxError) + }) + + it('should throw for an empty raw string', () => { + const invalidInput = toDecoderInput('') + expect(() => decoder(invalidInput)).toThrow() + }) + + it('should throw for a malformed/truncated JSON object', () => { + const invalidInput = toDecoderInput('{"a":1,') + expect(() => decoder(invalidInput)).toThrow(SyntaxError) + }) + }) +}) diff --git a/src/authoring/utils/read-meta-and-json/channel.test.ts b/src/authoring/utils/read-meta-and-json/channel.test.ts new file mode 100644 index 000000000..c0036d45e --- /dev/null +++ b/src/authoring/utils/read-meta-and-json/channel.test.ts @@ -0,0 +1,35 @@ +jest.mock('../S3/read', () => ({ + readFromS3: jest.fn(), +})) + +import { readFromS3 } from '../S3/read' +import { extractChannelData } from './channel' + +const mockReadFromS3 = readFromS3 as jest.Mock + +beforeEach(() => { + mockReadFromS3.mockReset() +}) + +/** + * @description Verifies extractChannelData delegates to readFromS3 with the + * given URL and returns/propagates its result. + */ +describe('extractChannelData', () => { + it('should resolve with the data readFromS3 resolves with', async () => { + mockReadFromS3.mockResolvedValue({ channel: 'c1' }) + + const result = await extractChannelData('https://s3.test/channel.json') + + expect(mockReadFromS3).toHaveBeenCalledWith('https://s3.test/channel.json') + expect(result).toEqual({ channel: 'c1' }) + }) + + it('should propagate a rejection from readFromS3', async () => { + mockReadFromS3.mockRejectedValue(new Error('s3 unavailable')) + + await expect(extractChannelData('https://s3.test/channel.json')).rejects.toThrow( + 's3 unavailable' + ) + }) +}) diff --git a/src/authoring/utils/upload-meta-and-json/channel.test.ts b/src/authoring/utils/upload-meta-and-json/channel.test.ts new file mode 100644 index 000000000..07d4ab449 --- /dev/null +++ b/src/authoring/utils/upload-meta-and-json/channel.test.ts @@ -0,0 +1,30 @@ +jest.mock('../S3/upload', () => ({ + uploadToS3: jest.fn(), +})) + +import { uploadToS3 } from '../S3/upload' +import { uploadChannelData } from './channel' + +const mockUploadToS3 = uploadToS3 as jest.Mock + +beforeEach(() => { + mockUploadToS3.mockReset() +}) + +/** + * @description Verifies uploadChannelData delegates to uploadToS3 with the + * request's data/path and the fixed 'channel.json' filename. + */ +describe('uploadChannelData', () => { + it('should upload the given data/path with the channel.json filename', async () => { + mockUploadToS3.mockResolvedValue({ artifactUrl: 'a', downloadUrl: 'd', error: null }) + + const result = await uploadChannelData({ + data: { name: 'Channel One' }, + path: 'content/type/id', + } as any) + + expect(mockUploadToS3).toHaveBeenCalledWith({ name: 'Channel One' }, 'content/type/id', 'channel.json') + expect(result).toEqual({ artifactUrl: 'a', downloadUrl: 'd', error: null }) + }) +}) diff --git a/src/authoring/utils/upload-meta-and-json/quiz.test.ts b/src/authoring/utils/upload-meta-and-json/quiz.test.ts new file mode 100644 index 000000000..8e94fed11 --- /dev/null +++ b/src/authoring/utils/upload-meta-and-json/quiz.test.ts @@ -0,0 +1,30 @@ +jest.mock('../S3/upload', () => ({ + uploadToS3: jest.fn(), +})) + +import { uploadToS3 } from '../S3/upload' +import { uploadQuizData } from './quiz' + +const mockUploadToS3 = uploadToS3 as jest.Mock + +beforeEach(() => { + mockUploadToS3.mockReset() +}) + +/** + * @description Verifies uploadQuizData delegates to uploadToS3 with the + * request's data/path and the fixed 'quiz.json' filename. + */ +describe('uploadQuizData', () => { + it('should upload the given data/path with the quiz.json filename', async () => { + mockUploadToS3.mockResolvedValue({ artifactUrl: 'a', downloadUrl: 'd', error: null }) + + const result = await uploadQuizData({ + data: { questions: [] }, + path: 'content/type/id', + } as any) + + expect(mockUploadToS3).toHaveBeenCalledWith({ questions: [] }, 'content/type/id', 'quiz.json') + expect(result).toEqual({ artifactUrl: 'a', downloadUrl: 'd', error: null }) + }) +}) diff --git a/src/authoring/utils/upload-meta-and-json/unkown.test.ts b/src/authoring/utils/upload-meta-and-json/unkown.test.ts new file mode 100644 index 000000000..a3cc9a37e --- /dev/null +++ b/src/authoring/utils/upload-meta-and-json/unkown.test.ts @@ -0,0 +1,38 @@ +jest.mock('../S3/upload', () => ({ + uploadToS3: jest.fn(), +})) + +import { uploadToS3 } from '../S3/upload' +import { uploadUnKownData } from './unkown' + +const mockUploadToS3 = uploadToS3 as jest.Mock + +beforeEach(() => { + mockUploadToS3.mockReset() +}) + +/** + * @description Verifies uploadUnKownData delegates to uploadToS3 using the + * request's own name when present, and falls back to 'unkown' when it isn't. + */ +describe('uploadUnKownData', () => { + it('should upload using the request name when provided', async () => { + mockUploadToS3.mockResolvedValue({ artifactUrl: 'a', downloadUrl: 'd', error: null }) + + await uploadUnKownData({ + data: { foo: 'bar' }, + name: 'custom.json', + path: 'content/type/id', + } as any) + + expect(mockUploadToS3).toHaveBeenCalledWith({ foo: 'bar' }, 'content/type/id', 'custom.json') + }) + + it("should fall back to 'unkown' as the filename when no name is provided", async () => { + mockUploadToS3.mockResolvedValue({ artifactUrl: 'a', downloadUrl: 'd', error: null }) + + await uploadUnKownData({ data: { foo: 'bar' }, path: 'content/type/id' } as any) + + expect(mockUploadToS3).toHaveBeenCalledWith({ foo: 'bar' }, 'content/type/id', 'unkown') + }) +}) diff --git a/src/protectedApi_v8/admin/bulkUploadUser.test.ts b/src/protectedApi_v8/admin/bulkUploadUser.test.ts index 7bbbd856c..2e0e7ea22 100644 --- a/src/protectedApi_v8/admin/bulkUploadUser.test.ts +++ b/src/protectedApi_v8/admin/bulkUploadUser.test.ts @@ -20,8 +20,15 @@ */ jest.mock('axios') +// `Client` is instantiated once at module load, but jest.config.js's global +// `clearMocks: true` wipes every mock's call/result history before each +// test — including a factory-created inner `execute` jest.fn(). Capturing a +// single stable `execute` reference here (outside any test, so it survives +// clearMocks) is the proven fix for this, matching the pattern used for the +// same issue in publicCertifcateFlinkv2.test.ts. +const mockCassandraExecute = jest.fn() jest.mock('cassandra-driver', () => ({ - Client: jest.fn(() => ({ execute: jest.fn(), on: jest.fn() })), + Client: jest.fn(() => ({ execute: mockCassandraExecute, on: jest.fn() })), })) jest.mock('../../utils/logger', () => ({ logError: jest.fn(), logInfo: jest.fn() })) jest.mock('../../utils/requestExtract', () => ({ @@ -45,9 +52,11 @@ jest.mock('../../utils/env', () => ({ import axios from 'axios' import { networkError, upstreamOk } from '../../test-support/mockAxios' import { mountRouter } from '../../test-support/mountRouter' +import { logInfo } from '../../utils/logger' import { bulkUploadUserApi } from './bulkUploadUser' const mockAxiosCallable = axios as unknown as jest.Mock +const mockLogInfo = logInfo as jest.Mock const csvWithTwoRows = [ 'first_name,last_name,username,phone,type,channel,usertype,Cadre', @@ -72,6 +81,7 @@ const settle = () => new Promise((resolve) => setImmediate(resolve)) beforeEach(() => { mockAxiosCallable.mockReset() + mockCassandraExecute.mockReset() }) describe('POST /create-users', () => { @@ -110,4 +120,203 @@ describe('POST /create-users', () => { // NOTE: a CSV with 0 or 1 data rows is a documented hang bug (userProcessing() // is only invoked when result.length > 1, and nothing else ever responds) — // not reproduced live. + + it('responds 500 when userProcessing itself throws after the rows settle (its own outer catch)', async () => { + // Forces logInfo to throw only on the specific call made right before the + // 200 response, at the top level of userProcessing's own try block — not + // inside any of the nested per-row try/catches, which would swallow it. + mockAxiosCallable.mockResolvedValue(upstreamOk({ result: { userId: 'u1' } })) + mockLogInfo.mockImplementation((msg: unknown) => { + if (typeof msg === 'string' && msg.startsWith('Data inside user processing')) { + throw new Error('logging blew up') + } + }) + try { + const response = await agentWithFile(csvWithTwoRows).post('/create-users') + expect(response.status).toBe(500) + expect(response.body.message).toBe('Error While Creating the user ') + } finally { + mockLogInfo.mockImplementation(() => undefined) + } + }) + + it('drives a non-ASHA row through role assignment, password reset and a successful welcome email', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/v2/read/')) { + return Promise.resolve(upstreamOk({ result: { response: { organisations: [{ organisationId: 'org-1' }] } } })) + } + if (config.url.includes('/user/v1/role/assign')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('/password/reset')) { + return Promise.resolve(upstreamOk({ result: { link: 'https://reset.test/link' } })) + } + if (config.url.includes('/notification/email')) { + return Promise.resolve(upstreamOk({ params: { status: 'success' } })) + } + return Promise.resolve(upstreamOk({ result: { userId: 'u1' } })) + }) + const response = await agentWithFile(csvWithTwoRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('logs a failure when the welcome-email upstream reports a non-success status', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/v2/read/')) { + return Promise.resolve(upstreamOk({ result: { response: { organisations: [{ organisationId: 'org-1' }] } } })) + } + if (config.url.includes('/user/v1/role/assign')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('/password/reset')) { + return Promise.resolve(upstreamOk({ result: { link: 'https://reset.test/link' } })) + } + if (config.url.includes('/notification/email')) { + return Promise.resolve(upstreamOk({ params: { status: 'failed' } })) + } + return Promise.resolve(upstreamOk({ result: { userId: 'u1' } })) + }) + const response = await agentWithFile(csvWithTwoRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('swallows an upstream failure from the welcome-email call for a non-ASHA row', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/v2/read/')) { + return Promise.resolve(upstreamOk({ result: { response: { organisations: [{ organisationId: 'org-1' }] } } })) + } + if (config.url.includes('/user/v1/role/assign')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('/password/reset')) { + return Promise.resolve(upstreamOk({ result: { link: 'https://reset.test/link' } })) + } + if (config.url.includes('/notification/email')) { + return Promise.reject(networkError()) + } + return Promise.resolve(upstreamOk({ result: { userId: 'u1' } })) + }) + const response = await agentWithFile(csvWithTwoRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('swallows an upstream failure resetting the password for a non-ASHA row', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/v2/read/')) { + return Promise.resolve(upstreamOk({ result: { response: { organisations: [{ organisationId: 'org-1' }] } } })) + } + if (config.url.includes('/user/v1/role/assign')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('/password/reset')) { + return Promise.reject(networkError()) + } + return Promise.resolve(upstreamOk({ result: { userId: 'u1' } })) + }) + const response = await agentWithFile(csvWithTwoRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('swallows an upstream failure assigning a role for a non-ASHA row', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/v2/read/')) { + return Promise.resolve(upstreamOk({ result: { response: { organisations: [{ organisationId: 'org-1' }] } } })) + } + if (config.url.includes('/user/v1/role/assign')) { + return Promise.reject(networkError()) + } + return Promise.resolve(upstreamOk({ result: { userId: 'u1' } })) + }) + const response = await agentWithFile(csvWithTwoRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('covers the non-ASHA row outer catch when logging itself throws before the user-creation call', async () => { + mockAxiosCallable.mockResolvedValue(upstreamOk({ result: { userId: 'u1' } })) + mockLogInfo.mockImplementation((msg: unknown) => { + if (msg === 'CSV data present more than one row') { + throw new Error('log boom') + } + }) + try { + const response = await agentWithFile(csvWithTwoRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + } finally { + mockLogInfo.mockImplementation(() => undefined) + } + }) + + it('swallows an upstream failure creating an ASHA worker', async () => { + mockAxiosCallable.mockRejectedValue(networkError()) + const response = await agentWithFile(csvWithAshaRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('swallows an upstream failure reading a newly created ASHA worker', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/v2/read/')) { + return Promise.reject(networkError()) + } + return Promise.resolve(upstreamOk({ result: { userId: 'u-asha-2' } })) + }) + const response = await agentWithFile(csvWithAshaRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('swallows an upstream failure assigning a role to an ASHA worker', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/v2/read/')) { + return Promise.resolve(upstreamOk({ result: { response: { organisations: [{ organisationId: 'org-2' }] } } })) + } + if (config.url.includes('/user/v1/role/assign')) { + return Promise.reject(networkError()) + } + return Promise.resolve(upstreamOk({ result: { userId: 'u-asha-3' } })) + }) + const response = await agentWithFile(csvWithAshaRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('swallows a Cassandra insert failure after an ASHA worker is fully provisioned', async () => { + mockCassandraExecute.mockImplementationOnce(() => { + throw new Error('cassandra insert failed') + }) + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/v2/read/')) { + return Promise.resolve(upstreamOk({ result: { response: { organisations: [{ organisationId: 'org-3' }] } } })) + } + if (config.url.includes('/user/v1/role/assign')) { + return Promise.resolve(upstreamOk({})) + } + return Promise.resolve(upstreamOk({ result: { userId: 'u-asha-4' } })) + }) + const response = await agentWithFile(csvWithAshaRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + }) + + it('covers the ASHA row outer catch when logging itself throws before the user-creation call', async () => { + mockAxiosCallable.mockResolvedValue(upstreamOk({ result: { userId: 'u-asha-5' } })) + mockLogInfo.mockImplementation((msg: unknown) => { + if (msg === 'CSV data present more than one row') { + throw new Error('log boom') + } + }) + try { + const response = await agentWithFile(csvWithAshaRows).post('/create-users') + expect(response.status).toBe(200) + await settle() + } finally { + mockLogInfo.mockImplementation(() => undefined) + } + }) }) diff --git a/src/protectedApi_v8/certifications.test.ts b/src/protectedApi_v8/certifications.test.ts index 336721543..7a7200de3 100644 --- a/src/protectedApi_v8/certifications.test.ts +++ b/src/protectedApi_v8/certifications.test.ts @@ -18,6 +18,9 @@ import { mountRouter } from '../test-support/mountRouter' import { certificationApi } from './certifications' const mockAxios = axios as jest.Mocked +// The booking route calls axios as a callable (`axios({...})`) rather than +// via .get/.post/etc, matching the pattern used in assessment.test.ts. +const mockAxiosCallable = axios as unknown as jest.Mock const agent = () => mountRouter(certificationApi) beforeEach(() => { @@ -25,6 +28,7 @@ beforeEach(() => { mockAxios.post.mockReset() mockAxios.delete.mockReset() mockAxios.patch.mockReset() + mockAxiosCallable.mockReset() }) describe('GET /:certificationId/bookingInfo', () => { @@ -52,6 +56,53 @@ describe('GET /:certificationId/testCenters', () => { const response = await agent().get('/cert-1/testCenters') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/cert-1/testCenters') + expect(response.status).toBe(400) + }) +}) + +describe('GET /:certificationId/locations/:location/testCenters/:testCenter/slots', () => { + it('returns ACC slots for the given location and test center', async () => { + mockAxios.get.mockResolvedValue(upstreamOk([{ slot: 'morning' }])) + const response = await agent().get('/cert-1/locations/loc-1/testCenters/tc-1/slots') + expect(response.status).toBe(200) + expect(mockAxios.get).toHaveBeenCalledWith( + expect.stringContaining( + '/certifications/cert-1/locations/loc-1/test-centers/tc-1/slots' + ), + expect.anything() + ) + }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/cert-1/locations/loc-1/testCenters/tc-1/slots') + expect(response.status).toBe(400) + }) +}) + +describe('POST /:certificationId/booking/:slotNo', () => { + it('books/updates an ACC slot, using only the local part of the email', async () => { + mockAxiosCallable.mockResolvedValue(upstreamOk({ booked: true })) + const response = await agent().post('/cert-1/booking/3').send({}) + expect(response.status).toBe(200) + // getEmailLocalPart('user@Example.com') -> 'user' + expect(mockAxiosCallable).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'POST', + url: expect.stringContaining('/users/user/certifications/cert-1/booking/3'), + }) + ) + }) + + it('returns 400 on an upstream failure', async () => { + mockAxiosCallable.mockRejectedValue(networkError()) + const response = await agent().post('/cert-1/booking/3').send({}) + expect(response.status).toBe(400) + }) }) describe('GET /countries', () => { @@ -74,6 +125,12 @@ describe('GET /countries/:countryCode/locations', () => { const response = await agent().get('/countries/IN/locations') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/countries/IN/locations') + expect(response.status).toBe(400) + }) }) describe('GET /slots', () => { @@ -82,6 +139,12 @@ describe('GET /slots', () => { const response = await agent().get('/slots') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/slots') + expect(response.status).toBe(400) + }) }) describe('POST /:certificationId/atDeskBooking', () => { @@ -104,6 +167,12 @@ describe('DELETE /:certificationId/slots/:slotNo', () => { const response = await agent().delete('/cert-1/slots/3') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.delete.mockRejectedValue(networkError()) + const response = await agent().delete('/cert-1/slots/3') + expect(response.status).toBe(400) + }) }) describe('GET /currencies', () => { @@ -112,6 +181,12 @@ describe('GET /currencies', () => { const response = await agent().get('/currencies') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/currencies') + expect(response.status).toBe(400) + }) }) describe('POST /:certificationId/budgetRequest', () => { @@ -120,6 +195,12 @@ describe('POST /:certificationId/budgetRequest', () => { const response = await agent().post('/cert-1/budgetRequest').send({}) expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/cert-1/budgetRequest').send({}) + expect(response.status).toBe(400) + }) }) describe('DELETE /:certificationId/budgetRequest', () => { @@ -128,6 +209,12 @@ describe('DELETE /:certificationId/budgetRequest', () => { const response = await agent().delete('/cert-1/budgetRequest') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.delete.mockRejectedValue(networkError()) + const response = await agent().delete('/cert-1/budgetRequest') + expect(response.status).toBe(400) + }) }) describe('POST /:certificationId/result', () => { @@ -168,6 +255,12 @@ describe('PATCH /:certificationId/result', () => { const response = await agent().patch('/cert-1/result').query({ action: 'submit' }).send({}) expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.patch.mockRejectedValue(networkError()) + const response = await agent().patch('/cert-1/result').query({ action: 'submit' }).send({}) + expect(response.status).toBe(400) + }) }) describe('GET /submittedDocument', () => { @@ -176,6 +269,12 @@ describe('GET /submittedDocument', () => { const response = await agent().get('/submittedDocument') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/submittedDocument') + expect(response.status).toBe(400) + }) }) describe('DELETE /:certificationId/document', () => { @@ -184,6 +283,12 @@ describe('DELETE /:certificationId/document', () => { const response = await agent().delete('/cert-1/document') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.delete.mockRejectedValue(networkError()) + const response = await agent().delete('/cert-1/document') + expect(response.status).toBe(400) + }) }) describe('GET /certificationApprovals', () => { @@ -192,6 +297,12 @@ describe('GET /certificationApprovals', () => { const response = await agent().get('/certificationApprovals') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/certificationApprovals') + expect(response.status).toBe(400) + }) }) describe('POST /atDeskRequests/:icfdId', () => { @@ -200,6 +311,12 @@ describe('POST /atDeskRequests/:icfdId', () => { const response = await agent().post('/atDeskRequests/icfd-1').send({}) expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/atDeskRequests/icfd-1').send({}) + expect(response.status).toBe(400) + }) }) describe('POST /:certificationId/budgetRequestApproval', () => { @@ -208,6 +325,12 @@ describe('POST /:certificationId/budgetRequestApproval', () => { const response = await agent().post('/cert-1/budgetRequestApproval').send({}) expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/cert-1/budgetRequestApproval').send({}) + expect(response.status).toBe(400) + }) }) describe('POST /:certificationId/resultVerificationRequests', () => { @@ -216,6 +339,12 @@ describe('POST /:certificationId/resultVerificationRequests', () => { const response = await agent().post('/cert-1/resultVerificationRequests').send({}) expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/cert-1/resultVerificationRequests').send({}) + expect(response.status).toBe(400) + }) }) describe('GET /', () => { @@ -238,6 +367,12 @@ describe('GET /certificationRequests', () => { const response = await agent().get('/certificationRequests') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/certificationRequests') + expect(response.status).toBe(400) + }) }) describe('GET /:certificationId/submissions', () => { @@ -246,6 +381,12 @@ describe('GET /:certificationId/submissions', () => { const response = await agent().get('/cert-1/submissions') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/cert-1/submissions') + expect(response.status).toBe(400) + }) }) describe('GET /:emailId/privileges', () => { @@ -254,6 +395,12 @@ describe('GET /:emailId/privileges', () => { const response = await agent().get('/someone@example.com/privileges') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/someone@example.com/privileges') + expect(response.status).toBe(400) + }) }) describe('GET /defaultProctor', () => { @@ -262,4 +409,10 @@ describe('GET /defaultProctor', () => { const response = await agent().get('/defaultProctor') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/defaultProctor') + expect(response.status).toBe(400) + }) }) diff --git a/src/protectedApi_v8/connections_v2.test.ts b/src/protectedApi_v8/connections_v2.test.ts index 37d407155..85eee6d8a 100644 --- a/src/protectedApi_v8/connections_v2.test.ts +++ b/src/protectedApi_v8/connections_v2.test.ts @@ -4,6 +4,17 @@ * Line-for-line identical logic to connections.ts, just under /v2/*-prefixed * paths and a different export name — confirmed by diffing the two files * before writing this. Same test shape, ported directly. + * + * PHASE 2 — extended missing-rootorg / missing-userId / upstream-500 branch + * coverage for every route (98.25% lines). Two lines remain uncovered by + * design, not oversight: + * - Line 20: `getUserRegistryById` in apiEndpoints is defined but never + * called by any route in this file — dead code, nothing to invoke. + * - Lines 143-144: the `!userId` branch in GET /established/:id, where + * userId = req.params.id. Express's default path-to-regexp requires at + * least one character for a named param, so req.params.id can never be + * falsy for a route that matched — this branch is unreachable via real + * HTTP routing and cannot be exercised through the mounted router. */ jest.mock('axios') @@ -27,9 +38,12 @@ jest.mock('../utils/env', () => ({ import axios from 'axios' import { networkError, upstreamOk } from '../test-support/mockAxios' import { mountRouter } from '../test-support/mountRouter' +import { extractUserId, extractUserIdFromRequest } from '../utils/requestExtract' import { connectionsV2Api } from './connections_v2' const mockAxios = axios as jest.Mocked +const mockExtractUserId = extractUserId as jest.Mock +const mockExtractUserIdFromRequest = extractUserIdFromRequest as jest.Mock const agent = () => mountRouter(connectionsV2Api) const withOrg = (req: ReturnType) => req.set('rootorg', 'r1') @@ -51,6 +65,13 @@ describe('GET /v2/connections/requested', () => { expect(response.status).toBe(400) }) + it('rejects a request when the user id cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().get('/v2/connections/requested')) + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + it('returns 500 on an upstream failure', async () => { mockAxios.get.mockRejectedValue(networkError()) const response = await withOrg(agent().get('/v2/connections/requested')) @@ -69,6 +90,19 @@ describe('GET /v2/connections/requests/received', () => { const response = await agent().get('/v2/connections/requests/received') expect(response.status).toBe(400) }) + + it('rejects a request when the user id cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().get('/v2/connections/requests/received')) + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withOrg(agent().get('/v2/connections/requests/received')) + expect(response.status).toBe(500) + }) }) describe('GET /connections/established', () => { @@ -83,6 +117,18 @@ describe('GET /connections/established', () => { const response = await withOrg(agent().get('/v2/connections/established')) expect(response.status).toBe(500) }) + + it('rejects a request missing the rootorg header', async () => { + const response = await agent().get('/v2/connections/established') + expect(response.status).toBe(400) + }) + + it('rejects a request when the user id cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().get('/v2/connections/established')) + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) }) describe('GET /connections/established/:id', () => { @@ -91,6 +137,17 @@ describe('GET /connections/established/:id', () => { const response = await withOrg(agent().get('/v2/connections/established/c1')) expect(response.status).toBe(200) }) + + it('rejects a request missing the rootorg header', async () => { + const response = await agent().get('/v2/connections/established/c1') + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withOrg(agent().get('/v2/connections/established/c1')) + expect(response.status).toBe(500) + }) }) describe('GET /v2/connections/suggests', () => { @@ -104,6 +161,19 @@ describe('GET /v2/connections/suggests', () => { const response = await agent().get('/v2/connections/suggests') expect(response.status).toBe(400) }) + + it('rejects a request when the user id cannot be resolved', async () => { + mockExtractUserId.mockReturnValueOnce('') + const response = await withOrg(agent().get('/v2/connections/suggests')) + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withOrg(agent().get('/v2/connections/suggests')) + expect(response.status).toBe(500) + }) }) describe('POST /add/connection', () => { @@ -140,16 +210,18 @@ describe('POST /add/connection', () => { }) describe('POST /update/connection', () => { + const body = { + status: 'accepted', + userDepartmentFrom: 'd1', + userDepartmentTo: 'd2', + userIdTo: 'u2', + userNameFrom: 'A', + userNameTo: 'B', + } + it('updates the connection', async () => { mockAxios.post.mockResolvedValue(upstreamOk({ updated: true })) - const response = await withOrg(agent().post('/v2/update/connection')).send({ - status: 'accepted', - userDepartmentFrom: 'd1', - userDepartmentTo: 'd2', - userIdTo: 'u2', - userNameFrom: 'A', - userNameTo: 'B', - }) + const response = await withOrg(agent().post('/v2/update/connection')).send(body) expect(response.status).toBe(200) }) @@ -158,6 +230,17 @@ describe('POST /update/connection', () => { expect(response.status).toBe(400) expect(mockAxios.post).not.toHaveBeenCalled() }) + + it('rejects a request missing the rootorg header', async () => { + const response = await agent().post('/v2/update/connection').send(body) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withOrg(agent().post('/v2/update/connection')).send(body) + expect(response.status).toBe(500) + }) }) describe('POST /connections/recommended', () => { @@ -172,6 +255,18 @@ describe('POST /connections/recommended', () => { const response = await withOrg(agent().post('/v2/connections/recommended')).send({}) expect(response.status).toBe(500) }) + + it('rejects a request missing the rootorg header', async () => { + const response = await agent().post('/v2/connections/recommended').send({}) + expect(response.status).toBe(400) + }) + + it('rejects a request when the user id cannot be resolved', async () => { + mockExtractUserId.mockReturnValueOnce('') + const response = await withOrg(agent().post('/v2/connections/recommended')).send({}) + expect(response.status).toBe(400) + expect(mockAxios.post).not.toHaveBeenCalled() + }) }) describe('POST /v2/connections/recommended/userDepartment', () => { @@ -203,4 +298,17 @@ describe('POST /v2/connections/recommended/userDepartment', () => { const response = await withOrg(agent().post('/v2/connections/recommended/userDepartment')).send({}) expect(response.status).toBe(500) }) + + it('rejects a request missing the rootorg header', async () => { + const response = await agent().post('/v2/connections/recommended/userDepartment').send({}) + expect(response.status).toBe(400) + expect(mockAxios.post).not.toHaveBeenCalled() + }) + + it('rejects a request when the user id cannot be resolved', async () => { + mockExtractUserId.mockReturnValueOnce('') + const response = await withOrg(agent().post('/v2/connections/recommended/userDepartment')).send({}) + expect(response.status).toBe(400) + expect(mockAxios.post).not.toHaveBeenCalled() + }) }) diff --git a/src/protectedApi_v8/content.test.ts b/src/protectedApi_v8/content.test.ts index 55b3f6b30..0e67d3921 100644 --- a/src/protectedApi_v8/content.test.ts +++ b/src/protectedApi_v8/content.test.ts @@ -148,6 +148,47 @@ describe('POST /likeCount', () => { }) }) +describe('GET /searchAutoComplete', () => { + it('returns matching suggestions and filters out empty search terms', async () => { + mockAxios.request.mockResolvedValue( + upstreamOk({ + hits: { hits: [{ _source: { searchTerm: 'hello' } }, { _source: { searchTerm: '' } }] }, + }) + ) + + const response = await agent().get('/searchAutoComplete').query({ q: 'hel', l: 'en' }) + + expect(response.status).toBe(200) + expect(response.body).toHaveLength(1) + }) + + it('falls back to suggested terms when the query is empty', async () => { + mockAxios.request.mockResolvedValue(upstreamOk({ hits: { hits: [] } })) + + const response = await agent().get('/searchAutoComplete').query({ q: '', l: 'en' }) + + expect(response.status).toBe(200) + expect(response.body).toEqual([]) + }) + + it('returns an empty array when upstream sends no hits', async () => { + mockAxios.request.mockResolvedValue(upstreamOk({})) + + const response = await agent().get('/searchAutoComplete').query({ q: 'hel', l: 'en' }) + + expect(response.status).toBe(200) + expect(response.body).toEqual([]) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.request.mockRejectedValue(networkError()) + + const response = await agent().get('/searchAutoComplete').query({ q: 'hel', l: 'en' }) + + expect(response.status).toBe(500) + }) +}) + describe('POST /searchV5', () => { it('forwards the mapped search response', async () => { mockAxios.post.mockResolvedValue( @@ -168,6 +209,63 @@ describe('POST /searchV5', () => { }) }) +describe('POST /searchRegionRecommendation', () => { + it('rejects a request missing org/rootOrg headers', async () => { + const response = await agent().post('/searchRegionRecommendation').send({ request: {} }) + expect(response.status).toBe(400) + }) + + it('returns child content metadata when the first search has hits', async () => { + mockAxios.post.mockResolvedValue( + upstreamOk({ + result: { + response: { + result: [{ children: [{ identifier: 'do_2' }] }], + totalHits: 1, + }, + }, + }) + ) + mockAxiosCallable.mockResolvedValue(upstreamOk([contentItem({ identifier: 'do_2' })])) + + const response = await withOrgHeaders(agent().post('/searchRegionRecommendation')).send({ + request: {}, + }) + + expect(response.status).toBe(200) + expect(response.body.contents).toHaveLength(1) + }) + + it('retries with the defaultLabel filter when the first search has no hits', async () => { + mockAxios.post + .mockResolvedValueOnce( + upstreamOk({ result: { response: { result: [], totalHits: 0 } } }) + ) + .mockResolvedValueOnce( + upstreamOk({ + result: { response: { result: [{ children: [{ identifier: 'do_3' }] }], totalHits: 0 } }, + }) + ) + mockAxiosCallable.mockResolvedValue(upstreamOk([])) + + const response = await withOrgHeaders(agent().post('/searchRegionRecommendation')).send({ + request: { defaultLabel: 'some-label' }, + }) + + expect(response.status).toBe(200) + expect(mockAxios.post).toHaveBeenCalledTimes(2) + expect(response.body.contents).toEqual([]) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withOrgHeaders(agent().post('/searchRegionRecommendation')).send({ + request: {}, + }) + expect(response.status).toBe(500) + }) +}) + describe('POST /searchV6', () => { it('forwards the search response', async () => { mockAxios.post.mockResolvedValue(upstreamOk({ results: [] })) @@ -175,6 +273,13 @@ describe('POST /searchV6', () => { expect(response.status).toBe(200) }) + it('maps upstream result items through processContent when result is an array', async () => { + mockAxios.post.mockResolvedValue(upstreamOk({ result: [contentItem()] })) + const response = await agent().post('/searchV6').send({ query: 'x' }) + expect(response.status).toBe(200) + expect(response.body.result).toHaveLength(1) + }) + it('returns 500 on an upstream failure', async () => { mockAxios.post.mockRejectedValue(networkError()) const response = await agent().post('/searchV6').send({}) @@ -277,6 +382,28 @@ describe('POST /:contentId', () => { const response = await withOrgHeaders(agent().post('/do_1')).send({}) expect(response.status).toBe(500) }) + + it('uses the minimal content field set when hierarchyType=minimal', async () => { + mockAxiosCallable.mockResolvedValue(upstreamOk(contentItem({ identifier: 'do_1' }))) + + const response = await withOrgHeaders( + agent().post('/do_1').query({ hierarchyType: 'minimal' }) + ).send({}) + + expect(response.status).toBe(200) + expect(response.body.identifier).toBe('do_1') + }) + + it('skips field filtering for an unrecognised-but-valid hierarchyType of all', async () => { + mockAxiosCallable.mockResolvedValue(upstreamOk(contentItem({ identifier: 'do_1' }))) + + const response = await withOrgHeaders( + agent().post('/do_1').query({ hierarchyType: 'all' }) + ).send({}) + + expect(response.status).toBe(200) + expect(response.body.identifier).toBe('do_1') + }) }) describe('GET /external-access/:id', () => { @@ -291,6 +418,11 @@ describe('GET /external-access/:id', () => { const response = await withOrgHeaders(agent().get('/external-access/do_1')) expect(response.status).toBe(500) }) + + it('rejects a request missing org/rootOrg headers', async () => { + const response = await agent().get('/external-access/do_1') + expect(response.status).toBe(400) + }) }) describe('POST /:contentId/parent', () => { @@ -299,6 +431,17 @@ describe('POST /:contentId/parent', () => { const response = await withOrgHeaders(agent().post('/do_1/parent')).send({ parentId: 'do_0' }) expect(response.status).toBe(200) }) + + it('rejects a request missing org/rootOrg headers', async () => { + const response = await agent().post('/do_1/parent').send({ parentId: 'do_0' }) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withOrgHeaders(agent().post('/do_1/parent')).send({ parentId: 'do_0' }) + expect(response.status).toBe(500) + }) }) describe('POST /kb/v3/reorder', () => { @@ -313,6 +456,11 @@ describe('POST /kb/v3/reorder', () => { const response = await withOrgHeaders(agent().post('/kb/v3/reorder')).send({}) expect(response.status).toBe(500) }) + + it('rejects a request missing org/rootOrg headers', async () => { + const response = await agent().post('/kb/v3/reorder').send({}) + expect(response.status).toBe(400) + }) }) describe('POST /kb/v2/:apiType', () => { @@ -321,6 +469,17 @@ describe('POST /kb/v2/:apiType', () => { const response = await withOrgHeaders(agent().post('/kb/v2/publish')).send({}) expect(response.status).toBe(200) }) + + it('rejects a request missing org/rootOrg headers', async () => { + const response = await agent().post('/kb/v2/publish').send({}) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withOrgHeaders(agent().post('/kb/v2/publish')).send({}) + expect(response.status).toBe(500) + }) }) describe('POST /kb/:updateType', () => { @@ -329,6 +488,17 @@ describe('POST /kb/:updateType', () => { const response = await withOrgHeaders(agent().post('/kb/status')).send({}) expect(response.status).toBe(200) }) + + it('rejects a request missing org/rootOrg headers', async () => { + const response = await agent().post('/kb/status').send({}) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withOrgHeaders(agent().post('/kb/status')).send({}) + expect(response.status).toBe(500) + }) }) describe('POST /hierarchy/update', () => { @@ -337,6 +507,17 @@ describe('POST /hierarchy/update', () => { const response = await withOrgHeaders(agent().post('/hierarchy/update')).send({}) expect(response.status).toBe(200) }) + + it('rejects a request missing org/rootOrg headers', async () => { + const response = await agent().post('/hierarchy/update').send({}) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withOrgHeaders(agent().post('/hierarchy/update')).send({}) + expect(response.status).toBe(500) + }) }) describe('POST /getWebModuleManifest', () => { @@ -345,6 +526,19 @@ describe('POST /getWebModuleManifest', () => { const response = await agent().post('/getWebModuleManifest').send({ url: 'https://x.test' }) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().post('/getWebModuleManifest').send({ url: 'https://x.test' }) + expect(response.status).toBe(500) + }) + + // NOT covered live: sending a request with no `url` (or an empty one). + // The handler does `res.status(400).send()` with no `return` afterwards, then + // falls through to `axios.get(...)` and `res.json(response.data)` — a second + // response on the same `res`. Reproducing this hits Express's double-send + // path (matches campaign "Pattern A"), which is unsafe to exercise live. + // Flagged to the requester as a real bug rather than reproduced here. }) describe('GET /getWebModuleFiles', () => { @@ -353,4 +547,10 @@ describe('GET /getWebModuleFiles', () => { const response = await agent().get('/getWebModuleFiles').query({ url: 'https://x.test' }) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/getWebModuleFiles').query({ url: 'https://x.test' }) + expect(response.status).toBe(500) + }) }) diff --git a/src/protectedApi_v8/discussionHub/users.test.ts b/src/protectedApi_v8/discussionHub/users.test.ts index 58ed10da1..980507820 100644 --- a/src/protectedApi_v8/discussionHub/users.test.ts +++ b/src/protectedApi_v8/discussionHub/users.test.ts @@ -28,6 +28,7 @@ jest.mock('../../utils/env', () => ({ import axios from 'axios' import { networkError, upstreamOk } from '../../test-support/mockAxios' import { mountRouter } from '../../test-support/mountRouter' +import { extractUserIdFromRequest } from '../../utils/requestExtract' import { getUserByEmail, getUserByUsername, usersApi } from './users' const mockAxios = axios as jest.Mocked @@ -92,6 +93,17 @@ describe('GET /email/:email', () => { expect(response.status).toBe(200) expect(mockAxios.get).not.toHaveBeenCalled() }) + + it('returns 500 when a request-extraction helper throws before getUserByEmail is reached', async () => { + // Covers the route's own catch block (a different code path from the + // documented closure bug below) by making a synchronous dependency throw. + ;(extractUserIdFromRequest as jest.Mock).mockImplementationOnce(() => { + throw new Error('boom') + }) + const response = await agent().get('/email/user@test.com') + expect(response.status).toBe(500) + expect(mockAxios.get).not.toHaveBeenCalled() + }) }) describe('getUserByEmail / getUserByUsername', () => { @@ -106,4 +118,23 @@ describe('getUserByEmail / getUserByUsername', () => { expect(typeof result).toBe('function') expect(mockAxios.get).not.toHaveBeenCalled() }) + + it('getUserByEmail catches a synchronous error thrown while building the request URL', async () => { + // This exercises getUserByEmail's OWN try/catch (a genuinely reachable + // path, unrelated to the documented "closure never invoked" bug) by + // handing it an email whose implicit string coercion throws. + const boom = new Error('bad email') + const badEmail = { toString: () => { throw boom } } + const result = await getUserByEmail(badEmail) + expect(result).toBe(boom) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + + it('getUserByUsername catches a synchronous error thrown while building the request URL', async () => { + const boom = new Error('bad username') + const badUsername = { toString: () => { throw boom } } + const result = await getUserByUsername(badUsername) + expect(result).toBe(boom) + expect(mockAxios.get).not.toHaveBeenCalled() + }) }) diff --git a/src/protectedApi_v8/navigator.test.ts b/src/protectedApi_v8/navigator.test.ts index 9208e98cd..aeedd56d1 100644 --- a/src/protectedApi_v8/navigator.test.ts +++ b/src/protectedApi_v8/navigator.test.ts @@ -161,6 +161,39 @@ describe('GET /role/:roleId/:variantId', () => { expect(response.status).toBe(404) expect(response.body).toEqual({ error: 'Variant Id incorrect' }) }) + + it('rewrites images on every group within the matching variant', async () => { + const roleWithGroup = buildRole({ + variants: [ + { + group: [ + { + certification_mandatory: true, + group_member: [], + lp_groupdesc: 'group desc', + lp_groupid: 'group-1', + lp_groupimage: 'group1.png', + lp_groupname: 'Group One', + }, + ], + variant_description: 'variant desc', + variant_id: 'var-1', + variant_image: 'variant1.png', + variant_name: 'Variant One', + }, + ], + }) + mockAxios.get.mockResolvedValue( + upstreamOk({ + nso_data: [{ arm_id: 1, arm_name: 'Accelerate', roles: [roleWithGroup] }], + }) + ) + const response = await agent().get('/role/role-1/var-1') + expect(response.status).toBe(200) + expect(response.body.group).toHaveLength(1) + expect(response.body.group[0].lp_groupimage).toBe(`${IMG_PREFIX}group1.png`) + expect(response.body.group[0].lp_groupid).toBe('group-1') + }) }) function buildLp(overrides: object = {}) { diff --git a/src/protectedApi_v8/network.test.ts b/src/protectedApi_v8/network.test.ts index f97e991ee..52d5b6dc0 100644 --- a/src/protectedApi_v8/network.test.ts +++ b/src/protectedApi_v8/network.test.ts @@ -24,9 +24,11 @@ jest.mock('../utils/env', () => ({ import axios from 'axios' import { networkError, upstreamOk } from '../test-support/mockAxios' import { mountRouter } from '../test-support/mountRouter' +import { extractUserIdFromRequest } from '../utils/requestExtract' import { networkConnectionApi } from './network' const mockAxios = axios as jest.Mocked +const mockExtractUserIdFromRequest = extractUserIdFromRequest as jest.Mock const agent = () => mountRouter(networkConnectionApi) const withOrg = (req: ReturnType) => req.set('rootorg', 'r1') @@ -52,6 +54,13 @@ describe('GET /connections/requested', () => { const response = await withOrg(agent().get('/connections/requested')) expect(response.status).toBe(500) }) + + it('rejects a request when userId cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().get('/connections/requested')) + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) }) describe('GET /connections/requests/received', () => { @@ -65,6 +74,19 @@ describe('GET /connections/requests/received', () => { const response = await agent().get('/connections/requests/received') expect(response.status).toBe(400) }) + + it('rejects a request when userId cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().get('/connections/requests/received')) + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withOrg(agent().get('/connections/requests/received')) + expect(response.status).toBe(500) + }) }) describe('GET /connections/established', () => { @@ -84,6 +106,13 @@ describe('GET /connections/established', () => { const response = await withOrg(agent().get('/connections/established')) expect(response.status).toBe(500) }) + + it('rejects a request when userId cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().get('/connections/established')) + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) }) describe('GET /connections/established/:id', () => { @@ -97,6 +126,18 @@ describe('GET /connections/established/:id', () => { const response = await agent().get('/connections/established/c1') expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withOrg(agent().get('/connections/established/c1')) + expect(response.status).toBe(500) + }) + + // NOTE: the `!userId` branch here (userId = req.params.id) is unreachable + // live — Express 404s '/connections/established/' and + // '/connections/established//' rather than routing to this handler with an + // empty :id, so there is no HTTP request that produces a falsy req.params.id + // for this route. Left uncovered rather than forced. }) describe('GET /connections/suggests', () => { @@ -110,6 +151,19 @@ describe('GET /connections/suggests', () => { const response = await agent().get('/connections/suggests') expect(response.status).toBe(400) }) + + it('rejects a request when userId cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().get('/connections/suggests')) + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withOrg(agent().get('/connections/suggests')) + expect(response.status).toBe(500) + }) }) describe('POST /add/connection', () => { @@ -152,6 +206,23 @@ describe('POST /update/connection', () => { expect(response.status).toBe(400) expect(mockAxios.post).not.toHaveBeenCalled() }) + + it('rejects a request missing rootorg', async () => { + const response = await agent() + .post('/update/connection') + .send({ connectionId: 'c1', status: 'accepted' }) + expect(response.status).toBe(400) + expect(mockAxios.post).not.toHaveBeenCalled() + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withOrg(agent().post('/update/connection')).send({ + connectionId: 'c1', + status: 'accepted', + }) + expect(response.status).toBe(500) + }) }) describe('POST /connections/recommended', () => { @@ -171,6 +242,13 @@ describe('POST /connections/recommended', () => { const response = await withOrg(agent().post('/connections/recommended')).send({}) expect(response.status).toBe(500) }) + + it('rejects a request when userId cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().post('/connections/recommended')).send({}) + expect(response.status).toBe(400) + expect(mockAxios.post).not.toHaveBeenCalled() + }) }) describe('POST /connections/recommended/userDepartment', () => { @@ -211,4 +289,11 @@ describe('POST /connections/recommended/userDepartment', () => { const response = await withOrg(agent().post('/connections/recommended/userDepartment')).send({}) expect(response.status).toBe(500) }) + + it('rejects a request when userId cannot be resolved', async () => { + mockExtractUserIdFromRequest.mockReturnValueOnce('') + const response = await withOrg(agent().post('/connections/recommended/userDepartment')).send({}) + expect(response.status).toBe(400) + expect(mockAxios.post).not.toHaveBeenCalled() + }) }) diff --git a/src/protectedApi_v8/portal-v3.test.ts b/src/protectedApi_v8/portal-v3.test.ts index 215636b46..1a5c8d1b0 100644 --- a/src/protectedApi_v8/portal-v3.test.ts +++ b/src/protectedApi_v8/portal-v3.test.ts @@ -82,6 +82,13 @@ describe('shared helper: updateDepartment', () => { await updateDepartment('mdo', { body: {}, headers: { wid: 'u1' } }, res) expect(res.statusCode).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.patch.mockRejectedValue(networkError()) + const res = mockRes() + await updateDepartment('mdo', { body: {}, headers: { wid: 'u1' } }, res) + expect(res.statusCode).toBe(500) + }) }) describe('shared helper: addUserRole', () => { @@ -107,6 +114,13 @@ describe('shared helper: updateUserRole', () => { await updateUserRole('mdo', { body: {}, headers: {} }, res) expect(res.statusCode).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.patch.mockRejectedValue(networkError()) + const res = mockRes() + await updateUserRole('mdo', { body: {}, headers: {} }, res) + expect(res.statusCode).toBe(500) + }) }) describe('getRoles (exported helper)', () => { @@ -244,4 +258,137 @@ describe('route wrappers delegate to the shared helpers (smoke check)', () => { const response = await agent().patch('/spv/deptAction/userrole').send({}) expect(response.status).toBe(200) }) + + it('PATCH /spv/department reaches updateDepartment', async () => { + mockAxios.patch.mockResolvedValue(upstreamOk({ updated: true })) + const response = await agent().patch('/spv/department').set('wid', 'u1').send({}) + expect(response.status).toBe(200) + }) + + it('POST /spv/deptAction/userrole reaches addUserRole', async () => { + mockAxios.post.mockResolvedValue(upstreamOk({ added: true })) + const response = await agent().post('/spv/deptAction/userrole').send({}) + expect(response.status).toBe(200) + }) + + it('PATCH /mdo/department reaches updateDepartment', async () => { + mockAxios.patch.mockResolvedValue(upstreamOk({ updated: true })) + const response = await agent().patch('/mdo/department').set('wid', 'u1').send({}) + expect(response.status).toBe(200) + }) + + it('POST /mdo/deptAction/userrole reaches addUserRole', async () => { + mockAxios.post.mockResolvedValue(upstreamOk({ added: true })) + const response = await agent().post('/mdo/deptAction/userrole').send({}) + expect(response.status).toBe(200) + }) + + it('PATCH /mdo/deptAction/userrole reaches updateUserRole', async () => { + mockAxios.patch.mockResolvedValue(upstreamOk({ updated: true })) + const response = await agent().patch('/mdo/deptAction/userrole').send({}) + expect(response.status).toBe(200) + }) + + it('GET /cbp/mydepartment reaches getMyDepartment', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ portal: 'cbp' })) + const response = await agent().get('/cbp/mydepartment') + expect(response.status).toBe(200) + }) + + it('PATCH /cbp/department reaches updateDepartment', async () => { + mockAxios.patch.mockResolvedValue(upstreamOk({ updated: true })) + const response = await agent().patch('/cbp/department').set('wid', 'u1').send({}) + expect(response.status).toBe(200) + }) + + it('POST /cbp/deptAction/userrole reaches addUserRole', async () => { + mockAxios.post.mockResolvedValue(upstreamOk({ added: true })) + const response = await agent().post('/cbp/deptAction/userrole').send({}) + expect(response.status).toBe(200) + }) + + it('PATCH /cbp/deptAction/userrole reaches updateUserRole', async () => { + mockAxios.patch.mockResolvedValue(upstreamOk({ updated: true })) + const response = await agent().patch('/cbp/deptAction/userrole').send({}) + expect(response.status).toBe(200) + }) + + it('GET /frac/mydepartment reaches getMyDepartment', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ portal: 'frac' })) + const response = await agent().get('/frac/mydepartment') + expect(response.status).toBe(200) + }) + + it('GET /cbc/mydepartment reaches getMyDepartment', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ portal: 'cbc' })) + const response = await agent().get('/cbc/mydepartment') + expect(response.status).toBe(200) + }) + + it('PATCH /cbc/department reaches updateDepartment', async () => { + mockAxios.patch.mockResolvedValue(upstreamOk({ updated: true })) + const response = await agent().patch('/cbc/department').set('wid', 'u1').send({}) + expect(response.status).toBe(200) + }) + + it('PATCH /cbc/deptAction/userrole reaches updateUserRole', async () => { + mockAxios.patch.mockResolvedValue(upstreamOk({ updated: true })) + const response = await agent().patch('/cbc/deptAction/userrole').send({}) + expect(response.status).toBe(200) + }) +}) + +/** Branches of the wid-guarded routes not exercised above: the 400 rejection + * path for routes only smoke-tested on their happy path, and the catch-block + * (upstream failure) path for the two full standalone handlers that embed + * their own try/catch (spv & cbc department-by-id lookups). + */ +describe('additional guarded-route branches', () => { + it('GET /spv/department/:deptId rejects without wid', async () => { + const response = await agent().get('/spv/department/d1') + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + + it('GET /spv/department/:deptId forwards an upstream error status', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/spv/department/d1').set('wid', 'u1') + expect(response.status).toBe(500) + }) + + it('POST /spv/department rejects without wid', async () => { + const response = await agent().post('/spv/department').send({}) + expect(response.status).toBe(400) + expect(mockAxios.post).not.toHaveBeenCalled() + }) + + it('DELETE /spv/deleteDepartment/:deptId rejects without wid', async () => { + const response = await agent().delete('/spv/deleteDepartment/d1') + expect(response.status).toBe(400) + expect(mockAxios.delete).not.toHaveBeenCalled() + }) + + it('GET /cbc/department rejects without wid', async () => { + const response = await agent().get('/cbc/department') + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + + it('GET /cbc/department forwards an upstream error status', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/cbc/department').set('wid', 'u1') + expect(response.status).toBe(500) + }) + + it('GET /cbc/department/:deptId rejects without wid', async () => { + const response = await agent().get('/cbc/department/d1') + expect(response.status).toBe(400) + expect(mockAxios.get).not.toHaveBeenCalled() + }) + + it('GET /cbc/department/:deptId forwards an upstream error status', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/cbc/department/d1').set('wid', 'u1') + expect(response.status).toBe(500) + }) }) diff --git a/src/protectedApi_v8/training.test.ts b/src/protectedApi_v8/training.test.ts index 60d61855f..541461f02 100644 --- a/src/protectedApi_v8/training.test.ts +++ b/src/protectedApi_v8/training.test.ts @@ -51,6 +51,21 @@ describe('GET /content/:contentId/trainings', () => { }) }) +describe('GET /trainingsId/sessions', () => { + it('forwards training sessions', async () => { + mockAxios.get.mockResolvedValue(upstreamOk([{ id: 's1' }])) + const response = await agent().get('/trainingsId/sessions') + expect(response.status).toBe(200) + expect(response.body).toEqual([{ id: 's1' }]) + }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/trainingsId/sessions') + expect(response.status).toBe(400) + }) +}) + describe('GET /content/:contentId/trainings/count', () => { it('forwards the training count', async () => { mockAxios.post.mockResolvedValue(upstreamOk({ count: 3 })) @@ -72,6 +87,12 @@ describe('POST /count', () => { const response = await agent().post('/count').send({ identifiers: ['c1', 'c2'] }) expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/count').send({ identifiers: ['c1'] }) + expect(response.status).toBe(400) + }) }) describe('POST /:trainingId (register)', () => { @@ -142,6 +163,12 @@ describe('POST /:trainingId/share', () => { expect(response.status).toBe(200) expect(mockAxios.post).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ shared_with: [] })) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/t1/share').send({}) + expect(response.status).toBe(400) + }) }) describe('GET /watchlist', () => { @@ -150,6 +177,12 @@ describe('GET /watchlist', () => { const response = await agent().get('/watchlist') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/watchlist') + expect(response.status).toBe(400) + }) }) describe('GET /watchlist/content/:contentId/status', () => { @@ -166,6 +199,12 @@ describe('GET /watchlist/content/:contentId/status', () => { expect(response.status).toBe(200) expect(response.body).toEqual({ inWatchlist: false }) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/watchlist/content/c1/status') + expect(response.status).toBe(400) + }) }) describe('POST /watchlist/content/:contentId', () => { @@ -174,6 +213,12 @@ describe('POST /watchlist/content/:contentId', () => { const response = await agent().post('/watchlist/content/c1') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/watchlist/content/c1') + expect(response.status).toBe(400) + }) }) describe('DELETE /watchlist/content/:contentId', () => { @@ -182,6 +227,12 @@ describe('DELETE /watchlist/content/:contentId', () => { const response = await agent().delete('/watchlist/content/c1') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.delete.mockRejectedValue(networkError()) + const response = await agent().delete('/watchlist/content/c1') + expect(response.status).toBe(400) + }) }) describe('GET /trainings/jit', () => { @@ -190,6 +241,12 @@ describe('GET /trainings/jit', () => { const response = await agent().get('/trainings/jit') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/trainings/jit') + expect(response.status).toBe(400) + }) }) describe('POST /trainings/jit', () => { @@ -218,6 +275,12 @@ describe('GET /trainingsForApproval', () => { const response = await agent().get('/trainingsForApproval') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/trainingsForApproval') + expect(response.status).toBe(400) + }) }) describe('PATCH /:trainingId', () => { @@ -226,6 +289,12 @@ describe('PATCH /:trainingId', () => { const response = await agent().patch('/t1').send({ status: 'rejected' }) expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.patch.mockRejectedValue(networkError()) + const response = await agent().patch('/t1').send({ status: 'rejected' }) + expect(response.status).toBe(400) + }) }) describe('GET /trainings/feedback', () => { @@ -242,6 +311,12 @@ describe('GET /trainings/feedback', () => { expect(response.status).toBe(200) expect(response.body[0].date_range).toBe('') }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/trainings/feedback') + expect(response.status).toBe(400) + }) }) describe('GET /feedback/:formId', () => { @@ -250,6 +325,12 @@ describe('GET /feedback/:formId', () => { const response = await agent().get('/feedback/f1') expect(response.status).toBe(200) }) + + it('returns 400 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/feedback/f1') + expect(response.status).toBe(400) + }) }) describe('POST /trainings/:trainingId/feedback', () => { diff --git a/src/protectedApi_v8/user/details.test.ts b/src/protectedApi_v8/user/details.test.ts index 8efa42b29..106fd7f80 100644 --- a/src/protectedApi_v8/user/details.test.ts +++ b/src/protectedApi_v8/user/details.test.ts @@ -192,4 +192,30 @@ describe('wTokenApiMock', () => { }) await expect(wTokenApiMock(req, 'kc-token')).rejects.toThrow('connect ECONNREFUSED') }) + + it('creates a DiscussionHub user when a 404 shows the user is missing, resolving even if creation fails', async () => { + mockRequestPost.mockImplementation((_url, _options, callback) => { + callback(null, {}, { user: { email: 'a@b.com', first_name: 'A', last_name: 'B', wid: 'w1' } }) + }) + mockGetUserByEmail.mockRejectedValue({ response: { status: 404 } }) + mockCreateDiscussionHubUser.mockRejectedValue(new Error('create failed')) + const result = await wTokenApiMock(req, 'kc-token') + expect(result).toEqual({ user: { email: 'a@b.com', first_name: 'A', last_name: 'B', wid: 'w1' } }) + expect(mockCreateDiscussionHubUser).toHaveBeenCalledWith({ + email: 'a@b.com', + fullname: 'A B', + password: 'nbb-pass', + username: 'w1', + }) + }) + + // Covers the outer try/catch of wTokenApiMock (details.ts lines 176-179): + // a request object with no `body` throws synchronously when the code + // reads `req.body.department`, which is caught inside the same + // try/catch and rejects the promise (with `reject()`, no argument) — + // no double-send, no crash. Safe to test live. + it('rejects when building the request options throws synchronously', async () => { + const bodylessReq = { header: () => undefined } + await expect(wTokenApiMock(bodylessReq, 'kc-token')).rejects.toBeUndefined() + }) }) diff --git a/src/protectedApi_v8/user/feedbackV2.test.ts b/src/protectedApi_v8/user/feedbackV2.test.ts index 224d5fa33..5aa617d33 100644 --- a/src/protectedApi_v8/user/feedbackV2.test.ts +++ b/src/protectedApi_v8/user/feedbackV2.test.ts @@ -294,6 +294,18 @@ describe('GET /categories (routing bug)', () => { // intended `${FEEDBACK_API_BASE}/v1/config` endpoint. This is safe to // exercise live (exactly one response is sent), so we assert the actual // (buggy) behavior here rather than skipping it. + // + // COVERAGE NOTE: the body of the '/categories' handler itself (feedbackV2.ts + // lines 278-292 — the rootOrg check, the axios .get(`${...}/config`) call, + // and its try/catch) can NEVER execute through the real mounted router, + // precisely because of the shadowing bug demonstrated below. Reaching those + // lines live would require either fixing the route-registration order in + // feedbackV2.ts (out of scope for this test-only pass) or pulling the + // handler function off feedbackV2Api's internal router stack and invoking + // it directly, bypassing Express dispatch — which contradicts this file's + // convention (see mountRouter.ts) of asserting on real HTTP responses + // rather than on how a handler was reached. Left uncovered on purpose; + // reported upstream instead of faked. it('is actually handled by the :feedbackId route, not the categories handler', async () => { mockAxios.get.mockResolvedValue(upstreamOk({ notActuallyCategories: true })) const response = await withRootOrg(agent().get('/categories')) diff --git a/src/protectedApi_v8/user/goals.test.ts b/src/protectedApi_v8/user/goals.test.ts index c4037e9f5..04019771d 100644 --- a/src/protectedApi_v8/user/goals.test.ts +++ b/src/protectedApi_v8/user/goals.test.ts @@ -31,6 +31,10 @@ import { mountRouter } from '../../test-support/mountRouter' import { goalsApi } from './goals' const mockAxios = axios as jest.Mocked +// Two routes (`POST /` and `PATCH /:goalId`) call axios as a bare function +// (`axios({...})`) rather than via `.get`/`.post`/etc — mocked separately, +// matching the established convention in scoring.test.ts. +const mockAxiosCallable = axios as unknown as jest.Mock const agent = () => mountRouter(goalsApi) const withRootOrg = (req: ReturnType) => req.set('rootOrg', 'r1') @@ -39,6 +43,8 @@ beforeEach(() => { mockAxios.post.mockReset() mockAxios.put.mockReset() mockAxios.delete.mockReset() + mockAxios.patch.mockReset() + mockAxiosCallable.mockReset() }) describe('GET /updateDurationCommonGoal/:goalType/:goalId', () => { @@ -71,6 +77,12 @@ describe('POST /share/:goalType/:goalId', () => { const response = await agent().post('/share/common/g1').send({}) expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().post('/share/common/g1')).send({}) + expect(response.status).toBe(500) + }) }) describe('POST /sharev2/:goalType/:goalId', () => { @@ -79,6 +91,17 @@ describe('POST /sharev2/:goalType/:goalId', () => { const response = await withRootOrg(agent().post('/sharev2/common/g1')).send({}) expect(response.status).toBe(200) }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().post('/sharev2/common/g1').send({}) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().post('/sharev2/common/g1')).send({}) + expect(response.status).toBe(500) + }) }) describe('POST /action/:type/:goalType/:goalId', () => { @@ -112,6 +135,12 @@ describe('GET /action', () => { const response = await agent().get('/action') expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().get('/action')).query({ sourceFields: 'name' }) + expect(response.status).toBe(500) + }) }) describe('GET /common', () => { @@ -122,6 +151,11 @@ describe('GET /common', () => { expect(response.body).toEqual([{ id: 'grp1' }]) }) + it('rejects a request missing rootOrg', async () => { + const response = await agent().get('/common') + expect(response.status).toBe(400) + }) + it('returns 500 on an upstream failure', async () => { mockAxios.get.mockRejectedValue(networkError()) const response = await withRootOrg(agent().get('/common')) @@ -135,6 +169,17 @@ describe('GET /common/:groupId', () => { const response = await withRootOrg(agent().get('/common/grp1')) expect(response.status).toBe(200) }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().get('/common/grp1') + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().get('/common/grp1')) + expect(response.status).toBe(500) + }) }) describe('GET /for-others', () => { @@ -143,6 +188,17 @@ describe('GET /for-others', () => { const response = await withRootOrg(agent().get('/for-others')).query({ sourceFields: 'name' }) expect(response.status).toBe(200) }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().get('/for-others').query({ sourceFields: 'name' }) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().get('/for-others')).query({ sourceFields: 'name' }) + expect(response.status).toBe(500) + }) }) describe('GET /track/:goalType/:goalId', () => { @@ -156,6 +212,12 @@ describe('GET /track/:goalType/:goalId', () => { const response = await agent().get('/track/common/g1') expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().get('/track/common/g1')) + expect(response.status).toBe(500) + }) }) describe('DELETE /:goalType/:goalId', () => { @@ -185,6 +247,19 @@ describe('POST /removeUsers/:goalType/:goalId', () => { }) expect(response.status).toBe(200) }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().post('/removeUsers/common/g1').send({ userIds: ['u2'] }) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().post('/removeUsers/common/g1')).send({ + userIds: ['u2'], + }) + expect(response.status).toBe(500) + }) }) describe('GET /:type', () => { @@ -219,4 +294,83 @@ describe('DELETE /removeContent/:goalId/:contentId', () => { const response = await agent().delete('/removeContent/g1/c1') expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.delete.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().delete('/removeContent/g1/c1')) + expect(response.status).toBe(500) + }) +}) + +describe('PATCH /addContent/:goalId/:contentId', () => { + it('adds content to the goal', async () => { + mockAxios.patch.mockResolvedValue(upstreamOk({ added: true })) + const response = await withRootOrg(agent().patch('/addContent/g1/c1')).query({ + goal_type: 'common', + }) + expect(response.status).toBe(200) + }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().patch('/addContent/g1/c1').query({ goal_type: 'common' }) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.patch.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().patch('/addContent/g1/c1')).query({ + goal_type: 'common', + }) + expect(response.status).toBe(500) + }) +}) + +describe('POST /', () => { + it('creates the goal and updates the content hierarchy', async () => { + mockAxiosCallable + .mockResolvedValueOnce(upstreamOk({ id: 'created-1' })) + .mockResolvedValueOnce(upstreamOk({ id: 'created-1', updated: true })) + const response = await withRootOrg(agent().post('/')).send({ title: 'New goal' }) + expect(response.status).toBe(200) + }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().post('/').send({ title: 'New goal' }) + expect(response.status).toBe(400) + }) + + it('returns 500 when the create call fails', async () => { + mockAxiosCallable.mockRejectedValueOnce(networkError()) + const response = await withRootOrg(agent().post('/')).send({ title: 'New goal' }) + expect(response.status).toBe(500) + }) + + it('returns 500 when the hierarchy-update call fails', async () => { + mockAxiosCallable + .mockResolvedValueOnce(upstreamOk({ id: 'created-1' })) + .mockRejectedValueOnce(networkError()) + const response = await withRootOrg(agent().post('/')).send({ title: 'New goal' }) + expect(response.status).toBe(500) + }) +}) + +describe('PATCH /:goalId', () => { + it('updates the playlist title and content hierarchy', async () => { + mockAxiosCallable + .mockResolvedValueOnce(upstreamOk({ updated: true })) + .mockResolvedValueOnce(upstreamOk({ updated: true })) + const response = await withRootOrg(agent().patch('/g1')).send({ title: 'Renamed' }) + expect(response.status).toBe(200) + }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().patch('/g1').send({ title: 'Renamed' }) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxiosCallable.mockRejectedValueOnce(networkError()) + const response = await withRootOrg(agent().patch('/g1')).send({ title: 'Renamed' }) + expect(response.status).toBe(500) + }) }) diff --git a/src/protectedApi_v8/user/myAnalytics.test.ts b/src/protectedApi_v8/user/myAnalytics.test.ts index 866d06e1e..1fcb2b321 100644 --- a/src/protectedApi_v8/user/myAnalytics.test.ts +++ b/src/protectedApi_v8/user/myAnalytics.test.ts @@ -53,6 +53,61 @@ describe('GET /certification', () => { expect(response.status).toBe(200) expect(response.body).toEqual({ achievements: [{ id: 'c1' }] }) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/certification') + expect(response.status).toBe(500) + }) +}) + +describe('GET /assessment/:contentType', () => { + it('forwards the assessment progress for a content type', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ assessment: [{ id: 'a1' }] })) + const response = await agent() + .get('/assessment/course') + .query({ isCompleted: 'true' }) + expect(response.status).toBe(200) + expect(response.body).toEqual({ assessment: [{ id: 'a1' }] }) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/assessment/course') + expect(response.status).toBe(500) + }) +}) + +describe('GET /timespent/:contentType', () => { + it('forwards the time spent for a content type', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ timespent: 120 })) + const response = await agent() + .get('/timespent/course') + .query({ startDate: '2020-01-01' }) + expect(response.status).toBe(200) + expect(response.body).toEqual({ timespent: 120 }) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/timespent/course') + expect(response.status).toBe(500) + }) +}) + +describe('GET /nsoArtifactsAndCollaborators/:contentType', () => { + it('forwards the nso artifacts and collaborators for a content type', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ artifacts: [] })) + const response = await agent().get('/nsoArtifactsAndCollaborators/course') + expect(response.status).toBe(200) + expect(response.body).toEqual({ artifacts: [] }) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/nsoArtifactsAndCollaborators/course') + expect(response.status).toBe(500) + }) }) describe('GET /skills', () => { @@ -86,6 +141,12 @@ describe('GET /myskills', () => { expect.objectContaining({ headers: expect.objectContaining({ wid: 'other-user' }) }) ) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/myskills') + expect(response.status).toBe(500) + }) }) describe('GET /recommendedSkills', () => { @@ -94,6 +155,12 @@ describe('GET /recommendedSkills', () => { const response = await agent().get('/recommendedSkills') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/recommendedSkills') + expect(response.status).toBe(500) + }) }) describe('GET /allSkills', () => { @@ -102,6 +169,12 @@ describe('GET /allSkills', () => { const response = await agent().get('/allSkills').query({ category: 'tech', pageNo: 1 }) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/allSkills') + expect(response.status).toBe(500) + }) }) describe('GET /isAdmin', () => { @@ -110,6 +183,12 @@ describe('GET /isAdmin', () => { const response = await agent().get('/isAdmin') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/isAdmin') + expect(response.status).toBe(500) + }) }) describe('GET /role/get', () => { @@ -118,6 +197,12 @@ describe('GET /role/get', () => { const response = await agent().get('/role/get') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/role/get') + expect(response.status).toBe(500) + }) }) describe('GET /skillquotient', () => { @@ -126,6 +211,12 @@ describe('GET /skillquotient', () => { const response = await agent().get('/skillquotient') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/skillquotient') + expect(response.status).toBe(500) + }) }) describe('GET /rolequotient', () => { @@ -134,6 +225,12 @@ describe('GET /rolequotient', () => { const response = await agent().get('/rolequotient') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/rolequotient') + expect(response.status).toBe(500) + }) }) describe('GET /skills-role/:roleId', () => { @@ -142,6 +239,12 @@ describe('GET /skills-role/:roleId', () => { const response = await agent().get('/skills-role/role-1') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/skills-role/role-1') + expect(response.status).toBe(500) + }) }) describe('GET /role/getExisting', () => { @@ -150,6 +253,12 @@ describe('GET /role/getExisting', () => { const response = await agent().get('/role/getExisting') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/role/getExisting') + expect(response.status).toBe(500) + }) }) describe('POST /role/add', () => { @@ -172,6 +281,12 @@ describe('POST /skills/add', () => { const response = await agent().post('/skills/add').send({ skill: 'JS' }) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/skills/add').send({}) + expect(response.status).toBe(500) + }) }) describe('POST /role/shareRole', () => { @@ -180,6 +295,12 @@ describe('POST /role/shareRole', () => { const response = await agent().post('/role/shareRole').send({}) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/role/shareRole').send({}) + expect(response.status).toBe(500) + }) }) describe('GET /skill/search', () => { @@ -188,6 +309,12 @@ describe('GET /skill/search', () => { const response = await agent().get('/skill/search').query({ q: 'j' }) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/skill/search').query({ q: 'j' }) + expect(response.status).toBe(500) + }) }) describe('GET /role/delete', () => { @@ -196,6 +323,12 @@ describe('GET /role/delete', () => { const response = await agent().get('/role/delete') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.delete.mockRejectedValue(networkError()) + const response = await agent().get('/role/delete') + expect(response.status).toBe(500) + }) }) describe('POST /role/update', () => { @@ -204,6 +337,12 @@ describe('POST /role/update', () => { const response = await agent().post('/role/update').send({}) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/role/update').send({}) + expect(response.status).toBe(500) + }) }) describe('GET /isApprover', () => { @@ -212,6 +351,12 @@ describe('GET /isApprover', () => { const response = await agent().get('/isApprover') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/isApprover') + expect(response.status).toBe(500) + }) }) describe('GET /skillData', () => { @@ -220,6 +365,12 @@ describe('GET /skillData', () => { const response = await agent().get('/skillData').query({ skill: 'JS' }) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/skillData').query({ skill: 'JS' }) + expect(response.status).toBe(500) + }) }) describe('GET /search', () => { @@ -228,6 +379,12 @@ describe('GET /search', () => { const response = await agent().get('/search').query({ q: 'x' }) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/search').query({ q: 'x' }) + expect(response.status).toBe(500) + }) }) describe('GET /projectEndorsement/getList', () => { @@ -236,6 +393,12 @@ describe('GET /projectEndorsement/getList', () => { const response = await agent().get('/projectEndorsement/getList') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/projectEndorsement/getList') + expect(response.status).toBe(500) + }) }) describe('GET /projectEndorsement/get', () => { @@ -244,6 +407,12 @@ describe('GET /projectEndorsement/get', () => { const response = await agent().get('/projectEndorsement/get') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/projectEndorsement/get') + expect(response.status).toBe(500) + }) }) describe('POST /projectEndorsement/endorseRequest', () => { @@ -252,6 +421,12 @@ describe('POST /projectEndorsement/endorseRequest', () => { const response = await agent().post('/projectEndorsement/endorseRequest').send({}) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/projectEndorsement/endorseRequest').send({}) + expect(response.status).toBe(500) + }) }) describe('POST /projectEndorsement/add', () => { @@ -260,4 +435,74 @@ describe('POST /projectEndorsement/add', () => { const response = await agent().post('/projectEndorsement/add').send({}) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/projectEndorsement/add').send({}) + expect(response.status).toBe(500) + }) +}) + +describe('GET /userProgress/:contentType', () => { + it('returns the analytics data produced by the getMyAnalytics middleware', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ progress: 42 })) + const response = await agent().get('/userProgress/course') + expect(response.status).toBe(200) + expect(response.body).toEqual({ progress: 42 }) + }) + + it('returns 500 when the getMyAnalytics middleware upstream call fails', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/userProgress/course') + expect(response.status).toBe(500) + }) + + it('returns 500 when res.send itself throws (e.g. a circular payload)', async () => { + // tslint:disable-next-line: no-any + const circular: any = {} + circular.self = circular + mockAxios.get.mockResolvedValue(upstreamOk(circular)) + const response = await agent().get('/userProgress/course') + expect(response.status).toBe(500) + }) +}) + +describe('GET /:contentType/learning-history', () => { + it('returns the learning history and progress range extracted from the analytics data', async () => { + mockAxios.get.mockResolvedValue( + upstreamOk({ + learning_history: [{ id: 'lh1' }], + learning_history_progress_range: { low: 1, high: 10 }, + }) + ) + const response = await agent().get('/course/learning-history') + expect(response.status).toBe(200) + expect(response.body).toEqual({ + learningHistory: [{ id: 'lh1' }], + learningHistoryProgress: { low: 1, high: 10 }, + }) + }) + + it('returns 500 when the getMyAnalytics middleware upstream call fails', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/course/learning-history') + expect(response.status).toBe(500) + }) + + it('returns 500 when the analytics data is missing from res.locals', async () => { + // upstreamOk's default parameter only kicks in for `undefined`, so pass + // `null` explicitly to make response.data (and therefore res.locals.myAnalyticsData) null. + mockAxios.get.mockResolvedValue(upstreamOk(null)) + const response = await agent().get('/course/learning-history') + expect(response.status).toBe(500) + }) + + it('returns 500 when res.send itself throws (e.g. a circular payload)', async () => { + // tslint:disable-next-line: no-any + const circular: any = {} + circular.self = circular + mockAxios.get.mockResolvedValue(upstreamOk({ learning_history: circular })) + const response = await agent().get('/course/learning-history') + expect(response.status).toBe(500) + }) }) diff --git a/src/protectedApi_v8/user/playlist.test.ts b/src/protectedApi_v8/user/playlist.test.ts index f3a2740bd..b88cfa9bc 100644 --- a/src/protectedApi_v8/user/playlist.test.ts +++ b/src/protectedApi_v8/user/playlist.test.ts @@ -90,6 +90,12 @@ describe('GET /recent', () => { const response = await agent().get('/recent') expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().get('/recent')).set('org', 'o1') + expect(response.status).toBe(500) + }) }) describe('POST /accept/:playlistId', () => { @@ -112,6 +118,13 @@ describe('POST /accept/:playlistId', () => { const response = await agent().post('/accept/pl-1').send({}) expect(response.status).toBe(400) }) + + it('returns 500 when the accept upstream call fails', async () => { + mockAxiosGet.mockResolvedValue(upstreamOk([{ id: 'pl-1', name: 'x' }])) + mockAxios.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().post('/accept/pl-1')).send({}) + expect(response.status).toBe(500) + }) }) describe('POST /reject/:playlistId', () => { @@ -126,6 +139,11 @@ describe('POST /reject/:playlistId', () => { const response = await withRootOrg(agent().post('/reject/pl-1')).send({}) expect(response.status).toBe(500) }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().post('/reject/pl-1').send({}) + expect(response.status).toBe(400) + }) }) describe('POST /share/:playlistId', () => { @@ -141,6 +159,14 @@ describe('POST /share/:playlistId', () => { const response = await agent().post('/share/pl-1').send({}) expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().post('/share/pl-1')) + .set('Authorization', 'Bearer tok') + .send({ users: ['u2'] }) + expect(response.status).toBe(500) + }) }) describe('GET /:type/:playlistId', () => { @@ -155,6 +181,12 @@ describe('GET /:type/:playlistId', () => { const response = await agent().get('/goal/pl-1') expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().get('/goal/pl-1')) + expect(response.status).toBe(500) + }) }) describe('DELETE /:playlistId', () => { @@ -169,6 +201,12 @@ describe('DELETE /:playlistId', () => { const response = await agent().delete('/pl-1') expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().delete('/pl-1')) + expect(response.status).toBe(500) + }) }) describe('GET /', () => { @@ -182,6 +220,24 @@ describe('GET /', () => { const response = await agent().get('/') expect(response.status).toBe(400) }) + + it('splits shared and owned playlists using shared_by', async () => { + // playlists.result.response items with/without shared_by exercise both + // filter predicates; the pending-playlists fetch reuses the same mock. + mockAxiosGet.mockResolvedValue( + upstreamOk([{ id: 'p1', shared_by: 'other-user' }, { id: 'p2', shared_by: null }]) + ) + const response = await withRootOrg(agent().get('/')) + expect(response.status).toBe(200) + expect(response.body.share).toEqual([{ id: 'p1', shared_by: 'other-user' }]) + expect(response.body.user).toEqual([{ id: 'p2', shared_by: null }]) + }) + + it('returns 500 when the upstream playlists fetch fails', async () => { + mockAxiosGet.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().get('/')) + expect(response.status).toBe(500) + }) }) describe('PATCH /:playlistId', () => { @@ -204,3 +260,97 @@ describe('PATCH /:playlistId', () => { expect(response.status).toBe(500) }) }) + +describe('POST /create', () => { + // Shares the "create content, then patch hierarchy" two-step flow used by + // goals.ts's create-goal route (docs/DUPLICATE-CODE-CLEANUP.md L1-15/L2-10): + // one axios call creates the content, a second PATCHes the hierarchy. + it('creates the playlist content then patches the hierarchy', async () => { + mockAxios + .mockResolvedValueOnce(upstreamOk({ node_id: 'new-content-1' })) // create + .mockResolvedValueOnce(upstreamOk({}, 201)) // hierarchy patch + const response = await withRootOrg(agent().post('/create')) + .set('Authorization', 'Bearer tok') + .send({ playlist_title: 'New Playlist' }) + expect(response.status).toBe(201) + }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().post('/create').send({}) + expect(response.status).toBe(400) + }) + + it('returns 500 when the create upstream call fails', async () => { + mockAxios.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().post('/create')) + .set('Authorization', 'Bearer tok') + .send({ playlist_title: 'New Playlist' }) + expect(response.status).toBe(500) + }) +}) + +describe('POST /:playlistId/:type', () => { + it('adds a content id to the playlist hierarchy', async () => { + mockAxios + .mockResolvedValueOnce(upstreamOk({ result: { content: { childNodes: ['existing-1'] } } })) // hierarchy fetch + .mockResolvedValueOnce(upstreamOk({ updated: true })) // hierarchy update patch + const response = await withRootOrg(agent().post('/pl-1/add')) + .set('Authorization', 'Bearer tok') + .send({ contentIds: ['c1'] }) + expect(response.status).toBe(200) + expect(response.body).toEqual({ updated: true }) + }) + + it('deletes a content id present in the playlist hierarchy', async () => { + mockAxios + .mockResolvedValueOnce(upstreamOk({ result: { content: { childNodes: ['c1'] } } })) + .mockResolvedValueOnce(upstreamOk({ updated: true })) + const response = await withRootOrg(agent().post('/pl-1/delete')) + .set('Authorization', 'Bearer tok') + .send({ contentIds: ['c1'] }) + expect(response.status).toBe(200) + }) + + it('deletes a content id absent from the playlist hierarchy (index not found)', async () => { + mockAxios + .mockResolvedValueOnce(upstreamOk({ result: { content: { childNodes: ['other'] } } })) + .mockResolvedValueOnce(upstreamOk({ updated: true })) + const response = await withRootOrg(agent().post('/pl-1/delete')) + .set('Authorization', 'Bearer tok') + .send({ contentIds: ['c1'] }) + expect(response.status).toBe(200) + }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().post('/pl-1/add').send({ contentIds: ['c1'] }) + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().post('/pl-1/add')) + .set('Authorization', 'Bearer tok') + .send({ contentIds: ['c1'] }) + expect(response.status).toBe(500) + }) +}) + +describe('GET /:type', () => { + it('returns pending playlists', async () => { + mockAxiosGet.mockResolvedValue(upstreamOk([{ id: 'pl-1' }])) + const response = await withRootOrg(agent().get('/pending')) + expect(response.status).toBe(200) + expect(response.body).toEqual([{ id: 'pl-1' }]) + }) + + it('rejects a request missing rootOrg', async () => { + const response = await agent().get('/pending') + expect(response.status).toBe(400) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxiosGet.mockRejectedValue(networkError()) + const response = await withRootOrg(agent().get('/pending')) + expect(response.status).toBe(500) + }) +}) diff --git a/src/protectedApi_v8/user/profile-registry.test.ts b/src/protectedApi_v8/user/profile-registry.test.ts index d5e31c936..0c244671c 100644 --- a/src/protectedApi_v8/user/profile-registry.test.ts +++ b/src/protectedApi_v8/user/profile-registry.test.ts @@ -21,7 +21,15 @@ import axios from 'axios' import fs from 'fs' import { networkError, upstreamOk } from '../../test-support/mockAxios' import { mountRouter } from '../../test-support/mountRouter' -import { profileRegistryApi } from './profile-registry' +import { + degreesMeta, + designationMeta, + getProfileStatus, + govtOrgMeta, + industreisMeta, + profileRegistryApi, + statesMeta, +} from './profile-registry' const mockAxios = axios as jest.Mocked const mockReadFile = fs.readFile as unknown as jest.Mock @@ -91,6 +99,12 @@ describe('POST /updateUserWorkflowRegistry', () => { const response = await agent().post('/updateUserWorkflowRegistry').send({}) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/updateUserWorkflowRegistry').send({}) + expect(response.status).toBe(500) + }) }) describe('GET /getUserRegistry/:osid', () => { @@ -99,6 +113,12 @@ describe('GET /getUserRegistry/:osid', () => { const response = await agent().get('/getUserRegistry/os-1') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().get('/getUserRegistry/os-1') + expect(response.status).toBe(500) + }) }) describe('GET /getUserRegistryById', () => { @@ -121,6 +141,12 @@ describe('POST /searchUserRegistry', () => { const response = await agent().post('/searchUserRegistry').send({ query: 'x' }) expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/searchUserRegistry').send({ query: 'x' }) + expect(response.status).toBe(500) + }) }) describe('GET /getUserRegistryByUser/:id', () => { @@ -129,6 +155,18 @@ describe('GET /getUserRegistryByUser/:id', () => { const response = await agent().get('/getUserRegistryByUser/explicit-id') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/getUserRegistryByUser/explicit-id') + expect(response.status).toBe(500) + }) + + // The `if (!userId) userId = extractUserIdFromRequest(req)` fallback (line + // 186) is not covered here: Express's default param matcher for `:id` + // never produces an empty-string match, so there is no HTTP request that + // reaches this handler with a falsy `req.params.id`. Left uncovered rather + // than forced. }) describe('GET /getMasterNationalities', () => { @@ -140,6 +178,15 @@ describe('GET /getMasterNationalities', () => { expect(response.status).toBe(200) expect(response.body).toEqual(['IN', 'US']) }) + + it('returns 500 when the static file contains invalid JSON', async () => { + // fs.readFile's callback runs synchronously inside our mock, so + // JSON.parse throwing here is caught by the handler's own try/catch + // (single res.status(500).send, no double-send / hang risk). + mockReadFile.mockImplementation((_path, cb) => cb(null, 'not-json')) + const response = await agent().get('/getMasterNationalities') + expect(response.status).toBe(500) + }) }) describe('GET /getMasterLanguages', () => { @@ -151,6 +198,12 @@ describe('GET /getMasterLanguages', () => { expect(response.status).toBe(200) expect(response.body).toEqual({ languages: [{ name: 'English' }, { name: 'Hindi' }] }) }) + + it('returns 500 when the static file contains invalid JSON', async () => { + mockReadFile.mockImplementation((_path, cb) => cb(null, 'not-json')) + const response = await agent().get('/getMasterLanguages') + expect(response.status).toBe(500) + }) }) describe('GET /getProfilePageMeta', () => { @@ -161,4 +214,264 @@ describe('GET /getProfilePageMeta', () => { const response = await agent().get('/getProfilePageMeta') expect(response.status).toBe(200) }) + + // The five `.catch(...)` handlers (govtOrgMeta/industreisMeta/degreesMeta/ + // designationMeta/statesMeta) and the outer catch are not reachable from + // this route: each `xMeta()` call is an `async function` that just returns + // an inner arrow function without invoking it or awaiting anything that + // can reject, so `govtOrgMeta().catch(...)` etc. can never actually settle + // to a rejection. Those functions' real bodies are covered directly below + // instead (see `describe('govtOrgMeta', ...)` etc.). +}) + +describe('POST /createUserRegistryV2/:userId', () => { + it('creates a new registry when none exists', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ result: { UserProfile: [] } })) + mockAxios.post.mockResolvedValue(upstreamOk({ id: 'reg-1' })) + + const response = await agent() + .post('/createUserRegistryV2/user-2') + .send({ name: 'x' }) + + expect(response.status).toBe(200) + expect(mockAxios.post).toHaveBeenCalledWith( + expect.stringContaining('/v1/user/create/profile'), + expect.anything(), + expect.anything() + ) + }) + + it('updates the existing registry when one is found', async () => { + mockAxios.get.mockResolvedValue( + upstreamOk({ result: { UserProfile: [{ osid: 'existing' }] } }) + ) + mockAxios.post.mockResolvedValue(upstreamOk({ updated: true })) + + const response = await agent() + .post('/createUserRegistryV2/user-2') + .send({ name: 'x' }) + + expect(response.status).toBe(200) + expect(mockAxios.post).toHaveBeenCalledWith( + expect.stringContaining('/v1/user/update/profile'), + expect.anything(), + expect.anything() + ) + }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().post('/createUserRegistryV2/user-2').send({}) + expect(response.status).toBe(500) + }) +}) + +/** + * govtOrgMeta / industreisMeta / degreesMeta / statesMeta / designationMeta + * are exported directly, so their real bodies (unreachable through + * /getProfilePageMeta, see comment above) are exercised here as plain unit + * calls: `await xMeta()` returns an inner arrow function, which is then + * invoked and awaited itself. + * + * Note: none of these inner arrow functions have a trailing return after + * `await fs.readFile(...)` — the object built inside the readFile callback + * is computed and then discarded. So every case here resolves `undefined`; + * what differs per test is which branch of the callback (and its + * surrounding try/catch) actually executes. + */ +describe('govtOrgMeta', () => { + it('runs the success branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => + cb(null, JSON.stringify({ cadre: ['C1'], ministries: ['M1'], services: ['S1'] })) + ) + const fn = await govtOrgMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('runs the error branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => cb(new Error('read fail'), null)) + const fn = await govtOrgMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('propagates a synchronous readFile throw', async () => { + mockReadFile.mockImplementation(() => { + throw new Error('boom') + }) + const fn = await govtOrgMeta() + await expect(fn()).rejects.toThrow('boom') + }) +}) + +describe('industreisMeta', () => { + it('runs the success branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => + cb(null, JSON.stringify({ industries: ['IT'] })) + ) + const fn = await industreisMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('runs the error branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => cb(new Error('read fail'), null)) + const fn = await industreisMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('propagates a synchronous readFile throw', async () => { + mockReadFile.mockImplementation(() => { + throw new Error('boom') + }) + const fn = await industreisMeta() + await expect(fn()).rejects.toThrow('boom') + }) +}) + +describe('degreesMeta', () => { + it('runs the success branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => + cb(null, JSON.stringify({ graduations: ['B.Tech'], postGraduations: ['M.Tech'] })) + ) + const fn = await degreesMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('runs the error branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => cb(new Error('read fail'), null)) + const fn = await degreesMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('propagates a synchronous readFile throw', async () => { + mockReadFile.mockImplementation(() => { + throw new Error('boom') + }) + const fn = await degreesMeta() + await expect(fn()).rejects.toThrow('boom') + }) +}) + +describe('statesMeta', () => { + it('runs the success branch of the readFile callback', async () => { + // statesMeta reads `obj.industries` (not `obj.states`) in the real code. + mockReadFile.mockImplementation((_path, cb) => + cb(null, JSON.stringify({ industries: ['State1'] })) + ) + const fn = await statesMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('runs the error branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => cb(new Error('read fail'), null)) + const fn = await statesMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('propagates a synchronous readFile throw', async () => { + mockReadFile.mockImplementation(() => { + throw new Error('boom') + }) + const fn = await statesMeta() + await expect(fn()).rejects.toThrow('boom') + }) +}) + +describe('designationMeta', () => { + it('runs the success branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => + cb(null, JSON.stringify({ designations: ['Officer'], gradePay: ['GP1'] })) + ) + const fn = await designationMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('runs the error branch of the readFile callback', async () => { + mockReadFile.mockImplementation((_path, cb) => cb(new Error('read fail'), null)) + const fn = await designationMeta() + await expect(fn()).resolves.toBeUndefined() + }) + + it('propagates a synchronous readFile throw', async () => { + mockReadFile.mockImplementation(() => { + throw new Error('boom') + }) + const fn = await designationMeta() + await expect(fn()).rejects.toThrow('boom') + }) +}) + +describe('getProfileStatus', () => { + it('returns false when the upstream response has no UserProfile', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ result: {} })) + await expect(getProfileStatus('user-1')).resolves.toBe(false) + }) + + it('returns false when UserProfile is empty', async () => { + mockAxios.get.mockResolvedValue(upstreamOk({ result: { UserProfile: [] } })) + await expect(getProfileStatus('user-1')).resolves.toBe(false) + }) + + it('returns false when the returned profile belongs to a different user', async () => { + mockAxios.get.mockResolvedValue( + upstreamOk({ result: { UserProfile: [{ userId: 'someone-else' }] } }) + ) + await expect(getProfileStatus('user-1')).resolves.toBe(false) + }) + + it('returns false when personalDetails is entirely missing', async () => { + mockAxios.get.mockResolvedValue( + upstreamOk({ result: { UserProfile: [{ userId: 'user-1' }] } }) + ) + await expect(getProfileStatus('user-1')).resolves.toBe(false) + }) + + it('returns false when a required personalDetails field is missing', async () => { + mockAxios.get.mockResolvedValue( + upstreamOk({ + result: { + UserProfile: [ + { + personalDetails: { firstname: 'A' }, + userId: 'user-1', + }, + ], + }, + }) + ) + await expect(getProfileStatus('user-1')).resolves.toBe(false) + }) + + it('returns true when every required personalDetails field is present', async () => { + mockAxios.get.mockResolvedValue( + upstreamOk({ + result: { + UserProfile: [ + { + personalDetails: { + category: 'gen', + dob: '2000-01-01', + domicileMedium: 'en', + firstname: 'A', + gender: 'f', + maritalStatus: 'single', + mobile: '123', + nationality: 'IN', + pincode: '110001', + postalAddress: 'addr', + primaryEmail: 'a@b.com', + surname: 'B', + }, + userId: 'user-1', + }, + ], + }, + }) + ) + await expect(getProfileStatus('user-1')).resolves.toBe(true) + }) + + it('returns false on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + await expect(getProfileStatus('user-1')).resolves.toBe(false) + }) }) diff --git a/src/protectedApi_v8/user/roles.test.ts b/src/protectedApi_v8/user/roles.test.ts index 74c3d188d..23eca2e7d 100644 --- a/src/protectedApi_v8/user/roles.test.ts +++ b/src/protectedApi_v8/user/roles.test.ts @@ -75,6 +75,22 @@ describe('GET /', () => { expect(response.status).toBe(200) expect(response.body).toEqual(['author']) }) + + // The route's catch block (lines 46-47) is otherwise unreachable, since + // getUserRoles() never rethrows. It IS reachable if something else inside + // the try throws synchronously — here, res.json() itself, when handed a + // circular object it can't JSON.stringify. That throw happens before any + // bytes are written, so the catch's single res.status().send() is still + // the only response sent. Safe to run live. + it('falls back to the catch handler when the response body cannot be serialized', async () => { + // tslint:disable-next-line: no-any + const circular: any = {} + circular.self = circular + mockAxiosMethods.get.mockResolvedValue(upstreamOk(circular)) + const response = await withRootOrg(agent().get('/')) + expect(response.status).toBe(500) + expect(response.body).toEqual({ error: 'Failed due to unknown reason' }) + }) }) describe('GET /allRoles', () => { @@ -89,6 +105,21 @@ describe('GET /allRoles', () => { const response = await agent().get('/allRoles') expect(response.status).toBe(400) }) + + // Same reasoning as GET / above: getUserRoles() never rethrows, so the + // only live way to reach this route's catch block (lines 66-67) is a + // synchronous throw elsewhere in the try — here, res.json() choking on a + // circular body. Safe to run live: the throw preempts any response being + // sent, so the catch's send is still the only one. + it('falls back to the catch handler when the response body cannot be serialized', async () => { + // tslint:disable-next-line: no-any + const circular: any = {} + circular.self = circular + mockAxiosMethods.get.mockResolvedValue(upstreamOk(circular)) + const response = await withRootOrg(agent().get('/allRoles')) + expect(response.status).toBe(500) + expect(response.body).toEqual({ error: 'Failed due to unknown reason' }) + }) }) describe('GET /getRolesV2/:userId', () => { @@ -103,6 +134,20 @@ describe('GET /getRolesV2/:userId', () => { const response = await agent().get('/getRolesV2/u2') expect(response.status).toBe(400) }) + + // Same reasoning as GET / above: getUserRoles() never rethrows, so the + // only live way to reach this route's catch block (lines 133-134) is a + // synchronous throw elsewhere in the try — here, res.send() delegating to + // res.json() and choking on a circular body. Safe to run live. + it('falls back to the catch handler when the response body cannot be serialized', async () => { + // tslint:disable-next-line: no-any + const circular: any = {} + circular.self = circular + mockAxiosMethods.get.mockResolvedValue(upstreamOk(circular)) + const response = await withRootOrg(agent().get('/getRolesV2/u2')) + expect(response.status).toBe(500) + expect(response.body).toEqual({ error: 'Failed due to unknown reason' }) + }) }) describe('GET /:userId', () => { @@ -117,6 +162,20 @@ describe('GET /:userId', () => { const response = await agent().get('/u3') expect(response.status).toBe(400) }) + + // Same reasoning as GET / above: getUserRoles() never rethrows, so the + // only live way to reach this route's catch block (lines 86-87) is a + // synchronous throw elsewhere in the try — here, res.json() choking on a + // circular body. Safe to run live. + it('falls back to the catch handler when the response body cannot be serialized', async () => { + // tslint:disable-next-line: no-any + const circular: any = {} + circular.self = circular + mockAxiosMethods.get.mockResolvedValue(upstreamOk(circular)) + const response = await withRootOrg(agent().get('/u3')) + expect(response.status).toBe(500) + expect(response.body).toEqual({ error: 'Failed due to unknown reason' }) + }) }) describe('GET /getUsersV2/:role', () => { diff --git a/src/protectedApi_v8/user/validate.test.ts b/src/protectedApi_v8/user/validate.test.ts new file mode 100644 index 000000000..aa236252f --- /dev/null +++ b/src/protectedApi_v8/user/validate.test.ts @@ -0,0 +1,102 @@ +/** + * PHASE 1 — user/validate.ts. + * + * One route: GET '/'. Builds a body from three extract helpers + * (extractUserEmailFromRequest, extractUserNameFromRequest, + * extractUserIdFromRequest), each with an `|| 'default'` fallback, then calls + * logInfo() and res.send(body). No axios call, no validation branch to gate + * access, and — notably — NO try/catch anywhere in the handler. + * + * SKIPPED LIVE (real bug, not reproduced against a live HTTP response) — + * Pattern E, flagged URGENT/CRITICAL: the three extract calls happen with no + * surrounding try/catch, inside an `async (req, res) => { ... }` handler. In + * production these helpers only ever optional-chain off `req.kauth` and can't + * throw, so this is latent rather than currently triggered. But it only takes + * one of them throwing (e.g. a future refactor, or a caller with a malformed + * req) to turn into an unhandled promise rejection with res.send() never + * called — the request hangs with zero response (Pattern B), and depending on + * Node's unhandledRejection policy can crash the whole process. Reproducing + * that live in this suite (e.g. `mockExtractUserEmailFromRequest.mockImplementation(() => { + * throw new Error('boom') })`) would hang the Jest worker, so it is + * deliberately NOT exercised below. See the "real bugs found" note in the + * test report for MUST VERIFY IN PROD detail. + */ + +jest.mock('../../utils/logger', () => ({ logInfo: jest.fn() })) +jest.mock('../../utils/requestExtract', () => ({ + extractUserEmailFromRequest: jest.fn(), + extractUserIdFromRequest: jest.fn(), + extractUserNameFromRequest: jest.fn(), +})) + +import { mountRouter } from '../../test-support/mountRouter' +import { validateApi } from './validate' +import { + extractUserEmailFromRequest, + extractUserIdFromRequest, + extractUserNameFromRequest, +} from '../../utils/requestExtract' + +const mockExtractUserEmailFromRequest = extractUserEmailFromRequest as jest.Mock +const mockExtractUserIdFromRequest = extractUserIdFromRequest as jest.Mock +const mockExtractUserNameFromRequest = extractUserNameFromRequest as jest.Mock +const agent = () => mountRouter(validateApi) + +beforeEach(() => { + mockExtractUserEmailFromRequest.mockReset() + mockExtractUserIdFromRequest.mockReset() + mockExtractUserNameFromRequest.mockReset() +}) + +/** + * @description Verifies the GET / route echoes back the email, name and + * userId derived from the request when the extract helpers return real + * values, and falls back to the hardcoded demo defaults for whichever + * helper(s) return a falsy value (undefined, null, or empty string). + */ +describe('GET /', () => { + it('should return the email, name and userId from the extract helpers when all are present', async () => { + mockExtractUserEmailFromRequest.mockReturnValue('real.user@example.com') + mockExtractUserNameFromRequest.mockReturnValue('Real User') + mockExtractUserIdFromRequest.mockReturnValue('user-123') + + const response = await agent().get('/') + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + email: 'real.user@example.com', + name: 'Real User', + userId: 'user-123', + }) + }) + + it('should fall back to the demo defaults when all extract helpers return undefined', async () => { + mockExtractUserEmailFromRequest.mockReturnValue(undefined) + mockExtractUserNameFromRequest.mockReturnValue(undefined) + mockExtractUserIdFromRequest.mockReturnValue(undefined) + + const response = await agent().get('/') + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + email: 'user@demo.com', + name: 'demo user', + userId: 'user@demo.com', + }) + }) + + it('should fall back per-field when only some extract helpers return a falsy value', async () => { + mockExtractUserEmailFromRequest.mockReturnValue('real.user@example.com') + mockExtractUserNameFromRequest.mockReturnValue('') + mockExtractUserIdFromRequest.mockReturnValue(null) + + const response = await agent().get('/') + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + email: 'real.user@example.com', + name: 'demo user', + userId: 'user@demo.com', + }) + }) +}) diff --git a/src/protectedApi_v8/workallocation.test.ts b/src/protectedApi_v8/workallocation.test.ts index ffa40d860..d79deb918 100644 --- a/src/protectedApi_v8/workallocation.test.ts +++ b/src/protectedApi_v8/workallocation.test.ts @@ -1,6 +1,16 @@ /** * PHASE 1 — workallocation.ts. Twelve routes, all the same axios-proxy shape: * an optional userId/param guard (400) then a proxied axios call (200 / 500). + * + * PHASE 2 note: the `!workOrderId` / `!workAllocationId` / `!userId` guards on + * GET /getWorkOrderById/:workOrderId, GET /getWorkAllocationById/:workAllocationId, + * GET /getUserBasicInfo/:userId and GET /getWOPdf/:workOrderId read from a + * required route param (`:xxx`), which Express's router only matches against a + * non-empty path segment — confirmed empirically (empty segment, trailing + * slash, double slash, %00 all either 404 or produce a non-empty string param). + * There is no supertest/HTTP request that reaches the handler with that param + * falsy, so those four guard branches are left uncovered rather than faked + * through a non-HTTP path. */ jest.mock('axios') @@ -67,6 +77,12 @@ describe('POST /update', () => { const response = await agent().post('/update').send({}) expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/update').send({}) + expect(response.status).toBe(500) + }) }) describe('POST /userSearch', () => { @@ -89,6 +105,12 @@ describe('GET /user/autocomplete/:searchTerm', () => { const response = await agent().get('/user/autocomplete/abc') expect(response.status).toBe(200) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.get.mockRejectedValue(networkError()) + const response = await agent().get('/user/autocomplete/abc') + expect(response.status).toBe(500) + }) }) describe('POST /v2/add', () => { @@ -103,6 +125,12 @@ describe('POST /v2/add', () => { const response = await agent().post('/v2/add').send({}) expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/v2/add').send({}) + expect(response.status).toBe(500) + }) }) describe('POST /v2/update', () => { @@ -117,6 +145,12 @@ describe('POST /v2/update', () => { const response = await agent().post('/v2/update').send({}) expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/v2/update').send({}) + expect(response.status).toBe(500) + }) }) describe('POST /add/workorder', () => { @@ -151,6 +185,12 @@ describe('POST /update/workorder', () => { const response = await agent().post('/update/workorder').send({}) expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/update/workorder').send({}) + expect(response.status).toBe(500) + }) }) describe('POST /getWorkOrders', () => { @@ -207,6 +247,12 @@ describe('POST /copy/workOrder', () => { const response = await agent().post('/copy/workOrder').send({}) expect(response.status).toBe(400) }) + + it('returns 500 on an upstream failure', async () => { + mockAxios.post.mockRejectedValue(networkError()) + const response = await agent().post('/copy/workOrder').send({}) + expect(response.status).toBe(500) + }) }) describe('GET /getUserBasicInfo/:userId', () => { diff --git a/src/proxies_v8/proxies_v8.test.ts b/src/proxies_v8/proxies_v8.test.ts index 1e7fdfd0f..06ca11a97 100644 --- a/src/proxies_v8/proxies_v8.test.ts +++ b/src/proxies_v8/proxies_v8.test.ts @@ -53,6 +53,9 @@ jest.mock('../utils/env', () => ({ CDN_DOMAIN: 'https://cdn.test', HTTPS_HOST: 'https://auth.test', KONG_API_BASE: 'https://kong.test', + // Only this one notify body needs real '#contentLink' content, to reach + // the contentBody.replace() branch in POST /notifyContentState. + NOTIFY_SEND_FOR_REVIEW_BODY: 'Please review the content #contentLink', S3_BUCKET_URL: 'https://bucket.test/', SB_API_KEY: 'sb-api-key', TIMEOUT: '10000', @@ -63,6 +66,7 @@ import { PassThrough, Readable } from 'stream' import axios from 'axios' import FormData from 'form-data' import request from 'request' +import { replaceCdnUrls } from '../authoring/utils/cdn-url-replacer' import { mountRouter } from '../test-support/mountRouter' import { proxiesV8 } from './proxies_v8' @@ -156,6 +160,185 @@ describe('GET /logout/user', () => { }) }) +describe('POST /upload/action/*', () => { + it('uploads the file and forwards the upstream artifact details', async () => { + mockAxios.mockResolvedValue({ + data: { + params: { status: 'Live' }, + result: { + artifactUrl: 'https://cdn.test/artifact.png', + content_url: 'https://cdn.test/content.png', + identifier: 'do_123', + }, + }, + }) + ;(FormData as unknown as jest.Mock).mockImplementation(() => ({ + append: jest.fn(), + getHeaders: jest.fn(() => ({})), + })) + + const response = await mountRouter(proxiesV8, { + basePath: BASE, + requestProps: { + files: { data: { data: Buffer.from('x'), mimetype: 'image/png', name: 'artifact.png' } }, + }, + }) + .post(`${BASE}/upload/action/upload/content/v3/do_123`) + .send({}) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + artifactUrl: 'https://cdn.test/artifact.png', + content_url: 'https://cdn.test/content.png', + identifier: 'do_123', + status: 'Live', + }) + }) + + it('reports an upload error when the upstream call fails', async () => { + mockAxios.mockRejectedValue(new Error('upload failed')) + ;(FormData as unknown as jest.Mock).mockImplementation(() => ({ + append: jest.fn(), + getHeaders: jest.fn(() => ({})), + })) + + const response = await mountRouter(proxiesV8, { + basePath: BASE, + requestProps: { + files: { data: { data: Buffer.from('x'), mimetype: 'image/png', name: 'artifact.png' } }, + }, + }) + .post(`${BASE}/upload/action/upload/content/v3/do_123`) + .send({}) + + expect(response.status).toBe(200) + expect(response.text).toBe('Error while uploading ..') + }) + + it('reports missing file when no file is attached', async () => { + const response = await agent().post(`${BASE}/upload/action/upload/content/v3/do_123`).send({}) + expect(response.status).toBe(200) + expect(response.text).toBe('File not found') + }) +}) + +describe('POST /private/upload/*', () => { + it('submits the file and forwards a parsed 2xx upstream body', async () => { + const mockSubmit = jest.fn((_opts, cb) => { + const fakeResponse = Object.assign(new Readable({ read() { /* noop */ } }), { + statusCode: 200, + }) + cb(null, fakeResponse) + fakeResponse.emit('data', Buffer.from(JSON.stringify({ uploaded: true }))) + }) + ;(FormData as unknown as jest.Mock).mockImplementation(() => ({ + append: jest.fn(), + submit: mockSubmit, + })) + + const response = await mountRouter(proxiesV8, { + basePath: BASE, + requestProps: { + files: { data: { data: Buffer.from('x'), mimetype: 'text/csv', name: 'private.csv' } }, + }, + }) + .post(`${BASE}/private/upload/some/path`) + .send({}) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ uploaded: true }) + }) + + it('forwards the raw body for a non-2xx upstream response', async () => { + const mockSubmit = jest.fn((_opts, cb) => { + const fakeResponse = Object.assign(new Readable({ read() { /* noop */ } }), { + statusCode: 500, + }) + cb(null, fakeResponse) + fakeResponse.emit('data', Buffer.from('upstream failure')) + }) + ;(FormData as unknown as jest.Mock).mockImplementation(() => ({ + append: jest.fn(), + submit: mockSubmit, + })) + + const response = await mountRouter(proxiesV8, { + basePath: BASE, + requestProps: { + files: { data: { data: Buffer.from('x'), mimetype: 'text/csv', name: 'private.csv' } }, + }, + }) + .post(`${BASE}/private/upload/some/path`) + .send({}) + + expect(response.status).toBe(200) + expect(response.text).toBe('upstream failure') + }) + + // The response's 'data' event is deliberately never emitted here: emitting + // it alongside the error callback would double-send (once from the 'data' + // handler, again from the `if (_err)` branch below it) — so only the error + // path is exercised, matching the identical shape already covered safely + // for /userData/v1/bulkUpload below. + it('sends the raw error when the submit callback errors', async () => { + const mockSubmit = jest.fn((_opts, cb) => { + cb(new Error('submit failed'), { on: jest.fn() }) + }) + ;(FormData as unknown as jest.Mock).mockImplementation(() => ({ + append: jest.fn(), + submit: mockSubmit, + })) + + const response = await mountRouter(proxiesV8, { + basePath: BASE, + requestProps: { + files: { data: { data: Buffer.from('x'), mimetype: 'text/csv', name: 'private.csv' } }, + }, + }) + .post(`${BASE}/private/upload/some/path`) + .send({}) + + expect(response.status).toBe(200) + }) + + it('reports missing file when no file is attached', async () => { + const response = await agent().post(`${BASE}/private/upload/some/path`).send({}) + expect(response.status).toBe(200) + expect(response.text).toBe('File not found') + }) +}) + +describe('GET /action/content/v3/read/*', () => { + it('applies CDN URL replacement to the upstream content', async () => { + mockAxios.mockResolvedValue({ data: { contentUrl: 'https://raw.test/asset.png' } }) + + const response = await agent().get(`${BASE}/action/content/v3/read/do_123`) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ contentUrl: 'https://raw.test/asset.png' }) + }) + + it('falls back to the raw upstream body when CDN replacement throws', async () => { + mockAxios.mockResolvedValue({ data: { contentUrl: 'https://raw.test/asset.png' } }) + ;(replaceCdnUrls as jest.Mock).mockImplementationOnce(() => { + throw new Error('replacement failed') + }) + + const response = await agent().get(`${BASE}/action/content/v3/read/do_123`) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ contentUrl: 'https://raw.test/asset.png' }) + }) + + it('passes an upstream failure to the error middleware', async () => { + mockAxios.mockRejectedValue(new Error('read failed')) + + const response = await agent().get(`${BASE}/action/content/v3/read/do_123`) + + expect(response.status).toBe(500) + }) +}) + describe('POST /userData/v1/bulkUpload', () => { it('rejects a request with no file attached', async () => { const response = await agent().post(`${BASE}/userData/v1/bulkUpload`).send({}) @@ -190,6 +373,57 @@ describe('POST /userData/v1/bulkUpload', () => { expect(response.status).toBe(200) expect(response.body).toEqual({ uploaded: true }) }) + + it('forwards the raw body for a non-2xx bulk upload response', async () => { + mockAxios.mockResolvedValue({ data: { result: { response: { channel: 'ch1' } } } }) + const mockSubmit = jest.fn((_opts, cb) => { + const fakeResponse = Object.assign(new Readable({ read() { /* noop */ } }), { + statusCode: 500, + }) + cb(null, fakeResponse) + fakeResponse.emit('data', Buffer.from('upstream failure')) + }) + ;(FormData as unknown as jest.Mock).mockImplementation(() => ({ + append: jest.fn(), + submit: mockSubmit, + })) + + const response = await mountRouter(proxiesV8, { + basePath: BASE, + requestProps: { + files: { data: { data: Buffer.from('x'), mimetype: 'text/csv', name: 'users.csv' } }, + }, + }) + .post(`${BASE}/userData/v1/bulkUpload`) + .send({}) + + expect(response.status).toBe(200) + expect(response.text).toBe('upstream failure') + }) + + // The response's 'data' event is deliberately never emitted here: see the + // identical reasoning on the /private/upload/* error test above. + it('sends the raw error when the bulk upload submit callback errors', async () => { + mockAxios.mockResolvedValue({ data: { result: { response: { channel: 'ch1' } } } }) + const mockSubmit = jest.fn((_opts, cb) => { + cb(new Error('submit failed'), { on: jest.fn() }) + }) + ;(FormData as unknown as jest.Mock).mockImplementation(() => ({ + append: jest.fn(), + submit: mockSubmit, + })) + + const response = await mountRouter(proxiesV8, { + basePath: BASE, + requestProps: { + files: { data: { data: Buffer.from('x'), mimetype: 'text/csv', name: 'users.csv' } }, + }, + }) + .post(`${BASE}/userData/v1/bulkUpload`) + .send({}) + + expect(response.status).toBe(200) + }) }) describe('GET /userData/v1/bulkUpload', () => { @@ -232,6 +466,65 @@ describe('POST /notifyContentState', () => { expect(response.status).toBe(400) }) + it('sends the notification for contentState=reviewFailed', async () => { + mockAxios.mockResolvedValue({ data: { result: { response: true } } }) + + const response = await agent() + .post(`${BASE}/notifyContentState`) + .send({ contentState: 'reviewFailed' }) + + expect(response.status).toBe(200) + expect(response.text).toBe('Email sent successfully.') + }) + + it('sends the notification for contentState=sendForPublish', async () => { + mockAxios.mockResolvedValue({ data: { result: { response: true } } }) + + const response = await agent() + .post(`${BASE}/notifyContentState`) + .send({ contentState: 'sendForPublish' }) + + expect(response.status).toBe(200) + expect(response.text).toBe('Email sent successfully.') + }) + + it('sends the notification for contentState=publishCompleted', async () => { + mockAxios.mockResolvedValue({ data: { result: { response: true } } }) + + const response = await agent() + .post(`${BASE}/notifyContentState`) + .send({ contentState: 'publishCompleted' }) + + expect(response.status).toBe(200) + expect(response.text).toBe('Email sent successfully.') + }) + + it('sends the notification for contentState=publishFailed', async () => { + mockAxios.mockResolvedValue({ data: { result: { response: true } } }) + + const response = await agent() + .post(`${BASE}/notifyContentState`) + .send({ contentState: 'publishFailed' }) + + expect(response.status).toBe(200) + expect(response.text).toBe('Email sent successfully.') + }) + + it('replaces the #contentLink placeholder when contentLink and contentName are given', async () => { + mockAxios.mockResolvedValue({ data: { result: { response: true } } }) + + const response = await agent() + .post(`${BASE}/notifyContentState`) + .send({ + contentLink: 'https://example.test/content/123', + contentName: 'My Content', + contentState: 'sendForReview', + }) + + expect(response.status).toBe(200) + expect(response.text).toBe('Email sent successfully.') + }) + // Neither a missing contentState NOR an unrecognised one is tested live. // The WHOLE handler has no try/catch at all. Both the initial guard and the // switch's default case call res.status(400).send() WITHOUT returning, so diff --git a/src/publicApi_v8/emailOrMobileLoginSignIn.test.ts b/src/publicApi_v8/emailOrMobileLoginSignIn.test.ts index ca4f9070e..31fd156bb 100644 --- a/src/publicApi_v8/emailOrMobileLoginSignIn.test.ts +++ b/src/publicApi_v8/emailOrMobileLoginSignIn.test.ts @@ -116,6 +116,43 @@ describe('POST /signup', () => { expect(response.status).toBe(500) }) + + it('returns 400 when user creation upstream reports a non-OK response', async () => { + // createuserWithmobileOrEmail's own try/catch swallows the thrown Error + // internally (logs and returns undefined normally, no rejection), so + // `.catch(handleCreateUserError)` never fires; newUserDetails ends up + // falsy and the (safe, single-response, `return`-guarded) else branch + // reports "already exists". + mockAxios + .mockResolvedValueOnce(userSearchResponse(false)) // fetchUserBymobileorEmail + .mockResolvedValueOnce( + upstreamOk({ responseCode: 'CLIENT_ERROR', params: { errmsg: 'Bad email' } }) + ) // createuserWithmobileOrEmail's own axios call + + const response = await agent() + .post('/signup') + .send({ email: 'bad@example.com', firstName: 'A', lastName: 'B' }) + + expect(response.status).toBe(400) + expect(response.body.msg).toContain('already exists') + }) + + it('creates a new user even when the initial existence search errors', async () => { + // fetchUserBymobileorEmail catches its own axios failure and returns + // undefined, so isUserExist is falsy and signup proceeds normally. + mockAxios + .mockRejectedValueOnce(networkError()) // fetchUserBymobileorEmail + .mockResolvedValueOnce(createUserResponse('user-uuid-3')) // createuserWithmobileOrEmail + .mockResolvedValueOnce(upstreamOk()) // updateRoles + mockGetOTP.mockResolvedValue(upstreamOk({ result: { response: 'SUCCESS' } })) + + const response = await agent() + .post('/signup') + .send({ email: 'new2@example.com', firstName: 'A', lastName: 'B' }) + + expect(response.status).toBe(200) + expect(response.body.status).toBe('success') + }) }) describe('POST /generateOtp', () => { @@ -179,6 +216,18 @@ describe('POST /generateOtp', () => { expect(response.status).toBe(500) }) + + it('returns 500 when the user existence lookup throws unexpectedly', async () => { + // getUserDetails (the searchUser call) has no try/catch of its own; a + // rejection there is only caught by the route's own outer catch, giving + // a single, safe 500 response. + mockAxios.mockRejectedValueOnce(networkError()) + + const response = await agent().post('/generateOtp').send({ email: 'a@b.com' }) + + expect(response.status).toBe(500) + expect(response.body.message).toBe('OTP regeneration failed') + }) }) describe('POST /validateOtp', () => { @@ -217,6 +266,21 @@ describe('POST /validateOtp', () => { expect(response.status).toBe(500) }) + + it('still validates successfully when updateRoles fails internally', async () => { + // updateRoles has its own try/catch and returns 'false' on failure rather + // than throwing, so its result (awaited but unused) can't affect the + // response here. + mockAxios.mockRejectedValueOnce(networkError()) // updateRoles' axios call + mockValidateOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/validateOtp') + .send({ email: 'a@b.com', otp: '1234', userUUId: 'u1' }) + + expect(response.status).toBe(200) + expect(response.body.status).toBe(200) + }) }) describe('POST /registerUserWithMobile', () => { @@ -253,6 +317,30 @@ describe('POST /registerUserWithMobile', () => { // until the client times out. Confirmed empirically: this scenario timed out // a real supertest request at the default 30s Jest timeout. Recorded in // docs/PROD-VERIFICATION.md instead of reproduced here. + + // A missing phone is deliberately NOT tested by sending the request either: + // the handler sends 400 WITHOUT returning (no `return` after that + // res.status(400) call, same bug shape as /signup's missing-email case), + // then keeps processing with phone=undefined. Depending on how deep it gets, + // that either double-sends on top of the already-sent 400 (ERR_HTTP_HEADERS_SENT, + // re-thrown unhandled from the outer catch) or silently no-ops past it — not + // safe to reproduce live. Recorded in docs/PROD-VERIFICATION.md instead. + + it('returns 500 when the first name is missing during creation', async () => { + // createuserWithmobileOrEmail throws synchronously (before its own + // try/catch) when fname is falsy; `.catch(handleCreateUserError)` catches + // that rejection and re-throws a string, which is NOT caught by anything + // inside the `if (!isUserExist)` block, so it propagates to the route's + // own outer catch — a single, safe 500 response. + mockAxios.mockResolvedValueOnce(userSearchResponse(false)) // fetchUserBymobileorEmail + + const response = await agent() + .post('/registerUserWithMobile') + .send({ phone: '9876543210' }) // no firstName + + expect(response.status).toBe(500) + expect(response.body.error).toBe('Failed due to unknown reason') + }) }) describe('POST /auth', () => { @@ -309,6 +397,26 @@ describe('POST /auth', () => { expect(response.status).toBe(400) expect(response.body.msg).toBe('Mobile no. or Email Id can not be empty') }) + + it('returns 302 when the token endpoint responds without a data payload', async () => { + mockAxios + .mockResolvedValueOnce(userSearchResponse(true)) // email exists + .mockResolvedValueOnce(userSearchResponse(false)) // mobile check + .mockResolvedValueOnce(upstreamOk(null)) // token endpoint, no data + + const response = await mountRouter(emailOrMobileLogin, { session: session() }) + .post('/auth') + .send({ email: 'a@b.com', password: 'pw' }) + + expect(response.status).toBe(302) + expect(response.body.msg).toMatch(/Authentication failed/) + }) + + // The outermost catch (after the inner token-exchange try/catch) is + // deliberately NOT exercised: everything ahead of it in this handler either + // has its own internal try/catch (fetchUserBymobileorEmail) or is inert + // synchronous code, so there is no legitimate input that reaches it without + // fabricating a broken session/mocks. Left uncovered rather than contrived. }) describe('POST /authv2/*', () => { @@ -337,4 +445,21 @@ describe('POST /authv2/*', () => { expect(response.status).toBe(400) expect(response.body.error).toMatch(/Authentication failed/) }) + + it('returns 302 when the token endpoint responds without a data payload', async () => { + mockAxios.mockResolvedValueOnce(upstreamOk(null)) + + const response = await mountRouter(emailOrMobileLogin, { session: workingSession() }) + .post('/authv2/callback') + .query({ code: 'auth-code-456' }) + .send({}) + + expect(response.status).toBe(302) + expect(response.body.msg).toMatch(/Authentication failed/) + }) + + // As with /auth, the outermost catch is deliberately NOT exercised: the + // only code ahead of the inner token-exchange try/catch is two logInfo + // calls (mocked, non-throwing), so there is no legitimate input that + // reaches it. Left uncovered rather than contrived. }) diff --git a/src/publicApi_v8/nodebbUser.test.ts b/src/publicApi_v8/nodebbUser.test.ts index b34d5cf17..eed95914b 100644 --- a/src/publicApi_v8/nodebbUser.test.ts +++ b/src/publicApi_v8/nodebbUser.test.ts @@ -182,3 +182,38 @@ describe('failure path (single try/catch wraps the whole body — safe to test l expect(result).toBe('uid-10-retry') }) }) + +describe('LRU eviction once the cache reaches MAX_CACHE_SIZE (module-level Map; pure in-memory Map/timestamp bookkeeping, no I/O — safe to exercise live)', () => { + it('evicts the least-recently-used entry once the 50k cap is reached, and re-fetches it from upstream afterwards', async () => { + // Fill the module-level cache up to its 50k cap with brand-new + // identifiers. Every entry inserted here is strictly newer (by + // Date.now()/lastAccessed) than anything cached by earlier describe + // blocks in this file, so the LRU sweep below consumes those older + // entries first and none of the filler entries get evicted mid-fill. + mockAxiosCallable.mockImplementation(async () => upstreamOk({ result: { userId: { uid: 'filler-uid' } } })) + + const FILL_COUNT = 50000 + await Promise.all( + Array.from({ length: FILL_COUNT }, (_, i) => + fetchnodebbUserDetails(`lru-filler-${i}`, 'filler-user', 'Filler User', {}) + ) + ) + + // lru-filler-0 was inserted first and never accessed again, so it is + // the least-recently-used entry once the cache is at capacity. + mockAxiosCallable.mockClear() + mockAxiosCallable.mockImplementation(async () => upstreamOk({ result: { userId: { uid: 'overflow-uid' } } })) + const overflowResult = await fetchnodebbUserDetails('lru-overflow', 'overflow-user', 'Overflow User', {}) + + expect(overflowResult).toBe('overflow-uid') + + // The eviction above must have removed lru-filler-0; fetching it again + // is therefore a cache miss and hits upstream once more. + mockAxiosCallable.mockClear() + mockAxiosCallable.mockImplementation(async () => upstreamOk({ result: { userId: { uid: 'refetched-uid' } } })) + const refetched = await fetchnodebbUserDetails('lru-filler-0', 'filler-user', 'Filler User', {}) + + expect(refetched).toBe('refetched-uid') + expect(mockAxiosCallable).toHaveBeenCalledTimes(1) + }, 60000) +}) diff --git a/src/publicApi_v8/sashaktAuth.test.ts b/src/publicApi_v8/sashaktAuth.test.ts index b28a24436..c3bcc0a98 100644 --- a/src/publicApi_v8/sashaktAuth.test.ts +++ b/src/publicApi_v8/sashaktAuth.test.ts @@ -152,4 +152,73 @@ describe('GET /login', () => { // NOTE: authTokenResponse.data resolving falsy (the `else` branch at // sashaktAuth.ts's 302 response) is a documented double-send bug — not // reproduced live. See docs/PROD-VERIFICATION.md. + + // NOTE: sashaktAuth.ts lines 56-63 (`if (!sashaktData) { res.status(400)... }`, + // no `return`) is a second double-send bug, reachable when + // `userDetails[0]` is a falsy-but-non-throwing primitive (e.g. `0`, `''`, + // `false`) — undefined/null would throw at the `sashaktData.email` access + // one line earlier and land safely in the outer catch instead. Not + // reproduced live; flagged for docs/PROD-VERIFICATION.md. + + it('logs and continues when the exists/email lookup call fails', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('sashakt.test/userDetails')) { + return Promise.resolve(upstreamOk({ userDetails: [sashaktUser], userId: 'sashakt-1' })) + } + if (config.url.includes('exists/email')) return Promise.reject(networkError()) + if (config.url.includes('exists/phone')) return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + if (config.url.includes('user/v3/create')) return Promise.resolve(upstreamOk({ result: { userId: 'new-1' } })) + if (config.url.includes('assign/role') || config.url.includes('private/v1/update')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('openid-connect/token')) return Promise.resolve(upstreamOk({ access_token: 'tok-1' })) + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const response = await agent().get('/login?moduleId=m1&token=tok') + expect(response.status).toBe(200) + expect(response.body.message).toBe('success') + }) + + it('backfills missing academics details on an existing user profile', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('sashakt.test/userDetails')) { + return Promise.resolve(upstreamOk({ userDetails: [sashaktUser], userId: 'sashakt-1' })) + } + if (config.url.includes('exists/email')) return Promise.resolve(upstreamOk({ responseCode: 'OK', result: { exists: true } })) + if (config.url.includes('exists/phone')) return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + if (config.url.includes('private/user/v1/search')) { + return Promise.resolve( + upstreamOk({ result: { response: { content: [{ id: 'u1', profileDetails: { profileReq: {} } }] } } }) + ) + } + if (config.url.includes('private/v1/update')) return Promise.resolve(upstreamOk({})) + if (config.url.includes('openid-connect/token')) return Promise.resolve(upstreamOk({ access_token: 'tok-1' })) + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const response = await agent().get('/login?moduleId=m1&token=tok') + expect(response.status).toBe(200) + expect(response.body.message).toBe('success') + }) + + it('logs and continues when the mandatory-profile-details search call fails', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('sashakt.test/userDetails')) { + return Promise.resolve(upstreamOk({ userDetails: [sashaktUser], userId: 'sashakt-1' })) + } + if (config.url.includes('exists/email')) return Promise.resolve(upstreamOk({ responseCode: 'OK', result: { exists: true } })) + if (config.url.includes('exists/phone')) return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + if (config.url.includes('private/user/v1/search')) return Promise.reject(networkError()) + if (config.url.includes('openid-connect/token')) return Promise.resolve(upstreamOk({ access_token: 'tok-1' })) + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const response = await agent().get('/login?moduleId=m1&token=tok') + expect(response.status).toBe(200) + expect(response.body.message).toBe('success') + }) }) diff --git a/src/publicApi_v8/signup.test.ts b/src/publicApi_v8/signup.test.ts index 2076e338b..f0c3b70fd 100644 --- a/src/publicApi_v8/signup.test.ts +++ b/src/publicApi_v8/signup.test.ts @@ -189,6 +189,33 @@ describe('POST / (signup)', () => { // handler unconditionally calls res.json(...) right after its .catch sends // a 400, a guaranteed double-send with no way to avoid it via mocking (see // file header). + + // These two cover the route's own try/catch (lines 61-65), reached here by + // making checkUniqueKey itself throw synchronously when called — a plain + // synchronous throw inside the try block, safely caught by the handler's + // own catch with a single response. This is distinct from the detached + // async-callback bugs documented above, which the outer catch can't see. + it('returns the upstream status/data when checkUniqueKey throws synchronously', async () => { + mockCheckUniqueKey.mockImplementation(() => { + throw upstreamError(403, { error: 'boom' }) + }) + + const response = await agent().post('/').send(body) + + expect(response.status).toBe(403) + expect(response.body).toEqual({ error: 'boom' }) + }) + + it('falls back to 500 when checkUniqueKey throws synchronously with no .response', async () => { + mockCheckUniqueKey.mockImplementation(() => { + throw new Error('boom') + }) + + const response = await agent().post('/').send(body) + + expect(response.status).toBe(500) + expect(response.body).toEqual({}) + }) }) describe('POST /create/:uniqueId', () => { @@ -301,4 +328,18 @@ describe('POST /create/:uniqueId', () => { // NOTE: checkUUIDMaster rejecting (the "invalid/expired code" case) is NOT // tested live — it is the URGENT double-send-cascading-to-a-process-crash // bug documented in the file header above. + + // Safe variant of the "no result" path: checkUUIDMaster RESOLVES (does not + // reject) with a falsy value, so the .catch handler never runs at all and + // only the `else` branch (line 113) sends a response — a single, safe send. + it('returns 400 when checkUUIDMaster resolves with no result', async () => { + mockCheckUUIDMaster.mockResolvedValue(undefined) + + const response = await agent().post('/create/CODE1').send() + + expect(response.status).toBe(400) + expect(response.body).toEqual({ + msg: 'Could not process the request, please try again after some time!!', + }) + }) }) diff --git a/src/publicApi_v8/signupWithAutoLogin.test.ts b/src/publicApi_v8/signupWithAutoLogin.test.ts index 89c1132b5..d2fc389d8 100644 --- a/src/publicApi_v8/signupWithAutoLogin.test.ts +++ b/src/publicApi_v8/signupWithAutoLogin.test.ts @@ -140,6 +140,81 @@ describe('POST /register', () => { // NOTE: "email/phone both missing" and "user already exists" paths are // documented double-send bugs above — not reproduced live. + + it('returns 500 when account creation fails (createAccount swallows its own error and returns undefined; accessing newUserDetail.data then throws, caught by the outer handler)', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email') || config.url.includes('exists/phone')) { + return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + } + if (config.url.includes('user/v3/create')) { + return Promise.reject(networkError()) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const response = await agent().post('/register').send({ phone: '9876543210' }) + expect(response.status).toBe(500) + expect(response.body.message).toBe('Sorry ! User not created. Please try again in sometime.') + }) + + it('still creates the account and sends an OTP when profileUpdate fails (profileUpdate swallows its own error internally)', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email') || config.url.includes('exists/phone')) { + return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + } + if (config.url.includes('user/v3/create')) { + return Promise.resolve(upstreamOk({ result: { userId: 'new-user-1' } })) + } + if (config.url.includes('private/v1/update')) { + return Promise.reject(networkError()) + } + if (config.url.includes('control.msg91.com/api/v5/otp') && !config.url.includes('verify') && !config.url.includes('retry')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const response = await agent().post('/register').send({ phone: '9876543210' }) + expect(response.status).toBe(200) + }) + + it('treats a responseCode "OK" / exists:false lookup as "user does not exist" and proceeds with registration', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email') || config.url.includes('exists/phone')) { + return Promise.resolve(upstreamOk({ responseCode: 'OK', result: { exists: false } })) + } + if (config.url.includes('user/v3/create')) { + return Promise.resolve(upstreamOk({ result: { userId: 'new-user-1' } })) + } + if (config.url.includes('private/v1/update')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('control.msg91.com/api/v5/otp') && !config.url.includes('verify') && !config.url.includes('retry')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const response = await agent().post('/register').send({ phone: '9876543210' }) + expect(response.status).toBe(200) + }) + + it('proceeds with registration when the exists-lookup call itself rejects (fetchUserBymobileorEmail swallows its own error)', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email') || config.url.includes('exists/phone')) { + return Promise.reject(networkError()) + } + if (config.url.includes('user/v3/create')) { + return Promise.resolve(upstreamOk({ result: { userId: 'new-user-1' } })) + } + if (config.url.includes('private/v1/update')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('control.msg91.com/api/v5/otp') && !config.url.includes('verify') && !config.url.includes('retry')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const response = await agent().post('/register').send({ phone: '9876543210' }) + expect(response.status).toBe(200) + }) }) describe('POST /validateOtpWithLogin', () => { @@ -197,4 +272,69 @@ describe('POST /validateOtpWithLogin', () => { .send({ otp: '1234', phone: '9876543210', userUUId: 'user-1' }) expect(response.status).toBe(500) }) + + it('completes phone autologin even when role-assignment fails (updateRoles swallows its own error internally)', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('msg91.com/api/v5/otp/verify')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + if (config.url.includes('user/private/v1/assign/role')) { + return Promise.reject(networkError()) + } + if (config.url.includes('openid-connect/token')) { + return Promise.resolve(upstreamOk({ access_token: 'access-tok-1' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const { agent: sessionAgent } = agentWithSession() + const response = await sessionAgent + .post('/validateOtpWithLogin') + .send({ otp: '1234', phone: '9876543210', userUUId: 'user-1' }) + + expect(response.status).toBe(200) + }) + + it('completes email autologin: verifies OTP, regenerates the session, exchanges for a Keycloak token', async () => { + mockValidateOTP.mockResolvedValue({ data: { result: { response: 'SUCCESS' } } }) + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('user/private/v1/assign/role')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('openid-connect/token')) { + return Promise.resolve(upstreamOk({ access_token: 'access-tok-2' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-2' }) + + const { agent: sessionAgent } = agentWithSession() + const response = await sessionAgent + .post('/validateOtpWithLogin') + .send({ otp: '1234', email: 'a@b.com', userUUId: 'user-2' }) + + expect(response.status).toBe(200) + expect(mockGetCurrentUserRoles).toHaveBeenCalledWith(expect.anything(), 'access-tok-2') + }) + + it('returns 400 when the Keycloak token exchange fails after OTP verification', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('msg91.com/api/v5/otp/verify')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + if (config.url.includes('user/private/v1/assign/role')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('openid-connect/token')) { + return Promise.reject(networkError()) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const { agent: sessionAgent } = agentWithSession() + const response = await sessionAgent + .post('/validateOtpWithLogin') + .send({ otp: '1234', phone: '9876543210', userUUId: 'user-1' }) + expect(response.status).toBe(400) + }) }) diff --git a/src/publicApi_v8/signupWithAutoLoginOrgForm.test.ts b/src/publicApi_v8/signupWithAutoLoginOrgForm.test.ts index a6e79acdd..8ae55e7be 100644 --- a/src/publicApi_v8/signupWithAutoLoginOrgForm.test.ts +++ b/src/publicApi_v8/signupWithAutoLoginOrgForm.test.ts @@ -15,6 +15,10 @@ jest.mock('../utils/logger', () => ({ logError: jest.fn(), logInfo: jest.fn() }) jest.mock('../utils/emailHashPasswordGenerator', () => ({ encryptData: jest.fn(() => 'enc-pw') })) jest.mock('./otp', () => ({ getOTP: jest.fn(), validateOTP: jest.fn() })) jest.mock('./rolePermission', () => ({ getCurrentUserRoles: jest.fn() })) +// Real uuid v4 output is fine for every existing test (nothing asserts on the +// generated id), but one new test needs to force it to throw synchronously +// to reach updateUserStatusInDatabase's outer catch block. +jest.mock('uuid', () => ({ v4: jest.fn(() => 'mock-uuid-1') })) jest.mock('../utils/env', () => ({ CONSTANTS: { APP_SSO_KEYCLOAK_SECRET: 'secret', @@ -34,7 +38,10 @@ jest.mock('../utils/env', () => ({ import axios from 'axios' import jwtDecode from 'jwt-decode' -import { upstreamOk } from '../test-support/mockAxios' +import { Pool } from 'pg' +import { v4 as uuidv4 } from 'uuid' +import { encryptData } from '../utils/emailHashPasswordGenerator' +import { networkError, upstreamError, upstreamOk } from '../test-support/mockAxios' import { mountRouter } from '../test-support/mountRouter' import { getOTP, validateOTP } from './otp' import { signupWithAutoLoginOrgForm } from './signupWithAutoLoginOrgForm' @@ -43,6 +50,22 @@ const mockAxios = axios as unknown as jest.Mock const mockGetOTP = getOTP as jest.Mock const mockValidateOTP = validateOTP as jest.Mock const mockJwtDecode = jwtDecode as jest.Mock +const mockEncryptData = encryptData as jest.Mock +const mockUuidv4 = uuidv4 as jest.Mock + +// The pg Pool is constructed once at import time inside the module under +// test (`new (require('pg')).Pool(...)`), and the mock factory above hands +// back a fresh { on, query } per call. Pull out the single instance actually +// wired to pgPool so tests can drive its query() behaviour. +// +// NOTE: this does NOT give access to the historical pgPool.on('error'/ +// 'connect'/'remove', ...) calls made at that same import time — jest.config.js +// sets `clearMocks: true`, which wipes every mock's recorded calls before +// EACH test runs (including the first), so that history is gone before any +// it() body executes. Those three handler bodies are therefore left +// uncovered; see final report. +const mockPoolInstance = (Pool as unknown as jest.Mock).mock.results[0].value +const mockPgQuery = mockPoolInstance.query as jest.Mock const agent = () => mountRouter(signupWithAutoLoginOrgForm) @@ -59,7 +82,11 @@ function workingSession() { const notFound = upstreamOk({ responseCode: 'OK', result: { exists: false } }) const created = upstreamOk({ responseCode: 'OK', result: { userId: 'new-user-1' } }) -beforeEach(() => mockAxios.mockReset()) +beforeEach(() => { + mockAxios.mockReset() + mockPgQuery.mockReset() + mockPgQuery.mockResolvedValue(undefined) +}) describe('POST /register', () => { it('rejects a request with neither email nor phone', async () => { @@ -154,6 +181,238 @@ describe('POST /register', () => { expect(response.status).toBe(200) expect(response.body.userId).toBe('u1') }) + + it('treats a failed existence-check call as "does not exist" and proceeds with creation', async () => { + mockAxios + .mockRejectedValueOnce(networkError()) // fetch by email fails -> caught, returns undefined + .mockResolvedValueOnce(notFound) // fetch by phone + .mockResolvedValueOnce(created) // createAccount + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + mockGetOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/register') + .send({ email: 'fetch-check-fails@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + expect(response.body.userId).toBe('new-user-1') + }) + + it('falls back to "User already exists" when the migration search call itself rejects', async () => { + const alreadyExists = upstreamOk({ responseCode: 'OK', result: { exists: true } }) + mockAxios + .mockResolvedValueOnce(alreadyExists) // fetch by email -> exists + .mockResolvedValueOnce(notFound) // fetch by phone + .mockRejectedValueOnce(networkError()) // searchSb rejects -> caught by inner try/catch + + const response = await agent() + .post('/register') + .send({ email: 'search-fail@example.com', firstName: 'A' }) + + expect(response.status).toBe(400) + expect(response.body.msg).toBe('User already exists') + }) + + it('reports a successful migration when the migrate API itself returns SUCCESS', async () => { + const alreadyExists = upstreamOk({ responseCode: 'OK', result: { exists: true } }) + mockAxios + .mockResolvedValueOnce(alreadyExists) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce( + upstreamOk({ + result: { response: { content: [{ identifier: 'u2', rootOrgName: 'aastrika' }] } }, + }) + ) + .mockResolvedValueOnce(upstreamOk({ result: { response: 'SUCCESS' } })) // migrateUserToOrg succeeds + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + + const response = await agent() + .post('/register') + .send({ email: 'migrate-success@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + expect(response.body.userId).toBe('u2') + }) + + it('still reports success (with migration marked failed) when the migrate API call itself rejects', async () => { + const alreadyExists = upstreamOk({ responseCode: 'OK', result: { exists: true } }) + mockAxios + .mockResolvedValueOnce(alreadyExists) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce( + upstreamOk({ + result: { response: { content: [{ identifier: 'u3', rootOrgName: 'aastrika' }] } }, + }) + ) + .mockRejectedValueOnce(networkError()) // migrateUserToOrg axios call rejects + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + + const response = await agent() + .post('/register') + .send({ email: 'migrate-reject@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + expect(response.body.userId).toBe('u3') + }) + + it('returns 500 when account creation succeeds but no userId is returned', async () => { + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(upstreamOk({ result: {} })) // createAccount "succeeds" without a userId + + const response = await agent() + .post('/register') + .send({ email: 'no-userid@example.com', firstName: 'A' }) + + expect(response.status).toBe(500) + }) + + it('returns 500 when password generation throws before any upstream call is made', async () => { + mockEncryptData.mockImplementationOnce(() => { + throw new Error('hash boom') + }) + + const response = await agent() + .post('/register') + .send({ email: 'no-password@example.com', firstName: 'A' }) + + expect(response.status).toBe(500) + }) + + it('creates a new user and still succeeds when role assignment reports success via rolesAssigned', async () => { + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockResolvedValueOnce(upstreamOk({ responseCode: 'OK', result: { rolesAssigned: true } })) // updateRoles success + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + mockGetOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/register') + .send({ email: 'roles-ok@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + }) + + it('still succeeds when role assignment fails with an axios-style error (has .response)', async () => { + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockRejectedValueOnce(upstreamError(500, { error: 'role service down' })) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + mockGetOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/register') + .send({ email: 'roles-axios-err@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + }) + + it('still succeeds when role assignment throws a plain Error', async () => { + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockRejectedValueOnce(new Error('boom')) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + mockGetOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/register') + .send({ email: 'roles-plain-err@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + }) + + it('still succeeds when role assignment rejects with a non-Error value', async () => { + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockRejectedValueOnce({ weird: 'rejection' }) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + mockGetOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/register') + .send({ email: 'roles-weird-err@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + }) + + it('still succeeds when profile update fails', async () => { + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockRejectedValueOnce(networkError()) // profileUpdate rejects + mockGetOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/register') + .send({ email: 'profile-fail@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + }) + + it('returns 500 when sending the phone OTP via msg91 fails', async () => { + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + .mockRejectedValueOnce(networkError()) // msg91 send fails + + const response = await agent() + .post('/register') + .send({ firstName: 'A', phone: '9998887776' }) + + expect(response.status).toBe(500) + }) + + it('returns 500 when sending the email OTP fails', async () => { + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + mockGetOTP.mockRejectedValueOnce(new Error('otp service down')) + + const response = await agent() + .post('/register') + .send({ email: 'otp-fail@example.com', firstName: 'A' }) + + expect(response.status).toBe(500) + }) + + it('retries and logs when the PostgreSQL audit insert fails on every attempt', async () => { + mockPgQuery.mockReset() + mockPgQuery.mockRejectedValue(new Error('db down')) + + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + mockGetOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/register') + .send({ email: 'db-retry@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + }, 10000) }) describe('POST /validateOtpWithLogin', () => { @@ -208,4 +467,88 @@ describe('POST /validateOtpWithLogin', () => { expect(response.status).toBe(400) }) + + it('reports OTP validation failed when neither phone nor email is provided', async () => { + const response = await agent() + .post('/validateOtpWithLogin') + .send({ otp: '1234', userId: 'uid-x' }) + + expect(response.status).toBe(400) + expect(response.body.message).toBe('OTP validation failed') + }) + + it('rejects when the phone OTP verification call itself fails', async () => { + mockAxios.mockRejectedValueOnce(networkError()) // msg91 verify rejects + + const response = await agent() + .post('/validateOtpWithLogin') + .send({ otp: '1234', phone: '9876543210', userId: 'uid-3' }) + + expect(response.status).toBe(400) + expect(response.body.message).toBe('Phone OTP validation failed') + }) + + it('rejects when the email OTP verification call itself fails', async () => { + mockValidateOTP.mockRejectedValueOnce(new Error('otp service down')) + + const response = await agent() + .post('/validateOtpWithLogin') + .send({ email: 'a@b.com', otp: '1234', userId: 'uid-4' }) + + expect(response.status).toBe(400) + expect(response.body.message).toBe('Email OTP validation failed') + }) + + it('returns 400 when the final token grant call fails after OTP verification succeeds', async () => { + mockAxios + .mockResolvedValueOnce(upstreamOk({ type: 'success' })) // msg91 verify + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockRejectedValueOnce(networkError()) // grantAccessToken rejects + + const response = await mountRouter(signupWithAutoLoginOrgForm, { session: workingSession() }) + .post('/validateOtpWithLogin') + .send({ otp: '1234', phone: '9876543210', userId: 'uid-5' }) + + expect(response.status).toBe(400) + expect(response.body.error).toBe('Authentication failed ! Please check credentials and try again.') + }) + + // Uses the DEFAULT agent() (no session middleware injected), so req.session + // is genuinely undefined once OTP verification succeeds — reproducing what + // happens if this route is ever hit without session middleware wired up. + // `req.session.user = null` throws synchronously; it's still inside the + // route's outer try/catch, so this is a safe, single-response test. + it('returns 500 when the session is unexpectedly missing after OTP verification', async () => { + mockAxios + .mockResolvedValueOnce(upstreamOk({ type: 'success' })) // msg91 verify + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + + const response = await agent() + .post('/validateOtpWithLogin') + .send({ otp: '1234', phone: '9876543210', userId: 'uid-6' }) + + expect(response.status).toBe(500) + }) +}) + +describe('updateUserStatusInDatabase outer catch (audit logging must never break registration)', () => { + it('still returns 200 when unique id generation throws before the PostgreSQL insert is attempted', async () => { + mockUuidv4.mockImplementationOnce(() => { + throw new Error('uuid boom') + }) + + mockAxios + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(notFound) + .mockResolvedValueOnce(created) + .mockResolvedValueOnce(upstreamOk({})) // updateRoles + .mockResolvedValueOnce(upstreamOk({})) // profileUpdate + mockGetOTP.mockResolvedValue(upstreamOk({})) + + const response = await agent() + .post('/register') + .send({ email: 'uuid-throws@example.com', firstName: 'A' }) + + expect(response.status).toBe(200) + }) }) diff --git a/src/publicApi_v8/signupWithAutoLoginV2.test.ts b/src/publicApi_v8/signupWithAutoLoginV2.test.ts index 85f6330d3..9e3444282 100644 --- a/src/publicApi_v8/signupWithAutoLoginV2.test.ts +++ b/src/publicApi_v8/signupWithAutoLoginV2.test.ts @@ -136,6 +136,103 @@ describe('POST /register', () => { const response = await agent().post('/register').send({ email: 'a@b.com' }) expect(response.status).toBe(500) }) + + it('returns 500 when account creation itself fails (createAccount swallows the error internally, leaving newUserDetail undefined)', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email') || config.url.includes('exists/phone')) { + return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + } + if (config.url.includes('user/v3/create')) { + return Promise.reject(networkError()) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const response = await agent().post('/register').send({ email: 'a@b.com', firstName: 'A' }) + expect(response.status).toBe(500) + }) + + it('continues and returns 200 when role assignment fails (updateRoles swallows the error internally)', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email') || config.url.includes('exists/phone')) { + return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + } + if (config.url.includes('user/v3/create')) { + return Promise.resolve(upstreamOk({ result: { userId: 'new-user-4' } })) + } + if (config.url.includes('assign/role')) { + return Promise.reject(networkError()) + } + if (config.url.includes('private/v1/update')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('control.msg91.com/api/v5/otp') && !config.url.includes('verify') && !config.url.includes('retry')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const response = await agent().post('/register').send({ firstName: 'A', phone: '9876543210' }) + expect(response.status).toBe(200) + }) + + it('continues and returns 200 when the profile update fails (profileUpdate swallows the error internally)', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email') || config.url.includes('exists/phone')) { + return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + } + if (config.url.includes('user/v3/create')) { + return Promise.resolve(upstreamOk({ result: { userId: 'new-user-5' } })) + } + if (config.url.includes('assign/role')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('private/v1/update')) { + return Promise.reject(networkError()) + } + if (config.url.includes('control.msg91.com/api/v5/otp') && !config.url.includes('verify') && !config.url.includes('retry')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const response = await agent().post('/register').send({ firstName: 'A', phone: '9876543210' }) + expect(response.status).toBe(200) + }) + + it('continues registration when the user-exists lookup itself fails (fetchUserBymobileorEmail swallows the error internally)', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email') || config.url.includes('exists/phone')) { + return Promise.reject(networkError()) + } + if (config.url.includes('user/v3/create')) { + return Promise.resolve(upstreamOk({ result: { userId: 'new-user-6' } })) + } + if (config.url.includes('assign/role') || config.url.includes('private/v1/update')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('control.msg91.com/api/v5/otp') && !config.url.includes('verify') && !config.url.includes('retry')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const response = await agent().post('/register').send({ firstName: 'A', phone: '9876543210' }) + expect(response.status).toBe(200) + expect(response.body.userId).toBe('new-user-6') + }) + + // Traced by hand: when both email and phone are absent, the validation + // response at line 139 is missing a `return` (same Pattern-A shape as the + // documented bug below), so execution falls through with userEmail === '' + // and userPhone === ''. That guarantees BOTH later `if (userPhone)` / + // `if (userEmail)` blocks (which are the only other res.* calls in this + // handler) are skipped, so no second response is ever attempted here — safe + // to exercise live. This is still a real defect (wasted downstream calls, + // and fragile: it relies on userPhone/userEmail staying falsy) worth fixing + // by adding `return` at line 139, but it does not double-send today. + it('does not double-send when both email and phone are missing, despite the missing `return` after the validation response', async () => { + mockUserDoesNotExist() + const response = await agent().post('/register').send({ firstName: 'A' }) + expect(response.status).toBe(400) + expect(response.body.msg).toBe('Email id or phone both can not be empty') + }) }) describe('POST /validateOtpWithLogin', () => { @@ -193,6 +290,48 @@ describe('POST /validateOtpWithLogin', () => { expect(response.status).toBe(500) }) + it('completes email autologin: verifies OTP via validateOTP, regenerates the session, exchanges for a Keycloak token', async () => { + mockValidateOTP.mockResolvedValue({ data: { result: { response: 'SUCCESS' } } }) + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('assign/role')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('openid-connect/token')) { + return Promise.resolve(upstreamOk({ access_token: 'access-tok-2' })) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-2' }) + + const { agent: sessionAgent } = agentWithSession() + const response = await sessionAgent + .post('/validateOtpWithLogin') + .send({ otp: '1234', email: 'a@b.com', userId: 'user-2' }) + + expect(response.status).toBe(200) + expect(mockGetCurrentUserRoles).toHaveBeenCalledWith(expect.anything(), 'access-tok-2') + }) + + it('returns 400 when the Keycloak token exchange fails inside the regenerate callback', async () => { + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('msg91.com/api/v5/otp/verify')) { + return Promise.resolve(upstreamOk({ type: 'success' })) + } + if (config.url.includes('assign/role')) { + return Promise.resolve(upstreamOk({})) + } + if (config.url.includes('openid-connect/token')) { + return Promise.reject(networkError()) + } + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + const { agent: sessionAgent } = agentWithSession() + const response = await sessionAgent + .post('/validateOtpWithLogin') + .send({ otp: '1234', phone: '9876543210', userId: 'user-1' }) + expect(response.status).toBe(400) + }) + // NOTE: a request with no `otp` field at all is a documented double-send // bug (section R) — not reproduced live here. }) diff --git a/src/publicApi_v8/tnnmcAuthV2.test.ts b/src/publicApi_v8/tnnmcAuthV2.test.ts index 4efbf2f96..8449944b8 100644 --- a/src/publicApi_v8/tnnmcAuthV2.test.ts +++ b/src/publicApi_v8/tnnmcAuthV2.test.ts @@ -170,4 +170,102 @@ describe('POST /login', () => { // NOTE: authTokenResponse.data being falsy (else branch) is a documented // double-send bug above — not reproduced live. + + it('falls back to new-user registration when the exists/email check throws', async () => { + mockAxios.post.mockResolvedValue(upstreamOk({ data: tnnmcUser, success: true })) + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email')) return Promise.reject(networkError()) + if (config.url.includes('private/user/v1/search')) { + return Promise.resolve(upstreamOk({ result: { response: { content: [] } } })) + } + if (config.url.includes('user/v3/create')) return Promise.resolve(upstreamOk({ result: { userId: 'new-1' } })) + if (config.url.includes('assign/role')) return Promise.resolve(upstreamOk({ result: { response: 'SUCCESS' } })) + if (config.url.includes('private/v1/update')) return Promise.resolve(upstreamOk({ result: { response: 'SUCCESS' } })) + if (config.url.includes('openid-connect/token')) return Promise.resolve(upstreamOk({ access_token: 'tok-1' })) + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const response = await agent().post('/login').send({ token: encodeURIComponent('tnnmc-token') }) + expect(response.status).toBe(200) + expect(mockAxiosCallable).toHaveBeenCalledWith( + expect.objectContaining({ url: expect.stringContaining('user/v3/create') }) + ) + }) + + it('falls back gracefully when the user-search lookup throws', async () => { + mockAxios.post.mockResolvedValue(upstreamOk({ data: tnnmcUser, success: true })) + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email')) return Promise.resolve(upstreamOk({ responseCode: 'OK', result: { exists: true } })) + if (config.url.includes('private/user/v1/search')) return Promise.reject(networkError()) + if (config.url.includes('openid-connect/token')) return Promise.resolve(upstreamOk({ access_token: 'tok-1' })) + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const response = await agent().post('/login').send({ token: encodeURIComponent('tnnmc-token') }) + expect(response.status).toBe(200) + }) + + it('handles a single-word name and continues when the profile update call throws', async () => { + const singleNameUser = { ...tnnmcUser, name: 'Madonna' } + mockAxios.post.mockResolvedValue(upstreamOk({ data: singleNameUser, success: true })) + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email')) return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + if (config.url.includes('private/user/v1/search')) { + return Promise.resolve(upstreamOk({ result: { response: { content: [] } } })) + } + if (config.url.includes('user/v3/create')) return Promise.resolve(upstreamOk({ result: { userId: 'new-1' } })) + if (config.url.includes('assign/role')) return Promise.resolve(upstreamOk({ result: { response: 'SUCCESS' } })) + if (config.url.includes('private/v1/update')) return Promise.reject(networkError()) + if (config.url.includes('openid-connect/token')) return Promise.resolve(upstreamOk({ access_token: 'tok-1' })) + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const response = await agent().post('/login').send({ token: encodeURIComponent('tnnmc-token') }) + expect(response.status).toBe(200) + }) + + it('continues login when migrating an existing user throws', async () => { + mockAxios.post.mockResolvedValue(upstreamOk({ data: tnnmcUser, success: true })) + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email')) return Promise.resolve(upstreamOk({ responseCode: 'OK', result: { exists: true } })) + if (config.url.includes('private/user/v1/search')) { + return Promise.resolve( + upstreamOk({ + result: { response: { content: [{ id: 'u1', rootOrgName: 'aastrika', userId: 'u1' }] } }, + }) + ) + } + if (config.url.includes('user/v1/migrate')) return Promise.reject(networkError()) + if (config.url.includes('assign/role')) return Promise.resolve(upstreamOk({ result: { response: 'SUCCESS' } })) + if (config.url.includes('private/v1/update')) return Promise.resolve(upstreamOk({ result: { response: 'SUCCESS' } })) + if (config.url.includes('openid-connect/token')) return Promise.resolve(upstreamOk({ access_token: 'tok-1' })) + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const response = await agent().post('/login').send({ token: encodeURIComponent('tnnmc-token') }) + expect(response.status).toBe(200) + }) + + it('continues login when assigning the user role throws', async () => { + mockAxios.post.mockResolvedValue(upstreamOk({ data: tnnmcUser, success: true })) + mockAxiosCallable.mockImplementation((config: { url: string }) => { + if (config.url.includes('exists/email')) return Promise.resolve(upstreamOk({ responseCode: 'FAILED' })) + if (config.url.includes('private/user/v1/search')) { + return Promise.resolve(upstreamOk({ result: { response: { content: [] } } })) + } + if (config.url.includes('user/v3/create')) return Promise.resolve(upstreamOk({ result: { userId: 'new-1' } })) + if (config.url.includes('assign/role')) return Promise.reject(networkError()) + if (config.url.includes('private/v1/update')) return Promise.resolve(upstreamOk({ result: { response: 'SUCCESS' } })) + if (config.url.includes('openid-connect/token')) return Promise.resolve(upstreamOk({ access_token: 'tok-1' })) + return Promise.reject(new Error(`Unexpected axios call: ${config.url}`)) + }) + mockJwtDecode.mockReturnValue({ sub: 'f:org:user-1' }) + + const response = await agent().post('/login').send({ token: encodeURIComponent('tnnmc-token') }) + expect(response.status).toBe(200) + }) }) diff --git a/src/publicApi_v8/userDeactivation.test.ts b/src/publicApi_v8/userDeactivation.test.ts index 3c4bdf93b..d8cb521d3 100644 --- a/src/publicApi_v8/userDeactivation.test.ts +++ b/src/publicApi_v8/userDeactivation.test.ts @@ -116,6 +116,31 @@ describe('GET / — successful deactivation', () => { const [config] = assignRoleCall as [{ data: { request: { organisationId: unknown } } }] expect(config.data.request.organisationId).toBeInstanceOf(Promise) }) + + // Covers userDetails()'s own catch block. userDetails() is invoked without + // `await` from updateUserRoles (see bug note above), so its axios call and + // internal try/catch run as a fire-and-forget floating promise: whatever it + // resolves/rejects to is never awaited or read anywhere by the caller. + // Rejecting its axios call here only exercises userDetails()'s own + // try/catch (returning false internally) — it cannot affect the response, + // reject anything the route awaits, or produce an unhandled rejection, so + // it's safe to reproduce live. + it('still returns 200 when userDetails()\'s own fire-and-forget lookup call rejects internally', async () => { + mockAxiosCallable.mockImplementation((config: { method: string; url: string }) => { + if (config.url === UPDATE_URL && config.method === 'PATCH') { + return Promise.resolve(upstreamOk({ responseCode: 'OK', result: { response: 'SUCCESS' } })) + } + if (config.url === ASSIGN_ROLE_URL && config.method === 'POST') { + return Promise.resolve(upstreamOk({ result: { response: 'SUCCESS' } })) + } + // POST to UPDATE_URL: userDetails()'s own lookup — reject to exercise + // its internal catch block (line 33-35 of userDeactivation.ts). + return Promise.reject(new Error('lookup failed')) + }) + const response = await agent().get('/').set('key', 'secret-key').query({ userId: 'u1' }) + expect(response.status).toBe(200) + expect(response.body).toEqual({ message: 'User deactivated successfully' }) + }) }) // NOT reproduced live (Pattern B — zero response / hang): when either @@ -128,3 +153,11 @@ describe('GET / — successful deactivation', () => { // an upstream failure or malformed response here would hang the Jest worker // waiting on supertest's response. Reported in the final summary as a MUST // VERIFY IN PROD item instead. +// +// Also NOT reproduced live: the route's own outer catch block (which does +// call res.status(400)) looks unreachable given the current code. Both +// updateNullProfileDetails() and updateUserRoles() already catch every error +// they can produce internally and resolve to `false`/`true` — they never +// throw or reject — so nothing inside the outer try block can realistically +// throw for the outer catch to receive. Left uncovered rather than forced +// with an artificial/unrealistic mock. diff --git a/src/utils/apiWhiteList.test.ts b/src/utils/apiWhiteList.test.ts index fcdf740a7..e367f3ee2 100644 --- a/src/utils/apiWhiteList.test.ts +++ b/src/utils/apiWhiteList.test.ts @@ -94,6 +94,74 @@ describe('isAllowed', () => { expect(res.status).toHaveBeenCalledWith(403) expect(next).not.toHaveBeenCalled() }) + + // '/reset' is a real API_LIST.URL entry with `checksNeeded: []` (one of only + // three such entries in the whole config; the other two are the /v1/form + // routes). This exercises the `_.isEmpty(URL_RULE_OBJ.checksNeeded)` branch + // that calls next() directly, without building any check promises at all — + // distinct from the ROLE_CHECK-resolves path already covered above. + it('calls next() immediately for a whitelisted route whose config needs no checks', () => { + const { req, res, next } = mockReqRes({ path: '/reset', session: {} }) + isAllowed()(req as any, res as any, next) + expect(next).toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) + + // NOTE (left uncovered on purpose, not a live-test candidate): + // urlChecks.ROLE_CHECK's `_.includes(rolesForURL, 'ALL') && data.length > 0` + // branch (source line 61) can only be reached if some API_LIST entry's + // ROLE_CHECK array contains the literal string 'ALL'. Checked the full + // whitelistApis.ts (ROLE enum + every ROLE_CHECK usage): no entry ever + // uses 'ALL' as a role. So this branch is unreachable via any real, + // currently-configured route and cannot be exercised live without either + // editing whitelistApis.ts (a shared data file, out of scope here) or + // fabricating a fake API_LIST entry (which would misrepresent what the + // live config actually does). Dead code under the current config, not a + // access-control bypass — data.length > 0 with 'ALL' would still require a + // non-empty session role list, it just never gets the chance to run. + + // NOTE (left uncovered on purpose, not a live-test candidate): + // urlChecks.SCOPE_CHECK (source lines 70-94) is likewise unreachable via + // any real route today. It IS wired into one config entry's data + // ('/protected/v8/workallocation/getWorkOrderById/:workOrderId' has a + // SCOPE_CHECK: [MDO_ADMIN] property), but isAllowed() only ever invokes a + // urlChecks. function when it appears in that entry's + // `checksNeeded` array — and that route's checksNeeded is `[CHECK.ROLE]` + // only, never `[CHECK.SCOPE]`. Grepping the entire 1928-line + // whitelistApis.ts confirms checksNeeded is always either `[CHECK.ROLE]` + // or `[]` — CHECK.SCOPE never appears in any checksNeeded array anywhere. + // So SCOPE_CHECK's own logic is entirely dead code under the live config: + // that route is effectively protected by ROLE_CHECK: [PUBLIC] alone, and + // the org-scoped MDO_ADMIN restriction its data suggests was intended is + // never enforced. Flagging this as a real finding (see final report) — + // not something to "fix" by writing a test that pretends it runs. +}) + +// The module-level `jest.mock('./env', ...)` above forces +// PORTAL_API_WHITELIST_CHECK to 'true' for every test in this file, which is +// necessary so the ROLE_CHECK/SCOPE_CHECK/whitelist logic above actually +// runs instead of always short-circuiting to next(). To cover the opposite +// branch (whitelist checking disabled entirely), this describe uses the same +// jest.resetModules() + require() pattern already established in +// firebase-manager.test.ts to load a fresh copy of the module with a +// different CONSTANTS value, scoped to just this one test. The already-bound +// `isAllowed`/`apiWhiteListLogger` imported at the top of this file are +// plain object references captured at initial module load, so they keep +// their original ('true') closure regardless of resetModules() being called +// later — this does not affect any other test in this file. +describe('isAllowed with API whitelist checking turned off', () => { + it('calls next() unconditionally when PORTAL_API_WHITELIST_CHECK is not "true"', () => { + jest.resetModules() + jest.doMock('./env', () => ({ + CONSTANTS: { PORTAL_API_WHITELIST_CHECK: 'false' }, + })) + // tslint:disable-next-line: no-var-requires + const { isAllowed: isAllowedWhenDisabled } = require('./apiWhiteList') + const { req, res, next } = mockReqRes({ path: '/not/a/real/route' }) + isAllowedWhenDisabled()(req as any, res as any, next) + expect(next).toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) }) describe('apiWhiteListLogger', () => { diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts index 160c43d2c..7ed4dcf22 100644 --- a/src/utils/helpers.test.ts +++ b/src/utils/helpers.test.ts @@ -56,6 +56,13 @@ describe('getEmailLocalPart', () => { it('handles an empty string', () => { expect(getEmailLocalPart('')).toBe('') }) + + it('falls back to returning the input unchanged when indexOf throws', () => { + // Passing a non-string bypasses the type annotation at runtime and makes + // `.indexOf` throw, exercising the catch branch. + // tslint:disable-next-line: no-any + expect(getEmailLocalPart(null as any)).toBeNull() + }) }) describe('esBasicAuth', () => { @@ -102,4 +109,11 @@ describe('validateInputWithRegex', () => { await new Promise((resolve) => setTimeout(resolve, 50)) expect(settled).not.toHaveBeenCalled() }) + + it('still never settles when input is falsy (the early "return false" is inside the executor, not resolve)', async () => { + const settled = jest.fn() + validateInputWithRegex('', /abc/).then(settled).catch(settled) + await new Promise((resolve) => setTimeout(resolve, 50)) + expect(settled).not.toHaveBeenCalled() + }) }) diff --git a/src/utils/proxyCreator.test.ts b/src/utils/proxyCreator.test.ts index 95882801a..614ae2838 100644 --- a/src/utils/proxyCreator.test.ts +++ b/src/utils/proxyCreator.test.ts @@ -119,6 +119,40 @@ describe('module load', () => { it('upload proxy error handler logs without throwing', () => { expect(() => uploadOnHandlers.error(new Error('boom'), {}, {})).not.toThrow() }) + + it('upload proxy error handler stringifies a non-Error value without throwing', () => { + expect(() => uploadOnHandlers.error('boom-string', {}, {})).not.toThrow() + }) + + it('proxyRes handler stores the nodebb auth token on the session for the create-user route', () => { + const proxyRes = { headers: { nodebb_auth_token: 'nb-token-1' } } + // tslint:disable-next-line: no-any + const req: any = { originalUrl: '/discussion/user/v1/create', session: {} } + onHandlers.proxyRes(proxyRes, req, mockRes()) + expect(req.session.nodebb_authorization_token).toBe('nb-token-1') + }) + + it('proxyRes handler does nothing when there is no session on the create-user route', () => { + const proxyRes = { headers: { nodebb_auth_token: 'nb-token-1' } } + const req = { originalUrl: '/discussion/user/v1/create' } + expect(() => onHandlers.proxyRes(proxyRes, req, mockRes())).not.toThrow() + }) + + it('proxyRes handler buffers and transforms a hierarchy edit response before ending it', () => { + // tslint:disable-next-line: no-any + const dataHandlers: Record = {} + const proxyRes = { + // tslint:disable-next-line: no-any + on: jest.fn((event: string, handler: Function) => { dataHandlers[event] = handler }), + } + const req = { originalUrl: '/content/v3/hierarchy?mode=edit&src=sunbird' } + const res = mockRes() + onHandlers.proxyRes(proxyRes, req, res) + dataHandlers.data(Buffer.from('{"a":1}')) + dataHandlers.end() + expect(mockReturnData).toHaveBeenCalledWith({ a: 1 }, null, 'hierarchy') + expect(res.end).toHaveBeenCalledWith(JSON.stringify({ transformed: { a: 1 } })) + }) }) describe('proxyCreatorRoute', () => { @@ -248,6 +282,54 @@ describe('proxyCreatorDiscussionSunbird', () => { expect(res.status).toHaveBeenCalledWith(401) expect(mockWeb).not.toHaveBeenCalled() }) + + it('logs the raw error and returns 401 when a non-Error value is thrown', async () => { + mockJwtDecode.mockImplementationOnce(() => { throw 'boom-string' }) + const handler = captureHandler(proxyCreator.proxyCreatorDiscussionSunbird, 'https://discuss.test') + const res = mockRes() + await handler({ originalUrl: '/proxies/v8/discussion/posts', session: {} }, res) + expect(res.status).toHaveBeenCalledWith(401) + expect(mockWeb).not.toHaveBeenCalled() + }) + + it('strips a /uid segment from the URL before proxying', async () => { + mockJwtDecode.mockReturnValue({ name: 'Test', preferred_username: 'test', sub: 'realm:user:u1' }) + mockFetchNodebbUser.mockResolvedValue('nb-99') + const handler = captureHandler(proxyCreator.proxyCreatorDiscussionSunbird, 'https://discuss.test') + const req = { originalUrl: '/proxies/v8/discussion/uid/posts', session: {} } + await handler(req, mockRes()) + expect(mockWeb).toHaveBeenCalledWith( + req, + expect.anything(), + expect.objectContaining({ target: 'https://discuss.test/discussion/posts?_uid=nb-99' }) + ) + }) + + it('collapses a duplicated discussion/topic path segment before proxying', async () => { + mockJwtDecode.mockReturnValue({ name: 'Test', preferred_username: 'test', sub: 'realm:user:u1' }) + mockFetchNodebbUser.mockResolvedValue('nb-100') + const handler = captureHandler(proxyCreator.proxyCreatorDiscussionSunbird, 'https://discuss.test') + const req = { originalUrl: '/proxies/v8/x/discussion/topic/topic/extra', session: {} } + await handler(req, mockRes()) + expect(mockWeb).toHaveBeenCalledWith( + req, + expect.anything(), + expect.objectContaining({ target: 'https://discuss.test/x/discussion/topic/extra?_uid=nb-100' }) + ) + }) + + it('appends the nodebb uid with & when the cleaned URL already has a query string', async () => { + mockJwtDecode.mockReturnValue({ name: 'Test', preferred_username: 'test', sub: 'realm:user:u1' }) + mockFetchNodebbUser.mockResolvedValue('nb-101') + const handler = captureHandler(proxyCreator.proxyCreatorDiscussionSunbird, 'https://discuss.test') + const req = { originalUrl: '/proxies/v8/discussion/posts?foo=bar', session: {} } + await handler(req, mockRes()) + expect(mockWeb).toHaveBeenCalledWith( + req, + expect.anything(), + expect.objectContaining({ target: 'https://discuss.test/discussion/posts?foo=bar&_uid=nb-101' }) + ) + }) }) describe('proxyCreatorKnowledge', () => { @@ -483,4 +565,34 @@ describe('proxyCreatorEtlFrac', () => { const result = res.send('payload') expect(result).toBe(res) }) + + it('logs entity API errors via the registered res error handler', () => { + const { logError } = require('./logger') + const handler = captureHandler(proxyCreator.proxyCreatorEtlFrac, 'https://kong.test') + const req = { originalUrl: '/proxies/v8/entity/v1/search' } + const res = mockRes() + // tslint:disable-next-line: no-any + const onSpy = jest.fn() + res.on = onSpy + handler(req, res) + // tslint:disable-next-line: no-any + const errorHandler = onSpy.mock.calls.find((call: any[]) => call[0] === 'error')[1] + errorHandler(new Error('kong down')) + expect(logError).toHaveBeenCalledWith(expect.stringContaining('kong down')) + }) + + it("logs 'Unknown' when the res error event carries no message", () => { + const { logError } = require('./logger') + const handler = captureHandler(proxyCreator.proxyCreatorEtlFrac, 'https://kong.test') + const req = { originalUrl: '/proxies/v8/entity/v1/search' } + const res = mockRes() + // tslint:disable-next-line: no-any + const onSpy = jest.fn() + res.on = onSpy + handler(req, res) + // tslint:disable-next-line: no-any + const errorHandler = onSpy.mock.calls.find((call: any[]) => call[0] === 'error')[1] + errorHandler({}) + expect(logError).toHaveBeenCalledWith(expect.stringContaining('Unknown')) + }) }) diff --git a/src/utils/test.test.ts b/src/utils/test.test.ts new file mode 100644 index 000000000..ff8d5cd7a --- /dev/null +++ b/src/utils/test.test.ts @@ -0,0 +1,10 @@ +import { test } from './test' + +/** + * @description Verifies the exported `test` constant has its expected shape. + */ +describe('test', () => { + it('should export the expected static key/value object', () => { + expect(test).toEqual({ key: 'a', key1: 'b', key2: 'c' }) + }) +})