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

Restructure AuthenticationServiceException handling #12134

Open
jzheaux opened this issue Nov 2, 2022 · 5 comments
Open

Restructure AuthenticationServiceException handling #12134

jzheaux opened this issue Nov 2, 2022 · 5 comments
Labels
in: core An issue in spring-security-core type: enhancement A general enhancement
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Nov 2, 2022

An AuthenticationServiceException represents something that went wrong on the server side. As such, it shouldn't be handled by AuthenticationEntryPoints.

This means that likely is shouldn't be handled by ExceptionTranslationFilter or any of the authentication filters.

However, because this class extends AuthenticationException, it is required for each component to somehow opt-out of handing to its AuthenticationEntryPoint.

One way to address this is to change AuthenticationServiceException to no longer inherit from AuthenticationException. Another way would be to add a new exception like AuthenticationServerErrorException -- similar to spring-web's HttpServerErrorException -- that doesn't inherit from AuthenticationException.

@jzheaux jzheaux added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Nov 2, 2022
@jzheaux jzheaux added this to the 7.0.0-M1 milestone Nov 2, 2022
@jzheaux jzheaux added in: core An issue in spring-security-core and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 2, 2022
@rpvilao
Copy link

rpvilao commented Jan 20, 2023

EDIT:
For now, I am using your solution from #10818 by adding an ObjectPostProcessor.

Hi Josh! I bumped into this ticket after I upgraded spring-security-core to 5.7.6 where my authenticationEntryPoint functions are no longer picking up exceptions thrown by the oauth2 introspector (wrapped in an AuthenticationServiceException even though it still extends from AuthenticationException).

So now I am confused... is that already in place? If so, what's the best way to handle these exceptions? Even with a @RestControllerAdvice they are not handled.

If not... what's happening?

Thanks!

@amitbhoraniya
Copy link

Hi @rpvilao @jzheaux - I am not sure what went wrong, atleast previously we were able to handle all exceptions with RestControllerAdvice. But now if anything unexpected happens in AuthenticationService, then its not being handled using RestControllerAdvice and throws response in different way. It doesn't allow us to modify error response in standard way.

Would you please help with same ?

@rpvilao
Copy link

rpvilao commented Aug 5, 2023

Hi @amitbhoraniya,

It was some time ago but I guess I just ended up implementing an object post processor:

.withObjectPostProcessor(object : ObjectPostProcessor<BearerTokenAuthenticationFilter> {
                        override fun <O : BearerTokenAuthenticationFilter> postProcess(o: O): O {
                            o.setAuthenticationFailureHandler { _: HttpServletRequest, httpServletResponse: HttpServletResponse, e: AuthenticationException ->
                                httpServletResponse.contentType = ContentType.APPLICATION_JSON.mimeType
                                httpServletResponse.status = HttpServletResponse.SC_UNAUTHORIZED
                                JsonUtils.objectMapper.writeValue(
                                    httpServletResponse.outputStream,
                                    OAuth2ErrorResponse.builder()
                                        .withError("invalid_token")
                                        .withErrorDescription(e.message)
                                        .build()
                                )
                            }

                            return o
                        }
                    })
            

The example is in koltin, you can adapt to your needs.

@amitbhoraniya
Copy link

@rpvilao - Yes, I did same and working for me. I am still wondering why RestControllerAdvice is not able to handle such exceptions. Is it expected behavior or is it bug ??

@rpvilao
Copy link

rpvilao commented Aug 7, 2023

I don't know, maybe someone from spring security can answer that question.

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

No branches or pull requests

3 participants