rabbitmq / rabbitmq-web-stomp

Provides support for STOMP over WebSockets
Other
89 stars 26 forks source link

Allow fallback to default_login when using a CONNECT frame #110

Closed ngoossens closed 5 years ago

ngoossens commented 5 years ago

When the STOMP default_login is set and a CONNECT frame is sent via a WebSocket without credentials the connection fails with bad login. This is because rabbitmq-web-stomp does not forward the default_login to rabbitmq-stomp as part of the state.

If use_http_auth is set to true this state is passed to rabbitmq-stomp but then the credentials in the CONNECT frame are ignored and the default_login is always used if no HTTP Authorization header is set.

This pull request slightly modifies the flow to also send the default_login to rabbitmq-stomp even if use_http_auth is false. In this way one can use a CONNECT frame both with credentials and without (falling back to the default user if configured).

pivotal-issuemaster commented 5 years ago

@ngoossens Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-issuemaster commented 5 years ago

@ngoossens Thank you for signing the Contributor License Agreement!

michaelklishin commented 5 years ago

@ngoossens thank you for taking the time to contribute. How would you go about testing this scenario? I can add an integration test but I need to understand what your clients do (and ideally also why).

Anything related to optional authentication makes me wonder if it's a good idea. I know, STOMP supports it already 🤷‍♂️

ngoossens commented 5 years ago

@michaelklishin, thanks for your reply.

We have an application where we create subscriptions to variables published from a service via RabbitMQ. Some of these variables are accessible without a login (e.g. server time) and others are not. When opening the web application, before logging in, we would like to create subscription to the publicly accessible variables using the default user. Later once the user has logged in we would use their new credentials to subscribe to all the available variables.

This is technically possibly with the current code but in a convoluted way. If you set use_http_auth to true in advanced.config WebSTOMP will read the HTTP Authorization header for credentials and fall back to the default login if no Authorization header is provided (ignoring any credentials sent using the CONNECT frame). Based on my reading of the STOMP specification this should also be possible using a CONNECT frame without any HTTP headers, but it is currently not.

An integration test would have a few parts. The following would have use_http_auth set to false.

  1. With a default_login configured: 1.1. Over a WebSocket send a CONNECT frame that includes valid credentials and expect that user to be authenticated. 1.2. Over a WebSocket send a CONNECT frame omitting any credentials and expect that the default login is used.
  2. Without a default_login configured: 2.1. Over a WebSocket send a CONNECT frame that includes valid credentials and expect that user to be authenticated. 2.2. Over a WebSocket send a CONNECT frame omitting any credentials and expect the connection to fail.
michaelklishin commented 5 years ago

@ngoossens thanks. I will look into adding a few tests. If the team agrees this can go into 3.7.15 the earliest.

ngoossens commented 5 years ago

Thanks @michaelklishin. No rush, we can use a custom compiled version for now. Please let me know if I can assist in any way.

michaelklishin commented 5 years ago

Tests added in #112. Thanks again @ngoossens 👍👍.