From 5fdae657b4eef2f7a53193e03cf99e8a0dc5d79a Mon Sep 17 00:00:00 2001 From: Samuel Bodin <1637651+bodinsamuel@users.noreply.github.com> Date: Mon, 2 Dec 2024 20:36:33 +0100 Subject: [PATCH] fix(api): improve user model usage (#3076) ## Context Extracted from #3072 - Clarify our use of user's session `req.user` is painful because session can be out of date, and it's hard to add/remove fields from session. It's easier to always use `res.locals[user] that's always fresh. - Pass UUID, will be used later - Fix regex for integration tests --- packages/server/lib/controllers/v1/meta/getMeta.ts | 6 +----- .../lib/controllers/v1/team/getTeam.integration.test.ts | 3 ++- packages/server/lib/controllers/v1/user/getUser.ts | 2 +- packages/server/lib/controllers/v1/user/patchUser.ts | 4 ++-- packages/server/lib/express.d.ts | 4 ++++ packages/server/lib/formatters/user.ts | 5 +++-- packages/server/lib/middleware/access.middleware.ts | 2 +- packages/server/lib/utils/express.ts | 2 +- packages/server/lib/utils/utils.ts | 1 + packages/types/lib/user/api.ts | 1 + tests/setupFiles.ts | 4 ++-- 11 files changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/server/lib/controllers/v1/meta/getMeta.ts b/packages/server/lib/controllers/v1/meta/getMeta.ts index 8ce66b94d9..190cd3563a 100644 --- a/packages/server/lib/controllers/v1/meta/getMeta.ts +++ b/packages/server/lib/controllers/v1/meta/getMeta.ts @@ -10,11 +10,7 @@ export const getMeta = asyncWrapper(async (req, res) => { return; } - const sessionUser = req.user; - if (!sessionUser) { - res.status(400).send({ error: { code: 'user_not_found' } }); - return; - } + const sessionUser = res.locals['user']; const environments = await environmentService.getEnvironmentsByAccountId(sessionUser.account_id); const onboarding = await getOnboarding(sessionUser.id); diff --git a/packages/server/lib/controllers/v1/team/getTeam.integration.test.ts b/packages/server/lib/controllers/v1/team/getTeam.integration.test.ts index ff6aaf1711..3323b3adb4 100644 --- a/packages/server/lib/controllers/v1/team/getTeam.integration.test.ts +++ b/packages/server/lib/controllers/v1/team/getTeam.integration.test.ts @@ -57,7 +57,8 @@ describe(`GET ${route}`, () => { accountId: account.id, email: user.email, id: user.id, - name: user.name + name: user.name, + uuid: user.uuid } ] } diff --git a/packages/server/lib/controllers/v1/user/getUser.ts b/packages/server/lib/controllers/v1/user/getUser.ts index 440680ca70..f4696f4f6b 100644 --- a/packages/server/lib/controllers/v1/user/getUser.ts +++ b/packages/server/lib/controllers/v1/user/getUser.ts @@ -11,6 +11,6 @@ export const getUser = asyncWrapper((req, res) => { } res.status(200).send({ - data: userToAPI(req.user!) + data: userToAPI(res.locals['user']) }); }); diff --git a/packages/server/lib/controllers/v1/user/patchUser.ts b/packages/server/lib/controllers/v1/user/patchUser.ts index e3a3b438ad..9b8f289589 100644 --- a/packages/server/lib/controllers/v1/user/patchUser.ts +++ b/packages/server/lib/controllers/v1/user/patchUser.ts @@ -1,6 +1,6 @@ import { asyncWrapper } from '../../../utils/asyncWrapper.js'; import { requireEmptyQuery, zodErrorToHTTP } from '@nangohq/utils'; -import type { PatchUser } from '@nangohq/types'; +import type { DBUser, PatchUser } from '@nangohq/types'; import { userService } from '@nangohq/shared'; import { z } from 'zod'; import { userToAPI } from '../../../formatters/user.js'; @@ -26,7 +26,7 @@ export const patchUser = asyncWrapper(async (req, res) => { return; } - const user = req.user!; + const user = res.locals['user'] as DBUser; // type is slightly wrong because we are not in an endpoint with an ?env= const body: PatchUser['Body'] = val.data; const updated = await userService.update({ id: user.id, ...body }); diff --git a/packages/server/lib/express.d.ts b/packages/server/lib/express.d.ts index 5edfa120eb..a6862fae58 100644 --- a/packages/server/lib/express.d.ts +++ b/packages/server/lib/express.d.ts @@ -1,5 +1,9 @@ declare global { namespace Express { + /** + * You should avoid using this type (req.user) + * It's serialized in session, which means we can't easily add / remove fields + */ interface User { id: number; email: string; diff --git a/packages/server/lib/formatters/user.ts b/packages/server/lib/formatters/user.ts index 3a95d8b90c..2902679198 100644 --- a/packages/server/lib/formatters/user.ts +++ b/packages/server/lib/formatters/user.ts @@ -1,10 +1,11 @@ import type { ApiUser, DBUser } from '@nangohq/types'; -export function userToAPI(user: Pick): ApiUser { +export function userToAPI(user: Pick): ApiUser { return { id: user.id, accountId: user.account_id, email: user.email, - name: user.name + name: user.name, + uuid: user.uuid }; } diff --git a/packages/server/lib/middleware/access.middleware.ts b/packages/server/lib/middleware/access.middleware.ts index 90ee7e27b0..b6645ff08c 100644 --- a/packages/server/lib/middleware/access.middleware.ts +++ b/packages/server/lib/middleware/access.middleware.ts @@ -405,7 +405,7 @@ async function fillLocalsFromSession(req: Request, res: Response; + user?: DBUser; account?: DBTeam; environment?: DBEnvironment; connectSession?: ConnectSession; diff --git a/packages/server/lib/utils/utils.ts b/packages/server/lib/utils/utils.ts index 47eb4fc639..499c55d365 100644 --- a/packages/server/lib/utils/utils.ts +++ b/packages/server/lib/utils/utils.ts @@ -10,6 +10,7 @@ import { OrchestratorClient } from '@nangohq/nango-orchestrator'; const logger = getLogger('Server.Utils'); +/** @deprecated TODO delete this */ export async function getUserFromSession(req: Request): Promise> { const sessionUser = req.user; if (!sessionUser) { diff --git a/packages/types/lib/user/api.ts b/packages/types/lib/user/api.ts index 5e59f5e985..57a59d5f80 100644 --- a/packages/types/lib/user/api.ts +++ b/packages/types/lib/user/api.ts @@ -22,4 +22,5 @@ export interface ApiUser { accountId: number; email: string; name: string; + uuid: string; } diff --git a/tests/setupFiles.ts b/tests/setupFiles.ts index f0ffdac5e8..2110e044af 100644 --- a/tests/setupFiles.ts +++ b/tests/setupFiles.ts @@ -1,7 +1,7 @@ import { expect } from 'vitest'; -const dateRegex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{1,6}Z$/; -const dateRegexWithTZ = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{1,6}\+\d{2}:\d{2}$/; +const dateRegex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d{1,6})?Z$/; +const dateRegexWithTZ = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d{1,6})?\+\d{2}:\d{2}$/; const uuidRegex = /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i; const sha256Regex = /^[a-f0-9]{64}$/;