From 8a2b6c2d4632302db83c70997b61e817c72f3720 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 10:57:25 -0500 Subject: [PATCH 01/15] fix: Suspended users cannot authenticate - Fix httpmw tests - Merge roles and apikey extract httpmw - Add member account to make dev --- cli/server.go | 59 ++++++++++++++++---- coderd/coderd.go | 13 +---- coderd/database/databasefake/databasefake.go | 1 + coderd/database/queries.sql.go | 18 ++++-- coderd/database/queries/users.sql | 4 +- coderd/httpmw/apikey.go | 22 +++++++- coderd/httpmw/apikey_test.go | 32 +++++++++++ coderd/httpmw/authorize_test.go | 1 - scripts/develop.sh | 2 + 9 files changed, 120 insertions(+), 32 deletions(-) diff --git a/cli/server.go b/cli/server.go index ac6f49d5079a7..8c0fa66923ae9 100644 --- a/cli/server.go +++ b/cli/server.go @@ -68,9 +68,12 @@ func server() *cobra.Command { pprofAddress string cacheDir string dev bool - devUserEmail string - devUserPassword string - postgresURL string + devFirstEmail string + devFirstPassword string + devMemberEmail string + devMemberPassword string + + postgresURL string // provisionerDaemonCount is a uint8 to ensure a number > 0. provisionerDaemonCount uint8 oauth2GithubClientID string @@ -330,18 +333,30 @@ func server() *cobra.Command { config := createConfig(cmd) if dev { - if devUserPassword == "" { - devUserPassword, err = cryptorand.String(10) + if devFirstPassword == "" { + devFirstPassword, 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 devMemberPassword == "" { + devMemberPassword, err = cryptorand.String(10) + if err != nil { + return xerrors.Errorf("generate random member password for dev: %w", err) + } + } + + // Create first user with 1 additional user as a member of the same org. + err = createFirstUser(cmd, client, config, devFirstEmail, devFirstPassword, extraUsers{ + Username: "member", + Email: devMemberEmail, + Password: devMemberPassword, + }) if err != nil { return xerrors.Errorf("create first user: %w", err) } - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "email: %s\n", devUserEmail) - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "password: %s\n", devUserPassword) + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "email: %s\n", devFirstEmail) + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "password: %s\n", devFirstPassword) _, _ = 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 `+ @@ -474,8 +489,10 @@ 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(), &devFirstEmail, "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(), &devFirstPassword, "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(), &devMemberEmail, "dev-member-email", "", "CODER_DEV_MEMBER_EMAIL", "member@coder.com", "Specifies the member email to be used in dev mode (--dev)") + cliflag.StringVarP(root.Flags(), &devMemberPassword, "dev-member-password", "", "CODER_DEV_MEMBER_PASSWORD", "", "Specifies the member 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", "", @@ -518,14 +535,20 @@ func server() *cobra.Command { return root } -func createFirstUser(cmd *cobra.Command, client *codersdk.Client, cfg config.Root, email, password string) error { +type extraUsers struct { + Username string + Email string + Password string +} + +func createFirstUser(cmd *cobra.Command, client *codersdk.Client, cfg config.Root, email, password string, users ...extraUsers) error { if email == "" { return xerrors.New("email is empty") } if password == "" { return xerrors.New("password is empty") } - _, err := client.CreateFirstUser(cmd.Context(), codersdk.CreateFirstUserRequest{ + first, err := client.CreateFirstUser(cmd.Context(), codersdk.CreateFirstUserRequest{ Email: email, Username: "developer", Password: password, @@ -551,6 +574,18 @@ func createFirstUser(cmd *cobra.Command, client *codersdk.Client, cfg config.Roo if err != nil { return xerrors.Errorf("write session token: %w", err) } + + for _, user := range users { + _, err := client.CreateUser(cmd.Context(), codersdk.CreateUserRequest{ + Email: user.Email, + Username: user.Username, + Password: user.Password, + OrganizationID: first.OrganizationID, + }) + if err != nil { + return xerrors.Errorf("create extra user %q: %w", user.Email, err) + } + } return nil } diff --git a/coderd/coderd.go b/coderd/coderd.go index 82797d452d205..a60312c2da873 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -82,8 +82,6 @@ func New(options *Options) *API { apiKeyMiddleware := httpmw.ExtractAPIKey(options.Database, &httpmw.OAuth2Configs{ Github: options.GithubOAuth2Config, }) - // TODO: @emyrk we should just move this into 'ExtractAPIKey'. - authRolesMiddleware := httpmw.ExtractUserRoles(options.Database) r.Use( func(next http.Handler) http.Handler { @@ -125,7 +123,6 @@ func New(options *Options) *API { r.Route("/files", func(r chi.Router) { r.Use( apiKeyMiddleware, - authRolesMiddleware, // This number is arbitrary, but reading/writing // file content is expensive so it should be small. httpmw.RateLimitPerMinute(12), @@ -136,7 +133,6 @@ func New(options *Options) *API { r.Route("/provisionerdaemons", func(r chi.Router) { r.Use( apiKeyMiddleware, - authRolesMiddleware, ) r.Get("/", api.provisionerDaemons) }) @@ -144,7 +140,6 @@ func New(options *Options) *API { r.Use( apiKeyMiddleware, httpmw.ExtractOrganizationParam(options.Database), - authRolesMiddleware, ) r.Get("/", api.organization) r.Post("/templateversions", api.postTemplateVersionsByOrganization) @@ -174,7 +169,7 @@ func New(options *Options) *API { }) }) r.Route("/parameters/{scope}/{id}", func(r chi.Router) { - r.Use(apiKeyMiddleware, authRolesMiddleware) + r.Use(apiKeyMiddleware) r.Post("/", api.postParameter) r.Get("/", api.parameters) r.Route("/{name}", func(r chi.Router) { @@ -184,7 +179,6 @@ func New(options *Options) *API { r.Route("/templates/{template}", func(r chi.Router) { r.Use( apiKeyMiddleware, - authRolesMiddleware, httpmw.ExtractTemplateParam(options.Database), ) @@ -199,7 +193,6 @@ func New(options *Options) *API { r.Route("/templateversions/{templateversion}", func(r chi.Router) { r.Use( apiKeyMiddleware, - authRolesMiddleware, httpmw.ExtractTemplateVersionParam(options.Database), ) @@ -225,7 +218,6 @@ func New(options *Options) *API { r.Group(func(r chi.Router) { r.Use( apiKeyMiddleware, - authRolesMiddleware, ) r.Post("/", api.postUser) r.Get("/", api.users) @@ -288,7 +280,6 @@ func New(options *Options) *API { r.Route("/workspaceresources/{workspaceresource}", func(r chi.Router) { r.Use( apiKeyMiddleware, - authRolesMiddleware, httpmw.ExtractWorkspaceResourceParam(options.Database), httpmw.ExtractWorkspaceParam(options.Database), ) @@ -297,7 +288,6 @@ func New(options *Options) *API { r.Route("/workspaces", func(r chi.Router) { r.Use( apiKeyMiddleware, - authRolesMiddleware, ) r.Get("/", api.workspaces) r.Route("/{workspace}", func(r chi.Router) { @@ -323,7 +313,6 @@ func New(options *Options) *API { r.Route("/workspacebuilds/{workspacebuild}", func(r chi.Router) { r.Use( apiKeyMiddleware, - authRolesMiddleware, httpmw.ExtractWorkspaceBuildParam(options.Database), httpmw.ExtractWorkspaceParam(options.Database), ) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 6acc5d156bdac..f2fa2fa99d280 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -287,6 +287,7 @@ func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (data return database.GetAllUserRolesRow{ ID: userID, Username: user.Username, + Status: user.Status, Roles: roles, }, nil } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 137fe36b4456b..c693a58b4318c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2078,7 +2078,9 @@ func (q *sqlQuerier) UpdateTemplateVersionDescriptionByJobID(ctx context.Context const getAllUserRoles = `-- name: GetAllUserRoles :one SELECT -- username is returned just to help for logging purposes - id, username, array_cat(users.rbac_roles, organization_members.roles) :: text[] AS roles + -- status is used to enforce 'suspended' users, as all roles are ignored + -- when suspended. + id, username, status, array_cat(users.rbac_roles, organization_members.roles) :: text[] AS roles FROM users LEFT JOIN organization_members @@ -2088,15 +2090,21 @@ WHERE ` type GetAllUserRolesRow struct { - ID uuid.UUID `db:"id" json:"id"` - Username string `db:"username" json:"username"` - Roles []string `db:"roles" json:"roles"` + ID uuid.UUID `db:"id" json:"id"` + Username string `db:"username" json:"username"` + Status UserStatus `db:"status" json:"status"` + Roles []string `db:"roles" json:"roles"` } func (q *sqlQuerier) GetAllUserRoles(ctx context.Context, userID uuid.UUID) (GetAllUserRolesRow, error) { row := q.db.QueryRowContext(ctx, getAllUserRoles, userID) var i GetAllUserRolesRow - err := row.Scan(&i.ID, &i.Username, pq.Array(&i.Roles)) + err := row.Scan( + &i.ID, + &i.Username, + &i.Status, + pq.Array(&i.Roles), + ) return i, err } diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index d9f68ec440bc3..6d650366b217f 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -135,7 +135,9 @@ WHERE -- name: GetAllUserRoles :one SELECT -- username is returned just to help for logging purposes - id, username, array_cat(users.rbac_roles, organization_members.roles) :: text[] AS roles + -- status is used to enforce 'suspended' users, as all roles are ignored + -- when suspended. + id, username, status, array_cat(users.rbac_roles, organization_members.roles) :: text[] AS roles FROM users LEFT JOIN organization_members diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index abf55128d55d7..1e4098c0be431 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -175,7 +175,27 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h } } - ctx := context.WithValue(r.Context(), apiKeyContextKey{}, key) + // If the key is valid, we also fetch the user roles and status. + // The roles are used for RBAC authorize checks, and the status + // is to block 'suspended' users from accessing the platform. + roles, err := db.GetAllUserRoles(r.Context(), key.UserID) + if err != nil { + httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ + Message: "roles not found", + }) + return + } + + if roles.Status != database.UserStatusActive { + httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ + Message: fmt.Sprintf("user is not active (status = %q), contact an admin to reactivate your account", roles.Status), + }) + return + } + + ctx := r.Context() + ctx = context.WithValue(ctx, apiKeyContextKey{}, key) + ctx = context.WithValue(ctx, userRolesKey{}, roles) next.ServeHTTP(rw, r.WithContext(ctx)) }) } diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index 3be2877426134..d55924dc1532d 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -9,6 +9,8 @@ import ( "testing" "time" + "github.com/google/uuid" + "github.com/stretchr/testify/require" "golang.org/x/oauth2" @@ -128,6 +130,7 @@ func TestAPIKey(t *testing.T) { id, secret = randomAPIKeyParts() r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) ) r.AddCookie(&http.Cookie{ Name: httpmw.SessionTokenKey, @@ -139,6 +142,7 @@ func TestAPIKey(t *testing.T) { _, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{ ID: id, HashedSecret: hashed[:], + UserID: user.ID, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil)(successHandler).ServeHTTP(rw, r) @@ -155,6 +159,7 @@ func TestAPIKey(t *testing.T) { hashed = sha256.Sum256([]byte(secret)) r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) ) r.AddCookie(&http.Cookie{ Name: httpmw.SessionTokenKey, @@ -164,6 +169,7 @@ func TestAPIKey(t *testing.T) { _, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{ ID: id, HashedSecret: hashed[:], + UserID: user.ID, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil)(successHandler).ServeHTTP(rw, r) @@ -180,6 +186,7 @@ func TestAPIKey(t *testing.T) { hashed = sha256.Sum256([]byte(secret)) r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) ) r.AddCookie(&http.Cookie{ Name: httpmw.SessionTokenKey, @@ -190,6 +197,7 @@ func TestAPIKey(t *testing.T) { ID: id, HashedSecret: hashed[:], ExpiresAt: database.Now().AddDate(0, 0, 1), + UserID: user.ID, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -217,6 +225,7 @@ func TestAPIKey(t *testing.T) { hashed = sha256.Sum256([]byte(secret)) r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) ) q := r.URL.Query() q.Add(httpmw.SessionTokenKey, fmt.Sprintf("%s-%s", id, secret)) @@ -226,6 +235,7 @@ func TestAPIKey(t *testing.T) { ID: id, HashedSecret: hashed[:], ExpiresAt: database.Now().AddDate(0, 0, 1), + UserID: user.ID, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { @@ -248,6 +258,7 @@ func TestAPIKey(t *testing.T) { hashed = sha256.Sum256([]byte(secret)) r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) ) r.AddCookie(&http.Cookie{ Name: httpmw.SessionTokenKey, @@ -259,6 +270,7 @@ func TestAPIKey(t *testing.T) { HashedSecret: hashed[:], LastUsed: database.Now().AddDate(0, 0, -1), ExpiresAt: database.Now().AddDate(0, 0, 1), + UserID: user.ID, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil)(successHandler).ServeHTTP(rw, r) @@ -281,6 +293,7 @@ func TestAPIKey(t *testing.T) { hashed = sha256.Sum256([]byte(secret)) r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) ) r.AddCookie(&http.Cookie{ Name: httpmw.SessionTokenKey, @@ -292,6 +305,7 @@ func TestAPIKey(t *testing.T) { HashedSecret: hashed[:], LastUsed: database.Now(), ExpiresAt: database.Now().Add(time.Minute), + UserID: user.ID, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil)(successHandler).ServeHTTP(rw, r) @@ -314,6 +328,7 @@ func TestAPIKey(t *testing.T) { hashed = sha256.Sum256([]byte(secret)) r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) ) r.AddCookie(&http.Cookie{ Name: httpmw.SessionTokenKey, @@ -326,6 +341,7 @@ func TestAPIKey(t *testing.T) { LoginType: database.LoginTypeGithub, LastUsed: database.Now(), ExpiresAt: database.Now().AddDate(0, 0, 1), + UserID: user.ID, }) require.NoError(t, err) httpmw.ExtractAPIKey(db, nil)(successHandler).ServeHTTP(rw, r) @@ -348,6 +364,7 @@ func TestAPIKey(t *testing.T) { hashed = sha256.Sum256([]byte(secret)) r = httptest.NewRequest("GET", "/", nil) rw = httptest.NewRecorder() + user = createUser(r.Context(), t, db) ) r.AddCookie(&http.Cookie{ Name: httpmw.SessionTokenKey, @@ -360,6 +377,7 @@ func TestAPIKey(t *testing.T) { LoginType: database.LoginTypeGithub, LastUsed: database.Now(), OAuthExpiry: database.Now().AddDate(0, 0, -1), + UserID: user.ID, }) require.NoError(t, err) token := &oauth2.Token{ @@ -387,6 +405,20 @@ func TestAPIKey(t *testing.T) { }) } +func createUser(ctx context.Context, t *testing.T, db database.Store) database.User { + user, err := db.InsertUser(ctx, database.InsertUserParams{ + ID: uuid.New(), + Email: "email@coder.com", + Username: "username", + HashedPassword: []byte{}, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + RBACRoles: []string{}, + }) + require.NoError(t, err, "create user") + return user +} + type oauth2Config struct { tokenSource oauth2TokenSource } diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go index 0e2466da403cd..279c037e2ae92 100644 --- a/coderd/httpmw/authorize_test.go +++ b/coderd/httpmw/authorize_test.go @@ -84,7 +84,6 @@ func TestExtractUserRoles(t *testing.T) { ) rtr.Use( httpmw.ExtractAPIKey(db, &httpmw.OAuth2Configs{}), - httpmw.ExtractUserRoles(db), ) rtr.Get("/", func(_ http.ResponseWriter, r *http.Request) { roles := httpmw.UserRoles(r) diff --git a/scripts/develop.sh b/scripts/develop.sh index da481317ad2dc..8182aa4bed372 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -14,6 +14,8 @@ echo '== Without these binaries, workspaces will fail to start!' # Use static credentials for development export CODER_DEV_ADMIN_EMAIL=admin@coder.com export CODER_DEV_ADMIN_PASSWORD=password +export CODER_DEV_MEMBER_EMAIL=member@coder.com +export CODER_DEV_MEMBER_PASSWORD=password # This is a way to run multiple processes in parallel, and have Ctrl-C work correctly # to kill both at the same time. For more details, see: From 87f28a24eef33e523c18c290cebb04615a4104bb Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 11:11:25 -0500 Subject: [PATCH 02/15] Add unit test for log into suspended account --- coderd/users.go | 8 ++++++++ coderd/users_test.go | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/coderd/users.go b/coderd/users.go index 8e927a9466d7b..6f7e097adb334 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -637,6 +637,14 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { return } + // If the user logged into a suspended account, reject the login request. + if user.Status != database.UserStatusActive { + httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ + Message: fmt.Sprintf("user is not active (status = %q), contact an admin to reactivate your account", user.Status), + }) + return + } + sessionToken, created := api.createAPIKey(rw, r, database.InsertAPIKeyParams{ UserID: user.ID, LoginType: database.LoginTypePassword, diff --git a/coderd/users_test.go b/coderd/users_test.go index b1c3bddc2e89c..d6f62100c7582 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -81,6 +81,28 @@ func TestPostLogin(t *testing.T) { require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) }) + t.Run("Suspended", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + first := coderdtest.CreateFirstUser(t, client) + + member := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + memberUser, err := member.User(context.Background(), codersdk.Me) + require.NoError(t, err, "fetch member user") + + _, err = client.UpdateUserStatus(context.Background(), memberUser.Username, codersdk.UserStatusSuspended) + require.NoError(t, err, "suspend member") + + _, err = client.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{ + Email: memberUser.Email, + Password: "testpass", + }) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) + require.Contains(t, apiErr.Message, "suspended") + }) + t.Run("Success", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) From 047c234aad3e1a03dfb68e8b0a019a949050517c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 11:37:05 -0500 Subject: [PATCH 03/15] Formatting --- cli/server.go | 3 +-- coderd/httpmw/apikey_test.go | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/cli/server.go b/cli/server.go index 8c0fa66923ae9..e2c1d912fa722 100644 --- a/cli/server.go +++ b/cli/server.go @@ -72,8 +72,7 @@ func server() *cobra.Command { devFirstPassword string devMemberEmail string devMemberPassword string - - postgresURL string + postgresURL string // provisionerDaemonCount is a uint8 to ensure a number > 0. provisionerDaemonCount uint8 oauth2GithubClientID string diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index d55924dc1532d..c841798094b08 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -10,7 +10,6 @@ import ( "time" "github.com/google/uuid" - "github.com/stretchr/testify/require" "golang.org/x/oauth2" From d2bc5b82ea79baa0967c3c55c9e3b367c4019e62 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 12:02:18 -0500 Subject: [PATCH 04/15] feat: UI Shows suspended error logging into suspended account --- coderd/users.go | 5 +++-- site/src/components/SignInForm/SignInForm.tsx | 2 +- site/src/pages/LoginPage/LoginPage.tsx | 8 +++++++- site/src/xServices/auth/authXService.ts | 3 ++- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 6f7e097adb334..13b93f44794c0 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -632,15 +632,16 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { // This message is the same as above to remove ease in detecting whether // users are registered or not. Attackers still could with a timing attack. httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ - Message: "invalid email or password", + Message: "Incorrect email or password", }) return } // If the user logged into a suspended account, reject the login request. if user.Status != database.UserStatusActive { + httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ - Message: fmt.Sprintf("user is not active (status = %q), contact an admin to reactivate your account", user.Status), + Message: "You are suspended, contact an admin to reactivate your account", }) return } diff --git a/site/src/components/SignInForm/SignInForm.tsx b/site/src/components/SignInForm/SignInForm.tsx index 9ec486221a03d..502726c09fd1e 100644 --- a/site/src/components/SignInForm/SignInForm.tsx +++ b/site/src/components/SignInForm/SignInForm.tsx @@ -110,7 +110,7 @@ export const SignInForm: React.FC = ({ type="password" variant="outlined" /> - {authErrorMessage && {Language.authErrorMessage}} + {authErrorMessage && {authErrorMessage}} {methodsErrorMessage && {Language.methodsErrorMessage}}
diff --git a/site/src/pages/LoginPage/LoginPage.tsx b/site/src/pages/LoginPage/LoginPage.tsx index fe4bcedae5646..cb012977d80df 100644 --- a/site/src/pages/LoginPage/LoginPage.tsx +++ b/site/src/pages/LoginPage/LoginPage.tsx @@ -6,6 +6,8 @@ import { Footer } from "../../components/Footer/Footer" import { SignInForm } from "../../components/SignInForm/SignInForm" import { retrieveRedirect } from "../../util/redirect" import { XServiceContext } from "../../xServices/StateContext" +import { AxiosError } from "axios" +import {isApiError} from "../../api/errors"; export const useStyles = makeStyles((theme) => ({ root: { @@ -33,7 +35,11 @@ export const LoginPage: React.FC = () => { const [authState, authSend] = useActor(xServices.authXService) const isLoading = authState.hasTag("loading") const redirectTo = retrieveRedirect(location.search) - const authErrorMessage = authState.context.authError ? (authState.context.authError as Error).message : undefined + //{ + // "message": "user is not active (status = \"suspended\"), contact an admin to reactivate your account" + // } + + const authErrorMessage = isApiError(authState.context.authError) ? authState.context.authError.response.data.message : undefined const getMethodsError = authState.context.getMethodsError ? (authState.context.getMethodsError as Error).message : undefined diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 3e634fc8b58d8..eb0d3512fe60a 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -2,6 +2,7 @@ import { assign, createMachine } from "xstate" import * as API from "../../api/api" import * as TypesGen from "../../api/typesGenerated" import { displayError, displaySuccess } from "../../components/GlobalSnackbar/utils" +import { AxiosError } from "axios" export const Language = { successProfileUpdate: "Updated settings.", @@ -48,7 +49,7 @@ type Permissions = Record export interface AuthContext { getUserError?: Error | unknown getMethodsError?: Error | unknown - authError?: Error | unknown + authError?: Error | AxiosError | unknown updateProfileError?: Error | unknown me?: TypesGen.User methods?: TypesGen.AuthMethods From 8654dd164a98a10727426e1f4f1afab62d0bac8d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 12:29:44 -0500 Subject: [PATCH 05/15] cleanup --- cli/server.go | 10 +++++----- coderd/users.go | 1 - scripts/develop.sh | 2 -- site/src/pages/LoginPage/LoginPage.tsx | 4 ---- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/cli/server.go b/cli/server.go index 2af3470ec14bd..48d7ee237a1ca 100644 --- a/cli/server.go +++ b/cli/server.go @@ -648,16 +648,16 @@ func newProvisionerDaemon(ctx context.Context, coderAPI *coderd.API, func printLogo(cmd *cobra.Command, spooky bool) { if spooky { _, _ = fmt.Fprintf(cmd.OutOrStdout(), ` - ▄████▄ ▒█████ ▓█████▄ ▓█████ ██▀███ + ▄████▄ ▒█████ ▓█████▄ ▓█████ ██▀███ ▒██▀ ▀█ ▒██▒ ██▒▒██▀ ██▌▓█ ▀ ▓██ ▒ ██▒ ▒▓█ ▄ ▒██░ ██▒░██ █▌▒███ ▓██ ░▄█ ▒ - ▒▓▓▄ ▄██▒▒██ ██░░▓█▄ ▌▒▓█ ▄ ▒██▀▀█▄ + ▒▓▓▄ ▄██▒▒██ ██░░▓█▄ ▌▒▓█ ▄ ▒██▀▀█▄ ▒ ▓███▀ ░░ ████▓▒░░▒████▓ ░▒████▒░██▓ ▒██▒ ░ ░▒ ▒ ░░ ▒░▒░▒░ ▒▒▓ ▒ ░░ ▒░ ░░ ▒▓ ░▒▓░ ░ ▒ ░ ▒ ▒░ ░ ▒ ▒ ░ ░ ░ ░▒ ░ ▒░ - ░ ░ ░ ░ ▒ ░ ░ ░ ░ ░░ ░ - ░ ░ ░ ░ ░ ░ ░ ░ - ░ ░ + ░ ░ ░ ░ ▒ ░ ░ ░ ░ ░░ ░ + ░ ░ ░ ░ ░ ░ ░ ░ + ░ ░ `) return diff --git a/coderd/users.go b/coderd/users.go index c9b01fe74a5e4..b7bb34d67387b 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -574,7 +574,6 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { // If the user logged into a suspended account, reject the login request. if user.Status != database.UserStatusActive { - httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ Message: "You are suspended, contact an admin to reactivate your account", }) diff --git a/scripts/develop.sh b/scripts/develop.sh index a62accc0340e0..41418e4ae3f09 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -14,8 +14,6 @@ echo '== Without these binaries, workspaces will fail to start!' # Use static credentials for development export CODER_DEV_ADMIN_EMAIL=admin@coder.com export CODER_DEV_ADMIN_PASSWORD=password -export CODER_DEV_MEMBER_EMAIL=member@coder.com -export CODER_DEV_MEMBER_PASSWORD=password # This is a way to run multiple processes in parallel, and have Ctrl-C work correctly # to kill both at the same time. For more details, see: diff --git a/site/src/pages/LoginPage/LoginPage.tsx b/site/src/pages/LoginPage/LoginPage.tsx index cb012977d80df..710e0607e1b87 100644 --- a/site/src/pages/LoginPage/LoginPage.tsx +++ b/site/src/pages/LoginPage/LoginPage.tsx @@ -35,10 +35,6 @@ export const LoginPage: React.FC = () => { const [authState, authSend] = useActor(xServices.authXService) const isLoading = authState.hasTag("loading") const redirectTo = retrieveRedirect(location.search) - //{ - // "message": "user is not active (status = \"suspended\"), contact an admin to reactivate your account" - // } - const authErrorMessage = isApiError(authState.context.authError) ? authState.context.authError.response.data.message : undefined const getMethodsError = authState.context.getMethodsError ? (authState.context.getMethodsError as Error).message From 489b2fe788582478ace03d467430a12480c2015d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 13:24:55 -0500 Subject: [PATCH 06/15] fix: Show suspended users in the UI --- coderd/database/databasefake/databasefake.go | 8 ++++--- coderd/database/queries.sql.go | 22 ++++++++++--------- coderd/database/queries/users.sql | 10 +++++---- coderd/users.go | 20 ++++++++++++++--- scripts/develop.sh | 6 +++++ site/src/api/api.ts | 2 +- site/src/components/UsersTable/UsersTable.tsx | 20 ++++++++++++----- 7 files changed, 61 insertions(+), 27 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index f2fa2fa99d280..0c06829c0e624 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -216,11 +216,13 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams users = tmp } - if params.Status != "" { + if len(params.Status) > 0 { usersFilteredByStatus := make([]database.User, 0, len(users)) for i, user := range users { - if params.Status == string(user.Status) { - usersFilteredByStatus = append(usersFilteredByStatus, users[i]) + for _, status := range params.Status { + if user.Status == status { + usersFilteredByStatus = append(usersFilteredByStatus, users[i]) + } } } users = usersFilteredByStatus diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index c693a58b4318c..4696832ff3a8a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2213,17 +2213,19 @@ WHERE WHEN $2 :: text != '' THEN ( email LIKE concat('%', $2, '%') OR username LIKE concat('%', $2, '%') - ) + ) ELSE true END -- Filter by status AND CASE -- @status needs to be a text because it can be empty, If it was -- user_status enum, it would not. - WHEN $3 :: text != '' THEN ( - status = $3 :: user_status + WHEN cardinality($3 :: user_status[]) > 0 THEN ( + status = ANY($3 :: user_status[]) ) - ELSE true + ELSE + -- Only show active by default + status = 'active' END -- End of filters ORDER BY @@ -2236,18 +2238,18 @@ LIMIT ` type GetUsersParams struct { - AfterID uuid.UUID `db:"after_id" json:"after_id"` - Search string `db:"search" json:"search"` - Status string `db:"status" json:"status"` - OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` - LimitOpt int32 `db:"limit_opt" json:"limit_opt"` + AfterID uuid.UUID `db:"after_id" json:"after_id"` + Search string `db:"search" json:"search"` + Status []UserStatus `db:"status" json:"status"` + OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` + LimitOpt int32 `db:"limit_opt" json:"limit_opt"` } func (q *sqlQuerier) GetUsers(ctx context.Context, arg GetUsersParams) ([]User, error) { rows, err := q.db.QueryContext(ctx, getUsers, arg.AfterID, arg.Search, - arg.Status, + pq.Array(arg.Status), arg.OffsetOpt, arg.LimitOpt, ) diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 6d650366b217f..a4a8bcfc7bec6 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -101,17 +101,19 @@ WHERE WHEN @search :: text != '' THEN ( email LIKE concat('%', @search, '%') OR username LIKE concat('%', @search, '%') - ) + ) ELSE true END -- Filter by status AND CASE -- @status needs to be a text because it can be empty, If it was -- user_status enum, it would not. - WHEN @status :: text != '' THEN ( - status = @status :: user_status + WHEN cardinality(@status :: user_status[]) > 0 THEN ( + status = ANY(@status :: user_status[]) ) - ELSE true + ELSE + -- Only show active by default + status = 'active' END -- End of filters ORDER BY diff --git a/coderd/users.go b/coderd/users.go index b7bb34d67387b..2c3d727a8f232 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "strings" "time" "github.com/go-chi/chi/v5" @@ -105,10 +106,23 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { func (api *API) users(rw http.ResponseWriter, r *http.Request) { var ( - searchName = r.URL.Query().Get("search") - statusFilter = r.URL.Query().Get("status") + searchName = r.URL.Query().Get("search") + statusFilters = strings.Split(r.URL.Query().Get("status"), ",") ) + statuses := make([]database.UserStatus, 0) + for _, filter := range statusFilters { + switch database.UserStatus(filter) { + case database.UserStatusSuspended, database.UserStatusActive: + statuses = append(statuses, database.UserStatus(filter)) + default: + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("%q is not a valid user status", filter), + }) + return + } + } + // Reading all users across the site if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser) { return @@ -124,7 +138,7 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) { OffsetOpt: int32(paginationParams.Offset), LimitOpt: int32(paginationParams.Limit), Search: searchName, - Status: statusFilter, + Status: statuses, }) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusOK, []codersdk.User{}) diff --git a/scripts/develop.sh b/scripts/develop.sh index 41418e4ae3f09..dff553113e2f3 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -24,5 +24,11 @@ export CODER_DEV_ADMIN_PASSWORD=password trap 'kill 0' SIGINT CODERV2_HOST=http://127.0.0.1:3000 INSPECT_XSTATE=true yarn --cwd=./site dev & go run -tags embed cmd/coder/main.go server --dev --tunnel=true & + + # Just a minor sleep to ensure the first user was created to make the member. + sleep 2 + # || yes to always exit code 0. If this fails, whelp. + go run cmd/coder/main.go users create --email=member@coder.com --username=member --password="${CODER_DEV_ADMIN_PASSWORD}" || yes wait ) + diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 0697200681513..69effd9a31691 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -63,7 +63,7 @@ export const getApiKey = async (): Promise => { } export const getUsers = async (): Promise => { - const response = await axios.get("/api/v2/users?status=active") + const response = await axios.get("/api/v2/users?status=active,suspended") return response.data } diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 1ed0552caa976..5ce3c694832dd 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -18,8 +18,10 @@ export const Language = { emptyMessage: "No users found", usernameLabel: "User", suspendMenuItem: "Suspend", + activateMenuItem: "Activate", resetPasswordMenuItem: "Reset password", rolesLabel: "Roles", + statusLabel: "Status", } export interface UsersTableProps { @@ -49,6 +51,7 @@ export const UsersTable: React.FC = ({ {Language.usernameLabel} {Language.rolesLabel} + {Language.statusLabel} {/* 1% is a trick to make the table cell width fit the content */} {canEditUsers && } @@ -62,6 +65,9 @@ export const UsersTable: React.FC = ({ + + {u.status} + {canEditUsers ? ( = ({ )} From 15a9b365a831d87f345d96e1d57c381084681d9c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 13:25:28 -0500 Subject: [PATCH 07/15] Fix column order --- site/src/components/UsersTable/UsersTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 5ce3c694832dd..1af25820d42a5 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -50,8 +50,8 @@ export const UsersTable: React.FC = ({ {Language.usernameLabel} - {Language.rolesLabel} {Language.statusLabel} + {Language.rolesLabel} {/* 1% is a trick to make the table cell width fit the content */} {canEditUsers && } From 55c5fe6ccef1826cdbb827cfaf057377ebb147b6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 15:01:39 -0500 Subject: [PATCH 08/15] Currently "activate" on ui does nothing --- site/src/api/api.ts | 5 ++++ site/src/components/UsersTable/UsersTable.tsx | 29 ++++++++++++------- site/src/pages/LoginPage/LoginPage.tsx | 7 +++-- site/src/xServices/auth/authXService.ts | 2 +- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 69effd9a31691..ddc4a20cac8d5 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -218,6 +218,11 @@ export const updateProfile = async ( return response.data } +export const activateUser = async (userId: TypesGen.User["id"]): Promise => { + const response = await axios.put(`/api/v2/users/${userId}/status/active`) + return response.data +} + export const suspendUser = async (userId: TypesGen.User["id"]): Promise => { const response = await axios.put(`/api/v2/users/${userId}/status/suspend`) return response.data diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 1af25820d42a5..9708eccfb5b16 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -65,9 +65,7 @@ export const UsersTable: React.FC = ({ - - {u.status} - + {u.status} {canEditUsers ? ( = ({ data={u} menuItems={ // Return either suspend or activate depending on status - (u.status == "active" ? [{ - label: Language.suspendMenuItem, - onClick: onSuspendUser, - }] : [{ - label: Language.activateMenuItem, - onClick: onSuspendUser, - }]).concat({ + (u.status == "active" + ? [ + { + label: Language.suspendMenuItem, + onClick: onSuspendUser, + }, + ] + : [ + { + label: Language.activateMenuItem, + // TODO: Activate user + onClick: function () {}, + }, + ] + ).concat({ label: Language.resetPasswordMenuItem, onClick: onResetUserPassword, - })} + }) + } /> )} diff --git a/site/src/pages/LoginPage/LoginPage.tsx b/site/src/pages/LoginPage/LoginPage.tsx index 710e0607e1b87..cc3d8eb6e6efb 100644 --- a/site/src/pages/LoginPage/LoginPage.tsx +++ b/site/src/pages/LoginPage/LoginPage.tsx @@ -2,12 +2,11 @@ import { makeStyles } from "@material-ui/core/styles" import { useActor } from "@xstate/react" import React, { useContext } from "react" import { Navigate, useLocation } from "react-router-dom" +import { isApiError } from "../../api/errors" import { Footer } from "../../components/Footer/Footer" import { SignInForm } from "../../components/SignInForm/SignInForm" import { retrieveRedirect } from "../../util/redirect" import { XServiceContext } from "../../xServices/StateContext" -import { AxiosError } from "axios" -import {isApiError} from "../../api/errors"; export const useStyles = makeStyles((theme) => ({ root: { @@ -35,7 +34,9 @@ export const LoginPage: React.FC = () => { const [authState, authSend] = useActor(xServices.authXService) const isLoading = authState.hasTag("loading") const redirectTo = retrieveRedirect(location.search) - const authErrorMessage = isApiError(authState.context.authError) ? authState.context.authError.response.data.message : undefined + const authErrorMessage = isApiError(authState.context.authError) + ? authState.context.authError.response.data.message + : undefined const getMethodsError = authState.context.getMethodsError ? (authState.context.getMethodsError as Error).message : undefined diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index eb0d3512fe60a..4c5a951d34a86 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -1,8 +1,8 @@ +import { AxiosError } from "axios" import { assign, createMachine } from "xstate" import * as API from "../../api/api" import * as TypesGen from "../../api/typesGenerated" import { displayError, displaySuccess } from "../../components/GlobalSnackbar/utils" -import { AxiosError } from "axios" export const Language = { successProfileUpdate: "Updated settings.", From c5f33a507fab52a0b51a36c8debbfa0b7e1ad70c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 20:13:06 +0000 Subject: [PATCH 09/15] Linting fixes --- site/src/components/UsersTable/UsersTable.tsx | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 9708eccfb5b16..3bbd9aaaf8c0f 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -84,20 +84,21 @@ export const UsersTable: React.FC = ({ data={u} menuItems={ // Return either suspend or activate depending on status - (u.status == "active" + (u.status === "active" ? [ - { - label: Language.suspendMenuItem, - onClick: onSuspendUser, - }, - ] + { + label: Language.suspendMenuItem, + onClick: onSuspendUser, + }, + ] : [ - { - label: Language.activateMenuItem, - // TODO: Activate user - onClick: function () {}, - }, - ] + { + label: Language.activateMenuItem, + // TODO: Activate user + // eslint-disable-next-line @typescript-eslint/no-empty-function + onClick: function () {}, + }, + ] ).concat({ label: Language.resetPasswordMenuItem, onClick: onResetUserPassword, From f9a47a93bb9546da368c5acf1a0bc76344783f93 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 15:17:31 -0500 Subject: [PATCH 10/15] Account for no status filters sent --- coderd/users.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 2c3d727a8f232..7ee07737bcb99 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -107,19 +107,23 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { func (api *API) users(rw http.ResponseWriter, r *http.Request) { var ( searchName = r.URL.Query().Get("search") - statusFilters = strings.Split(r.URL.Query().Get("status"), ",") + statusFilters = r.URL.Query().Get("status") ) statuses := make([]database.UserStatus, 0) - for _, filter := range statusFilters { - switch database.UserStatus(filter) { - case database.UserStatusSuspended, database.UserStatusActive: - statuses = append(statuses, database.UserStatus(filter)) - default: - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("%q is not a valid user status", filter), - }) - return + + if statusFilters != "" { + // Split on commas if present to account for it being a list + for _, filter := range strings.Split(statusFilters, ",") { + switch database.UserStatus(filter) { + case database.UserStatusSuspended, database.UserStatusActive: + statuses = append(statuses, database.UserStatus(filter)) + default: + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("%q is not a valid user status", filter), + }) + return + } } } From 71ba33271909bb107f1a243a79ae45d9b44b8dfa Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 15:24:14 -0500 Subject: [PATCH 11/15] Linting fix eslint --- site/src/components/UsersTable/UsersTable.tsx | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 3bbd9aaaf8c0f..c4a9e5417873c 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -86,19 +86,19 @@ export const UsersTable: React.FC = ({ // Return either suspend or activate depending on status (u.status === "active" ? [ - { - label: Language.suspendMenuItem, - onClick: onSuspendUser, - }, - ] + { + label: Language.suspendMenuItem, + onClick: onSuspendUser, + }, + ] : [ - { - label: Language.activateMenuItem, - // TODO: Activate user - // eslint-disable-next-line @typescript-eslint/no-empty-function - onClick: function () {}, - }, - ] + { + label: Language.activateMenuItem, + // TODO: Activate user + // eslint-disable-next-line @typescript-eslint/no-empty-function + onClick: function () {}, + }, + ] ).concat({ label: Language.resetPasswordMenuItem, onClick: onResetUserPassword, From 54f44d906b707b5d1dcbc178fa76fbfcf5c51997 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 15:25:12 -0500 Subject: [PATCH 12/15] Shfmt --- scripts/develop.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/develop.sh b/scripts/develop.sh index dff553113e2f3..745446db0140a 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -31,4 +31,3 @@ export CODER_DEV_ADMIN_PASSWORD=password go run cmd/coder/main.go users create --email=member@coder.com --username=member --password="${CODER_DEV_ADMIN_PASSWORD}" || yes wait ) - From 8dbf46432c631a3bdee9c85f7c91a964017c5565 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 15:25:50 -0500 Subject: [PATCH 13/15] Fix BE error message --- coderd/users.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/users.go b/coderd/users.go index 7ee07737bcb99..0e4f911ec3759 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -585,7 +585,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { // This message is the same as above to remove ease in detecting whether // users are registered or not. Attackers still could with a timing attack. httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ - Message: "Incorrect email or password", + Message: "Incorrect email or password.", }) return } From 60ac7e4bbed4465dd2fea3ced2b9a162511b942f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 15:49:55 -0500 Subject: [PATCH 14/15] Add correct error message --- site/src/pages/LoginPage/LoginPage.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/LoginPage/LoginPage.test.tsx b/site/src/pages/LoginPage/LoginPage.test.tsx index f81c3ebddeae6..ed54435abea48 100644 --- a/site/src/pages/LoginPage/LoginPage.test.tsx +++ b/site/src/pages/LoginPage/LoginPage.test.tsx @@ -31,7 +31,7 @@ describe("LoginPage", () => { server.use( // Make login fail rest.post("/api/v2/users/login", async (req, res, ctx) => { - return res(ctx.status(500), ctx.json({ message: "nope" })) + return res(ctx.status(500), ctx.json({ message: Language.authErrorMessage })) }), ) From df96f3da464c017d59407ba161ade02a8ae5021d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 31 May 2022 07:43:49 -0500 Subject: [PATCH 15/15] change 'active' route to 'activate' - Remove "activate" button --- coderd/coderd.go | 2 +- coderd/users_test.go | 9 ++++++++- codersdk/users.go | 2 +- site/src/api/api.ts | 2 +- site/src/components/UsersTable/UsersTable.tsx | 12 ++++++------ 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 670cc6cf44913..dbb5deb7fe9aa 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -236,7 +236,7 @@ func New(options *Options) *API { r.Put("/profile", api.putUserProfile) r.Route("/status", func(r chi.Router) { r.Put("/suspend", api.putUserStatus(database.UserStatusSuspended)) - r.Put("/active", api.putUserStatus(database.UserStatusActive)) + r.Put("/activate", api.putUserStatus(database.UserStatusActive)) }) r.Route("/password", func(r chi.Router) { r.Put("/", api.putUserPassword) diff --git a/coderd/users_test.go b/coderd/users_test.go index 48937231f34bf..17d8afa9578f6 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -96,11 +96,18 @@ func TestPostLogin(t *testing.T) { _, err = client.UpdateUserStatus(context.Background(), memberUser.Username, codersdk.UserStatusSuspended) require.NoError(t, err, "suspend member") + // Test an existing session + _, err = member.User(context.Background(), codersdk.Me) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) + require.Contains(t, apiErr.Message, "contact an admin") + + // Test a new session _, err = client.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{ Email: memberUser.Email, Password: "testpass", }) - var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) require.Contains(t, apiErr.Message, "suspended") diff --git a/codersdk/users.go b/codersdk/users.go index c41b660b4b3ec..658f9b9dbdb0c 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -218,7 +218,7 @@ func (c *Client) UpdateUserStatus(ctx context.Context, user string, status UserS path := fmt.Sprintf("/api/v2/users/%s/status/", user) switch status { case UserStatusActive: - path += "active" + path += "activate" case UserStatusSuspended: path += "suspend" default: diff --git a/site/src/api/api.ts b/site/src/api/api.ts index ddc4a20cac8d5..46d3d50dbfc6d 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -219,7 +219,7 @@ export const updateProfile = async ( } export const activateUser = async (userId: TypesGen.User["id"]): Promise => { - const response = await axios.put(`/api/v2/users/${userId}/status/active`) + const response = await axios.put(`/api/v2/users/${userId}/status/activate`) return response.data } diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index c4a9e5417873c..f71dfde32b770 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -92,12 +92,12 @@ export const UsersTable: React.FC = ({ }, ] : [ - { - label: Language.activateMenuItem, - // TODO: Activate user - // eslint-disable-next-line @typescript-eslint/no-empty-function - onClick: function () {}, - }, + // TODO: Uncomment this and add activate user functionality. + // { + // label: Language.activateMenuItem, + // // eslint-disable-next-line @typescript-eslint/no-empty-function + // onClick: function () {}, + // }, ] ).concat({ label: Language.resetPasswordMenuItem,