mirrors/remark42
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: make user deletion idempotent for users without comments

Browse Source 
deleteUser now succeeds for a user who has no comments (e.g. one who only logged
in) instead of failing on the missing per-user bucket. In hard mode the per-user
bucket is deleted, tolerating bbolt's ErrBucketNotFound so a bucket left behind by
an earlier partial removal is still removed; the comment-deletion failure path now
wraps the actual error.

Because the engine cannot distinguish a valid login-only user from a never-existed
one, deletion is idempotent: /admin/deleteme returns 200 for an unknown (but validly
signed) token rather than 400. The deleteme test is updated to this contract, engine
tests cover hard and soft deletion of login-only and unknown users, and the API docs
note the idempotent behaviour.
This commit is contained in:
Dmitry Verkhoturov
2026-07-01 15:05:27 -05:00
committed by Umputun
parent 380aa3c828
commit 3fc5d6b970
4 changed files with 81 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/rest/api/admin_test.go
+3 -2
View File
@@ -802,7 +802,8 @@ func TestAdmin_DeleteMeRequestFailed(t *testing.T) {
assert.NoError(t, resp.Body.Close())
assert.Equal(t, http.StatusForbidden, resp.StatusCode)
// try bad user
// unknown user: deletion is idempotent, so a valid (signed) delete_me token for a user with
// no stored data is a no-op success rather than an error
badClaimsUser := claims
badClaimsUser.User.ID = "no-such-id"
tkn, err = srv.Authenticator.TokenService().Token(badClaimsUser)
@@ -813,7 +814,7 @@ func TestAdmin_DeleteMeRequestFailed(t *testing.T) {
resp, err = client.Do(req)
assert.NoError(t, err)
assert.NoError(t, resp.Body.Close())
assert.Equal(t, http.StatusBadRequest, resp.StatusCode, resp.Status)
assert.Equal(t, http.StatusOK, resp.StatusCode, resp.Status)
badClaimsUser.User.ID = "provider1_user1"
// try without deleteme flag
backend/app/store/engine/bolt.go
+19 -20
View File
@@ -3,6 +3,7 @@ package engine
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"strings"
"time"
@@ -10,6 +11,7 @@ import (
log "github.com/go-pkgz/lgr"
"github.com/hashicorp/go-multierror"
bolt "go.etcd.io/bbolt"
berrors "go.etcd.io/bbolt/errors"
"github.com/umputun/remark42/backend/app/store"
)
@@ -890,29 +892,26 @@ func (b *BoltDB) deleteUser(bdb *bolt.DB, siteID, userID string, mode store.Dele
log.Printf("[DEBUG] comments for removal=%d", len(comments))
if len(comments) > 0 {
// delete collected comments
for _, ci := range comments {
if e := b.deleteComment(bdb, ci.locator, ci.commentID, mode); e != nil {
return fmt.Errorf("failed to delete comment %+v: %w", ci, err)
}
// delete collected comments
for _, ci := range comments {
if e := b.deleteComment(bdb, ci.locator, ci.commentID, mode); e != nil {
return fmt.Errorf("failed to delete comment %+v: %w", ci, e)
}
}
// delete user bucket in hard mode
if mode == store.HardDelete {
err = bdb.Update(func(tx *bolt.Tx) error {
usersBkt := tx.Bucket([]byte(userBucketName))
if usersBkt != nil {
if e := usersBkt.DeleteBucket([]byte(userID)); e != nil {
return fmt.Errorf("failed to delete user bucket for %s: %w", userID, e)
}
}
return nil
})
if err != nil {
return fmt.Errorf("can't delete user meta: %w", err)
// delete the user's bucket in hard mode. A user who only logged in but never commented has
// no per-user bucket, so tolerate ErrBucketNotFound; the top-level users bucket is created
// by NewBoltDB and is always present.
if mode == store.HardDelete {
err = bdb.Update(func(tx *bolt.Tx) error {
usersBkt := tx.Bucket([]byte(userBucketName))
if e := usersBkt.DeleteBucket([]byte(userID)); e != nil && !errors.Is(e, berrors.ErrBucketNotFound) {
return fmt.Errorf("failed to delete user bucket for %s: %w", userID, e)
}
return nil
})
if err != nil {
return fmt.Errorf("can't delete user meta: %w", err)
}
}
backend/app/store/engine/bolt_test.go
+57
View File
@@ -843,6 +843,63 @@ func TestBoltAdmin_DeleteUserHard(t *testing.T) {
assert.EqualError(t, err, `site "radio-t-bad" not found`)
}
// TestBoltAdmin_DeleteUserHard_NoComments covers hard-deleting a user who has no comments
// (and therefore no user bucket) — e.g. one who only logged in. This must succeed rather than
// fail on the missing bucket, and must still remove any stored user details.
func TestBoltAdmin_DeleteUserHard_NoComments(t *testing.T) {
b, teardown := prep(t)
defer teardown()
t.Run("login-only user with a detail but no comments", func(t *testing.T) {
const userID = "login-only-user"
loc := store.Locator{SiteID: "radio-t"}
// user logged in and has a stored detail, but never commented (no user bucket)
_, err := b.UserDetail(UserDetailRequest{Locator: loc, UserID: userID, Detail: UserEmail, Update: "user@example.com"})
require.NoError(t, err)
err = b.Delete(DeleteRequest{Locator: loc, UserID: userID, DeleteMode: store.HardDelete})
require.NoError(t, err, "hard delete must not fail on a missing user bucket")
details, err := b.UserDetail(UserDetailRequest{Locator: loc, UserID: userID, Detail: UserEmail})
require.NoError(t, err)
assert.Empty(t, details, "stored user detail must be removed on hard delete")
})
t.Run("unknown user is a no-op", func(t *testing.T) {
err := b.Delete(DeleteRequest{Locator: store.Locator{SiteID: "radio-t"}, UserID: "never-seen-user", DeleteMode: store.HardDelete})
assert.NoError(t, err, "hard-deleting an unknown user must not error")
})
}
// TestBoltAdmin_DeleteUserSoft_NoComments covers soft-deleting a user with no comments. As with the
// hard path (and the existing soft path for users who do have comments) it cleans stored user
// details and is a no-op for an unknown user.
func TestBoltAdmin_DeleteUserSoft_NoComments(t *testing.T) {
b, teardown := prep(t)
defer teardown()
t.Run("login-only user with a detail but no comments", func(t *testing.T) {
const userID = "login-only-soft"
loc := store.Locator{SiteID: "radio-t"}
_, err := b.UserDetail(UserDetailRequest{Locator: loc, UserID: userID, Detail: UserEmail, Update: "user@example.com"})
require.NoError(t, err)
err = b.Delete(DeleteRequest{Locator: loc, UserID: userID, DeleteMode: store.SoftDelete})
require.NoError(t, err)
details, err := b.UserDetail(UserDetailRequest{Locator: loc, UserID: userID, Detail: UserEmail})
require.NoError(t, err)
assert.Empty(t, details, "soft delete cleans stored user details, consistent with the has-comments path")
})
t.Run("unknown user is a no-op", func(t *testing.T) {
err := b.Delete(DeleteRequest{Locator: store.Locator{SiteID: "radio-t"}, UserID: "never-seen-soft", DeleteMode: store.SoftDelete})
assert.NoError(t, err, "soft-deleting an unknown user must not error")
})
}
func TestBoltAdmin_DeleteUserSoft(t *testing.T) {
b, teardown := prep(t)
defer teardown()
site/src/docs/contributing/api/index.md
+2 -2
View File
@@ -228,9 +228,9 @@ http://oldsite.com/from-old-page/1 https://newsite.com/to-new-page/1
- `GET /api/v1/admin/wait?site=site-id` - wait for completion for any async migration ops (import or remap)
- `PUT /api/v1/admin/pin/{id}?site=site-id&url=post-url&pin=1` - pin or unpin comment
- `GET /api/v1/admin/user/{userid}?site=site-id` - get user's info
- `DELETE /api/v1/admin/user/{userid}?site=site-id` - delete all user's comments
- `DELETE /api/v1/admin/user/{userid}?site=site-id` - delete the user's comments and stored details; succeeds even if the user has no comments or is already absent
- `PUT /api/v1/admin/readonly?site=site-id&url=post-url&ro=1` - set read-only status
- `PUT /api/v1/admin/verify/{userid}?site=site-id&verified=1` - set verified status
- `GET /api/v1/admin/deleteme?token=token` - process deleteme user's request
- `GET /api/v1/admin/deleteme?token=token` - process a user's deleteme request; already-deleted or dataless users return success (idempotent)
_all admin calls require auth and admin privilege_