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
Obtain spclient access token using login5 instead of keymaster (Fixes #1179 and #1245) #1246
base: dev
Are you sure you want to change the base?
Obtain spclient access token using login5 instead of keymaster (Fixes #1179 and #1245) #1246
Conversation
… of keymaster (Fixes librespot-org#1179)
… of keymaster (Fixes librespot-org#1179)
… of keymaster (Fixes librespot-org#1179)
…ibrespot into auth-token-from-login5 Pulling commit: fix startup log
Bumps [zerocopy](https://github.com/google/zerocopy) from 0.7.26 to 0.7.31. - [Release notes](https://github.com/google/zerocopy/releases) - [Changelog](https://github.com/google/zerocopy/blob/main/CHANGELOG.md) - [Commits](google/zerocopy@v0.7.26...v0.7.31) --- updated-dependencies: - dependency-name: zerocopy dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
data-encoding was already a transitive dependency via tungstenite
This change collects all those audio fetch parameters that were defined as static constants into a dedicated struct, AudioFetchParams. This struct can be read and set statically, allowing a user of the library to modify those parameters without the need to recompile.
Bumps [h2](https://github.com/hyperium/h2) from 0.3.21 to 0.3.24. - [Release notes](https://github.com/hyperium/h2/releases) - [Changelog](https://github.com/hyperium/h2/blob/v0.3.24/CHANGELOG.md) - [Commits](hyperium/h2@v0.3.21...v0.3.24) --- updated-dependencies: - dependency-name: h2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Hey @lmore377! I use this library in my project and faced the same issue. I really want this pull request to be merged ASAP. What's left to do? Maybe I can finish this. |
The review comments on #1220 just need to be looked over and applied here. I've been pretty preoccupied with other things and haven't really gotten around to sitting down and looking at this. One quick note, I noticed that auth broke again the other day even with this patch so there might be something more going on at Spotify's end. I'll check real quick. |
Yeah it seems like all forms of auth (even passing username and password with |
@roderickvd @kingosticks Do you guys have any idea about what could be going on here? |
I don't know, I've been out of this for quite some time now. I checked that log and could be totally wrong, but it seems that librespot is doing a hard fail (shutting down) because it cannot dispatch a "social-connect/v2/session_update" command. That command probably really does not matter, so it should soft fail and ignore. Spotify server-side does different things depending on the type of platform connecting, type of authentication you're using, current visibility of the moon, and the amount of fairy dust in the air, so it could be that authenticating this way triggers a bug that was not visible in librespot before. |
AFAICS this error is being handled and shouldn't cause a hard fail: if let Err(e) = session.dispatch(cmd, data) {
debug!("could not dispatch command: {}", e);
} in struct DispatchTask<S>(S, SessionWeak)
where
S: TryStream<Ok = (u8, Bytes)> + Unpin;
impl<S> Future for DispatchTask<S>
where
S: TryStream<Ok = (u8, Bytes)> + Unpin,
<S as TryStream>::Ok: std::fmt::Debug,
{
type Output = Result<(), S::Error>;
fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
let session = match self.1.try_upgrade() {
Some(session) => session,
None => return Poll::Ready(Ok(())),
};
loop {
let (cmd, data) = match ready!(self.0.try_poll_next_unpin(cx)) {
Some(Ok(t)) => t,
None => {
warn!("Connection to server closed.");
session.shutdown();
return Poll::Ready(Ok(()));
}
Some(Err(e)) => {
session.shutdown();
return Poll::Ready(Err(e));
}
};
if let Err(e) = session.dispatch(cmd, data) {
debug!("could not dispatch command: {}", e);
}
}
}
} I also checked the flow |
I'm also on linux and getting the same authentication problem. It's not a hard fail though, I think the log also includes the gracefully shutting via interruption as well. I believe the relevant part is:
which I can provide the exact part with no-crash but in a non-playable state. important part of the image as text:
I don't know how the client token is relevant to this process but that's the part just before the error and we cannot see any outcome of the client token process. Building main and using no-discover cached authentication, I can verify that the client token step warns (but playable state) via important part of the image as text:
Lastly, I don't know it's relevant but the comment on client token logic indicates on linux the old keymaster id is used.
It might be possible that this new new login5 auth (that replaces keymaster auth) and mix usage of this old keymaster id on linux is causing issues. Edit: Added the relevant part of the images as blocks as well for who are not able to see github images for various reasons. |
@gokberkkocak images are 404 |
Images are github hosted. I edited the comment to add the relevant parts as quote blocks if you are not able to load them. |
I'll try take a look this week. Probably worth checking what the latest official clients are doing. |
Hello, any updates? @kingosticks did you have a chance to take a look? |
I'm sorry, I didn't. I've had a backlog to get through but I'm basically done to the point where librespot is blocking me so I have to get into this again. Will try asap. |
This PR is a copy of #1220 as that one seems stale. This resolves the issue where authentication fails when spotify connect is started from a desktop client.
Fixes #1179 and #1245. Tested and works with clients from Android, iOS, Windows, Linux and macOS.
I still need to resolve the review comments from #1220, I'm just opening this now to keep track of changes.
I'm very inexperienced when it comes to rust so please be gentle 😆
Credits:
@kingosticks - Created the initial branch with the fix
@Gerrelt - Created the PR that this one is based on