Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HiddenHttpMethodFilter configuration in the hello-mvc-security example project? #167

Open
mklinkj opened this issue Dec 2, 2023 · 1 comment

Comments

@mklinkj
Copy link

mklinkj commented Dec 2, 2023

Inquiry about the HiddenHttpMethodFilter configuration in the hello-mvc-security example project

Hello,

In the hello-mvc-security example project, to use the HiddenHttpMethodFilter correctly, wouldn’t it be better to place this filter before the Spring Security Filter?

public class SecurityWebApplicationInitializer extends AbstractSecurityWebApplicationInitializer {
  ...
  // ✨  I am inquiring whether additional configuration is necessary?”
  @Override
  protected void beforeSpringSecurityFilterChain(ServletContext servletContext) {
    FilterRegistration.Dynamic encodingFilter =
        servletContext.addFilter("hiddenHttpMethodFilter", new HiddenHttpMethodFilter());
    encodingFilter.addMappingForUrlPatterns(
        null, false, "/*");
  }
}

If the HiddenHttpMethodFilter is not placed before the security filter, there have been times when setting up requestMatchers() resulted in unintended behavior.

For example, after setting the hidden input below in the form

...
<input type="hidden" name="_method" value="DELETE"/>
...

If the requestMatchers() configuration is set up as follows…

...
.requestMatchers(HttpMethod.DELETE, "/targetUrl/*")
.hasAuthority("ADMIN")
...

There were times when it didn't work as I intended.
When I requested deletion with "USER" permission, it was deleted.

Even so, since there are no web pages in the hello-mvc-security example project that utilize the HiddenHttpMethodFilter, it seems that it might be okay to remove the HiddenHttpMethodFilter.

Inquiry Summary

  1. Wouldn’t it be better to remove the HiddenHttpMethodFilter configuration from the security settings of the hello-mvc-security example project?
  2. If not removed, shouldn’t the HiddenHttpMethodFilter be positioned before the SpringSecurityFilterChain?

Thank you. Have a great day. 👍

mklinkj added a commit to mklinkj/spring-security-samples that referenced this issue Dec 5, 2023
I have modified the hello-mvc-security example.

Remove Unused HiddenHttpMethodFilter
1. Remove Unused HiddenHttpMethodFilter
2. Gretty 4.x Requirement: Gretty 4.x requires the /src/main/webapp path.
   This prevents failure when running the example project with the appRun task.

* spring-projects#167
* gretty-gradle-plugin/gretty#298

thanks.
mklinkj added a commit to mklinkj/spring-security-samples that referenced this issue Dec 6, 2023
hello.

I have modified the hello-mvc-security example.

Remove Unused HiddenHttpMethodFilter
1. Remove Unused HiddenHttpMethodFilter
2. Gretty 4.x Requirement: Gretty 4.x requires the /src/main/webapp path.
   This prevents failure when running the example project with the appRun task.

* spring-projects#167
* gretty-gradle-plugin/gretty#298

thanks.
@mklinkj
Copy link
Author

mklinkj commented Jan 15, 2024

hello.

I created an independent project to reproduce the issue.

  • Separate the hello-mvc-security project into an independent project and create an example project containing code that uses HiddenHttpMethodFilter.

  • example project zip

I will rewrite the Pull Request by deleting only the part where HiddenHttpMethodFilter is declared.

Other than that, I will not include the part where Gretty requires the existence of the src/main/webapp directory.
This problem will be resolved later when Gretty is modified.
Since the integrationTest does not fail, I thought there was no need to intentionally include defensive code in gretty.gradle.

thank you have a good day. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant