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

Throwing an Exception inside AuthenticationProvider.authentication() causes a return 200. #7630

Closed
tggm opened this issue Nov 7, 2019 · 6 comments
Assignees
Labels
status: invalid An issue that we don't feel is valid

Comments

@tggm
Copy link

tggm commented Nov 7, 2019

Summary

AuthenticationProvider implementations deal with exceptions differently in the authenticate() method.

Actual Behavior

Consider an AuthenticationProvider (org.springframework.security.authentication.AuthenticationProvider) implementation. If the authenticate(Authentication authentication) method throws a java.lang.Exception the framework will return an HTTP 200 code to the client.

If, however, an AuthenticationException is raised (for instance a concrete AuthenticationServiceException) the framework will return a 403 response code.

Expected Behavior

Any exception inside an authentication provider should cause a 403 error.

Configuration

WebSecurity config

	http.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.NEVER);
	http.csrf().disable();

	http.addFilterBefore(bearerTokenFilter(), BasicAuthenticationFilter.class)
		.anonymous()
		.authorities("ROLE_ANONYMOUS")
		.and().authorizeRequests()
		.antMatchers("/heartbeat").permitAll()
		.anyRequest().authenticated()
		.and().headers().cacheControl().disable()
		.and().exceptionHandling();
}

ResponseEntityExceptionHandler


@ControllerAdvice
public class RestApiExceptionHandler extends ResponseEntityExceptionHandler {
	@ExceptionHandler({Exception.class})
	public ResponseEntity<Object> handleAccessDeniedException(Exception ex, WebRequest request) {
		return new ResponseEntity<Object>(new RestOperationResult(false, "auth_session_expired"), new HttpHeaders(), HttpStatus.UNAUTHORIZED);
	}

	@ExceptionHandler({Throwable.class})
	public ResponseEntity<Object> handleAll(Exception ex, WebRequest request) {
		return new ResponseEntity<Object>(new RestOperationResult(false, "Service Error"), new HttpHeaders(), HttpStatus.INTERNAL_SERVER_ERROR);
	}
}

Version

spring-security-core-5.2.0.RELEASE

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 7, 2019
@eleftherias eleftherias self-assigned this Nov 8, 2019
@eleftherias
Copy link
Contributor

@tggm Can you provide some more detail on the type of exception you are trying to throw?
The method signature is as follows

Authentication authenticate(Authentication authentication) throws AuthenticationException;

so this method would not be allowed to throw a java.lang.Exception.
It can only throw an AuthenticationException or an unchecked exception.

@eleftherias eleftherias added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 8, 2019
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Nov 15, 2019
@tggm
Copy link
Author

tggm commented Nov 15, 2019

Apologies for the delay.

Yes, in this instance the exception thrown was an Unchecked Exception (Derived from RuntimeException). This caused the framework to return a HTTP 200 code to the client.

@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 status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Nov 15, 2019
@eleftherias
Copy link
Contributor

@tggm I'm having difficulty reproducing the problem.
I am seeing an HTTP 500 status returned when throwing a RuntimeException in the authenticate method.
Can you please provide a sample that reproduces this issue?

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

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Nov 26, 2019
@spring-projects-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Dec 3, 2019
@eleftherias eleftherias added the status: invalid An issue that we don't feel is valid label Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants