From 478ef6396337e63f0e5f0ef12f558fe16afb6896 Mon Sep 17 00:00:00 2001 From: Ademola Fadumo Date: Tue, 23 Sep 2025 16:34:50 +0100 Subject: [PATCH 1/3] feat: added preconditions for specific auth provider configurations --- .../compose/configuration/AuthProvider.kt | 102 +++++++++++++++++- .../configuration/AuthUIConfiguration.kt | 22 ++-- 2 files changed, 113 insertions(+), 11 deletions(-) diff --git a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt index ef8bb0771..f3048d8a2 100644 --- a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt +++ b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt @@ -14,8 +14,15 @@ package com.firebase.ui.auth.compose.configuration +import android.content.Context import android.graphics.Color +import android.util.Log import androidx.compose.ui.graphics.vector.ImageVector +import com.firebase.ui.auth.AuthUI +import com.firebase.ui.auth.R +import com.firebase.ui.auth.util.Preconditions +import com.firebase.ui.auth.util.data.PhoneNumberUtils +import com.firebase.ui.auth.util.data.ProviderAvailability import com.google.firebase.auth.ActionCodeSettings import com.google.firebase.auth.EmailAuthProvider import com.google.firebase.auth.FacebookAuthProvider @@ -78,6 +85,15 @@ abstract class AuthProvider(open val providerId: String) { */ val isEmailLinkSignInEnabled: Boolean = false, + /** + * Forces email link sign-in to complete on the same device that initiated it. + * + * When enabled, prevents email links from being opened on different devices, + * which is required for security when upgrading anonymous users. Defaults to true. + */ + + val isEmailLinkForceSameDeviceEnabled: Boolean = true, + /** * Settings for email link actions. */ @@ -118,6 +134,11 @@ abstract class AuthProvider(open val providerId: String) { * Phone number authentication provider configuration. */ class Phone( + /** + * The phone number in international format. + */ + val defaultNumber: String?, + /** * The default country code to pre-select. */ @@ -147,7 +168,29 @@ abstract class AuthProvider(open val providerId: String) { * Enables automatic retrieval of the SMS code. Defaults to true. */ val isAutoRetrievalEnabled: Boolean = true - ) : AuthProvider(providerId = Provider.PHONE.id) + ) : AuthProvider(providerId = Provider.PHONE.id) { + fun validate() { + defaultNumber?.let { + check(PhoneNumberUtils.isValid(it)) { + "Invalid phone number: $it" + } + } + + check(PhoneNumberUtils.isValidIso(defaultCountryCode)) { + "Invalid country iso: $defaultCountryCode" + } + + allowedCountries?.forEach { code -> + check( + PhoneNumberUtils.isValidIso(code) || + PhoneNumberUtils.isValid(code) + ) { + "Invalid input: You must provide a valid country iso (alpha-2) " + + "or code (e-164). e.g. 'us' or '+1'. Invalid code: $code" + } + } + } + } /** * Google Sign-In provider configuration. @@ -186,7 +229,30 @@ abstract class AuthProvider(open val providerId: String) { providerId = Provider.GOOGLE.id, scopes = scopes, customParameters = customParameters - ) + ) { + fun validate(context: Context) { + // TODO(demolaf): do we need this? since we are requesting this in AuthProvider.Google? + // if serverClientId is nullable do we still need to throw an IllegalStateException? + Preconditions.checkConfigured( + context, + "Check your google-services plugin configuration, the" + + " default_web_client_id string wasn't populated.", + R.string.default_web_client_id + ) + + for (scope in scopes) { + if ("email" == scope) { + Log.w( + "AuthProvider.Google", + "The GoogleSignInOptions passed to setSignInOptions does not " + + "request the 'email' scope. In most cases this is a mistake! " + + "Call requestEmail() on the GoogleSignInOptions object." + ) + break + } + } + } + } /** * Facebook Login provider configuration. @@ -210,7 +276,26 @@ abstract class AuthProvider(open val providerId: String) { providerId = Provider.FACEBOOK.id, scopes = scopes, customParameters = customParameters - ) + ) { + fun validate(context: Context) { + if (ProviderAvailability.IS_FACEBOOK_AVAILABLE) { + throw RuntimeException( + "Facebook provider cannot be configured " + + "without dependency. Did you forget to add " + + "'com.facebook.android:facebook-login:VERSION' dependency?" + ) + } + + // TODO(demolaf): is this required? or should we add appId to AuthProvider.Facebook + // parameters above? + Preconditions.checkConfigured( + context, + "Facebook provider unconfigured. Make sure to " + + "add a `facebook_application_id` string.", + R.string.facebook_application_id + ) + } + } /** * Twitter/X authentication provider configuration. @@ -314,7 +399,16 @@ abstract class AuthProvider(open val providerId: String) { /** * Anonymous authentication provider. It has no configurable properties. */ - object Anonymous : AuthProvider(providerId = Provider.ANONYMOUS.id) + object Anonymous : AuthProvider(providerId = Provider.ANONYMOUS.id) { + fun validate(providers: List) { + if (providers.size == 1 && providers.first() is Anonymous) { + throw IllegalStateException( + "Sign in as guest cannot be the only sign in method. " + + "In this case, sign the user in anonymously your self; no UI is needed." + ) + } + } + } /** * A generic OAuth provider for any unsupported provider. diff --git a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthUIConfiguration.kt b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthUIConfiguration.kt index ef70c5f3b..fac50e8e6 100644 --- a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthUIConfiguration.kt +++ b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthUIConfiguration.kt @@ -68,12 +68,7 @@ class AuthUIConfigurationBuilder { } // Cannot have only anonymous provider - if (providers.size == 1 && providers.first() is AuthProvider.Anonymous) { - throw IllegalStateException( - "Sign in as guest cannot be the only sign in method. " + - "In this case, sign the user in anonymously your self; no UI is needed." - ) - } + AuthProvider.Anonymous.validate(providers) // Check for duplicate providers val providerIds = providers.map { it.providerId } @@ -89,7 +84,20 @@ class AuthUIConfigurationBuilder { // Provider specific validations providers.forEach { provider -> when (provider) { - is AuthProvider.Email -> provider.validate() + is AuthProvider.Email -> { + provider.validate() + + if (isAnonymousUpgradeEnabled && provider.isEmailLinkSignInEnabled) { + check(provider.isEmailLinkForceSameDeviceEnabled) { + "You must force the same device flow when using email link sign in " + + "with anonymous user upgrade" + } + } + } + + is AuthProvider.Phone -> provider.validate() + is AuthProvider.Google -> provider.validate(context) + is AuthProvider.Facebook -> provider.validate(context) else -> null } } From 715e659746dee7f7f5066cab46c55e330f0d884a Mon Sep 17 00:00:00 2001 From: Ademola Fadumo Date: Tue, 23 Sep 2025 20:40:57 +0100 Subject: [PATCH 2/3] test: covers auth providers with config validations --- .../compose/configuration/AuthProvider.kt | 91 +++--- .../configuration/AuthUIConfiguration.kt | 1 + .../compose/configuration/AuthProviderTest.kt | 286 ++++++++++++++++++ .../configuration/AuthUIConfigurationTest.kt | 48 +-- 4 files changed, 345 insertions(+), 81 deletions(-) create mode 100644 auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt diff --git a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt index f3048d8a2..c805faff3 100644 --- a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt +++ b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt @@ -116,11 +116,10 @@ abstract class AuthProvider(open val providerId: String) { ) : AuthProvider(providerId = Provider.EMAIL.id) { fun validate() { if (isEmailLinkSignInEnabled) { - val actionCodeSettings = actionCodeSettings - ?: requireNotNull(actionCodeSettings) { - "ActionCodeSettings cannot be null when using " + - "email link sign in." - } + val actionCodeSettings = requireNotNull(actionCodeSettings) { + "ActionCodeSettings cannot be null when using " + + "email link sign in." + } check(actionCodeSettings.canHandleCodeInApp()) { "You must set canHandleCodeInApp in your " + @@ -176,8 +175,10 @@ abstract class AuthProvider(open val providerId: String) { } } - check(PhoneNumberUtils.isValidIso(defaultCountryCode)) { - "Invalid country iso: $defaultCountryCode" + defaultCountryCode?.let { + check(PhoneNumberUtils.isValidIso(it)) { + "Invalid country iso: $it" + } } allowedCountries?.forEach { code -> @@ -233,23 +234,25 @@ abstract class AuthProvider(open val providerId: String) { fun validate(context: Context) { // TODO(demolaf): do we need this? since we are requesting this in AuthProvider.Google? // if serverClientId is nullable do we still need to throw an IllegalStateException? - Preconditions.checkConfigured( - context, - "Check your google-services plugin configuration, the" + - " default_web_client_id string wasn't populated.", - R.string.default_web_client_id - ) - - for (scope in scopes) { - if ("email" == scope) { - Log.w( - "AuthProvider.Google", - "The GoogleSignInOptions passed to setSignInOptions does not " + - "request the 'email' scope. In most cases this is a mistake! " + - "Call requestEmail() on the GoogleSignInOptions object." - ) - break - } + // if (serverClientId == null) { + // Preconditions.checkConfigured( + // context, + // "Check your google-services plugin configuration, the" + + // " default_web_client_id string wasn't populated.", + // R.string.default_web_client_id + // ) + // } else { + // require(serverClientId.isNotBlank()) { + // "Server client ID cannot be blank." + // } + // } + + val hasEmailScope = scopes.contains("email") + if (!hasEmailScope) { + Log.w( + "AuthProvider.Google", + "The scopes do not include 'email'. In most cases this is a mistake!" + ) } } } @@ -258,6 +261,11 @@ abstract class AuthProvider(open val providerId: String) { * Facebook Login provider configuration. */ class Facebook( + /** + * The Facebook application ID. + */ + val applicationId: String? = null, + /** * The list of scopes (permissions) to request. Defaults to email and public_profile. */ @@ -278,7 +286,7 @@ abstract class AuthProvider(open val providerId: String) { customParameters = customParameters ) { fun validate(context: Context) { - if (ProviderAvailability.IS_FACEBOOK_AVAILABLE) { + if (!ProviderAvailability.IS_FACEBOOK_AVAILABLE) { throw RuntimeException( "Facebook provider cannot be configured " + "without dependency. Did you forget to add " + @@ -286,14 +294,19 @@ abstract class AuthProvider(open val providerId: String) { ) } - // TODO(demolaf): is this required? or should we add appId to AuthProvider.Facebook - // parameters above? - Preconditions.checkConfigured( - context, - "Facebook provider unconfigured. Make sure to " + - "add a `facebook_application_id` string.", - R.string.facebook_application_id - ) + // Check application ID - either from parameter or string resources + // if (applicationId == null) { + // Preconditions.checkConfigured( + // context, + // "Facebook provider unconfigured. Make sure to " + + // "add a `facebook_application_id` string or provide applicationId parameter.", + // R.string.facebook_application_id + // ) + // } else { + // require(applicationId.isNotBlank()) { + // "Facebook application ID cannot be blank" + // } + // } } } @@ -447,5 +460,15 @@ abstract class AuthProvider(open val providerId: String) { providerId = providerId, scopes = scopes, customParameters = customParameters - ) + ) { + fun validate() { + require(providerId.isNotBlank()) { + "Provider ID cannot be null or empty" + } + + require(buttonLabel.isNotBlank()) { + "Button label cannot be null or empty" + } + } + } } \ No newline at end of file diff --git a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthUIConfiguration.kt b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthUIConfiguration.kt index fac50e8e6..98be20ac0 100644 --- a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthUIConfiguration.kt +++ b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthUIConfiguration.kt @@ -98,6 +98,7 @@ class AuthUIConfigurationBuilder { is AuthProvider.Phone -> provider.validate() is AuthProvider.Google -> provider.validate(context) is AuthProvider.Facebook -> provider.validate(context) + is AuthProvider.GenericOAuth -> provider.validate() else -> null } } diff --git a/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt b/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt new file mode 100644 index 000000000..9a3f6aae0 --- /dev/null +++ b/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt @@ -0,0 +1,286 @@ +/* + * Copyright 2025 Google Inc. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.firebase.ui.auth.compose.configuration + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.google.firebase.auth.actionCodeSettings +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Unit tests for [AuthProvider] covering provider validation rules, configuration constraints, + * and error handling for all supported authentication providers. + * + * @suppress Internal test class + */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE) +class AuthProviderTest { + + private lateinit var applicationContext: Context + + @Before + fun setUp() { + applicationContext = ApplicationProvider.getApplicationContext() + } + + // ============================================================================================= + // Email Provider Tests + // ============================================================================================= + + @Test + fun `email provider with valid configuration should succeed`() { + val provider = AuthProvider.Email( + actionCodeSettings = null, + passwordValidationRules = listOf() + ) + + provider.validate() + } + + @Test + fun `email provider with email link enabled and valid action code settings should succeed`() { + val actionCodeSettings = actionCodeSettings { + url = "https://example.com/verify" + handleCodeInApp = true + } + + val provider = AuthProvider.Email( + isEmailLinkSignInEnabled = true, + actionCodeSettings = actionCodeSettings, + passwordValidationRules = listOf() + ) + + provider.validate() + } + + @Test(expected = IllegalArgumentException::class) + fun `email provider with email link enabled but null action code settings should throw`() { + val provider = AuthProvider.Email( + isEmailLinkSignInEnabled = true, + actionCodeSettings = null, + passwordValidationRules = listOf() + ) + + provider.validate() + } + + @Test(expected = IllegalStateException::class) + fun `email provider with email link enabled but canHandleCodeInApp false should throw`() { + val actionCodeSettings = actionCodeSettings { + url = "https://example.com/verify" + handleCodeInApp = false + } + + val provider = AuthProvider.Email( + isEmailLinkSignInEnabled = true, + actionCodeSettings = actionCodeSettings, + passwordValidationRules = listOf() + ) + + provider.validate() + } + + // ============================================================================================= + // Phone Provider Tests + // ============================================================================================= + + @Test + fun `phone provider with valid configuration should succeed`() { + val provider = AuthProvider.Phone( + defaultNumber = null, + defaultCountryCode = null, + allowedCountries = null + ) + + provider.validate() + } + + @Test + fun `phone provider with valid default number should succeed`() { + val provider = AuthProvider.Phone( + defaultNumber = "+1234567890", + defaultCountryCode = null, + allowedCountries = null + ) + + provider.validate() + } + + @Test(expected = IllegalStateException::class) + fun `phone provider with invalid default number should throw`() { + val provider = AuthProvider.Phone( + defaultNumber = "invalid_number", + defaultCountryCode = null, + allowedCountries = null + ) + + provider.validate() + } + + @Test + fun `phone provider with valid default country code should succeed`() { + val provider = AuthProvider.Phone( + defaultNumber = null, + defaultCountryCode = "US", + allowedCountries = null + ) + + provider.validate() + } + + @Test(expected = IllegalStateException::class) + fun `phone provider with invalid default country code should throw`() { + val provider = AuthProvider.Phone( + defaultNumber = null, + defaultCountryCode = "invalid", + allowedCountries = null + ) + + provider.validate() + } + + @Test + fun `phone provider with valid allowed countries should succeed`() { + val provider = AuthProvider.Phone( + defaultNumber = null, + defaultCountryCode = null, + allowedCountries = listOf("US", "CA", "+1") + ) + + provider.validate() + } + + @Test(expected = IllegalStateException::class) + fun `phone provider with invalid country in allowed list should throw`() { + val provider = AuthProvider.Phone( + defaultNumber = null, + defaultCountryCode = null, + allowedCountries = listOf("US", "invalid_country") + ) + + provider.validate() + } + + @Test + fun `phone provider with valid default number, country code and compatible allowed countries should succeed`() { + val provider = AuthProvider.Phone( + defaultNumber = "+1234567890", + defaultCountryCode = "US", + allowedCountries = listOf("US", "CA") + ) + + provider.validate() + } + + // ============================================================================================= + // Google Provider Tests + // ============================================================================================= + + @Test + fun `google provider with valid configuration should succeed`() { + val provider = AuthProvider.Google( + scopes = listOf("email"), + serverClientId = "test_client_id" + ) + + provider.validate(applicationContext) + } + + // ============================================================================================= + // Facebook Provider Tests + // ============================================================================================= + + @Test + fun `facebook provider with valid configuration should succeed`() { + val provider = AuthProvider.Facebook(applicationId = "application_id") + + provider.validate(applicationContext) + } + + // ============================================================================================= + // Anonymous Provider Tests + // ============================================================================================= + + @Test(expected = IllegalStateException::class) + fun `anonymous provider as only provider should throw`() { + val providers = listOf(AuthProvider.Anonymous) + + AuthProvider.Anonymous.validate(providers) + } + + @Test + fun `anonymous provider with other providers should succeed`() { + val providers = listOf( + AuthProvider.Anonymous, + AuthProvider.Email( + actionCodeSettings = null, + passwordValidationRules = listOf() + ) + ) + + AuthProvider.Anonymous.validate(providers) + } + + // ============================================================================================= + // GenericOAuth Provider Tests + // ============================================================================================= + + @Test + fun `generic oauth provider with valid configuration should succeed`() { + val provider = AuthProvider.GenericOAuth( + providerId = "custom.provider", + scopes = listOf("read"), + customParameters = mapOf(), + buttonLabel = "Sign in with Custom", + buttonIcon = null, + buttonColor = null + ) + + provider.validate() + } + + @Test(expected = IllegalArgumentException::class) + fun `generic oauth provider with blank provider id should throw`() { + val provider = AuthProvider.GenericOAuth( + providerId = "", + scopes = listOf("read"), + customParameters = mapOf(), + buttonLabel = "Sign in with Custom", + buttonIcon = null, + buttonColor = null + ) + + provider.validate() + } + + @Test(expected = IllegalArgumentException::class) + fun `generic oauth provider with blank button label should throw`() { + val provider = AuthProvider.GenericOAuth( + providerId = "custom.provider", + scopes = listOf("read"), + customParameters = mapOf(), + buttonLabel = "", + buttonIcon = null, + buttonColor = null + ) + + provider.validate() + } +} \ No newline at end of file diff --git a/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthUIConfigurationTest.kt b/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthUIConfigurationTest.kt index 95164f638..86b1c4dff 100644 --- a/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthUIConfigurationTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthUIConfigurationTest.kt @@ -292,7 +292,7 @@ class AuthUIConfigurationTest { provider(AuthProvider.Microsoft(customParameters = mapOf(), tenant = null)) provider(AuthProvider.Yahoo(customParameters = mapOf())) provider(AuthProvider.Apple(customParameters = mapOf(), locale = null)) - provider(AuthProvider.Phone(defaultCountryCode = null, allowedCountries = null)) + provider(AuthProvider.Phone(defaultNumber = null, defaultCountryCode = null, allowedCountries = null)) provider( AuthProvider.Email( actionCodeSettings = null, @@ -323,16 +323,6 @@ class AuthUIConfigurationTest { } } - @Test(expected = IllegalStateException::class) - fun `validate throws when only anonymous provider is configured`() { - authUIConfiguration { - context = applicationContext - providers { - provider(AuthProvider.Anonymous) - } - } - } - @Test(expected = IllegalArgumentException::class) fun `validate throws for duplicate providers`() { authUIConfiguration { @@ -349,42 +339,6 @@ class AuthUIConfigurationTest { } } - @Test(expected = IllegalArgumentException::class) - fun `validate throws for enableEmailLinkSignIn true when actionCodeSettings is null`() { - authUIConfiguration { - context = applicationContext - providers { - provider( - AuthProvider.Email( - isEmailLinkSignInEnabled = true, - actionCodeSettings = null, - passwordValidationRules = listOf() - ) - ) - } - } - } - - @Test(expected = IllegalStateException::class) - fun `validate throws for enableEmailLinkSignIn true when actionCodeSettings canHandleCodeInApp false`() { - val customActionCodeSettings = actionCodeSettings { - url = "https://example.com" - handleCodeInApp = false - } - authUIConfiguration { - context = applicationContext - providers { - provider( - AuthProvider.Email( - isEmailLinkSignInEnabled = true, - actionCodeSettings = customActionCodeSettings, - passwordValidationRules = listOf() - ) - ) - } - } - } - // ============================================================================================= // Builder Immutability Tests // ============================================================================================= From 676a4c599157beabaf31abbfbed5bc24df8ef402 Mon Sep 17 00:00:00 2001 From: Ademola Fadumo Date: Wed, 24 Sep 2025 10:41:03 +0100 Subject: [PATCH 3/3] fix: auth provider validation and tests - validate serverClientId empty string - validate applicationId empty string - remove @Test(expected=) not descriptive - tests covering AuthProviders Google and Facebook config validation --- .../compose/configuration/AuthProvider.kt | 52 +++---- .../compose/configuration/AuthProviderTest.kt | 145 ++++++++++++++++-- .../configuration/AuthUIConfigurationTest.kt | 64 ++++---- 3 files changed, 191 insertions(+), 70 deletions(-) diff --git a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt index c805faff3..d44a9d6d9 100644 --- a/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt +++ b/auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt @@ -91,7 +91,6 @@ abstract class AuthProvider(open val providerId: String) { * When enabled, prevents email links from being opened on different devices, * which is required for security when upgrading anonymous users. Defaults to true. */ - val isEmailLinkForceSameDeviceEnabled: Boolean = true, /** @@ -232,20 +231,18 @@ abstract class AuthProvider(open val providerId: String) { customParameters = customParameters ) { fun validate(context: Context) { - // TODO(demolaf): do we need this? since we are requesting this in AuthProvider.Google? - // if serverClientId is nullable do we still need to throw an IllegalStateException? - // if (serverClientId == null) { - // Preconditions.checkConfigured( - // context, - // "Check your google-services plugin configuration, the" + - // " default_web_client_id string wasn't populated.", - // R.string.default_web_client_id - // ) - // } else { - // require(serverClientId.isNotBlank()) { - // "Server client ID cannot be blank." - // } - // } + if (serverClientId == null) { + Preconditions.checkConfigured( + context, + "Check your google-services plugin configuration, the" + + " default_web_client_id string wasn't populated.", + R.string.default_web_client_id + ) + } else { + require(serverClientId.isNotBlank()) { + "Server client ID cannot be blank." + } + } val hasEmailScope = scopes.contains("email") if (!hasEmailScope) { @@ -294,19 +291,18 @@ abstract class AuthProvider(open val providerId: String) { ) } - // Check application ID - either from parameter or string resources - // if (applicationId == null) { - // Preconditions.checkConfigured( - // context, - // "Facebook provider unconfigured. Make sure to " + - // "add a `facebook_application_id` string or provide applicationId parameter.", - // R.string.facebook_application_id - // ) - // } else { - // require(applicationId.isNotBlank()) { - // "Facebook application ID cannot be blank" - // } - // } + if (applicationId == null) { + Preconditions.checkConfigured( + context, + "Facebook provider unconfigured. Make sure to " + + "add a `facebook_application_id` string or provide applicationId parameter.", + R.string.facebook_application_id + ) + } else { + require(applicationId.isNotBlank()) { + "Facebook application ID cannot be blank" + } + } } } diff --git a/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt b/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt index 9a3f6aae0..27685f859 100644 --- a/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt @@ -16,6 +16,7 @@ package com.firebase.ui.auth.compose.configuration import android.content.Context import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat import com.google.firebase.auth.actionCodeSettings import org.junit.Before import org.junit.Test @@ -70,7 +71,7 @@ class AuthProviderTest { provider.validate() } - @Test(expected = IllegalArgumentException::class) + @Test fun `email provider with email link enabled but null action code settings should throw`() { val provider = AuthProvider.Email( isEmailLinkSignInEnabled = true, @@ -78,10 +79,18 @@ class AuthProviderTest { passwordValidationRules = listOf() ) - provider.validate() + try { + provider.validate() + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalArgumentException::class.java) + assertThat(e.message).isEqualTo( + "ActionCodeSettings cannot be null when using " + + "email link sign in." + ) + } } - @Test(expected = IllegalStateException::class) + @Test fun `email provider with email link enabled but canHandleCodeInApp false should throw`() { val actionCodeSettings = actionCodeSettings { url = "https://example.com/verify" @@ -94,7 +103,15 @@ class AuthProviderTest { passwordValidationRules = listOf() ) - provider.validate() + try { + provider.validate() + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalStateException::class.java) + assertThat(e.message).isEqualTo( + "You must set canHandleCodeInApp in your " + + "ActionCodeSettings to true for Email-Link Sign-in." + ) + } } // ============================================================================================= @@ -123,7 +140,7 @@ class AuthProviderTest { provider.validate() } - @Test(expected = IllegalStateException::class) + @Test fun `phone provider with invalid default number should throw`() { val provider = AuthProvider.Phone( defaultNumber = "invalid_number", @@ -131,7 +148,12 @@ class AuthProviderTest { allowedCountries = null ) - provider.validate() + try { + provider.validate() + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalStateException::class.java) + assertThat(e.message).isEqualTo("Invalid phone number: invalid_number") + } } @Test @@ -145,7 +167,7 @@ class AuthProviderTest { provider.validate() } - @Test(expected = IllegalStateException::class) + @Test fun `phone provider with invalid default country code should throw`() { val provider = AuthProvider.Phone( defaultNumber = null, @@ -153,7 +175,12 @@ class AuthProviderTest { allowedCountries = null ) - provider.validate() + try { + provider.validate() + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalStateException::class.java) + assertThat(e.message).isEqualTo("Invalid country iso: invalid") + } } @Test @@ -167,7 +194,7 @@ class AuthProviderTest { provider.validate() } - @Test(expected = IllegalStateException::class) + @Test fun `phone provider with invalid country in allowed list should throw`() { val provider = AuthProvider.Phone( defaultNumber = null, @@ -175,7 +202,15 @@ class AuthProviderTest { allowedCountries = listOf("US", "invalid_country") ) - provider.validate() + try { + provider.validate() + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalStateException::class.java) + assertThat(e.message).isEqualTo( + "Invalid input: You must provide a valid country iso (alpha-2) " + + "or code (e-164). e.g. 'us' or '+1'. Invalid code: invalid_country" + ) + } } @Test @@ -203,6 +238,39 @@ class AuthProviderTest { provider.validate(applicationContext) } + @Test + fun `google provider with empty serverClientId string throws`() { + val provider = AuthProvider.Google( + scopes = listOf("email"), + serverClientId = "" + ) + + try { + provider.validate(applicationContext) + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalArgumentException::class.java) + assertThat(e.message).isEqualTo("Server client ID cannot be blank.") + } + } + + @Test + fun `google provider validates default_web_client_id when serverClientId is null`() { + val provider = AuthProvider.Google( + scopes = listOf("email"), + serverClientId = null + ) + + try { + provider.validate(applicationContext) + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalStateException::class.java) + assertThat(e.message).isEqualTo( + "Check your google-services plugin " + + "configuration, the default_web_client_id string wasn't populated." + ) + } + } + // ============================================================================================= // Facebook Provider Tests // ============================================================================================= @@ -214,15 +282,50 @@ class AuthProviderTest { provider.validate(applicationContext) } + @Test + fun `facebook provider with empty application id throws`() { + val provider = AuthProvider.Facebook(applicationId = "") + + try { + provider.validate(applicationContext) + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalArgumentException::class.java) + assertThat(e.message).isEqualTo("Facebook application ID cannot be blank") + } + } + + @Test + fun `facebook provider validates facebook_application_id when applicationId is null`() { + val provider = AuthProvider.Facebook() + + try { + provider.validate(applicationContext) + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalStateException::class.java) + assertThat(e.message).isEqualTo( + "Facebook provider unconfigured. Make sure to " + + "add a `facebook_application_id` string or provide applicationId parameter." + ) + } + } + // ============================================================================================= // Anonymous Provider Tests // ============================================================================================= - @Test(expected = IllegalStateException::class) + @Test fun `anonymous provider as only provider should throw`() { val providers = listOf(AuthProvider.Anonymous) - AuthProvider.Anonymous.validate(providers) + try { + AuthProvider.Anonymous.validate(providers) + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalStateException::class.java) + assertThat(e.message).isEqualTo( + "Sign in as guest cannot be the only sign in method. " + + "In this case, sign the user in anonymously your self; no UI is needed." + ) + } } @Test @@ -256,7 +359,7 @@ class AuthProviderTest { provider.validate() } - @Test(expected = IllegalArgumentException::class) + @Test fun `generic oauth provider with blank provider id should throw`() { val provider = AuthProvider.GenericOAuth( providerId = "", @@ -267,10 +370,15 @@ class AuthProviderTest { buttonColor = null ) - provider.validate() + try { + provider.validate() + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalArgumentException::class.java) + assertThat(e.message).isEqualTo("Provider ID cannot be null or empty") + } } - @Test(expected = IllegalArgumentException::class) + @Test fun `generic oauth provider with blank button label should throw`() { val provider = AuthProvider.GenericOAuth( providerId = "custom.provider", @@ -281,6 +389,11 @@ class AuthProviderTest { buttonColor = null ) - provider.validate() + try { + provider.validate() + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalArgumentException::class.java) + assertThat(e.message).isEqualTo("Button label cannot be null or empty") + } } } \ No newline at end of file diff --git a/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthUIConfigurationTest.kt b/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthUIConfigurationTest.kt index 86b1c4dff..96a13795e 100644 --- a/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthUIConfigurationTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthUIConfigurationTest.kt @@ -64,7 +64,7 @@ class AuthUIConfigurationTest { provider( AuthProvider.Google( scopes = listOf(), - serverClientId = "" + serverClientId = "test_client_id" ) ) } @@ -103,7 +103,7 @@ class AuthUIConfigurationTest { provider( AuthProvider.Google( scopes = listOf(), - serverClientId = "" + serverClientId = "test_client_id" ) ) provider( @@ -152,7 +152,7 @@ class AuthUIConfigurationTest { provider( AuthProvider.Google( scopes = listOf(), - serverClientId = "" + serverClientId = "test_client_id" ) ) } @@ -190,7 +190,7 @@ class AuthUIConfigurationTest { provider( AuthProvider.Google( scopes = listOf(), - serverClientId = "" + serverClientId = "test_client_id" ) ) } @@ -215,7 +215,7 @@ class AuthUIConfigurationTest { provider( AuthProvider.Google( scopes = listOf(), - serverClientId = "" + serverClientId = "test_client_id" ) ) } @@ -235,8 +235,8 @@ class AuthUIConfigurationTest { providers { provider( AuthProvider.Google( - scopes = listOf(), serverClientId - = "" + scopes = listOf(), + serverClientId = "test_client_id" ) ) } @@ -261,7 +261,7 @@ class AuthUIConfigurationTest { authUIConfiguration { context = applicationContext providers { - provider(AuthProvider.Google(scopes = listOf(), serverClientId = "")) + provider(AuthProvider.Google(scopes = listOf(), serverClientId = "test_client_id")) } } } catch (e: Exception) { @@ -285,8 +285,8 @@ class AuthUIConfigurationTest { val config = authUIConfiguration { context = applicationContext providers { - provider(AuthProvider.Google(scopes = listOf(), serverClientId = "")) - provider(AuthProvider.Facebook()) + provider(AuthProvider.Google(scopes = listOf(), serverClientId = "test_client_id")) + provider(AuthProvider.Facebook(applicationId = "test_app_id")) provider(AuthProvider.Twitter(customParameters = mapOf())) provider(AuthProvider.Github(customParameters = mapOf())) provider(AuthProvider.Microsoft(customParameters = mapOf(), tenant = null)) @@ -304,7 +304,7 @@ class AuthUIConfigurationTest { assertThat(config.providers).hasSize(9) } - @Test(expected = IllegalArgumentException::class) + @Test fun `validation throws for unsupported provider`() { val mockProvider = AuthProvider.GenericOAuth( providerId = "unsupported.provider", @@ -315,27 +315,39 @@ class AuthUIConfigurationTest { buttonColor = null ) - authUIConfiguration { - context = applicationContext - providers { - provider(mockProvider) + try { + authUIConfiguration { + context = applicationContext + providers { + provider(mockProvider) + } } + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalArgumentException::class.java) + assertThat(e.message).isEqualTo("Unknown providers: unsupported.provider") } } - @Test(expected = IllegalArgumentException::class) + @Test fun `validate throws for duplicate providers`() { - authUIConfiguration { - context = applicationContext - providers { - provider(AuthProvider.Google(scopes = listOf(), serverClientId = "")) - provider( - AuthProvider.Google( - scopes = listOf("email"), - serverClientId = "different" + try { + authUIConfiguration { + context = applicationContext + providers { + provider(AuthProvider.Google(scopes = listOf(), serverClientId = "")) + provider( + AuthProvider.Google( + scopes = listOf("email"), + serverClientId = "different" + ) ) - ) + } } + } catch (e: Exception) { + assertThat(e).isInstanceOf(IllegalArgumentException::class.java) + assertThat(e.message).isEqualTo( + "Each provider can only be set once. Duplicates: google.com" + ) } } @@ -351,7 +363,7 @@ class AuthUIConfigurationTest { provider( AuthProvider.Google( scopes = listOf(), - serverClientId = "" + serverClientId = "test_client_id" ) ) }