Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] Allow delivery to sharedInboxes where possible #847

Merged
merged 13 commits into from
Sep 23, 2022
13 changes: 13 additions & 0 deletions docs/configuration/instance.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
13 changes: 13 additions & 0 deletions example/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 #####
###########################
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
30 changes: 30 additions & 0 deletions internal/ap/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 6 additions & 0 deletions internal/ap/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type Accountable interface {
WithFollowers
WithFeatured
WithManuallyApprovesFollowers
WithEndpoints
}

// Statusable represents the minimum activitypub interface for representing a 'status'.
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions internal/cache/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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."`
Expand Down
5 changes: 3 additions & 2 deletions internal/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions internal/config/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
25 changes: 25 additions & 0 deletions internal/config/helpers.gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}
34 changes: 30 additions & 4 deletions internal/federation/dereferencing/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,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)
Expand Down Expand Up @@ -279,6 +281,30 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar
}
}

// check if we need to look for a sharedInbox for the account
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)
tsmethurst marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err)
return
}
}

// only set a shared inbox for this account if it's on the same host or domain as the account
var sharedInbox string
if sharedInboxURI := ap.ExtractSharedInbox(accountable); sharedInboxURI != nil {
if sharedInboxURI.Host == params.RemoteAccountID.Host || sharedInboxURI.Host == foundAccount.Domain {
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
Expand All @@ -293,7 +319,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 {
Expand Down
18 changes: 18 additions & 0 deletions internal/federation/dereferencing/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
2 changes: 1 addition & 1 deletion internal/federation/federatingactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 18 additions & 2 deletions internal/federation/federatingdb/inbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
tsmethurst marked this conversation as resolved.
Show resolved Hide resolved
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)
}
Expand All @@ -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)
}
Expand Down
21 changes: 21 additions & 0 deletions internal/federation/federatingdb/inbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
}
1 change: 1 addition & 0 deletions internal/gtsmodel/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ type Account struct {
URL string `validate:"required_without=Domain,omitempty,url" bun:",nullzero,unique"` // Web URL for this account's profile
LastWebfingeredAt time.Time `validate:"required_with=Domain" bun:"type:timestamptz,nullzero"` // Last time this account was refreshed/located with webfinger.
InboxURI string `validate:"required_without=Domain,omitempty,url" bun:",nullzero,unique"` // Address of this account's ActivityPub inbox, for sending activity to
SharedInboxURI *string `validate:"-" bun:""` // Address of this account's ActivityPub sharedInbox, if it has one. If we don't yet know, this will be nil. If it doesn't have one, will be an empty string.
OutboxURI string `validate:"required_without=Domain,omitempty,url" bun:",nullzero,unique"` // Address of this account's activitypub outbox
FollowingURI string `validate:"required_without=Domain,omitempty,url" bun:",nullzero,unique"` // URI for getting the following list of this account
FollowersURI string `validate:"required_without=Domain,omitempty,url" bun:",nullzero,unique"` // URI for getting the followers list of this account
Expand Down
Loading