Everything else besides the proposed Logical parameter is committed for review. Also, I noticed there's some inconsistency in the Subject interface, specifically there's these two operations: void checkPermissions(Collection<Permission> permissions) throws AuthorizationException; void checkPermissions(String... permissions) throws AuthorizationException;
but only: void checkRoles(Collection<String> roleIdentifiers) throws AuthorizationException; For completeness, I think we should add: void checkRoles(String... roleIdentifiers) throws AuthorizationException; I'd rather deprecate both checkXXX(String) and checkXXXs(Collection) and leave only checkXXXs(String...) - I don't see value for the additional methods but not sure if cleaning that up is worth the effort. Kalle On Mon, Jul 26, 2010 at 4:32 PM, Kalle Korhonen <[email protected]> wrote: > I'm rethinking my position on implementing @RequiresAnyRoles and > @RequiresAnyPermissions for SHIRO-175 since there's a fair bit of > logic that would need to be duplicated. The AnnotationHandler > hierarchy pretty much assumes that a specific type of handler deals > with only one type of annotations. The simplest way to include OR > logic would be to add a parameter to the annotations to indicate the > logical type that should be used (e.g. Logical.AND or Logical.OR), AND > probably being the default since that's how it's currently interpreted > (though I think OR as a default might be more useful but then again, > AND is the more secure default assumption). The drawback is that if > you want OR logic, you'd always have to explicitly name the value > parameter as well (e.g @RequiresRoles(value={"admin", "user"}, > logic=Logical.OR). Is that a reasonable price to pay? > > One other thing is that currently the value parameter is defined as > plain String rather a string array, forcing us to parse the String > ourselves. I think that's just a mistake and I'll fix that as part of > the issue. > > I already implemented extending all annotations to allow > @Target(ElementType.TYPE) (but haven't committed) with accompanied > tests. > > Kalle > > > On Mon, Jun 28, 2010 at 2:37 PM, Les Hazlewood <[email protected]> wrote: >> Sounds good. Also, if necessary, I'm good with getting 1.1 release out as >> quickly as would be prudent. No need to delay unless we have a complex >> feature we want to add. >> >> Les >> >> On Mon, Jun 28, 2010 at 12:55 PM, Kalle Korhonen <[email protected] >>> wrote: >> >>> On Mon, Jun 28, 2010 at 11:21 AM, Les Hazlewood <[email protected]> >>> wrote: >>> >> > then downgrade to 1.0.0, then their software could break because that >>> >> > behavior won't be enabled, right? >>> >> After downgrading, it'd show up as a compiler error - which makes all >>> >> the sense to me. >>> > This means it's not backwards compatibile according to the APR guidelines >>> > (that guarantee both binary and source compatibility across patch >>> versions): >>> >>> Sure, let's put it all in for 1.1.0 only. >>> >>> Kalle >>> >> >
