spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.71k stars 5.86k forks source link

JspAuthorizeTag cannot be used if spring-security configurations (beans) is not put in ROOT context (XML config). #8843

Open bboyz269 opened 4 years ago

bboyz269 commented 4 years ago

Describe the bug I have a empty root context and an app (child) context contains all my beans. I have a separated xml file for spring security config, imported in child context. Received the following error when using JSPAuthorizeTag <sec:authorize>

javax.servlet.ServletException: javax.servlet.jsp.JspException: java.io.IOException: No visible WebSecurityExpressionHandler instance could be found in the application context. There must be at least one in order to support expressions in JSP 'authorize' tags.
    org.apache.jasper.runtime.PageContextImpl.doHandlePageException(PageContextImpl.java:907)
    org.apache.jasper.runtime.PageContextImpl.handlePageException(PageContextImpl.java:840)

To Reproduce As above Create an app (Spring mvc) with empty root context and child context, contains spring-security configurations. Error happens if you try to use <sec:authorize> tag in JSP.

Expected behavior If it's not by design that spring security configs must be in root context. I expect to use jsp authorize tag normally.

Sample I don't have a sample ready, but got a point to suspected piece of code that cause the problem (again, if it's not by design). org.springframework.security.taglibs.authz.AbstractAuthorizeTag#getExpressionHandler() calling org.springframework.security.web.context.support.SecurityWebApplicationContextUtils#_findWebApplicationContext()

The logic was "Look for a bean of type SecurityExpressionHandler in root context. If there's no root context, look for the bean in any child context". While I think the correct one should be "Look for a bean of type in preferred context first, then root context then, the rest" Similar to how DelegatingFilterProxy of spring-web works ...

If I could have confirmation that it's not required spring security to be in root context, I could make a pull request with suggested fix.

rwinch commented 4 years ago

Similar to how DelegatingFilterProxy of spring-web works

As far as I can tell DelegatingFilterProxy and JspAuthorizeTag are consistent in how they look up the beans. We should be consistent to ensure that the same ApplicationContext that is used to create the FilterChainProxy is used by JspAuthorizeTag.

DelegatingFilterProxy calls WebApplicationContextUtils.findWebApplicationContext to get the ApplicationContext to use.

JspAuthorizeTags super class AbstractAuthorizeTag calls SecurityWebApplicationContextUtils.findRequiredWebApplicationContext which has the same logic as WebApplicationContextUtils.

It sounds as though you have noticed a difference? If so, can you please help me to see the difference?

bboyz269 commented 4 years ago

@rwinch

The logic was "Look for a bean of type SecurityExpressionHandler in root context. If there's no root context, look for the bean in any child context". While I think the correct one should be "Look for a bean of type in preferred context first, then root context then, the rest" Similar to how DelegatingFilterProxy of spring-web works ...

I already mentioned in my post above but let me rephrase it.

How DelegateFilterProxy (the correct way) look up beans is "find the beans in root context, if no beans found, proceeed to find in child contexts". While JspAuthorizeTag "find beans in the root context, if no root context found, proceed to find in child context".

I mark the difference in bold this time. Also, DelegateFilterProxy allows you to specify child context as well.

rwinch commented 4 years ago

The way you describe it doesn't look like the way it is coded. The links I provided make it appear that they behavior is consistent between the two. Can you provide a complete/minimal sample that demonstrates the difference in behavior you are seeing or explain why my analysis is incorrect?

bboyz269 commented 4 years ago

@rwinch Sorry I made this more confusing than it should be.

You're correct that SecurityWebApplicationContextUtils has the same logic with WebApplicationContextUtils. What I meant to say is that DelegatingFilterProxy#findWebApplicationContext allow us to specify which context to looks for beans with #getContextAttribute(). While AbstractAuthorizeTag only looks for bean in root context.

I created a test here for demonstration. Please check it out.

rwinch commented 4 years ago

@bboyz269 Thank you clarifying the issue. I think the best approach would be to allow users to specify the WebSecurityExpressionHandler on an attribute similar to how WebInvocationPrivilegeEvaluator is looked up. This decouples WebSecurityExpressionHandler from the ApplicationContext entirely.

Would you be interested in submitting a pull request?

bboyz269 commented 4 years ago

Would you be interested in submitting a pull request?

Surely!

bboyz269 commented 4 years ago

@rwinch I'm looking for an example but couldn't find request attribute WebAttributes.WEB_INVOCATION_PRIVILEGE_EVALUATOR_ATTRIBUTE set anywhere within this project.
Is it supposed to be an extension point for user to provide their own beans?

WebInvocationPrivilegeEvaluator privEvaluatorFromRequest = (WebInvocationPrivilegeEvaluator) getRequest()
                .getAttribute(WebAttributes.WEB_INVOCATION_PRIVILEGE_EVALUATOR_ATTRIBUTE);