Skip to content

AuthorizeReturnObject should target the authorized object within Spring Data components #15994

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

Open
noshua opened this issue Oct 25, 2024 · 5 comments · May be fixed by #16953
Open

AuthorizeReturnObject should target the authorized object within Spring Data components #15994

noshua opened this issue Oct 25, 2024 · 5 comments · May be fixed by #16953
Assignees
Labels
type: bug A general bug
Milestone

Comments

@noshua
Copy link

noshua commented Oct 25, 2024

Describe the bug
Using Authorizing Arbitrary Objects of Spring Security in combination with a Pageable Spring Data result fails.

To Reproduce

  1. Add "@PreAuthorize" to an Entity class.
  2. Add "@AuthorizeReturnObject" to a repository class method with return type Page.
  3. Call the repo method.

java.lang.ClassCastException: class org.springframework.security.authorization.method.AuthorizationAdvisorProxyFactory$ContainerTypeVisitor$$Lambda/0x0000791458a2cb00 cannot be cast to class org.springframework.data.domain.Page (org.springframework.security.authorization.method.AuthorizationAdvisorProxyFactory$ContainerTypeVisitor$$Lambda/0x0000791458a2cb00 and org.springframework.data.domain.Page are in unnamed module of loader 'app')

Expected behavior
A paged result of security proxied objects should be returned from the repository method.

Sample
https://github.com/noshua/authorize-spring-data

@noshua noshua added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Oct 25, 2024
@jzheaux jzheaux self-assigned this Oct 25, 2024
@jzheaux jzheaux removed the status: waiting-for-triage An issue we've not yet triaged label Oct 25, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Oct 25, 2024

Thanks for this report, @noshua. Spring Data types are important for @AuthorizeReturnObject to be able to proxy.

For now, you can place @AuthorizeReturnObject on your controller method instead.

Or, if you want to support @AuthorizationReturnObject with Page, you can add this support yourself like so:

private final TargetVisitor page = (proxyFactory, target) -> {
    if (target instanceof PageImpl<?> page) {
        List<Object> content = (List<Object>) proxyFactory.proxy(page.getContent());
        return new PageImpl<>(content, page.getPageable(), page.getTotalElements());
    }
    return null;
};

@Bean
Customizer<AuthorizationAdvisorProxyFactory> addVisitors() {
    return (factory) -> factory.setTargetVisitor(TargetVisitor.of(page, TargetVisitor.defaults()));
}

@noshua
Copy link
Author

noshua commented Nov 5, 2024

Sorry for the delayed response @jzheaux but I needed some time to investigate.

Putting @AuthorizeReturnObject directly onto @RestController doesn't work at all in combination with ResponseEntity<T> method responses.
When I implement a @Component in between JpaRepository and @RestController, with return type T to work around this, but no getter is called in my business logic the AuthorizationDeniedException will happen in JSON response mapping. Then it get's translated into a HttpMessageNotWritableException with status 500 instead of the correct 403 error.

Is it possible to achieve that the @PreAuthorize on T is evaluated when getting the object itself instead of calling it's attributes getters?

I created a new sample branch to show this behaviour: https://github.com/noshua/authorize-spring-data/tree/authorizeController

@jzheaux
Copy link
Contributor

jzheaux commented Nov 11, 2024

Thanks for the sample, that's very helpful.

As with Page any complex object that Spring Security does not yet know how to handle, you can add the appropriate TargetVisitor. A TargetVisitor for ResponseType would be something like:

@Bean
Customizer<AuthorizationAdvisorProxyFactory> addVisitors() {
    return (factory) -> factory.setTargetVisitor(TargetVisitor.of(responseEntity, TargetVisitor.defaults()));
}

private final TargetVisitor responseEntity = (proxyFactory, target) -> {
    if (target instanceof ResponseEntity<?> entity) {
        Object body = entity.getBody();
        HttpHeaders header = entity.getHeaders();
        HttpStatusCode code = entity.getStatusCode();
        return new ResponseEntity<>(proxyFactory.proxy(body), header, code);
    }
    return null;
};

To propagate the AuthorizeDeniedException, I think it would be helpful for Spring Security to add an MVC component that assists with propagation. For now, you can get that to work in the following way:

@ExceptionHandler(HttpMessageNotWritableException.class)
View handleWrite(HttpMessageNotWritableException ex) {
    if (ex.getRootCause() instanceof AuthorizationDeniedException denied) {
       return new AbstractView() {
          @Override
          protected void renderMergedOutputModel(Map<String, Object> model,
                HttpServletRequest request, HttpServletResponse response)
                throws Exception {
             throw ex;
          }
       };
    }
    throw ex;
}

I've added a PR to your sample repo to demonstrate both of these.

@jzheaux jzheaux changed the title Authorized Objects with Pageable Spring Data throws ClassCastException Authorized Objects should correctly wrap Spring Data components Nov 11, 2024
@jzheaux jzheaux added this to the 6.5.x milestone Nov 11, 2024
@jzheaux jzheaux changed the title Authorized Objects should correctly wrap Spring Data components AuthorizeReturnObject should target the authorized object within Spring Data components Nov 11, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Nov 11, 2024

Related to #14717

evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue Apr 17, 2025
@evgeniycheban
Copy link
Contributor

evgeniycheban commented Apr 17, 2025

Hi, @jzheaux @noshua I have added a DataTargetVisitor that adds support for Spring Data container types such as GeoResults, GeoResult, GeoPage, Page and Slice, it is registered automatically if Spring Data is available in classpath.

evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue Apr 17, 2025
evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue Apr 17, 2025
evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue Apr 17, 2025
evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue Apr 17, 2025
evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue Apr 18, 2025
evgeniycheban added a commit to evgeniycheban/spring-security that referenced this issue Apr 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
Status: No status
3 participants