diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java index fc3496c3c3..97df56deb9 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java @@ -1,5 +1,5 @@ /* - * Copyright 2004, 2005, 2006, 2017 Acegi Technology Pty Limited + * Copyright 2004, 2005, 2006, 2017, 2018 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ import javax.sql.DataSource; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.core.convert.ConversionService; +import org.springframework.jdbc.core.JdbcOperations; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.security.acls.domain.ObjectIdentityImpl; @@ -67,7 +68,7 @@ public class JdbcAclService implements AclService { // ~ Instance fields // ================================================================================================ - protected final JdbcTemplate jdbcTemplate; + protected final JdbcOperations jdbcOperations; private final LookupStrategy lookupStrategy; private boolean aclClassIdSupported; private String findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL; @@ -77,9 +78,13 @@ public class JdbcAclService implements AclService { // =================================================================================================== public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) { - Assert.notNull(dataSource, "DataSource required"); + this(new JdbcTemplate(dataSource), lookupStrategy); + } + + public JdbcAclService(JdbcOperations jdbcOperations, LookupStrategy lookupStrategy) { + Assert.notNull(jdbcOperations, "JdbcOperations required"); Assert.notNull(lookupStrategy, "LookupStrategy required"); - this.jdbcTemplate = new JdbcTemplate(dataSource); + this.jdbcOperations = jdbcOperations; this.lookupStrategy = lookupStrategy; this.aclClassIdUtils = new AclClassIdUtils(); } @@ -88,15 +93,14 @@ public class JdbcAclService implements AclService { // ======================================================================================================== public List findChildren(ObjectIdentity parentIdentity) { - Object[] args = { parentIdentity.getIdentifier(), parentIdentity.getType() }; - List objects = jdbcTemplate.query(findChildrenSql, args, + Object[] args = { parentIdentity.getIdentifier().toString(), parentIdentity.getType() }; + List objects = jdbcOperations.query(findChildrenSql, args, new RowMapper() { public ObjectIdentity mapRow(ResultSet rs, int rowNum) throws SQLException { String javaType = rs.getString("class"); Serializable identifier = (Serializable) rs.getObject("obj_id"); identifier = aclClassIdUtils.identifierFrom(identifier, rs); - return new ObjectIdentityImpl(javaType, identifier); } }); diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java index 6100fe1074..7a1698fe8a 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java @@ -1,5 +1,5 @@ /* - * Copyright 2004, 2005, 2006, 2017 Acegi Technology Pty Limited + * Copyright 2004, 2005, 2006, 2017, 2018 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -135,7 +135,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS if (acl.getEntries().isEmpty()) { return; } - jdbcTemplate.batchUpdate(insertEntry, new BatchPreparedStatementSetter() { + jdbcOperations.batchUpdate(insertEntry, new BatchPreparedStatementSetter() { public int getBatchSize() { return acl.getEntries().size(); } @@ -170,7 +170,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS protected void createObjectIdentity(ObjectIdentity object, Sid owner) { Long sidId = createOrRetrieveSidPrimaryKey(owner, true); Long classId = createOrRetrieveClassPrimaryKey(object.getType(), true, object.getIdentifier().getClass()); - jdbcTemplate.update(insertObjectIdentity, classId, object.getIdentifier(), sidId, + jdbcOperations.update(insertObjectIdentity, classId, object.getIdentifier().toString(), sidId, Boolean.TRUE); } @@ -184,7 +184,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS * @return the primary key or null if not found */ protected Long createOrRetrieveClassPrimaryKey(String type, boolean allowCreate, Class idType) { - List classIds = jdbcTemplate.queryForList(selectClassPrimaryKey, + List classIds = jdbcOperations.queryForList(selectClassPrimaryKey, new Object[] { type }, Long.class); if (!classIds.isEmpty()) { @@ -193,13 +193,13 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS if (allowCreate) { if (!isAclClassIdSupported()) { - jdbcTemplate.update(insertClass, type); + jdbcOperations.update(insertClass, type); } else { - jdbcTemplate.update(insertClass, type, idType.getCanonicalName()); + jdbcOperations.update(insertClass, type, idType.getCanonicalName()); } Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(), "Transaction must be running"); - return jdbcTemplate.queryForObject(classIdentityQuery, Long.class); + return jdbcOperations.queryForObject(classIdentityQuery, Long.class); } return null; @@ -248,7 +248,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS protected Long createOrRetrieveSidPrimaryKey(String sidName, boolean sidIsPrincipal, boolean allowCreate) { - List sidIds = jdbcTemplate.queryForList(selectSidPrimaryKey, new Object[] { + List sidIds = jdbcOperations.queryForList(selectSidPrimaryKey, new Object[] { Boolean.valueOf(sidIsPrincipal), sidName }, Long.class); if (!sidIds.isEmpty()) { @@ -256,10 +256,10 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS } if (allowCreate) { - jdbcTemplate.update(insertSid, Boolean.valueOf(sidIsPrincipal), sidName); + jdbcOperations.update(insertSid, Boolean.valueOf(sidIsPrincipal), sidName); Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(), "Transaction must be running"); - return jdbcTemplate.queryForObject(sidIdentityQuery, Long.class); + return jdbcOperations.queryForObject(sidIdentityQuery, Long.class); } return null; @@ -311,7 +311,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS * @param oidPrimaryKey the rows in acl_entry to delete */ protected void deleteEntries(Long oidPrimaryKey) { - jdbcTemplate.update(deleteEntryByObjectIdentityForeignKey, oidPrimaryKey); + jdbcOperations.update(deleteEntryByObjectIdentityForeignKey, oidPrimaryKey); } /** @@ -325,7 +325,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS */ protected void deleteObjectIdentity(Long oidPrimaryKey) { // Delete the acl_object_identity row - jdbcTemplate.update(deleteObjectIdentityByPrimaryKey, oidPrimaryKey); + jdbcOperations.update(deleteObjectIdentityByPrimaryKey, oidPrimaryKey); } /** @@ -339,8 +339,8 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS */ protected Long retrieveObjectIdentityPrimaryKey(ObjectIdentity oid) { try { - return jdbcTemplate.queryForObject(selectObjectIdentityPrimaryKey, Long.class, - oid.getType(), oid.getIdentifier()); + return jdbcOperations.queryForObject(selectObjectIdentityPrimaryKey, Long.class, + oid.getType(), oid.getIdentifier().toString()); } catch (DataAccessException notFound) { return null; @@ -409,7 +409,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS Assert.notNull(acl.getOwner(), "Owner is required in this implementation"); Long ownerSid = createOrRetrieveSidPrimaryKey(acl.getOwner(), true); - int count = jdbcTemplate.update(updateObjectIdentity, parentId, ownerSid, + int count = jdbcOperations.update(updateObjectIdentity, parentId, ownerSid, Boolean.valueOf(acl.isEntriesInheriting()), acl.getId()); if (count != 1) { diff --git a/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java b/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java index 5016ab290e..a27bf918d5 100644 --- a/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java +++ b/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,21 +15,16 @@ */ package org.springframework.security.acls.jdbc; -import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.Mockito.when; - -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import javax.sql.DataSource; - +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.jdbc.core.JdbcOperations; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; import org.springframework.security.acls.domain.ObjectIdentityImpl; import org.springframework.security.acls.domain.PrincipalSid; import org.springframework.security.acls.model.Acl; @@ -37,19 +32,54 @@ import org.springframework.security.acls.model.NotFoundException; import org.springframework.security.acls.model.ObjectIdentity; import org.springframework.security.acls.model.Sid; +import javax.sql.DataSource; +import java.util.*; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.AdditionalMatchers.aryEq; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.when; + +/** + * Unit and Integration tests the ACL JdbcAclService using an + * in-memory database. + * + * @author Nena Raab + */ @RunWith(MockitoJUnitRunner.class) public class JdbcAclServiceTests { + + private EmbeddedDatabase embeddedDatabase; + @Mock private DataSource dataSource; @Mock private LookupStrategy lookupStrategy; + @Mock + JdbcOperations jdbcOperations; + + private JdbcAclService aclServiceIntegration; private JdbcAclService aclService; @Before public void setUp() { - aclService = new JdbcAclService(dataSource, lookupStrategy); + aclService = new JdbcAclService(jdbcOperations, lookupStrategy); + aclServiceIntegration = new JdbcAclService(embeddedDatabase, lookupStrategy); + } + + @Before + public void setUpEmbeddedDatabase() { + embeddedDatabase = new EmbeddedDatabaseBuilder()// + .addScript("createAclSchemaWithAclClassIdType.sql") + .addScript("db/sql/test_data_hierarchy.sql") + .build(); + } + + @After + public void tearDownEmbeddedDatabase() { + embeddedDatabase.shutdown(); } // SEC-1898 @@ -60,8 +90,110 @@ public class JdbcAclServiceTests { lookupStrategy.readAclsById(anyList(), anyList())).thenReturn(result); ObjectIdentity objectIdentity = new ObjectIdentityImpl(Object.class, 1); - List sids = Arrays. asList(new PrincipalSid("user")); + List sids = Arrays.asList(new PrincipalSid("user")); aclService.readAclById(objectIdentity, sids); } + + @Test + public void findOneChildren() { + List result = new ArrayList<>(); + result.add(new ObjectIdentityImpl(Object.class, "5577")); + Object[] args = {"1", "org.springframework.security.acls.jdbc.JdbcAclServiceTests$MockLongIdDomainObject"}; + when( + jdbcOperations.query(anyString(), + aryEq(args), any(RowMapper.class))).thenReturn(result); + ObjectIdentity objectIdentity = new ObjectIdentityImpl(MockLongIdDomainObject.class, 1L); + + List objectIdentities = aclService.findChildren(objectIdentity); + assertThat(objectIdentities.size()).isEqualTo(1); + assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo("5577"); + } + + @Test + public void findNoChildren() { + ObjectIdentity objectIdentity = new ObjectIdentityImpl(MockLongIdDomainObject.class, 1L); + + List objectIdentities = aclService.findChildren(objectIdentity); + assertThat(objectIdentities).isNull(); + } + + // ~ Some integration tests + // ======================================================================================================== + + @Test + public void findChildrenWithoutIdType() { + ObjectIdentity objectIdentity = new ObjectIdentityImpl(MockLongIdDomainObject.class, 4711L); + + List objectIdentities = aclServiceIntegration.findChildren(objectIdentity); + assertThat(objectIdentities.size()).isEqualTo(1); + assertThat(objectIdentities.get(0).getType()).isEqualTo(MockUntypedIdDomainObject.class.getName()); + assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo(5000L); + } + + @Test + public void findChildrenForUnknownObject() { + ObjectIdentity objectIdentity = new ObjectIdentityImpl(Object.class, 33); + + List objectIdentities = aclServiceIntegration.findChildren(objectIdentity); + assertThat(objectIdentities).isNull(); + } + + @Test + public void findChildrenOfIdTypeLong() { + ObjectIdentity objectIdentity = new ObjectIdentityImpl("location", "US-PAL"); + + List objectIdentities = aclServiceIntegration.findChildren(objectIdentity); + assertThat(objectIdentities.size()).isEqualTo(2); + assertThat(objectIdentities.get(0).getType()).isEqualTo(MockLongIdDomainObject.class.getName()); + assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo(4711L); + assertThat(objectIdentities.get(1).getType()).isEqualTo(MockLongIdDomainObject.class.getName()); + assertThat(objectIdentities.get(1).getIdentifier()).isEqualTo(4712L); + } + + @Test + public void findChildrenOfIdTypeString() { + ObjectIdentity objectIdentity = new ObjectIdentityImpl("location", "US"); + + aclServiceIntegration.setAclClassIdSupported(true); + List objectIdentities = aclServiceIntegration.findChildren(objectIdentity); + assertThat(objectIdentities.size()).isEqualTo(1); + assertThat(objectIdentities.get(0).getType()).isEqualTo("location"); + assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo("US-PAL"); + } + + @Test + public void findChildrenOfIdTypeUUID() { + ObjectIdentity objectIdentity = new ObjectIdentityImpl(MockUntypedIdDomainObject.class, 5000L); + + aclServiceIntegration.setAclClassIdSupported(true); + List objectIdentities = aclServiceIntegration.findChildren(objectIdentity); + assertThat(objectIdentities.size()).isEqualTo(1); + assertThat(objectIdentities.get(0).getType()).isEqualTo("costcenter"); + assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo(UUID.fromString("25d93b3f-c3aa-4814-9d5e-c7c96ced7762")); + } + + private class MockLongIdDomainObject { + private Object id; + + public Object getId() { + return id; + } + + public void setId(Object id) { + this.id = id; + } + } + + private class MockUntypedIdDomainObject { + private Object id; + + public Object getId() { + return id; + } + + public void setId(Object id) { + this.id = id; + } + } } diff --git a/acl/src/test/resources/db/sql/test_data_hierarchy.sql b/acl/src/test/resources/db/sql/test_data_hierarchy.sql new file mode 100644 index 0000000000..a0c41793ae --- /dev/null +++ b/acl/src/test/resources/db/sql/test_data_hierarchy.sql @@ -0,0 +1,17 @@ +--- insert ACL data +INSERT INTO ACL_SID (ID, PRINCIPAL, SID) VALUES + (10, true, 'user'); + +INSERT INTO acl_class (id, class, class_id_type) VALUES + (20,'location','java.lang.String'), + (21,'org.springframework.security.acls.jdbc.JdbcAclServiceTests$MockLongIdDomainObject','java.lang.Long'), + (22,'org.springframework.security.acls.jdbc.JdbcAclServiceTests$MockUntypedIdDomainObject',''), + (23,'costcenter','java.util.UUID'); + +INSERT INTO acl_object_identity (id, object_id_class, object_id_identity, parent_object, owner_sid, entries_inheriting) VALUES + (1,20,'US',NULL,10,false), + (2,20,'US-PAL',1,10,true), + (3,21,'4711',2,10,true), + (4,21,'4712',2,10,true), + (5,22,'5000',3,10,true), + (6,23,'25d93b3f-c3aa-4814-9d5e-c7c96ced7762',5,10,true);