Polish gh-10081

This commit is contained in:
Steve Riesenberg 2021-11-23 15:45:25 -06:00
parent 86193b9540
commit 74e3abc992
4 changed files with 141 additions and 111 deletions

View File

@ -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
@ -158,7 +157,6 @@ public class BasicLookupStrategy implements LookupStrategy {
*/
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");

View File

@ -81,6 +81,7 @@ public class JdbcAclService implements AclService {
private String findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL;
private AclClassIdUtils aclClassIdUtils;
private ObjectIdentityGenerator objectIdentityGenerator;
public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) {
@ -92,13 +93,13 @@ public class JdbcAclService implements AclService {
Assert.notNull(lookupStrategy, "LookupStrategy required");
this.jdbcOperations = jdbcOperations;
this.lookupStrategy = lookupStrategy;
this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl();
this.aclClassIdUtils = new AclClassIdUtils();
this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl();
}
@Override
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,
(rs, rowNum) -> mapObjectIdentityRow(rs));
return (!objects.isEmpty()) ? objects : null;
@ -108,7 +109,7 @@ public class JdbcAclService implements AclService {
String javaType = rs.getString("class");
Serializable identifier = (Serializable) rs.getObject("obj_id");
identifier = this.aclClassIdUtils.identifierFrom(identifier, rs);
return objectIdentityGenerator.createObjectIdentity(identifier, javaType);
return this.objectIdentityGenerator.createObjectIdentity(identifier, javaType);
}
@Override
@ -145,7 +146,6 @@ public class JdbcAclService implements AclService {
/**
* Allows customization of the SQL query used to find child object identities.
*
* @param findChildrenSql
*/
public void setFindChildrenQuery(String findChildrenSql) {
@ -158,7 +158,8 @@ public class JdbcAclService implements AclService {
// 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 {
}
else {
log.debug("Find children statement has already been overridden, so not overridding the default");
}
}
@ -169,7 +170,7 @@ public class JdbcAclService implements AclService {
}
public 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;
}

View File

@ -320,6 +320,15 @@ 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
}
private static final class CacheManagerMock {
private final List<String> cacheNames;

View File

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