Authentication is checked after Multipart fileupload stream is consumed by Jetty

[a-sayyed]

Currently when you make a POST request for a file upload on a secure endpoint, the Underlying Jetty Server consumes the whole file first then passes the request to Spring security. This means that security is checked after the file is already uploaded.

To replicate this issue, you can clone the demo repository I have created and simply upload a file to the endpoint /upload with no authentication, you can see in the logs that the inputstream from the file is getting consumed by Jetty before the request is checked for Authentication.

The spring.servlet.multipart.resolve-lazily property is also not respected.

For testing, (on a linux machine), you can create a random file with the command head -c 256MB /dev/urandom > randomFile.txt
If you would like to try the request out with authentication, the credentials are admin:admin

[wilkinsona]

Thanks for the sample. The behaviour that you have observed is due to the auto-configured HiddenHttpMethodFilter. It calls javax.servlet.ServletRequest.getParameter(String) which causes Jetty to parse the request to see what parameters are contained in the body. HiddenHttpMethodFilter has to be ordered before Spring Security's filter as Spring Security allows different security rules to be configured for different HTTP methods.

The hidden HTTP method filter can be disabled by setting spring.mvc.hiddenmethod.filter.enabled=false. However, this does not completely avoid the problem. The request now reaches Spring Security and it throws an AccessDeniedException. It's handled in ExceptionTranslationFilter.handleSpringSecurityException(HttpServletRequest, HttpServletResponse, FilterChain, RuntimeException) which results in the creation of a DefaultSavedRequest. This calls javax.servlet.ServletRequest.getParameterMap() which causes the multipart request to be consumed and parsed.

Please open a Spring Security issue for the second part of the problem. Hopefully it's possible for the AccessDeniedException to be handled and the 401 response sent back without the parsing of the multipart request. /cc @rwinch

I'll leave this issue open so that we can consider what to do about HTTP method filters role in the problem. We can certainly update the documentation to highlight the effect that it is. I wonder if we should also consider no longer enabling it by default.

[vpavic]

This is somewhat related to #16953 in sense that both issues describe problematic behavior caused by HiddenHttpMethodFilter in combination with multipart request.

I wonder if we should also consider no longer enabling it by default.

I also believe that HiddenHttpMethodFilter shouldn't be enabled by default, because even if you do server side generated HTML it doesn't mean you're going to need that filter.

[wilkinsona]

We've decided to disable HttpMethodFilter by default. Thanks for any extra data point that helped us to make the decision, @a-sayyed. I'll close this issue in favour of #16953.

[a-sayyed]

@wilkinsona @vpavic Thank you for your help trying to track down this issue. I have created the Spring Security issue as suggested.