diff --git a/CHANGELOG.md b/CHANGELOG.md index a0950256..65d4f10e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,15 @@ It follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - SSH connections being blocked while non-blocking startup scripts run or after startup scripts fail or time out. - Automatic connection not starting when a workspace agent becomes connectable after the initial workspace load. +- Duplicate CLI sign-ins that could send the setup wizard back to the token screen after a successful connection. +- Opening a workspace link for another deployment could leave the previous setup page on screen until Toolbox regained focus. ### Security +- Reject ambiguous workspace links and OAuth callbacks. Workspace links must identify the deployment by its HTTP(S) origin. + +- Restrict HTTP redirects and deployment credentials to their original destination. Disable redirects in CLI v2.38.0 and newer. + - Ask for confirmation before a workspace link opens an unknown Coder deployment. Links select the deployment by scheme, host, and port. For a known deployment, Toolbox asks before using a link token if the accounts differ or it cannot verify the saved account. diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt index e7610993..43434706 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt @@ -34,6 +34,7 @@ import com.coder.toolbox.views.CoderSetupWizardPage import com.coder.toolbox.views.NewEnvironmentPage import com.coder.toolbox.views.SuspendBiConsumer import com.coder.toolbox.views.state.CoderOAuthSessionContext +import com.coder.toolbox.views.state.ConnectionOrigin import com.coder.toolbox.views.state.Credentials import com.coder.toolbox.views.state.PageRouter import com.coder.toolbox.views.state.PendingOAuthConnection @@ -51,11 +52,13 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.isActive +import kotlinx.coroutines.job import kotlinx.coroutines.launch import kotlinx.coroutines.selects.onTimeout import kotlinx.coroutines.selects.select @@ -418,6 +421,11 @@ class CoderRemoteProvider( * Also called as part of our own logout. */ override fun close() { + resetConnection() + router.clear() + } + + private fun resetConnection() { val sessionIds = lastEnvironments.currentSessionIds() softClose() client = null @@ -428,7 +436,6 @@ class CoderRemoteProvider( environments.value = LoadableState.Value(emptyList()) isInitialized.update { false } accountDropdownField.visibility.update { false } - router.clear() context.logger.info(sessionIds, "Coder plugin is now closed") context.logger.flush("provider_closed") } @@ -501,9 +508,14 @@ class CoderRemoteProvider( */ override suspend fun handleUri(uri: URI) { try { + require(uri.scheme.equals("jetbrains", ignoreCase = true) && + uri.rawAuthority.equals("gateway", ignoreCase = true) && uri.rawFragment == null && + uri.rawPath in setOf("/com.coder.toolbox", "/coder", "/com.coder.toolbox/auth")) { + "Unrecognized Toolbox link." + } val params = uri.toQueryParameters() when { - uri.toString().startsWith("jetbrains://gateway/com.coder.toolbox/auth") -> handleOAuthUri(params) + uri.rawPath == "/com.coder.toolbox/auth" -> handleOAuthUri(params) // An empty query can indicate plugin installation. params.isEmpty() -> context.logger.logAndShowInfo("URI will not be handled", "No query parameters were provided") @@ -515,8 +527,6 @@ class CoderRemoteProvider( } catch (ex: Exception) { val reason = (ex as? APIResponseException)?.reason?.takeIf { it.isNotBlank() } context.logger.logAndShowError("Error encountered while handling Coder URI", reason ?: ex.message.orEmpty()) - } finally { - firstRun = false } } @@ -580,6 +590,7 @@ class CoderRemoteProvider( onConnect = onConnect, onTokenRefreshed = ::onTokenRefreshed, ) + firstRun = false router.navigate(wizard) context.envPageManager.showPluginEnvironmentsPage(false) @@ -604,6 +615,7 @@ class CoderRemoteProvider( val activeClient = client?.takeIf { it.url.hasSameOrigin(deployment.url) } if (!confirmDeploymentSignIn(deployment, activeClient)) return + firstRun = false if (activeClient != null) { openLinkInActiveSession(params, activeClient, deployment.token) } else { @@ -623,7 +635,7 @@ class CoderRemoteProvider( context.logger.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"${URL}\" is missing from URI") return null } - val validationResult = deploymentURL.validateStrictWebUrl() + val validationResult = deploymentURL.validateStrictWebUrl(originOnly = true) if (validationResult is Invalid) { context.logger.logAndShowError(CAN_T_HANDLE_URI_TITLE, "\"$URL\" is invalid: ${validationResult.reason}") return null @@ -632,6 +644,10 @@ class CoderRemoteProvider( val origin = requireNotNull(deploymentURL.toHttpUrlOrNull()?.origin()) { "The deployment URL must have a valid HTTP(S) host and port." } + val saved = context.secrets.savedDeploymentUrl(origin.toURL()) + if (!saved.isNullOrEmpty() && saved.validateStrictWebUrl() !is Invalid && saved.toHttpUrlOrNull()?.origin() == origin) { + return saved.toURL() + } return origin.toURL() } @@ -711,6 +727,7 @@ class CoderRemoteProvider( } private fun autoSetupCredentials(url: URL): Credentials? { + if (url.toString().validateStrictWebUrl() is Invalid) return null if (context.settingsStore.requiresMTlsAuth) return Credentials.MTls val tokenCredentials = context.secrets.apiTokenFor(url) @@ -811,7 +828,7 @@ class CoderRemoteProvider( linkHandler.handle(params, url, client, cli) } - private fun connectFromLink(params: Map, url: URL, token: String?) { + private suspend fun connectFromLink(params: Map, url: URL, token: String?) { val credentials = token?.let { Credentials.Token(it) } ?: Credentials.MTls val wizard = CoderSetupWizardPage.connectStep( context, settingsPage, @@ -820,10 +837,12 @@ class CoderRemoteProvider( onConnect = onConnect.andThen(deferredLinkHandler(params, url)), onTokenRefreshed = ::onTokenRefreshed, ) - // The router tears down any in-flight wizard and installs the new one before - // showPluginEnvironmentsPage() asks getOverrideUiPage() for the page to display. + // Replace any in-flight wizard before showPluginEnvironmentsPage() requests a refresh. + // If Toolbox calls getOverrideUiPage(), it must get the wizard for this link. router.navigate(wizard) context.popupPluginMainPage() + // An already visible provider can keep showing the old wizard, so show the new one explicitly. + context.ui.showUiPageSuspending(wizard) } /** Returns a [SuspendBiConsumer] that handles the given link parameters in a background coroutine. */ @@ -849,21 +868,16 @@ class CoderRemoteProvider( } } - /** - * Return the sign-in page if we do not have a valid client. - - * Otherwise, return null, which causes Toolbox to display the environment - * list. - */ + /** Show interactive setup when needed. Saved sessions restore behind the workspace list. */ override fun getOverrideUiPage(): UiPage? { // Show the setup wizard if one is already scheduled. - router.activePage?.let { return it } + router.activePage?.let { return it.pageForDisplay() } // Let the default workspace UI render if the HTTP client is initialized. if (client != null) return null // Otherwise, schedule our own setup wizard. - return router.getOrCreate { buildSetupWizard() } + return router.getOrCreate { buildSetupWizard() }.pageForDisplay() } /** @@ -887,6 +901,7 @@ class CoderRemoteProvider( credentials = credentials, onConnect = onConnect, onTokenRefreshed = ::onTokenRefreshed, + origin = ConnectionOrigin.SAVED_SESSION, ) } catch (ex: Exception) { context.logger.logAndShowError( @@ -915,9 +930,13 @@ class CoderRemoteProvider( fun canAutoLogin(): Boolean = autoSetupCredentials(context.deploymentUrl) != null private val onConnect: SuspendBiConsumer = SuspendBiConsumer { client, cli -> + router.finishSignIn(currentCoroutineContext().job) { startSession(client, cli) } + } + + private fun startSession(client: CoderRestClient, cli: CoderCLIManager) { // Store the URL and token for use next time. storeSession(client) - close() + resetConnection() this.client = client this.cli = cli lastEnvironments.forEach { it.updateClientAndCli(client, cli) } diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 1d3a3770..2f7ef7dd 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -113,6 +113,7 @@ class CoderCLIManager( // Keep credential storage consistent with existing SSH commands until the next sign-in. private val useKeyring = context.settingsStore.useKeyring && usesTokenAuth + private val supportsRedirectFlag by lazy { tryVersion()?.let { it >= SemVer(2, 38, 0) } == true } private val downloader = createDownloadService() private val gpgVerifier = GPGVerifier(context) @@ -661,6 +662,7 @@ class CoderCLIManager( private fun globalConfigArgs(): List = listOf("--global-config", coderConfigPath.toString()) private fun workspaceAuthArgs(): List = buildList { + if (supportsRedirectFlag) add("--allow-redirects=false") val useKeyringAuth = shouldUseKeyringAuth() if (!useKeyringAuth) addAll(globalConfigArgs()) // Override inherited CODER_URL and CODER_USE_KEYRING for every authenticated command. diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt index da5e2f21..21b3d69a 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -4,6 +4,7 @@ import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.cli.ex.ResponseException import com.coder.toolbox.util.OS import com.coder.toolbox.util.SemVer +import com.coder.toolbox.util.hasSameOrigin import com.coder.toolbox.util.getHeaders import com.coder.toolbox.util.getOS import com.coder.toolbox.util.sha1 @@ -130,7 +131,7 @@ class CoderDownloadService( } private fun getRequestHeaders(): Map { - return if (context.settingsStore.headerCommand.isNullOrBlank()) { + return if (!remoteBinaryURL.hasSameOrigin(deploymentUrl) || context.settingsStore.headerCommand.isNullOrBlank()) { emptyMap() } else { getHeaders(deploymentUrl, context.settingsStore.headerCommand) diff --git a/src/main/kotlin/com/coder/toolbox/sdk/CoderHttpClientBuilder.kt b/src/main/kotlin/com/coder/toolbox/sdk/CoderHttpClientBuilder.kt index b14479db..34d46f3e 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/CoderHttpClientBuilder.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/CoderHttpClientBuilder.kt @@ -21,6 +21,8 @@ object CoderHttpClientBuilder { .sslSocketFactory(tlsContext.sslSocketFactory, tlsContext.trustManager) .hostnameVerifier(CoderHostnameVerifier(context.settingsStore.tls.altHostname)) .retryOnConnectionFailure(true) + .followRedirects(false) + .addInterceptor(Interceptors.sameOriginRedirects()) // Detect silent connection loss even when a workspace build produces no log messages. .pingInterval(30, TimeUnit.SECONDS) diff --git a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt index e1843ecc..188cddc3 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt @@ -95,11 +95,7 @@ open class CoderRestClient( val interceptors = buildList { if (context.settingsStore.requiresTokenAuth) { - val oauthOrApiToken = oauthContext?.tokenResponse?.accessToken ?: token - if (oauthOrApiToken.isNullOrBlank()) { - throw IllegalStateException("OAuth or API token is required for $url deployment") - } - add(Interceptors.tokenAuth(oauthOrApiToken)) + add(Interceptors.tokenAuth(requireSessionToken(), url)) } add((Interceptors.userAgent(pluginVersion))) add(Interceptors.externalHeaders(context, url)) @@ -123,6 +119,12 @@ open class CoderRestClient( .build().create(CoderV2RestFacade::class.java) } + internal fun requireSessionToken(): String { + val sessionToken = oauthContext?.tokenResponse?.accessToken ?: token + check(!sessionToken.isNullOrBlank()) { "OAuth or API token is required for $url deployment" } + return sessionToken + } + /** * Load information about the current user and the build version. * diff --git a/src/main/kotlin/com/coder/toolbox/sdk/interceptors/Interceptors.kt b/src/main/kotlin/com/coder/toolbox/sdk/interceptors/Interceptors.kt index 8d64cbeb..e1de716d 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/interceptors/Interceptors.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/interceptors/Interceptors.kt @@ -4,7 +4,9 @@ import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.util.getArch import com.coder.toolbox.util.getHeaders import com.coder.toolbox.util.getOS +import com.coder.toolbox.util.origin import okhttp3.Interceptor +import java.net.ProtocolException import java.net.URL const val CODER_SESSION_TOKEN_HEADER_NAME = "Coder-Session-Token" @@ -14,11 +16,34 @@ const val CODER_SESSION_TOKEN_HEADER_NAME = "Coder-Session-Token" */ object Interceptors { + /** Follow redirects for reads only. Credential-bearing POST bodies must never be replayed. */ + fun sameOriginRedirects(): Interceptor = Interceptor { chain -> + var request = chain.request() + val origin = request.url.origin() + var redirects = 0 + var response = chain.proceed(request) + while (response.code in setOf(300, 301, 302, 303, 307, 308)) { + val location = response.header("Location") ?: break + val target = request.url.resolve(location) + if (target == null || target.origin() != origin || target.username.isNotEmpty() || target.password.isNotEmpty()) { + response.close() + throw ProtocolException("Refused a redirect outside the request origin or to an invalid URL.") + } + if (request.method != "GET" && request.method != "HEAD") break + response.close() + if (++redirects > 20) throw ProtocolException("Too many redirects.") + request = request.newBuilder().url(target).build() + response = chain.proceed(request) + } + response + } + /** * Creates a token authentication interceptor */ - fun tokenAuth(token: String): Interceptor { + fun tokenAuth(token: String, deploymentUrl: URL): Interceptor { return Interceptor { chain -> + if (chain.request().url.origin() != deploymentUrl.origin()) return@Interceptor chain.proceed(chain.request()) chain.proceed( chain.request().newBuilder() .addHeader(CODER_SESSION_TOKEN_HEADER_NAME, token) @@ -47,6 +72,7 @@ object Interceptors { val settings = context.settingsStore.readOnly() return Interceptor { chain -> var request = chain.request() + if (request.url.origin() != url.origin()) return@Interceptor chain.proceed(request) val headers = getHeaders(url, settings.headerCommand) if (headers.isNotEmpty()) { val reqBuilder = request.newBuilder() diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSecretsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSecretsStore.kt index 192f6bdc..c0ec8dd2 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSecretsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSecretsStore.kt @@ -49,7 +49,7 @@ class CoderSecretsStore(private val store: PluginSecretStore) { fun isUrlTrusted(url: URL): Boolean { val requested = url.toString().toHttpUrlOrNull() ?: return false val origin = requested.origin() - val saved = store["deployment-url:$origin"] + val saved = savedDeploymentUrl(url) return if (saved != null) { saved.toHttpUrlOrNull() == requested } else { @@ -58,6 +58,8 @@ class CoderSecretsStore(private val store: PluginSecretStore) { } } + fun savedDeploymentUrl(url: URL): String? = url.origin()?.let { store["deployment-url:$it"] } + fun trustDeployment(url: URL) { val address = requireNotNull(url.toString().toHttpUrlOrNull()) { "Deployment trust requires an HTTP(S) URL" } val key = "deployment-url:${address.origin()}" diff --git a/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt b/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt index 9c037fdd..e41b41ba 100644 --- a/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt +++ b/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt @@ -32,40 +32,27 @@ fun String.toBaseURL(): URL { return URI.create("${url.protocol}://${url.host}$port").toURL() } -fun String.validateStrictWebUrl(): WebUrlValidationResult = try { +/** Validate the supplied text before URL normalization can discard unsafe components. */ +fun String.validateStrictWebUrl(originOnly: Boolean = false): WebUrlValidationResult = try { val uri = URI(this) - + val address = toHttpUrlOrNull() when { - uri.isOpaque -> Invalid( - "The URL \"$this\" is invalid because it is not in the standard format. " + - "Please enter a full web address like \"https://example.com\"" - ) - - !uri.isAbsolute -> Invalid( - "The URL \"$this\" is missing a scheme (like https://). " + - "Please enter a full web address like \"https://example.com\"" - ) - - uri.scheme?.lowercase() !in setOf("http", "https") -> - Invalid( - "The URL \"$this\" must start with http:// or https://, not \"${uri.scheme}\"" - ) - - uri.authority.isNullOrBlank() -> - Invalid( - "The URL \"$this\" does not include a valid website name. " + - "Please enter a full web address like \"https://example.com\"" - ) - + address == null || uri.rawAuthority.isNullOrEmpty() || uri.isOpaque || address.port == 0 -> + Invalid("The deployment URL must have a valid HTTP(S) host and port.") + uri.rawUserInfo != null || uri.rawQuery != null || uri.rawFragment != null -> + Invalid("The deployment URL cannot include credentials, a query, or a fragment.") + !SAFE_DEPLOYMENT_URL.matches(this) || uri.rawPath.split('/').any { it == "." || it == ".." } -> + Invalid("The deployment URL contains unsupported characters or path segments.") + originOnly && uri.rawPath.isNotEmpty() && uri.rawPath != "/" -> + Invalid("A workspace link must specify a deployment origin without a path.") else -> Valid } -} catch (_: Exception) { - Invalid( - "The input \"$this\" is not a valid web address. " + - "Please enter a full web address like \"https://example.com\"" - ) +} catch (_: java.net.URISyntaxException) { + Invalid("The deployment URL must have a valid HTTP(S) host and port.") } +private val SAFE_DEPLOYMENT_URL = Regex("""[\p{L}\p{N}._~:/\[\]+,=\-]+""") + fun URL.withPath(path: String): URL = URL( this.protocol, this.host, @@ -79,17 +66,20 @@ fun URL.withPath(path: String): URL = URL( */ fun URL.safeHost(): String = IDN.toASCII(this.host, IDN.ALLOW_UNASSIGNED) -fun URI.toQueryParameters(): Map = (this.query ?: "") - .split("&").filter { - it.isNotEmpty() - }.associate { - val parts = it.split("=", limit = 2) - if (parts.size == 2) { - parts[0] to URLDecoder.decode(parts[1], Charsets.UTF_8) - } else { - parts[0] to "" - } +fun URI.toQueryParameters(): Map = buildMap { + for (field in rawQuery.orEmpty().split('&')) { + if (field.isEmpty()) continue + val name = decodeQueryComponent(field.substringBefore('=')) + val value = decodeQueryComponent(field.substringAfter('=', "")) + require(put(name, value) == null) { "A query parameter was supplied more than once." } } +} + +private fun decodeQueryComponent(raw: String): String { + val decoded = URLDecoder.decode(raw, Charsets.UTF_8) + require('�' !in decoded) { "Invalid query parameter encoding." } + return decoded +} sealed class WebUrlValidationResult { object Valid : WebUrlValidationResult() diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSetupWizardPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSetupWizardPage.kt index 2481bd8e..7b742298 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSetupWizardPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSetupWizardPage.kt @@ -4,12 +4,14 @@ import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.cli.CoderCLIManager import com.coder.toolbox.sdk.CoderRestClient import com.coder.toolbox.views.state.CoderOAuthSessionContext +import com.coder.toolbox.views.state.ConnectionOrigin import com.coder.toolbox.views.state.Credentials import com.coder.toolbox.views.state.PendingOAuthConnection import com.coder.toolbox.views.state.WizardModel import com.coder.toolbox.views.state.WizardStep import com.jetbrains.toolbox.api.ui.actions.RunnableActionDescription import com.jetbrains.toolbox.api.ui.components.UiField +import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update @@ -19,7 +21,6 @@ import java.net.URL class CoderSetupWizardPage private constructor( private val context: CoderToolboxContext, private val settingsPage: CoderSettingsPage, - private var autoLogin: Boolean = false, onConnect: SuspendBiConsumer, onTokenRefreshed: (suspend (url: URL, oauthSessionCtx: CoderOAuthSessionContext) -> Unit)? = null ) : CoderPage(MutableStateFlow(context.i18n.ptrl("Setting up Coder")), false) { @@ -38,6 +39,8 @@ class CoderSetupWizardPage private constructor( onTokenRefreshed = onTokenRefreshed ) private var stateCollectJob: Job? = null + @Volatile + private var disposed = false /** * Fields for this page, displayed in order. @@ -45,29 +48,42 @@ class CoderSetupWizardPage private constructor( override val fields: MutableStateFlow> = MutableStateFlow(emptyList()) override val actionButtons: MutableStateFlow> = MutableStateFlow(emptyList()) + fun pageForDisplay(): CoderSetupWizardPage? = when (model.currentStep()) { + WizardStep.CONNECT -> if (model.connectionOrigin == ConnectionOrigin.SAVED_SESSION) null else this + WizardStep.SIGNED_IN -> null + else -> this + } + + internal fun startSavedSessionRestore() { + if (model.connectionOrigin == ConnectionOrigin.SAVED_SESSION) connectStep.startConnection() + } + override fun beforeShow() { - stateCollectJob?.cancel() - stateCollectJob = context.cs.launch { + prepareStepObserver()?.start() + } + + @Synchronized + private fun prepareStepObserver(): Job? { + if (disposed || stateCollectJob?.isCompleted == false) return null + val job = context.cs.launch(start = CoroutineStart.LAZY) { model.step.collect { step -> context.logger.info("Wizard step changed to $step") - displaySteps() + displayStep(step) } } + stateCollectJob = job + return job } - private fun displaySteps() { - when (model.currentStep()) { + private fun displayStep(step: WizardStep) { + when (step) { WizardStep.URL_REQUEST -> { fields.update { listOf(deploymentUrlStep.panel) } actionButtons.update { listOf( - Action(context, "Next", closesPage = false, actionBlock = { - if (deploymentUrlStep.onNext()) { - displaySteps() - } - }), + Action(context, "Next", closesPage = false) { deploymentUrlStep.onNext() }, settingsAction ) } @@ -80,16 +96,9 @@ class CoderSetupWizardPage private constructor( } actionButtons.update { listOf( - Action(context, "Connect", closesPage = false, actionBlock = { - if (tokenStep.onNext()) { - displaySteps() - } - }), + Action(context, "Connect", closesPage = false) { tokenStep.onNext() }, settingsAction, - Action(context, "Back", closesPage = false, actionBlock = { - tokenStep.onBack() - displaySteps() - }) + Action(context, "Back", closesPage = false) { tokenStep.onBack() } ) } tokenStep.onVisible() @@ -102,14 +111,15 @@ class CoderSetupWizardPage private constructor( actionButtons.update { listOf( settingsAction, - Action(context, "Back", closesPage = false, actionBlock = { - connectStep.onBack() - displaySteps() - }) + Action(context, "Back", closesPage = false) { connectStep.onBack() } ) } connectStep.onVisible() } + + // Toolbox can bring this wizard back after Settings or during startup. + // Ask for the workspace list when the completed wizard is actually shown. + WizardStep.SIGNED_IN -> context.envPageManager.showPluginEnvironmentsPage(false) } } @@ -120,16 +130,18 @@ class CoderSetupWizardPage private constructor( } private fun navigateBackFromConnect() { - if (autoLogin) { - autoLogin = false - model.clearFormData() - model.goToFirst() + if (model.connectionOrigin == ConnectionOrigin.FORM) { + model.goTo(if (context.settingsStore.requiresTokenAuth) WizardStep.TOKEN_REQUEST else WizardStep.URL_REQUEST) return } - if (context.settingsStore.requiresTokenAuth) { - model.goToPrevious() - } else { - model.goToFirst() + val revealForm = model.connectionOrigin == ConnectionOrigin.SAVED_SESSION + model.clearFormData() + model.goToFirst() + model.connectionOrigin = ConnectionOrigin.FORM + if (revealForm) { + context.cs.launch { + if (!disposed) context.ui.showUiPageSuspending(this@CoderSetupWizardPage) + } } } @@ -138,13 +150,17 @@ class CoderSetupWizardPage private constructor( * when the wizard is being replaced (e.g. by a deep link to a different * deployment) so its connect job doesn't keep running and clobber the new one. */ + @Synchronized fun dispose() { - stateCollectJob?.cancel() + disposed = true + afterHide() connectStep.dispose() } + @Synchronized override fun afterHide() { stateCollectJob?.cancel() + stateCollectJob = null } companion object { @@ -166,12 +182,13 @@ class CoderSetupWizardPage private constructor( credentials: Credentials, onConnect: SuspendBiConsumer, onTokenRefreshed: (suspend (url: URL, oauthSessionCtx: CoderOAuthSessionContext) -> Unit)? = null, + origin: ConnectionOrigin = ConnectionOrigin.SUPPLIED_CREDENTIALS, ): CoderSetupWizardPage = CoderSetupWizardPage( context, settingsPage, - autoLogin = true, onConnect = onConnect, onTokenRefreshed = onTokenRefreshed, ).apply { + model.connectionOrigin = origin model.url = url when (credentials) { is Credentials.MTls -> Unit diff --git a/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt b/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt index 4adc782e..14cad55c 100644 --- a/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt +++ b/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt @@ -6,17 +6,26 @@ import com.coder.toolbox.cli.ensureCLI import com.coder.toolbox.oauth.OAuth2Client import com.coder.toolbox.plugin.PluginManager import com.coder.toolbox.sdk.CoderRestClient +import com.coder.toolbox.util.origin import com.coder.toolbox.views.state.CoderOAuthSessionContext +import com.coder.toolbox.views.state.ConnectionOrigin import com.coder.toolbox.views.state.WizardModel +import com.coder.toolbox.views.state.WizardStep.CONNECT +import com.coder.toolbox.views.state.WizardStep.SIGNED_IN import com.jetbrains.toolbox.api.ui.components.LabelField import com.jetbrains.toolbox.api.ui.components.RowGroup import com.jetbrains.toolbox.api.ui.components.ValidationErrorField import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineName -import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Job +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.update +import kotlinx.coroutines.job import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.yield import java.net.URL @@ -34,6 +43,8 @@ class ConnectStep( private val onTokenRefreshed: (suspend (url: URL, oauthSessionCtx: CoderOAuthSessionContext) -> Unit)? = null ) : WizardStep { private var signInJob: Job? = null + private var disposed = false + private val connectionMutex = Mutex() private val statusField = LabelField(context.i18n.pnotr("")) private val errorField = ValidationErrorField(context.i18n.pnotr("")) @@ -43,135 +54,101 @@ class ConnectStep( RowGroup.RowField(errorField) ) - override fun onVisible() { - errorField.textState.update { - context.i18n.pnotr("") - } + override fun onVisible() = startConnection() + + fun startConnection() { + prepareConnection()?.start() + } + + @Synchronized + private fun prepareConnection(): Job? { + if (disposed || signInJob != null || model.currentStep() != CONNECT) return null + errorField.textState.update { context.i18n.pnotr("") } if (context.settingsStore.requiresTokenAuth && model.isNotReadyForAuth()) { errorField.textState.update { context.i18n.pnotr("URL and token were not properly configured. Please go back and provide a proper URL and token!") } - return - } - - // Don't launch another connection attempt if one is already in progress. - if (signInJob?.isActive == true) { - context.logger.info("Connection already in progress, skipping duplicate") - return + return null } - - statusField.textState.update { context.i18n.pnotr("Connecting to ${model.url?.host ?: "unknown host"}...") } - connect() - } - - /** - * Try connecting to Coder with the provided URL and token. - */ - private fun connect() { val url = model.url if (url == null) { errorField.textState.update { context.i18n.ptrl("URL is required") } - return + return null } - if (context.settingsStore.requiresTokenAuth && !model.hasToken() && !model.hasOAuthSession()) { - errorField.textState.update { context.i18n.ptrl("Token is required") } - return + statusField.textState.update { context.i18n.pnotr("Connecting to ${url.origin() ?: url.host}...") } + // Record the job before dispatching it. Toolbox can show the same step again during startup. + val job = context.cs.launch(CoroutineName("Async Http and CLI Setup"), start = CoroutineStart.LAZY) { + // A retry waits for a cancelled CLI process to finish cleaning up. + connectionMutex.withLock { connect(url) } } + signInJob = job + return job + } - // Capture the host name early for error reporting - val hostName = url.host - - // Cancel previous job regardless of the new mode - signInJob?.cancel() - - // 1. Extract the logic into a reusable suspend lambda - val connectionLogic: suspend CoroutineScope.() -> Unit = { - try { - var oauthSession: CoderOAuthSessionContext? = null - if (context.settingsStore.requiresTokenAuth && context.settingsStore.preferOAuth2IfAvailable && model.hasOAuthSession()) { - refreshOAuthToken() - oauthSession = model.oauthSession!!.copy() - } - - val apiToken = if (context.settingsStore.requiresTokenAuth) model.token else null - - context.logger.info("Setting up the HTTP client...") - val client = CoderRestClient( - context, - url, - apiToken, - oauthSession, - PluginManager.pluginInfo.version, - onTokenRefreshed - ) - // allows interleaving with the back/cancel action - yield() - client.initializeSession() - logAndReportProgress("Checking Coder CLI...") - val cli = ensureCLI( - context, client.url, - client.buildVersion - ) { progress -> - statusField.textState.update { (context.i18n.pnotr(progress)) } - } - // We only need to log in if we are using token-based auth. - if (context.settingsStore.requiresTokenAuth) { - logAndReportProgress("Configuring Coder CLI...") - // allows interleaving with the back/cancel action - yield() - if (oauthSession != null) { - cli.login(oauthSession.tokenResponse!!.accessToken) - } else { - cli.login(apiToken!!) - } - } - logAndReportProgress("Successfully configured ${hostName}...") + private suspend fun connect(url: URL) { + var pendingClient: CoderRestClient? = null + try { + val requiresTokenAuth = context.settingsStore.requiresTokenAuth + val oauthSession = if (requiresTokenAuth && context.settingsStore.preferOAuth2IfAvailable) { + oauthSessionForConnection() + } else null + val apiToken = if (requiresTokenAuth) model.token else null + + context.logger.info("Setting up the HTTP client...") + val client = CoderRestClient( + context, + url, + apiToken, + oauthSession, + PluginManager.pluginInfo.version, + onTokenRefreshed + ) + pendingClient = client + // allows interleaving with the back/cancel action + yield() + client.initializeSession() + logAndReportProgress("Checking Coder CLI...") + val cli = ensureCLI( + context, client.url, + client.buildVersion + ) { progress -> + statusField.textState.update { (context.i18n.pnotr(progress)) } + } + if (context.settingsStore.requiresTokenAuth) { + logAndReportProgress("Configuring Coder CLI...") // allows interleaving with the back/cancel action yield() - context.logger.info("Connection setup done, initializing the workspace poller...") - onConnect.accept(client, cli) - // Only invoke onTokenRefreshed when we actually have an OAuth session - oauthSession?.let { session -> - onTokenRefreshed?.invoke(client.url, session) - } - // The provider's onConnect ran close() which clears the router; combined - // with client now being non-null this drops the wizard from getOverrideUiPage. - context.envPageManager.showPluginEnvironmentsPage(false) - } catch (ex: CancellationException) { - // Back-button cancellation already navigates in onBack(), while - // dispose() must cancel without navigating. Treat these control-flow - // cancellations separately so we do not run navigateBack() twice. - if (ex.message != USER_HIT_THE_BACK_BUTTON && ex.message != WIZARD_WAS_DISPOSED) { - context.logger.logAndShowError( - "Error encountered while setting up Coder", - "Failed to configure $hostName. ${ex.message}", - ex - ) - navigateBack() - } - } catch (ex: Exception) { - context.logger.logAndShowError( - "Error encountered while setting up Coder", - "Failed to configure $hostName. ${ex.message}", - ex - ) - navigateBack() + cli.login(client.requireSessionToken()) + } + logAndReportProgress("Successfully configured ${url.host}...") + // allows interleaving with the back/cancel action + yield() + context.logger.info("Connection setup done, initializing the workspace poller...") + onConnect.accept(client, cli) + pendingClient = null // The provider now owns this client. + // The CLI already uses this token. Save the OAuth session for the next sign-in. + oauthSession?.let { session -> + context.secrets.storeOAuthFor(client.url.toString(), session) } + } catch (ex: Exception) { + reportConnectionFailure(currentCoroutineContext().job, url, ex) + } finally { + pendingClient?.close() } - - signInJob = context.cs.launch(CoroutineName("Async Http and CLI Setup"), block = connectionLogic) } - private suspend fun refreshOAuthToken() { - val session = model.oauthSession ?: return - if (!session.tokenResponse?.accessToken.isNullOrBlank()) return + private suspend fun oauthSessionForConnection(): CoderOAuthSessionContext? { + val session = model.oauthSession ?: return null + val response = session.tokenResponse ?: return null + if (response.accessToken.isNotBlank()) return session logAndReportProgress("Refreshing OAuth token...") - val tokenResponse = OAuth2Client(context).refreshToken(session) + val refreshed = session.copy(tokenResponse = OAuth2Client(context).refreshToken(session)) context.logger.info("Successfully refreshed access token") - model.oauthSession = session.copy(tokenResponse = tokenResponse) + model.oauthSession = refreshed + return refreshed } private fun logAndReportProgress(msg: String) { @@ -179,15 +156,35 @@ class ConnectStep( statusField.textState.update { context.i18n.pnotr(msg) } } + @Synchronized + private fun reportConnectionFailure(job: Job, url: URL, ex: Exception) { + job.ensureActive() + if (signInJob !== job) return + if (model.currentStep() == SIGNED_IN) { + context.logger.warn(ex, "Failed to update session credentials after sign-in") + return + } + signInJob = null + context.logger.logAndShowError( + "Error encountered while setting up Coder", + "Failed to configure ${url.origin() ?: url.host}. ${ex.message}", + ex, + ) + navigateBack() + } + override suspend fun onNext(): Boolean { return false } + @Synchronized override fun onBack() { + if (disposed || model.currentStep() != CONNECT || model.connectionOrigin == ConnectionOrigin.SAVED_SESSION) return try { context.logger.info("Back button was pressed, cancelling in-progress connection setup...") signInJob?.cancel(CancellationException(USER_HIT_THE_BACK_BUTTON)) } finally { + signInJob = null navigateBack() } } @@ -197,7 +194,9 @@ class ConnectStep( * itself is being torn down by an external trigger (e.g. a deep link to a * different deployment). */ + @Synchronized fun dispose() { + disposed = true signInJob?.cancel(CancellationException(WIZARD_WAS_DISPOSED)) signInJob = null } diff --git a/src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt b/src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt index 72325344..ceac44b3 100644 --- a/src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt +++ b/src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt @@ -12,6 +12,8 @@ import com.coder.toolbox.util.toURL import com.coder.toolbox.util.validateStrictWebUrl import com.coder.toolbox.views.state.CoderOAuthSessionContext import com.coder.toolbox.views.state.WizardModel +import com.coder.toolbox.views.state.WizardStep.CONNECT +import com.coder.toolbox.views.state.WizardStep.TOKEN_REQUEST import com.jetbrains.toolbox.api.localization.LocalizableString import com.jetbrains.toolbox.api.ui.components.CheckboxField import com.jetbrains.toolbox.api.ui.components.FieldModifier @@ -88,7 +90,7 @@ class DeploymentUrlStep( } if (context.settingsStore.requiresMTlsAuth) { - model.goToLast() + model.goTo(CONNECT) return true } if (context.settingsStore.requiresTokenAuth && context.settingsStore.preferOAuth2IfAvailable) { @@ -102,7 +104,7 @@ class DeploymentUrlStep( } } // if all else fails try the good old API token auth - model.goToNext() + model.goTo(TOKEN_REQUEST) return true } diff --git a/src/main/kotlin/com/coder/toolbox/views/TokenStep.kt b/src/main/kotlin/com/coder/toolbox/views/TokenStep.kt index 902f14fd..42b290e6 100644 --- a/src/main/kotlin/com/coder/toolbox/views/TokenStep.kt +++ b/src/main/kotlin/com/coder/toolbox/views/TokenStep.kt @@ -3,6 +3,8 @@ package com.coder.toolbox.views import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.util.withPath import com.coder.toolbox.views.state.WizardModel +import com.coder.toolbox.views.state.WizardStep.CONNECT +import com.coder.toolbox.views.state.WizardStep.URL_REQUEST import com.jetbrains.toolbox.api.localization.LocalizableString import com.jetbrains.toolbox.api.ui.components.FieldModifier import com.jetbrains.toolbox.api.ui.components.LinkField @@ -56,12 +58,12 @@ class TokenStep( } model.token = token - model.goToNext() + model.goTo(CONNECT) return true } override fun onBack() { - model.goToPrevious() + model.goTo(URL_REQUEST) } private fun reportError(message: LocalizableString?) { diff --git a/src/main/kotlin/com/coder/toolbox/views/state/PageRouter.kt b/src/main/kotlin/com/coder/toolbox/views/state/PageRouter.kt index a78b27cb..e428ba35 100644 --- a/src/main/kotlin/com/coder/toolbox/views/state/PageRouter.kt +++ b/src/main/kotlin/com/coder/toolbox/views/state/PageRouter.kt @@ -1,45 +1,34 @@ package com.coder.toolbox.views.state -import com.coder.toolbox.views.CoderPage import com.coder.toolbox.views.CoderSetupWizardPage -import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.Job +import kotlinx.coroutines.ensureActive /** - * The page that should currently be rendered in place of the environment list. - */ -sealed interface PageRoute { - object None : PageRoute - data class Wizard(val page: CoderSetupWizardPage) : PageRoute -} - -/** - * Holds the active [PageRoute]. The same page instance is returned across + * Holds the active wizard. The same page instance is returned across * Toolbox visibility cycles so in-flight work (e.g. an ongoing connect) is * preserved instead of being thrown away every time the window reopens. */ class PageRouter { - private val route = MutableStateFlow(PageRoute.None) - - private val activeWizard: CoderSetupWizardPage? - get() = (route.value as? PageRoute.Wizard)?.page - - val activePage: CoderPage? - get() = activeWizard + @Volatile + var activePage: CoderSetupWizardPage? = null + private set val hasActiveWizard: Boolean - get() = activeWizard != null + get() = activePage != null val pendingOAuthConnection: PendingOAuthConnection? - get() = activeWizard?.pendingOAuthConnection() + get() = activePage?.pendingOAuthConnection() /** * Returns the page already on this route, or builds a new one and * registers it. */ - fun getOrCreate(build: () -> CoderSetupWizardPage): CoderPage { - (route.value as? PageRoute.Wizard)?.let { return it.page } + @Synchronized + fun getOrCreate(build: () -> CoderSetupWizardPage): CoderSetupWizardPage { + activePage?.let { return it } - return build().also { route.value = PageRoute.Wizard(it) } + return build().also(::navigate) } /** @@ -47,13 +36,27 @@ class PageRouter { * (e.g. a deep link to a different deployment) needs to forcibly install * a new wizard. */ + @Synchronized fun navigate(page: CoderSetupWizardPage) { - activeWizard?.dispose() - route.value = PageRoute.Wizard(page) + activePage?.dispose() + activePage = page + // A restore can finish immediately. Publish its wizard before starting the job. + page.startSavedSessionRestore() } + @Synchronized fun clear() { - activeWizard?.dispose() - route.value = PageRoute.None + activePage?.dispose() + activePage = null + } + + @Synchronized + fun finishSignIn(connection: Job, startSession: () -> Unit) { + // Replacing a wizard cancels its connection. Only the winner can publish a session. + connection.ensureActive() + startSession() + val wizard = activePage ?: return + activePage = null + wizard.model.goTo(WizardStep.SIGNED_IN) } } diff --git a/src/main/kotlin/com/coder/toolbox/views/state/WizardModel.kt b/src/main/kotlin/com/coder/toolbox/views/state/WizardModel.kt index 0104a5f6..0251c13e 100644 --- a/src/main/kotlin/com/coder/toolbox/views/state/WizardModel.kt +++ b/src/main/kotlin/com/coder/toolbox/views/state/WizardModel.kt @@ -18,6 +18,8 @@ class WizardModel { var url: URL? = null var token: String? = null var oauthSession: CoderOAuthSessionContext? = null + @Volatile + var connectionOrigin = ConnectionOrigin.FORM fun currentStep(): WizardStep = _step.value @@ -25,24 +27,10 @@ class WizardModel { _step.value = step } - fun goToNext() { - val entries = WizardStep.entries - _step.value = entries[(_step.value.ordinal + 1) % entries.size] - } - - fun goToPrevious() { - val entries = WizardStep.entries - _step.value = entries[(_step.value.ordinal - 1 + entries.size) % entries.size] - } - fun goToFirst() { _step.value = WizardStep.URL_REQUEST } - fun goToLast() { - _step.value = WizardStep.CONNECT - } - fun hasUrl(): Boolean = url != null fun hasToken(): Boolean = !token.isNullOrBlank() fun hasOAuthSession(): Boolean = oauthSession?.tokenResponse?.accessToken != null @@ -57,7 +45,11 @@ class WizardModel { } enum class WizardStep { - URL_REQUEST, TOKEN_REQUEST, CONNECT; + URL_REQUEST, TOKEN_REQUEST, CONNECT, SIGNED_IN; +} + +enum class ConnectionOrigin { + FORM, SUPPLIED_CREDENTIALS, SAVED_SESSION } sealed interface Credentials { diff --git a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt index bbac7437..ba33e135 100644 --- a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt +++ b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt @@ -1,7 +1,9 @@ package com.coder.toolbox import com.coder.toolbox.cli.CoderCLIManager +import com.coder.toolbox.cli.ensureCLI import com.coder.toolbox.diagnostics.CoderLogger +import com.coder.toolbox.oauth.OAuth2Client import com.coder.toolbox.oauth.TokenEndpointAuthMethod import com.coder.toolbox.sdk.CoderRestClient import com.coder.toolbox.sdk.DataGen @@ -16,8 +18,10 @@ import com.coder.toolbox.sdk.v2.models.WorkspaceResource import com.coder.toolbox.sdk.v2.models.WorkspaceStatus import com.coder.toolbox.session.SessionId import com.coder.toolbox.settings.Environment +import com.coder.toolbox.store.CoderSecretsStore import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.util.CoderProtocolHandler +import com.coder.toolbox.util.InMemoryPluginSecretStore import com.coder.toolbox.util.pluginTestSettingsStore import com.coder.toolbox.views.CoderSetupWizardPage import com.coder.toolbox.views.SuspendBiConsumer @@ -35,16 +39,20 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkStatic import io.mockk.slot import io.mockk.spyk +import io.mockk.unmockkConstructor +import io.mockk.unmockkStatic import io.mockk.verify -import io.mockk.verifyOrder import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.async +import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest @@ -71,6 +79,9 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertSame +private const val PRODUCTION_URL = "https://coder.example.com" +private const val STAGING_URL = "https://staging.example.com" + class CoderRemoteProviderTest { private lateinit var mockClient: CoderRestClient @@ -100,9 +111,9 @@ class CoderRemoteProviderTest { every { mockContext.settingsStore } returns settingsStore every { mockContext.logger } returns mockLogger every { mockContext.secrets.isUrlTrusted(any()) } returns false - every { mockClient.url } returns URI("https://coder.example.com").toURL() + every { mockClient.url } returns URI(PRODUCTION_URL).toURL() every { mockClient.me } returns DataGen.user() - every { mockCli.deploymentURL } returns URI("https://coder.example.com").toURL() + every { mockCli.deploymentURL } returns URI(PRODUCTION_URL).toURL() remoteProvider = CoderRemoteProvider(mockContext) } @@ -138,6 +149,57 @@ class CoderRemoteProviderTest { assertFalse(remoteProvider.getAccountDropDown().visibility.value) } + @CsvSource( + "logged out deployment, link target, other remembered deployment, expect prompt", + "$PRODUCTION_URL, $PRODUCTION_URL, none, true", + "$STAGING_URL, $STAGING_URL, $PRODUCTION_URL, true", + "$PRODUCTION_URL, $STAGING_URL, none, true", + "$STAGING_URL, $PRODUCTION_URL, $PRODUCTION_URL, false", + nullValues = ["none"], + useHeadersInDisplayName = true, + ) + @ParameterizedTest(name = "After logout, {arguments}") + fun `logout clears the active deployment and each link uses its target's trust history`( + loggedOutDeployment: URL, + linkTarget: URL, + otherRememberedDeployment: URL?, + expectPrompt: Boolean, + ) = runTest { + every { mockClient.url } returns loggedOutDeployment + every { mockCli.deploymentURL } returns loggedOutDeployment + val secrets = CoderSecretsStore(InMemoryPluginSecretStore()) + every { mockContext.secrets } returns secrets + every { mockContext.settingsStore.requiresTokenAuth } returns true + every { mockContext.settingsStore.requiresMTlsAuth } returns false + every { mockContext.settingsStore.preferOAuth2IfAvailable } returns false + secrets.storeApiTokenFor(loggedOutDeployment, "logged-out-token") + secrets.trustDeployment(loggedOutDeployment) + val provider = spyk(remoteProvider, recordPrivateCalls = true) + setPrivateField(provider, "client", mockClient) + setPrivateField(provider, "cli", mockCli) + if (otherRememberedDeployment != null) { + secrets.storeApiTokenFor(otherRememberedDeployment, "remembered-token") + secrets.trustDeployment(otherRememberedDeployment) + val rememberedUser = DataGen.user() + coEvery { provider["userForToken"](otherRememberedDeployment, "remembered-token") } returns rememberedUser + coEvery { provider["userForToken"](otherRememberedDeployment, "link-token") } returns rememberedUser + } + coEvery { mockUi.showYesNoPopup(any(), any(), any(), any()) } returns false + + provider.logout() + provider.handleUri(workspaceLink(linkTarget.toString())) + + assertNull(secrets.apiTokenFor(loggedOutDeployment)) + assertFalse(secrets.isUrlTrusted(loggedOutDeployment)) + assertEquals(!expectPrompt, secrets.isUrlTrusted(linkTarget)) + if (otherRememberedDeployment != null) { + assertTrue(secrets.isUrlTrusted(otherRememberedDeployment)) + assertEquals("remembered-token", secrets.apiTokenFor(otherRememberedDeployment)) + } + coVerify(exactly = if (expectPrompt) 1 else 0) { mockUi.showYesNoPopup(any(), any(), any(), any()) } + coVerify(exactly = if (expectPrompt) 0 else 1) { mockUi.showUiPageSuspending(any()) } + } + @Test fun `provider shutdown preserves credentials`() { setPrivateField(remoteProvider, "client", mockClient) @@ -160,6 +222,47 @@ class CoderRemoteProviderTest { coVerify(exactly = 0) { mockCli.logout() } } + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `restoring an OAuth session logs in to the CLI once and saves the rotated token`() = runTest { + val url = mockCli.deploymentURL + val stored = storedOAuthSession() + val session = stored.toSessionContext() + val refreshed = session.tokenResponse!!.copy(accessToken = "new-access-token", refreshToken = "new-refresh-token") + val secrets = mockContext.secrets + every { secrets.oauthSessionFor(url.toString()) } returns stored + every { mockContext.deploymentUrl } returns url + every { mockContext.cs } returns backgroundScope + every { mockContext.settingsStore } returns CoderSettingsStore( + pluginTestSettingsStore(), Environment(), underlyingLogger, + ).apply { updatePreferAuthViaOAuth2(true) } + every { mockContext.proxySettings.getProxy() } returns Proxy.NO_PROXY + every { mockContext.proxySettings.getProxyAuth() } returns null + + mockkConstructor(OAuth2Client::class, CoderRestClient::class) + mockkStatic(::ensureCLI) + try { + coEvery { anyConstructed().refreshToken(session) } returns refreshed + val user = DataGen.user() + coEvery { anyConstructed().initializeSession() } returns user + every { anyConstructed().me } returns user + every { anyConstructed().buildVersion } returns "2.39.0" + coEvery { anyConstructed().templates() } returns emptyList() + coEvery { anyConstructed().workspaces(any()) } returns emptyList() + coEvery { ensureCLI(mockContext, url, "2.39.0", any()) } returns mockCli + + assertNull(remoteProvider.getOverrideUiPage()) + runCurrent() + + coVerify(exactly = 1) { mockCli.login(refreshed.accessToken) } + verify(exactly = 1) { secrets.storeOAuthFor(url.toString(), session.copy(tokenResponse = refreshed)) } + } finally { + remoteProvider.close() + unmockkStatic(::ensureCLI) + unmockkConstructor(OAuth2Client::class, CoderRestClient::class) + } + } + @Test fun `OAuth refresh updates the CLI session for the active origin`() = runTest { setPrivateField(remoteProvider, "client", mockClient) @@ -604,41 +707,111 @@ class CoderRemoteProviderTest { verify(exactly = 0) { mockContext.popupPluginMainPage() } } - @ValueSource( - strings = [ - "https://coder2.example.com/", - "https://coder2.example.com/dashboard?view=recent#workspaces", - ] + @CsvSource( + "restoring deployment, link target, expected origin", + "$PRODUCTION_URL, $STAGING_URL/, $STAGING_URL", + "$STAGING_URL, https://CODER.example.com:443, $PRODUCTION_URL", + "$PRODUCTION_URL, $STAGING_URL:8443, $STAGING_URL:8443", + useHeadersInDisplayName = true, ) - @ParameterizedTest - fun `accepted deployment switch replaces the wizard before Toolbox requests its page`(target: String) = runTest { + @ParameterizedTest(name = "Cold-start link, {arguments}") + fun `accepted cold start link presents the new deployment while the old session restores`( + restoringDeployment: URL, + linkTarget: String, + expectedOrigin: String, + ) = runTest { + every { mockContext.cs } returns backgroundScope + every { mockContext.deploymentUrl } returns restoringDeployment + every { mockContext.secrets.apiTokenFor(restoringDeployment) } returns "saved-token" every { mockContext.settingsStore.requiresMTlsAuth } returns false every { mockContext.settingsStore.requiresTokenAuth } returns true coEvery { mockUi.showYesNoPopup(any(), any(), any(), any()) } returns true - setPrivateField(remoteProvider, "client", mockClient) - setPrivateField(remoteProvider, "cli", mockCli) assertNull(remoteProvider.getOverrideUiPage()) + val restoringWizard = setupWizard(remoteProvider) + assertEquals(restoringDeployment, restoringWizard.model.url) + assertEquals(WizardStep.CONNECT, restoringWizard.model.currentStep()) + val displayedWizard = slot() + coEvery { mockUi.showUiPageSuspending(capture(displayedWizard)) } returns Unit + + remoteProvider.handleUri(workspaceLink(linkTarget)) + + // Toolbox has not requested the override page again since the link arrived. + coVerify(exactly = 1) { mockUi.showUiPageSuspending(any()) } + val replacementWizard = displayedWizard.captured + assertEquals(WizardStep.CONNECT, replacementWizard.model.currentStep()) + assertEquals(expectedOrigin, replacementWizard.model.url.toString()) + assertEquals("link-token", replacementWizard.model.token) + assertSame(replacementWizard, remoteProvider.getOverrideUiPage()) + coVerify(exactly = 1) { mockUi.showYesNoPopup(any(), any(), any(), any()) } + } - val previousWizard = mockk(relaxed = true) - val router = PageRouter() - router.navigate(previousWizard) - setPrivateField(remoteProvider, "router", router) - var displayedWizard: CoderSetupWizardPage? = null - every { mockContext.popupPluginMainPage() } answers { - displayedWizard = remoteProvider.getOverrideUiPage() as? CoderSetupWizardPage - } + private fun setupWizard(provider: CoderRemoteProvider): CoderSetupWizardPage { + val router = + provider.javaClass.getDeclaredField("router").apply { isAccessible = true }.get(provider) as PageRouter + return requireNotNull(router.activePage) + } - remoteProvider.handleUri(workspaceLink(target)) + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `a cold link signs in to the linked deployment and hands off its workspace without a focus change`() = runTest { + val restoringDeployment = URI(PRODUCTION_URL).toURL() + val linkedDeployment = URI(STAGING_URL).toURL() + val secrets = mockContext.secrets + val environmentsPage = mockContext.envPageManager + every { mockContext.settingsStore } returns CoderSettingsStore( + pluginTestSettingsStore(), Environment(), underlyingLogger, + ) + every { mockContext.cs } returns backgroundScope + every { mockContext.proxySettings.getProxy() } returns Proxy.NO_PROXY + every { mockContext.proxySettings.getProxyAuth() } returns null + every { mockContext.deploymentUrl } returns restoringDeployment + every { secrets.apiTokenFor(restoringDeployment) } returns "saved-token" + val linkHandler = mockk(relaxed = true) + setPrivateField(remoteProvider, "linkHandler", linkHandler) + val user = DataGen.user() + val restoreStarted = CompletableDeferred() + val restoreCancelled = CompletableDeferred() + val displayedWizard = slot() + mockkConstructor(CoderRestClient::class) + mockkStatic(::ensureCLI) + try { + coEvery { anyConstructed().initializeSession() } coAnswers { + restoreStarted.complete(Unit) + try { + awaitCancellation() + } finally { + restoreCancelled.complete(Unit) + } + } andThen user + every { anyConstructed().me } returns user + every { anyConstructed().buildVersion } returns "2.39.0" + coEvery { anyConstructed().workspaces(any()) } returns emptyList() + coEvery { ensureCLI(mockContext, linkedDeployment, "2.39.0", any()) } returns mockCli + coEvery { mockUi.showYesNoPopup(any(), any(), any(), any()) } returns true + coEvery { mockUi.showUiPageSuspending(capture(displayedWizard)) } coAnswers { + displayedWizard.captured.beforeShow() + } + every { environmentsPage.showPluginEnvironmentsPage(false) } answers { + assertNull(remoteProvider.getOverrideUiPage(), "Finishing link sign-in should show the workspace list") + displayedWizard.captured.afterHide() + } + assertNull(remoteProvider.getOverrideUiPage(), "The first lookup should restore the saved session in the background") + runCurrent() + assertTrue(restoreStarted.isCompleted) + remoteProvider.handleUri(workspaceLink(linkedDeployment.toString())) + runCurrent() - val wizard = assertNotNull(displayedWizard) - assertEquals(WizardStep.CONNECT, wizard.model.currentStep()) - assertEquals("https://coder2.example.com", wizard.model.url.toString()) - assertEquals("link-token", wizard.model.token) - assertSame(wizard, remoteProvider.getOverrideUiPage()) - coVerify(exactly = 1) { mockUi.showYesNoPopup(any(), any(), any(), any()) } - verifyOrder { - previousWizard.dispose() - mockContext.popupPluginMainPage() + assertTrue(restoreCancelled.isCompleted) + verify(exactly = 1) { anyConstructed().close() } + coVerify(exactly = 1) { mockCli.login("link-token") } + verify(exactly = 1) { secrets.storeApiTokenFor(linkedDeployment, "link-token") } + coVerify(exactly = 1) { linkHandler.handle(match { it["workspace"] == "dev" }, linkedDeployment, any(), mockCli) } + verify(exactly = 1) { environmentsPage.showPluginEnvironmentsPage(false) } + assertNull(remoteProvider.getOverrideUiPage(), "The workspace list should still be visible after the handoff") + } finally { + remoteProvider.close() + unmockkStatic(::ensureCLI) + unmockkConstructor(CoderRestClient::class) } } @@ -659,7 +832,6 @@ class CoderRemoteProviderTest { val targets = listOf( "https://coder.example.com", "https://CODER.example.com:443/", - "https://coder.example.com/different/path/", ) for (target in targets) { provider.handleUri(workspaceLink(target)) @@ -724,7 +896,7 @@ class CoderRemoteProviderTest { coEvery { provider["userForToken"](url, "link-token") } returns user coEvery { provider["userForToken"](url, "saved-token") } returns user - provider.handleUri(workspaceLink("https://CODER.example.com:443/different/path")) + provider.handleUri(workspaceLink("https://CODER.example.com:443/")) val page = provider.getOverrideUiPage() as CoderSetupWizardPage assertEquals(WizardStep.CONNECT, page.model.currentStep()) @@ -839,6 +1011,7 @@ class CoderRemoteProviderTest { @Test fun `declining an account change preserves an in-flight auto-connect wizard`() = runTest { + every { mockContext.cs } returns backgroundScope val url = URI("https://coder.example.com").toURL() every { mockContext.deploymentUrl } returns url every { mockContext.settingsStore.requiresMTlsAuth } returns false @@ -850,11 +1023,13 @@ class CoderRemoteProviderTest { val provider = spyk(remoteProvider, recordPrivateCalls = true) coEvery { provider["userForToken"](url, "link-token") } returns DataGen.user().copy(username = "other-user") coEvery { provider["userForToken"](url, "saved-token") } returns DataGen.user().copy(username = "saved-user") - val existingWizard = provider.getOverrideUiPage() as CoderSetupWizardPage + assertNull(provider.getOverrideUiPage()) + val existingWizard = setupWizard(provider) provider.handleUri(workspaceLink("https://coder.example.com")) - assertSame(existingWizard, provider.getOverrideUiPage()) + assertNull(provider.getOverrideUiPage()) + assertSame(existingWizard, setupWizard(provider)) assertEquals(WizardStep.CONNECT, existingWizard.model.currentStep()) assertEquals("saved-token", existingWizard.model.token) verify(exactly = 0) { mockContext.popupPluginMainPage() } @@ -954,13 +1129,28 @@ class CoderRemoteProviderTest { verify(exactly = 0) { mockContext.popupPluginMainPage() } } + @Test + fun `an origin-only link preserves the saved deployment base path`() = runTest { + val origin = URI("https://coder.example.com").toURL() + val saved = "https://coder.example.com/team/" + every { mockContext.settingsStore.requiresMTlsAuth } returns true + every { mockContext.secrets.savedDeploymentUrl(origin) } returns saved + every { mockContext.secrets.isUrlTrusted(URI(saved).toURL()) } returns true + + remoteProvider.handleUri(URI("jetbrains://gateway/coder?url=https://coder.example.com/")) + + val page = remoteProvider.getOverrideUiPage() as CoderSetupWizardPage + assertEquals(saved, page.model.url.toString()) + coVerify(exactly = 0) { mockUi.showYesNoPopup(any(), any(), any(), any()) } + } + @Test fun `known certificate deployment reopens without a trust prompt`() = runTest { every { mockContext.settingsStore.requiresMTlsAuth } returns true every { mockContext.settingsStore.requiresTokenAuth } returns false every { mockContext.secrets.isUrlTrusted(URI("https://coder.example.com").toURL()) } returns true - remoteProvider.handleUri(URI("jetbrains://gateway/com.coder.toolbox?url=https://coder.example.com/path")) + remoteProvider.handleUri(URI("jetbrains://gateway/com.coder.toolbox?url=https://coder.example.com/")) val page = remoteProvider.getOverrideUiPage() as CoderSetupWizardPage assertEquals("https://coder.example.com", page.model.url.toString()) @@ -1029,16 +1219,45 @@ class CoderRemoteProviderTest { every { mockContext.settingsStore } returns settings every { mockContext.cs } returns backgroundScope every { mockClient.token } returns "saved-token" - val callback = remoteProvider.javaClass.getDeclaredField("onConnect").apply { isAccessible = true } - .get(remoteProvider) as SuspendBiConsumer - callback.accept(mockClient, mockCli) + completeSignIn() verify { secrets.storeApiTokenFor(url, "saved-token") } verify { secrets.trustDeployment(url) } assertEquals("https://coder.example.com", settings.lastDeploymentURL) } + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `returning from Settings after sign-in opens the workspace list once even if beforeShow runs twice`() = runTest { + every { mockContext.cs } returns backgroundScope + coEvery { mockClient.workspaces(any()) } returns emptyList() + val wizard = remoteProvider.getOverrideUiPage() as CoderSetupWizardPage + val pages = mockContext.envPageManager + wizard.beforeShow() + runCurrent() + wizard.afterHide() + + completeSignIn() + runCurrent() + verify(exactly = 0) { pages.showPluginEnvironmentsPage(false) } + assertNull(remoteProvider.getOverrideUiPage()) + + // Both visibility callbacks arrive before the step observer runs. + repeat(2) { wizard.beforeShow() } + runCurrent() + verify(exactly = 1) { pages.showPluginEnvironmentsPage(false) } + wizard.afterHide() + remoteProvider.close() + } + + @Suppress("UNCHECKED_CAST") + private suspend fun completeSignIn() { + val callback = remoteProvider.javaClass.getDeclaredField("onConnect").apply { isAccessible = true } + .get(remoteProvider) as SuspendBiConsumer + callback.accept(mockClient, mockCli) + } + @Test fun `declining a fresh link preserves the existing setup wizard`() = runTest { every { mockContext.settingsStore.requiresMTlsAuth } returns false @@ -1091,7 +1310,7 @@ class CoderRemoteProviderTest { coEvery { mockUi.showYesNoPopup(any(), any(), any(), any()) } returns false val i18n = mockContext.i18n - remoteProvider.handleUri(workspaceLink("https://user:password@OTHER.example.com:8443/path")) + remoteProvider.handleUri(workspaceLink("https://OTHER.example.com:8443/")) verify { i18n.pnotr(match { @@ -1111,6 +1330,42 @@ class CoderRemoteProviderTest { verify { underlyingLogger.error(match { it.contains("valid HTTP(S) host and port") }) } } + @ValueSource(strings = [ + "jetbrains://gateway/coder?url=https://coder.example.com/team&token=private-value", + "jetbrains://gateway/com.coder.toolbox?token=private-value&%74oken=private-value", + "jetbrains://gateway/com.coder.toolbox/auth?state=a&state=b", + "jetbrains://gateway/com.coder.toolbox/auth-extra?code=private-value", + "jetbrains://gateway/com.coder.toolbox?url=https://user@host&token=private-value", + ]) + @ParameterizedTest + fun `invalid workspace links and OAuth callbacks stop before sign-in`(link: String) = runTest { + remoteProvider.handleUri(URI(link)) + + verify { underlyingLogger.error(any()) } + verify(exactly = 0) { underlyingLogger.error(match { it.contains("private-value") }) } + coVerify(exactly = 0) { mockUi.showYesNoPopup(any(), any(), any(), any()) } + verify(exactly = 0) { mockContext.secrets.apiTokenFor(any()) } + verify(exactly = 0) { mockContext.popupPluginMainPage() } + } + + @Test + fun `rejecting a cold-start link preserves the saved deployment for session restore`() = runTest { + val savedUrl = URI(PRODUCTION_URL).toURL() + every { mockContext.cs } returns backgroundScope + every { mockContext.deploymentUrl } returns savedUrl + every { mockContext.settingsStore.requiresMTlsAuth } returns false + every { mockContext.settingsStore.requiresTokenAuth } returns true + every { mockContext.secrets.apiTokenFor(savedUrl) } returns "saved-token" + + remoteProvider.handleUri(URI("jetbrains://gateway/com.coder.toolbox/unknown")) + + assertNull(remoteProvider.getOverrideUiPage()) + val wizard = setupWizard(remoteProvider) + assertEquals(savedUrl, wizard.model.url) + assertEquals("saved-token", wizard.model.token) + assertEquals(WizardStep.CONNECT, wizard.model.currentStep()) + } + private fun workspaceLink(url: String): URI = URI( "jetbrains://gateway/com.coder.toolbox?url=${URLEncoder.encode(url, Charsets.UTF_8)}" + "&token=link-token&workspace=dev" @@ -1137,87 +1392,42 @@ class CoderRemoteProviderTest { assertFalse(accountDropdown.visibility.value) } - @Test - fun `given mTLS is required when auto setup has stored credentials then mTLS takes precedence`() { - // given - val url = URI("https://coder.example.com").toURL() - every { mockContext.deploymentUrl } returns url - every { mockContext.settingsStore.requiresMTlsAuth } returns true - every { mockContext.settingsStore.requiresTokenAuth } returns false - every { mockContext.settingsStore.preferOAuth2IfAvailable } returns true - every { mockContext.secrets.apiTokenFor(url) } returns "token" - every { mockContext.secrets.oauthSessionFor(url.toString()) } returns storedOAuthSession() - val provider = CoderRemoteProvider(mockContext) - - // when - val overridePage = provider.getOverrideUiPage() as CoderSetupWizardPage - - // then - assertEquals(WizardStep.CONNECT, overridePage.model.currentStep()) - assertNull(overridePage.model.token) - assertNull(overridePage.model.oauthSession) - } - - @Test - fun `given OAuth is preferred when auto setup has token and OAuth session then OAuth is used`() { - // given - val url = URI("https://coder.example.com").toURL() - every { mockContext.deploymentUrl } returns url - every { mockContext.settingsStore.requiresMTlsAuth } returns false - every { mockContext.settingsStore.requiresTokenAuth } returns true - every { mockContext.settingsStore.preferOAuth2IfAvailable } returns true - every { mockContext.secrets.apiTokenFor(url) } returns "token" - every { mockContext.secrets.oauthSessionFor(url.toString()) } returns storedOAuthSession() - val provider = CoderRemoteProvider(mockContext) - - // when - val overridePage = provider.getOverrideUiPage() as CoderSetupWizardPage - - // then - assertEquals(WizardStep.CONNECT, overridePage.model.currentStep()) - assertNull(overridePage.model.token) - assertNotNull(overridePage.model.oauthSession) - } - - @Test - fun `given OAuth is not preferred when auto setup has token and OAuth session then token is used`() { - // given - val url = URI("https://coder.example.com").toURL() - every { mockContext.deploymentUrl } returns url - every { mockContext.settingsStore.requiresMTlsAuth } returns false - every { mockContext.settingsStore.requiresTokenAuth } returns true - every { mockContext.settingsStore.preferOAuth2IfAvailable } returns false - every { mockContext.secrets.apiTokenFor(url) } returns "token" - every { mockContext.secrets.oauthSessionFor(url.toString()) } returns storedOAuthSession() - val provider = CoderRemoteProvider(mockContext) - - // when - val overridePage = provider.getOverrideUiPage() as CoderSetupWizardPage - - // then - assertEquals(WizardStep.CONNECT, overridePage.model.currentStep()) - assertEquals("token", overridePage.model.token) - assertNull(overridePage.model.oauthSession) - } - - @Test - fun `given OAuth is not preferred when auto setup has API token then token is used`() { - // given - val url = URI("https://coder.example.com").toURL() + @CsvSource( + "require mTLS, prefer OAuth, saved API token, saved OAuth session, expected API token, expect OAuth", + "true, true, api-token, true, none, false", + "false, true, api-token, true, none, true", + "false, false, api-token, true, api-token, false", + "false, false, api-token, false, api-token, false", + "false, true, api-token, false, api-token, false", + "false, true, none, true, none, true", + nullValues = ["none"], + useHeadersInDisplayName = true, + ) + @ParameterizedTest(name = "Restore credentials, {arguments}") + fun `session restore selects credentials using the deployment settings`( + requireMtls: Boolean, + preferOAuth: Boolean, + savedApiToken: String?, + hasSavedOAuthSession: Boolean, + expectedApiToken: String?, + expectOAuth: Boolean, + ) = runTest { + every { mockContext.cs } returns backgroundScope + val url = mockClient.url + val savedOAuthSession = if (hasSavedOAuthSession) storedOAuthSession() else null every { mockContext.deploymentUrl } returns url - every { mockContext.settingsStore.requiresMTlsAuth } returns false - every { mockContext.settingsStore.requiresTokenAuth } returns true - every { mockContext.settingsStore.preferOAuth2IfAvailable } returns false - every { mockContext.secrets.apiTokenFor(url) } returns "api-token" - val provider = CoderRemoteProvider(mockContext) + every { mockContext.settingsStore.requiresMTlsAuth } returns requireMtls + every { mockContext.settingsStore.requiresTokenAuth } returns !requireMtls + every { mockContext.settingsStore.preferOAuth2IfAvailable } returns preferOAuth + every { mockContext.secrets.apiTokenFor(url) } returns savedApiToken + every { mockContext.secrets.oauthSessionFor(url.toString()) } returns savedOAuthSession - // when - val overridePage = provider.getOverrideUiPage() as CoderSetupWizardPage + assertNull(remoteProvider.getOverrideUiPage()) + val model = setupWizard(remoteProvider).model - // then - assertEquals(WizardStep.CONNECT, overridePage.model.currentStep()) - assertEquals("api-token", overridePage.model.token) - assertNull(overridePage.model.oauthSession) + assertEquals(WizardStep.CONNECT, model.currentStep()) + assertEquals(expectedApiToken, model.token) + assertEquals(if (expectOAuth) requireNotNull(savedOAuthSession).toSessionContext() else null, model.oauthSession) } @Test diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 02adc5c1..f150f9bc 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -61,6 +61,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout +import org.junit.jupiter.api.Assumptions.assumeTrue import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.assertDoesNotThrow import org.junit.jupiter.api.condition.EnabledOnOs @@ -486,12 +487,8 @@ internal class CoderCLIManagerTest { // download a working CLI and that it runs on each platform. @Test fun testDownloadRealCLI() { - var url = System.getenv("CODER_TOOLBOX_TEST_DEPLOYMENT") - if (url == "mock") { - return - } else if (url == null) { - url = "https://dev.coder.com" - } + val url = System.getenv("CODER_TOOLBOX_TEST_DEPLOYMENT") + assumeTrue(!url.isNullOrBlank() && url != "mock", "Set a deployment that serves its CLI without a cross-origin redirect.") val ccm = CoderCLIManager( context.copy( @@ -504,7 +501,7 @@ internal class CoderCLIManagerTest { context.logger ) ), - url.toURL(), + requireNotNull(url).toURL(), ) assertTrue(runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) @@ -734,6 +731,7 @@ internal class CoderCLIManagerTest { session.writeText("old-token") val loginArgs = cli.login("token") + assertEquals(version != "v2.29.0", "--allow-redirects=false" in loginArgs) assertContains(loginArgs, "--url $url --use-keyring=true") assertFalse(loginArgs.contains("--global-config")) assertFalse(session.exists(), "Successful keyring login removes the plaintext token") @@ -743,6 +741,7 @@ internal class CoderCLIManagerTest { val config = cli.coderConfigPath.parent.resolve("ssh-config") cli.configSsh(setOf(address), sshConfigPath = config.toString()) assertContains(Files.readString(config), "--use-keyring=true") + assertEquals(version != "v2.29.0", "--allow-redirects=false" in Files.readString(config)) assertContains(cli.startWorkspace(address), "--use-keyring=true") assertContains(cli.logout(), "--url $url --use-keyring=true") } @@ -1231,8 +1230,8 @@ internal class CoderCLIManagerTest { verify(exactly = 1) { underlyingLogger.info("$sessionPrefix Finished configuring SSH config") } - // SSH setup no longer runs the CLI to detect features. - verify(exactly = 0) { + // SSH setup checks the binary once for redirect flag support. + verify(exactly = 1) { underlyingLogger.info(match { it.contains("${ccm.localBinaryPath} version --output json") }) } } @@ -1329,7 +1328,12 @@ internal class CoderCLIManagerTest { testDirectory.resolve("coder") } binaryPath.parent.toFile().mkdirs() - binaryPath.toFile().writeText(mkbin(script)) + val versionCommand = if (getOS() == OS.WINDOWS) { + """if "%1"=="version" (echo {"version":"2.38.0"} & exit /b 0)""" + } else { + """if [ "$1" = version ]; then echo '{"version":"2.38.0"}'; exit 0; fi""" + } + binaryPath.toFile().writeText(mkbin(versionCommand + System.lineSeparator() + script)) if (getOS() != OS.WINDOWS) { binaryPath.toFile().setExecutable(true) } diff --git a/src/test/kotlin/com/coder/toolbox/sdk/DeploymentRedirectTest.kt b/src/test/kotlin/com/coder/toolbox/sdk/DeploymentRedirectTest.kt new file mode 100644 index 00000000..7d6a1f28 --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/sdk/DeploymentRedirectTest.kt @@ -0,0 +1,57 @@ +package com.coder.toolbox.sdk + +import com.coder.toolbox.sdk.interceptors.Interceptors +import com.sun.net.httpserver.HttpServer +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody.Companion.toRequestBody +import java.net.InetSocketAddress +import java.net.ProtocolException +import java.util.concurrent.atomic.AtomicInteger +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +class DeploymentRedirectTest { + @Test + fun `reads can follow a relative redirect but cannot leave their origin`() { + val first = HttpServer.create(InetSocketAddress("127.0.0.1", 0), 0) + val second = HttpServer.create(InetSocketAddress("127.0.0.1", 0), 0) + val visits = AtomicInteger() + val origin = "http://127.0.0.1:${first.address.port}" + val http = OkHttpClient.Builder().followRedirects(false).addInterceptor(Interceptors.sameOriginRedirects()).build() + first.createContext("/local") { exchange -> + exchange.responseHeaders.add("Location", "/result") + exchange.sendResponseHeaders(302, -1) + exchange.close() + } + first.createContext("/result") { exchange -> + exchange.sendResponseHeaders(204, -1) + exchange.close() + } + first.createContext("/away") { exchange -> + exchange.responseHeaders.add("Location", "http://127.0.0.1:${second.address.port}/") + exchange.sendResponseHeaders(307, -1) + exchange.close() + } + second.createContext("/") { exchange -> + visits.incrementAndGet() + exchange.sendResponseHeaders(204, -1) + exchange.close() + } + try { + first.start() + second.start() + val local = Request.Builder().url("$origin/local").build() + http.newCall(local).execute().use { assertEquals(204, it.code) } + http.newCall(local.newBuilder().post("".toRequestBody()).build()).execute().use { assertEquals(302, it.code) } + val away = Request.Builder().url("$origin/away").build() + assertFailsWith { http.newCall(away).execute().close() } + assertEquals(0, visits.get()) + } finally { + first.stop(0) + second.stop(0) + http.connectionPool.evictAll() + } + } +} diff --git a/src/test/kotlin/com/coder/toolbox/util/InMemoryPluginSecretStore.kt b/src/test/kotlin/com/coder/toolbox/util/InMemoryPluginSecretStore.kt new file mode 100644 index 00000000..f90259ea --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/util/InMemoryPluginSecretStore.kt @@ -0,0 +1,17 @@ +package com.coder.toolbox.util + +import com.jetbrains.toolbox.api.core.PluginSecretStore + +internal class InMemoryPluginSecretStore : PluginSecretStore { + private val values = mutableMapOf() + + override fun get(key: String): String? = values[key] + + override fun set(key: String, value: String) { + values[key] = value + } + + override fun clear(key: String) { + values.remove(key) + } +} diff --git a/src/test/kotlin/com/coder/toolbox/util/URLExtensionsTest.kt b/src/test/kotlin/com/coder/toolbox/util/URLExtensionsTest.kt index 01e303fd..f97eb735 100644 --- a/src/test/kotlin/com/coder/toolbox/util/URLExtensionsTest.kt +++ b/src/test/kotlin/com/coder/toolbox/util/URLExtensionsTest.kt @@ -98,89 +98,33 @@ internal class URLExtensionsTest { } @Test - fun `valid https URL with path and query should return Valid`() { - val result = "https://coder.com/bin/coder-linux-amd64?query=1".validateStrictWebUrl() - assertEquals(WebUrlValidationResult.Valid, result) - } - - @Test - fun `relative URL should return Invalid with appropriate message`() { - val url = "/bin/coder-linux-amd64" - val result = url.validateStrictWebUrl() - assertEquals( - WebUrlValidationResult.Invalid("The URL \"/bin/coder-linux-amd64\" is missing a scheme (like https://). Please enter a full web address like \"https://example.com\""), - result - ) - } - - @Test - fun `opaque URI like mailto should return Invalid`() { - val url = "mailto:user@coder.com" - val result = url.validateStrictWebUrl() - assertEquals( - WebUrlValidationResult.Invalid("The URL \"mailto:user@coder.com\" is invalid because it is not in the standard format. Please enter a full web address like \"https://example.com\""), - result - ) - } - - @Test - fun `unsupported scheme like ftp should return Invalid`() { - val url = "ftp://coder.com" - val result = url.validateStrictWebUrl() - assertEquals( - WebUrlValidationResult.Invalid("The URL \"ftp://coder.com\" must start with http:// or https://, not \"ftp\""), - result - ) - } - - @Test - fun `http URL with missing authority should return Invalid`() { - val url = "http:///bin/coder-linux-amd64" - val result = url.validateStrictWebUrl() - assertEquals( - WebUrlValidationResult.Invalid("The URL \"http:///bin/coder-linux-amd64\" does not include a valid website name. Please enter a full web address like \"https://example.com\""), - result - ) - } - - @Test - fun `malformed URI should return Invalid with parsing error message`() { - val url = "http://[invalid-uri]" - val result = url.validateStrictWebUrl() - assertEquals( - WebUrlValidationResult.Invalid("The input \"http://[invalid-uri]\" is not a valid web address. Please enter a full web address like \"https://example.com\""), - result - ) - } - - @Test - fun `URI without colon should return Invalid as URI is not absolute`() { - val url = "http//coder.com" - val result = url.validateStrictWebUrl() - assertEquals( - WebUrlValidationResult.Invalid("The URL \"http//coder.com\" is missing a scheme (like https://). Please enter a full web address like \"https://example.com\""), - result - ) + fun `manual setup accepts base paths and links require an origin`() { + val url = "https://coder.example.com/team/" + assertEquals(WebUrlValidationResult.Valid, url.validateStrictWebUrl()) + assertTrue(url.validateStrictWebUrl(originOnly = true) is WebUrlValidationResult.Invalid) + for (origin in listOf("http://localhost:3000", "https://[::1]:8443/", "https://ほげ.example/")) { + assertEquals(WebUrlValidationResult.Valid, origin.validateStrictWebUrl(originOnly = true)) + } } @Test - fun `URI without double forward slashes should return Invalid because the URI is not hierarchical`() { - val url = "http:coder.com" - val result = url.validateStrictWebUrl() - assertEquals( - WebUrlValidationResult.Invalid("The URL \"http:coder.com\" is invalid because it is not in the standard format. Please enter a full web address like \"https://example.com\""), - result - ) + fun `invalid deployment URLs are rejected without displaying their values`() { + for (url in listOf("/relative", "file:///tmp", "https:/missing", "https://host:99999", + "https://user@host", "https://host/?private", "https://host/#private", "https://host/a%20b", + "https://host/dollar$", "https://host/../team", "https://host;other")) { + val result = url.validateStrictWebUrl() + assertTrue(result is WebUrlValidationResult.Invalid) + assertFalse(result.reason.contains(url)) + } } @Test - fun `URI without a single forward slash should return Invalid because the URI does not have a hostname`() { - val url = "https:/coder.com" - val result = url.validateStrictWebUrl() - assertEquals( - WebUrlValidationResult.Invalid("The URL \"https:/coder.com\" does not include a valid website name. Please enter a full web address like \"https://example.com\""), - result - ) + fun `query components are split before decoding and decoded exactly once`() { + val params = URI("jetbrains://gateway/coder?folder=a%26b%3Dc%2B%252F&owner=a+b").toQueryParameters() + assertEquals(mapOf("folder" to "a&b=c+%2F", "owner" to "a b"), params) + for (query in listOf("url=a&url=a", "state=a&%73tate=b", "token=%FF")) { + kotlin.test.assertFailsWith { URI("jetbrains://gateway/coder?$query").toQueryParameters() } + } } @Test diff --git a/src/test/kotlin/com/coder/toolbox/views/CoderSetupWizardPageTest.kt b/src/test/kotlin/com/coder/toolbox/views/CoderSetupWizardPageTest.kt new file mode 100644 index 00000000..5cf3adc2 --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/views/CoderSetupWizardPageTest.kt @@ -0,0 +1,347 @@ +package com.coder.toolbox.views + +import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.cli.CoderCLIManager +import com.coder.toolbox.cli.ensureCLI +import com.coder.toolbox.oauth.OAuthTokenResponse +import com.coder.toolbox.oauth.TokenEndpointAuthMethod +import com.coder.toolbox.sdk.CoderRestClient +import com.coder.toolbox.sdk.DataGen +import com.coder.toolbox.settings.Environment +import com.coder.toolbox.store.CoderSettingsStore +import com.coder.toolbox.util.pluginTestSettingsStore +import com.coder.toolbox.views.state.CoderOAuthSessionContext +import com.coder.toolbox.views.state.ConnectionOrigin +import com.coder.toolbox.views.state.Credentials +import com.coder.toolbox.views.state.PageRouter +import com.coder.toolbox.views.state.WizardModel +import com.coder.toolbox.views.state.WizardStep +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.justRun +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkStatic +import io.mockk.spyk +import io.mockk.unmockkConstructor +import io.mockk.unmockkStatic +import io.mockk.verify +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.job +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.EnumSource +import java.net.Proxy +import java.net.URI +import kotlin.coroutines.CoroutineContext +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.test.assertSame + +@OptIn(ExperimentalCoroutinesApi::class) +class CoderSetupWizardPageTest { + private val context = mockk(relaxed = true) + private val url = URI("https://coder.example.test").toURL() + private val model = WizardModel().apply { + url = this@CoderSetupWizardPageTest.url + token = "test-token" + goTo(WizardStep.CONNECT) + } + private val onConnect = mockk>(relaxed = true) + + @Test + fun `routing starts saved session restore and repeated page lookups do not log in again`() = runTest { + every { context.cs } returns backgroundScope + val page = CoderSetupWizardPage.connectStep( + context, mockk(relaxed = true), url, Credentials.Token("test-token"), onConnect, origin = ConnectionOrigin.SAVED_SESSION, + ) + val pages = context.envPageManager + val ui = context.ui + + withMockCli { cli -> + // Toolbox can ask for the page repeatedly. A lookup alone must not start a job. + repeat(2) { assertNull(page.pageForDisplay()) } + runCurrent() + coVerify(exactly = 0) { cli.login(any()) } + + val router = PageRouter() + router.getOrCreate { page } + runCurrent() + coVerify(exactly = 1) { cli.login("test-token") } + repeat(2) { assertNull(router.getOrCreate { error("The routed page should be reused") }.pageForDisplay()) } + runCurrent() + + coVerify(exactly = 1) { cli.login("test-token") } + coVerify(exactly = 1) { onConnect.accept(any(), cli) } + verify(exactly = 0) { pages.showPluginEnvironmentsPage(any()) } + coVerify(exactly = 0) { ui.showUiPageSuspending(any()) } + } + } + + @Test + fun `a failed restore presents the sign-in form and permits a new connection`() = runTest { + every { context.cs } returns backgroundScope + val page = CoderSetupWizardPage.connectStep( + context, mockk(relaxed = true), url, Credentials.Token("test-token"), onConnect, origin = ConnectionOrigin.SAVED_SESSION, + ) + val ui = context.ui + + withMockCli { cli -> + coEvery { cli.login("test-token") } throws IllegalStateException("Keychain is locked") andThen "Signed in" + PageRouter().navigate(page) + assertNull(page.pageForDisplay()) + runCurrent() + + assertEquals(WizardStep.URL_REQUEST, page.model.currentStep()) + assertEquals(ConnectionOrigin.FORM, page.model.connectionOrigin) + assertSame(page, page.pageForDisplay()) + coVerify(exactly = 1) { ui.showUiPageSuspending(page) } + coVerify(exactly = 0) { onConnect.accept(any(), any()) } + + page.model.url = url + page.model.token = "test-token" + page.model.goTo(WizardStep.CONNECT) + page.beforeShow() + runCurrent() + coVerify(exactly = 2) { cli.login("test-token") } + coVerify(exactly = 1) { onConnect.accept(any(), cli) } + page.dispose() + } + } + + @Test + fun `an OAuth credential update failure keeps the signed-in wizard on the workspace list`() = runTest { + every { context.cs } returns backgroundScope + val session = CoderOAuthSessionContext( + clientId = "client-id", + clientSecret = "client-secret", + tokenCodeVerifier = "verifier", + state = "state", + tokenEndpoint = "$url/oauth/token", + tokenResponse = OAuthTokenResponse("access-token", "Bearer", null, "refresh-token", null), + tokenAuthMethod = TokenEndpointAuthMethod.CLIENT_SECRET_BASIC, + ) + val router = PageRouter() + val failure = IllegalStateException("Keychain is locked") + every { context.secrets.storeOAuthFor(url.toString(), session) } throws failure + val page = router.getOrCreate { + CoderSetupWizardPage.connectStep( + context, mockk(), url, Credentials.OAuth(session), + onConnect = SuspendBiConsumer { _, _ -> router.finishSignIn(currentCoroutineContext().job) {} }, + ) + } + val pages = context.envPageManager + val logger = context.logger + val log = logger.delegate + + withMockCli { + context.settingsStore.updatePreferAuthViaOAuth2(true) + page.beforeShow() + runCurrent() + + assertEquals(WizardStep.SIGNED_IN, page.model.currentStep()) + verify(exactly = 1) { pages.showPluginEnvironmentsPage(false) } + verify(exactly = 1) { log.warn(failure, any()) } + verify(exactly = 0) { logger.showInfoPopup(any(), any()) } + verify(exactly = 0) { anyConstructed().close() } + page.dispose() + } + } + + @Test + fun `submitting a token displays the connection step once`() = runTest { + every { context.cs } returns backgroundScope + every { context.secrets.apiTokenFor(url) } returns "test-token" + mockkConstructor(ConnectStep::class) + try { + justRun { anyConstructed().onVisible() } + val page = CoderSetupWizardPage.deploymentUrlStep(context, mockk(), mockk()) + page.model.url = url + page.model.goTo(WizardStep.TOKEN_REQUEST) + page.beforeShow() + runCurrent() + + page.actionButtons.value.first().run() + runCurrent() + + assertEquals(WizardStep.CONNECT, page.model.currentStep()) + verify(exactly = 1) { anyConstructed().onVisible() } + + page.dispose() + // Toolbox delivers a late callback after the page observer has been disposed. + page.beforeShow() + runCurrent() + verify(exactly = 1) { anyConstructed().onVisible() } + } finally { + unmockkConstructor(ConnectStep::class) + } + } + + @EnumSource(ConnectionOrigin::class) + @ParameterizedTest(name = "Repeated visibility during sign-in from {0}") + fun `a second visibility callback during startup logs in to the CLI once`(origin: ConnectionOrigin) = runTest { + model.connectionOrigin = origin + lateinit var step: ConnectStep + val scheduler = StandardTestDispatcher(testScheduler) + val dispatcher = object : CoroutineDispatcher() { + private var firstDispatch = true + + override fun dispatch(context: CoroutineContext, block: Runnable) { + if (firstDispatch) { + firstDispatch = false + // Deliver the second callback while the first is still dispatching its sign-in job. + step.onVisible() + } + scheduler.dispatch(context, block) + } + } + every { context.cs } returns CoroutineScope(backgroundScope.coroutineContext + dispatcher) + step = ConnectStep(context, model, { model.goTo(WizardStep.TOKEN_REQUEST) }, onConnect) + withMockCli { cli -> + step.onVisible() + runCurrent() + + coVerify(exactly = 1) { cli.login("test-token") } + coVerify(exactly = 1) { onConnect.accept(any(), cli) } + } + } + + @Test + fun `certificate sign-in connects without a CLI login`() = runTest { + every { context.cs } returns backgroundScope + model.token = null + val step = ConnectStep(context, model, { model.goTo(WizardStep.URL_REQUEST) }, onConnect) + withMockCli { cli -> + val settings = spyk(context.settingsStore) + every { settings.requiresTokenAuth } returns false + every { context.settingsStore } returns settings + + step.onVisible() + runCurrent() + + coVerify(exactly = 0) { cli.login(any()) } + coVerify(exactly = 1) { onConnect.accept(any(), cli) } + } + } + + @Test + fun `switching from certificates to token authentication during setup reports the missing token`() = runTest { + every { context.cs } returns backgroundScope + model.token = null + val step = ConnectStep(context, model, { model.goTo(WizardStep.TOKEN_REQUEST) }, onConnect) + val logger = context.logger + withMockCli { cli -> + val settings = spyk(context.settingsStore) + var requiresToken = false + every { settings.requiresTokenAuth } answers { requiresToken } + every { context.settingsStore } returns settings + coEvery { anyConstructed().initializeSession() } coAnswers { + requiresToken = true + DataGen.user() + } + step.onVisible() + runCurrent() + + assertEquals(WizardStep.TOKEN_REQUEST, model.currentStep()) + verify { logger.showInfoPopup(any(), match { it.contains("OAuth or API token is required") }) } + coVerify(exactly = 0) { cli.login(any()) } + coVerify(exactly = 0) { onConnect.accept(any(), any()) } + } + } + + private fun withMockCli(test: (CoderCLIManager) -> Unit) { + every { context.settingsStore } returns CoderSettingsStore( + pluginTestSettingsStore(), Environment(), mockk(relaxed = true), + ) + every { context.proxySettings.getProxy() } returns Proxy.NO_PROXY + every { context.proxySettings.getProxyAuth() } returns null + val cli = mockk(relaxed = true) + mockkConstructor(CoderRestClient::class) + mockkStatic(::ensureCLI) + try { + coEvery { anyConstructed().initializeSession() } returns DataGen.user() + every { anyConstructed().buildVersion } returns "2.39.0" + coEvery { ensureCLI(context, url, "2.39.0", any()) } returns cli + test(cli) + } finally { + unmockkStatic(::ensureCLI) + unmockkConstructor(CoderRestClient::class) + } + } + + @Test + fun `a retry waits for a cancelled login and ignores its late error`() = runTest { + every { context.cs } returns backgroundScope + val step = ConnectStep(context, model, { model.goTo(WizardStep.TOKEN_REQUEST) }, onConnect) + val allowCleanup = CompletableDeferred() + withMockCli { cli -> + coEvery { cli.login("test-token") } coAnswers { + try { + awaitCancellation() + } catch (ex: CancellationException) { + withContext(NonCancellable) { allowCleanup.await() } + throw IllegalStateException("Cancelled CLI process exited", ex) + } + } andThen "Signed in" + try { + step.onVisible() + runCurrent() + step.onBack() + assertEquals(WizardStep.TOKEN_REQUEST, model.currentStep()) + + // Retry while the cancelled CLI login is still waiting for cleanup. + model.goTo(WizardStep.CONNECT) + step.onVisible() + runCurrent() + coVerify(exactly = 1) { cli.login("test-token") } + + allowCleanup.complete(Unit) + runCurrent() + + assertEquals(WizardStep.CONNECT, model.currentStep()) + coVerify(exactly = 2) { cli.login("test-token") } + coVerify(exactly = 1) { onConnect.accept(any(), cli) } + } finally { + allowCleanup.complete(Unit) + step.dispose() + } + } + } + + @Test + fun `a genuine CLI failure returns to token entry and allows a new sign-in`() = runTest { + every { context.cs } returns backgroundScope + val step = ConnectStep(context, model, { model.goTo(WizardStep.TOKEN_REQUEST) }, onConnect) + withMockCli { cli -> + coEvery { cli.login("test-token") } throws IllegalStateException("Keychain is locked") andThen "Signed in" + step.onVisible() + runCurrent() + assertEquals(WizardStep.TOKEN_REQUEST, model.currentStep()) + coVerify(exactly = 0) { onConnect.accept(any(), any()) } + + model.goTo(WizardStep.CONNECT) + step.onVisible() + runCurrent() + + step.dispose() + // A late visibility callback after disposal must leave the login count at two. + step.onVisible() + runCurrent() + + coVerify(exactly = 2) { cli.login("test-token") } + coVerify(exactly = 1) { onConnect.accept(any(), cli) } + } + } +} diff --git a/src/test/kotlin/com/coder/toolbox/views/state/PageRouterTest.kt b/src/test/kotlin/com/coder/toolbox/views/state/PageRouterTest.kt index c0fd8dbc..5c2fefde 100644 --- a/src/test/kotlin/com/coder/toolbox/views/state/PageRouterTest.kt +++ b/src/test/kotlin/com/coder/toolbox/views/state/PageRouterTest.kt @@ -4,10 +4,19 @@ import com.coder.toolbox.oauth.TokenEndpointAuthMethod import com.coder.toolbox.views.CoderSetupWizardPage import io.mockk.every import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Job import java.net.URI +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNull +import kotlin.test.assertSame +import kotlin.test.assertTrue class PageRouterTest { @@ -42,6 +51,51 @@ class PageRouterTest { assertEquals(pendingConnection, pendingOAuthConnection) } + @Test + fun `a workspace link replaces a wizard being built for auto-connect`() { + val router = PageRouter() + val automatic = mockk(relaxed = true) + val fromLink = mockk(relaxUnitFun = true) + val building = CountDownLatch(1) + val switching = CountDownLatch(1) + val workers = Executors.newFixedThreadPool(2) + try { + val autoConnect = workers.submit { + router.getOrCreate { + building.countDown() + check(switching.await(5, TimeUnit.SECONDS)) + automatic + } + } + assertTrue(building.await(5, TimeUnit.SECONDS)) + val openLink = workers.submit { + switching.countDown() + router.navigate(fromLink) + } + autoConnect.get(5, TimeUnit.SECONDS) + openLink.get(5, TimeUnit.SECONDS) + + assertSame(fromLink, router.activePage) + verify(exactly = 1) { automatic.dispose() } + } finally { + workers.shutdownNow() + } + } + + @Test + fun `a cancelled sign-in cannot publish a session or dismiss its replacement`() { + val router = PageRouter() + val replacement = mockk(relaxUnitFun = true) + router.navigate(replacement) + val cancelledConnection = Job().apply { cancel() } + + assertFailsWith { + router.finishSignIn(cancelledConnection) { error("The cancelled session was started") } + } + + assertSame(replacement, router.activePage) + } + private fun oauthSession(): CoderOAuthSessionContext = CoderOAuthSessionContext( clientId = "client-id", clientSecret = "client-secret",