block suspended BOS sessions and cascade feedbag deletes

- auth: RegisterBOSSession refuses accounts with a non-zero suspended
  status, covering saved login cookies, web tokens, and linked-account
  switches.
- state: migration 0044 adds a feedbag foreign key to users with
  ON DELETE CASCADE and drops orphaned rows and links to nonexistent
  users. DeleteUser also removes linked-account items that name the
  deleted user.
- mgmt api: PUT /feedbag/{sn}/group/{name} returns 404 for an unknown
  user.

foodgroup/auth.go

@@ -132,6 +132,10 @@ func (s AuthService) RegisterBOSSession(ctx context.Context, authCookie state.Se
 	if u == nil {
 		return nil, fmt.Errorf("user not found")
 	}
+	// the cookie may predate the account's suspension
+	if u.SuspendedStatus > 0x0 {
+		return nil, fmt.Errorf("user suspended (status 0x%04x)", u.SuspendedStatus)
+	}
 
 	ctx, cancel := context.WithTimeout(ctx, time.Second*5)
 	defer cancel()

foodgroup/auth_test.go

@@ -2407,6 +2407,44 @@ func TestAuthService_RegisterBOSSession(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:    "user suspended, return error without creating a session",
+			cookie:  aimAuthCookie,
+			wantErr: errors.New("user suspended"),
+			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: screenName.IdentScreenName(),
+							result: &state.User{
+								IdentScreenName:   screenName.IdentScreenName(),
+								DisplayScreenName: screenName,
+								SuspendedStatus:   wire.LoginErrSuspendedAccount,
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:    "user deleted, return error without creating a session",
+			cookie:  aimAuthCookie,
+			wantErr: errors.New("user suspended"),
+			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: screenName.IdentScreenName(),
+							result: &state.User{
+								IdentScreenName:   screenName.IdentScreenName(),
+								DisplayScreenName: screenName,
+								SuspendedStatus:   wire.LoginErrDeletedAccount,
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range cases {

server/http/mgmt_api.go

@@ -158,7 +158,7 @@ func NewManagementAPI(bld config.Build, listener string, userManager UserManager
 
 	// Handlers for '/feedbag/{screen_name}/group/{group_name}' route
 	mux.HandleFunc("PUT /feedbag/{screen_name}/group/{group_name}", func(w http.ResponseWriter, r *http.Request) {
-		putFeedbagGroupHandler(w, r, feedbagManager, messageRelayer, logger, rand.Intn)
+		putFeedbagGroupHandler(w, r, userManager, feedbagManager, messageRelayer, logger, rand.Intn)
 	})
 
 	// Handlers for '/feedbag/{screen_name}/group/{group_id}/buddy/{buddy_screen_name}' route
@@ -1788,7 +1788,7 @@ func getFeedbagBuddyHandler(w http.ResponseWriter, r *http.Request, feedbagManag
 }
 
 // putFeedbagGroupHandler handles the PUT /feedbag/{screen_name}/group/{group_name} endpoint.
-func putFeedbagGroupHandler(w http.ResponseWriter, r *http.Request, feedbagManager FeedbagManager, messageRelayer MessageRelayer, logger *slog.Logger, intn func(n int) int) {
+func putFeedbagGroupHandler(w http.ResponseWriter, r *http.Request, userManager UserManager, feedbagManager FeedbagManager, messageRelayer MessageRelayer, logger *slog.Logger, intn func(n int) int) {
 	w.Header().Set("Content-Type", "application/json")
 
 	screenName := r.PathValue("screen_name")
@@ -1805,6 +1805,17 @@ func putFeedbagGroupHandler(w http.ResponseWriter, r *http.Request, feedbagManag
 
 	user := state.NewIdentScreenName(screenName)
 
+	u, err := userManager.User(r.Context(), user)
+	if err != nil {
+		logger.Error("error retrieving user", "err", err.Error())
+		errorMsg(w, "internal server error", http.StatusInternalServerError)
+		return
+	}
+	if u == nil {
+		errorMsg(w, "user not found", http.StatusNotFound)
+		return
+	}
+
 	items, err := feedbagManager.Feedbag(r.Context(), user)
 	if err != nil {
 		logger.Error("error retrieving feedbag", "err", err.Error())

server/http/mgmt_api_test.go

@@ -3272,6 +3272,14 @@ func TestFeedbagGroupHandler_PUT(t *testing.T) {
 			wantStatusCode: http.StatusCreated,
 			wantResponse:   `{"group_id":5,"group_name":"Friends"}`,
 			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							result:     &state.User{IdentScreenName: state.NewIdentScreenName("userA")},
+						},
+					},
+				},
 				feedbagManagerParams: feedbagManagerParams{
 					feedbagParams: feedbagParams{
 						{
@@ -3345,6 +3353,14 @@ func TestFeedbagGroupHandler_PUT(t *testing.T) {
 			wantStatusCode: http.StatusCreated,
 			wantResponse:   `{"group_id":2,"group_name":"Coworkers"}`,
 			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							result:     &state.User{IdentScreenName: state.NewIdentScreenName("userA")},
+						},
+					},
+				},
 				feedbagManagerParams: feedbagManagerParams{
 					feedbagParams: feedbagParams{
 						{
@@ -3433,6 +3449,14 @@ func TestFeedbagGroupHandler_PUT(t *testing.T) {
 			wantStatusCode: http.StatusOK,
 			wantResponse:   `{"group_id":1,"group_name":"Friends"}`,
 			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							result:     &state.User{IdentScreenName: state.NewIdentScreenName("userA")},
+						},
+					},
+				},
 				feedbagManagerParams: feedbagManagerParams{
 					feedbagParams: feedbagParams{
 						{
@@ -3470,6 +3494,42 @@ func TestFeedbagGroupHandler_PUT(t *testing.T) {
 			wantStatusCode: http.StatusBadRequest,
 			wantResponse:   `{"message":"group_name is required"}`,
 		},
+		{
+			name:           "user not found",
+			screenName:     "userA",
+			groupName:      "Friends",
+			groupIDGen:     func(n int) int { return 5 },
+			wantStatusCode: http.StatusNotFound,
+			wantResponse:   `{"message":"user not found"}`,
+			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							result:     nil,
+						},
+					},
+				},
+			},
+		},
+		{
+			name:           "error retrieving user",
+			screenName:     "userA",
+			groupName:      "Friends",
+			groupIDGen:     func(n int) int { return 5 },
+			wantStatusCode: http.StatusInternalServerError,
+			wantResponse:   `{"message":"internal server error"}`,
+			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							err:        errors.New("database error"),
+						},
+					},
+				},
+			},
+		},
 		{
 			name:           "error retrieving feedbag",
 			screenName:     "userA",
@@ -3478,6 +3538,14 @@ func TestFeedbagGroupHandler_PUT(t *testing.T) {
 			wantStatusCode: http.StatusInternalServerError,
 			wantResponse:   `{"message":"internal server error"}`,
 			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							result:     &state.User{IdentScreenName: state.NewIdentScreenName("userA")},
+						},
+					},
+				},
 				feedbagManagerParams: feedbagManagerParams{
 					feedbagParams: feedbagParams{
 						{
@@ -3497,6 +3565,14 @@ func TestFeedbagGroupHandler_PUT(t *testing.T) {
 			wantStatusCode: http.StatusInternalServerError,
 			wantResponse:   `{"message":"internal server error"}`,
 			mockParams: mockParams{
+				userManagerParams: userManagerParams{
+					getUserParams: getUserParams{
+						{
+							screenName: state.NewIdentScreenName("userA"),
+							result:     &state.User{IdentScreenName: state.NewIdentScreenName("userA")},
+						},
+					},
+				},
 				feedbagManagerParams: feedbagManagerParams{
 					feedbagParams: feedbagParams{
 						{
@@ -3543,6 +3619,12 @@ func TestFeedbagGroupHandler_PUT(t *testing.T) {
 			}
 			responseRecorder := httptest.NewRecorder()
 
+			userManager := newMockUserManager(t)
+			for _, params := range tc.mockParams.getUserParams {
+				userManager.EXPECT().
+					User(matchContext(), params.screenName).
+					Return(params.result, params.err)
+			}
 			feedbagManager := newMockFeedbagManager(t)
 			messageRelayer := newMockMessageRelayer(t)
 			for _, params := range tc.mockParams.feedbagParams {
@@ -3560,7 +3642,7 @@ func TestFeedbagGroupHandler_PUT(t *testing.T) {
 					RelayToScreenName(matchContext(), params.screenName, params.msg)
 			}
 
-			putFeedbagGroupHandler(responseRecorder, request, feedbagManager, messageRelayer, slog.Default(), tc.groupIDGen)
+			putFeedbagGroupHandler(responseRecorder, request, userManager, feedbagManager, messageRelayer, slog.Default(), tc.groupIDGen)
 
 			assert.Equal(t, tc.wantStatusCode, responseRecorder.Code)
 			assert.JSONEq(t, tc.wantResponse, responseRecorder.Body.String())

state/migrations/0044_feedbag_fk.down.sql

@@ -0,0 +1,22 @@
+ALTER TABLE feedbag
+    RENAME TO feedbag_new;
+
+CREATE TABLE feedbag
+(
+    screenName   VARCHAR(16),
+    groupID      INTEGER,
+    itemID       INTEGER,
+    classID      INTEGER,
+    name         TEXT,
+    attributes   BLOB,
+    lastModified INTEGER,
+    pdMode       INT              DEFAULT 0,
+    authPending  BOOLEAN NOT NULL DEFAULT FALSE,
+    UNIQUE (screenName, groupID, itemID)
+);
+
+INSERT INTO feedbag (screenName, groupID, itemID, classID, name, attributes, lastModified, pdMode, authPending)
+SELECT screenName, groupID, itemID, classID, name, attributes, lastModified, pdMode, authPending
+FROM feedbag_new;
+
+DROP TABLE feedbag_new;

state/migrations/0044_feedbag_fk.up.sql

@@ -0,0 +1,33 @@
+-- Deleting a user deletes their feedbag. Rows that belong to no user are
+-- dropped, since they would violate the foreign key.
+-- classID 24 is wire.FeedbagClassIdAlInfo (linked account).
+ALTER TABLE feedbag
+    RENAME TO feedbag_old;
+
+CREATE TABLE feedbag
+(
+    screenName   VARCHAR(16),
+    groupID      INTEGER,
+    itemID       INTEGER,
+    classID      INTEGER,
+    name         TEXT,
+    attributes   BLOB,
+    lastModified INTEGER,
+    pdMode       INT              DEFAULT 0,
+    authPending  BOOLEAN NOT NULL DEFAULT FALSE,
+    UNIQUE (screenName, groupID, itemID),
+    FOREIGN KEY (screenName) REFERENCES users (identScreenName) ON DELETE CASCADE ON UPDATE CASCADE
+);
+
+INSERT INTO feedbag (screenName, groupID, itemID, classID, name, attributes, lastModified, pdMode, authPending)
+SELECT screenName, groupID, itemID, classID, name, attributes, lastModified, pdMode, authPending
+FROM feedbag_old
+WHERE screenName IN (SELECT identScreenName FROM users);
+
+DROP TABLE feedbag_old;
+
+-- Linked-account items whose target no longer exists would otherwise grant
+-- access to whoever registers that screen name next.
+DELETE FROM feedbag
+WHERE classID = 24
+  AND name NOT IN (SELECT identScreenName FROM users);

state/relationship_test.go

@@ -3218,6 +3218,7 @@ func TestSQLiteUserStore_AllRelationships(t *testing.T) {
 			}
 
 			for sn, list := range tt.serverSideLists {
+				insertTestUsers(t, feedbagStore, sn)
 				assert.NoError(t, feedbagStore.UseFeedbag(context.Background(), sn))
 				itemID := uint16(1)
 				items := []wire.FeedbagItem{

state/user_store.go

@@ -869,11 +869,19 @@ func (f SQLiteUserStore) InsertUser(ctx context.Context, u User) error {
 	return nil
 }
 
+// DeleteUser deletes a user, their feedbag, and every linked-account item in
+// other users' feedbags that points to them.
 func (f SQLiteUserStore) DeleteUser(ctx context.Context, screenName IdentScreenName) error {
+	tx, err := f.db.BeginTx(ctx, nil)
+	if err != nil {
+		return err
+	}
+	defer tx.Rollback()
+
 	q := `
 		DELETE FROM users WHERE identScreenName = ?
 	`
-	result, err := f.db.ExecContext(ctx, q, screenName.String())
+	result, err := tx.ExecContext(ctx, q, screenName.String())
 	if err != nil {
 		return err
 	}
@@ -886,7 +894,14 @@ func (f SQLiteUserStore) DeleteUser(ctx context.Context, screenName IdentScreenN
 		return ErrNoUser
 	}
 
-	return nil
+	q = `
+		DELETE FROM feedbag WHERE classID = ? AND name = ?
+	`
+	if _, err := tx.ExecContext(ctx, q, wire.FeedbagClassIdAlInfo, screenName.String()); err != nil {
+		return err
+	}
+
+	return tx.Commit()
 }
 
 func (f SQLiteUserStore) SetUserPassword(ctx context.Context, screenName IdentScreenName, newPassword string) error {

state/user_store_test.go

@@ -61,6 +61,7 @@ func TestSQLiteUserStore_FeedbagUpsert(t *testing.T) {
 		}
 
 		me := NewIdentScreenName("me")
+		insertTestUsers(t, f, me)
 		if err := f.FeedbagUpsert(context.Background(), me, given); err != nil {
 			t.Fatalf("failed to upsert: %s", err.Error())
 		}
@@ -92,6 +93,7 @@ func TestSQLiteUserStore_FeedbagUpsert(t *testing.T) {
 		}
 
 		me := NewIdentScreenName("me")
+		insertTestUsers(t, f, me)
 		err = f.FeedbagUpsert(context.Background(), me, given)
 		assert.NoError(t, err)
 
@@ -119,6 +121,7 @@ func TestSQLiteUserStore_FeedbagUpsert(t *testing.T) {
 		}
 
 		me := NewIdentScreenName("me")
+		insertTestUsers(t, f, me)
 		err = f.FeedbagUpsert(context.Background(), me, given)
 		assert.NoError(t, err)
 
@@ -137,6 +140,7 @@ func TestSQLiteUserStore_FeedbagUpsert(t *testing.T) {
 		require.NoError(t, err)
 
 		me := NewIdentScreenName("me")
+		insertTestUsers(t, f, me)
 		err = f.FeedbagUpsert(context.Background(), me, []wire.FeedbagItem{
 			{
 				GroupID: 0,
@@ -170,6 +174,7 @@ func TestSQLiteUserStore_FeedbagUpsert(t *testing.T) {
 		require.NoError(t, err)
 
 		me := NewIdentScreenName("me")
+		insertTestUsers(t, f, me)
 		err = f.FeedbagUpsert(context.Background(), me, []wire.FeedbagItem{
 			{
 				GroupID:   0,
@@ -227,6 +232,7 @@ func TestFeedbagDelete(t *testing.T) {
 			},
 		}
 
+		insertTestUsers(t, f, screenName)
 		if err := f.FeedbagUpsert(context.Background(), screenName, itemsIn); err != nil {
 			t.Fatalf("failed to upsert: %s", err.Error())
 		}
@@ -270,6 +276,7 @@ func TestFeedbagDelete(t *testing.T) {
 			},
 		}
 
+		insertTestUsers(t, f, screenName)
 		if err := f.FeedbagUpsert(context.Background(), screenName, itemsIn); err != nil {
 			t.Fatalf("failed to upsert: %s", err.Error())
 		}
@@ -341,6 +348,7 @@ func TestLastModifiedNotEmpty(t *testing.T) {
 			Name:    "Friends",
 		},
 	}
+	insertTestUsers(t, f, screenName)
 	if err := f.FeedbagUpsert(context.Background(), screenName, itemsIn); err != nil {
 		t.Fatalf("failed to upsert: %s", err.Error())
 	}
@@ -678,6 +686,69 @@ func TestSQLiteUserStore_DeleteUser_DeleteExistentUser(t *testing.T) {
 	assert.Equal(t, want, have)
 }
 
+func TestSQLiteUserStore_DeleteUser_DeletesFeedbagAndLinks(t *testing.T) {
+	defer func() {
+		assert.NoError(t, os.Remove(testFile))
+	}()
+
+	f, err := NewSQLiteUserStore(testFile)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	userA := NewIdentScreenName("userA")
+	userB := NewIdentScreenName("userB")
+	userC := NewIdentScreenName("userC")
+	insertTestUsers(t, f, userA, userB, userC)
+
+	linkTo := func(itemID uint16, sn IdentScreenName) wire.FeedbagItem {
+		return wire.FeedbagItem{ItemID: itemID, ClassID: wire.FeedbagClassIdAlInfo, Name: sn.String()}
+	}
+	buddy := func(itemID uint16, sn IdentScreenName) wire.FeedbagItem {
+		return wire.FeedbagItem{GroupID: 1, ItemID: itemID, ClassID: wire.FeedbagClassIdBuddy, Name: sn.String()}
+	}
+
+	require.NoError(t, f.FeedbagUpsert(ctx, userA, []wire.FeedbagItem{linkTo(1, userB), buddy(2, userC)}))
+	require.NoError(t, f.FeedbagUpsert(ctx, userB, []wire.FeedbagItem{linkTo(1, userA), buddy(2, userC)}))
+	require.NoError(t, f.FeedbagUpsert(ctx, userC, []wire.FeedbagItem{linkTo(1, userB), buddy(2, userB)}))
+
+	require.NoError(t, f.DeleteUser(ctx, userB))
+
+	// the deleted user's own feedbag is gone
+	have, err := f.Feedbag(ctx, userB)
+	require.NoError(t, err)
+	assert.Empty(t, have)
+
+	// links to the deleted user are gone; other items, including a buddy item
+	// that names the deleted user, remain
+	have, err = f.Feedbag(ctx, userA)
+	require.NoError(t, err)
+	assert.Equal(t, []wire.FeedbagItem{buddy(2, userC)}, have)
+
+	have, err = f.Feedbag(ctx, userC)
+	require.NoError(t, err)
+	assert.Equal(t, []wire.FeedbagItem{buddy(2, userB)}, have)
+
+	// a re-registered screen name starts with an empty feedbag
+	insertTestUsers(t, f, userB)
+	have, err = f.Feedbag(ctx, userB)
+	require.NoError(t, err)
+	assert.Empty(t, have)
+}
+
+func TestSQLiteUserStore_FeedbagUpsert_RequiresUser(t *testing.T) {
+	defer func() {
+		assert.NoError(t, os.Remove(testFile))
+	}()
+
+	f, err := NewSQLiteUserStore(testFile)
+	require.NoError(t, err)
+
+	err = f.FeedbagUpsert(context.Background(), NewIdentScreenName("nobody"), []wire.FeedbagItem{
+		{GroupID: 1, ClassID: wire.FeedbagClassIdGroup, Name: "Friends"},
+	})
+	assert.ErrorContains(t, err, "FOREIGN KEY constraint failed")
+}
+
 func TestSQLiteUserStore_DeleteUser_DeleteNonExistentUser(t *testing.T) {
 	defer func() {
 		assert.NoError(t, os.Remove(testFile))
@@ -690,6 +761,19 @@ func TestSQLiteUserStore_DeleteUser_DeleteNonExistentUser(t *testing.T) {
 	assert.ErrorIs(t, ErrNoUser, err)
 }
 
+// insertTestUsers creates a minimal account for each screen name, since every
+// feedbag row must belong to a user.
+func insertTestUsers(t *testing.T, f *SQLiteUserStore, screenNames ...IdentScreenName) {
+	t.Helper()
+	for _, sn := range screenNames {
+		require.NoError(t, f.InsertUser(context.Background(), User{
+			IdentScreenName:   sn,
+			DisplayScreenName: DisplayScreenName(sn.String()),
+			IsICQ:             sn.UIN() != 0,
+		}))
+	}
+}
+
 func newFeedbagItem(classID uint16, itemID uint16, name string, feedbagPending bool) wire.FeedbagItem {
 	item := wire.FeedbagItem{
 		ClassID: classID,
@@ -2829,6 +2913,7 @@ func TestSQLiteUserStore_BuddyIconMetadataExistingRef(t *testing.T) {
 			},
 		},
 	}
+	insertTestUsers(t, feedbagStore, screenName)
 	if err := feedbagStore.FeedbagUpsert(context.Background(), screenName, itemsIn); err != nil {
 		t.Fatalf("failed to upsert: %s", err.Error())
 	}
@@ -2866,6 +2951,7 @@ func TestSQLiteUserStore_BuddyIconMetadataMissingRef(t *testing.T) {
 			},
 		},
 	}
+	insertTestUsers(t, feedbagStore, existingScreenName)
 	if err := feedbagStore.FeedbagUpsert(context.Background(), existingScreenName, itemsIn); err != nil {
 		t.Fatalf("failed to upsert: %s", err.Error())
 	}