isEnabled can do that..
Because then this code is screwed up:
final Component component = page.get(pageRelativeComponentPath);
if (component == null || !component.isEnabled() ||
!component.isVisibleInHierarchy())
{
log
.info("component not enabled or visible, redirecting
to calling page, component: "
+ component);
return new RedirectPageRequestTarget(page);
}
if (!component.isEnableAllowed())
{
throw new UnauthorizedActionException(component,
Component.ENABLE);
}
See first the isEnabled and then the isEnabledAllowed() both doing 2
completely different actions when happening?
johan
On 10/8/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
Well, final isEnabled would be cleanest solution, though users could
have hard time adapting to it, as well as we might need to do the same
for isVisible, to be consistent.
isReallyEnabled sounds really bad :)
I can't think of a good name, maybe isEnableAllowed could do it,
currently it just clutters api imho, but I'm not very sure with it.
-Matej
Igor Vaynberg wrote:
> as i said, i would rather that little final method be called isEnabled()
> and
> have a new hook for the user - cant come up with a good name.
>
> i mean what do you propose for this helper? isReallyEnabled() { return
> isEnabled()&&isAllowedEnabled(); } ???
>
> -Igor
>
>
> On 10/8/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
>>
>> 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
>> >> >>
>> >> >
>> >>
>> >>
>> >
>>
>>
>
