fix(review): graceful logout; correct error messages; delete-before-validate comment

logout: degrade gracefully when refresh token not in DB (expired/purged) —
cookie is already cleared so user is effectively logged out regardless.
Fix misleading MISSING_USER_ID → REFRESH_TOKEN_REQUIRED error message.
Add test for token-not-found path.

refreshToken: add explanatory comment on the intentional delete-before-validate
ordering (token theft detection: nuke all sessions on any suspicious reuse).
Use guard clause to remove redundant if(token)/if(!token) double-check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: conorluddy

.github/type-coverage.json

@@ -2,8 +2,8 @@
   "succeeded": true,
   "atLeast": 90,
   "atLeastFailed": false,
-  "correctCount": 6556,
-  "percent": 98.55,
-  "percentString": "98.55",
-  "totalCount": 6652
+  "correctCount": 6488,
+  "percent": 98.54,
+  "percentString": "98.54",
+  "totalCount": 6584
 }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "residents",
-  "version": "0.3.8",
+  "version": "0.3.9",
   "author": "Conor Luddy <conorluddy@gmail.com>",
   "license": "MIT",
   "description": "Residents is a Node.js Express back-end foundation designed for bootstrapping new projects quickly and efficiently. Its main goal is to set up a robust infrastructure for user management, because users are the core of any application. These users are your Residents. It leverages a robust stack including Postgres, Drizzle ORM, JWT, PassportJS, Docker and Swagger to streamline development and deployment processes.",

src/controllers/auth/logout.test.ts

@@ -29,13 +29,21 @@ describe('Controller: Logout', () => {
     }
   })
 
-  it('Throws an error if missing the user data', async () => {
+  it('Throws an error if the refresh token cookie is absent', async () => {
     mockRequest.cookies = undefined
     await expect(logout(mockRequest as ResidentRequest, mockResponse as Response)).rejects.toThrow(
-      MESSAGES.MISSING_USER_ID
+      MESSAGES.REFRESH_TOKEN_REQUIRED
     )
   })
 
+  it('Still succeeds if the refresh token is not found in the DB (graceful degradation)', async () => {
+    const SERVICES = jest.requireMock('../../services/index')
+    SERVICES.getToken.mockImplementationOnce(() => null)
+    await logout(mockRequest as ResidentRequest, mockResponse as Response)
+    expect(mockResponse.json).toHaveBeenCalledWith({ message: MESSAGES.LOGOUT_SUCCESS })
+    expect(mockResponse.status).toHaveBeenCalledWith(HTTP_SUCCESS.OK)
+  })
+
   it('logs out a user by deleting any of their refresh tokens', async () => {
     await logout(mockRequest as ResidentRequest, mockResponse as Response)
     expect(mockResponse.json).toHaveBeenCalledWith({ message: MESSAGES.LOGOUT_SUCCESS })

src/controllers/auth/logout.ts

@@ -21,15 +21,15 @@ export const logout = async (req: ResidentRequest, res: Response<ResidentRespons
   // Look up userId via the refresh token — avoids trusting a separate cookie
   const refreshTokenId = req.cookies?.[REFRESH_TOKEN]
   if (!refreshTokenId) {
-    throw new BadRequestError(MESSAGES.MISSING_USER_ID)
+    throw new BadRequestError(MESSAGES.REFRESH_TOKEN_REQUIRED)
   }
 
+  // Best-effort DB cleanup: token may already be expired/deleted (e.g. admin purge).
+  // Cookie is already cleared above, so the user is effectively logged out regardless.
   const token = await SERVICES.getToken({ tokenId: refreshTokenId })
-  if (!token?.userId) {
-    throw new BadRequestError(MESSAGES.MISSING_USER_ID)
+  if (token?.userId) {
+    await SERVICES.deleteRefreshTokensByUserId({ userId: token.userId })
   }
 
-  await SERVICES.deleteRefreshTokensByUserId({ userId: token.userId })
-
   handleSuccessResponse({ res, message: MESSAGES.LOGOUT_SUCCESS })
 }

src/controllers/auth/refreshToken.ts

@@ -25,14 +25,15 @@ export const refreshToken = async (req: ResidentRequest, res: Response<ResidentR
   // Get the refresh token from the DB — userId is authoritative from here, not a cookie
   const token = await SERVICES.getToken({ tokenId: refreshTokenId })
 
-  // Regardless of validity, clear tokens once we've fetched the record
-  if (token) {
-    await SERVICES.deleteRefreshTokensByUserId({ userId: token.userId })
-  }
-
   if (!token) {
     throw new ForbiddenError(MESSAGES.TOKEN_NOT_FOUND)
   }
+
+  // Intentional: delete ALL sessions for this user before validating the token further.
+  // If the token is reused or tampered with, nuking all sessions is the correct response
+  // (indicates likely token theft). Trade-off: a replayed expired token will log out all
+  // active sessions for that user.
+  await SERVICES.deleteRefreshTokensByUserId({ userId: token.userId })
   if (token.used) {
     throw new ForbiddenError(MESSAGES.TOKEN_USED)
   }