Clean up and consolidate AP actor resolution logic

- Consolidate Person, Service and Application to adhere to a single
  interface type.
- Add additional error checking around actor properties.
- Remove redundant IRI->Actor resolution callbacks and use only the ones
  in the ResolveIRI method.

activitypub/apmodels/actor.go

@@ -1,6 +1,7 @@
 package apmodels
 
 import (
+	"errors"
 	"fmt"
 	"net/url"
 	"time"
@@ -40,61 +41,53 @@ type DeleteRequest struct {
 	ActorIri string
 }
 
-// MakeActorFromPerson takes a full ActivityPub Person and returns our internal
+// ExternalEntity represents an ActivityPub Person, Service or Application.
-// representation of an actor.
+type ExternalEntity interface {
-func MakeActorFromPerson(person vocab.ActivityStreamsPerson) ActivityPubActor {
+	GetJSONLDId() vocab.JSONLDIdProperty
+	GetActivityStreamsInbox() vocab.ActivityStreamsInboxProperty
+	GetActivityStreamsName() vocab.ActivityStreamsNameProperty
+	GetActivityStreamsPreferredUsername() vocab.ActivityStreamsPreferredUsernameProperty
+	GetActivityStreamsIcon() vocab.ActivityStreamsIconProperty
+	GetW3IDSecurityV1PublicKey() vocab.W3IDSecurityV1PublicKeyProperty
+}
+
+// MakeActorFromExernalAPEntity takes a full ActivityPub entity and returns our
+// internal representation of an actor.
+func MakeActorFromExernalAPEntity(entity ExternalEntity) (*ActivityPubActor, error) {
+	// Username is required (but not a part of the official ActivityPub spec)
+	if entity.GetActivityStreamsPreferredUsername() == nil || entity.GetActivityStreamsPreferredUsername().GetXMLSchemaString() == "" {
+		return nil, errors.New("remote activitypub entity does not have a preferred username set, rejecting")
+	}
+	username := GetFullUsernameFromExternalEntity(entity)
+
+	// Key is required
+	if entity.GetW3IDSecurityV1PublicKey() == nil {
+		return nil, errors.New("remote activitypub entity does not have a public key set, rejecting")
+	}
+
+	// Name is optional
+	var name string
+	if entity.GetActivityStreamsName() != nil && !entity.GetActivityStreamsName().Empty() {
+		name = entity.GetActivityStreamsName().At(0).GetXMLSchemaString()
+	}
+
+	// Image is optional
+	var image *url.URL
+	if entity.GetActivityStreamsIcon() != nil && !entity.GetActivityStreamsIcon().Empty() && entity.GetActivityStreamsIcon().At(0).GetActivityStreamsImage() != nil {
+		image = entity.GetActivityStreamsIcon().At(0).GetActivityStreamsImage().GetActivityStreamsUrl().Begin().GetIRI()
+	}
+
 	apActor := ActivityPubActor{
-		ActorIri:                person.GetJSONLDId().Get(),
+		ActorIri:                entity.GetJSONLDId().Get(),
-		Inbox:                   person.GetActivityStreamsInbox().GetIRI(),
+		Inbox:                   entity.GetActivityStreamsInbox().GetIRI(),
-		Name:                    person.GetActivityStreamsName().Begin().GetXMLSchemaString(),
+		Name:                    name,
-		Username:                person.GetActivityStreamsPreferredUsername().GetXMLSchemaString(),
+		Username:                entity.GetActivityStreamsPreferredUsername().GetXMLSchemaString(),
-		FullUsername:            GetFullUsernameFromPerson(person),
+		FullUsername:            username,
-		W3IDSecurityV1PublicKey: person.GetW3IDSecurityV1PublicKey(),
+		W3IDSecurityV1PublicKey: entity.GetW3IDSecurityV1PublicKey(),
+		Image:                   image,
 	}
 
-	if person.GetActivityStreamsIcon() != nil && person.GetActivityStreamsIcon().Len() > 0 && person.GetActivityStreamsIcon().At(0).GetActivityStreamsImage() != nil {
+	return &apActor, nil
-		apActor.Image = person.GetActivityStreamsIcon().At(0).GetActivityStreamsImage().GetActivityStreamsUrl().Begin().GetIRI()
-	}
-
-	return apActor
-}
-
-// MakeActorFromService takes a full ActivityPub Service and returns our internal
-// representation of an actor.
-func MakeActorFromService(service vocab.ActivityStreamsService) ActivityPubActor {
-	apActor := ActivityPubActor{
-		ActorIri:                service.GetJSONLDId().Get(),
-		Inbox:                   service.GetActivityStreamsInbox().GetIRI(),
-		Name:                    service.GetActivityStreamsName().Begin().GetXMLSchemaString(),
-		Username:                service.GetActivityStreamsPreferredUsername().GetXMLSchemaString(),
-		FullUsername:            GetFullUsernameFromService(service),
-		W3IDSecurityV1PublicKey: service.GetW3IDSecurityV1PublicKey(),
-	}
-
-	if service.GetActivityStreamsIcon() != nil && service.GetActivityStreamsIcon().Len() > 0 && service.GetActivityStreamsIcon().At(0).GetActivityStreamsImage() != nil {
-		apActor.Image = service.GetActivityStreamsIcon().At(0).GetActivityStreamsImage().GetActivityStreamsUrl().Begin().GetIRI()
-	}
-
-	return apActor
-}
-
-// MakeActorFromApplication takes a full ActivityPub application and returns
-// our internal representation of an actor.
-func MakeActorFromApplication(application vocab.ActivityStreamsApplication) ActivityPubActor {
-	apActor := ActivityPubActor{
-		ActorIri:                application.GetJSONLDId().Get(),
-		Inbox:                   application.GetActivityStreamsInbox().GetIRI(),
-		Name:                    application.GetActivityStreamsName().Begin().GetXMLSchemaString(),
-		Username:                application.GetActivityStreamsPreferredUsername().GetXMLSchemaString(),
-		FullUsername:            GetFullUsernameFromApplication(application),
-		W3IDSecurityV1PublicKey: application.GetW3IDSecurityV1PublicKey(),
-	}
-
-	if application.GetActivityStreamsIcon() != nil && application.GetActivityStreamsIcon().Len() > 0 && application.GetActivityStreamsIcon().At(0).GetActivityStreamsImage() != nil {
-		apActor.Image = application.GetActivityStreamsIcon().At(0).GetActivityStreamsImage().GetActivityStreamsUrl().Begin().GetIRI()
-	}
-
-	return apActor
 }
 
 // MakeActorPropertyWithID will return an actor property filled with the provided IRI.
@@ -239,28 +232,11 @@ func MakeServiceForAccount(accountName string) vocab.ActivityStreamsService {
 	return person
 }
 
-// GetFullUsernameFromPerson will return the user@host.tld formatted user given a person object.
+// GetFullUsernameFromExternalEntity will return the full username from an
-func GetFullUsernameFromPerson(person vocab.ActivityStreamsPerson) string {
+// internal representation of an ExternalEntity. Returns user@host.tld.
-	hostname := person.GetJSONLDId().GetIRI().Hostname()
+func GetFullUsernameFromExternalEntity(entity ExternalEntity) string {
-	username := person.GetActivityStreamsPreferredUsername().GetXMLSchemaString()
+	hostname := entity.GetJSONLDId().GetIRI().Hostname()
-	fullUsername := fmt.Sprintf("%s@%s", username, hostname)
+	username := entity.GetActivityStreamsPreferredUsername().GetXMLSchemaString()
-
-	return fullUsername
-}
-
-// GetFullUsernameFromService will return the user@host.tld formatted user given a service object.
-func GetFullUsernameFromService(person vocab.ActivityStreamsService) string {
-	hostname := person.GetJSONLDId().GetIRI().Hostname()
-	username := person.GetActivityStreamsPreferredUsername().GetXMLSchemaString()
-	fullUsername := fmt.Sprintf("%s@%s", username, hostname)
-
-	return fullUsername
-}
-
-// GetFullUsernameFromApplication will return the user@host.tld formatted user given a service object.
-func GetFullUsernameFromApplication(person vocab.ActivityStreamsApplication) string {
-	hostname := person.GetJSONLDId().GetIRI().Hostname()
-	username := person.GetActivityStreamsPreferredUsername().GetXMLSchemaString()
 	fullUsername := fmt.Sprintf("%s@%s", username, hostname)
 
 	return fullUsername

activitypub/apmodels/actor_test.go

@@ -44,6 +44,9 @@ func makeFakeService() vocab.ActivityStreamsService {
 	icon.AppendActivityStreamsImage(image)
 	service.SetActivityStreamsIcon(icon)
 
+	publicKeyProperty := streams.NewW3IDSecurityV1PublicKeyProperty()
+	service.SetW3IDSecurityV1PublicKey(publicKeyProperty)
+
 	return service
 }
 
@@ -59,9 +62,12 @@ func TestMain(m *testing.M) {
 	m.Run()
 }
 
-func TestMakeActorFromService(t *testing.T) {
+func TestMakeActorFromExternalAPEntity(t *testing.T) {
 	service := makeFakeService()
-	actor := MakeActorFromService(service)
+	actor, err := MakeActorFromExernalAPEntity(service)
+	if err != nil {
+		t.Error(err)
+	}
 
 	if actor.ActorIri != service.GetJSONLDId().GetIRI() {
 		t.Errorf("actor.ID = %v, want %v", actor.ActorIri, service.GetJSONLDId().GetIRI())
@@ -96,7 +102,7 @@ func TestMakeActorPropertyWithID(t *testing.T) {
 func TestGetFullUsernameFromPerson(t *testing.T) {
 	expected := "foodawg@fake.fediverse.server"
 	person := makeFakeService()
-	username := GetFullUsernameFromService(person)
+	username := GetFullUsernameFromExternalEntity(person)
 
 	if username != expected {
 		t.Errorf("actor.Username = %v, want %v", username, expected)

activitypub/resolvers/resolve.go

@@ -49,7 +49,7 @@ func Resolve(c context.Context, data []byte, callbacks ...interface{}) error {
 func ResolveIRI(c context.Context, iri string, callbacks ...interface{}) error {
 	log.Debugln("Resolving", iri)
 
-	req, _ := http.NewRequest("GET", iri, nil)
+	req, _ := http.NewRequest(http.MethodGet, iri, nil)
 
 	actor := apmodels.MakeLocalIRIForAccount(data.GetDefaultFederationUsername())
 	if err := crypto.SignRequest(req, nil, actor); err != nil {
@@ -72,56 +72,53 @@ func ResolveIRI(c context.Context, iri string, callbacks ...interface{}) error {
 	return Resolve(c, data, callbacks...)
 }
 
-// GetResolvedActorFromActorProperty resolve an actor property to a fully populated person.
+// GetResolvedActorFromActorProperty resolve an external actor property to a
+// fully populated internal actor representation.
 func GetResolvedActorFromActorProperty(actor vocab.ActivityStreamsActorProperty) (apmodels.ActivityPubActor, error) {
 	var err error
 	var apActor apmodels.ActivityPubActor
 	resolved := false
 
-	personCallback := func(c context.Context, person vocab.ActivityStreamsPerson) error {
+	if !actor.Empty() && actor.Len() > 0 && actor.At(0) != nil {
-		apActor = apmodels.MakeActorFromPerson(person)
+		// Explicitly use only the first actor that might be listed.
-		return nil
+		actorObjectOrIRI := actor.At(0)
+		var actorEntity apmodels.ExternalEntity
+
+		// If the actor is an unresolved IRI then we need to resolve it.
+		if actorObjectOrIRI.IsIRI() {
+			iri := actorObjectOrIRI.GetIRI().String()
+			return GetResolvedActorFromIRI(iri)
 		}
 
-	serviceCallback := func(c context.Context, s vocab.ActivityStreamsService) error {
+		if actorObjectOrIRI.IsActivityStreamsPerson() {
-		apActor = apmodels.MakeActorFromService(s)
+			actorEntity = actorObjectOrIRI.GetActivityStreamsPerson()
-		return nil
+		} else if actorObjectOrIRI.IsActivityStreamsService() {
+			actorEntity = actorObjectOrIRI.GetActivityStreamsService()
+		} else if actorObjectOrIRI.IsActivityStreamsApplication() {
+			actorEntity = actorObjectOrIRI.GetActivityStreamsApplication()
+		} else {
+			err = errors.New("unrecognized external ActivityPub type: " + actorObjectOrIRI.Name())
+			return apActor, err
 		}
 
-	applicationCallback := func(c context.Context, s vocab.ActivityStreamsApplication) error {
+		// If any of the resolution or population failed then return the error.
-		apActor = apmodels.MakeActorFromApplication(s)
+		if err != nil {
+			return apActor, err
+		}
+
+		// Convert the external AP entity into an internal actor representation.
+		apa, e := apmodels.MakeActorFromExernalAPEntity(actorEntity)
+		if apa != nil {
+			apActor = *apa
 			resolved = true
-		return nil
 		}
-
-	for iter := actor.Begin(); iter != actor.End(); iter = iter.Next() {
-		if iter.IsIRI() {
-			iri := iter.GetIRI()
-			if e := ResolveIRI(context.Background(), iri.String(), personCallback, serviceCallback, applicationCallback); e != nil {
 		err = e
 	}
-		} else if iter.IsActivityStreamsPerson() {
+
-			person := iter.GetActivityStreamsPerson()
+	if !resolved && err == nil {
-			apActor = apmodels.MakeActorFromPerson(person)
+		err = errors.New("unknown error resolving actor from property value")
-			resolved = true
-		} else if iter.IsActivityStreamsService() {
-			person := iter.GetActivityStreamsService()
-			apActor = apmodels.MakeActorFromService(person)
-			resolved = true
-		} else if iter.IsActivityStreamsApplication() {
-			person := iter.GetActivityStreamsApplication()
-			apActor = apmodels.MakeActorFromApplication(person)
-			resolved = true
-		}
 	}
 
-	if err != nil {
-		err = errors.Wrap(err, "error resolving actor from property value")
-	}
-
-	if !resolved {
-		err = errors.New("error resolving actor from property value")
-	}
 	return apActor, err
 }
 
@@ -131,21 +128,30 @@ func GetResolvedActorFromIRI(personOrServiceIRI string) (apmodels.ActivityPubAct
 	var apActor apmodels.ActivityPubActor
 	resolved := false
 	personCallback := func(c context.Context, person vocab.ActivityStreamsPerson) error {
-		apActor = apmodels.MakeActorFromPerson(person)
+		apa, e := apmodels.MakeActorFromExernalAPEntity(person)
+		if apa != nil {
+			apActor = *apa
 			resolved = true
-		return nil
+		}
+		return e
 	}
 
-	serviceCallback := func(c context.Context, s vocab.ActivityStreamsService) error {
+	serviceCallback := func(c context.Context, service vocab.ActivityStreamsService) error {
-		apActor = apmodels.MakeActorFromService(s)
+		apa, e := apmodels.MakeActorFromExernalAPEntity(service)
+		if apa != nil {
+			apActor = *apa
 			resolved = true
-		return nil
+		}
+		return e
 	}
 
-	applicationCallback := func(c context.Context, s vocab.ActivityStreamsApplication) error {
+	applicationCallback := func(c context.Context, app vocab.ActivityStreamsApplication) error {
-		apActor = apmodels.MakeActorFromApplication(s)
+		apa, e := apmodels.MakeActorFromExernalAPEntity(app)
+		if apa != nil {
+			apActor = *apa
 			resolved = true
-		return nil
+		}
+		return e
 	}
 
 	if e := ResolveIRI(context.Background(), personOrServiceIRI, personCallback, serviceCallback, applicationCallback); e != nil {