From b2cacd6b0188f983d1b99f968dfa465a98c9f925 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Sat, 20 Jan 2024 12:45:43 +0100 Subject: [PATCH] [bugfix] Prevent URL + URI for same account being used as alias target (#2545) * [bugfix] Ensure URL and URI for same account can't both be provided as alias * test whoopsie from previous PR --- internal/processing/account/alias.go | 34 +++++++++++++---------- internal/processing/account/alias_test.go | 11 ++++++++ internal/typeutils/astointernal_test.go | 2 +- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/internal/processing/account/alias.go b/internal/processing/account/alias.go index bd31e8cb2..a11be0305 100644 --- a/internal/processing/account/alias.go +++ b/internal/processing/account/alias.go @@ -96,26 +96,21 @@ type uri struct { newAKAs[i].str = newAKAURI.String() } - // Dedupe the URI/string pairs. - newAKAs = util.DeduplicateFunc( - newAKAs, - func(v uri) string { - return v.str - }, - ) - // For each deduped entry, get and // check the target account, and set. for _, newAKA := range newAKAs { // Don't let account do anything // daft by aliasing to itself. - if newAKA.str == account.URI { + if newAKA.str == account.URI || + newAKA.str == account.URL { continue } - // Ensure we have a valid, up-to-date - // representation of the target account. - targetAccount, _, err := p.federator.GetAccountByURI(ctx, account.Username, newAKA.uri) + // Ensure we have account dereferenced. + targetAccount, _, err := p.federator.GetAccountByURI(ctx, + account.Username, + newAKA.uri, + ) if err != nil { err := fmt.Errorf( "error dereferencing also_known_as_uri (%s) account: %w", @@ -124,7 +119,7 @@ func(v uri) string { return nil, gtserror.NewErrorUnprocessableEntity(err, err.Error()) } - // Alias target must not be suspended. + // Target must not be suspended. if !targetAccount.SuspendedAt.IsZero() { err := fmt.Errorf( "target account %s is suspended from this instance; "+ @@ -135,10 +130,21 @@ func(v uri) string { } // Alrighty-roo, looks good, add this one. - account.AlsoKnownAsURIs = append(account.AlsoKnownAsURIs, newAKA.str) + account.AlsoKnownAsURIs = append(account.AlsoKnownAsURIs, targetAccount.URI) account.AlsoKnownAs = append(account.AlsoKnownAs, targetAccount) } + // Dedupe URIs + accounts, in case someone + // provided both an account URL and an + // account URI above, for the same account. + account.AlsoKnownAsURIs = util.Deduplicate(account.AlsoKnownAsURIs) + account.AlsoKnownAs = util.DeduplicateFunc( + account.AlsoKnownAs, + func(a *gtsmodel.Account) string { + return a.URI + }, + ) + err := p.state.DB.UpdateAccount(ctx, account, "also_known_as_uris") if err != nil { err := gtserror.Newf("db error updating also_known_as_uri: %w", err) diff --git a/internal/processing/account/alias_test.go b/internal/processing/account/alias_test.go index 9be5721aa..80fdb81c1 100644 --- a/internal/processing/account/alias_test.go +++ b/internal/processing/account/alias_test.go @@ -132,6 +132,17 @@ func (suite *AliasTestSuite) TestAliasAccount() { "http://localhost:8080/users/admin", }, }, + // Alias zork to turtle using both URI and URL + // for turtle. Only URI should end up being used. + { + newAliases: []string{ + "http://localhost:8080/users/1happyturtle", + "http://localhost:8080/@1happyturtle", + }, + expectedAliases: []string{ + "http://localhost:8080/users/1happyturtle", + }, + }, } { var ( ctx = context.Background() diff --git a/internal/typeutils/astointernal_test.go b/internal/typeutils/astointernal_test.go index 006a26fe7..84fdfd064 100644 --- a/internal/typeutils/astointernal_test.go +++ b/internal/typeutils/astointernal_test.go @@ -631,7 +631,7 @@ func (suite *ASToInternalTestSuite) TestParseHonkAccount() { // Clear caches. suite.state.Caches.GTS = cache.GTSCaches{} - suite.state.Caches.GTS.Init() + suite.state.Caches.Init() dbAcct, err = suite.db.GetAccountByID(ctx, acct.ID) if err != nil {