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

Should return www-authenticate even for "X-Requested-With: XMLHttpRequest" requests #16103

Open
MartinEmrich opened this issue Nov 15, 2024 · 5 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: feedback-provided Feedback has been provided type: bug A general bug

Comments

@MartinEmrich
Copy link

Describe the bug
Since migrating to Spring Security 6, Calling APIs using simple jQuery/XHR with basic auth results in final 401 errors, despite being logged in through the browser basic auth dialog.

Analyzing the responses, they are missing the mandatory WWW-Authenticate header. Thus the browser will not attempt the (already present) basic auth credentials.

IMHO this is a bug, as that header is mandatory: https://datatracker.ietf.org/doc/html/rfc9110#name-www-authenticate

To Reproduce
Implement a simple app with

	public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
		return http
				.csrf(AbstractHttpConfigurer::disable)
				.cors(cors -> cors.disable())
				.authorizeHttpRequests(
						matchers -> matchers
								.requestMatchers("/some/**", "/some/more/**", "/error/**")
								.permitAll()
								.requestMatchers("/api/**", "/app/**")
								.authenticated()
								.anyRequest().authenticated())

				.sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
				.httpBasic(basic -> basic.realmName("my-realm"))
				.build();
	}
        // a valid AuthenticationProvider, too...

Expected behavior
have a (valid) www-authenticate response header in all 401 responses.

Additional Info

The behaviour was first implemented here: 4ef0460#diff-0f2a9f7a8a020191e00efb336582d7d71dd46130bc4b5cbc86eba681c498751fR92

Current state:

entryPoints.put(X_REQUESTED_WITH, new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED));

I guess this line of code just does not explicitly put the www-authenticate header.

Workaround

My workaround is to provide a simple BasicAuthenticationEntryPoint without that special treatment for XMLHttpRequest:

		BasicAuthenticationEntryPoint basicAuthEntryPoint = new BasicAuthenticationEntryPoint();
		basicAuthEntryPoint.setRealmName(BASIC_AUTH_REALM);

and setting it in the SecurityFilterChain

				.httpBasic(basic -> basic.realmName(BASIC_AUTH_REALM).authenticationEntryPoint(basicAuthEntryPoint))
@MartinEmrich MartinEmrich added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 15, 2024
@MartinEmrich
Copy link
Author

To be precise: I do not imply it is a regression from Spring Security 5, most probaby my old WebSecurityConfigurerAdapter code did just not trigger that piece of code.

@sjohnr sjohnr self-assigned this Nov 15, 2024
@sjohnr
Copy link
Member

sjohnr commented Nov 15, 2024

@MartinEmrich thanks for reaching out!

Since migrating to Spring Security 6, Calling APIs using simple jQuery/XHR with basic auth results in final 401 errors, despite being logged in through the browser basic auth dialog.

I am unable to see a change in behavior switching between Spring Security 5 and 6. Regarding...

I do not imply it is a regression from Spring Security 5, most probaby my old WebSecurityConfigurerAdapter code did just not trigger that piece of code.

Can you please provide a minimal, reproducible sample that somehow demonstrates the change in behavior between 5 and 6? Otherwise, I'm not quite sure I'm following how this has to do with migrating?

The behaviour was first implemented here: 4ef0460

This behavior seems to go back to 2013 as pointed out by the referenced commit you provided. If necessary, we could ask for clarification from the author but I think it's fairly clear from the commit message that it was an effort to define a clear set of defaults for Javascript clients. It's also clear that the behavior to not provide a WWW-Authenticate header by default when using XHR is intentional. Further, you can customize this behavior easily as demonstrated in your workaround.

IMHO this is a bug, as that header is mandatory: https://datatracker.ietf.org/doc/html/rfc9110#name-www-authenticate

I can see how this default does not align with your preference, and I also understand there could be arguments made for changing the behavior to simply align with the spec in all cases. The defaults provided by Spring Security are quite generic and are intended as "one size fits most" on a best-effort basis. However, Spring Security is designed to be customized when the defaults do not suit preference or requirements. I feel that customizing the behavior is realistic for your use case given the length of time this default has been in place in the framework.

I plan to close this issue with the above discussion points but I will wait for your response to see if I have missed something in the consideration since I was not able to reproduce the change in behavior you described.

@sjohnr sjohnr added status: waiting-for-feedback We need additional information before we can continue in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 15, 2024
@MartinEmrich
Copy link
Author

Hello @sjohnr

As stated, I do not claim that the behaviour has changed between Spring Security 5 or 6. So I cannot produce a test case showing such a difference. I just presented my train of thought leading to my discovery. If I mislead you, I apologizes.

I rather argue that the behaviour was violating an RFC all along.

I understand that the intent of this code is to prevent any human-interaction responses to be sent to Javascript-initiated requests, which clearly makes sense (like an overly long HTML error page). And on that point I agree, sending a 401 without a body is very efficient.

Thus I might even suggest that the missing www-authenticate header was not intentional for the original author; it just so happened, as that short-circuit 401 response is processed before the code that produces the (correct, including realm) www-authenticate header.

Doing research (meaning "using Google"), I can find lots of web developers complaining about the www-authenticate header provided by other web servers, but also developers complaining abouth the missing www-authenticate header from servers implementing a similar behaviour as Spring Security.

In my opinion, the RFC is very clear on this, so IMHO the default should be sending the www-authenticate header with every 401 (Though Spring Security might provide a simple switch like a .disableWwwAuthenticateOn401() method) to cater for less-careful frontend developers.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 15, 2024
@sjohnr
Copy link
Member

sjohnr commented Nov 15, 2024

IMHO the default should be sending the www-authenticate header with every 401 (Though Spring Security might provide a simple switch like a .disableWwwAuthenticateOn401() method) to cater for less-careful frontend developers.

Thanks for the update @MartinEmrich. Based on your response and the fact that it's been a number of years since this default was applied, it might be time to revisit.

However, we usually wait for upvotes on issues before making a change like this because we want to be going where the community needs us to. It's clear that the spec intends HTTP Basic responses to include a WWW-Authenticate so I would say many upvotes (and the absence of other comments on this issue) would imply developers agree and we might want to look at a change. If other folks have suggestions such as convenience methods to switch the behavior (I don't feel this is necessary personally given how easy it is) they can comment as such. We can leave this issue open and see what happens. Sound good?

@MartinEmrich
Copy link
Author

That sounds reasonable, changing a behaviour present for 10+ years might upset people just as much as not fixing it.

But maybe the documentation can include it, so it does not surprise people. I read this section, and then searched for the cause of my issue (with my frontend, which just uses Jquery, explicitly relying on the browser layer basic authentication). It was erratic, depending on browser vendor, OS, cookie/session state, privat browsing mode, etc).
Only after digging into the Spring Security source code, and bisecting the requests my browser made, I put the pieces together.

From https://docs.spring.io/spring-security/reference/servlet/authentication/passwords/basic.html

Since the user is not authenticated, ExceptionTranslationFilter initiates Start Authentication. The configured AuthenticationEntryPoint is an instance of BasicAuthenticationEntryPoint, which sends a WWW-Authenticate header. The RequestCache is typically a NullRequestCache that does not save the request since the client is capable of replaying the requests it originally requested.

I would happily provide a few sentences, but TBH even after years I do not feel as a confident Spring Boot developer knowing all the tech terms.

Maybe at that section something like

The default HTTP Basic Auth Provider will suppress both Response body and WWW-Authenticate header in the response, when the request was made with a X-Requested-By: XMLHttpRequest reader. To override, implement your own BasicAuthenticationEntryPoint like this:

As I have a working fix now, I won't nag you further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: feedback-provided Feedback has been provided type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants