katharsis-framework icon indicating copy to clipboard operation
katharsis-framework copied to clipboard

Support @RolesAllowed/@DenyAll/@PermitAll annotations on JAX-RS Repositories and Repository methods

Open cjc343 opened this issue 8 years ago • 5 comments

A common JAX-RS authorization implementation uses the @RolesAllowed/@DenyAll/@PermitAll annotations on classes and methods in combination with appropriately setting the SecurityContext and adding a RolesAllowedDynamicFeature which creates request filters for each JAX-RS endpoint.

Since Katharsis uses a @PreMatching filter and does its own method matching, DynamicFeature implementations like the RolesAllowedDynamicFeature do not receive configure calls for Katharsis endpoints (and even if they did, abortWith would cancel the handler chain before it calls the filter).

Ideally, Katharsis should honor RolesAllowed/DenyAll/PermitAll annotations, if present, using the same criteria RolesAllowedDynamicFeature applies:

  • Method @DenyAll takes precedence over
  • Method @RolesAllowed takes precedence over
  • Class @PermitAll takes precedence over
  • Class @RolesAllowed takes precedence over
  • Allow

When these result in a set of roles applied to a given endpoint for a given request, the SecurityContext should be checked to verify that the user is in the required role.

Honoring these annotations would make it easy to set appropriate access restrictions for sensitive endpoints in katharsis-rs:

@RolesAllowed({Permissions.VIEWER})
public class PermanentTaskRepository implements ResourceRepository<PermanentTask, Long> {
    @Override
    public Iterable<PermanentTask> findAll(QueryParams requestParams) {
        return Arrays.asList(new Task());
    }

    @RolesAllowed({Permissions.WRITER})
    @Override
    public <S extends T> S save(S entity) {
        // save entity only if user has write permissions
    }

    @RolesAllowed({Permissions.ADMIN})
    @Override
    public void delete(Long id) {
        // only admins can delete a PermanentTask
    }
}

cjc343 avatar Feb 03 '17 18:02 cjc343

Issue received. I will talk it over with the other people on the team - you are right on this. It would be, however, a serious undertaking. Currently, this isn't on our roadmap

chb0github avatar Feb 04 '17 00:02 chb0github

I do have a katharsis-security module lI would like to contribute for a while. Now that 3.0 changes are merged into the develop branch. I could finally do that. It should solve this problem in a bit of a different way.

=> I try to do that on monday

remmeier avatar Feb 04 '17 18:02 remmeier

I'm aware it's a serious undertaking - I started to look into what it would take and realized that Katharsis' codebase doesn't currently lend itself to an easy implementation of this.

I'm mostly hoping to encourage any discussions which lead to improvements in how Katharsis collects and manages information about repository classes and their endpoints in future versions, or at least to try to keep this need in others' minds, as any refactoring to better support this is likely to benefit any feature which relies on annotations applied to Repository classes and/or methods.

cjc343 avatar Feb 06 '17 18:02 cjc343

actually it is quite easy, DocumentFilter and RepositoryFilter allow to intercept and check any incoming request. But as a side note, it is not sufficient to just check the queried/updated resources, also the query parameters need to be checked to make sure no unauthorized accesses to relations happen.

BTW: it should also be possible to use JAX-RS annotations if the katharsis repositories carry e.g. a JAX-RS path annotation.

remmeier avatar Feb 06 '17 19:02 remmeier

We also want this feature, we have implement it in our local repository rep with aspectj. One issue is using template design pattern which means most of findOne and findMany may existing in AbstractRepository. AbstractRepository will hand most of duplicate works. It's hard to find good place to add RolesAllowed for get method. Temp solution: Add empty method in Repository then add RolesAllowed :-(

zlhades3 avatar Feb 10 '17 12:02 zlhades3