From 9265a09a656196e2a94c73e32c7b79399411a79e Mon Sep 17 00:00:00 2001
From: Adelie Paull <1208865+i-am-a-paull@users.noreply.github.com>
Date: Mon, 2 May 2022 09:23:37 -0400
Subject: [PATCH] [bugfix] Allow self-boosting for any visibility but direct
(#510)
* create visibility filter for boostability and allow self-boosting for any visbility but direct messages
* add a followers-only status to local_account_2
* fix typo in comment
* add license header, unwrap errors, be explicit about non-boostable visibility settings to avoid rogue boosting from miscoded clients, use ID compare for checking if self-boosting
* add tests for statusboostable filter
* fix tests that were affected by adding a new status to the test data
* fix the rest of tests affected by adding a status to the textrig data
---
internal/db/bundb/basic_test.go | 2 +-
internal/processing/status/boost.go | 9 +-
internal/timeline/get_test.go | 16 +-
internal/timeline/index_test.go | 4 +-
internal/timeline/manager_test.go | 10 +-
internal/visibility/filter.go | 5 +
internal/visibility/statusboostable.go | 65 ++++++++
internal/visibility/statusboostable_test.go | 155 ++++++++++++++++++++
testrig/testmodels.go | 24 +++
9 files changed, 268 insertions(+), 22 deletions(-)
create mode 100644 internal/visibility/statusboostable.go
create mode 100644 internal/visibility/statusboostable_test.go
diff --git a/internal/db/bundb/basic_test.go b/internal/db/bundb/basic_test.go
index 2719f1bb8..f3c4580c8 100644
--- a/internal/db/bundb/basic_test.go
+++ b/internal/db/bundb/basic_test.go
@@ -43,7 +43,7 @@ func (suite *BasicTestSuite) TestGetAllStatuses() {
s := []*gtsmodel.Status{}
err := suite.db.GetAll(context.Background(), &s)
suite.NoError(err)
- suite.Len(s, 15)
+ suite.Len(s, 16)
}
func (suite *BasicTestSuite) TestGetAllNotNull() {
diff --git a/internal/processing/status/boost.go b/internal/processing/status/boost.go
index b222544ca..b1abc017c 100644
--- a/internal/processing/status/boost.go
+++ b/internal/processing/status/boost.go
@@ -39,14 +39,11 @@ func (p *processor) Boost(ctx context.Context, requestingAccount *gtsmodel.Accou
return nil, gtserror.NewErrorNotFound(fmt.Errorf("no status owner for status %s", targetStatusID))
}
- visible, err := p.filter.StatusVisible(ctx, targetStatus, requestingAccount)
+ boostable, err := p.filter.StatusBoostable(ctx, targetStatus, requestingAccount)
if err != nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("error seeing if status %s is visible: %s", targetStatus.ID, err))
+ return nil, gtserror.NewErrorNotFound(fmt.Errorf("error seeing if status %s is boostable: %s", targetStatus.ID, err))
}
- if !visible {
- return nil, gtserror.NewErrorNotFound(errors.New("status is not visible"))
- }
- if !targetStatus.Boostable {
+ if !boostable {
return nil, gtserror.NewErrorForbidden(errors.New("status is not boostable"))
}
diff --git a/internal/timeline/get_test.go b/internal/timeline/get_test.go
index 8659efc5a..dacf391fb 100644
--- a/internal/timeline/get_test.go
+++ b/internal/timeline/get_test.go
@@ -84,8 +84,8 @@ func (suite *GetTestSuite) TestGetDefault() {
suite.FailNow(err.Error())
}
- // we only have 15 statuses in the test suite
- suite.Len(statuses, 15)
+ // we only have 16 statuses in the test suite
+ suite.Len(statuses, 16)
// statuses should be sorted highest to lowest ID
var highest string
@@ -177,8 +177,8 @@ func (suite *GetTestSuite) TestGetMinID() {
suite.FailNow(err.Error())
}
- // we should only get 8 statuses back, since we asked for a min ID that excludes some of our entries
- suite.Len(statuses, 8)
+ // we should only get 9 statuses back, since we asked for a min ID that excludes some of our entries
+ suite.Len(statuses, 9)
// statuses should be sorted highest to lowest ID
var highest string
@@ -199,8 +199,8 @@ func (suite *GetTestSuite) TestGetSinceID() {
suite.FailNow(err.Error())
}
- // we should only get 8 statuses back, since we asked for a since ID that excludes some of our entries
- suite.Len(statuses, 8)
+ // we should only get 9 statuses back, since we asked for a since ID that excludes some of our entries
+ suite.Len(statuses, 9)
// statuses should be sorted highest to lowest ID
var highest string
@@ -221,8 +221,8 @@ func (suite *GetTestSuite) TestGetSinceIDPrepareNext() {
suite.FailNow(err.Error())
}
- // we should only get 8 statuses back, since we asked for a since ID that excludes some of our entries
- suite.Len(statuses, 8)
+ // we should only get 9 statuses back, since we asked for a since ID that excludes some of our entries
+ suite.Len(statuses, 9)
// statuses should be sorted highest to lowest ID
var highest string
diff --git a/internal/timeline/index_test.go b/internal/timeline/index_test.go
index acefe2b4c..b08fbf296 100644
--- a/internal/timeline/index_test.go
+++ b/internal/timeline/index_test.go
@@ -76,7 +76,7 @@ func (suite *IndexTestSuite) TestIndexBeforeLowID() {
postID, err := suite.timeline.OldestIndexedItemID(context.Background())
suite.NoError(err)
- suite.Equal("01F8MHBBN8120SYH7D5S050MGK", postID)
+ suite.Equal("01F8MHBQCBTDKN6X5VHGMMN4MA", postID)
indexLength := suite.timeline.ItemIndexLength(context.Background())
suite.Equal(9, indexLength)
@@ -105,7 +105,7 @@ func (suite *IndexTestSuite) TestIndexBehindHighID() {
// the newest indexed post should be the highest one we have in our testrig
postID, err := suite.timeline.NewestIndexedItemID(context.Background())
suite.NoError(err)
- suite.Equal("01FN3VJGFH10KR7S2PB0GFJZYG", postID)
+ suite.Equal("01G20ZM733MGN8J344T4ZDDFY1", postID)
// indexLength should be 9 because that's all this user has hometimelineable
indexLength := suite.timeline.ItemIndexLength(context.Background())
diff --git a/internal/timeline/manager_test.go b/internal/timeline/manager_test.go
index 11b5dfb78..efa4ee261 100644
--- a/internal/timeline/manager_test.go
+++ b/internal/timeline/manager_test.go
@@ -77,9 +77,9 @@ func (suite *ManagerTestSuite) TestManagerIntegration() {
err = suite.manager.PrepareXFromTop(context.Background(), testAccount.ID, 20)
suite.NoError(err)
- // local_account_1 can see 14 statuses out of the testrig statuses in its home timeline
+ // local_account_1 can see 15 statuses out of the testrig statuses in its home timeline
indexedLen = suite.manager.GetIndexedLength(context.Background(), testAccount.ID)
- suite.Equal(14, indexedLen)
+ suite.Equal(15, indexedLen)
// oldest should now be set
oldestIndexed, err = suite.manager.GetOldestIndexedID(context.Background(), testAccount.ID)
@@ -89,7 +89,7 @@ func (suite *ManagerTestSuite) TestManagerIntegration() {
// get hometimeline
statuses, err := suite.manager.GetTimeline(context.Background(), testAccount.ID, "", "", "", 20, false)
suite.NoError(err)
- suite.Len(statuses, 14)
+ suite.Len(statuses, 15)
// now wipe the last status from all timelines, as though it had been deleted by the owner
err = suite.manager.WipeItemFromAllTimelines(context.Background(), "01F8MH75CBF9JFX4ZAD54N0W0R")
@@ -97,7 +97,7 @@ func (suite *ManagerTestSuite) TestManagerIntegration() {
// timeline should be shorter
indexedLen = suite.manager.GetIndexedLength(context.Background(), testAccount.ID)
- suite.Equal(13, indexedLen)
+ suite.Equal(14, indexedLen)
// oldest should now be different
oldestIndexed, err = suite.manager.GetOldestIndexedID(context.Background(), testAccount.ID)
@@ -111,7 +111,7 @@ func (suite *ManagerTestSuite) TestManagerIntegration() {
// timeline should be shorter
indexedLen = suite.manager.GetIndexedLength(context.Background(), testAccount.ID)
- suite.Equal(12, indexedLen)
+ suite.Equal(13, indexedLen)
// oldest should now be different
oldestIndexed, err = suite.manager.GetOldestIndexedID(context.Background(), testAccount.ID)
diff --git a/internal/visibility/filter.go b/internal/visibility/filter.go
index c8cd13681..3455b11b1 100644
--- a/internal/visibility/filter.go
+++ b/internal/visibility/filter.go
@@ -45,6 +45,11 @@ type Filter interface {
//
// This function will call StatusVisible internally, so it's not necessary to call it beforehand.
StatusPublictimelineable(ctx context.Context, targetStatus *gtsmodel.Status, timelineOwnerAccount *gtsmodel.Account) (bool, error)
+
+ // StatusBoostable returns true if targetStatus can be boosted by the requesting account.
+ //
+ // this function will call StatusVisible internally so it's not necessary to call it beforehand.
+ StatusBoostable(ctx context.Context, targetStatus *gtsmodel.Status, requestingAccount *gtsmodel.Account) (bool, error)
}
type filter struct {
diff --git a/internal/visibility/statusboostable.go b/internal/visibility/statusboostable.go
new file mode 100644
index 000000000..9eed9e3e9
--- /dev/null
+++ b/internal/visibility/statusboostable.go
@@ -0,0 +1,65 @@
+/*
+ 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 .
+*/
+
+package visibility
+
+import (
+ "context"
+ "errors"
+ "fmt"
+
+ "github.com/sirupsen/logrus"
+ "github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+)
+
+func (f *filter) StatusBoostable(ctx context.Context, targetStatus *gtsmodel.Status, requestingAccount *gtsmodel.Account) (bool, error) {
+ l := logrus.WithFields(logrus.Fields{
+ "func": "StatusBoostable",
+ })
+
+ // if the status isn't visible, it certainly isn't boostable
+ visible, err := f.StatusVisible(ctx, targetStatus, requestingAccount)
+ if err != nil {
+ return false, fmt.Errorf("error seeing if status %s is visible: %s", targetStatus.ID, err)
+ }
+ if !visible {
+ return false, errors.New("status is not visible")
+ }
+
+ // direct messages are never boostable, even if they're visible
+ if targetStatus.Visibility == gtsmodel.VisibilityDirect {
+ l.Trace("status is not boostable because it is a DM")
+ return false, nil
+ }
+
+ // the original account should always be able to boost its own non-DM statuses
+ if requestingAccount.ID == targetStatus.Account.ID {
+ l.Trace("status is boostable because author is booster")
+ return true, nil
+ }
+
+ // if status is followers-only and not the author's, it is not boostable
+ if targetStatus.Visibility == gtsmodel.VisibilityFollowersOnly {
+ l.Trace("status not boostable because it is followers-only")
+ return false, nil
+ }
+
+ // otherwise, status is as boostable as it says it is
+ l.Trace("defaulting to status.boostable value")
+ return targetStatus.Boostable, nil
+}
diff --git a/internal/visibility/statusboostable_test.go b/internal/visibility/statusboostable_test.go
new file mode 100644
index 000000000..f39467676
--- /dev/null
+++ b/internal/visibility/statusboostable_test.go
@@ -0,0 +1,155 @@
+/*
+ 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 .
+*/
+
+package visibility_test
+
+import (
+ "context"
+ "testing"
+
+ "github.com/stretchr/testify/suite"
+)
+
+type StatusBoostableTestSuite struct {
+ FilterStandardTestSuite
+}
+
+func (suite *StatusBoostableTestSuite) TestOwnPublicBoostable() {
+ testStatus := suite.testStatuses["local_account_1_status_1"]
+ testAccount := suite.testAccounts["local_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.True(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOwnUnlockedBoostable() {
+ testStatus := suite.testStatuses["local_account_1_status_2"]
+ testAccount := suite.testAccounts["local_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.True(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOwnMutualsOnlyNonInteractiveBoostable() {
+ testStatus := suite.testStatuses["local_account_1_status_3"]
+ testAccount := suite.testAccounts["local_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.True(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOwnMutualsOnlyBoostable() {
+ testStatus := suite.testStatuses["local_account_1_status_4"]
+ testAccount := suite.testAccounts["local_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.True(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOwnFollowersOnlyBoostable() {
+ testStatus := suite.testStatuses["local_account_1_status_5"]
+ testAccount := suite.testAccounts["local_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.True(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOwnDirectNotBoostable() {
+ testStatus := suite.testStatuses["local_account_2_status_6"]
+ testAccount := suite.testAccounts["local_account_2"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.False(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOtherPublicBoostable() {
+ testStatus := suite.testStatuses["local_account_2_status_1"]
+ testAccount := suite.testAccounts["local_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.True(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOtherUnlistedBoostable() {
+ testStatus := suite.testStatuses["local_account_1_status_2"]
+ testAccount := suite.testAccounts["local_account_2"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.True(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOtherFollowersOnlyNotBoostable() {
+ testStatus := suite.testStatuses["local_account_2_status_7"]
+ testAccount := suite.testAccounts["local_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.False(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestOtherDirectNotBoostable() {
+ testStatus := suite.testStatuses["local_account_2_status_6"]
+ testAccount := suite.testAccounts["local_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.NoError(err)
+
+ suite.False(boostable)
+}
+
+func (suite *StatusBoostableTestSuite) TestRemoteFollowersOnlyNotVisibleError() {
+ testStatus := suite.testStatuses["local_account_1_status_5"]
+ testAccount := suite.testAccounts["remote_account_1"]
+ ctx := context.Background()
+
+ boostable, err := suite.filter.StatusBoostable(ctx, testStatus, testAccount)
+ suite.Assert().Error(err)
+
+ suite.False(boostable)
+}
+
+func TestStatusBoostableTestSuite(t *testing.T) {
+ suite.Run(t, new(StatusBoostableTestSuite))
+}
diff --git a/testrig/testmodels.go b/testrig/testmodels.go
index 949bfa7bf..cc32aa39e 100644
--- a/testrig/testmodels.go
+++ b/testrig/testmodels.go
@@ -1263,6 +1263,30 @@ func NewTestStatuses() map[string]*gtsmodel.Status {
Likeable: true,
ActivityStreamsType: ap.ObjectNote,
},
+ "local_account_2_status_7": {
+ ID: "01G20ZM733MGN8J344T4ZDDFY1",
+ URI: "http://localhost:8080/users/1happyturtle/statuses/01G20ZM733MGN8J344T4ZDDFY1",
+ URL: "http://localhost:8080/@1happyturtle/statuses/01G20ZM733MGN8J344T4ZDDFY1",
+ Content: "🐢 hi followers! did u know i'm a turtle? 🐢",
+ AttachmentIDs: []string{},
+ CreatedAt: time.Now().Add(-1 * time.Minute),
+ UpdatedAt: time.Now().Add(-1 * time.Minute),
+ Local: true,
+ AccountURI: "http://localhost:8080/users/1happyturtle",
+ AccountID: "01F8MH5NBDF2MV7CTC4Q5128HF",
+ InReplyToID: "",
+ BoostOfID: "",
+ ContentWarning: "",
+ Visibility: gtsmodel.VisibilityFollowersOnly,
+ Sensitive: false,
+ Language: "en",
+ CreatedWithApplicationID: "01F8MGYG9E893WRHW0TAEXR8GJ",
+ Federated: true,
+ Boostable: true,
+ Replyable: true,
+ Likeable: true,
+ ActivityStreamsType: ap.ObjectNote,
+ },
"remote_account_1_status_1": {
ID: "01FVW7JHQFSFK166WWKR8CBA6M",
URI: "http://fossbros-anonymous.io/users/foss_satan/statuses/01FVW7JHQFSFK166WWKR8CBA6M",