From f4e8ccd3bd2d163508d84986de52126118e3d9ed Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Wed, 11 Dec 2024 14:52:27 -0600 Subject: [PATCH] Parse phone numbers using country codes Co-authored-by: Harry <109690906+harry-signal@users.noreply.github.com> --- .../Registration/PhoneNumberValidator.swift | 41 ++++------ .../RegistrationCoordinatorImpl.swift | 28 +++++-- ...honeNumberConfirmationViewController.swift | 5 +- ...ationChangePhoneNumberViewController.swift | 26 +++---- .../RegistrationPhoneNumberInputView.swift | 11 +-- ...egistrationPhoneNumberViewController.swift | 78 ++++++++++++------- .../RegistrationPhoneNumberViewState.swift | 67 +++++++++------- ...eteAccountConfirmationViewController.swift | 54 ++++++------- Signal/test/PhoneNumberValidatorTest.swift | 4 +- .../RegistrationCoordinatorTest.swift | 10 ++- .../Contacts/PhoneNumberUtil.swift | 18 +++-- .../FindByPhoneNumberViewController.swift | 56 ++++++------- .../RecipientPickers/PhoneNumberCountry.swift | 14 ++-- .../RecipientPickers/RegistrationValues.swift | 2 - 14 files changed, 216 insertions(+), 198 deletions(-) diff --git a/Signal/Registration/PhoneNumberValidator.swift b/Signal/Registration/PhoneNumberValidator.swift index 3588f0c8fa..b761581942 100644 --- a/Signal/Registration/PhoneNumberValidator.swift +++ b/Signal/Registration/PhoneNumberValidator.swift @@ -3,41 +3,28 @@ // SPDX-License-Identifier: AGPL-3.0-only // -public import Foundation -public import SignalServiceKit +import Foundation +import SignalServiceKit -public enum ValidatedCallingCode: Int { - case unitedStates = 1 - case brazil = 55 -} - -public class PhoneNumberValidator: NSObject { - - public func isValidForRegistration(phoneNumber: PhoneNumber) -> Bool { - guard let callingCode = phoneNumber.getCallingCode() else { - return false - } - - guard let validatedCallingCode = ValidatedCallingCode(rawValue: callingCode) else { - // no extra validation for this country - return true - } - - switch validatedCallingCode { - case .brazil: - return isValidForBrazilRegistration(phoneNumber: phoneNumber) - case .unitedStates: +struct PhoneNumberValidator { + func isValidForRegistration(phoneNumber: E164) -> Bool { + if phoneNumber.stringValue.hasPrefix("+1") { return isValidForUnitedStatesRegistration(phoneNumber: phoneNumber) } + if phoneNumber.stringValue.hasPrefix("+55") { + return isValidForBrazilRegistration(phoneNumber: phoneNumber) + } + // no extra validation for this country + return true } private let validBrazilPhoneNumberRegex = try! NSRegularExpression(pattern: "^\\+55\\d{2}9?\\d{8}$", options: []) - private func isValidForBrazilRegistration(phoneNumber: PhoneNumber) -> Bool { - validBrazilPhoneNumberRegex.hasMatch(input: phoneNumber.e164) + private func isValidForBrazilRegistration(phoneNumber: E164) -> Bool { + validBrazilPhoneNumberRegex.hasMatch(input: phoneNumber.stringValue) } private let validUnitedStatesPhoneNumberRegex = try! NSRegularExpression(pattern: "^\\+1\\d{10}$", options: []) - private func isValidForUnitedStatesRegistration(phoneNumber: PhoneNumber) -> Bool { - validUnitedStatesPhoneNumberRegex.hasMatch(input: phoneNumber.e164) + private func isValidForUnitedStatesRegistration(phoneNumber: E164) -> Bool { + validUnitedStatesPhoneNumberRegex.hasMatch(input: phoneNumber.stringValue) } } diff --git a/Signal/Registration/RegistrationCoordinatorImpl.swift b/Signal/Registration/RegistrationCoordinatorImpl.swift index 805a8cb3e9..1bcda8fce5 100644 --- a/Signal/Registration/RegistrationCoordinatorImpl.swift +++ b/Signal/Registration/RegistrationCoordinatorImpl.swift @@ -2128,7 +2128,7 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator { return strongSelf.nextStep() case .invalidArgument: return .value(.phoneNumberEntry(strongSelf.phoneNumberEntryState( - validationError: .invalidNumber(.init(invalidE164: e164)) + validationError: .invalidE164(.init(invalidE164: e164)) ))) case .retryAfter(let timeInterval): if timeInterval < Constants.autoRetryInterval { @@ -3937,20 +3937,34 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator { // MARK: - Step State Generation Helpers + private enum RemoteValidationError { + case invalidE164(RegistrationPhoneNumberViewState.ValidationError.InvalidE164) + case rateLimited(RegistrationPhoneNumberViewState.ValidationError.RateLimited) + + func asViewStateError() -> RegistrationPhoneNumberViewState.ValidationError { + switch self { + case let .invalidE164(error): + return .invalidE164(error) + case let .rateLimited(error): + return .rateLimited(error) + } + } + } + private func phoneNumberEntryState( - validationError: RegistrationPhoneNumberViewState.ValidationError? = nil + validationError: RemoteValidationError? = nil ) -> RegistrationPhoneNumberViewState { switch mode { case .registering: return .registration(.initialRegistration(.init( previouslyEnteredE164: persistedState.e164, - validationError: validationError, + validationError: validationError?.asViewStateError(), canExitRegistration: canExitRegistrationFlow().canExit ))) case .reRegistering(let state): return .registration(.reregistration(.init( e164: state.e164, - validationError: validationError, + validationError: validationError?.asViewStateError(), canExitRegistration: canExitRegistrationFlow().canExit ))) case .changingNumber(let state): @@ -3960,12 +3974,12 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator { break case .rateLimited(let error): rateLimitedError = error - case .invalidNumber(let invalidNumberError): + case .invalidE164(let invalidE164Error): return .changingNumber(.initialEntry(.init( oldE164: state.oldE164, newE164: inMemoryState.changeNumberProspectiveE164, hasConfirmed: inMemoryState.changeNumberProspectiveE164 != nil, - invalidNumberError: invalidNumberError + invalidE164Error: invalidE164Error ))) } if let newE164 = inMemoryState.changeNumberProspectiveE164 { @@ -3979,7 +3993,7 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator { oldE164: state.oldE164, newE164: nil, hasConfirmed: false, - invalidNumberError: nil + invalidE164Error: nil ))) } } diff --git a/Signal/Registration/UserInterface/RegistrationChangePhoneNumberConfirmationViewController.swift b/Signal/Registration/UserInterface/RegistrationChangePhoneNumberConfirmationViewController.swift index d124ddf7e1..8034ac2d07 100644 --- a/Signal/Registration/UserInterface/RegistrationChangePhoneNumberConfirmationViewController.swift +++ b/Signal/Registration/UserInterface/RegistrationChangePhoneNumberConfirmationViewController.swift @@ -169,13 +169,14 @@ class RegistrationChangePhoneNumberConfirmationViewController: OWSViewController phoneNumberStack ]) - if let warningLabelText = state.rateLimitedError?.warningLabelText(e164: self.state.newE164, dateProvider: Date.provider) { + let now = Date() + if let rateLimitedError = state.rateLimitedError, !rateLimitedError.canSubmit(e164: self.state.newE164, dateProvider: { now }) { let warningLabel = UILabel() warningLabel.textColor = .ows_accentRed warningLabel.numberOfLines = 0 warningLabel.font = UIFont.dynamicTypeSubheadlineClamped warningLabel.accessibilityIdentifier = "registration.phonenumber.validationWarningLabel" - warningLabel.text = warningLabelText + warningLabel.text = rateLimitedError.warningLabelText(dateProvider: { now }) rootView.addArrangedSubview(UIView.spacer(withHeight: 12)) rootView.addArrangedSubview(warningLabel) diff --git a/Signal/Registration/UserInterface/RegistrationChangePhoneNumberViewController.swift b/Signal/Registration/UserInterface/RegistrationChangePhoneNumberViewController.swift index de985f6236..e5024fc58c 100644 --- a/Signal/Registration/UserInterface/RegistrationChangePhoneNumberViewController.swift +++ b/Signal/Registration/UserInterface/RegistrationChangePhoneNumberViewController.swift @@ -49,15 +49,15 @@ class RegistrationChangePhoneNumberViewController: OWSTableViewController2 { self.state = newState updateTableContents() - if let invalidNumberError = state.invalidNumberError { - showInvalidPhoneNumberAlertIfNecessary(for: invalidNumberError.invalidE164.stringValue) + if let invalidNumberError = state.invalidE164Error { + showInvalidPhoneNumberAlertIfNecessary(for: invalidNumberError) } } - private var previousInvalidE164: String? + private var previousInvalidNumber: RegistrationPhoneNumberViewState.ValidationError.InvalidE164? - private func showInvalidPhoneNumberAlertIfNecessary(for e164: String) { - let shouldShowAlert = e164 != previousInvalidE164 + private func showInvalidPhoneNumberAlertIfNecessary(for invalidNumber: RegistrationPhoneNumberViewState.ValidationError.InvalidE164) { + let shouldShowAlert = invalidNumber != previousInvalidNumber if shouldShowAlert { OWSActionSheets.showActionSheet( title: OWSLocalizedString( @@ -71,7 +71,7 @@ class RegistrationChangePhoneNumberViewController: OWSTableViewController2 { ) } - previousInvalidE164 = e164 + previousInvalidNumber = invalidNumber } override func viewDidLoad() { @@ -144,11 +144,12 @@ class RegistrationChangePhoneNumberViewController: OWSTableViewController2 { case .newNumber: if let e164 = self.state.newE164, - let warningLabelText = state.invalidNumberError?.warningLabelText(e164: e164) + let invalidE164Error = state.invalidE164Error, + !invalidE164Error.canSubmit(e164: e164) { section.add(.init(customCellBlock: { let cell = OWSTableItem.buildCell( - itemName: warningLabelText, + itemName: invalidE164Error.warningLabelText(), textColor: .ows_accentRed ) cell.isUserInteractionEnabled = false @@ -211,7 +212,6 @@ class RegistrationChangePhoneNumberViewController: OWSTableViewController2 { showInvalidPhoneNumberAlert(isOldValue: isOldValue) return nil case .validNumber(let e164): - return e164 } } @@ -233,7 +233,7 @@ class RegistrationChangePhoneNumberViewController: OWSTableViewController2 { return nil } - guard state.invalidNumberError?.canSubmit(e164: newE164) != false else { + guard state.invalidE164Error?.canSubmit(e164: newE164) != false else { showInvalidPhoneNumberAlert(isOldValue: false) return nil } @@ -411,15 +411,15 @@ private class ChangePhoneNumberValueViews: NSObject { nationalNumber = nationalNumber.asciiDigitsOnly + let phoneNumberUtil = SSKEnvironment.shared.phoneNumberUtilRef guard - let e164 = RegistrationPhoneNumber(country: country, nationalNumber: nationalNumber).e164, - let phoneNumber = SSKEnvironment.shared.phoneNumberUtilRef.parseE164(e164), + let phoneNumber = E164(phoneNumberUtil.parsePhoneNumber(countryCode: country.countryCode, nationalNumber: nationalNumber)?.e164), PhoneNumberValidator().isValidForRegistration(phoneNumber: phoneNumber) else { return .invalidNumber } - return .validNumber(e164: e164) + return .validNumber(e164: phoneNumber) } } diff --git a/Signal/Registration/UserInterface/RegistrationPhoneNumberInputView.swift b/Signal/Registration/UserInterface/RegistrationPhoneNumberInputView.swift index 82f213a6aa..0834eb3bed 100644 --- a/Signal/Registration/UserInterface/RegistrationPhoneNumberInputView.swift +++ b/Signal/Registration/UserInterface/RegistrationPhoneNumberInputView.swift @@ -57,11 +57,8 @@ class RegistrationPhoneNumberInputView: UIStackView { public var nationalNumber: String { nationalNumberView.text?.asciiDigitsOnly ?? "" } - public var e164: E164? { - return RegistrationPhoneNumber( - country: country, - nationalNumber: nationalNumber - ).e164 + public var phoneNumber: RegistrationPhoneNumber { + return RegistrationPhoneNumber(country: country, nationalNumber: nationalNumber) } public var isEnabled: Bool = true { @@ -191,8 +188,8 @@ extension RegistrationPhoneNumberInputView: UITextFieldDelegate { if textField.text.isEmptyOrNil, - let fulle164 = E164(replacementString.removeCharacters(characterSet: CharacterSet(charactersIn: " -()"))), - let phoneNumber = RegistrationPhoneNumberParser(phoneNumberUtil: SSKEnvironment.shared.phoneNumberUtilRef).parseE164(fulle164) + let fullE164 = E164(replacementString.removeCharacters(characterSet: CharacterSet(charactersIn: " -()"))), + let phoneNumber = RegistrationPhoneNumberParser(phoneNumberUtil: SSKEnvironment.shared.phoneNumberUtilRef).parseE164(fullE164) { // If we got a full e164, it was probably from system autofill. // Split out the country code portion. diff --git a/Signal/Registration/UserInterface/RegistrationPhoneNumberViewController.swift b/Signal/Registration/UserInterface/RegistrationPhoneNumberViewController.swift index 6878b3cc19..d04ba3e1ed 100644 --- a/Signal/Registration/UserInterface/RegistrationPhoneNumberViewController.swift +++ b/Signal/Registration/UserInterface/RegistrationPhoneNumberViewController.swift @@ -74,7 +74,10 @@ class RegistrationPhoneNumberViewController: OWSViewController { private var nowTimer: Timer? private var nationalNumber: String { phoneNumberInput.nationalNumber } - private var e164: E164? { phoneNumberInput.e164 } + + private var countryCode: String { + return phoneNumberInput.country.countryCode + } private var localValidationError: RegistrationPhoneNumberViewState.ValidationError? { didSet { render() } @@ -98,23 +101,17 @@ class RegistrationPhoneNumberViewController: OWSViewController { } } - private var canSubmit: Bool { - guard !nationalNumber.isEmpty, let e164 else { + private func canSubmit(isBlockedByValidationError: Bool) -> Bool { + if phoneNumberInput.nationalNumber.isEmpty { return false } switch state { case .initialRegistration: - break + return !isBlockedByValidationError case .reregistration: return true } - - if validationError?.canSubmit(e164: e164, dateProvider: Date.provider) == false { - return false - } - - return true } // MARK: Rendering @@ -192,7 +189,7 @@ class RegistrationPhoneNumberViewController: OWSViewController { switch validationError { case .rateLimited: return false - case nil, .invalidNumber: + case nil, .invalidInput, .invalidE164: break } @@ -274,26 +271,40 @@ class RegistrationPhoneNumberViewController: OWSViewController { contextButton.setImage(Theme.iconImage(.buttonMore), for: .normal) contextButton.tintColor = Theme.accentBlueColor - navigationItem.rightBarButtonItem = canSubmit ? nextBarButton : nil + let now = Date() + + let isBlockedByValidationError = { () -> Bool in + switch validationError { + case let .invalidInput(error): + return !error.canSubmit(countryCode: countryCode, nationalNumber: nationalNumber) + case let .invalidE164(error): + return !error.canSubmit(e164: parseE164()) + case let .rateLimited(error): + return !error.canSubmit(e164: parseE164(), dateProvider: { now }) + case nil: + return false + } + }() + + navigationItem.rightBarButtonItem = canSubmit(isBlockedByValidationError: isBlockedByValidationError) ? nextBarButton : nil phoneNumberInput.isEnabled = canChangePhoneNumber phoneNumberInput.render() // We always render the warning label but sometimes invisibly. This avoids UI jumpiness. - if - let e164, - let warningLabelText = validationError?.warningLabelText(e164: e164, dateProvider: Date.provider) - { + if isBlockedByValidationError, let validationError { validationWarningLabel.alpha = 1 - validationWarningLabel.text = warningLabelText + validationWarningLabel.text = validationError.warningLabelText(dateProvider: { now }) } else { validationWarningLabel.alpha = 0 } switch validationError { case nil, .rateLimited: break - case let .invalidNumber(error): - showInvalidPhoneNumberAlertIfNecessary(for: error.invalidE164.stringValue) + case let .invalidInput(error): + showInvalidPhoneNumberAlertIfNecessary(for: .invalidInput(countryCode: error.invalidCountryCode, nationalNumber: error.invalidNationalNumber)) + case let .invalidE164(error): + showInvalidPhoneNumberAlertIfNecessary(for: .invalidE164(error.invalidE164)) } view.backgroundColor = Theme.backgroundColor @@ -310,10 +321,15 @@ class RegistrationPhoneNumberViewController: OWSViewController { view.layoutSubviews() } - private var previousInvalidE164: String? + private enum InvalidNumberError: Equatable { + case invalidInput(countryCode: String, nationalNumber: String) + case invalidE164(E164) + } - private func showInvalidPhoneNumberAlertIfNecessary(for e164: String) { - let shouldShowAlert = e164 != previousInvalidE164 + private var previousInvalidNumberError: InvalidNumberError? + + private func showInvalidPhoneNumberAlertIfNecessary(for invalidNumberError: InvalidNumberError) { + let shouldShowAlert = invalidNumberError != previousInvalidNumberError if shouldShowAlert { OWSActionSheets.showActionSheet( title: OWSLocalizedString( @@ -327,7 +343,7 @@ class RegistrationPhoneNumberViewController: OWSViewController { ) } - previousInvalidE164 = e164 + previousInvalidNumberError = invalidNumberError } // MARK: Events @@ -337,20 +353,22 @@ class RegistrationPhoneNumberViewController: OWSViewController { goToNextStep() } + private func parseE164() -> E164? { + let phoneNumberUtil = SSKEnvironment.shared.phoneNumberUtilRef + return E164(phoneNumberUtil.parsePhoneNumber(countryCode: countryCode, nationalNumber: nationalNumber)?.e164) + } + private func goToNextStep() { Logger.info("") phoneNumberInput.resignFirstResponder() - guard let e164 = self.e164 else { + guard let e164 = parseE164() else { + localValidationError = .invalidInput(.init(invalidCountryCode: countryCode, invalidNationalNumber: nationalNumber)) return } - - guard - let phoneNumber = SSKEnvironment.shared.phoneNumberUtilRef.parseE164(e164.stringValue), - PhoneNumberValidator().isValidForRegistration(phoneNumber: phoneNumber) - else { - localValidationError = .invalidNumber(.init(invalidE164: e164)) + guard PhoneNumberValidator().isValidForRegistration(phoneNumber: e164) else { + localValidationError = .invalidE164(.init(invalidE164: e164)) return } localValidationError = nil diff --git a/Signal/Registration/UserInterface/RegistrationPhoneNumberViewState.swift b/Signal/Registration/UserInterface/RegistrationPhoneNumberViewState.swift index 9c7adeda04..ecd463aa78 100644 --- a/Signal/Registration/UserInterface/RegistrationPhoneNumberViewState.swift +++ b/Signal/Registration/UserInterface/RegistrationPhoneNumberViewState.swift @@ -38,7 +38,7 @@ public enum RegistrationPhoneNumberViewState: Equatable { let oldE164: E164 let newE164: E164? let hasConfirmed: Bool - let invalidNumberError: ValidationError.InvalidNumber? + let invalidE164Error: ValidationError.InvalidE164? } public struct ChangeNumberConfirmation: Equatable { @@ -48,10 +48,19 @@ public enum RegistrationPhoneNumberViewState: Equatable { } public enum ValidationError: Equatable { - case invalidNumber(InvalidNumber) + case invalidInput(InvalidInput) + case invalidE164(InvalidE164) case rateLimited(RateLimited) - public struct InvalidNumber: Equatable { + /// The user typed something that couldn't be parsed. + public struct InvalidInput: Equatable { + let invalidCountryCode: String + let invalidNationalNumber: String + } + + /// The user submitted something that could be parsed, but local or server + /// validation rejected it as not a valid number for registration. + public struct InvalidE164: Equatable { let invalidE164: E164 } @@ -64,35 +73,39 @@ public enum RegistrationPhoneNumberViewState: Equatable { extension RegistrationPhoneNumberViewState.ValidationError { - func canSubmit(e164: E164, dateProvider: DateProvider) -> Bool { + func warningLabelText(dateProvider: DateProvider) -> String? { switch self { - case let .invalidNumber(error): - return error.canSubmit(e164: e164) + case let .invalidInput(error): + return error.warningLabelText() + case let .invalidE164(error): + return error.warningLabelText() case let .rateLimited(error): - return error.canSubmit(e164: e164, dateProvider: dateProvider) - } - } - - func warningLabelText(e164: E164, dateProvider: DateProvider) -> String? { - switch self { - case .invalidNumber(let error): - return error.warningLabelText(e164: e164) - case let .rateLimited(error): - return error.warningLabelText(e164: e164, dateProvider: dateProvider) + return error.warningLabelText(dateProvider: dateProvider) } } } -extension RegistrationPhoneNumberViewState.ValidationError.InvalidNumber { +extension RegistrationPhoneNumberViewState.ValidationError.InvalidInput { - func canSubmit(e164: E164) -> Bool { + func canSubmit(countryCode: String, nationalNumber: String) -> Bool { + return countryCode != invalidCountryCode || nationalNumber != invalidNationalNumber + } + + func warningLabelText() -> String { + return OWSLocalizedString( + "ONBOARDING_PHONE_NUMBER_VALIDATION_WARNING", + comment: "Label indicating that the phone number is invalid in the 'onboarding phone number' view." + ) + } +} + +extension RegistrationPhoneNumberViewState.ValidationError.InvalidE164 { + + func canSubmit(e164: E164?) -> Bool { return e164 != invalidE164 } - func warningLabelText(e164: E164) -> String? { - guard self.invalidE164 == e164 else { - return nil - } + func warningLabelText() -> String { return OWSLocalizedString( "ONBOARDING_PHONE_NUMBER_VALIDATION_WARNING", comment: "Label indicating that the phone number is invalid in the 'onboarding phone number' view." @@ -102,18 +115,12 @@ extension RegistrationPhoneNumberViewState.ValidationError.InvalidNumber { extension RegistrationPhoneNumberViewState.ValidationError.RateLimited { - func canSubmit(e164: E164, dateProvider: DateProvider) -> Bool { + func canSubmit(e164: E164?, dateProvider: DateProvider) -> Bool { return dateProvider() >= expiration || e164 != self.e164 } - func warningLabelText(e164: E164, dateProvider: DateProvider) -> String? { - guard e164 == self.e164 else { - return nil - } + func warningLabelText(dateProvider: DateProvider) -> String { let now = dateProvider() - if now >= expiration { - return nil - } let rateLimitFormat = OWSLocalizedString( "ONBOARDING_PHONE_NUMBER_RATE_LIMIT_WARNING_FORMAT", comment: "Label indicating that registration has been ratelimited. Embeds {{remaining time string}}." diff --git a/Signal/src/ViewControllers/AppSettings/Account/DeleteAccountConfirmationViewController.swift b/Signal/src/ViewControllers/AppSettings/Account/DeleteAccountConfirmationViewController.swift index a16d5841bb..52df37faf2 100644 --- a/Signal/src/ViewControllers/AppSettings/Account/DeleteAccountConfirmationViewController.swift +++ b/Signal/src/ViewControllers/AppSettings/Account/DeleteAccountConfirmationViewController.swift @@ -7,8 +7,7 @@ import SignalServiceKit import SignalUI class DeleteAccountConfirmationViewController: OWSTableViewController2 { - private var plusPrefixedCallingCode = "+1" - private var countryCode = "US" + private var country: PhoneNumberCountry! private let nationalNumberTextField = UITextField() private let nameLabel = UILabel() @@ -80,7 +79,7 @@ class DeleteAccountConfirmationViewController: OWSTableViewController2 { "DELETE_ACCOUNT_CONFIRMATION_COUNTRY_CODE_TITLE", comment: "Title for the 'country code' row of the 'delete account confirmation' view controller." ), - accessoryText: "\(plusPrefixedCallingCode) (\(countryCode))", + accessoryText: "\(country.plusPrefixedCallingCode) (\(country.countryCode))", actionBlock: { [weak self] in guard let self = self else { return } let countryCodeController = CountryCodeViewController() @@ -162,8 +161,8 @@ class DeleteAccountConfirmationViewController: OWSTableViewController2 { nationalNumberTextField.textColor = Theme.primaryTextColor nationalNumberTextField.font = OWSTableItem.accessoryLabelFont nationalNumberTextField.placeholder = TextFieldFormatting.examplePhoneNumber( - forCountryCode: countryCode, - plusPrefixedCallingCode: plusPrefixedCallingCode, + forCountryCode: country.countryCode, + plusPrefixedCallingCode: country.plusPrefixedCallingCode, includeExampleLabel: false ) @@ -351,7 +350,8 @@ class DeleteAccountConfirmationViewController: OWSTableViewController2 { } var hasEnteredLocalNumber: Bool { - guard let localNumber = DependenciesBridge.shared.tsAccountManager.localIdentifiersWithMaybeSneakyTransaction?.phoneNumber else { + let tsAccountManager = DependenciesBridge.shared.tsAccountManager + guard let localNumber = tsAccountManager.localIdentifiersWithMaybeSneakyTransaction?.phoneNumber else { owsFailDebug("local number unexpectedly nil") return false } @@ -360,40 +360,36 @@ class DeleteAccountConfirmationViewController: OWSTableViewController2 { return false } - let possiblePhoneNumber = plusPrefixedCallingCode + nationalNumber - let possibleNumbers = SSKEnvironment.shared.phoneNumberUtilRef.parsePhoneNumbers( - userSpecifiedText: possiblePhoneNumber, - localPhoneNumber: localNumber - ).map(\.e164) + let phoneNumberUtil = SSKEnvironment.shared.phoneNumberUtilRef + let parsedNumber = phoneNumberUtil.parsePhoneNumber(countryCode: country.countryCode, nationalNumber: nationalNumber) - return possibleNumbers.contains(localNumber) + return localNumber == parsedNumber?.e164 } } extension DeleteAccountConfirmationViewController: CountryCodeViewControllerDelegate { public func countryCodeViewController(_ vc: CountryCodeViewController, didSelectCountry country: PhoneNumberCountry) { - updateCountry(plusPrefixedCallingCode: country.plusPrefixedCallingCode, countryCode: country.countryCode) + updateCountry(country) } private func populateDefaultCountryCode() { - guard let localNumber = DependenciesBridge.shared.tsAccountManager.localIdentifiersWithMaybeSneakyTransaction?.phoneNumber else { - owsFailDebug("Local number unexpectedly nil") - return + let tsAccountManager = DependenciesBridge.shared.tsAccountManager + let phoneNumberUtil = SSKEnvironment.shared.phoneNumberUtilRef + let defaultCountry: PhoneNumberCountry + if + let localNumber = tsAccountManager.localIdentifiersWithMaybeSneakyTransaction?.phoneNumber, + let localCountry = PhoneNumberCountry.buildCountry(forCountryCode: phoneNumberUtil.preferredCountryCode(forLocalNumber: localNumber)) + { + defaultCountry = localCountry + } else { + owsFailDebug("Couldn't determine local country.") + defaultCountry = .defaultValue } - - let (countryCode, callingCode) = SSKEnvironment.shared.phoneNumberUtilRef.preferredCountryAndCallingCode(forLocalNumber: localNumber) - - updateCountry(plusPrefixedCallingCode: "+\(callingCode)", countryCode: countryCode) + updateCountry(defaultCountry) } - private func updateCountry(plusPrefixedCallingCode: String, countryCode: String) { - guard let plusPrefixedCallingCode = plusPrefixedCallingCode.nilIfEmpty, let countryCode = countryCode.nilIfEmpty else { - owsFailDebug("missing calling code for selected country") - return - } - - self.plusPrefixedCallingCode = plusPrefixedCallingCode - self.countryCode = countryCode + private func updateCountry(_ country: PhoneNumberCountry) { + self.country = country updateTableContents() } } @@ -405,7 +401,7 @@ extension DeleteAccountConfirmationViewController: UITextFieldDelegate { } func textField(_ textField: UITextField, shouldChangeCharactersIn range: NSRange, replacementString string: String) -> Bool { - TextFieldFormatting.phoneNumberTextField(textField, changeCharactersIn: range, replacementString: string, plusPrefixedCallingCode: plusPrefixedCallingCode) + TextFieldFormatting.phoneNumberTextField(textField, changeCharactersIn: range, replacementString: string, plusPrefixedCallingCode: country.plusPrefixedCallingCode) return false } } diff --git a/Signal/test/PhoneNumberValidatorTest.swift b/Signal/test/PhoneNumberValidatorTest.swift index 71ccaf061a..d2b8866c36 100644 --- a/Signal/test/PhoneNumberValidatorTest.swift +++ b/Signal/test/PhoneNumberValidatorTest.swift @@ -18,7 +18,7 @@ class PhoneNumberValidatorTest: XCTestCase { func assertValid(e164: String, file: StaticString = #filePath, line: UInt = #line) { let validator = PhoneNumberValidator() - guard let phoneNumber = phoneNumberUtilRef.parseE164(e164) else { + guard let phoneNumber = E164(phoneNumberUtilRef.parseE164(e164)?.e164) else { XCTFail("unparsable phone number", file: file, line: line) return } @@ -28,7 +28,7 @@ class PhoneNumberValidatorTest: XCTestCase { func assertInvalid(e164: String, file: StaticString = #filePath, line: UInt = #line) { let validator = PhoneNumberValidator() - guard let phoneNumber = phoneNumberUtilRef.parsePhoneNumber(userSpecifiedText: e164) else { + guard let phoneNumber = E164(phoneNumberUtilRef.parsePhoneNumber(userSpecifiedText: e164)?.e164) else { // number wasn't even parsable return } diff --git a/Signal/test/Registration/RegistrationCoordinatorTest.swift b/Signal/test/Registration/RegistrationCoordinatorTest.swift index b3c56ef9aa..0981abe8f7 100644 --- a/Signal/test/Registration/RegistrationCoordinatorTest.swift +++ b/Signal/test/Registration/RegistrationCoordinatorTest.swift @@ -3968,7 +3968,7 @@ public class RegistrationCoordinatorTest { case .success: validationError = nil case .invalidArgument: - validationError = .invalidNumber(.init(invalidE164: previouslyEnteredE164 ?? Stubs.e164)) + validationError = .invalidE164(.init(invalidE164: previouslyEnteredE164 ?? Stubs.e164)) case .retryAfter(let timeInterval): validationError = .rateLimited(.init( expiration: self.date.addingTimeInterval(timeInterval), @@ -4006,7 +4006,7 @@ public class RegistrationCoordinatorTest { oldE164: changeNumberParams.oldE164, newE164: nil, hasConfirmed: false, - invalidNumberError: nil + invalidE164Error: nil ))) } case .rateLimited(let error): @@ -4015,12 +4015,14 @@ public class RegistrationCoordinatorTest { newE164: previouslyEnteredE164!, rateLimitedError: error ))) - case .invalidNumber(let error): + case .invalidInput: + owsFail("Can't happen.") + case .invalidE164(let error): return .changingNumber(.initialEntry(.init( oldE164: changeNumberParams.oldE164, newE164: previouslyEnteredE164, hasConfirmed: previouslyEnteredE164 != nil, - invalidNumberError: error + invalidE164Error: error ))) } } diff --git a/SignalServiceKit/Contacts/PhoneNumberUtil.swift b/SignalServiceKit/Contacts/PhoneNumberUtil.swift index 03db72fb5b..d7bbf15bcd 100644 --- a/SignalServiceKit/Contacts/PhoneNumberUtil.swift +++ b/SignalServiceKit/Contacts/PhoneNumberUtil.swift @@ -161,7 +161,7 @@ extension PhoneNumberUtil { return Locale.current.regionCode ?? "US" } - /// Computes the country ("GB") and calling ("44") codes for localNumber. + /// Computes the country code ("GB") for localNumber. /// /// If multiple countries share a calling code (e.g., the US and Canada /// share "1"), then we don't know which country to return for "1". If @@ -169,17 +169,17 @@ extension PhoneNumberUtil { /// return `defaultCountryCode`. /// /// If localNumber can't be parsed, `defaultCountryCode` is returned. - public func preferredCountryAndCallingCode(forLocalNumber localNumber: String) -> (countryCode: String, callingCode: Int) { + public func preferredCountryCode(forLocalNumber localNumber: String) -> String { // TODO: Determine countryCode precisely from the phone number. let defaultCountryCode = Self.defaultCountryCode() if let localCallingCode = parseE164(localNumber)?.getCallingCode() { if getCallingCode(forRegion: defaultCountryCode) == localCallingCode { - return (defaultCountryCode, localCallingCode) + return defaultCountryCode } let localCountryCode = getFilteredRegionCodeForCallingCode(localCallingCode) - return (localCountryCode ?? "", localCallingCode) + return localCountryCode ?? "" } - return (defaultCountryCode, getCallingCode(forRegion: defaultCountryCode)) + return defaultCountryCode } private func format(_ phoneNumber: NBPhoneNumber, numberFormat: NBEPhoneNumberFormat) throws -> String { @@ -302,9 +302,9 @@ extension PhoneNumberUtil { return Locale.current.localizedString(forRegionCode: countryCode)?.nilIfEmpty ?? unknownValue } - private func _parsePhoneNumber(filteredValue: String) -> PhoneNumber? { + private func _parsePhoneNumber(filteredValue: String, countryCode: String = defaultCountryCode()) -> PhoneNumber? { do { - let phoneNumber = try parse(filteredValue, defaultRegion: Self.defaultCountryCode()) + let phoneNumber = try parse(filteredValue, defaultRegion: countryCode) guard nbPhoneNumberUtil.isPossibleNumber(phoneNumber) else { return nil } @@ -330,6 +330,10 @@ extension PhoneNumberUtil { return _parsePhoneNumber(filteredValue: phoneNumber.stringValue) } + public func parsePhoneNumber(countryCode: String, nationalNumber: String) -> PhoneNumber? { + return _parsePhoneNumber(filteredValue: nationalNumber.filteredAsE164, countryCode: countryCode) + } + public func parsePhoneNumbers(userSpecifiedText: String, localPhoneNumber: String?) -> [PhoneNumber] { var results = parsePhoneNumbers(normalizedText: userSpecifiedText, localPhoneNumber: localPhoneNumber) diff --git a/SignalUI/RecipientPickers/FindByPhoneNumberViewController.swift b/SignalUI/RecipientPickers/FindByPhoneNumberViewController.swift index 7075b6b046..581014e8c1 100644 --- a/SignalUI/RecipientPickers/FindByPhoneNumberViewController.swift +++ b/SignalUI/RecipientPickers/FindByPhoneNumberViewController.swift @@ -15,7 +15,7 @@ public class FindByPhoneNumberViewController: OWSTableViewController2 { let buttonText: String? let requiresRegisteredNumber: Bool - var plusPrefixedCallingCode: String = "+1" + private var country: PhoneNumberCountry! let countryCodeLabel = UILabel() private lazy var nationalNumberTextField = OWSTextField( keyboardType: .numberPad, @@ -194,24 +194,14 @@ public class FindByPhoneNumberViewController: OWSTableViewController2 { } func validPhoneNumber() -> String? { - guard let localNumber = DependenciesBridge.shared.tsAccountManager.localIdentifiersWithMaybeSneakyTransaction?.phoneNumber else { - owsFailDebug("local number unexpectedly nil") - return nil - } guard let nationalNumber = nationalNumberTextField.text else { return nil } - let possiblePhoneNumbers = SSKEnvironment.shared.phoneNumberUtilRef.parsePhoneNumbers( - userSpecifiedText: plusPrefixedCallingCode + nationalNumber, - localPhoneNumber: localNumber - ) - let possibleValidPhoneNumbers = possiblePhoneNumbers.map { $0.e164 }.filter { !$0.isEmpty } - - // There should only be one phone number, since we're explicitly specifying - // a country code and therefore parsing a number in e164 format. - owsAssertDebug(possibleValidPhoneNumbers.count <= 1) - - return possibleValidPhoneNumbers.first + let phoneNumberUtil = SSKEnvironment.shared.phoneNumberUtilRef + return phoneNumberUtil.parsePhoneNumber( + countryCode: country.countryCode, + nationalNumber: nationalNumber + )?.e164.nilIfEmpty } func hasValidPhoneNumber() -> Bool { @@ -276,7 +266,7 @@ extension FindByPhoneNumberViewController: SheetDismissalDelegate { extension FindByPhoneNumberViewController: CountryCodeViewControllerDelegate { public func countryCodeViewController(_ vc: CountryCodeViewController, didSelectCountry country: PhoneNumberCountry) { - updateCountry(plusPrefixedCallingCode: country.plusPrefixedCallingCode, countryCode: country.countryCode) + updateCountry(country) } private func didTapCountryRow() { @@ -286,31 +276,31 @@ extension FindByPhoneNumberViewController: CountryCodeViewControllerDelegate { } private func populateDefaultCountryCode() { - guard let localNumber = DependenciesBridge.shared.tsAccountManager.localIdentifiersWithMaybeSneakyTransaction?.phoneNumber else { - owsFailDebug("Local number unexpectedly nil") - return + let tsAccountManager = DependenciesBridge.shared.tsAccountManager + let phoneNumberUtil = SSKEnvironment.shared.phoneNumberUtilRef + let defaultCountry: PhoneNumberCountry + if + let localNumber = tsAccountManager.localIdentifiersWithMaybeSneakyTransaction?.phoneNumber, + let localCountry = PhoneNumberCountry.buildCountry(forCountryCode: phoneNumberUtil.preferredCountryCode(forLocalNumber: localNumber)) + { + defaultCountry = localCountry + } else { + owsFailDebug("Couldn't determine local country.") + defaultCountry = .defaultValue } - - let (countryCode, callingCode) = SSKEnvironment.shared.phoneNumberUtilRef.preferredCountryAndCallingCode(forLocalNumber: localNumber) - - updateCountry(plusPrefixedCallingCode: "+\(callingCode)", countryCode: countryCode) + updateCountry(defaultCountry) } - private func updateCountry(plusPrefixedCallingCode: String, countryCode: String) { - guard let plusPrefixedCallingCode = plusPrefixedCallingCode.nilIfEmpty, let countryCode = countryCode.nilIfEmpty else { - owsFailDebug("missing calling code for selected country") - return - } - - self.plusPrefixedCallingCode = plusPrefixedCallingCode + private func updateCountry(_ country: PhoneNumberCountry) { + self.country = country let labelFormat = CurrentAppContext().isRTL ? "(%2$@) %1$@" : "%1$@ (%2$@)" - countryCodeLabel.text = String(format: labelFormat, plusPrefixedCallingCode, countryCode.localizedUppercase) + countryCodeLabel.text = String(format: labelFormat, country.plusPrefixedCallingCode, country.countryCode.localizedUppercase) } } extension FindByPhoneNumberViewController: UITextFieldDelegate { public func textField(_ textField: UITextField, shouldChangeCharactersIn range: NSRange, replacementString string: String) -> Bool { - TextFieldFormatting.phoneNumberTextField(textField, changeCharactersIn: range, replacementString: string, plusPrefixedCallingCode: plusPrefixedCallingCode) + TextFieldFormatting.phoneNumberTextField(textField, changeCharactersIn: range, replacementString: string, plusPrefixedCallingCode: country.plusPrefixedCallingCode) updateButtonState() return false } diff --git a/SignalUI/RecipientPickers/PhoneNumberCountry.swift b/SignalUI/RecipientPickers/PhoneNumberCountry.swift index 1941d8540b..34e181426f 100644 --- a/SignalUI/RecipientPickers/PhoneNumberCountry.swift +++ b/SignalUI/RecipientPickers/PhoneNumberCountry.swift @@ -45,14 +45,18 @@ public struct PhoneNumberCountry: Equatable { owsFailDebug("Invalid countryCode.") return nil } - guard let plusPrefixedCallingCode = SSKEnvironment.shared.phoneNumberUtilRef.plusPrefixedCallingCode(fromCountryCode: countryCode) else { - owsFailDebug("Invalid callingCode.") - return nil - } - return buildCountry(countryCode: countryCode, plusPrefixedCallingCode: plusPrefixedCallingCode) + return buildCountry(forCountryCode: countryCode) } } + public static func buildCountry(forCountryCode countryCode: String) -> PhoneNumberCountry? { + guard let plusPrefixedCallingCode = SSKEnvironment.shared.phoneNumberUtilRef.plusPrefixedCallingCode(fromCountryCode: countryCode) else { + owsFailDebug("Invalid countryCode.") + return nil + } + return buildCountry(countryCode: countryCode, plusPrefixedCallingCode: plusPrefixedCallingCode) + } + public static func buildCountry(forCallingCode callingCode: Int) -> PhoneNumberCountry? { let phoneNumberUtil = SSKEnvironment.shared.phoneNumberUtilRef diff --git a/SignalUI/RecipientPickers/RegistrationValues.swift b/SignalUI/RecipientPickers/RegistrationValues.swift index 72805e358a..084e10855f 100644 --- a/SignalUI/RecipientPickers/RegistrationValues.swift +++ b/SignalUI/RecipientPickers/RegistrationValues.swift @@ -9,12 +9,10 @@ public import SignalServiceKit public struct RegistrationPhoneNumber { public let country: PhoneNumberCountry public let nationalNumber: String - public let e164: E164? public init(country: PhoneNumberCountry, nationalNumber: String) { self.country = country self.nationalNumber = nationalNumber - self.e164 = E164("\(country.plusPrefixedCallingCode)\(nationalNumber)") } }