Skip to content

feat: enables OIDC auth code flow #522

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.

Sign up for GitHub

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 5 commits into from
Apr 16, 2021
Merged

feat: enables OIDC auth code flow #522

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e4a9206
feat: enables OIDC auth code flow
lsirac Apr 2, 2021
99a7a26
fix: remove GenericJson from public API surface
lsirac Apr 15, 2021
2aa7f38
fix: remove duplication
lsirac Apr 15, 2021
0e58c51
fix: camel case for test cases
lsirac Apr 16, 2021
6943b90
fix: remove import
lsirac Apr 16, 2021
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
117 changes: 117 additions & 0 deletions src/main/java/com/google/firebase/auth/OidcProviderConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

import static com.google.common.base.Preconditions.checkArgument;

import com.google.api.client.json.GenericJson;
import com.google.api.client.util.Key;
import com.google.common.base.Strings;
import java.util.HashMap;
import java.util.Map;

/**
* Contains metadata associated with an OIDC Auth provider.
Expand All @@ -31,17 +34,31 @@ public final class OidcProviderConfig extends ProviderConfig {
@Key("clientId")
private String clientId;

@Key("clientSecret")
private String clientSecret;

@Key("issuer")
private String issuer;

@Key("responseType")
private GenericJson responseType;

public String getClientId() {
return clientId;
}

public String getClientSecret() {
return clientSecret;
}

public String getIssuer() {
return issuer;
}

public GenericJson getResponseType() {
return responseType;
}

/**
* Returns a new {@link UpdateRequest}, which can be used to update the attributes of this
* provider config.
Expand Down Expand Up @@ -99,6 +116,19 @@ public CreateRequest setClientId(String clientId) {
return this;
}

/**
* Sets the client secret for the new provider. This is required for the code flow.
*
* @param clientSecret A non-null, non-empty client secret string.
* @throws IllegalArgumentException If the client secret is null or empty.
*/
public CreateRequest setClientSecret(String clientSecret) {
checkArgument(!Strings.isNullOrEmpty(clientSecret),
"Client Secret must not be null or empty.");
properties.put("clientSecret", clientSecret);
return this;
}

/**
* Sets the issuer for the new provider.
*
Expand All @@ -113,6 +143,43 @@ public CreateRequest setIssuer(String issuer) {
return this;
}

/**
* Sets whether to enable the code response flow for the new provider. By default, this is not
* enabled if no response type is specified.
*
* <p>A client secret must be set for this response type.
*
* <p>Having both the code and ID token response flows is currently not supported.
*
* @param enabled A boolean signifying whether the code response type is supported.
*/
public CreateRequest setCodeResponseType(boolean enabled) {
if (properties.get("responseType") == null) {
properties.put("responseType", new HashMap<String, Boolean>());
}
Map<String, Boolean> map = (Map<String, Boolean>) properties.get("responseType");
map.put("code", enabled);
return this;
}

/**
* Sets whether to enable the ID token response flow for the new provider. By default, this is
* enabled if no response type is specified.
*
* <p>Having both the code and ID token response flows is currently not supported.
*
* @param enabled A boolean signifying whether the ID token response type is supported.
*/
public CreateRequest setIdTokenResponseType(boolean enabled) {
if (properties.get("responseType") == null) {
properties.put("responseType", new HashMap<String, Boolean>());
}

Map<String, Boolean> map = (Map<String, Boolean>) properties.get("responseType");
map.put("idToken", enabled);
return this;
}

CreateRequest getThis() {
return this;
}
Expand Down Expand Up @@ -156,6 +223,19 @@ public UpdateRequest setClientId(String clientId) {
return this;
}

/**
* Sets the client secret for the new provider. This is required for the code flow.
*
* @param clientSecret A non-null, non-empty client secret string.
* @throws IllegalArgumentException If the client secret is null or empty.
*/
public UpdateRequest setClientSecret(String clientSecret) {
checkArgument(!Strings.isNullOrEmpty(clientSecret),
"Client Secret must not be null or empty.");
properties.put("clientSecret", clientSecret);
return this;
}

/**
* Sets the issuer for the existing provider.
*
Expand All @@ -170,6 +250,43 @@ public UpdateRequest setIssuer(String issuer) {
return this;
}

/**
* Sets whether to enable the code response flow for the new provider. By default, this is not
* enabled if no response type is specified.
*
* <p>A client secret must be set for this response type.
*
* <p>Having both the code and ID token response flows is currently not supported.
*
* @param enabled A boolean signifying whether the code response type is supported.
*/
public UpdateRequest setCodeResponseType(boolean enabled) {
if (properties.get("responseType") == null) {
properties.put("responseType", new HashMap<String, Boolean>());
}
Map<String, Boolean> map = (Map<String, Boolean>) properties.get("responseType");
map.put("code", enabled);
return this;
}

/**
* Sets whether to enable the ID token response flow for the new provider. By default, this is
* enabled if no response type is specified.
*
* <p>Having both the code and ID token response flows is currently not supported.
*
* @param enabled A boolean signifying whether the ID token response type is supported.
*/
public UpdateRequest setIdTokenResponseType(boolean enabled) {
if (properties.get("responseType") == null) {
properties.put("responseType", new HashMap<String, Boolean>());
}

Map<String, Boolean> map = (Map<String, Boolean>) properties.get("responseType");
map.put("idToken", enabled);
return this;
}

UpdateRequest getThis() {
return this;
}
Expand Down
24 changes: 22 additions & 2 deletions src/test/java/com/google/firebase/auth/FirebaseAuthIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,34 +703,54 @@ public void testOidcProviderConfigLifecycle() throws Exception {
.setDisplayName("DisplayName")
.setEnabled(true)
.setClientId("ClientId")
.setIssuer("https://oidc.com/issuer"));
.setClientSecret("ClientSecret")
.setIssuer("https://oidc.com/issuer")
.setCodeResponseType(true)
.setIdTokenResponseType(false));

assertEquals(providerId, config.getProviderId());
assertEquals("DisplayName", config.getDisplayName());
assertTrue(config.isEnabled());
assertEquals("ClientId", config.getClientId());
assertEquals("ClientSecret", config.getClientSecret());
assertEquals("https://oidc.com/issuer", config.getIssuer());
GenericJson responseType = config.getResponseType();
assertTrue((boolean) responseType.get("code"));
assertNull(responseType.get("idToken"));

// Get provider config
config = auth.getOidcProviderConfigAsync(providerId).get();
assertEquals(providerId, config.getProviderId());
assertEquals("DisplayName", config.getDisplayName());
assertTrue(config.isEnabled());
assertEquals("ClientId", config.getClientId());
assertEquals("ClientSecret", config.getClientSecret());
assertEquals("https://oidc.com/issuer", config.getIssuer());
responseType = config.getResponseType();
assertTrue((boolean) responseType.get("code"));
assertNull(responseType.get("idToken"));

// Update provider config
OidcProviderConfig.UpdateRequest updateRequest =
new OidcProviderConfig.UpdateRequest(providerId)
.setDisplayName("NewDisplayName")
.setEnabled(false)
.setClientId("NewClientId")
.setIssuer("https://oidc.com/new-issuer");
.setClientSecret("NewClientSecret")
.setIssuer("https://oidc.com/new-issuer")
.setCodeResponseType(false)
.setIdTokenResponseType(true);

config = auth.updateOidcProviderConfigAsync(updateRequest).get();
assertEquals(providerId, config.getProviderId());
assertEquals("NewDisplayName", config.getDisplayName());
assertFalse(config.isEnabled());
assertEquals("NewClientId", config.getClientId());
assertEquals("NewClientSecret", config.getClientSecret());
assertEquals("https://oidc.com/new-issuer", config.getIssuer());
responseType = config.getResponseType();
assertTrue((boolean) responseType.get("idToken"));
assertNull(responseType.get("code"));

// Delete provider config
temporaryProviderConfig.deleteOidcProviderConfig(providerId);
Expand Down
43 changes: 37 additions & 6 deletions src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1490,7 +1490,10 @@ public void testCreateOidcProvider() throws Exception {
.setDisplayName("DISPLAY_NAME")
.setEnabled(true)
.setClientId("CLIENT_ID")
.setIssuer("https://oidc.com/issuer");
.setClientSecret("CLIENT_SECRET")
.setIssuer("https://oidc.com/issuer")
.setCodeResponseType(true)
.setIdTokenResponseType(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the Node.js SDK we are adding checks to make sure that exactly one is true, when both code and idToken response types are specified. If we want we can do something similar here too. But it will probably require some additional changes.

In the AbstractCreateRequest and AbstractUpdateRequest classes:

protected void validate() { }

Map<String, Object> getProperties() {
    this.validate();
    return ImmutableMap.copyOf(properties);
}

Now in OidcProviderConfig.CreateRequest and OidcProviderConfig.UpdateRequest:

private static void validateResponseTypes(Map<String, Object> properties) {
  // Check if clientSecret is set when code = true
  // Check that when both code and idToken are specified, they are not both true or both false
}

@Override
protected void validate() {
  validateResponseTypes(this.properties);
}

Take it as a suggestion. I think for the initial release, the way it's implemented now is probably fine.


OidcProviderConfig config = FirebaseAuth.getInstance().createOidcProviderConfig(createRequest);

Expand All @@ -1501,7 +1504,13 @@ public void testCreateOidcProvider() throws Exception {
assertEquals("DISPLAY_NAME", parsed.get("displayName"));
assertTrue((boolean) parsed.get("enabled"));
assertEquals("CLIENT_ID", parsed.get("clientId"));
assertEquals("CLIENT_SECRET", parsed.get("clientSecret"));
assertEquals("https://oidc.com/issuer", parsed.get("issuer"));

Map<String, Boolean> responseType = (Map<String, Boolean>) parsed.get("responseType");
assertTrue(responseType.get("code"));
assertTrue(responseType.get("idToken"));

GenericUrl url = interceptor.getResponse().getRequest().getUrl();
assertEquals("oidc.provider-id", url.getFirst("oauthIdpConfigId"));
}
Expand All @@ -1515,9 +1524,12 @@ public void testCreateOidcProviderAsync() throws Exception {
.setDisplayName("DISPLAY_NAME")
.setEnabled(true)
.setClientId("CLIENT_ID")
.setIssuer("https://oidc.com/issuer");
.setClientSecret("CLIENT_SECRET")
.setIssuer("https://oidc.com/issuer")
.setCodeResponseType(true)
.setIdTokenResponseType(true);

OidcProviderConfig config =
OidcProviderConfig config =
FirebaseAuth.getInstance().createOidcProviderConfigAsync(createRequest).get();

checkOidcProviderConfig(config, "oidc.provider-id");
Expand All @@ -1527,7 +1539,13 @@ public void testCreateOidcProviderAsync() throws Exception {
assertEquals("DISPLAY_NAME", parsed.get("displayName"));
assertTrue((boolean) parsed.get("enabled"));
assertEquals("CLIENT_ID", parsed.get("clientId"));
assertEquals("CLIENT_SECRET", parsed.get("clientSecret"));
assertEquals("https://oidc.com/issuer", parsed.get("issuer"));

Map<String, Boolean> responseType = (Map<String, Boolean>) parsed.get("responseType");
assertTrue(responseType.get("code"));
assertTrue(responseType.get("idToken"));

GenericUrl url = interceptor.getResponse().getRequest().getUrl();
assertEquals("oidc.provider-id", url.getFirst("oauthIdpConfigId"));
}
Expand Down Expand Up @@ -1730,7 +1748,10 @@ public void testTenantAwareUpdateOidcProvider() throws Exception {
.setDisplayName("DISPLAY_NAME")
.setEnabled(true)
.setClientId("CLIENT_ID")
.setIssuer("https://oidc.com/issuer");
.setClientSecret("CLIENT_SECRET")
.setIssuer("https://oidc.com/issuer")
.setCodeResponseType(true)
.setIdTokenResponseType(true);

OidcProviderConfig config = tenantAwareAuth.updateOidcProviderConfig(request);

Expand All @@ -1739,12 +1760,18 @@ public void testTenantAwareUpdateOidcProvider() throws Exception {
String expectedUrl = TENANTS_BASE_URL + "/TENANT_ID/oauthIdpConfigs/oidc.provider-id";
checkUrl(interceptor, "PATCH", expectedUrl);
GenericUrl url = interceptor.getResponse().getRequest().getUrl();
assertEquals("clientId,displayName,enabled,issuer", url.getFirst("updateMask"));
assertEquals("clientId,clientSecret,displayName,enabled,issuer,responseType.code,"
+ "responseType.idToken", url.getFirst("updateMask"));
GenericJson parsed = parseRequestContent(interceptor);
assertEquals("DISPLAY_NAME", parsed.get("displayName"));
assertTrue((boolean) parsed.get("enabled"));
assertEquals("CLIENT_ID", parsed.get("clientId"));
assertEquals("CLIENT_SECRET", parsed.get("clientSecret"));
assertEquals("https://oidc.com/issuer", parsed.get("issuer"));

Map<String, Boolean> responseType = (Map<String, Boolean>) parsed.get("responseType");
assertTrue(responseType.get("code"));
assertTrue(responseType.get("idToken"));
}

@Test
Expand Down Expand Up @@ -2792,7 +2819,12 @@ private static void checkOidcProviderConfig(OidcProviderConfig config, String pr
assertEquals("DISPLAY_NAME", config.getDisplayName());
assertTrue(config.isEnabled());
assertEquals("CLIENT_ID", config.getClientId());
assertEquals("CLIENT_SECRET", config.getClientSecret());
assertEquals("https://oidc.com/issuer", config.getIssuer());

GenericJson responseType = config.getResponseType();
assertTrue((boolean) responseType.get("code"));
assertFalse((boolean) responseType.get("idToken"));
}

private static void checkSamlProviderConfig(SamlProviderConfig config, String providerId) {
Expand Down Expand Up @@ -2824,5 +2856,4 @@ private static void checkUrl(TestResponseInterceptor interceptor, String method,
private interface UserManagerOp {
void call(FirebaseAuth auth) throws Exception;
}

}
Loading