Well, it's not a big deal to check if for call to isEnabled() whether
isEnableAllowed is checked too.
but if we have isEnableAllowed(), which basically does nothing else then
calls isActionAllowed(ENABLE), then maybe we could have a nice small
final method that would botm check isEnableAllowed() and isEnabled().
WDYT?
-Matje
Igor Vaynberg wrote:
we shouls always be checking both.
if we dont its a bug, its just so easy to forget to check for
isAllowedEnabled :)
-Igor
On 10/8/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
Well, the thing is that we don't check isEnableAllowed on many places
either. Take link as an example. Even if component is not allowed to be
enabled, it's still not rendered as disabled.
The question is: Should it be? I think so, but maybe there is a good
reason why it ain't. Anyone?
-Matej
Igor Vaynberg wrote:
> there was a long discussion on this. i first proposed what you propose,
but
> later i changed my mind, but still am on the fence :)
>
> the thing here is that isenabled() and isallowenabled() are orthogonal
in
> the sense that one is meant to be implemented by the user and the other
by
> the security policy.
>
> so if our isenabled() checks for isallowenabled() then anytime the user
> overrides it they have to remember to check for isallowenabled() - and
if
> they forget they break security.
>
> on the other hand if the user is implementing a component that needs to
> properly handle the disabled state they have to do the check
> isEnabled()&&isAllowedEnabled() which also has a potential for breaking
> security - but as a component _implementor_ they should be a bit more
> aware.
>
> the first case happens a lot more often so we cater for that one.
>
> the perfect situation would be to have another hook for the user to
> implement, and have isenabled() as final that performs both checks. the
> problem is coming up with a good name for that new method.
>
> -Igor
>
>
> On 10/8/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
>>
>> We have two methods that determine whether component should be
allowed.
>> Do we always check both?
>>
>> I mean, look at Link#onComponentTag, we only check
>> isEnabled. So even though enable is not allowed, we still render link
as
>> enabled. Is this right?
>>
>> Shouldn't e.g. isEnabled() return false if the ENABLE action is not
>> permitted?
>>
>> -Matej
>>
>