From ee90c4b63b46425a60c3b261347d5e8ab1ab03fe Mon Sep 17 00:00:00 2001
From: Isaac Lins <contact@isaaclins.com>
Date: Wed, 26 Nov 2025 13:58:06 +0100
Subject: [PATCH 1/4] fix(oauth): use RequestBody auth type for Spotify PKCE
 token refresh

- Add AuthType::RequestBody to OAuth client for PKCE compliance
- Spotify requires client_id in request body (not Basic auth header) for PKCE flow
- Add OAUTH_AUTH_REQUIRED command to handle expired session gracefully
- Improve error logging for OAuth refresh failures
- WebApi client now emits re-auth command when token refresh fails
---
 psst-core/src/oauth.rs              |  30 ++++-
 psst-gui/src/cmd.rs                 |   1 +
 psst-gui/src/controller/keybinds.rs | 195 ++++++++++++++++++++++++++++
 psst-gui/src/delegate.rs            |   5 +
 psst-gui/src/main.rs                |   4 +-
 psst-gui/src/webapi/client.rs       |  64 ++++++---
 6 files changed, 274 insertions(+), 25 deletions(-)
 create mode 100644 psst-gui/src/controller/keybinds.rs

diff --git a/psst-core/src/oauth.rs b/psst-core/src/oauth.rs
index 7f8bdb32..226b75d5 100644
--- a/psst-core/src/oauth.rs
+++ b/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()
diff --git a/psst-gui/src/cmd.rs b/psst-gui/src/cmd.rs
index 6d3eefd6..52640774 100644
--- a/psst-gui/src/cmd.rs
+++ b/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");
diff --git a/psst-gui/src/controller/keybinds.rs b/psst-gui/src/controller/keybinds.rs
new file mode 100644
index 00000000..f79a215c
--- /dev/null
+++ b/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);
+    }
+}
diff --git a/psst-gui/src/delegate.rs b/psst-gui/src/delegate.rs
index e4793c36..18d206bc 100644
--- a/psst-gui/src/delegate.rs
+++ b/psst-gui/src/delegate.rs
@@ -226,6 +226,11 @@ impl AppDelegate<AppState> for Delegate {
                 true,
             );
             Handled::Yes
+        } else if cmd.is(cmd::OAUTH_AUTH_REQUIRED) {
+            // Token refresh failed - show error and prompt user to re-login via preferences
+            log::info!("OAuth re-authentication required.");
+            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
diff --git a/psst-gui/src/main.rs b/psst-gui/src/main.rs
index 938197e4..b69e2a4b 100644
--- a/psst-gui/src/main.rs
+++ b/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)) => {
@@ -74,7 +75,8 @@ fn main() {
                 log::warn!(
                     "Failed to refresh OAuth token at startup: {e}. Falling back to persisted access token if any."
                 );
-                // Install tokens from config into runtime holders as-is
+                // Fall back to any persisted tokens - they might still work
+                // If they don't, the WebApi client will handle re-auth
                 TokenUtils::install_from_config(&state.session, &state.config);
             }
         }
diff --git a/psst-gui/src/webapi/client.rs b/psst-gui/src/webapi/client.rs
index 2c83e842..bf0d9631 100644
--- a/psst-gui/src/webapi/client.rs
+++ b/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())),

From 93fad3bf34158aa5c750cdd83e6facf025ecee43 Mon Sep 17 00:00:00 2001
From: Isaac Lins <contact@isaaclins.com>
Date: Wed, 26 Nov 2025 16:06:48 +0100
Subject: [PATCH 2/4] fix(oauth): clear persisted tokens when re-auth is
 required

Previously, OAUTH_AUTH_REQUIRED only showed an error but didn't clear
the invalid tokens from config. This caused an infinite error loop on
restart as the old invalid tokens would be reloaded.
---
 psst-gui/src/delegate.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/psst-gui/src/delegate.rs b/psst-gui/src/delegate.rs
index 18d206bc..1c4afddf 100644
--- a/psst-gui/src/delegate.rs
+++ b/psst-gui/src/delegate.rs
@@ -227,8 +227,9 @@ impl AppDelegate<AppState> for Delegate {
             );
             Handled::Yes
         } else if cmd.is(cmd::OAUTH_AUTH_REQUIRED) {
-            // Token refresh failed - show error and prompt user to re-login via preferences
-            log::info!("OAuth re-authentication 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) {

From a866332d54434b7d2f0a61a76b53f92ad0263604 Mon Sep 17 00:00:00 2001
From: Isaac Lins <contact@isaaclins.com>
Date: Wed, 26 Nov 2025 17:03:10 +0100
Subject: [PATCH 3/4] fix(logout): remove blocking session.shutdown() call

The session.shutdown() method calls worker.join() which blocks the UI
thread, causing the app to freeze during logout. Removed the blocking
call - session cleanup happens naturally on new login or app exit.
---
 psst-gui/src/ui/preferences.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/psst-gui/src/ui/preferences.rs b/psst-gui/src/ui/preferences.rs
index ebbe77dd..fecdd9be 100644
--- a/psst-gui/src/ui/preferences.rs
+++ b/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);
 }

From 0fed3cada9f405d6aed993d60cb27ce20c2b24b1 Mon Sep 17 00:00:00 2001
From: Isaac Lins <contact@isaaclins.com>
Date: Tue, 2 Dec 2025 14:28:07 +0100
Subject: [PATCH 4/4] fix(startup): clear invalid tokens when refresh fails at
 startup

When OAuth token refresh fails at startup (e.g., revoked token), now
properly clears credentials and tokens so user sees login screen
instead of falling back to invalid tokens that cause repeated errors.
---
 psst-gui/src/main.rs | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/psst-gui/src/main.rs b/psst-gui/src/main.rs
index b69e2a4b..6c3739ad 100644
--- a/psst-gui/src/main.rs
+++ b/psst-gui/src/main.rs
@@ -72,16 +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."
                 );
-                // Fall back to any persisted tokens - they might still work
-                // If they don't, the WebApi client will handle re-auth
-                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);
     }