[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, 90 insertions, 12 deletions

diff --git a/internal/typeutils/astointernal.go b/internal/typeutils/astointernal.go
index 2946c8d09..5ff60b09c 100644
--- a/internal/typeutils/astointernal.go
+++ b/internal/typeutils/astointernal.go
@@ -18,6 +18,7 @@
 package typeutils
 
 import (
+	"cmp"
 	"context"
 	"errors"
 	"net/url"
@@ -33,10 +34,24 @@ import (
 	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
-// ASRepresentationToAccount converts a remote account/person/application representation into a gts model account.
+// ASRepresentationToAccount converts a remote account / person
+// / application representation into a gts model account.
 //
-// If accountDomain is provided then this value will be used as the account's Domain, else the AP ID host.
-func (c *Converter) ASRepresentationToAccount(ctx context.Context, accountable ap.Accountable, accountDomain string) (*gtsmodel.Account, error) {
+// If accountDomain is provided then this value will be
+// used as the account's Domain, else the AP ID host.
+//
+// If accountUsername is provided then this is used as
+// a fallback when no preferredUsername is provided. Else
+// a lack of username will result in error return.
+func (c *Converter) ASRepresentationToAccount(
+	ctx context.Context,
+	accountable ap.Accountable,
+	accountDomain string,
+	accountUsername string,
+) (
+	*gtsmodel.Account,
+	error,
+) {
 	var err error
 
 	// Extract URI from accountable
@@ -70,10 +85,17 @@ func (c *Converter) ASRepresentationToAccount(ctx context.Context, accountable a
 		return nil, gtserror.SetMalformed(err)
 	}
 
-	// Extract preferredUsername, this is a *requirement*.
-	acct.Username, err = ap.ExtractPreferredUsername(accountable)
-	if err != nil {
-		err := gtserror.Newf("unusable username for %s", uri)
+	// Set account username.
+	acct.Username = cmp.Or(
+
+		// Prefer the AP model provided username.
+		ap.ExtractPreferredUsername(accountable),
+
+		// Fallback username.
+		accountUsername,
+	)
+	if acct.Username == "" {
+		err := gtserror.Newf("missing username for %s", uri)
 		return nil, gtserror.SetMalformed(err)
 	}
 
diff --git a/internal/typeutils/astointernal_test.go b/internal/typeutils/astointernal_test.go
index 3efc15999..998a3e974 100644
--- a/internal/typeutils/astointernal_test.go
+++ b/internal/typeutils/astointernal_test.go
@@ -65,7 +65,7 @@ func (suite *ASToInternalTestSuite) jsonToType(in string) vocab.Type {
 func (suite *ASToInternalTestSuite) TestParsePerson() {
 	testPerson := suite.testPeople["https://unknown-instance.com/users/brand_new_person"]
 
-	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), testPerson, "")
+	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), testPerson, "", "")
 	suite.NoError(err)
 
 	suite.Equal("https://unknown-instance.com/users/brand_new_person", acct.URI)
@@ -87,7 +87,7 @@ func (suite *ASToInternalTestSuite) TestParsePerson() {
 func (suite *ASToInternalTestSuite) TestParsePersonWithSharedInbox() {
 	testPerson := suite.testPeople["https://turnip.farm/users/turniplover6969"]
 
-	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), testPerson, "")
+	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), testPerson, "", "")
 	suite.NoError(err)
 
 	suite.Equal("https://turnip.farm/users/turniplover6969", acct.URI)
@@ -145,7 +145,7 @@ func (suite *ASToInternalTestSuite) TestParseGargron() {
 		suite.FailNow("type not coercible")
 	}
 
-	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), rep, "")
+	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), rep, "", "")
 	suite.NoError(err)
 	suite.Equal("https://mastodon.social/inbox", *acct.SharedInboxURI)
 	suite.Equal([]string{"https://tooting.ai/users/Gargron"}, acct.AlsoKnownAsURIs)
@@ -196,7 +196,7 @@ func (suite *ASToInternalTestSuite) TestParseOwncastService() {
 		suite.FailNow("type not coercible")
 	}
 
-	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), rep, "")
+	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), rep, "", "")
 	suite.NoError(err)
 
 	suite.Equal("rgh", acct.Username)
@@ -547,7 +547,7 @@ func (suite *ASToInternalTestSuite) TestParseHonkAccount() {
 		suite.FailNow("type not coercible")
 	}
 
-	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), rep, "")
+	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), rep, "", "")
 	suite.NoError(err)
 	suite.Equal("https://honk.example.org/u/honk_user/followers", acct.FollowersURI)
 	suite.Equal("https://honk.example.org/u/honk_user/following", acct.FollowingURI)
@@ -651,6 +651,62 @@ func (suite *ASToInternalTestSuite) TestParseHonkAccount() {
 	suite.False(*dbAcct.Discoverable)
 }
 
+func (suite *ASToInternalTestSuite) TestParseAccountableWithoutPreferredUsername() {
+	ctx, cncl := context.WithCancel(context.Background())
+	defer cncl()
+
+	testPerson := suite.testPeople["https://unknown-instance.com/users/brand_new_person"]
+	// preferredUsername := "newish_person_actually"
+	username := "brand_new_person"
+
+	// Specifically unset the preferred_username field.
+	testPerson.SetActivityStreamsPreferredUsername(nil)
+
+	// Attempt to parse account model from ActivityStreams.
+	// This should fall back to the passed username argument as no preferred_username is set.
+	acc, err := suite.typeconverter.ASRepresentationToAccount(ctx, testPerson, "", username)
+	suite.NoError(err)
+	suite.Equal(acc.Username, username)
+}
+
+func (suite *ASToInternalTestSuite) TestParseAccountableWithoutAnyUsername() {
+	ctx, cncl := context.WithCancel(context.Background())
+	defer cncl()
+
+	testPerson := suite.testPeople["https://unknown-instance.com/users/brand_new_person"]
+	// preferredUsername := "newish_person_actually"
+	// username := "brand_new_person"
+
+	// Specifically unset the preferred_username field.
+	testPerson.SetActivityStreamsPreferredUsername(nil)
+
+	// Attempt to parse account model from ActivityStreams.
+	// This should return error as we provide no username and no preferred_username is set.
+	acc, err := suite.typeconverter.ASRepresentationToAccount(ctx, testPerson, "", "")
+	suite.Equal(err.Error(), "ASRepresentationToAccount: missing username for https://unknown-instance.com/users/brand_new_person")
+	suite.Nil(acc)
+}
+
+func (suite *ASToInternalTestSuite) TestParseAccountableWithPreferredUsername() {
+	ctx, cncl := context.WithCancel(context.Background())
+	defer cncl()
+
+	testPerson := suite.testPeople["https://unknown-instance.com/users/brand_new_person"]
+	preferredUsername := "newish_person_actually"
+	username := "brand_new_person"
+
+	// Specifically set a known preferred_username field.
+	prop := streams.NewActivityStreamsPreferredUsernameProperty()
+	prop.SetXMLSchemaString(preferredUsername)
+	testPerson.SetActivityStreamsPreferredUsername(prop)
+
+	// Attempt to parse account model from ActivityStreams.
+	// This should use the ActivityStreams preferred_username, instead of the passed argument.
+	acc, err := suite.typeconverter.ASRepresentationToAccount(ctx, testPerson, "", username)
+	suite.NoError(err)
+	suite.Equal(acc.Username, preferredUsername)
+}
+
 func TestASToInternalTestSuite(t *testing.T) {
 	suite.Run(t, new(ASToInternalTestSuite))
 }