From 058b0885ed34ea72ea0c8f0eaca822a3a75ea9b4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 13:58:19 +0000 Subject: [PATCH 1/9] feat: Generate random admin user password in dev mode --- cli/server.go | 53 +++++++++++++++++++++++++++++----------------- cli/server_test.go | 23 ++++++++++++++------ 2 files changed, 50 insertions(+), 26 deletions(-) diff --git a/cli/server.go b/cli/server.go index 95deb2dcad413..8becc46153ffe 100644 --- a/cli/server.go +++ b/cli/server.go @@ -41,27 +41,23 @@ import ( "github.com/coder/coder/coderd/gitsshkey" "github.com/coder/coder/coderd/turnconn" "github.com/coder/coder/codersdk" + "github.com/coder/coder/cryptorand" "github.com/coder/coder/provisioner/terraform" "github.com/coder/coder/provisionerd" "github.com/coder/coder/provisionersdk" "github.com/coder/coder/provisionersdk/proto" ) -var defaultDevUser = codersdk.CreateFirstUserRequest{ - Email: "admin@coder.com", - Username: "developer", - Password: "password", - OrganizationName: "acme-corp", -} - // nolint:gocyclo func server() *cobra.Command { var ( - accessURL string - address string - cacheDir string - dev bool - postgresURL string + accessURL string + address string + cacheDir string + dev bool + devUserEmail string + devUserPassword string + postgresURL string // provisionerDaemonCount is a uint8 to ensure a number > 0. provisionerDaemonCount uint8 oauth2GithubClientID string @@ -278,12 +274,18 @@ func server() *cobra.Command { config := createConfig(cmd) if dev { - err = createFirstUser(cmd, client, config) + if devUserPassword == "" { + devUserPassword, err = cryptorand.String(10) + if err != nil { + return xerrors.Errorf("generate random admin password for dev: %w", err) + } + } + err = createFirstUser(cmd, client, config, devUserEmail, devUserPassword) if err != nil { return xerrors.Errorf("create first user: %w", err) } - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "email: %s\n", defaultDevUser.Email) - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "password: %s\n", defaultDevUser.Password) + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "email: %s\n", devUserEmail) + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "password: %s\n", devUserPassword) _, _ = fmt.Fprintln(cmd.ErrOrStderr()) _, _ = fmt.Fprintf(cmd.ErrOrStderr(), cliui.Styles.Wrap.Render(`Started in dev mode. All data is in-memory! `+cliui.Styles.Bold.Render("Do not use in production")+`. Press `+ @@ -409,6 +411,8 @@ func server() *cobra.Command { // systemd uses the CACHE_DIRECTORY environment variable! cliflag.StringVarP(root.Flags(), &cacheDir, "cache-dir", "", "CACHE_DIRECTORY", filepath.Join(os.TempDir(), "coder-cache"), "Specifies a directory to cache binaries for provision operations.") cliflag.BoolVarP(root.Flags(), &dev, "dev", "", "CODER_DEV_MODE", false, "Serve Coder in dev mode for tinkering") + cliflag.StringVarP(root.Flags(), &devUserEmail, "dev-admin-email", "", "CODER_DEV_ADMIN_EMAIL", "admin@coder.com", "Specifies the admin email to be used in dev mode (--dev)") + cliflag.StringVarP(root.Flags(), &devUserPassword, "dev-admin-password", "", "CODER_DEV_ADMIN_PASSWORD", "", "Specifies the admin password to be used in dev mode (--dev) instead of a randomly generated one") cliflag.StringVarP(root.Flags(), &postgresURL, "postgres-url", "", "CODER_PG_CONNECTION_URL", "", "URL of a PostgreSQL database to connect to") cliflag.Uint8VarP(root.Flags(), &provisionerDaemonCount, "provisioner-daemons", "", "CODER_PROVISIONER_DAEMONS", 3, "The amount of provisioner daemons to create on start.") cliflag.StringVarP(root.Flags(), &oauth2GithubClientID, "oauth2-github-client-id", "", "CODER_OAUTH2_GITHUB_CLIENT_ID", "", @@ -450,14 +454,25 @@ func server() *cobra.Command { return root } -func createFirstUser(cmd *cobra.Command, client *codersdk.Client, cfg config.Root) error { - _, err := client.CreateFirstUser(cmd.Context(), defaultDevUser) +func createFirstUser(cmd *cobra.Command, client *codersdk.Client, cfg config.Root, email, password string) error { + if email == "" { + return xerrors.New("email is empty") + } + if password == "" { + return xerrors.New("password is empty") + } + _, err := client.CreateFirstUser(cmd.Context(), codersdk.CreateFirstUserRequest{ + Email: email, + Username: "developer", + Password: password, + OrganizationName: "acme-corp", + }) if err != nil { return xerrors.Errorf("create first user: %w", err) } token, err := client.LoginWithPassword(cmd.Context(), codersdk.LoginWithPasswordRequest{ - Email: defaultDevUser.Email, - Password: defaultDevUser.Password, + Email: email, + Password: password, }) if err != nil { return xerrors.Errorf("login with first user: %w", err) diff --git a/cli/server_test.go b/cli/server_test.go index 100f80f75febf..4feb16d765849 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -1,7 +1,6 @@ package cli_test import ( - "bytes" "context" "crypto/ecdsa" "crypto/elliptic" @@ -10,12 +9,14 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "fmt" "math/big" "net" "net/http" "net/url" "os" "runtime" + "strings" "testing" "time" @@ -74,18 +75,26 @@ func TestServer(t *testing.T) { t.Parallel() ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() + + wantEmail := "admin@coder.com" + root, cfg := clitest.New(t, "server", "--dev", "--skip-tunnel", "--address", ":0") - var stdoutBuf bytes.Buffer - root.SetOutput(&stdoutBuf) + var buf strings.Builder + root.SetOutput(&buf) go func() { err := root.ExecuteContext(ctx) require.ErrorIs(t, err, context.Canceled) // Verify that credentials were output to the terminal. - wantEmail := "email: admin@coder.com" - wantPassword := "password: password" - assert.Contains(t, stdoutBuf.String(), wantEmail, "expected output %q; got no match", wantEmail) - assert.Contains(t, stdoutBuf.String(), wantPassword, "expected output %q; got no match", wantPassword) + assert.Contains(t, buf.String(), fmt.Sprintf("email: %s", wantEmail), "expected output %q; got no match", wantEmail) + // Check that the password line is output and that it's non-empty. + if _, after, found := strings.Cut(buf.String(), "password: "); found { + before, _, _ := strings.Cut(after, "\n") + before = strings.Trim(before, "\r") // Ensure no control character is left. + assert.NotEmpty(t, before, "expected non-empty password; got empty") + } else { + t.Error("expected password line output; got no match") + } }() var token string require.Eventually(t, func() bool { From e00c48c65be623153111532dc0c345c8858f6dcf Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 13:59:37 +0000 Subject: [PATCH 2/9] Fix flaky test with done channel --- cli/server_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cli/server_test.go b/cli/server_test.go index 4feb16d765849..a9704d0bec3e6 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -81,7 +81,10 @@ func TestServer(t *testing.T) { root, cfg := clitest.New(t, "server", "--dev", "--skip-tunnel", "--address", ":0") var buf strings.Builder root.SetOutput(&buf) + done := make(chan struct{}) go func() { + defer close(done) + err := root.ExecuteContext(ctx) require.ErrorIs(t, err, context.Canceled) @@ -111,6 +114,9 @@ func TestServer(t *testing.T) { client.SessionToken = token _, err = client.User(ctx, codersdk.Me) require.NoError(t, err) + + cancelFunc() + <-done }) t.Run("TLSBadVersion", func(t *testing.T) { t.Parallel() From 045260dd8b4d48402590116821f55ae4186c3e6d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 14:09:45 +0000 Subject: [PATCH 3/9] Add dev mode test with email/pass from env --- cli/server_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/cli/server_test.go b/cli/server_test.go index a9704d0bec3e6..6f6b0d1538026 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -118,6 +118,52 @@ func TestServer(t *testing.T) { cancelFunc() <-done }) + // Duplicated test from "Development" above to test setting email/password via env. + t.Run("Development with email and password from env", func(t *testing.T) { + // Cannot run parallell due to os.Setenv. + ctx, cancelFunc := context.WithCancel(context.Background()) + defer cancelFunc() + + wantEmail := "myadmin@coder.com" + wantPassword := "testpass42" + os.Setenv("CODER_DEV_ADMIN_EMAIL", wantEmail) + defer os.Unsetenv("CODER_DEV_ADMIN_EMAIL") + os.Setenv("CODER_DEV_ADMIN_PASSWORD", wantPassword) + defer os.Unsetenv("CODER_DEV_ADMIN_PASSWORD") + + root, cfg := clitest.New(t, "server", "--dev", "--skip-tunnel", "--address", ":0") + var buf strings.Builder + root.SetOutput(&buf) + done := make(chan struct{}) + go func() { + defer close(done) + + err := root.ExecuteContext(ctx) + require.ErrorIs(t, err, context.Canceled) + + // Verify that credentials were output to the terminal. + assert.Contains(t, buf.String(), fmt.Sprintf("email: %s", wantEmail), "expected output %q; got no match", wantEmail) + assert.Contains(t, buf.String(), fmt.Sprintf("password: %s", wantPassword), "expected output %q; got no match", wantPassword) + }() + var token string + require.Eventually(t, func() bool { + var err error + token, err = cfg.Session().Read() + return err == nil + }, 15*time.Second, 25*time.Millisecond) + // Verify that authentication was properly set in dev-mode. + accessURL, err := cfg.URL().Read() + require.NoError(t, err) + parsed, err := url.Parse(accessURL) + require.NoError(t, err) + client := codersdk.New(parsed) + client.SessionToken = token + _, err = client.User(ctx, codersdk.Me) + require.NoError(t, err) + + cancelFunc() + <-done + }) t.Run("TLSBadVersion", func(t *testing.T) { t.Parallel() ctx, cancelFunc := context.WithCancel(context.Background()) From 43f45fffe74f743695152e644734ff4b0d0648cd Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 14:19:31 +0000 Subject: [PATCH 4/9] Fix typo --- cli/server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/server_test.go b/cli/server_test.go index 6f6b0d1538026..255fdf61d9014 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -120,7 +120,7 @@ func TestServer(t *testing.T) { }) // Duplicated test from "Development" above to test setting email/password via env. t.Run("Development with email and password from env", func(t *testing.T) { - // Cannot run parallell due to os.Setenv. + // Cannot run parallel due to os.Setenv. ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() From 72908d47d3f42dde3fbb0f182f526980203f1702 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 14:31:34 +0000 Subject: [PATCH 5/9] Set nolint for non-parallel test --- cli/server_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/server_test.go b/cli/server_test.go index 255fdf61d9014..7f018831fd7ef 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -119,8 +119,9 @@ func TestServer(t *testing.T) { <-done }) // Duplicated test from "Development" above to test setting email/password via env. + // Cannot run parallel due to os.Setenv. + //nolint:paralleltest t.Run("Development with email and password from env", func(t *testing.T) { - // Cannot run parallel due to os.Setenv. ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() From ffcd5d9c5f192947835b3795c543c454f183340d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 14:33:01 +0000 Subject: [PATCH 6/9] Use t.Setenv for cleaner code --- cli/server_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cli/server_test.go b/cli/server_test.go index 7f018831fd7ef..8854afcdce904 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -127,10 +127,8 @@ func TestServer(t *testing.T) { wantEmail := "myadmin@coder.com" wantPassword := "testpass42" - os.Setenv("CODER_DEV_ADMIN_EMAIL", wantEmail) - defer os.Unsetenv("CODER_DEV_ADMIN_EMAIL") - os.Setenv("CODER_DEV_ADMIN_PASSWORD", wantPassword) - defer os.Unsetenv("CODER_DEV_ADMIN_PASSWORD") + t.Setenv("CODER_DEV_ADMIN_EMAIL", wantEmail) + t.Setenv("CODER_DEV_ADMIN_PASSWORD", wantPassword) root, cfg := clitest.New(t, "server", "--dev", "--skip-tunnel", "--address", ":0") var buf strings.Builder From a277e8cf8abcc1e2aadbf90393d262654c5aa6c2 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 14:53:31 +0000 Subject: [PATCH 7/9] Set email/pass for playwright e2e test via cli flags --- site/e2e/playwright.config.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index b1d36ee5c60ef..9f2cbf0ceb792 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -1,5 +1,6 @@ import { PlaywrightTestConfig } from "@playwright/test" import * as path from "path" +import * as constants from './constants'; const config: PlaywrightTestConfig = { testDir: "tests", @@ -17,7 +18,7 @@ const config: PlaywrightTestConfig = { // https://playwright.dev/docs/test-advanced#launching-a-development-web-server-during-the-tests webServer: { // Run the coder daemon directly. - command: `go run -tags embed ${path.join(__dirname, "../../cmd/coder/main.go")} server --dev --skip-tunnel`, + command: `go run -tags embed ${path.join(__dirname, "../../cmd/coder/main.go")} server --dev --skip-tunnel --dev-admin-email ${constants.email} --dev-admin-password ${constants.password}`, port: 3000, timeout: 120 * 10000, reuseExistingServer: false, From 80dddd2712f3b4bd0d6d188eb8d25b38aff3953a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 15:01:03 +0000 Subject: [PATCH 8/9] Run prettier --write --- site/e2e/playwright.config.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index 9f2cbf0ceb792..365e023d95f3e 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -1,6 +1,6 @@ import { PlaywrightTestConfig } from "@playwright/test" import * as path from "path" -import * as constants from './constants'; +import * as constants from "./constants" const config: PlaywrightTestConfig = { testDir: "tests", @@ -18,7 +18,10 @@ const config: PlaywrightTestConfig = { // https://playwright.dev/docs/test-advanced#launching-a-development-web-server-during-the-tests webServer: { // Run the coder daemon directly. - command: `go run -tags embed ${path.join(__dirname, "../../cmd/coder/main.go")} server --dev --skip-tunnel --dev-admin-email ${constants.email} --dev-admin-password ${constants.password}`, + command: `go run -tags embed ${path.join( + __dirname, + "../../cmd/coder/main.go", + )} server --dev --skip-tunnel --dev-admin-email ${constants.email} --dev-admin-password ${constants.password}`, port: 3000, timeout: 120 * 10000, reuseExistingServer: false, From 0026a1243c34db90ce834656ebe6ec70d0054ec9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 28 Apr 2022 15:39:46 +0000 Subject: [PATCH 9/9] Use sync.WaitGroup instead of channel --- cli/server_test.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/cli/server_test.go b/cli/server_test.go index 8854afcdce904..369334c48ec14 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -17,6 +17,7 @@ import ( "os" "runtime" "strings" + "sync" "testing" "time" @@ -81,9 +82,10 @@ func TestServer(t *testing.T) { root, cfg := clitest.New(t, "server", "--dev", "--skip-tunnel", "--address", ":0") var buf strings.Builder root.SetOutput(&buf) - done := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) go func() { - defer close(done) + defer wg.Done() err := root.ExecuteContext(ctx) require.ErrorIs(t, err, context.Canceled) @@ -116,7 +118,7 @@ func TestServer(t *testing.T) { require.NoError(t, err) cancelFunc() - <-done + wg.Wait() }) // Duplicated test from "Development" above to test setting email/password via env. // Cannot run parallel due to os.Setenv. @@ -133,9 +135,10 @@ func TestServer(t *testing.T) { root, cfg := clitest.New(t, "server", "--dev", "--skip-tunnel", "--address", ":0") var buf strings.Builder root.SetOutput(&buf) - done := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) go func() { - defer close(done) + defer wg.Done() err := root.ExecuteContext(ctx) require.ErrorIs(t, err, context.Canceled) @@ -161,7 +164,7 @@ func TestServer(t *testing.T) { require.NoError(t, err) cancelFunc() - <-done + wg.Wait() }) t.Run("TLSBadVersion", func(t *testing.T) { t.Parallel()