Skip to content

Country selector fixes #1345

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 12, 2018
Merged

Country selector fixes #1345

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74dca57
Fixes default country selection in the country spinner
lsirac Jun 8, 2018
0ac3b0c
Moves country spinner limitation logic away from CheckPhoneNumberFrag…
lsirac Jun 12, 2018
5edf405
Adds checks for null/empty input for country lists
lsirac Jun 12, 2018
0a259ae
Changes error message/adds some documentation
lsirac Jun 12, 2018
e359657
Adds changes from review
lsirac Jun 12, 2018
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
5 changes: 5 additions & 0 deletions auth/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,11 @@ The country code selector will exclude all countries with a country code of +1 a
Note: You can't provide both a list of countries to whitelist and blacklist. If you do, a runtime
exception will be thrown.

This change is purely UI based. We do not restrict users from signing in with their phone number.
They will simply be unable to choose their country in the selector, but there may be another country
sharing the same country code (e.g. US and CA are +1).


#####

### Handling the sign-in response
Expand Down
114 changes: 88 additions & 26 deletions auth/src/main/java/com/firebase/ui/auth/AuthUI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,16 +668,22 @@ public PhoneBuilder setDefaultCountryIso(@NonNull String iso) {
* https://en.wikipedia.org/wiki/ISO_3166-1
* and e-164 codes here: https://en.wikipedia.org/wiki/List_of_country_calling_codes
*
* @param whitelistedCountries a case insensitive list of country codes and/or
* isos to be whitelisted
* @param whitelistedCountries a non empty case insensitive list of country codes
* and/or isos to be whitelisted
*/
public PhoneBuilder setWhitelistedCountries(
@NonNull List<String> whitelistedCountries) {
if (getParams().containsKey(ExtraConstants.BLACKLISTED_COUNTRIES)) {
throw new RuntimeException(
throw new IllegalStateException(
"You can either whitelist or blacklist country codes for phone " +
"authentication.");
}

String message = "Invalid argument: The whitelist is %s. For this case, " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of: "For this case, don't use this method" how about something like "Only non-null and non-empty whitelists are valid. To specify no whitelist, do not call this method."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better. 👍

"don't use this method.";
Preconditions.checkNotNull(whitelistedCountries, String.format(message, "null"));
Preconditions.checkArgument(!whitelistedCountries.isEmpty(), String.format(message, "empty"));

addCountriesToBundle(whitelistedCountries, ExtraConstants.WHITELISTED_COUNTRIES);
return this;
}
Expand All @@ -695,16 +701,23 @@ public PhoneBuilder setWhitelistedCountries(
* For a list of country iso or codes, see Alpha-2 codes here:
* https://en.wikipedia.org/wiki/ISO_3166-1
* and e-164 codes here: https://en.wikipedia.org/wiki/List_of_country_calling_codes
* @param blacklistedCountries a case insensitive list of country codes and/or isos
* to be blacklisted
*
* @param blacklistedCountries a non empty case insensitive list of country codes
* and/or isos to be blacklisted
*/
public PhoneBuilder setBlacklistedCountries(
@NonNull List<String> blacklistedCountries) {
if (getParams().containsKey(ExtraConstants.WHITELISTED_COUNTRIES)) {
throw new RuntimeException(
throw new IllegalStateException(
"You can either whitelist or blacklist country codes for phone " +
"authentication.");
}

String message = "Invalid argument: The blacklist is %s. For this case, " +
"don't use this method.";
Preconditions.checkNotNull(blacklistedCountries, String.format(message, "null"));
Preconditions.checkArgument(!blacklistedCountries.isEmpty(), String.format(message, "empty"));

addCountriesToBundle(blacklistedCountries, ExtraConstants.BLACKLISTED_COUNTRIES);
return this;
}
Expand Down Expand Up @@ -732,41 +745,70 @@ private void validateInputs() {

if (whitelistedCountries != null &&
blacklistedCountries != null) {
throw new RuntimeException(
throw new IllegalStateException(
"You can either whitelist or blacklist country codes for phone " +
"authentication.");
} else if (whitelistedCountries != null) {
validateCountries(whitelistedCountries);
validateDefaultCountryIso(whitelistedCountries, true);
validateInputs(whitelistedCountries, true);

} else if (blacklistedCountries != null) {
validateCountries(blacklistedCountries);
validateDefaultCountryIso(blacklistedCountries, false);
validateInputs(blacklistedCountries, false);
}
}

private void validateCountries(List<String> codes) {
private void validateInputs(List<String> countries, boolean whitelisted) {
validateCountryInput(countries);
validateDefaultCountryInput(countries, whitelisted);
}

private void validateCountryInput(List<String> codes) {
for (String code : codes) {
if (!PhoneNumberUtils.isValidIso(code) && !PhoneNumberUtils.isValid(code)) {
throw new RuntimeException("Invalid input: You must provide a valid " +
"country iso (alpha-2) or code (e-164). e.g. 'us' or '+1'.");
throw new IllegalArgumentException("Invalid input: You must provide a " +
"valid country iso (alpha-2) or code (e-164). e.g. 'us' or '+1'.");
}
}
}

private void validateDefaultCountryIso(List<String> codes, boolean whitelisted) {
if (getParams().containsKey(ExtraConstants.COUNTRY_ISO) && codes != null) {
String defaultIso = getParams().getString(ExtraConstants.COUNTRY_ISO);
boolean containsIso = containsCountryIso(codes, defaultIso);
if (!containsIso && whitelisted || containsIso && !whitelisted) {
throw new RuntimeException("Invalid default country iso. Make sure it " +
"is either part of the whitelisted list or that you " +
"haven't blacklisted it.");
private void validateDefaultCountryInput(List<String> codes, boolean whitelisted) {
// A default iso/code can be set via #setDefaultCountryIso() or #setDefaultNumber()
if (getParams().containsKey(ExtraConstants.COUNTRY_ISO) ||
getParams().containsKey(ExtraConstants.PHONE)) {

if (!validateDefaultCountryIso(codes, whitelisted)
|| !validateDefaultPhoneIsos(codes, whitelisted)) {
throw new IllegalArgumentException("Invalid default country iso. Make " +
"sure it is either part of the whitelisted list or that you "
+ "haven't blacklisted it.");
}
}

}

private boolean validateDefaultCountryIso(List<String> codes, boolean whitelisted) {
String defaultIso = getDefaultIso();
return isValidDefaultIso(codes, defaultIso, whitelisted);
}

private boolean validateDefaultPhoneIsos(List<String> codes, boolean whitelisted) {
List<String> phoneIsos = getPhoneIsosFromCode();
for (String iso : phoneIsos) {
if (isValidDefaultIso(codes, iso, whitelisted)) {
return true;
}
}
return phoneIsos.isEmpty();
}

private boolean isValidDefaultIso(List<String> codes, String iso, boolean whitelisted) {
if (iso == null) return true;
boolean containsIso = containsCountryIso(codes, iso);
return containsIso && whitelisted || !containsIso && !whitelisted;

}

private boolean containsCountryIso(List<String> codes, String iso) {
iso = iso.toUpperCase(Locale.getDefault());
for (String code : codes) {
if (PhoneNumberUtils.isValidIso(code)) {
if (code.equals(iso)) {
Expand All @@ -781,6 +823,26 @@ private boolean containsCountryIso(List<String> codes, String iso) {
}
return false;
}

private List<String> getPhoneIsosFromCode() {
List<String> isos = new ArrayList<>();
String phone = getParams().getString(ExtraConstants.PHONE);
if (phone != null && phone.startsWith("+")) {
String countryCode = "+" + PhoneNumberUtils.getPhoneNumber(phone)
.getCountryCode();
List<String> isosToAdd = PhoneNumberUtils.
getCountryIsosFromCountryCode(countryCode);
if (isosToAdd != null) {
isos.addAll(isosToAdd);
}
}
return isos;
}

private String getDefaultIso() {
return getParams().containsKey(ExtraConstants.COUNTRY_ISO) ?
getParams().getString(ExtraConstants.COUNTRY_ISO) : null;
}
}

/**
Expand Down Expand Up @@ -943,8 +1005,8 @@ public T setLogo(@DrawableRes int logo) {
/**
* Specifies the terms-of-service URL for the application.
*
* @deprecated Please use {@link #setTosAndPrivacyPolicyUrls(String, String)}
* For the Tos link to be displayed a Privacy Policy url must also be provided.
* @deprecated Please use {@link #setTosAndPrivacyPolicyUrls(String, String)} For the Tos
* link to be displayed a Privacy Policy url must also be provided.
*/
@NonNull
@Deprecated
Expand All @@ -956,8 +1018,8 @@ public T setTosUrl(@Nullable String tosUrl) {
/**
* Specifies the privacy policy URL for the application.
*
* @deprecated Please use {@link #setTosAndPrivacyPolicyUrls(String, String)}
* For the Privacy Policy link to be displayed a Tos url must also be provided.
* @deprecated Please use {@link #setTosAndPrivacyPolicyUrls(String, String)} For the
* Privacy Policy link to be displayed a Tos url must also be provided.
*/
@NonNull
@Deprecated
Expand Down
125 changes: 0 additions & 125 deletions auth/src/main/java/com/firebase/ui/auth/data/client/CountryListLoadTask.java
View file
Open in desktop

This file was deleted.

Loading