User session gets logged out too

[Gravemind]

Adding a torrent via torrent-control logs out my deluge WebUI session. E.g.: I need to re-log into my deluge WebUI each time torrent-control adds a torrent. (It might happen with other torrent client, but I did not test it.)

I debug the plugin a bit, and I found the plugin listens to all the requests to the server (the plugin's requests, and my own requests when I visit the deluge WebUI). So the plugin catches and injects the same session cookie of all those requests, so when the plugin logOut() after adding a torrent, it actually logs me out too because it is the same session.

I cannot tell if hijacking all session cookie is a feature or a bug.

If sharing the same session cookie between the plugin and the user is a feature (instead of having the plugin create it's own session), then a fix could be to simply add an option to optionally not log out the session ?

(Tested on FF 58, Linux, torrent-control 0.1.10, deluge WebUI)

[Gravemind]

That said, I might feel relatively safer if the plugin logged-in and logged-out of it's own session, and not leaking it to all the requests to the server made across the whole browser. (Just a feeling)

[Mika-]

Actually this addon doesn't have permissions to access browsers cookie store, so the addon needs to create it's own session by design. Just as you described. I tried to duplicate this, but everything worked fine for me.

I'm getting feeling that event listeners somehow don't get cleaned up after extension logs out, and your browser continues to use extensions logged out session token.

[Gravemind]

Well, I tried to debug further but of course now I cannot seem reproduce the log-out bug... sorry about that.

About the shared session cookie, I tested by disabling logOut, added log of the listened requests session cookie, and after adding a torrent via torrent-control (to init listeners), I definitively see the requests made by a Deluge WebUI in an opened tab, and so, sharing the same cookie as torrent-control. But it's possible I missed something, maybe the firefox extension debugging-mode grants more permissions ? (I did not tested an "official" build) ...

But if it's really happening, a race condition could happen in the listeners on the session cookie if WebUI tab makes a request (it refreshes every ~1s) in-between a run of logIn-logOut of torrent-control. Even more probable if the torrent WebUI requests are slow. Still seems like it could happen only very rarely, but that could explain my log-out bug... (I'll test that further later)

@Mika- I just want to say it's just a minor annoyance, torrent-control is already awesome, Thank You.

[Mika-]

I took a second look at this and managed to duplicate. Looks like deluge sometimes handles requests very slowly and session may get revoked during that time. I marked internal requests with custom header so that only those get modified. Can you check if it now works for you?

[Gravemind]

Thanks for looking into it.

I'm just discovering web-extension, but I kind of got stunned by how they put restrictions on set-cookie headers for fetch (for security !?). But still, everything is accessible from requests listeners... (I understand fetch is not web-extension-only...)

And I was never able to get credentials: 'include' to work (only tested with deluge), even with the cookies permission, I still don't know why. But it would have been perfect with cookie origin isolation, so torrent-control would keep it's own session separate from the user's (and no more listeners needed, at least for deluge).

In listeners, instead of x-internal, I though of testing details.originUrl or details.requestHeaders['origin'] against browser.extension.getURL(''), but it might only work with Firefox. EDIT: did not see torrent-control was FF-only, that could work !

I found one example which uses details.requestId to index a global array to communicate information in-between different listeners calls: mdn/webextensions-examples/stored-credentials. But going through listeners and everything seems a lot of effort and very far from the trivial task of setting a cookie...

Anyway, your fixes should do, Thank You.

[Mika-]

Web-extensions API is lacking multiple things and it feels like browser vendors don't really understand what developers actually need.

The cookies permission only affects cookie store reading/writing. Which we don't really want as ideally everything should be isolated.

I was thinking about using referer or origin headers to detect all requests made by the addon. But it would make header manipulation a lot messier when dealing with clients that need valid referer and origin headers. We are currently Firefox only mostly because Chrome already has good addons to do the same thing.

Detecting requests with requestId would be nice, but sadly we can't solve ids for fetch requests :(

[Gravemind]

I gave it a try with originUrl: https://github.com/Mika-/torrent-control/compare/master...Gravemind:deluge-listeners It works, and it seems (to me) easier and safer than X-Internal because we can check both listeners'

I think it could even be moved in BaseClient, it feels like any client implem would only need to listen to torrent-control's own requests, but I would not want to break them if they worked fine this far. Note: sadly I did not find an originUrl for Auth listeners...

But Your call, no expectation.