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 Tue, 27 Jul 2010 06:14:10 GMT
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
<kalle.o.korhonen@gmail.com> 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 <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