fix: apply strict null checks updates to the User Module

This commit is contained in:
yassinedorbozgithub
2025-01-09 08:26:29 +01:00
parent 8326c8c121
commit 816db9817a
15 changed files with 132 additions and 110 deletions

View File

@@ -60,7 +60,7 @@ describe('AuthController', () => {
let invitationService: InvitationService;
let roleService: RoleService;
let jwtService: JwtService;
let role: Role;
let role: Role | null;
let baseUser: UserCreateDto;
beforeAll(async () => {
@@ -136,7 +136,7 @@ describe('AuthController', () => {
username: 'test',
first_name: 'test',
last_name: 'test',
roles: [role.id],
roles: [role!.id],
};
await invitationService.create(baseUser);
});

View File

@@ -87,7 +87,7 @@ export class LocalAuthController extends BaseAuthController {
logger: LoggerService,
private readonly userService: UserService,
private readonly validateAccountService: ValidateAccountService,
private readonly invitationService?: InvitationService,
private readonly invitationService: InvitationService,
) {
super(logger);
}

View File

@@ -28,7 +28,7 @@ import { ModelRepository } from '../repositories/model.repository';
import { PermissionRepository } from '../repositories/permission.repository';
import { RoleRepository } from '../repositories/role.repository';
import { UserRepository } from '../repositories/user.repository';
import { ModelModel } from '../schemas/model.schema';
import { ModelFull, ModelModel } from '../schemas/model.schema';
import { PermissionModel } from '../schemas/permission.schema';
import { RoleModel } from '../schemas/role.schema';
import { UserModel } from '../schemas/user.schema';
@@ -104,21 +104,24 @@ describe('ModelController', () => {
it('should find models, and for each model populate the corresponding permissions', async () => {
jest.spyOn(modelService, 'findAndPopulate');
const allRoles = await modelService.findAll();
const allModels = await modelService.findAll();
const allPermissions = await permissionService.findAll();
const result = await modelController.find(['permissions'], {});
const modelsWithPermissionsAndUsers = allRoles.reduce((acc, currRole) => {
const modelWithPermissionsAndUsers = {
...currRole,
permissions: allPermissions.filter((currPermission) => {
return currPermission.role === currRole.id;
}),
};
const modelsWithPermissionsAndUsers = allModels.reduce(
(acc, currRole) => {
const modelWithPermissionsAndUsers = {
...currRole,
permissions: allPermissions.filter((currPermission) => {
return currPermission.role === currRole.id;
}),
};
acc.push(modelWithPermissionsAndUsers);
return acc;
}, []);
acc.push(modelWithPermissionsAndUsers);
return acc;
},
[] as ModelFull[],
);
expect(modelService.findAndPopulate).toHaveBeenCalledWith({});
expect(result).toEqualPayload(modelsWithPermissionsAndUsers);

View File

@@ -47,7 +47,7 @@ import { UserRepository } from '../repositories/user.repository';
import { InvitationModel } from '../schemas/invitation.schema';
import { PermissionModel } from '../schemas/permission.schema';
import { Role, RoleModel } from '../schemas/role.schema';
import { User, UserModel } from '../schemas/user.schema';
import { User, UserFull, UserModel } from '../schemas/user.schema';
import { PasswordResetService } from '../services/passwordReset.service';
import { PermissionService } from '../services/permission.service';
import { RoleService } from '../services/role.service';
@@ -63,9 +63,9 @@ describe('UserController', () => {
let roleService: RoleService;
let invitationService: InvitationService;
let notFoundId: string;
let role: Role;
let role: Role | null;
let roles: Role[];
let user: User;
let user: User | null;
let passwordResetService: PasswordResetService;
let jwtService: JwtService;
beforeAll(async () => {
@@ -157,12 +157,12 @@ describe('UserController', () => {
describe('findOne', () => {
it('should find one user and populate its roles', async () => {
jest.spyOn(userService, 'findOneAndPopulate');
const result = await userController.findOne(user.id, ['roles']);
expect(userService.findOneAndPopulate).toHaveBeenCalledWith(user.id);
const result = await userController.findOne(user!.id, ['roles']);
expect(userService.findOneAndPopulate).toHaveBeenCalledWith(user!.id);
expect(result).toEqualPayload(
{
...userFixtures.find(({ username }) => username === 'admin'),
roles: roles.filter(({ id }) => user.roles.includes(id)),
roles: roles.filter(({ id }) => user!.roles.includes(id)),
},
[...IGNORED_FIELDS, 'password', 'provider'],
);
@@ -176,13 +176,17 @@ describe('UserController', () => {
jest.spyOn(userService, 'findPageAndPopulate');
const result = await userService.findPageAndPopulate({}, pageQuery);
const usersWithRoles = userFixtures.reduce((acc, currUser) => {
acc.push({
...currUser,
roles: roles.filter(({ id }) => user.roles.includes(id)),
});
return acc;
}, []);
const usersWithRoles = userFixtures.reduce(
(acc, currUser) => {
acc.push({
...currUser,
roles: roles.filter(({ id }) => user?.roles?.includes(id)),
avatar: null,
});
return acc;
},
[] as Omit<UserFull, 'id' | 'createdAt' | 'updatedAt'>[],
);
expect(userService.findPageAndPopulate).toHaveBeenCalledWith(
{},
@@ -205,7 +209,7 @@ describe('UserController', () => {
last_name: 'testUser',
email: 'test@test.test',
password: 'test',
roles: [role.id],
roles: [role!.id],
};
const result = await userController.create(userDto);
expect(userService.create).toHaveBeenCalledWith(userDto);
@@ -224,16 +228,16 @@ describe('UserController', () => {
it('should return updated user', async () => {
jest.spyOn(userService, 'updateOne');
const result = await userController.updateOne(
{ user: { id: user.id } } as any,
user.id,
{ user: { id: user!.id } } as any,
user!.id,
updateDto,
);
expect(userService.updateOne).toHaveBeenCalledWith(user.id, updateDto);
expect(userService.updateOne).toHaveBeenCalledWith(user!.id, updateDto);
expect(result).toEqualPayload(
{
...userFixtures.find(({ username }) => username === 'admin'),
...updateDto,
roles: user.roles,
roles: user!.roles,
},
[...IGNORED_FIELDS, 'password', 'provider'],
);
@@ -243,44 +247,43 @@ describe('UserController', () => {
describe('updateStateAndRoles', () => {
it('should return updated user', async () => {
const updateDto: UserUpdateStateAndRolesDto = {
roles: [role.id],
roles: [role!.id],
};
jest.spyOn(userService, 'updateOne');
const result = await userController.updateStateAndRoles(
user.id,
user!.id,
updateDto,
{
passport: {
user: { id: user.id },
user: { id: user!.id },
},
} as ExpressSession,
);
expect(userService.updateOne).toHaveBeenCalledWith(user.id, updateDto);
expect(userService.updateOne).toHaveBeenCalledWith(user!.id, updateDto);
expect(result).toEqualPayload(
{
...userFixtures.find(({ username }) => username === 'admin'),
...updateDto,
},
[...IGNORED_FIELDS, 'first_name', 'password', 'provider'],
);
});
it('should return updated user after adding an extra role', async () => {
const updateDto: UserUpdateStateAndRolesDto = {
roles: [role.id, roles[1].id],
roles: [role!.id, roles[1].id],
};
jest.spyOn(userService, 'updateOne');
const result = await userController.updateStateAndRoles(
user.id,
user!.id,
updateDto,
{
passport: {
user: { id: user.id },
user: { id: user!.id },
},
} as ExpressSession,
);
expect(userService.updateOne).toHaveBeenCalledWith(user.id, updateDto);
expect(userService.updateOne).toHaveBeenCalledWith(user!.id, updateDto);
expect(result).toEqualPayload(
{
...userFixtures.find(({ username }) => username === 'admin'),
@@ -296,9 +299,9 @@ describe('UserController', () => {
state: false,
};
await expect(
userController.updateStateAndRoles(user.id, updateDto, {
userController.updateStateAndRoles(user!.id, updateDto, {
passport: {
user: { id: user.id },
user: { id: user!.id },
},
} as ExpressSession),
).rejects.toThrow(ForbiddenException);
@@ -309,9 +312,9 @@ describe('UserController', () => {
roles: [],
};
await expect(
userController.updateStateAndRoles(user.id, updateDto, {
userController.updateStateAndRoles(user!.id, updateDto, {
passport: {
user: { id: user.id },
user: { id: user!.id },
},
} as ExpressSession),
).rejects.toThrow(ForbiddenException);
@@ -343,8 +346,8 @@ describe('UserController', () => {
describe('deleteOne', () => {
it('should delete user by id', async () => {
const result = await userController.deleteOne(user.id);
notFoundId = user.id;
const result = await userController.deleteOne(user!.id);
notFoundId = user!.id;
expect(result).toEqual({
acknowledged: true,
deletedCount: 1,

View File

@@ -142,12 +142,12 @@ export class ReadOnlyUserController extends BaseController<
);
const currentPermissions = await this.permissionService.findAndPopulate({
role: {
$in: currentUser.roles.map(({ id }) => id),
$in: currentUser?.roles.map(({ id }) => id),
},
});
return {
roles: currentUser.roles,
roles: currentUser?.roles,
permissions: currentPermissions.map((permission) => {
if (permission.model) {
return {
@@ -248,7 +248,9 @@ export class ReadWriteUserController extends ReadOnlyUserController {
roles: (await this.roleService.findAll())
.filter((role) => user.roles.includes(role.id))
.map((role) => role.id),
avatar: (await this.attachmentService.findOne(user.avatar))?.id,
avatar: user.avatar
? (await this.attachmentService.findOne(user.avatar))?.id
: undefined,
},
});
return await this.userService.create(user);
@@ -285,7 +287,7 @@ export class ReadWriteUserController extends ReadOnlyUserController {
@Body() userUpdate: UserEditProfileDto,
@UploadedFile() avatarFile?: Express.Multer.File,
) {
if (!('id' in req.user && req.user.id) || req.user.id !== id) {
if (!(req.user && 'id' in req.user && req.user.id) || req.user.id !== id) {
throw new ForbiddenException();
}
@@ -339,18 +341,20 @@ export class ReadWriteUserController extends ReadOnlyUserController {
@Body() body: UserUpdateStateAndRolesDto,
@Session() session: ExpressSession,
) {
const oldRoles = (await this.userService.findOne(id)).roles;
const oldRoles = (await this.userService.findOne(id))?.roles;
const newRoles = body.roles;
const { id: adminRoleId } = await this.roleService.findOne({
name: 'admin',
});
const { id: adminRoleId } =
(await this.roleService.findOne({
name: 'admin',
})) || {};
if (id === session.passport?.user?.id && body.state === false) {
throw new ForbiddenException('Your account state is protected');
}
if (
adminRoleId &&
session?.passport?.user?.id === id &&
oldRoles.includes(adminRoleId) &&
!newRoles.includes(adminRoleId)
oldRoles?.includes(adminRoleId) &&
!newRoles?.includes(adminRoleId)
) {
throw new ForbiddenException('Admin privileges are protected');
}