Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Normalise phoneNumber received in the input before processing #881

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres
to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [7.0.12] - 2023-11-16

- Adds Phone Number normalisation

## [7.0.11] - 2023-11-10

- Fixes email verification behaviour with user id mapping
Expand Down
5 changes: 4 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ compileTestJava { options.encoding = "UTF-8" }
// }
//}

version = "7.0.11"
version = "7.0.12"


repositories {
Expand Down Expand Up @@ -71,6 +71,9 @@ dependencies {
// https://mvnrepository.com/artifact/commons-codec/commons-codec
implementation group: 'commons-codec', name: 'commons-codec', version: '1.15'
anku255 marked this conversation as resolved.
Show resolved Hide resolved

// https://mvnrepository.com/artifact/com.googlecode.libphonenumber/libphonenumber/
implementation group: 'com.googlecode.libphonenumber', name: 'libphonenumber', version: '8.13.25'

compileOnly project(":supertokens-plugin-interface")
testImplementation project(":supertokens-plugin-interface")

Expand Down
5 changes: 5 additions & 0 deletions implementationDependencies.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@
"jar": "https://repo1.maven.org/maven2/commons-codec/commons-codec/1.15/commons-codec-1.15.jar",
"name": "Commons Codec 1.15",
"src": "https://repo1.maven.org/maven2/commons-codec/commons-codec/1.15/commons-codec-1.15-sources.jar"
},
{
"jar": "https://repo1.maven.org/maven2/com/googlecode/libphonenumber/libphonenumber/8.13.25/libphonenumber-8.13.25.jar",
"name": "Libphonenumber 8.13.25",
"src": "https://repo1.maven.org/maven2/com/googlecode/libphonenumber/libphonenumber/8.13.25/libphonenumber-8.13.25-sources.jar"
anku255 marked this conversation as resolved.
Show resolved Hide resolved
}
]
}
54 changes: 49 additions & 5 deletions src/main/java/io/supertokens/utils/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.i18n.phonenumbers.PhoneNumberUtil;
import com.google.i18n.phonenumbers.NumberParseException;
import com.google.i18n.phonenumbers.Phonenumber;
import io.supertokens.Main;
import io.supertokens.config.Config;
import io.supertokens.jwt.exceptions.UnsupportedJWTSigningAlgorithmException;
Expand Down Expand Up @@ -55,6 +58,42 @@

public class Utils {

/**
* Normalizes a phone number by trimming and formatting it according to the
* E.164 standard.
* <p>
* The function attempts to parse the given phone number using libphonenumber.
* If parsing is successful,
* it formats the phone number according to the E.164 standard. If parsing fails
* (throws a NumberParseException),
* it still trims the input and returns the original trimmed phone number.
*
* @param phoneNumber The input phone number to be normalized.
* @return The normalized phone number or the original trimmed phone number if
* it cannot be parsed.
*/
public static String normalizeIfPhoneNumber(String phoneNumber) {
if (phoneNumber == null) {
return null;
}

PhoneNumberUtil phoneNumberUtil = PhoneNumberUtil.getInstance();

try {
// Attempt to parse the phone number with default region code "ZZ" (unknown
// region)
Phonenumber.PhoneNumber parsedPhoneNumber = phoneNumberUtil.parse(phoneNumber.trim(), "ZZ");

// Format the parsed phone number according to E.164 standard
phoneNumber = phoneNumberUtil.format(parsedPhoneNumber, PhoneNumberUtil.PhoneNumberFormat.E164);
} catch (NumberParseException e) {
// Parsing failed, use the original trimmed phone number
phoneNumber = phoneNumber.trim();
}

return phoneNumber;
}

public static String normaliseEmail(String email) {
if (email == null) {
return null;
Expand All @@ -79,7 +118,8 @@ public static String convertToBase64(String str) {

// This function deserializes both B64 and B64URL encodings
public static String convertFromBase64(String str) {
return new String(Base64.getDecoder().decode(stringToBytes(str.replace("-", "+").replace("_", "/"))), StandardCharsets.UTF_8);
anku255 marked this conversation as resolved.
Show resolved Hide resolved
return new String(Base64.getDecoder().decode(stringToBytes(str.replace("-", "+").replace("_", "/"))),
StandardCharsets.UTF_8);
}

public static String throwableStacktraceToString(Throwable e) {
Expand Down Expand Up @@ -282,10 +322,13 @@ public static class PubPriKey {
}

public PubPriKey(String s) {
// We split by both | and ; because in old versions we used to use ";" in dynamic and "|" in static keys
// Now we are consolidating all of them to use "|", but by handling legacy keys, we can avoid the need
// We split by both | and ; because in old versions we used to use ";" in
// dynamic and "|" in static keys
// Now we are consolidating all of them to use "|", but by handling legacy keys,
// we can avoid the need
// for manual key migration.
// I.e.: this way only people who set access_token_signing_key_dynamic to false has to do manual
// I.e.: this way only people who set access_token_signing_key_dynamic to false
// has to do manual
// migration instead of everyone.
// for everyone else, the key rotation should get it done.
String[] parts = s.split("[|;]");
Expand Down Expand Up @@ -338,7 +381,8 @@ public static JsonObject addLegacySigningKeyInfos(AppIdentifier appIdentifier, M
TenantOrAppNotFoundException {
if (Config.getConfig(appIdentifier.getAsPublicTenantIdentifier(), main).getAccessTokenSigningKeyDynamic()) {
result.addProperty("jwtSigningPublicKey",
new Utils.PubPriKey(SigningKeys.getInstance(appIdentifier, main).getLatestIssuedDynamicKey().value).publicKey);
new Utils.PubPriKey(
SigningKeys.getInstance(appIdentifier, main).getLatestIssuedDynamicKey().value).publicKey);
result.addProperty("jwtSigningPublicKeyExpiryTime",
SigningKeys.getInstance(appIdentifier, main).getDynamicSigningKeyExpiryTime());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public String getPath() {
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException, ServletException {
// API is tenant specific.
String email = InputParser.getQueryParamOrThrowError(req, "email", true);
String phoneNumber = InputParser.getQueryParamOrThrowError(req, "phoneNumber", true);
String phoneNumber = Utils.normalizeIfPhoneNumber(
InputParser.getQueryParamOrThrowError(req, "phoneNumber", true));
String thirdPartyId = InputParser.getQueryParamOrThrowError(req, "thirdPartyId", true);
String thirdPartyUserId = InputParser.getQueryParamOrThrowError(req, "thirdPartyUserId", true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
String email = input.has("email")
? Utils.normaliseEmail(InputParser.parseStringOrThrowError(input, "email", false))
: null;
String phoneNumber = InputParser.parseStringOrThrowError(input, "phoneNumber", true);
String phoneNumber = Utils.normalizeIfPhoneNumber(
InputParser.parseStringOrThrowError(input, "phoneNumber", true));
String deviceId = InputParser.parseStringOrThrowError(input, "deviceId", true);

if (Stream.of(email, phoneNumber, deviceId).filter(Objects::nonNull).count() != 1) {
Expand All @@ -77,7 +78,8 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
}

try {
CreateCodeResponse createCodeResponse = Passwordless.createCode(this.getTenantIdentifierWithStorageFromRequest(req), main, email,
CreateCodeResponse createCodeResponse = Passwordless.createCode(
this.getTenantIdentifierWithStorageFromRequest(req), main, email,
phoneNumber, deviceId,
userInputCode);
long passwordlessCodeLifetime = Config.getConfig(this.getTenantIdentifierWithStorageFromRequest(req), main)
Expand All @@ -104,7 +106,8 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
JsonObject result = new JsonObject();
result.addProperty("status", "USER_INPUT_CODE_ALREADY_USED_ERROR");
super.sendJsonResponse(200, result, resp);
} catch (StorageQueryException | NoSuchAlgorithmException | InvalidKeyException | TenantOrAppNotFoundException | BadPermissionException e) {
} catch (StorageQueryException | NoSuchAlgorithmException | InvalidKeyException | TenantOrAppNotFoundException |
BadPermissionException e) {
throw new ServletException(e);
} catch (Base64EncodingException ex) {
throw new ServletException(new BadRequestException("Input encoding error in " + ex.source));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
JsonObject input = InputParser.parseJsonObjectOrThrowError(req);

String email = InputParser.parseStringOrThrowError(input, "email", true);
String phoneNumber = InputParser.parseStringOrThrowError(input, "phoneNumber", true);

String phoneNumber = Utils.normalizeIfPhoneNumber(
InputParser.parseStringOrThrowError(input, "phoneNumber", true));
if (phoneNumber != null && email != null) {
throw new ServletException(new BadRequestException("Please provide exactly one of email or phoneNumber"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO
// logic based on: https://app.code2flow.com/Odo88u7TNKIk

String email = InputParser.getQueryParamOrThrowError(req, "email", true);
String phoneNumber = InputParser.getQueryParamOrThrowError(req, "phoneNumber", true);
String phoneNumber = Utils.normalizeIfPhoneNumber(InputParser.getQueryParamOrThrowError(req, "phoneNumber", true));
String deviceId = InputParser.getQueryParamOrThrowError(req, "deviceId", true);
String deviceIdHash = InputParser.getQueryParamOrThrowError(req, "preAuthSessionId", true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO
// logic based on: https://app.code2flow.com/flowcharts/617a9aafdc97ee415448db74
String userId = InputParser.getQueryParamOrThrowError(req, "userId", true);
String email = InputParser.getQueryParamOrThrowError(req, "email", true);
String phoneNumber = InputParser.getQueryParamOrThrowError(req, "phoneNumber", true);
String phoneNumber = Utils.normalizeIfPhoneNumber(InputParser.getQueryParamOrThrowError(req, "phoneNumber", true));

if (Stream.of(userId, email, phoneNumber).filter(Objects::nonNull).count() != 1) {
throw new ServletException(
Expand Down Expand Up @@ -149,7 +149,7 @@ protected void doPut(HttpServletRequest req, HttpServletResponse resp) throws IO

FieldUpdate phoneNumberUpdate = !input.has("phoneNumber") ? null
: new FieldUpdate(input.get("phoneNumber").isJsonNull() ? null
: InputParser.parseStringOrThrowError(input, "phoneNumber", false));
: Utils.normalizeIfPhoneNumber(InputParser.parseStringOrThrowError(input, "phoneNumber", false)));

try {
AppIdentifierWithStorageAndUserIdMapping appIdentifierWithStorageAndUserIdMapping =
Expand Down
57 changes: 52 additions & 5 deletions src/test/java/io/supertokens/test/UtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.junit.Test;
import org.junit.rules.TestRule;

import static org.junit.Assert.*;

public class UtilsTest {
@Rule
public TestRule watchman = Utils.getOnFailure();
Expand All @@ -38,23 +40,68 @@ public void beforeEach() {

@Test
public void encodeDecodeBase64WithUTF() {
assert (io.supertokens.utils.Utils.convertFromBase64(io.supertokens.utils.Utils.convertToBase64("łukasz 馬 / 马"))
assert (io.supertokens.utils.Utils.convertFromBase64(
io.supertokens.utils.Utils.convertToBase64("łukasz 馬 / 马"))
.equals("łukasz 馬 / 马"));
}

@Test
public void pubPriKeyShouldHandleSemicolonSeparator() {
io.supertokens.utils.Utils.PubPriKey parsed = new io.supertokens.utils.Utils.PubPriKey("pub;pri");

assert ( parsed.privateKey.equals("pri"));
assert ( parsed.publicKey.equals("pub"));
assert (parsed.privateKey.equals("pri"));
assert (parsed.publicKey.equals("pub"));
}

@Test
public void pubPriKeyShouldHandleBarSeparator() {
io.supertokens.utils.Utils.PubPriKey parsed = new io.supertokens.utils.Utils.PubPriKey("pub|pri");

assert ( parsed.privateKey.equals("pri"));
assert ( parsed.publicKey.equals("pub"));
assert (parsed.privateKey.equals("pri"));
assert (parsed.publicKey.equals("pub"));
}

@Test
public void testNormalizeValidPhoneNumber() {
{
String inputPhoneNumber = "+1 650-555-1234";
String expectedNormalizedPhoneNumber = "+16505551234";
String actualNormalizedPhoneNumber = io.supertokens.utils.Utils.normalizeIfPhoneNumber(inputPhoneNumber);
assertEquals(expectedNormalizedPhoneNumber, actualNormalizedPhoneNumber);
}
{
String inputPhoneNumber = "+640223334444";
String expectedNormalizedPhoneNumber = "+64223334444";
String actualNormalizedPhoneNumber = io.supertokens.utils.Utils.normalizeIfPhoneNumber(inputPhoneNumber);
assertEquals(expectedNormalizedPhoneNumber, actualNormalizedPhoneNumber);
}
}

@Test
public void testNormalizeInvalidPhoneNumber() {
String inputPhoneNumber = "ThisIsNotAPhoneNumber";
String expectedTrimmedPhoneNumber = "ThisIsNotAPhoneNumber";
String actualNormalizedPhoneNumber = io.supertokens.utils.Utils.normalizeIfPhoneNumber(inputPhoneNumber);
assertEquals(expectedTrimmedPhoneNumber, actualNormalizedPhoneNumber);
}

@Test
public void testNormalizeNullPhoneNumber() {
String inputPhoneNumber = null;
assertNull(io.supertokens.utils.Utils.normalizeIfPhoneNumber(inputPhoneNumber));
}

@Test
public void testNormalizeEmptyPhoneNumber() {
// Test with an empty input
String inputPhoneNumber = "";
assertEquals("", io.supertokens.utils.Utils.normalizeIfPhoneNumber(inputPhoneNumber));
}

@Test
public void testNormalizeWhitespacePhoneNumber() {
// Test with a phone number containing only whitespace
String inputPhoneNumber = " ";
assertEquals("", io.supertokens.utils.Utils.normalizeIfPhoneNumber(inputPhoneNumber));
}
rishabhpoddar marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,33 @@ public void testListUsersByAccountInfoForUnlinkedAccounts() throws Exception {
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
}

@Test
public void testListUserByAccountInfoByUnnormalisedPhoneNumber() throws Exception {
String[] args = {"../"};
TestingProcessManager.TestingProcess process = TestingProcessManager.start(args, false);
FeatureFlagTestContent.getInstance(process.getProcess())
.setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{
EE_FEATURES.ACCOUNT_LINKING, EE_FEATURES.MULTI_TENANCY});
process.startProcess();
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED));

if (StorageLayer.getStorage(process.getProcess()).getType() != STORAGE_TYPE.SQL) {
return;
}

String phoneNumber = "+44-207 183 8750";
String normalisedPhoneNumber = io.supertokens.utils.Utils.normalizeIfPhoneNumber(phoneNumber);

AuthRecipeUserInfo user = createPasswordlessUserWithPhone(process.getProcess(), normalisedPhoneNumber);

JsonObject userJSON = getUserById(process.getProcess(), user.getSupertokensUserId());

assertEquals(userJSON, getUsersByAccountInfo(process.getProcess(), false, null, phoneNumber, null, null).get(0));

process.kill();
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
}

@Test
public void testListUsersByAccountInfoForUnlinkedAccountsWithUnionOption() throws Exception {
String[] args = {"../"};
Expand Down
Loading