Fix OAuth token refresh and persistence issues

- Add show_dialog=true to OAuth URL to ensure fresh refresh tokens
- Prevent token clearing when refresh fails to avoid total auth loss
- Add ConfigManager to SpotifyClient for token persistence
- Fix token handling flow to maintain authentication state

This resolves the issue where tokens worked once then failed permanently
due to missing refresh tokens and aggressive token clearing on errors.
This commit is contained in:
Benjamin Slingo 2025-09-04 09:04:44 -04:00
parent e09e8b2d67
commit 084f2d7cb7
2 changed files with 19 additions and 8 deletions

View file

@ -31,7 +31,8 @@ impl SpotifyAuth {
.append_pair("response_type", "code") .append_pair("response_type", "code")
.append_pair("client_id", &self.config.client_id) .append_pair("client_id", &self.config.client_id)
.append_pair("scope", &self.config.scopes.join(" ")) .append_pair("scope", &self.config.scopes.join(" "))
.append_pair("redirect_uri", &self.config.redirect_uri); .append_pair("redirect_uri", &self.config.redirect_uri)
.append_pair("show_dialog", "true");
if let Some(state) = state { if let Some(state) = state {
url.query_pairs_mut().append_pair("state", state); url.query_pairs_mut().append_pair("state", state);

View file

@ -1,4 +1,5 @@
use crate::auth::SpotifyAuth; use crate::auth::SpotifyAuth;
use crate::config::ConfigManager;
use crate::error::{Result, SpotifyError}; use crate::error::{Result, SpotifyError};
use crate::models::{CurrentTrack, SimplifiedCurrentTrack, SpotifyConfig, TokenInfo}; use crate::models::{CurrentTrack, SimplifiedCurrentTrack, SpotifyConfig, TokenInfo};
use chrono::Utc; use chrono::Utc;
@ -12,10 +13,11 @@ pub struct SpotifyClient {
client: Client, client: Client,
auth: SpotifyAuth, auth: SpotifyAuth,
token_info: Arc<RwLock<Option<TokenInfo>>>, token_info: Arc<RwLock<Option<TokenInfo>>>,
config_manager: ConfigManager,
} }
impl SpotifyClient { impl SpotifyClient {
pub fn new(config: SpotifyConfig) -> Self { pub fn new(config: SpotifyConfig, config_manager: ConfigManager) -> Self {
let client = Client::builder() let client = Client::builder()
.user_agent("SpotifyTracker/1.0") .user_agent("SpotifyTracker/1.0")
.timeout(std::time::Duration::from_secs(30)) .timeout(std::time::Duration::from_secs(30))
@ -29,6 +31,7 @@ impl SpotifyClient {
client, client,
auth, auth,
token_info, token_info,
config_manager,
} }
} }
@ -140,9 +143,9 @@ impl SpotifyClient {
let token_guard = self.token_info.read().await; let token_guard = self.token_info.read().await;
if let Some(token_info) = token_guard.as_ref() { if let Some(token_info) = token_guard.as_ref() {
// Check if token expires in the next 5 minutes // Check if token expires in the next 5 minutes - refresh if within 5 min of expiry
let expires_soon = token_info.expires_at - chrono::Duration::minutes(5); let expires_with_buffer = token_info.expires_at - chrono::Duration::minutes(5);
Utc::now() < expires_soon Utc::now() < expires_with_buffer
} else { } else {
false false
} }
@ -170,13 +173,18 @@ impl SpotifyClient {
log::info!("Refreshing expired token..."); log::info!("Refreshing expired token...");
match self.auth.refresh_token(refresh_token).await { match self.auth.refresh_token(refresh_token).await {
Ok(new_token) => { Ok(new_token) => {
// Save token to disk
if let Err(e) = self.config_manager.save_token(&new_token) {
log::error!("Failed to save refreshed token to disk: {}", e);
}
*token_guard = Some(new_token); *token_guard = Some(new_token);
log::info!("Token refreshed successfully"); log::info!("Token refreshed successfully");
return Ok(()); return Ok(());
} }
Err(e) => { Err(e) => {
log::error!("Failed to refresh token: {}", e); log::error!("Failed to refresh token: {}", e);
*token_guard = None; // Keep the existing token rather than clearing it entirely
// This allows manual re-auth instead of losing the token completely
return Err(e); return Err(e);
} }
} }
@ -207,7 +215,8 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn test_client_creation() { async fn test_client_creation() {
let config = SpotifyConfig::default(); let config = SpotifyConfig::default();
let client = SpotifyClient::new(config); let config_manager = ConfigManager::new().unwrap();
let client = SpotifyClient::new(config, config_manager);
assert!(!client.is_token_valid().await); assert!(!client.is_token_valid().await);
} }
@ -215,7 +224,8 @@ mod tests {
#[test] #[test]
fn test_authorization_url() { fn test_authorization_url() {
let config = SpotifyConfig::default(); let config = SpotifyConfig::default();
let client = SpotifyClient::new(config); let config_manager = ConfigManager::new().unwrap();
let client = SpotifyClient::new(config, config_manager);
let url = client.get_authorization_url(Some("test_state")); let url = client.get_authorization_url(Some("test_state"));
assert!(url.contains("spotify.com")); assert!(url.contains("spotify.com"));