Skip to content

Commit 74e3abc

Browse files
author
Steve Riesenberg
committed
Polish gh-10081 
1 parent 86193b9 commit 74e3abc

File tree

4 files changed

+163
-133
lines changed

4 files changed

+163
-133
lines changed

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

+7-8
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,14 @@
3535
import org.springframework.core.convert.ConversionService;
3636
import org.springframework.jdbc.core.JdbcTemplate;
3737
import org.springframework.jdbc.core.ResultSetExtractor;
38-
import org.springframework.security.acls.domain.*;
3938
import org.springframework.security.acls.domain.AccessControlEntryImpl;
4039
import org.springframework.security.acls.domain.AclAuthorizationStrategy;
4140
import org.springframework.security.acls.domain.AclImpl;
4241
import org.springframework.security.acls.domain.AuditLogger;
4342
import org.springframework.security.acls.domain.DefaultPermissionFactory;
4443
import org.springframework.security.acls.domain.DefaultPermissionGrantingStrategy;
4544
import org.springframework.security.acls.domain.GrantedAuthoritySid;
45+
import org.springframework.security.acls.domain.ObjectIdentityRetrievalStrategyImpl;
4646
import org.springframework.security.acls.domain.PermissionFactory;
4747
import org.springframework.security.acls.domain.PrincipalSid;
4848
import org.springframework.security.acls.model.AccessControlEntry;
@@ -137,7 +137,6 @@ public class BasicLookupStrategy implements LookupStrategy {
137137

138138
private AclClassIdUtils aclClassIdUtils;
139139

140-
141140
/**
142141
* Constructor accepting mandatory arguments
143142
* @param dataSource to access the database
@@ -156,9 +155,8 @@ public BasicLookupStrategy(DataSource dataSource, AclCache aclCache,
156155
* @param aclAuthorizationStrategy authorization strategy (required)
157156
* @param grantingStrategy the PermissionGrantingStrategy
158157
*/
159-
public BasicLookupStrategy(DataSource dataSource, AclCache aclCache,
160-
AclAuthorizationStrategy aclAuthorizationStrategy, PermissionGrantingStrategy grantingStrategy) {
161-
158+
public BasicLookupStrategy(DataSource dataSource, AclCache aclCache,
159+
AclAuthorizationStrategy aclAuthorizationStrategy, PermissionGrantingStrategy grantingStrategy) {
162160
Assert.notNull(dataSource, "DataSource required");
163161
Assert.notNull(aclCache, "AclCache required");
164162
Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required");
@@ -494,8 +492,8 @@ public final void setAclClassIdSupported(boolean aclClassIdSupported) {
494492
}
495493
}
496494

497-
public void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) {
498-
Assert.notNull(objectIdentityGenerator,"The provided strategy has to be not null!");
495+
public final void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) {
496+
Assert.notNull(objectIdentityGenerator, "objectIdentityGenerator cannot be null");
499497
this.objectIdentityGenerator = objectIdentityGenerator;
500498
}
501499

@@ -580,7 +578,8 @@ private void convertCurrentResultIntoObject(Map<Serializable, Acl> acls, ResultS
580578
// target id type, e.g. UUID.
581579
Serializable identifier = (Serializable) rs.getObject("object_id_identity");
582580
identifier = BasicLookupStrategy.this.aclClassIdUtils.identifierFrom(identifier, rs);
583-
ObjectIdentity objectIdentity = objectIdentityGenerator.createObjectIdentity(identifier,rs.getString("class"));
581+
ObjectIdentity objectIdentity = BasicLookupStrategy.this.objectIdentityGenerator
582+
.createObjectIdentity(identifier, rs.getString("class"));
584583

585584
Acl parentAcl = null;
586585
long parentAclId = rs.getLong("parent_object");

‎acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java

+126-125
Original file line numberDiff line numberDiff line change
@@ -51,130 +51,131 @@
5151
*/
5252
public class JdbcAclService implements AclService {
5353

54-
protected static final Log log = LogFactory.getLog(JdbcAclService.class);
55-
56-
private static final String DEFAULT_SELECT_ACL_CLASS_COLUMNS = "class.class as class";
57-
58-
private static final String DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE = DEFAULT_SELECT_ACL_CLASS_COLUMNS
59-
+ ", class.class_id_type as class_id_type";
60-
61-
private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL = "select obj.object_id_identity as obj_id, "
62-
+ DEFAULT_SELECT_ACL_CLASS_COLUMNS
63-
+ " from acl_object_identity obj, acl_object_identity parent, acl_class class "
64-
+ "where obj.parent_object = parent.id and obj.object_id_class = class.id "
65-
+ "and parent.object_id_identity = ? and parent.object_id_class = ("
66-
+ "select id FROM acl_class where acl_class.class = ?)";
67-
68-
private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE = "select obj.object_id_identity as obj_id, "
69-
+ DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE
70-
+ " from acl_object_identity obj, acl_object_identity parent, acl_class class "
71-
+ "where obj.parent_object = parent.id and obj.object_id_class = class.id "
72-
+ "and parent.object_id_identity = ? and parent.object_id_class = ("
73-
+ "select id FROM acl_class where acl_class.class = ?)";
74-
75-
protected final JdbcOperations jdbcOperations;
76-
77-
private final LookupStrategy lookupStrategy;
78-
79-
private boolean aclClassIdSupported;
80-
81-
private String findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL;
82-
83-
private AclClassIdUtils aclClassIdUtils;
84-
private ObjectIdentityGenerator objectIdentityGenerator;
85-
86-
public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) {
87-
this(new JdbcTemplate(dataSource), lookupStrategy);
88-
}
89-
90-
public JdbcAclService(JdbcOperations jdbcOperations, LookupStrategy lookupStrategy) {
91-
Assert.notNull(jdbcOperations, "JdbcOperations required");
92-
Assert.notNull(lookupStrategy, "LookupStrategy required");
93-
this.jdbcOperations = jdbcOperations;
94-
this.lookupStrategy = lookupStrategy;
95-
this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl();
96-
this.aclClassIdUtils = new AclClassIdUtils();
97-
}
98-
99-
@Override
100-
public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
101-
Object[] args = {parentIdentity.getIdentifier().toString(), parentIdentity.getType()};
102-
List<ObjectIdentity> objects = this.jdbcOperations.query(this.findChildrenSql, args,
103-
(rs, rowNum) -> mapObjectIdentityRow(rs));
104-
return (!objects.isEmpty()) ? objects : null;
105-
}
106-
107-
private ObjectIdentity mapObjectIdentityRow(ResultSet rs) throws SQLException {
108-
String javaType = rs.getString("class");
109-
Serializable identifier = (Serializable) rs.getObject("obj_id");
110-
identifier = this.aclClassIdUtils.identifierFrom(identifier, rs);
111-
return objectIdentityGenerator.createObjectIdentity(identifier, javaType);
112-
}
113-
114-
@Override
115-
public Acl readAclById(ObjectIdentity object, List<Sid> sids) throws NotFoundException {
116-
Map<ObjectIdentity, Acl> map = readAclsById(Collections.singletonList(object), sids);
117-
Assert.isTrue(map.containsKey(object),
118-
() -> "There should have been an Acl entry for ObjectIdentity " + object);
119-
return map.get(object);
120-
}
121-
122-
@Override
123-
public Acl readAclById(ObjectIdentity object) throws NotFoundException {
124-
return readAclById(object, null);
125-
}
126-
127-
@Override
128-
public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects) throws NotFoundException {
129-
return readAclsById(objects, null);
130-
}
131-
132-
@Override
133-
public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids)
134-
throws NotFoundException {
135-
Map<ObjectIdentity, Acl> result = this.lookupStrategy.readAclsById(objects, sids);
136-
// Check every requested object identity was found (throw NotFoundException if
137-
// needed)
138-
for (ObjectIdentity oid : objects) {
139-
if (!result.containsKey(oid)) {
140-
throw new NotFoundException("Unable to find ACL information for object identity '" + oid + "'");
141-
}
142-
}
143-
return result;
144-
}
145-
146-
/**
147-
* Allows customization of the SQL query used to find child object identities.
148-
*
149-
* @param findChildrenSql
150-
*/
151-
public void setFindChildrenQuery(String findChildrenSql) {
152-
this.findChildrenSql = findChildrenSql;
153-
}
154-
155-
public void setAclClassIdSupported(boolean aclClassIdSupported) {
156-
this.aclClassIdSupported = aclClassIdSupported;
157-
if (aclClassIdSupported) {
158-
// Change the default children select if it hasn't been overridden
159-
if (this.findChildrenSql.equals(DEFAULT_SELECT_ACL_WITH_PARENT_SQL)) {
160-
this.findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE;
161-
} else {
162-
log.debug("Find children statement has already been overridden, so not overridding the default");
163-
}
164-
}
165-
}
166-
167-
public void setConversionService(ConversionService conversionService) {
168-
this.aclClassIdUtils = new AclClassIdUtils(conversionService);
169-
}
170-
171-
public void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) {
172-
Assert.notNull(objectIdentityGenerator,"The provided strategy has to be not null!");
173-
this.objectIdentityGenerator = objectIdentityGenerator;
174-
}
175-
176-
protected boolean isAclClassIdSupported() {
177-
return this.aclClassIdSupported;
178-
}
54+
protected static final Log log = LogFactory.getLog(JdbcAclService.class);
55+
56+
private static final String DEFAULT_SELECT_ACL_CLASS_COLUMNS = "class.class as class";
57+
58+
private static final String DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE = DEFAULT_SELECT_ACL_CLASS_COLUMNS
59+
+ ", class.class_id_type as class_id_type";
60+
61+
private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL = "select obj.object_id_identity as obj_id, "
62+
+ DEFAULT_SELECT_ACL_CLASS_COLUMNS
63+
+ " from acl_object_identity obj, acl_object_identity parent, acl_class class "
64+
+ "where obj.parent_object = parent.id and obj.object_id_class = class.id "
65+
+ "and parent.object_id_identity = ? and parent.object_id_class = ("
66+
+ "select id FROM acl_class where acl_class.class = ?)";
67+
68+
private static final String DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE = "select obj.object_id_identity as obj_id, "
69+
+ DEFAULT_SELECT_ACL_CLASS_COLUMNS_WITH_ID_TYPE
70+
+ " from acl_object_identity obj, acl_object_identity parent, acl_class class "
71+
+ "where obj.parent_object = parent.id and obj.object_id_class = class.id "
72+
+ "and parent.object_id_identity = ? and parent.object_id_class = ("
73+
+ "select id FROM acl_class where acl_class.class = ?)";
74+
75+
protected final JdbcOperations jdbcOperations;
76+
77+
private final LookupStrategy lookupStrategy;
78+
79+
private boolean aclClassIdSupported;
80+
81+
private String findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL;
82+
83+
private AclClassIdUtils aclClassIdUtils;
84+
85+
private ObjectIdentityGenerator objectIdentityGenerator;
86+
87+
public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) {
88+
this(new JdbcTemplate(dataSource), lookupStrategy);
89+
}
90+
91+
public JdbcAclService(JdbcOperations jdbcOperations, LookupStrategy lookupStrategy) {
92+
Assert.notNull(jdbcOperations, "JdbcOperations required");
93+
Assert.notNull(lookupStrategy, "LookupStrategy required");
94+
this.jdbcOperations = jdbcOperations;
95+
this.lookupStrategy = lookupStrategy;
96+
this.aclClassIdUtils = new AclClassIdUtils();
97+
this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl();
98+
}
99+
100+
@Override
101+
public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
102+
Object[] args = { parentIdentity.getIdentifier().toString(), parentIdentity.getType() };
103+
List<ObjectIdentity> objects = this.jdbcOperations.query(this.findChildrenSql, args,
104+
(rs, rowNum) -> mapObjectIdentityRow(rs));
105+
return (!objects.isEmpty()) ? objects : null;
106+
}
107+
108+
private ObjectIdentity mapObjectIdentityRow(ResultSet rs) throws SQLException {
109+
String javaType = rs.getString("class");
110+
Serializable identifier = (Serializable) rs.getObject("obj_id");
111+
identifier = this.aclClassIdUtils.identifierFrom(identifier, rs);
112+
return this.objectIdentityGenerator.createObjectIdentity(identifier, javaType);
113+
}
114+
115+
@Override
116+
public Acl readAclById(ObjectIdentity object, List<Sid> sids) throws NotFoundException {
117+
Map<ObjectIdentity, Acl> map = readAclsById(Collections.singletonList(object), sids);
118+
Assert.isTrue(map.containsKey(object),
119+
() -> "There should have been an Acl entry for ObjectIdentity " + object);
120+
return map.get(object);
121+
}
122+
123+
@Override
124+
public Acl readAclById(ObjectIdentity object) throws NotFoundException {
125+
return readAclById(object, null);
126+
}
127+
128+
@Override
129+
public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects) throws NotFoundException {
130+
return readAclsById(objects, null);
131+
}
132+
133+
@Override
134+
public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids)
135+
throws NotFoundException {
136+
Map<ObjectIdentity, Acl> result = this.lookupStrategy.readAclsById(objects, sids);
137+
// Check every requested object identity was found (throw NotFoundException if
138+
// needed)
139+
for (ObjectIdentity oid : objects) {
140+
if (!result.containsKey(oid)) {
141+
throw new NotFoundException("Unable to find ACL information for object identity '" + oid + "'");
142+
}
143+
}
144+
return result;
145+
}
146+
147+
/**
148+
* Allows customization of the SQL query used to find child object identities.
149+
* @param findChildrenSql
150+
*/
151+
public void setFindChildrenQuery(String findChildrenSql) {
152+
this.findChildrenSql = findChildrenSql;
153+
}
154+
155+
public void setAclClassIdSupported(boolean aclClassIdSupported) {
156+
this.aclClassIdSupported = aclClassIdSupported;
157+
if (aclClassIdSupported) {
158+
// Change the default children select if it hasn't been overridden
159+
if (this.findChildrenSql.equals(DEFAULT_SELECT_ACL_WITH_PARENT_SQL)) {
160+
this.findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL_WITH_CLASS_ID_TYPE;
161+
}
162+
else {
163+
log.debug("Find children statement has already been overridden, so not overridding the default");
164+
}
165+
}
166+
}
167+
168+
public void setConversionService(ConversionService conversionService) {
169+
this.aclClassIdUtils = new AclClassIdUtils(conversionService);
170+
}
171+
172+
public void setObjectIdentityGenerator(ObjectIdentityGenerator objectIdentityGenerator) {
173+
Assert.notNull(objectIdentityGenerator, "objectIdentityGenerator cannot be null");
174+
this.objectIdentityGenerator = objectIdentityGenerator;
175+
}
176+
177+
protected boolean isAclClassIdSupported() {
178+
return this.aclClassIdSupported;
179+
}
179180

180181
}

‎acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java

+9
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,15 @@ public void testCreateGrantedAuthority() {
320320
assertThat(((GrantedAuthoritySid) result).getGrantedAuthority()).isEqualTo("sid");
321321
}
322322

323+
@Test
324+
public void setObjectIdentityGeneratorWhenNullThenThrowsIllegalArgumentException() {
325+
// @formatter:off
326+
assertThatIllegalArgumentException()
327+
.isThrownBy(() -> this.strategy.setObjectIdentityGenerator(null))
328+
.withMessage("objectIdentityGenerator cannot be null");
329+
// @formatter:on
330+
}
331+
323332
private static final class CacheManagerMock {
324333

325334
private final List<String> cacheNames;

‎acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java

+21
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545

4646
import static org.assertj.core.api.Assertions.assertThat;
4747
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
48+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
4849
import static org.mockito.ArgumentMatchers.any;
4950
import static org.mockito.ArgumentMatchers.anyList;
5051
import static org.mockito.ArgumentMatchers.anyString;
@@ -170,6 +171,26 @@ public void findChildrenOfIdTypeUUID() {
170171
.isEqualTo(UUID.fromString("25d93b3f-c3aa-4814-9d5e-c7c96ced7762"));
171172
}
172173

174+
@Test
175+
public void setObjectIdentityGeneratorWhenNullThenThrowsIllegalArgumentException() {
176+
assertThatIllegalArgumentException()
177+
.isThrownBy(() -> this.aclServiceIntegration.setObjectIdentityGenerator(null))
178+
.withMessage("objectIdentityGenerator cannot be null");
179+
}
180+
181+
@Test
182+
public void findChildrenWhenObjectIdentityGeneratorSetThenUsed() {
183+
this.aclServiceIntegration
184+
.setObjectIdentityGenerator((id, type) -> new ObjectIdentityImpl(type, "prefix:" + id));
185+
186+
ObjectIdentity objectIdentity = new ObjectIdentityImpl("location", "US");
187+
this.aclServiceIntegration.setAclClassIdSupported(true);
188+
List<ObjectIdentity> objectIdentities = this.aclServiceIntegration.findChildren(objectIdentity);
189+
assertThat(objectIdentities.size()).isEqualTo(1);
190+
assertThat(objectIdentities.get(0).getType()).isEqualTo("location");
191+
assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo("prefix:US-PAL");
192+
}
193+
173194
class MockLongIdDomainObject {
174195

175196
private Object id;

0 commit comments

Comments
 (0)