Repository: firebase/firebase-admin-java
Pull request #410 (2020-05-12 15:12:27)

Add validation to provider config ID.
The OIDC Auth provider config ID must begin with "oidc.". This validation is being done in other APIs (e.g. Go), so we should do it here as well.

I've moved assertValidUrl to the base class so that it can be reused for `SamlProviderConfig`, once that class is added.


Patch:
diff --git a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java
index 689298b..e2587da 100644
--- a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java
+++ b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java
@@ -20,13 +20,8 @@ import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.api.client.util.Key;
 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;
-import java.util.Map;
 
 /**
  * Contains metadata associated with an OIDC Auth provider.
@@ -59,14 +54,6 @@ public final class OidcProviderConfig extends ProviderConfig {
     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.
    *
@@ -90,7 +77,7 @@ public final class OidcProviderConfig extends ProviderConfig {
      * @param clientId a non-null, non-empty client ID string.
      */
     public CreateRequest setClientId(String clientId) {
-      checkArgument(!Strings.isNullOrEmpty(clientId), "client ID must not be null or empty");
+      checkArgument(!Strings.isNullOrEmpty(clientId), "Client ID must not be null or empty.");
       properties.put("clientId", clientId);
       return this;
     }
@@ -98,10 +85,10 @@ public final class OidcProviderConfig extends ProviderConfig {
     /**
      * Sets the issuer for the new provider.
      *
-     * @param issuer a non-null, non-empty issuer string.
+     * @param issuer a non-null, non-empty issuer URL string.
      */
     public CreateRequest setIssuer(String issuer) {
-      checkArgument(!Strings.isNullOrEmpty(issuer), "issuer must not be null or empty");
+      checkArgument(!Strings.isNullOrEmpty(issuer), "Issuer must not be null or empty.");
       assertValidUrl(issuer);
       properties.put("issuer", issuer);
       return this;
@@ -130,10 +117,12 @@ public final class OidcProviderConfig extends ProviderConfig {
      * information persistently.
      *
      * @param tenantId a non-null, non-empty provider ID string.
-     * @throws IllegalArgumentException If the provider ID is null or empty.
+     * @throws IllegalArgumentException If the provider ID is null or empty, or if it's an invalid
+     *     format
      */
     public UpdateRequest(String providerId) {
       super(providerId);
+      checkArgument(providerId.startsWith("oidc."), "Invalid OIDC provider ID: " + providerId);
     }
 
     /**
@@ -142,7 +131,7 @@ public final class OidcProviderConfig extends ProviderConfig {
      * @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");
+      checkArgument(!Strings.isNullOrEmpty(clientId), "Client ID must not be null or empty.");
       properties.put("clientId", clientId);
       return this;
     }
@@ -150,10 +139,10 @@ public final class OidcProviderConfig extends ProviderConfig {
     /**
      * Sets the issuer for the existing provider.
      *
-     * @param issuer a non-null, non-empty issuer string.
+     * @param issuer a non-null, non-empty issuer URL string.
      */
     public UpdateRequest setIssuer(String issuer) {
-      checkArgument(!Strings.isNullOrEmpty(issuer), "issuer must not be null or empty");
+      checkArgument(!Strings.isNullOrEmpty(issuer), "Issuer must not be null or empty.");
       assertValidUrl(issuer);
       properties.put("issuer", issuer);
       return this;
diff --git a/src/main/java/com/google/firebase/auth/ProviderConfig.java b/src/main/java/com/google/firebase/auth/ProviderConfig.java
index 434daab..09ed965 100644
--- a/src/main/java/com/google/firebase/auth/ProviderConfig.java
+++ b/src/main/java/com/google/firebase/auth/ProviderConfig.java
@@ -21,6 +21,8 @@ import static com.google.common.base.Preconditions.checkArgument;
 import com.google.api.client.util.Key;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -50,6 +52,14 @@ public abstract class ProviderConfig {
     return enabled;
   }
 
+  static void assertValidUrl(String url) throws IllegalArgumentException {
+    try {
+      new URL(url);
+    } catch (MalformedURLException e) {
+      throw new IllegalArgumentException(url + " is a malformed URL.", e);
+    }
+  }
+
   /**
    * A base specification class for creating a new provider.
    *
@@ -68,7 +78,7 @@ public abstract class ProviderConfig {
      */
     public T setProviderId(String providerId) {
       checkArgument(
-          !Strings.isNullOrEmpty(providerId), "provider ID name must not be null or empty");
+          !Strings.isNullOrEmpty(providerId), "Provider ID name must not be null or empty.");
       this.providerId = providerId;
       return getThis();
     }
@@ -83,7 +93,7 @@ public abstract class ProviderConfig {
      * @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");
+      checkArgument(!Strings.isNullOrEmpty(displayName), "Display name must not be null or empty.");
       properties.put("displayName", displayName);
       return getThis();
     }
@@ -114,7 +124,7 @@ public abstract class ProviderConfig {
     final Map<String,Object> properties = new HashMap<>();
 
     AbstractUpdateRequest(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.");
       this.providerId = providerId;
     }
 
@@ -128,7 +138,7 @@ public abstract class ProviderConfig {
      * @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");
+      checkArgument(!Strings.isNullOrEmpty(displayName), "Display name must not be null or empty.");
       properties.put("displayName", displayName);
       return getThis();
     }


Test patch:
diff --git a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java
index 2c62e57..8079c5d 100644
--- a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java
+++ b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java
@@ -114,6 +114,11 @@ public class OidcProviderConfigTest {
     new OidcProviderConfig.UpdateRequest(null);
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testUpdateRequestInvalidProviderId() {
+    new OidcProviderConfig.UpdateRequest("saml.provider-id");
+  }
+
   @Test(expected = IllegalArgumentException.class)
   public void testUpdateRequestMissingClientId() {
     new OidcProviderConfig.UpdateRequest("oidc.provider-id").setClientId(null);