From 36a352a1a7eea48ebeb94558aa1c8af5ac8b5205 Mon Sep 17 00:00:00 2001 From: Micah Stairs Date: Tue, 12 May 2020 19:02:41 -0400 Subject: [PATCH 1/3] Rename OIDC delete operation and refactor integration tests. --- .../firebase/auth/AbstractFirebaseAuth.java | 24 +-- .../firebase/auth/FirebaseUserManager.java | 12 +- .../google/firebase/auth/FirebaseAuthIT.java | 187 ++++++++---------- .../auth/FirebaseUserManagerTest.java | 6 +- .../auth/ProviderConfigTestUtils.java | 74 +++++++ .../auth/TenantAwareFirebaseAuthIT.java | 153 ++++++-------- 6 files changed, 233 insertions(+), 223 deletions(-) create mode 100644 src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java diff --git a/src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java b/src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java index a90dcdb01..4315a26ac 100644 --- a/src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java +++ b/src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java @@ -970,7 +970,7 @@ public ApiFuture createOidcProviderConfigAsync( private CallableOperation createOidcProviderConfigOp(final OidcProviderConfig.CreateRequest request) { checkNotDestroyed(); - checkNotNull(request, "create request must not be null"); + checkNotNull(request, "Create request must not be null."); final FirebaseUserManager userManager = getUserManager(); return new CallableOperation() { @Override @@ -1010,7 +1010,7 @@ public ApiFuture updateOidcProviderConfigAsync( private CallableOperation updateOidcProviderConfigOp( final OidcProviderConfig.UpdateRequest request) { checkNotDestroyed(); - checkNotNull(request, "update request must not be null"); + checkNotNull(request, "Update request must not be null."); final FirebaseUserManager userManager = getUserManager(); return new CallableOperation() { @Override @@ -1051,7 +1051,7 @@ public ApiFuture getOidcProviderConfigAsync(@NonNull String private CallableOperation getOidcProviderConfigOp(final String providerId) { checkNotDestroyed(); - checkArgument(!Strings.isNullOrEmpty(providerId), "provider ID must not be null or empty"); + checkArgument(!Strings.isNullOrEmpty(providerId), "Provider ID must not be null or empty."); final FirebaseUserManager userManager = getUserManager(); return new CallableOperation() { @Override @@ -1149,18 +1149,18 @@ protected ListProviderConfigsPage execute() } /** - * Deletes the provider config identified by the specified provider ID. + * Deletes the OIDC Auth provider config identified by the specified provider ID. * * @param providerId A provider ID string. * @throws IllegalArgumentException If the provider ID string is null or empty. * @throws FirebaseAuthException If an error occurs while deleting the provider config. */ - public void deleteProviderConfig(@NonNull String providerId) throws FirebaseAuthException { - deleteProviderConfigOp(providerId).call(); + public void deleteOidcProviderConfig(@NonNull String providerId) throws FirebaseAuthException { + deleteOidcProviderConfigOp(providerId).call(); } /** - * Similar to {@link #deleteProviderConfig} but performs the operation asynchronously. + * Similar to {@link #deleteOidcProviderConfig} but performs the operation asynchronously. * * @param providerId A provider ID string. * @return An {@code ApiFuture} which will complete successfully when the specified provider @@ -1168,19 +1168,19 @@ public void deleteProviderConfig(@NonNull String providerId) throws FirebaseAuth * throws a {@link FirebaseAuthException}. * @throws IllegalArgumentException If the provider ID string is null or empty. */ - public ApiFuture deleteProviderConfigAsync(String providerId) { - return deleteProviderConfigOp(providerId).callAsync(firebaseApp); + public ApiFuture deleteOidcProviderConfigAsync(String providerId) { + return deleteOidcProviderConfigOp(providerId).callAsync(firebaseApp); } - private CallableOperation deleteProviderConfigOp( + private CallableOperation deleteOidcProviderConfigOp( final String providerId) { checkNotDestroyed(); - checkArgument(!Strings.isNullOrEmpty(providerId), "provider ID must not be null or empty"); + checkArgument(!Strings.isNullOrEmpty(providerId), "Provider ID must not be null or empty."); final FirebaseUserManager userManager = getUserManager(); return new CallableOperation() { @Override protected Void execute() throws FirebaseAuthException { - userManager.deleteProviderConfig(providerId); + userManager.deleteOidcProviderConfig(providerId); return null; } }; diff --git a/src/main/java/com/google/firebase/auth/FirebaseUserManager.java b/src/main/java/com/google/firebase/auth/FirebaseUserManager.java index 2605d62b8..690a832fa 100644 --- a/src/main/java/com/google/firebase/auth/FirebaseUserManager.java +++ b/src/main/java/com/google/firebase/auth/FirebaseUserManager.java @@ -319,7 +319,7 @@ OidcProviderConfig createOidcProviderConfig( OidcProviderConfig.CreateRequest request) throws FirebaseAuthException { GenericUrl url = new GenericUrl(idpConfigMgtBaseUrl + "/oauthIdpConfigs"); String providerId = request.getProviderId(); - checkArgument(!Strings.isNullOrEmpty(providerId), "provider ID must not be null or empty"); + checkArgument(!Strings.isNullOrEmpty(providerId), "Provider ID must not be null or empty."); url.set("oauthIdpConfigId", providerId); return sendRequest("POST", url, request.getProperties(), OidcProviderConfig.class); } @@ -328,7 +328,7 @@ OidcProviderConfig updateOidcProviderConfig(OidcProviderConfig.UpdateRequest req throws FirebaseAuthException { Map properties = request.getProperties(); checkArgument(!properties.isEmpty(), - "provider config update must have at least one property set"); + "Provider config update must have at least one property set."); GenericUrl url = new GenericUrl(idpConfigMgtBaseUrl + getOidcUrlSuffix(request.getProviderId())); url.put("updateMask", generateMask(properties)); @@ -346,7 +346,7 @@ ListOidcProviderConfigsResponse listOidcProviderConfigs(int maxResults, String p ImmutableMap.builder().put("pageSize", maxResults); if (pageToken != null) { checkArgument(!pageToken.equals( - ListTenantsPage.END_OF_LIST), "invalid end of list page token"); + ListTenantsPage.END_OF_LIST), "Invalid end of list page token."); builder.put("nextPageToken", pageToken); } @@ -360,7 +360,7 @@ ListOidcProviderConfigsResponse listOidcProviderConfigs(int maxResults, String p return response; } - void deleteProviderConfig(String providerId) throws FirebaseAuthException { + void deleteOidcProviderConfig(String providerId) throws FirebaseAuthException { GenericUrl url = new GenericUrl(idpConfigMgtBaseUrl + getOidcUrlSuffix(providerId)); sendRequest("DELETE", url, null, GenericJson.class); } @@ -374,12 +374,12 @@ private static String generateMask(Map properties) { } private static String getTenantUrlSuffix(String tenantId) { - checkArgument(!Strings.isNullOrEmpty(tenantId), "tenant ID must not be null or empty"); + checkArgument(!Strings.isNullOrEmpty(tenantId), "Tenant ID must not be null or empty."); return "/tenants/" + tenantId; } private static String getOidcUrlSuffix(String providerId) { - checkArgument(!Strings.isNullOrEmpty(providerId), "provider ID must not be null or empty"); + checkArgument(!Strings.isNullOrEmpty(providerId), "Provider ID must not be null or empty."); return "/oauthIdpConfigs/" + providerId; } diff --git a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java index bbc61ddf3..151eed800 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -46,6 +46,7 @@ import com.google.firebase.FirebaseApp; import com.google.firebase.FirebaseOptions; import com.google.firebase.ImplFirebaseTrampolines; +import com.google.firebase.auth.ProviderConfigTestUtils.TemporaryProviderConfig; import com.google.firebase.auth.hash.Scrypt; import com.google.firebase.internal.Nullable; import com.google.firebase.testing.IntegrationTestUtils; @@ -84,8 +85,9 @@ public class FirebaseAuthIT { private static final FirebaseAuth auth = FirebaseAuth.getInstance( IntegrationTestUtils.ensureDefaultApp()); - @Rule - public final TemporaryUser temporaryUser = new TemporaryUser(); + @Rule public final TemporaryUser temporaryUser = new TemporaryUser(); + @Rule public final TemporaryProviderConfig temporaryProviderConfig = + new TemporaryProviderConfig(auth); @Test public void testGetNonExistingUser() throws Exception { @@ -563,124 +565,113 @@ public void testGenerateSignInWithEmailLink() throws Exception { public void testOidcProviderConfigLifecycle() throws Exception { // Create config provider String providerId = "oidc.provider-id"; - OidcProviderConfig.CreateRequest createRequest = + OidcProviderConfig config = temporaryProviderConfig.createOidcProviderConfig( new OidcProviderConfig.CreateRequest() .setProviderId(providerId) .setDisplayName("DisplayName") .setEnabled(true) .setClientId("ClientId") - .setIssuer("https://oidc.com/issuer"); - OidcProviderConfig config = auth.createOidcProviderConfigAsync(createRequest).get(); + .setIssuer("https://oidc.com/issuer")); assertEquals(providerId, config.getProviderId()); assertEquals("DisplayName", config.getDisplayName()); assertTrue(config.isEnabled()); assertEquals("ClientId", config.getClientId()); assertEquals("https://oidc.com/issuer", config.getIssuer()); - try { - // Get config provider - config = auth.getOidcProviderConfigAsync(providerId).get(); - assertEquals(providerId, config.getProviderId()); - assertEquals("DisplayName", config.getDisplayName()); - assertTrue(config.isEnabled()); - assertEquals("ClientId", config.getClientId()); - assertEquals("https://oidc.com/issuer", config.getIssuer()); + // Get config provider + config = auth.getOidcProviderConfigAsync(providerId).get(); + assertEquals(providerId, config.getProviderId()); + assertEquals("DisplayName", config.getDisplayName()); + assertTrue(config.isEnabled()); + assertEquals("ClientId", config.getClientId()); + assertEquals("https://oidc.com/issuer", config.getIssuer()); - // Update config provider - OidcProviderConfig.UpdateRequest updateRequest = - new OidcProviderConfig.UpdateRequest(providerId) - .setDisplayName("NewDisplayName") - .setEnabled(false) - .setClientId("NewClientId") - .setIssuer("https://oidc.com/new-issuer"); - config = auth.updateOidcProviderConfigAsync(updateRequest).get(); - assertEquals(providerId, config.getProviderId()); - assertEquals("NewDisplayName", config.getDisplayName()); - assertFalse(config.isEnabled()); - assertEquals("NewClientId", config.getClientId()); - assertEquals("https://oidc.com/new-issuer", config.getIssuer()); - } finally { - // Delete config provider - auth.deleteProviderConfigAsync(providerId).get(); - } + // Update config provider + OidcProviderConfig.UpdateRequest updateRequest = + new OidcProviderConfig.UpdateRequest(providerId) + .setDisplayName("NewDisplayName") + .setEnabled(false) + .setClientId("NewClientId") + .setIssuer("https://oidc.com/new-issuer"); + config = auth.updateOidcProviderConfigAsync(updateRequest).get(); + assertEquals(providerId, config.getProviderId()); + assertEquals("NewDisplayName", config.getDisplayName()); + assertFalse(config.isEnabled()); + assertEquals("NewClientId", config.getClientId()); + assertEquals("https://oidc.com/new-issuer", config.getIssuer()); - assertOidcProviderConfigDoesNotExist(auth, providerId); + // Delete config provider + auth.deleteOidcProviderConfigAsync(providerId).get(); + ProviderConfigTestUtils.assertOidcProviderConfigDoesNotExist(auth, providerId); } @Test public void testListOidcProviderConfigs() throws Exception { final List providerIds = new ArrayList<>(); - try { - // Create provider configs - for (int i = 0; i < 3; i++) { - String providerId = "oidc.provider-id" + i; - providerIds.add(providerId); - OidcProviderConfig.CreateRequest createRequest = new OidcProviderConfig.CreateRequest() + // Create provider configs + for (int i = 0; i < 3; i++) { + String providerId = "oidc.provider-id" + i; + providerIds.add(providerId); + OidcProviderConfig config = temporaryProviderConfig.createOidcProviderConfig( + new OidcProviderConfig.CreateRequest() .setProviderId(providerId) .setClientId("CLIENT_ID") - .setIssuer("https://oidc.com/issuer"); - auth.createOidcProviderConfig(createRequest); - } - - // Test list by batches - final AtomicInteger collected = new AtomicInteger(0); - ListProviderConfigsPage page = - auth.listOidcProviderConfigsAsync(null).get(); - while (page != null) { - for (OidcProviderConfig providerConfig : page.getValues()) { - if (checkProviderConfig(providerIds, providerConfig)) { - collected.incrementAndGet(); - } - } - page = page.getNextPage(); - } - assertEquals(providerIds.size(), collected.get()); + .setIssuer("https://oidc.com/issuer")); + } - // Test iterate all - collected.set(0); - page = auth.listOidcProviderConfigsAsync(null).get(); - for (OidcProviderConfig providerConfig : page.iterateAll()) { + // Test list by batches + final AtomicInteger collected = new AtomicInteger(0); + ListProviderConfigsPage page = + auth.listOidcProviderConfigsAsync(null).get(); + while (page != null) { + for (OidcProviderConfig providerConfig : page.getValues()) { if (checkProviderConfig(providerIds, providerConfig)) { collected.incrementAndGet(); } } - assertEquals(providerIds.size(), collected.get()); - - // Test iterate async - collected.set(0); - final Semaphore semaphore = new Semaphore(0); - final AtomicReference error = new AtomicReference<>(); - ApiFuture> pageFuture = - auth.listOidcProviderConfigsAsync(null); - ApiFutures.addCallback( - pageFuture, - new ApiFutureCallback>() { - @Override - public void onFailure(Throwable t) { - error.set(t); - semaphore.release(); - } + page = page.getNextPage(); + } + assertEquals(providerIds.size(), collected.get()); - @Override - public void onSuccess(ListProviderConfigsPage result) { - for (OidcProviderConfig providerConfig : result.iterateAll()) { - if (checkProviderConfig(providerIds, providerConfig)) { - collected.incrementAndGet(); - } - } - semaphore.release(); - } - }, MoreExecutors.directExecutor()); - semaphore.acquire(); - assertEquals(providerIds.size(), collected.get()); - assertNull(error.get()); - } finally { - // Delete provider configs - for (String providerId : providerIds) { - auth.deleteProviderConfigAsync(providerId).get(); + // Test iterate all + collected.set(0); + page = auth.listOidcProviderConfigsAsync(null).get(); + for (OidcProviderConfig providerConfig : page.iterateAll()) { + if (checkProviderConfig(providerIds, providerConfig)) { + collected.incrementAndGet(); } } + assertEquals(providerIds.size(), collected.get()); + + // Test iterate async + collected.set(0); + final Semaphore semaphore = new Semaphore(0); + final AtomicReference error = new AtomicReference<>(); + ApiFuture> pageFuture = + auth.listOidcProviderConfigsAsync(null); + ApiFutures.addCallback( + pageFuture, + new ApiFutureCallback>() { + @Override + public void onFailure(Throwable t) { + error.set(t); + semaphore.release(); + } + + @Override + public void onSuccess(ListProviderConfigsPage result) { + for (OidcProviderConfig providerConfig : result.iterateAll()) { + if (checkProviderConfig(providerIds, providerConfig)) { + collected.incrementAndGet(); + } + } + semaphore.release(); + } + }, MoreExecutors.directExecutor()); + semaphore.acquire(); + assertEquals(providerIds.size(), collected.get()); + assertNull(error.get()); } private Map parseLinkParameters(String link) throws Exception { @@ -823,23 +814,11 @@ private boolean checkProviderConfig(List providerIds, OidcProviderConfig } - private static void assertOidcProviderConfigDoesNotExist( - AbstractFirebaseAuth firebaseAuth, String providerId) throws Exception { - try { - firebaseAuth.getOidcProviderConfigAsync(providerId).get(); - fail("No error thrown for getting a deleted provider config"); - } catch (ExecutionException e) { - assertTrue(e.getCause() instanceof FirebaseAuthException); - assertEquals(FirebaseUserManager.CONFIGURATION_NOT_FOUND_ERROR, - ((FirebaseAuthException) e.getCause()).getErrorCode()); - } - } - private static void assertUserDoesNotExist(AbstractFirebaseAuth firebaseAuth, String uid) throws Exception { try { firebaseAuth.getUserAsync(uid).get(); - fail("No error thrown for getting a user which was expected to be absent"); + fail("No error thrown for getting a user which was expected to be absent."); } catch (ExecutionException e) { assertTrue(e.getCause() instanceof FirebaseAuthException); assertEquals(FirebaseUserManager.USER_NOT_FOUND_ERROR, diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 31738dfa6..08d3c3dac 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -1709,7 +1709,7 @@ public void testTenantAwareListOidcProviderConfigs() throws Exception { public void testDeleteProviderConfig() throws Exception { TestResponseInterceptor interceptor = initializeAppForUserManagement("{}"); - FirebaseAuth.getInstance().deleteProviderConfig("PROVIDER_ID"); + FirebaseAuth.getInstance().deleteOidcProviderConfig("PROVIDER_ID"); checkRequestHeaders(interceptor); checkUrl(interceptor, "DELETE", PROJECT_BASE_URL + "/oauthIdpConfigs/PROVIDER_ID"); @@ -1721,7 +1721,7 @@ public void testDeleteProviderConfigWithNotFoundError() throws Exception { initializeAppForUserManagementWithStatusCode(404, "{\"error\": {\"message\": \"CONFIGURATION_NOT_FOUND\"}}"); try { - FirebaseAuth.getInstance().deleteProviderConfig("UNKNOWN"); + FirebaseAuth.getInstance().deleteOidcProviderConfig("UNKNOWN"); fail("No error thrown for invalid response"); } catch (FirebaseAuthException e) { assertEquals(FirebaseUserManager.CONFIGURATION_NOT_FOUND_ERROR, e.getErrorCode()); @@ -1737,7 +1737,7 @@ public void testTenantAwareDeleteProviderConfig() throws Exception { TenantAwareFirebaseAuth tenantAwareAuth = FirebaseAuth.getInstance().getTenantManager().getAuthForTenant("TENANT_ID"); - tenantAwareAuth.deleteProviderConfig("PROVIDER_ID"); + tenantAwareAuth.deleteOidcProviderConfig("PROVIDER_ID"); checkRequestHeaders(interceptor); checkUrl(interceptor, "DELETE", TENANTS_BASE_URL + "/TENANT_ID/oauthIdpConfigs/PROVIDER_ID"); diff --git a/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java b/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java new file mode 100644 index 000000000..7923c7dba --- /dev/null +++ b/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java @@ -0,0 +1,74 @@ +/* + * Copyright 2020 Google LLC + * + * 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.google.firebase.auth; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutionException; +import org.junit.rules.ExternalResource; + +class ProviderConfigTestUtils { + + static void assertOidcProviderConfigDoesNotExist( + AbstractFirebaseAuth firebaseAuth, String providerId) throws Exception { + try { + firebaseAuth.getOidcProviderConfigAsync(providerId).get(); + fail("No error thrown for getting a deleted OIDC provider config."); + } catch (ExecutionException e) { + assertTrue(e.getCause() instanceof FirebaseAuthException); + assertEquals(FirebaseUserManager.CONFIGURATION_NOT_FOUND_ERROR, + ((FirebaseAuthException) e.getCause()).getErrorCode()); + } + } + + /** + * Creates temporary provider configs for testing, and deletes them at the end of each test case. + */ + static final class TemporaryProviderConfig extends ExternalResource { + + private final AbstractFirebaseAuth auth; + private final List oidcIds = new ArrayList<>(); + + TemporaryProviderConfig(AbstractFirebaseAuth auth) { + this.auth = auth; + } + + OidcProviderConfig createOidcProviderConfig(OidcProviderConfig.CreateRequest request) + throws FirebaseAuthException { + OidcProviderConfig config = auth.createOidcProviderConfig(request); + oidcIds.add(config.getProviderId()); + return config; + } + + @Override + protected synchronized void after() { + for (String id : oidcIds) { + try { + auth.deleteOidcProviderConfig(id); + } catch (Exception ignore) { + // Ignore + } + } + oidcIds.clear(); + } + } +} + diff --git a/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java index ddfc9fe7e..776b75a2f 100644 --- a/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java @@ -38,6 +38,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.MoreExecutors; import com.google.firebase.FirebaseApp; +import com.google.firebase.auth.ProviderConfigTestUtils.TemporaryProviderConfig; import com.google.firebase.internal.Nullable; import com.google.firebase.testing.IntegrationTestUtils; import java.io.IOException; @@ -52,7 +53,9 @@ import org.junit.AfterClass; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExternalResource; public class TenantAwareFirebaseAuthIT { @@ -61,20 +64,21 @@ public class TenantAwareFirebaseAuthIT { private static final JsonFactory jsonFactory = Utils.getDefaultJsonFactory(); private static final HttpTransport transport = Utils.getDefaultTransport(); - private static FirebaseAuth auth; private static TenantManager tenantManager; - private static TenantAwareFirebaseAuth tenantAwareAuth; + private static TenantAwareFirebaseAuth auth; private static String tenantId; + @Rule public final TemporaryProviderConfig temporaryProviderConfig = + new TemporaryProviderConfig(auth); + @BeforeClass public static void setUpClass() throws Exception { FirebaseApp masterApp = IntegrationTestUtils.ensureDefaultApp(); - auth = FirebaseAuth.getInstance(masterApp); - tenantManager = auth.getTenantManager(); + tenantManager = FirebaseAuth.getInstance(masterApp).getTenantManager(); Tenant.CreateRequest tenantCreateRequest = new Tenant.CreateRequest().setDisplayName("DisplayName"); tenantId = tenantManager.createTenant(tenantCreateRequest).getTenantId(); - tenantAwareAuth = tenantManager.getAuthForTenant(tenantId); + auth = tenantManager.getAuthForTenant(tenantId); } @AfterClass @@ -85,11 +89,12 @@ public static void tearDownClass() throws Exception { @Test public void testUserLifecycle() throws Exception { // Create user - UserRecord userRecord = tenantAwareAuth.createUserAsync(new UserRecord.CreateRequest()).get(); + // TODO(micahstairs): Use a temporary user here. + UserRecord userRecord = auth.createUserAsync(new UserRecord.CreateRequest()).get(); String uid = userRecord.getUid(); // Get user - userRecord = tenantAwareAuth.getUserAsync(userRecord.getUid()).get(); + userRecord = auth.getUserAsync(userRecord.getUid()).get(); assertEquals(uid, userRecord.getUid()); assertEquals(tenantId, userRecord.getTenantId()); assertNull(userRecord.getDisplayName()); @@ -113,7 +118,7 @@ public void testUserLifecycle() throws Exception { .setPhotoUrl("https://example.com/photo.png") .setEmailVerified(true) .setPassword("secret"); - userRecord = tenantAwareAuth.updateUserAsync(request).get(); + userRecord = auth.updateUserAsync(request).get(); assertEquals(uid, userRecord.getUid()); assertEquals(tenantId, userRecord.getTenantId()); assertEquals("Updated Name", userRecord.getDisplayName()); @@ -126,7 +131,7 @@ public void testUserLifecycle() throws Exception { assertTrue(userRecord.getCustomClaims().isEmpty()); // Get user by email - userRecord = tenantAwareAuth.getUserByEmailAsync(userRecord.getEmail()).get(); + userRecord = auth.getUserByEmailAsync(userRecord.getEmail()).get(); assertEquals(uid, userRecord.getUid()); // Disable user and remove properties @@ -135,7 +140,7 @@ public void testUserLifecycle() throws Exception { .setDisplayName(null) .setPhoneNumber(null) .setDisabled(true); - userRecord = tenantAwareAuth.updateUserAsync(request).get(); + userRecord = auth.updateUserAsync(request).get(); assertEquals(uid, userRecord.getUid()); assertEquals(tenantId, userRecord.getTenantId()); assertNull(userRecord.getDisplayName()); @@ -148,8 +153,8 @@ public void testUserLifecycle() throws Exception { assertTrue(userRecord.getCustomClaims().isEmpty()); // Delete user - tenantAwareAuth.deleteUserAsync(userRecord.getUid()).get(); - assertUserDoesNotExist(tenantAwareAuth, userRecord.getUid()); + auth.deleteUserAsync(userRecord.getUid()).get(); + assertUserDoesNotExist(auth, userRecord.getUid()); } @Test @@ -158,14 +163,15 @@ public void testListUsers() throws Exception { try { for (int i = 0; i < 3; i++) { + // TODO(micahstairs): Use a temporary user here. UserRecord.CreateRequest createRequest = new UserRecord.CreateRequest().setPassword("password"); - uids.add(tenantAwareAuth.createUserAsync(createRequest).get().getUid()); + uids.add(auth.createUserAsync(createRequest).get().getUid()); } // Test list by batches final AtomicInteger collected = new AtomicInteger(0); - ListUsersPage page = tenantAwareAuth.listUsersAsync(null).get(); + ListUsersPage page = auth.listUsersAsync(null).get(); while (page != null) { for (ExportedUserRecord user : page.getValues()) { if (uids.contains(user.getUid())) { @@ -183,7 +189,7 @@ public void testListUsers() throws Exception { // Test iterate all collected.set(0); - page = tenantAwareAuth.listUsersAsync(null).get(); + page = auth.listUsersAsync(null).get(); for (ExportedUserRecord user : page.iterateAll()) { if (uids.contains(user.getUid())) { collected.incrementAndGet(); @@ -198,7 +204,7 @@ public void testListUsers() throws Exception { collected.set(0); final Semaphore semaphore = new Semaphore(0); final AtomicReference error = new AtomicReference<>(); - ApiFuture pageFuture = tenantAwareAuth.listUsersAsync(null); + ApiFuture pageFuture = auth.listUsersAsync(null); ApiFutures.addCallback(pageFuture, new ApiFutureCallback() { @Override public void onFailure(Throwable t) { @@ -224,60 +230,27 @@ public void onSuccess(ListUsersPage result) { assertNull(error.get()); } finally { for (String uid : uids) { - tenantAwareAuth.deleteUserAsync(uid).get(); + auth.deleteUserAsync(uid).get(); } } } - @Test - public void testGetUserWithMultipleTenantIds() throws Exception { - // Create second tenant. - Tenant.CreateRequest tenantCreateRequest = - new Tenant.CreateRequest().setDisplayName("DisplayName2"); - String tenantId2 = tenantManager.createTenant(tenantCreateRequest).getTenantId(); - - // Create three users (one without a tenant ID, and two with different tenant IDs). - UserRecord.CreateRequest createRequest = new UserRecord.CreateRequest(); - UserRecord nonTenantUserRecord = auth.createUser(createRequest); - UserRecord tenantUserRecord1 = tenantAwareAuth.createUser(createRequest); - TenantAwareFirebaseAuth tenantAwareAuth2 = auth.getTenantManager().getAuthForTenant(tenantId2); - UserRecord tenantUserRecord2 = tenantAwareAuth2.createUser(createRequest); - - // Make sure only non-tenant users can be fetched using the standard client. - assertNotNull(auth.getUser(nonTenantUserRecord.getUid())); - assertUserDoesNotExist(auth, tenantUserRecord1.getUid()); - assertUserDoesNotExist(auth, tenantUserRecord2.getUid()); - - // Make sure tenant-aware client cannot fetch users outside that tenant. - assertUserDoesNotExist(tenantAwareAuth, nonTenantUserRecord.getUid()); - assertUserDoesNotExist(tenantAwareAuth, tenantUserRecord2.getUid()); - assertUserDoesNotExist(tenantAwareAuth2, nonTenantUserRecord.getUid()); - assertUserDoesNotExist(tenantAwareAuth2, tenantUserRecord1.getUid()); - - // Make sure tenant-aware client can fetch users under that tenant. - assertNotNull(tenantAwareAuth.getUser(tenantUserRecord1.getUid())); - assertNotNull(tenantAwareAuth2.getUser(tenantUserRecord2.getUid())); - - // Delete second tenant. - tenantManager.deleteTenant(tenantId2); - } - @Test public void testCustomToken() throws Exception { - String customToken = tenantAwareAuth.createCustomTokenAsync("user1").get(); + String customToken = auth.createCustomTokenAsync("user1").get(); String idToken = signInWithCustomToken(customToken, tenantId); - FirebaseToken decoded = tenantAwareAuth.verifyIdTokenAsync(idToken).get(); + FirebaseToken decoded = auth.verifyIdTokenAsync(idToken).get(); assertEquals("user1", decoded.getUid()); assertEquals(tenantId, decoded.getTenantId()); } @Test public void testVerifyTokenWithWrongTenantAwareClient() throws Exception { - String customToken = tenantAwareAuth.createCustomTokenAsync("user").get(); + String customToken = auth.createCustomTokenAsync("user").get(); String idToken = signInWithCustomToken(customToken, tenantId); try { - auth.getTenantManager().getAuthForTenant("OTHER").verifyIdTokenAsync(idToken).get(); + tenantManager.getAuthForTenant("OTHER").verifyIdTokenAsync(idToken).get(); fail("No error thrown for verifying a token with the wrong tenant-aware client"); } catch (ExecutionException e) { assertTrue(e.getCause() instanceof FirebaseAuthException); @@ -297,7 +270,7 @@ public void testOidcProviderConfigLifecycle() throws Exception { .setEnabled(true) .setClientId("ClientId") .setIssuer("https://oidc.com/issuer"); - OidcProviderConfig config = tenantAwareAuth.createOidcProviderConfigAsync(createRequest).get(); + OidcProviderConfig config = auth.createOidcProviderConfigAsync(createRequest).get(); assertEquals(providerId, config.getProviderId()); assertEquals("DisplayName", config.getDisplayName()); assertEquals("ClientId", config.getClientId()); @@ -305,7 +278,7 @@ public void testOidcProviderConfigLifecycle() throws Exception { try { // Get config provider - config = tenantAwareAuth.getOidcProviderConfigAsync(providerId).get(); + config = auth.getOidcProviderConfigAsync(providerId).get(); assertEquals(providerId, config.getProviderId()); assertEquals("DisplayName", config.getDisplayName()); assertEquals("ClientId", config.getClientId()); @@ -318,7 +291,7 @@ public void testOidcProviderConfigLifecycle() throws Exception { .setEnabled(false) .setClientId("NewClientId") .setIssuer("https://oidc.com/new-issuer"); - config = tenantAwareAuth.updateOidcProviderConfigAsync(updateRequest).get(); + config = auth.updateOidcProviderConfigAsync(updateRequest).get(); assertEquals(providerId, config.getProviderId()); assertEquals("NewDisplayName", config.getDisplayName()); assertFalse(config.isEnabled()); @@ -326,45 +299,41 @@ public void testOidcProviderConfigLifecycle() throws Exception { assertEquals("https://oidc.com/new-issuer", config.getIssuer()); } finally { // Delete config provider - tenantAwareAuth.deleteProviderConfigAsync(providerId).get(); - assertOidcProviderConfigDoesNotExist(tenantAwareAuth, providerId); + auth.deleteOidcProviderConfigAsync(providerId).get(); + ProviderConfigTestUtils.assertOidcProviderConfigDoesNotExist(auth, providerId); } } @Test public void testListOidcProviderConfigs() throws Exception { final List providerIds = new ArrayList<>(); - try { - // Create provider configs - for (int i = 0; i < 3; i++) { - String providerId = "oidc.provider-id" + i; - providerIds.add(providerId); - OidcProviderConfig.CreateRequest createRequest = new OidcProviderConfig.CreateRequest() + + // Create provider configs + for (int i = 0; i < 3; i++) { + String providerId = "oidc.provider-id" + i; + providerIds.add(providerId); + temporaryProviderConfig.createOidcProviderConfig( + new OidcProviderConfig.CreateRequest() .setProviderId(providerId) .setClientId("CLIENT_ID") - .setIssuer("https://oidc.com/issuer"); - tenantAwareAuth.createOidcProviderConfig(createRequest); - } - - // List provider configs - final AtomicInteger collected = new AtomicInteger(0); - ListProviderConfigsPage page = - tenantAwareAuth.listOidcProviderConfigsAsync(null).get(); - for (OidcProviderConfig providerConfig : page.iterateAll()) { - if (providerIds.contains(providerConfig.getProviderId())) { - collected.incrementAndGet(); - assertEquals("CLIENT_ID", providerConfig.getClientId()); - assertEquals("https://oidc.com/issuer", providerConfig.getIssuer()); - } - } - assertEquals(providerIds.size(), collected.get()); + .setIssuer("https://oidc.com/issuer")); + } - } finally { - // Delete provider configs - for (String providerId : providerIds) { - tenantAwareAuth.deleteProviderConfigAsync(providerId).get(); + // List provider configs + // NOTE: We do not need to test all of the different ways we can iterate over the provider + // configs, since this testing is already performed in FirebaseAuthIT with the tenant-agnostic + // tests. + final AtomicInteger collected = new AtomicInteger(0); + ListProviderConfigsPage page = + auth.listOidcProviderConfigsAsync(null).get(); + for (OidcProviderConfig providerConfig : page.iterateAll()) { + if (providerIds.contains(providerConfig.getProviderId())) { + collected.incrementAndGet(); + assertEquals("CLIENT_ID", providerConfig.getClientId()); + assertEquals("https://oidc.com/issuer", providerConfig.getIssuer()); } } + assertEquals(providerIds.size(), collected.get()); } private String randomPhoneNumber() { @@ -415,23 +384,11 @@ static RandomUser create() { } } - private static void assertOidcProviderConfigDoesNotExist( - AbstractFirebaseAuth firebaseAuth, String providerId) throws Exception { - try { - firebaseAuth.getOidcProviderConfigAsync(providerId).get(); - fail("No error thrown for getting a deleted provider config"); - } catch (ExecutionException e) { - assertTrue(e.getCause() instanceof FirebaseAuthException); - assertEquals(FirebaseUserManager.CONFIGURATION_NOT_FOUND_ERROR, - ((FirebaseAuthException) e.getCause()).getErrorCode()); - } - } - private static void assertUserDoesNotExist(AbstractFirebaseAuth firebaseAuth, String uid) throws Exception { try { firebaseAuth.getUserAsync(uid).get(); - fail("No error thrown for getting a user which was expected to be absent"); + fail("No error thrown for getting a user which was expected to be absent."); } catch (ExecutionException e) { assertTrue(e.getCause() instanceof FirebaseAuthException); assertEquals(FirebaseUserManager.USER_NOT_FOUND_ERROR, From 3845b2c31b2065b7b8b51e8a9b699b39b42b8cbb Mon Sep 17 00:00:00 2001 From: Micah Stairs Date: Wed, 13 May 2020 17:33:55 -0400 Subject: [PATCH 2/3] Address pull request feedback. --- .../auth/ProviderConfigTestUtils.java | 35 ++++-- .../auth/TenantAwareFirebaseAuthIT.java | 104 +++++++++--------- 2 files changed, 76 insertions(+), 63 deletions(-) diff --git a/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java b/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java index 7923c7dba..e4a6fc77c 100644 --- a/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java +++ b/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java @@ -16,6 +16,7 @@ package com.google.firebase.auth; +import static com.google.common.base.Preconditions.checkArgument; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -51,23 +52,37 @@ static final class TemporaryProviderConfig extends ExternalResource { this.auth = auth; } - OidcProviderConfig createOidcProviderConfig(OidcProviderConfig.CreateRequest request) - throws FirebaseAuthException { - OidcProviderConfig config = auth.createOidcProviderConfig(request); - oidcIds.add(config.getProviderId()); + OidcProviderConfig createOidcProviderConfig( + OidcProviderConfig.CreateRequest request) throws FirebaseAuthException { + OidcProviderConfig config; + synchronized (oidcIds) { + config = auth.createOidcProviderConfig(request); + oidcIds.add(config.getProviderId()); + } return config; } + void deleteOidcProviderConfig(String providerId) throws FirebaseAuthException { + synchronized (oidcIds) { + checkArgument(oidcIds.contains(providerId), + "Provider ID is not currently associated with a temporary user."); + auth.deleteOidcProviderConfig(providerId); + oidcIds.remove(providerId); + } + } + @Override protected synchronized void after() { - for (String id : oidcIds) { - try { - auth.deleteOidcProviderConfig(id); - } catch (Exception ignore) { - // Ignore + synchronized (oidcIds) { + for (String id : oidcIds) { + try { + auth.deleteOidcProviderConfig(id); + } catch (Exception ignore) { + // Ignore + } } + oidcIds.clear(); } - oidcIds.clear(); } } } diff --git a/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java index 776b75a2f..e585f87b9 100644 --- a/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java @@ -65,11 +65,11 @@ public class TenantAwareFirebaseAuthIT { private static final HttpTransport transport = Utils.getDefaultTransport(); private static TenantManager tenantManager; - private static TenantAwareFirebaseAuth auth; + private static TenantAwareFirebaseAuth tenantAwareAuth; private static String tenantId; @Rule public final TemporaryProviderConfig temporaryProviderConfig = - new TemporaryProviderConfig(auth); + new TemporaryProviderConfig(tenantAwareAuth); @BeforeClass public static void setUpClass() throws Exception { @@ -78,7 +78,7 @@ public static void setUpClass() throws Exception { Tenant.CreateRequest tenantCreateRequest = new Tenant.CreateRequest().setDisplayName("DisplayName"); tenantId = tenantManager.createTenant(tenantCreateRequest).getTenantId(); - auth = tenantManager.getAuthForTenant(tenantId); + tenantAwareAuth = tenantManager.getAuthForTenant(tenantId); } @AfterClass @@ -90,11 +90,11 @@ public static void tearDownClass() throws Exception { public void testUserLifecycle() throws Exception { // Create user // TODO(micahstairs): Use a temporary user here. - UserRecord userRecord = auth.createUserAsync(new UserRecord.CreateRequest()).get(); + UserRecord userRecord = tenantAwareAuth.createUserAsync(new UserRecord.CreateRequest()).get(); String uid = userRecord.getUid(); // Get user - userRecord = auth.getUserAsync(userRecord.getUid()).get(); + userRecord = tenantAwareAuth.getUserAsync(userRecord.getUid()).get(); assertEquals(uid, userRecord.getUid()); assertEquals(tenantId, userRecord.getTenantId()); assertNull(userRecord.getDisplayName()); @@ -118,7 +118,7 @@ public void testUserLifecycle() throws Exception { .setPhotoUrl("https://example.com/photo.png") .setEmailVerified(true) .setPassword("secret"); - userRecord = auth.updateUserAsync(request).get(); + userRecord = tenantAwareAuth.updateUserAsync(request).get(); assertEquals(uid, userRecord.getUid()); assertEquals(tenantId, userRecord.getTenantId()); assertEquals("Updated Name", userRecord.getDisplayName()); @@ -131,7 +131,7 @@ public void testUserLifecycle() throws Exception { assertTrue(userRecord.getCustomClaims().isEmpty()); // Get user by email - userRecord = auth.getUserByEmailAsync(userRecord.getEmail()).get(); + userRecord = tenantAwareAuth.getUserByEmailAsync(userRecord.getEmail()).get(); assertEquals(uid, userRecord.getUid()); // Disable user and remove properties @@ -140,7 +140,7 @@ public void testUserLifecycle() throws Exception { .setDisplayName(null) .setPhoneNumber(null) .setDisabled(true); - userRecord = auth.updateUserAsync(request).get(); + userRecord = tenantAwareAuth.updateUserAsync(request).get(); assertEquals(uid, userRecord.getUid()); assertEquals(tenantId, userRecord.getTenantId()); assertNull(userRecord.getDisplayName()); @@ -153,8 +153,8 @@ public void testUserLifecycle() throws Exception { assertTrue(userRecord.getCustomClaims().isEmpty()); // Delete user - auth.deleteUserAsync(userRecord.getUid()).get(); - assertUserDoesNotExist(auth, userRecord.getUid()); + tenantAwareAuth.deleteUserAsync(userRecord.getUid()).get(); + assertUserDoesNotExist(tenantAwareAuth, userRecord.getUid()); } @Test @@ -166,12 +166,12 @@ public void testListUsers() throws Exception { // TODO(micahstairs): Use a temporary user here. UserRecord.CreateRequest createRequest = new UserRecord.CreateRequest().setPassword("password"); - uids.add(auth.createUserAsync(createRequest).get().getUid()); + uids.add(tenantAwareAuth.createUserAsync(createRequest).get().getUid()); } // Test list by batches final AtomicInteger collected = new AtomicInteger(0); - ListUsersPage page = auth.listUsersAsync(null).get(); + ListUsersPage page = tenantAwareAuth.listUsersAsync(null).get(); while (page != null) { for (ExportedUserRecord user : page.getValues()) { if (uids.contains(user.getUid())) { @@ -189,7 +189,7 @@ public void testListUsers() throws Exception { // Test iterate all collected.set(0); - page = auth.listUsersAsync(null).get(); + page = tenantAwareAuth.listUsersAsync(null).get(); for (ExportedUserRecord user : page.iterateAll()) { if (uids.contains(user.getUid())) { collected.incrementAndGet(); @@ -204,7 +204,7 @@ public void testListUsers() throws Exception { collected.set(0); final Semaphore semaphore = new Semaphore(0); final AtomicReference error = new AtomicReference<>(); - ApiFuture pageFuture = auth.listUsersAsync(null); + ApiFuture pageFuture = tenantAwareAuth.listUsersAsync(null); ApiFutures.addCallback(pageFuture, new ApiFutureCallback() { @Override public void onFailure(Throwable t) { @@ -230,23 +230,23 @@ public void onSuccess(ListUsersPage result) { assertNull(error.get()); } finally { for (String uid : uids) { - auth.deleteUserAsync(uid).get(); + tenantAwareAuth.deleteUserAsync(uid).get(); } } } @Test public void testCustomToken() throws Exception { - String customToken = auth.createCustomTokenAsync("user1").get(); + String customToken = tenantAwareAuth.createCustomTokenAsync("user1").get(); String idToken = signInWithCustomToken(customToken, tenantId); - FirebaseToken decoded = auth.verifyIdTokenAsync(idToken).get(); + FirebaseToken decoded = tenantAwareAuth.verifyIdTokenAsync(idToken).get(); assertEquals("user1", decoded.getUid()); assertEquals(tenantId, decoded.getTenantId()); } @Test public void testVerifyTokenWithWrongTenantAwareClient() throws Exception { - String customToken = auth.createCustomTokenAsync("user").get(); + String customToken = tenantAwareAuth.createCustomTokenAsync("user").get(); String idToken = signInWithCustomToken(customToken, tenantId); try { @@ -263,45 +263,43 @@ public void testVerifyTokenWithWrongTenantAwareClient() throws Exception { public void testOidcProviderConfigLifecycle() throws Exception { // Create config provider String providerId = "oidc.provider-id"; - OidcProviderConfig.CreateRequest createRequest = - new OidcProviderConfig.CreateRequest() - .setProviderId(providerId) - .setDisplayName("DisplayName") - .setEnabled(true) - .setClientId("ClientId") - .setIssuer("https://oidc.com/issuer"); - OidcProviderConfig config = auth.createOidcProviderConfigAsync(createRequest).get(); + OidcProviderConfig config = + temporaryProviderConfig.createOidcProviderConfig( + new OidcProviderConfig.CreateRequest() + .setProviderId(providerId) + .setDisplayName("DisplayName") + .setEnabled(true) + .setClientId("ClientId") + .setIssuer("https://oidc.com/issuer")); assertEquals(providerId, config.getProviderId()); assertEquals("DisplayName", config.getDisplayName()); assertEquals("ClientId", config.getClientId()); assertEquals("https://oidc.com/issuer", config.getIssuer()); - try { - // Get config provider - config = auth.getOidcProviderConfigAsync(providerId).get(); - assertEquals(providerId, config.getProviderId()); - assertEquals("DisplayName", config.getDisplayName()); - assertEquals("ClientId", config.getClientId()); - assertEquals("https://oidc.com/issuer", config.getIssuer()); - - // Update config provider - OidcProviderConfig.UpdateRequest updateRequest = - new OidcProviderConfig.UpdateRequest(providerId) - .setDisplayName("NewDisplayName") - .setEnabled(false) - .setClientId("NewClientId") - .setIssuer("https://oidc.com/new-issuer"); - config = auth.updateOidcProviderConfigAsync(updateRequest).get(); - assertEquals(providerId, config.getProviderId()); - assertEquals("NewDisplayName", config.getDisplayName()); - assertFalse(config.isEnabled()); - assertEquals("NewClientId", config.getClientId()); - assertEquals("https://oidc.com/new-issuer", config.getIssuer()); - } finally { - // Delete config provider - auth.deleteOidcProviderConfigAsync(providerId).get(); - ProviderConfigTestUtils.assertOidcProviderConfigDoesNotExist(auth, providerId); - } + // Get config provider + config = tenantAwareAuth.getOidcProviderConfigAsync(providerId).get(); + assertEquals(providerId, config.getProviderId()); + assertEquals("DisplayName", config.getDisplayName()); + assertEquals("ClientId", config.getClientId()); + assertEquals("https://oidc.com/issuer", config.getIssuer()); + + // Update config provider + OidcProviderConfig.UpdateRequest updateRequest = + new OidcProviderConfig.UpdateRequest(providerId) + .setDisplayName("NewDisplayName") + .setEnabled(false) + .setClientId("NewClientId") + .setIssuer("https://oidc.com/new-issuer"); + config = tenantAwareAuth.updateOidcProviderConfigAsync(updateRequest).get(); + assertEquals(providerId, config.getProviderId()); + assertEquals("NewDisplayName", config.getDisplayName()); + assertFalse(config.isEnabled()); + assertEquals("NewClientId", config.getClientId()); + assertEquals("https://oidc.com/new-issuer", config.getIssuer()); + + // Delete config provider + temporaryProviderConfig.deleteOidcProviderConfig(providerId); + ProviderConfigTestUtils.assertOidcProviderConfigDoesNotExist(tenantAwareAuth, providerId); } @Test @@ -325,7 +323,7 @@ public void testListOidcProviderConfigs() throws Exception { // tests. final AtomicInteger collected = new AtomicInteger(0); ListProviderConfigsPage page = - auth.listOidcProviderConfigsAsync(null).get(); + tenantAwareAuth.listOidcProviderConfigsAsync(null).get(); for (OidcProviderConfig providerConfig : page.iterateAll()) { if (providerIds.contains(providerConfig.getProviderId())) { collected.incrementAndGet(); From 91416b161205039fee9c6b13909f6b4781d47359 Mon Sep 17 00:00:00 2001 From: Micah Stairs Date: Wed, 13 May 2020 19:08:11 -0400 Subject: [PATCH 3/3] Simplify synchonization. --- .../auth/ProviderConfigTestUtils.java | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java b/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java index e4a6fc77c..0cc636d47 100644 --- a/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java +++ b/src/test/java/com/google/firebase/auth/ProviderConfigTestUtils.java @@ -52,37 +52,30 @@ static final class TemporaryProviderConfig extends ExternalResource { this.auth = auth; } - OidcProviderConfig createOidcProviderConfig( + synchronized OidcProviderConfig createOidcProviderConfig( OidcProviderConfig.CreateRequest request) throws FirebaseAuthException { - OidcProviderConfig config; - synchronized (oidcIds) { - config = auth.createOidcProviderConfig(request); - oidcIds.add(config.getProviderId()); - } + OidcProviderConfig config = auth.createOidcProviderConfig(request); + oidcIds.add(config.getProviderId()); return config; } - void deleteOidcProviderConfig(String providerId) throws FirebaseAuthException { - synchronized (oidcIds) { - checkArgument(oidcIds.contains(providerId), - "Provider ID is not currently associated with a temporary user."); - auth.deleteOidcProviderConfig(providerId); - oidcIds.remove(providerId); - } + synchronized void deleteOidcProviderConfig(String providerId) throws FirebaseAuthException { + checkArgument(oidcIds.contains(providerId), + "Provider ID is not currently associated with a temporary user."); + auth.deleteOidcProviderConfig(providerId); + oidcIds.remove(providerId); } @Override protected synchronized void after() { - synchronized (oidcIds) { - for (String id : oidcIds) { - try { - auth.deleteOidcProviderConfig(id); - } catch (Exception ignore) { - // Ignore - } + for (String id : oidcIds) { + try { + auth.deleteOidcProviderConfig(id); + } catch (Exception ignore) { + // Ignore } - oidcIds.clear(); } + oidcIds.clear(); } } }