-
Notifications
You must be signed in to change notification settings - Fork 289
feat(auth): Link federatedid #345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
52608d5
dface3a
a9e09c2
e5847c2
0bcaf69
cf34cf1
e18af15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<String> deleteProviderIterable = (Iterable<String>)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; | ||
} | ||
|
@@ -548,6 +571,52 @@ public UpdateRequest setCustomClaims(Map<String,Object> customClaims) { | |
return this; | ||
} | ||
|
||
/** | ||
* Links this user to the specified provider. | ||
* | ||
* <p>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 setProviderToLink(@NonNull UserProvider providerToLink) { | ||
properties.put("linkProviderUserInfo", checkNotNull(providerToLink)); | ||
return this; | ||
} | ||
|
||
/** | ||
* Unlinks this user from the specified providers. | ||
* | ||
* @param providerIds list of identifiers for the identity providers. | ||
*/ | ||
public UpdateRequest setProvidersToUnlink(Iterable<String> 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); | ||
rsgowman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return this; | ||
} | ||
|
||
UpdateRequest setValidSince(long epochSeconds) { | ||
checkValidSince(epochSeconds); | ||
properties.put("validSince", epochSeconds); | ||
|
@@ -569,7 +638,20 @@ Map<String, Object> 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) { | ||
// Due to java's type erasure, we can't fully check the type. :( | ||
@SuppressWarnings("unchecked") | ||
Iterable<String> deleteProviderIterable = (Iterable<String>)deleteProvider; | ||
|
||
copy.put("deleteProvider", new ImmutableList.Builder<String>() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we handle the double specification case?
I remember handling this case is other languages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Note that this results in the IllegalArgumentException being wrapped in an ExecutionException, which is a little awkward. (Option 1) Option 2:
(NB: This is not a public interface.) Option 3: I've left it as option 1 for now. lmk if you'd prefer 2 or 3. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to enforce this constraint in the setters? We'd have to do so in both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's better; done. I've removed the logic here, i.e. if we somehow get into a bad state, we won't notice it. (But now we shouldn't get into that state.) I didn't create a helper; there wasn't much duplication (though the exception itself is duplicated). If we set the value first and then checked, it would work out better... but that risks leaving the object in a bad state (eg if user catches and ignores the exception and then proceeds to use the object anyways.) |
||
.addAll(deleteProviderIterable) | ||
.add("phone") | ||
.build()); | ||
} else { | ||
copy.put("deleteProvider", ImmutableList.of("phone")); | ||
} | ||
|
||
copy.remove("phoneNumber"); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -307,6 +308,65 @@ public void testUserLifecycle() throws Exception { | |
assertEquals(2, userRecord.getProviderData().length); | ||
assertTrue(userRecord.getCustomClaims().isEmpty()); | ||
|
||
// Link user to IDP providers | ||
request = userRecord.updateRequest() | ||
.setProviderToLink( | ||
UserProvider | ||
.builder() | ||
.setUid("testuid") | ||
.setProviderId("google.com") | ||
.setEmail("[email protected]") | ||
.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.getEmail(), userRecord.getEmail()); | ||
assertEquals(randomUser.getPhoneNumber(), userRecord.getPhoneNumber()); | ||
assertEquals("https://example.com/photo.png", userRecord.getPhotoUrl()); | ||
assertTrue(userRecord.isEmailVerified()); | ||
assertFalse(userRecord.isDisabled()); | ||
assertEquals(3, userRecord.getProviderData().length); | ||
List<String> 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().setProvidersToUnlink(ImmutableList.of("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.getEmail(), 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().setProvidersToUnlink(ImmutableList.of("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.getEmail(), 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()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,6 +84,13 @@ public class FirebaseUserManagerTest { | |
|
||
private static final Map<String, Object> ACTION_CODE_SETTINGS_MAP = | ||
ACTION_CODE_SETTINGS.getProperties(); | ||
private static final UserProvider USER_PROVIDER = UserProvider.builder() | ||
.setUid("testuid") | ||
.setProviderId("facebook.com") | ||
.setEmail("[email protected]") | ||
.setDisplayName("Test User") | ||
.setPhotoUrl("https://test.com/user.png") | ||
.build(); | ||
|
||
private static final String PROJECT_BASE_URL = | ||
"https://identitytoolkit.googleapis.com/v2/projects/test-project-id"; | ||
|
@@ -1091,8 +1098,10 @@ public void testUserUpdater() throws IOException { | |
.setEmailVerified(true) | ||
.setPassword("secret") | ||
.setCustomClaims(claims) | ||
.setProviderToLink(USER_PROVIDER) | ||
.setProvidersToUnlink(ImmutableList.of("google.com")) | ||
.getProperties(JSON_FACTORY); | ||
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")); | ||
|
@@ -1101,6 +1110,8 @@ public void testUserUpdater() throws IOException { | |
assertTrue((Boolean) map.get("emailVerified")); | ||
assertEquals("secret", map.get("password")); | ||
assertEquals(JSON_FACTORY.toString(claims), map.get("customAttributes")); | ||
assertEquals(USER_PROVIDER, map.get("linkProviderUserInfo")); | ||
assertEquals(ImmutableList.of("google.com"), map.get("deleteProvider")); | ||
} | ||
|
||
@Test | ||
|
@@ -1138,6 +1149,64 @@ public void testEmptyCustomClaims() { | |
assertEquals("{}", map.get("customAttributes")); | ||
} | ||
|
||
@Test | ||
public void testLinkProvider() { | ||
UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("test"); | ||
Map<String, Object> map = update | ||
.setProviderToLink(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() { | ||
UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("test"); | ||
Map<String, Object> map = update | ||
.setProvidersToUnlink(ImmutableList.of("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 testDeleteProviderAndPhone() { | ||
UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("test"); | ||
Map<String, Object> map = update | ||
.setProvidersToUnlink(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 testDoubleDeletePhoneProvider() throws Exception { | ||
UserRecord.UpdateRequest update = new UserRecord.UpdateRequest("uid") | ||
.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")); | ||
|
||
try { | ||
update.setPhoneNumber(null); | ||
fail("No error thrown for double delete phone provider"); | ||
} catch (IllegalArgumentException expected) { | ||
} | ||
} | ||
|
||
@Test | ||
public void testDeleteDisplayName() { | ||
Map<String, Object> map = new UserRecord.UpdateRequest("test") | ||
|
Uh oh!
There was an error while loading. Please reload this page.