AclAssignerAspect handles Iterables

security/src/main/java/org/genesys/blocks/security/component/AclAssignerAspect.java

+/*
+ * Copyright 2017 Global Crop Diversity Trust
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package org.genesys.blocks.security.component;
 
 import org.aspectj.lang.JoinPoint;
@@ -17,6 +32,7 @@ import org.springframework.stereotype.Component;
  * Using aspects to generate default ACL permissions when entities are persisted and remove permissions on delete.
  * 
  * @author Maxym Borodenko
+ * @author Matija Obreza
  */
 @Aspect
 @Component
@@ -27,37 +43,42 @@ public class AclAssignerAspect {
 	@Autowired
 	private CustomAclService aclService;
 
+	public AclAssignerAspect() {
+		LOG.warn("Enabling {}", getClass().getName());
+	}
+
 	/**
 	 * Create owner permissions on persist
 	 * 
 	 * @param result
 	 * @return
 	 */
-	@AfterReturning(pointcut = "execution(* org.springframework.data.repository.*.save(..))", returning = "result")
+	@AfterReturning(pointcut = "execution(* org.springframework.data.repository.*.save(..)) || execution(* org.springframework.data.jpa.repository.*.save(..)) || execution(* org.springframework.data.jpa.repository.*.saveAndFlush(..))", returning = "result")
 	public Object afterSaveAclObject(Object result) {
-		boolean needsAcl = false;
 		final Authentication auth = SecurityContextHolder.getContext().getAuthentication();
 
 		if (auth != null) {
 			if (result instanceof AclAwareModel) {
-				final AclAwareModel aclModel = (AclAwareModel) result;
-				if (aclModel.getId() != null) {
-					needsAcl = true;
+				maybeAddCreatorPermissions(result);
+			} else if (result instanceof Iterable) {
+				// Handle collections of AclAwareModel
+				Iterable<?> i = (Iterable<?>) result;
+				for (Object o : i) {
+					maybeAddCreatorPermissions(o);
 				}
+			} else {
+				LOG.trace("{} is not instance of AclAwareModel", result);
 			}
-			// TODO What about collections of AclAwareModel?
 		}
 
-		try {
-			if (needsAcl) {
-				// TODO What about collections of AclAwareModel?
-				final AclAwareModel aclModel = (AclAwareModel) result;
-				LOG.debug("Inserting ACL entries for owner={}", aclModel.getId());
-				aclService.addCreatorPermissions(aclModel);
-			}
-			return result;
-		} finally {
-			// Nothing to do here
+		return result;
+	}
+
+	private void maybeAddCreatorPermissions(Object obj) {
+		if (obj instanceof AclAwareModel) {
+			aclService.addCreatorPermissions((AclAwareModel) obj);
+		} else {
+			LOG.trace("{} is not instance of AclAwareModel", obj);
 		}
 	}
 
@@ -66,19 +87,36 @@ public class AclAssignerAspect {
 	 * 
 	 * @param joinPoint
 	 */
-	@After("execution(* org.springframework.data.repository.*.delete(..))")
+	@After("execution(* org.springframework.data.repository.*.delete(..)) || execution(* org.springframework.data.jpa.repository.*.deleteInBatch(..))")
 	public void afterDeleteAclObject(JoinPoint joinPoint) {
 		final Object arg0 = joinPoint.getArgs()[0];
 
 		try {
-			if (arg0 instanceof AclAwareModel) {
+			if (arg0 instanceof Long) {
+				// NOOP
+			} else if (arg0 instanceof AclAwareModel) {
 				final AclAwareModel aclModel = (AclAwareModel) arg0;
-				LOG.debug("Removing ACL entries for model={}", aclModel);
-				aclService.removePermissions(aclModel);
+				maybeRemovePermissions(aclModel);
+			} else if (arg0 instanceof Iterable) {
+				// Handle collections of AclAwareModel
+				Iterable<?> i = (Iterable<?>) arg0;
+				for (Object o : i) {
+					maybeRemovePermissions(o);
+				}
+			} else {
+				LOG.trace("{} is not instance of AclAwareModel", arg0);
 			}
-			// TODO What about collections of AclAwareModel?
 		} finally {
 			// Nothing to do here
 		}
 	}
+
+	private void maybeRemovePermissions(final Object obj) {
+		if (obj instanceof AclAwareModel) {
+			LOG.debug("Removing ACL entries for model={}", obj);
+			aclService.removePermissions((AclAwareModel) obj);
+		} else {
+			LOG.trace("{} is not instance of AclAwareModel", obj);
+		}
+	}
 }

security/src/main/java/org/genesys/blocks/security/service/CustomAclService.java

+/*
+ * Copyright 2017 Global Crop Diversity Trust
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package org.genesys.blocks.security.service;
 
 import java.util.List;

security/src/main/java/org/genesys/blocks/security/service/impl/CustomAclServiceImpl.java

@@ -19,6 +19,7 @@ import org.genesys.blocks.security.service.CustomAclService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.cache.Cache;
 import org.springframework.cache.CacheManager;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.acls.domain.BasePermission;
@@ -58,7 +59,7 @@ public class CustomAclServiceImpl implements CustomAclService {
 	@Override
 	@Transactional(propagation = Propagation.REQUIRED)
 	public void addCreatorPermissions(final AclAwareModel target) {
-		if (target == null) {
+		if (target == null || target.getId() <= 0l) {
 			LOG.warn("No target specified for ACL permissions, bailing out!");
 			return;
 		}
@@ -68,6 +69,8 @@ public class CustomAclServiceImpl implements CustomAclService {
 			LOG.warn("No user in security context, not doing ACL");
 			return;
 		}
+		
+		LOG.debug("Inserting owner ACL entries for owner={} class={} id={}", uuid, target.getClass().getName(), target.getId());
 
 		// it can be pre-authorized Admin
 		final AclSid aclSid = ensureSid(uuid, true);
@@ -125,7 +128,10 @@ public class CustomAclServiceImpl implements CustomAclService {
 			// save ACL
 			aclEntryPersistence.save(aclEntry);
 		}
-		cacheManager.getCache("aclCache").clear();
+		
+		Cache aclCache = cacheManager.getCache("aclCache");
+		if (aclCache != null)
+			aclCache.clear();
 	}
 
 	/**
@@ -185,7 +191,14 @@ public class CustomAclServiceImpl implements CustomAclService {
 	@Override
 	@Transactional(readOnly = true)
 	public AclObjectIdentity getObjectIdentity(final AclAwareModel entity) {
-		return aclObjectIdentityPersistence.findByObjectIdIdentityAndClassName((long) entity.getId(), entity.getClass().getName());
+		if (entity == null) {
+			LOG.error("getObjectIdentity: Entity is null");
+		}
+		AclObjectIdentity oid = aclObjectIdentityPersistence.findByObjectIdIdentityAndClassName((long) entity.getId(), entity.getClass().getName());
+		if (oid == null) {
+			LOG.warn("ACL object identity not found for class={} id={}", entity.getClass().getName(), entity.getId());
+		}
+		return oid;
 	}
 
 	@Override

security/src/test/java/org/genesys/blocks/security/component/AclAssignerTest.java

+/*
+ * Copyright 2017 Global Crop Diversity Trust
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.genesys.blocks.security.component;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.Assert.assertThat;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.UUID;
+
+import org.genesys.blocks.security.model.AclEntity;
+import org.genesys.blocks.security.persistence.AclEntityPersistence;
+import org.genesys.blocks.security.persistence.AclEntryPersistence;
+import org.genesys.blocks.security.persistence.AclObjectIdentityPersistence;
+import org.genesys.blocks.security.service.CustomAclService;
+import org.genesys.blocks.security.tests.BaseTest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.core.userdetails.UserDetails;
+import org.springframework.test.context.transaction.AfterTransaction;
+import org.springframework.transaction.annotation.Transactional;
+
+import com.google.common.collect.Lists;
+
+/**
+ * @author Matija Obreza
+ */
+public class AclAssignerTest extends BaseTest {
+
+	private final UUID USER_UUID = UUID.randomUUID();
+	@Autowired
+	private CustomAclService aclService;
+	@Autowired
+	private AclEntityPersistence aclEntityPersistence;
+	@Autowired
+	private AclEntryPersistence aclEntryPersistence;
+	@Autowired
+	private AclObjectIdentityPersistence aclObjectIdentityPersistence;
+
+	@AfterTransaction
+	@Transactional
+	public void cleanup() {
+		super.cleanup();
+		aclEntityPersistence.deleteAllInBatch();
+		aclEntryPersistence.deleteAllInBatch();
+		aclObjectIdentityPersistence.deleteAllInBatch();
+	}
+
+	@Before
+	public void setupSecurityContext() {
+		UserDetails userDetails = new UserDetails() {
+			private static final long serialVersionUID = 1L;
+			private UUID uuid = USER_UUID;
+
+			@Override
+			public Collection<? extends GrantedAuthority> getAuthorities() {
+				return null;
+			}
+
+			@Override
+			public String getPassword() {
+				return null;
+			}
+
+			@Override
+			public String getUsername() {
+				return uuid.toString();
+			}
+
+			@Override
+			public boolean isAccountNonExpired() {
+				return true;
+			}
+
+			@Override
+			public boolean isAccountNonLocked() {
+				return true;
+			}
+
+			@Override
+			public boolean isCredentialsNonExpired() {
+				return true;
+			}
+
+			@Override
+			public boolean isEnabled() {
+				return true;
+			}
+		};
+
+		SecurityContextHolder.getContext()
+				.setAuthentication(new UsernamePasswordAuthenticationToken(userDetails, "bar"));
+	}
+
+	@After
+	public void clearSecurityContext() {
+		SecurityContextHolder.getContext().setAuthentication(null);
+	}
+
+	@Test
+	@Transactional
+	public void testSave1() {
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		AclEntity acl1 = new AclEntity();
+		acl1.setName("Test 1");
+		acl1 = aclEntityPersistence.save(acl1);
+		assertThat(aclService.getAclEntries(acl1), hasSize(greaterThan(0)));
+
+		LOG.info("Removing entity");
+		aclEntityPersistence.delete(acl1);
+		LOG.info("Removed entity");
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		assertThat(aclObjectIdentityPersistence.count(), equalTo(0l));
+	}
+
+
+	@Test
+	@Transactional
+	@Ignore // We don't properly handle delete by ID
+	public void testDeleteById() {
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		AclEntity acl1 = new AclEntity();
+		acl1.setName("Test 1");
+		acl1 = aclEntityPersistence.save(acl1);
+		assertThat(aclService.getAclEntries(acl1), hasSize(greaterThan(0)));
+
+		LOG.info("Removing entity by ID");
+		aclEntityPersistence.delete(acl1.getId());
+		LOG.info("Removed entity");
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		assertThat(aclObjectIdentityPersistence.count(), equalTo(0l));
+	}
+
+	@Test
+	@Transactional
+	public void testSaveAndFlush1() {
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		AclEntity acl1 = new AclEntity();
+		acl1.setName("Test 1");
+		acl1 = aclEntityPersistence.saveAndFlush(acl1);
+		assertThat(aclService.getAclEntries(acl1), hasSize(greaterThan(0)));
+
+		LOG.info("Removing entity");
+		aclEntityPersistence.delete(acl1);
+		LOG.info("Removed entity");
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		assertThat(aclObjectIdentityPersistence.count(), equalTo(0l));
+	}
+
+	@Test
+	@Transactional
+	public void testSaveMany() {
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		AclEntity acl1 = new AclEntity();
+		acl1.setName("Test 1");
+		AclEntity acl2 = new AclEntity();
+		acl2.setName("Test 2");
+
+		List<AclEntity> saved = aclEntityPersistence.save(Lists.newArrayList(acl1, acl2));
+		assertThat(aclService.getAclEntries(saved.get(0)), hasSize(greaterThan(0)));
+
+		LOG.info("Removing entities");
+		aclEntityPersistence.delete(aclEntityPersistence.findAll());
+		LOG.info("Removed entities");
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		assertThat(aclObjectIdentityPersistence.count(), equalTo(0l));
+	}
+
+	@Test
+	@Transactional
+	public void testDeleteInBatch() {
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		AclEntity acl1 = new AclEntity();
+		acl1.setName("Test 1");
+		AclEntity acl2 = new AclEntity();
+		acl2.setName("Test 2");
+
+		List<AclEntity> saved = aclEntityPersistence.save(Lists.newArrayList(acl1, acl2));
+		assertThat(aclService.getAclEntries(saved.get(0)), hasSize(greaterThan(0)));
+
+		LOG.info("Removing entities");
+		aclEntityPersistence.deleteInBatch(aclEntityPersistence.findAll());
+		LOG.info("Removed entities");
+		assertThat(aclEntryPersistence.count(), equalTo(0l));
+		assertThat(aclObjectIdentityPersistence.count(), equalTo(0l));
+	}
+
+}

security/src/test/java/org/genesys/blocks/security/config/ApplicationConfig.java

@@ -46,7 +46,7 @@ import org.springframework.transaction.annotation.Transactional;
 @Configuration
 @EnableAspectJAutoProxy
 @Import({ DatabaseConfig.class })
-@ComponentScan(basePackages = { "org.genesys.blocks.oauth.service" })
+@ComponentScan(basePackages = { "org.genesys.blocks.oauth.service", "org.genesys.blocks.security.service", "org.genesys.blocks.security.component" })
 public class ApplicationConfig {
 
     @Bean

security/src/test/java/org/genesys/blocks/security/model/AclEntity.java

+/*
+ * Copyright 2017 Global Crop Diversity Trust
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.genesys.blocks.security.model;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+
+import org.genesys.blocks.model.BasicModel;
+
+/**
+ * Sample entity with ACL support
+ * 
+ * @author Matija Obreza
+ */
+@Entity
+public class AclEntity extends BasicModel implements AclAwareModel {
+	private static final long serialVersionUID = 1L;
+
+	@Column(unique = true)
+	private String name;
+
+	public String getName() {
+		return name;
+	}
+
+	public void setName(String name) {
+		this.name = name;
+	}
+}
\ No newline at end of file

security/src/test/java/org/genesys/blocks/security/persistence/AclEntityPersistence.java

+/*
+ * Copyright 2017 Global Crop Diversity Trust
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.genesys.blocks.security.persistence;
+
+import org.genesys.blocks.security.model.AclEntity;
+import org.springframework.data.jpa.repository.JpaRepository;
+
+public interface AclEntityPersistence extends JpaRepository<AclEntity, Long> {
+
+	AclEntity findByName(String name);
+
+}