Skip to content

Commit

Permalink
fix(api): improve user model usage (#3076)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
bodinsamuel authored Dec 2, 2024
1 parent 3fe2c41 commit 5fdae65
Show file tree
Hide file tree
Showing 11 changed files with 19 additions and 15 deletions.
6 changes: 1 addition & 5 deletions packages/server/lib/controllers/v1/meta/getMeta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ export const getMeta = asyncWrapper<GetMeta>(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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
]
}
Expand Down
2 changes: 1 addition & 1 deletion packages/server/lib/controllers/v1/user/getUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ export const getUser = asyncWrapper<GetUser, never>((req, res) => {
}

res.status(200).send({
data: userToAPI(req.user!)
data: userToAPI(res.locals['user'])
});
});
4 changes: 2 additions & 2 deletions packages/server/lib/controllers/v1/user/patchUser.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -26,7 +26,7 @@ export const patchUser = asyncWrapper<PatchUser, never>(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 });
Expand Down
4 changes: 4 additions & 0 deletions packages/server/lib/express.d.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
5 changes: 3 additions & 2 deletions packages/server/lib/formatters/user.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import type { ApiUser, DBUser } from '@nangohq/types';

export function userToAPI(user: Pick<DBUser, 'id' | 'account_id' | 'email' | 'name'>): ApiUser {
export function userToAPI(user: Pick<DBUser, 'id' | 'account_id' | 'email' | 'name' | 'uuid'>): ApiUser {
return {
id: user.id,
accountId: user.account_id,
email: user.email,
name: user.name
name: user.name,
uuid: user.uuid
};
}
2 changes: 1 addition & 1 deletion packages/server/lib/middleware/access.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ async function fillLocalsFromSession(req: Request, res: Response<any, RequestLoc
return;
}

res.locals['user'] = req.user!;
res.locals['user'] = user;

if (ignoreEnvPaths.includes(req.route.path)) {
next();
Expand Down
2 changes: 1 addition & 1 deletion packages/server/lib/utils/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type { DBEnvironment, DBTeam, ConnectSession, DBUser, EndUser } from '@na

export interface RequestLocals {
authType?: 'secretKey' | 'publicKey' | 'basic' | 'adminKey' | 'none' | 'session' | 'connectSession';
user?: Pick<DBUser, 'id' | 'email' | 'name'>;
user?: DBUser;
account?: DBTeam;
environment?: DBEnvironment;
connectSession?: ConnectSession;
Expand Down
1 change: 1 addition & 0 deletions packages/server/lib/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>): Promise<Result<DBUser, NangoError>> {
const sessionUser = req.user;
if (!sessionUser) {
Expand Down
1 change: 1 addition & 0 deletions packages/types/lib/user/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ export interface ApiUser {
accountId: number;
email: string;
name: string;
uuid: string;
}
4 changes: 2 additions & 2 deletions tests/setupFiles.ts
Original file line number Diff line number Diff line change
@@ -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}$/;

Expand Down

0 comments on commit 5fdae65

Please sign in to comment.