Make Javadoc comments consistent and validate provider ID in CreateRequest.

Author: micahstairs

src/main/java/com/google/firebase/auth/OidcProviderConfig.java

@@ -48,7 +48,7 @@ public String getIssuer() {
    * Returns a new {@link UpdateRequest}, which can be used to update the attributes of this
    * provider config.
    *
-   * @return a non-null {@link UpdateRequest} instance.
+   * @return A non-null {@link UpdateRequest} instance.
    */
   public UpdateRequest updateRequest() {
     return new UpdateRequest(getProviderId());
@@ -74,7 +74,8 @@ public CreateRequest() { }
     /**
      * Sets the client ID for the new provider.
      *
-     * @param clientId a non-null, non-empty client ID string.
+     * @param clientId A non-null, non-empty client ID string.
+     * @throws IllegalArgumentException If the client ID is null or empty.
      */
     public CreateRequest setClientId(String clientId) {
       checkArgument(!Strings.isNullOrEmpty(clientId), "Client ID must not be null or empty.");
@@ -85,7 +86,9 @@ public CreateRequest setClientId(String clientId) {
     /**
      * Sets the issuer for the new provider.
      *
-     * @param issuer a non-null, non-empty issuer URL string.
+     * @param issuer A non-null, non-empty issuer URL string.
+     * @throws IllegalArgumentException If the issuer URL is null or empty, or if the format is
+     *     invalid.
      */
     public CreateRequest setIssuer(String issuer) {
       checkArgument(!Strings.isNullOrEmpty(issuer), "Issuer must not be null or empty.");
@@ -97,6 +100,10 @@ public CreateRequest setIssuer(String issuer) {
     CreateRequest getThis() {
       return this;
     }
+
+    void assertValidProviderIdFormat(String providerId) {
+      checkArgument(providerId.startsWith("oidc."), "Invalid OIDC provider ID: " + providerId);
+    }
   }
 
   /**
@@ -116,9 +123,9 @@ public static final class UpdateRequest extends AbstractUpdateRequest<UpdateRequ
      * {@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, or if it's an invalid
-     *     format
+     * @param tenantId A non-null, non-empty provider ID string.
+     * @throws IllegalArgumentException If the provider ID is null or empty, or if the format is
+     *     invalid.
      */
     public UpdateRequest(String providerId) {
       super(providerId);
@@ -128,7 +135,8 @@ public UpdateRequest(String providerId) {
     /**
      * Sets the client ID for the exsting provider.
      *
-     * @param clientId a non-null, non-empty client ID string.
+     * @param clientId A non-null, non-empty client ID string.
+     * @throws IllegalArgumentException If the client ID is null or empty.
      */
     public UpdateRequest setClientId(String clientId) {
       checkArgument(!Strings.isNullOrEmpty(clientId), "Client ID must not be null or empty.");
@@ -139,7 +147,9 @@ public UpdateRequest setClientId(String clientId) {
     /**
      * Sets the issuer for the existing provider.
      *
-     * @param issuer a non-null, non-empty issuer URL string.
+     * @param issuer A non-null, non-empty issuer URL string.
+     * @throws IllegalArgumentException If the issuer URL is null or empty, or if the format is
+     *     invalid.
      */
     public UpdateRequest setIssuer(String issuer) {
       checkArgument(!Strings.isNullOrEmpty(issuer), "Issuer must not be null or empty.");

src/main/java/com/google/firebase/auth/ProviderConfig.java

@@ -74,11 +74,14 @@ public abstract static class AbstractCreateRequest<T extends AbstractCreateReque
     /**
      * Sets the ID for the new provider.
      *
-     * @param providerId a non-null, non-empty provider ID string.
+     * @param providerId A non-null, non-empty provider ID string.
+     * @throws IllegalArgumentException If the provider ID is null or empty, or if the format is
+     *     invalid.
      */
     public T setProviderId(String providerId) {
       checkArgument(
           !Strings.isNullOrEmpty(providerId), "Provider ID name must not be null or empty.");
+      assertValidProviderIdFormat(providerId);
       this.providerId = providerId;
       return getThis();
     }
@@ -90,7 +93,8 @@ String getProviderId() {
     /**
      * Sets the display name for the new provider.
      *
-     * @param displayName a non-null, non-empty display name string.
+     * @param displayName A non-null, non-empty display name string.
+     * @throws IllegalArgumentException If the display name is null or empty.
      */
     public T setDisplayName(String displayName) {
       checkArgument(!Strings.isNullOrEmpty(displayName), "Display name must not be null or empty.");
@@ -101,7 +105,7 @@ public T setDisplayName(String displayName) {
     /**
      * 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
+     * @param enabled A boolean indicating whether the user can sign in with the provider.
      */
     public T setEnabled(boolean enabled) {
       properties.put("enabled", enabled);
@@ -113,6 +117,8 @@ Map<String, Object> getProperties() {
     }
 
     abstract T getThis();
+
+    abstract void assertValidProviderIdFormat(String providerId);
   }
 
   /**
@@ -135,7 +141,8 @@ String getProviderId() {
     /**
      * Sets the display name for the existing provider.
      *
-     * @param displayName a non-null, non-empty display name string.
+     * @param displayName A non-null, non-empty display name string.
+     * @throws IllegalArgumentException If the display name is null or empty.
      */
     public T setDisplayName(String displayName) {
       checkArgument(!Strings.isNullOrEmpty(displayName), "Display name must not be null or empty.");
@@ -146,7 +153,7 @@ public T setDisplayName(String displayName) {
     /**
      * 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
+     * @param enabled A boolean indicating whether the user can sign in with the provider.
      */
     public T setEnabled(boolean enabled) {
       properties.put("enabled", enabled);

src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java

@@ -32,23 +32,23 @@ public class OidcProviderConfigTest {
   private static final JsonFactory jsonFactory = Utils.getDefaultJsonFactory();
 
   private static final String OIDC_JSON_STRING =
-      "{"
-        + "\"name\":\"projects/projectId/oauthIdpConfigs/oidc.provider-id\","
-        + "\"displayName\":\"DISPLAY_NAME\","
-        + "\"enabled\":true,"
-        + "\"clientId\":\"CLIENT_ID\","
-        + "\"issuer\":\"https://oidc.com/issuer\""
-        + "}";
+      ("{"
+        + "  'name':        'projects/projectId/oauthIdpConfigs/oidc.provider-id',"
+        + "  'displayName': 'DISPLAY_NAME',"
+        + "  'enabled':      true,"
+        + "  'clientId':    'CLIENT_ID',"
+        + "  'issuer':      'https://oidc.com/issuer'"
+        + "}").replace("'", "\"");
 
   @Test
   public void testJsonSerialization() throws IOException {
     OidcProviderConfig config = jsonFactory.fromString(OIDC_JSON_STRING, OidcProviderConfig.class);
 
-    assertEquals(config.getProviderId(), "oidc.provider-id");
-    assertEquals(config.getDisplayName(), "DISPLAY_NAME");
+    assertEquals("oidc.provider-id", config.getProviderId());
+    assertEquals("DISPLAY_NAME", config.getDisplayName());
     assertTrue(config.isEnabled());
-    assertEquals(config.getClientId(), "CLIENT_ID");
-    assertEquals(config.getIssuer(), "https://oidc.com/issuer");
+    assertEquals("CLIENT_ID", config.getClientId());
+    assertEquals("https://oidc.com/issuer", config.getIssuer());
   }
 
   @Test
@@ -70,11 +70,31 @@ public void testCreateRequest() throws IOException {
     assertEquals("https://oidc.com/issuer", (String) properties.get("issuer"));
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateRequestMissingProviderId() {
+    new OidcProviderConfig.CreateRequest().setProviderId(null);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateRequestInvalidProviderId() {
+    new OidcProviderConfig.CreateRequest().setProviderId("saml.provider-id");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateRequestMissingDisplayName() {
+    new OidcProviderConfig.CreateRequest().setDisplayName(null);
+  }
+
   @Test(expected = IllegalArgumentException.class)
   public void testCreateRequestMissingClientId() {
     new OidcProviderConfig.CreateRequest().setClientId(null);
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateRequestMissingIssuer() {
+    new OidcProviderConfig.CreateRequest().setIssuer(null);
+  }
+
   @Test(expected = IllegalArgumentException.class)
   public void testCreateRequestInvalidIssuerUrl() {
     new OidcProviderConfig.CreateRequest().setIssuer("not a valid url");
@@ -119,11 +139,21 @@ public void testUpdateRequestInvalidProviderId() {
     new OidcProviderConfig.UpdateRequest("saml.provider-id");
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testUpdateRequestMissingDisplayName() {
+    new OidcProviderConfig.UpdateRequest("oidc.provider-id").setDisplayName(null);
+  }
+
   @Test(expected = IllegalArgumentException.class)
   public void testUpdateRequestMissingClientId() {
     new OidcProviderConfig.UpdateRequest("oidc.provider-id").setClientId(null);
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testUpdateRequestMissingIssuer() {
+    new OidcProviderConfig.UpdateRequest("oidc.provider-id").setIssuer(null);
+  }
+
   @Test(expected = IllegalArgumentException.class)
   public void testUpdateRequestInvalidIssuerUrl() {
     new OidcProviderConfig.UpdateRequest("oidc.provider-id").setIssuer("not a valid url");