Skip to content

Commit

Permalink
Improve logging
Browse files Browse the repository at this point in the history 
Ensure that the SAML IdentityProviderDefinition has consistent values in the idpEntityAlias as well as the zoneId.
https://www.pivotaltracker.com/story/show/91865364
[#91865364]
  • Loading branch information
@fhanik
fhanik committed Apr 10, 2015
1 parent c009ca0 commit d68b39c
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 18 deletions.
4 changes: 0 additions & 4 deletions ...on/src/main/java/org/cloudfoundry/identity/uaa/login/saml/IdentityProviderDefinition.java
View file
Expand Up @@ -87,10 +87,6 @@ public String getIdpEntityAlias() {
}

public void setIdpEntityAlias(String idpEntityAlias) {
if (idpEntityAlias==null) {
throw new NullPointerException("Alias can not be null");
}

this.idpEntityAlias = idpEntityAlias;
}

Expand Down
17 changes: 11 additions & 6 deletions common/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java
View file
Expand Up @@ -48,7 +48,7 @@
@RequestMapping("/identity-zones")
public class IdentityZoneEndpoints {

private static final Logger log = LoggerFactory.getLogger(IdentityZoneEndpoints.class);
private static final Logger logger = LoggerFactory.getLogger(IdentityZoneEndpoints.class);
private final IdentityZoneProvisioning zoneDao;
private final IdentityProviderProvisioning idpDao;
private final IdentityZoneEndpointClientRegistrationService clientRegistrationService;
Expand All @@ -75,6 +75,7 @@ public ResponseEntity<IdentityZone> createIdentityZone(@RequestBody @Valid Ident
}
IdentityZone previous = IdentityZoneHolder.get();
try {
logger.debug("Zone - creating id["+body.getId()+"] subdomain["+body.getSubdomain()+"]");
IdentityZone created = zoneDao.create(body);
IdentityZoneHolder.set(created);
IdentityProvider defaultIdp = new IdentityProvider();
Expand All @@ -83,7 +84,7 @@ public ResponseEntity<IdentityZone> createIdentityZone(@RequestBody @Valid Ident
defaultIdp.setOriginKey(Origin.UAA);
defaultIdp.setIdentityZoneId(created.getId());
idpDao.create(defaultIdp);

logger.debug("Zone - created id[" + created.getId() + "] subdomain[" + created.getSubdomain() + "]");
return new ResponseEntity<>(created, CREATED);
} finally {
IdentityZoneHolder.set(previous);
Expand All @@ -93,15 +94,16 @@ public ResponseEntity<IdentityZone> createIdentityZone(@RequestBody @Valid Ident
@RequestMapping(value = "{id}", method = PUT)
public ResponseEntity<IdentityZone> updateIdentityZone(
@RequestBody @Valid IdentityZone body, @PathVariable String id) {

IdentityZone previous = IdentityZoneHolder.get();
try {
logger.debug("Zone - updating id["+id+"] subdomain["+body.getSubdomain()+"]");
// make sure it exists
zoneDao.retrieve(id);
// ignore the id in the body, the id in the path is the only one that matters
body.setId(id);
IdentityZone updated = zoneDao.update(body);
IdentityZoneHolder.set(updated);
IdentityZoneHolder.set(updated); //what???
logger.debug("Zone - updated id[" + updated.getId() + "] subdomain[" + updated.getSubdomain() + "]");
return new ResponseEntity<>(updated, OK);
} finally {
IdentityZoneHolder.set(previous);
Expand All @@ -114,9 +116,11 @@ public ResponseEntity<? extends ClientDetails> createClient(

IdentityZone previous = IdentityZoneHolder.get();
try {
logger.debug("Zone creating client zone["+identityZoneId+"] client["+clientDetails.getClientId()+"]");
IdentityZone identityZone = zoneDao.retrieve(identityZoneId);
IdentityZoneHolder.set(identityZone);
ClientDetails createdClient = clientRegistrationService.createClient(clientDetails);
logger.debug("Zone client created zone["+identityZoneId+"] client["+clientDetails.getClientId()+"]");
return new ResponseEntity<>(removeSecret(createdClient), CREATED);
} finally {
IdentityZoneHolder.set(previous);
Expand All @@ -135,10 +139,11 @@ public ResponseEntity<? extends ClientDetails> deleteClient(

IdentityZone previous = IdentityZoneHolder.get();
try {
logger.debug("Zone deleting client zone["+identityZoneId+ "] client[" + clientId+"]");
IdentityZone identityZone = zoneDao.retrieve(identityZoneId);
IdentityZoneHolder.set(identityZone);
ClientDetails deleted = clientRegistrationService.deleteClient(clientId);

logger.debug("Zone client deleted zone["+identityZoneId+"] client["+clientId+"]");
return new ResponseEntity<>(removeSecret(deleted), OK);
} finally {
IdentityZoneHolder.set(previous);
Expand Down Expand Up @@ -183,7 +188,7 @@ public ResponseEntity<Void> handleAccessDeniedException(MethodArgumentNotValidEx

@ExceptionHandler(Exception.class)
public ResponseEntity<Void> handleException(Exception e) {
log.error(e.getClass() + ": " + e.getMessage(), e);
logger.error(e.getClass() + ": " + e.getMessage(), e);
return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
}

Expand Down
11 changes: 11 additions & 0 deletions ...on/src/main/java/org/cloudfoundry/identity/uaa/zone/JdbcIdentityProviderProvisioning.java
View file
Expand Up @@ -12,13 +12,17 @@
*******************************************************************************/
package org.cloudfoundry.identity.uaa.zone;

import org.cloudfoundry.identity.uaa.authentication.Origin;
import org.cloudfoundry.identity.uaa.login.saml.IdentityProviderDefinition;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.dao.DuplicateKeyException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.PreparedStatementSetter;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.RequestBody;

import java.sql.PreparedStatement;
import java.sql.ResultSet;
Expand Down Expand Up @@ -134,6 +138,13 @@ protected void validate(IdentityProvider provider) {
if (!StringUtils.hasText(provider.getIdentityZoneId())) {
throw new DataIntegrityViolationException("Identity zone ID must be set.");
}
//ensure that SAML IDPs have reduntant fields synchronized
if (Origin.SAML.equals(provider.getType()) && provider.getConfig()!=null) {
IdentityProviderDefinition saml = provider.getConfigValue(IdentityProviderDefinition.class);
saml.setIdpEntityAlias(provider.getOriginKey());
saml.setZoneId(provider.getIdentityZoneId());
provider.setConfig(JsonUtils.writeValueAsString(saml));
}
}

private static final class IdentityProviderRowMapper implements RowMapper<IdentityProvider> {
Expand Down
3 changes: 3 additions & 0 deletions common/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityProviderBootstrapTest.java
View file
Expand Up @@ -218,6 +218,7 @@ public void testSamlBootstrap() throws Exception {

IdentityProvider samlProvider = provisioning.retrieveByOrigin(definition.getIdpEntityAlias(), IdentityZoneHolder.get().getId());
assertNotNull(samlProvider);
definition.setZoneId(IdentityZoneHolder.get().getId());
assertEquals(new ObjectMapper().writeValueAsString(definition), samlProvider.getConfig());
assertNotNull(samlProvider.getCreated());
assertNotNull(samlProvider.getLastModified());
Expand Down Expand Up @@ -250,6 +251,7 @@ public void testRemovedSamlBootstrapIsInactive() throws Exception {

IdentityProvider samlProvider = provisioning.retrieveByOrigin(definition.getIdpEntityAlias(), IdentityZoneHolder.get().getId());
assertNotNull(samlProvider);
definition.setZoneId(IdentityZoneHolder.get().getId());
assertEquals(new ObjectMapper().writeValueAsString(definition), samlProvider.getConfig());
assertNotNull(samlProvider.getCreated());
assertNotNull(samlProvider.getLastModified());
Expand All @@ -258,6 +260,7 @@ public void testRemovedSamlBootstrapIsInactive() throws Exception {

IdentityProvider samlProvider2 = provisioning.retrieveByOrigin(definition2.getIdpEntityAlias(), IdentityZoneHolder.get().getId());
assertNotNull(samlProvider2);
definition2.setZoneId(IdentityZoneHolder.get().getId());
assertEquals(new ObjectMapper().writeValueAsString(definition2), samlProvider2.getConfig());
assertNotNull(samlProvider2.getCreated());
assertNotNull(samlProvider2.getLastModified());
Expand Down
6 changes: 0 additions & 6 deletions ...c/test/java/org/cloudfoundry/identity/uaa/login/saml/IdentityProviderDefinitionTests.java
View file
Expand Up @@ -40,12 +40,6 @@ public void testSetIdpEntityAlias() throws Exception {
}


@Test(expected = NullPointerException.class)
public void testSetNullIdpEntityAlias() throws Exception {
IdentityProviderDefinition def = new IdentityProviderDefinition();
def.setIdpEntityAlias(null);
}

@Test
public void testGetSocketFactoryClassName() throws Exception {
IdentityProviderDefinition def = new IdentityProviderDefinition();
Expand Down
9 changes: 7 additions & 2 deletions uaa/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityProviderEndpoints.java
View file
Expand Up @@ -14,12 +14,15 @@

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.cloudfoundry.identity.uaa.authentication.Origin;
import org.cloudfoundry.identity.uaa.authentication.manager.DynamicLdapAuthenticationManager;
import org.cloudfoundry.identity.uaa.authentication.manager.LdapLoginAuthenticationManager;
import org.cloudfoundry.identity.uaa.ldap.LdapIdentityProviderDefinition;
import org.cloudfoundry.identity.uaa.login.saml.IdentityProviderDefinition;
import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMembershipManager;
import org.cloudfoundry.identity.uaa.scim.ScimGroupProvisioning;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.cloudfoundry.identity.uaa.util.JsonUtils.JsonUtilException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.security.authentication.BadCredentialsException;
Expand Down Expand Up @@ -66,15 +69,17 @@ public IdentityProviderEndpoints(

@RequestMapping(method = POST)
public ResponseEntity<IdentityProvider> createIdentityProvider(@RequestBody IdentityProvider body) {
body.setIdentityZoneId(IdentityZoneHolder.get().getId());
String zoneId = IdentityZoneHolder.get().getId();
body.setIdentityZoneId(zoneId);
IdentityProvider createdIdp = identityProviderProvisioning.create(body);
return new ResponseEntity<>(createdIdp, HttpStatus.CREATED);
}

@RequestMapping(value = "{id}", method = PUT)
public ResponseEntity<IdentityProvider> updateIdentityProvider(@PathVariable String id, @RequestBody IdentityProvider body) {
body.setId(id);
body.setIdentityZoneId(IdentityZoneHolder.get().getId());
String zoneId = IdentityZoneHolder.get().getId();
body.setIdentityZoneId(zoneId);
IdentityProvider updatedIdp = identityProviderProvisioning.update(body);
return new ResponseEntity<>(updatedIdp, OK);
}
Expand Down
28 changes: 28 additions & 0 deletions .../java/org/cloudfoundry/identity/uaa/mock/zones/IdentityProviderEndpointsMockMvcTests.java
View file
Expand Up @@ -16,6 +16,8 @@
import org.apache.commons.lang.RandomStringUtils;
import org.cloudfoundry.identity.uaa.TestClassNullifier;
import org.cloudfoundry.identity.uaa.audit.AuditEventType;
import org.cloudfoundry.identity.uaa.authentication.Origin;
import org.cloudfoundry.identity.uaa.login.saml.IdentityProviderDefinition;
import org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils;
import org.cloudfoundry.identity.uaa.scim.ScimGroup;
import org.cloudfoundry.identity.uaa.scim.ScimGroupMember;
Expand Down Expand Up @@ -122,6 +124,32 @@ public void testCreateAndUpdateIdentityProvider() throws Exception {
createAndUpdateIdentityProvider(accessToken, null);
}

@Test
public void testCreateSamlProvider() throws Exception {
String clientId = RandomStringUtils.randomAlphabetic(6);
BaseClientDetails client = new BaseClientDetails(clientId,null,"idps.write","password",null);
client.setClientSecret("test-client-secret");
mockMvcUtils.createClient(mockMvc, adminToken, client);
ScimUser user = createAdminForZone("idps.write");
String accessToken = mockMvcUtils.getUserOAuthAccessToken(mockMvc, client.getClientId(), client.getClientSecret(), user.getUserName(), "password", "idps.write");

String origin = "my-saml-provider-"+new RandomValueStringGenerator().generate();
IdentityProvider provider = new IdentityProvider();
provider.setActive(true);
provider.setName(origin);
provider.setIdentityZoneId(IdentityZone.getUaa().getId());
provider.setType(Origin.SAML);
provider.setOriginKey(origin);
IdentityProviderDefinition samlDefinition = new IdentityProviderDefinition("http://localhost:9999/metadata", null, null, 0, false, true, "Test SAML Provider", null, null);
provider.setConfig(JsonUtils.writeValueAsString(samlDefinition));

IdentityProvider created = createIdentityProvider(null, provider, accessToken, status().isCreated());
assertNotNull(created.getConfig());
IdentityProviderDefinition samlCreated = created.getConfigValue(IdentityProviderDefinition.class);
assertEquals(IdentityZone.getUaa().getId(), samlCreated.getZoneId());
assertEquals(provider.getOriginKey(), samlCreated.getIdpEntityAlias());
}

@Test
public void testEnsureWeRetrieveInactiveIDPsToo() throws Exception {
String clientId = RandomStringUtils.randomAlphabetic(6);
Expand Down

0 comments on commit d68b39c

Please sign in to comment.