Dev

psst-core/src/oauth.rs

@@ -1,7 +1,8 @@
 use crate::error::Error;
 use oauth2::{
-    basic::BasicClient, reqwest::http_client, AuthUrl, AuthorizationCode, ClientId, CsrfToken,
-    PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, RefreshToken, Scope, TokenResponse, TokenUrl,
+    basic::BasicClient, reqwest::http_client, AuthType, AuthUrl, AuthorizationCode, ClientId,
+    CsrfToken, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, RefreshToken, Scope,
+    TokenResponse, TokenUrl,
 };
 use std::{
     io::{BufRead, BufReader, Write},
@@ -152,11 +153,13 @@ fn create_spotify_oauth_client(redirect_port: u16) -> BasicClient {
 
     BasicClient::new(
         ClientId::new(crate::session::access_token::CLIENT_ID.to_string()),
-        None,
+        None, // No client secret for PKCE flow
         AuthUrl::new("https://accounts.spotify.com/authorize".to_string()).unwrap(),
         Some(TokenUrl::new("https://accounts.spotify.com/api/token".to_string()).unwrap()),
     )
     .set_redirect_uri(RedirectUrl::new(redirect_uri).expect("Invalid redirect URL"))
+    // Use RequestBody auth type to include client_id in the body (required for PKCE)
+    .set_auth_type(AuthType::RequestBody)
 }
 
 pub fn generate_auth_url(redirect_port: u16) -> (String, PkceCodeVerifier) {
@@ -204,14 +207,29 @@ fn get_scopes() -> Vec<Scope> {
 /// Refresh an access token using a stored refresh token. Returns the new access token and
 /// an optional new refresh token if Spotify rotates it.
 pub fn refresh_access_token(refresh_token: &str) -> Result<(String, Option<String>), Error> {
-    // Reuse the same OAuth client configuration; redirect URI is irrelevant for refresh flow.
-    let client = create_spotify_oauth_client(0);
+    // For PKCE flow, Spotify requires client_id in the request body (not Basic auth header).
+    // We create a minimal client just for refresh - redirect URI is not needed for refresh flow.
+    let client = BasicClient::new(
+        ClientId::new(crate::session::access_token::CLIENT_ID.to_string()),
+        None, // No client secret for PKCE
+        AuthUrl::new("https://accounts.spotify.com/authorize".to_string()).unwrap(),
+        Some(TokenUrl::new("https://accounts.spotify.com/api/token".to_string()).unwrap()),
+    )
+    // Use RequestBody auth type to include client_id in the body instead of Authorization header
+    .set_auth_type(AuthType::RequestBody);
 
+    log::debug!("Attempting OAuth token refresh with client_id: {}", crate::session::access_token::CLIENT_ID);
+
     let token_response = client
         .exchange_refresh_token(&RefreshToken::new(refresh_token.to_string()))
         .request(http_client)
-        .map_err(|e| Error::OAuthError(format!("Failed to refresh token: {e}")))?;
+        .map_err(|e| {
+            // Log detailed error for debugging
+            log::error!("OAuth refresh failed: {:?}", e);
+            Error::OAuthError(format!("Failed to refresh token: {e}"))
+        })?;
 
+    log::info!("OAuth token refresh successful");
     let access = token_response.access_token().secret().to_string();
     let refresh = token_response
         .refresh_token()

psst-gui/src/cmd.rs

@@ -33,6 +33,7 @@ pub const FIND_IN_SAVED_TRACKS: Selector<Find> = Selector::new("find-in-saved-tr
 // Session
 pub const SESSION_CONNECT: Selector = Selector::new("app.session-connect");
 pub const LOG_OUT: Selector = Selector::new("app.log-out");
+pub const OAUTH_AUTH_REQUIRED: Selector = Selector::new("app.oauth-auth-required");
 
 // Navigation
 pub const NAVIGATE: Selector<Nav> = Selector::new("app.navigates");

psst-gui/src/controller/keybinds.rs

@@ -0,0 +1,195 @@
+use druid::{
+    commands,
+    widget::{prelude::*, Controller},
+};
+
+use crate::{
+    cmd,
+    data::{config::KeybindAction, AppState, Nav, QueueBehavior},
+};
+
+/// Controller that handles global keybinds
+pub struct KeybindsController;
+
+impl KeybindsController {
+    pub fn new() -> Self {
+        Self
+    }
+
+    fn handle_keybind_action(
+        &self,
+        ctx: &mut EventCtx,
+        action: KeybindAction,
+        data: &mut AppState,
+    ) {
+        match action {
+            // Playback controls
+            KeybindAction::PlayPause => {
+                ctx.submit_command(cmd::PLAY_PAUSE);
+            }
+            KeybindAction::Play => {
+                ctx.submit_command(cmd::PLAY_RESUME);
+            }
+            KeybindAction::Pause => {
+                ctx.submit_command(cmd::PLAY_PAUSE);
+            }
+            KeybindAction::Next => {
+                ctx.submit_command(cmd::PLAY_NEXT);
+            }
+            KeybindAction::Previous => {
+                ctx.submit_command(cmd::PLAY_PREVIOUS);
+            }
+            KeybindAction::SeekForward => {
+                // Seeking is handled by PlaybackController
+            }
+            KeybindAction::SeekBackward => {
+                // Seeking is handled by PlaybackController
+            }
+            KeybindAction::VolumeUp => {
+                data.playback.volume = (data.playback.volume + 0.1).min(1.0);
+            }
+            KeybindAction::VolumeDown => {
+                data.playback.volume = (data.playback.volume - 0.1).max(0.0);
+            }
+            KeybindAction::Stop => {
+                ctx.submit_command(cmd::PLAY_STOP);
+            }
+
+            // Navigation
+            KeybindAction::NavigateHome => {
+                ctx.submit_command(cmd::NAVIGATE.with(Nav::Home));
+            }
+            KeybindAction::NavigateSavedTracks => {
+                ctx.submit_command(cmd::NAVIGATE.with(Nav::SavedTracks));
+            }
+            KeybindAction::NavigateSavedAlbums => {
+                ctx.submit_command(cmd::NAVIGATE.with(Nav::SavedAlbums));
+            }
+            KeybindAction::NavigateShows => {
+                ctx.submit_command(cmd::NAVIGATE.with(Nav::Shows));
+            }
+            KeybindAction::NavigateSearch => {
+                ctx.submit_command(cmd::SET_FOCUS.to(cmd::WIDGET_SEARCH_INPUT));
+            }
+            KeybindAction::NavigateBack => {
+                ctx.submit_command(cmd::NAVIGATE_BACK.with(1));
+            }
+            KeybindAction::NavigateRefresh => {
+                ctx.submit_command(cmd::NAVIGATE_REFRESH);
+            }
+
+            // UI Controls
+            KeybindAction::ToggleSidebar => {
+                data.config.sidebar_visible = !data.config.sidebar_visible;
+                data.config.save();
+            }
+            KeybindAction::ToggleLyrics => {
+                ctx.submit_command(cmd::TOGGLE_LYRICS);
+            }
+            KeybindAction::OpenPreferences => {
+                ctx.submit_command(commands::SHOW_PREFERENCES);
+            }
+            KeybindAction::CloseWindow => {
+                ctx.submit_command(cmd::CLOSE_ALL_WINDOWS);
+            }
+            KeybindAction::ToggleFinder => {
+                ctx.submit_command(cmd::TOGGLE_FINDER);
+            }
+            KeybindAction::FocusSearch => {
+                ctx.submit_command(cmd::SET_FOCUS.to(cmd::WIDGET_SEARCH_INPUT));
+            }
+
+            // Queue controls
+            KeybindAction::QueueBehaviorSequential => {
+                ctx.submit_command(cmd::PLAY_QUEUE_BEHAVIOR.with(QueueBehavior::Sequential));
+            }
+            KeybindAction::QueueBehaviorRandom => {
+                ctx.submit_command(cmd::PLAY_QUEUE_BEHAVIOR.with(QueueBehavior::Random));
+            }
+            KeybindAction::QueueBehaviorLoopTrack => {
+                ctx.submit_command(cmd::PLAY_QUEUE_BEHAVIOR.with(QueueBehavior::LoopTrack));
+            }
+            KeybindAction::QueueBehaviorLoopAll => {
+                ctx.submit_command(cmd::PLAY_QUEUE_BEHAVIOR.with(QueueBehavior::LoopAll));
+            }
+        }
+
+        self.post_action_feedback(data, action);
+    }
+
+    fn post_action_feedback(&self, data: &mut AppState, action: KeybindAction) {
+        if let Some(message) = Self::feedback_message(action, data) {
+            data.info_alert(message);
+        }
+    }
+
+    fn feedback_message(action: KeybindAction, data: &AppState) -> Option<String> {
+        let message = match action {
+            KeybindAction::VolumeUp | KeybindAction::VolumeDown => {
+                let volume = (data.playback.volume * 100.0).round() as i32;
+                format!("Volume: {volume}%")
+            }
+            KeybindAction::ToggleSidebar => {
+                if data.config.sidebar_visible {
+                    "Sidebar visible".to_string()
+                } else {
+                    "Sidebar hidden".to_string()
+                }
+            }
+            KeybindAction::QueueBehaviorSequential => "Queue: Sequential".to_string(),
+            KeybindAction::QueueBehaviorRandom => "Queue: Random".to_string(),
+            KeybindAction::QueueBehaviorLoopTrack => "Queue: Loop Track".to_string(),
+            KeybindAction::QueueBehaviorLoopAll => "Queue: Loop All".to_string(),
+            _ => return None,
+        };
+
+        Some(message)
+    }
+}
+
+impl<W> Controller<AppState, W> for KeybindsController
+where
+    W: Widget<AppState>,
+{
+    fn event(
+        &mut self,
+        child: &mut W,
+        ctx: &mut EventCtx,
+        event: &Event,
+        data: &mut AppState,
+        env: &Env,
+    ) {
+        if let Event::Command(cmd) = event {
+            if let Some(action) = cmd.get(cmd::PERFORM_KEYBIND_ACTION) {
+                self.handle_keybind_action(ctx, *action, data);
+                ctx.set_handled();
+                return;
+            }
+        }
+
+        if let Event::KeyDown(key_event) = event {
+            // Check if this key event matches any configured keybind
+            if let Some(action) = data.config.keybinds.find_action(key_event) {
+                // Handle certain actions that should not override default behavior
+                let should_handle = match action {
+                    // Don't override Space and arrow keys if they're already being handled
+                    // by PlaybackController
+                    KeybindAction::PlayPause
+                    | KeybindAction::SeekForward
+                    | KeybindAction::SeekBackward
+                    | KeybindAction::Next
+                    | KeybindAction::Previous => false,
+                    _ => true,
+                };
+
+                if should_handle {
+                    self.handle_keybind_action(ctx, action, data);
+                    ctx.set_handled();
+                    return;
+                }
+            }
+        }
+
+        child.event(ctx, event, data, env);
+    }
+}

psst-gui/src/delegate.rs

@@ -226,6 +226,12 @@ impl AppDelegate<AppState> for Delegate {
                 true,
             );
             Handled::Yes
+        } else if cmd.is(cmd::OAUTH_AUTH_REQUIRED) {
+            // Token refresh failed - clear invalid tokens and prompt user to re-login
+            log::info!("OAuth re-authentication required. Clearing invalid tokens.");
+            TokenUtils::clear_all(&data.session, &mut data.config, true);
+            data.error_alert("Your session has expired. Please go to Settings and log in again.");
+            Handled::Yes
         } else if let Some(file_info) = cmd.get(commands::OPEN_FILE) {
             let context = self
                 .pending_open_dialog

psst-gui/src/main.rs

@@ -59,6 +59,7 @@ fn main() {
     )
     .install_as_global();
 
+    // Try to refresh OAuth token at startup if we have a refresh token
     if let Some(refresh_token) = state.config.oauth_refresh_token.clone() {
         match refresh_access_token(&refresh_token) {
             Ok((access_token, maybe_refresh_token)) => {
@@ -71,15 +72,19 @@ fn main() {
                 );
             }
             Err(e) => {
+                // Token refresh failed - the refresh token is likely revoked or invalid.
+                // Clear all tokens and credentials so the user is prompted to log in again.
                 log::warn!(
-                    "Failed to refresh OAuth token at startup: {e}. Falling back to persisted access token if any."
+                    "Failed to refresh OAuth token at startup: {e}. Clearing invalid session."
                 );
-                // Install tokens from config into runtime holders as-is
-                TokenUtils::install_from_config(&state.session, &state.config);
+                TokenUtils::clear_all(&state.session, &mut state.config, true);
+                state.config.clear_credentials();
+                state.config.save();
             }
         }
-    } else {
-        // No refresh token; install any persisted tokens as-is
+    } else if state.config.oauth_bearer.is_some() {
+        // We have an access token but no refresh token - try to use it
+        // (It will fail if expired, and WebApi will request re-auth)
         TokenUtils::install_from_config(&state.session, &state.config);
     }
 

psst-gui/src/ui/preferences.rs

@@ -997,7 +997,8 @@ impl Authenticate {
 fn logout_and_reset(ctx: &mut EventCtx, data: &mut AppState) {
     data.config.clear_credentials();
     crate::token_utils::TokenUtils::clear_all(&data.session, &mut data.config, true);
-    data.session.shutdown();
+    // Note: Don't call data.session.shutdown() here as it blocks the UI thread.
+    // The session will be cleaned up when a new session is created or app exits.
     ctx.submit_command(cmd::CLOSE_ALL_WINDOWS);
     ctx.submit_command(cmd::SHOW_ACCOUNT_SETUP);
 }

psst-gui/src/webapi/client.rs

@@ -134,30 +134,58 @@ impl WebApi {
             Ok(resp) => resp,
             Err(ureq::Error::StatusCode(code)) if code == 401 || code == 403 => {
                 if let Some(rtok) = self.oauth_refresh_token.lock().clone() {
-                    if let Ok((new_access, new_refresh)) = refresh_access_token(&rtok) {
-                        *self.oauth_bearer.lock() = Some(new_access.clone());
-                        {
-                            let mut refresh_lock = self.oauth_refresh_token.lock();
-                            if let Some(ref r) = new_refresh {
-                                *refresh_lock = Some(r.clone());
+                    match refresh_access_token(&rtok) {
+                        Ok((new_access, new_refresh)) => {
+                            *self.oauth_bearer.lock() = Some(new_access.clone());
+                            {
+                                let mut refresh_lock = self.oauth_refresh_token.lock();
+                                if let Some(ref r) = new_refresh {
+                                    *refresh_lock = Some(r.clone());
+                                }
                             }
+                            if let Some(sink) = self.event_sink.lock().as_ref().cloned() {
+                                let payload = (new_access.clone(), new_refresh.clone());
+                                if let Err(err) = sink.submit_command(
+                                    cmd::OAUTH_TOKENS_REFRESHED,
+                                    payload,
+                                    Target::Global,
+                                ) {
+                                    log::warn!("failed to submit OAuth refresh command to UI: {err}");
+                                }
+                            }
+                            call(&new_access)?
                         }
-                        if let Some(sink) = self.event_sink.lock().as_ref().cloned() {
-                            let payload = (new_access.clone(), new_refresh.clone());
-                            if let Err(err) = sink.submit_command(
-                                cmd::OAUTH_TOKENS_REFRESHED,
-                                payload,
-                                Target::Global,
-                            ) {
-                                log::warn!("failed to submit OAuth refresh command to UI: {err}");
+                        Err(refresh_err) => {
+                            log::warn!("OAuth token refresh failed: {refresh_err}. Clearing tokens and requesting re-authentication.");
+                            // Clear the invalid tokens
+                            *self.oauth_bearer.lock() = None;
+                            *self.oauth_refresh_token.lock() = None;
+                            // Notify the UI that re-authentication is required
+                            if let Some(sink) = self.event_sink.lock().as_ref().cloned() {
+                                if let Err(err) = sink.submit_command(
+                                    cmd::OAUTH_AUTH_REQUIRED,
+                                    (),
+                                    Target::Global,
+                                ) {
+                                    log::warn!("failed to submit OAuth auth required command to UI: {err}");
+                                }
                             }
+                            return Err(Error::WebApiError("Session expired. Please log in again.".to_string()));
                         }
-                        call(&new_access)?
-                    } else {
-                        return Err(Error::WebApiError("Failed to refresh token".to_string()));
                     }
                 } else {
-                    return Err(Error::WebApiError("Missing refresh token".to_string()));
+                    // No refresh token available, request re-authentication
+                    log::warn!("No refresh token available. Requesting re-authentication.");
+                    if let Some(sink) = self.event_sink.lock().as_ref().cloned() {
+                        if let Err(err) = sink.submit_command(
+                            cmd::OAUTH_AUTH_REQUIRED,
+                            (),
+                            Target::Global,
+                        ) {
+                            log::warn!("failed to submit OAuth auth required command to UI: {err}");
+                        }
+                    }
+                    return Err(Error::WebApiError("Session expired. Please log in again.".to_string()));
                 }
             }
             Err(err) => return Err(Error::WebApiError(err.to_string())),