Skip to content

Commit d1a754f

Browse files
nenaraabrwinch
authored andcommitted
JdbcAclService: fixes PostgreSql issue
When setup the acl tables as specified in the Spring.io documentation I have faced the following error on a PostgreSql database: org.postgresql.util.PSQLException: ERROR: operator does not exist: bigint = character varying. This is because the acl_object_identity.object_id_identity column is of type varchar(36) but it is not necessarily accessed with a value of type String. - JdbcAclService / JdbcMutableAclService: SQL query must match object_id_identity column specification - JdbcAclService: changed JdbcTemplate to JdbcOperations for testability - JdbcAclServiceTest: Increased test coverage, the integration tests using embedded db relates to this commit thomasdarimont@cd8d207 Fixes gh-5508
1 parent 1bfa38b commit d1a754f

File tree

4 files changed

+188
-35
lines changed

4 files changed

+188
-35
lines changed

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2004, 2005, 2006, 2017 Acegi Technology Pty Limited
2+
* Copyright 2004, 2005, 2006, 2017, 2018 Acegi Technology Pty Limited
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -27,6 +27,7 @@
2727
import org.apache.commons.logging.Log;
2828
import org.apache.commons.logging.LogFactory;
2929
import org.springframework.core.convert.ConversionService;
30+
import org.springframework.jdbc.core.JdbcOperations;
3031
import org.springframework.jdbc.core.JdbcTemplate;
3132
import org.springframework.jdbc.core.RowMapper;
3233
import org.springframework.security.acls.domain.ObjectIdentityImpl;
@@ -67,7 +68,7 @@ public class JdbcAclService implements AclService {
6768
// ~ Instance fields
6869
// ================================================================================================
6970

70-
protected final JdbcTemplate jdbcTemplate;
71+
protected final JdbcOperations jdbcOperations;
7172
private final LookupStrategy lookupStrategy;
7273
private boolean aclClassIdSupported;
7374
private String findChildrenSql = DEFAULT_SELECT_ACL_WITH_PARENT_SQL;
@@ -77,9 +78,13 @@ public class JdbcAclService implements AclService {
7778
// ===================================================================================================
7879

7980
public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) {
80-
Assert.notNull(dataSource, "DataSource required");
81+
this(new JdbcTemplate(dataSource), lookupStrategy);
82+
}
83+
84+
public JdbcAclService(JdbcOperations jdbcOperations, LookupStrategy lookupStrategy) {
85+
Assert.notNull(jdbcOperations, "JdbcOperations required");
8186
Assert.notNull(lookupStrategy, "LookupStrategy required");
82-
this.jdbcTemplate = new JdbcTemplate(dataSource);
87+
this.jdbcOperations = jdbcOperations;
8388
this.lookupStrategy = lookupStrategy;
8489
this.aclClassIdUtils = new AclClassIdUtils();
8590
}
@@ -88,15 +93,14 @@ public JdbcAclService(DataSource dataSource, LookupStrategy lookupStrategy) {
8893
// ========================================================================================================
8994

9095
public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
91-
Object[] args = { parentIdentity.getIdentifier(), parentIdentity.getType() };
92-
List<ObjectIdentity> objects = jdbcTemplate.query(findChildrenSql, args,
96+
Object[] args = { parentIdentity.getIdentifier().toString(), parentIdentity.getType() };
97+
List<ObjectIdentity> objects = jdbcOperations.query(findChildrenSql, args,
9398
new RowMapper<ObjectIdentity>() {
9499
public ObjectIdentity mapRow(ResultSet rs, int rowNum)
95100
throws SQLException {
96101
String javaType = rs.getString("class");
97102
Serializable identifier = (Serializable) rs.getObject("obj_id");
98103
identifier = aclClassIdUtils.identifierFrom(identifier, rs);
99-
100104
return new ObjectIdentityImpl(javaType, identifier);
101105
}
102106
});

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2004, 2005, 2006, 2017 Acegi Technology Pty Limited
2+
* Copyright 2004, 2005, 2006, 2017, 2018 Acegi Technology Pty Limited
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -135,7 +135,7 @@ protected void createEntries(final MutableAcl acl) {
135135
if (acl.getEntries().isEmpty()) {
136136
return;
137137
}
138-
jdbcTemplate.batchUpdate(insertEntry, new BatchPreparedStatementSetter() {
138+
jdbcOperations.batchUpdate(insertEntry, new BatchPreparedStatementSetter() {
139139
public int getBatchSize() {
140140
return acl.getEntries().size();
141141
}
@@ -170,7 +170,7 @@ public void setValues(PreparedStatement stmt, int i) throws SQLException {
170170
protected void createObjectIdentity(ObjectIdentity object, Sid owner) {
171171
Long sidId = createOrRetrieveSidPrimaryKey(owner, true);
172172
Long classId = createOrRetrieveClassPrimaryKey(object.getType(), true, object.getIdentifier().getClass());
173-
jdbcTemplate.update(insertObjectIdentity, classId, object.getIdentifier(), sidId,
173+
jdbcOperations.update(insertObjectIdentity, classId, object.getIdentifier().toString(), sidId,
174174
Boolean.TRUE);
175175
}
176176

@@ -184,7 +184,7 @@ protected void createObjectIdentity(ObjectIdentity object, Sid owner) {
184184
* @return the primary key or null if not found
185185
*/
186186
protected Long createOrRetrieveClassPrimaryKey(String type, boolean allowCreate, Class idType) {
187-
List<Long> classIds = jdbcTemplate.queryForList(selectClassPrimaryKey,
187+
List<Long> classIds = jdbcOperations.queryForList(selectClassPrimaryKey,
188188
new Object[] { type }, Long.class);
189189

190190
if (!classIds.isEmpty()) {
@@ -193,13 +193,13 @@ protected Long createOrRetrieveClassPrimaryKey(String type, boolean allowCreate,
193193

194194
if (allowCreate) {
195195
if (!isAclClassIdSupported()) {
196-
jdbcTemplate.update(insertClass, type);
196+
jdbcOperations.update(insertClass, type);
197197
} else {
198-
jdbcTemplate.update(insertClass, type, idType.getCanonicalName());
198+
jdbcOperations.update(insertClass, type, idType.getCanonicalName());
199199
}
200200
Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(),
201201
"Transaction must be running");
202-
return jdbcTemplate.queryForObject(classIdentityQuery, Long.class);
202+
return jdbcOperations.queryForObject(classIdentityQuery, Long.class);
203203
}
204204

205205
return null;
@@ -248,18 +248,18 @@ else if (sid instanceof GrantedAuthoritySid) {
248248
protected Long createOrRetrieveSidPrimaryKey(String sidName, boolean sidIsPrincipal,
249249
boolean allowCreate) {
250250

251-
List<Long> sidIds = jdbcTemplate.queryForList(selectSidPrimaryKey, new Object[] {
251+
List<Long> sidIds = jdbcOperations.queryForList(selectSidPrimaryKey, new Object[] {
252252
Boolean.valueOf(sidIsPrincipal), sidName }, Long.class);
253253

254254
if (!sidIds.isEmpty()) {
255255
return sidIds.get(0);
256256
}
257257

258258
if (allowCreate) {
259-
jdbcTemplate.update(insertSid, Boolean.valueOf(sidIsPrincipal), sidName);
259+
jdbcOperations.update(insertSid, Boolean.valueOf(sidIsPrincipal), sidName);
260260
Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(),
261261
"Transaction must be running");
262-
return jdbcTemplate.queryForObject(sidIdentityQuery, Long.class);
262+
return jdbcOperations.queryForObject(sidIdentityQuery, Long.class);
263263
}
264264

265265
return null;
@@ -311,7 +311,7 @@ public void deleteAcl(ObjectIdentity objectIdentity, boolean deleteChildren)
311311
* @param oidPrimaryKey the rows in acl_entry to delete
312312
*/
313313
protected void deleteEntries(Long oidPrimaryKey) {
314-
jdbcTemplate.update(deleteEntryByObjectIdentityForeignKey, oidPrimaryKey);
314+
jdbcOperations.update(deleteEntryByObjectIdentityForeignKey, oidPrimaryKey);
315315
}
316316

317317
/**
@@ -325,7 +325,7 @@ protected void deleteEntries(Long oidPrimaryKey) {
325325
*/
326326
protected void deleteObjectIdentity(Long oidPrimaryKey) {
327327
// Delete the acl_object_identity row
328-
jdbcTemplate.update(deleteObjectIdentityByPrimaryKey, oidPrimaryKey);
328+
jdbcOperations.update(deleteObjectIdentityByPrimaryKey, oidPrimaryKey);
329329
}
330330

331331
/**
@@ -339,8 +339,8 @@ protected void deleteObjectIdentity(Long oidPrimaryKey) {
339339
*/
340340
protected Long retrieveObjectIdentityPrimaryKey(ObjectIdentity oid) {
341341
try {
342-
return jdbcTemplate.queryForObject(selectObjectIdentityPrimaryKey, Long.class,
343-
oid.getType(), oid.getIdentifier());
342+
return jdbcOperations.queryForObject(selectObjectIdentityPrimaryKey, Long.class,
343+
oid.getType(), oid.getIdentifier().toString());
344344
}
345345
catch (DataAccessException notFound) {
346346
return null;
@@ -409,7 +409,7 @@ protected void updateObjectIdentity(MutableAcl acl) {
409409
Assert.notNull(acl.getOwner(), "Owner is required in this implementation");
410410

411411
Long ownerSid = createOrRetrieveSidPrimaryKey(acl.getOwner(), true);
412-
int count = jdbcTemplate.update(updateObjectIdentity, parentId, ownerSid,
412+
int count = jdbcOperations.update(updateObjectIdentity, parentId, ownerSid,
413413
Boolean.valueOf(acl.isEntriesInheriting()), acl.getId());
414414

415415
if (count != 1) {
Lines changed: 145 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -15,41 +15,71 @@
1515
*/
1616
package org.springframework.security.acls.jdbc;
1717

18-
import static org.mockito.ArgumentMatchers.anyList;
19-
import static org.mockito.Mockito.when;
20-
21-
import java.util.Arrays;
22-
import java.util.HashMap;
23-
import java.util.List;
24-
import java.util.Map;
25-
26-
import javax.sql.DataSource;
27-
18+
import org.junit.After;
2819
import org.junit.Before;
2920
import org.junit.Test;
3021
import org.junit.runner.RunWith;
3122
import org.mockito.Mock;
3223
import org.mockito.junit.MockitoJUnitRunner;
24+
import org.springframework.jdbc.core.JdbcOperations;
25+
import org.springframework.jdbc.core.RowMapper;
26+
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
27+
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
3328
import org.springframework.security.acls.domain.ObjectIdentityImpl;
3429
import org.springframework.security.acls.domain.PrincipalSid;
3530
import org.springframework.security.acls.model.Acl;
3631
import org.springframework.security.acls.model.NotFoundException;
3732
import org.springframework.security.acls.model.ObjectIdentity;
3833
import org.springframework.security.acls.model.Sid;
3934

35+
import javax.sql.DataSource;
36+
import java.util.*;
37+
38+
import static org.assertj.core.api.Assertions.assertThat;
39+
import static org.mockito.AdditionalMatchers.aryEq;
40+
import static org.mockito.ArgumentMatchers.*;
41+
import static org.mockito.Mockito.when;
42+
43+
/**
44+
* Unit and Integration tests the ACL JdbcAclService using an
45+
* in-memory database.
46+
*
47+
* @author Nena Raab
48+
*/
4049
@RunWith(MockitoJUnitRunner.class)
4150
public class JdbcAclServiceTests {
51+
52+
private EmbeddedDatabase embeddedDatabase;
53+
4254
@Mock
4355
private DataSource dataSource;
4456

4557
@Mock
4658
private LookupStrategy lookupStrategy;
4759

60+
@Mock
61+
JdbcOperations jdbcOperations;
62+
63+
private JdbcAclService aclServiceIntegration;
4864
private JdbcAclService aclService;
4965

5066
@Before
5167
public void setUp() {
52-
aclService = new JdbcAclService(dataSource, lookupStrategy);
68+
aclService = new JdbcAclService(jdbcOperations, lookupStrategy);
69+
aclServiceIntegration = new JdbcAclService(embeddedDatabase, lookupStrategy);
70+
}
71+
72+
@Before
73+
public void setUpEmbeddedDatabase() {
74+
embeddedDatabase = new EmbeddedDatabaseBuilder()//
75+
.addScript("createAclSchemaWithAclClassIdType.sql")
76+
.addScript("db/sql/test_data_hierarchy.sql")
77+
.build();
78+
}
79+
80+
@After
81+
public void tearDownEmbeddedDatabase() {
82+
embeddedDatabase.shutdown();
5383
}
5484

5585
// SEC-1898
@@ -60,8 +90,110 @@ public void readAclByIdMissingAcl() {
6090
lookupStrategy.readAclsById(anyList(),
6191
anyList())).thenReturn(result);
6292
ObjectIdentity objectIdentity = new ObjectIdentityImpl(Object.class, 1);
63-
List<Sid> sids = Arrays.<Sid> asList(new PrincipalSid("user"));
93+
List<Sid> sids = Arrays.<Sid>asList(new PrincipalSid("user"));
6494

6595
aclService.readAclById(objectIdentity, sids);
6696
}
97+
98+
@Test
99+
public void findOneChildren() {
100+
List<ObjectIdentity> result = new ArrayList<>();
101+
result.add(new ObjectIdentityImpl(Object.class, "5577"));
102+
Object[] args = {"1", "org.springframework.security.acls.jdbc.JdbcAclServiceTests$MockLongIdDomainObject"};
103+
when(
104+
jdbcOperations.query(anyString(),
105+
aryEq(args), any(RowMapper.class))).thenReturn(result);
106+
ObjectIdentity objectIdentity = new ObjectIdentityImpl(MockLongIdDomainObject.class, 1L);
107+
108+
List<ObjectIdentity> objectIdentities = aclService.findChildren(objectIdentity);
109+
assertThat(objectIdentities.size()).isEqualTo(1);
110+
assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo("5577");
111+
}
112+
113+
@Test
114+
public void findNoChildren() {
115+
ObjectIdentity objectIdentity = new ObjectIdentityImpl(MockLongIdDomainObject.class, 1L);
116+
117+
List<ObjectIdentity> objectIdentities = aclService.findChildren(objectIdentity);
118+
assertThat(objectIdentities).isNull();
119+
}
120+
121+
// ~ Some integration tests
122+
// ========================================================================================================
123+
124+
@Test
125+
public void findChildrenWithoutIdType() {
126+
ObjectIdentity objectIdentity = new ObjectIdentityImpl(MockLongIdDomainObject.class, 4711L);
127+
128+
List<ObjectIdentity> objectIdentities = aclServiceIntegration.findChildren(objectIdentity);
129+
assertThat(objectIdentities.size()).isEqualTo(1);
130+
assertThat(objectIdentities.get(0).getType()).isEqualTo(MockUntypedIdDomainObject.class.getName());
131+
assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo(5000L);
132+
}
133+
134+
@Test
135+
public void findChildrenForUnknownObject() {
136+
ObjectIdentity objectIdentity = new ObjectIdentityImpl(Object.class, 33);
137+
138+
List<ObjectIdentity> objectIdentities = aclServiceIntegration.findChildren(objectIdentity);
139+
assertThat(objectIdentities).isNull();
140+
}
141+
142+
@Test
143+
public void findChildrenOfIdTypeLong() {
144+
ObjectIdentity objectIdentity = new ObjectIdentityImpl("location", "US-PAL");
145+
146+
List<ObjectIdentity> objectIdentities = aclServiceIntegration.findChildren(objectIdentity);
147+
assertThat(objectIdentities.size()).isEqualTo(2);
148+
assertThat(objectIdentities.get(0).getType()).isEqualTo(MockLongIdDomainObject.class.getName());
149+
assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo(4711L);
150+
assertThat(objectIdentities.get(1).getType()).isEqualTo(MockLongIdDomainObject.class.getName());
151+
assertThat(objectIdentities.get(1).getIdentifier()).isEqualTo(4712L);
152+
}
153+
154+
@Test
155+
public void findChildrenOfIdTypeString() {
156+
ObjectIdentity objectIdentity = new ObjectIdentityImpl("location", "US");
157+
158+
aclServiceIntegration.setAclClassIdSupported(true);
159+
List<ObjectIdentity> objectIdentities = aclServiceIntegration.findChildren(objectIdentity);
160+
assertThat(objectIdentities.size()).isEqualTo(1);
161+
assertThat(objectIdentities.get(0).getType()).isEqualTo("location");
162+
assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo("US-PAL");
163+
}
164+
165+
@Test
166+
public void findChildrenOfIdTypeUUID() {
167+
ObjectIdentity objectIdentity = new ObjectIdentityImpl(MockUntypedIdDomainObject.class, 5000L);
168+
169+
aclServiceIntegration.setAclClassIdSupported(true);
170+
List<ObjectIdentity> objectIdentities = aclServiceIntegration.findChildren(objectIdentity);
171+
assertThat(objectIdentities.size()).isEqualTo(1);
172+
assertThat(objectIdentities.get(0).getType()).isEqualTo("costcenter");
173+
assertThat(objectIdentities.get(0).getIdentifier()).isEqualTo(UUID.fromString("25d93b3f-c3aa-4814-9d5e-c7c96ced7762"));
174+
}
175+
176+
private class MockLongIdDomainObject {
177+
private Object id;
178+
179+
public Object getId() {
180+
return id;
181+
}
182+
183+
public void setId(Object id) {
184+
this.id = id;
185+
}
186+
}
187+
188+
private class MockUntypedIdDomainObject {
189+
private Object id;
190+
191+
public Object getId() {
192+
return id;
193+
}
194+
195+
public void setId(Object id) {
196+
this.id = id;
197+
}
198+
}
67199
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--- insert ACL data
2+
INSERT INTO ACL_SID (ID, PRINCIPAL, SID) VALUES
3+
(10, true, 'user');
4+
5+
INSERT INTO acl_class (id, class, class_id_type) VALUES
6+
(20,'location','java.lang.String'),
7+
(21,'org.springframework.security.acls.jdbc.JdbcAclServiceTests$MockLongIdDomainObject','java.lang.Long'),
8+
(22,'org.springframework.security.acls.jdbc.JdbcAclServiceTests$MockUntypedIdDomainObject',''),
9+
(23,'costcenter','java.util.UUID');
10+
11+
INSERT INTO acl_object_identity (id, object_id_class, object_id_identity, parent_object, owner_sid, entries_inheriting) VALUES
12+
(1,20,'US',NULL,10,false),
13+
(2,20,'US-PAL',1,10,true),
14+
(3,21,'4711',2,10,true),
15+
(4,21,'4712',2,10,true),
16+
(5,22,'5000',3,10,true),
17+
(6,23,'25d93b3f-c3aa-4814-9d5e-c7c96ced7762',5,10,true);

0 commit comments

Comments
 (0)