[bugfix] relax missing preferred_username, instead using webfingered username (#3189)

* support no preferred_username, instead using webfingered username

* add tests for the new preferred_username behaviour

2 files changed, 35 insertions, 22 deletions

diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go
index 3abf628d1..b0118380d 100644
--- a/internal/federation/dereferencing/account.go
+++ b/internal/federation/dereferencing/account.go
@@ -18,6 +18,7 @@
 package dereferencing
 
 import (
+	"cmp"
 	"context"
 	"errors"
 	"net/url"
@@ -509,10 +510,16 @@ func (d *Dereferencer) enrichAccount(
 	}
 
 	if account.Username != "" {
-		// A username was provided so we can attempt a webfinger, this ensures up-to-date accountdomain info.
-		accDomain, accURI, err := d.fingerRemoteAccount(ctx, tsport, account.Username, account.Domain)
-		switch {
+		// A username was provided so we can attempt to webfinger,
+		// this ensures up-to-date account domain, and handles some
+		// edge cases where servers don't provide a preferred_username.
+		accUsername, accDomain, accURI, err := d.fingerRemoteAccount(ctx,
+			tsport,
+			account.Username,
+			account.Domain,
+		)
 
+		switch {
 		case err != nil && account.URI == "":
 			// This is a new account (to us) with username@domain
 			// but failed webfinger, nothing more we can do.
@@ -554,6 +561,9 @@ func (d *Dereferencer) enrichAccount(
 			account.URI = accURI.String()
 			account.Domain = accDomain
 			uri = accURI
+
+			// Specifically only update username if not already set.
+			account.Username = cmp.Or(account.Username, accUsername)
 		}
 	}
 
@@ -609,7 +619,7 @@ func (d *Dereferencer) enrichAccount(
 		if err != nil {
 			// ResolveAccountable will set gtserror.WrongType
 			// on the returned error, so we don't need to do it here.
-			err = gtserror.Newf("error resolving accountable %s: %w", uri, err)
+			err := gtserror.Newf("error resolving accountable %s: %w", uri, err)
 			return nil, nil, err
 		}
 
@@ -656,15 +666,18 @@ func (d *Dereferencer) enrichAccount(
 	latestAcc, err := d.converter.ASRepresentationToAccount(ctx,
 		apubAcc,
 		account.Domain,
+		account.Username,
 	)
 	if err != nil {
 		// ASRepresentationToAccount will set Malformed on the
 		// returned error, so we don't need to do it here.
-		err = gtserror.Newf("error converting %s to gts model: %w", uri, err)
+		err := gtserror.Newf("error converting %s to gts model: %w", uri, err)
 		return nil, nil, err
 	}
 
 	if account.Username == "" {
+		var accUsername string
+
 		// Assume the host from the
 		// ActivityPub representation.
 		id := ap.GetJSONLDId(apubAcc)
@@ -685,7 +698,7 @@ func (d *Dereferencer) enrichAccount(
 		// https://example.org/@someone@somewhere.else and we've been redirected
 		// from example.org to somewhere.else: we want to take somewhere.else
 		// as the accountDomain then, not the example.org we were redirected from.
-		latestAcc.Domain, _, err = d.fingerRemoteAccount(ctx,
+		accUsername, latestAcc.Domain, _, err = d.fingerRemoteAccount(ctx,
 			tsport,
 			latestAcc.Username,
 			accHost,
@@ -698,6 +711,9 @@ func (d *Dereferencer) enrichAccount(
 				latestAcc.Username, accHost, err,
 			)
 		}
+
+		// Specifically only update username if not already set.
+		latestAcc.Username = cmp.Or(latestAcc.Username, accUsername)
 	}
 
 	if latestAcc.Domain == "" {
@@ -706,23 +722,20 @@ func (d *Dereferencer) enrichAccount(
 		return nil, nil, gtserror.Newf("empty domain for %s", uri)
 	}
 
-	// Ensure the final parsed account URI or URL matches
+	// Ensure the final parsed account URI matches
 	// the input URI we fetched (or received) it as.
-	matches, err := util.URIMatches(
+	if matches, err := util.URIMatches(
 		uri,
 		append(
 			ap.GetURL(apubAcc),      // account URL(s)
 			ap.GetJSONLDId(apubAcc), // account URI
 		)...,
-	)
-	if err != nil {
+	); err != nil {
 		return nil, nil, gtserror.Newf(
 			"error checking dereferenced account uri %s: %w",
 			latestAcc.URI, err,
 		)
-	}
-
-	if !matches {
+	} else if !matches {
 		return nil, nil, gtserror.Newf(
 			"dereferenced account uri %s does not match %s",
 			latestAcc.URI, uri.String(),
diff --git a/internal/federation/dereferencing/finger.go b/internal/federation/dereferencing/finger.go
index 1b3e915ba..d11950a1e 100644
--- a/internal/federation/dereferencing/finger.go
+++ b/internal/federation/dereferencing/finger.go
@@ -45,6 +45,7 @@ func (d *Dereferencer) fingerRemoteAccount(
 	username string,
 	host string,
 ) (
+	string, // discovered username
 	string, // discovered account domain
 	*url.URL, // discovered account URI
 	error,
@@ -55,31 +56,30 @@ func (d *Dereferencer) fingerRemoteAccount(
 	b, err := transport.Finger(ctx, username, host)
 	if err != nil {
 		err = gtserror.Newf("error webfingering %s: %w", target, err)
-		return "", nil, err
+		return "", "", nil, err
 	}
 
 	var resp apimodel.WellKnownResponse
 	if err := json.Unmarshal(b, &resp); err != nil {
 		err = gtserror.Newf("error parsing response as JSON for %s: %w", target, err)
-		return "", nil, err
+		return "", "", nil, err
 	}
 
 	if len(resp.Links) == 0 {
 		err = gtserror.Newf("no links found in response for %s", target)
-		return "", nil, err
+		return "", "", nil, err
 	}
 
 	if resp.Subject == "" {
 		err = gtserror.Newf("no subject found in response for %s", target)
-		return "", nil, err
+		return "", "", nil, err
 	}
 
 	accUsername, accDomain, err := util.ExtractWebfingerParts(resp.Subject)
 	if err != nil {
-		err = gtserror.Newf("error extracting subject parts for %s: %w", target, err)
-		return "", nil, err
+		return "", "", nil, gtserror.Newf("error extracting subject parts for %s: %w", target, err)
 	} else if accUsername != username {
-		return "", nil, gtserror.Newf("response username does not match input for %s: %w", target, err)
+		return "", "", nil, gtserror.Newf("response username does not match input for %s: %w", target, err)
 	}
 
 	// Look through links for the first
@@ -122,8 +122,8 @@ func (d *Dereferencer) fingerRemoteAccount(
 		}
 
 		// All looks good, return happily!
-		return accDomain, uri, nil
+		return accUsername, accDomain, uri, nil
 	}
 
-	return "", nil, gtserror.Newf("no suitable self, AP-type link found in webfinger response for %s", target)
+	return "", "", nil, gtserror.Newf("no suitable self, AP-type link found in webfinger response for %s", target)
 }