fix(security): rate limit register endpoint; remove RESIDENT_TOKEN cookie

.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": 6502,
+  "percent": 98.53,
+  "percentString": "98.53",
+  "totalCount": 6599
 }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "residents",
-  "version": "0.3.9",
+  "version": "0.3.14",
   "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/constants/keys.ts

@@ -1,2 +1 @@
 export const REFRESH_TOKEN = 'refreshToken'
-export const RESIDENT_TOKEN = 'residentToken'

src/constants/messages.ts

@@ -118,7 +118,6 @@ const MESSAGES = {
   PASSWORD_ERROR: 'Password Error.',
   PASSWORD_WAS_RESET: 'Password was reset.',
   PASSWORD_UPDATE_ERROR: 'Error updating password.',
-  REFRESH_TOKEN_COUNTERPART_REQUIRED: 'Refresh token counterpart is required.',
   REFRESH_TOKEN_REQUIRED: 'Refresh token is required.',
   RESET_PASSWORD_PROMPT: 'Reset your password.',
   ROLE_REQUIRED: 'Role must be provided.',
@@ -133,7 +132,6 @@ const MESSAGES = {
   TOKEN_REQUIRES_USER_ID: 'Token requires a UserID, none provided.',
   TOKEN_TYPE_REQUIRED: 'Token type is required, none provided.',
   TOKEN_TYPE_INVALID: 'Token type is invalid.',
-  TOKEN_USER_INVALID: 'Token user not valid.',
   EXPIRED_TOKENS_DELETED: 'Expired tokens deleted.',
   ERROR_DISCARDING_TOKEN: 'Error discarding token.',
   USER_VALIDATED: 'User validated.',

src/controllers/auth/googleCallback.ts

@@ -6,7 +6,7 @@ import { ResidentRequest, ResidentResponse } from '../../types'
 import { generateJwtFromUser } from '../../utils/generateJwt'
 import SERVICES from '../../services'
 import { TOKEN_TYPE } from '../../constants/database'
-import { REFRESH_TOKEN, RESIDENT_TOKEN } from '../../constants/keys'
+import { REFRESH_TOKEN } from '../../constants/keys'
 import { EXPIRATION_REFRESH_TOKEN_MS } from '../../config'
 
 const isSecureCookie = process.env.NODE_ENV === 'production'
@@ -48,14 +48,6 @@ export const googleCallback = async (req: ResidentRequest, res: Response<Residen
     maxAge: EXPIRATION_REFRESH_TOKEN_MS,
   })
 
-  // This is probably redundant as the id is in the jwt anyway... Revisit
-  res.cookie(RESIDENT_TOKEN, user.id, {
-    secure: isSecureCookie,
-    sameSite: 'strict',
-    httpOnly: true,
-    maxAge: EXPIRATION_REFRESH_TOKEN_MS,
-  })
-
   // This will likely only be used by browsers/apps,
   // so we send a redirect with the token rather than a JSON response.
   // the browser can then pull the JWT from the redirect URL params.

src/controllers/auth/login.ts

@@ -6,7 +6,7 @@ import { BadRequestError, ForbiddenError, LoginError } from '../../errors'
 import SERVICES from '../../services'
 import { validateHash } from '../../utils/crypt'
 import { generateJwtFromUser } from '../../utils/generateJwt'
-import { REFRESH_TOKEN, RESIDENT_TOKEN } from '../../constants/keys'
+import { REFRESH_TOKEN } from '../../constants/keys'
 import { EXPIRATION_REFRESH_TOKEN_MS } from '../../config'
 import { handleSuccessResponse } from '../../middleware/util/successHandler'
 import MESSAGES from '../../constants/messages'
@@ -72,14 +72,6 @@ export const login = async (req: ResidentRequest, res: Response<ResidentResponse
     maxAge: EXPIRATION_REFRESH_TOKEN_MS,
   })
 
-  // This is probably redundant as the id is in the jwt anyway... Revisit
-  res.cookie(RESIDENT_TOKEN, user.id, {
-    httpOnly: true,
-    secure: process.env.NODE_ENV === 'production',
-    sameSite: 'strict',
-    maxAge: EXPIRATION_REFRESH_TOKEN_MS,
-  })
-
   const accessToken = generateJwtFromUser(user)
 
   handleSuccessResponse({ res, token: accessToken })

src/controllers/auth/logout.test.ts

@@ -3,12 +3,13 @@ import { HTTP_SUCCESS } from '../../constants/http'
 import { SafeUser } from '../../db/types'
 import { logout } from './logout'
 import { REQUEST_USER } from '../../types/requestSymbols'
-import { RESIDENT_TOKEN, REFRESH_TOKEN } from '../../constants/keys'
+import { REFRESH_TOKEN } from '../../constants/keys'
 import MESSAGES from '../../constants/messages'
 import { ResidentRequest } from '../../types'
 
 jest.mock('../../services/index', () => ({
   deleteRefreshTokensByUserId: jest.fn().mockImplementation(() => '123'),
+  getToken: jest.fn().mockImplementation(() => ({ userId: '123' })),
 }))
 
 describe('Controller: Logout', () => {
@@ -18,7 +19,6 @@ describe('Controller: Logout', () => {
   beforeEach(() => {
     mockRequest = {
       cookies: {
-        [RESIDENT_TOKEN]: '123',
         [REFRESH_TOKEN]: '123',
       },
     }
@@ -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 })
@@ -46,11 +54,5 @@ describe('Controller: Logout', () => {
       sameSite: 'strict',
       secure: false,
     })
-    expect(mockResponse.cookie).toHaveBeenCalledWith('residentToken', '', {
-      httpOnly: true,
-      expires: expect.any(Date),
-      sameSite: 'strict',
-      secure: false,
-    })
   })
 })

src/controllers/auth/logout.ts

@@ -1,7 +1,7 @@
 import SERVICES from '../../services'
 import { Response } from 'express'
 import { BadRequestError } from '../../errors'
-import { REFRESH_TOKEN, RESIDENT_TOKEN } from '../../constants/keys'
+import { REFRESH_TOKEN } from '../../constants/keys'
 import { handleSuccessResponse } from '../../middleware/util/successHandler'
 import MESSAGES from '../../constants/messages'
 import { ResidentRequest, ResidentResponse } from '../../types'
@@ -10,29 +10,26 @@ import { ResidentRequest, ResidentResponse } from '../../types'
  * logout
  */
 export const logout = async (req: ResidentRequest, res: Response<ResidentResponse>): Promise<void> => {
-  // Clear the cookies regardless of whether we have any existing ones
-
+  // Clear the cookie regardless of whether we have an existing one
   res.cookie(REFRESH_TOKEN, '', {
     httpOnly: true,
     secure: process.env.NODE_ENV === 'production',
     sameSite: 'strict',
     expires: new Date(0),
   })
 
-  res.cookie(RESIDENT_TOKEN, '', {
-    httpOnly: true,
-    secure: process.env.NODE_ENV === 'production',
-    sameSite: 'strict',
-    expires: new Date(0),
-  })
-
-  // Delete the refresh tokens from the database
-  const userId = req.cookies?.[RESIDENT_TOKEN]
-  if (!userId) {
-    throw new BadRequestError(MESSAGES.MISSING_USER_ID)
+  // Look up userId via the refresh token — avoids trusting a separate cookie
+  const refreshTokenId = req.cookies?.[REFRESH_TOKEN]
+  if (!refreshTokenId) {
+    throw new BadRequestError(MESSAGES.REFRESH_TOKEN_REQUIRED)
   }
 
-  await SERVICES.deleteRefreshTokensByUserId({ userId })
+  // 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) {
+    await SERVICES.deleteRefreshTokensByUserId({ userId: token.userId })
+  }
 
   handleSuccessResponse({ res, message: MESSAGES.LOGOUT_SUCCESS })
 }

src/controllers/auth/magicLoginWithToken.test.ts

@@ -67,15 +67,5 @@ describe('Controller: MagicLoginWithToken', () => {
         secure: false,
       })
     )
-    expect(mockResponse.cookie).toHaveBeenCalledWith(
-      'residentToken',
-      expect.any(String),
-      expect.objectContaining({
-        httpOnly: true,
-        maxAge: 60000,
-        sameSite: 'strict',
-        secure: false,
-      })
-    )
   })
 })

src/controllers/auth/magicLoginWithToken.ts

@@ -1,7 +1,7 @@
 import { Response } from 'express'
 import { EXPIRATION_REFRESH_TOKEN_MS } from '../../config'
 import { TOKEN_TYPE } from '../../constants/database'
-import { REFRESH_TOKEN, RESIDENT_TOKEN } from '../../constants/keys'
+import { REFRESH_TOKEN } from '../../constants/keys'
 import MESSAGES from '../../constants/messages'
 import { TIMESPAN } from '../../constants/time'
 import { ForbiddenError } from '../../errors'
@@ -47,12 +47,5 @@ export const magicLoginWithToken = async (req: ResidentRequest, res: Response<Re
     maxAge: EXPIRATION_REFRESH_TOKEN_MS,
   })
 
-  res.cookie(RESIDENT_TOKEN, user.id, {
-    httpOnly: true,
-    secure: process.env.NODE_ENV === 'production',
-    sameSite: 'strict',
-    maxAge: EXPIRATION_REFRESH_TOKEN_MS,
-  })
-
   handleSuccessResponse({ res, token: jwt })
 }

src/controllers/auth/refreshToken.test.ts

@@ -7,7 +7,6 @@ import { generateJwtFromUser } from '../../utils/generateJwt'
 import { User } from '../../db/types'
 import { logger } from '../../utils/logger'
 import jwt from 'jsonwebtoken'
-import { RESIDENT_TOKEN } from '../../constants/keys'
 import MESSAGES from '../../constants/messages'
 import { ResidentRequest } from '../../types'
 
@@ -30,10 +29,6 @@ jest.mock('../../services/index', () => ({
       userId: mockDefaultUser.id,
     }))
     .mockImplementationOnce(() => undefined)
-    .mockImplementationOnce(() => ({
-      id: 'tok1',
-      userId: '456',
-    }))
     .mockImplementationOnce(() => ({
       id: 'tok3',
       used: true,
@@ -63,7 +58,7 @@ describe('Controller: Refresh token: Happy path', () => {
     mockRequest = {
       body: {},
       headers: { authorization: `Bearer ${token}` },
-      cookies: { refreshToken: 'REFRESHME', [RESIDENT_TOKEN]: mockDefaultUser.id },
+      cookies: { refreshToken: 'REFRESHME' },
     }
     mockResponse = {
       status: jest.fn().mockReturnThis(),
@@ -77,19 +72,13 @@ describe('Controller: Refresh token: Happy path', () => {
     expect(logger.error).not.toHaveBeenCalled()
     expect(mockResponse.json).toHaveBeenCalledWith({ token: 'testAccessToken' })
     expect(mockResponse.status).toHaveBeenCalledWith(HTTP_SUCCESS.OK)
-    expect(mockResponse.cookie).toHaveBeenCalledTimes(2)
+    expect(mockResponse.cookie).toHaveBeenCalledTimes(1)
     expect(mockResponse.cookie).toHaveBeenNthCalledWith(1, 'refreshToken', 'tok1', {
       httpOnly: true,
       maxAge: 60000,
       sameSite: 'strict',
       secure: false,
     })
-    expect(mockResponse.cookie).toHaveBeenNthCalledWith(2, 'residentToken', mockDefaultUser.id, {
-      httpOnly: true,
-      maxAge: 60000,
-      sameSite: 'strict',
-      secure: false,
-    })
   })
 })
 
@@ -114,7 +103,7 @@ describe('Should return errors if', () => {
       headers: {
         authorization: `Bearer ${token}`,
       },
-      cookies: { refreshToken: 'REFRESHME', [RESIDENT_TOKEN]: mockDefaultUser.id },
+      cookies: { refreshToken: 'REFRESHME' },
     }
     mockResponse = {
       status: jest.fn().mockReturnThis(),
@@ -130,22 +119,11 @@ describe('Should return errors if', () => {
     )
   })
 
-  it('theres no UserId in the cookies', async () => {
-    delete mockRequest.cookies?.[RESIDENT_TOKEN]
-    await expect(refreshToken(mockRequest as ResidentRequest, mockResponse as Response)).rejects.toThrow(
-      MESSAGES.REFRESH_TOKEN_COUNTERPART_REQUIRED
-    )
-  })
   it('the token isnt found in the database', async () => {
     await expect(refreshToken(mockRequest as ResidentRequest, mockResponse as Response)).rejects.toThrow(
       MESSAGES.TOKEN_NOT_FOUND
     )
   })
-  it('the token user does not match the JWT user', async () => {
-    await expect(refreshToken(mockRequest as ResidentRequest, mockResponse as Response)).rejects.toThrow(
-      MESSAGES.TOKEN_USER_INVALID
-    )
-  })
   it('the token has a USED flag set', async () => {
     await expect(refreshToken(mockRequest as ResidentRequest, mockResponse as Response)).rejects.toThrow(
       MESSAGES.TOKEN_USED