ShiroUnauthorizedComponentListener ignores loginPage() and unauthorizedPage arg

[tkruse]

In ShiroUnauthorizedComponentListener.onUnauthorizedInstantiation()

public void onUnauthorizedInstantiation(final Component component)
    {
        final Subject subject = SecurityUtils.getSubject();
        final boolean notLoggedIn = !subject.isAuthenticated();
        final Class<? extends Page> page = notLoggedIn ? loginPage : unauthorizedPage;

        if (annotationStrategy != null)
        {
            final ShiroSecurityConstraint fail = annotationStrategy.checkInvalidInstantiation(component.getClass());
            if (fail != null)
                if (notLoggedIn)
                    // !!!! ...GetPage called, but not assigned to Page
                    addLoginMessagesAndGetPage(fail, component, page);
                else
                    // !!!! ...GetPage called, but not assigned to Page
                    addUnauthorizedMessagesAndGetPage(fail, component, page);
        }

        if (notLoggedIn)
            // the login page
            throw new RestartResponseAtInterceptPageException(page);
        // the unauthorized page
        throw new RestartResponseException(page);
    }

See above in added code comment, "// !!!! ...GetPage called, but not assigned to Page" should probably be

page = ...

so that later restart goes to page specified in annotation.

Example annotation with values:

@ShiroSecurityConstraints({
        @ShiroSecurityConstraint(action = ShiroAction.INSTANTIATE, constraint = ShiroConstraint.HasRole, value = "foo", unauthorizedPage = FooUnauthPage.class),
        @ShiroSecurityConstraint(action = ShiroAction.INSTANTIATE, constraint = ShiroConstraint.IsAuthenticated, loginPage = BarUnauthPage.class)})
public class FooPage extends WebPage {

[tkruse]

Also, while you're at it, ShiroUnauthorizedComponentListener is a pain to subclass and test, because there are no getters for the fields, nor an easy way to check what class was set for the RestartExceptions. Could be improved in several ways.

[martin-g]

I'm afraid no one maintains the module at the moment. Pull requests are welcome!

[jsarman]

Have you tried setting the FORWARD dispatcher in web.xml. This is required when I use the servlet security container and I would like to use a wicket generated login page. I learned about needing the FORWARD through reading through some Shiro docs, and it was the answer to my issue. Its at least worth a try. Below is the filter-mapping xml snippet you need to test it.

YourWicketApp /* REQUEST FORWARD

John

On Thu, Mar 26, 2015 at 7:16 AM, Thibault Kruse [email protected] wrote:

In ShiroUnauthorizedComponentListener.onUnauthorizedInstantiation()

public void onUnauthorizedInstantiation(final Component component) { final Subject subject = SecurityUtils.getSubject(); final boolean notLoggedIn = !subject.isAuthenticated(); final Class<? extends Page> page = notLoggedIn ? loginPage : unauthorizedPage;

    if (annotationStrategy != null)
    {
        final ShiroSecurityConstraint fail =

annotationStrategy.checkInvalidInstantiation(component.getClass()); if (fail != null) if (notLoggedIn) // !!!! ...GetPage called, but not assigned to Page addLoginMessagesAndGetPage(fail, component, page); else // !!!! ...GetPage called, but not assigned to Page addUnauthorizedMessagesAndGetPage(fail, component, page); }

    if (notLoggedIn)
        // the login page
        throw new RestartResponseAtInterceptPageException(page);
    // the unauthorized page
    throw new RestartResponseException(page);
}

See above in added code comment, "// !!!! ...GetPage called, but not assigned to Page" should probably be

page = ...

so that later restart goes to page specified in annotation.

Example annotation with values:

@ShiroSecurityConstraints({ @ShiroSecurityConstraint(action = ShiroAction.INSTANTIATE, constraint = ShiroConstraint.HasRole, value = "foo", unauthorizedPage = FooUnauthPage.class), @ShiroSecurityConstraint(action = ShiroAction.INSTANTIATE, constraint = ShiroConstraint.IsAuthenticated, loginPage = BarUnauthPage.class)}) public class FooPage extends WebPage {

— Reply to this email directly or view it on GitHub.

[tkruse]

Hi John, thanks for the snippet, but I don't think it relates to my issue.