> These all make sense to me. Class level annotations should only be
> overwritten by method annotations of the same type IMO. If a method
> has a different annotation type than the class one, it should be
> compounded ('and'ed) IMO.
The further I look into this, the more mired I become. The latest
issue is with AnnotationsAuthorizingMethodInterceptor, which iterates
through all the available method interceptors and checks whether the
target method has the corresponding annotations. This works fine until
you take into account class annotations and the feature in which
method annotations can only override class annotations of the same
type (authentication or authorization).
The desired behaviour can be implemented in a couple of ways. First,
AnnotationsAuthorizingMethodInterceptor can be made aware of the
semantics of the different @Requires* annotations and deal with them
internally. I think this requires the fewest changes, but it won't
work if the plan is to allow users to add their own annotations.
The second option is to break the annotation handlers into two types:
authentication and authorization. The annotation handlers themselves
will have to include the information about which group they belong in,
for example via an enum property or by implementing a particular
interface.
It's also not clear what should happen when multiple annotations of
the same group are attached to the same method (or class). For
example, what if both @RequiresRoles and @RequiresPermissions are
declared on a method? What about @RequiresUser and
@RequiresAuthentication? I think we should log a warning in the latter
case and only act on the first one found. But for the former, I'm not
sure. Should we 'or' them together? 'and' them? Or log a warning as
with the authentication annotations?
BTW, I've made good progress and I've implemented a bunch of
functional tests for the plugin, which also test Shiro's behaviour.
Just in case anyone else was thinking of tackling the issue.
Cheers,
Peter
