From 6a44fb9e2922c4287bd4f6c166c55ae8a0db8847 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 30 Jun 2023 09:30:22 -0400 Subject: [PATCH 1/5] feat: coder login --token generates a new session Makes sure /logout does not delete the inputted token --- cli/login.go | 27 ++++++++++++++++++++++----- cli/login_test.go | 4 +++- codersdk/apikey.go | 5 ++++- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/cli/login.go b/cli/login.go index d4238c4d3df11..6cbbfe457e21b 100644 --- a/cli/login.go +++ b/cli/login.go @@ -51,6 +51,7 @@ func (r *RootCmd) login() *clibase.Cmd { Short: "Authenticate with Coder deployment", Middleware: clibase.RequireRangeArgs(0, 1), Handler: func(inv *clibase.Invocation) error { + ctx := inv.Context() rawURL := "" if len(inv.Args) == 0 { rawURL = r.clientURL.String() @@ -89,7 +90,7 @@ func (r *RootCmd) login() *clibase.Cmd { _, _ = fmt.Fprintln(inv.Stderr, cliui.DefaultStyles.Warn.Render(err.Error())) } - hasInitialUser, err := client.HasFirstUser(inv.Context()) + hasInitialUser, err := client.HasFirstUser(ctx) if err != nil { return xerrors.Errorf("Failed to check server %q for first user, is the URL correct and is coder accessible from your browser? Error - has initial user: %w", serverURL.String(), err) } @@ -182,7 +183,7 @@ func (r *RootCmd) login() *clibase.Cmd { trial = v == "yes" || v == "y" } - _, err = client.CreateFirstUser(inv.Context(), codersdk.CreateFirstUserRequest{ + _, err = client.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{ Email: email, Username: username, Password: password, @@ -191,7 +192,7 @@ func (r *RootCmd) login() *clibase.Cmd { if err != nil { return xerrors.Errorf("create initial user: %w", err) } - resp, err := client.LoginWithPassword(inv.Context(), codersdk.LoginWithPasswordRequest{ + resp, err := client.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{ Email: email, Password: password, }) @@ -235,7 +236,7 @@ func (r *RootCmd) login() *clibase.Cmd { Secret: true, Validate: func(token string) error { client.SetSessionToken(token) - _, err := client.User(inv.Context(), codersdk.Me) + _, err := client.User(ctx, codersdk.Me) if err != nil { return xerrors.New("That's not a valid token!") } @@ -245,11 +246,27 @@ func (r *RootCmd) login() *clibase.Cmd { if err != nil { return xerrors.Errorf("paste token prompt: %w", err) } + } else { + // If a session token is provided on the cli, use it to generate + // a new one. This is because the cli `--token` flag provides + // a token for the command being invoked. We should not store + // this token, and `/logout` should not delete it. + // /login should generate a new token and store that. + client.SetSessionToken(sessionToken) + // Use CreateAPIKey over CreateToken because this is a session + // key that should not show on the `tokens` page. This should + // match the same behavior of the `/cli-auth` page for generating + // a session token. + key, err := client.CreateAPIKey(ctx, "me") + if err != nil { + return xerrors.Errorf("create api key: %w", err) + } + sessionToken = key.Key } // Login to get user data - verify it is OK before persisting client.SetSessionToken(sessionToken) - resp, err := client.User(inv.Context(), codersdk.Me) + resp, err := client.User(ctx, codersdk.Me) if err != nil { return xerrors.Errorf("get user: %w", err) } diff --git a/cli/login_test.go b/cli/login_test.go index f8e24170c641e..1bab4721ea181 100644 --- a/cli/login_test.go +++ b/cli/login_test.go @@ -197,6 +197,7 @@ func TestLogin(t *testing.T) { <-doneChan }) + // TokenFlag should generate a new session token and store it in the session file. t.Run("TokenFlag", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) @@ -206,6 +207,7 @@ func TestLogin(t *testing.T) { require.NoError(t, err) sessionFile, err := cfg.Session().Read() require.NoError(t, err) - require.Equal(t, client.SessionToken(), sessionFile) + // This **should not be equal** to the token we passed in. + require.NotEqual(t, client.SessionToken(), sessionFile) }) } diff --git a/codersdk/apikey.go b/codersdk/apikey.go index 95f9458043e87..a040f9bef0751 100644 --- a/codersdk/apikey.go +++ b/codersdk/apikey.go @@ -78,7 +78,10 @@ func (c *Client) CreateToken(ctx context.Context, userID string, req CreateToken } // CreateAPIKey generates an API key for the user ID provided. -// DEPRECATED: use CreateToken instead. +// CreateToken should be used over CreateAPIKey. CreateToken allows better +// tracking of the token's usage and allows for custom expiration. +// Only use CreateAPIKey if you want to emulate the session created for +// a brower like login. func (c *Client) CreateAPIKey(ctx context.Context, user string) (GenerateAPIKeyResponse, error) { res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/users/%s/keys", user), nil) if err != nil { From cff7859039f1b6694093cd78df1a8ad2e9245b88 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 30 Jun 2023 09:34:14 -0400 Subject: [PATCH 2/5] flag to enable previous behavior if needed --- cli/login.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/cli/login.go b/cli/login.go index 6cbbfe457e21b..cfda22db9822c 100644 --- a/cli/login.go +++ b/cli/login.go @@ -41,10 +41,11 @@ func (r *RootCmd) login() *clibase.Cmd { const firstUserTrialEnv = "CODER_FIRST_USER_TRIAL" var ( - email string - username string - password string - trial bool + email string + username string + password string + trial bool + useTokenForSession bool ) cmd := &clibase.Cmd{ Use: "login ", @@ -246,7 +247,7 @@ func (r *RootCmd) login() *clibase.Cmd { if err != nil { return xerrors.Errorf("paste token prompt: %w", err) } - } else { + } else if !useTokenForSession { // If a session token is provided on the cli, use it to generate // a new one. This is because the cli `--token` flag provides // a token for the command being invoked. We should not store @@ -310,6 +311,11 @@ func (r *RootCmd) login() *clibase.Cmd { Description: "Specifies whether a trial license should be provisioned for the Coder deployment or not.", Value: clibase.BoolOf(&trial), }, + { + Flag: "use-token-for-session", + Description: "By default, the CLI will generate a new session token when logging in. This flag will instead use the provided token as the session token.", + Value: clibase.BoolOf(&useTokenForSession), + }, } return cmd } From 11c4a11db6b48f954c454a00b61752d50af4acc1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 30 Jun 2023 09:34:50 -0400 Subject: [PATCH 3/5] fixup! flag to enable previous behavior if needed --- cli/login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/login.go b/cli/login.go index cfda22db9822c..e16118dfec0d6 100644 --- a/cli/login.go +++ b/cli/login.go @@ -312,7 +312,7 @@ func (r *RootCmd) login() *clibase.Cmd { Value: clibase.BoolOf(&trial), }, { - Flag: "use-token-for-session", + Flag: "use-token-as-session", Description: "By default, the CLI will generate a new session token when logging in. This flag will instead use the provided token as the session token.", Value: clibase.BoolOf(&useTokenForSession), }, From 2f85bd3ce56a6148c258e3198789c1256cdc6ed3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 30 Jun 2023 10:20:00 -0400 Subject: [PATCH 4/5] Typo --- codersdk/apikey.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codersdk/apikey.go b/codersdk/apikey.go index a040f9bef0751..514b519f5ffda 100644 --- a/codersdk/apikey.go +++ b/codersdk/apikey.go @@ -81,7 +81,7 @@ func (c *Client) CreateToken(ctx context.Context, userID string, req CreateToken // CreateToken should be used over CreateAPIKey. CreateToken allows better // tracking of the token's usage and allows for custom expiration. // Only use CreateAPIKey if you want to emulate the session created for -// a brower like login. +// a browser like login. func (c *Client) CreateAPIKey(ctx context.Context, user string) (GenerateAPIKeyResponse, error) { res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/users/%s/keys", user), nil) if err != nil { From de2d90df5b020ca9129ef605720ccf54a88a8169 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 30 Jun 2023 10:32:43 -0400 Subject: [PATCH 5/5] Make gen --- cli/testdata/coder_login_--help.golden | 4 ++++ docs/cli/login.md | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/cli/testdata/coder_login_--help.golden b/cli/testdata/coder_login_--help.golden index 7169b369fce1d..bb458ca999854 100644 --- a/cli/testdata/coder_login_--help.golden +++ b/cli/testdata/coder_login_--help.golden @@ -19,5 +19,9 @@ Authenticate with Coder deployment Specifies a username to use if creating the first user for the deployment. + --use-token-as-session bool + By default, the CLI will generate a new session token when logging in. + This flag will instead use the provided token as the session token. + --- Run `coder --help` for a list of global options. diff --git a/docs/cli/login.md b/docs/cli/login.md index 7af68c30e8de2..f7604d42db7b0 100644 --- a/docs/cli/login.md +++ b/docs/cli/login.md @@ -47,3 +47,11 @@ Specifies whether a trial license should be provisioned for the Coder deployment | Environment | $CODER_FIRST_USER_USERNAME | Specifies a username to use if creating the first user for the deployment. + +### --use-token-as-session + +| | | +| ---- | ----------------- | +| Type | bool | + +By default, the CLI will generate a new session token when logging in. This flag will instead use the provided token as the session token.