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.

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

Secure all entities #182

Merged
merged 3 commits into from
May 2, 2016
Merged

Conversation

buehner
Copy link
Member

@buehner buehner commented Apr 29, 2016

This PR contains a major refactoring of the permission handling:

  • SecuredPersistentObject has been removed and its userPermissions and groupPermissions properties have been moved up to the PersistentObject model, which allows us to protect all entities now (based on project specific needs). The services and tests have been adapted according to this change.
  • When starting the system in a vanilla state (with hibernate.hbm2ddl.auto=create) there will only be one table for all user permissions and one table for all group permissions (of all entities). This is a huge simplification on the database side! Existing projects that can not start in a clean state need a data migration.
  • An AbstractPermissionAwareCrudService has been introduced and should be extended by ALL services. There is only one expection: PermissionCollectionService. Have a look at the release note changes within this PR for details.
  • To obtain the previous behaviour of "unsecured" objects (that did not extend from the removed class SecuredPersistentObject) an AlwaysAllowReadPermissionEvaluator has been introduced. By default, this evaluator allows READ access for the following entities (and all their subclasses):
    • Extent
    • InterceptorRule
    • LayerAppearance
    • LayerDataSource
    • Layout
    • MapConfig
    • MapControl
    • Module
    • Role
    • TileGrid
    • Token
  • By default (in the sense of SHOGun2), the following types (and all subclasses) need a set of explicit permissions (previously SecuredPersistentObjects):
    • AbstractLayer
    • Application
    • File
    • Person
    • UserGroup

Please review!

@@ -9,7 +9,8 @@
* The `saveOrUpdate` method of the services are now void. Existing projects that are using this method may need some simple adaptions like changes from `PersistentObject newObject = object.saveOrUpdate()` to `object.saveOrUpdate()`
* The webapp-archetype has been extended to demonstrate how custom permission evaluators for project specific solutions can be used.
* New features:
* An `AbstractSecuredPersistentObjectService` has been introduced. This service provides useful methods to add and remove permissions for certain objects. `PermissionCollection`s will be persisted in the database when using these methods. All services of entities that extend `SecuredPersistentObject` should extend the abstract service mentioned above.
* All `PersistentObject`s now have a set of user and group permissions, i.e. all entities can be protected if needed. In this context, the permission evaluators have been overworked. The database structure has changed here, which means that existing projects are affected by this change and would need a data migration if they can not boot in a vanilla state (with `hibernate.hbm2ddl.auto=create`).
Copy link
Member

Choose a reason for hiding this comment

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

overworked => overhauled?

@marcjansen
Copy link
Member

👏 👏 👏 @buehner: nice work! I really like the simplification this brings to both code and applications.

Some minor questions (which may even end up in the release notes file, who knows)

  • Can you outline how an application using SHOGun2 would implement a different (more restrictive) permission check, say for e.g. Module?
  • Can you outline how an application using SHOGun2 would implement a different (more lenient) permission check, say for e.g. AbstractLayer?
  • In case one cannot use hbm2ddl=create, what would the migrations basically look like (just one example)?

I'd love to hear anything from @weskamm about this (and the needed restructurings that will follow for his projects).

And as always… we may want to release the next dev-version before we merge this, right?

@buehner
Copy link
Member Author

buehner commented May 2, 2016

  • Can you outline how an application using SHOGun2 would implement a different (more restrictive) permission check, say for e.g. Module?
  • Can you outline how an application using SHOGun2 would implement a different (more lenient) permission check, say for e.g. AbstractLayer?

Custom permission evaluation can be controlled by injecting a custom implementation of a permissionEvaluatorFactory into the de.terrestris.shogun2.security.access.Shogun2PermissionEvaluator bean. The webapp archetype demonstrates this by using a ProjectEntityPermissionEvaluatorFactory, which is configured in the ***-context-security.xml file.

<beans:bean id="permissionEvaluator" class="de.terrestris.shogun2.security.access.Shogun2PermissionEvaluator">
    <beans:property name="permissionEvaluatorFactory">
        <!-- This is how project specific permission evaluators can be used/configured -->
        <beans:bean class="de.terrestris.sandbox.security.access.factory.ProjectEntityPermissionEvaluatorFactory" />
    </beans:property>
</beans:bean>

The factory is responsible to return the correct permission evaluator for a given entity class, e.g.

if(ProjectApplication.class.isAssignableFrom(entityClass)) {
    return new ProjectApplicationPermissionEvaluator();
}

The concrete permission evaluators need to implement the method

@Override
public boolean hasPermission(User user, E entity, Permission permission) {
...
}

It is recommended to extend the following SHOGun2 base classes when implementing a factory or a permission evaluator:

  • public class EntityPermissionEvaluatorFactory<E extends PersistentObject> for factories
  • public class PersistentObjectPermissionEvaluator<E extends PersistentObject> for permission evaluators

Then you can implement your custom logic and call the methods from the parent classes as a fallback/default.

@weskamm
Copy link
Member

weskamm commented May 2, 2016

nice restructuring, i like the idea to have every entity secured by default and configuring access on a project level. Maybe you want to put your explanations from above somewhere in the readme / docs?

@buehner
Copy link
Member Author

buehner commented May 2, 2016

  • In case one cannot use hbm2ddl=create, what would the migrations basically look like (just one example)?

Previously there were tables - e.g. applications_userpermissions or applications_grouppermissions - for each subclass of type SecuredPersistentObject (like Application, AbstractLayer, File etc.).

As the SecuredPersistentObject class has been removed and all subclasses of PersistentObject can be protected now, only the following tables remain for the storage of the permission information:

  • userpermissions
  • grouppermissions

So the data from applications_userpermissions or files_userpermissions need a migration to the new userpermissions table.

@weskamm
Copy link
Member

weskamm commented May 2, 2016

👍

@buehner
Copy link
Member Author

buehner commented May 2, 2016

any concerns merging this now @marcjansen ?

i could successfully run this PR in a project!

@marcjansen
Copy link
Member

  • typo: the permission evaluators have been overworked overhauled.
  • If possible, please do as @weskamm suggested and put your (very nice and detailed) instructions into the migration notes.

If you are in a hurry, we can do it later.

Looks great otherwise 👍

@buehner
Copy link
Member Author

buehner commented May 2, 2016

We'll do the documentation later. I created a separate issue for this: #183

@buehner buehner merged commit b0c1b90 into terrestris:master May 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants