On Fri, Jun 25, 2010 at 2:18 PM, Les Hazlewood <lhazlew...@apache.org> wrote: > My initial concern is that 1.0.1 should be backwards and forwards compatible > w/ 1.0.0 and 1.0.3 respectively per traditional APR compatibility
Yes, for 1.0.1 I only suggested adding ElementType.Type for @Target, that should be perfectly backwards compatible right? 1.0.0 only allows ElementType.Method and so this is just an addition to an existing feature, not a change. > As to the Annotations, Jeremy and I have talked a number of times (long ago) > about adding expression support to annotations using a proper grammar. > Something like: > @SecurityCheck("perm(user:read:{0}.id) && (role[admin] || role[developer])") > public void someSecureMethod(User user) { ... } > where {0} represents the first method argument, {1} the second, etc. I have to say the syntax looks super ugly :P. Yes, I recall reading some of these discussions and I'm keen to have it but at the same time, I'd add this very conservatively to the Shiro core through existing use cases. It's easy to extend Shiro and it might be better to see first what users come up with. > As for the existing annotations, I believe they're both supposed to be > aggregating - i.e. 'ALL' for both of them. My personal preference is that > we keep the existing annotations and have retain 'ALL' semantics. > �...@requiresroles(new String[] { "dev", "teamLead"}) to me definitely reads > as > I should have both the 'dev' and 'teamLead' roles in order for the method to > execute. �...@requiresanyroles (again, to me) sounds like a good alternative > for 'OR' semantics. Agree. Javadoc for @RequiresRoles says "Requires the currently executing Subject to have one or more specified roles" so at least I could read it as "any of these roles". Maybe best to rewrite it as "Requires the currently executing Subject to have all of the specified roles" and be done with it. We can then keep the existing annotations for ALL semantics and introduce @RequiresAnyXXX in 1.1. Kalle > On Fri, Jun 25, 2010 at 1:24 PM, Kalle Korhonen > <kalle.o.korho...@gmail.com>wrote: > >> Related to https://issues.apache.org/jira/browse/SHIRO-175 and some of >> the Shiro extensions we (Tynamo.org) have made, I'd like to change >> @Target(ElementType.METHOD) of all annotations to >> @Target({ElementType.TYPE, ElementType.METHOD}). The obvious semantics >> of attaching a security annotation to a class is that the same >> permission would apply to all method invocations in that class. >> >> SHIRO-175 suggests that we'd deprecate the existing annotations in >> favor of RequiresAnyXXX and RequiresAllXXX annotations, forcing to >> target 1.1 release for the issue resolution. We could also keep the >> existing annotations and pick either ANY or ALL semantics in order to >> avoid deprecating anything - not sure which one is better. The current >> semantics are unclear, the Javadoc implies ANY for RequiresRoles but >> ALL for RequiresPermissions. >> >> Here's what I'd suggest: >> - 1.0.1 change @Target to apply to TYPEs as well, fix ambiguity of >> current annotations (especially if we decide to keep them) >> - 1.1.0 resolve SHIRO-175 and add RequiresAnyXXX and RequiresAllXXX >> annotations (and deprecate existing annotations if we choose to do so) >> >> I can handle implementing the changes. Any comments? >> >> Kalle >> >