Spring Security + Spring Data REST: HTTP 400 instead of 403 [DATAREST-289]

Moritz Schulze opened DATAREST-289 and commented

Under certain circumstances a method that is secured via Spring Security Pre/Post annotations can return an HTTP 400 error code instead of an expected HTTP 403.

Example (a fully working application is on github):
We have an EmployeeRepository where each employee may only access him/herself.

public interface EmployeeRepository extends JpaRepository<Employee, Long> {
    @PreAuthorize("hasRole('ROLE_ADMIN') or ( isAuthenticated() and #id == principal.id )")
    @Override
    Employee findOne(Long id);
}

Additionally, updating (PUT) of employees is only allowed to admins via the EmployeeEventHandler:

@RepositoryEventHandler(Employee.class)
public class EmployeeEventHandler {
    @HandleBeforeSave
    @PreAuthorize("hasRole('ROLE_ADMIN')")
    public void checkUpdateAuthority(Employee employee) {
        //only authority check
    }
}

When accessing employees via GET this works fine, the employee with id 1 can access /employees/1 but gets HTTP 403 for /employees/2.

But when performing a PUT to another employee the status code will be HTTP 400.

This code can be executed on the provided example:

curl -v -X PUT http://employee:employee@localhost:8080/employees/1 --header "Content-Type: application/json" --data "{\"name\": \"Test\", \"salary\": 400}"
1. HTTP 403, not admin

curl -v -X PUT http://employee:employee@localhost:8080/employees/2 --header "Content-Type: application/json" --data "{\"name\": \"Test\", \"salary\": 400}"
1. HTTP 400, "Failed to convert from type java.lang.String to type de.techdev.springtest.domain.Employee for value '2';"

This happens because in RepositoryEntityController.putEntity the invocation of the conversion service will fail with a ConversionFailedException. AbstractRepositoryRestController.handleMiscFailures will return HTTP 400.

I would expect an HTTP status of 403, too

Issue Links:

• DATAREST-397 Write tests verifying proper integration with Spring Security

1 votes, 3 watchers