Spring-security-oauth: Make WebResponseExceptionTranslator generic to ease custom error formats for resource servers

[michaeltecourt]

-> Pull Request #598

The OAuth 2.0 specification defines how the authorization server error responses must be, but it does not force resource servers to adopt the same format.
While adding OAuth 2.0 security to our services, we found it confusing for our clients to have to deal with two different error formats : one for "business errors" the other for authorization errors.

Making WebResponseExceptionTranslator<T> generic allows Resource Servers to easily implement their own error format on top of framework classes, instead of having to copy a bunch of classes :
AbstractOauth2SecurityExceptionHandler, etc.

On the Authorization Server side, the only change is that components
explicitely define that the error type should be OAuth2Exception :
WebResponseExceptionTranslator<OAuth2Exception>

[frankskywalker]

Hi,@michaeltecourt
Could you give a example of how currently you customize your WebResponseExceptionTranslator.
I'm currently have no idea how to inject custom ExceptionTranslator into resource server.

[michaeltecourt]

Here you go :

/** 
  * Sample Spring Security OAuth config class
  */
@Configuration
@EnableResourceServer
public class SecurityConfig extends ResourceServerConfigurerAdapter {

    @Override
    public void configure(final ResourceServerSecurityConfigurer resources) throws Exception {
        resources
            // This is where you inject your custom error management
            .accessDeniedHandler(accessDeniedHandler())
            .authenticationEntryPoint(authenticationEntryPoint());
    }


    /** Define your custom exception translator bean here */
    @Bean
    public WebResponseExceptionTranslator<?> exceptionTranslator() {
        return new ApiErrorWebResponseExceptionTranslator();
    }

    /**
     * Inject your custom exception translator into the OAuth2 {@link AuthenticationEntryPoint}.
     *
     * @return AuthenticationEntryPoint
     */
    @Bean
    public AuthenticationEntryPoint authenticationEntryPoint() {
        final OAuth2AuthenticationEntryPoint entryPoint = new OAuth2AuthenticationEntryPoint();
        entryPoint.setExceptionTranslator(exceptionTranslator());
        return entryPoint;
    }

    /**
     * Classic Spring Security stuff, defining how to handle {@link AccessDeniedException}s.
     * Inject your custom exception translator into the OAuth2AccessDeniedHandler.
     * (if you don't add this access denied exceptions may use a different format)
     * 
     * @return AccessDeniedHandler
     */
    @Bean
    public AccessDeniedHandler accessDeniedHandler() {
        final OAuth2AccessDeniedHandler handler = new OAuth2AccessDeniedHandler();
        handler.setExceptionTranslator(exceptionTranslator());
        return handler;
    }

}

[baratrion]

Hi @michaeltecourt,

Thanks for the explanation. But I wonder, how you added a type parameter to WebResponseExceptionTranslator? I'm using spring-security-oauth2 v2.0.12 and the interface is like this:

public interface WebResponseExceptionTranslator {

    ResponseEntity<OAuth2Exception> translate(Exception e) throws Exception;

}

So the ResponseEntity will always have a OAuth2Exception type, so how would I pass it a completely different Exception, say, my own generic ApiException?

[michaeltecourt]

That's the point of the pull request ☺ I rewrote WebResponseExceptionTranslator and its implementations with a generic error type

[baratrion]

Alrighty :) Would you mind sharing how you do it currently then, since PR is not merged yet? i.e. details of ApiErrorWebResponseExceptionTranslator? @michaeltecourt

[michaeltecourt]

Of course. The idea is to wrap the default exception translator, and to build your own object from the translated OAuth2Exception :

public class ApiErrorWebResponseExceptionTranslator implements WebResponseExceptionTranslator<ApiError> {

    /** The default WebResponseExceptionTranslator. */
    private final WebResponseExceptionTranslator<OAuth2Exception> defaultTranslator;

    // Constructor omitted

    @Override
    public ResponseEntity<ApiError> translate(final Exception e) throws Exception {
        // Translate the exception with the default translator
        ResponseEntity<OAuth2Exception> defaultResponse = defaultTranslator.translate(e);
        // Build your own error object
        String errorCode = defaultResponse.getBody().getOAuth2ErrorCode();
        ApiError yourError = new ApiError(errorCode, defaultResponse.getBody().getMessage());
        // Use the same status code as the default OAuth2 error
        return new ResponseEntity<ApiError>(yourError, defaultResponse.getStatusCode());
    }
}

[pakkk]

Hi,

When will it be available? I have the version:

<dependency>
  <groupId>org.springframework.security.oauth</groupId>
  <artifactId>spring-security-oauth2</artifactId>
  <version>2.2.0.RELEASE</version>
</dependency>

I am trying to customize the Exception to provide more fields (ApiError contains a new field 'myCustomMessageField'):

public class ApiErrorWebResponseExceptionTranslator implements WebResponseExceptionTranslator {

    /** The default WebResponseExceptionTranslator. */
    private WebResponseExceptionTranslator defaultTranslator = new DefaultWebResponseExceptionTranslator();

    // Constructor omitted

    @Override
    public ResponseEntity<OAuth2Exception> translate(final Exception e) throws Exception {
        // Translate the exception with the default translator
        ResponseEntity<OAuth2Exception> defaultResponse = this.defaultTranslator.translate(e);
        // Build your own error object
        String errorCode = defaultResponse.getBody().getOAuth2ErrorCode();
        ApiError yourError = new ApiError(errorCode, defaultResponse.getBody().getMessage());
        // Use the same status code as the default OAuth2 error
        return new ResponseEntity<OAuth2Exception>(yourError, defaultResponse.getStatusCode()) ;
    }
}

I had to change the shared the shared code by @michaeltecourt because it could not compile, and I am getting a no custom error yet (it is the default yet):

{
  "error": "invalid_request",
  "error_description": "Access is denied"
}

Thank you :)

[jgrandja]

@michaeltecourt Have you considered providing your own extension of OAuth2Exception that would represent your applications specific API or Business error?

For example, let's say you provide your own custom OAuth2Exception type as follows:

public class ApiException extends OAuth2Exception {

    public ApiException(String message) {
        super(message);
    }

    public ApiException(String message, Map<String, String> errorAttrs) {
        super(message);
        for (Map.Entry<String, String> errorAttr : errorAttrs.entrySet()) {
            this.addAdditionalInformation(errorAttr.getKey(), errorAttr.getValue());
        }
    }

    public ApiException(String message, Throwable cause) {
        super(message, cause);
    }

    @Override
    public String getOAuth2ErrorCode() {
        return "api_error";
    }

    @Override
    public int getHttpErrorCode() {
        // Return Bad Request or some other client-specific error
        return 400;
    }
}

Somewhere in your Resource Server you would throw this exception as follows:

Map<String, String> errorAttrs = new LinkedHashMap<String, String>();
errorAttrs.put("api_error_code", "2000");
errorAttrs.put("api_error_message", "some error message specific to the api resource");
errorAttrs.put("custom_attribute_1", "value1");
errorAttrs.put("custom_attribute_2", "value2");

throw new ApiException("the api error message", errorAttrs);

The response back to the client (with status 400) would be:

{
  "error": "api_error",
  "error_description": "the api error message"
  "api_error_code": "2000",
  "api_error_message": "some error message specific to the api resource",
  "custom_attribute_1": "value1",
  "custom_attribute_2": "value1",
}

This strategy should allow you to customize the error response back to the client in order to distinguish business/api related errors. Let me know how it goes.

[michaeltecourt]

This workaround could do the job for a couple of exceptions, but people often have a lot of them distributed across many repositories/modules. Having every module depend on spring-security-oauth2 to circumvent the issue feels hacky, I'd rather have the generic classes of this PR duplicated in some common lib :)

Also, I do understand this project is in maintenance mode, but I don't think the end result looks good with the workaround for production APIs 😅 (sorry I'm superficial).

[jgrandja]

Understood that you don't want to depend on spring-security-oauth2 for your common lib that contains your application/business module, such as, ApiError as the example you provided.

Couldn't you do this than:

public class ApiException extends OAuth2Exception {

    public ApiException(ApiError error) {
        ...
    }
}

ApiError lives in your common lib and ApiException lives in the module that contains your Resource Server common lib (I'm assuming you have a common lib for your Resource Server modules) which depends on spring-security-oauth2. The constructor of ApiException would copy all the attributes from ApiError to itself.

Also, as an alternative to creating your own extension of OAuth2Exception, you can even do this in your Resource Server code:

Map<String, String> errorAttrs = new LinkedHashMap<String, String>();
errorAttrs.put("error", "api_error");
errorAttrs.put("api_error_code", "2000");
errorAttrs.put("api_error_message", "some error message specific to the api resource");
errorAttrs.put("custom_attribute_1", "value1");
errorAttrs.put("custom_attribute_2", "value2");

throw OAuth2Exception.valueOf(errorAttrs);

The response would than be:

{
  "error": "invalid_request",
  "error_description": "api_error"
  "api_error_code": "2000",
  "api_error_message": "some error message specific to the api resource",
  "custom_attribute_1": "value1",
  "custom_attribute_2": "value1",
}

This alternative solution would not require you to create any extension classes of OAuth2Exception.

[michaeltecourt]

I think you misunderstood the problem that this PR tries to fix : the goal of this PR is not to make business errors look like OAuth2Exceptions, it is to make all resource server errors respect the same format as business errors (including access denied and unauthorized).

For example we use RFC 7807 application/problem+json for our business errors, and we used the classes of this PR and the configuration showed above to render our resource server's security errors as application/problem+json.

The idea is that OAuth 2.0 never specified how the resource server errors should look like, only the authorization server _must_ use { error + error_description } (and hence OAuth2Exception).
Spring Security OAuth 2.0 uses an AccessDeniedHandler and AuthenticationEntryPoint that force the resource server to render security errors as OAuth 2.0 errors, without extension points. This PR brings a possibility to customize 401s and 403s format on resource server, while preserving backward compatibility.

[jgrandja]

@michaeltecourt I do understand the goal of this PR. In a nutshell, you don't want to see error or error_description as attributes in your Resource Server responses. I understand this may be confusing having a mixture of Authorization Server response attributes (error and error_description) and your custom Resource Server attributes, for example, api_error_code.

The goal I'm trying to achieve here is minimal change as this PR touches on a few interfaces/classes. I was hoping the options I provided earlier may suit your needs but I understand they fall short as they still keep those attributes that you prefer not to be included in your Resource Server responses.

Here is another possible solution. Can you try this out and let me know if this will work for you.

public class ResourceServerAccessDeniedHandler extends AbstractOAuth2SecurityExceptionHandler implements AccessDeniedHandler {

    @Override
    public void handle(HttpServletRequest request, HttpServletResponse response, AccessDeniedException ex) throws IOException, ServletException {
        doHandle(request, response, ex);
    }

    @Override
    protected ResponseEntity<?> enhanceResponse(ResponseEntity<?> result, Exception ex) {
        // result is a ResponseEntity<OAuth2Exception> as it's translated by DefaultWebResponseExceptionTranslator

        ResponseEntity<ApiError> otherResult;
        // You can now create your ResponseEntity<ApiError> and return that
        // instead of returning result -> ResponseEntity<OAuth2Exception>
        //
        // This essentially overrides the response body which is what you are trying to achieve

        return otherResult;
    }
}

Then configure it

@Configuration
@EnableResourceServer
public class SecurityConfig extends ResourceServerConfigurerAdapter {

    @Override
    public void configure(final ResourceServerSecurityConfigurer resources) throws Exception {
        resources
            .accessDeniedHandler(accessDeniedHandler())
            .authenticationEntryPoint(authenticationEntryPoint());
    }

    @Bean
    public AuthenticationEntryPoint authenticationEntryPoint() {
        final OAuth2AuthenticationEntryPoint entryPoint = new OAuth2AuthenticationEntryPoint();
        return entryPoint;
    }

    @Bean
    public AccessDeniedHandler accessDeniedHandler() {
        final AccessDeniedHandler handler = new ResourceServerAccessDeniedHandler();
        return handler;
    }
}

[michaeltecourt]

This solution leaves the OAuth2AuthenticationEntryPoint out of the equation, meaning that "unauthorized" (401) errors would remain the same.

The responsibility of the WebResponseExceptionTranslator is clearly to be shared by error handlers, I cannot think of a cleaner solution.
Even though multiple classes are impacted the changes are trivial (using a generic), do you see any risk I didn't think of ?

[jgrandja]

This solution leaves the OAuth2AuthenticationEntryPoint out of the equation, meaning that "unauthorized" (401) errors would remain the same.

Correct, the current behaviour of OAuth2AuthenticationEntryPoint would remain the same...if the Authentication could not be established in the case where the Bearer token is not passed in the request than the following response would be rendered:

{
  "error": "unauthorized"
}

with 401 status and WWW-Authenticate header. This response makes sense.

Do you also want to customize this response to your business-specific error message?

The responsibility of the WebResponseExceptionTranslator is clearly to be shared by error handlers, I cannot think of a cleaner solution.

I do agree here but I also prefer to make minimal changes if possible so I'm exploring if there is another solution.

[michaeltecourt]

Yes, we want the same error format for all APIs including 401/unauthorized errors.

[jgrandja]

Ok makes sense that you would want one standard error message rather than 2. Let me look at merging this.