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

Improve Integration between Authorized Objects and Spring Data #15746

Open
Tracked by #14595
jzheaux opened this issue Sep 5, 2024 · 7 comments
Open
Tracked by #14595

Improve Integration between Authorized Objects and Spring Data #15746

jzheaux opened this issue Sep 5, 2024 · 7 comments
Assignees
Labels
in: core An issue in spring-security-core type: enhancement A general enhancement
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Sep 5, 2024

If an authorized object is sent to Spring Data, for example using CrudRepository#save, the call fails since it tries to look up model metadata by the class name, a CGLIB name in this case.

Moreover, if an authorized object is sent to CrudRepository#save (and the call succeeded), then the associated masks and other authorization handling would apply if its methods are called.

Consider the following sample controller method:

@PutMapping("/{id}")
public Message update(@PathVariable("id") Long id, @RequestBody String body) {
    Message message = this.messageRepository.findById(id); // authorized, if using `@AuthorizeReturnObject`
    // ... 
    // only authorized operations on the object
    // ...
    return this.messageRepository.save(message); // if still wrapped, then unwanted masking or other error handling could ensue when persisting
}

Because a proxied object could be used as a method parameter anywhere in the application, Security can't know on its own any circumstances where it should unwrap the object.

One way to address this could be for Spring Data to detect AuthorizationProxy-implementing domain objects and unwrap them. The following sample illustrates the issue in its updateMessage method.

@jzheaux jzheaux added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Sep 5, 2024
@jzheaux jzheaux self-assigned this Sep 5, 2024
@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 Sep 5, 2024
@jzheaux jzheaux added this to the 6.4.x milestone Sep 5, 2024
@mp911de
Copy link
Member

mp911de commented Sep 11, 2024

Calling a Spring Data repository with an authorized return object requires unwrapping. Spring Data repositories are proxies already hence the advisor would be a good place to unwrap the target object.

You can identify Spring Data repositories as these implement org.springframework.data.repository.Repository.

@jzheaux
Copy link
Contributor Author

jzheaux commented Sep 13, 2024

Calling a Spring Data repository with an authorized return object requires unwrapping. Spring Data repositories are proxies already hence the advisor would be a good place to unwrap the target object.

@mp911de I think this is a good point. Truly, it's better for Spring Security to unproxy its own proxies.

I'm not certain yet how generally applicable such a heuristic would be (outside of repositories), but I reason it's likely every proxied method parameter in a Repository class would want to be unproxied.

A more general-purposes way we could do it is add a companion annotation like @PermitParameter that applications can use to identify which parameters should no longer be advised:

@Repository 
public class MessageRepository ... {
    @AuthorizeReturnObject
    Optional<Message> findById(Long id);

    void save(@PermitParameter Message message);
}

Ostensibly, it could be added at the method or class level to reduce duplication.

Down the road, an application could more generally apply this to repositories though its proposed mixin support.

@noshua
Copy link

noshua commented Oct 16, 2024

@jzheaux Can you give me a hint please how I can unwrap a CGLib proxy to call save() on my entity using Spring 6.3 as I'm currently suffering from this issue? Thanks in advance.

If anyone interested, here is my solution to unwrap a proxy under Spring 6.3:

    @SuppressWarnings("unchecked")
    public static <T> T unwrap(T proxy) {
        try {
            return proxy instanceof Advised advised ? (T) advised.getTargetSource().getTarget() : proxy;
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

@noshua
Copy link

noshua commented Oct 23, 2024

@jzheaux I ran into another problem. When I try to fetch an pageable search results of authorized entities I get:

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

@jzheaux
Copy link
Contributor Author

jzheaux commented Oct 25, 2024

Thanks, @noshua, can you please open a separate ticket with a reproducer, and we can link that ticket to this one?

@noshua
Copy link

noshua commented Oct 25, 2024

@jzheaux Sure, here you go: #15994

@noshua
Copy link

noshua commented Nov 12, 2024

@jzheaux dunno if you have already thought about that so I wanted to mention. If you have 2 entities A and @AuthorizeReturnObject B with any relationship like @OneToOne. When you save A it also fails because the Spring Data stack thinks B$Cglib is an unsaved transient instance.

org.hibernate.TransientPropertyValueException: object references an unsaved transient instance - save the transient instance before flushing : com.example.A.b -> com.example.B

So a solution like you supposed would also need to unwrap relations recursively even if A isn't wrapped itself.
void save(@PermitParameter A a);

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