diff --git a/apps/user-service/src/app.module.ts b/apps/user-service/src/app.module.ts index b1d786f3b..37620b248 100644 --- a/apps/user-service/src/app.module.ts +++ b/apps/user-service/src/app.module.ts @@ -10,6 +10,7 @@ import { ResetPasswordService } from "./auth/reset-password.service"; import { VerificationUserController } from "./auth/verification-user.controller"; import { VerificationUserService } from "./auth/verification-user.service"; import { UserCreationService } from "./users/user-creation.service"; +import { UsersService } from "./users/users.service"; import { HealthModule } from "@terramatch-microservices/common/health/health.module"; import { OrganisationsController } from "./organisations/organisations.controller"; import { OrganisationsService } from "./organisations/organisations.service"; @@ -44,6 +45,7 @@ import { UserAssociationService } from "./user-association/user-association.serv ResetPasswordService, VerificationUserService, UserCreationService, + UsersService, OrganisationsService, OrganisationCreationService, ActionsService, diff --git a/apps/user-service/src/organisations/organisations.controller.ts b/apps/user-service/src/organisations/organisations.controller.ts index d0e6c01a5..e85add1ef 100644 --- a/apps/user-service/src/organisations/organisations.controller.ts +++ b/apps/user-service/src/organisations/organisations.controller.ts @@ -215,7 +215,10 @@ export class OrganisationsController { const document = buildJsonApi(OrganisationLightDto); const orgResource = document.addData(organisation.uuid, new OrganisationLightDto(organisation)); if (user != null) { - const userResource = document.addData(user.uuid ?? "no-uuid", new UserDto(user, await user.myFrameworks())); + const userResource = document.addData( + user.uuid ?? "no-uuid", + new UserDto(user, user.frameworks, await user.myFrameworks()) + ); userResource.relateTo("org", orgResource, { meta: { userStatus: "na" } }); } return document; diff --git a/apps/user-service/src/user-association/user-association.controller.spec.ts b/apps/user-service/src/user-association/user-association.controller.spec.ts index 4fb28847e..5642bcb4d 100644 --- a/apps/user-service/src/user-association/user-association.controller.spec.ts +++ b/apps/user-service/src/user-association/user-association.controller.spec.ts @@ -155,6 +155,56 @@ describe("UserAssociationController", () => { }); }); + describe("inviteOrganisationUser", () => { + it("should authorize and invite an organisation user", async () => { + const organisation = await OrganisationFactory.create(); + const invite = { + uuid: "invite-uuid", + emailAddress: "invitee@example.com", + callbackUrl: "https://example.com/invite", + organisationUuid: organisation.uuid + }; + (stubProcessor.getEntity as jest.Mock).mockResolvedValue(organisation); + userAssociationService.inviteOrganisationUser.mockResolvedValue(invite as never); + + const result = serialize( + await controller.inviteOrganisationUser( + { uuid: organisation.uuid as string, model: "organisations" }, + { + emailAddress: "invitee@example.com", + callbackUrl: "https://example.com/invite" + } + ) + ); + + expect(userAssociationService.createProcessor).toHaveBeenCalledWith("organisations", organisation.uuid); + expect(policyService.authorize).toHaveBeenCalledWith("update", organisation); + expect(userAssociationService.inviteOrganisationUser).toHaveBeenCalledWith( + organisation, + "invitee@example.com", + "https://example.com/invite" + ); + expect(result.data).toBeDefined(); + expect((result.data as Resource).id).toBe("invite-uuid"); + }); + + it("should propagate UnauthorizedException when policy denies", async () => { + const organisation = await OrganisationFactory.create(); + (stubProcessor.getEntity as jest.Mock).mockResolvedValue(organisation); + policyService.authorize.mockRejectedValue(new UnauthorizedException()); + + await expect( + controller.inviteOrganisationUser( + { uuid: organisation.uuid as string, model: "organisations" }, + { + emailAddress: "invitee@example.com", + callbackUrl: "https://example.com/invite" + } + ) + ).rejects.toThrow(UnauthorizedException); + }); + }); + describe("deleteUserAssociations", () => { it("should call handleDelete and return deleted response", async () => { const project = await ProjectFactory.create(); diff --git a/apps/user-service/src/users/dto/admin-user-create.dto.ts b/apps/user-service/src/users/dto/admin-user-create.dto.ts new file mode 100644 index 000000000..19bf610a5 --- /dev/null +++ b/apps/user-service/src/users/dto/admin-user-create.dto.ts @@ -0,0 +1,23 @@ +import { CreateDataDto, JsonApiBodyDto } from "@terramatch-microservices/common/util/json-api-update-dto"; +import { IsArray, IsNotEmpty, IsString } from "class-validator"; +import { ApiProperty } from "@nestjs/swagger"; +import { UserCreateBaseAttributes } from "./user-create.dto"; + +export class AdminUserCreateAttributes extends UserCreateBaseAttributes { + @IsNotEmpty() + @ApiProperty() + role: string; + + @IsNotEmpty() + @ApiProperty() + organisationUuid: string; + + @ApiProperty({ isArray: true, type: String }) + @IsArray() + @IsString({ each: true }) + directFrameworks: string[]; +} + +export class AdminUserCreateBody extends JsonApiBodyDto( + class AdminUserCreateData extends CreateDataDto("users", AdminUserCreateAttributes) {} +) {} diff --git a/apps/user-service/src/users/dto/user-create.dto.ts b/apps/user-service/src/users/dto/user-create.dto.ts index 8f3cd7ea2..b9a4a6690 100644 --- a/apps/user-service/src/users/dto/user-create.dto.ts +++ b/apps/user-service/src/users/dto/user-create.dto.ts @@ -2,7 +2,7 @@ import { IsEmail, IsIn, IsNotEmpty, IsOptional } from "class-validator"; import { ApiProperty } from "@nestjs/swagger"; import { CreateDataDto, JsonApiBodyDto } from "@terramatch-microservices/common/util/json-api-update-dto"; -export class UserCreateAttributes { +export class UserCreateBaseAttributes { @IsNotEmpty() @ApiProperty() firstName: string; @@ -11,10 +11,6 @@ export class UserCreateAttributes { @ApiProperty() lastName: string; - @IsNotEmpty() - @ApiProperty() - password: string; - @IsEmail() @ApiProperty() emailAddress: string; @@ -27,11 +23,6 @@ export class UserCreateAttributes { @ApiProperty() jobRole: string; - @IsNotEmpty() - @IsIn(["project-developer", "funder", "government"]) - @ApiProperty() - role: string; - @IsOptional() @ApiProperty() country: string; @@ -39,6 +30,21 @@ export class UserCreateAttributes { @IsOptional() @ApiProperty() program: string; +} + +export class UserCreateBaseBody extends JsonApiBodyDto( + class UserCreateBaseData extends CreateDataDto("users", UserCreateBaseAttributes) {} +) {} + +export class UserCreateAttributes extends UserCreateBaseAttributes { + @IsNotEmpty() + @ApiProperty() + password: string; + + @IsNotEmpty() + @IsIn(["project-developer", "funder", "government"]) + @ApiProperty() + role: string; @IsNotEmpty() @ApiProperty() diff --git a/apps/user-service/src/users/dto/user-query.dto.ts b/apps/user-service/src/users/dto/user-query.dto.ts new file mode 100644 index 000000000..1f90fea99 --- /dev/null +++ b/apps/user-service/src/users/dto/user-query.dto.ts @@ -0,0 +1,15 @@ +import { IsOptional } from "class-validator"; +import { ApiProperty } from "@nestjs/swagger"; +import { IndexQueryDto } from "@terramatch-microservices/common/dto/index-query.dto"; +import { TransformBooleanString } from "@terramatch-microservices/common/decorators/transform-boolean-string.decorator"; + +export class UserQueryDto extends IndexQueryDto { + @ApiProperty({ required: false }) + @IsOptional() + search?: string; + + @ApiProperty({ required: false, description: "Filter users by email address verification status" }) + @IsOptional() + @TransformBooleanString() + isVerified?: boolean; +} diff --git a/apps/user-service/src/users/dto/user-update.dto.ts b/apps/user-service/src/users/dto/user-update.dto.ts index 78ebcf8e0..b64804a74 100644 --- a/apps/user-service/src/users/dto/user-update.dto.ts +++ b/apps/user-service/src/users/dto/user-update.dto.ts @@ -1,12 +1,44 @@ -import { IsEnum } from "class-validator"; +import { IsArray, IsEnum, IsString } from "class-validator"; import { ApiProperty } from "@nestjs/swagger"; import { JsonApiBodyDto, JsonApiDataDto } from "@terramatch-microservices/common/util/json-api-update-dto"; import { VALID_LOCALES, ValidLocale } from "@terramatch-microservices/database/constants/locale"; -class UserUpdateAttributes { +export class UserUpdateAttributes { + @ApiProperty({ description: "Organisation UUID", nullable: true, required: false, format: "uuid" }) + organisationUuid?: string | null; + + @ApiProperty({ description: "First name", nullable: true, required: false }) + firstName?: string | null; + + @ApiProperty({ description: "Last name", nullable: true, required: false }) + lastName?: string | null; + + @ApiProperty({ description: "Email address", nullable: true, required: false, format: "email" }) + emailAddress?: string | null; + + @ApiProperty({ description: "Job role", nullable: true, required: false }) + jobRole?: string | null; + + @ApiProperty({ description: "Phone number", nullable: true, required: false }) + phoneNumber?: string | null; + + @ApiProperty({ description: "Country", nullable: true, required: false }) + country?: string | null; + + @ApiProperty({ description: "Program", nullable: true, required: false }) + program?: string | null; + @IsEnum(VALID_LOCALES) @ApiProperty({ description: "New default locale for the given user", nullable: true, enum: VALID_LOCALES }) locale?: ValidLocale | null; + + @ApiProperty() + primaryRole?: string | null; + + @ApiProperty({ isArray: true, type: String }) + @IsArray() + @IsString({ each: true }) + directFrameworks?: string[] | null; } export class UserUpdateBody extends JsonApiBodyDto( diff --git a/apps/user-service/src/users/user-creation.service.spec.ts b/apps/user-service/src/users/user-creation.service.spec.ts index c9f11937e..72ef35bb1 100644 --- a/apps/user-service/src/users/user-creation.service.spec.ts +++ b/apps/user-service/src/users/user-creation.service.spec.ts @@ -3,22 +3,34 @@ import { createMock, DeepMocked } from "@golevelup/ts-jest"; import { LocalizationKey, ModelHasRole, - Role, - User, - Verification, - PasswordReset, + Organisation, OrganisationInvite, + PasswordReset, ProjectInvite, - ProjectUser + ProjectUser, + Role, + User, + Verification } from "@terramatch-microservices/database/entities"; import { EmailService } from "@terramatch-microservices/common/email/email.service"; import { LocalizationService } from "@terramatch-microservices/common/localization/localization.service"; import { UserCreationService } from "./user-creation.service"; import { UserCreateAttributes } from "./dto/user-create.dto"; -import { InternalServerErrorException, NotFoundException, UnprocessableEntityException } from "@nestjs/common"; -import { RoleFactory, UserFactory, ProjectFactory } from "@terramatch-microservices/database/factories"; +import { + BadRequestException, + InternalServerErrorException, + NotFoundException, + UnprocessableEntityException +} from "@nestjs/common"; +import { + RoleFactory, + UserFactory, + ProjectFactory, + FrameworkFactory +} from "@terramatch-microservices/database/factories"; import { LocalizationKeyFactory } from "@terramatch-microservices/database/factories/localization-key.factory"; import { TemplateService } from "@terramatch-microservices/common/templates/template.service"; +import { AdminUserCreateAttributes } from "./dto/admin-user-create.dto"; describe("UserCreationService", () => { let service: UserCreationService; @@ -128,7 +140,7 @@ describe("UserCreationService", () => { emailService.sendI18nTemplateEmail.mockReturnValue(Promise.resolve()); templateService.render.mockReturnValue("rendered template"); - const result = await service.createNewUser(userNewRequest); + const result = await service.createNewUser(false, userNewRequest); expect(reloadSpy).toHaveBeenCalled(); expect(emailService.sendI18nTemplateEmail).toHaveBeenCalled(); expect(result).toBeDefined(); @@ -148,7 +160,7 @@ describe("UserCreationService", () => { Promise.resolve([localizationBody, localizationSubject, localizationTitle, localizationCta]) ); - await expect(service.createNewUser(userNewRequest)).rejects.toThrow( + await expect(service.createNewUser(false, userNewRequest)).rejects.toThrow( new UnprocessableEntityException("User already exists") ); }); @@ -168,7 +180,7 @@ describe("UserCreationService", () => { Promise.resolve([localizationBody, localizationSubject, localizationTitle, localizationCta]) ); - await expect(service.createNewUser(userNewRequest)).rejects.toThrow(new NotFoundException("Role not found")); + await expect(service.createNewUser(false, userNewRequest)).rejects.toThrow(new NotFoundException("Role not found")); }); it("should generate a error when create user in DB", async () => { @@ -190,7 +202,7 @@ describe("UserCreationService", () => { jest.spyOn(Role, "findOne").mockImplementation(() => Promise.resolve(role)); jest.spyOn(User, "create").mockImplementation(() => Promise.reject()); - await expect(service.createNewUser(userNewRequest)).rejects.toThrow(InternalServerErrorException); + await expect(service.createNewUser(false, userNewRequest)).rejects.toThrow(InternalServerErrorException); }); it("should generate a error when save token verification in DB", async () => { @@ -215,7 +227,7 @@ describe("UserCreationService", () => { jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue(null); jest.spyOn(Verification, "findOrCreate").mockImplementation(() => Promise.reject()); - await expect(service.createNewUser(userNewRequest)).rejects.toThrow(InternalServerErrorException); + await expect(service.createNewUser(false, userNewRequest)).rejects.toThrow(InternalServerErrorException); }); it("should generate a error when send email verification", async () => { @@ -240,7 +252,7 @@ describe("UserCreationService", () => { ); emailService.sendI18nTemplateEmail.mockRejectedValue(null); - await expect(service.createNewUser(userNewRequest)).rejects.toThrow(InternalServerErrorException); + await expect(service.createNewUser(false, userNewRequest)).rejects.toThrow(InternalServerErrorException); }); it("should return an error when User.create fails", async () => { @@ -262,7 +274,7 @@ describe("UserCreationService", () => { jest.spyOn(Role, "findOne").mockImplementation(() => Promise.resolve(role)); jest.spyOn(User, "create").mockRejectedValue(new Error("User creation failed")); - await expect(service.createNewUser(userNewRequest)).rejects.toThrow(InternalServerErrorException); + await expect(service.createNewUser(false, userNewRequest)).rejects.toThrow(InternalServerErrorException); }); it("should return an error when ModelHasRole.findOrCreate fails", async () => { @@ -285,7 +297,7 @@ describe("UserCreationService", () => { jest.spyOn(User, "create").mockImplementation(() => Promise.resolve(user)); jest.spyOn(ModelHasRole, "findOrCreate").mockRejectedValue(new Error("ModelHasRole creation failed")); - await expect(service.createNewUser(userNewRequest)).rejects.toThrow(InternalServerErrorException); + await expect(service.createNewUser(false, userNewRequest)).rejects.toThrow(InternalServerErrorException); }); it("should return an error when Verification.findOrCreate fails", async () => { @@ -311,7 +323,7 @@ describe("UserCreationService", () => { jest.spyOn(Verification, "findOrCreate").mockRejectedValue(new Error("Verification creation failed")); - await expect(service.createNewUser(userNewRequest)).rejects.toThrow(InternalServerErrorException); + await expect(service.createNewUser(false, userNewRequest)).rejects.toThrow(InternalServerErrorException); }); describe("invite-based signup completion", () => { @@ -321,11 +333,11 @@ describe("UserCreationService", () => { return request; }; - it("should throw NotFoundException when token is invalid", async () => { + it("should throw BadRequestException when token is invalid", async () => { const request = getInviteRequest("test@example.com", "invalid-token"); jest.spyOn(PasswordReset, "findOne").mockResolvedValue(null); - await expect(service.createNewUser(request)).rejects.toThrow(NotFoundException); + await expect(service.createNewUser(false, request)).rejects.toThrow(BadRequestException); }); it("should complete invite signup with organisation invite", async () => { @@ -348,7 +360,7 @@ describe("UserCreationService", () => { jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); jest.spyOn(user, "save").mockResolvedValue(user); - const result = await service.createNewUser(request); + const result = await service.createNewUser(false, request); expect(organisationInvite.emailAddress).toBe(request.emailAddress); expect(organisationInvite.acceptedAt).not.toBeNull(); @@ -382,7 +394,7 @@ describe("UserCreationService", () => { jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); jest.spyOn(user, "save").mockResolvedValue(user); - const result = await service.createNewUser(request); + const result = await service.createNewUser(false, request); expect(projectInvite.emailAddress).toBe(request.emailAddress); expect(projectInvite.acceptedAt).not.toBeNull(); @@ -408,9 +420,117 @@ describe("UserCreationService", () => { jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); jest.spyOn(user, "save").mockResolvedValue(user); - await service.createNewUser(request); + await service.createNewUser(false, request); expect(Role.findOne).toHaveBeenCalledWith({ where: { name: "project-developer" } }); }); + + it("should update existing project-user relation if already present", async () => { + const user = await UserFactory.create(); + const project = await ProjectFactory.create(); + const request = getInviteRequest("new@example.com", "valid-token"); + const passwordReset = { + user, + destroy: jest.fn().mockResolvedValue(undefined) + } as unknown as PasswordReset; + const projectInvite = { + emailAddress: user.emailAddress, + project, + acceptedAt: null, + save: jest.fn().mockResolvedValue(undefined) + } as unknown as ProjectInvite; + const role = RoleFactory.create({ name: "project-developer" }); + const projectUser = { + isMonitoring: false, + status: "inactive", + save: jest.fn().mockResolvedValue(undefined) + } as unknown as ProjectUser; + + jest.spyOn(PasswordReset, "findOne").mockResolvedValue(passwordReset); + jest.spyOn(OrganisationInvite, "findAll").mockResolvedValue([]); + jest.spyOn(ProjectInvite, "findAll").mockResolvedValue([projectInvite]); + jest.spyOn(ProjectUser, "findOrCreate").mockResolvedValue([projectUser, false]); + jest.spyOn(Role, "findOne").mockResolvedValue(role); + jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); + jest.spyOn(user, "save").mockResolvedValue(user); + + await service.createNewUser(false, request); + + expect(projectUser.isMonitoring).toBe(true); + expect(projectUser.status).toBe("active"); + expect(projectUser.save).toHaveBeenCalled(); + }); + }); + + describe("authenticated/admin user creation", () => { + const getAdminRequest = (email: string, role: string) => { + const frameworkSlug = "ppc"; + RoleFactory.create({ name: role }); + FrameworkFactory.create({ slug: frameworkSlug }); + const request = new AdminUserCreateAttributes(); + request.emailAddress = email; + request.firstName = "firstName"; + request.lastName = "lastName"; + request.jobRole = "developer"; + request.phoneNumber = "1234567890"; + request.role = role; + request.organisationUuid = "org-uuid"; + request.directFrameworks = [frameworkSlug]; + return request; + }; + + it("should create user for authenticated request", async () => { + const request = getAdminRequest("admin-created@example.com", "project-manager"); + const role = RoleFactory.create({ name: request.role }); + const createdUser = await UserFactory.create(); + const organisation = { id: 999 } as Organisation; + + jest.spyOn(Organisation, "findOne").mockResolvedValue(organisation); + jest.spyOn(User, "count").mockResolvedValue(0); + jest.spyOn(User, "create").mockResolvedValue(createdUser); + jest.spyOn(Role, "findOne").mockResolvedValue(role); + jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); + + const result = await service.createNewUser(true, request); + + expect(Organisation.findOne).toHaveBeenCalledWith({ + where: { uuid: request.organisationUuid }, + attributes: ["id"] + }); + expect(User.create).toHaveBeenCalledWith(expect.objectContaining({ organisationId: organisation.id })); + expect(result).toBe(createdUser); + }); + + it("should fail when organisation does not exist", async () => { + const request = getAdminRequest("admin-created@example.com", "project-manager"); + + jest.spyOn(Organisation, "findOne").mockResolvedValue(null); + + await expect(service.createNewUser(true, request)).rejects.toThrow( + new NotFoundException("Organisation not found") + ); + }); + + it("should fail when admin role does not exist", async () => { + const request = getAdminRequest("admin-created@example.com", "project-manager"); + const createdUser = await UserFactory.create(); + const organisation = { id: 888 } as Organisation; + + jest.spyOn(Organisation, "findOne").mockResolvedValue(organisation); + jest.spyOn(User, "count").mockResolvedValue(0); + jest.spyOn(User, "create").mockResolvedValue(createdUser); + jest.spyOn(Role, "findOne").mockResolvedValue(null); + + await expect(service.createNewUser(true, request)).rejects.toThrow(new BadRequestException("Role not found")); + }); + + it("should fail validation for malformed admin payload", async () => { + const request = getAdminRequest("invalid-email", "project-manager"); + request.organisationUuid = ""; + // @ts-expect-error test invalid payload + request.phoneNumber = null; + + await expect(service.createNewUser(true, request)).rejects.toThrow(BadRequestException); + }); }); }); diff --git a/apps/user-service/src/users/user-creation.service.ts b/apps/user-service/src/users/user-creation.service.ts index 30b366070..eec2fdf9c 100644 --- a/apps/user-service/src/users/user-creation.service.ts +++ b/apps/user-service/src/users/user-creation.service.ts @@ -1,7 +1,7 @@ import { + BadRequestException, Injectable, InternalServerErrorException, - NotFoundException, UnprocessableEntityException } from "@nestjs/common"; import { @@ -10,16 +10,22 @@ import { User, Verification, PasswordReset, + Organisation, OrganisationInvite, ProjectInvite, - ProjectUser + ProjectUser, + FrameworkUser, + Framework } from "@terramatch-microservices/database/entities"; import { EmailService } from "@terramatch-microservices/common/email/email.service"; -import { UserCreateAttributes } from "./dto/user-create.dto"; +import { UserCreateAttributes, UserCreateBaseAttributes } from "./dto/user-create.dto"; import crypto from "node:crypto"; import { omit } from "lodash"; import bcrypt from "bcryptjs"; +import { validate } from "class-validator"; import { TMLogger } from "@terramatch-microservices/common/util/tm-logger"; +import { AdminUserCreateAttributes } from "./dto/admin-user-create.dto"; +import { plainToInstance } from "class-transformer"; const EMAIL_KEYS = { body: "user-verification.body", @@ -35,14 +41,26 @@ export class UserCreationService { constructor(private readonly emailService: EmailService) {} - async createNewUser(request: UserCreateAttributes): Promise { + async createNewUser(isAuthenticated: boolean, request: UserCreateBaseAttributes): Promise { + if (isAuthenticated) { + return await this.adminUserCreateProcess(request as AdminUserCreateAttributes); + } + return await this.unauthenticatedUserCreateProcess(request as UserCreateAttributes); + } + + private async unauthenticatedUserCreateProcess(request: UserCreateAttributes): Promise { if (request.token != null) { return this.completeInviteSignup(request); } - return this.createRegularUser(request); + return await this.signUpProcess(request); } - private async createRegularUser(request: UserCreateAttributes): Promise { + private async signUpProcess(request: UserCreateAttributes): Promise { + const dto = plainToInstance(UserCreateAttributes, request); + const errors = await validate(dto); + if (errors.length > 0) { + throw new BadRequestException(errors); + } const role = request.role; if (!this.roles.includes(role)) { throw new UnprocessableEntityException("Role not valid"); @@ -50,7 +68,7 @@ export class UserCreationService { const roleEntity = await Role.findOne({ where: { name: role } }); if (roleEntity == null) { - throw new NotFoundException("Role not found"); + throw new BadRequestException("Role not found"); } const userExists = (await User.count({ where: { emailAddress: request.emailAddress } })) !== 0; @@ -61,7 +79,7 @@ export class UserCreationService { try { const hashPassword = await bcrypt.hash(request.password, 10); const callbackUrl = request.callbackUrl; - const newUser = omit(request, ["callbackUrl", "role", "password", "token"]); + const newUser = omit(request, ["callbackUrl", "role", "password"]); const user = await User.create({ ...newUser, password: hashPassword } as User); @@ -82,6 +100,55 @@ export class UserCreationService { } } + private async adminUserCreateProcess(request: AdminUserCreateAttributes): Promise { + const dto = plainToInstance(AdminUserCreateAttributes, request); + const errors = await validate(dto); + if (errors.length > 0) { + throw new BadRequestException(errors); + } + const organisation = await Organisation.findOne({ + where: { uuid: request.organisationUuid }, + attributes: ["id"] + }); + if (organisation == null) { + throw new BadRequestException("Organisation not found"); + } + const userExists = (await User.count({ where: { emailAddress: request.emailAddress } })) !== 0; + if (userExists) { + throw new UnprocessableEntityException("User already exists"); + } + const roleEntity = await Role.findOne({ where: { name: request.role } }); + if (roleEntity == null) { + throw new BadRequestException("Role not found"); + } + const frameworkEntities = await Framework.findAll({ where: { slug: request.directFrameworks } }); + if (frameworkEntities.length !== request.directFrameworks.length) { + throw new BadRequestException("One or more frameworks not found"); + } + try { + const newUser = omit(request, ["role", "organisationUuid", "directFrameworks"]); + const user = await User.create({ ...newUser, organisationId: organisation.id } as User); + await ModelHasRole.findOrCreate({ + where: { modelId: user.id, roleId: roleEntity.id }, + defaults: { modelId: user.id, roleId: roleEntity.id, modelType: User.LARAVEL_TYPE } as ModelHasRole + }); + + if (frameworkEntities.length > 0) { + await FrameworkUser.bulkCreate( + frameworkEntities.map(framework => ({ userId: user.id, frameworkId: framework.id })) as FrameworkUser[] + ); + } + + const reloadedUser = await user.reload({ + include: [{ association: "organisation", attributes: ["id", "uuid", "name"] }, { association: "frameworks" }] + }); + return reloadedUser; + } catch (error) { + this.logger.error(error); + throw new InternalServerErrorException("User creation failed"); + } + } + private async completeInviteSignup(request: UserCreateAttributes): Promise { const token = request.token; const passwordReset = await PasswordReset.findOne({ @@ -90,7 +157,7 @@ export class UserCreationService { }); if (passwordReset == null || passwordReset.user == null) { - throw new NotFoundException("Invalid signup token"); + throw new BadRequestException("Invalid signup token"); } const user = passwordReset.user; @@ -158,7 +225,7 @@ export class UserCreationService { const role = "project-developer"; const roleEntity = await Role.findOne({ where: { name: role } }); if (roleEntity == null) { - throw new NotFoundException("Role not found"); + throw new BadRequestException("Role not found"); } await ModelHasRole.findOrCreate({ diff --git a/apps/user-service/src/users/users.controller.spec.ts b/apps/user-service/src/users/users.controller.spec.ts index 3182f8dc5..9bd03a808 100644 --- a/apps/user-service/src/users/users.controller.spec.ts +++ b/apps/user-service/src/users/users.controller.spec.ts @@ -10,6 +10,7 @@ import { UserCreateAttributes } from "./dto/user-create.dto"; import { UserCreationService } from "./user-creation.service"; import { ValidLocale } from "@terramatch-microservices/database/constants/locale"; import { mockUserId, serialize } from "@terramatch-microservices/common/util/testing"; +import { UsersService } from "./users.service"; import { User } from "@terramatch-microservices/database/entities"; const createRequest = (attributes: UserCreateAttributes = new UserCreateAttributes()) => ({ @@ -20,17 +21,24 @@ describe("UsersController", () => { let controller: UsersController; let policyService: DeepMocked; let userCreationService: DeepMocked; - + let usersService: DeepMocked; + let realUsersService: UsersService; beforeEach(async () => { + realUsersService = new UsersService(); const module: TestingModule = await Test.createTestingModule({ controllers: [UsersController], providers: [ { provide: PolicyService, useValue: (policyService = createMock()) }, - { provide: UserCreationService, useValue: (userCreationService = createMock()) } + { provide: UserCreationService, useValue: (userCreationService = createMock()) }, + { provide: UsersService, useValue: (usersService = createMock()) } ] }).compile(); controller = module.get(UsersController); + usersService.update.mockImplementation((user, attrs) => realUsersService.update(user, attrs)); + usersService.delete.mockImplementation(async u => { + await realUsersService.delete(u); + }); }); afterEach(() => { @@ -137,6 +145,64 @@ describe("UsersController", () => { }); }); + describe("delete", () => { + it("should throw not found if the user is not found", async () => { + await expect(controller.delete({ uuid: "00000000-0000-4000-8000-000000000000" })).rejects.toThrow( + NotFoundException + ); + }); + + it("should throw unauthorized if policy does not authorize", async () => { + const { uuid } = await UserFactory.create(); + policyService.authorize.mockRejectedValue(new UnauthorizedException()); + + await expect(controller.delete({ uuid: uuid! })).rejects.toThrow(UnauthorizedException); + }); + + it("should soft-delete the user and return a JSON:API deleted document", async () => { + const user = await UserFactory.create(); + policyService.authorize.mockResolvedValue(undefined); + usersService.delete.mockImplementation(async u => { + await u.destroy(); + }); + + const result = serialize(await controller.delete({ uuid: user.uuid! })); + + expect(policyService.authorize).toHaveBeenCalledWith("delete", expect.objectContaining({ id: user.id })); + expect(usersService.delete).toHaveBeenCalledWith(expect.objectContaining({ id: user.id })); + expect(result.meta.resourceType).toBe("users"); + expect(result.meta.resourceIds).toEqual([user.uuid]); + + const reloaded = await User.findOne({ where: { id: user.id }, paranoid: false }); + expect(reloaded?.deletedAt).not.toBeNull(); + }); + }); + + describe("userIndex", () => { + it("authorizes and enriches the document when users are returned", async () => { + const user = await UserFactory.create(); + const query = { page: { number: 2 } }; + usersService.findMany.mockResolvedValue({ users: [user], paginationTotal: 1 }); + usersService.addUsersToDocument.mockImplementation(async document => document); + + await controller.userIndex(query); + + expect(usersService.findMany).toHaveBeenCalledWith(query); + expect(policyService.authorize).toHaveBeenCalledWith("read", [user]); + expect(usersService.addUsersToDocument).toHaveBeenCalledWith(expect.anything(), [user]); + }); + + it("does not authorize when no users are returned", async () => { + usersService.findMany.mockResolvedValue({ users: [], paginationTotal: 0 }); + usersService.addUsersToDocument.mockImplementation(async document => document); + + await controller.userIndex({}); + + expect(policyService.authorize).not.toHaveBeenCalled(); + expect(usersService.addUsersToDocument).toHaveBeenCalledWith(expect.anything(), []); + }); + }); + describe("update", () => { const makeValidBody = (uuid: string, locale?: ValidLocale) => ({ data: { @@ -176,6 +242,30 @@ describe("UsersController", () => { }); describe("create", () => { + it("authorizes creation when request is authenticated", async () => { + const user = await UserFactory.create(); + const attributes = new UserCreateAttributes(); + mockUserId(1); + userCreationService.createNewUser.mockResolvedValue(user); + + await controller.create(createRequest(attributes)); + + expect(policyService.authorize).toHaveBeenCalledWith("create", User); + expect(userCreationService.createNewUser).toHaveBeenCalledWith(true, attributes); + }); + + it("does not authorize creation when request is unauthenticated", async () => { + const user = await UserFactory.create(); + const attributes = new UserCreateAttributes(); + mockUserId(); + userCreationService.createNewUser.mockResolvedValue(user); + + await controller.create(createRequest(attributes)); + + expect(policyService.authorize).not.toHaveBeenCalled(); + expect(userCreationService.createNewUser).toHaveBeenCalledWith(false, attributes); + }); + it("should create a new user", async () => { const user = await UserFactory.create(); const attributes = new UserCreateAttributes(); diff --git a/apps/user-service/src/users/users.controller.ts b/apps/user-service/src/users/users.controller.ts index 0abf32762..6f1604ea6 100644 --- a/apps/user-service/src/users/users.controller.ts +++ b/apps/user-service/src/users/users.controller.ts @@ -2,6 +2,7 @@ import { BadRequestException, Body, Controller, + Delete, Get, HttpCode, HttpStatus, @@ -9,18 +10,29 @@ import { Param, Patch, Post, + Query, UnauthorizedException } from "@nestjs/common"; import { User } from "@terramatch-microservices/database/entities"; import { PolicyService } from "@terramatch-microservices/common"; import { ApiOperation, ApiParam } from "@nestjs/swagger"; import { OrganisationLightDto, UserDto } from "@terramatch-microservices/common/dto"; +import { SingleResourceDto } from "@terramatch-microservices/common/dto/single-resource.dto"; import { ExceptionResponse, JsonApiResponse } from "@terramatch-microservices/common/decorators"; -import { buildJsonApi, DocumentBuilder } from "@terramatch-microservices/common/util"; +import { JsonApiDeletedResponse } from "@terramatch-microservices/common/decorators/json-api-response.decorator"; +import { + buildDeletedResponse, + buildJsonApi, + DocumentBuilder, + getDtoType, + getStableRequestQuery +} from "@terramatch-microservices/common/util"; import { UserUpdateBody } from "./dto/user-update.dto"; -import { NoBearerAuth } from "@terramatch-microservices/common/guards"; -import { UserCreateBody } from "./dto/user-create.dto"; +import { OptionalBearerAuth } from "@terramatch-microservices/common/guards"; +import { UserCreateBaseBody } from "./dto/user-create.dto"; import { UserCreationService } from "./user-creation.service"; +import { UserQueryDto } from "./dto/user-query.dto"; +import { UsersService } from "./users.service"; import { authenticatedUserId } from "@terramatch-microservices/common/guards/auth.guard"; export const USER_ORG_RELATIONSHIP = { @@ -45,9 +57,30 @@ const USER_RESPONSE_SHAPE = { export class UsersController { constructor( private readonly policyService: PolicyService, - private readonly userCreationService: UserCreationService + private readonly userCreationService: UserCreationService, + private readonly usersService: UsersService ) {} + @Get() + @ApiOperation({ operationId: "userIndex", description: "Fetch a paginated list of users" }) + @JsonApiResponse([{ data: UserDto, pagination: "number" }]) + @ExceptionResponse(UnauthorizedException, { description: "Authorization failed" }) + async userIndex(@Query() query: UserQueryDto) { + const { users, paginationTotal } = await this.usersService.findMany(query); + if (users.length > 0) { + await this.policyService.authorize("read", users); + } + + const document = buildJsonApi(UserDto, { forceDataArray: true }).addIndex({ + requestPath: `/users/v3/users${getStableRequestQuery(query)}`, + total: paginationTotal, + pageNumber: query.page?.number ?? 1 + }); + + await this.usersService.addUsersToDocument(document, users); + return document; + } + @Get(":uuid") @ApiOperation({ operationId: "usersFind", description: "Fetch a user by UUID, or with the 'me' identifier" }) @ApiParam({ name: "uuid", example: "me", description: 'A valid user UUID or "me"' }) @@ -87,26 +120,41 @@ export class UsersController { await this.policyService.authorize("update", user); - // The only thing allowed to update for now is the locale - const { locale } = updatePayload.data.attributes; - if (locale != null) { - user.locale = locale; - await user.save(); - } + const updatedUser = await this.usersService.update(user, updatePayload.data.attributes); - return await this.addUserResource(buildJsonApi(UserDto), user); + return await this.addUserResource(buildJsonApi(UserDto), updatedUser as User); + } + + @Delete(":uuid") + @ApiOperation({ operationId: "userDelete", summary: "Delete a user by UUID" }) + @JsonApiDeletedResponse(getDtoType(UserDto), { description: "User was deleted" }) + @ExceptionResponse(NotFoundException, { description: "User with that UUID not found" }) + @ExceptionResponse(UnauthorizedException, { description: "User is not authorized to delete this user" }) + async delete(@Param() { uuid }: SingleResourceDto) { + const user = await User.findOne({ where: { uuid }, attributes: ["id", "uuid"] }); + if (user == null) throw new NotFoundException(); + + await this.policyService.authorize("delete", user); + + await this.usersService.delete(user); + + return buildDeletedResponse(getDtoType(UserDto), uuid); } @Post() - @NoBearerAuth + @OptionalBearerAuth @ApiOperation({ operationId: "userCreation", description: "Create a new user" }) @JsonApiResponse(USER_RESPONSE_SHAPE) @ExceptionResponse(UnauthorizedException, { description: "user creation failed." }) - async create(@Body() payload: UserCreateBody) { - const user = await this.userCreationService.createNewUser(payload.data.attributes); + async create(@Body() payload: UserCreateBaseBody) { + const isAuthenticated = authenticatedUserId() != null; + if (isAuthenticated) { + await this.policyService.authorize("create", User); + } + const user = await this.userCreationService.createNewUser(isAuthenticated, payload.data.attributes); return await this.addUserResource(buildJsonApi(UserDto), user); } @Patch("verifyUser/:uuid") @@ -137,7 +185,10 @@ export class UsersController { } private async addUserResource(document: DocumentBuilder, user: User) { - const userResource = document.addData(user.uuid ?? "no-uuid", new UserDto(user, await user.myFrameworks())); + const userResource = document.addData( + user.uuid ?? "no-uuid", + new UserDto(user, user.frameworks, await user.myFrameworks()) + ); const org = await user.primaryOrganisation(); if (org != null) { diff --git a/apps/user-service/src/users/users.service.spec.ts b/apps/user-service/src/users/users.service.spec.ts new file mode 100644 index 000000000..420f06ba7 --- /dev/null +++ b/apps/user-service/src/users/users.service.spec.ts @@ -0,0 +1,352 @@ +import { BadRequestException, NotFoundException } from "@nestjs/common"; +import { Op } from "sequelize"; +import { UsersService } from "./users.service"; +import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; +import { + Framework, + FrameworkUser, + ModelHasRole, + Organisation, + Role, + User +} from "@terramatch-microservices/database/entities"; +import { UserQueryDto } from "./dto/user-query.dto"; +import { DocumentBuilder } from "@terramatch-microservices/common/util"; +import { UserUpdateAttributes } from "./dto/user-update.dto"; + +describe("UsersService", () => { + let service: UsersService; + + const createBuilderMock = () => ({ + where: jest.fn(), + order: jest.fn(), + execute: jest.fn().mockResolvedValue([{ id: 1 }]), + paginationTotal: jest.fn().mockResolvedValue(42) + }); + + beforeEach(() => { + service = new UsersService(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe("findMany", () => { + it("should return users and pagination total", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + const query = { page: 1 } as UserQueryDto; + const result = await service.findMany(query); + + expect(PaginatedQueryBuilder.forNumberPage).toHaveBeenCalledWith( + User, + query.page, + expect.arrayContaining([ + expect.objectContaining({ association: "organisation" }), + expect.objectContaining({ association: "roles" }) + ]) + ); + expect(builder.order).toHaveBeenCalledWith(["createdAt", "DESC"]); + expect(result).toEqual({ + users: [{ id: 1 }], + paginationTotal: 42 + }); + }); + + it("should apply search filter when search is provided", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ page: 1, search: " john " } as UserQueryDto); + + expect(builder.where).toHaveBeenCalledWith({ + [Op.or]: [ + { emailAddress: { [Op.like]: "%john%" } }, + { firstName: { [Op.like]: "%john%" } }, + { lastName: { [Op.like]: "%john%" } }, + { "$organisation.name$": { [Op.like]: "%john%" } } + ] + }); + }); + + it("should ignore blank search values", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ page: 1, search: " " } as UserQueryDto); + + expect(builder.where).not.toHaveBeenCalledWith( + expect.objectContaining({ + [Op.or]: expect.anything() + }) + ); + }); + + it("should filter verified users when isVerified is true", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ page: 1, isVerified: true } as UserQueryDto); + + expect(builder.where).toHaveBeenCalledWith({ + emailAddressVerifiedAt: { + [Op.ne]: null + } + }); + }); + + it("should filter unverified users when isVerified is false", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ page: 1, isVerified: false } as UserQueryDto); + + expect(builder.where).toHaveBeenCalledWith({ + emailAddressVerifiedAt: { + [Op.eq]: null + } + }); + }); + + it("should sort by direct fields", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ + page: 1, + sort: { field: "firstName", direction: "ASC" } + } as UserQueryDto); + + expect(builder.order).toHaveBeenCalledWith(["firstName", "ASC"]); + }); + + it("should sort by organisation name", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ + page: 1, + sort: { field: "organisationName", direction: "DESC" } + } as UserQueryDto); + + expect(builder.order).toHaveBeenCalledWith(["organisation", "name", "DESC"]); + }); + + it("should default sort direction to DESC when direction is missing", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ + page: 1, + sort: { field: "emailAddress" } + } as UserQueryDto); + + expect(builder.order).toHaveBeenCalledWith(["emailAddress", "DESC"]); + }); + + it("should allow id sort field without adding explicit order", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ + page: 1, + sort: { field: "id", direction: "ASC" } + } as UserQueryDto); + + expect(builder.order).not.toHaveBeenCalled(); + }); + + it("should throw for invalid sort field", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await expect( + service.findMany({ + page: 1, + sort: { field: "invalidField", direction: "ASC" } + } as UserQueryDto) + ).rejects.toThrow(new BadRequestException("Invalid sort field: invalidField")); + }); + }); + + describe("addUsersToDocument", () => { + it("should add each user with uuid key", async () => { + const document = { + addData: jest.fn() + } as unknown as DocumentBuilder; + + const users = [{ uuid: "user-1" } as User, { uuid: "user-2" } as User]; + + const result = await service.addUsersToDocument(document, users); + + expect(document.addData).toHaveBeenNthCalledWith(1, "user-1", expect.anything()); + expect(document.addData).toHaveBeenNthCalledWith(2, "user-2", expect.anything()); + expect(result).toBe(document); + }); + + it('should use "no-uuid" when user uuid is missing', async () => { + const document = { + addData: jest.fn() + } as unknown as DocumentBuilder; + + const users = [{ uuid: null } as unknown as User]; + await service.addUsersToDocument(document, users); + + expect(document.addData).toHaveBeenCalledWith("no-uuid", expect.anything()); + }); + }); + + describe("update", () => { + const createUserMock = () => { + const user = { + id: 99, + frameworks: [] as Framework[], + organisationId: null as number | null, + firstName: "Old", + lastName: "Name", + emailAddress: "old@example.com", + jobRole: "old-job" as string | null, + phoneNumber: "111" as string | null, + country: "US" as string | null, + program: "p1" as string | null, + locale: "en-US" as const, + save: jest.fn(), + reload: jest.fn() + }; + user.save.mockResolvedValue(user); + user.reload.mockResolvedValue(user); + return user as unknown as User; + }; + + it("should persist scalar field changes and return the user", async () => { + const user = createUserMock(); + const update: UserUpdateAttributes = { + firstName: "New", + lastName: "Person", + emailAddress: "new@example.com", + jobRole: "engineer", + phoneNumber: "555", + country: "CA", + program: "p2", + locale: "fr-FR" + }; + + const result = await service.update(user, update); + + expect(user.firstName).toBe("New"); + expect(user.lastName).toBe("Person"); + expect(user.emailAddress).toBe("new@example.com"); + expect(user.jobRole).toBe("engineer"); + expect(user.phoneNumber).toBe("555"); + expect(user.country).toBe("CA"); + expect(user.program).toBe("p2"); + expect(user.locale).toBe("fr-FR"); + expect(user.save).toHaveBeenCalledTimes(1); + expect(result).toBe(user); + }); + + it("should throw when organisation uuid does not exist", async () => { + const user = createUserMock(); + jest.spyOn(Organisation, "findOne").mockResolvedValue(null); + + await expect(service.update(user, { organisationUuid: "00000000-0000-0000-0000-000000000001" })).rejects.toThrow( + new NotFoundException("Organisation not found") + ); + + expect(Organisation.findOne).toHaveBeenCalledWith({ + where: { uuid: "00000000-0000-0000-0000-000000000001" } + }); + expect(user.save).not.toHaveBeenCalled(); + }); + + it("should set organisationId when organisation exists", async () => { + const user = createUserMock(); + jest.spyOn(Organisation, "findOne").mockResolvedValue({ id: 7 } as Organisation); + + await service.update(user, { organisationUuid: "org-uuid" }); + + expect(user.organisationId).toBe(7); + expect(user.save).toHaveBeenCalled(); + }); + + it("should replace direct frameworks when directFrameworks is provided", async () => { + const user = createUserMock(); + jest.spyOn(FrameworkUser, "destroy").mockResolvedValue(0); + jest.spyOn(Framework, "findAll").mockResolvedValue([{ id: 10 } as Framework, { id: 20 } as Framework]); + const findOrCreateSpy = jest.spyOn(FrameworkUser, "findOrCreate").mockResolvedValue([{} as FrameworkUser, true]); + + await service.update(user, { directFrameworks: ["fw-a", "fw-b"] }); + + expect(Framework.findAll).toHaveBeenCalledWith({ where: { slug: ["fw-a", "fw-b"] } }); + expect(findOrCreateSpy).toHaveBeenNthCalledWith(1, { + where: { frameworkId: 10, userId: 99 } + }); + expect(findOrCreateSpy).toHaveBeenNthCalledWith(2, { + where: { frameworkId: 20, userId: 99 } + }); + }); + + it("should throw when a direct framework slug is unknown", async () => { + const user = createUserMock(); + jest.spyOn(Framework, "findAll").mockResolvedValue([]); + + await expect(service.update(user, { directFrameworks: ["missing"] })).rejects.toThrow( + new BadRequestException("One or more frameworks not found") + ); + }); + + it("should replace primary role when primaryRole is provided", async () => { + const user = createUserMock(); + const destroySpy = jest.spyOn(ModelHasRole, "destroy").mockResolvedValue(0); + jest.spyOn(Role, "findOne").mockResolvedValue({ id: 5 } as Role); + const findOrCreateSpy = jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); + + const result = await service.update(user, { primaryRole: "admin" }); + + expect(destroySpy).toHaveBeenCalledWith({ + where: { modelId: 99, modelType: User.LARAVEL_TYPE } + }); + expect(Role.findOne).toHaveBeenCalledWith({ where: { name: "admin" } }); + expect(findOrCreateSpy).toHaveBeenCalledWith({ + where: { modelId: 99, roleId: 5 }, + defaults: { modelId: 99, roleId: 5, modelType: User.LARAVEL_TYPE } as ModelHasRole + }); + expect(user.reload).toHaveBeenCalled(); + expect(result).toBe(user); + }); + + it("should throw when primary role does not exist", async () => { + const user = createUserMock(); + jest.spyOn(ModelHasRole, "destroy").mockResolvedValue(0); + jest.spyOn(Role, "findOne").mockResolvedValue(null); + + await expect(service.update(user, { primaryRole: "unknown-role" })).rejects.toThrow( + new NotFoundException("Role not found") + ); + }); + }); + + describe("delete", () => { + it("should call destroy on the given user", async () => { + const user = { + destroy: jest.fn().mockResolvedValue(undefined) + } as unknown as User; + + await service.delete(user); + + expect(user.destroy).toHaveBeenCalledTimes(1); + }); + + it("should propagate errors thrown by destroy", async () => { + const destroyError = new Error("delete failed"); + const user = { + destroy: jest.fn().mockRejectedValue(destroyError) + } as unknown as User; + + await expect(service.delete(user)).rejects.toThrow(destroyError); + expect(user.destroy).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts new file mode 100644 index 000000000..a8a48a99a --- /dev/null +++ b/apps/user-service/src/users/users.service.ts @@ -0,0 +1,151 @@ +import { BadRequestException, Injectable } from "@nestjs/common"; +import { Op } from "sequelize"; +import { Framework, FrameworkUser, ModelHasRole, Role, User } from "@terramatch-microservices/database/entities"; +import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; +import { UserQueryDto } from "./dto/user-query.dto"; +import { UserDto } from "@terramatch-microservices/common/dto"; +import { DocumentBuilder } from "@terramatch-microservices/common/util"; +import { UserUpdateAttributes } from "./dto/user-update.dto"; +import { ValidLocale } from "@terramatch-microservices/database/constants/locale"; +import { Organisation } from "@terramatch-microservices/database/entities/organisation.entity"; + +@Injectable() +export class UsersService { + async findMany(query: UserQueryDto) { + const includes = [ + { + association: "organisation", + attributes: ["id", "uuid", "name"] + }, + { + association: "roles", + attributes: ["name"] + }, + { + association: "frameworks", + attributes: ["id", "name", "slug"] + } + ]; + + const builder = PaginatedQueryBuilder.forNumberPage(User, query.page, includes); + + if (query.search != null && query.search.trim() !== "") { + const search = `%${query.search.trim()}%`; + builder.where({ + [Op.or]: [ + { emailAddress: { [Op.like]: search } }, + { firstName: { [Op.like]: search } }, + { lastName: { [Op.like]: search } }, + { "$organisation.name$": { [Op.like]: search } } + ] + }); + } + + if (query.isVerified != null) { + builder.where({ + emailAddressVerifiedAt: { + [query.isVerified ? Op.ne : Op.eq]: null + } + }); + } + + if (query.sort?.field != null) { + const sortField = query.sort.field; + const direction = query.sort.direction ?? "DESC"; + + const directFields = ["createdAt", "firstName", "lastName", "emailAddress", "lastLoggedInAt"]; + + if (directFields.includes(sortField)) { + builder.order([sortField, direction]); + } else if (sortField === "organisationName") { + builder.order(["organisation", "name", direction]); + } else if (sortField !== "id") { + throw new BadRequestException(`Invalid sort field: ${query.sort.field}`); + } + } else { + builder.order(["createdAt", "DESC"]); + } + + return { + users: await builder.execute(), + paginationTotal: await builder.paginationTotal() + }; + } + + async addUsersToDocument(document: DocumentBuilder, users: User[]) { + for (const user of users) { + const userFrameworks = typeof user.myFrameworks === "function" ? await user.myFrameworks() : []; + document.addData(user.uuid ?? "no-uuid", new UserDto(user, user.frameworks, userFrameworks)); + } + return document; + } + + async update(user: User, update: UserUpdateAttributes) { + let organisationEntity: Organisation | null = null; + let frameworkEntities: Framework[] = []; + let roleEntity: Role | null = null; + if (update.organisationUuid != null) { + organisationEntity = await Organisation.findOne({ where: { uuid: update.organisationUuid } }); + if (organisationEntity == null) { + throw new BadRequestException("Organisation not found"); + } + } + if (update.directFrameworks != null) { + frameworkEntities = await Framework.findAll({ where: { slug: update.directFrameworks } }); + if (frameworkEntities.length !== update.directFrameworks.length) { + throw new BadRequestException("One or more frameworks not found"); + } + } + if (update.primaryRole != null) { + roleEntity = await Role.findOne({ where: { name: update.primaryRole } }); + if (roleEntity == null) { + throw new BadRequestException("Role not found"); + } + } + + if (update.directFrameworks != null) { + const userPreviousFrameworks = user.frameworks ?? []; + const frameworksToDelete = userPreviousFrameworks.filter( + framework => !frameworkEntities.some(entity => entity.id === framework.id) + ); + const frameworksToAdd = frameworkEntities.filter( + framework => !userPreviousFrameworks.some(previousFramework => previousFramework.id === framework.id) + ); + + if (frameworksToDelete.length > 0) { + await FrameworkUser.destroy({ + where: { userId: user.id, frameworkId: { [Op.in]: frameworksToDelete.map(framework => framework.id) } } + }); + } + for (const framework of frameworksToAdd) { + await FrameworkUser.findOrCreate({ where: { userId: user.id, frameworkId: framework.id } }); + } + } + + user.organisationId = organisationEntity?.id ?? user.organisationId; + user.firstName = update.firstName ?? user.firstName; + user.lastName = update.lastName ?? user.lastName; + user.emailAddress = update.emailAddress ?? user.emailAddress; + user.jobRole = update.jobRole ?? user.jobRole; + user.phoneNumber = update.phoneNumber ?? user.phoneNumber; + user.country = update.country ?? user.country; + user.program = update.program ?? user.program; + user.locale = (update.locale as ValidLocale | undefined) ?? user.locale; + + user = await user.save(); + + if (roleEntity != null) { + await ModelHasRole.destroy({ where: { modelId: user.id, modelType: User.LARAVEL_TYPE } }); + await ModelHasRole.findOrCreate({ + where: { modelId: user.id, roleId: roleEntity?.id }, + defaults: { modelId: user.id, roleId: roleEntity?.id, modelType: User.LARAVEL_TYPE } as ModelHasRole + }); + await user.reload(); + } + return user; + } + + async delete(user: User): Promise { + await user.destroy(); + } +} diff --git a/libs/common/src/lib/dto/user.dto.ts b/libs/common/src/lib/dto/user.dto.ts index 0efc33342..73193593a 100644 --- a/libs/common/src/lib/dto/user.dto.ts +++ b/libs/common/src/lib/dto/user.dto.ts @@ -14,10 +14,15 @@ class UserFramework { @JsonApiDto({ type: "users" }) export class UserDto { - constructor(user: User, frameworks: Framework[]) { + constructor(user: User, directFrameworks: Framework[], frameworks: Framework[]) { populateDto>(this, user, { uuid: user.uuid ?? "", - frameworks: frameworks + organisationName: user.organisation?.name ?? null, + organisationUuid: user.organisation?.uuid ?? null, + frameworks: (frameworks ?? []) + .filter(({ slug }) => slug != null) + .map(({ name, slug }) => ({ name, slug })) as UserFramework[], + directFrameworks: (directFrameworks ?? []) .filter(({ slug }) => slug != null) .map(({ name, slug }) => ({ name, slug })) as UserFramework[] }); @@ -48,9 +53,36 @@ export class UserDto { @ApiProperty({ nullable: true, type: Date }) emailAddressVerifiedAt: Date | null; + @ApiProperty({ nullable: true, type: String }) + phoneNumber: string | null; + + @ApiProperty({ nullable: true, type: String, description: "Name of the user's primary organisation, if any." }) + organisationName: string | null; + + @ApiProperty({ nullable: true, type: String, description: "UUID of the user's primary organisation, if any." }) + organisationUuid: string | null; + + @ApiProperty({ type: Date }) + createdAt: Date; + + @ApiProperty({ nullable: true, type: Date }) + lastLoggedInAt: Date | null; + + @ApiProperty({ nullable: true, type: String }) + jobRole: string | null; + + @ApiProperty({ nullable: true, type: String }) + country: string | null; + + @ApiProperty({ nullable: true, type: String }) + program: string | null; + @ApiProperty({ nullable: true, type: String }) locale: string | null; @ApiProperty({ type: () => UserFramework, isArray: true }) frameworks: UserFramework[]; + + @ApiProperty({ type: () => UserFramework, isArray: true }) + directFrameworks: UserFramework[]; } diff --git a/libs/common/src/lib/guards/auth.guard.spec.ts b/libs/common/src/lib/guards/auth.guard.spec.ts index 7b580668e..d76f8a22d 100644 --- a/libs/common/src/lib/guards/auth.guard.spec.ts +++ b/libs/common/src/lib/guards/auth.guard.spec.ts @@ -1,4 +1,4 @@ -import { AuthGuard, NoBearerAuth } from "./auth.guard"; +import { AuthGuard, NoBearerAuth, OptionalBearerAuth } from "./auth.guard"; import { Test } from "@nestjs/testing"; import { APP_GUARD } from "@nestjs/core"; import { createMock, DeepMocked } from "@golevelup/ts-jest"; @@ -19,6 +19,12 @@ class TestController { noAuth() { return "no-auth"; } + + @OptionalBearerAuth + @Get("/optional-auth") + optionalAuth() { + return "optional-auth"; + } } describe("AuthGuard", () => { @@ -79,4 +85,27 @@ describe("AuthGuard", () => { .set("Authorization", "Bearer foobar") .expect(HttpStatus.UNAUTHORIZED); }); + + it("should allow missing auth on an endpoint with @OptionalBearerAuth", async () => { + await request(app.getHttpServer()).get("/test/optional-auth").expect(HttpStatus.OK); + }); + + it("should allow invalid auth on an endpoint with @OptionalBearerAuth", async () => { + jwtService.verifyAsync.mockRejectedValue(new Error("Invalid token")); + + await request(app.getHttpServer()) + .get("/test/optional-auth") + .set("Authorization", "Bearer invalid-token") + .expect(HttpStatus.OK); + }); + + it("should allow valid auth on an endpoint with @OptionalBearerAuth", async () => { + const token = "valid-token"; + jwtService.verifyAsync.mockResolvedValue({ sub: "fakeuserid" }); + + await request(app.getHttpServer()) + .get("/test/optional-auth") + .set("Authorization", `Bearer ${token}`) + .expect(HttpStatus.OK); + }); }); diff --git a/libs/common/src/lib/guards/auth.guard.ts b/libs/common/src/lib/guards/auth.guard.ts index 9f0e9f677..b8505732d 100644 --- a/libs/common/src/lib/guards/auth.guard.ts +++ b/libs/common/src/lib/guards/auth.guard.ts @@ -7,6 +7,9 @@ import { RequestContext } from "nestjs-request-context"; const NO_BEARER_AUTH = "noBearerAuth"; export const NoBearerAuth = SetMetadata(NO_BEARER_AUTH, true); +const OPTIONAL_BEARER_AUTH = "optionalBearerAuth"; +export const OptionalBearerAuth = SetMetadata(OPTIONAL_BEARER_AUTH, true); + export const authenticatedUserId = () => RequestContext.currentContext?.req?.authenticatedUserId as number | undefined; @Injectable() @@ -20,12 +23,23 @@ export class AuthGuard implements CanActivate { ]); if (skipAuth) return true; + const optionalAuth = this.reflector.getAllAndOverride(OPTIONAL_BEARER_AUTH, [ + context.getHandler(), + context.getClass() + ]); + const request = context.switchToHttp().getRequest(); const [type, token] = request.headers.authorization?.split(" ") ?? []; - if (type !== "Bearer" || token == null) throw new UnauthorizedException(); + if (type !== "Bearer" || token == null) { + if (optionalAuth) return true; + throw new UnauthorizedException(); + } const userId = this.isJwtToken(token) ? await this.getJwtUserId(token) : await this.getApiKeyUserId(token); - if (userId == null) throw new UnauthorizedException(); + if (userId == null) { + if (optionalAuth) return true; + throw new UnauthorizedException(); + } // Most requests won't need the actual user object; instead, the roles and permissions // are fetched from other (smaller) tables, and only the user id is needed. diff --git a/libs/common/src/lib/guards/index.ts b/libs/common/src/lib/guards/index.ts index c3b34854c..c4bb23da4 100644 --- a/libs/common/src/lib/guards/index.ts +++ b/libs/common/src/lib/guards/index.ts @@ -1 +1 @@ -export { AuthGuard, NoBearerAuth } from "./auth.guard"; +export { AuthGuard, NoBearerAuth, OptionalBearerAuth } from "./auth.guard"; diff --git a/libs/common/src/lib/policies/user.policy.spec.ts b/libs/common/src/lib/policies/user.policy.spec.ts index 890dc5c4a..74c58888a 100644 --- a/libs/common/src/lib/policies/user.policy.spec.ts +++ b/libs/common/src/lib/policies/user.policy.spec.ts @@ -27,6 +27,21 @@ describe("UserPolicy", () => { await expectCan(service, "read", new User()); }); + it("allows reading all users as admin", async () => { + mockPermissions("users-manage"); + await expectCan(service, "readAll", User); + }); + + it("allows creating users as admin", async () => { + mockPermissions("users-manage"); + await expectCan(service, "create", User); + }); + + it("disallows creating users as non-admin", async () => { + mockPermissions(); + await expectCannot(service, "create", User); + }); + it("disallows reading other users as non-admin", async () => { mockPermissions(); await expectCannot(service, "read", new User()); @@ -78,4 +93,82 @@ describe("UserPolicy", () => { await expectCan(service, "verify", new User()); }); + + it("allows reading all users for verified admin without users-manage", async () => { + mockPermissions(); + + const verifiedAdminUser = new User(); + (verifiedAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = new Date(); + (verifiedAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "admin-terrafund" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(verifiedAdminUser); + + await expectCan(service, "readAll", User); + }); + + it("allows updating any user for verified admin without users-manage", async () => { + mockPermissions(); + + const verifiedAdminUser = new User(); + (verifiedAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = new Date(); + (verifiedAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "admin-terrafund" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(verifiedAdminUser); + + await expectCan(service, "update", new User()); + }); + + it("disallows verify any user when admin role is unverified", async () => { + mockPermissions(); + + const unverifiedAdminUser = new User(); + (unverifiedAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = null; + (unverifiedAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "admin-terrafund" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(unverifiedAdminUser); + + await expectCannot(service, "verify", await UserFactory.build({ id: 999 })); + }); + + it("disallows verify any user when verified user has no admin role", async () => { + mockPermissions(); + + const verifiedNonAdminUser = new User(); + (verifiedNonAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = new Date(); + (verifiedNonAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "project-manager" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(verifiedNonAdminUser); + + await expectCannot(service, "verify", await UserFactory.build({ id: 999 })); + }); + + it("disallows verify any user when authenticated user is not found", async () => { + mockPermissions(); + jest.spyOn(User, "findOne").mockResolvedValue(null); + + await expectCannot(service, "verify", await UserFactory.build({ id: 999 })); + }); + + it("allows deleting any user as users-manage", async () => { + mockPermissions("users-manage"); + await expectCan(service, "delete", new User()); + }); + + it("disallows deleting users as non-admin", async () => { + mockPermissions(); + await expectCannot(service, "delete", new User()); + await expectCannot(service, "delete", await UserFactory.build({ id: 123 })); + }); + + it("allows deleting any user for verified admin without users-manage", async () => { + mockPermissions(); + + const verifiedAdminUser = new User(); + (verifiedAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = new Date(); + (verifiedAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "admin-terrafund" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(verifiedAdminUser); + + await expectCan(service, "delete", new User()); + }); }); diff --git a/libs/common/src/lib/policies/user.policy.ts b/libs/common/src/lib/policies/user.policy.ts index c985e2407..c233f01c8 100644 --- a/libs/common/src/lib/policies/user.policy.ts +++ b/libs/common/src/lib/policies/user.policy.ts @@ -4,19 +4,14 @@ import { UserPermissionsPolicy } from "./user-permissions.policy"; export class UserPolicy extends UserPermissionsPolicy { async addRules() { if (this.permissions.includes("users-manage")) { - this.builder.can("read", User); - this.builder.can("update", User); - this.builder.can("verify", User); - } else { - this.builder.can("read", User, { id: this.userId }); - this.builder.can("update", User, { id: this.userId }); + this.builder.can(["read", "readAll", "create", "update", "verify", "delete"], User); } if (await this.isVerifiedAdmin()) { - this.builder.can("verify", User); + this.builder.can(["readAll", "update", "verify", "delete"], User); } - this.builder.can("verify", User, { id: this.userId }); + this.builder.can(["read", "update", "verify"], User, { id: this.userId }); } protected _isVerifiedAdmin?: boolean;