Skip to content

Commit

Permalink
fix: Protect codersdk.Client SessionToken so it can be updated (#4965)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mafredri committed Nov 9, 2022
1 parent 8cadb33 commit 26ab0d3
Show file tree
Hide file tree
Showing 25 changed files with 82 additions and 64 deletions.
6 changes: 3 additions & 3 deletions cli/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func workspaceAgent() *cobra.Command {
if err != nil {
return xerrors.Errorf("CODER_AGENT_TOKEN must be set for token auth: %w", err)
}
client.SessionToken = token
client.SetSessionToken(token)
case "google-instance-identity":
// This is *only* done for testing to mock client authentication.
// This will never be set in a production scenario.
Expand Down Expand Up @@ -153,13 +153,13 @@ func workspaceAgent() *cobra.Command {
Logger: logger,
ExchangeToken: func(ctx context.Context) (string, error) {
if exchangeToken == nil {
return client.SessionToken, nil
return client.SessionToken(), nil
}
resp, err := exchangeToken(ctx)
if err != nil {
return "", err
}
client.SessionToken = resp.SessionToken
client.SetSessionToken(resp.SessionToken)
return resp.SessionToken, nil
},
EnvironmentVariables: map[string]string{
Expand Down
2 changes: 1 addition & 1 deletion cli/clitest/clitest.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func NewWithSubcommands(

// SetupConfig applies the URL and SessionToken of the client to the config.
func SetupConfig(t *testing.T, client *codersdk.Client, root config.Root) {
err := root.Session().Write(client.SessionToken)
err := root.Session().Write(client.SessionToken())
require.NoError(t, err)
err = root.URL().Write(client.URL.String())
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion cli/configssh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestConfigSSH(t *testing.T) {
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
agentClient := codersdk.New(client.URL)
agentClient.SessionToken = authToken
agentClient.SetSessionToken(authToken)
agentCloser := agent.New(agent.Options{
Client: agentClient,
Logger: slogtest.Make(t, nil).Named("agent"),
Expand Down
4 changes: 2 additions & 2 deletions cli/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func login() *cobra.Command {
Text: "Paste your token here:",
Secret: true,
Validate: func(token string) error {
client.SessionToken = token
client.SetSessionToken(token)
_, err := client.User(cmd.Context(), codersdk.Me)
if err != nil {
return xerrors.New("That's not a valid token!")
Expand All @@ -228,7 +228,7 @@ func login() *cobra.Command {
}

// Login to get user data - verify it is OK before persisting
client.SessionToken = sessionToken
client.SetSessionToken(sessionToken)
resp, err := client.User(cmd.Context(), codersdk.Me)
if err != nil {
return xerrors.Errorf("get user: %w", err)
Expand Down
6 changes: 3 additions & 3 deletions cli/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestLogin(t *testing.T) {
}()

pty.ExpectMatch("Paste your token here:")
pty.WriteLine(client.SessionToken)
pty.WriteLine(client.SessionToken())
pty.ExpectMatch("Welcome to Coder")
<-doneChan
})
Expand Down Expand Up @@ -183,11 +183,11 @@ func TestLogin(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
coderdtest.CreateFirstUser(t, client)
root, cfg := clitest.New(t, "login", client.URL.String(), "--token", client.SessionToken)
root, cfg := clitest.New(t, "login", client.URL.String(), "--token", client.SessionToken())
err := root.Execute()
require.NoError(t, err)
sessionFile, err := cfg.Session().Read()
require.NoError(t, err)
require.Equal(t, client.SessionToken, sessionFile)
require.Equal(t, client.SessionToken(), sessionFile)
})
}
2 changes: 1 addition & 1 deletion cli/logout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func login(t *testing.T, pty *ptytest.PTY) config.Root {
}()

pty.ExpectMatch("Paste your token here:")
pty.WriteLine(client.SessionToken)
pty.WriteLine(client.SessionToken())
pty.ExpectMatch("Welcome to Coder")
<-doneChan

Expand Down
4 changes: 2 additions & 2 deletions cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ func CreateClient(cmd *cobra.Command) (*codersdk.Client, error) {
if err != nil {
return nil, err
}
client.SessionToken = token
client.SetSessionToken(token)
return client, nil
}

Expand Down Expand Up @@ -347,7 +347,7 @@ func createAgentClient(cmd *cobra.Command) (*codersdk.Client, error) {
return nil, err
}
client := codersdk.New(serverURL)
client.SessionToken = token
client.SetSessionToken(token)
return client, nil
}

Expand Down
2 changes: 1 addition & 1 deletion cli/speedtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestSpeedtest(t *testing.T) {
}
client, workspace, agentToken := setupWorkspaceForAgent(t)
agentClient := codersdk.New(client.URL)
agentClient.SessionToken = agentToken
agentClient.SetSessionToken(agentToken)
agentCloser := agent.New(agent.Options{
Client: agentClient,
Logger: slogtest.Make(t, nil).Named("agent"),
Expand Down
6 changes: 3 additions & 3 deletions cli/ssh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestSSH(t *testing.T) {
pty.ExpectMatch("Waiting")

agentClient := codersdk.New(client.URL)
agentClient.SessionToken = agentToken
agentClient.SetSessionToken(agentToken)
agentCloser := agent.New(agent.Options{
Client: agentClient,
Logger: slogtest.Make(t, nil).Named("agent"),
Expand All @@ -107,7 +107,7 @@ func TestSSH(t *testing.T) {
// Run this async so the SSH command has to wait for
// the build and agent to connect!
agentClient := codersdk.New(client.URL)
agentClient.SessionToken = agentToken
agentClient.SetSessionToken(agentToken)
agentCloser := agent.New(agent.Options{
Client: agentClient,
Logger: slogtest.Make(t, nil).Named("agent"),
Expand Down Expand Up @@ -174,7 +174,7 @@ func TestSSH(t *testing.T) {
client, workspace, agentToken := setupWorkspaceForAgent(t)

agentClient := codersdk.New(client.URL)
agentClient.SessionToken = agentToken
agentClient.SetSessionToken(agentToken)
agentCloser := agent.New(agent.Options{
Client: agentClient,
Logger: slogtest.Make(t, nil).Named("agent"),
Expand Down
4 changes: 2 additions & 2 deletions coderd/coderdtest/coderdtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ func CreateFirstUser(t *testing.T, client *codersdk.Client) codersdk.CreateFirst
Password: FirstUserParams.Password,
})
require.NoError(t, err)
client.SessionToken = login.SessionToken
client.SetSessionToken(login.SessionToken)
return resp
}

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

other := codersdk.New(client.URL)
other.SessionToken = login.SessionToken
other.SetSessionToken(login.SessionToken)

if len(roles) > 0 {
// Find the roles for the org vs the site wide roles
Expand Down
2 changes: 1 addition & 1 deletion coderd/gitsshkey_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func TestAgentGitSSHKey(t *testing.T) {
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)

agentClient := codersdk.New(client.URL)
agentClient.SessionToken = authToken
agentClient.SetSessionToken(authToken)

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
Expand Down
2 changes: 1 addition & 1 deletion coderd/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ func TestTemplateMetrics(t *testing.T) {
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)

agentClient := codersdk.New(client.URL)
agentClient.SessionToken = authToken
agentClient.SetSessionToken(authToken)
agentCloser := agent.New(agent.Options{
Logger: slogtest.Make(t, nil),
Client: agentClient,
Expand Down
10 changes: 5 additions & 5 deletions coderd/userauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func TestUserOAuth2Github(t *testing.T) {
resp := oauth2Callback(t, client)
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)

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

if tc.Username != "" {
client.SessionToken = authCookieValue(resp.Cookies())
client.SetSessionToken(authCookieValue(resp.Cookies()))
user, err := client.User(ctx, "me")
require.NoError(t, err)
require.Equal(t, tc.Username, user.Username)
}

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

ctx, _ := testutil.Context(t)

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

client.SessionToken = authCookieValue(resp.Cookies())
client.SetSessionToken(authCookieValue(resp.Cookies()))
user, err = client.User(ctx, "me")
require.NoError(t, err)
require.True(t, strings.HasPrefix(user.Username, "jon-"), "username %q should have prefix %q", user.Username, "jon-")
Expand Down
8 changes: 4 additions & 4 deletions coderd/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func TestPostLogin(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

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

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

client.SessionToken = resp.SessionToken
client.SetSessionToken(resp.SessionToken)

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

newUserClient := codersdk.New(client.URL)
newUserClient.SessionToken = auth.SessionToken
newUserClient.SetSessionToken(auth.SessionToken)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
Expand Down
20 changes: 10 additions & 10 deletions coderd/workspaceagents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestWorkspaceAgentListen(t *testing.T) {
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)

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

agentClient := codersdk.New(client.URL)
agentClient.SessionToken = authToken
agentClient.SetSessionToken(authToken)

_, err = agentClient.ListenWorkspaceAgent(ctx)
require.Error(t, err)
Expand Down Expand Up @@ -245,7 +245,7 @@ func TestWorkspaceAgentTailnet(t *testing.T) {
daemonCloser.Close()

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

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

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

agentClient := codersdk.New(client.URL)
agentClient.SessionToken = authToken
agentClient.SetSessionToken(authToken)

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

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

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

agentClient := codersdk.New(client.URL)
agentClient.SessionToken = authToken
agentClient.SetSessionToken(authToken)

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

0 comments on commit 26ab0d3

Please sign in to comment.