shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kalle Korhonen <kalle.o.korho...@gmail.com>
Subject Re: Proposed changes to Shiro annotations
Date Mon, 26 Jul 2010 23:32:20 GMT
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 <lhazlewood@apache.org> 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 <kalle.o.korhonen@gmail.com
>> wrote:
>
>> On Mon, Jun 28, 2010 at 11:21 AM, Les Hazlewood <lhazlewood@apache.org>
>> 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
>>
>

Mime
View raw message