Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Protect by id and type of entity #186

Merged
merged 3 commits into from
May 11, 2016
Merged

Protect by id and type of entity #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ae6f92
Refactor GenericHibernateDao
buehner May 10, 2016
01cc631
Implement hasPermission based on ID and type
buehner May 10, 2016
25d8288
Protect with new hasPermission implementation
buehner May 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 24 additions & 16 deletions src/shogun2-core/src/main/java/de/terrestris/shogun2/dao/GenericHibernateDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import de.terrestris.shogun2.paging.PagingResult;

/**
* The abstract superclass for all data access objects. Provides basic CRUD
* The superclass for all data access objects. Provides basic CRUD
* functionality and a logger instance for all subclasses.
*
* @author Nils Bühner
Expand All @@ -37,7 +37,7 @@ public class GenericHibernateDao<E extends PersistentObject, ID extends Serializ
/**
* Represents the class of the entity
*/
private final Class<E> clazz;
private final Class<E> entityClass;

/**
* Default constructor
Expand All @@ -53,7 +53,7 @@ public GenericHibernateDao() {
* @param clazz
*/
protected GenericHibernateDao(Class<E> clazz) {
this.clazz = clazz;
this.entityClass = clazz;
}

/**
Expand Down Expand Up @@ -82,8 +82,8 @@ private Session getSession() {
* @return The object from the database or null if it does not exist
*/
public E findById(ID id) {
LOG.trace("Finding " + clazz.getSimpleName() + " with ID " + id);
return (E) getSession().get(clazz, id);
LOG.trace("Finding " + entityClass.getSimpleName() + " with ID " + id);
return (E) getSession().get(entityClass, id);
}

/**
Expand All @@ -98,8 +98,8 @@ public E findById(ID id) {
* @return
*/
public E loadById(ID id) {
LOG.trace("Loading " + clazz.getSimpleName() + " with ID " + id);
return (E) getSession().load(clazz, id);
LOG.trace("Loading " + entityClass.getSimpleName() + " with ID " + id);
return (E) getSession().load(entityClass, id);
}

/**
Expand All @@ -109,7 +109,7 @@ public E loadById(ID id) {
* @return All entities
*/
public List<E> findAll() throws HibernateException {
LOG.trace("Finding all instances of " + clazz.getSimpleName());
LOG.trace("Finding all instances of " + entityClass.getSimpleName());
return findByCriteria();
}

Expand All @@ -124,7 +124,7 @@ public void saveOrUpdate(E e) {
String createOrUpdatePrefix = hasId ? "Updating" : "Creating a new";
String idSuffix = hasId ? "with ID " + id : "";

LOG.trace(createOrUpdatePrefix + " instance of " + clazz.getSimpleName()
LOG.trace(createOrUpdatePrefix + " instance of " + entityClass.getSimpleName()
+ idSuffix);

e.setModified(DateTime.now());
Expand All @@ -137,7 +137,7 @@ public void saveOrUpdate(E e) {
* @param e The entity to remove from the database.
*/
public void delete(E e) {
LOG.trace("Deleting " + clazz.getSimpleName() + " with ID " + e.getId());
LOG.trace("Deleting " + entityClass.getSimpleName() + " with ID " + e.getId());
getSession().delete(e);
}

Expand All @@ -151,7 +151,7 @@ public void delete(E e) {
*/
@SuppressWarnings("unchecked")
public List<E> findByCriteria(Criterion... criterion) throws HibernateException {
LOG.trace("Finding instances of " + clazz.getSimpleName()
LOG.trace("Finding instances of " + entityClass.getSimpleName()
+ " based on " + criterion.length + " criteria");

Criteria criteria = createDistinctRootEntityCriteria(criterion);
Expand All @@ -170,7 +170,7 @@ public List<E> findByCriteria(Criterion... criterion) throws HibernateException
*/
@SuppressWarnings("unchecked")
public E findByUniqueCriteria(Criterion... criterion) throws HibernateException {
LOG.trace("Finding one unique " + clazz.getSimpleName()
LOG.trace("Finding one unique " + entityClass.getSimpleName()
+ " based on " + criterion.length + " criteria");

Criteria criteria = createDistinctRootEntityCriteria(criterion);
Expand All @@ -195,7 +195,7 @@ public PagingResult<E> findByCriteriaWithSortingAndPaging(Integer firstResult,

int nrOfSorters = sorters == null ? 0 : sorters.size();

LOG.trace("Finding instances of " + clazz.getSimpleName()
LOG.trace("Finding instances of " + entityClass.getSimpleName()
+ " based on " + criterion.length + " criteria"
+ " with " + nrOfSorters + " sorters");

Expand Down Expand Up @@ -223,15 +223,15 @@ public PagingResult<E> findByCriteriaWithSortingAndPaging(Integer firstResult,
}

/**
* Helper method: Creates a criteria for the {@link #clazz} of this dao.
* Helper method: Creates a criteria for the {@link #entityClass} of this dao.
* The query results will be handled with a
* {@link DistinctRootEntityResultTransformer}. The criteria will contain
* all passed criterions.
*
* @return
*/
protected Criteria createDistinctRootEntityCriteria(Criterion... criterion) {
Criteria criteria = getSession().createCriteria(clazz);
Criteria criteria = getSession().createCriteria(entityClass);
addCriterionsToCriteria(criteria, criterion);
criteria.setResultTransformer(Criteria.DISTINCT_ROOT_ENTITY);
return criteria;
Expand All @@ -244,7 +244,7 @@ protected Criteria createDistinctRootEntityCriteria(Criterion... criterion) {
* @return
*/
private Number getTotalCount(Criterion... criterion) throws HibernateException {
Criteria criteria = getSession().createCriteria(clazz);
Criteria criteria = getSession().createCriteria(entityClass);
addCriterionsToCriteria(criteria, criterion);
criteria.setProjection(Projections.rowCount());
return (Long) criteria.uniqueResult();
Expand All @@ -265,4 +265,12 @@ private void addCriterionsToCriteria(Criteria criteria, Criterion... criterion)
}
}
}

/**
* @return the entityClass
*/
public Class<E> getEntityClass() {
return entityClass;
}

}
74 changes: 72 additions & 2 deletions ...-core/src/main/java/de/terrestris/shogun2/security/access/Shogun2PermissionEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package de.terrestris.shogun2.security.access;

import java.io.Serializable;
import java.util.Collection;

import org.apache.log4j.Logger;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.ApplicationContext;
import org.springframework.security.access.PermissionEvaluator;
import org.springframework.security.core.Authentication;

import de.terrestris.shogun2.dao.GenericHibernateDao;
import de.terrestris.shogun2.dao.UserDao;
import de.terrestris.shogun2.model.PersistentObject;
import de.terrestris.shogun2.model.User;
Expand All @@ -26,6 +29,9 @@ public class Shogun2PermissionEvaluator implements PermissionEvaluator {
*/
private final static Logger LOG = Logger.getLogger(Shogun2PermissionEvaluator.class);

@Autowired
private ApplicationContext appContext;

/**
* We have to use the DAO here. If we would use the service, we would end
* with StackOverflow errors as a call to (secured) service methods triggers
Expand Down Expand Up @@ -85,12 +91,76 @@ public boolean hasPermission(Authentication authentication,
}

/**
* We do currently do not support/use this implementation.
*
*/
@SuppressWarnings({ "rawtypes", "unchecked" })
@Override
public boolean hasPermission(Authentication authentication,
Serializable targetId, String targetType, Object permission) {
return false;

Class<?> entityClass;

try {
entityClass = Class.forName(targetType);
} catch (ClassNotFoundException e) {
LOG.error("Could not create class for type: " + targetType + "(" + e.getMessage() + ")");
return false;
}

// get all available DAOs from the app context
Collection<GenericHibernateDao> allDaos = appContext.getBeansOfType(GenericHibernateDao.class).values();

// the DAO we'll use to get the entity from the database
GenericHibernateDao daoToUse = null;

// we'll first try to find the exact matching DAO
for (GenericHibernateDao dao : allDaos) {
if(dao.getEntityClass().equals(entityClass)) {
// we found a matching DAO
daoToUse = dao;
LOG.debug("Found an exactly matching DAO for type " + entityClass);
break;
}
}

// if we could not find an exact match, we'll try to use the "next best"
// from the entity hierarchy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will possibly give you true for a class that isn't the best choice, right?

Consider:

A.java (With a matching DAO)
⇧
B.java (With a matching DAO)
⇧
C.java (With a matching DAO)
⇧
D.java (No DAO)

If entityClass is an instance of D, we'll drop here, as we do not have a direct dao-match. The best bet for a dao would be the C-Dao, but when we iterate, isAssignableFrom will be true for any of the parent classes (btw. their daos). We might end up with the not-well-suited A-Dao, for example.

Did I understand this correctly? Amnd if so, can getSuperclass() help? We could then factor ot the doa finding into a small methdi that we could call reapeatedly unto i we actually have the best bet. What do you think?

if(daoToUse == null) {
for (GenericHibernateDao dao : allDaos) {
if(dao.getEntityClass().isAssignableFrom(entityClass)) {
// we found a DAO that will work (e.g. PersonDao for User.class)
daoToUse = dao;
LOG.debug("Found a matching DAO from the hierarchy of type " + entityClass);
break;
}
}
}

// LOG warning if we could NOT find a matching DAO
if(daoToUse == null) {
LOG.warn("Could not find a DAO for type:" + entityClass);
return false;
}

// finally get the entity from the DB
PersistentObject entity = daoToUse.findById(targetId);

// call implementation based on entity
return this.hasPermission(authentication, entity, permission);
}

/**
* @return the appContext
*/
public ApplicationContext getAppContext() {
return appContext;
}

/**
* @param appContext the appContext to set
*/
public void setAppContext(ApplicationContext appContext) {
this.appContext = appContext;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/shogun2-core/src/main/java/de/terrestris/shogun2/service/LayerGroupService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.security.access.prepost.PostFilter;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.stereotype.Service;

import de.terrestris.shogun2.dao.AbstractLayerDao;
Expand Down Expand Up @@ -81,6 +82,7 @@ public Set<E> findLayerGroupsOfAbstractLayer(Integer abstractLayerId) {
* @return
* @throws Exception
*/
@PreAuthorize("hasRole(@configHolder.getSuperAdminRoleName()) or hasPermission(#layerGroupId, 'de.terrestris.shogun2.model.layer.LayerGroup', 'UPDATE')")
public List<AbstractLayer> setLayersForLayerGroup (Integer layerGroupId, List<Integer> abstractLayerIds) {
E layerGroup = this.findById(layerGroupId);
List<AbstractLayer> layers = new ArrayList<AbstractLayer>();
Expand Down
1 change: 1 addition & 0 deletions src/shogun2-core/src/main/java/de/terrestris/shogun2/service/MapService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public Set<E> findMapsWithLayer(AbstractLayer layer) {
* @throws Exception
*/
@SuppressWarnings("unchecked")
@PreAuthorize("hasRole(@configHolder.getSuperAdminRoleName()) or hasPermission(#mapModuleId, 'de.terrestris.shogun2.model.module.Map', 'UPDATE')")
public List<AbstractLayer> setLayersForMap (Integer mapModuleId, List<Integer> abstractLayerIds) throws Exception{
E module = this.findById(mapModuleId);
List<AbstractLayer> layers = new ArrayList<AbstractLayer>();
Expand Down