fix(user): forbid user update for other users

apps/backend-e2e/src/backend/http/v1/users.http.spec.ts

@@ -13,9 +13,11 @@ describe(`Backend HTTP ${USERS_ENDPOINT_PREFIX}`, () => {
 
 	const dbHelper = DbE2eHelper.getHelper("base");
 	const db = JSON.parse(JSON.stringify(dbHelper.db)) as Jsonify<typeof dbHelper.db>;
+	const { users } = db;
+	const [user] = users;
 
 	beforeEach(async () => {
-		const [{ email, password }] = db.users;
+		const { email, password } = user;
 
 		await dbHelper.refresh();
 		await client.setAuth(email, password);
@@ -191,10 +193,9 @@ describe(`Backend HTTP ${USERS_ENDPOINT_PREFIX}`, () => {
 				pagination: { total: beforeTotal }
 			} = await client.findMany();
 
-			const [, user] = db.users;
-			const toUpdate: UserUpdateDto = { email: `${user.email}.mail` };
+			const toUpdate: UserUpdateDto = { firstname: `${user.firstname || "abc"}.123` };
 			const updated = await client.update(user._id, toUpdate);
-			expect(updated.email).toBe(toUpdate.email);
+			expect(updated.firstname).toBe(toUpdate.firstname);
 			expect(updated._updated_at > user._updated_at).toBeTrue();
 
 			const {
@@ -208,48 +209,27 @@ describe(`Backend HTTP ${USERS_ENDPOINT_PREFIX}`, () => {
 			expect(updated).toStrictEqual(found);
 		});
 
-		it("should not update the date if there's no change", async () => {
-			const [, user] = db.users;
-			const toUpdate: UserUpdateDto = { email: user.email };
+		it("should not be able to update a user's email", async () => {
+			const toUpdate = { email: `abc.${user.email}` } satisfies Pick<UserDto, "email">;
 			const updated = await client.update(user._id, toUpdate);
-			expect(updated).toStrictEqual(omit(user, ["password"]));
+
+			expect(updated.email).toBe(user.email);
+			expect(updated.email).not.toBe(toUpdate.email);
 		});
 
-		it("should fail when a uniqueness constraint is not respected", async () => {
-			const [user1, user2] = db.users;
+		it("should not be able to update another user (404)", async () => {
+			const { _id } = users.find(({ _id }) => _id !== user._id)!;
 
-			const toUpdate: UserUpdateDto = { email: user1.email };
 			const response = await client
-				.updateResponse(user2._id, toUpdate)
+				.updateResponse(_id, { firstname: "abc123" } satisfies UserUpdateDto)
 				.catch(({ response }: AxiosError) => response!);
-			expect(response.status).toBe(HttpStatusCode.Conflict);
+			expect(response.status).toBe(404);
 		});
 
-		// Use theses to test when the values are valid or not.
-		// Use another to determine the correct error(s) returned.
-		it("should fail with a BAD_REQUEST status code when the payload is invalid", async () => {
-			const [user] = db.users;
-			for (const toUpdate of [
-				{ email: "" },
-				{ email: "not an email" }
-			] satisfies UserUpdateDto[]) {
-				const response = await client
-					.updateResponse(user._id, toUpdate)
-					.catch(({ response }: AxiosError) => response!);
-				expect(response.status).toBe(HttpStatusCode.BadRequest);
-			}
-		});
-
-		it("should fail with a BAD_REQUEST status code when the payload is incorrectly typed", async () => {
-			const [user] = db.users;
-			for (const toUpdate of [
-				{ email: 123 as unknown as string }
-			] satisfies UserUpdateDto[]) {
-				const response = await client
-					.updateResponse(user._id, toUpdate)
-					.catch(({ response }: AxiosError) => response!);
-				expect(response.status).toBe(HttpStatusCode.BadRequest);
-			}
+		it("should not update the date if there's no change", async () => {
+			const toUpdate: UserUpdateDto = { firstname: user.firstname };
+			const updated = await client.update(user._id, toUpdate);
+			expect(updated).toStrictEqual(omit(user, ["password"]));
 		});
 	});
 
@@ -259,10 +239,11 @@ describe(`Backend HTTP ${USERS_ENDPOINT_PREFIX}`, () => {
 				pagination: { total: beforeTotal }
 			} = await client.findMany();
 
-			const [, user] = db.users;
 			const deleted = await client.delete(user._id);
 			expect(deleted).toStrictEqual(omit(user, ["password"]));
 
+			await client.setAuth(users[1].email, users[1].password);
+
 			const {
 				data: after,
 				pagination: { total: afterTotal }
@@ -271,6 +252,15 @@ describe(`Backend HTTP ${USERS_ENDPOINT_PREFIX}`, () => {
 			expect(after.some(({ _id }) => _id === deleted._id)).toBeFalse();
 		});
 
+		it("should not be able to delete another user (404)", async () => {
+			const { _id } = users.find(({ _id }) => _id !== user._id)!;
+
+			const response = await client
+				.deleteResponse(_id)
+				.catch(({ response }: AxiosError) => response!);
+			expect(response.status).toBe(404);
+		});
+
 		it("should not delete an unknown id", async () => {
 			const id = Math.max(...db.users.map(({ _id }) => _id)) + 1;
 			const response = await client

apps/backend/src/app/auth/auth.controller.ts

@@ -1,4 +1,4 @@
-import { Body, Controller, Get, Post, Req, Res, UseGuards } from "@nestjs/common";
+import { Body, Controller, Get, Post, Res, UseGuards } from "@nestjs/common";
 import { ApiCreatedResponse, ApiOkResponse, ApiTags } from "@nestjs/swagger";
 import { Response } from "express";
 import { CookieOptions } from "express-serve-static-core";
@@ -11,16 +11,9 @@ import { authOptions } from "~/lib/common/options";
 import { UseAuth } from "./auth.guard";
 import { AuthLocalGuard } from "./auth.local-guard";
 import { AuthService } from "./auth.service";
+import { AuthUser, AuthUserParam } from "./auth.user.param";
 import { UserEntity } from "../user/user.entity";
 
-declare global {
-	// eslint-disable-next-line @typescript-eslint/no-namespace -- TODO: set it as a custom global type AND still working for e2e tests
-	namespace Express {
-		// eslint-disable-next-line @typescript-eslint/no-empty-interface -- For declaration merging
-		interface User extends UserEntity {}
-	}
-}
-
 /**
  * The controller for authentication.
  */
@@ -47,9 +40,9 @@ export class AuthController implements AuthEndpoint {
 	@ApiOkResponse({ type: UserDto })
 	@Get(AuthEndpoints.PROFILE)
 	@UseAuth()
-	public getProfile(@Req() req?: Express.Request): Promise<UserDto> {
+	public getProfile(@AuthUserParam() user?: AuthUser): Promise<UserDto> {
 		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- From decorator and the Guard
-		return Promise.resolve(req!.user!);
+		return Promise.resolve(user!);
 	}
 
 	/** @inheritDoc */
@@ -58,11 +51,11 @@ export class AuthController implements AuthEndpoint {
 	@UseGuards(AuthLocalGuard)
 	public login(
 		@Body() body: AuthLoginDto,
-		@Req() req?: Express.Request,
+		@AuthUserParam() user?: AuthUser,
 		@Res({ passthrough: true }) res?: Response
 	) {
 		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- From guards and decorators (optional for the interface)
-		return this.loginOrRefresh(req!.user!, body, res!);
+		return this.loginOrRefresh(user!, body, res!);
 	}
 
 	/** @inheritDoc */
@@ -80,11 +73,11 @@ export class AuthController implements AuthEndpoint {
 	@UseAuth()
 	public refresh(
 		@Body() body: AuthRefreshDto,
-		@Req() req?: Express.Request,
+		@AuthUserParam() user?: AuthUser,
 		@Res({ passthrough: true }) res?: Response
 	) {
 		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- From guards and decorators (optional for the interface)
-		return this.loginOrRefresh(req!.user!, body, res!);
+		return this.loginOrRefresh(user!, body, res!);
 	}
 
 	private loginOrRefresh(user: UserEntity, body: AuthRefreshDto, res: Response) {

apps/backend/src/app/auth/auth.user.param.ts

@@ -0,0 +1,23 @@
+import { createParamDecorator, ExecutionContext } from "@nestjs/common";
+
+import { UserEntity } from "../user/user.entity";
+
+/**
+ * The authenticated user data on Request
+ */
+export type AuthUser = UserEntity;
+
+/**
+ * Inject the connected user from the request if exists
+ */
+export const AuthUserParam = createParamDecorator<never, ExecutionContext>(
+	(_, context) => context.switchToHttp().getRequest<Express.Request>().user
+);
+
+declare global {
+	// eslint-disable-next-line @typescript-eslint/no-namespace -- TODO: set it as a custom global type AND still working for e2e tests
+	namespace Express {
+		// eslint-disable-next-line @typescript-eslint/no-empty-interface -- For declaration merging
+		interface User extends AuthUser {}
+	}
+}

apps/backend/src/app/user/user.controller.ts

@@ -1,4 +1,14 @@
-import { Body, Controller, Delete, Get, Param, Patch, Post, Query } from "@nestjs/common";
+import {
+	Body,
+	Controller,
+	Delete,
+	Get,
+	NotFoundException,
+	Param,
+	Patch,
+	Post,
+	Query
+} from "@nestjs/common";
 import { ApiCreatedResponse, ApiOkResponse, ApiTags } from "@nestjs/swagger";
 import {
 	UserCreateDto,
@@ -12,6 +22,7 @@ import { UserEndpoint, USERS_ENDPOINT_PREFIX } from "~/lib/common/app/user/endpo
 import { UserEntity } from "./user.entity";
 import { UserService } from "./user.service";
 import { UseAuth } from "../auth/auth.guard";
+import { AuthUser, AuthUserParam } from "../auth/auth.user.param";
 
 /**
  * The main controller for [users]{@link UserDto}.
@@ -51,14 +62,24 @@ export class UserController implements UserEndpoint<UserEntity> {
 	/** @inheritDoc */
 	@ApiOkResponse({ type: UserDto })
 	@Patch("/:id")
-	public update(@Param("id") id: number, @Body() body: UserUpdateDto) {
+	public update(
+		@Param("id") id: number,
+		@Body() body: UserUpdateDto,
+		@AuthUserParam() user?: AuthUser
+	) {
+		if (user?._id !== id) {
+			return Promise.reject(new NotFoundException());
+		}
 		return this.service.update(id, body);
 	}
 
 	/** @inheritDoc */
 	@ApiOkResponse({ type: UserDto })
 	@Delete("/:id")
-	public delete(@Param("id") id: number) {
+	public delete(@Param("id") id: number, @AuthUserParam() user?: AuthUser) {
+		if (user?._id !== id) {
+			return Promise.reject(new NotFoundException());
+		}
 		return this.service.delete(id);
 	}
 }

apps/backend/src/app/user/user.service.spec.ts

@@ -1,9 +1,9 @@
 import { NotFoundError, UniqueConstraintViolationException } from "@mikro-orm/core";
 import { Test, TestingModule } from "@nestjs/testing";
-import { UserUpdateDto } from "~/lib/common/app/user/dtos";
 import { omit } from "~/lib/common/utils/object-fns";
 
 import { UserModule } from "./user.module";
+import { UserUpdateEntity } from "./user.service";
 import { UserService } from "./user.service";
 import { DbTestHelper } from "../../../test/db-test";
 import { OrmModule } from "../../orm/orm.module";
@@ -114,7 +114,7 @@ describe("UserService", () => {
 
 				// Update an entity and check its content
 				const [user] = dbTest.db.users;
-				const toUpdate: UserUpdateDto = { email: `${user.email}-${user.email}` };
+				const toUpdate: UserUpdateEntity = { email: `${user.email}-${user.email}` };
 				const updated = await service.update(user._id, toUpdate);
 				expect(updated.email).toBe(toUpdate.email);
 
@@ -130,15 +130,15 @@ describe("UserService", () => {
 
 			it("should not update the date if there's no change", async () => {
 				const [, user] = dbTest.db.users;
-				const toUpdate: UserUpdateDto = { email: user.email };
+				const toUpdate: UserUpdateEntity = { email: user.email };
 				const updated = await service.update(user._id, toUpdate);
 				expect(updated.toJSON()).toStrictEqual(omit(user, ["password"]));
 			});
 
 			it("should fail when a uniqueness constraint is not respected", async () => {
 				const [user1, user2] = dbTest.db.users;
 
-				const toUpdate: UserUpdateDto = { email: user1.email };
+				const toUpdate: UserUpdateEntity = { email: user1.email };
 				await expect(service.update(user2._id, toUpdate)).rejects.toThrow(
 					UniqueConstraintViolationException
 				);

apps/backend/src/app/user/user.service.ts

@@ -1,15 +1,20 @@
 import { Injectable } from "@nestjs/common";
-import { UserCreateDto, UserUpdateDto } from "~/lib/common/app/user/dtos";
+import { UserCreateDto, UserDto, UserUpdateDto } from "~/lib/common/app/user/dtos";
 
 import { UserEntity } from "./user.entity";
 import { UserRepository } from "./user.repository";
 import { EntityService } from "../_lib/entity";
 
+/**
+ * Update of an user from the service
+ */
+export interface UserUpdateEntity extends UserUpdateDto, Pick<Partial<UserDto>, "email"> {}
+
 /**
  * Service to manages [users]{@link UserEntity}.
  */
 @Injectable()
-export class UserService extends EntityService<UserEntity, UserCreateDto, UserUpdateDto> {
+export class UserService extends EntityService<UserEntity, UserCreateDto, UserUpdateEntity> {
 	/**
 	 * Constructor with "dependency injection"
 	 *

libs/common/src/app/user/dtos/user.update.dto.ts

@@ -1,9 +1,9 @@
-import { PartialType } from "@nestjs/mapped-types";
+import { OmitType, PartialType } from "@nestjs/mapped-types";
 
 import { UserCreateDto } from "./user.create.dto";
 
 /**
  * DTO used to update [users]{@link UserDto}
  * in its {@link UserEndpoint endpoint}.
  */
-export class UserUpdateDto extends PartialType(UserCreateDto) {}
+export class UserUpdateDto extends PartialType(OmitType(UserCreateDto, ["email"])) {}