Polish gh-10081

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,
+	public BasicLookupStrategy(DataSource dataSource, AclCache aclCache,
-							   AclAuthorizationStrategy aclAuthorizationStrategy, PermissionGrantingStrategy grantingStrategy) {
+			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) {
+	public final void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) {
-		Assert.notNull(objectIdentityGenerator,"The provided strategy has to be not null!");
+		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");

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
+	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";
+			+ ", 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, "
+	private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL = "select obj.object_id_identity as obj_id, "
-            + DEFAULT_SELECT_ACL_CLASS_COLUMNS
+			+ DEFAULT_SELECT_ACL_CLASS_COLUMNS
-            + " from acl_object_identity obj, acl_object_identity parent, acl_class class "
+			+ " 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 "
+			+ "where obj.parent_object = parent.id and obj.object_id_class = class.id "
-            + "and parent.object_id_identity = ? and parent.object_id_class = ("
+			+ "and parent.object_id_identity = ? and parent.object_id_class = ("
-            + "select id FROM acl_class where acl_class.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, "
+	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
+			+ DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE
-            + " from acl_object_identity obj, acl_object_identity parent, acl_class class "
+			+ " 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 "
+			+ "where obj.parent_object = parent.id and obj.object_id_class = class.id "
-            + "and parent.object_id_identity = ? and parent.object_id_class = ("
+			+ "and parent.object_id_identity = ? and parent.object_id_class = ("
-            + "select id FROM acl_class where acl_class.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 AclClassIdUtils aclClassIdUtils;
-    private ObjectIdentityGenerator objectIdentityGenerator;
 
-    public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) {
+	private ObjectIdentityGenerator objectIdentityGenerator;
-        this(new JdbcTemplate(dataSource), lookupStrategy);
-    }
 
-    public JdbcAclService(JdbcOperations jdbcOperations, LookupStrategy lookupStrategy) {
+	public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) {
-        Assert.notNull(jdbcOperations, "JdbcOperations required");
+		this(new JdbcTemplate(dataSource), lookupStrategy);
-        Assert.notNull(lookupStrategy, "LookupStrategy required");
+	}
-        this.jdbcOperations = jdbcOperations;
-        this.lookupStrategy = lookupStrategy;
-        this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl();
-        this.aclClassIdUtils = new AclClassIdUtils();
-    }
 
-    @Override
+	public JdbcAclService(JdbcOperations jdbcOperations, LookupStrategy lookupStrategy) {
-    public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
+		Assert.notNull(jdbcOperations, "JdbcOperations required");
-        Object[] args = {parentIdentity.getIdentifier().toString(), parentIdentity.getType()};
+		Assert.notNull(lookupStrategy, "LookupStrategy required");
-        List<ObjectIdentity> objects = this.jdbcOperations.query(this.findChildrenSql, args,
+		this.jdbcOperations = jdbcOperations;
-                (rs, rowNum) -> mapObjectIdentityRow(rs));
+		this.lookupStrategy = lookupStrategy;
-        return (!objects.isEmpty()) ? objects : null;
+		this.aclClassIdUtils = new AclClassIdUtils();
-    }
+		this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl();
+	}
 
-    private ObjectIdentity mapObjectIdentityRow(ResultSet rs) throws SQLException {
+	@Override
-        String javaType = rs.getString("class");
+	public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
-        Serializable identifier = (Serializable) rs.getObject("obj_id");
+		Object[] args = { parentIdentity.getIdentifier().toString(), parentIdentity.getType() };
-        identifier = this.aclClassIdUtils.identifierFrom(identifier, rs);
+		List<ObjectIdentity> objects = this.jdbcOperations.query(this.findChildrenSql, args,
-        return objectIdentityGenerator.createObjectIdentity(identifier, javaType);
+				(rs, rowNum) -> mapObjectIdentityRow(rs));
-    }
+		return (!objects.isEmpty()) ? objects : null;
+	}
 
-    @Override
+	private ObjectIdentity mapObjectIdentityRow(ResultSet rs) throws SQLException {
-    public Acl readAclById(ObjectIdentity object, List<Sid> sids) throws NotFoundException {
+		String javaType = rs.getString("class");
-        Map<ObjectIdentity, Acl> map = readAclsById(Collections.singletonList(object), sids);
+		Serializable identifier = (Serializable) rs.getObject("obj_id");
-        Assert.isTrue(map.containsKey(object),
+		identifier = this.aclClassIdUtils.identifierFrom(identifier, rs);
-                () -> "There should have been an Acl entry for ObjectIdentity " + object);
+		return this.objectIdentityGenerator.createObjectIdentity(identifier, javaType);
-        return map.get(object);
+	}
-    }
 
-    @Override
+	@Override
-    public Acl readAclById(ObjectIdentity object) throws NotFoundException {
+	public Acl readAclById(ObjectIdentity object, List<Sid> sids) throws NotFoundException {
-        return readAclById(object, null);
+		Map<ObjectIdentity, Acl> 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
+	@Override
-    public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects) throws NotFoundException {
+	public Acl readAclById(ObjectIdentity object) throws NotFoundException {
-        return readAclsById(objects, null);
+		return readAclById(object, null);
-    }
+	}
 
-    @Override
+	@Override
-    public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids)
+	public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects) throws NotFoundException {
-            throws NotFoundException {
+		return readAclsById(objects, null);
-        Map<ObjectIdentity, Acl> 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
-     * Allows customization of the SQL query used to find child object identities.
+	public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids)
-     *
+			throws NotFoundException {
-     * @param findChildrenSql
+		Map<ObjectIdentity, Acl> result = this.lookupStrategy.readAclsById(objects, sids);
-     */
+		// Check every requested object identity was found (throw NotFoundException if
-    public void setFindChildrenQuery(String findChildrenSql) {
+		// needed)
-        this.findChildrenSql = findChildrenSql;
+		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;
+	 * Allows customization of the SQL query used to find child object identities.
-        if (aclClassIdSupported) {
+	 * @param findChildrenSql
-            // Change the default children select if it hasn't been overridden
+	 */
-            if (this.findChildrenSql.equals(DEFAULT_SELECT_ACL_WITH_PARENT_SQL)) {
+	public void setFindChildrenQuery(String findChildrenSql) {
-                this.findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE;
+		this.findChildrenSql = findChildrenSql;
-            } else {
+	}
-                log.debug("Find children statement has already been overridden, so not overridding the default");
-            }
-        }
-    }
 
-    public void setConversionService(ConversionService conversionService) {
+	public void setAclClassIdSupported(boolean aclClassIdSupported) {
-        this.aclClassIdUtils = new AclClassIdUtils(conversionService);
+		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) {
+	public void setConversionService(ConversionService conversionService) {
-        Assert.notNull(objectIdentityGenerator,"The provided strategy has to be not null!");
+		this.aclClassIdUtils = new AclClassIdUtils(conversionService);
-        this.objectIdentityGenerator = objectIdentityGenerator;
+	}
-    }
 
-    protected boolean isAclClassIdSupported() {
+	public void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) {
-        return this.aclClassIdSupported;
+		Assert.notNull(objectIdentityGenerator, "objectIdentityGenerator cannot be null");
-    }
+		this.objectIdentityGenerator = objectIdentityGenerator;
+	}
+
+	protected boolean isAclClassIdSupported() {
+		return this.aclClassIdSupported;
+	}
 
 }

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
+	}
+
 }

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<ObjectIdentity> 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;