Add operation to update OIDC provider configs. (#402)

micahstairs · web-flow · commit ee98321d4156 · 2020-05-01T17:03:16.000-04:00
This adds an operation to update OIDC provider configs (can be done using either the tenant-aware or standard Firebase client). This work is part of adding multi-tenancy support (see issue #332).
diff --git a/src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java b/src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java
@@ -978,6 +978,46 @@ protected OidcProviderConfig execute() throws FirebaseAuthException {
     };
   }
 
+  /**
+   * Updates an existing OIDC Auth provider config with the attributes contained in the specified
+   * {@link OidcProviderConfig.UpdateRequest}.
+   *
+   * @param request A non-null {@link OidcProviderConfig.UpdateRequest} instance.
+   * @return A {@link OidcProviderConfig} instance corresponding to the updated provider config.
+   * @throws NullPointerException if the provided update request is null.
+   * @throws FirebaseAuthException if an error occurs while updating the provider config.
+   */
+  public OidcProviderConfig updateOidcProviderConfig(
+      @NonNull OidcProviderConfig.UpdateRequest request) throws FirebaseAuthException {
+    return updateOidcProviderConfigOp(request).call();
+  }
+
+  /**
+   * Similar to {@link #updateOidcProviderConfig} but performs the operation asynchronously.
+   *
+   * @param request A non-null {@link OidcProviderConfig.UpdateRequest} instance.
+   * @return An {@code ApiFuture} which will complete successfully with a {@link OidcProviderConfig}
+   *     instance corresponding to the updated provider config. If an error occurs while updating
+   *     the provider config, the future throws a {@link FirebaseAuthException}.
+   */
+  public ApiFuture<OidcProviderConfig> updateOidcProviderConfigAsync(
+      @NonNull OidcProviderConfig.UpdateRequest request) {
+    return updateOidcProviderConfigOp(request).callAsync(firebaseApp);
+  }
+
+  private CallableOperation<OidcProviderConfig, FirebaseAuthException> updateOidcProviderConfigOp(
+      final OidcProviderConfig.UpdateRequest request) {
+    checkNotDestroyed();
+    checkNotNull(request, "update request must not be null");
+    final FirebaseUserManager userManager = getUserManager();
+    return new CallableOperation<OidcProviderConfig, FirebaseAuthException>() {
+      @Override
+      protected OidcProviderConfig execute() throws FirebaseAuthException {
+        return userManager.updateOidcProviderConfig(request);
+      }
+    };
+  }
+
   /**
    * Gets the provider OIDC Auth config corresponding to the specified provider ID.
    *
diff --git a/src/main/java/com/google/firebase/auth/FirebaseUserManager.java b/src/main/java/com/google/firebase/auth/FirebaseUserManager.java
@@ -256,14 +256,6 @@ Tenant updateTenant(Tenant.UpdateRequest request) throws FirebaseAuthException {
     return sendRequest("PATCH", url, properties, Tenant.class);
   }
 
-  private static String generateMask(Map<String, Object> properties) {
-    // This implementation does not currently handle the case of nested properties. This is fine
-    // since we do not currently generate masks for any properties with nested values. When it
-    // comes time to implement this, we can check if a property has nested properties by checking
-    // if it is an instance of the Map class.
-    return Joiner.on(",").join(ImmutableSortedSet.copyOf(properties.keySet()));
-  }
-
   void deleteTenant(String tenantId) throws FirebaseAuthException {
     GenericUrl url = new GenericUrl(tenantMgtBaseUrl + getTenantUrlSuffix(tenantId));
     sendRequest("DELETE", url, null, GenericJson.class);
@@ -330,21 +322,45 @@ OidcProviderConfig createOidcProviderConfig(
     return sendRequest("POST", url, request.getProperties(), OidcProviderConfig.class);
   }
 
+  OidcProviderConfig updateOidcProviderConfig(OidcProviderConfig.UpdateRequest request)
+      throws FirebaseAuthException {
+    Map<String, Object> properties = request.getProperties();
+    checkArgument(!properties.isEmpty(),
+        "provider config update must have at least one property set");
+    GenericUrl url =
+        new GenericUrl(idpConfigMgtBaseUrl + getOidcUrlSuffix(request.getProviderId()));
+    url.put("updateMask", generateMask(properties));
+    return sendRequest("PATCH", url, properties, OidcProviderConfig.class);
+  }
+
   OidcProviderConfig getOidcProviderConfig(String providerId) throws FirebaseAuthException {
-    GenericUrl url = new GenericUrl(idpConfigMgtBaseUrl + "/oauthIdpConfigs/" + providerId);
+    GenericUrl url = new GenericUrl(idpConfigMgtBaseUrl + getOidcUrlSuffix(providerId));
     return sendRequest("GET", url, null, OidcProviderConfig.class);
   }
 
   void deleteProviderConfig(String providerId) throws FirebaseAuthException {
-    GenericUrl url = new GenericUrl(idpConfigMgtBaseUrl + "/oauthIdpConfigs/" + providerId);
+    GenericUrl url = new GenericUrl(idpConfigMgtBaseUrl + getOidcUrlSuffix(providerId));
     sendRequest("DELETE", url, null, GenericJson.class);
   }
 
+  private static String generateMask(Map<String, Object> properties) {
+    // This implementation does not currently handle the case of nested properties. This is fine
+    // since we do not currently generate masks for any properties with nested values. When it
+    // comes time to implement this, we can check if a property has nested properties by checking
+    // if it is an instance of the Map class.
+    return Joiner.on(",").join(ImmutableSortedSet.copyOf(properties.keySet()));
+  }
+
   private static String getTenantUrlSuffix(String tenantId) {
-    checkArgument(!Strings.isNullOrEmpty(tenantId));
+    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");
+    return "/oauthIdpConfigs/" + providerId;
+  }
+
   private <T> T post(String path, Object content, Class<T> clazz) throws FirebaseAuthException {
     checkArgument(!Strings.isNullOrEmpty(path), "path must not be null or empty");
     checkNotNull(content, "content must not be null for POST requests");
diff --git a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java
@@ -22,6 +22,7 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import com.google.firebase.auth.ProviderConfig.AbstractCreateRequest;
+import com.google.firebase.auth.ProviderConfig.AbstractUpdateRequest;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.HashMap;
@@ -48,6 +49,24 @@ public String getIssuer() {
     return issuer;
   }
 
+  /**
+   * Returns a new {@link UpdateRequest}, which can be used to update the attributes of this
+   * provider config.
+   *
+   * @return a non-null {@link UpdateRequest} instance.
+   */
+  public UpdateRequest updateRequest() {
+    return new UpdateRequest(getProviderId());
+  }
+
+  private static void assertValidUrl(String url) throws IllegalArgumentException {
+    try {
+      new URL(url);
+    } catch (MalformedURLException e) {
+      throw new IllegalArgumentException(url + " is a malformed URL", e);
+    }
+  }
+
   /**
    * A specification class for creating a new OIDC Auth provider.
    *
@@ -83,11 +102,7 @@ public CreateRequest setClientId(String clientId) {
      */
     public CreateRequest setIssuer(String issuer) {
       checkArgument(!Strings.isNullOrEmpty(issuer), "issuer must not be null or empty");
-      try {
-        new URL(issuer);
-      } catch (MalformedURLException e) {
-        throw new IllegalArgumentException(issuer + " is a malformed URL", e);
-      }
+      assertValidUrl(issuer);
       properties.put("issuer", issuer);
       return this;
     }
@@ -96,4 +111,56 @@ CreateRequest getThis() {
       return this;
     }
   }
+
+  /**
+   * A specification class for updating an existing OIDC Auth provider.
+   *
+   * <p>An instance of this class can be obtained via a {@link OidcProviderConfig} object, or from
+   * a provider ID string. Specify the changes to be made to the provider config by calling the
+   * various setter methods available in this class.
+   */
+  public static final class UpdateRequest extends AbstractUpdateRequest<UpdateRequest> {
+
+    /**
+     * Creates a new {@link UpdateRequest}, which can be used to updates an existing OIDC Auth
+     * provider.
+     *
+     * <p>The returned object should be passed to
+     * {@link AbstractFirebaseAuth#updateOidcProviderConfig(CreateRequest)} to update the provider
+     * information persistently.
+     *
+     * @param tenantId a non-null, non-empty provider ID string.
+     * @throws IllegalArgumentException If the provider ID is null or empty.
+     */
+    public UpdateRequest(String providerId) {
+      super(providerId);
+    }
+
+    /**
+     * Sets the client ID for the exsting provider.
+     *
+     * @param clientId a non-null, non-empty client ID string.
+     */
+    public UpdateRequest setClientId(String clientId) {
+      checkArgument(!Strings.isNullOrEmpty(clientId), "client ID must not be null or empty");
+      properties.put("clientId", clientId);
+      return this;
+    }
+
+    /**
+     * Sets the issuer for the existing provider.
+     *
+     * @param issuer a non-null, non-empty issuer string.
+     */
+    public UpdateRequest setIssuer(String issuer) {
+      checkArgument(!Strings.isNullOrEmpty(issuer), "issuer must not be null or empty");
+      assertValidUrl(issuer);
+      properties.put("issuer", issuer);
+      return this;
+    }
+
+    UpdateRequest getThis() {
+      return this;
+    }
+  }
 }
diff --git a/src/main/java/com/google/firebase/auth/ProviderConfig.java b/src/main/java/com/google/firebase/auth/ProviderConfig.java
@@ -104,4 +104,49 @@ Map<String, Object> getProperties() {
 
     abstract T getThis();
   }
+
+  /**
+   * A base class for updating the attributes of an existing provider.
+   */
+  public abstract static class AbstractUpdateRequest<T extends AbstractUpdateRequest<T>> {
+
+    final String providerId;
+    final Map<String,Object> properties = new HashMap<>();
+
+    AbstractUpdateRequest(String providerId) {
+      checkArgument(!Strings.isNullOrEmpty(providerId), "provider ID must not be null or empty");
+      this.providerId = providerId;
+    }
+
+    String getProviderId() {
+      return providerId;
+    }
+
+    /**
+     * Sets the display name for the existing provider.
+     *
+     * @param displayName a non-null, non-empty display name string.
+     */
+    public T setDisplayName(String displayName) {
+      checkArgument(!Strings.isNullOrEmpty(displayName), "display name must not be null or empty");
+      properties.put("displayName", displayName);
+      return getThis();
+    }
+
+    /**
+     * Sets whether to allow the user to sign in with the provider.
+     *
+     * @param enabled a boolean indicating whether the user can sign in with the provider
+     */
+    public T setEnabled(boolean enabled) {
+      properties.put("enabled", enabled);
+      return getThis();
+    }
+
+    Map<String, Object> getProperties() {
+      return ImmutableMap.copyOf(properties);
+    }
+
+    abstract T getThis();
+  }
 }
diff --git a/src/main/java/com/google/firebase/auth/Tenant.java b/src/main/java/com/google/firebase/auth/Tenant.java
@@ -60,10 +60,9 @@ public boolean isEmailLinkSignInEnabled() {
   }
 
   /**
-   * Returns a new {@link UpdateRequest}, which can be used to update the attributes
-   * of this tenant.
+   * Returns a new {@link UpdateRequest}, which can be used to update the attributes of this tenant.
    *
-   * @return a non-null Tenant.UpdateRequest instance.
+   * @return a non-null {@link UpdateRequest} instance.
    */
   public UpdateRequest updateRequest() {
     return new UpdateRequest(getTenantId());
@@ -157,7 +156,7 @@ String getTenantId() {
     }
 
     /**
-     * Sets the display name of the existingtenant.
+     * Sets the display name of the existing tenant.
      *
      * @param displayName a non-null, non-empty display name string.
      */
diff --git a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java
@@ -973,6 +973,7 @@ public void testOidcProviderConfigLifecycle() throws Exception {
     OidcProviderConfig config = auth.createOidcProviderConfigAsync(createRequest).get();
     assertEquals(providerId, config.getProviderId());
     assertEquals("DisplayName", config.getDisplayName());
+    assertTrue(config.isEnabled());
     assertEquals("ClientId", config.getClientId());
     assertEquals("https://oidc.com/issuer", config.getIssuer());
 
@@ -981,11 +982,23 @@ public void testOidcProviderConfigLifecycle() throws Exception {
       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());
 
-      // TODO(micahstairs): Test updateProviderConfig operation
-
+      // 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();
@@ -1028,8 +1041,19 @@ public void testTenantAwareOidcProviderConfigLifecycle() throws Exception {
         assertEquals("ClientId", config.getClientId());
         assertEquals("https://oidc.com/issuer", config.getIssuer());
 
-        // TODO(micahstairs): Test updateProviderConfig operation
-
+        // 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());
       } finally {
         // Delete config provider
         tenantAwareAuth.deleteProviderConfigAsync(providerId).get();
diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java
diff --git a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java

Original file line number	Diff line number	Diff line change
`@@ -60,10 +60,9 @@ public boolean isEmailLinkSignInEnabled() {`
`60`	`60`	`}`
`61`	`61`
`62`	`62`	`/**`
`63`		`- * Returns a new {@link UpdateRequest}, which can be used to update the attributes`
`64`		`- * of this tenant.`
	`63`	`+ * Returns a new {@link UpdateRequest}, which can be used to update the attributes of this tenant.`
`65`	`64`	`*`
`66`		`- * @return a non-null Tenant.UpdateRequest instance.`
	`65`	`+ * @return a non-null {@link UpdateRequest} instance.`
`67`	`66`	`*/`
`68`	`67`	`public UpdateRequest updateRequest() {`
`69`	`68`	`return new UpdateRequest(getTenantId());`
`@@ -157,7 +156,7 @@ String getTenantId() {`
`157`	`156`	`}`
`158`	`157`
`159`	`158`	`/**`
`160`		`- * Sets the display name of the existingtenant.`
	`159`	`+ * Sets the display name of the existing tenant.`
`161`	`160`	`*`
`162`	`161`	`* @param displayName a non-null, non-empty display name string.`
`163`	`162`	`*/`