ACL: acl_class class vs class_id_type for BasicLookupStrategy conflict

[ChristianSch]

Summary

Hi! I'm trying to use ACL with Spring Boot.

Actual Behavior

I used the schemas as specified here (in this case H2) and I try to use the BasicLookupStrategy. I tried both names, class and class_id_type for the table acl_class, neither work out of the box because BasicLookupStrategy tries to use class_id_type AND class, but the schema above specifies class for the column name. Now here's where the strategy tries to actually use both column names:

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

Line 96 in c1db1aa

private final static String DEFAULT_SELECT_CLAUSE_ACL_CLASS_ID_TYPE_COLUMN = ", acl_class.class_id_type ";

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

Line 110 in c1db1aa

private final static String DEFAULT_LOOKUP_IDENTITIES_WHERE_CLAUSE = "(acl_object_identity.object_id_identity = ? and acl_class.class = ?)";

Is the schema missing something? Am I supposed to implement something else? I don't want to implement my own LookupStrategy just for this.

Expected Behavior

No schema issues, both calls should succeed (see sample).

Configuration

Version

5.2.0.RELEASE

Sample

Leaving the column named class causes:

org.h2.jdbc.JdbcSQLSyntaxErrorException: Column "class_id_type" not found [42122-200]
	at org.h2.message.DbException.getJdbcSQLException(DbException.java:453) ~[h2-1.4.200.jar:1.4.200]
	at org.h2.message.DbException.getJdbcSQLException(DbException.java:429) ~[h2-1.4.200.jar:1.4.200]
	at org.h2.message.DbException.get(DbException.java:205) ~[h2-1.4.200.jar:1.4.200]
	at org.h2.message.DbException.get(DbException.java:181) ~[h2-1.4.200.jar:1.4.200]
	at org.h2.jdbc.JdbcResultSet.getColumnIndex(JdbcResultSet.java:3169) ~[h2-1.4.200.jar:1.4.200]
	at org.h2.jdbc.JdbcResultSet.get(JdbcResultSet.java:3268) ~[h2-1.4.200.jar:1.4.200]
	at org.h2.jdbc.JdbcResultSet.getString(JdbcResultSet.java:316) ~[h2-1.4.200.jar:1.4.200]
	at com.zaxxer.hikari.pool.HikariProxyResultSet.getString(HikariProxyResultSet.java) ~[HikariCP-3.4.1.jar:na]
	at org.springframework.security.acls.jdbc.AclClassIdUtils.classIdTypeFrom(AclClassIdUtils.java:88) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.AclClassIdUtils.hasValidClassIdType(AclClassIdUtils.java:80) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.AclClassIdUtils.identifierFrom(AclClassIdUtils.java:65) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.BasicLookupStrategy$ProcessResultSet.convertCurrentResultIntoObject(BasicLookupStrategy.java:634) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.BasicLookupStrategy$ProcessResultSet.extractData(BasicLookupStrategy.java:583) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.BasicLookupStrategy$ProcessResultSet.extractData(BasicLookupStrategy.java:558) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.jdbc.core.JdbcTemplate$1.doInPreparedStatement(JdbcTemplate.java:679) ~[spring-jdbc-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.jdbc.core.JdbcTemplate.execute(JdbcTemplate.java:617) ~[spring-jdbc-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.jdbc.core.JdbcTemplate.query(JdbcTemplate.java:669) ~[spring-jdbc-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.jdbc.core.JdbcTemplate.query(JdbcTemplate.java:700) ~[spring-jdbc-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.BasicLookupStrategy.lookupObjectIdentities(BasicLookupStrategy.java:381) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.BasicLookupStrategy.readAclsById(BasicLookupStrategy.java:336) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.JdbcAclService.readAclsById(JdbcAclService.java:129) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.JdbcAclService.readAclById(JdbcAclService.java:111) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.JdbcAclService.readAclById(JdbcAclService.java:119) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at org.springframework.security.acls.jdbc.JdbcMutableAclService.createAcl(JdbcMutableAclService.java:122) ~[spring-security-acl-5.2.0.RELEASE.jar:5.2.0.RELEASE]
	at de.redacted.data.service.PermissionService.mutableAclFactory(PermissionService.java:33) ~[classes/:na]

schema:

create table acl_class
(
    id    bigint generated by default as identity (start with 100) not null primary key,
    class varchar_ignorecase(100)                                  not null,
    constraint unique_uk_2 unique (class)
);

Renaming column to class_id_type causes:

org.springframework.jdbc.BadSqlGrammarException: PreparedStatementCallback; bad SQL grammar [select acl_object_identity.object_id_identity, acl_entry.ace_order,  acl_object_identity.id as acl_id, acl_object_identity.parent_object, acl_object_identity.entries_inheriting, acl_entry.id as ace_id, acl_entry.mask,  acl_entry.granting,  acl_entry.audit_success, acl_entry.audit_failure,  acl_sid.principal as ace_principal, acl_sid.sid as ace_sid,  acli_sid.principal as acl_principal, acli_sid.sid as acl_sid, acl_class.class from acl_object_identity left join acl_sid acli_sid on acli_sid.id = acl_object_identity.owner_sid left join acl_class on acl_class.id = acl_object_identity.object_id_class   left join acl_entry on acl_object_identity.id = acl_entry.acl_object_identity left join acl_sid on acl_entry.sid = acl_sid.id  where ( (acl_object_identity.object_id_identity = ? and acl_class.class = ?)) order by acl_object_identity.object_id_identity asc, acl_entry.ace_order asc]; nested exception is org.h2.jdbc.JdbcSQLSyntaxErrorException: Column "ACL_CLASS.CLASS" not found; SQL statement:
select acl_object_identity.object_id_identity, acl_entry.ace_order,  acl_object_identity.id as acl_id, acl_object_identity.parent_object, acl_object_identity.entries_inheriting, acl_entry.id as ace_id, acl_entry.mask,  acl_entry.granting,  acl_entry.audit_success, acl_entry.audit_failure,  acl_sid.principal as ace_principal, acl_sid.sid as ace_sid,  acli_sid.principal as acl_principal, acli_sid.sid as acl_sid, acl_class.class from acl_object_identity left join acl_sid acli_sid on acli_sid.id = acl_object_identity.owner_sid left join acl_class on acl_class.id = acl_object_identity.object_id_class   left join acl_entry on acl_object_identity.id = acl_entry.acl_object_identity left join acl_sid on acl_entry.sid = acl_sid.id  where ( (acl_object_identity.object_id_identity = ? and acl_class.class = ?)) order by acl_object_identity.object_id_identity asc, acl_entry.ace_order asc [42122-200]

with code:

final ObjectIdentity objectIdentity = new ObjectIdentityImpl(targetObj.getClass(), targetObj.getId());
final MutableAcl acl = mutableAclFactory(objectIdentity);

    public MutableAcl mutableAclFactory(ObjectIdentity objectIdentity) {
        try {
            return (MutableAcl) aclService.readAclById(objectIdentity);
        } catch (final NotFoundException e) {
            return aclService.createAcl(objectIdentity);
        }
    }

schema:

create table acl_class
(
    id    bigint generated by default as identity (start with 100) not null primary key,
    class_id_type varchar_ignorecase(100)                                  not null,
    constraint unique_uk_2 unique (class_id_type)
);

That being said, AclClassIdUtils also uses class_id_type:

spring-security/acl/src/main/java/org/springframework/security/acls/jdbc/AclClassIdUtils.java

Line 39 in c1db1aa

private static final String DEFAULT_CLASS_ID_TYPE_COLUMN_NAME = "class_id_type";

[jzheaux]

Sorry to hear you are having trouble, @ChristianSch.

Since BasicLookupStrategy doesn't include class_id_type by default, I believe the correct fix here is to change BasicLookupStrategy to not collaborate with AclClassIdUtils by default.

I think these lines:

Serializable identifier = (Serializable) rs.getObject("object_id_identity");
identifier = aclClassIdUtils.identifierFrom(identifier, rs);

Are the ones in error since they assume that BasicLookupStrategy wants to work with AclClassIdUtils (and by association class_id_type) by default.

A better approach might be:

Serializable identifier;
if (usingDefaultSelectClause) {
    identifier = rs.getLong("object_id_identity");
} else {
    identifier = (Serializable) rs.getObject("object_id_identity");
    identifier = aclClassIdUtils.identifierFrom(identifier, rs);
}

Since the original behavior before #4424 was to treat the object_id_identity column as a long.

Would you be interested in submitting a PR to fix the problem?

A possible workaround in the meantime might be for you to have both class and class_id_type columns in your table.

[ChristianSch]

Hi,
Thanks for getting back to me. hasPermission doesn't really work for me, I don't know why yet. Neither adding both columns to the table acl_class nor your proprosal for a fix (well identifier = rs.getLong("object_id_identity");) works for me, so I can't even validate any fixes. I'm quite time constrained and need to get this done, but I am interested in a proper fix. So don't wait for me to come around, but I might if I find out why nothing works atm and get a PR done.

/edit: I found out why my implementation didn't work. First it was using the wrong beans, then I didn't implement UserDetails on my user but in a separate UserPrincipal, which was causing the Object.toString to be looked up by the lib (what horrible and bad practice imho), then I added the permission for targetObj.getClass() instead of targetObj.getClass().getSimpleName() while having hasPermission(#id, 'SimpleName', $permission). Sooo many gotchas which the documentation doesn't seem to do justice and the (few) proper reference implementations do wrong (like the baeldung one, which is over two years old tho).

I'll need to catch up on my stuff and see what I can gather from this and see if I can contribute something. But as I said, don't wait for it.

[jzheaux]

Glad you got things working, @ChristianSch.

As a side note, the reference documentation is something we're working on improving - feel free to file a ticket if you have specific recommendations. Regarding the Baeldung article, I know that they are pretty open to you opening issues on their repo so they can update their articles accordingly.

[dstr89]

I have the same issue on Postgrees (spring-boot 2.2.2). I really like the idea, but duo to the lack of proper documentation I moved toward implementing my own PermissionEvaluator for ACL.

org.postgresql.util.PSQLException: The column name class_id_type was not found in this ResultSet.
	at org.postgresql.jdbc.PgResultSet.findColumn(PgResultSet.java:2584) ~[postgresql-42.2.8.jar:42.2.8]
	at org.postgresql.jdbc.PgResultSet.getString(PgResultSet.java:2457) ~[postgresql-42.2.8.jar:42.2.8]
	at com.zaxxer.hikari.pool.HikariProxyResultSet.getString(HikariProxyResultSet.java) ~[HikariCP-3.4.1.jar:na]
	at org.springframework.security.acls.jdbc.AclClassIdUtils.classIdTypeFrom(AclClassIdUtils.java:88) ~[spring-security-acl-5.2.1.RELEASE.jar:5.2.1.RELEASE]
	at org.springframework.security.acls.jdbc.AclClassIdUtils.hasValidClassIdType(AclClassIdUtils.java:80) ~[spring-security-acl-5.2.1.RELEASE.jar:5.2.1.RELEASE]
	at org.springframework.security.acls.jdbc.AclClassIdUtils.identifierFrom(AclClassIdUtils.java:65) ~[spring-security-acl-5.2.1.RELEASE.jar:5.2.1.RELEASE]
	at org.springframework.security.acls.jdbc.BasicLookupStrategy$ProcessResultSet.convertCurrentResultIntoObject(BasicLookupStrategy.java:634) ~[spring-security-acl-5.2.1.RELEASE.jar:5.2.1.RELEASE]
	at org.springframework.security.acls.jdbc.BasicLookupStrategy$ProcessResultSet.extractData(BasicLookupStrategy.java:583) ~[spring-security-acl-5.2.1.RELEASE.jar:5.2.1.RELEASE]
	at org.springframework.security.acls.jdbc.BasicLookupStrategy$ProcessResultSet.extractData(BasicLookupStrategy.java:558) ~[spring-security-acl-5.2.1.RELEASE.jar:5.2.1.RELEASE]