From 844f6a8d2b028b1eebbd033fcd1ec76236bd9102 Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 19:06:35 +0900 Subject: [PATCH 1/9] :construction: Update comments --- src/users/users.service.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/users/users.service.ts b/src/users/users.service.ts index af47f69..75ce08c 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -35,18 +35,18 @@ export class UsersService { if (updateUserProfileDto.organizationCode) { console.log('updateUserProfileDto.organizationCode : ', updateUserProfileDto.organizationCode) // TODO @yashmurty : - // 2. If `orgCode` exists in payload, check if user already has existing `orgCode`. + // 1. Check if org code matches any org. If does not match then return error. + + // 2. Fetch user from DB and check for existing `orgCode`. const userProfile = await this.findOneUserProfileById(updateUserProfileDto.userId) console.log('userProfile : ', userProfile) - // A - If existing DB value is empty, check if payload `orgCode` matches any org, - // then add it to DB and also add custom claim. + // A - If existing DB value is empty, then add it to DB and also add custom claim. // B - If existing DB value is same as payload, do nothing. // C - If existing DB value is different from payload: - // - Check if org code matches any org. // - Perform step D defined below (delete org code). // - Then, Perform step A. } From d011e53135306fc25f39da919eb2f33182610a58 Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 19:28:25 +0900 Subject: [PATCH 2/9] :construction: Organization code validity function --- src/organizations/organizations.service.ts | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/organizations/organizations.service.ts b/src/organizations/organizations.service.ts index 70b28b2..ca738ff 100644 --- a/src/organizations/organizations.service.ts +++ b/src/organizations/organizations.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common' +import { Injectable, BadRequestException } from '@nestjs/common' import { OrganizationsRepository } from './organizations.repository' import { CreateOrganizationRequestDto, @@ -36,6 +36,27 @@ export class OrganizationsService { return this.organizationsRepository.updateOne(updateOrganizationRequest) } + /** + * Checks if organization code/id provided is valid or not. + * @param organizationId: string + */ + async isOrganizationCodeValid(organizationId: string): Promise { + const organization = await this.organizationsRepository.findOneById(organizationId) + if (!organization) { + return false + } + // This is just a sanity check. Since organization id and code should always have same value. + if (organizationId !== organization.organizationCode) { + throw new BadRequestException('organization code does not match organization id') + } + + return true + } + + /** + * Generates a random unique organization code. + * Calls itself again in case the generated code is already being used. + */ private async generateUniqueOrganizationCode(): Promise { const randomCode = randomBytes(4) // Creates a 8 (4*2) string code. .toString('hex') // Use hex to avoid special characters. From 8e7212df258124c8ba0d1cabe8b83b71d3a8aac6 Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 19:34:21 +0900 Subject: [PATCH 3/9] :construction: Profile - Organization code validity --- src/users/users.module.ts | 3 ++- src/users/users.service.ts | 13 ++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/users/users.module.ts b/src/users/users.module.ts index 4115e6f..cf9f687 100644 --- a/src/users/users.module.ts +++ b/src/users/users.module.ts @@ -3,9 +3,10 @@ import { UsersService } from './users.service' import { UsersRepository } from './users.repository' import { SharedModule } from '../shared/shared.module' import { UsersController } from './users.controller' +import { OrganizationsModule } from '../organizations/organizations.module' @Module({ - imports: [SharedModule], + imports: [SharedModule, OrganizationsModule], providers: [UsersService, UsersRepository], exports: [UsersService], controllers: [UsersController], diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 75ce08c..1f9a19a 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -4,10 +4,15 @@ import { CreateUserDto, CreateUserProfileDto, UpdateUserProfileDto } from './dto import { User, UserProfile } from './classes/user.class' import { CreateDiagnosisKeysForOrgDto } from './dto/create-diagnosis-keys.dto' import { FirebaseService } from '../shared/firebase/firebase.service' +import { OrganizationsService } from '../organizations/organizations.service' @Injectable() export class UsersService { - constructor(private usersRepository: UsersRepository, private firebaseService: FirebaseService) {} + constructor( + private usersRepository: UsersRepository, + private firebaseService: FirebaseService, + private organizationsService: OrganizationsService + ) {} createOneUser(user: CreateUserDto, userProfile?: CreateUserProfileDto) { return this.usersRepository.createOne(user, userProfile) @@ -36,6 +41,12 @@ export class UsersService { console.log('updateUserProfileDto.organizationCode : ', updateUserProfileDto.organizationCode) // TODO @yashmurty : // 1. Check if org code matches any org. If does not match then return error. + console.log( + 'isOrganizationCodeValid : ', + await this.organizationsService.isOrganizationCodeValid( + updateUserProfileDto.organizationCode + ) + ) // 2. Fetch user from DB and check for existing `orgCode`. From 85e7152fe4b01ae95448f5cb8c529bad3c17bbe3 Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 21:34:25 +0900 Subject: [PATCH 4/9] :construction: Profile - Organization code to uppercase --- src/users/dto/create-user.dto.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/users/dto/create-user.dto.ts b/src/users/dto/create-user.dto.ts index 1fab0a3..2aac5e6 100644 --- a/src/users/dto/create-user.dto.ts +++ b/src/users/dto/create-user.dto.ts @@ -1,5 +1,6 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger' import { IsString, IsNumber, IsOptional, IsNotEmpty } from 'class-validator' +import { Transform } from 'class-transformer' export class CreateUserDto { @ApiProperty() @@ -29,6 +30,7 @@ export class UpdateUserProfileDto { @ApiPropertyOptional({ example: 'A12B34' }) @IsString() @IsOptional() + @Transform((value) => value.toUpperCase(), { toClassOnly: true }) organizationCode: string // Keys without any decorators are non-Whitelisted. Validator will throw error if it's passed in payload. From 91b5e65acb245be4b5f42aea0f80d3b160b9208a Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 21:50:30 +0900 Subject: [PATCH 5/9] :construction: Update comments --- src/users/users.service.ts | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 1f9a19a..5267861 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common' +import { Injectable, BadRequestException } from '@nestjs/common' import { UsersRepository } from './users.repository' import { CreateUserDto, CreateUserProfileDto, UpdateUserProfileDto } from './dto/create-user.dto' import { User, UserProfile } from './classes/user.class' @@ -30,6 +30,14 @@ export class UsersService { return this.usersRepository.uploadDiagnosisKeysForOrgList() } + /** + * Updates user profile. Takes care of multiple cases for organization code update. + * Organization code update case: + * - Check organization code validity. + * - Add organization code to user profile and add custom claims to JWT. + * - In case of empty string payload, remove organization code from user profile and add custom claims. + * @param updateUserProfileDto: UpdateUserProfileDto + */ async updateUserProfile(updateUserProfileDto: UpdateUserProfileDto): Promise { if (updateUserProfileDto.prefecture) { await this.usersRepository.updateUserProfilePrefecture(updateUserProfileDto) @@ -38,15 +46,12 @@ export class UsersService { console.log('updateUserProfileDto : ', updateUserProfileDto) if (updateUserProfileDto.organizationCode) { - console.log('updateUserProfileDto.organizationCode : ', updateUserProfileDto.organizationCode) - // TODO @yashmurty : - // 1. Check if org code matches any org. If does not match then return error. - console.log( - 'isOrganizationCodeValid : ', - await this.organizationsService.isOrganizationCodeValid( - updateUserProfileDto.organizationCode - ) + const isOrganizationCodeValid = await this.organizationsService.isOrganizationCodeValid( + updateUserProfileDto.organizationCode ) + if (!isOrganizationCodeValid) { + throw new BadRequestException('Organization code does not match any existing organization') + } // 2. Fetch user from DB and check for existing `orgCode`. @@ -66,6 +71,7 @@ export class UsersService { await this.removeUserOrganizationCode(updateUserProfileDto.userId) } + console.log('---- FINISH UPDATE FUNCTION') return } From d7eff4dae74fca394e57429ab6e47d83d10cf072 Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 22:22:07 +0900 Subject: [PATCH 6/9] :construction: Profile - Add org code to user --- src/shared/firebase/firebase.service.ts | 1 - src/users/users.service.ts | 45 +++++++++++++++++++++---- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/shared/firebase/firebase.service.ts b/src/shared/firebase/firebase.service.ts index 02ccfbc..f521fe9 100644 --- a/src/shared/firebase/firebase.service.ts +++ b/src/shared/firebase/firebase.service.ts @@ -51,7 +51,6 @@ export class FirebaseService { const existingCustomClaims = firebaseUser.customClaims for (const customClaimKey of deleteCustomClaimKeys) { - console.log(customClaimKey) delete existingCustomClaims[customClaimKey] } diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 5267861..7a06312 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -58,13 +58,31 @@ export class UsersService { const userProfile = await this.findOneUserProfileById(updateUserProfileDto.userId) console.log('userProfile : ', userProfile) - // A - If existing DB value is empty, then add it to DB and also add custom claim. - - // B - If existing DB value is same as payload, do nothing. - - // C - If existing DB value is different from payload: - // - Perform step D defined below (delete org code). - // - Then, Perform step A. + switch (true) { + case !userProfile.organizationCode: + // Organization code does not exist in user profile, + // so add it to user profile and JWT custom claims. + await this.addUserOrganizationCode(updateUserProfileDto) + + break + case userProfile.organizationCode && + userProfile.organizationCode === updateUserProfileDto.organizationCode: + // Organization code in user profile exists and is same as update payload value, + // so do nothing. + break + case userProfile.organizationCode && + userProfile.organizationCode !== updateUserProfileDto.organizationCode: + // Organization code in user profile exists and is different from update payload value, + // so first remove existing user profile value and add new one from payload. + await this.removeUserOrganizationCode(updateUserProfileDto.userId) + await this.addUserOrganizationCode(updateUserProfileDto) + + break + default: + throw new BadRequestException( + 'Organization code does not match any existing organization' + ) + } } if (updateUserProfileDto.organizationCode === '') { @@ -75,6 +93,19 @@ export class UsersService { return } + /** + * Adds the organization code to user profile DB and user JWT custom claim. + * @param updateUserProfileDto: UpdateUserProfileDto + */ + private async addUserOrganizationCode(updateUserProfileDto: UpdateUserProfileDto): Promise { + await this.usersRepository.updateUserProfileOrganizationCode(updateUserProfileDto) + + // Adds the custom claim organization code to user JWT. + await this.firebaseService.UpsertCustomClaims(updateUserProfileDto.userId, { + organizationCode: updateUserProfileDto.organizationCode, + }) + } + /** * Removes the organization code from user profile DB and user JWT custom claim. * @param userId: string From a07daeee7dd706ebf1e853d07f83b95e30b82d61 Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 22:25:52 +0900 Subject: [PATCH 7/9] :construction: Remove logs --- src/users/users.service.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 7a06312..5c0e618 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -43,8 +43,6 @@ export class UsersService { await this.usersRepository.updateUserProfilePrefecture(updateUserProfileDto) } - console.log('updateUserProfileDto : ', updateUserProfileDto) - if (updateUserProfileDto.organizationCode) { const isOrganizationCodeValid = await this.organizationsService.isOrganizationCodeValid( updateUserProfileDto.organizationCode @@ -53,10 +51,7 @@ export class UsersService { throw new BadRequestException('Organization code does not match any existing organization') } - // 2. Fetch user from DB and check for existing `orgCode`. - const userProfile = await this.findOneUserProfileById(updateUserProfileDto.userId) - console.log('userProfile : ', userProfile) switch (true) { case !userProfile.organizationCode: @@ -88,9 +83,6 @@ export class UsersService { if (updateUserProfileDto.organizationCode === '') { await this.removeUserOrganizationCode(updateUserProfileDto.userId) } - - console.log('---- FINISH UPDATE FUNCTION') - return } /** From c30f7f38585d01d1d212e34068f69afd62ef71b7 Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 22:29:47 +0900 Subject: [PATCH 8/9] :construction: Profile - Update error message --- src/users/users.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 5c0e618..ce65388 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -75,7 +75,7 @@ export class UsersService { break default: throw new BadRequestException( - 'Organization code does not match any existing organization' + 'Organization code could not be added to user profile, please contact support' ) } } From 182d684c3a6a4809cde9425ad9cbd04be871c40f Mon Sep 17 00:00:00 2001 From: Yash Murty Date: Sat, 2 May 2020 22:33:04 +0900 Subject: [PATCH 9/9] :construction: Fix tests --- src/users/users.service.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/users/users.service.spec.ts b/src/users/users.service.spec.ts index e90be91..7bf565c 100644 --- a/src/users/users.service.spec.ts +++ b/src/users/users.service.spec.ts @@ -2,20 +2,24 @@ import { Test, TestingModule } from '@nestjs/testing' import { UsersService } from './users.service' import { UsersRepository } from './users.repository' import { FirebaseService } from '../shared/firebase/firebase.service' +import { OrganizationsService } from '../organizations/organizations.service' describe('UsersService', () => { let service: UsersService const usersRepository = { findAll: () => ['test'] } const firebaseService = {} + const organizationsService = {} beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ - providers: [UsersService, UsersRepository, FirebaseService], + providers: [UsersService, UsersRepository, FirebaseService, OrganizationsService], }) .overrideProvider(UsersRepository) .useValue(usersRepository) .overrideProvider(FirebaseService) .useValue(firebaseService) + .overrideProvider(OrganizationsService) + .useValue(organizationsService) .compile() service = module.get(UsersService)