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

Provide a way to customize the default RequestCache without replacing the entire implementation #15707

Open
eric-creekside opened this issue Aug 28, 2024 · 5 comments
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement

Comments

@eric-creekside
Copy link

eric-creekside commented Aug 28, 2024

Spring Security 6.2.5
Without any customization, the default RequestCache is HttpSessionRequestCache (created by private methods in RequestCacheConfigurer). For some situations, it would be necessary to customize that cache. An example I have run into is needing to extend the cache's RequestMatcher to exclude certain requests from being cached (see this SO question for specifics).
As far as I can see, the only customization option for a typical SecurityFilterChain bean is to completely replace the RequestCache object via RequestCacheConfigurer<HttpSecurity>.requestCache(). That's far from ideal as many applications will want the default configured cache with only minor changes. Since the configurer's methods that create or use the default are all private, that's not currently possible.
It would be useful for RequestCacheConfigurer to expose a way to get to the default cache, so the application can customize it. Even if it required a cast, that would likely be a "lesser of 2 evils" choice for many developers.

@eric-creekside eric-creekside added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Aug 28, 2024
@marcusdacoregio marcusdacoregio added in: config An issue in spring-security-config and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 29, 2024
@marcusdacoregio marcusdacoregio self-assigned this Aug 29, 2024
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Aug 29, 2024

Hi @eric-creekside, thanks for the report.

I'd like to clarify what you are asking to see what can be done to help:

Do you want an easier way to provide your own RequestMatcher to HttpSessionRequestCache without having to do:

HttpSessionRequestCache myCache = new HttpSessionRequestCache();
myCache.setRequestMatcher(myRequestMatcher);
http.requestCache(cache -> cache.requestCache(myCache))

Or do you want to customize the RequestMatcher created by createDefaultSavedRequestMatcher?

It would be useful for RequestCacheConfigurer to expose a way to get to the default cache, so the application can customize it. Even if it required a cast, that would likely be a "lesser of 2 evils" choice for many developers.

I don't think this is something that we usually do, I don't remember any other configurer that does that. And, as you mentioned, it would require cast and other stuff that won't make it easier to configure.

With that said, I think it would be fine to add a requestMatcher(RequestMatcher) method to RequestCacheConfigurer.

@eric-creekside
Copy link
Author

eric-creekside commented Aug 29, 2024

In the end, what I'm hoping for is a way to extend the default RequestCache without having to duplicate the somewhat extensive code in createDefaultSavedRequestMatcher(). Most of what's in that default is sensible and useful; it just needs to be extended. The only way I found was to use configurer.requestCache(myOwnCache) and copy the implementation of RequestCacheConfigurer.createDefaultSavedRequestMatcher().
Basically the difference between extend vs. replace is what I'm talking about. Only being able to replace the RequestMatcher isn't very useful since It only saves 1 or 2 lines of code. However, being able to get the default matcher and layering my own rule on top of it would be useful.

Here's what I had to do to just to add my own API pattern to be excluded:

	private RequestMatcher createSavedRequestMatcher(HttpSecurity http) {
		List<RequestMatcher> matchers = new ArrayList<>();

		RequestMatcher notFavIcon = new NegatedRequestMatcher(new AntPathRequestMatcher("/**/favicon.*"));
		RequestMatcher notXRequestedWith = new NegatedRequestMatcher(
				new RequestHeaderRequestMatcher("X-Requested-With", "XMLHttpRequest"));

		@SuppressWarnings("unchecked")
		boolean isCsrfEnabled = http.getConfigurer(CsrfConfigurer.class) != null;

		if (isCsrfEnabled) {
			RequestMatcher getRequests = new AntPathRequestMatcher("/**", "GET");
			matchers.add(0, getRequests);
		}

		matchers.add(notFavIcon);
		matchers.add(notMatchingMediaType(http, MediaType.APPLICATION_JSON));
		matchers.add(notXRequestedWith);
		matchers.add(notMatchingMediaType(http, MediaType.MULTIPART_FORM_DATA));
		matchers.add(notMatchingMediaType(http, MediaType.TEXT_EVENT_STREAM));

		// Exclude our API endpoints from the matcher
		RequestMatcher notAPI = new NegatedRequestMatcher(new AntPathRequestMatcher("/**/api/**", "GET"));
		matchers.add(notAPI);

		return new AndRequestMatcher(matchers);
	}

Everything except the 2 lines below the //Exclude our API... is duplicated from the private RequestCacheConfigurer.createDefaultSavedRequestMatcher() method. That feels like a lot of code just to add one more "not matching" pattern.

@marcusdacoregio
Copy link
Contributor

Hi @eric-creekside, I brought this to the team's attention and I'll list here some thoughts.

Allowing the customization of the default request matcher is not something that we want to do. The DSL is limited to the configurations that cover the majority of use cases. Usually, customizations like this are achieved by creating a bean and injecting it into the DSL.

We could introduce a static factory method that could be reused, or, we could post-process the default RequestMatcher.
However, since both the factory and post-processor will receive the fully configured request matcher, that would be somewhat limited: you would only be able to add RequestMatchers before or after the existing matcher. Reordering the default matchers would not be possible as well.

Given that the solutions thought so far won't be flexible to cover "all cases", I'd like to propose that we keep this ticket open and see the community's interest in this (by gathering upvotes). Let me know what you think.

@eric-creekside
Copy link
Author

eric-creekside commented Sep 11, 2024

I'm skeptical the team will want to, but I think simply making RequestCacheConfigurer.createDefaultSavedRequestMatcher() accessible (eg, make that method public) would help. As you said, I could not re-order or change the existing default matchers but that's not what I'm asking for. If I want that level of customization, that's a different usage pattern, more like replacing instead of extending. Again, if I want to replace the matcher I already can do that.
Extending it would be possible if I could just get my hands on it.

Also, I'm curious about this statement you made:

Usually, customizations like this are achieved by creating a bean and injecting it into the DSL.

Can you clarify (maybe with an example) what you mean?

@marcusdacoregio marcusdacoregio removed their assignment Sep 17, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Sep 24, 2024

I think that a potential solution for you is to create a delegator like so:

public class MyRequestCache implements RequestCache {
    private final RequestCache delegate;

    public MyRequestCache(RequestCache delegate) {
        this.delegate = delegate;
    }

    public void saveRequest(HttpServletRequest request, HttpServletResponse response) {
        if (myCustomRequestMatchingPasses) {
            this.delegate.saveRequest(request, response);
        }
    }

    public SavedRequest getRequest(HttpServletRequest request, HttpServletResponse response) {
        return this.delegate.getRequest(request, response);
    }

    public HttpServletRequest getMatchingRequest(HttpServletRequest request, HttpServletResponse response) {
        return this.delegate.getMatchingRequest(request, response);
    }

    public void removeRequest(HttpServletRequest request, HttpServletResponse response) {
        this.delegate.removeRequest(request, response);
    }
]

And then wrap the configured request cache using an object post processor:

http
    // ...
    .requestCache((cache) -> cache
        .addObjectPostProcessor(new ObjectPostProcessor<RequestCache>() {
            @Override
            RequestCache postProcess(RequestCache delegate) {
                return MyRequestCache(delegate);
            }
        });
    )
    // ...

This will allow for your custom request matching criteria to be executed and then the Spring Security internal one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants