Skip to content

fix: Protect codersdk.Client SessionToken so it can be updated #4965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading