[bugfix] Allow instance accounts to be shown in search results in certain circumstances (#2053)

internal/api/client/search/searchget_test.go

@@ -19,6 +19,8 @@
 
 import (
 	"context"
+	"crypto/rand"
+	"crypto/rsa"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -30,6 +32,7 @@
 	"testing"
 
 	"github.com/stretchr/testify/suite"
+	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/internal/api/client/search"
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
 	apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
@@ -1001,7 +1004,7 @@ func (suite *SearchGetTestSuite) TestSearchAAccounts() {
 	suite.Len(searchResult.Hashtags, 0)
 }
 
-func (suite *SearchGetTestSuite) TestSearchAAccountsLimit1() {
+func (suite *SearchGetTestSuite) TestSearchAccountsLimit1() {
 	var (
 		requestingAccount          = suite.testAccounts["local_account_1"]
 		token                      = suite.testTokens["local_account_1"]
@@ -1078,12 +1081,14 @@ func (suite *SearchGetTestSuite) TestSearchLocalInstanceAccountByURI() {
 		suite.FailNow(err.Error())
 	}
 
-	suite.Len(searchResult.Accounts, 0)
+	// Should be able to get instance
+	// account by exact URI.
+	suite.Len(searchResult.Accounts, 1)
 	suite.Len(searchResult.Statuses, 0)
 	suite.Len(searchResult.Hashtags, 0)
 }
 
-func (suite *SearchGetTestSuite) TestSearchInstanceAccountFull() {
+func (suite *SearchGetTestSuite) TestSearchLocalInstanceAccountFull() {
 	// Namestring excludes ':' in usernames, so we
 	// need to fiddle with the instance account a
 	// bit to get it to look like a different domain.
@@ -1125,12 +1130,14 @@ func (suite *SearchGetTestSuite) TestSearchInstanceAccountFull() {
 		suite.FailNow(err.Error())
 	}
 
-	suite.Len(searchResult.Accounts, 0)
+	// Should be able to get instance
+	// account by full namestring.
+	suite.Len(searchResult.Accounts, 1)
 	suite.Len(searchResult.Statuses, 0)
 	suite.Len(searchResult.Hashtags, 0)
 }
 
-func (suite *SearchGetTestSuite) TestSearchInstanceAccountPartial() {
+func (suite *SearchGetTestSuite) TestSearchLocalInstanceAccountPartial() {
 	// Namestring excludes ':' in usernames, so we
 	// need to fiddle with the instance account a
 	// bit to get it to look like a different domain.
@@ -1172,6 +1179,131 @@ func (suite *SearchGetTestSuite) TestSearchInstanceAccountPartial() {
 		suite.FailNow(err.Error())
 	}
 
+	// Query was a partial namestring from our
+	// instance, so will return the instance account.
+	suite.Len(searchResult.Accounts, 1)
+	suite.Len(searchResult.Statuses, 0)
+	suite.Len(searchResult.Hashtags, 0)
+}
+
+func (suite *SearchGetTestSuite) TestSearchLocalInstanceAccountEvenMorePartial() {
+	// Namestring excludes ':' in usernames, so we
+	// need to fiddle with the instance account a
+	// bit to get it to look like a different domain.
+	newDomain := "example.org"
+	suite.bodgeLocalInstance(newDomain)
+
+	var (
+		requestingAccount          = suite.testAccounts["local_account_1"]
+		token                      = suite.testTokens["local_account_1"]
+		user                       = suite.testUsers["local_account_1"]
+		maxID              *string = nil
+		minID              *string = nil
+		limit              *int    = nil
+		offset             *int    = nil
+		resolve            *bool   = nil
+		query                      = newDomain
+		queryType          *string = nil
+		following          *bool   = nil
+		expectedHTTPStatus         = http.StatusOK
+		expectedBody               = ""
+	)
+
+	searchResult, err := suite.getSearch(
+		requestingAccount,
+		token,
+		apiutil.APIv2,
+		user,
+		maxID,
+		minID,
+		limit,
+		offset,
+		query,
+		queryType,
+		resolve,
+		following,
+		expectedHTTPStatus,
+		expectedBody)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// Query was just 'example.org' which doesn't
+	// look like a namestring, so search should
+	// fall back to text search and therefore give
+	// 0 results back.
+	suite.Len(searchResult.Accounts, 0)
+	suite.Len(searchResult.Statuses, 0)
+	suite.Len(searchResult.Hashtags, 0)
+}
+
+func (suite *SearchGetTestSuite) TestSearchRemoteInstanceAccountPartial() {
+	// Insert an instance account that's not
+	// from our instance, and try to search
+	// for it with a partial namestring.
+	theirDomain := "example.org"
+
+	key, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	if err := suite.db.PutAccount(context.Background(), &gtsmodel.Account{
+		ID:                    "01H6RWPG8T6DNW6VNXPBCJBH5S",
+		Username:              theirDomain,
+		Domain:                theirDomain,
+		URI:                   "http://" + theirDomain + "/users/" + theirDomain,
+		URL:                   "http://" + theirDomain + "/@" + theirDomain,
+		PublicKeyURI:          "http://" + theirDomain + "/users/" + theirDomain + "#main-key",
+		InboxURI:              "http://" + theirDomain + "/users/" + theirDomain + "/inbox",
+		OutboxURI:             "http://" + theirDomain + "/users/" + theirDomain + "/outbox",
+		FollowersURI:          "http://" + theirDomain + "/users/" + theirDomain + "/followers",
+		FollowingURI:          "http://" + theirDomain + "/users/" + theirDomain + "/following",
+		FeaturedCollectionURI: "http://" + theirDomain + "/users/" + theirDomain + "/collections/featured",
+		ActorType:             ap.ActorPerson,
+		PrivateKey:            key,
+		PublicKey:             &key.PublicKey,
+	}); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	var (
+		requestingAccount          = suite.testAccounts["local_account_1"]
+		token                      = suite.testTokens["local_account_1"]
+		user                       = suite.testUsers["local_account_1"]
+		maxID              *string = nil
+		minID              *string = nil
+		limit              *int    = nil
+		offset             *int    = nil
+		resolve            *bool   = nil
+		query                      = "@" + theirDomain
+		queryType          *string = nil
+		following          *bool   = nil
+		expectedHTTPStatus         = http.StatusOK
+		expectedBody               = ""
+	)
+
+	searchResult, err := suite.getSearch(
+		requestingAccount,
+		token,
+		apiutil.APIv2,
+		user,
+		maxID,
+		minID,
+		limit,
+		offset,
+		query,
+		queryType,
+		resolve,
+		following,
+		expectedHTTPStatus,
+		expectedBody)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// Search for instance account from
+	// another domain should return 0 results.
 	suite.Len(searchResult.Accounts, 0)
 	suite.Len(searchResult.Statuses, 0)
 	suite.Len(searchResult.Hashtags, 0)

internal/processing/search/accounts.go

@@ -49,6 +49,13 @@ func (p *Processor) Accounts(
 	resolve bool,
 	following bool,
 ) ([]*apimodel.Account, gtserror.WithCode) {
+	// Don't include instance accounts in this search.
+	//
+	// We don't want someone to start typing '@mastodon'
+	// and then get a million instance service accounts
+	// in their search results.
+	const includeInstanceAccounts = false
+
 	var (
 		foundAccounts = make([]*gtsmodel.Account, 0, limit)
 		appendAccount = func(foundAccount *gtsmodel.Account) { foundAccounts = append(foundAccounts, foundAccount) }
@@ -83,7 +90,12 @@ func (p *Processor) Accounts(
 	// if caller supplied an offset greater than 0, return
 	// nothing as though there were no additional results.
 	if offset > 0 {
-		return p.packageAccounts(ctx, requestingAccount, foundAccounts)
+		return p.packageAccounts(
+			ctx,
+			requestingAccount,
+			foundAccounts,
+			includeInstanceAccounts,
+		)
 	}
 
 	// Return all accounts we can find that match the
@@ -106,5 +118,10 @@ func (p *Processor) Accounts(
 	}
 
 	// Return whatever we got (if anything).
-	return p.packageAccounts(ctx, requestingAccount, foundAccounts)
+	return p.packageAccounts(
+		ctx,
+		requestingAccount,
+		foundAccounts,
+		includeInstanceAccounts,
+	)
 }

internal/processing/search/get.go

@@ -70,6 +70,13 @@ func (p *Processor) Get(
 		queryType = strings.TrimSpace(strings.ToLower(req.QueryType)) // Trim trailing/leading whitespace; convert to lowercase.
 		resolve   = req.Resolve
 		following = req.Following
+
+		// Include instance accounts in the first
+		// parts of this search. This will be
+		// changed to 'false' when doing text
+		// search in the database in the latter
+		// parts of this function.
+		includeInstanceAccounts = true
 	)
 
 	// Validate query.
@@ -109,7 +116,12 @@ func (p *Processor) Get(
 	// supply an offset greater than 0, return nothing as
 	// though there were no additional results.
 	if req.Offset > 0 {
-		return p.packageSearchResult(ctx, account, nil, nil, nil, req.APIv1)
+		return p.packageSearchResult(
+			ctx,
+			account,
+			nil, nil, nil, // No results.
+			req.APIv1, includeInstanceAccounts,
+		)
 	}
 
 	var (
@@ -167,6 +179,7 @@ func (p *Processor) Get(
 				foundStatuses,
 				foundTags,
 				req.APIv1,
+				includeInstanceAccounts,
 			)
 		}
 	}
@@ -196,6 +209,7 @@ func (p *Processor) Get(
 			foundStatuses,
 			foundTags,
 			req.APIv1,
+			includeInstanceAccounts,
 		)
 	}
 
@@ -236,11 +250,20 @@ func (p *Processor) Get(
 			foundStatuses,
 			foundTags,
 			req.APIv1,
+			includeInstanceAccounts,
 		)
 	}
 
 	// As a last resort, search for accounts and
 	// statuses using the query as arbitrary text.
+	//
+	// At this point we no longer want to include
+	// instance accounts in the results, since searching
+	// for something like 'mastodon', for example, will
+	// include a million instance/service accounts that
+	// have 'mastodon' in the domain, and therefore in
+	// the username, making the search results useless.
+	includeInstanceAccounts = false
 	if err := p.byText(
 		ctx,
 		account,
@@ -267,6 +290,7 @@ func (p *Processor) Get(
 		foundStatuses,
 		foundTags,
 		req.APIv1,
+		includeInstanceAccounts,
 	)
 }
 

internal/processing/search/lookup.go

@@ -44,6 +44,13 @@ func (p *Processor) Lookup(
 	requestingAccount *gtsmodel.Account,
 	query string,
 ) (*apimodel.Account, gtserror.WithCode) {
+	// Include instance accounts in this search.
+	//
+	// Lookup is for one specific account so we
+	// can't return loads of instance accounts by
+	// accident.
+	const includeInstanceAccounts = true
+
 	// Validate query.
 	query = strings.TrimSpace(query)
 	if query == "" {
@@ -96,7 +103,12 @@ func (p *Processor) Lookup(
 	// using the packageAccounts function to return it. This
 	// may cause the account to be filtered out if it's not
 	// visible to the caller, so anticipate this.
-	accounts, errWithCode := p.packageAccounts(ctx, requestingAccount, []*gtsmodel.Account{account})
+	accounts, errWithCode := p.packageAccounts(
+		ctx,
+		requestingAccount,
+		[]*gtsmodel.Account{account},
+		includeInstanceAccounts,
+	)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}

internal/processing/search/util.go

@@ -48,11 +48,12 @@ func (p *Processor) packageAccounts(
 	ctx context.Context,
 	requestingAccount *gtsmodel.Account,
 	accounts []*gtsmodel.Account,
+	includeInstanceAccounts bool,
 ) ([]*apimodel.Account, gtserror.WithCode) {
 	apiAccounts := make([]*apimodel.Account, 0, len(accounts))
 
 	for _, account := range accounts {
-		if account.IsInstance() {
+		if !includeInstanceAccounts && account.IsInstance() {
 			// No need to show instance accounts.
 			continue
 		}
@@ -169,8 +170,9 @@ func (p *Processor) packageSearchResult(
 	statuses []*gtsmodel.Status,
 	tags []*gtsmodel.Tag,
 	v1 bool,
+	includeInstanceAccounts bool,
 ) (*apimodel.SearchResult, gtserror.WithCode) {
-	apiAccounts, errWithCode := p.packageAccounts(ctx, requestingAccount, accounts)
+	apiAccounts, errWithCode := p.packageAccounts(ctx, requestingAccount, accounts, includeInstanceAccounts)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}

internal/visibility/account.go

@@ -19,10 +19,10 @@
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/superseriousbusiness/gotosocial/internal/cache"
 	"github.com/superseriousbusiness/gotosocial/internal/config"
+	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/log"
 )
@@ -66,7 +66,7 @@ func (f *Filter) isAccountVisibleTo(ctx context.Context, requester *gtsmodel.Acc
 	// Check whether target account is visible to anyone.
 	visible, err := f.isAccountVisible(ctx, account)
 	if err != nil {
-		return false, fmt.Errorf("isAccountVisibleTo: error checking account %s visibility: %w", account.ID, err)
+		return false, gtserror.Newf("error checking account %s visibility: %w", account.ID, err)
 	}
 
 	if !visible {
@@ -83,7 +83,7 @@ func (f *Filter) isAccountVisibleTo(ctx context.Context, requester *gtsmodel.Acc
 	// If requester is not visible, they cannot *see* either.
 	visible, err = f.isAccountVisible(ctx, requester)
 	if err != nil {
-		return false, fmt.Errorf("isAccountVisibleTo: error checking account %s visibility: %w", account.ID, err)
+		return false, gtserror.Newf("error checking account %s visibility: %w", account.ID, err)
 	}
 
 	if !visible {
@@ -97,7 +97,7 @@ func (f *Filter) isAccountVisibleTo(ctx context.Context, requester *gtsmodel.Acc
 		account.ID,
 	)
 	if err != nil {
-		return false, fmt.Errorf("isAccountVisibleTo: error checking account blocks: %w", err)
+		return false, gtserror.Newf("error checking account blocks: %w", err)
 	}
 
 	if blocked {
@@ -121,6 +121,7 @@ func (f *Filter) isAccountVisible(ctx context.Context, account *gtsmodel.Account
 		// Fetch the local user model for this account.
 		user, err := f.state.DB.GetUserByAccountID(ctx, account.ID)
 		if err != nil {
+			err := gtserror.Newf("db error getting user for account %s: %w", account.ID, err)
 			return false, err
 		}