Skip to content

Commit 676a4c5

Browse files
demolafdemolaf
committed
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
1 parent 715e659 commit 676a4c5

File tree

3 files changed

+191
-70
lines changed

3 files changed

+191
-70
lines changed

‎auth/src/main/java/com/firebase/ui/auth/compose/configuration/AuthProvider.kt‎

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ abstract class AuthProvider(open val providerId: String) {
9191
* When enabled, prevents email links from being opened on different devices,
9292
* which is required for security when upgrading anonymous users. Defaults to true.
9393
*/
94-
9594
val isEmailLinkForceSameDeviceEnabled: Boolean = true,
9695

9796
/**
@@ -232,20 +231,18 @@ abstract class AuthProvider(open val providerId: String) {
232231
customParameters = customParameters
233232
) {
234233
fun validate(context: Context) {
235-
// TODO(demolaf): do we need this? since we are requesting this in AuthProvider.Google?
236-
// if serverClientId is nullable do we still need to throw an IllegalStateException?
237-
// if (serverClientId == null) {
238-
// Preconditions.checkConfigured(
239-
// context,
240-
// "Check your google-services plugin configuration, the" +
241-
// " default_web_client_id string wasn't populated.",
242-
// R.string.default_web_client_id
243-
// )
244-
// } else {
245-
// require(serverClientId.isNotBlank()) {
246-
// "Server client ID cannot be blank."
247-
// }
248-
// }
234+
if (serverClientId == null) {
235+
Preconditions.checkConfigured(
236+
context,
237+
"Check your google-services plugin configuration, the" +
238+
" default_web_client_id string wasn't populated.",
239+
R.string.default_web_client_id
240+
)
241+
} else {
242+
require(serverClientId.isNotBlank()) {
243+
"Server client ID cannot be blank."
244+
}
245+
}
249246

250247
val hasEmailScope = scopes.contains("email")
251248
if (!hasEmailScope) {
@@ -294,19 +291,18 @@ abstract class AuthProvider(open val providerId: String) {
294291
)
295292
}
296293

297-
// Check application ID - either from parameter or string resources
298-
// if (applicationId == null) {
299-
// Preconditions.checkConfigured(
300-
// context,
301-
// "Facebook provider unconfigured. Make sure to " +
302-
// "add a `facebook_application_id` string or provide applicationId parameter.",
303-
// R.string.facebook_application_id
304-
// )
305-
// } else {
306-
// require(applicationId.isNotBlank()) {
307-
// "Facebook application ID cannot be blank"
308-
// }
309-
// }
294+
if (applicationId == null) {
295+
Preconditions.checkConfigured(
296+
context,
297+
"Facebook provider unconfigured. Make sure to " +
298+
"add a `facebook_application_id` string or provide applicationId parameter.",
299+
R.string.facebook_application_id
300+
)
301+
} else {
302+
require(applicationId.isNotBlank()) {
303+
"Facebook application ID cannot be blank"
304+
}
305+
}
310306
}
311307
}
312308

‎auth/src/test/java/com/firebase/ui/auth/compose/configuration/AuthProviderTest.kt‎

Lines changed: 129 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package com.firebase.ui.auth.compose.configuration
1616

1717
import android.content.Context
1818
import androidx.test.core.app.ApplicationProvider
19+
import com.google.common.truth.Truth.assertThat
1920
import com.google.firebase.auth.actionCodeSettings
2021
import org.junit.Before
2122
import org.junit.Test
@@ -70,18 +71,26 @@ class AuthProviderTest {
7071
provider.validate()
7172
}
7273

73-
@Test(expected = IllegalArgumentException::class)
74+
@Test
7475
fun `email provider with email link enabled but null action code settings should throw`() {
7576
val provider = AuthProvider.Email(
7677
isEmailLinkSignInEnabled = true,
7778
actionCodeSettings = null,
7879
passwordValidationRules = listOf()
7980
)
8081

81-
provider.validate()
82+
try {
83+
provider.validate()
84+
} catch (e: Exception) {
85+
assertThat(e).isInstanceOf(IllegalArgumentException::class.java)
86+
assertThat(e.message).isEqualTo(
87+
"ActionCodeSettings cannot be null when using " +
88+
"email link sign in."
89+
)
90+
}
8291
}
8392

84-
@Test(expected = IllegalStateException::class)
93+
@Test
8594
fun `email provider with email link enabled but canHandleCodeInApp false should throw`() {
8695
val actionCodeSettings = actionCodeSettings {
8796
url = "https://example.com/verify"
@@ -94,7 +103,15 @@ class AuthProviderTest {
94103
passwordValidationRules = listOf()
95104
)
96105

97-
provider.validate()
106+
try {
107+
provider.validate()
108+
} catch (e: Exception) {
109+
assertThat(e).isInstanceOf(IllegalStateException::class.java)
110+
assertThat(e.message).isEqualTo(
111+
"You must set canHandleCodeInApp in your " +
112+
"ActionCodeSettings to true for Email-Link Sign-in."
113+
)
114+
}
98115
}
99116

100117
// =============================================================================================
@@ -123,15 +140,20 @@ class AuthProviderTest {
123140
provider.validate()
124141
}
125142

126-
@Test(expected = IllegalStateException::class)
143+
@Test
127144
fun `phone provider with invalid default number should throw`() {
128145
val provider = AuthProvider.Phone(
129146
defaultNumber = "invalid_number",
130147
defaultCountryCode = null,
131148
allowedCountries = null
132149
)
133150

134-
provider.validate()
151+
try {
152+
provider.validate()
153+
} catch (e: Exception) {
154+
assertThat(e).isInstanceOf(IllegalStateException::class.java)
155+
assertThat(e.message).isEqualTo("Invalid phone number: invalid_number")
156+
}
135157
}
136158

137159
@Test
@@ -145,15 +167,20 @@ class AuthProviderTest {
145167
provider.validate()
146168
}
147169

148-
@Test(expected = IllegalStateException::class)
170+
@Test
149171
fun `phone provider with invalid default country code should throw`() {
150172
val provider = AuthProvider.Phone(
151173
defaultNumber = null,
152174
defaultCountryCode = "invalid",
153175
allowedCountries = null
154176
)
155177

156-
provider.validate()
178+
try {
179+
provider.validate()
180+
} catch (e: Exception) {
181+
assertThat(e).isInstanceOf(IllegalStateException::class.java)
182+
assertThat(e.message).isEqualTo("Invalid country iso: invalid")
183+
}
157184
}
158185

159186
@Test
@@ -167,15 +194,23 @@ class AuthProviderTest {
167194
provider.validate()
168195
}
169196

170-
@Test(expected = IllegalStateException::class)
197+
@Test
171198
fun `phone provider with invalid country in allowed list should throw`() {
172199
val provider = AuthProvider.Phone(
173200
defaultNumber = null,
174201
defaultCountryCode = null,
175202
allowedCountries = listOf("US", "invalid_country")
176203
)
177204

178-
provider.validate()
205+
try {
206+
provider.validate()
207+
} catch (e: Exception) {
208+
assertThat(e).isInstanceOf(IllegalStateException::class.java)
209+
assertThat(e.message).isEqualTo(
210+
"Invalid input: You must provide a valid country iso (alpha-2) " +
211+
"or code (e-164). e.g. 'us' or '+1'. Invalid code: invalid_country"
212+
)
213+
}
179214
}
180215

181216
@Test
@@ -203,6 +238,39 @@ class AuthProviderTest {
203238
provider.validate(applicationContext)
204239
}
205240

241+
@Test
242+
fun `google provider with empty serverClientId string throws`() {
243+
val provider = AuthProvider.Google(
244+
scopes = listOf("email"),
245+
serverClientId = ""
246+
)
247+
248+
try {
249+
provider.validate(applicationContext)
250+
} catch (e: Exception) {
251+
assertThat(e).isInstanceOf(IllegalArgumentException::class.java)
252+
assertThat(e.message).isEqualTo("Server client ID cannot be blank.")
253+
}
254+
}
255+
256+
@Test
257+
fun `google provider validates default_web_client_id when serverClientId is null`() {
258+
val provider = AuthProvider.Google(
259+
scopes = listOf("email"),
260+
serverClientId = null
261+
)
262+
263+
try {
264+
provider.validate(applicationContext)
265+
} catch (e: Exception) {
266+
assertThat(e).isInstanceOf(IllegalStateException::class.java)
267+
assertThat(e.message).isEqualTo(
268+
"Check your google-services plugin " +
269+
"configuration, the default_web_client_id string wasn't populated."
270+
)
271+
}
272+
}
273+
206274
// =============================================================================================
207275
// Facebook Provider Tests
208276
// =============================================================================================
@@ -214,15 +282,50 @@ class AuthProviderTest {
214282
provider.validate(applicationContext)
215283
}
216284

285+
@Test
286+
fun `facebook provider with empty application id throws`() {
287+
val provider = AuthProvider.Facebook(applicationId = "")
288+
289+
try {
290+
provider.validate(applicationContext)
291+
} catch (e: Exception) {
292+
assertThat(e).isInstanceOf(IllegalArgumentException::class.java)
293+
assertThat(e.message).isEqualTo("Facebook application ID cannot be blank")
294+
}
295+
}
296+
297+
@Test
298+
fun `facebook provider validates facebook_application_id when applicationId is null`() {
299+
val provider = AuthProvider.Facebook()
300+
301+
try {
302+
provider.validate(applicationContext)
303+
} catch (e: Exception) {
304+
assertThat(e).isInstanceOf(IllegalStateException::class.java)
305+
assertThat(e.message).isEqualTo(
306+
"Facebook provider unconfigured. Make sure to " +
307+
"add a `facebook_application_id` string or provide applicationId parameter."
308+
)
309+
}
310+
}
311+
217312
// =============================================================================================
218313
// Anonymous Provider Tests
219314
// =============================================================================================
220315

221-
@Test(expected = IllegalStateException::class)
316+
@Test
222317
fun `anonymous provider as only provider should throw`() {
223318
val providers = listOf(AuthProvider.Anonymous)
224319

225-
AuthProvider.Anonymous.validate(providers)
320+
try {
321+
AuthProvider.Anonymous.validate(providers)
322+
} catch (e: Exception) {
323+
assertThat(e).isInstanceOf(IllegalStateException::class.java)
324+
assertThat(e.message).isEqualTo(
325+
"Sign in as guest cannot be the only sign in method. " +
326+
"In this case, sign the user in anonymously your self; no UI is needed."
327+
)
328+
}
226329
}
227330

228331
@Test
@@ -256,7 +359,7 @@ class AuthProviderTest {
256359
provider.validate()
257360
}
258361

259-
@Test(expected = IllegalArgumentException::class)
362+
@Test
260363
fun `generic oauth provider with blank provider id should throw`() {
261364
val provider = AuthProvider.GenericOAuth(
262365
providerId = "",
@@ -267,10 +370,15 @@ class AuthProviderTest {
267370
buttonColor = null
268371
)
269372

270-
provider.validate()
373+
try {
374+
provider.validate()
375+
} catch (e: Exception) {
376+
assertThat(e).isInstanceOf(IllegalArgumentException::class.java)
377+
assertThat(e.message).isEqualTo("Provider ID cannot be null or empty")
378+
}
271379
}
272380

273-
@Test(expected = IllegalArgumentException::class)
381+
@Test
274382
fun `generic oauth provider with blank button label should throw`() {
275383
val provider = AuthProvider.GenericOAuth(
276384
providerId = "custom.provider",
@@ -281,6 +389,11 @@ class AuthProviderTest {
281389
buttonColor = null
282390
)
283391

284-
provider.validate()
392+
try {
393+
provider.validate()
394+
} catch (e: Exception) {
395+
assertThat(e).isInstanceOf(IllegalArgumentException::class.java)
396+
assertThat(e.message).isEqualTo("Button label cannot be null or empty")
397+
}
285398
}
286399
}

0 commit comments

Comments
 (0)