From c514748cc178ed1b766e69656ca3c6553acae602 Mon Sep 17 00:00:00 2001 From: Yazeed Loonat Date: Thu, 29 Feb 2024 09:43:38 -0700 Subject: [PATCH] fix: 3898/passwordless schema changes (#3902) * feat: mfaCode -> singleUseCode * feat: adding new field to jurisdiction --- .../migration.sql | 7 ++++ .../migration.sql | 2 ++ api/prisma/schema.prisma | 18 +++++----- api/prisma/seed-helpers/user-factory.ts | 6 ++-- api/prisma/seed-staging.ts | 2 +- .../dtos/jurisdictions/jurisdiction.dto.ts | 6 ++++ api/src/passports/mfa.strategy.ts | 33 +++++++++-------- api/src/services/auth.service.ts | 12 +++---- api/src/services/email.service.ts | 4 +-- api/src/services/sms.service.ts | 4 +-- api/test/integration/auth.e2e-spec.ts | 14 ++++---- api/test/integration/jurisdiction.e2e-spec.ts | 3 ++ .../integration/permission-tests/helpers.ts | 2 ++ api/test/unit/passports/mfa.strategy.spec.ts | 36 +++++++++---------- api/test/unit/services/auth.service.spec.ts | 10 +++--- 15 files changed, 92 insertions(+), 67 deletions(-) create mode 100644 api/prisma/migrations/03_mfa_code_to_single_use_code/migration.sql create mode 100644 api/prisma/migrations/04_allow_single_use_code_login_jurisdiction_flag/migration.sql diff --git a/api/prisma/migrations/03_mfa_code_to_single_use_code/migration.sql b/api/prisma/migrations/03_mfa_code_to_single_use_code/migration.sql new file mode 100644 index 0000000000..5d8a35af39 --- /dev/null +++ b/api/prisma/migrations/03_mfa_code_to_single_use_code/migration.sql @@ -0,0 +1,7 @@ +-- AlterTable + +ALTER TABLE "user_accounts" RENAME COLUMN "mfa_code" TO "single_use_code"; + + +ALTER TABLE "user_accounts" RENAME COLUMN "mfa_code_updated_at" TO "single_use_code_updated_at"; + diff --git a/api/prisma/migrations/04_allow_single_use_code_login_jurisdiction_flag/migration.sql b/api/prisma/migrations/04_allow_single_use_code_login_jurisdiction_flag/migration.sql new file mode 100644 index 0000000000..219cd57348 --- /dev/null +++ b/api/prisma/migrations/04_allow_single_use_code_login_jurisdiction_flag/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "jurisdictions" ADD COLUMN "allow_single_use_code_login" BOOLEAN NOT NULL DEFAULT false; diff --git a/api/prisma/schema.prisma b/api/prisma/schema.prisma index 13f6e4231d..bc8f354b17 100644 --- a/api/prisma/schema.prisma +++ b/api/prisma/schema.prisma @@ -329,6 +329,7 @@ model Jurisdictions { enableAccessibilityFeatures Boolean @default(false) @map("enable_accessibility_features") enableUtilitiesIncluded Boolean @default(false) @map("enable_utilities_included") enableGeocodingPreferences Boolean @default(false) @map("enable_geocoding_preferences") + allowSingleUseCodeLogin Boolean @default(false) @map("allow_single_use_code_login") amiChart AmiChart[] multiselectQuestions MultiselectQuestions[] listings Listings[] @@ -571,10 +572,10 @@ model Listings { } model MapLayers { - id String @id() @default(dbgenerated("uuid_generate_v4()")) @db.Uuid + id String @id() @default(dbgenerated("uuid_generate_v4()")) @db.Uuid name String - jurisdictionId String @map("jurisdiction_id") - featureCollection Json @map("feature_collection") @default("{}") + jurisdictionId String @map("jurisdiction_id") + featureCollection Json @default("{}") @map("feature_collection") @@map("map_layers") } @@ -650,9 +651,9 @@ model Translations { // Note: [name] formerly max length 256 model UnitAccessibilityPriorityTypes { - id String @id() @default(dbgenerated("uuid_generate_v4()")) @db.Uuid - createdAt DateTime @default(now()) @map("created_at") @db.Timestamp(6) - updatedAt DateTime @updatedAt @map("updated_at") @db.Timestamp(6) + id String @id() @default(dbgenerated("uuid_generate_v4()")) @db.Uuid + createdAt DateTime @default(now()) @map("created_at") @db.Timestamp(6) + updatedAt DateTime @updatedAt @map("updated_at") @db.Timestamp(6) name String units Units[] unitGroup UnitGroup[] @@ -779,8 +780,8 @@ model UserAccounts { updatedAt DateTime @updatedAt @map("updated_at") @db.Timestamp(6) language LanguagesEnum? mfaEnabled Boolean @default(false) @map("mfa_enabled") - mfaCode String? @map("mfa_code") @db.VarChar - mfaCodeUpdatedAt DateTime? @map("mfa_code_updated_at") @db.Timestamptz(6) + singleUseCode String? @map("single_use_code") @db.VarChar + singleUseCodeUpdatedAt DateTime? @map("single_use_code_updated_at") @db.Timestamptz(6) lastLoginAt DateTime @default(now()) @map("last_login_at") @db.Timestamp(6) failedLoginAttemptsCount Int @default(0) @map("failed_login_attempts_count") phoneNumberVerified Boolean? @default(false) @map("phone_number_verified") @@ -809,7 +810,6 @@ model UserRoles { @@map("user_roles") } - // START DETROIT SPECIFIC model ListingNeighborhoodAmenities { id String @id() @default(dbgenerated("uuid_generate_v4()")) @db.Uuid diff --git a/api/prisma/seed-helpers/user-factory.ts b/api/prisma/seed-helpers/user-factory.ts index f081035d24..b25a3ef279 100644 --- a/api/prisma/seed-helpers/user-factory.ts +++ b/api/prisma/seed-helpers/user-factory.ts @@ -7,7 +7,7 @@ export const userFactory = async (optionalParams?: { firstName?: string; lastName?: string; email?: string; - mfaCode?: string; + singleUseCode?: string; mfaEnabled?: boolean; confirmedAt?: Date; phoneNumber?: string; @@ -30,10 +30,10 @@ export const userFactory = async (optionalParams?: { isPartner: optionalParams?.roles?.isPartner || false, }, }, - mfaCode: optionalParams?.mfaCode || null, + singleUseCode: optionalParams?.singleUseCode || null, mfaEnabled: optionalParams?.mfaEnabled || false, confirmedAt: optionalParams?.confirmedAt || null, - mfaCodeUpdatedAt: optionalParams?.mfaEnabled ? new Date() : undefined, + singleUseCodeUpdatedAt: optionalParams?.mfaEnabled ? new Date() : undefined, phoneNumber: optionalParams?.phoneNumber || null, phoneNumberVerified: optionalParams?.phoneNumberVerified || null, agreedToTermsOfService: optionalParams?.acceptedTerms || false, diff --git a/api/prisma/seed-staging.ts b/api/prisma/seed-staging.ts index 7062a3bb9b..d9c3247476 100644 --- a/api/prisma/seed-staging.ts +++ b/api/prisma/seed-staging.ts @@ -85,7 +85,7 @@ export const stagingSeed = async ( jurisdictionIds: [jurisdiction.id], acceptedTerms: true, mfaEnabled: true, - mfaCode: '12345', + singleUseCode: '12345', }), }); // add jurisdiction specific translations and default ones diff --git a/api/src/dtos/jurisdictions/jurisdiction.dto.ts b/api/src/dtos/jurisdictions/jurisdiction.dto.ts index a298872f3b..34c77b84e1 100644 --- a/api/src/dtos/jurisdictions/jurisdiction.dto.ts +++ b/api/src/dtos/jurisdictions/jurisdiction.dto.ts @@ -95,6 +95,12 @@ export class Jurisdiction extends AbstractDTO { @ApiProperty() enableUtilitiesIncluded: boolean; + @Expose() + @IsBoolean({ groups: [ValidationsGroupsEnum.default] }) + @IsDefined({ groups: [ValidationsGroupsEnum.default] }) + @ApiProperty() + allowSingleUseCodeLogin: boolean; + @Expose() @IsArray({ groups: [ValidationsGroupsEnum.default] }) @IsEnum(UserRoleEnum, { diff --git a/api/src/passports/mfa.strategy.ts b/api/src/passports/mfa.strategy.ts index ee37adebaf..2101e289bc 100644 --- a/api/src/passports/mfa.strategy.ts +++ b/api/src/passports/mfa.strategy.ts @@ -113,8 +113,12 @@ export class MfaStrategy extends PassportStrategy(Strategy, 'mfa') { } let authSuccess = true; - if (!dto.mfaCode || !rawUser.mfaCode || !rawUser.mfaCodeUpdatedAt) { - // if an mfaCode was not sent, and an mfaCode wasn't stored in the db for the user + if ( + !dto.mfaCode || + !rawUser.singleUseCode || + !rawUser.singleUseCodeUpdatedAt + ) { + // if an mfaCode was not sent, and a singleUseCode wasn't stored in the db for the user // signal to the front end to request an mfa code await this.updateFailedLoginCount(0, rawUser.id); throw new UnauthorizedException({ @@ -122,24 +126,25 @@ export class MfaStrategy extends PassportStrategy(Strategy, 'mfa') { }); } else if ( new Date( - rawUser.mfaCodeUpdatedAt.getTime() + Number(process.env.MFA_CODE_VALID), + rawUser.singleUseCodeUpdatedAt.getTime() + + Number(process.env.MFA_CODE_VALID), ) < new Date() || - rawUser.mfaCode !== dto.mfaCode + rawUser.singleUseCode !== dto.mfaCode ) { // if mfaCode TTL has expired, or if the mfa code input was incorrect authSuccess = false; } else { // if mfaCode login was a success - rawUser.mfaCode = null; - rawUser.mfaCodeUpdatedAt = new Date(); + rawUser.singleUseCode = null; + rawUser.singleUseCodeUpdatedAt = new Date(); } if (!authSuccess) { // if we failed login validation rawUser.failedLoginAttemptsCount += 1; await this.updateStoredUser( - rawUser.mfaCode, - rawUser.mfaCodeUpdatedAt, + rawUser.singleUseCode, + rawUser.singleUseCodeUpdatedAt, rawUser.phoneNumberVerified, rawUser.failedLoginAttemptsCount, rawUser.id, @@ -161,8 +166,8 @@ export class MfaStrategy extends PassportStrategy(Strategy, 'mfa') { } await this.updateStoredUser( - rawUser.mfaCode, - rawUser.mfaCodeUpdatedAt, + rawUser.singleUseCode, + rawUser.singleUseCodeUpdatedAt, rawUser.phoneNumberVerified, rawUser.failedLoginAttemptsCount, rawUser.id, @@ -188,16 +193,16 @@ export class MfaStrategy extends PassportStrategy(Strategy, 'mfa') { } async updateStoredUser( - mfaCode: string, - mfaCodeUpdatedAt: Date, + singleUseCode: string, + singleUseCodeUpdatedAt: Date, phoneNumberVerified: boolean, failedLoginAttemptsCount: number, userId: string, ): Promise { await this.prisma.userAccounts.update({ data: { - mfaCode, - mfaCodeUpdatedAt, + singleUseCode, + singleUseCodeUpdatedAt, phoneNumberVerified, failedLoginAttemptsCount, lastLoginAt: new Date(), diff --git a/api/src/services/auth.service.ts b/api/src/services/auth.service.ts index b00a7103df..f0c2591042 100644 --- a/api/src/services/auth.service.ts +++ b/api/src/services/auth.service.ts @@ -219,11 +219,11 @@ export class AuthService { } } - const mfaCode = this.generateMfaCode(); + const singleUseCode = this.generateSingleUseCode(); await this.prisma.userAccounts.update({ data: { - mfaCode, - mfaCodeUpdatedAt: new Date(), + singleUseCode, + singleUseCodeUpdatedAt: new Date(), phoneNumber: user.phoneNumber, }, where: { @@ -232,9 +232,9 @@ export class AuthService { }); if (dto.mfaType === MfaType.email) { - await this.emailsService.sendMfaCode(mapTo(User, user), mfaCode); + await this.emailsService.sendMfaCode(mapTo(User, user), singleUseCode); } else if (dto.mfaType === MfaType.sms) { - await this.smsService.sendMfaCode(user.phoneNumber, mfaCode); + await this.smsService.sendMfaCode(user.phoneNumber, singleUseCode); } return dto.mfaType === MfaType.email @@ -328,7 +328,7 @@ export class AuthService { /* generates a numeric mfa code */ - generateMfaCode() { + generateSingleUseCode() { let out = ''; const characters = '0123456789'; for (let i = 0; i < Number(process.env.MFA_CODE_LENGTH); i++) { diff --git a/api/src/services/email.service.ts b/api/src/services/email.service.ts index e1c58c9721..02901edabc 100644 --- a/api/src/services/email.service.ts +++ b/api/src/services/email.service.ts @@ -299,7 +299,7 @@ export class EmailService { ); } - public async sendMfaCode(user: User, mfaCode: string) { + public async sendMfaCode(user: User, singleUseCode: string) { const jurisdiction = await this.getJurisdiction(user.jurisdictions); void (await this.loadTranslations(jurisdiction, user.language)); const emailFromAddress = await this.getEmailToSendFrom( @@ -312,7 +312,7 @@ export class EmailService { 'Partners Portal account access token', this.template('mfa-code')({ user: user, - mfaCodeOptions: { mfaCode }, + mfaCodeOptions: { singleUseCode }, }), ); } diff --git a/api/src/services/sms.service.ts b/api/src/services/sms.service.ts index 2325a73d73..5a1f0a5e35 100644 --- a/api/src/services/sms.service.ts +++ b/api/src/services/sms.service.ts @@ -15,13 +15,13 @@ export class SmsService { } public async sendMfaCode( phoneNumber: string, - mfaCode: string, + singleUseCode: string, ): Promise { if (!this.client) { return; } await this.client.messages.create({ - body: `Your Partners Portal account access token: ${mfaCode}`, + body: `Your Partners Portal account access token: ${singleUseCode}`, from: process.env.TWILIO_PHONE_NUMBER, to: phoneNumber, }); diff --git a/api/test/integration/auth.e2e-spec.ts b/api/test/integration/auth.e2e-spec.ts index 96bb70321b..07bccd767a 100644 --- a/api/test/integration/auth.e2e-spec.ts +++ b/api/test/integration/auth.e2e-spec.ts @@ -44,7 +44,7 @@ describe('Auth Controller Tests', () => { const storedUser = await prisma.userAccounts.create({ data: await userFactory({ roles: { isAdmin: true }, - mfaCode: 'abcdef', + singleUseCode: 'abcdef', mfaEnabled: true, confirmedAt: new Date(), }), @@ -54,7 +54,7 @@ describe('Auth Controller Tests', () => { .send({ email: storedUser.email, password: 'abcdef', - mfaCode: storedUser.mfaCode, + mfaCode: storedUser.singleUseCode, mfaType: MfaType.email, } as Login) .expect(201); @@ -78,7 +78,7 @@ describe('Auth Controller Tests', () => { }); expect(loggedInUser.lastLoginAt).not.toBeNull(); - expect(loggedInUser.mfaCode).toBeNull(); + expect(loggedInUser.singleUseCode).toBeNull(); expect(loggedInUser.activeAccessToken).not.toBeNull(); expect(loggedInUser.activeRefreshToken).not.toBeNull(); }); @@ -118,7 +118,7 @@ describe('Auth Controller Tests', () => { }); expect(loggedInUser.lastLoginAt).not.toBeNull(); - expect(loggedInUser.mfaCode).toBeNull(); + expect(loggedInUser.singleUseCode).toBeNull(); expect(loggedInUser.activeAccessToken).not.toBeNull(); expect(loggedInUser.activeRefreshToken).not.toBeNull(); }); @@ -163,7 +163,7 @@ describe('Auth Controller Tests', () => { }); expect(loggedInUser.lastLoginAt).not.toBeNull(); - expect(loggedInUser.mfaCode).toBeNull(); + expect(loggedInUser.singleUseCode).toBeNull(); expect(loggedInUser.activeAccessToken).toBeNull(); expect(loggedInUser.activeRefreshToken).toBeNull(); }); @@ -208,8 +208,8 @@ describe('Auth Controller Tests', () => { }, }); - expect(user.mfaCode).not.toBeNull(); - expect(user.mfaCodeUpdatedAt).not.toBeNull(); + expect(user.singleUseCode).not.toBeNull(); + expect(user.singleUseCodeUpdatedAt).not.toBeNull(); }); it('should update password', async () => { diff --git a/api/test/integration/jurisdiction.e2e-spec.ts b/api/test/integration/jurisdiction.e2e-spec.ts index 999f3ac1dd..83f8d0b47f 100644 --- a/api/test/integration/jurisdiction.e2e-spec.ts +++ b/api/test/integration/jurisdiction.e2e-spec.ts @@ -124,6 +124,7 @@ describe('Jurisdiction Controller Tests', () => { enablePartnerSettings: true, enableAccessibilityFeatures: true, enableUtilitiesIncluded: true, + allowSingleUseCodeLogin: true, listingApprovalPermissions: [], }; const res = await request(app.getHttpServer()) @@ -149,6 +150,7 @@ describe('Jurisdiction Controller Tests', () => { enablePartnerSettings: true, enableAccessibilityFeatures: true, enableUtilitiesIncluded: true, + allowSingleUseCodeLogin: true, listingApprovalPermissions: [], }; const res = await request(app.getHttpServer()) @@ -178,6 +180,7 @@ describe('Jurisdiction Controller Tests', () => { enablePartnerSettings: true, enableAccessibilityFeatures: true, enableUtilitiesIncluded: true, + allowSingleUseCodeLogin: true, listingApprovalPermissions: [], }; diff --git a/api/test/integration/permission-tests/helpers.ts b/api/test/integration/permission-tests/helpers.ts index dd34c20f65..df1c36bf62 100644 --- a/api/test/integration/permission-tests/helpers.ts +++ b/api/test/integration/permission-tests/helpers.ts @@ -107,6 +107,7 @@ export const buildJurisdictionCreateMock = ( enablePartnerSettings: true, enableAccessibilityFeatures: true, enableUtilitiesIncluded: true, + allowSingleUseCodeLogin: true, listingApprovalPermissions: [], }; }; @@ -127,6 +128,7 @@ export const buildJurisdictionUpdateMock = ( enablePartnerSettings: true, enableAccessibilityFeatures: true, enableUtilitiesIncluded: true, + allowSingleUseCodeLogin: true, listingApprovalPermissions: [], }; }; diff --git a/api/test/unit/passports/mfa.strategy.spec.ts b/api/test/unit/passports/mfa.strategy.spec.ts index 3a4f505299..2d5f684623 100644 --- a/api/test/unit/passports/mfa.strategy.spec.ts +++ b/api/test/unit/passports/mfa.strategy.spec.ts @@ -247,8 +247,8 @@ describe('Testing mfa strategy', () => { expect(prisma.userAccounts.update).toHaveBeenCalledWith({ data: { - mfaCode: null, - mfaCodeUpdatedAt: null, + singleUseCode: null, + singleUseCodeUpdatedAt: null, phoneNumberVerified: null, lastLoginAt: expect.anything(), failedLoginAttemptsCount: 0, @@ -428,8 +428,8 @@ describe('Testing mfa strategy', () => { passwordHash: await passwordToHash('abcdef'), mfaEnabled: true, phoneNumberVerified: false, - mfaCode: 'zyxwv', - mfaCodeUpdatedAt: new Date(), + singleUseCode: 'zyxwv', + singleUseCodeUpdatedAt: new Date(), }); prisma.userAccounts.update = jest.fn().mockResolvedValue({ id }); @@ -460,8 +460,8 @@ describe('Testing mfa strategy', () => { expect(prisma.userAccounts.update).toHaveBeenCalledWith({ data: { - mfaCode: 'zyxwv', - mfaCodeUpdatedAt: expect.anything(), + singleUseCode: 'zyxwv', + singleUseCodeUpdatedAt: expect.anything(), phoneNumberVerified: false, lastLoginAt: expect.anything(), failedLoginAttemptsCount: 1, @@ -485,8 +485,8 @@ describe('Testing mfa strategy', () => { passwordHash: await passwordToHash('abcdef'), mfaEnabled: true, phoneNumberVerified: false, - mfaCode: 'zyxwv', - mfaCodeUpdatedAt: new Date(0), + singleUseCode: 'zyxwv', + singleUseCodeUpdatedAt: new Date(0), }); prisma.userAccounts.update = jest.fn().mockResolvedValue({ id }); @@ -517,8 +517,8 @@ describe('Testing mfa strategy', () => { expect(prisma.userAccounts.update).toHaveBeenCalledWith({ data: { - mfaCode: 'zyxwv', - mfaCodeUpdatedAt: expect.anything(), + singleUseCode: 'zyxwv', + singleUseCodeUpdatedAt: expect.anything(), phoneNumberVerified: false, lastLoginAt: expect.anything(), failedLoginAttemptsCount: 1, @@ -542,8 +542,8 @@ describe('Testing mfa strategy', () => { passwordHash: await passwordToHash('abcdef'), mfaEnabled: true, phoneNumberVerified: false, - mfaCode: 'zyxwv', - mfaCodeUpdatedAt: new Date(), + singleUseCode: 'zyxwv', + singleUseCodeUpdatedAt: new Date(), }); prisma.userAccounts.update = jest.fn().mockResolvedValue({ id }); @@ -572,8 +572,8 @@ describe('Testing mfa strategy', () => { expect(prisma.userAccounts.update).toHaveBeenCalledWith({ data: { - mfaCode: null, - mfaCodeUpdatedAt: expect.anything(), + singleUseCode: null, + singleUseCodeUpdatedAt: expect.anything(), phoneNumberVerified: true, lastLoginAt: expect.anything(), failedLoginAttemptsCount: 0, @@ -597,8 +597,8 @@ describe('Testing mfa strategy', () => { passwordHash: await passwordToHash('abcdef'), mfaEnabled: true, phoneNumberVerified: false, - mfaCode: 'zyxwv', - mfaCodeUpdatedAt: new Date(), + singleUseCode: 'zyxwv', + singleUseCodeUpdatedAt: new Date(), }); prisma.userAccounts.update = jest.fn().mockResolvedValue({ id }); @@ -627,8 +627,8 @@ describe('Testing mfa strategy', () => { expect(prisma.userAccounts.update).toHaveBeenCalledWith({ data: { - mfaCode: null, - mfaCodeUpdatedAt: expect.anything(), + singleUseCode: null, + singleUseCodeUpdatedAt: expect.anything(), phoneNumberVerified: false, lastLoginAt: expect.anything(), failedLoginAttemptsCount: 0, diff --git a/api/test/unit/services/auth.service.spec.ts b/api/test/unit/services/auth.service.spec.ts index 666eb4835f..54b53f7bfb 100644 --- a/api/test/unit/services/auth.service.spec.ts +++ b/api/test/unit/services/auth.service.spec.ts @@ -483,8 +483,8 @@ describe('Testing auth service', () => { }); expect(prisma.userAccounts.update).toHaveBeenCalledWith({ data: { - mfaCode: expect.anything(), - mfaCodeUpdatedAt: expect.anything(), + singleUseCode: expect.anything(), + singleUseCodeUpdatedAt: expect.anything(), }, where: { id, @@ -532,8 +532,8 @@ describe('Testing auth service', () => { }); expect(prisma.userAccounts.update).toHaveBeenCalledWith({ data: { - mfaCode: expect.anything(), - mfaCodeUpdatedAt: expect.anything(), + singleUseCode: expect.anything(), + singleUseCodeUpdatedAt: expect.anything(), phoneNumber: '520-781-8711', }, where: { @@ -609,7 +609,7 @@ describe('Testing auth service', () => { }); it('should generate mfa code', () => { - expect(authService.generateMfaCode().length).toEqual( + expect(authService.generateSingleUseCode().length).toEqual( Number(process.env.MFA_CODE_LENGTH), ); });