From 098aa4f781e123c1089664f9f5c87862c88221eb Mon Sep 17 00:00:00 2001 From: tsmethurst Date: Wed, 1 Jun 2022 16:10:41 +0200 Subject: [PATCH] rework GetRemoteAccount --- internal/federation/dereferencing/account.go | 204 ++++++++++++------- 1 file changed, 136 insertions(+), 68 deletions(-) diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go index fda77807b..a93e64376 100644 --- a/internal/federation/dereferencing/account.go +++ b/internal/federation/dereferencing/account.go @@ -33,6 +33,7 @@ "github.com/superseriousbusiness/activity/streams" "github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/gotosocial/internal/ap" + "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/id" "github.com/superseriousbusiness/gotosocial/internal/media" @@ -67,25 +68,10 @@ type GetRemoteAccountParams struct { // If false, then the account's media and other fields will be dereferenced in the background, // so only a minimal account representation will be returned by GetRemoteAccount. Blocking bool -} - -func (d *deref) populateAccountBeforeReturn(ctx context.Context, params GetRemoteAccountParams, remoteAccount *gtsmodel.Account) (*gtsmodel.Account, error) { - // make sure the account fields are populated before returning: - // even if we're not doing a refresh, the caller might want to block - // until everything is loaded - changed, err := d.populateAccountFields(ctx, remoteAccount, params.RequestingUsername, false, params.Blocking) - if err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error populating remoteAccount fields: %s", err) - } - - if changed { - remoteAccount, err = d.db.UpdateAccount(ctx, remoteAccount) - if err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error updating remoteAccount: %s", err) - } - } - - return remoteAccount, nil + // Whether to skip making calls to remote instances. This is useful when you want to + // quickly fetch a remote account from the database or fail, and don't want to cause + // http requests to go flying around. + SkipResolve bool } // GetRemoteAccount completely dereferences a remote account, converts it to a GtS model account, @@ -97,34 +83,81 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar accountDomain set, while making as few calls to remote instances as possible to save time and bandwidth. There are a few different paths through this function, and the path taken depends on how much - initial information we are provided with via parameters, and how much information we already have stored. + initial information we are provided with via parameters, how much information we already have stored, + and what we're allowed to do according to the parameters we've been passed. + + Scenario 1: We're not allowed to resolve remotely, but we've got either the account URI or the + account username + host, so we can check in our database and return if possible. + + Scenario 2: We are allowed to resolve remotely, and we have an account URI but no username or host. + In this case, we can use the URI to resolve the remote account and find the username, + and then we can webfinger the account to discover the accountDomain. + + Scenario 3: We are allowed to resolve remotely, and we have the username and host but no URI. + In this case, we webfinger the account to discover the URI. We then check in the db + */ - var accountDomain string - var accountable ap.Accountable - - if params.RemoteAccountID == nil && (params.RemoteAccountUsername == "" || params.RemoteAccountHost == "") { - err = errors.New("GetRemoteAccount: no identifying parameters were set so we cannot dereference account") + // first check if we can retrieve the account locally just with what we've been given + if params.RemoteAccountID != nil { + // try with uri + if a, dbErr := d.db.GetAccountByURI(ctx, params.RemoteAccountID.String()); dbErr == nil { + remoteAccount = a + } else if dbErr != db.ErrNoEntries { + err = fmt.Errorf("GetRemoteAccount: database error looking for account %s: %s", params.RemoteAccountID, err) + } + } else if params.RemoteAccountUsername != "" && params.RemoteAccountHost != "" { + // try with username/host + a := >smodel.Account{} + where := []db.Where{{Key: "username", Value: params.RemoteAccountUsername}, {Key: "domain", Value: params.RemoteAccountHost}} + if dbErr := d.db.GetWhere(ctx, where, a); dbErr == nil { + remoteAccount = a + } else if dbErr != db.ErrNoEntries { + err = fmt.Errorf("GetRemoteAccount: database error looking for account with username %s and host %s: %s", params.RemoteAccountUsername, params.RemoteAccountHost, err) + } + } else { + err = errors.New("GetRemoteAccount: no identifying parameters were set so we cannot get account") return } - if params.RemoteAccountUsername == "" || params.RemoteAccountHost == "" { - accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err) - return + if params.SkipResolve { + // if we can't resolve, return already since there's nothing more we can do + if remoteAccount == nil { + err = errors.New("GetRemoteAccount: error retrieving account with skipResolve set true") } + return + } + var accountable ap.Accountable + if params.RemoteAccountUsername == "" || params.RemoteAccountHost == "" { + // try to populate the missing params + // the first one is easy ... params.RemoteAccountHost = params.RemoteAccountID.Host - params.RemoteAccountUsername, err = ap.ExtractPreferredUsername(accountable) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error extracting accountable username: %s", err) - return + // ... but we still need the username so we can do a finger for the accountDomain + + // check if we had the account stored already and got it earlier + if remoteAccount != nil { + params.RemoteAccountUsername = remoteAccount.Username + } else { + // if we didn't already have it, we have dereference it from remote and just... + accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) + if err != nil { + err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err) + return + } + + // ... take the username (for now) + params.RemoteAccountUsername, err = ap.ExtractPreferredUsername(accountable) + if err != nil { + err = fmt.Errorf("GetRemoteAccount: error extracting accountable username: %s", err) + return + } } } // if we reach this point, params.RemoteAccountHost and params.RemoteAccountUsername must be set - // params.RemoteAccountID may or may not be set, but we have enough information to fetch it again in any case + // params.RemoteAccountID may or may not be set, but we have enough information to fetch it in any case + var accountDomain string accountDomain, params.RemoteAccountID, err = d.fingerRemoteAccount(ctx, params.RequestingUsername, params.RemoteAccountUsername, params.RemoteAccountHost) if err != nil { err = fmt.Errorf("GetRemoteAccount: error while fingering: %s", err) @@ -134,40 +167,75 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar // if we reach here then all params must now be set, // either in original params or through fingering - // see if we have this account stored already and just return it if so - if a, dbErr := d.db.GetAccountByURI(ctx, params.RemoteAccountID.String()); dbErr == nil { - remoteAccount, err = d.populateAccountBeforeReturn(ctx, params, a) - return - } - - // if we reach here then we haven't seen this account before: dereference it from remote (f we haven't already) - if accountable == nil { - accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) - if err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err) + // we may also have some extra information already, like the account we had in the db, or the + // accountable representation that we dereferenced from remote + if remoteAccount == nil { + // we still don't have the account, so deference it if we didn't earlier + if accountable == nil { + accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) + if err != nil { + err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err) + return + } } - } - newAccount, err := d.typeConverter.ASRepresentationToAccount(ctx, accountable, accountDomain, false) - if err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error converting accountable to account: %s", err) - } + // then convert + remoteAccount, err = d.typeConverter.ASRepresentationToAccount(ctx, accountable, accountDomain, false) + if err != nil { + err = fmt.Errorf("GetRemoteAccount: error converting accountable to account: %s", err) + return + } - ulid, err := id.NewRandomULID() - if err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error generating new id for account: %s", err) - } - newAccount.ID = ulid + // this is a new account so we need to generate a new ID for it + var ulid string + ulid, err = id.NewRandomULID() + if err != nil { + err = fmt.Errorf("GetRemoteAccount: error generating new id for account: %s", err) + return + } + remoteAccount.ID = ulid - if _, err := d.populateAccountFields(ctx, newAccount, params.RequestingUsername, params.Blocking, false); err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error populating further account fields: %s", err) - } + _, err = d.populateAccountFields(ctx, remoteAccount, params.RequestingUsername, params.Blocking) + if err != nil { + err = fmt.Errorf("GetRemoteAccount: error populating further account fields: %s", err) + return + } - if err := d.db.Put(ctx, newAccount); err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error putting new account: %s", err) - } + err = d.db.Put(ctx, remoteAccount) + if err != nil { + err = fmt.Errorf("GetRemoteAccount: error putting new account: %s", err) + return + } - return newAccount, nil + return // the new account + } else { + // we had the account already, but now we know the account domain, so update it if it's different + if !strings.EqualFold(remoteAccount.Domain, accountDomain) { + remoteAccount.Domain = accountDomain + remoteAccount, err = d.db.UpdateAccount(ctx, remoteAccount) + if err != nil { + err = fmt.Errorf("GetRemoteAccount: error updating account: %s", err) + return + } + } + + // make sure the account fields are populated before returning: + // the caller might want to block until everything is loaded + var changed bool + changed, err = d.populateAccountFields(ctx, remoteAccount, params.RequestingUsername, params.Blocking) + if err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error populating remoteAccount fields: %s", err) + } + + if changed { + remoteAccount, err = d.db.UpdateAccount(ctx, remoteAccount) + if err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error updating remoteAccount: %s", err) + } + } + + return // the account we already had + possibly updated + } } // dereferenceAccountable calls remoteAccountID with a GET request, and tries to parse whatever @@ -240,7 +308,7 @@ func (d *deref) dereferenceAccountable(ctx context.Context, username string, rem // populateAccountFields populates any fields on the given account that weren't populated by the initial // dereferencing. This includes things like header and avatar etc. -func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Account, requestingUsername string, blocking bool, refresh bool) (bool, error) { +func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Account, requestingUsername string, blocking bool) (bool, error) { // if we're dealing with an instance account, just bail, we don't need to do anything if instanceAccount(account) { return false, nil @@ -261,7 +329,7 @@ func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Acc } // fetch the header and avatar - changed, err := d.fetchRemoteAccountMedia(ctx, account, t, refresh, blocking) + changed, err := d.fetchRemoteAccountMedia(ctx, account, t, blocking) if err != nil { return false, fmt.Errorf("populateAccountFields: error fetching header/avi for account: %s", err) } @@ -281,7 +349,7 @@ func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Acc // // If blocking is true, then the calls to the media manager made by this function will be blocking: // in other words, the function won't return until the header and the avatar have been fully processed. -func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsmodel.Account, t transport.Transport, blocking bool, refresh bool) (bool, error) { +func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsmodel.Account, t transport.Transport, blocking bool) (bool, error) { changed := false accountURI, err := url.Parse(targetAccount.URI) @@ -293,7 +361,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm return changed, fmt.Errorf("fetchRemoteAccountMedia: domain %s is blocked", accountURI.Host) } - if targetAccount.AvatarRemoteURL != "" && (targetAccount.AvatarMediaAttachmentID == "" || refresh) { + if targetAccount.AvatarRemoteURL != "" && (targetAccount.AvatarMediaAttachmentID == "") { var processingMedia *media.ProcessingMedia d.dereferencingAvatarsLock.Lock() // LOCK HERE @@ -351,7 +419,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm changed = true } - if targetAccount.HeaderRemoteURL != "" && (targetAccount.HeaderMediaAttachmentID == "" || refresh) { + if targetAccount.HeaderRemoteURL != "" && (targetAccount.HeaderMediaAttachmentID == "") { var processingMedia *media.ProcessingMedia d.dereferencingHeadersLock.Lock() // LOCK HERE