ryukinix / mal

MAL: A MyAnimeList Command Line Interface [BROKEN: BLAME MyAnimeList]
https://mal.readthedocs.io
Other
109 stars 9 forks source link

Store login credentials in encrypted form #81

Open taktoa opened 6 years ago

taktoa commented 6 years ago

I noticed that after running mal login, ~/.config/mal/myanimelist.ini contains my MyAnimeList login credentials in plaintext. I think there should at least be an option to store it as a GPG encrypted blob (so whenever I run mal it will ask the GPG agent to decrypt the blob, prompting me for a password to my GPG private key every so often). I may take a look at implementing this myself.

ryukinix commented 6 years ago

Yes, this is indeed a problem, I probably warn that in the README (or in https://lerax.me/mal). The problem overall is: there is no Oauth or other more secure way to communicate our credentials with the currently MAL API. It's pretty horrible.

Probably you can implement that, but sincerely I'm not very happy to depend to gpg, the GNU PGP client, to mal works. I use gpg a quite often, but I know that some people don't use it and some of them don't even understand how to deal with PGP keys. I don't want obey people get into all the mess because the MAL API fault.

However, yes, I care about my own credentials and as well the other users of mal. If would possible implement that as optional behavior... I don't know, probably mal/config.py stuff? This is stored at ~/.config/mal/myanimelist.ini. That way people that have a nice setup of GPG can use it and people that don't want to worry about gpg can just ignore it (btw, it's a quite surprise for me, but even don't be supported, there is some people trying to use this client on Windows... so think a little adding gpg dependency for windows users :v ).

I have some hope to MyAnimeList release a better API with oauth in some day... but probably I'm just dreaming a lot about it.

But, anyway, it's free software. If this really bother you, you can fork freely, implement it as default behavior and use your own version.

Thanks for your thoughts @taktoa, this is a important thing. Since you implement a nice optional feature don't being the default behavior, I probably can merge it on master (or at least in some other branch of the repository).

kutsan commented 6 years ago

Instead of living with GPG-only world, why not make an eval option for ini file that can be used while reading our passwords?

[login]
username = kutsan
passwordeval = gpg --no-tty --for-your-eyes-only --quiet --decrypt ~/.config/mal/secret.gpg

That way, we -even so-called Windows users- can define whatever command we want.

Besides, as default, other than storing as a plain text file, I'm even okay with ROT13. It just shouldn't be that easy to get. Also, mal definitely needs to chmod 600 to that file.

I have some hope to MyAnimeList release a better API with oauth in some day... but probably I'm just dreaming a lot about it.

They need to rebuild the whole website from scratch. There are a lot of things that wrong about it. API is just one of them. But, well, at least there is an API.

evanjs commented 6 years ago

This might also be an option: https://github.com/Netflix-Skunkworks/go-jira/blob/master/README.md#keyring-password-source

Not sure what we'd do about Windows, though. Looks like credential locker is supported here: https://pypi.python.org/pypi/keyring

ryukinix commented 6 years ago

We can implement that as well: tea-tiny-encryption-algorithm.pdf

Which will be done with no costs about relying on encrypt software, external dependency or hacky configurations for evaluating encrypt commands.

kutsan commented 6 years ago

@ryukinix I really want to have passwordeval functionality. Do you have any plans on that? Can I create another issue?

ryukinix commented 6 years ago

I don't have any plans to implement something as passwordeval or using gpg explicitly (though passwordeval is indeed better than that). I think that can be a minimal solution, but too dependent to config stuff... I don't like very much that. I would prefer something like TEA, which can work fine by default without the user knows about it because we'll implement all the mechanism of encrypt/decrypt (it's really simple).

Moreover, reducing always the number of dependencies and config we'll have more control of our software... we have until a bug with argparse (#79), this is quite awful.

But, if you really want this, and you want implement this, you are free to implement that by yourself. Since you show your point and we like it, we can merge it. Otherwise, I don't have plans on that, as I said.

I think this issue is already sufficient to discuss about that.

kutsan commented 6 years ago

My point was provide a simple algorithm as a default security model and apart from that let them have passwordeval config if they want more advanced security. Obviously, no one wants passwordeval as a default option.

ryukinix commented 6 years ago

My point was provide a simple algorithm as a default security model and apart from that let them have passwordeval config if they want more advanced security. Obviously, no one wants passwordeval as a default option.

Okay, since this your point and I get it right now, I'm ok with that. Probably, if I implement the tea, I'd add it as a especial tag like passwordeval=@tea to avoid conflicts on namespace if for some weird reason have a tea binary.

Are you ok with that, @kutsan? Probably everyone will be happy with that... :thinking: I guess.

ryukinix commented 6 years ago

I recommend read about tea that I attached here, it's nice and simple. A think as default can be great since we can embed directly without relying on conf/dep of environment of the users, with that we don't have problems with windows users as well, since this can implemented as portable really easily.

ryukinix commented 6 years ago

Do you have any plan to help implement that @kutsan ? Or anyone on this thread.

ryukinix commented 6 years ago

I can try look implement the TEA by myself, the passwordeval way a help can be useful. We need make that a little flexible to change, although seems a simple thing to do.

ryukinix commented 6 years ago

password_eval btw I think it's better since we already have date_format

kutsan commented 6 years ago

password_eval btw I think it's better since we already have date_format

Yes, you are right. I agree with that.

Probably, if I implement the tea, I'd add it as a especial tag like passwordeval=@tea to avoid conflicts on namespace if for some weird reason have a tea binary.

I think password_eval should be only contain commands to be executed. Why bother it with @tea thing since there is only one algorithm to select?

IMO make password_eval empty as default, it would mean TEA will in charge for security.

[login]
username = kutsan
password_eval =

If I see that option and read the docs about it, I can fill it with my own commands to activate that functionality?

[login]
username = kutsan
password_eval = gpg --no-tty --for-your-eyes-only --quiet --decrypt ~/.config/mal/secret.gpg

I recommend read about tea that I attached here, it's nice and simple.

I checked out and got the general idea. It looks cool, thanks for sharing!

Do you have any plan to help implement that @kutsan ?

I wish I could but as I said before I don't have any motivation to learn Python right now. :(

ryukinix commented 6 years ago

IMO make password_eval empty as default, it would mean TEA will in charge for security

Ok, this makes sense and it's even better don't mention tea on the config, can facility hacking.

If I see that option and read the docs about it, I can fill it with my own commands to activate that functionality?

Yes, this indeed is something that the user can look at it and think "hey, there is something strange here... Should I look to the docs". Or they will try so weird thing and break the software and will need read the docs anyway :v -- btw, we need improve this docs... https://mal.readthedocs.io is a lie hahah I just used pandoc to translate the README! So evil... we need change that.

I wish I could but as I said before I don't have any motivation to learn Python right now. :(

Yes... I remember that. That is okay. I probably will delay this a little since I'm pretty busy on college and with a new job... Actually I should being on the bed right now xD ~ I have a cold.

Thanks for all this discussion, I hope this fill your use-case @taktoa

kutsan commented 6 years ago

I probably will delay this a little since I'm pretty busy on college and with a new job... Actually I should being on the bed right now xD ~ I have a cold.

That's okay too. Take care of yourself. ><