search

yamcs / python-yamcs-client

Yamcs Client for Python
https://docs.yamcs.org/python-yamcs-client/
GNU Lesser General Public License v3.0
16 stars 11 forks source link

Authorization header not populated before creating WebSocketApp in subscription actions #6

Closed mschnur closed 4 years ago

mschnur commented 4 years ago

When connecting to a YAMCS server with security enabled, if the first action that is done is attempting to construct a subscription of some kind then the request will fail due to being unauthorized. In WebSocketSubscriptionManager.open() it attempts to construct a websocket.WebSocketApp, and the exception is raised within this constructor. I've seen this error with a parameter subscription and command history subscription, but I imagine it applies to all subscriptions that use WebSocketSubscriptionManager (and maybe V2?)

I'm not super familiar with this library but from my digging it looks like what is happening is that in the constructor call of WebSocketApp, the headers from the current session are passed as the headers parameter. However, if the first thing that is done with a YamcsClient is to construct a Processor then create a subscription, it appears that the Authentication header is not in the current session headers. I see that pretty much all of the non-subscription actions that are done end up being sent via BaseClient.request(), and in this function the session headers are updated:

if self.credentials:
    self.credentials.before_request(self.session, self.auth_root)

I tried adding an identical snippet of code before the construction of the WebSocketApp in WebSocketSubscriptionManager.open(), and the error went away. Similarly, if I do a non-subscription action before doing a subscription one, then the error does not occur.

fqqb commented 4 years ago

Thanks, you nailed it.

I'm a bit stunned to not have encountered this yet.

Fix pushed to master. I would wait for a few more commits before another release, but if you prefer it earlier, I'm happy to do so.

It happened with both V1 and V2-style. V2 doesn't change anything authentication-wise. It is mainly a change related to the subprotocol which will make it easier to manage multiple subscriptions on the same WebSocket connection. But a full migration is still pending on transitioning (and documenting) all existing subscription topics server-side.