beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.94k stars 1.82k forks source link

Use OAuth for MusicBrainz authentication #216

Open sampsyo opened 11 years ago

sampsyo commented 11 years ago

MusicBrainz has added an option for OAuth (rather than username/password) authentication. This is way more secure, so we should move over posthaste.

nogweii commented 11 years ago

My concern here is normal OAuth flow - where will you store the secrets? Generating a global set for Beets means thta you're at risk for abuse. However, there really isn't anything much better than copying the same workflow from the acoustid plugin. Ah well.

sampsyo commented 11 years ago

Yes, such is the problem with token-based authentication in open-source desktop software.

adamcik commented 8 years ago

In the case of supporting Spotify's web APIs over auth in mopidy-spotify-web I ended creating https://github.com/adamcik/oauthclientbridge which we host under mopidy.com This allows us to register once and have a stable redirect end point for Spotify's OAuth. Saving end users from having to register, and the secrets stays safe on our server. What we give the client instead is a "Client Credentials Grant" in OAuth speak, basically a user/pw which can be used to get/refresh the OAuth token.

This might also be applicable for the use case you had in mind here.

sampsyo commented 8 years ago

Cool! This is a nifty idea; thanks.

adamcik commented 8 years ago

An other alternative would just be to ask musicbrainz to support Client Credentials Grants directly (or whatever OAuth grant type best fits) and avoid having any bridge like we have to with Spotify :-)

opatel99 commented 8 years ago

Since consumption of the MusicBrainz API is done through https://github.com/alastair/python-musicbrainzngs, OAuth implementation would need to be implemented there first.

Corresponding Issue: https://github.com/alastair/python-musicbrainzngs/issues/89

tigranl commented 7 years ago

I would like to get this task

sampsyo commented 7 years ago

Hi, @tigranl! That's great! Welcome to the beets project and to GCI.

I think the first step here is to get a proof of concept working using a Python library for OAuth. I recommend the library called rauth: https://rauth.readthedocs.io/en/latest/

Can you start by setting up a simple example script that uses rauth to authenticate with the MusicBrainz API? It would be helpful to go through that module's documentation. It might also be helpful to glean some parameters from other clients for the MusicBrainz OAuth2 API, like this one: https://github.com/LordSputnik/passport-musicbrainz-oauth2/blob/master/lib/passport-musicbrainz-oauth2/strategy.js

Once that's working, we can try integrating this strategy into the mbcollection plugin that currently uses password authentication. We'll need to look into implementing the strategy in the python-musicbrainzngs library or in our own code.

tigranl commented 7 years ago

Hi, @sampsyo. I have sent you an example script.

sampsyo commented 7 years ago

Great! This is a good start.

Next, I think we'll need to set up a way to get the "OOB" code that MusicBrainz produces. According to this documentation, that works by setting the redirect URI to urn:ietf:wg:oauth:2.0:oob. We'll then need to ask the user for input to get the code and send it back to the server to verify. You can see a similar workflow going on in the Beatport plugin, which uses OAuth1: https://github.com/beetbox/beets/blob/9de27c6b70d460a43839728704332fc71587191c/beetsplug/beatport.py#L303

In the future, could you please paste code here? Either in a Markdown code block or in a linked Gist will do nicely. That way, everyone in the beets community can see and help out.

tigranl commented 7 years ago

Why do I get "Mismatched redirect URI" when I use urn:ietf:wg:oauth:2.0:oob? I set up installed application type tokens.

sampsyo commented 7 years ago

It's hard to say! Can you please include the full error output and the code that produced it? Is there a traceback?

tigranl commented 7 years ago

Error: invalid_request Mismatched redirect URI And that's all.

sampsyo commented 7 years ago

OK, and can you please include the code that produced the error? What's the Python program you're running, and how did you invoke it?

FWIW, I think the error you're seeing is coming from here in the MusicBrainz server code: https://github.com/metabrainz/musicbrainz-server/blob/60ece227912132de3da59d6cf7132dcf3dbdfc89/lib/MusicBrainz/Server/Controller/OAuth2.pm#L51

Here's the point in that code that the server checks for the OOB parameter: https://github.com/metabrainz/musicbrainz-server/blob/60ece227912132de3da59d6cf7132dcf3dbdfc89/lib/MusicBrainz/Server/Controller/OAuth2.pm#L263

tigranl commented 7 years ago

I forgot to delete slash from redirect_uri string, can you imagine? Sorry to bother you. So final script is https://gist.github.com/tigranl/edcdb8b9f7770d0d918ab14273724fa6

sampsyo commented 7 years ago

Awesome; thank you! I ran python3 test.py and it worked great—when I complete the authentication, it prints my profile info. :sparkles:

To finish up this task, the last step is to integrate this into the mbcollection plugin. Using the beatport plugin as a guide, can you try adding this authentication code to the setup stage of mbcollection? You can open a pull request with even some initial, non-working code and we'll go from there.

Also, there's no need to preserve any backwards compatibility with password-based authentication. We'll sort that out after you're done.

Thanks again for working on this!

tigranl commented 7 years ago

According to https://github.com/beetbox/beets/blob/db782a2404fa8a6827c10a6536b4a960d19af135/docs/plugins/mbcollection.rst user needs to add password and login to configuration file. In our case we must get user's token from console input. Am I right?

sampsyo commented 7 years ago

Yes! Just like in the beatport plugin, we'll need to call the beets.ui.input_ function to get the token from the user interactively.

tigranl commented 7 years ago

@sampsyo I have been struggling with rauth error for 2 hours. It throws AttributeError: 'str' object has no attribute 'get'. Code: https://gist.github.com/tigranl/f6725c2d6e76dab891f3447bd34f3ed6 And without a decoder lambda it returns KeyError: 'Decoder failed to handle access_token with data as returned by provider. A different decoder may be needed.

sampsyo commented 7 years ago

Hi, @tigranl! When reporting exceptions, it's really important to include the full traceback. That way, we can see where errors are coming from. In other words… http://i.imgur.com/jacoj.jpg :smiley:

Here, it's probably coming from the session.get call. Can you try printing out the value of session to see if it matches what you expect?

tigranl commented 7 years ago

@sampsyo My pull request https://github.com/beetbox/beets/pull/2298

tigranl commented 7 years ago

Please, review it. I have 4 hours left to submit my work.

Louson commented 1 year ago

Hello, based on what I saw here and in #2298, I have suggested a PR to musicbrainzngs

I have not chosen the same oauth library because it does not support revokation but the usage is quite the same.

If this is accepted, it should be easy to include OAuth2 support in beets