mirrors
/
gotosocial
mirror of
1
Fork 0
Code Issues Releases Activity

[bugfix] Allow blocked accounts to show in precise search (#2321)

This commit is contained in:
tobi 2023-10-30 19:01:00 +01:00 committed by GitHub
parent 4dc0547dc0
commit dd4b0241ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 390 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

189
internal/api/client/search/searchget_test.go
View File

@ -38,6 +38,7 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/config"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/id"
"github.com/superseriousbusiness/gotosocial/internal/oauth"
"github.com/superseriousbusiness/gotosocial/testrig"
)
@ -1179,8 +1180,9 @@ func (suite *SearchGetTestSuite) TestSearchLocalInstanceAccountPartial() {
}
// Query was a partial namestring from our
// instance, so will return the instance account.
suite.Len(searchResult.Accounts, 1)
// instance, instance account should be
// excluded from results.
suite.Len(searchResult.Accounts, 0)
suite.Len(searchResult.Statuses, 0)
suite.Len(searchResult.Hashtags, 0)
}
@ -1546,6 +1548,189 @@ func (suite *SearchGetTestSuite) TestSearchNotHashtagButWithTypeHashtag() {
suite.Len(searchResult.Hashtags, 1)
}
func (suite *SearchGetTestSuite) TestSearchBlockedAccountFullNamestring() {
var (
requestingAccount = suite.testAccounts["local_account_1"]
targetAccount = suite.testAccounts["remote_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 = func() *bool { i := true; return &i }()
query = "@" + targetAccount.Username + "@" + targetAccount.Domain
queryType *string = func() *string { i := "accounts"; return &i }()
following *bool = nil
expectedHTTPStatus = http.StatusOK
expectedBody = ""
)
// Block the account
// we're about to search.
if err := suite.db.PutBlock(
context.Background(),
&gtsmodel.Block{
ID: id.NewULID(),
URI: "https://example.org/nooooooo",
AccountID: requestingAccount.ID,
TargetAccountID: targetAccount.ID,
},
); err != nil {
suite.FailNow(err.Error())
}
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 was for full namestring;
// we should still be able to see
// the account we've blocked.
if !suite.Len(searchResult.Accounts, 1) {
suite.FailNow("expected 1 account in search results but got 0")
}
gotAccount := searchResult.Accounts[0]
suite.NotNil(gotAccount)
}
func (suite *SearchGetTestSuite) TestSearchBlockedAccountPartialNamestring() {
var (
requestingAccount = suite.testAccounts["local_account_1"]
targetAccount = suite.testAccounts["remote_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 = func() *bool { i := true; return &i }()
query = "@" + targetAccount.Username
queryType *string = func() *string { i := "accounts"; return &i }()
following *bool = nil
expectedHTTPStatus = http.StatusOK
expectedBody = ""
)
// Block the account
// we're about to search.
if err := suite.db.PutBlock(
context.Background(),
&gtsmodel.Block{
ID: id.NewULID(),
URI: "https://example.org/nooooooo",
AccountID: requestingAccount.ID,
TargetAccountID: targetAccount.ID,
},
); err != nil {
suite.FailNow(err.Error())
}
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 was for partial namestring;
// we should not be able to see
// the account we've blocked.
if !suite.Empty(searchResult.Accounts) {
suite.FailNow("expected 0 accounts in search results")
}
}
func (suite *SearchGetTestSuite) TestSearchBlockedAccountURI() {
var (
requestingAccount = suite.testAccounts["local_account_1"]
targetAccount = suite.testAccounts["remote_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 = func() *bool { i := true; return &i }()
query = targetAccount.URI
queryType *string = func() *string { i := "accounts"; return &i }()
following *bool = nil
expectedHTTPStatus = http.StatusOK
expectedBody = ""
)
// Block the account
// we're about to search.
if err := suite.db.PutBlock(
context.Background(),
&gtsmodel.Block{
ID: id.NewULID(),
URI: "https://example.org/nooooooo",
AccountID: requestingAccount.ID,
TargetAccountID: targetAccount.ID,
},
); err != nil {
suite.FailNow(err.Error())
}
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 was for precise URI;
// we should still be able to see
// the account we've blocked.
if !suite.Len(searchResult.Accounts, 1) {
suite.FailNow("expected 1 account in search results but got 0")
}
gotAccount := searchResult.Accounts[0]
suite.NotNil(gotAccount)
}
func TestSearchGetTestSuite(t *testing.T) {
suite.Run(t, &SearchGetTestSuite{})
}

5
internal/processing/account/statuses.go
View File

@ -52,8 +52,9 @@ func (p *Processor) StatusesGet(
}
if blocked {
err := errors.New("block exists between accounts")
return nil, gtserror.NewErrorNotFound(err)
// Block exists between accounts.
// Just return empty statuses.
return util.EmptyPageableResponse(), nil
}
}

57
internal/processing/search/accounts.go
View File

@ -29,6 +29,7 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/id"
"github.com/superseriousbusiness/gotosocial/internal/log"
"github.com/superseriousbusiness/gotosocial/internal/util"
)
// Accounts does a partial search for accounts that
@ -56,6 +57,11 @@ func (p *Processor) Accounts(
// in their search results.
const includeInstanceAccounts = false
// We *might* want to include blocked accounts
// in this search, but only if it's a search
// for a specific account.
includeBlockedAccounts := false
var (
foundAccounts = make([]*gtsmodel.Account, 0, limit)
appendAccount = func(foundAccount *gtsmodel.Account) { foundAccounts = append(foundAccounts, foundAccount) }
@ -95,26 +101,42 @@ func (p *Processor) Accounts(
requestingAccount,
foundAccounts,
includeInstanceAccounts,
includeBlockedAccounts,
)
}
// Return all accounts we can find that match the
// provided query. If it's not a namestring, this
// won't return an error, it'll just return 0 results.
if _, err := p.accountsByNamestring(
ctx,
requestingAccount,
id.Highest,
id.Lowest,
limit,
offset,
query,
resolve,
following,
appendAccount,
); err != nil && !errors.Is(err, db.ErrNoEntries) {
err = gtserror.Newf("error searching by namestring: %w", err)
return nil, gtserror.NewErrorInternalError(err)
// See if we have something that looks like a namestring.
username, domain, err := util.ExtractNamestringParts(query)
if err != nil {
log.Warnf(ctx, "couldn't parse '%s' as namestring: %v", query, err)
} else {
if domain != "" {
// Search was an exact namestring;
// we can safely assume caller is
// searching for a specific account,
// and show it to them even if they
// have it blocked.
includeBlockedAccounts = true
}
// Get all accounts we can find
// that match the provided query.
if err := p.accountsByNamestring(
ctx,
requestingAccount,
id.Highest,
id.Lowest,
limit,
offset,
username,
domain,
resolve,
following,
appendAccount,
); err != nil && !errors.Is(err, db.ErrNoEntries) {
err = gtserror.Newf("error searching by namestring: %w", err)
return nil, gtserror.NewErrorInternalError(err)
}
}
// Return whatever we got (if anything).
@ -123,5 +145,6 @@ func (p *Processor) Accounts(
requestingAccount,
foundAccounts,
includeInstanceAccounts,
includeBlockedAccounts,
)
}

212
internal/processing/search/get.go
View File

@ -77,6 +77,12 @@ func (p *Processor) Get(
// search in the database in the latter
// parts of this function.
includeInstanceAccounts = true
// Assume caller doesn't want to see
// blocked accounts. This will change
// to 'true' if caller is searching
// for a specific account.
includeBlockedAccounts = false
)
// Validate query.
@ -120,7 +126,9 @@ func (p *Processor) Get(
ctx,
account,
nil, nil, nil, // No results.
req.APIv1, includeInstanceAccounts,
req.APIv1,
includeInstanceAccounts,
includeBlockedAccounts,
)
}
@ -131,7 +139,6 @@ func (p *Processor) Get(
appendStatus = func(s *gtsmodel.Status) { foundStatuses = append(foundStatuses, s) }
appendAccount = func(a *gtsmodel.Account) { foundAccounts = append(foundAccounts, a) }
appendTag = func(t *gtsmodel.Tag) { foundTags = append(foundTags, t) }
keepLooking bool
err error
)
@ -152,56 +159,83 @@ func (p *Processor) Get(
}
}
// Search using what may or may not be a namestring.
keepLooking, err = p.accountsByNamestring(
// See if we have something that looks like a namestring.
username, domain, err := util.ExtractNamestringParts(queryC)
if err == nil {
// We managed to parse query as a namestring.
// If domain was set, this is a very specific
// search for a particular account, so show
// that account to the caller even if they
// have it blocked. They might be looking
// for it to unblock it again!
domainSet := (domain != "")
includeBlockedAccounts = domainSet
err = p.accountsByNamestring(
ctx,
account,
maxID,
minID,
limit,
offset,
username,
domain,
resolve,
following,
appendAccount,
)
if err != nil && !errors.Is(err, db.ErrNoEntries) {
err = gtserror.Newf("error searching by namestring: %w", err)
return nil, gtserror.NewErrorInternalError(err)
}
// If domain was set, we know this is
// a full namestring, and not a url or
// just a username, so we should stop
// looking now and just return what we
// have (if anything). Otherwise we'll
// let the search keep going.
if domainSet {
return p.packageSearchResult(
ctx,
account,
foundAccounts,
foundStatuses,
foundTags,
req.APIv1,
includeInstanceAccounts,
includeBlockedAccounts,
)
}
}
}
// Check if we're searching by a known URI scheme.
// (This might just be a weirdly-parsed URI,
// since Go's url package tends to be a bit
// trigger-happy when deciding things are URIs).
uri, err := url.Parse(query)
if err == nil && (uri.Scheme == "https" || uri.Scheme == "http") {
// URI is pretty specific so we can safely assume
// caller wants to include blocked accounts too.
includeBlockedAccounts = true
if err := p.byURI(
ctx,
account,
maxID,
minID,
limit,
offset,
queryC,
uri,
queryType,
resolve,
following,
appendAccount,
)
if err != nil && !errors.Is(err, db.ErrNoEntries) {
err = gtserror.Newf("error searching by namestring: %w", err)
appendStatus,
); err != nil && !errors.Is(err, db.ErrNoEntries) {
err = gtserror.Newf("error searching by URI: %w", err)
return nil, gtserror.NewErrorInternalError(err)
}
if !keepLooking {
// Return whatever we have.
return p.packageSearchResult(
ctx,
account,
foundAccounts,
foundStatuses,
foundTags,
req.APIv1,
includeInstanceAccounts,
)
}
}
// Check if the query is a URI with a recognizable
// scheme and use it to look for accounts or statuses.
keepLooking, err = p.byURI(
ctx,
account,
query,
queryType,
resolve,
appendAccount,
appendStatus,
)
if err != nil && !errors.Is(err, db.ErrNoEntries) {
err = gtserror.Newf("error searching by URI: %w", err)
return nil, gtserror.NewErrorInternalError(err)
}
if !keepLooking {
// Return whatever we have.
// This was a URI, so at this point just return
// whatever we have. You can't search hashtags by
// URI, and shouldn't do full-text with a URI either.
return p.packageSearchResult(
ctx,
account,
@ -210,6 +244,7 @@ func (p *Processor) Get(
foundTags,
req.APIv1,
includeInstanceAccounts,
includeBlockedAccounts,
)
}
@ -226,7 +261,7 @@ func (p *Processor) Get(
// We know that none of the subsequent searches
// would show any good results either, and those
// searches are *much* more expensive.
keepLooking, err = p.hashtag(
keepLooking, err := p.hashtag(
ctx,
maxID,
minID,
@ -251,6 +286,7 @@ func (p *Processor) Get(
foundTags,
req.APIv1,
includeInstanceAccounts,
includeBlockedAccounts,
)
}
@ -291,15 +327,15 @@ func (p *Processor) Get(
foundTags,
req.APIv1,
includeInstanceAccounts,
includeBlockedAccounts,
)
}
// accountsByNamestring searches for accounts using the
// provided namestring query. If domain is not set in
// the namestring, it may return more than one result
// by doing a text search in the database for accounts
// matching the query. Otherwise, it tries to return an
// exact match.
// provided username and domain. If domain is not set,
// it may return more than one result by doing a text
// search in the database for accounts matching the query.
// Otherwise, it tries to return an exact match.
func (p *Processor) accountsByNamestring(
ctx context.Context,
requestingAccount *gtsmodel.Account,
@ -307,26 +343,18 @@ func (p *Processor) accountsByNamestring(
minID string,
limit int,
offset int,
query string,
username string,
domain string,
resolve bool,
following bool,
appendAccount func(*gtsmodel.Account),
) (bool, error) {
// See if we have something that looks like a namestring.
username, domain, err := util.ExtractNamestringParts(query)
if err != nil {
// No need to return error; just not a namestring
// we can search with. Caller should keep looking
// with another search method.
return true, nil //nolint:nilerr
}
) error {
if domain == "" {
// No error, but no domain set. That means the query
// looked like '@someone' which is not an exact search.
// Try to search for any accounts that match the query
// string, and let the caller know they should stop.
return false, p.accountsByText(
return p.accountsByText(
ctx,
requestingAccount.ID,
maxID,
@ -355,18 +383,14 @@ func (p *Processor) accountsByNamestring(
// Check for semi-expected error types.
// On one of these, we can continue.
if !gtserror.Unretrievable(err) && !gtserror.WrongType(err) {
err = gtserror.Newf("error looking up %s as account: %w", query, err)
return false, gtserror.NewErrorInternalError(err)
err = gtserror.Newf("error looking up @%s@%s as account: %w", username, domain, err)
return gtserror.NewErrorInternalError(err)
}
} else {
appendAccount(foundAccount)
}
// Regardless of whether we have a hit at this point,
// return false to indicate caller should stop looking;
// namestrings are a very specific format so it's unlikely
// the caller was looking for something other than an account.
return false, nil
return nil
}
// accountByUsernameDomain looks for one account with the given
@ -443,38 +467,22 @@ func (p *Processor) accountByUsernameDomain(
func (p *Processor) byURI(
ctx context.Context,
requestingAccount *gtsmodel.Account,
query string,
uri *url.URL,
queryType string,
resolve bool,
appendAccount func(*gtsmodel.Account),
appendStatus func(*gtsmodel.Status),
) (bool, error) {
uri, err := url.Parse(query)
if err != nil {
// No need to return error; just not a URI
// we can search with. Caller should keep
// looking with another search method.
return true, nil //nolint:nilerr
}
if !(uri.Scheme == "https" || uri.Scheme == "http") {
// This might just be a weirdly-parsed URI,
// since Go's url package tends to be a bit
// trigger-happy when deciding things are URIs.
// Indicate caller should keep looking.
return true, nil
}
) error {
blocked, err := p.state.DB.IsURIBlocked(ctx, uri)
if err != nil {
err = gtserror.Newf("error checking domain block: %w", err)
return false, gtserror.NewErrorInternalError(err)
return gtserror.NewErrorInternalError(err)
}
if blocked {
// Don't search for blocked domains.
// Caller should stop looking.
return false, nil
// Don't search for
// blocked domains.
return nil
}
if includeAccounts(queryType) {
@ -485,14 +493,13 @@ func (p *Processor) byURI(
// On one of these, we can continue.
if !gtserror.Unretrievable(err) && !gtserror.WrongType(err) {
err = gtserror.Newf("error looking up %s as account: %w", uri, err)
return false, gtserror.NewErrorInternalError(err)
return gtserror.NewErrorInternalError(err)
}
} else {
// Hit; return false to indicate caller should
// stop looking, since it's extremely unlikely
// Hit! Return early since it's extremely unlikely
// a status and an account will have the same URL.
appendAccount(foundAccount)
return false, nil
return nil
}
}
@ -504,20 +511,19 @@ func (p *Processor) byURI(
// On one of these, we can continue.
if !gtserror.Unretrievable(err) && !gtserror.WrongType(err) {
err = gtserror.Newf("error looking up %s as status: %w", uri, err)
return false, gtserror.NewErrorInternalError(err)
return gtserror.NewErrorInternalError(err)
}
} else {
// Hit; return false to indicate caller should
// stop looking, since it's extremely unlikely
// Hit! Return early since it's extremely unlikely
// a status and an account will have the same URL.
appendStatus(foundStatus)
return false, nil
return nil
}
}
// No errors, but no hits either; since this
// was a URI, caller should stop looking.
return false, nil
// No errors, but no hits
// either; that's fine.
return nil
}
// accountByURI looks for one account with the given URI.

6
internal/processing/search/lookup.go
View File

@ -51,6 +51,11 @@ func (p *Processor) Lookup(
// accident.
const includeInstanceAccounts = true
// Since lookup is always for a specific
// account, it's fine to include a blocked
// account in the results.
const includeBlockedAccounts = true
// Validate query.
query = strings.TrimSpace(query)
if query == "" {
@ -108,6 +113,7 @@ func (p *Processor) Lookup(
requestingAccount,
[]*gtsmodel.Account{account},
includeInstanceAccounts,
includeBlockedAccounts,
)
if errWithCode != nil {
return nil, errWithCode

29
internal/processing/search/util.go
View File

@ -49,6 +49,7 @@ func (p *Processor) packageAccounts(
requestingAccount *gtsmodel.Account,
accounts []*gtsmodel.Account,
includeInstanceAccounts bool,
includeBlockedAccounts bool,
) ([]*apimodel.Account, gtserror.WithCode) {
apiAccounts := make([]*apimodel.Account, 0, len(accounts))
@ -58,19 +59,26 @@ func (p *Processor) packageAccounts(
continue
}
// Ensure requester can see result account.
visible, err := p.filter.AccountVisible(ctx, requestingAccount, account)
// Check if block exists between searcher and searchee.
blocked, err := p.state.DB.IsEitherBlocked(ctx, requestingAccount.ID, account.ID)
if err != nil {
err = gtserror.Newf("error checking visibility of account %s for account %s: %w", account.ID, requestingAccount.ID, err)
err = gtserror.Newf("error checking block between searching account %s and searched account %s: %w", requestingAccount.ID, account.ID, err)
return nil, gtserror.NewErrorInternalError(err)
}
if !visible {
log.Debugf(ctx, "account %s is not visible to account %s, skipping this result", account.ID, requestingAccount.ID)
if blocked && !includeBlockedAccounts {
// Don't include
// this result.
continue
}
apiAccount, err := p.converter.AccountToAPIAccountPublic(ctx, account)
var apiAccount *apimodel.Account
if blocked {
apiAccount, err = p.converter.AccountToAPIAccountBlocked(ctx, account)
} else {
apiAccount, err = p.converter.AccountToAPIAccountPublic(ctx, account)
}
if err != nil {
log.Debugf(ctx, "skipping account %s because it couldn't be converted to its api representation: %s", account.ID, err)
continue
@ -171,8 +179,15 @@ func (p *Processor) packageSearchResult(
tags []*gtsmodel.Tag,
v1 bool,
includeInstanceAccounts bool,
includeBlockedAccounts bool,
) (*apimodel.SearchResult, gtserror.WithCode) {
apiAccounts, errWithCode := p.packageAccounts(ctx, requestingAccount, accounts, includeInstanceAccounts)
apiAccounts, errWithCode := p.packageAccounts(
ctx,
requestingAccount,
accounts,
includeInstanceAccounts,
includeBlockedAccounts,
)
if errWithCode != nil {
return nil, errWithCode
}

34
internal/typeutils/internaltofrontend.go
View File

@ -277,7 +277,7 @@ func (c *Converter) AccountToAPIAccountBlocked(ctx context.Context, a *gtsmodel.
// de-punify it just in case.
d, err := util.DePunify(a.Domain)
if err != nil {
return nil, fmt.Errorf("AccountToAPIAccountBlocked: error de-punifying domain %s for account id %s: %w", a.Domain, a.ID, err)
return nil, gtserror.Newf("error de-punifying domain %s for account id %s: %w", a.Domain, a.ID, err)
}
acct = a.Username + "@" + d
@ -288,7 +288,7 @@ func (c *Converter) AccountToAPIAccountBlocked(ctx context.Context, a *gtsmodel.
if !a.IsInstance() {
user, err := c.state.DB.GetUserByAccountID(ctx, a.ID)
if err != nil {
return nil, fmt.Errorf("AccountToAPIAccountPublic: error getting user from database for account id %s: %w", a.ID, err)
return nil, gtserror.Newf("error getting user from database for account id %s: %w", a.ID, err)
}
switch {
@ -304,17 +304,25 @@ func (c *Converter) AccountToAPIAccountBlocked(ctx context.Context, a *gtsmodel.
acct = a.Username // omit domain
}
return &apimodel.Account{
ID: a.ID,
Username: a.Username,
Acct: acct,
DisplayName: a.DisplayName,
Bot: *a.Bot,
CreatedAt: util.FormatISO8601(a.CreatedAt),
URL: a.URL,
Suspended: !a.SuspendedAt.IsZero(),
Role: role,
}, nil
account := &apimodel.Account{
ID: a.ID,
Username: a.Username,
Acct: acct,
Bot: *a.Bot,
CreatedAt: util.FormatISO8601(a.CreatedAt),
URL: a.URL,
Suspended: !a.SuspendedAt.IsZero(),
Role: role,
}
// Don't show the account's actual
// avatar+header since it may be
// upsetting to the blocker. Just
// show generic avatar+header instead.
c.ensureAvatar(account)
c.ensureHeader(account)
return account, nil
}
func (c *Converter) AccountToAdminAPIAccount(ctx context.Context, a *gtsmodel.Account) (*apimodel.AdminAccountInfo, error) {

4
internal/typeutils/internaltofrontend_test.go
View File

@ -313,8 +313,8 @@ func (suite *InternalToFrontendTestSuite) TestLocalInstanceAccountToFrontendBloc
"url": "http://localhost:8080/@localhost:8080",
"avatar": "",
"avatar_static": "",
"header": "",
"header_static": "",
"header": "http://localhost:8080/assets/default_header.png",
"header_static": "http://localhost:8080/assets/default_header.png",
"followers_count": 0,
"following_count": 0,
"statuses_count": 0,