Skip to content

Commit a78d1ab

Browse files
mafredripull[bot]
authored andcommitted
fix: Protect codersdk.Client SessionToken so it can be updated (#4965)
This feature is used by the coder agent to exchange a new token. By protecting the SessionToken via mutex we ensure there are no data races when accessing it.
1 parent a32c344 commit a78d1ab

25 files changed

+82
-64
lines changed

cli/agent.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func workspaceAgent() *cobra.Command {
9797
if err != nil {
9898
return xerrors.Errorf("CODER_AGENT_TOKEN must be set for token auth: %w", err)
9999
}
100-
client.SessionToken = token
100+
client.SetSessionToken(token)
101101
case "google-instance-identity":
102102
// This is *only* done for testing to mock client authentication.
103103
// This will never be set in a production scenario.
@@ -153,13 +153,13 @@ func workspaceAgent() *cobra.Command {
153153
Logger: logger,
154154
ExchangeToken: func(ctx context.Context) (string, error) {
155155
if exchangeToken == nil {
156-
return client.SessionToken, nil
156+
return client.SessionToken(), nil
157157
}
158158
resp, err := exchangeToken(ctx)
159159
if err != nil {
160160
return "", err
161161
}
162-
client.SessionToken = resp.SessionToken
162+
client.SetSessionToken(resp.SessionToken)
163163
return resp.SessionToken, nil
164164
},
165165
EnvironmentVariables: map[string]string{

cli/clitest/clitest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func NewWithSubcommands(
4343

4444
// SetupConfig applies the URL and SessionToken of the client to the config.
4545
func SetupConfig(t *testing.T, client *codersdk.Client, root config.Root) {
46-
err := root.Session().Write(client.SessionToken)
46+
err := root.Session().Write(client.SessionToken())
4747
require.NoError(t, err)
4848
err = root.URL().Write(client.URL.String())
4949
require.NoError(t, err)

cli/configssh_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func TestConfigSSH(t *testing.T) {
105105
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
106106
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
107107
agentClient := codersdk.New(client.URL)
108-
agentClient.SessionToken = authToken
108+
agentClient.SetSessionToken(authToken)
109109
agentCloser := agent.New(agent.Options{
110110
Client: agentClient,
111111
Logger: slogtest.Make(t, nil).Named("agent"),

cli/login.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func login() *cobra.Command {
214214
Text: "Paste your token here:",
215215
Secret: true,
216216
Validate: func(token string) error {
217-
client.SessionToken = token
217+
client.SetSessionToken(token)
218218
_, err := client.User(cmd.Context(), codersdk.Me)
219219
if err != nil {
220220
return xerrors.New("That's not a valid token!")
@@ -228,7 +228,7 @@ func login() *cobra.Command {
228228
}
229229

230230
// Login to get user data - verify it is OK before persisting
231-
client.SessionToken = sessionToken
231+
client.SetSessionToken(sessionToken)
232232
resp, err := client.User(cmd.Context(), codersdk.Me)
233233
if err != nil {
234234
return xerrors.Errorf("get user: %w", err)

cli/login_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func TestLogin(t *testing.T) {
148148
}()
149149

150150
pty.ExpectMatch("Paste your token here:")
151-
pty.WriteLine(client.SessionToken)
151+
pty.WriteLine(client.SessionToken())
152152
pty.ExpectMatch("Welcome to Coder")
153153
<-doneChan
154154
})
@@ -183,11 +183,11 @@ func TestLogin(t *testing.T) {
183183
t.Parallel()
184184
client := coderdtest.New(t, nil)
185185
coderdtest.CreateFirstUser(t, client)
186-
root, cfg := clitest.New(t, "login", client.URL.String(), "--token", client.SessionToken)
186+
root, cfg := clitest.New(t, "login", client.URL.String(), "--token", client.SessionToken())
187187
err := root.Execute()
188188
require.NoError(t, err)
189189
sessionFile, err := cfg.Session().Read()
190190
require.NoError(t, err)
191-
require.Equal(t, client.SessionToken, sessionFile)
191+
require.Equal(t, client.SessionToken(), sessionFile)
192192
})
193193
}

cli/logout_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func login(t *testing.T, pty *ptytest.PTY) config.Root {
209209
}()
210210

211211
pty.ExpectMatch("Paste your token here:")
212-
pty.WriteLine(client.SessionToken)
212+
pty.WriteLine(client.SessionToken())
213213
pty.ExpectMatch("Welcome to Coder")
214214
<-doneChan
215215

cli/root.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ func CreateClient(cmd *cobra.Command) (*codersdk.Client, error) {
306306
if err != nil {
307307
return nil, err
308308
}
309-
client.SessionToken = token
309+
client.SetSessionToken(token)
310310
return client, nil
311311
}
312312

@@ -347,7 +347,7 @@ func createAgentClient(cmd *cobra.Command) (*codersdk.Client, error) {
347347
return nil, err
348348
}
349349
client := codersdk.New(serverURL)
350-
client.SessionToken = token
350+
client.SetSessionToken(token)
351351
return client, nil
352352
}
353353

cli/speedtest_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestSpeedtest(t *testing.T) {
2222
}
2323
client, workspace, agentToken := setupWorkspaceForAgent(t)
2424
agentClient := codersdk.New(client.URL)
25-
agentClient.SessionToken = agentToken
25+
agentClient.SetSessionToken(agentToken)
2626
agentCloser := agent.New(agent.Options{
2727
Client: agentClient,
2828
Logger: slogtest.Make(t, nil).Named("agent"),

cli/ssh_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func TestSSH(t *testing.T) {
8787
pty.ExpectMatch("Waiting")
8888

8989
agentClient := codersdk.New(client.URL)
90-
agentClient.SessionToken = agentToken
90+
agentClient.SetSessionToken(agentToken)
9191
agentCloser := agent.New(agent.Options{
9292
Client: agentClient,
9393
Logger: slogtest.Make(t, nil).Named("agent"),
@@ -107,7 +107,7 @@ func TestSSH(t *testing.T) {
107107
// Run this async so the SSH command has to wait for
108108
// the build and agent to connect!
109109
agentClient := codersdk.New(client.URL)
110-
agentClient.SessionToken = agentToken
110+
agentClient.SetSessionToken(agentToken)
111111
agentCloser := agent.New(agent.Options{
112112
Client: agentClient,
113113
Logger: slogtest.Make(t, nil).Named("agent"),
@@ -174,7 +174,7 @@ func TestSSH(t *testing.T) {
174174
client, workspace, agentToken := setupWorkspaceForAgent(t)
175175

176176
agentClient := codersdk.New(client.URL)
177-
agentClient.SessionToken = agentToken
177+
agentClient.SetSessionToken(agentToken)
178178
agentCloser := agent.New(agent.Options{
179179
Client: agentClient,
180180
Logger: slogtest.Make(t, nil).Named("agent"),

coderd/coderdtest/coderdtest.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ func CreateFirstUser(t *testing.T, client *codersdk.Client) codersdk.CreateFirst
360360
Password: FirstUserParams.Password,
361361
})
362362
require.NoError(t, err)
363-
client.SessionToken = login.SessionToken
363+
client.SetSessionToken(login.SessionToken)
364364
return resp
365365
}
366366

@@ -400,7 +400,7 @@ func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationI
400400
require.NoError(t, err)
401401

402402
other := codersdk.New(client.URL)
403-
other.SessionToken = login.SessionToken
403+
other.SetSessionToken(login.SessionToken)
404404

405405
if len(roles) > 0 {
406406
// Find the roles for the org vs the site wide roles

coderd/gitsshkey_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func TestAgentGitSSHKey(t *testing.T) {
134134
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
135135

136136
agentClient := codersdk.New(client.URL)
137-
agentClient.SessionToken = authToken
137+
agentClient.SetSessionToken(authToken)
138138

139139
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
140140
defer cancel()

coderd/templates_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ func TestTemplateMetrics(t *testing.T) {
601601
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
602602

603603
agentClient := codersdk.New(client.URL)
604-
agentClient.SessionToken = authToken
604+
agentClient.SetSessionToken(authToken)
605605
agentCloser := agent.New(agent.Options{
606606
Logger: slogtest.Make(t, nil),
607607
Client: agentClient,

coderd/userauth_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ func TestUserOAuth2Github(t *testing.T) {
275275
resp := oauth2Callback(t, client)
276276
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
277277

278-
client.SessionToken = authCookieValue(resp.Cookies())
278+
client.SetSessionToken(authCookieValue(resp.Cookies()))
279279
user, err := client.User(context.Background(), "me")
280280
require.NoError(t, err)
281281
require.Equal(t, "kyle@coder.com", user.Email)
@@ -485,14 +485,14 @@ func TestUserOIDC(t *testing.T) {
485485
ctx, _ := testutil.Context(t)
486486

487487
if tc.Username != "" {
488-
client.SessionToken = authCookieValue(resp.Cookies())
488+
client.SetSessionToken(authCookieValue(resp.Cookies()))
489489
user, err := client.User(ctx, "me")
490490
require.NoError(t, err)
491491
require.Equal(t, tc.Username, user.Username)
492492
}
493493

494494
if tc.AvatarURL != "" {
495-
client.SessionToken = authCookieValue(resp.Cookies())
495+
client.SetSessionToken(authCookieValue(resp.Cookies()))
496496
user, err := client.User(ctx, "me")
497497
require.NoError(t, err)
498498
require.Equal(t, tc.AvatarURL, user.AvatarURL)
@@ -520,7 +520,7 @@ func TestUserOIDC(t *testing.T) {
520520

521521
ctx, _ := testutil.Context(t)
522522

523-
client.SessionToken = authCookieValue(resp.Cookies())
523+
client.SetSessionToken(authCookieValue(resp.Cookies()))
524524
user, err := client.User(ctx, "me")
525525
require.NoError(t, err)
526526
require.Equal(t, "jon", user.Username)
@@ -534,7 +534,7 @@ func TestUserOIDC(t *testing.T) {
534534
resp = oidcCallback(t, client, code)
535535
assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
536536

537-
client.SessionToken = authCookieValue(resp.Cookies())
537+
client.SetSessionToken(authCookieValue(resp.Cookies()))
538538
user, err = client.User(ctx, "me")
539539
require.NoError(t, err)
540540
require.True(t, strings.HasPrefix(user.Username, "jon-"), "username %q should have prefix %q", user.Username, "jon-")

coderd/users_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ func TestPostLogin(t *testing.T) {
280280
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
281281
defer cancel()
282282

283-
split := strings.Split(client.SessionToken, "-")
283+
split := strings.Split(client.SessionToken(), "-")
284284
key, err := client.GetAPIKey(ctx, admin.UserID.String(), split[0])
285285
require.NoError(t, err, "fetch login key")
286286
require.Equal(t, int64(86400), key.LifetimeSeconds, "default should be 86400")
@@ -356,7 +356,7 @@ func TestPostLogout(t *testing.T) {
356356
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
357357
defer cancel()
358358

359-
keyID := strings.Split(client.SessionToken, "-")[0]
359+
keyID := strings.Split(client.SessionToken(), "-")[0]
360360
apiKey, err := client.GetAPIKey(ctx, admin.UserID.String(), keyID)
361361
require.NoError(t, err)
362362
require.Equal(t, keyID, apiKey.ID, "API key should exist in the database")
@@ -676,7 +676,7 @@ func TestUpdateUserPassword(t *testing.T) {
676676
})
677677
require.NoError(t, err)
678678

679-
client.SessionToken = resp.SessionToken
679+
client.SetSessionToken(resp.SessionToken)
680680

681681
// Trying to get an API key should fail since all keys are deleted
682682
// on password change.
@@ -1359,7 +1359,7 @@ func TestWorkspacesByUser(t *testing.T) {
13591359
require.NoError(t, err)
13601360

13611361
newUserClient := codersdk.New(client.URL)
1362-
newUserClient.SessionToken = auth.SessionToken
1362+
newUserClient.SetSessionToken(auth.SessionToken)
13631363
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
13641364
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
13651365
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)

coderd/workspaceagents_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func TestWorkspaceAgentListen(t *testing.T) {
113113
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
114114

115115
agentClient := codersdk.New(client.URL)
116-
agentClient.SessionToken = authToken
116+
agentClient.SetSessionToken(authToken)
117117
agentCloser := agent.New(agent.Options{
118118
Client: agentClient,
119119
Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
@@ -205,7 +205,7 @@ func TestWorkspaceAgentListen(t *testing.T) {
205205
coderdtest.AwaitWorkspaceBuildJob(t, client, stopBuild.ID)
206206

207207
agentClient := codersdk.New(client.URL)
208-
agentClient.SessionToken = authToken
208+
agentClient.SetSessionToken(authToken)
209209

210210
_, err = agentClient.ListenWorkspaceAgent(ctx)
211211
require.Error(t, err)
@@ -245,7 +245,7 @@ func TestWorkspaceAgentTailnet(t *testing.T) {
245245
daemonCloser.Close()
246246

247247
agentClient := codersdk.New(client.URL)
248-
agentClient.SessionToken = authToken
248+
agentClient.SetSessionToken(authToken)
249249
agentCloser := agent.New(agent.Options{
250250
Client: agentClient,
251251
Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
@@ -311,7 +311,7 @@ func TestWorkspaceAgentPTY(t *testing.T) {
311311
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
312312

313313
agentClient := codersdk.New(client.URL)
314-
agentClient.SessionToken = authToken
314+
agentClient.SetSessionToken(authToken)
315315
agentCloser := agent.New(agent.Options{
316316
Client: agentClient,
317317
Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
@@ -408,7 +408,7 @@ func TestWorkspaceAgentListeningPorts(t *testing.T) {
408408
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
409409

410410
agentClient := codersdk.New(client.URL)
411-
agentClient.SessionToken = authToken
411+
agentClient.SetSessionToken(authToken)
412412
agentCloser := agent.New(agent.Options{
413413
Client: agentClient,
414414
Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
@@ -670,7 +670,7 @@ func TestWorkspaceAgentAppHealth(t *testing.T) {
670670
defer cancel()
671671

672672
agentClient := codersdk.New(client.URL)
673-
agentClient.SessionToken = authToken
673+
agentClient.SetSessionToken(authToken)
674674

675675
metadata, err := agentClient.WorkspaceAgentMetadata(ctx)
676676
require.NoError(t, err)
@@ -754,7 +754,7 @@ func TestWorkspaceAgentsGitAuth(t *testing.T) {
754754
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
755755

756756
agentClient := codersdk.New(client.URL)
757-
agentClient.SessionToken = authToken
757+
agentClient.SetSessionToken(authToken)
758758
_, err := agentClient.WorkspaceAgentGitAuth(context.Background(), "github.com", false)
759759
var apiError *codersdk.Error
760760
require.ErrorAs(t, err, &apiError)
@@ -799,7 +799,7 @@ func TestWorkspaceAgentsGitAuth(t *testing.T) {
799799
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
800800

801801
agentClient := codersdk.New(client.URL)
802-
agentClient.SessionToken = authToken
802+
agentClient.SetSessionToken(authToken)
803803
token, err := agentClient.WorkspaceAgentGitAuth(context.Background(), "github.com/asd/asd", false)
804804
require.NoError(t, err)
805805
require.True(t, strings.HasSuffix(token.URL, fmt.Sprintf("/gitauth/%s", "github")))
@@ -879,7 +879,7 @@ func TestWorkspaceAgentsGitAuth(t *testing.T) {
879879
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
880880

881881
agentClient := codersdk.New(client.URL)
882-
agentClient.SessionToken = authToken
882+
agentClient.SetSessionToken(authToken)
883883

884884
token, err := agentClient.WorkspaceAgentGitAuth(context.Background(), "github.com/asd/asd", false)
885885
require.NoError(t, err)
@@ -920,7 +920,7 @@ func gitAuthCallback(t *testing.T, id string, client *codersdk.Client) *http.Res
920920
})
921921
req.AddCookie(&http.Cookie{
922922
Name: codersdk.SessionTokenKey,
923-
Value: client.SessionToken,
923+
Value: client.SessionToken(),
924924
})
925925
res, err := client.HTTPClient.Do(req)
926926
require.NoError(t, err)

0 commit comments

Comments
 (0)