fix: remove user avatar on deleteme request

The delete_me token built by deleteMeCtrl omitted the user's Picture, so the
avatar-removal branch in deleteMeRequestCtrl never ran for real requests and
avatars survived account deletion. Carry Picture in the token so the stored
avatar is removed when the request is processed.

Make the removal best-effort: the avatar stores report an already-missing
avatar as an error with no distinguishable sentinel, and the user's data is
already deleted at that point, so a missing avatar (e.g. a repeated request)
no longer fails the whole deletion with a 400.

Only remove a well-formed avatar id ("<hash>.image") so a malformed picture
can't make a filesystem-backed store target an unexpected path.

backend/app/rest/api/admin.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 	"path"
+	"strings"
 	"time"
 
 	"github.com/go-pkgz/auth/v2"
@@ -122,10 +123,14 @@ func (a *admin) deleteMeRequestCtrl(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if claims.User.Picture != "" && a.authenticator.AvatarProxy() != nil {
-		avatarStore := a.authenticator.AvatarProxy().Store
-		if err = avatarStore.Remove(path.Base(claims.User.Picture)); err != nil {
-			rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't delete user's avatar", rest.ErrInternal)
-			return
+		if avatarID := avatarIDFromPicture(claims.User.Picture); avatarID != "" {
+			// best-effort removal: the user's data is already gone and the avatar store gives no way to tell
+			// an already-removed avatar from a real failure, so a missing avatar must not fail the deletion
+			if err = a.authenticator.AvatarProxy().Store.Remove(avatarID); err != nil {
+				log.Printf("[WARN] can't delete avatar for user %s on site %s: %v", claims.User.ID, audience, err)
+			}
+		} else {
+			log.Printf("[WARN] unexpected avatar picture %q for user %s on site %s, skipping removal", claims.User.Picture, claims.User.ID, audience)
 		}
 	}
 
@@ -133,6 +138,17 @@ func (a *admin) deleteMeRequestCtrl(w http.ResponseWriter, r *http.Request) {
 	R.RenderJSON(w, R.JSON{"user_id": claims.User.ID, "site_id": claims.Audience})
 }
 
+// avatarIDFromPicture returns the avatar-store object id for a user picture, or "" if the picture
+// does not resolve to a well-formed id (the store names its objects "<hash>.image"). Guarding on the
+// id shape keeps a malformed picture, e.g. a path sentinel, from making a filesystem-backed store
+// target an unexpected path.
+func avatarIDFromPicture(picture string) string {
+	if id := path.Base(picture); strings.HasSuffix(id, ".image") {
+		return id
+	}
+	return ""
+}
+
 // PUT /user/{userid}?site=side-id&block=1&ttl=7d - block or unblock user
 func (a *admin) setBlockCtrl(w http.ResponseWriter, r *http.Request) {
 	userID := r.PathValue("userid")

backend/app/rest/api/admin_test.go

@@ -717,7 +717,7 @@ func TestAdmin_DeleteMeRequest(t *testing.T) {
 		},
 		User: &token.User{
 			ID:      "user1",
-			Picture: "pic.image",
+			Picture: "https://demo.remark42.com/api/v1/avatar/pic.image", // production-shaped URL: removal must path.Base it to the avatar id
 			Attributes: map[string]any{
 				"delete_me": true,
 			},
@@ -747,6 +747,76 @@ func TestAdmin_DeleteMeRequest(t *testing.T) {
 	email, err = srv.DataService.GetUserEmail("remark42", "user1")
 	assert.NoError(t, err)
 	assert.Empty(t, email, "user1 email was deleted")
+
+	assert.NoFileExists(t, os.TempDir()+"/ava-remark42/42/pic.image", "user's avatar should be removed on deleteme")
+}
+
+// a delete_me request whose token carries a picture must still succeed when the avatar is
+// already gone from the store: the user data is deleted and a missing avatar is tolerated
+func TestAdmin_DeleteMeRequestMissingAvatar(t *testing.T) {
+	ts, srv, teardown := startupT(t)
+	defer teardown()
+
+	c1 := store.Comment{Text: "test test #1", Locator: store.Locator{SiteID: "remark42",
+		URL: "https://radio-t.com/blah"}, User: store.User{Name: "user3 name", ID: "user3"}}
+	_, err := srv.DataService.Create(c1)
+	require.NoError(t, err)
+
+	claims := token.Claims{
+		SessionOnly: true,
+		RegisteredClaims: jwt.RegisteredClaims{
+			Audience:  jwt.ClaimStrings{"remark42"},
+			ID:        "2345678",
+			Issuer:    "remark42",
+			NotBefore: jwt.NewNumericDate(time.Now().Add(-1 * time.Minute)),
+			ExpiresAt: jwt.NewNumericDate(time.Now().Add(30 * time.Minute)),
+		},
+		User: &token.User{
+			ID:      "user3",
+			Picture: "missing.image", // no avatar file exists for this picture in the store
+			Attributes: map[string]any{
+				"delete_me": true,
+			},
+		},
+	}
+
+	tkn, err := srv.Authenticator.TokenService().Token(claims)
+	require.NoError(t, err)
+
+	client := http.Client{}
+	defer client.CloseIdleConnections()
+	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/v1/admin/deleteme?token=%s", ts.URL, tkn), http.NoBody)
+	require.NoError(t, err)
+	req.SetBasicAuth("admin", "password")
+	resp, err := client.Do(req)
+	require.NoError(t, err)
+	require.NoError(t, resp.Body.Close())
+	assert.Equal(t, http.StatusOK, resp.StatusCode, "a missing avatar must not fail the deletion")
+
+	_, err = srv.DataService.User("remark42", "user3", 0, 0, store.User{})
+	assert.EqualError(t, err, "no comments for user user3 in store", "user3 comments should be deleted")
+}
+
+func TestAvatarIDFromPicture(t *testing.T) {
+	tbl := []struct {
+		name    string
+		picture string
+		want    string
+	}{
+		{"local avatar url", "https://demo.remark42.com/api/v1/avatar/cb42ff493ade696d88a3a590f136ae9e34de7c1b.image", "cb42ff493ade696d88a3a590f136ae9e34de7c1b.image"},
+		{"bare avatar id", "pic.image", "pic.image"},
+		{"parent sentinel", "https://demo.remark42.com/api/v1/avatar/..", ""},
+		{"trailing slash", "https://demo.remark42.com/api/v1/avatar/", ""},
+		{"root", "/", ""},
+		{"dotdot", "..", ""},
+		{"empty", "", ""},
+		{"provider url without image suffix", "https://example.com/pic.png", ""},
+	}
+	for _, tc := range tbl {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.want, avatarIDFromPicture(tc.picture))
+		})
+	}
 }
 
 func TestAdmin_DeleteMeRequestFailed(t *testing.T) {

backend/app/rest/api/rest_private.go

@@ -708,8 +708,9 @@ func (s *private) deleteMeCtrl(w http.ResponseWriter, r *http.Request) {
 			NotBefore: jwt.NewNumericDate(time.Now().Add(-1 * time.Minute)),
 		},
 		User: &token.User{
-			ID:   user.ID,
-			Name: user.Name,
+			ID:      user.ID,
+			Name:    user.Name,
+			Picture: user.Picture, // carried so the avatar can be removed when the request is processed
 			Attributes: map[string]any{
 				"delete_me": true, // prevents this token from being used for login
 			},

backend/app/rest/api/rest_private_test.go

@@ -1512,6 +1512,8 @@ func TestRest_DeleteMe(t *testing.T) {
 	claims, err := srv.Authenticator.TokenService().Parse(tkn)
 	assert.NoError(t, err)
 	assert.Equal(t, "provider1_dev", claims.User.ID)
+	assert.Equal(t, "http://example.com/pic.png", claims.User.Picture,
+		"delete_me token must carry the user's picture so the avatar can be removed when the request is processed")
 	assert.Equal(t, "https://demo.remark42.com/web/deleteme.html?token="+tkn, m["link"])
 
 	req, err = http.NewRequest(http.MethodPost, fmt.Sprintf("%s/api/v1/deleteme?site=remark42", ts.URL), http.NoBody)