[feature] Allow delivery to sharedInboxes where possible (#847)

* update Activity

* add instance-deliver-to-shared-inboxes setting

* update activity version again

* add SharedInboxURI field to accounts

* serdes for endpoints/sharedInbox

* deliver to sharedInbox if one is available

* update tests

* only assign shared inbox if shared domain

* look for shared inbox if currently nil

* go fmt

* finger to get params.RemoteAccountID if necessary

* make comments clearer

* compare dns more consistently

docs/configuration/instance.md

@@ -23,4 +23,17 @@ instance-expose-peers: false
 # Options: [true, false]
 # Default: false
 instance-expose-suspended: false
+
+# Bool. This flag tweaks whether GoToSocial will deliver ActivityPub messages
+# to the shared inbox of a recipient, if one is available, instead of delivering
+# each message to each actor who should receive a message individually.
+#
+# Shared inbox delivery can significantly reduce network load when delivering
+# to multiple recipients share an inbox (eg., on large Mastodon instances).
+#
+# See: https://www.w3.org/TR/activitypub/#shared-inbox-delivery
+#
+# Options: [true, false]
+# Default: true
+instance-deliver-to-shared-inboxes: true
 ```

example/config.yaml

@@ -186,6 +186,19 @@ instance-expose-peers: false
 # Default: false
 instance-expose-suspended: false
 
+# Bool. This flag tweaks whether GoToSocial will deliver ActivityPub messages
+# to the shared inbox of a recipient, if one is available, instead of delivering
+# each message to each actor who should receive a message individually.
+#
+# Shared inbox delivery can significantly reduce network load when delivering
+# to multiple recipients share an inbox (eg., on large Mastodon instances).
+#
+# See: https://www.w3.org/TR/activitypub/#shared-inbox-delivery
+#
+# Options: [true, false]
+# Default: true
+instance-deliver-to-shared-inboxes: true
+
 ###########################
 ##### ACCOUNTS CONFIG #####
 ###########################

go.mod

@@ -38,11 +38,11 @@ require (
 	github.com/spf13/cobra v1.4.0
 	github.com/spf13/viper v1.11.0
 	github.com/stretchr/testify v1.8.0
-	github.com/superseriousbusiness/activity v1.1.0-gts
+	github.com/superseriousbusiness/activity v1.2.1-gts
 	github.com/superseriousbusiness/exif-terminator v0.4.0
 	github.com/superseriousbusiness/oauth2/v4 v4.3.2-SSB
-	github.com/ulule/limiter/v3 v3.10.0
 	github.com/tdewolff/minify/v2 v2.12.0
+	github.com/ulule/limiter/v3 v3.10.0
 	github.com/uptrace/bun v1.1.7
 	github.com/uptrace/bun/dialect/pgdialect v1.1.7
 	github.com/uptrace/bun/dialect/sqlitedialect v1.1.7

go.sum

@@ -511,8 +511,8 @@ github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PK
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
-github.com/superseriousbusiness/activity v1.1.0-gts h1:BSnMzs/84s0Zme7BngE9iJAHV7g1Bv1nhLCP0aJtU3I=
-github.com/superseriousbusiness/activity v1.1.0-gts/go.mod h1:AZw0Xb4Oju8rmaJCZ21gc5CPg47MmNgyac+Hx5jo8VM=
+github.com/superseriousbusiness/activity v1.2.1-gts h1:wh7v0zYa1mJmqB35PSfvgl4cs51Dh5PyfKvcZLSxMQU=
+github.com/superseriousbusiness/activity v1.2.1-gts/go.mod h1:AZw0Xb4Oju8rmaJCZ21gc5CPg47MmNgyac+Hx5jo8VM=
 github.com/superseriousbusiness/exif-terminator v0.4.0 h1:pzAg7luCi8oc2LVDwgTLvTinh/+/2UuWgJZrM8MMaT4=
 github.com/superseriousbusiness/exif-terminator v0.4.0/go.mod h1:OPfOSEDWjXaW3BILJBN89j0VLD8bglmHwHHwwwSLb5A=
 github.com/superseriousbusiness/go-jpeg-image-structure/v2 v2.0.0-20220321154430-d89a106fdabe h1:ksl2oCx/Qo8sNDc3Grb8WGKBM9nkvhCm25uvlT86azE=

internal/ap/extract.go

@@ -702,3 +702,33 @@ func ExtractSensitive(withSensitive WithSensitive) bool {
 
 	return false
 }
+
+// ExtractSharedInbox extracts the sharedInbox URI properly from an Actor.
+// Returns nil if this property is not set.
+func ExtractSharedInbox(withEndpoints WithEndpoints) *url.URL {
+	endpointsProp := withEndpoints.GetActivityStreamsEndpoints()
+	if endpointsProp == nil {
+		return nil
+	}
+
+	for iter := endpointsProp.Begin(); iter != endpointsProp.End(); iter = iter.Next() {
+		if iter.IsActivityStreamsEndpoints() {
+			endpoints := iter.Get()
+			if endpoints == nil {
+				return nil
+			}
+			sharedInboxProp := endpoints.GetActivityStreamsSharedInbox()
+			if sharedInboxProp == nil {
+				return nil
+			}
+
+			if !sharedInboxProp.IsIRI() {
+				return nil
+			}
+
+			return sharedInboxProp.GetIRI()
+		}
+	}
+
+	return nil
+}

internal/ap/interfaces.go

@@ -40,6 +40,7 @@ type Accountable interface {
 	WithFollowers
 	WithFeatured
 	WithManuallyApprovesFollowers
+	WithEndpoints
 }
 
 // Statusable represents the minimum activitypub interface for representing a 'status'.
@@ -337,3 +338,8 @@ type WithItems interface {
 type WithManuallyApprovesFollowers interface {
 	GetActivityStreamsManuallyApprovesFollowers() vocab.ActivityStreamsManuallyApprovesFollowersProperty
 }
+
+// WithEndpoints represents a Person or profile with the endpoints property
+type WithEndpoints interface {
+	GetActivityStreamsEndpoints() vocab.ActivityStreamsEndpointsProperty
+}

internal/cache/account.go

@@ -136,6 +136,7 @@ func copyAccount(account *gtsmodel.Account) *gtsmodel.Account {
 		URL:                     account.URL,
 		LastWebfingeredAt:       account.LastWebfingeredAt,
 		InboxURI:                account.InboxURI,
+		SharedInboxURI:          account.SharedInboxURI,
 		OutboxURI:               account.OutboxURI,
 		FollowingURI:            account.FollowingURI,
 		FollowersURI:            account.FollowersURI,

internal/config/config.go

@@ -67,8 +67,9 @@ type Configuration struct {
 	WebTemplateBaseDir string `name:"web-template-base-dir" usage:"Basedir for html templating files for rendering pages and composing emails."`
 	WebAssetBaseDir    string `name:"web-asset-base-dir" usage:"Directory to serve static assets from, accessible at example.org/assets/"`
 
-	InstanceExposePeers     bool `name:"instance-expose-peers" usage:"Allow unauthenticated users to query /api/v1/instance/peers?filter=open"`
-	InstanceExposeSuspended bool `name:"instance-expose-suspended" usage:"Expose suspended instances via web UI, and allow unauthenticated users to query /api/v1/instance/peers?filter=suspended"`
+	InstanceExposePeers            bool `name:"instance-expose-peers" usage:"Allow unauthenticated users to query /api/v1/instance/peers?filter=open"`
+	InstanceExposeSuspended        bool `name:"instance-expose-suspended" usage:"Expose suspended instances via web UI, and allow unauthenticated users to query /api/v1/instance/peers?filter=suspended"`
+	InstanceDeliverToSharedInboxes bool `name:"instance-deliver-to-shared-inboxes" usage:"Deliver federated messages to shared inboxes, if they're available."`
 
 	AccountsRegistrationOpen bool `name:"accounts-registration-open" usage:"Allow anyone to submit an account signup request. If false, server will be invite-only."`
 	AccountsApprovalRequired bool `name:"accounts-approval-required" usage:"Do account signups require approval by an admin or moderator before user can log in? If false, new registrations will be automatically approved."`

internal/config/defaults.go

@@ -46,8 +46,9 @@ var Defaults = Configuration{
 	WebTemplateBaseDir: "./web/template/",
 	WebAssetBaseDir:    "./web/assets/",
 
-	InstanceExposePeers:     false,
-	InstanceExposeSuspended: false,
+	InstanceExposePeers:            false,
+	InstanceExposeSuspended:        false,
+	InstanceDeliverToSharedInboxes: true,
 
 	AccountsRegistrationOpen: true,
 	AccountsApprovalRequired: true,

internal/config/flags.go

@@ -63,6 +63,7 @@ func AddServerFlags(cmd *cobra.Command) {
 		// Instance
 		cmd.Flags().Bool(InstanceExposePeersFlag(), cfg.InstanceExposePeers, fieldtag("InstanceExposePeers", "usage"))
 		cmd.Flags().Bool(InstanceExposeSuspendedFlag(), cfg.InstanceExposeSuspended, fieldtag("InstanceExposeSuspended", "usage"))
+		cmd.Flags().Bool(InstanceDeliverToSharedInboxesFlag(), cfg.InstanceDeliverToSharedInboxes, fieldtag("InstanceDeliverToSharedInboxes", "usage"))
 
 		// Accounts
 		cmd.Flags().Bool(AccountsRegistrationOpenFlag(), cfg.AccountsRegistrationOpen, fieldtag("AccountsRegistrationOpen", "usage"))

internal/config/helpers.gen.go

@@ -593,6 +593,31 @@ func GetInstanceExposeSuspended() bool { return global.GetInstanceExposeSuspende
 // SetInstanceExposeSuspended safely sets the value for global configuration 'InstanceExposeSuspended' field
 func SetInstanceExposeSuspended(v bool) { global.SetInstanceExposeSuspended(v) }
 
+// GetInstanceDeliverToSharedInboxes safely fetches the Configuration value for state's 'InstanceDeliverToSharedInboxes' field
+func (st *ConfigState) GetInstanceDeliverToSharedInboxes() (v bool) {
+	st.mutex.Lock()
+	v = st.config.InstanceDeliverToSharedInboxes
+	st.mutex.Unlock()
+	return
+}
+
+// SetInstanceDeliverToSharedInboxes safely sets the Configuration value for state's 'InstanceDeliverToSharedInboxes' field
+func (st *ConfigState) SetInstanceDeliverToSharedInboxes(v bool) {
+	st.mutex.Lock()
+	defer st.mutex.Unlock()
+	st.config.InstanceDeliverToSharedInboxes = v
+	st.reloadToViper()
+}
+
+// InstanceDeliverToSharedInboxesFlag returns the flag name for the 'InstanceDeliverToSharedInboxes' field
+func InstanceDeliverToSharedInboxesFlag() string { return "instance-deliver-to-shared-inboxes" }
+
+// GetInstanceDeliverToSharedInboxes safely fetches the value for global configuration 'InstanceDeliverToSharedInboxes' field
+func GetInstanceDeliverToSharedInboxes() bool { return global.GetInstanceDeliverToSharedInboxes() }
+
+// SetInstanceDeliverToSharedInboxes safely sets the value for global configuration 'InstanceDeliverToSharedInboxes' field
+func SetInstanceDeliverToSharedInboxes(v bool) { global.SetInstanceDeliverToSharedInboxes(v) }
+
 // GetAccountsRegistrationOpen safely fetches the Configuration value for state's 'AccountsRegistrationOpen' field
 func (st *ConfigState) GetAccountsRegistrationOpen() (v bool) {
 	st.mutex.Lock()

internal/db/bundb/migrations/20220922142408_shared_inbox_delivery.go

@@ -0,0 +1,46 @@
+/*
+   GoToSocial
+   Copyright (C) 2021-2022 GoToSocial Authors admin@gotosocial.org
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU Affero General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU Affero General Public License for more details.
+
+   You should have received a copy of the GNU Affero General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+package migrations
+
+import (
+	"context"
+	"strings"
+
+	"github.com/uptrace/bun"
+)
+
+func init() {
+	up := func(ctx context.Context, db *bun.DB) error {
+		_, err := db.ExecContext(ctx, "ALTER TABLE ? ADD COLUMN ? TEXT", bun.Ident("accounts"), bun.Ident("shared_inbox_uri"))
+		if err != nil && !(strings.Contains(err.Error(), "already exists") || strings.Contains(err.Error(), "duplicate column name") || strings.Contains(err.Error(), "SQLSTATE 42701")) {
+			return err
+		}
+		return nil
+	}
+
+	down := func(ctx context.Context, db *bun.DB) error {
+		return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error {
+			return nil
+		})
+	}
+
+	if err := Migrations.Register(up, down); err != nil {
+		panic(err)
+	}
+}

internal/federation/dereferencing/account.go

@@ -29,6 +29,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/miekg/dns"
 	"github.com/superseriousbusiness/activity/streams"
 	"github.com/superseriousbusiness/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
@@ -197,10 +198,12 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar
 		accountDomain = params.RemoteAccountHost
 	}
 
-	// to save on remote calls: only webfinger if we don't have a remoteAccount yet, or if we haven't
-	// fingered the remote account for at least 2 days; don't finger instance accounts
+	// to save on remote calls, only webfinger if:
+	// - we don't know the remote account ActivityPub ID yet OR
+	// - we haven't found the account yet in some other way OR
+	// - we haven't webfingered the account for two days AND the account isn't an instance account
 	var fingered time.Time
-	if foundAccount == nil || (foundAccount.LastWebfingeredAt.Before(time.Now().Add(webfingerInterval)) && !instanceAccount(foundAccount)) {
+	if params.RemoteAccountID == nil || foundAccount == nil || (foundAccount.LastWebfingeredAt.Before(time.Now().Add(webfingerInterval)) && !instanceAccount(foundAccount)) {
 		accountDomain, params.RemoteAccountID, err = d.fingerRemoteAccount(ctx, params.RequestingUsername, params.RemoteAccountUsername, params.RemoteAccountHost)
 		if err != nil {
 			err = fmt.Errorf("GetRemoteAccount: error while fingering: %s", err)
@@ -279,6 +282,37 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar
 		}
 	}
 
+	// if SharedInboxURI is nil, that means we don't know yet if this account has
+	// a shared inbox available for it, so we need to check this here
+	var sharedInboxChanged bool
+	if foundAccount.SharedInboxURI == nil {
+		// we need the accountable for this, so get it if we don't have it yet
+		if accountable == nil {
+			accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID)
+			if err != nil {
+				err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err)
+				return
+			}
+		}
+
+		// This can be:
+		// - an empty string (we know it doesn't have a shared inbox) OR
+		// - a string URL (we know it does a shared inbox).
+		// Set it either way!
+		var sharedInbox string
+
+		if sharedInboxURI := ap.ExtractSharedInbox(accountable); sharedInboxURI != nil {
+			// only trust shared inbox if it has at least two domains,
+			// from the right, in common with the domain of the account
+			if dns.CompareDomainName(foundAccount.Domain, sharedInboxURI.Host) >= 2 {
+				sharedInbox = sharedInboxURI.String()
+			}
+		}
+
+		sharedInboxChanged = true
+		foundAccount.SharedInboxURI = &sharedInbox
+	}
+
 	// make sure the account fields are populated before returning:
 	// the caller might want to block until everything is loaded
 	var fieldsChanged bool
@@ -293,7 +327,7 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar
 		foundAccount.LastWebfingeredAt = fingered
 	}
 
-	if fieldsChanged || fingeredChanged {
+	if fieldsChanged || fingeredChanged || sharedInboxChanged {
 		foundAccount.UpdatedAt = time.Now()
 		foundAccount, err = d.db.UpdateAccount(ctx, foundAccount)
 		if err != nil {

internal/federation/dereferencing/account_test.go

@@ -101,6 +101,24 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsRemoteURL() {
 	suite.Empty(fetchedAccount.Domain)
 }
 
+func (suite *AccountTestSuite) TestDereferenceLocalAccountAsRemoteURLNoSharedInboxYet() {
+	fetchingAccount := suite.testAccounts["local_account_1"]
+	targetAccount := suite.testAccounts["local_account_2"]
+
+	targetAccount.SharedInboxURI = nil
+	if _, err := suite.db.UpdateAccount(context.Background(), targetAccount); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{
+		RequestingUsername: fetchingAccount.Username,
+		RemoteAccountID:    testrig.URLMustParse(targetAccount.URI),
+	})
+	suite.NoError(err)
+	suite.NotNil(fetchedAccount)
+	suite.Empty(fetchedAccount.Domain)
+}
+
 func (suite *AccountTestSuite) TestDereferenceLocalAccountAsUsername() {
 	fetchingAccount := suite.testAccounts["local_account_1"]
 	targetAccount := suite.testAccounts["local_account_2"]

internal/federation/federatingactor_test.go

@@ -117,7 +117,7 @@ func (suite *FederatingActorTestSuite) TestSendRemoteFollower() {
 	// because we added 1 remote follower for zork, there should be a url in sentMessage
 	var sent [][]byte
 	if !testrig.WaitFor(func() bool {
-		sentI, ok := httpClient.SentMessages.Load(testRemoteAccount.InboxURI)
+		sentI, ok := httpClient.SentMessages.Load(*testRemoteAccount.SharedInboxURI)
 		if ok {
 			sent, ok = sentI.([][]byte)
 			if !ok {

internal/federation/federatingdb/inbox.go

@@ -108,7 +108,15 @@ func (f *federatingDB) InboxesForIRI(c context.Context, iri *url.URL) (inboxIRIs
 				follow.Account = followingAccount
 			}
 
-			inboxIRI, err := url.Parse(follow.Account.InboxURI)
+			// deliver to a shared inbox if we have that option
+			var inbox string
+			if config.GetInstanceDeliverToSharedInboxes() && follow.Account.SharedInboxURI != nil && *follow.Account.SharedInboxURI != "" {
+				inbox = *follow.Account.SharedInboxURI
+			} else {
+				inbox = follow.Account.InboxURI
+			}
+
+			inboxIRI, err := url.Parse(inbox)
 			if err != nil {
 				return nil, fmt.Errorf("error parsing inbox uri of following account %s: %s", follow.Account.InboxURI, err)
 			}
@@ -119,7 +127,15 @@ func (f *federatingDB) InboxesForIRI(c context.Context, iri *url.URL) (inboxIRIs
 
 	// check if this is just an account IRI...
 	if account, err := f.db.GetAccountByURI(c, iri.String()); err == nil {
-		inboxIRI, err := url.Parse(account.InboxURI)
+		// deliver to a shared inbox if we have that option
+		var inbox string
+		if config.GetInstanceDeliverToSharedInboxes() && account.SharedInboxURI != nil && *account.SharedInboxURI != "" {
+			inbox = *account.SharedInboxURI
+		} else {
+			inbox = account.InboxURI
+		}
+
+		inboxIRI, err := url.Parse(inbox)
 		if err != nil {
 			return nil, fmt.Errorf("error parsing account inbox uri %s: %s", account.InboxURI, account.InboxURI)
 		}

internal/federation/federatingdb/inbox_test.go

@@ -63,6 +63,27 @@ func (suite *InboxTestSuite) TestInboxesForAccountIRI() {
 	suite.Contains(asStrings, suite.testAccounts["local_account_1"].InboxURI)
 }
 
+func (suite *InboxTestSuite) TestInboxesForAccountIRIWithSharedInbox() {
+	ctx := context.Background()
+	testAccount := suite.testAccounts["local_account_1"]
+	sharedInbox := "http://some-inbox-iri/weeeeeeeeeeeee"
+	testAccount.SharedInboxURI = &sharedInbox
+	if _, err := suite.db.UpdateAccount(ctx, testAccount); err != nil {
+		suite.FailNow("error updating account")
+	}
+
+	inboxIRIs, err := suite.federatingDB.InboxesForIRI(ctx, testrig.URLMustParse(testAccount.URI))
+	suite.NoError(err)
+
+	asStrings := []string{}
+	for _, i := range inboxIRIs {
+		asStrings = append(asStrings, i.String())
+	}
+
+	suite.Len(asStrings, 1)
+	suite.Contains(asStrings, "http://some-inbox-iri/weeeeeeeeeeeee")
+}
+
 func TestInboxTestSuite(t *testing.T) {
 	suite.Run(t, &InboxTestSuite{})
 }