From 346e5e44629a2c4438cba0ed64fd7734e9569b93 Mon Sep 17 00:00:00 2001 From: Bruno Date: Wed, 4 May 2022 19:47:47 +0000 Subject: [PATCH 01/15] Add UpdateUserHashedPassword query --- coderd/database/queries/users.sql | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 2f0ecb3709b9b..d0c3d4c3e7b66 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -59,6 +59,14 @@ WHERE id = @id RETURNING *; +-- name: UpdateUserHashedPassword :exec +UPDATE + users +SET + hashed_password = $2 +WHERE + id = $1; + -- name: GetUsers :many SELECT * @@ -133,4 +141,4 @@ FROM LEFT JOIN organization_members ON id = user_id WHERE - id = @user_id; \ No newline at end of file + id = @user_id; From de39cf56a5463f84ec910b8ad2faa0380b0d7496 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 12:43:34 +0000 Subject: [PATCH 02/15] Add database functions --- coderd/database/databasefake/databasefake.go | 15 +++++++++++++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 19 +++++++++++++++++++ coderd/database/queries/users.sql | 2 +- 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 5726c9f7befe5..f33adf922f100 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1314,6 +1314,21 @@ func (q *fakeQuerier) UpdateUserStatus(_ context.Context, arg database.UpdateUse return database.User{}, sql.ErrNoRows } +func (q *fakeQuerier) UpdateUserHashedPassword(_ context.Context, arg database.UpdateUserHashedPasswordParams) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + for index, user := range q.users { + if user.ID != arg.ID { + continue + } + user.HashedPassword = arg.HashedPassword + q.users[index] = user + return nil + } + return sql.ErrNoRows +} + func (q *fakeQuerier) InsertWorkspace(_ context.Context, arg database.InsertWorkspaceParams) (database.Workspace, error) { q.mutex.Lock() defer q.mutex.Unlock() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index db2f3f0d49987..dbf6b5cfed8cd 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -100,6 +100,7 @@ type querier interface { UpdateTemplateActiveVersionByID(ctx context.Context, arg UpdateTemplateActiveVersionByIDParams) error UpdateTemplateDeletedByID(ctx context.Context, arg UpdateTemplateDeletedByIDParams) error UpdateTemplateVersionByID(ctx context.Context, arg UpdateTemplateVersionByIDParams) error + UpdateUserHashedPassword(ctx context.Context, arg UpdateUserHashedPasswordParams) error UpdateUserProfile(ctx context.Context, arg UpdateUserProfileParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index f91070915c097..450d540a192ac 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2264,6 +2264,25 @@ func (q *sqlQuerier) InsertUser(ctx context.Context, arg InsertUserParams) (User return i, err } +const updateUserHashedPassword = `-- name: UpdateUserHashedPassword :exec +UPDATE + users +SET + hashed_password = $2 +WHERE + id = $1 +` + +type UpdateUserHashedPasswordParams struct { + ID uuid.UUID `db:"id" json:"id"` + HashedPassword []byte `db:"hashed_password" json:"hashed_password"` +} + +func (q *sqlQuerier) UpdateUserHashedPassword(ctx context.Context, arg UpdateUserHashedPasswordParams) error { + _, err := q.db.ExecContext(ctx, updateUserHashedPassword, arg.ID, arg.HashedPassword) + return err +} + const updateUserProfile = `-- name: UpdateUserProfile :one UPDATE users diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index d0c3d4c3e7b66..35ade0e9762a4 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -70,7 +70,7 @@ WHERE -- name: GetUsers :many SELECT * -FROM +FROM users WHERE CASE From 2fe171618e5698f9f38ba42ffd42965f2d14e911 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 13:10:42 +0000 Subject: [PATCH 03/15] Add update user password endpoint --- coderd/coderd.go | 1 + coderd/users.go | 58 +++++++++++++++++++++++++++++++++++++++++++++++ codersdk/users.go | 6 +++++ 3 files changed, 65 insertions(+) diff --git a/coderd/coderd.go b/coderd/coderd.go index 191d3339aa2b1..a1d028c7295f7 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -240,6 +240,7 @@ func New(options *Options) (http.Handler, func()) { r.Get("/", api.userByName) r.Put("/profile", api.putUserProfile) r.Put("/suspend", api.putUserSuspend) + r.Put("/password", api.putUserPassword) r.Get("/organizations", api.organizationsByUser) r.Post("/organizations", api.postOrganizationsByUser) // These roles apply to the site wide permissions. diff --git a/coderd/users.go b/coderd/users.go index 310bee8e79597..a56602d28ed89 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -360,6 +360,64 @@ func (api *api) putUserSuspend(rw http.ResponseWriter, r *http.Request) { httpapi.Write(rw, http.StatusOK, convertUser(suspendedUser, organizations)) } +func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { + user := httpmw.UserParam(r) + + var params codersdk.UpdateUserHashedPasswordRequest + if !httpapi.Read(rw, r, ¶ms) { + return + } + + // Check if the password is correct + equal, err := userpassword.Compare(string(user.HashedPassword), params.Password) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: fmt.Sprintf("compare: %s", err.Error()), + }) + } + if !equal { + // 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", + }) + return + } + + // Check if the new password and the confirmation match + if params.NewPassword != params.ConfirmNewPassword { + errors := []httpapi.Error{ + { + Field: "confirm_new_password", + Detail: "The value does not match the new password", + }, + } + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("The new password and the new password confirmation don't match"), + Errors: errors, + }) + return + } + + // Hash password and update it in the database + hashedPassword, hashError := userpassword.Hash(params.NewPassword) + if hashError != nil { + xerrors.Errorf("hash password: %w", hashError) + return + } + databaseError := api.Database.UpdateUserHashedPassword(r.Context(), database.UpdateUserHashedPasswordParams{ + HashedPassword: []byte(hashedPassword), + }) + if databaseError != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: fmt.Sprintf("put user password: %s", err.Error()), + }) + return + } + + httpapi.Write(rw, http.StatusNoContent, nil) +} + func (api *api) userRoles(rw http.ResponseWriter, r *http.Request) { user := httpmw.UserParam(r) diff --git a/codersdk/users.go b/codersdk/users.go index e2c8261febdec..d7b0566755600 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -72,6 +72,12 @@ type UpdateUserProfileRequest struct { Username string `json:"username" validate:"required,username"` } +type UpdateUserHashedPasswordRequest struct { + Password string `json:"password" validate:"required"` + NewPassword string `json:"new_password" validate:"required"` + ConfirmNewPassword string `json:"confirm_new_password" validate:"required"` +} + type UpdateRoles struct { Roles []string `json:"roles" validate:"required"` } From 212020aaf102abdebca84e7ae8ccd0eb91d34470 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 14:01:01 +0000 Subject: [PATCH 04/15] Add tests and fixes --- coderd/coderdtest/coderdtest.go | 19 ++++++------ coderd/users.go | 6 ++-- coderd/users_test.go | 51 +++++++++++++++++++++++++++++++++ codersdk/users.go | 15 +++++++++- 4 files changed, 78 insertions(+), 13 deletions(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 074767f35920c..e205e272db3ce 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -174,21 +174,22 @@ func NewProvisionerDaemon(t *testing.T, client *codersdk.Client) io.Closer { return closer } +var FirstUserParams = codersdk.CreateFirstUserRequest{ + Email: "testuser@coder.com", + Username: "testuser", + Password: "testpass", + OrganizationName: "testorg", +} + // CreateFirstUser creates a user with preset credentials and authenticates // with the passed in codersdk client. func CreateFirstUser(t *testing.T, client *codersdk.Client) codersdk.CreateFirstUserResponse { - req := codersdk.CreateFirstUserRequest{ - Email: "testuser@coder.com", - Username: "testuser", - Password: "testpass", - OrganizationName: "testorg", - } - resp, err := client.CreateFirstUser(context.Background(), req) + resp, err := client.CreateFirstUser(context.Background(), FirstUserParams) require.NoError(t, err) login, err := client.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{ - Email: req.Email, - Password: req.Password, + Email: FirstUserParams.Email, + Password: FirstUserParams.Password, }) require.NoError(t, err) client.SessionToken = login.SessionToken diff --git a/coderd/users.go b/coderd/users.go index a56602d28ed89..7412ef9f9f475 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -363,7 +363,7 @@ func (api *api) putUserSuspend(rw http.ResponseWriter, r *http.Request) { func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { user := httpmw.UserParam(r) - var params codersdk.UpdateUserHashedPasswordRequest + var params codersdk.UpdateUserPasswordRequest if !httpapi.Read(rw, r, ¶ms) { return } @@ -406,11 +406,12 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { return } databaseError := api.Database.UpdateUserHashedPassword(r.Context(), database.UpdateUserHashedPasswordParams{ + ID: user.ID, HashedPassword: []byte(hashedPassword), }) if databaseError != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("put user password: %s", err.Error()), + Message: fmt.Sprintf("put user password: %s", databaseError.Error()), }) return } @@ -635,7 +636,6 @@ func (api *api) postLogin(rw http.ResponseWriter, r *http.Request) { } // If the user doesn't exist, it will be a default struct. - equal, err := userpassword.Compare(string(user.HashedPassword), loginWithPassword.Password) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ diff --git a/coderd/users_test.go b/coderd/users_test.go index 531ac93be7aa6..e98d0417f942a 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -287,6 +287,57 @@ func TestUpdateUserProfile(t *testing.T) { }) } +func TestUpdateUserPassword(t *testing.T) { + t.Parallel() + + t.Run("WrongPassword", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + coderdtest.CreateFirstUser(t, client) + err := client.UpdateUserPassword(context.Background(), codersdk.Me, codersdk.UpdateUserPasswordRequest{ + Password: "wrongpassword", + NewPassword: "newpassword", + ConfirmNewPassword: "newpassword", + }) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) + }) + + t.Run("DifferentPasswordConfirmation", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + coderdtest.CreateFirstUser(t, client) + err := client.UpdateUserPassword(context.Background(), codersdk.Me, codersdk.UpdateUserPasswordRequest{ + Password: coderdtest.FirstUserParams.Password, + NewPassword: "newpassword", + ConfirmNewPassword: "wrongconfirmation", + }) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + }) + + t.Run("Success", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + coderdtest.CreateFirstUser(t, client) + err := client.UpdateUserPassword(context.Background(), codersdk.Me, codersdk.UpdateUserPasswordRequest{ + Password: coderdtest.FirstUserParams.Password, + NewPassword: "newpassword", + ConfirmNewPassword: "newpassword", + }) + require.NoError(t, err, "update password request should be successful") + + // Check if the user can login using the new password + _, err = client.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{ + Email: coderdtest.FirstUserParams.Email, + Password: "newpassword", + }) + require.NoError(t, err, "login should be successful") + }) +} + func TestGrantRoles(t *testing.T) { t.Parallel() t.Run("UpdateIncorrectRoles", func(t *testing.T) { diff --git a/codersdk/users.go b/codersdk/users.go index d7b0566755600..42e933add9286 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -72,7 +72,7 @@ type UpdateUserProfileRequest struct { Username string `json:"username" validate:"required,username"` } -type UpdateUserHashedPasswordRequest struct { +type UpdateUserPasswordRequest struct { Password string `json:"password" validate:"required"` NewPassword string `json:"new_password" validate:"required"` ConfirmNewPassword string `json:"confirm_new_password" validate:"required"` @@ -187,6 +187,19 @@ func (c *Client) SuspendUser(ctx context.Context, userID uuid.UUID) (User, error return user, json.NewDecoder(res.Body).Decode(&user) } +// Update user password +func (c *Client) UpdateUserPassword(ctx context.Context, userID uuid.UUID, req UpdateUserPasswordRequest) error { + res, err := c.request(ctx, http.MethodPut, fmt.Sprintf("/api/v2/users/%s/password", uuidOrMe(userID)), req) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return readBodyAsError(res) + } + return nil +} + // UpdateUserRoles grants the userID the specified roles. // Include ALL roles the user has. func (c *Client) UpdateUserRoles(ctx context.Context, userID uuid.UUID, req UpdateRoles) (User, error) { From 2699445fac340dbd72b3ea4aa5d0dd8f69c6a196 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 14:25:34 +0000 Subject: [PATCH 05/15] Remove confirmation and fix lint issues --- coderd/users.go | 32 ++++++++------------------------ coderd/users_test.go | 14 -------------- 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 7412ef9f9f475..0fb107f8a4c7c 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -360,33 +360,17 @@ func (api *api) putUserSuspend(rw http.ResponseWriter, r *http.Request) { httpapi.Write(rw, http.StatusOK, convertUser(suspendedUser, organizations)) } -func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { +func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) error { user := httpmw.UserParam(r) var params codersdk.UpdateUserPasswordRequest if !httpapi.Read(rw, r, ¶ms) { - return - } - - // Check if the password is correct - equal, err := userpassword.Compare(string(user.HashedPassword), params.Password) - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("compare: %s", err.Error()), - }) - } - if !equal { - // 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", - }) - return + return nil } // Check if the new password and the confirmation match if params.NewPassword != params.ConfirmNewPassword { - errors := []httpapi.Error{ + requestErrors := []httpapi.Error{ { Field: "confirm_new_password", Detail: "The value does not match the new password", @@ -394,16 +378,15 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { } httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ Message: fmt.Sprintf("The new password and the new password confirmation don't match"), - Errors: errors, + Errors: requestErrors, }) - return + return nil } // Hash password and update it in the database hashedPassword, hashError := userpassword.Hash(params.NewPassword) if hashError != nil { - xerrors.Errorf("hash password: %w", hashError) - return + return xerrors.Errorf("hash password: %w", hashError) } databaseError := api.Database.UpdateUserHashedPassword(r.Context(), database.UpdateUserHashedPasswordParams{ ID: user.ID, @@ -413,10 +396,11 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("put user password: %s", databaseError.Error()), }) - return + return nil } httpapi.Write(rw, http.StatusNoContent, nil) + return nil } func (api *api) userRoles(rw http.ResponseWriter, r *http.Request) { diff --git a/coderd/users_test.go b/coderd/users_test.go index e98d0417f942a..a326d4b593f35 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -290,20 +290,6 @@ func TestUpdateUserProfile(t *testing.T) { func TestUpdateUserPassword(t *testing.T) { t.Parallel() - t.Run("WrongPassword", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, nil) - coderdtest.CreateFirstUser(t, client) - err := client.UpdateUserPassword(context.Background(), codersdk.Me, codersdk.UpdateUserPasswordRequest{ - Password: "wrongpassword", - NewPassword: "newpassword", - ConfirmNewPassword: "newpassword", - }) - var apiErr *codersdk.Error - require.ErrorAs(t, err, &apiErr) - require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) - }) - t.Run("DifferentPasswordConfirmation", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) From 355f1631928a87675275d58ed2b4656e70fdff57 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 14:30:46 +0000 Subject: [PATCH 06/15] Return hash error as server error --- coderd/users.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 0fb107f8a4c7c..7fe0226b686c9 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -360,12 +360,12 @@ func (api *api) putUserSuspend(rw http.ResponseWriter, r *http.Request) { httpapi.Write(rw, http.StatusOK, convertUser(suspendedUser, organizations)) } -func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) error { +func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { user := httpmw.UserParam(r) var params codersdk.UpdateUserPasswordRequest if !httpapi.Read(rw, r, ¶ms) { - return nil + return } // Check if the new password and the confirmation match @@ -380,13 +380,15 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) error { Message: fmt.Sprintf("The new password and the new password confirmation don't match"), Errors: requestErrors, }) - return nil + return } // Hash password and update it in the database hashedPassword, hashError := userpassword.Hash(params.NewPassword) if hashError != nil { - return xerrors.Errorf("hash password: %w", hashError) + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: fmt.Sprintf("hash password: %s", hashError.Error()), + }) } databaseError := api.Database.UpdateUserHashedPassword(r.Context(), database.UpdateUserHashedPasswordParams{ ID: user.ID, @@ -396,11 +398,10 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) error { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("put user password: %s", databaseError.Error()), }) - return nil + return } httpapi.Write(rw, http.StatusNoContent, nil) - return nil } func (api *api) userRoles(rw http.ResponseWriter, r *http.Request) { From 56b29fd821a0afff85b667a38b94c5e3b2fef51c Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 5 May 2022 12:07:36 -0500 Subject: [PATCH 07/15] Update coderd/database/databasefake/databasefake.go Co-authored-by: Mathias Fredriksson --- coderd/database/databasefake/databasefake.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index f33adf922f100..a9c7dea5fdbd5 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1318,12 +1318,12 @@ func (q *fakeQuerier) UpdateUserHashedPassword(_ context.Context, arg database.U q.mutex.Lock() defer q.mutex.Unlock() - for index, user := range q.users { + for i, user := range q.users { if user.ID != arg.ID { continue } user.HashedPassword = arg.HashedPassword - q.users[index] = user + q.users[i] = user return nil } return sql.ErrNoRows From 30b8f15172b50e6b80b422c68c1acf1501eeb0bc Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 17:12:18 +0000 Subject: [PATCH 08/15] Improve readbility --- coderd/database/queries/users.sql | 2 +- coderd/users.go | 13 +++++++------ codersdk/users.go | 3 ++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 35ade0e9762a4..d0c3d4c3e7b66 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -70,7 +70,7 @@ WHERE -- name: GetUsers :many SELECT * -FROM +FROM users WHERE CASE diff --git a/coderd/users.go b/coderd/users.go index 7fe0226b686c9..269a08f9b1214 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -384,19 +384,20 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { } // Hash password and update it in the database - hashedPassword, hashError := userpassword.Hash(params.NewPassword) - if hashError != nil { + hashedPassword, err := userpassword.Hash(params.NewPassword) + if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("hash password: %s", hashError.Error()), + Message: fmt.Sprintf("hash password: %s", err.Error()), }) + return } - databaseError := api.Database.UpdateUserHashedPassword(r.Context(), database.UpdateUserHashedPasswordParams{ + err = api.Database.UpdateUserHashedPassword(r.Context(), database.UpdateUserHashedPasswordParams{ ID: user.ID, HashedPassword: []byte(hashedPassword), }) - if databaseError != nil { + if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("put user password: %s", databaseError.Error()), + Message: fmt.Sprintf("put user password: %s", err.Error()), }) return } diff --git a/codersdk/users.go b/codersdk/users.go index 42e933add9286..baea5484225a5 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -187,7 +187,8 @@ func (c *Client) SuspendUser(ctx context.Context, userID uuid.UUID) (User, error return user, json.NewDecoder(res.Body).Decode(&user) } -// Update user password +// UpdateUserPassword updates a user password. +// It calls PUT /users/{user}/password func (c *Client) UpdateUserPassword(ctx context.Context, userID uuid.UUID, req UpdateUserPasswordRequest) error { res, err := c.request(ctx, http.MethodPut, fmt.Sprintf("/api/v2/users/%s/password", uuidOrMe(userID)), req) if err != nil { From f6be255e05f3d45af6ade198ca82a99757c67643 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 17:14:34 +0000 Subject: [PATCH 09/15] Add RBAC --- coderd/coderd.go | 5 ++++- coderd/rbac/object.go | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index a1d028c7295f7..8e6e727cac2db 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -240,7 +240,10 @@ func New(options *Options) (http.Handler, func()) { r.Get("/", api.userByName) r.Put("/profile", api.putUserProfile) r.Put("/suspend", api.putUserSuspend) - r.Put("/password", api.putUserPassword) + r.Route("/password", func(r chi.Router) { + r.Use(httpmw.WithRBACObject(rbac.ResourceUserPasswordRole)) + r.Put("/password", authorize(api.putUserPassword, rbac.ActionUpdate)) + }) r.Get("/organizations", api.organizationsByUser) r.Post("/organizations", api.postOrganizationsByUser) // These roles apply to the site wide permissions. diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index a4be9b1edab5b..dd12efc25fcb2 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -24,6 +24,10 @@ var ( Type: "user_role", } + ResourceUserPasswordRole = Object{ + Type: "user_password", + } + // ResourceWildcard represents all resource types ResourceWildcard = Object{ Type: WildcardSymbol, From 5df5763204f707c02e7efb522ae21fd4c6c15559 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 17:15:20 +0000 Subject: [PATCH 10/15] Fix route --- coderd/coderd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 8e6e727cac2db..aa5cdbb21b86c 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -242,7 +242,7 @@ func New(options *Options) (http.Handler, func()) { r.Put("/suspend", api.putUserSuspend) r.Route("/password", func(r chi.Router) { r.Use(httpmw.WithRBACObject(rbac.ResourceUserPasswordRole)) - r.Put("/password", authorize(api.putUserPassword, rbac.ActionUpdate)) + r.Put("/", authorize(api.putUserPassword, rbac.ActionUpdate)) }) r.Get("/organizations", api.organizationsByUser) r.Post("/organizations", api.postOrganizationsByUser) From 69af903ca89098d763cfce38d805959bef38a69e Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 17:17:35 +0000 Subject: [PATCH 11/15] Add missing TS types --- site/src/api/typesGenerated.ts | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index ee4add928bb11..acef091100952 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -12,7 +12,7 @@ export interface AgentGitSSHKey { readonly private_key: string } -// From codersdk/users.go:105:6 +// From codersdk/users.go:111:6 export interface AuthMethods { readonly password: boolean readonly github: boolean @@ -44,7 +44,7 @@ export interface CreateFirstUserResponse { readonly organization_id: string } -// From codersdk/users.go:100:6 +// From codersdk/users.go:106:6 export interface CreateOrganizationRequest { readonly name: string } @@ -101,7 +101,7 @@ export interface CreateWorkspaceRequest { readonly parameter_values: CreateParameterRequest[] } -// From codersdk/users.go:96:6 +// From codersdk/users.go:102:6 export interface GenerateAPIKeyResponse { readonly key: string } @@ -119,13 +119,13 @@ export interface GoogleInstanceIdentityToken { readonly json_web_token: string } -// From codersdk/users.go:85:6 +// From codersdk/users.go:91:6 export interface LoginWithPasswordRequest { readonly email: string readonly password: string } -// From codersdk/users.go:91:6 +// From codersdk/users.go:97:6 export interface LoginWithPasswordResponse { readonly session_token: string } @@ -255,11 +255,18 @@ export interface UpdateActiveTemplateVersion { readonly id: string } -// From codersdk/users.go:75:6 +// From codersdk/users.go:81:6 export interface UpdateRoles { readonly roles: string[] } +// From codersdk/users.go:75:6 +export interface UpdateUserPasswordRequest { + readonly password: string + readonly new_password: string + readonly confirm_new_password: string +} + // From codersdk/users.go:70:6 export interface UpdateUserProfileRequest { readonly email: string @@ -291,7 +298,7 @@ export interface User { readonly organization_ids: string[] } -// From codersdk/users.go:79:6 +// From codersdk/users.go:85:6 export interface UserRoles { readonly roles: string[] readonly organization_roles: Record From d85092b86fb60db5bae2d52a97370fb64ed9fde5 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 18:17:21 +0000 Subject: [PATCH 12/15] Update update password request params --- coderd/users.go | 4 ++-- coderd/users_test.go | 10 ++++------ codersdk/users.go | 5 ++--- site/src/api/typesGenerated.ts | 15 +++++++-------- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 269a08f9b1214..5be762a820855 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -369,7 +369,7 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { } // Check if the new password and the confirmation match - if params.NewPassword != params.ConfirmNewPassword { + if params.Password != params.ConfirmPassword { requestErrors := []httpapi.Error{ { Field: "confirm_new_password", @@ -384,7 +384,7 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) { } // Hash password and update it in the database - hashedPassword, err := userpassword.Hash(params.NewPassword) + hashedPassword, err := userpassword.Hash(params.Password) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("hash password: %s", err.Error()), diff --git a/coderd/users_test.go b/coderd/users_test.go index a326d4b593f35..d20be1c9c4bac 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -295,9 +295,8 @@ func TestUpdateUserPassword(t *testing.T) { client := coderdtest.New(t, nil) coderdtest.CreateFirstUser(t, client) err := client.UpdateUserPassword(context.Background(), codersdk.Me, codersdk.UpdateUserPasswordRequest{ - Password: coderdtest.FirstUserParams.Password, - NewPassword: "newpassword", - ConfirmNewPassword: "wrongconfirmation", + Password: "newpassword", + ConfirmPassword: "wrongconfirmation", }) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) @@ -309,9 +308,8 @@ func TestUpdateUserPassword(t *testing.T) { client := coderdtest.New(t, nil) coderdtest.CreateFirstUser(t, client) err := client.UpdateUserPassword(context.Background(), codersdk.Me, codersdk.UpdateUserPasswordRequest{ - Password: coderdtest.FirstUserParams.Password, - NewPassword: "newpassword", - ConfirmNewPassword: "newpassword", + Password: "newpassword", + ConfirmPassword: "newpassword", }) require.NoError(t, err, "update password request should be successful") diff --git a/codersdk/users.go b/codersdk/users.go index baea5484225a5..c1f17fa7ad128 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -73,9 +73,8 @@ type UpdateUserProfileRequest struct { } type UpdateUserPasswordRequest struct { - Password string `json:"password" validate:"required"` - NewPassword string `json:"new_password" validate:"required"` - ConfirmNewPassword string `json:"confirm_new_password" validate:"required"` + Password string `json:"password" validate:"required"` + ConfirmPassword string `json:"confirm_new_password" validate:"required"` } type UpdateRoles struct { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index acef091100952..de81c5f25542e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -12,7 +12,7 @@ export interface AgentGitSSHKey { readonly private_key: string } -// From codersdk/users.go:111:6 +// From codersdk/users.go:110:6 export interface AuthMethods { readonly password: boolean readonly github: boolean @@ -44,7 +44,7 @@ export interface CreateFirstUserResponse { readonly organization_id: string } -// From codersdk/users.go:106:6 +// From codersdk/users.go:105:6 export interface CreateOrganizationRequest { readonly name: string } @@ -101,7 +101,7 @@ export interface CreateWorkspaceRequest { readonly parameter_values: CreateParameterRequest[] } -// From codersdk/users.go:102:6 +// From codersdk/users.go:101:6 export interface GenerateAPIKeyResponse { readonly key: string } @@ -119,13 +119,13 @@ export interface GoogleInstanceIdentityToken { readonly json_web_token: string } -// From codersdk/users.go:91:6 +// From codersdk/users.go:90:6 export interface LoginWithPasswordRequest { readonly email: string readonly password: string } -// From codersdk/users.go:97:6 +// From codersdk/users.go:96:6 export interface LoginWithPasswordResponse { readonly session_token: string } @@ -255,7 +255,7 @@ export interface UpdateActiveTemplateVersion { readonly id: string } -// From codersdk/users.go:81:6 +// From codersdk/users.go:80:6 export interface UpdateRoles { readonly roles: string[] } @@ -263,7 +263,6 @@ export interface UpdateRoles { // From codersdk/users.go:75:6 export interface UpdateUserPasswordRequest { readonly password: string - readonly new_password: string readonly confirm_new_password: string } @@ -298,7 +297,7 @@ export interface User { readonly organization_ids: string[] } -// From codersdk/users.go:85:6 +// From codersdk/users.go:84:6 export interface UserRoles { readonly roles: string[] readonly organization_roles: Record From d35139c4dbe26e8b18e181df77353731701ef97b Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 5 May 2022 19:53:43 +0000 Subject: [PATCH 13/15] Add FE for reset user password --- site/src/api/index.ts | 3 + site/src/components/CodeBlock/CodeBlock.tsx | 6 +- .../ResetPasswordDialog.tsx | 54 +++++++++++++ site/src/components/UsersTable/UsersTable.tsx | 8 +- site/src/pages/UsersPage/UsersPage.tsx | 19 ++++- site/src/pages/UsersPage/UsersPageView.tsx | 8 +- site/src/util/random.ts | 19 +++++ site/src/xServices/users/usersXService.ts | 75 ++++++++++++++++++- 8 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx create mode 100644 site/src/util/random.ts diff --git a/site/src/api/index.ts b/site/src/api/index.ts index 2c42d96a2e0d3..104f9ec167746 100644 --- a/site/src/api/index.ts +++ b/site/src/api/index.ts @@ -140,3 +140,6 @@ export const suspendUser = async (userId: TypesGen.User["id"]): Promise(`/api/v2/users/${userId}/suspend`) return response.data } + +export const updateUserPassword = async (password: string, userId: TypesGen.User["id"]): Promise => + axios.put(`/api/v2/users/${userId}/password`, { password }) diff --git a/site/src/components/CodeBlock/CodeBlock.tsx b/site/src/components/CodeBlock/CodeBlock.tsx index e32468f705afc..a3dff970f499c 100644 --- a/site/src/components/CodeBlock/CodeBlock.tsx +++ b/site/src/components/CodeBlock/CodeBlock.tsx @@ -1,16 +1,18 @@ import { makeStyles } from "@material-ui/core/styles" import React from "react" import { MONOSPACE_FONT_FAMILY } from "../../theme/constants" +import { combineClasses } from "../../util/combineClasses" export interface CodeBlockProps { lines: string[] + className?: string } -export const CodeBlock: React.FC = ({ lines }) => { +export const CodeBlock: React.FC = ({ lines, className = "" }) => { const styles = useStyles() return ( -
+
{lines.map((line, idx) => (
{line} diff --git a/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx b/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx new file mode 100644 index 0000000000000..3ad2d4f192ec8 --- /dev/null +++ b/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx @@ -0,0 +1,54 @@ +import DialogActions from "@material-ui/core/DialogActions" +import DialogContent from "@material-ui/core/DialogContent" +import DialogContentText from "@material-ui/core/DialogContentText" +import { makeStyles } from "@material-ui/core/styles" +import React from "react" +import * as TypesGen from "../../api/typesGenerated" +import { CodeBlock } from "../CodeBlock/CodeBlock" +import { Dialog, DialogActionButtons, DialogTitle } from "../Dialog/Dialog" + +interface ResetPasswordDialogProps { + open: boolean + onClose: () => void + onConfirm: () => void + user?: TypesGen.User + newPassword?: string +} + +export const ResetPasswordDialog: React.FC = ({ + open, + onClose, + onConfirm, + user, + newPassword, +}) => { + const styles = useStyles() + + return ( + + + + + + You will need to send {user?.username} the following password: + + + + + + + + + + + + ) +} + +const useStyles = makeStyles(() => ({ + codeBlock: { + minHeight: "auto", + userSelect: "all", + width: "100%", + }, +})) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 952c86c5b3a0b..bf5fb2298dd10 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -11,6 +11,7 @@ export const Language = { emptyMessage: "No users found", usernameLabel: "User", suspendMenuItem: "Suspend", + resetPasswordMenuItem: "Reset password", } const emptyState = @@ -28,9 +29,10 @@ const columns: Column[] = [ export interface UsersTableProps { users: UserResponse[] onSuspendUser: (user: UserResponse) => void + onResetUserPassword: (user: UserResponse) => void } -export const UsersTable: React.FC = ({ users, onSuspendUser }) => { +export const UsersTable: React.FC = ({ users, onSuspendUser, onResetUserPassword }) => { return ( = ({ users, onSuspendUser }) label: Language.suspendMenuItem, onClick: onSuspendUser, }, + { + label: Language.resetPasswordMenuItem, + onClick: onResetUserPassword, + }, ]} /> )} diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 28332235604b3..3efc9304c55e7 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -3,6 +3,7 @@ import React, { useContext, useEffect } from "react" import { useNavigate } from "react-router" import { ConfirmDialog } from "../../components/ConfirmDialog/ConfirmDialog" import { FullScreenLoader } from "../../components/Loader/FullScreenLoader" +import { ResetPasswordDialog } from "../../components/ResetPasswordDialog/ResetPasswordDialog" import { XServiceContext } from "../../xServices/StateContext" import { UsersPageView } from "./UsersPageView" @@ -15,9 +16,10 @@ export const Language = { export const UsersPage: React.FC = () => { const xServices = useContext(XServiceContext) const [usersState, usersSend] = useActor(xServices.usersXService) - const { users, getUsersError, userIdToSuspend } = usersState.context + const { users, getUsersError, userIdToSuspend, userIdToResetPassword, newUserPassword } = usersState.context const navigate = useNavigate() const userToBeSuspended = users?.find((u) => u.id === userIdToSuspend) + const userToResetPassword = users?.find((u) => u.id === userIdToResetPassword) /** * Fetch users on component mount @@ -39,6 +41,9 @@ export const UsersPage: React.FC = () => { onSuspendUser={(user) => { usersSend({ type: "SUSPEND_USER", userId: user.id }) }} + onResetUserPassword={(user) => { + usersSend({ type: "RESET_USER_PASSWORD", userId: user.id }) + }} error={getUsersError} /> @@ -61,6 +66,18 @@ export const UsersPage: React.FC = () => { } /> + + { + usersSend("CANCEL_USER_PASSWORD_RESET") + }} + onConfirm={() => { + usersSend("CONFIRM_USER_PASSWORD_RESET") + }} + /> ) } diff --git a/site/src/pages/UsersPage/UsersPageView.tsx b/site/src/pages/UsersPage/UsersPageView.tsx index 855f15a1d7e18..4872e02d76085 100644 --- a/site/src/pages/UsersPage/UsersPageView.tsx +++ b/site/src/pages/UsersPage/UsersPageView.tsx @@ -15,6 +15,7 @@ export interface UsersPageViewProps { users: UserResponse[] openUserCreationDialog: () => void onSuspendUser: (user: UserResponse) => void + onResetUserPassword: (user: UserResponse) => void error?: unknown } @@ -22,13 +23,18 @@ export const UsersPageView: React.FC = ({ users, openUserCreationDialog, onSuspendUser, + onResetUserPassword, error, }) => { return (
- {error ? : } + {error ? ( + + ) : ( + + )} ) diff --git a/site/src/util/random.ts b/site/src/util/random.ts new file mode 100644 index 0000000000000..e4d51da67b360 --- /dev/null +++ b/site/src/util/random.ts @@ -0,0 +1,19 @@ +/** + * Generate a cryptographically secure random string using the specified number + * of bytes then encode with base64. + * + * Base64 encodes 6 bits per character and pads with = so the length will not + * equal the number of randomly generated bytes. + * @see + */ +export const generateRandomString = (bytes: number): string => { + const byteArr = window.crypto.getRandomValues(new Uint8Array(bytes)) + // The types for `map` don't seem to support mapping from one array type to + // another and `String.fromCharCode.apply` wants `number[]` so loop like this + // instead. + const strArr: string[] = [] + for (const byte of byteArr) { + strArr.push(String.fromCharCode(byte)) + } + return btoa(strArr.join("")) +} diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index b08aad97ff21a..eb41dfd43bce7 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -4,28 +4,42 @@ import { ApiError, FieldErrors, isApiError, mapApiErrorToFieldErrors } from "../ import * as Types from "../../api/types" import * as TypesGen from "../../api/typesGenerated" import { displayError, displaySuccess } from "../../components/GlobalSnackbar/utils" +import { generateRandomString } from "../../util/random" export const Language = { createUserSuccess: "Successfully created user.", suspendUserSuccess: "Successfully suspended the user.", - suspendUserError: "Error on suspend the user", + suspendUserError: "Error on suspend the user.", + resetUserPasswordSuccess: "Successfully updated the user password.", + resetUserPasswordError: "Error on reset the user password.", } export interface UsersContext { + // Get users users?: TypesGen.User[] - userIdToSuspend?: TypesGen.User["id"] getUsersError?: Error | unknown createUserError?: Error | unknown createUserFormErrors?: FieldErrors + // Suspend user + userIdToSuspend?: TypesGen.User["id"] suspendUserError?: Error | unknown + // Reset user password + userIdToResetPassword?: TypesGen.User["id"] + resetUserPasswordError?: Error | unknown + newUserPassword?: string } export type UsersEvent = | { type: "GET_USERS" } | { type: "CREATE"; user: Types.CreateUserRequest } + // Suspend events | { type: "SUSPEND_USER"; userId: TypesGen.User["id"] } | { type: "CONFIRM_USER_SUSPENSION" } | { type: "CANCEL_USER_SUSPENSION" } + // Reset password events + | { type: "RESET_USER_PASSWORD"; userId: TypesGen.User["id"] } + | { type: "CONFIRM_USER_PASSWORD_RESET" } + | { type: "CANCEL_USER_PASSWORD_RESET" } export const usersMachine = createMachine( { @@ -43,6 +57,9 @@ export const usersMachine = createMachine( suspendUser: { data: TypesGen.User } + updateUserPassword: { + data: undefined + } }, }, id: "usersState", @@ -59,6 +76,10 @@ export const usersMachine = createMachine( target: "confirmUserSuspension", actions: ["assignUserIdToSuspend"], }, + RESET_USER_PASSWORD: { + target: "confirmUserPasswordReset", + actions: ["assignUserIdToResetPassword", "generateRandomPassword"], + }, }, }, gettingUsers: { @@ -124,6 +145,27 @@ export const usersMachine = createMachine( }, }, }, + confirmUserPasswordReset: { + on: { + CONFIRM_USER_PASSWORD_RESET: "resettingUserPassword", + CANCEL_USER_PASSWORD_RESET: "idle", + }, + }, + resettingUserPassword: { + entry: "clearResetUserPasswordError", + invoke: { + src: "resetUserPassword", + id: "resetUserPassword", + onDone: { + target: "idle", + actions: ["displayResetPasswordSuccess"], + }, + onError: { + target: "idle", + actions: ["assignResetUserPasswordError", "displayResetPasswordErrorMessage"], + }, + }, + }, error: { on: { GET_USERS: "gettingUsers", @@ -145,6 +187,17 @@ export const usersMachine = createMachine( return API.suspendUser(context.userIdToSuspend) }, + resetUserPassword: (context) => { + if (!context.userIdToResetPassword) { + throw new Error("userIdToResetPassword is undefined") + } + + if (!context.newUserPassword) { + throw new Error("newUserPassword not generated") + } + + return API.updateUserPassword(context.newUserPassword, context.userIdToResetPassword) + }, }, guards: { isFormError: (_, event) => isApiError(event.data), @@ -159,6 +212,9 @@ export const usersMachine = createMachine( assignUserIdToSuspend: assign({ userIdToSuspend: (_, event) => event.userId, }), + assignUserIdToResetPassword: assign({ + userIdToResetPassword: (_, event) => event.userId, + }), clearGetUsersError: assign((context: UsersContext) => ({ ...context, getUsersError: undefined, @@ -173,6 +229,9 @@ export const usersMachine = createMachine( assignSuspendUserError: assign({ suspendUserError: (_, event) => event.data, }), + assignResetUserPasswordError: assign({ + resetUserPasswordError: (_, event) => event.data, + }), clearCreateUserError: assign((context: UsersContext) => ({ ...context, createUserError: undefined, @@ -180,6 +239,9 @@ export const usersMachine = createMachine( clearSuspendUserError: assign({ suspendUserError: (_) => undefined, }), + clearResetUserPasswordError: assign({ + resetUserPasswordError: (_) => undefined, + }), displayCreateUserSuccess: () => { displaySuccess(Language.createUserSuccess) }, @@ -189,6 +251,15 @@ export const usersMachine = createMachine( displaySuspendedErrorMessage: () => { displayError(Language.suspendUserError) }, + displayResetPasswordSuccess: () => { + displaySuccess(Language.resetUserPasswordSuccess) + }, + displayResetPasswordErrorMessage: () => { + displaySuccess(Language.resetUserPasswordError) + }, + generateRandomPassword: assign({ + newUserPassword: (_) => generateRandomString(12), + }), }, }, ) From 89573399df28356dd82d4126e67c80f0cb25593c Mon Sep 17 00:00:00 2001 From: Bruno Date: Fri, 6 May 2022 14:28:33 +0000 Subject: [PATCH 14/15] Add reset password dialog to storybook --- .../ResetPasswordDialog.stories.tsx | 23 +++++++++++++++++++ .../ResetPasswordDialog.tsx | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 site/src/components/ResetPasswordDialog/ResetPasswordDialog.stories.tsx diff --git a/site/src/components/ResetPasswordDialog/ResetPasswordDialog.stories.tsx b/site/src/components/ResetPasswordDialog/ResetPasswordDialog.stories.tsx new file mode 100644 index 0000000000000..8a0c1f19a6c46 --- /dev/null +++ b/site/src/components/ResetPasswordDialog/ResetPasswordDialog.stories.tsx @@ -0,0 +1,23 @@ +import { Story } from "@storybook/react" +import React from "react" +import { MockUser } from "../../testHelpers" +import { generateRandomString } from "../../util/random" +import { ResetPasswordDialog, ResetPasswordDialogProps } from "./ResetPasswordDialog" + +export default { + title: "components/ResetPasswordDialog", + component: ResetPasswordDialog, + argTypes: { + onClose: { action: "onClose" }, + onConfirm: { action: "onConfirm" }, + }, +} + +const Template: Story = (args: ResetPasswordDialogProps) => + +export const Example = Template.bind({}) +Example.args = { + open: true, + user: MockUser, + newPassword: generateRandomString(12), +} diff --git a/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx b/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx index 3ad2d4f192ec8..ac47593fbb4e0 100644 --- a/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx +++ b/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx @@ -7,7 +7,7 @@ import * as TypesGen from "../../api/typesGenerated" import { CodeBlock } from "../CodeBlock/CodeBlock" import { Dialog, DialogActionButtons, DialogTitle } from "../Dialog/Dialog" -interface ResetPasswordDialogProps { +export interface ResetPasswordDialogProps { open: boolean onClose: () => void onConfirm: () => void From e388f8a057cab9d943a5e298a2bec29b0d170b4d Mon Sep 17 00:00:00 2001 From: Bruno Date: Fri, 6 May 2022 15:06:13 +0000 Subject: [PATCH 15/15] Add tests --- site/jest.setup.ts | 10 +++ .../ResetPasswordDialog.tsx | 25 ++++-- site/src/pages/UsersPage/UsersPage.test.tsx | 76 ++++++++++++++++++- site/src/pages/UsersPage/UsersPage.tsx | 1 + site/src/xServices/users/usersXService.ts | 2 +- 5 files changed, 107 insertions(+), 7 deletions(-) diff --git a/site/jest.setup.ts b/site/jest.setup.ts index 89b3bde7688e9..52df1ac6ed8f8 100644 --- a/site/jest.setup.ts +++ b/site/jest.setup.ts @@ -1,6 +1,16 @@ import "@testing-library/jest-dom" +import crypto from "crypto" import { server } from "./src/testHelpers/server" +// Polyfill the getRandomValues that is used on utils/random.ts +Object.defineProperty(global.self, "crypto", { + value: { + getRandomValues: function (buffer: Buffer) { + return crypto.randomFillSync(buffer) + }, + }, +}) + // Establish API mocking before all tests through MSW. beforeAll(() => server.listen({ diff --git a/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx b/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx index ac47593fbb4e0..472e233688f27 100644 --- a/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx +++ b/site/src/components/ResetPasswordDialog/ResetPasswordDialog.tsx @@ -13,6 +13,17 @@ export interface ResetPasswordDialogProps { onConfirm: () => void user?: TypesGen.User newPassword?: string + loading: boolean +} + +export const Language = { + title: "Reset password", + message: (username?: string): JSX.Element => ( + <> + You will need to send {username} the following password: + + ), + confirmText: "Reset password", } export const ResetPasswordDialog: React.FC = ({ @@ -21,17 +32,16 @@ export const ResetPasswordDialog: React.FC = ({ onConfirm, user, newPassword, + loading, }) => { const styles = useStyles() return ( - + - - You will need to send {user?.username} the following password: - + {Language.message(user?.username)} @@ -39,7 +49,12 @@ export const ResetPasswordDialog: React.FC = ({ - + ) diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index f7efb64cea22b..2aeb7f21ba31f 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -2,6 +2,7 @@ import { fireEvent, screen, waitFor, within } from "@testing-library/react" import React from "react" import * as API from "../../api" import { GlobalSnackbar } from "../../components/GlobalSnackbar/GlobalSnackbar" +import { Language as ResetPasswordDialogLanguage } from "../../components/ResetPasswordDialog/ResetPasswordDialog" import { Language as UsersTableLanguage } from "../../components/UsersTable/UsersTable" import { MockUser, MockUser2, render } from "../../testHelpers" import { Language as usersXServiceLanguage } from "../../xServices/users/usersXService" @@ -34,6 +35,33 @@ const suspendUser = async (setupActionSpies: () => void) => { fireEvent.click(confirmButton) } +const resetUserPassword = async (setupActionSpies: () => void) => { + // Get the first user in the table + const users = await screen.findAllByText(/.*@coder.com/) + const firstUserRow = users[0].closest("tr") + if (!firstUserRow) { + throw new Error("Error on get the first user row") + } + + // Click on the "more" button to display the "Suspend" option + const moreButton = within(firstUserRow).getByLabelText("more") + fireEvent.click(moreButton) + const menu = screen.getByRole("menu") + const resetPasswordButton = within(menu).getByText(UsersTableLanguage.resetPasswordMenuItem) + fireEvent.click(resetPasswordButton) + + // Check if the confirm message is displayed + const confirmDialog = screen.getByRole("dialog") + expect(confirmDialog).toHaveTextContent(`You will need to send ${MockUser.username} the following password:`) + + // Setup spies to check the actions after + setupActionSpies() + + // Click on the "Confirm" button + const confirmButton = within(confirmDialog).getByRole("button", { name: ResetPasswordDialogLanguage.confirmText }) + fireEvent.click(confirmButton) +} + describe("Users Page", () => { it("shows users", async () => { render() @@ -81,7 +109,7 @@ describe("Users Page", () => { jest.spyOn(API, "suspendUser").mockRejectedValueOnce({}) }) - // Check if the success message is displayed + // Check if the error message is displayed await screen.findByText(usersXServiceLanguage.suspendUserError) // Check if the API was called correctly @@ -90,4 +118,50 @@ describe("Users Page", () => { }) }) }) + + describe("reset user password", () => { + describe("when it is success", () => { + it("shows a success message", async () => { + render( + <> + + + , + ) + + await resetUserPassword(() => { + jest.spyOn(API, "updateUserPassword").mockResolvedValueOnce(undefined) + }) + + // Check if the success message is displayed + await screen.findByText(usersXServiceLanguage.resetUserPasswordSuccess) + + // Check if the API was called correctly + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(expect.any(String), MockUser.id) + }) + }) + + describe("when it fails", () => { + it("shows an error message", async () => { + render( + <> + + + , + ) + + await resetUserPassword(() => { + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce({}) + }) + + // Check if the error message is displayed + await screen.findByText(usersXServiceLanguage.resetUserPasswordError) + + // Check if the API was called correctly + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(expect.any(String), MockUser.id) + }) + }) + }) }) diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 3efc9304c55e7..0ce09831728f7 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -68,6 +68,7 @@ export const UsersPage: React.FC = () => { /> { - displaySuccess(Language.resetUserPasswordError) + displayError(Language.resetUserPasswordError) }, generateRandomPassword: assign({ newUserPassword: (_) => generateRandomString(12),