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

Use MD5 hash in mopidy.conf instead of plaintext passwords #27

Closed
wants to merge 6 commits into from

Conversation

evamvid
Copy link

@evamvid evamvid commented Sep 14, 2018

This requires the user to put the MD5 of their password in the Scrobbler section of mopidy.conf instead of their plaintext password.

@jjok
Copy link

jjok commented Sep 15, 2018

I think you'd definitely need something in the README explaining how users can create hash of their password.

@jjok
Copy link

jjok commented Sep 21, 2018

Relates to mopidy/mopidy-spotify#65

@kingosticks
Copy link
Member

We should be really clear that the extra security this provides is minimal. If some has your md5 hash, they can likely get your password from that in a reasonable amount of time. Or they could just use the hash as-is to log in as you.

@evamvid
Copy link
Author

evamvid commented Sep 26, 2018

Agreed that this isn't a huge leap in terms of added security.

I guess using OAuth would be better, but can that be done through pyLast? And how do we feel about requiring the user to do setup in the browser? Maybe we'd need to have some sort of installation wizard that users can run for authentication?

@kingosticks
Copy link
Member

Yes, we could use their Web Application OAuth-style flow and have the user save the session key in their Mopidy config. It'd be very similar to what we already require users to do for Spotify and Soundcloud access where they must store the client_secret. pylast appears to support last.fm session keys.

The session key should remain secret and if you don't trust the config file's security (which seems to be the concern but not something I'm personally convinced about) then you are still exposed. However, I do agree it's better since you have not exposed your actual password and can revoke the session key using the last.fm website. Additionally, there isn't much you can actually do through last.fm's API.

However, it would take some work to do this. Proper Mopidy integration with the system keychain might be a better alternative since other extensions could also benefit from that.

@jjok
Copy link

jjok commented Sep 26, 2018

Proper Mopidy integration with the system keychain might be a better alternative since other extensions could also benefit from that.
I don't really know anything about keychains, but it would definitely be good to have one solution that works for all extensions. I think most extensions have a "Password stored in plain text" issue open against them.

@kingosticks
Copy link
Member

Keyrings are kind of supported (see mopidy/mopidy#116 (comment)) but I don't know how this works, if at all, when running Mopidy as a service. I think we had talk of added mopidy config subcommands to make setting passwords more user-friendly.

Maybe the session key support is worth doing here after all...

@jodal
Copy link
Member

jodal commented Dec 14, 2020

Closing this as:

  • I'm not interested in a one-off solution for a single extension,
  • keeping passwords in plain text isn't that bad if you don't reuse passwords for multiple services,
  • hasing with MD5 adds close to zero extra security.

Sorry I didn't respond to this two years ago!

@jodal jodal closed this Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants