ABaumher / galaxy-integration-steam

Integration with Steam for Galaxy
Other
743 stars 17 forks source link

WIP: Upgrade deps and fix tests #16

Open don-de-marco opened 1 year ago

don-de-marco commented 1 year ago

I took the liberty and upgraded all dependencies to the latest version available (except the MacOS runtime stuff because I don't have a Mac and can't test it). Mainly because I got annoyed by the pinned pip version. tl;dr: fixed since ages, we just needed to upgrade pip-tools.

Also I made it so that the tests can be executed again. Not that they all run smoothly or return green but at least we can now see to that and fix everything that is broken there. I already removed a lot of tests for which there is no code anymore (mostly only the public profile related tests).

My observations so far: about half of the tests are green, a quarter is red, and the rest just deadlocks and never finishes (imo that's because of websocket stuff where some coroutines just end up waiting endlessly for results).

ABaumher commented 1 year ago

@TheSentry - Tagging you in so you see this.

I know @TheSentry was trying to find time to get to fixing some of the test cases but other things (i.e. work and personal life) have taken priority, which is completely understandable. I personally don't have nearly the time i used to, most of my work here lately has just been reading the various comments, commits, etc.

I'm assuming everything still builds and works properly with the newer versions? For the most part i inherited the pinned versions and ran into breaking changes whenever i tried to update them, so i just left them alone. One thing i did find though was that pip-tools needs to be at least 5.4.0 or it does not work on MacOS (credit @bgebhardt for that). One issue i was constantly getting was a get_argspec call which is deprecated to hell but some older modules used it. using the newest pip would break that, but maybe that's taken care of by updating a bunch of other pinned versions as well. I'll check their changelogs when i get a moment.

don-de-marco commented 1 year ago

I'm assuming everything still builds and works properly with the newer versions? For the most part i inherited the pinned versions and ran into breaking changes whenever i tried to update them, so i just left them alone. One thing i did find though was that pip-tools needs to be at least 5.4.0 or it does not work on MacOS (credit @bgebhardt for that). One issue i was constantly getting was a get_argspec call which is deprecated to hell but some older modules used it. using the newest pip would break that, but maybe that's taken care of by updating a bunch of other pinned versions as well. I'll check their changelogs when i get a moment.

I've been using the updated dependencies locally for a few days now and there seem to be no errors so far; so I assume that the most important components still work as expected. However, I didn't do much manual testing besides that so I'd appreciate if someone else could try it out and report back how it went.

ABaumher commented 1 year ago

I'm working on a new version that uses betterproto. I'm going to copy in the pinned version changes here. I'm going to release this as alpha 1.0.6 so hopefully we can get a few test dummies to see if both work properly. If this all still works, i'm going to move forward cleaning up the entire code base, except the caches, which i barely understand and likely need a clean-up as well. As far as test cases are concerned, the outside calls should not change so that should not be affected.

Unfortunately, there are some people out there that do not trust GOG's browser for authentication. I do not understand how they expect it to work, but in this case it's actually (unfortunately) something we can handle. We literally are acting like a full steam client, just like steam.exe uses. Our communications are over TLS just like the OpenID web version they'd prefer. 🤨 Anyway, it involves a new html page and requires they manually copy/paste RSA info - we provide them the pulic key steam gives us, they take that key and generate their enciphered password and paste it back in. Our code (and also, critically, GOG) can't decipher it even if it wanted to. Normally, we get the user's password and do this ourselves, but w/e.

this is so dumb.png Then again, how can they trust that whatever rsa tool they decide to use won't steal their password?

engiefox commented 1 year ago

That...seems excessive. I understand not trusting "foreign" programs, but adding more hoops doesn't seem like the solution after a certain point. That whole issue thread is a mess of opinions lol.

engiefox commented 1 year ago

I especially love the guy running sandboxes for every program he uses, just....wow. I cannot imagine the overhead they expend for that.

engiefox commented 1 year ago

I think the analogy of a lock is apt here. Locks aren't meant to be impenetrable, they just add enough difficulty to make what's inside not worth the effort of breaking them. I honestly doubt a bad actor is going to waste all the effort to exploit Steam's login process for a few video games and in my experience things like phishing are much more of a concern to the regular user than any questionable security shortfall from Steam itself. apologies for sharing opinions in a technical thread, but you're already dealing with a lot and adding other people's hangups to the pile isn't going to help.

don-de-marco commented 1 year ago

I'm working on a new version that uses betterproto. I'm going to copy in the pinned version changes here. I'm going to release this as alpha 1.0.6 so hopefully we can get a few test dummies to see if both work properly. If this all still works, i'm going to move forward cleaning up the entire code base, except the caches, which i barely understand and likely need a clean-up as well. As far as test cases are concerned, the outside calls should not change so that should not be affected.

From what I saw, these caches are more or less just data classes which Galaxy only serializes to JSON and then saves the result into the SQLite database somewhere in %PROGRAMDATA%. There's probably not much we can clean up since this is just how it works.

Unfortunately, there are some people out there that do not trust GOG's browser for authentication. I do not understand how they expect it to work, but in this case it's actually (unfortunately) something we can handle. We literally are acting like a full steam client, just like steam.exe uses. Our communications are over TLS just like the OpenID web version they'd prefer. 🤨 Anyway, it involves a new html page and requires they manually copy/paste RSA info - we provide them the pulic key steam gives us, they take that key and generate their enciphered password and paste it back in. Our code (and also, critically, GOG) can't decipher it even if it wanted to. Normally, we get the user's password and do this ourselves, but w/e.

There's one crucial thing wrong with their arguments: we don't use the browser embedded into Galaxy for authentication. We only and solely use the browser to render our HTML to get the credentials from the user. All the authentication and authorization stuff is performed purely in the Python code. No browsers involved.

And, fwiw, implementing OpenID is not that easy on our end. Yes, we could ask the user to extract the token returned by Valve's authentication website (it's probably just a JWT) and provide it via a HTML form during authentication similar to how the MFA code thing is implemented. But that will be a nasty and really ugly hack since the OpenID authentication websites expect you to provide a return URL to which the token is sent (usually as a parameter in the URL). What website can we use? Our authentication attempt comes from within an application, there's no way we can let the OpenID website know that. If Galaxy supported data URIs we could maybe return the token via a URI like galaxy://openid-return/steam?token=<token> and have them inject it by a message via their plugin IPC comm channel. But that's nothing we use/do if it's not already implemented by Galaxy - and I do honestly suspect that they implemented that or would be willing to do so "just for us".

ABaumher commented 1 year ago

From what I saw, these caches are more or less just data classes which Galaxy only serializes to JSON and then saves the result into the SQLite database somewhere in %PROGRAMDATA%. There's probably not much we can clean up since this is just how it works

I'm not sure but it may be possible to optimize those writes. If we write to the cache all the time it's incredibly inefficient, especially if the db file is closed and reopened. Most OS have an optimal block size for reading/writing, so for extremely large amounts of data that arrives at different times it may be useful to have a stream and write in chunks. It may be necessary when dealing with huge libraries to get all their data before that 10 minute timeout.

That said, this is definitely a very low priority thing. I guess a higher priority would be making it easier to understand if it's poorly written (which it probably is because everything here is)

ABaumher commented 1 year ago

But that will be a nasty and really ugly hack since the OpenID authentication websites expect you to provide a return URL to which the token is sent (usually as a parameter in the URL). What website can we use?

It's technically possible to provide a callback url and use a locally-hosted site, but to do this we need to use an http server (the url would be roughly http://127.0.0.1/<port number> It'd also be http, afaik Mozilla is deprecating http so this approach would fail eventually), but this would massively increase the difficulty of code development/maintenance. Python has a module for that, but it'd further bloat our install size for minimal gain. It would allow us to make the UI a little cleaner - that site we display can send requests back to our python code so it wouldn't close and pop back up, it'd always be up. But the maintainability cost of this would be nuts. We'd be running full js instead of the bare minimum in our html currently, and pulling data from an HTTP server python instance, which would likely need to be in a separate process to not bottleneck our code, etc.

Ngl if someone raises an issue on this it's getting WontFixed