Ah, I just saw your example where @RequiresAuthentication is at the class-level and @RequiresUser is at the method level. The method-level one is loosening the requirements on authentication state a bit and should override the class-level. Now I understand the concept of breaking the annotation handlers into two types.
Not sure how to solve this cleanly though. Maybe using Comparable to indicate which one 'wins'? Any other ideas? On Thu, Mar 18, 2010 at 5:43 PM, Les Hazlewood <[email protected]> wrote: > On Sat, Feb 27, 2010 at 6:28 AM, Peter Ledbrook <[email protected]> wrote: >>> 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... >> >> The second option is to break the annotation handlers into two types: >> authentication and authorization... > > I'm not sure I follow - I think the overriding behavior can be > accomplished without modifying the > AnnotationsAuthorizingMethodInterceptor at all (maybe). > > My idea is that each of the iterated interceptors (which in turn just > delegate to an AnnotationHandler) can still just perform their > authorization assertion. The handler code will need to be modified to > look at the class level as well, but that shouldn't be too hard since > there is already an abstract parent class where this can be > consolidated. > > Each handler doesn't need to 'know' about annotations of other types - > it checks the class-level, and a possibly overriding method-level, and > if it finds either, it uses the most specific one to perform the > authorization assertion. > > Doing things this way means 2 things though: > > 1) Assertions will be compounded ('and' and not 'or') since handler > iteration will always occur and will continue until an assertion > failure is encountered. > > 2) In the event that two annotations are declared that are exact > opposites of each other (e.g. @RequiresUser and @RequiresGuest), then > the method will never execute - either of the two will always fail. > This would be considered developer error. > > Granted, it'd be nice to alert the developer some way if they do this > accidentally, but don't you want something like this to fail in a > security framework? It could be dangerous to let one of the two 'win' > and let the invocation continue - it might have undesired security > implications. > > By adding some class-then-method logic in the > AuthorizingAnnotationHandler, I think this issue would be solved, as > long as we're ok with the 2 caveats above. > > Thoughts? > > Les >
