You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Igor Vaynberg (Resolved) (JIRA)" <ji...@apache.org> on 2011/11/23 07:47:40 UTC

[jira] [Resolved] (WICKET-4256) onBeforeRender() is called on components that are not allowed to render

     [ https://issues.apache.org/jira/browse/WICKET-4256?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Igor Vaynberg resolved WICKET-4256.
-----------------------------------

    Resolution: Fixed
    
> onBeforeRender() is called on components that are not allowed to render
> -----------------------------------------------------------------------
>
>                 Key: WICKET-4256
>                 URL: https://issues.apache.org/jira/browse/WICKET-4256
>             Project: Wicket
>          Issue Type: Bug
>    Affects Versions: 1.5.3
>            Reporter: Igor Vaynberg
>            Assignee: Igor Vaynberg
>             Fix For: 1.5.4, 6.0.0
>
>
> pasted from the list:
> Hi,
> I ran into an odd problem this week. A model fed to a ListView was
> calling service methods the current user wasn't allowed to use, and I
> was wondering how that could happen. A panel far above this ListView in
> the hierarchy had been secured (using Shiro annotations, but that turns
> out to not matter at all) and was not supposed to be rendered for this
> user. From this I had expected the ListView not to be rendered either,
> but here it was trying to assemble itself in onBeforeRender and thus
> calling the "forbidden" service methods.
> I investigated Component and friends for a bit, and have found a
> potential problem.
> internalBeforeRender() checks determineVisibility() before doing
> anything. So far so good. determineVisibility() then checks
> isRenderAllowed() so the application's IAuthorizationStrategy can block
> certain components. This is where it goes wrong though:
> isRenderAllowed() only looks at FLAG_IS_RENDER_ALLOWED for performance
> reasons, and that flag hasn't been set yet! internalPrepareForRender()
> only calls setRenderAllowed() *after* beforeRender().
> Due to this, the supposedly secure panel was going through its own
> beforeRender and thus calling that ListView's beforeRender.
> I think this can be a serious problem, such as in my case described
> above. I'd expect that if isActionAuthorized(RENDER) is false, the
> secured component should basically never get to the beforeRender phase.
> My questions are now:
> - Is this intentional? If yes, please explain the reasoning behind it,
>  because it isn't obvious to me.
> - If not, can we fix it? My intuitive suggestion would be to simply
>  move the call to setRenderAllowed() from the end of
>  internalBeforeRender() (prepareForRender in 1.4) to the beginning of
>  that method, so beforeRender() can reliably look at that flag.
> - If we can fix it, when and where do we fix it? This hit me in 1.4,
>  and looking at the code it's still there in 1.5. I'd *really* like it
>  fixed in the last 1.4 release, and certainly in 1.5, given that this
>  has the potential to impact security.
>  It's not an API break, but I'm not sure whether the implications for
>  application behavior are tolerable for all existing applications. On
>  the other hand, it seems to be a real security problem, so maybe the
>  change is justified. I'd like some core dev opinions please :-)
> If this is in fact a bug, I'm of course willing to provide a ticket and
> a patch :-)
> Thanks!
> Carl-Eric
> www.wicketbuch.de

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira