improved identity management

internal/storage/udl_admin.go

@@ -0,0 +1,245 @@
+package storage
+
+// Account-scoped UDL administration: what a uid holds, and removing all of it.
+//
+// Both operations enumerate the same thing — every `udl:rec:<uid>/<cid>/<key>`
+// cell — because there is no per-user cid index to consult. The Redis model
+// (see udl.go) indexes user+key → cids and user+cid → keys, which answer
+// "what is in My List", not "everything this account ever wrote". Adding a
+// third index would put a write on every UDL upsert to serve two admin calls,
+// so these SCAN instead and accept being O(keyspace).
+//
+// That cost is why UDLAllUserStats exists as a bulk call rather than a per-uid
+// one: the accounts page needs a number for every account, and MATCH filters
+// server-side without indexing, so N per-uid calls would walk the keyspace N
+// times. One walk buckets them all.
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"time"
+)
+
+// UDLUserStats counts what one account holds. Records is the number of cells,
+// including tombstones — a retracted rating is still a row this account owns
+// and still something a purge has to remove.
+type UDLUserStats struct {
+	Records int `json:"records"`
+	Cids    int `json:"cids"`
+	Keys    int `json:"keys"`
+}
+
+// udlScanBatch is the SCAN COUNT hint, matching the rest of this package.
+const udlScanBatch = 1000
+
+// safeUIDForPattern rejects anything that would change the meaning of a SCAN
+// glob or reach across the key structure. meta-core's uids are alphanumeric
+// (identity.validUID), so this only ever fires on a hostile or malformed one —
+// but it fires here, in the layer that builds the pattern, rather than relying
+// on every caller having validated first.
+func safeUIDForPattern(uid string) error {
+	if uid == "" {
+		return fmt.Errorf("empty uid")
+	}
+	if strings.ContainsAny(uid, "*?[]\\/:") {
+		return fmt.Errorf("uid contains characters that are not valid in a uid: %q", uid)
+	}
+	return nil
+}
+
+// splitUDLRecSuffix splits the "<uid>/<cid>/<key>" tail of a udl:rec key.
+//
+// SplitN with a limit of 3 so a key containing a slash survives intact. cids
+// never contain one; keys are a controlled vocabulary today, but a namespaced
+// one arriving later must not silently truncate here — a truncated key would
+// make the purge SREM the wrong index member and leave the real one behind.
+func splitUDLRecSuffix(suffix string) (uid, cid, key string, ok bool) {
+	parts := strings.SplitN(suffix, "/", 3)
+	if len(parts) != 3 || parts[0] == "" || parts[1] == "" || parts[2] == "" {
+		return "", "", "", false
+	}
+	return parts[0], parts[1], parts[2], true
+}
+
+// scanKeys walks the keyspace for pattern and returns every match. Caller holds
+// the lock.
+//
+// SCAN can return the same key more than once across cursor iterations (it
+// guarantees no misses, not no duplicates), so callers that count must
+// de-duplicate — both callers below collect into maps or sets for that reason.
+func (c *Client) scanKeys(ctx context.Context, pattern string) ([]string, error) {
+	var (
+		out    []string
+		cursor uint64
+	)
+	for {
+		keys, next, err := c.client.Scan(ctx, cursor, pattern, udlScanBatch).Result()
+		if err != nil {
+			return nil, fmt.Errorf("scan %q: %w", pattern, err)
+		}
+		out = append(out, keys...)
+		cursor = next
+		if cursor == 0 {
+			return out, nil
+		}
+	}
+}
+
+// UDLUserCells returns every (cid, key) pair this account holds a cell for.
+// De-duplicated, since SCAN may repeat a key.
+func (c *Client) UDLUserCells(uid string) (map[string]string, error) {
+	// Keyed by the full "cid/key" tail so the pair stays a pair; the value is
+	// the cid, which the purge needs grouped.
+	if err := safeUIDForPattern(uid); err != nil {
+		return nil, err
+	}
+
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+	if c.client == nil {
+		return nil, fmt.Errorf("not connected")
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancel()
+
+	prefix := c.buildKey("udl:rec:")
+	keys, err := c.scanKeys(ctx, prefix+uid+"/*")
+	if err != nil {
+		return nil, err
+	}
+	out := make(map[string]string, len(keys))
+	for _, full := range keys {
+		gotUID, cid, key, ok := splitUDLRecSuffix(strings.TrimPrefix(full, prefix))
+		// The glob can only match this uid, but a uid that is a prefix of
+		// another would still be caught by the '/' in the pattern; check anyway
+		// so a malformed key can never be attributed to the wrong account.
+		if !ok || gotUID != uid {
+			continue
+		}
+		out[cid+"/"+key] = cid
+	}
+	return out, nil
+}
+
+// UDLAllUserStats counts every account's cells in one keyspace walk.
+//
+// Returns only accounts that actually hold records; a uid absent from the map
+// holds none. Callers render a zero for those rather than treating it as an
+// error — an account created a minute ago legitimately has nothing.
+func (c *Client) UDLAllUserStats() (map[string]UDLUserStats, error) {
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+	if c.client == nil {
+		return nil, fmt.Errorf("not connected")
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+	defer cancel()
+
+	prefix := c.buildKey("udl:rec:")
+	keys, err := c.scanKeys(ctx, prefix+"*")
+	if err != nil {
+		return nil, err
+	}
+
+	// Sets rather than counters: SCAN may hand back the same key twice, which
+	// would otherwise inflate every number on the page.
+	type acc struct {
+		cells map[string]struct{}
+		cids  map[string]struct{}
+		keys  map[string]struct{}
+	}
+	byUID := make(map[string]*acc)
+	for _, full := range keys {
+		uid, cid, key, ok := splitUDLRecSuffix(strings.TrimPrefix(full, prefix))
+		if !ok {
+			continue
+		}
+		a := byUID[uid]
+		if a == nil {
+			a = &acc{
+				cells: make(map[string]struct{}),
+				cids:  make(map[string]struct{}),
+				keys:  make(map[string]struct{}),
+			}
+			byUID[uid] = a
+		}
+		a.cells[cid+"/"+key] = struct{}{}
+		a.cids[cid] = struct{}{}
+		a.keys[key] = struct{}{}
+	}
+
+	out := make(map[string]UDLUserStats, len(byUID))
+	for uid, a := range byUID {
+		out[uid] = UDLUserStats{Records: len(a.cells), Cids: len(a.cids), Keys: len(a.keys)}
+	}
+	return out, nil
+}
+
+// UDLPurgeUser removes every trace of one account from the User Data Layer:
+// its cells, its two user-scoped indexes, and its membership in the shared
+// cid-scoped indexes. Returns what was removed.
+//
+// The shared indexes are the part that is easy to miss and expensive to leave.
+// `udl:idx:cid:<cid>:key:<key>` and `udl:idx:cid:<cid>:users` are keyed by cid,
+// not by uid, so a purge that only deleted `udl:rec:<uid>/*` would leave this
+// uid listed as an opinion-holder on every title it ever touched. Aggregation
+// then HGETs a cell that no longer exists — UDLPublicValues skips it, so the
+// rating average stays right, but uids_for_cid keeps naming a deleted account
+// forever and every aggregate read pays for the lookup.
+//
+// Idempotent: purging an account with nothing stored is a no-op returning
+// zeroes, so a retry after a partial failure is safe.
+func (c *Client) UDLPurgeUser(uid string) (UDLUserStats, error) {
+	cells, err := c.UDLUserCells(uid)
+	if err != nil {
+		return UDLUserStats{}, err
+	}
+
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if c.client == nil {
+		return UDLUserStats{}, fmt.Errorf("not connected")
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+	defer cancel()
+
+	stats := UDLUserStats{Records: len(cells)}
+	distinctCids := make(map[string]struct{})
+	distinctKeys := make(map[string]struct{})
+
+	pipe := c.client.Pipeline()
+	for cidKey, cid := range cells {
+		key := strings.TrimPrefix(cidKey, cid+"/")
+		distinctCids[cid] = struct{}{}
+		distinctKeys[key] = struct{}{}
+		pipe.Del(ctx, c.buildKey(udlRecKey(uid, cid, key)))
+		pipe.SRem(ctx, c.buildKey(udlIdxCidKey(cid, key)), uid)
+	}
+	for cid := range distinctCids {
+		pipe.SRem(ctx, c.buildKey(udlIdxCidUsers(cid)), uid)
+	}
+	stats.Cids = len(distinctCids)
+	stats.Keys = len(distinctKeys)
+
+	// The two user-scoped indexes are enumerable directly, and are dropped
+	// whole rather than member-by-member. Scanning them separately also catches
+	// index entries whose cell is already gone — exactly the state a previously
+	// half-finished purge leaves behind.
+	idxPrefix := c.buildKey("udl:idx:user:") + uid + ":"
+	idxKeys, err := c.scanKeys(ctx, idxPrefix+"*")
+	if err != nil {
+		return UDLUserStats{}, err
+	}
+	for _, k := range idxKeys {
+		pipe.Del(ctx, k)
+	}
+
+	if _, err := pipe.Exec(ctx); err != nil {
+		return UDLUserStats{}, fmt.Errorf("purge pipeline: %w", err)
+	}
+	return stats, nil
+}

internal/storage/udl_admin_test.go

@@ -0,0 +1,253 @@
+package storage
+
+import (
+	"testing"
+)
+
+// seedCell writes one live UDL cell the way a real client would.
+func seedCell(t *testing.T, c *Client, uid, cid, key string) {
+	t.Helper()
+	if ok, err := c.UDLUpsertIfNewer(uid, cid, key, 1, 100, "rec", "", false, false); err != nil || !ok {
+		t.Fatalf("seed %s/%s/%s: ok=%v err=%v", uid, cid, key, ok, err)
+	}
+}
+
+// The purge has to reach the cid-scoped indexes, not just the user's own rows.
+//
+// This is the failure that motivated the whole endpoint: deleting an account
+// while leaving `udl:idx:cid:<cid>:users` intact means every aggregate read on
+// every title that account ever touched keeps naming a uid nobody can resolve,
+// forever — and the rows are unreachable through any API, so nothing will ever
+// clean them up.
+func TestUDLPurgeUser_RemovesCellsAndBothIndexFamilies(t *testing.T) {
+	c, _ := newTestClient(t, "")
+
+	seedCell(t, c, "zAlice", "cidOne", "like")
+	seedCell(t, c, "zAlice", "cidOne", "rating")
+	seedCell(t, c, "zAlice", "cidTwo", "seek")
+
+	stats, err := c.UDLPurgeUser("zAlice")
+	if err != nil {
+		t.Fatalf("purge: %v", err)
+	}
+	if stats.Records != 3 || stats.Cids != 2 || stats.Keys != 3 {
+		t.Fatalf("stats = %+v, want 3 records / 2 cids / 3 keys", stats)
+	}
+
+	// Cells gone.
+	if _, ok, err := c.UDLGetCell("zAlice", "cidOne", "like"); err != nil || ok {
+		t.Fatalf("cell survived the purge: ok=%v err=%v", ok, err)
+	}
+	// User-scoped indexes gone.
+	if entries, err := c.UDLListUserKey("zAlice", "like"); err != nil || len(entries) != 0 {
+		t.Fatalf("user+key index survived: %d entries, err=%v", len(entries), err)
+	}
+	if entries, err := c.UDLListUserCid("zAlice", "cidOne"); err != nil || len(entries) != 0 {
+		t.Fatalf("user+cid index survived: %d entries, err=%v", len(entries), err)
+	}
+	// Cid-scoped index gone — the one that is easy to miss.
+	for _, cid := range []string{"cidOne", "cidTwo"} {
+		uids, err := c.UDLCidUsers(cid)
+		if err != nil {
+			t.Fatalf("cid users %s: %v", cid, err)
+		}
+		for _, u := range uids {
+			if u == "zAlice" {
+				t.Fatalf("%s still lists the deleted account in uids_for_cid", cid)
+			}
+		}
+	}
+}
+
+// Purging one account must not touch another's rows, including on a cid they
+// both have an opinion about — where the two share an index set and a careless
+// DEL of the whole key would take the survivor's membership with it.
+func TestUDLPurgeUser_LeavesOtherAccountsIntact(t *testing.T) {
+	c, _ := newTestClient(t, "")
+
+	seedCell(t, c, "zAlice", "shared", "rating")
+	seedCell(t, c, "zBob", "shared", "rating")
+
+	if _, err := c.UDLPurgeUser("zAlice"); err != nil {
+		t.Fatalf("purge: %v", err)
+	}
+
+	if _, ok, err := c.UDLGetCell("zBob", "shared", "rating"); err != nil || !ok {
+		t.Fatalf("bob's cell must survive alice's purge: ok=%v err=%v", ok, err)
+	}
+	uids, err := c.UDLCidUsers("shared")
+	if err != nil {
+		t.Fatalf("cid users: %v", err)
+	}
+	if len(uids) != 1 || uids[0] != "zBob" {
+		t.Fatalf("uids_for_cid = %v, want exactly [zBob]", uids)
+	}
+	entries, err := c.UDLListUserKey("zBob", "rating")
+	if err != nil || len(entries) != 1 {
+		t.Fatalf("bob's user+key index must survive: %d entries, err=%v", len(entries), err)
+	}
+}
+
+// A uid that is a prefix of another must not drag its neighbour's rows out.
+// The key layout separates them with '/' and ':', so this passes only as long
+// as the scan patterns keep those separators.
+func TestUDLPurgeUser_DoesNotMatchUIDsByPrefix(t *testing.T) {
+	c, _ := newTestClient(t, "")
+
+	seedCell(t, c, "zAlice", "cidOne", "like")
+	seedCell(t, c, "zAliceExtra", "cidOne", "like")
+
+	stats, err := c.UDLPurgeUser("zAlice")
+	if err != nil {
+		t.Fatalf("purge: %v", err)
+	}
+	if stats.Records != 1 {
+		t.Fatalf("purged %d records, want exactly alice's 1", stats.Records)
+	}
+	if _, ok, err := c.UDLGetCell("zAliceExtra", "cidOne", "like"); err != nil || !ok {
+		t.Fatalf("zAliceExtra's cell must survive: ok=%v err=%v", ok, err)
+	}
+}
+
+// Tombstoned cells still belong to the account. They are absent from the
+// user+key index by design (see udl.go), so a purge that enumerated *that*
+// index instead of the cells would leave every retracted rating behind — the
+// rows most likely to have accumulated over an account's life.
+func TestUDLPurgeUser_RemovesTombstonedCellsToo(t *testing.T) {
+	c, _ := newTestClient(t, "")
+
+	seedCell(t, c, "zAlice", "cidOne", "like")
+	if ok, err := c.UDLUpsertIfNewer("zAlice", "cidOne", "like", 2, 200, "rec-tomb", "", false, true); err != nil || !ok {
+		t.Fatalf("tombstone: ok=%v err=%v", ok, err)
+	}
+	// Precondition: the tombstone left the cell but dropped it from the index.
+	if entries, _ := c.UDLListUserKey("zAlice", "like"); len(entries) != 0 {
+		t.Fatalf("precondition: tombstone should be out of the user+key index")
+	}
+
+	stats, err := c.UDLPurgeUser("zAlice")
+	if err != nil {
+		t.Fatalf("purge: %v", err)
+	}
+	if stats.Records != 1 {
+		t.Fatalf("purged %d records, want the 1 tombstoned cell", stats.Records)
+	}
+	if _, ok, _ := c.UDLGetCell("zAlice", "cidOne", "like"); ok {
+		t.Fatal("tombstoned cell survived the purge")
+	}
+}
+
+// Retry-safe: the delete handler purges before removing the key, so a caller
+// that retries after a partial failure runs this a second time.
+func TestUDLPurgeUser_IsIdempotent(t *testing.T) {
+	c, _ := newTestClient(t, "")
+	seedCell(t, c, "zAlice", "cidOne", "like")
+
+	if _, err := c.UDLPurgeUser("zAlice"); err != nil {
+		t.Fatalf("first purge: %v", err)
+	}
+	stats, err := c.UDLPurgeUser("zAlice")
+	if err != nil {
+		t.Fatalf("second purge must be a no-op, got: %v", err)
+	}
+	if stats.Records != 0 || stats.Cids != 0 {
+		t.Fatalf("second purge = %+v, want zeroes", stats)
+	}
+}
+
+func TestUDLPurgeUser_RefusesGlobInjection(t *testing.T) {
+	c, _ := newTestClient(t, "")
+	seedCell(t, c, "zAlice", "cidOne", "like")
+
+	// "*" would otherwise match every account in the keyspace — a wildcard
+	// purge dressed up as a uid.
+	for _, uid := range []string{"*", "z*", "", "z:Alice", "z/Alice"} {
+		if _, err := c.UDLPurgeUser(uid); err == nil {
+			t.Errorf("uid %q must be refused before it reaches a SCAN pattern", uid)
+		}
+	}
+	if _, ok, _ := c.UDLGetCell("zAlice", "cidOne", "like"); !ok {
+		t.Fatal("a refused purge must not have deleted anything")
+	}
+}
+
+func TestUDLAllUserStats_CountsEachAccountSeparately(t *testing.T) {
+	c, _ := newTestClient(t, "")
+
+	seedCell(t, c, "zAlice", "cidOne", "like")
+	seedCell(t, c, "zAlice", "cidOne", "rating")
+	seedCell(t, c, "zAlice", "cidTwo", "like")
+	seedCell(t, c, "zBob", "cidOne", "seek")
+
+	stats, err := c.UDLAllUserStats()
+	if err != nil {
+		t.Fatalf("stats: %v", err)
+	}
+	if got := stats["zAlice"]; got.Records != 3 || got.Cids != 2 || got.Keys != 2 {
+		t.Fatalf("alice = %+v, want 3 records / 2 cids / 2 keys", got)
+	}
+	if got := stats["zBob"]; got.Records != 1 || got.Cids != 1 || got.Keys != 1 {
+		t.Fatalf("bob = %+v, want 1/1/1", got)
+	}
+	// An account with nothing stored is simply absent — the caller renders 0.
+	if _, present := stats["zNobody"]; present {
+		t.Fatal("an account with no records must not appear in the map")
+	}
+}
+
+// The stats and the purge must agree: the number shown in the delete
+// confirmation is the number of rows the delete then removes.
+func TestUDLAllUserStats_AgreesWithWhatPurgeRemoves(t *testing.T) {
+	c, _ := newTestClient(t, "")
+
+	seedCell(t, c, "zAlice", "cidOne", "like")
+	seedCell(t, c, "zAlice", "cidTwo", "rating")
+	seedCell(t, c, "zAlice", "cidTwo", "seek")
+
+	before, err := c.UDLAllUserStats()
+	if err != nil {
+		t.Fatalf("stats: %v", err)
+	}
+	purged, err := c.UDLPurgeUser("zAlice")
+	if err != nil {
+		t.Fatalf("purge: %v", err)
+	}
+	if before["zAlice"] != purged {
+		t.Fatalf("stats said %+v but purge removed %+v", before["zAlice"], purged)
+	}
+}
+
+// The key prefix (multi-tenant Redis) must not leak into the parsed uid, or
+// every account on a prefixed node would be reported under a mangled name and
+// the purge would find nothing.
+func TestUDLAdmin_RespectsKeyPrefix(t *testing.T) {
+	c, _ := newTestClient(t, "tenantA:")
+
+	seedCell(t, c, "zAlice", "cidOne", "like")
+
+	stats, err := c.UDLAllUserStats()
+	if err != nil {
+		t.Fatalf("stats: %v", err)
+	}
+	if got := stats["zAlice"]; got.Records != 1 {
+		t.Fatalf("stats under a prefix = %+v, want 1 record for zAlice", stats)
+	}
+	if s, err := c.UDLPurgeUser("zAlice"); err != nil || s.Records != 1 {
+		t.Fatalf("purge under a prefix: %+v err=%v", s, err)
+	}
+}
+
+func TestSplitUDLRecSuffix_KeepsSlashesInTheKey(t *testing.T) {
+	// A namespaced key arriving later must not be truncated: the purge SREMs
+	// `udl:idx:cid:<cid>:key:<key>`, so a half-read key removes the wrong
+	// member and leaves the real one behind.
+	uid, cid, key, ok := splitUDLRecSuffix("zAlice/cidOne/ns/sub:key")
+	if !ok || uid != "zAlice" || cid != "cidOne" || key != "ns/sub:key" {
+		t.Fatalf("got (%q,%q,%q,%v)", uid, cid, key, ok)
+	}
+	for _, bad := range []string{"", "zAlice", "zAlice/cidOne", "zAlice//key", "/cid/key"} {
+		if _, _, _, ok := splitUDLRecSuffix(bad); ok {
+			t.Errorf("%q should not parse", bad)
+		}
+	}
+}