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

Cannot combine @RestController @SubscribeMapping @RequestMapping and @PreAuthorize [SPR-13367] #17951

Closed
spring-projects-issues opened this issue Aug 18, 2015 · 6 comments
Assignees
Labels
in: web status: declined type: enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 18, 2015

Omer Kudat opened SPR-13367 and commented

We have controller beans that combine both REST and WebSocket function. Generally the classes look like this:

@RestController
@RequestMapping("/api")
public class Controller {

    @SubscribeMapping("/topic/{subject}")
    public Object snapshot(@DestinationVariable String subject) {
        // code
    }

    @RequestMapping(value = "/add", method = POST)
    //@PreAuthorize // Can't use this here
    public void add(@RequestBody Object obj) {
        // code
    }
}

These classes work fine as they are listed above (although I have omitted some attributed for brevity.)

However, the above class no longer maps the subscription topic if the add method is annotated with @PreAuthorize. This happens because the presence of @PreAuthorize breaks org.springframework.messaging.handler.invocation.AbstractMethodMessageHandler#afterPropertiesSet(). This method calls applicationContext.getType(beanName) which returns Proxy with the annotation, but Controller without. When Proxy is returned AbstractMethodMessageHandler thinks the class is not a handler and does not try to detect handler methods.

For now, we have had to revert to doing the auth checks manually.


Affects: 4.1.5

Referenced from: commits spring-attic/spring-framework-issues@52b6352

1 votes, 3 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 19, 2015

Rossen Stoyanchev commented

The CGLIB proxy is a sub-class of the controller and since we use AnnotationUtils.findAnnotation there should be no issue locating the @Controller annotation on the parent class.

I've created a repro project with the exact controller source https://github.com/spring-projects/spring-framework-issues/tree/master/SPR-13367 but I'm unable to reproduce the behavior you're describing. I confirmed the controller is proxied. However both the @SubscribeMapping and @RequestMapping methods are detected respectively as expected.

You can clone the repository and then cd into or import the #17951 sub-directory into an IDE. The project isn't fully set up to serve requests but there is enough configuration to start up and initialize the mappings + security proxying. You can see in the logs on startup the mappings and you can check that it's proxied with the debugger.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 11, 2015

Tong Chen commented

So with
public class Controller implements ApplicationListener
Spring no longer proxy Controller using CGLIB but instead using JDK Dynamic Proxy.
Should we handle JDK Dynamic Proxy as well?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 21, 2015

Tong Chen commented

@Rossen Stoyanchev any comments on above?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 21, 2015

Rossen Stoyanchev commented

Yes if controller implements any interface(s), JDK dynamic proxy is used. You can change that I believe to fix it to use class-based proxying instead.

Should we handle JDK Dynamic Proxy as well?

Not sure what you mean by this.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 21, 2015

Tong Chen commented

I meant that can Spring messaging/websocket handle both cases (JDK dynamic proxy or CGlib)? We could configure it to use class based proxy but it feels like that the library should be able to cope with both cases?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 21, 2015

Rossen Stoyanchev commented

Ah I see what you mean now. The support for @MessageMapping methods is equivalent to what we have for @RequestMapping. In my comments above I indicted I created a sample project and demonstrated it works.

As for adding ApplicationListener, if that's the only interface your controller implements and you have not chosen class-based proxying, then it can't work. If you want JDK based proxying you need to create an interface with @RequestMapping annotations.

@spring-projects-issues spring-projects-issues added status: declined type: enhancement in: web labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web status: declined type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants