Add Simple way of whitelisting origin [SPR-12226]

[spring-projects-issues]

Rob Winch opened SPR-12226 and commented

It is best practice to verify the origin of a WebSocket connection. It would be nice if Spring WebSocket support provided a convenient way to whitelist a set of origins.

NOTE: If interested, I am willing to provide a PR for this.

---

Affects: 4.1 GA

Reference URL: http://docs.oracle.com/middleware/1213/wls/WLPRG/websockets.htm#BABEDBBB

This issue is a sub-task of #16921

Issue Links:

• SEC-2667 Consider Oracle Guidelines ("is depended on by")
• AbstractSockJsService.checkAndAddCorsHeaders fails for same origin requests when setAllowedOrigins is set [SPR-12660] #17260 AbstractSockJsService.checkAndAddCorsHeaders fails for same origin requests when setAllowedOrigins is set

Referenced from: commits 743356f

0 votes, 6 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

I presume you mean a property in DefaultHandshakeHandler? Feel free to send a PR.

[spring-projects-issues]

Rob Winch commented

Rossen Stoyanchev Thanks for the fast response! I had actually had originally planned on providing a HandshakeInterceptor since it appeared it would also work with custom HandshakeHandler implementations. Thoughts?

[spring-projects-issues]

Rossen Stoyanchev commented

The DefaultHandshakeHandler currently has a protected method (isValidOrigin) that always returns true. It seems natural to start with some extra options there. A HandshakeInterceptor might be useful as in addition although I can't imagine that a completely custom HandshakeHandler is very common but I could be wrong.

[spring-projects-issues]

Rob Winch commented

Are you set on this being in DefaultHandshakeHandler? I feel like HandshakeInterceptor is a better fit because:

• I like the idea of being able to use the HandshakeInterceptor for any HandshakeHandler implementation. Especially since this can be done very easily.

• Placing it in a HandshakeInterceptor keeps the logic very simple and isolated as apposed to making DefaultHandshakeHandler more complex

• It doesn't feel ideal to provide a property on the DefaultHandshakeHandler that impacts a protected method (isValidOrigin) that is intended to be overridden. This means if users do extend and override isValidOrigin to provide something more advanced (i.e. regular expression based origin matching), the class will have a property that is not used.

• The documentation states that HandshakeInterceptor is the simplest approach (I tend to agree) to modify the handshake. If it is simpler and works, it would be nice to use it.

• It seems a bit more difficult to figure out how to configure there. For example, the easiest way I can figure out to configure it is the following:

DefaultHandshakeHandler handshakeHandler = new DefaultHandshakeHandler();
handshakeHandler.setAllowedOrigins("http://localhost:8080");
registry.addEndpoint("/messages")
            .withSockJS()
            .setTransportHandlerOverrides(new WebSocketTransportHandler(handshakeHandler));

For something so common, it would be nice to configure with fewer lines of code:

registry.addEndpoint("/messages")
            .withSockJS()
            .setInterceptors(new AllowedOriginsInterceptor("http://localhost:8080"));

Perhaps there is an easier way to configure DefaultHandshakeHandler that I am not aware of?

The one (small) advantage of placing this in DefaultHandshakeHandler is that it will automatically set the HTTP status code. This is trivial to add to a HandshakeInterceptor implementation though.

[spring-projects-issues]

Sébastien Deleuze commented

Pull request #670 submitted.

[spring-projects-issues]

Sébastien Deleuze commented

I have just merged the support for WebSocket origin whitelist in master.

As discussed previously, this whitelist is also used to check SockJS CORS request headers, when SockJS is enabled.