search

librespot-org / librespot

Open Source Spotify client library
MIT License
4.7k stars 573 forks source link

[Discovery] Support add user with access token #1100

Closed GiviMAD closed 1 year ago

GiviMAD commented 1 year ago

Hi, I have added this changes in order to support adding the client to my account using and access token with the "streaming" scope through the addUser action. It's inspired by the project https://github.com/TimotheeGerber/spotify-connect.

Do you think this can this be merged agains the 0.4.x version and included in the next 0.4.x release? Or is better that I open a pr agains the dev branch?

Hope the code is ok I don't have too much experience with rust.

Regards!

Related to: https://github.com/librespot-org/librespot/issues/1086#issuecomment-1381871262 https://github.com/librespot-org/librespot/pull/1098

kingosticks commented 1 year ago

Do you know what software uses the access token variant other than the spotify-connect tool?

GiviMAD commented 1 year ago

No. I want to use it for this project https://github.com/GiviMAD/openhab-addons/tree/marketplace/bundles/org.openhab.voice.habspeaker.

I think that will be also nice to allow to create the credential files manually with a Spotify token. Because right now it assumes it's a credential with password if it's not a reusable one.

But I think that this is one is more flexible as it allows an easy to implement remote way to authenticate the speaker.

Already using it and seems to work nicely.

As a side topic, the get info call returns an empty string for the field "activeUser". If we expose there the active user id, a remote system will be able to know if the speaker is already authenticated, and also to detect that the credentials are not longer valid because it can check the user id for its active credentials, this way it will now that it should remove the current speaker credentials (witch is currently not possible to do but I read some where that you can do that by sending a delete to the addUser method, so maybe was possible on other implementations or in the dev branch?) and adding the speaker to the new user account.

Does this makes sense to you?

It's actually not needed for my project because I'm doing those checks by accessing the credentials file, and I'm spawning librespot from my application so I can restart it without any trouble, but I think that is a something nice to have to allow people to handle this remotely.

Regards!

GiviMAD commented 1 year ago

To give some context, in my project I keep a valid Spotify token centralized in the server (openHAB) that is obtained from a refresh token you obtain once through the oAuth Spotify authentication flow. This token is always synchronized to the connected clients (a web ui that is connected to the server through ws) so they can use the Web Playback SDK. In the electron version this same token is used to authenticate the built-in Librespot client into the user account.

roderickvd commented 1 year ago

Hi everyone, thanks for this!

Do you think this can this be merged agains the 0.4.x version and included in the next 0.4.x release? Or is better that I open a pr agains the dev branch?

Please base it on dev. The old-api (0.4.x) is in maintenance-mode only.

As a side topic, the get info call returns an empty string for the field "activeUser". If we expose there the active user id, a remote system will be able to know if the speaker is already authenticated, and also to detect that the credentials are not longer valid because it can check the user id for its active credentials, this way it will now that it should remove the current speaker credentials (witch is currently not possible to do but I read some where that you can do that by sending a delete to the addUser method, so maybe was possible on other implementations or in the dev branch?) and adding the speaker to the new user account.

Does this makes sense to you?

Absolutely. This is also on my list, but not immediately trivial so not something I was planning to take up anytime soon. Any work on this would be highly welcomed.

GiviMAD commented 1 year ago

Hi everyone, thanks for this!

Do you think this can this be merged agains the 0.4.x version and included in the next 0.4.x release? Or is better that I open a pr agains the dev branch?

Please base it on dev. The old-api (0.4.x) is in maintenance-mode only.

As a side topic, the get info call returns an empty string for the field "activeUser". If we expose there the active user id, a remote system will be able to know if the speaker is already authenticated, and also to detect that the credentials are not longer valid because it can check the user id for its active credentials, this way it will now that it should remove the current speaker credentials (witch is currently not possible to do but I read some where that you can do that by sending a delete to the addUser method, so maybe was possible on other implementations or in the dev branch?) and adding the speaker to the new user account. Does this makes sense to you?

Absolutely. This is also on my list, but not immediately trivial so not something I was planning to take up anytime soon. Any work on this would be highly welcomed.

Sure, I'll rebase this work to dev, and I will try to add the rest of the functionality mentioned there, thank you!

kingosticks commented 1 year ago

There's some more background at https://github.com/TimotheeGerber/spotify-connect/issues/1

GiviMAD commented 1 year ago

Didn't have time to look at it, I'll reopen it when I have some time, thank you for the feedback!