diff --git a/apps/user-service/jest.config.ts b/apps/user-service/jest.config.ts index 6ea34375..919cf8b4 100644 --- a/apps/user-service/jest.config.ts +++ b/apps/user-service/jest.config.ts @@ -1,11 +1,12 @@ /* eslint-disable */ export default { - displayName: 'user-service', - preset: '../../jest.preset.js', - testEnvironment: 'node', + displayName: "user-service", + preset: "../../jest.preset.js", + testEnvironment: "node", transform: { - '^.+\\.[tj]s$': ['ts-jest', { tsconfig: '/tsconfig.spec.json' }], + "^.+\\.[tj]s$": ["ts-jest", { tsconfig: "/tsconfig.spec.json" }] }, - moduleFileExtensions: ['ts', 'js', 'html'], - coverageDirectory: '../../coverage/apps/user-service', + moduleFileExtensions: ["ts", "js", "html"], + coveragePathIgnorePatterns: [".dto.ts"], + coverageDirectory: "../../coverage/apps/user-service" }; diff --git a/apps/user-service/src/users/dto/user-update.dto.ts b/apps/user-service/src/users/dto/user-update.dto.ts new file mode 100644 index 00000000..d8f95a74 --- /dev/null +++ b/apps/user-service/src/users/dto/user-update.dto.ts @@ -0,0 +1,33 @@ +import { Equals, IsEnum, IsUUID, ValidateNested } from "class-validator"; +import { ApiProperty } from "@nestjs/swagger"; +import { Type } from "class-transformer"; + +const VALID_LOCALES = ["en-US", "es-MX", "fr-FR", "pt-BR"]; + +class UserUpdateAttributes { + @IsEnum(VALID_LOCALES) + @ApiProperty({ description: "New default locale for the given user", nullable: true, enum: VALID_LOCALES }) + locale?: string | null; +} + +class UserUpdate { + @Equals("users") + @ApiProperty({ enum: ["users"] }) + type: string; + + @IsUUID() + @ApiProperty({ format: "uuid" }) + id: string; + + @ValidateNested() + @Type(() => UserUpdateAttributes) + @ApiProperty({ type: () => UserUpdateAttributes }) + attributes: UserUpdateAttributes; +} + +export class UserUpdateBodyDto { + @ValidateNested() + @Type(() => UserUpdate) + @ApiProperty({ type: () => UserUpdate }) + data: UserUpdate; +} diff --git a/apps/user-service/src/users/users.controller.spec.ts b/apps/user-service/src/users/users.controller.spec.ts index d19a9a66..632bcdf2 100644 --- a/apps/user-service/src/users/users.controller.spec.ts +++ b/apps/user-service/src/users/users.controller.spec.ts @@ -2,7 +2,7 @@ import { Test, TestingModule } from "@nestjs/testing"; import { UsersController } from "./users.controller"; import { PolicyService } from "@terramatch-microservices/common"; import { createMock, DeepMocked } from "@golevelup/ts-jest"; -import { NotFoundException, UnauthorizedException } from "@nestjs/common"; +import { BadRequestException, NotFoundException, UnauthorizedException } from "@nestjs/common"; import { OrganisationFactory, UserFactory } from "@terramatch-microservices/database/factories"; import { Relationship, Resource } from "@terramatch-microservices/common/util"; @@ -23,58 +23,106 @@ describe("UsersController", () => { jest.restoreAllMocks(); }); - it("should throw not found if the user is not found", async () => { - await expect(controller.findOne("0", { authenticatedUserId: 1 })).rejects.toThrow(NotFoundException); - }); + describe("findOne", () => { + it("should throw not found if the user is not found", async () => { + await expect(controller.findOne("0", { authenticatedUserId: 1 })).rejects.toThrow(NotFoundException); + }); - it("should throw an error if the policy does not authorize", async () => { - policyService.authorize.mockRejectedValue(new UnauthorizedException()); - const { id } = await UserFactory.create(); - await expect(controller.findOne(`${id}`, { authenticatedUserId: 1 })).rejects.toThrow(UnauthorizedException); - }); + it("should throw an error if the policy does not authorize", async () => { + policyService.authorize.mockRejectedValue(new UnauthorizedException()); + const { uuid } = await UserFactory.create(); + await expect(controller.findOne(uuid, { authenticatedUserId: 1 })).rejects.toThrow(UnauthorizedException); + }); - it('should return the currently logged in user if the id is "me"', async () => { - const { id, uuid } = await UserFactory.create(); - const result = await controller.findOne("me", { authenticatedUserId: id }); - expect((result.data as Resource).id).toBe(uuid); - }); + it('should return the currently logged in user if the id is "me"', async () => { + const { id, uuid } = await UserFactory.create(); + const result = await controller.findOne("me", { authenticatedUserId: id }); + expect((result.data as Resource).id).toBe(uuid); + }); - it("should return the indicated user if the logged in user is allowed to access", async () => { - policyService.authorize.mockResolvedValue(undefined); - const { id, uuid } = await UserFactory.create(); - const result = await controller.findOne(`${id}`, { authenticatedUserId: id + 1 }); - expect((result.data as Resource).id).toBe(uuid); - }); + it("should return the indicated user if the logged in user is allowed to access", async () => { + policyService.authorize.mockResolvedValue(undefined); + const { id, uuid } = await UserFactory.create(); + const result = await controller.findOne(uuid, { authenticatedUserId: id + 1 }); + expect((result.data as Resource).id).toBe(uuid); + }); - it("should return a document without includes if there is no org", async () => { - const { id } = await UserFactory.create(); - const result = await controller.findOne("me", { authenticatedUserId: id }); - expect(result.included).not.toBeDefined(); - }); + it("should return a document without includes if there is no org", async () => { + const { id } = await UserFactory.create(); + const result = await controller.findOne("me", { authenticatedUserId: id }); + expect(result.included).not.toBeDefined(); + }); + + it("should include the primary org for the user", async () => { + const user = await UserFactory.create(); + const org = await OrganisationFactory.create(); + await user.$add("organisationsConfirmed", org); + const result = await controller.findOne("me", { authenticatedUserId: user.id }); + expect(result.included).toHaveLength(1); + expect(result.included[0]).toMatchObject({ type: "organisations", id: org.uuid }); + const data = result.data as Resource; + expect(data.relationships.org).toBeDefined(); + const relationship = data.relationships.org.data as Relationship; + expect(relationship).toMatchObject({ + type: "organisations", + id: org.uuid, + meta: { userStatus: "approved" } + }); + }); - it("should include the primary org for the user", async () => { - const user = await UserFactory.create(); - const org = await OrganisationFactory.create(); - await user.$add("organisationsConfirmed", org); - const result = await controller.findOne("me", { authenticatedUserId: user.id }); - expect(result.included).toHaveLength(1); - expect(result.included[0]).toMatchObject({ type: "organisations", id: org.uuid }); - const data = result.data as Resource; - expect(data.relationships.org).toBeDefined(); - const relationship = data.relationships.org.data as Relationship; - expect(relationship).toMatchObject({ type: "organisations", id: org.uuid, meta: { userStatus: "approved" } }); + it('should return "na" for userStatus if there is no many to many relationship', async () => { + const user = await UserFactory.create(); + const org = await OrganisationFactory.create(); + await user.$set("organisation", org); + const result = await controller.findOne("me", { authenticatedUserId: user.id }); + expect(result.included).toHaveLength(1); + expect(result.included[0]).toMatchObject({ type: "organisations", id: org.uuid }); + const data = result.data as Resource; + expect(data.relationships.org).toBeDefined(); + const relationship = data.relationships.org.data as Relationship; + expect(relationship).toMatchObject({ + type: "organisations", + id: org.uuid, + meta: { userStatus: "na" } + }); + }); }); - it('should return "na" for userStatus if there is no many to many relationship', async () => { - const user = await UserFactory.create(); - const org = await OrganisationFactory.create(); - await user.$set("organisation", org); - const result = await controller.findOne("me", { authenticatedUserId: user.id }); - expect(result.included).toHaveLength(1); - expect(result.included[0]).toMatchObject({ type: "organisations", id: org.uuid }); - const data = result.data as Resource; - expect(data.relationships.org).toBeDefined(); - const relationship = data.relationships.org.data as Relationship; - expect(relationship).toMatchObject({ type: "organisations", id: org.uuid, meta: { userStatus: "na" } }); + describe("update", () => { + const makeValidBody = (uuid: string, locale?: string) => ({ + data: { + id: uuid, + type: "users", + attributes: { locale } + } + }); + + beforeEach(async () => { + policyService.authorize.mockResolvedValue(undefined); + }); + + it("should throw if the body and path UUIDs don't match", async () => { + await expect(controller.update("foo", makeValidBody("bar"))).rejects.toThrow(BadRequestException); + }); + + it("should throw if the user is not found", async () => { + await expect(controller.update("foo", makeValidBody("foo"))).rejects.toThrow(NotFoundException); + }); + + it("update the user with a new locale", async () => { + const user = await UserFactory.create(); + const result = await controller.update(user.uuid, makeValidBody(user.uuid, "es-MX")); + expect((result.data as Resource).attributes.locale).toEqual("es-MX"); + await user.reload(); + expect(user.locale).toEqual("es-MX"); + }); + + it("does not change the locale if it's missing in the update request", async () => { + const user = await UserFactory.create({ locale: "es-MX" }); + const result = await controller.update(user.uuid, makeValidBody(user.uuid)); + expect((result.data as Resource).attributes.locale).toEqual("es-MX"); + await user.reload(); + expect(user.locale).toEqual("es-MX"); + }); }); }); diff --git a/apps/user-service/src/users/users.controller.ts b/apps/user-service/src/users/users.controller.ts index 2cc64ced..da21850c 100644 --- a/apps/user-service/src/users/users.controller.ts +++ b/apps/user-service/src/users/users.controller.ts @@ -1,76 +1,113 @@ import { + BadRequestException, + Body, Controller, Get, NotFoundException, Param, + Patch, Request, - UnauthorizedException, -} from '@nestjs/common'; -import { User } from '@terramatch-microservices/database/entities'; -import { PolicyService } from '@terramatch-microservices/common'; -import { ApiOperation, ApiParam } from '@nestjs/swagger'; -import { OrganisationDto, UserDto } from '@terramatch-microservices/common/dto'; -import { ApiException } from '@nanogiants/nestjs-swagger-api-exception-decorator'; -import { JsonApiResponse } from '@terramatch-microservices/common/decorators'; -import { - buildJsonApi, - JsonApiDocument, -} from '@terramatch-microservices/common/util'; + UnauthorizedException +} from "@nestjs/common"; +import { User } from "@terramatch-microservices/database/entities"; +import { PolicyService } from "@terramatch-microservices/common"; +import { ApiOperation, ApiParam } from "@nestjs/swagger"; +import { OrganisationDto, UserDto } from "@terramatch-microservices/common/dto"; +import { ApiException } from "@nanogiants/nestjs-swagger-api-exception-decorator"; +import { JsonApiResponse } from "@terramatch-microservices/common/decorators"; +import { buildJsonApi, DocumentBuilder } from "@terramatch-microservices/common/util"; +import { UserUpdateBodyDto } from "./dto/user-update.dto"; + +const USER_RESPONSE_SHAPE = { + data: { + type: UserDto, + relationships: [ + { + name: "org", + type: OrganisationDto, + meta: { + userStatus: { + type: "string", + enum: ["approved", "requested", "rejected", "na"] + } + } + } + ] + }, + included: [{ type: OrganisationDto }] +}; -@Controller('users/v3/users') +@Controller("users/v3/users") export class UsersController { constructor(private readonly policyService: PolicyService) {} - @Get(':id') - @ApiOperation({ operationId: 'usersFind', description: "Fetch a user by ID, or with the 'me' identifier" }) - @ApiParam({ name: 'id', example: 'me', description: 'A valid user id or "me"' }) - @JsonApiResponse({ - data: { - type: UserDto, - relationships: [ - { - name: 'org', - type: OrganisationDto, - meta: { - userStatus: { - type: 'string', - enum: ['approved', 'requested', 'rejected', 'na'], - }, - }, - }, - ], - }, - included: [{ type: OrganisationDto }], + @Get(":uuid") + @ApiOperation({ operationId: "usersFind", description: "Fetch a user by ID, or with the 'me' identifier" }) + @ApiParam({ name: "uuid", example: "me", description: 'A valid user uuid or "me"' }) + @JsonApiResponse(USER_RESPONSE_SHAPE) + @ApiException(() => UnauthorizedException, { + description: "Authorization failed" + }) + @ApiException(() => NotFoundException, { + description: "User with that ID not found" }) + async findOne(@Param("uuid") pathId: string, @Request() { authenticatedUserId }) { + const userWhere = pathId === "me" ? { id: authenticatedUserId } : { uuid: pathId }; + const user = await User.findOne({ + include: ["roles", "organisation", "frameworks"], + where: userWhere + }); + if (user == null) throw new NotFoundException(); + + await this.policyService.authorize("read", user); + + return (await this.addUserResource(buildJsonApi(), user)).serialize(); + } + + @Patch(":uuid") + @ApiOperation({ operationId: "userUpdate", description: "Update a user by ID" }) + @ApiParam({ name: "uuid", description: "A valid user uuid" }) + @JsonApiResponse(USER_RESPONSE_SHAPE) @ApiException(() => UnauthorizedException, { - description: 'Authorization failed', + description: "Authorization failed" }) @ApiException(() => NotFoundException, { - description: 'User with that ID not found', + description: "User with that ID not found" }) - async findOne( - @Param('id') pathId: string, - @Request() { authenticatedUserId } - ): Promise { - const userId = pathId === 'me' ? authenticatedUserId : parseInt(pathId); + @ApiException(() => BadRequestException, { description: "Something is malformed about the request" }) + async update(@Param("uuid") uuid: string, @Body() updatePayload: UserUpdateBodyDto) { + if (uuid !== updatePayload.data.id) { + throw new BadRequestException(`Path uuid and payload id do not match`); + } + const user = await User.findOne({ - include: ['roles', 'organisation', 'frameworks'], - where: { id: userId }, + include: ["roles", "organisation", "frameworks"], + where: { uuid } }); if (user == null) throw new NotFoundException(); - await this.policyService.authorize('read', user); + 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(); + } + + return (await this.addUserResource(buildJsonApi(), user)).serialize(); + } - const document = buildJsonApi(); + private async addUserResource(document: DocumentBuilder, user: User) { const userResource = document.addData(user.uuid, new UserDto(user, await user.myFrameworks())); const org = await user.primaryOrganisation(); if (org != null) { const orgResource = document.addIncluded(org.uuid, new OrganisationDto(org)); - const userStatus = org.OrganisationUser?.status ?? 'na'; - userResource.relateTo('org', orgResource, { userStatus }); + const userStatus = org.OrganisationUser?.status ?? "na"; + userResource.relateTo("org", orgResource, { userStatus }); } - return document.serialize(); + return document; } } diff --git a/libs/common/src/lib/dto/user.dto.ts b/libs/common/src/lib/dto/user.dto.ts index e45da5c7..4bfcc980 100644 --- a/libs/common/src/lib/dto/user.dto.ts +++ b/libs/common/src/lib/dto/user.dto.ts @@ -47,5 +47,4 @@ export class UserDto extends JsonApiAttributes { @ApiProperty({ type: () => UserFramework, isArray: true }) frameworks: UserFramework[]; - } diff --git a/libs/common/src/lib/policies/user.policy.spec.ts b/libs/common/src/lib/policies/user.policy.spec.ts index 3ad48896..88354117 100644 --- a/libs/common/src/lib/policies/user.policy.spec.ts +++ b/libs/common/src/lib/policies/user.policy.spec.ts @@ -37,4 +37,22 @@ describe("UserPolicy", () => { mockPermissions(); await expect(service.authorize("read", await UserFactory.build({ id: 123 }))).resolves.toBeUndefined(); }); + + it("allows updating any user as admin", async () => { + mockUserId(123); + mockPermissions("users-manage"); + await expect(service.authorize("update", new User())).resolves.toBeUndefined(); + }); + + it("disallows updating other users as non-admin", async () => { + mockUserId(123); + mockPermissions(); + await expect(service.authorize("update", new User())).rejects.toThrow(UnauthorizedException); + }); + + it("allows updating own user as non-admin", async () => { + mockUserId(123); + mockPermissions(); + await expect(service.authorize("update", await UserFactory.build({ id: 123 }))).resolves.toBeUndefined(); + }); }); diff --git a/libs/common/src/lib/policies/user.policy.ts b/libs/common/src/lib/policies/user.policy.ts index 4fd8ccee..fb30b93d 100644 --- a/libs/common/src/lib/policies/user.policy.ts +++ b/libs/common/src/lib/policies/user.policy.ts @@ -1,12 +1,14 @@ -import { User } from '@terramatch-microservices/database/entities'; -import { EntityPolicy } from './entity.policy'; +import { User } from "@terramatch-microservices/database/entities"; +import { EntityPolicy } from "./entity.policy"; export class UserPolicy extends EntityPolicy { async addRules() { - if (this.permissions.includes('users-manage')) { - this.builder.can('read', User); + if (this.permissions.includes("users-manage")) { + this.builder.can("read", User); + this.builder.can("update", User); } else { - this.builder.can('read', User, { id: this.userId }); + this.builder.can("read", User, { id: this.userId }); + this.builder.can("update", User, { id: this.userId }); } } } diff --git a/libs/common/src/lib/util/json-api-builder.ts b/libs/common/src/lib/util/json-api-builder.ts index e52e3ae9..856fb555 100644 --- a/libs/common/src/lib/util/json-api-builder.ts +++ b/libs/common/src/lib/util/json-api-builder.ts @@ -113,7 +113,7 @@ type DocumentBuilderOptions = { forceDataArray?: boolean; }; -class DocumentBuilder { +export class DocumentBuilder { data: ResourceBuilder[] = []; included: ResourceBuilder[] = [];