fix: allow OAuth and RHSSO auth to be cancelled (#23844)

src/main/kotlin/com/redhat/devtools/gateway/auth/session/AbstractAuthSessionManager.kt

@@ -13,141 +13,182 @@ package com.redhat.devtools.gateway.auth.session
 
 import com.intellij.openapi.diagnostic.thisLogger
 import com.redhat.devtools.gateway.auth.code.JBPasswordSafeTokenStorage
+import com.redhat.devtools.gateway.auth.code.Parameters
 import com.redhat.devtools.gateway.auth.code.SSOToken
 import com.redhat.devtools.gateway.auth.code.SecureTokenStorage
 import com.redhat.devtools.gateway.auth.server.CallbackServer
-import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.TimeoutCancellationException
+import kotlinx.coroutines.cancelAndJoin
+import kotlinx.coroutines.isActive
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withTimeout
-import java.util.concurrent.atomic.AtomicBoolean
+import java.net.URI
 import kotlin.time.Duration.Companion.milliseconds
 
 /**
  * Abstract base class for authentication session managers.
  *
- * Provides common functionality for OAuth-based authentication including:
- * - Token storage and retrieval with expiration handling
- * - Login state management with concurrent access protection
- * - Session listener notifications
- * - Callback server lifecycle management
- *
- * ## Thread Safety
- * This class is designed for concurrent access:
- * - Login state is protected by [AtomicBoolean]
- * - Token access is protected by [Mutex]
- * - Listener notifications are fail-safe and copy-on-iterate
- *
- * ## Subclass Responsibilities
- * Implementations must provide:
- * - [initialize]: One-time setup on plugin startup
- * - [startLogin]: Provider-specific OAuth flow initialization
- * - [loginWithCredentials]: Provider-specific credential-based login (or throw UnsupportedOperationException)
- * - [callbackServer]: The callback server instance for OAuth flows
- *
- * @param tokenStorage Storage mechanism for persisting tokens securely. Defaults to [JBPasswordSafeTokenStorage]
+ * Browser logins use a single [BrowserLogin] at a time.
+ * Starting a new login or calling [BrowserLogin.cancel] ends any previous in-flight login.
  */
 abstract class AbstractAuthSessionManager(
     protected val tokenStorage: SecureTokenStorage = JBPasswordSafeTokenStorage()
 ) : AuthSessionManager {
 
     protected abstract val callbackServer: CallbackServer
 
-    private val listeners = mutableSetOf<AuthSessionListener>()
+    companion object {
+        const val LOGIN_TIMEOUT_MS = 2 * 60_000L
+    }
+
     private val tokenMutex = Mutex()
-    private var _currentToken: SSOToken? = null
+    private val loginMutex = Mutex()
+    private val loginScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
 
+    private var _currentToken: SSOToken? = null
     protected var currentToken: SSOToken?
         get() = _currentToken
         set(value) { _currentToken = value }
 
-    protected val loginInProgress = AtomicBoolean(false)
-    protected var pendingLogin: CompletableDeferred<SSOToken>? = null
+    private var activeLogin: BrowserLogin? = null
 
     /**
-     * Checks if a login operation is currently in progress.
-     *
-     * @return true if login is in progress, false otherwise
+     * Creates a [BrowserLogin] and registers it as active. Caller must run inside [withBrowserLoginLock].
      */
-    fun isLoginInProgress(): Boolean = loginInProgress.get()
+    protected fun createBrowserLogin(authorizationUri: URI): BrowserLogin {
+        val login = BrowserLogin.create(this, authorizationUri)
+        activeLogin = login
+        return login
+    }
 
     /**
-     * Adds a session change listener.
-     * Thread-safe and can be called during listener notification.
-     *
-     * @param listener The listener to add
+     * Stops the current browser login, if any. Caller must run inside [withBrowserLoginLock].
      */
-    fun addListener(listener: AuthSessionListener) {
-        synchronized(listeners) {
-            listeners += listener
+    protected suspend fun endActiveLogin() {
+        val previous = activeLogin ?: return
+        thisLogger().debug("Ending in-flight browser login")
+        previous.job?.cancelAndJoin()
+        previous.result.takeIf { !it.isCompleted }?.completeExceptionally(SsoLoginException.Cancelled())
+        callbackServer.stop()
+        activeLogin = null
+    }
+
+    internal suspend fun awaitBrowserLoginResult(login: BrowserLogin, timeoutMs: Long): SSOToken {
+        check(activeLogin === login) { "Login is no longer active" }
+        thisLogger().debug("Awaiting browser login result with timeout ${timeoutMs}ms")
+        return try {
+            val token = withTimeout(timeoutMs.milliseconds) {
+                login.result.await()
+            }
+            thisLogger().info("Login result received successfully")
+            token
+        } catch (e: TimeoutCancellationException) {
+            thisLogger().warn("Login timed out after ${timeoutMs}ms")
+            cancelBrowserLogin(login)
+            throw SsoLoginException.Timeout()
         }
     }
 
-    /**
-     * Removes a session change listener.
-     * Thread-safe and can be called during listener notification.
-     *
-     * @param listener The listener to remove
-     */
-    fun removeListener(listener: AuthSessionListener) {
-        synchronized(listeners) {
-            listeners -= listener
+    internal suspend fun cancelBrowserLogin(login: BrowserLogin) = withBrowserLoginLock {
+        if (activeLogin !== login) {
+            thisLogger().debug("Ignoring cancel for stale browser login")
+            return@withBrowserLoginLock
         }
+        thisLogger().debug("Cancelling browser login")
+        login.job?.cancelAndJoin()
+        login.result.takeIf { !it.isCompleted }?.completeExceptionally(SsoLoginException.Cancelled())
+        callbackServer.stop()
+        activeLogin = null
     }
 
-    /**
-     * Notifies all registered listeners of a session change.
-     * Notification failures are logged but do not prevent other listeners from being notified.
-     */
-    protected fun notifyChanged() {
-        val listenersCopy = synchronized(listeners) { listeners.toList() }
-        listenersCopy.forEach { listener ->
+    protected fun launchCallbackHandler(
+        login: BrowserLogin,
+        callbackTimeoutMs: Long,
+        onCallbackTimeout: () -> Unit,
+        handleCallback: suspend (Parameters) -> SSOToken,
+    ) {
+        login.job = loginScope.launch {
             try {
-                listener.sessionChanged()
+                thisLogger().debug("Waiting for OAuth callback...")
+                val params = callbackServer.awaitCallback(callbackTimeoutMs)
+                if (params == null) {
+                    if (!login.result.isCompleted) {
+                        val error = if (isActive) {
+                            SsoLoginException.Timeout()
+                        } else {
+                            SsoLoginException.Cancelled()
+                        }
+                        login.result.completeExceptionally(error)
+                    }
+                    if (isActive) {
+                        thisLogger().warn("OAuth callback timed out")
+                        onCallbackTimeout()
+                    }
+                    return@launch
+                }
+
+                thisLogger().debug("OAuth callback received, handling...")
+                val token = handleCallback(params)
+                currentToken = token
+                completeLoginSuccess(login, token)
+            } catch (e: CancellationException) {
+                completeLoginCancelled(login)
+                throw e
             } catch (e: Exception) {
-                thisLogger().error("Session listener notification failed", e)
+                thisLogger().error("Browser login failed", e)
+                completeLoginFailed(login, e)
+            } finally {
+                withBrowserLoginLock { releaseActiveLogin(login) }
             }
         }
     }
 
-    /**
-     * Awaits the result of a login operation started via [startLogin].
-     *
-     * @param timeoutMs Maximum time to wait in milliseconds
-     * @return The received SSO token
-     * @throws IllegalStateException if login was not started
-     * @throws SsoLoginException.Timeout if the operation times out
-     */
-    override suspend fun awaitLoginResult(timeoutMs: Long): SSOToken {
-        val deferred = pendingLogin ?: throw IllegalStateException("Login was not started")
-        thisLogger().debug("Awaiting login result with timeout ${timeoutMs}ms")
-        return try {
-            val token = withTimeout(timeoutMs.milliseconds) { deferred.await() }
-            thisLogger().info("Login result received successfully")
-            token
-        } catch (e: TimeoutCancellationException) {
-            thisLogger().warn("Login timed out after ${timeoutMs}ms")
-            throw SsoLoginException.Timeout()
+    protected fun completeLoginSuccess(login: BrowserLogin, token: SSOToken) {
+        if (!login.result.isCompleted) {
+            login.result.complete(token)
+        }
+    }
+
+    protected fun completeLoginFailed(login: BrowserLogin, e: Exception) {
+        if (login.result.isCompleted) return
+        val error = when (e) {
+            is SsoLoginException -> e
+            else -> SsoLoginException.Failed(e.message ?: "Login failed")
+        }
+        login.result.completeExceptionally(error)
+    }
+
+    protected fun completeLoginCancelled(login: BrowserLogin) {
+        if (!login.result.isCompleted) {
+            login.result.completeExceptionally(SsoLoginException.Cancelled())
         }
     }
 
     /**
-     * Cancels the current login operation and cleans up resources.
+     * Stops the callback server and clears [activeLogin] when [login] is still current.
+     * Caller must run inside [withBrowserLoginLock].
      */
-    protected suspend fun cancelLogin() {
-        thisLogger().debug("Cancelling login")
-        loginInProgress.set(false)
-        notifyChanged()
+    protected suspend fun releaseActiveLogin(login: BrowserLogin) {
+        if (activeLogin !== login) return
+        thisLogger().debug("Releasing browser login")
         callbackServer.stop()
+        activeLogin = null
     }
 
-    /**
-     * Returns a valid (non-expired) token or null.
-     * If the current token is expired, automatically logs out and returns null.
-     *
-     * @return A valid token or null if not logged in or token is expired
-     */
+    protected suspend fun failBrowserLoginStart(login: BrowserLogin, e: Exception) {
+        completeLoginFailed(login, e)
+        releaseActiveLogin(login)
+    }
+
+    protected suspend fun <T> withBrowserLoginLock(block: suspend () -> T): T =
+        loginMutex.withLock { block() }
+
     override suspend fun getValidToken(): SSOToken? = tokenMutex.withLock {
         val token = currentToken
         if (token == null) {
@@ -161,42 +202,23 @@ abstract class AbstractAuthSessionManager(
         }
 
         thisLogger().info("Token expired for account: ${token.accountLabel}, logging out")
-        // Call private logout method to avoid deadlock (already holding tokenMutex)
         doLogout()
         return null
     }
 
-    /**
-     * Logs out the current user by clearing the token from storage and memory.
-     * Notifies all registered listeners of the session change.
-     */
     override suspend fun logout() = tokenMutex.withLock {
         doLogout()
     }
 
-    /**
-     * Internal logout implementation that doesn't acquire the lock.
-     * Must be called while holding tokenMutex.
-     */
     private suspend fun doLogout() {
         val account = currentToken?.accountLabel
         thisLogger().info("Logging out${if (account != null) " account: $account" else ""}")
+        withBrowserLoginLock { endActiveLogin() }
         currentToken = null
         tokenStorage.clearToken()
-        notifyChanged()
     }
 
-    /**
-     * Checks if a user is currently logged in.
-     *
-     * @return true if a token exists (regardless of expiration), false otherwise
-     */
     override fun isLoggedIn(): Boolean = currentToken != null
 
-    /**
-     * Returns the account label of the current token.
-     *
-     * @return The account label or null if not logged in
-     */
     override fun currentAccount(): String? = currentToken?.accountLabel
 }

src/main/kotlin/com/redhat/devtools/gateway/auth/session/AuthSessionManager.kt

@@ -12,19 +12,15 @@
 package com.redhat.devtools.gateway.auth.session
 
 import com.redhat.devtools.gateway.auth.code.SSOToken
-import java.net.URI
 import javax.net.ssl.SSLContext
 
 interface AuthSessionManager {
 
-    /** Called once on plugin startup to load any existing token. */
-    suspend fun initialize()
-
-    /** Starts login and returns browser URL */
-    suspend fun startLogin(apiServerUrl: String? = null, sslContext: SSLContext): URI
-
-    /** Awaits for the browser login result */
-    suspend fun awaitLoginResult(timeoutMs: Long): SSOToken
+    /**
+     * Starts a browser OAuth login and returns a handle to open the authorization URL
+     * and await the result.
+     */
+    suspend fun startBrowserLogin(apiServerUrl: String? = null, sslContext: SSLContext): BrowserLogin
 
     /** Starts login using the given credentials and returns a valid token */
     suspend fun loginWithCredentials(apiServerUrl: String, username: String, password: String, sslContext: SSLContext): SSOToken
@@ -40,4 +36,4 @@ interface AuthSessionManager {
 
     /** Returns the current account label, if logged in. */
     fun currentAccount(): String?
-}
+}

src/main/kotlin/com/redhat/devtools/gateway/auth/session/BrowserLogin.kt

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2025-2026 Red Hat, Inc.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package com.redhat.devtools.gateway.auth.session
+
+import com.redhat.devtools.gateway.auth.code.SSOToken
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.Job
+import java.net.URI
+
+/**
+ * In-flight browser OAuth login started via [AuthSessionManager.startBrowserLogin].
+ */
+class BrowserLogin private constructor(
+    private val manager: AbstractAuthSessionManager,
+    val authorizationUri: URI,
+) {
+
+    internal val result = CompletableDeferred<SSOToken>()
+    internal var job: Job? = null
+
+    suspend fun awaitResult(timeoutMs: Long): SSOToken =
+        manager.awaitBrowserLoginResult(this, timeoutMs)
+
+    suspend fun cancel() {
+        manager.cancelBrowserLogin(this)
+    }
+
+    internal companion object {
+        fun create(manager: AbstractAuthSessionManager, authorizationUri: URI): BrowserLogin =
+            BrowserLogin(manager, authorizationUri)
+    }
+}