From 52608d5664e611c76a6552e7cb9f871bdc8538cb Mon Sep 17 00:00:00 2001 From: Narges Simjour Date: Wed, 8 Jan 2020 18:48:42 -0500 Subject: [PATCH 1/6] Add API to link/unlink provider info to/from user record. --- .../com/google/firebase/auth/UserRecord.java | 19 +- .../google/firebase/auth/FirebaseAuthIT.java | 194 ++++++++++++------ .../auth/FirebaseUserManagerTest.java | 35 +++- 3 files changed, 179 insertions(+), 69 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/UserRecord.java b/src/main/java/com/google/firebase/auth/UserRecord.java index e00450079..8bd236c8c 100644 --- a/src/main/java/com/google/firebase/auth/UserRecord.java +++ b/src/main/java/com/google/firebase/auth/UserRecord.java @@ -190,9 +190,9 @@ public UserInfo[] getProviderData() { } /** - * Returns a timestamp in milliseconds since epoch, truncated down to the closest second. + * Returns a timestamp in milliseconds since epoch, truncated down to the closest second. * Tokens minted before this timestamp are considered invalid. - * + * * @return Timestamp in milliseconds since the epoch. Tokens minted before this timestamp are * considered invalid. */ @@ -261,6 +261,10 @@ private static void checkPassword(String password) { checkArgument(password.length() >= 6, "password must be at least 6 characters long"); } + private static void checkProviderId(String providerId) { + checkArgument(!Strings.isNullOrEmpty(providerId), "providerId cannot be null or empty"); + } + static void checkCustomClaims(Map customClaims) { if (customClaims == null) { return; @@ -528,6 +532,17 @@ UpdateRequest setValidSince(long epochSeconds) { return this; } + UpdateRequest linkProvider(@NonNull UserProvider userProvider) { + properties.put("linkProviderUserInfo", userProvider); + return this; + } + + UpdateRequest deleteProvider(String providerId) { + checkProviderId(providerId); + properties.put("deleteProvider", ImmutableList.of(providerId)); + return this; + } + Map getProperties(JsonFactory jsonFactory) { Map copy = new HashMap<>(properties); List remove = new ArrayList<>(); diff --git a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java index badb9ead6..5e09d12e4 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -178,73 +179,134 @@ public void testCreateUserWithParams() throws Exception { public void testUserLifecycle() throws Exception { // Create user UserRecord userRecord = auth.createUserAsync(new CreateRequest()).get(); - String uid = userRecord.getUid(); - - // Get user - userRecord = auth.getUserAsync(userRecord.getUid()).get(); - assertEquals(uid, userRecord.getUid()); - assertNull(userRecord.getDisplayName()); - assertNull(userRecord.getEmail()); - assertNull(userRecord.getPhoneNumber()); - assertNull(userRecord.getPhotoUrl()); - assertFalse(userRecord.isEmailVerified()); - assertFalse(userRecord.isDisabled()); - assertTrue(userRecord.getUserMetadata().getCreationTimestamp() > 0); - assertEquals(0, userRecord.getUserMetadata().getLastSignInTimestamp()); - assertEquals(0, userRecord.getProviderData().length); - assertTrue(userRecord.getCustomClaims().isEmpty()); - - // Update user - RandomUser randomUser = RandomUser.create(); - String phone = randomPhoneNumber(); - UpdateRequest request = userRecord.updateRequest() - .setDisplayName("Updated Name") - .setEmail(randomUser.email) - .setPhoneNumber(phone) - .setPhotoUrl("https://example.com/photo.png") - .setEmailVerified(true) - .setPassword("secret"); - userRecord = auth.updateUserAsync(request).get(); - assertEquals(uid, userRecord.getUid()); - assertEquals("Updated Name", userRecord.getDisplayName()); - assertEquals(randomUser.email, userRecord.getEmail()); - assertEquals(phone, userRecord.getPhoneNumber()); - assertEquals("https://example.com/photo.png", userRecord.getPhotoUrl()); - assertTrue(userRecord.isEmailVerified()); - assertFalse(userRecord.isDisabled()); - assertEquals(2, userRecord.getProviderData().length); - assertTrue(userRecord.getCustomClaims().isEmpty()); - - // Get user by email - userRecord = auth.getUserByEmailAsync(userRecord.getEmail()).get(); - assertEquals(uid, userRecord.getUid()); - - // Disable user and remove properties - request = userRecord.updateRequest() - .setPhotoUrl(null) - .setDisplayName(null) - .setPhoneNumber(null) - .setDisabled(true); - userRecord = auth.updateUserAsync(request).get(); - assertEquals(uid, userRecord.getUid()); - assertNull(userRecord.getDisplayName()); - assertEquals(randomUser.email, userRecord.getEmail()); - assertNull(userRecord.getPhoneNumber()); - assertNull(userRecord.getPhotoUrl()); - assertTrue(userRecord.isEmailVerified()); - assertTrue(userRecord.isDisabled()); - assertEquals(1, userRecord.getProviderData().length); - assertTrue(userRecord.getCustomClaims().isEmpty()); - - // Delete user - auth.deleteUserAsync(userRecord.getUid()).get(); try { - auth.getUserAsync(userRecord.getUid()).get(); - fail("No error thrown for deleted user"); - } catch (ExecutionException e) { - assertTrue(e.getCause() instanceof FirebaseAuthException); - assertEquals(FirebaseUserManager.USER_NOT_FOUND_ERROR, - ((FirebaseAuthException) e.getCause()).getErrorCode()); + String uid = userRecord.getUid(); + + // Get user + userRecord = auth.getUserAsync(userRecord.getUid()).get(); + assertEquals(uid, userRecord.getUid()); + assertNull(userRecord.getDisplayName()); + assertNull(userRecord.getEmail()); + assertNull(userRecord.getPhoneNumber()); + assertNull(userRecord.getPhotoUrl()); + assertFalse(userRecord.isEmailVerified()); + assertFalse(userRecord.isDisabled()); + assertTrue(userRecord.getUserMetadata().getCreationTimestamp() > 0); + assertEquals(0, userRecord.getUserMetadata().getLastSignInTimestamp()); + assertEquals(0, userRecord.getProviderData().length); + assertTrue(userRecord.getCustomClaims().isEmpty()); + + // Update user + RandomUser randomUser = RandomUser.create(); + UpdateRequest request = userRecord.updateRequest() + .setDisplayName("Updated Name") + .setEmail(randomUser.email) + .setPhoneNumber(randomUser.phone) + .setPhotoUrl("https://example.com/photo.png") + .setEmailVerified(true) + .setPassword("secret"); + userRecord = auth.updateUserAsync(request).get(); + assertEquals(uid, userRecord.getUid()); + assertEquals("Updated Name", userRecord.getDisplayName()); + assertEquals(randomUser.email, userRecord.getEmail()); + assertEquals(randomUser.phone, userRecord.getPhoneNumber()); + assertEquals("https://example.com/photo.png", userRecord.getPhotoUrl()); + assertTrue(userRecord.isEmailVerified()); + assertFalse(userRecord.isDisabled()); + assertEquals(2, userRecord.getProviderData().length); + assertTrue(userRecord.getCustomClaims().isEmpty()); + + // Link user to IDP providers + request = userRecord.updateRequest() + .linkProvider( + UserProvider + .builder() + .setUid("testuid") + .setProviderId("google.com") + .setEmail("test@example.com") + .setDisplayName("Test User") + .setPhotoUrl("https://test.com/user.png") + .build()); + userRecord = auth.updateUserAsync(request).get(); + assertEquals(uid, userRecord.getUid()); + assertEquals("Updated Name", userRecord.getDisplayName()); + assertEquals(randomUser.email, userRecord.getEmail()); + assertEquals(randomUser.phone, userRecord.getPhoneNumber()); + assertEquals("https://example.com/photo.png", userRecord.getPhotoUrl()); + assertTrue(userRecord.isEmailVerified()); + assertFalse(userRecord.isDisabled()); + assertEquals(3, userRecord.getProviderData().length); + List providers = new ArrayList<>(); + for (UserInfo provider : userRecord.getProviderData()) { + providers.add(provider.getProviderId()); + } + assertTrue(providers.contains("google.com")); + assertTrue(userRecord.getCustomClaims().isEmpty()); + + // Unlink phone provider + request = userRecord.updateRequest().deleteProvider("phone"); + userRecord = auth.updateUserAsync(request).get(); + assertNull(userRecord.getPhoneNumber()); + assertEquals(2, userRecord.getProviderData().length); + providers.clear(); + for (UserInfo provider : userRecord.getProviderData()) { + providers.add(provider.getProviderId()); + } + assertFalse(providers.contains("phone")); + assertEquals(uid, userRecord.getUid()); + assertEquals("Updated Name", userRecord.getDisplayName()); + assertEquals(randomUser.email, userRecord.getEmail()); + assertEquals("https://example.com/photo.png", userRecord.getPhotoUrl()); + assertTrue(userRecord.isEmailVerified()); + assertFalse(userRecord.isDisabled()); + assertTrue(userRecord.getCustomClaims().isEmpty()); + + // Unlink IDP provider + request = userRecord.updateRequest().deleteProvider("google.com"); + userRecord = auth.updateUserAsync(request).get(); + assertEquals(1, userRecord.getProviderData().length); + assertNotEquals("google.com", userRecord.getProviderData()[0].getProviderId()); + assertEquals(uid, userRecord.getUid()); + assertEquals("Updated Name", userRecord.getDisplayName()); + assertEquals(randomUser.email, userRecord.getEmail()); + assertNull(userRecord.getPhoneNumber()); + assertEquals("https://example.com/photo.png", userRecord.getPhotoUrl()); + assertTrue(userRecord.isEmailVerified()); + assertFalse(userRecord.isDisabled()); + assertTrue(userRecord.getCustomClaims().isEmpty()); + + // Get user by email + userRecord = auth.getUserByEmailAsync(userRecord.getEmail()).get(); + assertEquals(uid, userRecord.getUid()); + + // Disable user and remove properties + request = userRecord.updateRequest() + .setPhotoUrl(null) + .setDisplayName(null) + .setPhoneNumber(null) + .setDisabled(true); + userRecord = auth.updateUserAsync(request).get(); + assertEquals(uid, userRecord.getUid()); + assertNull(userRecord.getDisplayName()); + assertEquals(randomUser.email, userRecord.getEmail()); + assertNull(userRecord.getPhoneNumber()); + assertNull(userRecord.getPhotoUrl()); + assertTrue(userRecord.isEmailVerified()); + assertTrue(userRecord.isDisabled()); + assertEquals(1, userRecord.getProviderData().length); + assertTrue(userRecord.getCustomClaims().isEmpty()); + + } finally { + // Delete user + auth.deleteUserAsync(userRecord.getUid()).get(); + try { + auth.getUserAsync(userRecord.getUid()).get(); + fail("No error thrown for deleted user"); + } catch (ExecutionException e) { + assertTrue(e.getCause() instanceof FirebaseAuthException); + assertEquals(FirebaseUserManager.USER_NOT_FOUND_ERROR, + ((FirebaseAuthException) e.getCause()).getErrorCode()); + } } } diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 97ff7447a..ab77e3ee0 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -74,6 +74,13 @@ public class FirebaseUserManagerTest { .build(); private static final Map ACTION_CODE_SETTINGS_MAP = ACTION_CODE_SETTINGS.getProperties(); + private static final UserProvider USER_PROVIDER = UserProvider.builder() + .setUid("testuid") + .setProviderId("facebook.com") + .setEmail("test@example.com") + .setDisplayName("Test User") + .setPhotoUrl("https://test.com/user.png") + .build(); @After public void tearDown() { @@ -826,8 +833,10 @@ public void testUserUpdater() throws IOException { .setEmailVerified(true) .setPassword("secret") .setCustomClaims(claims) + .linkProvider(USER_PROVIDER) + .deleteProvider("google.com") .getProperties(Utils.getDefaultJsonFactory()); - assertEquals(8, map.size()); + assertEquals(10, map.size()); assertEquals(update.getUid(), map.get("localId")); assertEquals("Display Name", map.get("displayName")); assertEquals("http://test.com/example.png", map.get("photoUrl")); @@ -836,6 +845,8 @@ public void testUserUpdater() throws IOException { assertTrue((Boolean) map.get("emailVerified")); assertEquals("secret", map.get("password")); assertEquals(Utils.getDefaultJsonFactory().toString(claims), map.get("customAttributes")); + assertEquals(USER_PROVIDER, map.get("linkProviderUserInfo")); + assertEquals(ImmutableList.of("google.com"), map.get("deleteProvider")); } @Test @@ -873,6 +884,28 @@ public void testEmptyCustomClaims() { assertEquals("{}", map.get("customAttributes")); } + @Test + public void testLinkProvider() { + UpdateRequest update = new UpdateRequest("test"); + Map map = update + .linkProvider(USER_PROVIDER) + .getProperties(Utils.getDefaultJsonFactory()); + assertEquals(2, map.size()); + assertEquals(update.getUid(), map.get("localId")); + assertEquals(USER_PROVIDER, map.get("linkProviderUserInfo")); + } + + @Test + public void testDeleteProvider() { + UpdateRequest update = new UpdateRequest("test"); + Map map = update + .deleteProvider("google.com") + .getProperties(Utils.getDefaultJsonFactory()); + assertEquals(2, map.size()); + assertEquals(update.getUid(), map.get("localId")); + assertEquals(ImmutableList.of("google.com"), map.get("deleteProvider")); + } + @Test public void testDeleteDisplayName() { Map map = new UpdateRequest("test") From dface3afbb1bfac907b3a0c01d74a6a4c2d094e4 Mon Sep 17 00:00:00 2001 From: Narges Simjour Date: Thu, 9 Jan 2020 16:36:56 -0500 Subject: [PATCH 2/6] Make changes in respond to first round of feedback. --- .../com/google/firebase/auth/UserRecord.java | 30 +++++++++++-------- .../google/firebase/auth/FirebaseAuthIT.java | 13 ++++---- .../auth/FirebaseUserManagerTest.java | 8 ++--- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/UserRecord.java b/src/main/java/com/google/firebase/auth/UserRecord.java index 8bd236c8c..1077babff 100644 --- a/src/main/java/com/google/firebase/auth/UserRecord.java +++ b/src/main/java/com/google/firebase/auth/UserRecord.java @@ -261,10 +261,6 @@ private static void checkPassword(String password) { checkArgument(password.length() >= 6, "password must be at least 6 characters long"); } - private static void checkProviderId(String providerId) { - checkArgument(!Strings.isNullOrEmpty(providerId), "providerId cannot be null or empty"); - } - static void checkCustomClaims(Map customClaims) { if (customClaims == null) { return; @@ -526,20 +522,30 @@ public UpdateRequest setCustomClaims(Map customClaims) { return this; } - UpdateRequest setValidSince(long epochSeconds) { - checkValidSince(epochSeconds); - properties.put("validSince", epochSeconds); + /** + * Updates the provider to be linked to this user\'s account. + * + * @param userProvider provider info to be linked to this user\'s account. + */ + public UpdateRequest setLinkProvider(@NonNull UserProvider userProvider) { + properties.put("linkProviderUserInfo", userProvider); return this; } - UpdateRequest linkProvider(@NonNull UserProvider userProvider) { - properties.put("linkProviderUserInfo", userProvider); + /** + * Updates the identity providers to unlink from this user\'s account. + * + * @param providerIds list of identifiers for the identity providers. + */ + public UpdateRequest setDeleteProviders(List providerIds) { + checkNotNull(providerIds); + properties.put("deleteProvider", providerIds); return this; } - UpdateRequest deleteProvider(String providerId) { - checkProviderId(providerId); - properties.put("deleteProvider", ImmutableList.of(providerId)); + UpdateRequest setValidSince(long epochSeconds) { + checkValidSince(epochSeconds); + properties.put("validSince", epochSeconds); return this; } diff --git a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java index 5e09d12e4..32914771d 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -198,10 +198,11 @@ public void testUserLifecycle() throws Exception { // Update user RandomUser randomUser = RandomUser.create(); + String phone = randomPhoneNumber(); UpdateRequest request = userRecord.updateRequest() .setDisplayName("Updated Name") .setEmail(randomUser.email) - .setPhoneNumber(randomUser.phone) + .setPhoneNumber(phone) .setPhotoUrl("https://example.com/photo.png") .setEmailVerified(true) .setPassword("secret"); @@ -209,7 +210,7 @@ public void testUserLifecycle() throws Exception { assertEquals(uid, userRecord.getUid()); assertEquals("Updated Name", userRecord.getDisplayName()); assertEquals(randomUser.email, userRecord.getEmail()); - assertEquals(randomUser.phone, userRecord.getPhoneNumber()); + assertEquals(phone, userRecord.getPhoneNumber()); assertEquals("https://example.com/photo.png", userRecord.getPhotoUrl()); assertTrue(userRecord.isEmailVerified()); assertFalse(userRecord.isDisabled()); @@ -218,7 +219,7 @@ public void testUserLifecycle() throws Exception { // Link user to IDP providers request = userRecord.updateRequest() - .linkProvider( + .setLinkProvider( UserProvider .builder() .setUid("testuid") @@ -231,7 +232,7 @@ public void testUserLifecycle() throws Exception { assertEquals(uid, userRecord.getUid()); assertEquals("Updated Name", userRecord.getDisplayName()); assertEquals(randomUser.email, userRecord.getEmail()); - assertEquals(randomUser.phone, userRecord.getPhoneNumber()); + assertEquals(phone, userRecord.getPhoneNumber()); assertEquals("https://example.com/photo.png", userRecord.getPhotoUrl()); assertTrue(userRecord.isEmailVerified()); assertFalse(userRecord.isDisabled()); @@ -244,7 +245,7 @@ public void testUserLifecycle() throws Exception { assertTrue(userRecord.getCustomClaims().isEmpty()); // Unlink phone provider - request = userRecord.updateRequest().deleteProvider("phone"); + request = userRecord.updateRequest().setDeleteProviders(ImmutableList.of("phone")); userRecord = auth.updateUserAsync(request).get(); assertNull(userRecord.getPhoneNumber()); assertEquals(2, userRecord.getProviderData().length); @@ -262,7 +263,7 @@ public void testUserLifecycle() throws Exception { assertTrue(userRecord.getCustomClaims().isEmpty()); // Unlink IDP provider - request = userRecord.updateRequest().deleteProvider("google.com"); + request = userRecord.updateRequest().setDeleteProviders(ImmutableList.of("google.com")); userRecord = auth.updateUserAsync(request).get(); assertEquals(1, userRecord.getProviderData().length); assertNotEquals("google.com", userRecord.getProviderData()[0].getProviderId()); diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index ab77e3ee0..32adafc53 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -833,8 +833,8 @@ public void testUserUpdater() throws IOException { .setEmailVerified(true) .setPassword("secret") .setCustomClaims(claims) - .linkProvider(USER_PROVIDER) - .deleteProvider("google.com") + .setLinkProvider(USER_PROVIDER) + .setDeleteProviders(ImmutableList.of("google.com")) .getProperties(Utils.getDefaultJsonFactory()); assertEquals(10, map.size()); assertEquals(update.getUid(), map.get("localId")); @@ -888,7 +888,7 @@ public void testEmptyCustomClaims() { public void testLinkProvider() { UpdateRequest update = new UpdateRequest("test"); Map map = update - .linkProvider(USER_PROVIDER) + .setLinkProvider(USER_PROVIDER) .getProperties(Utils.getDefaultJsonFactory()); assertEquals(2, map.size()); assertEquals(update.getUid(), map.get("localId")); @@ -899,7 +899,7 @@ public void testLinkProvider() { public void testDeleteProvider() { UpdateRequest update = new UpdateRequest("test"); Map map = update - .deleteProvider("google.com") + .setDeleteProviders(ImmutableList.of("google.com")) .getProperties(Utils.getDefaultJsonFactory()); assertEquals(2, map.size()); assertEquals(update.getUid(), map.get("localId")); From a9e09c293d91407dc86c9370872948aa9b205cd9 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 17 Mar 2021 17:00:43 -0400 Subject: [PATCH 3/6] review feedback --- .../com/google/firebase/auth/UserRecord.java | 25 +++++++++++++++++-- .../auth/FirebaseUserManagerTest.java | 12 +++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/UserRecord.java b/src/main/java/com/google/firebase/auth/UserRecord.java index 1077babff..fedc4a33f 100644 --- a/src/main/java/com/google/firebase/auth/UserRecord.java +++ b/src/main/java/com/google/firebase/auth/UserRecord.java @@ -537,8 +537,12 @@ public UpdateRequest setLinkProvider(@NonNull UserProvider userProvider) { * * @param providerIds list of identifiers for the identity providers. */ - public UpdateRequest setDeleteProviders(List providerIds) { + public UpdateRequest setDeleteProviders(Iterable providerIds) { checkNotNull(providerIds); + for (String id : providerIds) { + checkArgument(!Strings.isNullOrEmpty(id), "providerIds must not be null or empty"); + } + properties.put("deleteProvider", providerIds); return this; } @@ -564,7 +568,24 @@ Map getProperties(JsonFactory jsonFactory) { } if (copy.containsKey("phoneNumber") && copy.get("phoneNumber") == null) { - copy.put("deleteProvider", ImmutableList.of("phone")); + Object deleteProvider = copy.get("deleteProvider"); + if (deleteProvider != null) { + if (!(deleteProvider instanceof Iterable)) { + throw new IllegalStateException("Contents of deleteProvider was not iterable"); + } + + // Due to java's type erasure, we can't fully check the type. :( + @SuppressWarnings("unchecked") + Iterable deleteProviderIterable = (Iterable)deleteProvider; + + copy.put("deleteProvider", new ImmutableList.Builder() + .addAll(deleteProviderIterable) + .add("phone") + .build()); + } else { + copy.put("deleteProvider", ImmutableList.of("phone")); + } + copy.remove("phoneNumber"); } diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 32adafc53..c7113e9dd 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -906,6 +906,18 @@ public void testDeleteProvider() { assertEquals(ImmutableList.of("google.com"), map.get("deleteProvider")); } + @Test + public void testDeleteProviderAndPhone() { + UpdateRequest update = new UpdateRequest("test"); + Map map = update + .setDeleteProviders(ImmutableList.of("google.com")) + .setPhoneNumber(null) + .getProperties(Utils.getDefaultJsonFactory()); + assertEquals(2, map.size()); + assertEquals(update.getUid(), map.get("localId")); + assertEquals(ImmutableList.of("google.com", "phone"), map.get("deleteProvider")); + } + @Test public void testDeleteDisplayName() { Map map = new UpdateRequest("test") From 0bcaf694339d0e209291cb77a5c2ed61a6edcd25 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 17 Mar 2021 17:27:07 -0400 Subject: [PATCH 4/6] Update API to reflect changes made during api review --- .../com/google/firebase/auth/UserRecord.java | 22 ++++++++++++++----- .../google/firebase/auth/FirebaseAuthIT.java | 10 ++++----- .../auth/FirebaseUserManagerTest.java | 10 ++++----- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/UserRecord.java b/src/main/java/com/google/firebase/auth/UserRecord.java index e8f33422d..75e73c98b 100644 --- a/src/main/java/com/google/firebase/auth/UserRecord.java +++ b/src/main/java/com/google/firebase/auth/UserRecord.java @@ -549,21 +549,31 @@ public UpdateRequest setCustomClaims(Map customClaims) { } /** - * Updates the provider to be linked to this user\'s account. + * Links this user to the specified provider. * - * @param userProvider provider info to be linked to this user\'s account. + *

Linking a provider to an existing user account does not invalidate the + * refresh token of that account. In other words, the existing account + * would continue to be able to access resources, despite not having used + * the newly linked provider to log in. If you wish to force the user to + * authenticate with this new provider, you need to (a) revoke their + * refresh token (see + * https://firebase.google.com/docs/auth/admin/manage-sessions#revoke_refresh_tokens), + * and (b) ensure no other authentication methods are present on this + * account. + * + * @param providerToLink provider info to be linked to this user\'s account. */ - public UpdateRequest setLinkProvider(@NonNull UserProvider userProvider) { - properties.put("linkProviderUserInfo", userProvider); + public UpdateRequest setProviderToLink(@NonNull UserProvider providerToLink) { + properties.put("linkProviderUserInfo", providerToLink); return this; } /** - * Updates the identity providers to unlink from this user\'s account. + * Unlinks this user from the specified providers. * * @param providerIds list of identifiers for the identity providers. */ - public UpdateRequest setDeleteProviders(Iterable providerIds) { + public UpdateRequest setProvidersToUnlink(Iterable providerIds) { checkNotNull(providerIds); for (String id : providerIds) { checkArgument(!Strings.isNullOrEmpty(id), "providerIds must not be null or empty"); diff --git a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java index 1943fed6f..7c1da1080 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -269,7 +269,7 @@ public void testCreateUserWithParams() throws Exception { @Test public void testUserLifecycle() throws Exception { // Create user - UserRecord userRecord = auth.createUserAsync(new CreateRequest()).get(); + UserRecord userRecord = auth.createUserAsync(new UserRecord.CreateRequest()).get(); String uid = userRecord.getUid(); // Get user @@ -289,7 +289,7 @@ public void testUserLifecycle() throws Exception { // Update user RandomUser randomUser = UserTestUtils.generateRandomUserInfo(); - UpdateRequest request = userRecord.updateRequest() + UserRecord.UpdateRequest request = userRecord.updateRequest() .setDisplayName("Updated Name") .setEmail(randomUser.getEmail()) .setPhoneNumber(randomUser.getPhoneNumber()) @@ -310,7 +310,7 @@ public void testUserLifecycle() throws Exception { // Link user to IDP providers request = userRecord.updateRequest() - .setLinkProvider( + .setProviderToLink( UserProvider .builder() .setUid("testuid") @@ -336,7 +336,7 @@ public void testUserLifecycle() throws Exception { assertTrue(userRecord.getCustomClaims().isEmpty()); // Unlink phone provider - request = userRecord.updateRequest().setDeleteProviders(ImmutableList.of("phone")); + request = userRecord.updateRequest().setProvidersToUnlink(ImmutableList.of("phone")); userRecord = auth.updateUserAsync(request).get(); assertNull(userRecord.getPhoneNumber()); assertEquals(2, userRecord.getProviderData().length); @@ -354,7 +354,7 @@ public void testUserLifecycle() throws Exception { assertTrue(userRecord.getCustomClaims().isEmpty()); // Unlink IDP provider - request = userRecord.updateRequest().setDeleteProviders(ImmutableList.of("google.com")); + request = userRecord.updateRequest().setProvidersToUnlink(ImmutableList.of("google.com")); userRecord = auth.updateUserAsync(request).get(); assertEquals(1, userRecord.getProviderData().length); assertNotEquals("google.com", userRecord.getProviderData()[0].getProviderId()); diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 9f8e591f5..53a212e88 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -1098,8 +1098,8 @@ public void testUserUpdater() throws IOException { .setEmailVerified(true) .setPassword("secret") .setCustomClaims(claims) - .setLinkProvider(USER_PROVIDER) - .setDeleteProviders(ImmutableList.of("google.com")) + .setProviderToLink(USER_PROVIDER) + .setProvidersToUnlink(ImmutableList.of("google.com")) .getProperties(JSON_FACTORY); assertEquals(10, map.size()); assertEquals(update.getUid(), map.get("localId")); @@ -1153,7 +1153,7 @@ public void testEmptyCustomClaims() { public void testLinkProvider() { UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("test"); Map map = update - .setLinkProvider(USER_PROVIDER) + .setProviderToLink(USER_PROVIDER) .getProperties(Utils.getDefaultJsonFactory()); assertEquals(2, map.size()); assertEquals(update.getUid(), map.get("localId")); @@ -1164,7 +1164,7 @@ public void testLinkProvider() { public void testDeleteProvider() { UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("test"); Map map = update - .setDeleteProviders(ImmutableList.of("google.com")) + .setProvidersToUnlink(ImmutableList.of("google.com")) .getProperties(Utils.getDefaultJsonFactory()); assertEquals(2, map.size()); assertEquals(update.getUid(), map.get("localId")); @@ -1175,7 +1175,7 @@ public void testDeleteProvider() { public void testDeleteProviderAndPhone() { UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("test"); Map map = update - .setDeleteProviders(ImmutableList.of("google.com")) + .setProvidersToUnlink(ImmutableList.of("google.com")) .setPhoneNumber(null) .getProperties(Utils.getDefaultJsonFactory()); assertEquals(2, map.size()); From cf34cf1e71d71355e8c26a07e79dedda2ced9b3c Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 31 Mar 2021 15:19:30 -0400 Subject: [PATCH 5/6] review feedback --- .../com/google/firebase/auth/UserRecord.java | 21 ++++++++++++++----- .../auth/FirebaseUserManagerTest.java | 15 +++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/UserRecord.java b/src/main/java/com/google/firebase/auth/UserRecord.java index 75e73c98b..67c8ed684 100644 --- a/src/main/java/com/google/firebase/auth/UserRecord.java +++ b/src/main/java/com/google/firebase/auth/UserRecord.java @@ -564,7 +564,7 @@ public UpdateRequest setCustomClaims(Map customClaims) { * @param providerToLink provider info to be linked to this user\'s account. */ public UpdateRequest setProviderToLink(@NonNull UserProvider providerToLink) { - properties.put("linkProviderUserInfo", providerToLink); + properties.put("linkProviderUserInfo", checkNotNull(providerToLink)); return this; } @@ -606,14 +606,25 @@ Map getProperties(JsonFactory jsonFactory) { if (copy.containsKey("phoneNumber") && copy.get("phoneNumber") == null) { Object deleteProvider = copy.get("deleteProvider"); if (deleteProvider != null) { - if (!(deleteProvider instanceof Iterable)) { - throw new IllegalStateException("Contents of deleteProvider was not iterable"); - } - // Due to java's type erasure, we can't fully check the type. :( @SuppressWarnings("unchecked") Iterable deleteProviderIterable = (Iterable)deleteProvider; + // If we've been told to unlink the phone provider both via setting + // phoneNumber to null *and* by setting providersToUnlink to include + // 'phone', then we'll reject that. Though it might also be reasonable + // to relax this restriction and just unlink it. + for (String dp : deleteProviderIterable) { + if (dp == "phone") { + throw new IllegalArgumentException( + "Both UpdateRequest.setPhoneNumber(null) and " + + "UpdateRequest.setProvidersToUnlink(['phone']) were set. To " + + "unlink from a phone provider, only specify " + + "UpdateRequest.setPhoneNumber(null)."); + + } + } + copy.put("deleteProvider", new ImmutableList.Builder() .addAll(deleteProviderIterable) .add("phone") diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 53a212e88..6e4a03b00 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -1183,6 +1183,21 @@ public void testDeleteProviderAndPhone() { assertEquals(ImmutableList.of("google.com", "phone"), map.get("deleteProvider")); } + @Test + public void testDoubleDeletePhoneProvider() throws Exception { + UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("uid") + .setPhoneNumber(null) + .setProvidersToUnlink(ImmutableList.of("phone")); + + initializeAppForUserManagement(); + try { + FirebaseAuth.getInstance().updateUserAsync(update).get(); + fail("No error thrown for double delete phone provider"); + } catch (ExecutionException e) { + assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); + } + } + @Test public void testDeleteDisplayName() { Map map = new UserRecord.UpdateRequest("test") From e18af152e49548b4a54668210288d935b7577a0d Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Fri, 9 Apr 2021 14:55:31 -0400 Subject: [PATCH 6/6] review feedback --- .../com/google/firebase/auth/UserRecord.java | 49 +++++++++++++------ .../auth/FirebaseUserManagerTest.java | 19 +++++-- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/UserRecord.java b/src/main/java/com/google/firebase/auth/UserRecord.java index 67c8ed684..dea8fc94c 100644 --- a/src/main/java/com/google/firebase/auth/UserRecord.java +++ b/src/main/java/com/google/firebase/auth/UserRecord.java @@ -475,6 +475,29 @@ public UpdateRequest setPhoneNumber(@Nullable String phone) { if (phone != null) { checkPhoneNumber(phone); } + + if (phone == null && properties.containsKey("deleteProvider")) { + Object deleteProvider = properties.get("deleteProvider"); + if (deleteProvider != null) { + // Due to java's type erasure, we can't fully check the type. :( + @SuppressWarnings("unchecked") + Iterable deleteProviderIterable = (Iterable)deleteProvider; + + // If we've been told to unlink the phone provider both via setting phoneNumber to null + // *and* by setting providersToUnlink to include 'phone', then we'll reject that. Though + // it might also be reasonable to relax this restriction and just unlink it. + for (String dp : deleteProviderIterable) { + if (dp == "phone") { + throw new IllegalArgumentException( + "Both UpdateRequest.setPhoneNumber(null) and " + + "UpdateRequest.setProvidersToUnlink(['phone']) were set. To unlink from a " + + "phone provider, only specify UpdateRequest.setPhoneNumber(null)."); + + } + } + } + } + properties.put("phoneNumber", phone); return this; } @@ -577,6 +600,17 @@ public UpdateRequest setProvidersToUnlink(Iterable providerIds) { checkNotNull(providerIds); for (String id : providerIds) { checkArgument(!Strings.isNullOrEmpty(id), "providerIds must not be null or empty"); + + if (id == "phone" && properties.containsKey("phoneNumber") + && properties.get("phoneNumber") == null) { + // If we've been told to unlink the phone provider both via setting phoneNumber to null + // *and* by setting providersToUnlink to include 'phone', then we'll reject that. Though + // it might also be reasonable to relax this restriction and just unlink it. + throw new IllegalArgumentException( + "Both UpdateRequest.setPhoneNumber(null) and " + + "UpdateRequest.setProvidersToUnlink(['phone']) were set. To unlink from a phone " + + "provider, only specify UpdateRequest.setPhoneNumber(null)."); + } } properties.put("deleteProvider", providerIds); @@ -610,21 +644,6 @@ Map getProperties(JsonFactory jsonFactory) { @SuppressWarnings("unchecked") Iterable deleteProviderIterable = (Iterable)deleteProvider; - // If we've been told to unlink the phone provider both via setting - // phoneNumber to null *and* by setting providersToUnlink to include - // 'phone', then we'll reject that. Though it might also be reasonable - // to relax this restriction and just unlink it. - for (String dp : deleteProviderIterable) { - if (dp == "phone") { - throw new IllegalArgumentException( - "Both UpdateRequest.setPhoneNumber(null) and " - + "UpdateRequest.setProvidersToUnlink(['phone']) were set. To " - + "unlink from a phone provider, only specify " - + "UpdateRequest.setPhoneNumber(null)."); - - } - } - copy.put("deleteProvider", new ImmutableList.Builder() .addAll(deleteProviderIterable) .add("phone") diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 6e4a03b00..9b0a47b2c 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -1186,15 +1186,24 @@ public void testDeleteProviderAndPhone() { @Test public void testDoubleDeletePhoneProvider() throws Exception { UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("uid") - .setPhoneNumber(null) + .setPhoneNumber(null); + + try { + update.setProvidersToUnlink(ImmutableList.of("phone")); + fail("No error thrown for double delete phone provider"); + } catch (IllegalArgumentException expected) { + } + } + + @Test + public void testDoubleDeletePhoneProviderReverseOrder() throws Exception { + UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("uid") .setProvidersToUnlink(ImmutableList.of("phone")); - initializeAppForUserManagement(); try { - FirebaseAuth.getInstance().updateUserAsync(update).get(); + update.setPhoneNumber(null); fail("No error thrown for double delete phone provider"); - } catch (ExecutionException e) { - assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); + } catch (IllegalArgumentException expected) { } }