fix: delete all sessions on password change (#4659)

- Prevent users from reusing their old password
  as their new password.

coderd/database/databasefake/databasefake.go

@@ -368,6 +368,19 @@ func (q *fakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error {
 	return sql.ErrNoRows
 }
 
+func (q *fakeQuerier) DeleteAPIKeysByUserID(_ context.Context, userID uuid.UUID) error {
+	q.mutex.Lock()
+	defer q.mutex.Unlock()
+
+	for i := len(q.apiKeys) - 1; i >= 0; i-- {
+		if q.apiKeys[i].UserID == userID {
+			q.apiKeys = append(q.apiKeys[:i], q.apiKeys[i+1:]...)
+		}
+	}
+
+	return nil
+}
+
 func (q *fakeQuerier) GetFileByHashAndCreator(_ context.Context, arg database.GetFileByHashAndCreatorParams) (database.File, error) {
 	q.mutex.RLock()
 	defer q.mutex.RUnlock()

coderd/database/querier.go

@@ -20,6 +20,7 @@ type sqlcQuerier interface {
 	// https://www.postgresql.org/docs/9.5/sql-select.html#SQL-FOR-UPDATE-SHARE
 	AcquireProvisionerJob(ctx context.Context, arg AcquireProvisionerJobParams) (ProvisionerJob, error)
 	DeleteAPIKeyByID(ctx context.Context, id string) error
+	DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error
 	DeleteGitSSHKey(ctx context.Context, userID uuid.UUID) error
 	DeleteGroupByID(ctx context.Context, id uuid.UUID) error
 	DeleteGroupMember(ctx context.Context, userID uuid.UUID) error

coderd/database/queries.sql.go

@@ -145,6 +145,18 @@ func (q *sqlQuerier) DeleteAPIKeyByID(ctx context.Context, id string) error {
 	return err
 }
 
+const deleteAPIKeysByUserID = `-- name: DeleteAPIKeysByUserID :exec
+DELETE FROM
+	api_keys
+WHERE
+	user_id = $1
+`
+
+func (q *sqlQuerier) DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error {
+	_, err := q.db.ExecContext(ctx, deleteAPIKeysByUserID, userID)
+	return err
+}
+
 const getAPIKeyByID = `-- name: GetAPIKeyByID :one
 SELECT
 	id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope

coderd/database/queries/apikeys.sql

@@ -54,3 +54,9 @@ FROM
 	api_keys
 WHERE
 	id = $1;
+
+-- name: DeleteAPIKeysByUserID :exec
+DELETE FROM
+	api_keys
+WHERE
+	user_id = $1;

coderd/users.go

@@ -632,6 +632,14 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// Prevent users reusing their old password.
+	if match, _ := userpassword.Compare(string(user.HashedPassword), params.Password); match {
+		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
+			Message: "New password cannot match old password.",
+		})
+		return
+	}
+
 	hashedPassword, err := userpassword.Hash(params.Password)
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -640,9 +648,22 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) {
 		})
 		return
 	}
-	err = api.Database.UpdateUserHashedPassword(ctx, database.UpdateUserHashedPasswordParams{
-		ID:             user.ID,
-		HashedPassword: []byte(hashedPassword),
+
+	err = api.Database.InTx(func(tx database.Store) error {
+		err = tx.UpdateUserHashedPassword(ctx, database.UpdateUserHashedPasswordParams{
+			ID:             user.ID,
+			HashedPassword: []byte(hashedPassword),
+		})
+		if err != nil {
+			return xerrors.Errorf("update user hashed password: %w", err)
+		}
+
+		err = tx.DeleteAPIKeysByUserID(ctx, user.ID)
+		if err != nil {
+			return xerrors.Errorf("delete api keys by user ID: %w", err)
+		}
+
+		return nil
 	})
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{

coderd/users_test.go

@@ -644,6 +644,67 @@ func TestUpdateUserPassword(t *testing.T) {
 		assert.Len(t, auditor.AuditLogs, 1)
 		assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs[0].Action)
 	})
+
+	t.Run("ChangingPasswordDeletesKeys", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		ctx, _ := testutil.Context(t)
+
+		apikey1, err := client.CreateToken(ctx, user.UserID.String(), codersdk.CreateTokenRequest{})
+		require.NoError(t, err)
+
+		apikey2, err := client.CreateToken(ctx, user.UserID.String(), codersdk.CreateTokenRequest{})
+		require.NoError(t, err)
+
+		err = client.UpdateUserPassword(ctx, "me", codersdk.UpdateUserPasswordRequest{
+			Password: "newpassword",
+		})
+		require.NoError(t, err)
+
+		// Trying to get an API key should fail since our client's token
+		// has been deleted.
+		_, err = client.GetAPIKey(ctx, user.UserID.String(), apikey1.Key)
+		require.Error(t, err)
+		cerr := coderdtest.SDKError(t, err)
+		require.Equal(t, http.StatusUnauthorized, cerr.StatusCode())
+
+		resp, err := client.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{
+			Email:    coderdtest.FirstUserParams.Email,
+			Password: "newpassword",
+		})
+		require.NoError(t, err)
+
+		client.SessionToken = resp.SessionToken
+
+		// Trying to get an API key should fail since all keys are deleted
+		// on password change.
+		_, err = client.GetAPIKey(ctx, user.UserID.String(), apikey1.Key)
+		require.Error(t, err)
+		cerr = coderdtest.SDKError(t, err)
+		require.Equal(t, http.StatusNotFound, cerr.StatusCode())
+
+		_, err = client.GetAPIKey(ctx, user.UserID.String(), apikey2.Key)
+		require.Error(t, err)
+		cerr = coderdtest.SDKError(t, err)
+		require.Equal(t, http.StatusNotFound, cerr.StatusCode())
+	})
+
+	t.Run("PasswordsMustDiffer", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		_ = coderdtest.CreateFirstUser(t, client)
+		ctx, _ := testutil.Context(t)
+
+		err := client.UpdateUserPassword(ctx, "me", codersdk.UpdateUserPasswordRequest{
+			Password: coderdtest.FirstUserParams.Password,
+		})
+		require.Error(t, err)
+		cerr := coderdtest.SDKError(t, err)
+		require.Equal(t, http.StatusBadRequest, cerr.StatusCode())
+	})
 }
 
 func TestGrantSiteRoles(t *testing.T) {