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

Error page is not accessible even "/error" added to web.ignoring() #28768

Closed
quaff opened this issue Nov 22, 2021 · 7 comments
Closed

Error page is not accessible even "/error" added to web.ignoring() #28768

quaff opened this issue Nov 22, 2021 · 7 comments
Labels
for: external-project For an external project and not something we can fix

Comments

@quaff
Copy link
Contributor

quaff commented Nov 22, 2021

I'm using response.sendError to forward response to BasicErrorController

	@Override
	public void configure(WebSecurity web) {
		web.ignoring().antMatchers("/error");
	}

	@Override
	protected void configure(HttpSecurity http) throws Exception {
		http.formLogin().loginPage(getLoginPage()).failureHandler(authenticationFailureHandler());
	}

	AuthenticationFailureHandler authenticationFailureHandler() {
		return (request, response, ex) -> {
			if (RequestUtils.isRequestedFromApi(request)) {
				response.sendError(SC_UNAUTHORIZED, ex.getLocalizedMessage());
				// generate restful error response
			}
			else {
				response.sendRedirect(getLoginPage() + "?error");
			}
		};
	}

It's broken by dd1d148
@mbhave

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 22, 2021
@wilkinsona
Copy link
Member

Duplicates #28759.

@wilkinsona wilkinsona added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 22, 2021
@quaff
Copy link
Contributor Author

quaff commented Nov 22, 2021

Duplicates #28759.

FYI @wilkinsona, I fixed it by registry.antMatchers("/error").permitAll(), perhaps it doesn't duplicate #28759.

@wilkinsona
Copy link
Member

If you'd like us to double-check, please provide a complete and minimal sample that reproduces the behaviour you've described.

@mgr32
Copy link

mgr32 commented Nov 24, 2021

Hi @wilkinsona, I also noticed the same issue and it's not related to MockMvc - please find a sample here: https://github.com/mgr32/spring-boot-issue-28768 with steps to reproduce in readme.

Basically, for the following configuration and controller :

@Configuration
public class SecurityConfig extends WebSecurityConfigurerAdapter {
    @Override
    public void configure(WebSecurity web) {
        web.ignoring().mvcMatchers("/hello500", "/error");
    }
}
@RestController
public class TestController {
    @GetMapping(path = "/hello500")
    public String hello500() {
        throw new IllegalStateException();
    }
}

Spring Boot 2.5.7 returns a json error response for GET /hello500 (returned from BasicErrorController), while Spring Boot 2.6.0 returns an empty response because of ErrorPageSecurityFilter (it returns to the previous behaviour when errorPageSecurityInterceptor is removed).

@wilkinsona
Copy link
Member

Thanks, @mgr32. Your sample has enabled me to double-check and, as @quaff suspected, this is not a duplicate of #28759.

The problem here is a combination of your use of ignoring(), which results in there being no Authentication, and Spring Security's DefaultWebInvocationPrivilegeEvaluator denying access when authentication is null. You can work around this behaviour by permitting all access instead:

@Configuration
public class SecurityConfig extends WebSecurityConfigurerAdapter {

	@Override
	protected void configure(HttpSecurity http) throws Exception {
		http.authorizeRequests().mvcMatchers("/hello500", "/error").permitAll();
	}

}

These feels like a bug to me so I will bring it to the Security team's attention.

@wilkinsona wilkinsona added status: waiting-for-triage An issue we've not yet triaged and removed status: duplicate A duplicate of another issue labels Nov 24, 2021
@wilkinsona wilkinsona reopened this Nov 24, 2021
@DavidTheProgrammer
Copy link

I think I'm facing a similar challenge.
@wilkinsona I tried your workaround and it didn't quite work for me because I disabled anonymous authentication, leaving the default (enabled) seems to work fine. I though this might be related to the new path matching strategy in 2.6.0 but that doesn't seem to be the case ( suspected this for me because it was controller mappings on a Rest API).

The live application works just fine, so I figured it was mockMvc with the problem. Switched to WebTestClient since it's also supported now and this picked up the configuration correctly, the configured ignored URLs too, without having to add them to the permitAll config.

Hoping this info can help isolate the issue whether it's security or mockMvc with the issue.

@wilkinsona
Copy link
Member

The Security team are now aware of the problem. AIUI, the fix for spring-projects/spring-security#10554 will also address the problem described here with ignoring(). @quaff, you may want to comment on spring-projects/spring-security#10554 to ensure that's the case.

@wilkinsona wilkinsona added for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix
Projects
None yet
Development

No branches or pull requests

5 participants