Skip to content

Make Javadoc comments consistent and validate provider ID in CreateRequest. #418

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

Merged
merged 2 commits into from
May 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/main/java/com/google/firebase/auth/OidcProviderConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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.");
Expand All @@ -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.");
Expand All @@ -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);
}
}

/**
Expand All @@ -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 is not prefixed with
* "oidc.".
*/
public UpdateRequest(String providerId) {
super(providerId);
Expand All @@ -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.");
Expand All @@ -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.");
Expand Down
17 changes: 12 additions & 5 deletions src/main/java/com/google/firebase/auth/ProviderConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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.");
Expand All @@ -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);
Expand All @@ -113,6 +117,8 @@ Map<String, Object> getProperties() {
}

abstract T getThis();

abstract void assertValidProviderIdFormat(String providerId);
}

/**
Expand All @@ -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.");
Expand All @@ -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);
Expand Down
52 changes: 41 additions & 11 deletions src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand Down