Add userProfile controller and PUT /userProfile/:id endpoint

CHANGELOG.md

@@ -14,6 +14,10 @@ All notable changes to this project will be documented in this file. The format
   - Add `isPartner` filter to GET /user/list endpoint ([#1830](https://github.com/bloom-housing/bloom/pull/1830))
   - Changes to applications done through `PUT /applications/:id` are now reflected in AFS ([#1810](https://github.com/bloom-housing/bloom/pull/1810))
   - Add logic for connecting newly created user account to existing applications (matching based on applicant.emailAddress) ([#1807](https://github.com/bloom-housing/bloom/pull/1807))
+  - Added new userProfile resource and endpoint `PUT /userProfile/:id` suited specifically for users updating their own profiles ([#1862](https://github.com/bloom-housing/bloom/pull/1862))
+
+- Changed:
+  - ** Breaking Change**: Endpoint `PUT /user/:id` is admin only now, because it allows edits over entire `user` table ([#1862](https://github.com/bloom-housing/bloom/pull/1862))
   - Changes to applications done through `PUT /applications/:id` are now reflected in AFS ([#1810](https://github.com/bloom-housing/bloom/pull/1810))
   - Adds confirmationCode to applications table ([#1854](https://github.com/bloom-housing/bloom/pull/1854))
 

backend/core/src/auth/auth.module.ts

@@ -17,6 +17,7 @@ import { EmailModule } from "../shared/email/email.module"
 import { PasswordService } from "./services/password.service"
 import { JurisdictionsModule } from "../jurisdictions/jurisdictions.module"
 import { Application } from "../applications/entities/application.entity"
+import { UserProfileController } from "./controllers/user-profile.controller"
 
 @Module({
   imports: [
@@ -38,6 +39,6 @@ import { Application } from "../applications/entities/application.entity"
   ],
   providers: [LocalStrategy, JwtStrategy, AuthService, AuthzService, UserService, PasswordService],
   exports: [AuthzService, AuthService, UserService],
-  controllers: [AuthController, UserController],
+  controllers: [AuthController, UserController, UserProfileController],
 })
 export class AuthModule {}

backend/core/src/auth/authz_policy.csv

@@ -3,7 +3,9 @@ p, user, application, !r.obj || (r.sub == r.obj.user_id), (read|submit)
 p, anonymous, application, true, submit
 
 p, admin, user, true, .*
-p, user, user, !r.obj || (r.sub == r.obj.id), (read|update)
+p, admin, userProfile, true, .*
+p, user, user, !r.obj || (r.sub == r.obj.id), read
+p, user, userProfile, !r.obj || (r.sub == r.obj.id), (read|update)
 p, anonymous, user, true, create
 
 p, admin, asset, true, .*

backend/core/src/auth/controllers/user-profile.controller.ts

@@ -0,0 +1,31 @@
+import { Body, Controller, Put, Request, UseGuards, UsePipes, ValidationPipe } from "@nestjs/common"
+import { ApiBearerAuth, ApiOperation, ApiTags } from "@nestjs/swagger"
+import { Request as ExpressRequest } from "express"
+import { ResourceType } from "../decorators/resource-type.decorator"
+import { defaultValidationPipeOptions } from "../../shared/default-validation-pipe-options"
+import { UserService } from "../services/user.service"
+import { AuthzGuard } from "../guards/authz.guard"
+import { UserDto } from "../dto/user.dto"
+import { mapTo } from "../../shared/mapTo"
+import { AuthContext } from "../types/auth-context"
+import { User } from "../entities/user.entity"
+import { DefaultAuthGuard } from "../guards/default.guard"
+import { UserProfileUpdateDto } from "../dto/user-profile.dto"
+
+@Controller("userProfile")
+@ApiBearerAuth()
+@ApiTags("userProfile")
+@ResourceType("userProfile")
+@UsePipes(new ValidationPipe(defaultValidationPipeOptions))
+export class UserProfileController {
+  constructor(private readonly userService: UserService) {}
+  @Put(":id")
+  @UseGuards(DefaultAuthGuard, AuthzGuard)
+  @ApiOperation({ summary: "Update profile user", operationId: "update" })
+  async update(
+    @Request() req: ExpressRequest,
+    @Body() dto: UserProfileUpdateDto
+  ): Promise<UserDto> {
+    return mapTo(UserDto, await this.userService.update(dto, new AuthContext(req.user as User)))
+  }
+}

backend/core/src/auth/controllers/user.controller.ts

@@ -38,6 +38,7 @@ import { LoginResponseDto } from "../dto/login-response.dto"
 import { authzActions } from "../enum/authz-actions.enum"
 import { UserCreateQueryParams } from "../dto/user-create-query-params"
 import { UserFilterParams } from "../dto/user-filter-params"
+import { DefaultAuthGuard } from "../guards/default.guard"
 
 @Controller("user")
 @ApiBearerAuth()
@@ -48,7 +49,7 @@ export class UserController {
   constructor(private readonly userService: UserService) {}
 
   @Get()
-  @UseGuards(OptionalAuthGuard, AuthzGuard)
+  @UseGuards(DefaultAuthGuard, AuthzGuard)
   profile(@Request() req): UserDto {
     return mapTo(UserDto, req.user)
   }
@@ -101,7 +102,7 @@ export class UserController {
   }
 
   @Put(":id")
-  @UseGuards(OptionalAuthGuard, AuthzGuard)
+  @UseGuards(DefaultAuthGuard, AuthzGuard)
   @ApiOperation({ summary: "Update user", operationId: "update" })
   async update(@Request() req: ExpressRequest, @Body() dto: UserUpdateDto): Promise<UserDto> {
     return mapTo(UserDto, await this.userService.update(dto, new AuthContext(req.user as User)))

backend/core/src/auth/dto/user-profile.dto.ts

@@ -0,0 +1,46 @@
+import { PickType } from "@nestjs/swagger"
+import { User } from "../entities/user.entity"
+import { Expose, Type } from "class-transformer"
+import {
+  IsDefined,
+  IsNotEmpty,
+  IsOptional,
+  IsString,
+  Matches,
+  ValidateIf,
+  ValidateNested,
+} from "class-validator"
+import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
+import { passwordRegex } from "../../shared/password-regex"
+import { IdDto } from "../../shared/dto/id.dto"
+
+export class UserProfileUpdateDto extends PickType(User, [
+  "id",
+  "firstName",
+  "middleName",
+  "lastName",
+  "dob",
+  "createdAt",
+  "updatedAt",
+  "language",
+] as const) {
+  @Expose()
+  @IsOptional({ groups: [ValidationsGroupsEnum.default] })
+  @IsString({ groups: [ValidationsGroupsEnum.default] })
+  @Matches(passwordRegex, {
+    message: "passwordTooWeak",
+    groups: [ValidationsGroupsEnum.default],
+  })
+  password?: string
+
+  @Expose()
+  @ValidateIf((o) => o.password, { groups: [ValidationsGroupsEnum.default] })
+  @IsNotEmpty({ groups: [ValidationsGroupsEnum.default] })
+  currentPassword?: string
+
+  @Expose()
+  @IsDefined({ groups: [ValidationsGroupsEnum.default] })
+  @ValidateNested({ groups: [ValidationsGroupsEnum.default], each: true })
+  @Type(() => IdDto)
+  jurisdictions: IdDto[]
+}

backend/core/src/auth/dto/user-update.dto.ts

@@ -3,6 +3,7 @@ import { Expose, Type } from "class-transformer"
 import {
   IsDate,
   IsDefined,
+  IsEmail,
   IsNotEmpty,
   IsOptional,
   IsString,
@@ -18,6 +19,7 @@ import { UserDto } from "./user.dto"
 
 export class UserUpdateDto extends OmitType(UserDto, [
   "id",
+  "email",
   "createdAt",
   "updatedAt",
   "leasingAgentInListings",
@@ -29,6 +31,11 @@ export class UserUpdateDto extends OmitType(UserDto, [
   @IsUUID(4, { groups: [ValidationsGroupsEnum.default] })
   id?: string
 
+  @Expose()
+  @IsOptional({ groups: [ValidationsGroupsEnum.default] })
+  @IsEmail({}, { groups: [ValidationsGroupsEnum.default] })
+  email?: string
+
   @Expose()
   @IsOptional({ groups: [ValidationsGroupsEnum.default] })
   @IsDate({ groups: [ValidationsGroupsEnum.default] })

backend/core/src/auth/guards/authz.guard.ts

@@ -25,6 +25,13 @@ export class AuthzGuard implements CanActivate {
       this.reflector.get<string>("authz_action", context.getHandler()) ||
       httpMethodsToAction[req.method]
 
-    return this.authzService.can(authUser, type, action)
+    let resource
+    if (req.params.id) {
+      // NOTE: implicit assumption that if request.params contains an ID it also means that body contains one too and it should be the same
+      //  This prevents a security hole where user specifies params.id different than dto.id to pass authorization but actually edit a different resource
+      resource = { id: req.body.id }
+    }
+
+    return this.authzService.can(authUser, type, action, resource)
   }
 }

backend/core/src/auth/services/user.service.ts

@@ -94,24 +94,14 @@ export class UserService {
     return result
   }
 
-  async update(dto: Partial<UserUpdateDto>, authContext: AuthContext) {
+  async update(dto: UserUpdateDto, authContext: AuthContext) {
     const user = await this.find({
       id: dto.id,
     })
     if (!user) {
       throw new NotFoundException()
     }
 
-    await this.authzService.canOrThrow(authContext.user, "user", authzActions.update, {
-      ...dto,
-    })
-
-    if (user.confirmedAt?.getTime() !== dto.confirmedAt?.getTime()) {
-      await this.authzService.canOrThrow(authContext.user, "user", authzActions.confirm, {
-        ...dto,
-      })
-    }
-
     let passwordHash
     if (dto.password) {
       if (!dto.currentPassword) {

backend/core/test/authz/authz.e2e-spec.ts

@@ -106,7 +106,7 @@ describe("Authz", () => {
 
   describe("user", () => {
     it(`should not allow anonymous user to GET to get any user profile`, async () => {
-      await supertest(app.getHttpServer()).get(userEndpoint).expect(403)
+      await supertest(app.getHttpServer()).get(userEndpoint).expect(401)
     })
 
     it(`should allow a logged in user to GET to get any user profile`, async () => {
@@ -225,7 +225,7 @@ describe("Authz", () => {
 
       expect(profileRes.body.roles).toBe(null)
       await supertest(app.getHttpServer())
-        .put(`/user/${profileRes.body.id}`)
+        .put(`/userProfile/${profileRes.body.id}`)
         .send({ ...profileRes.body, roles: { isAdmin: true, isPartner: false } })
         .set(...setAuthorization(userAccessToken))
         .expect(200)

backend/core/test/user/user.e2e-spec.ts

@@ -19,6 +19,8 @@ import { UserInviteDto } from "../../src/auth/dto/user-invite.dto"
 import { Listing } from "../../src/listings/entities/listing.entity"
 import { Repository } from "typeorm"
 import { Jurisdiction } from "../../src/jurisdictions/entities/jurisdiction.entity"
+import { UserProfileUpdateDto } from "../../src/auth/dto/user-profile.dto"
+import { Language } from "../../src/shared/types/language-enum"
 
 // Cypress brings in Chai types for the global expect, but we want to use jest
 // expect here so we need to re-declare it.
@@ -270,7 +272,7 @@ describe("Applications", () => {
     await supertest(app.getHttpServer())
       .put(`/user/${user2UpdateDto.id}`)
       .send(user2UpdateDto)
-      .expect(403)
+      .expect(401)
   })
 
   it("should allow user to resend confirmation", async () => {
@@ -398,4 +400,131 @@ describe("Applications", () => {
     const token = await getUserAccessToken(app, newUser.email, password)
     expect(token).toBeDefined()
   })
+
+  it("should allow user to update user profile throguh PUT /userProfile/:id endpoint", async () => {
+    const userCreateDto: UserCreateDto = {
+      password: "Abcdef1!",
+      passwordConfirmation: "Abcdef1!",
+      email: "userProfile@b.com",
+      emailConfirmation: "userProfile@b.com",
+      firstName: "First",
+      middleName: "Mid",
+      lastName: "Last",
+      dob: new Date(),
+      language: Language.en,
+    }
+
+    const userCreateResponse = await supertest(app.getHttpServer())
+      .post(`/user/`)
+      .set("jurisdictionName", "Alameda")
+      .send(userCreateDto)
+      .expect(201)
+
+    const userService = await app.resolve<UserService>(UserService)
+    const user = await userService.findByEmail(userCreateDto.email)
+
+    await supertest(app.getHttpServer())
+      .put(`/user/confirm/`)
+      .send({ token: user.confirmationToken })
+      .expect(200)
+
+    const userAccessToken = await getUserAccessToken(
+      app,
+      userCreateDto.email,
+      userCreateDto.password
+    )
+
+    const userProfileUpdateDto: UserProfileUpdateDto = {
+      id: userCreateResponse.body.id,
+      createdAt: userCreateResponse.body.createdAt,
+      updatedAt: userCreateResponse.body.updatedAt,
+      jurisdictions: userCreateResponse.body.jurisdictions,
+      ...userCreateDto,
+      currentPassword: userCreateDto.password,
+      firstName: "NewFirstName",
+    }
+
+    await supertest(app.getHttpServer())
+      .put(`/userProfile/${userCreateResponse.body.id}`)
+      .send(userProfileUpdateDto)
+      .expect(401)
+
+    const userProfileUpdateResponse = await supertest(app.getHttpServer())
+      .put(`/userProfile/${userCreateResponse.body.id}`)
+      .send(userProfileUpdateDto)
+      .set(...setAuthorization(userAccessToken))
+      .expect(200)
+    expect(userProfileUpdateResponse.body.firstName).toBe(userProfileUpdateDto.firstName)
+  })
+
+  it("should not allow user A to edit user B profile (with /userProfile)", async () => {
+    const createAndConfirmUser = async (createDto: UserCreateDto) => {
+      const userCreateResponse = await supertest(app.getHttpServer())
+        .post(`/user/`)
+        .set("jurisdictionName", "Alameda")
+        .send(createDto)
+        .expect(201)
+
+      const userService = await app.resolve<UserService>(UserService)
+      const user = await userService.findByEmail(createDto.email)
+
+      await supertest(app.getHttpServer())
+        .put(`/user/confirm/`)
+        .send({ token: user.confirmationToken })
+        .expect(200)
+
+      const accessToken = await getUserAccessToken(app, createDto.email, createDto.password)
+      return { accessToken, userId: userCreateResponse.body.id }
+    }
+
+    const userACreateDto: UserCreateDto = {
+      password: "Abcdef1!",
+      passwordConfirmation: "Abcdef1!",
+      email: "user-a@example.com",
+      emailConfirmation: "user-a@example.com",
+      firstName: "First",
+      middleName: "Mid",
+      lastName: "Last",
+      dob: new Date(),
+      language: Language.en,
+    }
+
+    const userBCreateDto: UserCreateDto = {
+      password: "Abcdef1!",
+      passwordConfirmation: "Abcdef1!",
+      email: "user-b@example.com",
+      emailConfirmation: "user-b@example.com",
+      firstName: "First",
+      middleName: "Mid",
+      lastName: "Last",
+      dob: new Date(),
+      language: Language.en,
+    }
+
+    const { userId: userAId } = await createAndConfirmUser(userACreateDto)
+    const { accessToken: userBAccessToken } = await createAndConfirmUser(userBCreateDto)
+
+    const userAProfileUpdateDto: UserProfileUpdateDto = {
+      id: userAId,
+      createdAt: new Date(),
+      updatedAt: new Date(),
+      ...userACreateDto,
+      password: undefined,
+      jurisdictions: [],
+    }
+
+    // Restrict user B editing user A's profile
+    await supertest(app.getHttpServer())
+      .put(`/userProfile/${userAId}`)
+      .send(userAProfileUpdateDto)
+      .set(...setAuthorization(userBAccessToken))
+      .expect(403)
+
+    // Allow admin to edit userA
+    await supertest(app.getHttpServer())
+      .put(`/userProfile/${userAId}`)
+      .send(userAProfileUpdateDto)
+      .set(...setAuthorization(adminAccessToken))
+      .expect(200)
+  })
 })