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

Loading user credentials from keyring. #65

Closed
wants to merge 2 commits into from
Closed

Loading user credentials from keyring. #65

wants to merge 2 commits into from

Conversation

robin92
Copy link

@robin92 robin92 commented Aug 30, 2015

Username & password values in configuration are now optional. When configuration lacks user credentials application will try fetching data from keyring.

When configuration lacks user credentials application will try fetching
data from keyring.
@adamcik
Copy link
Member

adamcik commented Aug 30, 2015

Awesome that you are looking at this, we've been (partially) missing this for quite some time. Did you know that mopidy actually already supports getting config values from the keyring? We've had that for ages, but lost interest so we never added commands to populate the keyring, nor did we document it.

mopidy/mopidy#116
mopidy/mopidy#587

@adamcik
Copy link
Member

adamcik commented Aug 30, 2015

And looking more closely at you PR I do see you are using our helpers already :-)

@robin92
Copy link
Author

robin92 commented Aug 30, 2015

@adamcik
Sure thing, I needed this feature and was really happy when found that most of implementation (dbus) is already done. As soon as this is merged I may try to implement the same feature for other plugins.

@adamcik
Copy link
Member

adamcik commented Aug 30, 2015

https://github.com/mopidy/mopidy/blob/develop/mopidy/config/__init__.py#L80 is the line of interest in our existing code. This loads and merges config files, defaults, keyring data and finally any commandline overrides. The intention of this was that extensions should not have to be aware of if the data comes from the keyring or one of the other sources.

@robin92
Copy link
Author

robin92 commented Aug 30, 2015

@adamcik
Mhm, I see. Your approach seems much better. I can handle this, but I guess this PR should be abandoned then, shouldn't it?

@adamcik
Copy link
Member

adamcik commented Aug 30, 2015

Probably, as it if we can fix it once and for all in Mopidy it would at least save you having to update the rest of the extensions :-)

@jodal
Copy link
Member

jodal commented Aug 30, 2015

If you want to, you can force-push to the same branch to replace the commits here with another implementation.

@robin92
Copy link
Author

robin92 commented Aug 30, 2015

It looks like the only needed change in here is to set username & password to optional in config schema and implement behaviour for these fields not set at all.

@adamcik
Copy link
Member

adamcik commented Aug 30, 2015

My memory for this part of everything might be a bit rusty, but I think we still want it to be mandatory, as we can't run without login credentials. And if they come from the keyring or the config file should make no difference to the config validation.

On the other hand, if the intent is to support the spotify auth blob, then making it optional makes a lot of sense. But then we would "just" need a mopidy spotify ... sub-command for authenticating and getting the initial auth blob (I think we might have an open issue for this, but didn't check).

@robin92
Copy link
Author

robin92 commented Aug 30, 2015

@adamcik
You're right. So this feature has in fact already been implemented. I'm closing this PR, will take a look at auth_blob though.

@robin92 robin92 closed this Aug 30, 2015
@Wqrld
Copy link

Wqrld commented Apr 17, 2017

@robin92 can you explain to me how to set that up? (encrypted)

@robin92
Copy link
Author

robin92 commented Apr 19, 2017

@aycgit
I have something like this in my mopidy.conf:

[spotify]
username =
password =

and then the following entries in my keyring

[/org/freedesktop/secrets/collection/login/11748]
attribute.section = spotify
attribute.service = mopidy
attribute.key = password
secret = 12345
[/org/freedesktop/secrets/collection/login/11747]
attribute.section = spotify
attribute.service = mopidy
attribute.key = username
secret = spotify_user

What mopidy does is matching (section, option) from config file with (attribute.section, attribute.key) from keyring entries. Secret stored by each entry becomes the value. It works for any parameter supported by mopidy, not only for credentials.

This is equivalent to explicitly specifying credentials in the config file:

[spotify]
username = spotify_user
password = 12345

Please note that only entries with attribute.service = mopidy are visible to the application.

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