Add ObjectIdentityGenerator customization to JdbcAclService

Providing the possibility to change, how ObjectIdentitys are created inside the BasicLookupStrategy,JdbcAclService

There was a problem with hard coded object identity creation inside the BasicLookupStrategy and the JdbcAclService. It was overkill to overwrite
these classes only for changing this, so introducing an ObjectIdentityGenerator seems the be the better solution here. At default, the standard
ObjectIdentityRetrievalStrategyImpl is used, but can be customized due to setters.

Closes gh-10079

acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java

@@ -35,6 +35,7 @@ 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;
@@ -42,7 +43,6 @@ 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.ObjectIdentityImpl;
 import org.springframework.security.acls.domain.PermissionFactory;
 import org.springframework.security.acls.domain.PrincipalSid;
 import org.springframework.security.acls.model.AccessControlEntry;
@@ -51,6 +51,7 @@ import org.springframework.security.acls.model.AclCache;
 import org.springframework.security.acls.model.MutableAcl;
 import org.springframework.security.acls.model.NotFoundException;
 import org.springframework.security.acls.model.ObjectIdentity;
+import org.springframework.security.acls.model.ObjectIdentityGenerator;
 import org.springframework.security.acls.model.Permission;
 import org.springframework.security.acls.model.PermissionGrantingStrategy;
 import org.springframework.security.acls.model.Sid;
@@ -109,6 +110,8 @@ public class BasicLookupStrategy implements LookupStrategy {
 
 	private final AclAuthorizationStrategy aclAuthorizationStrategy;
 
+	private ObjectIdentityGenerator objectIdentityGenerator;
+
 	private PermissionFactory permissionFactory = new DefaultPermissionFactory();
 
 	private final AclCache aclCache;
@@ -134,6 +137,7 @@ public class BasicLookupStrategy implements LookupStrategy {
 
 	private AclClassIdUtils aclClassIdUtils;
 
+
 	/**
 	 * Constructor accepting mandatory arguments
 	 * @param dataSource to access the database
@@ -152,8 +156,9 @@ 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");
@@ -162,6 +167,7 @@ public class BasicLookupStrategy implements LookupStrategy {
 		this.aclCache = aclCache;
 		this.aclAuthorizationStrategy = aclAuthorizationStrategy;
 		this.grantingStrategy = grantingStrategy;
+		this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl();
 		this.aclClassIdUtils = new AclClassIdUtils();
 		this.fieldAces.setAccessible(true);
 		this.fieldAcl.setAccessible(true);
@@ -488,6 +494,11 @@ public class BasicLookupStrategy implements LookupStrategy {
 		}
 	}
 
+	public void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) {
+		Assert.notNull(objectIdentityGenerator,"The provided strategy has to be not null!");
+		this.objectIdentityGenerator = objectIdentityGenerator;
+	}
+
 	public final void setConversionService(ConversionService conversionService) {
 		this.aclClassIdUtils = new AclClassIdUtils(conversionService);
 	}
@@ -569,7 +580,7 @@ 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 = new ObjectIdentityImpl(rs.getString("class"), identifier);
+				ObjectIdentity objectIdentity = 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

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