mirror of
https://github.com/librespot-org/librespot.git
synced 2026-04-27 08:15:50 +03:00
[GH-ISSUE #1331] Token error for some scopes #609
Labels
No labels
A-Alsa
SpotifyAPI
Tokio 1.0
audio
bug
can't reproduce
compilation
dependencies
duplicate
enhancement
good first issue
help wanted
high priority
imported
imported
invalid
new api
pull-request
question
reverse engineering
wiki
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/librespot#609
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @fivebanger on GitHub (Sep 8, 2024).
Original GitHub issue: https://github.com/librespot-org/librespot/issues/1331
Describe the bug
I have compiled a recent dev-version of Librelspot (3rd Sep., build with --release) and tried to get a token for different scopes. Some scopes returns an error instead of a valid token. I have used a single scope always, not a list of scopes. Following scopes results in an error:
user-top-read
user-read-recently-played
user-read-playback-state
user-modify-playback-state
user-read-currently-playing
To reproduce
Steps to reproduce the behavior:
Log
A log sequence for scope "user-read-currently-playing":
Host (what you are running
librespoton):Additional context
Although I'm running Libresopt on Linux, I can see in the log that the ANDROID_CLIENT_ID from config.rs is used.
Edit:
Steps to reproduce updated
@roderickvd commented on GitHub (Sep 16, 2024):
The payload numbers are ASCII codes. Maybe they give some idea what’s going on.
@fivebanger commented on GitHub (Sep 19, 2024):
The payload results in
{"code":3,"errorDescription":"Invalid scope"}.I have some update on this in general. I have two different use cases where I request tokens for some scopes by librespot's
get_token()function:Use case 1 is based on librespot's player example, i.e. I'm opening a session, loading a track and start to play it. I would call this "session-approach".
Use case 2 is more or less librespot's main loop where I connect to librespot by using the Spotify app (Android). I would call this "connect-approach".
For use case 1 (session-approach) I don't see any issue, I get a token for whatever scope I request it for.
For use use case 2 (connect-approach) I see the afore described error. The error is not returned for the connect-approach when using v0.4.2, whereas I have to provide my own client-id in that case when calling
get_token().I have read a comment in some issue discussion here that not all scopes are allowed for all client-ids. Unfortunately I cannot find this comment anymore. Maybe the observed behavior is expected and this is not an issue at all but just a different behavior compared to v0.4.2.
Just wondering (related to connect-approach): Even though when changing the values in config.rs
to e.g. (all KEYMASTER_CLIENT_ID)
I can see in the log that the ANDROID_CLIENT_ID is used anyway when connected by Spotify app:
hm://keymaster/token/authenticated?scope=user-read-currently-playing&client_id=9a8d2f0ce77a4e248bb71fefcb557637&device_id=XXX. I have not checked the same for the session-approach. Just wondering...Maybe this is the expected way and the issue can get closed.
@kingosticks commented on GitHub (Sep 19, 2024):
When using connect-approach, do any scopes work?
0.4 and 0.5 do work differently w.r.t session client id. When using connect in 0.5, it uses the controller's client ID for the
session.client_id, which will be coming from your android app. This makes sense and I'm presuming that is the correct behaviour (and we were doing it wrong before) but I don't know.It could be that the android client id isn't allowed to use keymaster at all, or is only allowed to use it for certain scopes. Or something else related to the type of session it is. Why not try hardcoding the client id used by
TokenProvider?I'll also add, it's not worth spending much time on keymaster since it's the outdated way of getting a token.
@fivebanger commented on GitHub (Sep 19, 2024):
AFAIR I have checked for all these scopes:
user-read-playback-state
user-modify-playback-state
user-read-currently-playing
app-remote-control
streaming
playlist-read-private
playlist-read-collaborative
playlist-modify-private
playlist-modify-public
user-follow-modify
user-follow-read
user-read-playback-position
user-top-read
user-read-recently-played
user-library-modify
user-library-read
user-read-email
user-read-private
Need to double check exactly when needed. But yes, the connect approach works for these scopes, beside the mentioned ones before which returns an error.
@fivebanger commented on GitHub (Sep 19, 2024):
I have tried to prepare some debug code to demonstrate the issue but now it gets really wired.... I have now two flavors of debug code, a working one and a failing one. I have placed the
get_token()debug code in two different places inside librespot's main loop in main.rs:First place:
Second place:
I started with putting the debug code only in the first place because I assume that's the time when we have a valid connection. I was surprised not getting an error for scope_fail.
Then I put my debug code only in the second place since that fits more to my connect-approach use case. I'm asking from time to time for a new token. This leads to an error for scope_fail.
Then I wanted to see that the problem occurs not in the first place but in the second place and put the debug code in both. Surprisingly the scope_fail case works at second place, if I ask for a token at the first place before.
This is reproducible in my environment.
@roderickvd commented on GitHub (Sep 20, 2024):
I'm not sure I completely follow. Can you post something like a Gist with two full working examples?
@fivebanger commented on GitHub (Sep 21, 2024):
I have added some debug code to my fork here: https://github.com/fivebanger/librespot/tree/token_error
I'll follow up on this next week but what I can tell so far: When hard code the keymaster's ID in TokenProvider::get_token() like already proposed I don't see an error with respect to different scopes. My understanding is that this is the same like get_token(client_id) in v0.4.2 in case I would have provided the keymaster's ID. I'll also check with my personal client_id's I have requested from Spotify some years ago.
@fivebanger commented on GitHub (Sep 22, 2024):
tl;dr:
TokenProvider::get_token(None), client_id evaluates to client_id = self.session().client_id() inside TokenProvider::get_token() (same as of current v0.5.0). When callingTokenProvider::get_token(string), the string is used inside TokenProvider::get_token(). A caller from outside can provide any appropriate client_id when calling get_token().More detailed logs (for code, pls. refer to https://github.com/fivebanger/librespot/tree/token_error):
Case 1: Start librespot regular and ask for a token on ctrl+c:
No additional TokenProvider::get_token() initiated by user at start-up,
First get_token() was initiated by librespot itself.
Client ID used for first token request = self.session().client_id() = ANDROID_CLIENT_ID.
Client ID for scopes = "user-read-playback-state" used on ctr+c = self.session().client_id() = ANDROID_CLIENT_ID.
Result: get_token() for scope "user-read-playback-state" fails.
Case 2: Start librespot and initiate a get_token() by user on connect. Ask for a token on ctrl+c:
Additional TokenProvider::get_token() for scopes = "user-read-playback-state" at start-up,
First get_token() was initiated by user on connect.
Client ID used for for scopes = "user-read-playback-state" token request = self.session().client_id() = KEYMASTER_CLIENT_ID (most likely the initial value, have not followed up on that).
get_token() for scopes = "user-read-playback-state" on ctr+c was already cached before.
Result: get_token() for scope "user-read-playback-state" ok (will fail probably once the cached token has expired).
Case 3: Start librespot regular and ask for a token on ctrl+c but using hard coded client_id = KEYMASTER_CLIENT_ID:
No additional TokenProvider::get_token() initiated by user at start-up.
First get_token() was initiated by librespot itself.
Client ID used for first token request = self.session().client_id() = ANDROID_CLIENT_ID.
Client ID for scopes = "user-read-playback-state" used on ctr+c = KEYMASTER_CLIENT_ID (hard coded).
Result: get_token() for scope "user-read-playback-state" ok.
I'm wondering why librespot itself asks always three times for a token (scopes = "playlist-read") on startup and why the second get_token() call cannot get covered by a cached token already (like seen for the third call).
@kingosticks commented on GitHub (Sep 22, 2024):
We just want to do what the desktop app does really. If someone who was interested could connect to the official client from an android device and trace the http requests and find.out what ID it uses.
This is seemingly a bug, we've had for a while. Please fix if you can.
@fivebanger commented on GitHub (Sep 22, 2024):
I doubt that's beyond my current skills... But even if, I think that's not really helpfull. My understanding is:
Librespot inits the client_id at startup according to the OS its running on, see
config.rs:That's why I see client_id = KEYMASTER_CLIENT_ID when calling get_token() direktly on connect.
The client_id gets updated when librespot receives an update frame, which comes from Spotify most likely? See
SpircTask,handle_remote_update()That's the only place I have found
set_client_id(). So the client_id used further on comes from Spotify, nothing to tweak or change at this point. Even a http trace as you have proposed would not change that fact, I guess.BTW, within the same function
handle_remote_update()we check for a changed client_id which then will emit an event.My understanding of the intention here is to signal that another device has connected to the outside. But if Spotify returns ANDROID_CLIENT_ID for (all?) Andriod devices, how will the if() statement ever becomes "true" in case just another Android device connects? Maybe client_id defaults to OS specific during a re-connect? I cannot check since I don't have another Spotify account yet.
BTW 2, due to
impl SessionConfig {...}I doubt the get_token example would never run on an Android device once compiled for Android? The client_id would default to ANDROID_CLIENT_ID which does not allow all used scopes, e.g. "user-read-playback-state".I absolutely appreciate to implement librespot according to the desktop app. However, is it worth to follow the discussion about an alternative implementation of TokenProvider::get_token() here? Something like
or simply an additional function like
This would not change any flow inside standard librespot but would enable an user sitting on top of the source code to request a token according to its needed scopes and maybe also using its own client_id. If this discussion is not desired I would like to stop here regarding the get_token() error issue since the error is then just an observation with respect to the current implementation.
Maybe better to open an additional issue to not mix topics?
Edit:
I just realized that the get_token() example has changed. So maybe my doubts are not correct at all, forget about this.
@kingosticks commented on GitHub (Sep 23, 2024):
No, we are free to ignore that client ID update if we decide that's more useful to us. This is exactly what happens in 0.4.
I think putting it back to how it worked in 0.4 is reasonable (your second example?). But keep in mind that the desktop app doesn't use keymaster any more and we will integrate the new method soon as it's ready (https://github.com/librespot-org/librespot/pull/1344). After that, maybe we keep keymaster, or maybe we don't. Personally, I think it would be more useful to have a look at that new Login5-based approach and ensure it handles client ID in a way that works for you.
Yes please.
@fivebanger commented on GitHub (Sep 23, 2024):
With respect to tracing the desktop app's traffic: Maybe the original app ignores the client_id update and still uses the KEYMASTER_ID which could be figured out by tracing the traffic?
I just realized that there is a lot of change going on with respect to authentication under the hood. I'm confused currently by Oauth and login5 and whether the one is an alternative for the other or if these are completely different topics and what keymaster means in this context.
As someone using librespot as kind of a lib with a small wrapper around (to be able to use Python's subprocess to communicate with librespot) I have more or less two requirements:
My current understanding is that TokenProvider::get_token() gets maybe removed in future and is considered to be deprecated (from Spotify's side?). So it would be more future-proof to check the PR for login5 you have mentioned? And there is no need to dig into that TokenProvider::get_token() topic since it's deprecated? I'll start to get familiar with the changes of the login5 PR....
Before opening a new issue, what exactly you do consider to be a bug? The fact that librespot asks three times at startup for a token? Or the fact that asking the second time does not use a cached token? Or both?
@fivebanger commented on GitHub (Oct 29, 2024):
I don't want to keep this issue pending so I added a PR that fixes the issue for me. Providing two similar
get_token()functions is maybe not the smartest way but fixes theget_token()error for some scopes and client_ids and does not break any existingget_token()function call.@roderickvd commented on GitHub (Oct 29, 2024):
Next release is going to be 0.6.0 anyway - a SemVer breaking change - so if you think it's cleaner to break the API then you can? Let me know what you want.
@fivebanger commented on GitHub (Oct 30, 2024):
I have the following understanding:
Current dev (v0.6.0-dev?):
get_token()is not used any longer by librespot core,login5().auth_token()is used instead.get_token()is only used by the get_token example.Release version v0.5.0:
get_token()is used by librespot core.get_token()is also used by the get_token example.If this PR is seen as a possible patch for v0.5.0 (as I'm using it), the original
get_token()function needs to remain, or theget_token()function insidespclientneeds to be changed as well in order to provide a client_id. Changingspclientwill not work using this PR todev, sinceget_token()is no longer used. That's at least my understanding.For future development (v0.6.0), only the get_token example is using the
get_token()function. I have seen that it's possible to provide a customclient_idin the get_token example as well:session_config.client_id = args[2].clone(). Even though I have doubts that this approach may not work in any case, sincesession_config.client_idis just an initial value that may get overwritten byset_client_id()insession.rswhich seems to get called in current dev. Pls. correct me when I'm wrong.That's maybey not an issue at all for the concrete get_token example since
set_client_id()is most likely never called in that case. I just want to point out from a coding perspective (I'm still struggling with the get_token example, can't get it to work with a token obtained by calling./librespot --enable-oauth --system-cache ./xxx --cache ./xxx. Most likely I'm not understanding the flow correctly. But therefore I cannot proof my before mentioned doubts).In any case, librespot
Sessionstill uses a client_id inSessionDatawhich seems to get used also in dev. This has an impact to the currentget_token()function as this internalclient_idis provided as of today. Does @kingosticks have a preference with respect to a solutionget_token(scopes, client_id)vs. PRget_token(scopes)andget_token_with_client_id(scopes, client_id)in parallel?I'm fine with both:
get_token(scopes, client_id)for dev which breaks the current get_token example and also with this PR, providingget_token()andget_token_with_client_id()in parallel. My personal preference is providing both,get_token()andget_token_with_client_id()since the PR can easily get used as a patch for stable v0.5.0 in case anyone else runs into the get_token error issue.@kingosticks commented on GitHub (Oct 30, 2024):
Remember that librespot is really a library. People can pick and choose what they use. What's currently used by us in our binary or our examples, are all just examples of how it can be used.
@roderickvd said above that next release will be 0.6 i.e not a patch release. There's no problem making breaking changes in this situation. We don't need to worry about backwards compatibility or random other forks or people who stick using old versions. We should focus on providing the most useful API.
It works fine in the case you're not using Connect (spirc). As I already said, librespot is a library where people can pick and choose what parts they use. This example is just about getting a token because that's a fundamental and useful thing to do. The current examples can be changed however we want/need, they are just examples to help people using/debugging/testing librespot. We don't need to worry about changing them, just do it.
I think if I was using librespot to do something really simple I'd appreciate not having to worry about finding a
client_idthat works, so I think that aspect ofget_token(scopes)is nice. However, given that it apparently doesn't work in some situations, I think it's better to replace it withget_token(scopes, client_id)and make people think a little more. We can mention in the documentation that Spotify's client_id is available and have an example which uses it.@kingosticks commented on GitHub (Oct 30, 2024):
Do we have a simple function that provides Spotify's platform-specific client_id? That would be a useful thing to also provide if we do decide to change the params for
get_token(), that would make life easier to anyone upgrading from 0.5.@fivebanger commented on GitHub (Oct 30, 2024):
returns
always when running librespot on Linux. Should be always this client_id, beside running librespot on IOS or Android (according to the code).
But that's not a generic way to get a client_id with respect to a given os, e.g.
get_client_id_for_os(os). Something similar seems not to be implemented.Edit:
Forget about my last sentence, I think this
would do the trick somehow when providing
os.@roderickvd commented on GitHub (Oct 30, 2024):
Props for the productive conversation. Though it's not the end of it, through it all it does seem it'd be smart to have both functions.
get_tokenjust being a convenience method which is quite common to Rust.@roderickvd commented on GitHub (Oct 30, 2024):
Didn't mean that as a statement to kill the conversation, so if you think differently do not hesitate to put it on the table.
@fivebanger commented on GitHub (Oct 30, 2024):
My feeling is, having both functions
get_token()andget_token_with_client_id()provides the most flexibility: One function for convenience and one in case someone wants to provide a custom client_id (or simply theKEYMASTER_CLIENT_ID) for whatever reason, e.g. because the combination of scopes/client_id returns an error when callingget_token().Maybe I should update the comment above the two functions to clearly point out that
get_token()may return an error in case of a specific combination of scopes and client_id.@fivebanger commented on GitHub (Oct 30, 2024):
I have updated the comment for function
get_token().BTW, there was another issue revealed when digging into the original problem with respect to getting tokens at startup. There were some changes with the login5 PR https://github.com/librespot-org/librespot/pull/1344 that fixes the issue with additional new token request when I get it correct. I still can see that the current dev requests a token several times during startup. But a new token is generated only one time at asrtup. I consider this issue to be fixed therefore.
@roderickvd commented on GitHub (Oct 30, 2024):
Merged, thanks!