nats-server icon indicating copy to clipboard operation
nats-server copied to clipboard

WIP: Add websocket browser_redirect_url config option

Open philpennock opened this issue 5 years ago • 10 comments

If an HTTP(S) request is received without the Upgrade: header, then if this new option is set then we'll assume it's a browser and issue a redirect.

There's no flexibility based upon the passed in URL; this should help with redirecting people to dashboards or other tooling.

There's no documentation and no tests, this should not be merged as-is, the PR exists for people to discuss, or replace with something better.

/cc @nats-io/core

philpennock avatar Oct 24 '20 00:10 philpennock

I left in that there's logging of an error for such a request, but the logging doesn't appear to pull in the source address. So two refinements suggest themselves:

  1. Make sure that we auto-decorate the returned error from the websocket upgrade to client IP information, for more useful logs
  2. Return a sentinel error and just don't bother logging these redirects at all

Either or both of these seem worthwhile. Thoughts?

philpennock avatar Oct 24 '20 01:10 philpennock

@derekcollison will add a corresponding jwt field to v2

matthiashanel avatar Oct 24 '20 02:10 matthiashanel

We should make sure we can eventually handle this properly with multi-operator setups.

derekcollison avatar Oct 24 '20 03:10 derekcollison

We should make sure we can eventually handle this properly with multi-operator setups.

@derekcollison the way I would go about handling this in a multi operator setup is by calling out one of the operators to be operating the server. This server would then pick the redirect from that particular operator only. The other operators would be trusted but not operating this server.
This could be as simple as documenting that the first operator in the list is operating this server or have extra config to specify which one.

matthiashanel avatar Oct 24 '20 20:10 matthiashanel

I like that approach..

derekcollison avatar Oct 24 '20 21:10 derekcollison

@derekcollison @matthiashanel I am a bit confused. This parameter that @philpennock added is for situations where something connects to the Websocket port that is detected as likely not being a client (since it does not have the proper http headers). So obviously the option cannot be linked to a specific account (since there is no auth at the point the error is detected). I assume then that you are referring to the JWT of the operator. But then the question is: why would this new configuration be different than any other? One would expect that if we decide that first operator in the list is the one operating the server, the config itself would be specific to this operator anyway. I am not saying that it is bad that configuration be specific to an operator, but I wonder why this new option specifically.

kozlovic avatar Oct 26 '20 15:10 kozlovic

Yes operator JWT, and I think anything we can do to skinny down the actual server config in operator mode is a good thing. Much rather update operator JWT then all configs for all servers etc.

derekcollison avatar Oct 26 '20 16:10 derekcollison

Since this is a websocket port, I am not sure why we do an HTTP redirect - websocket URLs for one are ws(s) - if an HTTP client hits the ws URL using a browser, redirecting doesn't seem like the right strat.

aricart avatar Oct 26 '20 17:10 aricart

TBH, if we are going to do this here, then why aren't we doing a similar assertion on if a browser is pointed to the nats service port or if a nats client is pointed to the monitoring port.

aricart avatar Oct 26 '20 17:10 aricart

@philpennock Is this still relevant?

bruth avatar Apr 23 '23 19:04 bruth