diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java b/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java index f9a0eb4ef6..5f8db850e5 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java @@ -35,7 +35,6 @@ import org.springframework.core.convert.ConversionException; import org.springframework.core.convert.ConversionService; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.ResultSetExtractor; -import org.springframework.security.acls.domain.*; import org.springframework.security.acls.domain.AccessControlEntryImpl; import org.springframework.security.acls.domain.AclAuthorizationStrategy; import org.springframework.security.acls.domain.AclImpl; @@ -43,6 +42,7 @@ import org.springframework.security.acls.domain.AuditLogger; import org.springframework.security.acls.domain.DefaultPermissionFactory; import org.springframework.security.acls.domain.DefaultPermissionGrantingStrategy; import org.springframework.security.acls.domain.GrantedAuthoritySid; +import org.springframework.security.acls.domain.ObjectIdentityRetrievalStrategyImpl; import org.springframework.security.acls.domain.PermissionFactory; import org.springframework.security.acls.domain.PrincipalSid; import org.springframework.security.acls.model.AccessControlEntry; @@ -137,7 +137,6 @@ public class BasicLookupStrategy implements LookupStrategy { private AclClassIdUtils aclClassIdUtils; - /** * Constructor accepting mandatory arguments * @param dataSource to access the database @@ -156,9 +155,8 @@ public class BasicLookupStrategy implements LookupStrategy { * @param aclAuthorizationStrategy authorization strategy (required) * @param grantingStrategy the PermissionGrantingStrategy */ - public BasicLookupStrategy(DataSource dataSource, AclCache aclCache, - AclAuthorizationStrategy aclAuthorizationStrategy, PermissionGrantingStrategy grantingStrategy) { - + public BasicLookupStrategy(DataSource dataSource, AclCache aclCache, + AclAuthorizationStrategy aclAuthorizationStrategy, PermissionGrantingStrategy grantingStrategy) { Assert.notNull(dataSource, "DataSource required"); Assert.notNull(aclCache, "AclCache required"); Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required"); @@ -494,8 +492,8 @@ public class BasicLookupStrategy implements LookupStrategy { } } - public void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) { - Assert.notNull(objectIdentityGenerator,"The provided strategy has to be not null!"); + public final void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) { + Assert.notNull(objectIdentityGenerator, "objectIdentityGenerator cannot be null"); this.objectIdentityGenerator = objectIdentityGenerator; } @@ -580,7 +578,8 @@ public class BasicLookupStrategy implements LookupStrategy { // target id type, e.g. UUID. Serializable identifier = (Serializable) rs.getObject("object_id_identity"); identifier = BasicLookupStrategy.this.aclClassIdUtils.identifierFrom(identifier, rs); - ObjectIdentity objectIdentity = objectIdentityGenerator.createObjectIdentity(identifier,rs.getString("class")); + ObjectIdentity objectIdentity = BasicLookupStrategy.this.objectIdentityGenerator + .createObjectIdentity(identifier, rs.getString("class")); Acl parentAcl = null; long parentAclId = rs.getLong("parent_object"); 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 f1cbc51609..e499577388 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 @@ -51,130 +51,131 @@ import org.springframework.util.Assert; */ public class JdbcAclService implements AclService { - protected static final Log log = LogFactory.getLog(JdbcAclService.class); + protected static final Log log = LogFactory.getLog(JdbcAclService.class); - private static final String DEFAULT_SELECT_ACL_CLASS_COLUMNS = "class.class as class"; + private static final String DEFAULT_SELECT_ACL_CLASS_COLUMNS = "class.class as class"; - private static final String DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE = DEFAULT_SELECT_ACL_CLASS_COLUMNS - + ", class.class_id_type as class_id_type"; + private static final String DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE = DEFAULT_SELECT_ACL_CLASS_COLUMNS + + ", class.class_id_type as class_id_type"; - private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL = "select obj.object_id_identity as obj_id, " - + DEFAULT_SELECT_ACL_CLASS_COLUMNS - + " from acl_object_identity obj, acl_object_identity parent, acl_class class " - + "where obj.parent_object = parent.id and obj.object_id_class = class.id " - + "and parent.object_id_identity = ? and parent.object_id_class = (" - + "select id FROM acl_class where acl_class.class = ?)"; + private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL = "select obj.object_id_identity as obj_id, " + + DEFAULT_SELECT_ACL_CLASS_COLUMNS + + " from acl_object_identity obj, acl_object_identity parent, acl_class class " + + "where obj.parent_object = parent.id and obj.object_id_class = class.id " + + "and parent.object_id_identity = ? and parent.object_id_class = (" + + "select id FROM acl_class where acl_class.class = ?)"; - private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE = "select obj.object_id_identity as obj_id, " - + DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE - + " from acl_object_identity obj, acl_object_identity parent, acl_class class " - + "where obj.parent_object = parent.id and obj.object_id_class = class.id " - + "and parent.object_id_identity = ? and parent.object_id_class = (" - + "select id FROM acl_class where acl_class.class = ?)"; + private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE = "select obj.object_id_identity as obj_id, " + + DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE + + " from acl_object_identity obj, acl_object_identity parent, acl_class class " + + "where obj.parent_object = parent.id and obj.object_id_class = class.id " + + "and parent.object_id_identity = ? and parent.object_id_class = (" + + "select id FROM acl_class where acl_class.class = ?)"; - protected final JdbcOperations jdbcOperations; + protected final JdbcOperations jdbcOperations; - private final LookupStrategy lookupStrategy; + private final LookupStrategy lookupStrategy; - private boolean aclClassIdSupported; + private boolean aclClassIdSupported; - private String findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL; + private String findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL; - private AclClassIdUtils aclClassIdUtils; - private ObjectIdentityGenerator objectIdentityGenerator; + private AclClassIdUtils aclClassIdUtils; - public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) { - this(new JdbcTemplate(dataSource), lookupStrategy); - } + private ObjectIdentityGenerator objectIdentityGenerator; - public JdbcAclService(JdbcOperations jdbcOperations, LookupStrategy lookupStrategy) { - Assert.notNull(jdbcOperations, "JdbcOperations required"); - Assert.notNull(lookupStrategy, "LookupStrategy required"); - this.jdbcOperations = jdbcOperations; - this.lookupStrategy = lookupStrategy; - this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl(); - this.aclClassIdUtils = new AclClassIdUtils(); - } + public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) { + this(new JdbcTemplate(dataSource), lookupStrategy); + } - @Override - public List findChildren(ObjectIdentity parentIdentity) { - Object[] args = {parentIdentity.getIdentifier().toString(), parentIdentity.getType()}; - List objects = this.jdbcOperations.query(this.findChildrenSql, args, - (rs, rowNum) -> mapObjectIdentityRow(rs)); - return (!objects.isEmpty()) ? objects : null; - } + public JdbcAclService(JdbcOperations jdbcOperations, LookupStrategy lookupStrategy) { + Assert.notNull(jdbcOperations, "JdbcOperations required"); + Assert.notNull(lookupStrategy, "LookupStrategy required"); + this.jdbcOperations = jdbcOperations; + this.lookupStrategy = lookupStrategy; + this.aclClassIdUtils = new AclClassIdUtils(); + this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl(); + } - private ObjectIdentity mapObjectIdentityRow(ResultSet rs) throws SQLException { - String javaType = rs.getString("class"); - Serializable identifier = (Serializable) rs.getObject("obj_id"); - identifier = this.aclClassIdUtils.identifierFrom(identifier, rs); - return objectIdentityGenerator.createObjectIdentity(identifier, javaType); - } + @Override + public List findChildren(ObjectIdentity parentIdentity) { + Object[] args = { parentIdentity.getIdentifier().toString(), parentIdentity.getType() }; + List objects = this.jdbcOperations.query(this.findChildrenSql, args, + (rs, rowNum) -> mapObjectIdentityRow(rs)); + return (!objects.isEmpty()) ? objects : null; + } - @Override - public Acl readAclById(ObjectIdentity object, List sids) throws NotFoundException { - Map map = readAclsById(Collections.singletonList(object), sids); - Assert.isTrue(map.containsKey(object), - () -> "There should have been an Acl entry for ObjectIdentity " + object); - return map.get(object); - } + private ObjectIdentity mapObjectIdentityRow(ResultSet rs) throws SQLException { + String javaType = rs.getString("class"); + Serializable identifier = (Serializable) rs.getObject("obj_id"); + identifier = this.aclClassIdUtils.identifierFrom(identifier, rs); + return this.objectIdentityGenerator.createObjectIdentity(identifier, javaType); + } - @Override - public Acl readAclById(ObjectIdentity object) throws NotFoundException { - return readAclById(object, null); - } + @Override + public Acl readAclById(ObjectIdentity object, List sids) throws NotFoundException { + Map map = readAclsById(Collections.singletonList(object), sids); + Assert.isTrue(map.containsKey(object), + () -> "There should have been an Acl entry for ObjectIdentity " + object); + return map.get(object); + } - @Override - public Map readAclsById(List objects) throws NotFoundException { - return readAclsById(objects, null); - } + @Override + public Acl readAclById(ObjectIdentity object) throws NotFoundException { + return readAclById(object, null); + } - @Override - public Map readAclsById(List objects, List sids) - throws NotFoundException { - Map result = this.lookupStrategy.readAclsById(objects, sids); - // Check every requested object identity was found (throw NotFoundException if - // needed) - for (ObjectIdentity oid : objects) { - if (!result.containsKey(oid)) { - throw new NotFoundException("Unable to find ACL information for object identity '" + oid + "'"); - } - } - return result; - } + @Override + public Map readAclsById(List objects) throws NotFoundException { + return readAclsById(objects, null); + } - /** - * Allows customization of the SQL query used to find child object identities. - * - * @param findChildrenSql - */ - public void setFindChildrenQuery(String findChildrenSql) { - this.findChildrenSql = findChildrenSql; - } + @Override + public Map readAclsById(List objects, List sids) + throws NotFoundException { + Map result = this.lookupStrategy.readAclsById(objects, sids); + // Check every requested object identity was found (throw NotFoundException if + // needed) + for (ObjectIdentity oid : objects) { + if (!result.containsKey(oid)) { + throw new NotFoundException("Unable to find ACL information for object identity '" + oid + "'"); + } + } + return result; + } - public void setAclClassIdSupported(boolean aclClassIdSupported) { - this.aclClassIdSupported = aclClassIdSupported; - if (aclClassIdSupported) { - // Change the default children select if it hasn't been overridden - if (this.findChildrenSql.equals(DEFAULT_SELECT_ACL_WITH_PARENT_SQL)) { - this.findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE; - } else { - log.debug("Find children statement has already been overridden, so not overridding the default"); - } - } - } + /** + * Allows customization of the SQL query used to find child object identities. + * @param findChildrenSql + */ + public void setFindChildrenQuery(String findChildrenSql) { + this.findChildrenSql = findChildrenSql; + } - public void setConversionService(ConversionService conversionService) { - this.aclClassIdUtils = new AclClassIdUtils(conversionService); - } + public void setAclClassIdSupported(boolean aclClassIdSupported) { + this.aclClassIdSupported = aclClassIdSupported; + if (aclClassIdSupported) { + // Change the default children select if it hasn't been overridden + if (this.findChildrenSql.equals(DEFAULT_SELECT_ACL_WITH_PARENT_SQL)) { + this.findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE; + } + else { + log.debug("Find children statement has already been overridden, so not overridding the default"); + } + } + } - public void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) { - Assert.notNull(objectIdentityGenerator,"The provided strategy has to be not null!"); - this.objectIdentityGenerator = objectIdentityGenerator; - } + public void setConversionService(ConversionService conversionService) { + this.aclClassIdUtils = new AclClassIdUtils(conversionService); + } - protected boolean isAclClassIdSupported() { - return this.aclClassIdSupported; - } + public void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) { + Assert.notNull(objectIdentityGenerator, "objectIdentityGenerator cannot be null"); + this.objectIdentityGenerator = objectIdentityGenerator; + } + + protected boolean isAclClassIdSupported() { + return this.aclClassIdSupported; + } } diff --git a/acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java b/acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java index 4a6b1d695f..e1be44924d 100644 --- a/acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java +++ b/acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java @@ -318,4 +318,13 @@ public abstract class AbstractBasicLookupStrategyTests { assertThat(((GrantedAuthoritySid) result).getGrantedAuthority()).isEqualTo("sid"); } + @Test + public void setObjectIdentityGeneratorWhenNullThenThrowsIllegalArgumentException() { + // @formatter:off + assertThatIllegalArgumentException() + .isThrownBy(() -> this.strategy.setObjectIdentityGenerator(null)) + .withMessage("objectIdentityGenerator cannot be null"); + // @formatter:on + } + } 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 903fd3264c..cd91ae1745 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 @@ -45,6 +45,7 @@ import org.springframework.security.acls.model.Sid; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyString; @@ -170,6 +171,26 @@ public class JdbcAclServiceTests { .isEqualTo(UUID.fromString("25d93b3f-c3aa-4814-9d5e-c7c96ced7762")); } + @Test + public void setObjectIdentityGeneratorWhenNullThenThrowsIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.aclServiceIntegration.setObjectIdentityGenerator(null)) + .withMessage("objectIdentityGenerator cannot be null"); + } + + @Test + public void findChildrenWhenObjectIdentityGeneratorSetThenUsed() { + this.aclServiceIntegration + .setObjectIdentityGenerator((id, type) -> new ObjectIdentityImpl(type, "prefix:" + id)); + + ObjectIdentity objectIdentity = new ObjectIdentityImpl("location", "US"); + this.aclServiceIntegration.setAclClassIdSupported(true); + List objectIdentities = this.aclServiceIntegration.findChildren(objectIdentity); + assertThat(objectIdentities.size()).isEqualTo(1); + assertThat(objectIdentities.get(0).getType()).isEqualTo("location"); + assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo("prefix:US-PAL"); + } + class MockLongIdDomainObject { private Object id;