Skip to content

Commit

Permalink
fix(core): add channel validation (#4734)
Browse files Browse the repository at this point in the history
* fix(core): add channel validation

* test: use enum in valiadtions

* fix: allow test numbers
  • Loading branch information
dolcalmi authored Feb 5, 2025
1 parent 7a3a2fd commit 8671344
Show file tree
Hide file tree
Showing 8 changed files with 192 additions and 28 deletions.
34 changes: 28 additions & 6 deletions core/api/src/app/authentication/request-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
getGeetestConfig,
getTestAccounts,
} from "@/config"
import { checkedToChannel } from "@/domain/phone-provider"
import { TestAccountsChecker } from "@/domain/accounts/test-accounts-checker"
import { PhoneAlreadyExistsError } from "@/domain/authentication/errors"
import { NotImplementedError } from "@/domain/errors"
Expand All @@ -29,7 +30,7 @@ export const requestPhoneCodeWithCaptcha = async ({
geetestValidate: string
geetestSeccode: string
ip: IpAddress
channel: ChannelType
channel: string
}): Promise<true | ApplicationError> => {
const geeTestConfig = getGeetestConfig()
const geetest = Geetest(geeTestConfig)
Expand Down Expand Up @@ -67,7 +68,14 @@ export const requestPhoneCodeWithCaptcha = async ({
const user = await UsersRepository().findByPhone(phone)
const phoneExists = !(user instanceof Error)

return TwilioClient().initiateVerify({ to: phone, channel, phoneExists })
const checkedChannel = checkedToChannel(phone, channel)
if (checkedChannel instanceof Error) return checkedChannel

return TwilioClient().initiateVerify({
to: phone,
channel: checkedChannel,
phoneExists,
})
}

export const requestPhoneCodeForAuthedUser = async ({
Expand All @@ -78,7 +86,7 @@ export const requestPhoneCodeForAuthedUser = async ({
}: {
phone: PhoneNumber
ip: IpAddress
channel: ChannelType
channel: string
user: User
}): Promise<true | PhoneProviderServiceError> => {
{
Expand Down Expand Up @@ -108,7 +116,14 @@ export const requestPhoneCodeForAuthedUser = async ({
return true
}

return TwilioClient().initiateVerify({ to: phone, channel, phoneExists: false })
const checkedChannel = checkedToChannel(phone, channel)
if (checkedChannel instanceof Error) return checkedChannel

return TwilioClient().initiateVerify({
to: phone,
channel: checkedChannel,
phoneExists: false,
})
}

export const requestPhoneCodeWithAppcheckJti = async ({
Expand All @@ -119,7 +134,7 @@ export const requestPhoneCodeWithAppcheckJti = async ({
}: {
phone: PhoneNumber
ip: IpAddress
channel: ChannelType
channel: string
appcheckJti: string
}): Promise<true | PhoneProviderServiceError> => {
{
Expand Down Expand Up @@ -155,7 +170,14 @@ export const requestPhoneCodeWithAppcheckJti = async ({
const user = await UsersRepository().findByPhone(phone)
const phoneExists = !(user instanceof Error)

return TwilioClient().initiateVerify({ to: phone, channel, phoneExists })
const checkedChannel = checkedToChannel(phone, channel)
if (checkedChannel instanceof Error) return checkedChannel

return TwilioClient().initiateVerify({
to: phone,
channel: checkedChannel,
phoneExists,
})
}

export const requestEmailCode = async ({
Expand Down
2 changes: 2 additions & 0 deletions core/api/src/domain/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ export class InvalidContactAlias extends ValidationError {}
export class InvalidCoordinatesError extends ValidationError {}
export class InvalidBusinessTitleLengthError extends ValidationError {}
export class InvalidPhoneNumber extends ValidationError {}
export class InvalidChannel extends ValidationError {}
export class InvalidChannelForCountry extends ValidationError {}
export class InvalidUserId extends ValidationError {}
export class InvalidLedgerExternalId extends ValidationError {}
export class InvalidEmailAddress extends ValidationError {}
Expand Down
49 changes: 49 additions & 0 deletions core/api/src/domain/phone-provider/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
import { parsePhoneNumberFromString } from "libphonenumber-js"

import {
InvalidPhoneNumber,
InvalidChannel,
InvalidChannelForCountry,
} from "@/domain/errors"
import {
getSmsAuthUnsupportedCountries,
getWhatsAppAuthUnsupportedCountries,
} from "@/config"

export * from "./errors"

// from https://www.twilio.com/docs/lookup/v2-api/line-type-intelligence#type-property-values
Expand Down Expand Up @@ -27,3 +39,40 @@ export const ChannelType = {
Sms: "sms",
Whatsapp: "whatsapp",
} as const

const CHANNEL_CONFIG = {
[ChannelType.Sms]: getSmsAuthUnsupportedCountries,
[ChannelType.Whatsapp]: getWhatsAppAuthUnsupportedCountries,
} as const

export const checkedToChannel = (
phone: string,
channel: string,
): ChannelType | ValidationError => {
if (!phone) {
return new InvalidPhoneNumber(phone)
}

if (!channel) {
return new InvalidChannel(channel)
}

const phoneNumber = parsePhoneNumberFromString(phone)
if (!phoneNumber?.country) {
return new InvalidPhoneNumber(phone)
}

const normalizedChannel = channel.toLowerCase()
const getUnsupportedCountries =
CHANNEL_CONFIG[normalizedChannel as keyof typeof CHANNEL_CONFIG]

if (!getUnsupportedCountries) {
return new InvalidChannel(channel)
}

if (getUnsupportedCountries().includes(phoneNumber.country as CountryCode)) {
return new InvalidChannelForCountry(channel)
}

return normalizedChannel as ChannelType
}
5 changes: 5 additions & 0 deletions core/api/src/graphql/error-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,11 @@ export const mapError = (error: ApplicationError): CustomGraphQLError => {
message = "Invalid invoice amount"
return new ValidationInternalError({ message, logger: baseLogger })

case "InvalidChannel":
case "InvalidChannelForCountry":
message = "Invalid channel"
return new ValidationInternalError({ message, logger: baseLogger })

// ----------
// Unhandled below here
// ----------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import Phone from "@/graphql/shared/types/scalar/phone"
import SuccessPayload from "@/graphql/shared/types/payload/success-payload"
import { Authentication } from "@/app"
import { mapAndParseErrorForGqlResponse } from "@/graphql/error-map"
import { ChannelType } from "@/domain/phone-provider"
import PhoneCodeChannelType from "@/graphql/shared/types/scalar/phone-code-channel-type"
import { InputValidationError } from "@/graphql/error"

Expand Down Expand Up @@ -45,7 +44,7 @@ const CaptchaRequestAuthCodeMutation = GT.Field<
challengeCode: geetestChallenge,
validationCode: geetestValidate,
secCode: geetestSeccode,
channel: channelInput,
channel,
} = args.input

if (phone instanceof Error) return { errors: [{ message: phone.message }] }
Expand All @@ -55,16 +54,12 @@ const CaptchaRequestAuthCodeMutation = GT.Field<
return { errors: [{ message: geetestValidate.message }] }
if (geetestSeccode instanceof Error)
return { errors: [{ message: geetestSeccode.message }] }
if (channelInput instanceof Error)
return { errors: [{ message: channelInput.message }] }
if (channel instanceof Error) return { errors: [{ message: channel.message }] }

if (ip === undefined) {
return { errors: [{ message: "ip is undefined" }] }
}

let channel: ChannelType = ChannelType.Sms
if (channelInput === "WHATSAPP") channel = ChannelType.Whatsapp

const result = await Authentication.requestPhoneCodeWithCaptcha({
phone,
geetestChallenge,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const UserPhoneRegistrationInitiateMutation = GT.Field<
input: { type: GT.NonNull(UserPhoneRegistrationInitiateInput) },
},
resolve: async (_, args, { ip, user }) => {
const { phone, channel: channelInput } = args.input
const { phone, channel } = args.input

if (ip === undefined) {
return { errors: [{ message: "ip is undefined" }] }
Expand All @@ -44,12 +44,7 @@ const UserPhoneRegistrationInitiateMutation = GT.Field<
return { errors: [{ message: phone.message }] }
}

if (channelInput instanceof Error)
return { errors: [{ message: channelInput.message }] }

let channel: ChannelType = ChannelType.Sms
if (channelInput?.toLowerCase() === ChannelType.Whatsapp)
channel = ChannelType.Whatsapp
if (channel instanceof Error) return { errors: [{ message: channel.message }] }

const success = await Authentication.requestPhoneCodeForAuthedUser({
phone,
Expand Down
10 changes: 2 additions & 8 deletions core/api/src/servers/authentication/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,7 @@ authRouter.post("/phone/code", async (req: Request, res: Response) => {
const challengeCodeRaw = req.body.challengeCode
const validationCodeRaw = req.body.validationCode
const secCodeRaw = req.body.secCode

// TODO: proper validation
const channel =
typeof req.body.channel === "string" ? req.body.channel.toLowerCase() : "sms"
const channel = req.body.channel

if (!phoneRaw || !challengeCodeRaw || !validationCodeRaw || !secCodeRaw)
return res.status(400).send({ error: "missing inputs" })
Expand Down Expand Up @@ -309,10 +306,7 @@ authRouter.post("/phone/code-appcheck", async (req: Request, res: Response) => {

const ip = req.originalIp
const phoneRaw = req.body.phone

// TODO: proper validation
const channel =
typeof req.body.channel === "string" ? req.body.channel.toLowerCase() : "sms"
const channel = req.body.channel

if (!phoneRaw) {
const error = "missing phone input"
Expand Down
102 changes: 102 additions & 0 deletions core/api/test/unit/domain/phone-provider/checked-to-channel.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { checkedToChannel, ChannelType } from "@/domain/phone-provider"
import {
InvalidPhoneNumber,
InvalidChannel,
InvalidChannelForCountry,
} from "@/domain/errors"

jest.mock("@/config", () => ({
...jest.requireActual("@/config"),
getSmsAuthUnsupportedCountries: () => ["GB"],
getWhatsAppAuthUnsupportedCountries: () => ["US"],
}))

describe("checkedToChannel", () => {
const validUsPhone = "+16505554321"
const validUkPhone = "+447874482105"

it("fails with empty phone", () => {
const result = checkedToChannel("", "sms")
expect(result).toBeInstanceOf(InvalidPhoneNumber)
})

it("fails with empty channel", () => {
const result = checkedToChannel(validUsPhone, "")
expect(result).toBeInstanceOf(InvalidChannel)
})

it("fails with invalid phone number", () => {
const result = checkedToChannel("+1", "sms")
expect(result).toBeInstanceOf(InvalidPhoneNumber)
})

it("fails with non-phone-number input", () => {
const result = checkedToChannel("not-a-phone", "sms")
expect(result).toBeInstanceOf(InvalidPhoneNumber)
})

it("fails with invalid channel type", () => {
const result = checkedToChannel(validUsPhone, "invalid-channel")
expect(result).toBeInstanceOf(InvalidChannel)
})

describe("SMS channel", () => {
it("succeeds for supported country", () => {
const result = checkedToChannel(validUsPhone, "sms")
expect(result).toBe(ChannelType.Sms)
})

it("fails for unsupported country", () => {
const result = checkedToChannel(validUkPhone, "sms")
expect(result).toBeInstanceOf(InvalidChannelForCountry)
})

it("is case insensitive", () => {
const result = checkedToChannel(validUsPhone, "SMS")
expect(result).toBe(ChannelType.Sms)
})
})

describe("WhatsApp channel", () => {
it("succeeds for supported country", () => {
const result = checkedToChannel(validUkPhone, "whatsapp")
expect(result).toBe(ChannelType.Whatsapp)
})

it("fails for unsupported country", () => {
const result = checkedToChannel(validUsPhone, "whatsapp")
expect(result).toBeInstanceOf(InvalidChannelForCountry)
})

it("is case insensitive", () => {
const result = checkedToChannel(validUkPhone, "WhatsApp")
expect(result).toBe(ChannelType.Whatsapp)
})
})

describe("edge cases", () => {
it("handles undefined phone", () => {
// @ts-expect-error Testing undefined case
const result = checkedToChannel(undefined, "sms")
expect(result).toBeInstanceOf(InvalidPhoneNumber)
})

it("handles undefined channel", () => {
// @ts-expect-error Testing undefined case
const result = checkedToChannel(validUsPhone, undefined)
expect(result).toBeInstanceOf(InvalidChannel)
})

it("handles null phone", () => {
// @ts-expect-error Testing null case
const result = checkedToChannel(null, "sms")
expect(result).toBeInstanceOf(InvalidPhoneNumber)
})

it("handles null channel", () => {
// @ts-expect-error Testing null case
const result = checkedToChannel(validUsPhone, null)
expect(result).toBeInstanceOf(InvalidChannel)
})
})
})

0 comments on commit 8671344

Please sign in to comment.