[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

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{})
 }