handle remote account deletion more systematically (#254)

internal/api/s2s/user/inboxpost_test.go

@@ -322,7 +322,6 @@ func (suite *InboxPostTestSuite) TestPostUpdate() {
 	suite.EqualValues(updatedAccount.HeaderMediaAttachmentID, dbUpdatedAccount.HeaderMediaAttachmentID)
 	suite.EqualValues(updatedAccount.HeaderMediaAttachment, dbUpdatedAccount.HeaderMediaAttachment)
 	suite.EqualValues(updatedAccount.HeaderRemoteURL, dbUpdatedAccount.HeaderRemoteURL)
-	// suite.EqualValues(updatedAccount.Fields, dbUpdatedAccount.Fields)
 	suite.EqualValues(updatedAccount.Note, dbUpdatedAccount.Note)
 	suite.EqualValues(updatedAccount.Memorial, dbUpdatedAccount.Memorial)
 	suite.EqualValues(updatedAccount.AlsoKnownAs, dbUpdatedAccount.AlsoKnownAs)
@@ -343,7 +342,6 @@ func (suite *InboxPostTestSuite) TestPostUpdate() {
 	suite.EqualValues(updatedAccount.FollowersURI, dbUpdatedAccount.FollowersURI)
 	suite.EqualValues(updatedAccount.FeaturedCollectionURI, dbUpdatedAccount.FeaturedCollectionURI)
 	suite.EqualValues(updatedAccount.ActorType, dbUpdatedAccount.ActorType)
-	// suite.EqualValues(updatedAccount.PrivateKey, dbUpdatedAccount.PrivateKey)
 	suite.EqualValues(updatedAccount.PublicKey, dbUpdatedAccount.PublicKey)
 	suite.EqualValues(updatedAccount.PublicKeyURI, dbUpdatedAccount.PublicKeyURI)
 	suite.EqualValues(updatedAccount.SensitizedAt, dbUpdatedAccount.SensitizedAt)
@@ -353,6 +351,111 @@ func (suite *InboxPostTestSuite) TestPostUpdate() {
 	suite.EqualValues(updatedAccount.SuspensionOrigin, dbUpdatedAccount.SuspensionOrigin)
 }
 
+func (suite *InboxPostTestSuite) TestPostDelete() {
+	deletedAccount := *suite.testAccounts["remote_account_1"]
+	receivingAccount := suite.testAccounts["local_account_1"]
+
+	// create a delete
+	delete := streams.NewActivityStreamsDelete()
+
+	// set the appropriate actor on it
+	deleteActor := streams.NewActivityStreamsActorProperty()
+	deleteActor.AppendIRI(testrig.URLMustParse(deletedAccount.URI))
+	delete.SetActivityStreamsActor(deleteActor)
+
+	// Set the account iri as the 'object' property.
+	deleteObject := streams.NewActivityStreamsObjectProperty()
+	deleteObject.AppendIRI(testrig.URLMustParse(deletedAccount.URI))
+	delete.SetActivityStreamsObject(deleteObject)
+
+	// Set the To of the delete as public
+	deleteTo := streams.NewActivityStreamsToProperty()
+	deleteTo.AppendIRI(testrig.URLMustParse("https://www.w3.org/ns/activitystreams#Public"))
+	delete.SetActivityStreamsTo(deleteTo)
+
+	// set some random-ass ID for the activity
+	deleteID := streams.NewJSONLDIdProperty()
+	deleteID.SetIRI(testrig.URLMustParse("http://fossbros-anonymous.io/d360613a-dc8d-4563-8f0b-b6161caf0f2b"))
+	delete.SetJSONLDId(deleteID)
+
+	targetURI := testrig.URLMustParse(receivingAccount.InboxURI)
+
+	signature, digestHeader, dateHeader := testrig.GetSignatureForActivity(delete, deletedAccount.PublicKeyURI, deletedAccount.PrivateKey, targetURI)
+	bodyI, err := streams.Serialize(delete)
+	suite.NoError(err)
+
+	bodyJson, err := json.Marshal(bodyI)
+	suite.NoError(err)
+	body := bytes.NewReader(bodyJson)
+
+	tc := testrig.NewTestTransportController(testrig.NewMockHTTPClient(nil), suite.db)
+	federator := testrig.NewTestFederator(suite.db, tc, suite.storage)
+	processor := testrig.NewTestProcessor(suite.db, suite.storage, federator)
+	err = processor.Start(context.Background())
+	suite.NoError(err)
+	userModule := user.New(suite.config, processor, suite.log).(*user.Module)
+
+	// setup request
+	recorder := httptest.NewRecorder()
+	ctx, _ := gin.CreateTestContext(recorder)
+	ctx.Request = httptest.NewRequest(http.MethodPost, targetURI.String(), body) // the endpoint we're hitting
+	ctx.Request.Header.Set("Signature", signature)
+	ctx.Request.Header.Set("Date", dateHeader)
+	ctx.Request.Header.Set("Digest", digestHeader)
+	ctx.Request.Header.Set("Content-Type", "application/activity+json")
+
+	// we need to pass the context through signature check first to set appropriate values on it
+	suite.securityModule.SignatureCheck(ctx)
+
+	// normally the router would populate these params from the path values,
+	// but because we're calling the function directly, we need to set them manually.
+	ctx.Params = gin.Params{
+		gin.Param{
+			Key:   user.UsernameKey,
+			Value: receivingAccount.Username,
+		},
+	}
+
+	// trigger the function being tested
+	userModule.InboxPOSTHandler(ctx)
+	result := recorder.Result()
+	defer result.Body.Close()
+	b, err := ioutil.ReadAll(result.Body)
+	suite.NoError(err)
+	suite.Empty(b)
+	suite.Equal(http.StatusOK, result.StatusCode)
+
+	// sleep for a sec so side effects can process in the background
+	time.Sleep(2 * time.Second)
+
+	// local account 2 blocked foss_satan, that block should be gone now
+	testBlock := suite.testBlocks["local_account_2_block_remote_account_1"]
+	dbBlock := &gtsmodel.Block{}
+	err = suite.db.GetByID(ctx, testBlock.ID, dbBlock)
+	suite.ErrorIs(err, db.ErrNoEntries)
+
+	// no statuses from foss satan should be left in the database
+	dbStatuses, err := suite.db.GetAccountStatuses(ctx, deletedAccount.ID, 0, false, "", false, false)
+	suite.ErrorIs(err, db.ErrNoEntries)
+	suite.Empty(dbStatuses)
+
+	dbAccount, err := suite.db.GetAccountByID(ctx, deletedAccount.ID)
+	suite.NoError(err)
+
+	suite.Empty(dbAccount.Note)
+	suite.Empty(dbAccount.DisplayName)
+	suite.Empty(dbAccount.AvatarMediaAttachmentID)
+	suite.Empty(dbAccount.AvatarRemoteURL)
+	suite.Empty(dbAccount.HeaderMediaAttachmentID)
+	suite.Empty(dbAccount.HeaderRemoteURL)
+	suite.Empty(dbAccount.Reason)
+	suite.Empty(dbAccount.Fields)
+	suite.True(dbAccount.HideCollections)
+	suite.False(dbAccount.Discoverable)
+	suite.WithinDuration(time.Now(), dbAccount.SuspendedAt, 30*time.Second)
+	suite.Equal(dbAccount.ID, dbAccount.SuspensionOrigin)
+}
+
 func TestInboxPostTestSuite(t *testing.T) {
 	suite.Run(t, &InboxPostTestSuite{})
 }

internal/api/s2s/user/user_test.go

@@ -53,6 +53,7 @@ type UserStandardTestSuite struct {
 	testAccounts     map[string]*gtsmodel.Account
 	testAttachments  map[string]*gtsmodel.MediaAttachment
 	testStatuses     map[string]*gtsmodel.Status
+	testBlocks       map[string]*gtsmodel.Block
 
 	// module being tested
 	userModule *user.Module
@@ -66,6 +67,7 @@ func (suite *UserStandardTestSuite) SetupSuite() {
 	suite.testAccounts = testrig.NewTestAccounts()
 	suite.testAttachments = testrig.NewTestAttachments()
 	suite.testStatuses = testrig.NewTestStatuses()
+	suite.testBlocks = testrig.NewTestBlocks()
 }
 
 func (suite *UserStandardTestSuite) SetupTest() {

internal/federation/federatingdb/delete.go

@@ -89,10 +89,7 @@ func (f *federatingDB) Delete(ctx context.Context, id *url.URL) error {
 	a, err := f.db.GetAccountByURI(ctx, id.String())
 	if err == nil {
 		// it's an account
-		l.Debugf("uri is for an account with id: %s", a.ID)
-		if err := f.db.DeleteByID(ctx, a.ID, &gtsmodel.Account{}); err != nil {
-			return fmt.Errorf("DELETE: err deleting account: %s", err)
-		}
+		l.Debugf("uri is for an account with id %s, passing delete message to the processor", a.ID)
 		fromFederatorChan <- messages.FromFederator{
 			APObjectType:     ap.ObjectProfile,
 			APActivityType:   ap.ActivityDelete,

internal/processing/account/delete.go

@@ -31,7 +31,7 @@
 
 // Delete handles the complete deletion of an account.
 //
-// TODO in this function:
+// To be done in this function:
 // 1. Delete account's application(s), clients, and oauth tokens
 // 2. Delete account's blocks
 // 3. Delete account's emoji
@@ -51,12 +51,16 @@
 // 17. Delete account's timeline
 // 18. Delete account itself
 func (p *processor) Delete(ctx context.Context, account *gtsmodel.Account, origin string) error {
-	l := p.log.WithFields(logrus.Fields{
+	fields := logrus.Fields{
 		"func":     "Delete",
 		"username": account.Username,
-	})
+	}
+	if account.Domain != "" {
+		fields["domain"] = account.Domain
+	}
+	l := p.log.WithFields(fields)
 
-	l.Debugf("beginning account delete process for username %s", account.Username)
+	l.Debug("beginning account delete process")
 
 	// 1. Delete account's application(s), clients, and oauth tokens
 	// we only need to do this step for local account since remote ones won't have any tokens or applications on our server
@@ -214,10 +218,16 @@ func (p *processor) Delete(ctx context.Context, account *gtsmodel.Account, origi
 
 	// 10. Delete account's notifications
 	l.Debug("deleting account notifications")
+	// first notifications created by account
 	if err := p.db.DeleteWhere(ctx, []db.Where{{Key: "origin_account_id", Value: account.ID}}, &[]*gtsmodel.Notification{}); err != nil {
 		l.Errorf("error deleting notifications created by account: %s", err)
 	}
 
+	// now notifications targeting account
+	if err := p.db.DeleteWhere(ctx, []db.Where{{Key: "target_account_id", Value: account.ID}}, &[]*gtsmodel.Notification{}); err != nil {
+		l.Errorf("error deleting notifications targeting account: %s", err)
+	}
+
 	// 11. Delete account's bookmarks
 	l.Debug("deleting account bookmarks")
 	if err := p.db.DeleteWhere(ctx, []db.Where{{Key: "account_id", Value: account.ID}}, &[]*gtsmodel.StatusBookmark{}); err != nil {
@@ -267,8 +277,6 @@ func (p *processor) Delete(ctx context.Context, account *gtsmodel.Account, origi
 	account.HideCollections = true
 	account.Discoverable = false
 
-	account.UpdatedAt = time.Now()
-
 	account.SuspendedAt = time.Now()
 	account.SuspensionOrigin = origin
 

internal/processing/fromfederator.go

@@ -181,7 +181,13 @@ func (p *processor) ProcessFromFederator(ctx context.Context, federatorMsg messa
 			return p.deleteStatusFromTimelines(ctx, statusToDelete)
 		case ap.ObjectProfile:
 			// DELETE A PROFILE/ACCOUNT
-			// TODO: handle side effects of account deletion here: delete all objects, statuses, media etc associated with account
+			// handle side effects of account deletion here: delete all objects, statuses, media etc associated with account
+			account, ok := federatorMsg.GTSModel.(*gtsmodel.Account)
+			if !ok {
+				return errors.New("account delete was not parseable as *gtsmodel.Account")
+			}
+
+			return p.accountProcessor.Delete(ctx, account, account.ID)
 		}
 	case ap.ActivityAccept:
 		// ACCEPT

internal/processing/fromfederator_test.go

@@ -20,6 +20,7 @@
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -276,6 +277,86 @@ func (suite *FromFederatorTestSuite) TestProcessFaveWithDifferentReceivingAccoun
 	suite.Empty(stream.Messages)
 }
 
+func (suite *FromFederatorTestSuite) TestProcessAccountDelete() {
+	ctx := context.Background()
+
+	deletedAccount := suite.testAccounts["remote_account_1"]
+	receivingAccount := suite.testAccounts["local_account_1"]
+
+	// before doing the delete....
+	// make local_account_1 and remote_account_1 into mufos
+	zorkFollowSatan := &gtsmodel.Follow{
+		ID:              "01FGRY72ASHBSET64353DPHK9T",
+		CreatedAt:       time.Now().Add(-1 * time.Hour),
+		UpdatedAt:       time.Now().Add(-1 * time.Hour),
+		AccountID:       deletedAccount.ID,
+		TargetAccountID: receivingAccount.ID,
+		ShowReblogs:     true,
+		URI:             fmt.Sprintf("%s/follows/01FGRY72ASHBSET64353DPHK9T", deletedAccount.URI),
+		Notify:          false,
+	}
+	err := suite.db.Put(ctx, zorkFollowSatan)
+	suite.NoError(err)
+
+	satanFollowZork := &gtsmodel.Follow{
+		ID:              "01FGRYAVAWWPP926J175QGM0WV",
+		CreatedAt:       time.Now().Add(-1 * time.Hour),
+		UpdatedAt:       time.Now().Add(-1 * time.Hour),
+		AccountID:       receivingAccount.ID,
+		TargetAccountID: deletedAccount.ID,
+		ShowReblogs:     true,
+		URI:             fmt.Sprintf("%s/follows/01FGRYAVAWWPP926J175QGM0WV", receivingAccount.URI),
+		Notify:          false,
+	}
+	err = suite.db.Put(ctx, satanFollowZork)
+	suite.NoError(err)
+
+	// now they are mufos!
+
+	err = suite.processor.ProcessFromFederator(ctx, messages.FromFederator{
+		APObjectType:     ap.ObjectProfile,
+		APActivityType:   ap.ActivityDelete,
+		GTSModel:         deletedAccount,
+		ReceivingAccount: receivingAccount,
+	})
+	suite.NoError(err)
+
+	// local account 2 blocked foss_satan, that block should be gone now
+	testBlock := suite.testBlocks["local_account_2_block_remote_account_1"]
+	dbBlock := &gtsmodel.Block{}
+	err = suite.db.GetByID(ctx, testBlock.ID, dbBlock)
+	suite.ErrorIs(err, db.ErrNoEntries)
+
+	// the mufos should be gone now too
+	satanFollowsZork, err := suite.db.IsFollowing(ctx, deletedAccount, receivingAccount)
+	suite.NoError(err)
+	suite.False(satanFollowsZork)
+	zorkFollowsSatan, err := suite.db.IsFollowing(ctx, receivingAccount, deletedAccount)
+	suite.NoError(err)
+	suite.False(zorkFollowsSatan)
+
+	// no statuses from foss satan should be left in the database
+	dbStatuses, err := suite.db.GetAccountStatuses(ctx, deletedAccount.ID, 0, false, "", false, false)
+	suite.ErrorIs(err, db.ErrNoEntries)
+	suite.Empty(dbStatuses)
+
+	dbAccount, err := suite.db.GetAccountByID(ctx, deletedAccount.ID)
+	suite.NoError(err)
+
+	suite.Empty(dbAccount.Note)
+	suite.Empty(dbAccount.DisplayName)
+	suite.Empty(dbAccount.AvatarMediaAttachmentID)
+	suite.Empty(dbAccount.AvatarRemoteURL)
+	suite.Empty(dbAccount.HeaderMediaAttachmentID)
+	suite.Empty(dbAccount.HeaderRemoteURL)
+	suite.Empty(dbAccount.Reason)
+	suite.Empty(dbAccount.Fields)
+	suite.True(dbAccount.HideCollections)
+	suite.False(dbAccount.Discoverable)
+	suite.WithinDuration(time.Now(), dbAccount.SuspendedAt, 30*time.Second)
+	suite.Equal(dbAccount.ID, dbAccount.SuspensionOrigin)
+}
+
 func TestFromFederatorTestSuite(t *testing.T) {
 	suite.Run(t, &FromFederatorTestSuite{})
 }

internal/processing/processor_test.go

@@ -62,6 +62,7 @@ type ProcessingStandardTestSuite struct {
 	testTags         map[string]*gtsmodel.Tag
 	testMentions     map[string]*gtsmodel.Mention
 	testAutheds      map[string]*oauth.Auth
+	testBlocks       map[string]*gtsmodel.Block
 
 	processor processing.Processor
 }
@@ -83,6 +84,7 @@ func (suite *ProcessingStandardTestSuite) SetupSuite() {
 			Account:     suite.testAccounts["local_account_1"],
 		},
 	}
+	suite.testBlocks = testrig.NewTestBlocks()
 }
 
 func (suite *ProcessingStandardTestSuite) SetupTest() {