Deep-Dive Authentication Doc — Adversarial Review Findings¶

Reviewed: 2026-02-13 Document: docs/deep-dive-authentication-system.md Review Type: Adversarial — 15 of 21 findings verified against source code (6 structural/organizational findings required no code verification) Total Findings: 21 (21 resolved, 0 open)

Finding Status Summary¶

Findings¶

1. Endpoint count inconsistency — RESOLVED¶

Original issue: Line 57 listed 15 endpoints but lines 44, 830, 1535 claimed "13 route registrations."

Resolution: Updated to 16 everywhere (lines 44, 57, 830, 923, 1537). /failure route added to the list.

Verified: auth.route.ts has 16 route registrations. Count matches.

Note: Count is now correct everywhere (16). See findings #20 and #21 for a separate issue: 3 of the 16 endpoints lack Data Entry Point and API Exposed entries.

2. Lockout logic: doc line 227 is wrong, lines 236/819/1388 are correct — RESOLVED (2026-02-13)¶

Category: Factual error (code-verified)

Problem: Line 227 says attempts 7-8 "lock without the correct duration." Lines 236, 819, and 1388 say they "fall through with no lock."

Source code verdict: auth.helpers.ts lines 107-119:

const handleFailedAttempt = async (attempt: LoginAttempt) => {
  const duration = calculateLockDuration(attempt.failedAttemptsCount) as number; // reads count BEFORE increment
  attempt.failedAttemptsCount += 1;                                              // increments
  if (attempt.failedAttemptsCount >= 2 && attempt.failedAttemptsCount <= 6) {    // checks AFTER increment
    attempt.lockUntil = new Date(Date.now() + duration);
  } else if (attempt.failedAttemptsCount >= 9) {
    attempt.lockUntil = new Date(Date.now() + duration);
    attempt.isBlocked = true;
  }
};

The gap: On the 7^th failed login attempt, failedAttemptsCount starts at 6. calculateLockDuration(6) returns 15 minutes. Then count increments to 7. The check 7 <= 6 is false, 7 >= 9 is false — falls through, no lock applied despite 15 minutes being calculated. Same on the 8^th attempt (count 7→8): calculateLockDuration(7) returns 30 minutes but 8 <= 6 is false, 8 >= 9 is false — no lock applied.

The bug is "no lock applied on the 7^th and 8^th failed attempts," NOT "lock with wrong duration" as line 227 claims.

Verdict: Line 227 is factually wrong. Lines 236, 819, 1388 are correct.

Fix: Change line 227 from "lock without the correct duration" to "fall through with no lock applied (7^th and 8^th failed attempts)."

3. setTokenCookie is httpOnly:true — doc line 79 is wrong — RESOLVED (2026-02-13)¶

Category: Security (code-verified) Severity: High — JWT token accessible to client-side JavaScript via XSS

Problem: Doc line 79 says "Cookie named 'token' is set as non-httpOnly in setTokenCookie." Doc line 253 says setTokenCookie uses httpOnly: true.

Source code verdict: auth.utils.ts lines 29-33:

res.cookie("token", token, {
    httpOnly: true,   // ← httpOnly IS true
    secure: process.env.NODE_ENV === "production",
    maxAge: 20 * 24 * 60 * 60 * 1000, // 20 days
});

However, the token cookie does end up non-httpOnly in practice because the frontend (api/auth/index.js line 30) overwrites it via setCookie("token", data.data.token, cookieOptions) using cookies-next, which creates a non-httpOnly cookie. The browser cookie with the same name gets overwritten by the last write (frontend JS runs after Set-Cookie headers are processed).

Also, in the non-rememberMe login path, the controller explicitly clears the backend cookie at line 69 (res.clearCookie("token")), so only the frontend-set non-httpOnly cookie persists.

Nuanced reality: - Backend setTokenCookie: httpOnly: true, 20 days - Backend Google OAuth (google-oauth.controller.ts:60-64): httpOnly: true, 7 days - Backend auth middleware refresh (auth.ts:189-194): httpOnly: true, 24 hours - Frontend useLogin (api/auth/index.js:26-30): non-httpOnly (js-cookie), 30d/7d - Frontend useRefreshToken (api/auth/index.js:129-134): non-httpOnly, 1 day - Net result: Frontend always overwrites → token cookie ends up non-httpOnly

Fix (documentation): - Line 79: Change to "The token cookie is set as httpOnly by setTokenCookie, but the frontend's setCookie call overwrites it with a non-httpOnly cookie — so in practice the token is accessible to client JavaScript." - Line 253: Correct as-is (describes the function accurately). - Lines 1054, 1126: Correct about the practical outcome (non-httpOnly) but should note the cause is frontend overwriting, not backend configuration.

Fix (code — security): The frontend (api/auth/index.js lines 24-31) should NOT set the token cookie at all — the backend setTokenCookie already sets it as httpOnly. The frontend should read the token from the API response body for Redux state but leave the cookie to the backend. This would close the XSS token-theft vector. See also review-findings.md Category B for related cookie security issue.

4. Findings 51–64 reference files outside declared scope and inventory — RESOLVED (2026-02-13)¶

Category: Structural / Scope creep

Verified: The files role.service.ts, subscription.helpers.ts, permission.routes.ts, user.controller.ts, user.service.ts, permission.service.ts, and user_setting.service.ts are real files in the codebase but have no entries in the "Complete File Inventory" section.

Fix (recommended: option b): Move findings 51–64 into a clearly labeled "Out-of-Scope Observations" section. This preserves the existing inventory's focused scope while retaining the observations.

Alternatives: (a) Add inventory entries for all files referenced in findings 51–64, or © expand the declared scope in the header.

5. "Complete File Inventory" omits 12+ referenced files — RESOLVED (2026-02-13)¶

Category: Missing content

Verified: All 12+ files listed in the original finding exist in the codebase and are referenced later in the document without inventory entries.

Fix: Rename to "Core Auth File Inventory" or add the missing entries.

6. "Refresh token rotation" — rotation does not occur in code — RESOLVED (2026-02-13)¶

Category: Factual error (code-verified)

Source code verdict: auth.service.ts refreshToken function (lines 509-557):

// Generates ONLY a new access token:
const accessToken = jwtHelpers.generateToken(jwtPayload, ...);
return accessToken;  // ← no new refresh token generated

Verdict: The code performs "access token refresh," NOT "refresh token rotation." Doc lines 11, 286, 294, 843, 1301 all incorrectly use "rotation."

Fix — exact replacements:

7. /verify-email is POST with body, not GET with query param — doc is wrong in 3 locations — RESOLVED (2026-02-13)¶

Category: Factual error (code-verified)

Original finding: Path format inconsistency. Upgraded during verification: the HTTP method and parameter passing are both wrong.

Source code (verified): - auth.route.ts:86-90: router.post("/verify-email", validateRequest(AuthValidation.verifyEmailValidation), AuthController.verifyEmailForRegistration) — POST - auth.validation.ts verifyEmailValidation: validates body: { token: z.string() } — token is in request body - Frontend authService.js:27-28: api.post("/auth/verify-email", { token }) — POST with body - Frontend page (auth)/verify-email/[token]/page.js: extracts token from URL path segment and sends it in a POST body — NOT a query parameter

Doc claims (all wrong): - Line 943 (Data Flow): GET /auth/verify-email?token=xxx - Line 1033 (Data Entry Points): GET /auth/verify-email?token=xxx - Line 1111 (APIs Exposed): GET /api/v1/auth/verify-email with "Query param token=JWT"

Fix — exact replacements:

8. 64 known issues listed with no severity ranking — RESOLVED (2026-02-13)¶

Category: Structural

No code verification needed — this is a document organization issue.

Fix: Add severity tags (Critical/High/Medium/Low) or group by severity.

9. VERIFY_EMAIL_REG is dead code, NOT an alias — doc line 1151 is wrong — RESOLVED (2026-02-13)¶

Category: Factual error (code-verified)

Source code verdict: eventTypes.ts:

VERIFY_EMAIL_REG: "verify_email_reg",        // value: "verify_email_reg"
SEND_REGISTRATION_EMAIL: "send_registration_email",  // value: "send_registration_email"

Grep confirms: VERIFY_EMAIL_REG is defined in eventTypes.ts and never emitted or listened to anywhere else in the codebase. SEND_REGISTRATION_EMAIL has both an emitter (auth.service.ts:123) and a handler (registerEmailHandler.ts:5).

Verdict: - Line 1151 ("alias for SEND_REGISTRATION_EMAIL") is factually wrong - Line 1411 ("dead event type") is correct

Fix: Change line 1151 from "alias for SEND_REGISTRATION_EMAIL" to "Dead event type — defined but never emitted or handled. See Known Issue #24."

10. createAccount vs registerAccount — different functions, relationship undocumented — RESOLVED (2026-02-13)¶

Category: Missing content (code-verified)

Source code verdict: These are two distinct functions in auth.service.ts:

• createAccount (lines 46-78): Internal DB function. Creates user directly in the database. Handles soft-deleted user restoration. Takes raw password (does NOT hash — this is the bug in Known Issue #31). Called internally by verifyEmailForRegistration (line 420).

• registerAccount (lines 94-130): Public-facing registration. Checks invite-only mode, hashes password, generates JWT verification token with embedded hash, emits email event. Does NOT create user yet — deferred to email verification.

• Controller mapping: The controller's createAccount method (line 10) calls AuthService.registerAccount() — confusingly, NOT AuthService.createAccount().

Fix: Add to Key Implementation Details:

createAccount vs registerAccount: These are two distinct functions in auth.service.ts. registerAccount is the public-facing flow — it checks invite-only mode, hashes the password, generates a JWT verification token, and emits an email event without creating a user. createAccount is the internal DB function — it inserts or restores the user record after email verification. The controller handler named createAccount calls AuthService.registerAccount() (not AuthService.createAccount()), which is a source of confusion.

11. Same issues repeated across 5 sections — RESOLVED (2026-02-13)¶

Category: Structural / Redundancy

No code verification needed — document structure issue.

Fix: Consolidate into a single canonical Known Issues section with cross-references.

12. rememberMe cookie lifecycle never fully documented — RESOLVED (2026-02-13)¶

Category: Missing content (code-verified)

Source code verdict — actual lifecycle:

1. Set by backend (auth.contoroller.ts:53-57): When login returns 2FA (no token yet) AND rememberMe was in request body → sets rememberMe cookie with value true, httpOnly: false, maxAge: 5 minutes
2. Read by backend (auth.contoroller.ts:122): verifyOtp reads req.cookies.rememberMe
3. Used by backend (auth.contoroller.ts:126-132): If rememberMe === "true" → clears the rememberMe cookie, calls setTokenCookie (20-day httpOnly cookie)
4. Cleared by backend (auth.contoroller.ts:70): On non-rememberMe login, res.clearCookie("rememberMe")

Security concern (plausible, not fully traced): The rememberMe cookie is non-httpOnly (httpOnly: false at line 55). It controls whether a 20-day persistent token cookie gets set. An XSS attacker could inject document.cookie = "rememberMe=true" before the victim completes OTP verification. When verifyOtp reads req.cookies.rememberMe (line 122), it would find "true" and call setTokenCookie — creating a 20-day httpOnly token cookie, making the session persist across browser restarts even though the user didn't choose "remember me."

Preconditions: (1) XSS vulnerability exists, (2) attacker can execute JS before the OTP POST, (3) cookie path/domain allows the injected cookie to reach the backend. Condition (3) is met by default since res.cookie("rememberMe", ...) at line 54 sets no explicit path (defaults to /). The 5-minute maxAge window is tight but exploitable if the XSS runs during the login flow.

Fix: Document this lifecycle in the Data Flow or Shared State section.

13. Suggested test for feature that does not exist — RESOLVED (2026-02-13)¶

Category: Factual error (code-verified)

Verified: Line 843 says "Integration test: Refresh token rotation." Code confirms no rotation occurs (see finding #6).

Fix: Change to "Integration test: Refresh token → new access token generation."

14. At least 6 different token cookie durations across code paths — RESOLVED (2026-02-13)¶

Category: Factual error (code-verified)

Source-code-verified cookie duration table:

JWT expiry in response body (NOT cookies):

Doc claims vs reality: - Line 79: "extends token cookie to 15 days" → Wrong: setTokenCookie always creates 20-day cookie. The 15-day value is the JWT expiry in the response JSON, not the cookie. - Line 253: "20 days" → Correct for setTokenCookie - Line 270: "7-day" → Correct for Google OAuth cookie - Line 903: "7 days default, 30 days remember me" → Correct for frontend useLogin - Line 975: "non-httpOnly, 7d/15d" → Wrong: The backend cookie is httpOnly, and the durations are 20d (setTokenCookie) or 7d/30d (frontend). 15d is only the JSON response token's JWT expiry with rememberMe.

Fix: Add the above table to the document. Correct lines 79 and 975.

15. Circular dependency analysis incomplete by document's own expanded scope — RESOLVED (2026-02-13)¶

Category: Structural

No code verification needed — logical scope issue.

Fix: Add caveat: "Analysis limited to files in the Complete File Inventory section."

16. Subscription entity inclusion justified but unexplained — RESOLVED (2026-02-13)¶

Category: Missing content (code-verified)

Verified: auth.service.ts line 167-174 loads subscription and subscription.package as relations in the login query. Subscription data is included in the login response (lines 272-276). Package is loaded via subscription.package but only its name and billingCycle fields are extracted. Blog and Purchase are NOT loaded in auth flows.

Verdict: Subscription's inclusion is justified. Package is loaded indirectly via Subscription but not inventoried (acceptable since it's a sub-relation). Blog and Purchase are correctly excluded.

Fix: Add a one-line note: "Included because subscription data is loaded and returned in the login response payload."

17. Config file: auth-relevant admin credentials undocumented — RESOLVED (2026-02-13)¶

Category: Missing content (code-verified)

Verified: config/index.ts is 95 lines (not "80+"). The doc missed these config sections: ssl (SSLCommerz), lemonSqueezy, aws (S3), resend, brevo, masterKey, admin (admin email/password credentials), imageStorage, cors.

The admin section (lines 68-71) contains ADMIN_EMAIL and ADMIN_PASSWORD env vars — directly auth-relevant for admin account seeding. The masterKey (line 64) is also auth-relevant.

Fix: - Primary: Add the missing auth-relevant config keys to the doc's inventory entry for config/index.ts: admin.email, admin.password, masterKey. - Minor: Update LOC from "80+" to 95.

18. New findings 63–64 worsen scope creep (finding #4) — RESOLVED (2026-02-13)¶

Category: Structural

Verified: Issue 64 references role.service.ts upsertRole which has no inventory entry.

Fix: Same as finding #4 — move to "Out-of-Scope Observations" section (recommended).

19. "Validated: 2026-02-13" header implies contradictions were resolved — RESOLVED (2026-02-13)¶

Category: Misleading

No code verification needed.

Fix: Change to "Validated: 2026-02-13 (structure and coverage — see deep-dive-auth-review-findings.md for open items)."

20. Data Entry Points missing 3 endpoints — RESOLVED (2026-02-13)¶

Category: Missing content (code-verified)

Verified in auth.route.ts: - Line 14: router.get("/profile", GoogleOAuthController.getProfile) — session-based profile retrieval - Line 15: router.post("/logout", GoogleOAuthController.logout) — session destroy + response - Line 17: router.get("/failure", GoogleOAuthController.authFailure) — redirect to frontend with error

All 3 exist in the route file but are absent from Data Entry Points (lines 1022-1036).

Fix: Add these endpoints or note the exclusion.

21. APIs Exposed missing 3 endpoints — RESOLVED (2026-02-13)¶

Category: Missing content (code-verified)

Same 3 endpoints as finding #20. No request/response specs exist for /profile, /logout, /failure.

Fix: Add API specifications for all 3.

Finding #22 has been merged into finding #7 (same issue: verify-email is POST not GET).

Recommended Fix Priority¶

Batch 1 — Factual errors + security (fix immediately — these mislead contributors): - #7 (verify-email is POST not GET — 3 doc locations wrong) - #2 (lockout logic — line 227 says wrong thing) - #6 (rotation terminology — 5 locations, see replacement table) - #9 (VERIFY_EMAIL_REG is dead code not alias — line 1151 wrong) - #14 (cookie durations — lines 79, 975 wrong, see corrected tables) - #13 (test for nonexistent rotation — depends on #6 terminology fix) - #3 (httpOnly security issue — doc fix + code fix recommendation)

Batch 2 — Missing content (fill gaps): - #20 and #21 (3 missing endpoints — same section, fix together) - #10 (createAccount vs registerAccount relationship) - #12 (rememberMe lifecycle + security concern) - #17 (admin credentials config undocumented) - #16 (Subscription inclusion note) - #5 (inventory completeness — rename or add entries)

Batch 3 — Structural (improve usability): - #8 (severity ranking for 64 known issues) - #11 (redundancy across 5 sections — consolidate) - #4/#18 (scope creep — recommended: move to "Out-of-Scope Observations") - #15 (circular deps caveat)

Batch 4 — Misleading signals: - #19 (Validated header)

Dependencies: - #13 depends on #6 (both about "rotation" terminology) - #4 and #18 are the same fix (scope creep) - #20 and #21 are the same section (Data Entry Points + APIs Exposed for 3 endpoints) - #3's doc fix is independent but code fix should be tracked in review-findings.md Category B

Cross-reference: The auth.contoroller.ts filename typo (noted in the deep-dive doc at line 75) is tracked in review-findings.md Category B code issues. Not a finding in this document since it's a code change, not a doc change.

Conventions¶

• Marking resolved: Change the heading suffix from — OPEN to — RESOLVED (YYYY-MM-DD). Update the summary table counts.
• Revision history: Use git history for tracking changes to this document.
• Cross-references: The deep-dive doc's "Validated" header should reference this file for open items (see finding #19).