Skip to content

Commit 0403ee2

Browse files
committed
impl: rework the handling of missing token code
- the existing code was trying indefinitely to ask for token until the user gets it right. This is not a bad idea, however Toolbox has a couple of limitations that make the existing approach almost unusable: - the input dialog doesn't allow custom actions through which we can spawn a browser at login page. The code always opened the login page when the token was wrong which ended up hammering the browser with too many tabs. - the token UI page can't be reused to request the login page (this one has a "Get token" action button) because once the user clicks on the Get token to open the webpage, Toolbox closes the window and forgets the last UI page that was visible. - instead with this patch we ask the token from the user only once. If something goes wrong (mostly during login) we show an error dialog and stop the flow.
1 parent 3108f7d commit 0403ee2

File tree

3 files changed

+38
-90
lines changed

3 files changed

+38
-90
lines changed

src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt

+14-32
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,10 @@ import com.coder.toolbox.cli.ensureCLI
66
import com.coder.toolbox.models.WorkspaceAndAgentStatus
77
import com.coder.toolbox.plugin.PluginManager
88
import com.coder.toolbox.sdk.CoderRestClient
9-
import com.coder.toolbox.sdk.ex.APIResponseException
109
import com.coder.toolbox.sdk.v2.models.Workspace
1110
import com.coder.toolbox.sdk.v2.models.WorkspaceAgent
1211
import com.coder.toolbox.sdk.v2.models.WorkspaceStatus
1312
import com.coder.toolbox.settings.CoderSettings
14-
import com.coder.toolbox.settings.Source
1513
import kotlinx.coroutines.flow.StateFlow
1614
import kotlinx.coroutines.flow.filter
1715
import kotlinx.coroutines.launch
@@ -48,12 +46,7 @@ open class CoderProtocolHandler(
4846
return
4947
}
5048

51-
val queryTokenRaw = params.token()
52-
val queryToken = if (!queryTokenRaw.isNullOrBlank()) {
53-
Pair(queryTokenRaw, Source.QUERY)
54-
} else {
55-
null
56-
}
49+
val queryToken = params.token()
5750
val restClient = try {
5851
authenticate(deploymentURL, queryToken)
5952
} catch (ex: MissingArgumentException) {
@@ -158,35 +151,29 @@ open class CoderProtocolHandler(
158151
}
159152

160153
/**
161-
* Return an authenticated Coder CLI, asking for the token as long as it
162-
* continues to result in an authentication failure and token authentication
163-
* is required.
164-
*
165-
* Throw MissingArgumentException if the user aborts. Any network or invalid
154+
* Return an authenticated Coder CLI, asking for the token.
155+
* Throw MissingArgumentException if the user aborts. Any network or invalid
166156
* token error may also be thrown.
167157
*/
168158
private suspend fun authenticate(
169159
deploymentURL: String,
170-
tryToken: Pair<String, Source>?,
171-
error: String? = null,
160+
tryToken: String?
172161
): CoderRestClient {
173162
val token =
174163
if (settings.requireTokenAuth) {
175164
// Try the provided token immediately on the first attempt.
176-
if (tryToken != null && error == null) {
165+
if (!tryToken.isNullOrBlank()) {
177166
tryToken
178167
} else {
168+
context.ui.showWindow()
169+
context.envPageManager.showPluginEnvironmentsPage(false)
179170
// Otherwise ask for a new token, showing the previous token.
180-
dialogUi.askToken(
181-
deploymentURL.toURL(),
182-
tryToken,
183-
useExisting = true,
184-
error,
185-
)
171+
dialogUi.askToken(deploymentURL.toURL())
186172
}
187173
} else {
188174
null
189175
}
176+
190177
if (settings.requireTokenAuth && token == null) { // User aborted.
191178
throw MissingArgumentException("Token is required")
192179
}
@@ -195,23 +182,18 @@ open class CoderProtocolHandler(
195182
val client = CoderRestClient(
196183
context,
197184
deploymentURL.toURL(),
198-
token?.first,
185+
token,
199186
settings,
200-
proxyValues = null,
187+
proxyValues = null, // TODO - not sure the above comment applies as we are creating our own http client
201188
PluginManager.pluginInfo.version,
202189
httpClient
203190
)
204191
return try {
205192
client.authenticate()
206193
client
207-
} catch (ex: APIResponseException) {
208-
// If doing token auth we can ask and try again.
209-
if (settings.requireTokenAuth && ex.isUnauthorized) {
210-
val msg = humanizeConnectionError(client.url, true, ex)
211-
authenticate(deploymentURL, token, msg)
212-
} else {
213-
throw ex
214-
}
194+
} catch (ex: Exception) {
195+
context.ui.showErrorInfoPopup(IllegalStateException(humanizeConnectionError(client.url, true, ex)))
196+
throw ex
215197
}
216198
}
217199

src/main/kotlin/com/coder/toolbox/util/Dialogs.kt

+23-57
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package com.coder.toolbox.util
33
import com.coder.toolbox.CoderToolboxContext
44
import com.coder.toolbox.browser.BrowserUtil
55
import com.coder.toolbox.settings.CoderSettings
6-
import com.coder.toolbox.settings.Source
76
import com.jetbrains.toolbox.api.localization.LocalizableString
87
import com.jetbrains.toolbox.api.ui.components.TextType
98
import java.net.URL
@@ -26,9 +25,6 @@ class DialogUi(
2625
title: LocalizableString,
2726
description: LocalizableString,
2827
placeholder: LocalizableString? = null,
29-
// TODO check: there is no link or error support in Toolbox so for now isError and link are unused.
30-
isError: Boolean = false,
31-
link: Pair<String, String>? = null,
3228
): String? {
3329
return context.ui.showTextInputPopup(
3430
title,
@@ -40,68 +36,38 @@ class DialogUi(
4036
)
4137
}
4238

39+
suspend fun askPassword(
40+
title: LocalizableString,
41+
description: LocalizableString,
42+
placeholder: LocalizableString? = null,
43+
): String? {
44+
return context.ui.showTextInputPopup(
45+
title,
46+
description,
47+
placeholder,
48+
TextType.Password,
49+
context.i18n.ptrl("OK"),
50+
context.i18n.ptrl("Cancel")
51+
)
52+
}
53+
4354
private suspend fun openUrl(url: URL) {
4455
BrowserUtil.browse(url.toString()) {
4556
context.ui.showErrorInfoPopup(it)
4657
}
4758
}
4859

4960
/**
50-
* Open a dialog for providing the token. Show any existing token so
51-
* the user can validate it if a previous connection failed.
52-
*
53-
* If we have not already tried once (no error) and the user has not checked
54-
* the existing token box then also open a browser to the auth page.
55-
*
56-
* If the user has checked the existing token box then return the token
57-
* on disk immediately and skip the dialog (this will overwrite any
58-
* other existing token) unless this is a retry to avoid clobbering the
59-
* token that just failed.
61+
* Open a dialog for providing the token.
6062
*/
6163
suspend fun askToken(
6264
url: URL,
63-
token: Pair<String, Source>?,
64-
useExisting: Boolean,
65-
error: String?,
66-
): Pair<String, Source>? {
67-
val getTokenUrl = url.withPath("/login?redirect=%2Fcli-auth")
68-
69-
// On the first run (no error) either open a browser to generate a new
70-
// token or, if using an existing token, use the token on disk if it
71-
// exists otherwise assume the user already copied an existing token and
72-
// they will paste in.
73-
if (error == null) {
74-
if (!useExisting) {
75-
openUrl(getTokenUrl)
76-
} else {
77-
// Look on disk in case we already have a token, either in
78-
// the deployment's config or the global config.
79-
val tryToken = settings.token(url)
80-
if (tryToken != null && tryToken.first != token?.first) {
81-
return tryToken
82-
}
83-
}
84-
}
85-
86-
// On subsequent tries or if not using an existing token, ask the user
87-
// for the token.
88-
val tokenFromUser =
89-
ask(
90-
title = context.i18n.ptrl("Session Token"),
91-
description = context.i18n.pnotr(
92-
error
93-
?: token?.second?.description("token")
94-
?: "No existing token for ${url.host} found."
95-
),
96-
placeholder = token?.first?.let { context.i18n.pnotr(it) },
97-
link = Pair("Session Token:", getTokenUrl.toString()),
98-
isError = error != null,
99-
)
100-
if (tokenFromUser.isNullOrBlank()) {
101-
return null
102-
}
103-
// If the user submitted the same token, keep the same source too.
104-
val source = if (tokenFromUser == token?.first) token.second else Source.USER
105-
return Pair(tokenFromUser, source)
65+
): String? {
66+
openUrl(url.withPath("/login?redirect=%2Fcli-auth"))
67+
return askPassword(
68+
title = context.i18n.ptrl("Session Token"),
69+
description = context.i18n.pnotr("Please paste the session token from the web-page"),
70+
placeholder = context.i18n.pnotr("")
71+
)
10672
}
10773
}

src/main/kotlin/com/coder/toolbox/views/TokenPage.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import java.net.URL
2020
* enter their own.
2121
*/
2222
class TokenPage(
23-
private val context: CoderToolboxContext,
23+
context: CoderToolboxContext,
2424
deploymentURL: URL,
2525
token: Pair<String, Source>?,
2626
private val onToken: ((token: String) -> Unit),

0 commit comments

Comments
 (0)