-
Notifications
You must be signed in to change notification settings - Fork 647
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
Token error for some scopes #1331
Comments
The payload numbers are ASCII codes. Maybe they give some idea what’s going on. |
The payload results in I have some update on this in general. I have two different use cases where I request tokens for some scopes by librespot's
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 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: Maybe this is the expected way and the issue can get closed. |
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 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 I'll also add, it's not worth spending much time on keymaster since it's the outdated way of getting a token. |
AFAIR I have checked for all these scopes: user-read-playback-state 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. |
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 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. |
I'm not sure I completely follow. Can you post something like a Gist with two full working examples? |
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. |
tl;dr:
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, 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, 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. 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). |
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. |
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
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
That's the only place I have found BTW, within the same function
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
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: |
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 (#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. |
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? |
I don't want to keep this issue pending so I added a PR that fixes the issue for me. Providing two similar |
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. |
I have the following understanding: Current dev (v0.6.0-dev?):
Release version v0.5.0:
If this PR is seen as a possible patch for v0.5.0 (as I'm using it), the original For future development (v0.6.0), only the get_token example is using the In any case, librespot I'm fine with both: |
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 |
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 |
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. Edit:
would do the trick somehow when providing |
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. |
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. |
My feeling is, having both functions Maybe I should update the comment above the two functions to clearly point out that |
I have updated the comment for function 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 #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. |
Merged, thanks! |
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
librespot
on):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
The text was updated successfully, but these errors were encountered: