diff --git a/internal/api/client/search/searchget_test.go b/internal/api/client/search/searchget_test.go index 2a6911430..2670fff92 100644 --- a/internal/api/client/search/searchget_test.go +++ b/internal/api/client/search/searchget_test.go @@ -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(), + >smodel.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(), + >smodel.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(), + >smodel.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{}) } diff --git a/internal/processing/account/statuses.go b/internal/processing/account/statuses.go index 716157a14..1bdd3906b 100644 --- a/internal/processing/account/statuses.go +++ b/internal/processing/account/statuses.go @@ -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 } } diff --git a/internal/processing/search/accounts.go b/internal/processing/search/accounts.go index cfcc65b2b..17002a29b 100644 --- a/internal/processing/search/accounts.go +++ b/internal/processing/search/accounts.go @@ -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, ) } diff --git a/internal/processing/search/get.go b/internal/processing/search/get.go index 830e3481b..30a2745af 100644 --- a/internal/processing/search/get.go +++ b/internal/processing/search/get.go @@ -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. diff --git a/internal/processing/search/lookup.go b/internal/processing/search/lookup.go index 8b48d4514..b6f1eef52 100644 --- a/internal/processing/search/lookup.go +++ b/internal/processing/search/lookup.go @@ -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 diff --git a/internal/processing/search/util.go b/internal/processing/search/util.go index 9d9175246..de91e5d51 100644 --- a/internal/processing/search/util.go +++ b/internal/processing/search/util.go @@ -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 } diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go index d663a1237..254bf9da3 100644 --- a/internal/typeutils/internaltofrontend.go +++ b/internal/typeutils/internaltofrontend.go @@ -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) { diff --git a/internal/typeutils/internaltofrontend_test.go b/internal/typeutils/internaltofrontend_test.go index 9f72c6d2e..16966d8cb 100644 --- a/internal/typeutils/internaltofrontend_test.go +++ b/internal/typeutils/internaltofrontend_test.go @@ -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,