Skip to content

Commit

Permalink
Move business logic from User controller to user service
Browse files Browse the repository at this point in the history
  • Loading branch information
pbn4 committed Jul 6, 2021
1 parent bfa0932 commit 7e3862f
Show file tree
Hide file tree
Showing 18 changed files with 177 additions and 188 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ All notable changes to this project will be documented in this file. The format
- Cleanup seed data generation and add more variety ([#1312](https://github.com/bloom-housing/bloom/pull/1312)) Emily Jablonski
- Moved Property model to Listing (https://github.com/bloom-housing/bloom/issues/1328)
- removed eager relation to listing from User model
- User module has been removed and incorporated into Auth module

### Frontend

Expand Down
18 changes: 1 addition & 17 deletions backend/core/index.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1 @@
/* Models */
export * from "./src/auth/entities/user.entity"
export * from "./src/listings/entities/listing.entity"
export * from "./src/units/entities/unit.entity"
export { ListingEventType } from "./src/listings/types/listing-event-type-enum"
export { ListingStatus } from "./src/listings/types/listing-status-enum"
export { ListingEventDto } from "./src/listings/dto/listing-event.dto"
export { ApplicationMethodType } from "./src/listings/types/application-method-type-enum"
export { ApplicationMethodDto } from "./src/listings/dto/application-method.dto"
export { MinMax } from "./src/units/types/min-max"
export { MinMaxCurrency } from "./src/units/types/min-max-currency"
export { UnitSummary } from "./src/units/types/unit-summary"
export { UnitSummaryByReservedType } from "./src/units/types/unit-summary-by-reserved-type"
export { UnitSummaryByAMI } from "./src/units/types/unit-summary-by-ami"
export { HMI } from "./src/units/types/hmi"
export { UnitsSummarized } from "./src/units/types/units-summarized"
export { UserRole } from "./src/auth/enum/user-role-enum"
export {}
4 changes: 2 additions & 2 deletions backend/core/ormconfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export = {
// https://stackoverflow.com/questions/59435293/typeorm-entity-in-nestjs-cannot-use-import-statement-outside-a-module
join(__dirname, "src/**", "*.entity.{js,ts}"),
],
migrations: [join(__dirname, "src/migration/**", "*.ts")],
subscribers: [join(__dirname, "src/subscriber/**", "*.ts")],
migrations: [join(__dirname, "src/migration", "*.{js,ts}")],
subscribers: [join(__dirname, "src/subscriber", "*.{js,ts}")],
cli: {
entitiesDir: "src/entity",
migrationsDir: "src/migration",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Expose, Type } from "class-transformer"
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
import { Application } from "../../applications/entities/application.entity"
import { User } from "../../auth/entities/user.entity"
import { Listing } from "../../.."
import { Listing } from "../../listings/entities/listing.entity"
import { FlaggedSetStatus } from "../types/flagged-set-status-enum"
import { Rule } from "../types/rule-enum"

Expand Down
5 changes: 3 additions & 2 deletions backend/core/src/auth/auth.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { User } from "./entities/user.entity"
import { UserService } from "./services/user.service"
import { UserController } from "./controllers/user.controller"
import { EmailModule } from "../shared/email/email.module"
import { PasswordService } from "./services/password.service"

@Module({
imports: [
Expand All @@ -32,8 +33,8 @@ import { EmailModule } from "../shared/email/email.module"
SharedModule,
EmailModule,
],
providers: [LocalStrategy, JwtStrategy, AuthService, AuthzService, UserService],
exports: [AuthzService, AuthService],
providers: [LocalStrategy, JwtStrategy, AuthService, AuthzService, UserService, PasswordService],
exports: [AuthzService, AuthService, UserService],
controllers: [AuthController, UserController],
})
export class AuthModule {}
6 changes: 1 addition & 5 deletions backend/core/src/auth/controllers/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,13 @@ import { DefaultAuthGuard } from "../guards/default.guard"
import { ApiBody, ApiOperation, ApiTags } from "@nestjs/swagger"
import { LoginDto, LoginResponseDto } from "../dto/login.dto"
import { mapTo } from "../../shared/mapTo"
import { UserService } from "../services/user.service"
import { defaultValidationPipeOptions } from "../../shared/default-validation-pipe-options"

@Controller("auth")
@ApiTags("auth")
@UsePipes(new ValidationPipe(defaultValidationPipeOptions))
export class AuthController {
constructor(
private readonly userService: UserService,
private readonly authService: AuthService
) {}
constructor(private readonly authService: AuthService) {}

@UseGuards(LocalAuthGuard)
@Post("login")
Expand Down
54 changes: 16 additions & 38 deletions backend/core/src/auth/controllers/user.controller.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,23 @@
import {
Body,
Controller,
Request,
Get,
UseGuards,
Post,
Body,
Put,
NotFoundException,
Query,
Request,
UseGuards,
UsePipes,
ValidationPipe,
Query,
} from "@nestjs/common"
import { ApiBearerAuth, ApiOperation, ApiProperty, ApiTags } from "@nestjs/swagger"

import { Request as ExpressRequest } from "express"
import { Expose, Transform } from "class-transformer"
import { IsBoolean, IsOptional } from "class-validator"
import { ValidationsGroupsEnum } from "../../shared/types/validations-groups-enum"
import { ResourceType } from "../decorators/resource-type.decorator"
import { defaultValidationPipeOptions } from "../../shared/default-validation-pipe-options"
import { UserService } from "../services/user.service"
import { AuthService } from "../services/auth.service"
import { EmailService } from "../../shared/email/email.service"
import { authzActions, AuthzService } from "../services/authz.service"
import { OptionalAuthGuard } from "../guards/optional-auth.guard"
import { AuthzGuard } from "../guards/authz.guard"
import { EmailDto, UserBasicDto, UserCreateDto, UserDto, UserUpdateDto } from "../dto/user.dto"
Expand All @@ -32,6 +27,8 @@ import { ConfirmDto } from "../dto/confirm.dto"
import { LoginResponseDto } from "../dto/login.dto"
import { ForgotPasswordDto, ForgotPasswordResponseDto } from "../dto/forgot-password.dto"
import { UpdatePasswordDto } from "../dto/update-password.dto"
import { AuthContext } from "../types/auth-context"
import { User } from "../entities/user.entity"

export class UserCreateQueryParams {
@Expose()
Expand All @@ -52,12 +49,7 @@ export class UserCreateQueryParams {
@ResourceType("user")
@UsePipes(new ValidationPipe(defaultValidationPipeOptions))
export class UserController {
constructor(
private readonly userService: UserService,
private readonly authService: AuthService,
private readonly emailService: EmailService,
private readonly authzService: AuthzService
) {}
constructor(private readonly userService: UserService) {}

@Get()
@UseGuards(OptionalAuthGuard, AuthzGuard)
Expand All @@ -72,59 +64,45 @@ export class UserController {
@Body() dto: UserCreateDto,
@Query() queryParams: UserCreateQueryParams
): Promise<UserBasicDto> {
const user = await this.userService.createUser(dto)
if (!queryParams.noWelcomeEmail) {
await this.emailService.welcome(user, dto.appUrl)
}
return mapTo(UserBasicDto, user)
return mapTo(
UserBasicDto,
await this.userService.createUser(dto, queryParams.noWelcomeEmail !== true)
)
}

@Post("resend-confirmation")
@UseGuards(OptionalAuthGuard, AuthzGuard)
@ApiOperation({ summary: "Resend confirmation", operationId: "resendConfirmation" })
async confirmation(@Body() dto: EmailDto): Promise<StatusDto> {
const user = await this.userService.resendConfirmation(dto)
await this.emailService.welcome(user, dto.appUrl)
await this.userService.resendConfirmation(dto)
return mapTo(StatusDto, { status: "ok" })
}

@Put("confirm")
@ApiOperation({ summary: "Confirm email", operationId: "confirm" })
async confirm(@Body() dto: ConfirmDto): Promise<LoginResponseDto> {
const user = await this.userService.confirm(dto)
const accessToken = this.authService.generateAccessToken(user)
const accessToken = await this.userService.confirm(dto)
return mapTo(LoginResponseDto, { accessToken })
}

@Put("forgot-password")
@ApiOperation({ summary: "Forgot Password", operationId: "forgot-password" })
async forgotPassword(@Body() dto: ForgotPasswordDto): Promise<ForgotPasswordResponseDto> {
const user = await this.userService.forgotPassword(dto.email)
await this.emailService.forgotPassword(user, dto.appUrl)
await this.userService.forgotPassword(dto)
return mapTo(ForgotPasswordResponseDto, { message: "Email was sent" })
}

@Put("update-password")
@ApiOperation({ summary: "Update Password", operationId: "update-password" })
async updatePassword(@Body() dto: UpdatePasswordDto): Promise<LoginResponseDto> {
const user = await this.userService.updatePassword(dto)

const accessToken = this.authService.generateAccessToken(user)
const accessToken = await this.userService.updatePassword(dto)
return mapTo(LoginResponseDto, { accessToken })
}

@Put(":id")
@UseGuards(OptionalAuthGuard, AuthzGuard)
@ApiOperation({ summary: "Update user", operationId: "update" })
async update(@Request() req: ExpressRequest, @Body() dto: UserUpdateDto): Promise<UserDto> {
const user = await this.userService.find({ id: dto.id })
if (!user) {
throw new NotFoundException()
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await this.authzService.canOrThrow(req.user as any, "user", authzActions.update, {
...dto,
})
return mapTo(UserDto, await this.userService.update(dto))
return mapTo(UserDto, await this.userService.update(dto, new AuthContext(req.user as User)))
}
}
11 changes: 8 additions & 3 deletions backend/core/src/auth/passport-strategies/jwt.strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import { PassportStrategy } from "@nestjs/passport"
import { Injectable, UnauthorizedException } from "@nestjs/common"
import { Request } from "express"
import { ConfigService } from "@nestjs/config"
import { UserService } from "../services/user.service"
import { AuthService } from "../services/auth.service"
import { InjectRepository } from "@nestjs/typeorm"
import { User } from "../entities/user.entity"
import { Repository } from "typeorm"

function extractTokenFromAuthHeader(req: Request) {
const authHeader = req.get("Authorization")
Expand All @@ -14,7 +16,7 @@ function extractTokenFromAuthHeader(req: Request) {
@Injectable()
export class JwtStrategy extends PassportStrategy(Strategy) {
constructor(
private userService: UserService,
@InjectRepository(User) private readonly userRepository: Repository<User>,
private authService: AuthService,
private configService: ConfigService
) {
Expand All @@ -33,6 +35,9 @@ export class JwtStrategy extends PassportStrategy(Strategy) {
throw new UnauthorizedException()
}
const userId = payload.sub
return this.userService.find({ id: userId })
return this.userRepository.findOne({
where: { id: userId },
relations: ["leasingAgentInListings"],
})
}
}
17 changes: 13 additions & 4 deletions backend/core/src/auth/passport-strategies/local.strategy.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,30 @@
import { Strategy } from "passport-local"
import { PassportStrategy } from "@nestjs/passport"
import { Injectable, UnauthorizedException } from "@nestjs/common"
import { UserService } from "../services/user.service"
import { User } from "../entities/user.entity"
import { InjectRepository } from "@nestjs/typeorm"
import { Repository } from "typeorm"
import { PasswordService } from "../services/password.service"

@Injectable()
export class LocalStrategy extends PassportStrategy(Strategy) {
constructor(private userService: UserService) {
constructor(
@InjectRepository(User) private readonly userRepository: Repository<User>,
private readonly passwordService: PasswordService
) {
super({
usernameField: "email",
})
}

async validate(email: string, password: string): Promise<User> {
const user = await this.userService.findByEmail(email)
const user = await this.userRepository.findOne({
where: { email },
relations: ["leasingAgentInListings"],
})

if (user) {
const validPassword = await this.userService.verifyUserPassword(user, password)
const validPassword = await this.passwordService.verifyUserPassword(user, password)
if (validPassword && user.confirmedAt) {
return user
}
Expand Down
46 changes: 46 additions & 0 deletions backend/core/src/auth/services/password.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { Injectable } from "@nestjs/common"
import { User } from "../../auth/entities/user.entity"
import { randomBytes, scrypt } from "crypto"
import { SALT_SIZE, SCRYPT_KEYLEN } from "../constants"
import { InjectRepository } from "@nestjs/typeorm"
import { Repository } from "typeorm"

@Injectable()
export class PasswordService {
constructor(@InjectRepository(User) private readonly userRepository: Repository<User>) {}

// passwordHash is a hidden field - we need to build a query to get it directly
public async getUserWithPassword(user: User) {
return await this.userRepository
.createQueryBuilder()
.addSelect("user.passwordHash")
.from(User, "user")
.where("user.id = :id", { id: user.id })
.getOne()
}

public async verifyUserPassword(user: User, password: string) {
const userWithPassword = await this.getUserWithPassword(user)
const [salt, savedPasswordHash] = userWithPassword.passwordHash.split("#")
const verifyPasswordHash = await this.hashPassword(password, Buffer.from(salt, "hex"))
return savedPasswordHash === verifyPasswordHash
}

public async passwordToHash(password: string) {
const salt = this.generateSalt()
const hash = await this.hashPassword(password, salt)
return `${salt.toString("hex")}#${hash}`
}

private async hashPassword(password: string, salt: Buffer) {
return new Promise<string>((resolve, reject) =>
scrypt(password, salt, SCRYPT_KEYLEN, (err, key) =>
err ? reject(err) : resolve(key.toString("hex"))
)
)
}

private generateSalt(size = SALT_SIZE) {
return randomBytes(size)
}
}
6 changes: 3 additions & 3 deletions backend/core/src/auth/services/user.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ describe("UserService", () => {
describe("forgotPassword", () => {
it("should return 400 if email is not found", async () => {
mockUserRepo.findOne = jest.fn().mockResolvedValue(null)
await expect(service.forgotPassword("abc@xyz.com")).rejects.toThrow(
await expect(service.forgotPassword({ email: "abc@xyz.com" })).rejects.toThrow(
new HttpException(USER_ERRORS.NOT_FOUND.message, USER_ERRORS.NOT_FOUND.status)
)
})

it("should set resetToken", async () => {
mockUserRepo.findOne = jest.fn().mockResolvedValue({ ...mockedUser, resetToken: null })
const user = await service.forgotPassword("abc@xyz.com")
const user = await service.forgotPassword({ email: "abc@xyz.com" })
expect(user["resetToken"]).toBeDefined()
})
})
Expand All @@ -62,7 +62,7 @@ describe("UserService", () => {
it("should set resetToken", async () => {
mockUserRepo.findOne = jest.fn().mockResolvedValue({ ...mockedUser })
// Sets resetToken
await service.forgotPassword("abc@xyz.com")
await service.forgotPassword({ email: "abc@xyz.com" })
const user = await service.updatePassword(updateDto)
expect(user["resetToken"]).toBeFalsy()
})
Expand Down
Loading

0 comments on commit 7e3862f

Please sign in to comment.