test(membership): cover user store golden/edge/error paths

Golden: add -> get -> IsAuthorized true, admin seeded. Edge: empty role
defaults to member, case-insensitive hex lookup, list ordering, revoke
denies authorization and stamps revoked_at. Error: duplicate key
(ErrUserExists), invalid role, empty sign_pub, unknown user not authorized,
revoke of unknown/already-revoked. Plus users-table migration idempotency.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

pkg/membership/users_test.go

@@ -0,0 +1,164 @@
+package membership
+
+import (
+	"errors"
+	"strings"
+	"testing"
+)
+
+// a valid-shape Ed25519 public key in hex (64 hex chars). The bytes are
+// arbitrary: the store treats sign_pub as an opaque identifier and only the CLI
+// validates the length, so any 64-hex string round-trips through the store.
+const (
+	pubAlice = "1111111111111111111111111111111111111111111111111111111111111111"
+	pubBob   = "2222222222222222222222222222222222222222222222222222222222222222"
+)
+
+// Golden: add a user, read it back, and confirm it authorizes.
+func TestAddGetIsAuthorized(t *testing.T) {
+	s := openTestStore(t)
+
+	if err := s.AddUser(pubAlice, "alice", RoleAdmin); err != nil {
+		t.Fatalf("AddUser: %v", err)
+	}
+	u, err := s.GetUser(pubAlice)
+	if err != nil {
+		t.Fatalf("GetUser: %v", err)
+	}
+	if u.Handle != "alice" || u.Role != RoleAdmin || u.Status != StatusActive {
+		t.Fatalf("GetUser mismatch: %+v", u)
+	}
+	if u.CreatedAt == "" {
+		t.Fatalf("CreatedAt not stamped")
+	}
+	if u.RevokedAt != "" {
+		t.Fatalf("RevokedAt should be empty for an active user, got %q", u.RevokedAt)
+	}
+	if !s.IsAuthorized(pubAlice) {
+		t.Fatalf("active user should be authorized")
+	}
+	if !s.HasAdmin() {
+		t.Fatalf("HasAdmin should be true after seeding an admin")
+	}
+}
+
+// Edge: an empty role defaults to member; case-insensitive lookup; list order.
+func TestAddDefaultsAndListing(t *testing.T) {
+	s := openTestStore(t)
+
+	if err := s.AddUser(pubBob, "bob", ""); err != nil {
+		t.Fatalf("AddUser bob: %v", err)
+	}
+	u, err := s.GetUser(pubBob)
+	if err != nil {
+		t.Fatalf("GetUser bob: %v", err)
+	}
+	if u.Role != RoleMember {
+		t.Fatalf("empty role should default to member, got %q", u.Role)
+	}
+	// Adding bob (a member only) must not make HasAdmin true.
+	if s.HasAdmin() {
+		t.Fatalf("HasAdmin should be false with only a member registered")
+	}
+
+	// Lookup is case-insensitive: uppercase hex matches the lowercase-stored key.
+	if !s.IsAuthorized(strings.ToUpper(pubBob)) {
+		t.Fatalf("IsAuthorized should be case-insensitive on the hex key")
+	}
+
+	if err := s.AddUser(pubAlice, "alice", RoleAdmin); err != nil {
+		t.Fatalf("AddUser alice: %v", err)
+	}
+	users, err := s.ListUsers()
+	if err != nil {
+		t.Fatalf("ListUsers: %v", err)
+	}
+	// Ordered by handle: alice before bob.
+	if len(users) != 2 || users[0].Handle != "alice" || users[1].Handle != "bob" {
+		t.Fatalf("ListUsers order/content wrong: %+v", users)
+	}
+}
+
+// Edge: revocation flips status, stamps revoked_at, and denies authorization on
+// the spot — the property both planes rely on for revoke-without-restart.
+func TestRevokeDeniesAuthorization(t *testing.T) {
+	s := openTestStore(t)
+
+	if err := s.AddUser(pubAlice, "alice", RoleMember); err != nil {
+		t.Fatalf("AddUser: %v", err)
+	}
+	if !s.IsAuthorized(pubAlice) {
+		t.Fatalf("precondition: user should be authorized before revoke")
+	}
+	if err := s.RevokeUser(pubAlice); err != nil {
+		t.Fatalf("RevokeUser: %v", err)
+	}
+	if s.IsAuthorized(pubAlice) {
+		t.Fatalf("revoked user must NOT be authorized")
+	}
+	u, err := s.GetUser(pubAlice)
+	if err != nil {
+		t.Fatalf("GetUser after revoke: %v", err)
+	}
+	if u.Status != StatusRevoked || u.RevokedAt == "" {
+		t.Fatalf("revoke should set status=revoked and stamp revoked_at, got %+v", u)
+	}
+}
+
+// Error path: duplicate key, unknown user, invalid role, revoke of unknown.
+func TestUserErrorPaths(t *testing.T) {
+	s := openTestStore(t)
+
+	if err := s.AddUser(pubAlice, "alice", RoleAdmin); err != nil {
+		t.Fatalf("AddUser: %v", err)
+	}
+	// Duplicate sign_pub -> typed ErrUserExists.
+	if err := s.AddUser(pubAlice, "alice2", RoleMember); !errors.Is(err, ErrUserExists) {
+		t.Fatalf("duplicate AddUser should return ErrUserExists, got %v", err)
+	}
+	// Invalid role rejected.
+	if err := s.AddUser(pubBob, "bob", "superuser"); err == nil {
+		t.Fatalf("invalid role should error")
+	}
+	// Missing handle/sign_pub rejected.
+	if err := s.AddUser("", "nobody", RoleMember); err == nil {
+		t.Fatalf("empty sign_pub should error")
+	}
+	// Unknown user is not authorized (fail closed) and GetUser errors.
+	if s.IsAuthorized(pubBob) {
+		t.Fatalf("unknown user must not be authorized")
+	}
+	if _, err := s.GetUser(pubBob); err == nil {
+		t.Fatalf("GetUser of unknown user should error")
+	}
+	// Revoking an unknown (or already-revoked) user errors (no active row).
+	if err := s.RevokeUser(pubBob); err == nil {
+		t.Fatalf("revoking unknown user should error")
+	}
+	if err := s.RevokeUser(pubAlice); err != nil {
+		t.Fatalf("first revoke should succeed: %v", err)
+	}
+	if err := s.RevokeUser(pubAlice); err == nil {
+		t.Fatalf("second revoke of same user should error (already revoked)")
+	}
+}
+
+// Migration safety: the users table and its index exist after Open, and the
+// users migration is idempotent on re-apply (mirrors TestMigrationsCreateSchema).
+func TestUsersMigrationIdempotent(t *testing.T) {
+	s := openTestStore(t)
+	var name string
+	if err := s.db.QueryRow(
+		`SELECT name FROM sqlite_master WHERE type='table' AND name='users'`,
+	).Scan(&name); err != nil {
+		t.Fatalf("users table not created: %v", err)
+	}
+	if err := s.db.QueryRow(
+		`SELECT name FROM sqlite_master WHERE type='index' AND name='idx_users_status'`,
+	).Scan(&name); err != nil {
+		t.Fatalf("idx_users_status not created: %v", err)
+	}
+	if err := s.applyMigrations(); err != nil {
+		t.Fatalf("re-apply migrations: %v", err)
+	}
+}