[feature] Handle retractions of domain permission subscription entries (#4261)

# Description > If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements. > > If this is a documentation change, please briefly describe what you've changed and why. This pull request adds logic for nicely handling retractions of entries from domain permission subscriptions. See docs for how this works but basically retracted entries will either be removed (and possibly picked up by a lower-prio subscription), or orphaned (and then possibly adopted), depending on the config of the domain permission subscription. closes https://codeberg.org/superseriousbusiness/gotosocial/issues/4101 ## Checklist Please put an x inside each checkbox to indicate that you've read and followed it: `[ ]` -> `[x]` If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want). - [x] I/we have read the [GoToSocial contribution guidelines](https://codeberg.org/superseriousbusiness/gotosocial/src/branch/main/CONTRIBUTING.md). - [x] I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat. - [x] I/we have not leveraged AI to create the proposed changes. - [x] I/we have performed a self-review of added code. - [x] I/we have written code that is legible and maintainable by others. - [x] I/we have commented the added code, particularly in hard-to-understand areas. - [x] I/we have made any necessary changes to documentation. - [x] I/we have added tests that cover new code. - [ ] I/we have run tests and they pass locally with the changes. - [x] I/we have run `go fmt ./...` and `golangci-lint run`. Reviewed-on: https://codeberg.org/superseriousbusiness/gotosocial/pulls/4261 Co-authored-by: tobi <tobi.smethurst@protonmail.com> Co-committed-by: tobi <tobi.smethurst@protonmail.com>
2025-10-29 07:02:26 -05:00 · 2025-06-15 12:36:51 +02:00 · 2025-06-15 12:36:51 +02:00 · a9b2d4ee35
commit a9b2d4ee35
parent 76e7fd62d2
19 changed files with 662 additions and 34 deletions
--- a/internal/subscriptions/domainperms.go
+++ b/internal/subscriptions/domainperms.go
@ -166,17 +166,19 @@ func (s *Subscriptions) ProcessDomainPermissionSubscriptions(
 		return
 	}

+	var skipCache bool
 	for i, permSub := range permSubs {
 		// Higher priority permission subs = everything
 		// above this permission sub in the slice.
 		higherPrios := permSubs[:i]

-		_, err := s.ProcessDomainPermissionSubscription(
+		_, retracted, err := s.ProcessDomainPermissionSubscription(
 			ctx,
 			permSub,
 			tsport,
 			higherPrios,
-			false, // Not dry. Wet, if you will.
+			false,     // Not dry. Wet, if you will.
+			skipCache, // Skip cache if necessary.
 		)
 		if err != nil {
 			// Real db error.
@ -187,6 +189,16 @@ func (s *Subscriptions) ProcessDomainPermissionSubscriptions(
 			return
 		}

+		// If any retractions have been done, skip caching
+		// when doing subsequent fetches. This makes it so
+		// that if an entry was present in a higher-priority
+		// list and a lower-priority list, but was retracted
+		// from the higher-priority list, it will be created
+		// and managed by the lower-priority list instead.
+		if retracted && !skipCache {
+			skipCache = true
+		}
+
 		// Update this perm sub.
 		err = s.state.DB.UpdateDomainPermissionSubscription(ctx, permSub)
 		if err != nil {
@ -203,7 +215,10 @@ func (s *Subscriptions) ProcessDomainPermissionSubscriptions(
 // entry in the database, or ignoring it if it's excluded or already
 // covered by a higher-priority subscription.
 //
-// On success, the slice of discovered DomainPermissions will be returned.
+// On success, the slice of discovered DomainPermissions will be returned,
+// including a boolean to indicate whether or not any retractions have been
+// performed since the list was last checked (if ever).
+//
 // In case of parsing error, or error on the remote side, permSub.Error
 // will be updated with the calling/parsing error, and `nil, nil` will be
 // returned. In case of an actual db error, `nil, err` will be returned and
@ -215,6 +230,10 @@ func (s *Subscriptions) ProcessDomainPermissionSubscriptions(
 // If dry == true, then the URI will still be called, and permissions
 // will be parsed, but they will not actually be created.
 //
+// If skipCache == true, then conditional HTTP request headers will not be
+// sent, and so cached values for the domain permission subscription list
+// will not be used (ie., the list will always be fetched "fresh").
+//
 // Note that while this function modifies fields on the given permSub,
 // it's up to the caller to update it in the database (if desired).
 func (s *Subscriptions) ProcessDomainPermissionSubscription(
@ -223,7 +242,8 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription(
 	tsport transport.Transport,
 	higherPrios []*gtsmodel.DomainPermissionSubscription,
 	dry bool,
-) ([]gtsmodel.DomainPermission, error) {
+	skipCache bool,
+) ([]gtsmodel.DomainPermission, bool, error) {
 	l := log.
 		WithContext(ctx).
 		WithFields(kv.Fields{
@ -235,10 +255,10 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription(
 	// going to attempt this now.
 	permSub.FetchedAt = time.Now()

-	// Call the URI, and only skip
-	// cache if we're doing a dry run.
+	// Call the URI, skipping conditional requests
+	// (caching) if we've been told to do so.
 	resp, err := tsport.DereferenceDomainPermissions(
-		ctx, permSub, dry,
+		ctx, permSub, skipCache,
 	)
 	if err != nil {
 		// Couldn't get this one,
@ -246,7 +266,7 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription(
 		errStr := err.Error()
 		l.Warnf("couldn't dereference permSubURI: %+v", err)
 		permSub.Error = errStr
-		return nil, nil
+		return nil, false, nil
 	}

 	// If the permissions at URI weren't modified
@ -257,7 +277,7 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription(
 		permSub.ETag = resp.ETag
 		permSub.LastModified = resp.LastModified
 		permSub.SuccessfullyFetchedAt = permSub.FetchedAt
-		return nil, nil
+		return nil, false, nil
 	}

 	// At this point we know we got a 200 OK
@ -289,7 +309,7 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription(
 		errStr := err.Error()
 		l.Warnf("couldn't parse results: %+v", err)
 		permSub.Error = errStr
-		return nil, nil
+		return nil, false, nil
 	}

 	if len(wantedPerms) == 0 {
@ -299,7 +319,7 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription(
 		const errStr = "fetch successful but parsed zero usable results"
 		l.Warn(errStr)
 		permSub.Error = errStr
-		return nil, nil
+		return nil, false, nil
 	}

 	// This can now be considered a successful fetch.
@ -325,7 +345,7 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription(
 		)
 		if err != nil {
 			// Proper db error.
-			return nil, err
+			return nil, false, err
 		}

 		if !created {
@ -335,7 +355,30 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription(
 		createdPerms = append(createdPerms, wantedPerm)
 	}

-	return createdPerms, nil
+	if dry {
+		// Don't do any further
+		// processing with a dry run.
+		return createdPerms, false, nil
+	}
+
+	// Process any retractions since
+	// the last time list was checked.
+	//
+	// Being unable to do retractions
+	// isn't the end of the world as it
+	// can be tried again next time the
+	// list is updated, so if there was
+	// an error just warn it.
+	retracted, err := s.processRetractions(
+		ctx, l,
+		permSub,
+		wantedPerms,
+	)
+	if err != nil {
+		l.Warnf("error doing retractions: %+v", err)
+	}
+
+	return createdPerms, retracted, nil
 }

 // processDomainPermission processes one wanted domain
@ -895,3 +938,150 @@ func (s *Subscriptions) adoptPerm(

 	return err
 }
+
+func (s *Subscriptions) processRetractions(
+	ctx context.Context,
+	l log.Entry,
+	permSub *gtsmodel.DomainPermissionSubscription,
+	wantedPerms []gtsmodel.DomainPermission,
+) (bool, error) {
+	var (
+		isBlocks        = permSub.PermissionType == gtsmodel.DomainPermissionBlock
+		removeRetracted = *permSub.RemoveRetracted
+
+		// True if at least one
+		// retraction has occurred.
+		retracted bool
+	)
+
+	// Gather existing perms into an interface type.
+	existingPerms := []gtsmodel.DomainPermission{}
+	if isBlocks {
+		existingBlocks, err := s.state.DB.GetDomainBlocksBySubscriptionID(ctx, permSub.ID)
+		if err != nil && !errors.Is(err, db.ErrNoEntries) {
+			// Proper db error.
+			err := gtserror.Newf("db error getting existing blocks owned by perm sub: %w", err)
+			return retracted, err
+		}
+		for _, existingBlock := range existingBlocks {
+			existingPerms = append(existingPerms, existingBlock)
+		}
+	} else {
+		existingAllows, err := s.state.DB.GetDomainAllowsBySubscriptionID(ctx, permSub.ID)
+		if err != nil && !errors.Is(err, db.ErrNoEntries) {
+			// Proper db error.
+			err := gtserror.Newf("db error getting existing allows owned by perm sub: %w", err)
+			return retracted, err
+		}
+		for _, existingAllow := range existingAllows {
+			existingPerms = append(existingPerms, existingAllow)
+		}
+	}
+
+	// For each existing permission, check if
+	// it's included in the list of wanted perms
+	// that's just been freshly fetched + created.
+	//
+	// If it's not, we should consider it retracted
+	// and handle retraction effects appropriately.
+	for _, existingPerm := range existingPerms {
+		if slices.ContainsFunc(
+			wantedPerms,
+			func(wantedPerm gtsmodel.DomainPermission) bool {
+				return existingPerm.GetDomain() == wantedPerm.GetDomain()
+			},
+		) {
+			// This permission from the
+			// database exists in wanted
+			// perms, so it's not been
+			// retracted, leave it alone.
+			continue
+		}
+
+		// This perm exists in the database but
+		// not in wanted perms, so it has been
+		// retracted, check what we need to do.
+		domain := existingPerm.GetDomain()
+		l.WithField("domain", domain).Info("handling retraction")
+
+		var (
+			dbF     func() error
+			action  *gtsmodel.AdminAction
+			actionF admin.ActionF
+		)
+
+		switch {
+
+		// Remove this block.
+		case isBlocks && removeRetracted:
+			dbF = func() error { return s.state.DB.DeleteDomainBlock(ctx, domain) }
+			action = &gtsmodel.AdminAction{
+				ID:             id.NewULID(),
+				TargetCategory: gtsmodel.AdminActionCategoryDomain,
+				TargetID:       domain,
+				Type:           gtsmodel.AdminActionUnsuspend,
+				AccountID:      permSub.CreatedByAccountID,
+			}
+			actionF = s.state.AdminActions.DomainUnblockF(
+				action.ID,
+				existingPerm.(*gtsmodel.DomainBlock),
+			)
+
+		// Remove this allow.
+		case !isBlocks && removeRetracted:
+			dbF = func() error { return s.state.DB.DeleteDomainAllow(ctx, domain) }
+			action = &gtsmodel.AdminAction{
+				ID:             id.NewULID(),
+				TargetCategory: gtsmodel.AdminActionCategoryDomain,
+				TargetID:       domain,
+				Type:           gtsmodel.AdminActionUnallow,
+				AccountID:      permSub.CreatedByAccountID,
+			}
+			actionF = s.state.AdminActions.DomainUnallowF(
+				action.ID,
+				existingPerm.(*gtsmodel.DomainAllow),
+			)
+
+		// Orphan this block.
+		case isBlocks:
+			block := existingPerm.(*gtsmodel.DomainBlock)
+			block.SubscriptionID = ""
+			dbF = func() error { return s.state.DB.UpdateDomainBlock(ctx, block, "subscription_id") }
+
+		// Orphan this allow.
+		case !isBlocks:
+			allow := existingPerm.(*gtsmodel.DomainAllow)
+			allow.SubscriptionID = ""
+			dbF = func() error { return s.state.DB.UpdateDomainAllow(ctx, allow, "subscription_id") }
+		}
+
+		// Run the retraction db
+		// func to either delete
+		// or update the perm.
+		if err := dbF(); err != nil {
+			return retracted, err
+		}
+
+		// Mark that at least one
+		// retraction has been done.
+		if !retracted {
+			retracted = true
+		}
+
+		if action == nil {
+			// No side effects;
+			// nothing else to do.
+			continue
+		}
+
+		// Run the side effects.
+		if err := s.state.AdminActions.Run(ctx, action, actionF); err != nil {
+			err := gtserror.Newf("error running side effects: %w", err)
+			return retracted, err
+		}
+
+		// TODO: Remove draft(s) as well?
+	}
+
+	return retracted, nil
+}
--- a/internal/subscriptions/subscriptions_test.go
+++ b/internal/subscriptions/subscriptions_test.go
@ -949,6 +949,138 @@ func (suite *SubscriptionsTestSuite) TestDomainAllowsAndBlocks() {
 	suite.Equal(existingAllow.SubscriptionID, testAllowSubscription.ID)
 }

+func (suite *SubscriptionsTestSuite) TestRemoveRetraction() {
+	var (
+		ctx           = suite.T().Context()
+		testStructs   = testrig.SetupTestStructs(rMediaPath, rTemplatePath)
+		testAccount   = suite.testAccounts["admin_account"]
+		subscriptions = subscriptions.New(
+			testStructs.State,
+			testStructs.TransportController,
+			testStructs.TypeConverter,
+		)
+
+		// A subscription for a plain list of
+		// baddies, which removes retracted entries.
+		testSubscription = &gtsmodel.DomainPermissionSubscription{
+			ID:                 "01JGE681TQSBPAV59GZXPKE62H",
+			Priority:           255,
+			Title:              "whatever!",
+			PermissionType:     gtsmodel.DomainPermissionBlock,
+			AsDraft:            util.Ptr(false),
+			AdoptOrphans:       util.Ptr(false),
+			CreatedByAccountID: testAccount.ID,
+			CreatedByAccount:   testAccount,
+			URI:                "https://lists.example.org/baddies.txt",
+			ContentType:        gtsmodel.DomainPermSubContentTypePlain,
+			RemoveRetracted:    util.Ptr(true),
+		}
+
+		// Block owned by testSubscription
+		// that no longer exists on the remote
+		// list, ie., it's been retracted.
+		retractedBlock = &gtsmodel.DomainBlock{
+			ID:                 "01JHX2V5WN250TKB6FQ1M3QE1H",
+			Domain:             "retracted.example.org",
+			CreatedByAccount:   testAccount,
+			CreatedByAccountID: testAccount.ID,
+			SubscriptionID:     "01JGE681TQSBPAV59GZXPKE62H",
+		}
+	)
+	defer testrig.TearDownTestStructs(testStructs)
+
+	// Store test subscription.
+	if err := testStructs.State.DB.PutDomainPermissionSubscription(
+		ctx, testSubscription,
+	); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// Store the retracted block.
+	if err := testStructs.State.DB.PutDomainBlock(
+		ctx, retractedBlock,
+	); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// Process subscriptions.
+	subscriptions.ProcessDomainPermissionSubscriptions(ctx, testSubscription.PermissionType)
+
+	// Retracted block should be removed.
+	if !testrig.WaitFor(func() bool {
+		_, err := testStructs.State.DB.GetDomainBlock(ctx, retractedBlock.Domain)
+		return errors.Is(err, db.ErrNoEntries)
+	}) {
+		suite.FailNow("timed out waiting for block to be removed")
+	}
+}
+
+func (suite *SubscriptionsTestSuite) TestOrphanRetraction() {
+	var (
+		ctx           = suite.T().Context()
+		testStructs   = testrig.SetupTestStructs(rMediaPath, rTemplatePath)
+		testAccount   = suite.testAccounts["admin_account"]
+		subscriptions = subscriptions.New(
+			testStructs.State,
+			testStructs.TransportController,
+			testStructs.TypeConverter,
+		)
+
+		// A subscription for a plain list of
+		// baddies, which orphans retracted entries.
+		testSubscription = &gtsmodel.DomainPermissionSubscription{
+			ID:                 "01JGE681TQSBPAV59GZXPKE62H",
+			Priority:           255,
+			Title:              "whatever!",
+			PermissionType:     gtsmodel.DomainPermissionBlock,
+			AsDraft:            util.Ptr(false),
+			AdoptOrphans:       util.Ptr(false),
+			CreatedByAccountID: testAccount.ID,
+			CreatedByAccount:   testAccount,
+			URI:                "https://lists.example.org/baddies.txt",
+			ContentType:        gtsmodel.DomainPermSubContentTypePlain,
+			RemoveRetracted:    util.Ptr(false),
+		}
+
+		// Block owned by testSubscription
+		// that no longer exists on the remote
+		// list, ie., it's been retracted.
+		retractedBlock = &gtsmodel.DomainBlock{
+			ID:                 "01JHX2V5WN250TKB6FQ1M3QE1H",
+			Domain:             "retracted.example.org",
+			CreatedByAccount:   testAccount,
+			CreatedByAccountID: testAccount.ID,
+			SubscriptionID:     "01JGE681TQSBPAV59GZXPKE62H",
+		}
+	)
+	defer testrig.TearDownTestStructs(testStructs)
+
+	// Store test subscription.
+	if err := testStructs.State.DB.PutDomainPermissionSubscription(
+		ctx, testSubscription,
+	); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// Store the retracted block.
+	if err := testStructs.State.DB.PutDomainBlock(
+		ctx, retractedBlock,
+	); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// Process subscriptions.
+	subscriptions.ProcessDomainPermissionSubscriptions(ctx, testSubscription.PermissionType)
+
+	// Retracted block should be orphaned.
+	if !testrig.WaitFor(func() bool {
+		block, err := testStructs.State.DB.GetDomainBlock(ctx, retractedBlock.Domain)
+		return err == nil && block.SubscriptionID == ""
+	}) {
+		suite.FailNow("timed out waiting for block to be orphaned")
+	}
+}
+
 func TestSubscriptionTestSuite(t *testing.T) {
 	suite.Run(t, new(SubscriptionsTestSuite))
 }