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.

Sign up for GitHub

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

Kingosticks change: Obtain spclient access token using login5 instead of keymaster (Fixes librespot-org#1179) #1220

Closed
wants to merge 7 commits into from
Closed

Kingosticks change: Obtain spclient access token using login5 instead of keymaster (Fixes librespot-org#1179) #1220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
96e02fd
Kingosticks change: Obtain spclient access token using login5 instead…
Gerrelt Nov 13, 2023
ba08273
Kingosticks change: Obtain spclient access token using login5 instead…
Gerrelt Dec 1, 2023
f065908
Merge branch 'librespot-org:dev' into auth-token-from-login5
Gerrelt Dec 1, 2023
a54cc8f
Merge branch 'librespot-org:dev' into auth-token-from-login5
Gerrelt Dec 4, 2023
f815318
Kingosticks change: Obtain spclient access token using login5 instead…
Gerrelt Dec 4, 2023
40c2a4d
Merge branch 'librespot-org:dev' into auth-token-from-login5
Gerrelt Dec 9, 2023
87dddf2
Merge branch 'auth-token-from-login5' of https://github.com/Gerrelt/l…
Gerrelt Dec 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions core/src/session.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ struct SessionData {
client_brand_name: String,
client_model_name: String,
connection_id: String,
auth_blob: Vec<u8>,
time_delta: i64,
invalid: bool,
user_data: UserData,
Expand Down Expand Up @@ -174,6 +175,7 @@ impl Session {

info!("Authenticated as \"{}\" !", reusable_credentials.username);
self.set_username(&reusable_credentials.username);
self.set_auth_blob(&reusable_credentials.auth_data);
if let Some(cache) = self.cache() {
if store_credentials {
let cred_changed = cache
Expand Down Expand Up @@ -471,6 +473,14 @@ impl Session {
self.0.data.write().user_data.canonical_username = username.to_owned();
}

pub fn auth_blob(&self) -> Vec<u8> {
self.0.data.read().auth_blob.clone()
}

pub fn set_auth_blob(&self, auth_blob: &[u8]) {
self.0.data.write().auth_blob = auth_blob.to_owned();
}

pub fn country(&self) -> String {
self.0.data.read().user_data.country.clone()
}
Expand Down
106 changes: 94 additions & 12 deletions core/src/spclient.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use crate::{
},
connect::PutStateRequest,
extended_metadata::BatchedEntityRequest,
login5::{LoginRequest, LoginResponse},
},
token::Token,
version::spotify_version,
Expand All @@ -43,6 +44,7 @@ component! {
accesspoint: Option<SocketAddress> = None,
strategy: RequestStrategy = RequestStrategy::default(),
client_token: Option<Token> = None,
auth_token: Option<Token> = None,
}
}

Expand Down Expand Up @@ -148,6 +150,91 @@ impl SpClient {
Ok(())
}

async fn auth_token_request<M: Message>(&self, message: &M) -> Result<Bytes, Error> {
let client_token = self.client_token().await?;
let body = message.write_to_bytes()?;

let request = Request::builder()
.method(&Method::POST)
.uri("https://login5.spotify.com/v3/login")
.header(ACCEPT, HeaderValue::from_static("application/x-protobuf"))
.header(CLIENT_TOKEN, HeaderValue::from_str(&client_token)?)
.body(Body::from(body))?;

self.session().http_client().request_body(request).await
}

pub async fn auth_token(&self) -> Result<Token, Error> {
let auth_token = self.lock(|inner| {
if let Some(token) = &inner.auth_token {
if token.is_expired() {
inner.auth_token = None;
}
}
inner.auth_token.clone()
});

if let Some(auth_token) = auth_token {
return Ok(auth_token);
}

let client_id = match OS {
"macos" | "windows" => self.session().client_id(),
_ => SessionConfig::default().client_id,
};

let mut login_request = LoginRequest::new();
login_request.client_info.mut_or_insert_default().client_id = client_id;
login_request.client_info.mut_or_insert_default().device_id =
self.session().device_id().to_string();

let stored_credential = login_request.mut_stored_credential();
stored_credential.username = self.session().username().to_string();
stored_credential.data = self.session().auth_blob().clone();

let mut response = self.auth_token_request(&login_request).await?;
let mut count = 0;
const MAX_TRIES: u8 = 3;

let token_response = loop {
count += 1;

let message = LoginResponse::parse_from_bytes(&response)?;
// TODO: Handle hash cash stuff
if message.has_ok() {
break message.ok().to_owned();
}

if count < MAX_TRIES {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this would be improved to check for the error cases where a retry is not sensible:

enum LoginError {
    UNKNOWN_ERROR = 0;
    INVALID_CREDENTIALS = 1;
    BAD_REQUEST = 2;
    UNSUPPORTED_LOGIN_PROTOCOL = 3;
    TIMEOUT = 4;
    UNKNOWN_IDENTIFIER = 5;
    TOO_MANY_ATTEMPTS = 6;
    INVALID_PHONENUMBER = 7;
    TRY_AGAIN_LATER = 8;
}

Copy link
Contributor

@kingosticks kingosticks Dec 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the very least, a retry in the face of BAD_REQUEST or a TOO_MANY_ATTEMPTS error response is a bit rude.

Copy link
Contributor

@kingosticks kingosticks Dec 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simple exponential backup would be nice too, ideally implemented in a reusable way. But could leave that to another day, I'm just trying to keep a record of what was left unfinished.

EDIT: Actually, there is a solid handling of the HTTP side of this in our HttpClient . Given this is protobuf over HTTP, it depends what Spotify do. i.e. do they reflect protobuf errors in the HTTP response? I don't think I ever looked into that. @roderickvd do you know?

response = self.auth_token_request(&login_request).await?;
} else {
return Err(Error::failed_precondition(format!(
"Unable to solve any of {MAX_TRIES} hash cash challenges"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As is, there is still no hash cash handling so this error message makes no sense.

)));
}
};

let auth_token = Token {
access_token: token_response.access_token.clone(),
expires_in: Duration::from_secs(
token_response
.access_token_expires_in
.try_into()
.unwrap_or(3600),
),
token_type: "Bearer".to_string(),
scopes: vec![],
timestamp: Instant::now(),
};
self.lock(|inner| {
inner.auth_token = Some(auth_token.clone());
});

trace!("Got auth token: {:?}", auth_token);

Ok(auth_token)
}

async fn client_token_request<M: Message>(&self, message: &M) -> Result<Bytes, Error> {
let body = message.write_to_bytes()?;

Expand Down Expand Up @@ -462,27 +549,22 @@ impl SpClient {
.body(Body::from(body.to_owned()))?;

// Reconnection logic: keep getting (cached) tokens because they might have expired.
let token = self
.session()
.token_provider()
.get_token("playlist-read")
.await?;
let auth_token = self.auth_token().await?;
Copy link
Contributor

@kingosticks kingosticks Dec 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also probably check the result of this and break the loop if it failed.

EDIT: Cancel that, I forgot what ? does. But maybe that's actually the wrong thing to do given "currently these endpoints seem to work fine without it"...


let headers_mut = request.headers_mut();
if let Some(ref hdrs) = headers {
*headers_mut = hdrs.clone();
}
headers_mut.insert(
AUTHORIZATION,
HeaderValue::from_str(&format!("{} {}", token.token_type, token.access_token,))?,
HeaderValue::from_str(&format!(
"{} {}",
auth_token.token_type, auth_token.access_token,
))?,
);

if let Ok(client_token) = self.client_token().await {
headers_mut.insert(CLIENT_TOKEN, HeaderValue::from_str(&client_token)?);
} else {
// currently these endpoints seem to work fine without it
warn!("Unable to get client token. Trying to continue without...");
}
let client_token = self.client_token().await?;
headers_mut.insert(CLIENT_TOKEN, HeaderValue::from_str(&client_token)?);
Copy link
Contributor

@kingosticks kingosticks Dec 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes how we handle the failing case. The old code used to continue anyway, maybe that's still what we want. I don't remember why I thought I had to change it...


last_response = self.session().http_client().request_body(request).await;

Expand Down
7 changes: 7 additions & 0 deletions protocol/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ fn compile() {
proto_dir.join("playlist_permission.proto"),
proto_dir.join("playlist4_external.proto"),
proto_dir.join("spotify/clienttoken/v0/clienttoken_http.proto"),
proto_dir.join("spotify/login5/v3/challenges/code.proto"),
proto_dir.join("spotify/login5/v3/challenges/hashcash.proto"),
proto_dir.join("spotify/login5/v3/client_info.proto"),
proto_dir.join("spotify/login5/v3/credentials/credentials.proto"),
proto_dir.join("spotify/login5/v3/identifiers/identifiers.proto"),
proto_dir.join("spotify/login5/v3/login5.proto"),
proto_dir.join("spotify/login5/v3/user_info.proto"),
proto_dir.join("storage-resolve.proto"),
proto_dir.join("user_attributes.proto"),
// TODO: remove these legacy protobufs when we are on the new API completely
Expand Down