Skip to content

Commit aa96da0

Browse files
committed
Finish implementing SAML provider config. (#428)
I've implemented addAllX509Certificates and removed the TODOs for the request signing, since we are not ready to expose that yet. I've also added unit tests for SamlProviderConfig.UpdateRequest, because those were missed in a previous PR. On an unrelated note, I've also added a comment describing why 'suppressLoadErrors' needs to be set.
1 parent 638240f commit aa96da0

File tree

4 files changed

+205
-11
lines changed

4 files changed

+205
-11
lines changed

checkstyle.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,8 @@
229229
<property name="allowedAnnotations" value="Override, Test"/>
230230
<property name="allowThrowsTagsForSubclasses" value="true"/>
231231
<property name="allowMissingJavadoc" value="true"/>
232+
<!-- Setting this property helps avoid some strange errors. For more information, see -->
233+
<!-- https://stackoverflow.com/questions/27938039/unable-to-get-class-information-for-checkstyle. -->
232234
<property name="suppressLoadErrors" value="true"/>
233235
</module>
234236
<module name="MethodName">

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

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.firebase.auth.ProviderConfig.AbstractCreateRequest;
2828
import com.google.firebase.auth.ProviderConfig.AbstractUpdateRequest;
2929
import java.util.ArrayList;
30+
import java.util.Collection;
3031
import java.util.HashMap;
3132
import java.util.List;
3233
import java.util.Map;
@@ -177,7 +178,23 @@ public CreateRequest addX509Certificate(String x509Certificate) {
177178
return this;
178179
}
179180

180-
// TODO(micahstairs): Add 'addAllX509Certificates' method.
181+
/**
182+
* Adds a collection of x509 certificates to the new provider.
183+
*
184+
* @param x509Certificates A non-null, non-empty collection of x509 certificate strings.
185+
* @throws IllegalArgumentException If the collection is null or empty, or if any x509
186+
* certificates are null or empty.
187+
*/
188+
public CreateRequest addAllX509Certificates(Collection<String> x509Certificates) {
189+
checkArgument(x509Certificates != null,
190+
"The collection of x509 certificates must not be null.");
191+
checkArgument(!x509Certificates.isEmpty(),
192+
"The collection of x509 certificates must not be empty.");
193+
for (String certificate : x509Certificates) {
194+
addX509Certificate(certificate);
195+
}
196+
return this;
197+
}
181198

182199
/**
183200
* Sets the RP entity ID for the new provider.
@@ -205,8 +222,6 @@ public CreateRequest setCallbackUrl(String callbackUrl) {
205222
return this;
206223
}
207224

208-
// TODO(micahstairs): Add 'setRequestSigningEnabled' method.
209-
210225
CreateRequest getThis() {
211226
return this;
212227
}
@@ -279,7 +294,23 @@ public UpdateRequest addX509Certificate(String x509Certificate) {
279294
return this;
280295
}
281296

282-
// TODO(micahstairs): Add 'addAllX509Certificates' method.
297+
/**
298+
* Adds a collection of x509 certificates to the existing provider.
299+
*
300+
* @param x509Certificates A non-null, non-empty collection of x509 certificate strings.
301+
* @throws IllegalArgumentException If the collection is null or empty, or if any x509
302+
* certificates are null or empty.
303+
*/
304+
public UpdateRequest addAllX509Certificates(Collection<String> x509Certificates) {
305+
checkArgument(x509Certificates != null,
306+
"The collection of x509 certificates must not be null.");
307+
checkArgument(!x509Certificates.isEmpty(),
308+
"The collection of x509 certificates must not be empty.");
309+
for (String certificate : x509Certificates) {
310+
addX509Certificate(certificate);
311+
}
312+
return this;
313+
}
283314

284315
/**
285316
* Sets the RP entity ID for the existing provider.
@@ -307,8 +338,6 @@ public UpdateRequest setCallbackUrl(String callbackUrl) {
307338
return this;
308339
}
309340

310-
// TODO(micahstairs): Add 'setRequestSigningEnabled' method.
311-
312341
UpdateRequest getThis() {
313342
return this;
314343
}

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2023,8 +2023,6 @@ public void testTenantAwareDeleteOidcProviderConfig() throws Exception {
20232023
public void testCreateSamlProvider() throws Exception {
20242024
TestResponseInterceptor interceptor = initializeAppForUserManagement(
20252025
TestUtils.loadResource("saml.json"));
2026-
// TODO(micahstairs): Add 'signRequest' to the create request once that field is added to
2027-
// SamlProviderConfig.
20282026
SamlProviderConfig.CreateRequest createRequest =
20292027
new SamlProviderConfig.CreateRequest()
20302028
.setProviderId("saml.provider-id")
@@ -2048,6 +2046,7 @@ public void testCreateSamlProvider() throws Exception {
20482046
GenericJson parsed = parseRequestContent(interceptor);
20492047
assertEquals("DISPLAY_NAME", parsed.get("displayName"));
20502048
assertTrue((boolean) parsed.get("enabled"));
2049+
20512050
Map<String, Object> idpConfig = (Map<String, Object>) parsed.get("idpConfig");
20522051
assertNotNull(idpConfig);
20532052
assertEquals(3, idpConfig.size());
@@ -2058,6 +2057,7 @@ public void testCreateSamlProvider() throws Exception {
20582057
assertEquals(2, idpCertificates.size());
20592058
assertEquals(ImmutableMap.of("x509Certificate", "certificate1"), idpCertificates.get(0));
20602059
assertEquals(ImmutableMap.of("x509Certificate", "certificate2"), idpCertificates.get(1));
2060+
20612061
Map<String, Object> spConfig = (Map<String, Object>) parsed.get("spConfig");
20622062
assertNotNull(spConfig);
20632063
assertEquals(2, spConfig.size());
@@ -2163,7 +2163,7 @@ public void testTenantAwareCreateSamlProvider() throws Exception {
21632163
TenantAwareFirebaseAuth tenantAwareAuth =
21642164
FirebaseAuth.getInstance().getTenantManager().getAuthForTenant("TENANT_ID");
21652165

2166-
SamlProviderConfig config = tenantAwareAuth.createSamlProviderConfig(createRequest);
2166+
tenantAwareAuth.createSamlProviderConfig(createRequest);
21672167

21682168
checkRequestHeaders(interceptor);
21692169
checkUrl(interceptor, "POST", TENANTS_BASE_URL + "/TENANT_ID/inboundSamlConfigs");
@@ -2173,8 +2173,6 @@ public void testTenantAwareCreateSamlProvider() throws Exception {
21732173
public void testUpdateSamlProvider() throws Exception {
21742174
TestResponseInterceptor interceptor = initializeAppForUserManagement(
21752175
TestUtils.loadResource("saml.json"));
2176-
// TODO(micahstairs): Add 'signRequest' to the create request once that field is added to
2177-
// SamlProviderConfig.
21782176
SamlProviderConfig.UpdateRequest updateRequest =
21792177
new SamlProviderConfig.UpdateRequest("saml.provider-id")
21802178
.setDisplayName("DISPLAY_NAME")

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

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,29 @@ public void testCreateRequest() throws IOException {
106106
assertEquals("https://projectId.firebaseapp.com/__/auth/handler", spConfig.get("callbackUri"));
107107
}
108108

109+
@Test
110+
public void testCreateRequestX509Certificates() throws IOException {
111+
SamlProviderConfig.CreateRequest createRequest =
112+
new SamlProviderConfig.CreateRequest()
113+
.addX509Certificate("certificate1")
114+
.addAllX509Certificates(ImmutableList.of("certificate2", "certificate3"))
115+
.addX509Certificate("certificate4");
116+
117+
Map<String,Object> properties = createRequest.getProperties();
118+
assertEquals(1, properties.size());
119+
Map<String, Object> idpConfig = (Map<String, Object>) properties.get("idpConfig");
120+
assertNotNull(idpConfig);
121+
assertEquals(1, idpConfig.size());
122+
123+
List<Object> idpCertificates = (List<Object>) idpConfig.get("idpCertificates");
124+
assertNotNull(idpCertificates);
125+
assertEquals(4, idpCertificates.size());
126+
assertEquals(ImmutableMap.of("x509Certificate", "certificate1"), idpCertificates.get(0));
127+
assertEquals(ImmutableMap.of("x509Certificate", "certificate2"), idpCertificates.get(1));
128+
assertEquals(ImmutableMap.of("x509Certificate", "certificate3"), idpCertificates.get(2));
129+
assertEquals(ImmutableMap.of("x509Certificate", "certificate4"), idpCertificates.get(3));
130+
}
131+
109132
@Test(expected = IllegalArgumentException.class)
110133
public void testCreateRequestMissingProviderId() {
111134
new SamlProviderConfig.CreateRequest().setProviderId(null);
@@ -141,6 +164,16 @@ public void testCreateRequestMissingX509Certificate() {
141164
new SamlProviderConfig.CreateRequest().addX509Certificate(null);
142165
}
143166

167+
@Test(expected = IllegalArgumentException.class)
168+
public void testCreateRequestNullX509CertificatesCollection() {
169+
new SamlProviderConfig.CreateRequest().addAllX509Certificates(null);
170+
}
171+
172+
@Test(expected = IllegalArgumentException.class)
173+
public void testCreateRequestEmptyX509CertificatesCollection() {
174+
new SamlProviderConfig.CreateRequest().addAllX509Certificates(ImmutableList.<String>of());
175+
}
176+
144177
@Test(expected = IllegalArgumentException.class)
145178
public void testCreateRequestMissingRpEntityId() {
146179
new SamlProviderConfig.CreateRequest().setRpEntityId(null);
@@ -155,4 +188,136 @@ public void testCreateRequestMissingCallbackUrl() {
155188
public void testCreateRequestInvalidCallbackUrl() {
156189
new SamlProviderConfig.CreateRequest().setCallbackUrl("not a valid url");
157190
}
191+
192+
@Test
193+
public void testUpdateRequestFromSamlProviderConfig() throws IOException {
194+
SamlProviderConfig config = jsonFactory.fromString(SAML_JSON_STRING, SamlProviderConfig.class);
195+
196+
SamlProviderConfig.UpdateRequest updateRequest = config.updateRequest();
197+
198+
assertEquals("saml.provider-id", updateRequest.getProviderId());
199+
assertTrue(updateRequest.getProperties().isEmpty());
200+
}
201+
202+
@Test
203+
public void testUpdateRequest() throws IOException {
204+
SamlProviderConfig.UpdateRequest updateRequest =
205+
new SamlProviderConfig.UpdateRequest("saml.provider-id");
206+
updateRequest
207+
.setDisplayName("DISPLAY_NAME")
208+
.setEnabled(false)
209+
.setIdpEntityId("IDP_ENTITY_ID")
210+
.setSsoUrl("https://example.com/login")
211+
.addX509Certificate("certificate1")
212+
.addX509Certificate("certificate2")
213+
.setRpEntityId("RP_ENTITY_ID")
214+
.setCallbackUrl("https://projectId.firebaseapp.com/__/auth/handler");
215+
216+
Map<String,Object> properties = updateRequest.getProperties();
217+
assertEquals(4, properties.size());
218+
assertEquals("DISPLAY_NAME", (String) properties.get("displayName"));
219+
assertFalse((boolean) properties.get("enabled"));
220+
221+
Map<String, Object> idpConfig = (Map<String, Object>) properties.get("idpConfig");
222+
assertNotNull(idpConfig);
223+
assertEquals(3, idpConfig.size());
224+
assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId"));
225+
assertEquals("https://example.com/login", idpConfig.get("ssoUrl"));
226+
List<Object> idpCertificates = (List<Object>) idpConfig.get("idpCertificates");
227+
assertNotNull(idpCertificates);
228+
assertEquals(2, idpCertificates.size());
229+
assertEquals(ImmutableMap.of("x509Certificate", "certificate1"), idpCertificates.get(0));
230+
assertEquals(ImmutableMap.of("x509Certificate", "certificate2"), idpCertificates.get(1));
231+
232+
Map<String, Object> spConfig = (Map<String, Object>) properties.get("spConfig");
233+
assertNotNull(spConfig);
234+
assertEquals(2, spConfig.size());
235+
assertEquals("RP_ENTITY_ID", spConfig.get("spEntityId"));
236+
assertEquals("https://projectId.firebaseapp.com/__/auth/handler", spConfig.get("callbackUri"));
237+
}
238+
239+
@Test
240+
public void testUpdateRequestX509Certificates() throws IOException {
241+
SamlProviderConfig.UpdateRequest updateRequest =
242+
new SamlProviderConfig.UpdateRequest("saml.provider-id");
243+
updateRequest
244+
.addX509Certificate("certificate1")
245+
.addAllX509Certificates(ImmutableList.of("certificate2", "certificate3"))
246+
.addX509Certificate("certificate4");
247+
248+
Map<String,Object> properties = updateRequest.getProperties();
249+
assertEquals(1, properties.size());
250+
Map<String, Object> idpConfig = (Map<String, Object>) properties.get("idpConfig");
251+
assertNotNull(idpConfig);
252+
assertEquals(1, idpConfig.size());
253+
254+
List<Object> idpCertificates = (List<Object>) idpConfig.get("idpCertificates");
255+
assertNotNull(idpCertificates);
256+
assertEquals(4, idpCertificates.size());
257+
assertEquals(ImmutableMap.of("x509Certificate", "certificate1"), idpCertificates.get(0));
258+
assertEquals(ImmutableMap.of("x509Certificate", "certificate2"), idpCertificates.get(1));
259+
assertEquals(ImmutableMap.of("x509Certificate", "certificate3"), idpCertificates.get(2));
260+
assertEquals(ImmutableMap.of("x509Certificate", "certificate4"), idpCertificates.get(3));
261+
}
262+
263+
@Test(expected = IllegalArgumentException.class)
264+
public void testUpdateRequestMissingProviderId() {
265+
new SamlProviderConfig.UpdateRequest(null);
266+
}
267+
268+
@Test(expected = IllegalArgumentException.class)
269+
public void testUpdateRequestInvalidProviderId() {
270+
new SamlProviderConfig.UpdateRequest("oidc.invalid-saml-provider-id");
271+
}
272+
273+
@Test(expected = IllegalArgumentException.class)
274+
public void testUpdateRequestMissingDisplayName() {
275+
new SamlProviderConfig.UpdateRequest("saml.provider-id").setDisplayName(null);
276+
}
277+
278+
@Test(expected = IllegalArgumentException.class)
279+
public void testUpdateRequestMissingIdpEntityId() {
280+
new SamlProviderConfig.UpdateRequest("saml.provider-id").setIdpEntityId(null);
281+
}
282+
283+
@Test(expected = IllegalArgumentException.class)
284+
public void testUpdateRequestMissingSsoUrl() {
285+
new SamlProviderConfig.UpdateRequest("saml.provider-id").setSsoUrl(null);
286+
}
287+
288+
@Test(expected = IllegalArgumentException.class)
289+
public void testUpdateRequestInvalidSsoUrl() {
290+
new SamlProviderConfig.UpdateRequest("saml.provider-id").setSsoUrl("not a valid url");
291+
}
292+
293+
@Test(expected = IllegalArgumentException.class)
294+
public void testUpdateRequestMissingX509Certificate() {
295+
new SamlProviderConfig.UpdateRequest("saml.provider-id").addX509Certificate(null);
296+
}
297+
298+
@Test(expected = IllegalArgumentException.class)
299+
public void testUpdateRequestNullX509CertificatesCollection() {
300+
new SamlProviderConfig.UpdateRequest("saml.provider-id").addAllX509Certificates(null);
301+
}
302+
303+
@Test(expected = IllegalArgumentException.class)
304+
public void testUpdateRequestEmptyX509CertificatesCollection() {
305+
new SamlProviderConfig.UpdateRequest("saml.provider-id")
306+
.addAllX509Certificates(ImmutableList.<String>of());
307+
}
308+
309+
@Test(expected = IllegalArgumentException.class)
310+
public void testUpdateRequestMissingRpEntityId() {
311+
new SamlProviderConfig.UpdateRequest("saml.provider-id").setRpEntityId(null);
312+
}
313+
314+
@Test(expected = IllegalArgumentException.class)
315+
public void testUpdateRequestMissingCallbackUrl() {
316+
new SamlProviderConfig.UpdateRequest("saml.provider-id").setCallbackUrl(null);
317+
}
318+
319+
@Test(expected = IllegalArgumentException.class)
320+
public void testUpdateRequestInvalidCallbackUrl() {
321+
new SamlProviderConfig.UpdateRequest("saml.provider-id").setCallbackUrl("not a valid url");
322+
}
158323
}

0 commit comments

Comments
 (0)