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

Provide Access to Spring WebSocket's Configured PathMatcher [SPR-12845] #17443

Closed
spring-issuemaster opened this issue Mar 23, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Mar 23, 2015

Rob Winch opened SPR-12845 and commented

Spring Security should be able to default to using the same PathMatcher that Spring WebSockets is using. This is ideal so that they both use the same PathMatcher (i.e. using . or / separators).

Using SimpAnnotationMethodMessageHandler PathMatcher property does not work because it fails with a circular bean reference. This happens because:

  • stompWebSocketHandlerMapping requests subProtocolWebSocketHandler
  • subProtocolWebSocketHandler requests clientInboundChannel
  • clientInboundChannel requests clientInboundChannelExecutor
  • clientInboundChannelExecutor requests simpAnnotationMethodMessageHandler. This is due to the fact the creation of clientInboundChannelExecutor invokes configureClientInboundChannel and in order to configureClientInboundChannel, we must lookup simpAnnotationMethodMessageHandler to determine the PatternMatcher
  • simpAnnotationMethodMessageHandler then looks up clientInboundChannel (which is a cycle)

You can find my attempt at https://github.com/rwinch/spring-security/tree/SEC-2864 The exact test is AbstractSecurityWebSocketMessageBrokerConfigurerTests#msmsRegistryCustomPatternMatcher

Note that I cannot use MessageBrokerRegistry directly because it is not exposed as a Bean and the accessor is protected.


Issue Links:

  • SEC-2864 Default PathMatcher for WebSocket Destination Matching

Referenced from: commits ae34171, e81862e

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 24, 2015

Rossen Stoyanchev commented

Okay I'll review this to see if we can break the circular bean reference and/or provide additional means to configure the PathMatcher. Thanks for the details.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 24, 2015

Rob Winch commented

I tried to update to use afterSingletonsInstantiated to retrieve the PathMatcher as suggested offline by Rossen. The problem is that the inboundMessageSecurityMetadataSource Bean, which needs the PathMatcher, is guaranteed to be already invoked before afterSingletonsInstantiated is invoked. The msmsRegistryCustomPatternMatcher test demonstrates this failing in SEC-2864-afterSingletonsInstantiated branch.

I'm not sure this can be done without one of the following options:

  • Exposing the MessageBrokerRegistry pathMatcher property (preferable to me). I used reflection to demonstrate this working in the SEC-2864-MessageBrokerRegistry-pathmatcher branch
  • Creating a delegate object (i.e. PathMatcher) that is lazily initialized with an implementation. You can find this working in the SEC-2864-delegating branch.
  • Making so inboundChannelSecurity and inboundMessageSecurityMetadataSource are not Beans allows afterSingletonsinstantiated to work because those methods are no longer invoked before afterSingletonsinstantiated. You can find the tests working on SEC-2864-nobean branch.
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 26, 2015

Rossen Stoyanchev commented

I've added a public getPathMatcher method, which was your preferred option, hence I'm marking this resolved. Should work but if not, give a holler.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 28, 2015

Gabriele Del Prete commented

Can this be backported to 4.1.7 too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.