Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save login session and add auto login on game launch #98

Closed
wants to merge 0 commits into from

Conversation

Enjoys-1
Copy link
Contributor

@Enjoys-1 Enjoys-1 commented Aug 9, 2023

After logging in, the newly generated session is saved and can be used again to automatically log in when starting the game. Both of these features are disabled by default, but can be enabled in the configuration. Great for people who frequently restart the game and need to log in every time.

@github-actions github-actions bot added the i18n Improvements or additions to localisation label Aug 9, 2023
@axieum
Copy link
Owner

axieum commented Aug 10, 2023

Welcome back @Enjoys-1 😂

Initial code review looks very well done. I'll need to check over the GUI changes once I get around to checking out the branch.

The main concern I have though, is that we need to think about how we store this access token (you can get up to speed by reading #4).

We should really try to avoid saving the plaintext access token to disk. Initial thoughts is that a malicious mod can just as easily write mixins, etc. to grab the tokens from memory - nothing we can really do about that. But, once they exit memory and are saved to disk, that's where I'm mainly concerned. If we can easily introduce encryption, let's do it.

In both scenarios, encrypted or plaintext, we should add a big disclaimer to that option stating that the user is enabling it at their own risk.

Thanks,

@Enjoys-1
Copy link
Contributor Author

Enjoys-1 commented Aug 10, 2023

I added a warning screen that will be shown when trying to login and having session saving enabled.

The access token is now encrypted and the key is randomly generated on every login.
Note: No amount of encryption will prevent malware from decrypting and stealing access tokens. If this mod can decrypt it, any software can. Especially when the encryption algorithm is publicly visible. We can only make it difficult for someone to socialy engineer the user to share their credentials. I was thinking about saving the key in a separate file or using CryptUnprotectData/CryptProtectData (iirc it's Windows only) but I'd like to hear your thoughts on that, since you would like to keep the mod simple and small.

@axieum
Copy link
Owner

axieum commented Aug 11, 2023

Agreed. This should at least make it harder to scan the disk for plaintext tokens - unless they are specifically targeting this mod.

I think the warnings are sufficient enough, and I'd only expect advanced users to use this new feature.

I'll check out the branch over the weekend and inspect the UI changes.

Thanks again,

@Enjoys-1
Copy link
Contributor Author

I updated my fork of the mod to 1.21.1, but it looks like I also accidentally closed this pull request. It wouldn't probably get merged anyway, but if someone wants it, you can build my fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n Improvements or additions to localisation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants