Derive visibility fixes (#271)

* use pub public const

* don't error on no summary

* move extract visibility to separate function

* extract visibility test

* add addressable interface

internal/ap/extract.go

@@ -253,7 +253,7 @@ func ExtractSummary(i WithSummary) (string, error) {
 		}
 	}
 
-	return "", errors.New("could not extract summary")
+	return "", nil
 }
 
 // ExtractDiscoverable extracts the Discoverable boolean of an interface.
@@ -610,3 +610,67 @@ func ExtractObject(i WithObject) (*url.URL, error) {
 	}
 	return nil, errors.New("no iri found for object prop")
 }
+
+// ExtractVisibility extracts the gtsmodel.Visibility of a given addressable with a To and CC property.
+//
+// ActorFollowersURI is needed to check whether the visibility is FollowersOnly or not. The passed-in value
+// should just be the string value representation of the followers URI of the actor who created the activity,
+// eg https://example.org/users/whoever/followers.
+func ExtractVisibility(addressable Addressable, actorFollowersURI string) (gtsmodel.Visibility, error) {
+	to, err := ExtractTos(addressable)
+	if err != nil {
+		return "", fmt.Errorf("deriveVisibility: error extracting TO values: %s", err)
+	}
+
+	cc, err := ExtractCCs(addressable)
+	if err != nil {
+		return "", fmt.Errorf("deriveVisibility: error extracting CC values: %s", err)
+	}
+
+	if len(to) == 0 && len(cc) == 0 {
+		return "", errors.New("deriveVisibility: message wasn't TO or CC anyone")
+	}
+
+	// for visibility derivation, we start by assuming most restrictive, and work our way to least restrictive
+	visibility := gtsmodel.VisibilityDirect
+
+	// if it's got followers in TO and it's not also CC'ed to public, it's followers only
+	if isFollowers(to, actorFollowersURI) {
+		visibility = gtsmodel.VisibilityFollowersOnly
+	}
+
+	// if it's CC'ed to public, it's unlocked
+	// mentioned SPECIFIC ACCOUNTS also get added to CC'es if it's not a direct message
+	if isPublic(cc) {
+		visibility = gtsmodel.VisibilityUnlocked
+	}
+
+	// if it's To public, it's just straight up public
+	if isPublic(to) {
+		visibility = gtsmodel.VisibilityPublic
+	}
+
+	return visibility, nil
+}
+
+// isPublic checks if at least one entry in the given uris slice equals
+// the activitystreams public uri.
+func isPublic(uris []*url.URL) bool {
+	for _, entry := range uris {
+		if strings.EqualFold(entry.String(), pub.PublicActivityPubIRI) {
+			return true
+		}
+	}
+	return false
+}
+
+// isFollowers checks if at least one entry in the given uris slice equals
+// the given followersURI.
+func isFollowers(uris []*url.URL, followersURI string) bool {
+	for _, entry := range uris {
+		if strings.EqualFold(entry.String(), followersURI) {
+			return true
+		}
+	}
+	return false
+}

internal/ap/extract_test.go

@@ -19,9 +19,14 @@
 package ap_test
 
 import (
+	"context"
+	"encoding/json"
+
+	"github.com/go-fed/activity/pub"
 	"github.com/go-fed/activity/streams"
 	"github.com/go-fed/activity/streams/vocab"
 	"github.com/stretchr/testify/suite"
+	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/testrig"
 )
 
@@ -86,15 +91,111 @@ func noteWithMentions1() vocab.ActivityStreamsNote {
 	return note
 }
 
+func addressable1() ap.Addressable {
+	// make a note addressed to public with followers in cc
+	note := streams.NewActivityStreamsNote()
+
+	toProp := streams.NewActivityStreamsToProperty()
+	toProp.AppendIRI(testrig.URLMustParse(pub.PublicActivityPubIRI))
+
+	note.SetActivityStreamsTo(toProp)
+
+	ccProp := streams.NewActivityStreamsCcProperty()
+	ccProp.AppendIRI(testrig.URLMustParse("http://localhost:8080/users/the_mighty_zork/followers"))
+
+	note.SetActivityStreamsCc(ccProp)
+
+	return note
+}
+
+func addressable2() ap.Addressable {
+	// make a note addressed to followers with public in cc
+	note := streams.NewActivityStreamsNote()
+
+	toProp := streams.NewActivityStreamsToProperty()
+	toProp.AppendIRI(testrig.URLMustParse("http://localhost:8080/users/the_mighty_zork/followers"))
+
+	note.SetActivityStreamsTo(toProp)
+
+	ccProp := streams.NewActivityStreamsCcProperty()
+	ccProp.AppendIRI(testrig.URLMustParse(pub.PublicActivityPubIRI))
+
+	note.SetActivityStreamsCc(ccProp)
+
+	return note
+}
+
+func addressable3() ap.Addressable {
+	// make a note addressed to followers
+	note := streams.NewActivityStreamsNote()
+
+	toProp := streams.NewActivityStreamsToProperty()
+	toProp.AppendIRI(testrig.URLMustParse("http://localhost:8080/users/the_mighty_zork/followers"))
+
+	note.SetActivityStreamsTo(toProp)
+
+	return note
+}
+
+func addressable4() vocab.ActivityStreamsAnnounce {
+	// https://github.com/superseriousbusiness/gotosocial/issues/267
+	announceJson := []byte(`
+{
+	"@context": "https://www.w3.org/ns/activitystreams",
+	"actor": "https://example.org/users/someone",
+	"cc": "https://another.instance/users/someone_else",
+	"id": "https://example.org/users/someone/statuses/107043888547829808/activity",
+	"object": "https://another.instance/users/someone_else/statuses/107026674805188668",
+	"published": "2021-10-04T15:08:35Z",
+	"to": "https://example.org/users/someone/followers",
+	"type": "Announce"
+}`)
+
+	var jsonAsMap map[string]interface{}
+	err := json.Unmarshal(announceJson, &jsonAsMap)
+	if err != nil {
+		panic(err)
+	}
+
+	t, err := streams.ToType(context.Background(), jsonAsMap)
+	if err != nil {
+		panic(err)
+	}
+
+	return t.(vocab.ActivityStreamsAnnounce)
+}
+
+func addressable5() ap.Addressable {
+	// make a note addressed to one person (direct message)
+	note := streams.NewActivityStreamsNote()
+
+	toProp := streams.NewActivityStreamsToProperty()
+	toProp.AppendIRI(testrig.URLMustParse("http://localhost:8080/users/1_happy_turtle"))
+
+	note.SetActivityStreamsTo(toProp)
+
+	return note
+}
+
 type ExtractTestSuite struct {
 	suite.Suite
 	document1         vocab.ActivityStreamsDocument
 	attachment1       vocab.ActivityStreamsAttachmentProperty
 	noteWithMentions1 vocab.ActivityStreamsNote
+	addressable1      ap.Addressable
+	addressable2      ap.Addressable
+	addressable3      ap.Addressable
+	addressable4      vocab.ActivityStreamsAnnounce
+	addressable5      ap.Addressable
 }
 
 func (suite *ExtractTestSuite) SetupTest() {
 	suite.document1 = document1()
 	suite.attachment1 = attachment1()
 	suite.noteWithMentions1 = noteWithMentions1()
+	suite.addressable1 = addressable1()
+	suite.addressable2 = addressable2()
+	suite.addressable3 = addressable3()
+	suite.addressable4 = addressable4()
+	suite.addressable5 = addressable5()
 }

internal/ap/extractvisibility_test.go

@@ -0,0 +1,71 @@
+/*
+   GoToSocial
+   Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU Affero General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU Affero General Public License for more details.
+
+   You should have received a copy of the GNU Affero General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+package ap_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+	"github.com/superseriousbusiness/gotosocial/internal/ap"
+	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+)
+
+type ExtractVisibilityTestSuite struct {
+	ExtractTestSuite
+}
+
+func (suite *ExtractVisibilityTestSuite) TestExtractVisibilityPublic() {
+	a := suite.addressable1
+	visibility, err := ap.ExtractVisibility(a, "http://localhost:8080/users/the_mighty_zork/followers")
+	suite.NoError(err)
+	suite.Equal(visibility, gtsmodel.VisibilityPublic)
+}
+
+func (suite *ExtractVisibilityTestSuite) TestExtractVisibilityUnlocked() {
+	a := suite.addressable2
+	visibility, err := ap.ExtractVisibility(a, "http://localhost:8080/users/the_mighty_zork/followers")
+	suite.NoError(err)
+	suite.Equal(visibility, gtsmodel.VisibilityUnlocked)
+}
+
+func (suite *ExtractVisibilityTestSuite) TestExtractVisibilityFollowersOnly() {
+	a := suite.addressable3
+	visibility, err := ap.ExtractVisibility(a, "http://localhost:8080/users/the_mighty_zork/followers")
+	suite.NoError(err)
+	suite.Equal(visibility, gtsmodel.VisibilityFollowersOnly)
+}
+
+func (suite *ExtractVisibilityTestSuite) TestExtractVisibilityFollowersOnlyAnnounce() {
+	// https://github.com/superseriousbusiness/gotosocial/issues/267
+	a := suite.addressable4
+	visibility, err := ap.ExtractVisibility(a, "https://example.org/users/someone/followers")
+	suite.NoError(err)
+	suite.Equal(visibility, gtsmodel.VisibilityFollowersOnly)
+}
+
+func (suite *ExtractVisibilityTestSuite) TestExtractVisibilityDirect() {
+	a := suite.addressable5
+	visibility, err := ap.ExtractVisibility(a, "http://localhost:8080/users/the_mighty_zork/followers")
+	suite.NoError(err)
+	suite.Equal(visibility, gtsmodel.VisibilityDirect)
+}
+
+func TestExtractVisibilityTestSuite(t *testing.T) {
+	suite.Run(t, &ExtractVisibilityTestSuite{})
+}

internal/ap/interfaces.go

@@ -133,6 +133,12 @@ type Announceable interface {
 	WithCC
 }
 
+// Addressable represents the minimum interface for an addressed activity.
+type Addressable interface {
+	WithTo
+	WithCC
+}
+
 // CollectionPageable represents the minimum interface for an activitystreams 'CollectionPage' object.
 type CollectionPageable interface {
 	WithJSONLDId

internal/api/s2s/user/inboxpost_test.go

@@ -29,6 +29,7 @@
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/go-fed/activity/pub"
 	"github.com/go-fed/activity/streams"
 	"github.com/stretchr/testify/suite"
 	"github.com/superseriousbusiness/gotosocial/internal/api/s2s/user"
@@ -247,7 +248,7 @@ func (suite *InboxPostTestSuite) TestPostUpdate() {
 
 	// Set the To of the update as public
 	updateTo := streams.NewActivityStreamsToProperty()
-	updateTo.AppendIRI(testrig.URLMustParse("https://www.w3.org/ns/activitystreams#Public"))
+	updateTo.AppendIRI(testrig.URLMustParse(pub.PublicActivityPubIRI))
 	update.SetActivityStreamsTo(updateTo)
 
 	// set the cc of the update to the receivingAccount
@@ -370,7 +371,7 @@ func (suite *InboxPostTestSuite) TestPostDelete() {
 
 	// Set the To of the delete as public
 	deleteTo := streams.NewActivityStreamsToProperty()
-	deleteTo.AppendIRI(testrig.URLMustParse("https://www.w3.org/ns/activitystreams#Public"))
+	deleteTo.AppendIRI(testrig.URLMustParse(pub.PublicActivityPubIRI))
 	delete.SetActivityStreamsTo(deleteTo)
 
 	// set some random-ass ID for the activity

internal/typeutils/astointernal.go

@@ -22,8 +22,6 @@
 	"context"
 	"errors"
 	"fmt"
-	"net/url"
-	"strings"
 
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
@@ -245,13 +243,13 @@ func (c *converter) ASStatusToStatus(ctx context.Context, statusable ap.Statusab
 	// if we don't know the account yet we can dereference it later
 	attributedTo, err := ap.ExtractAttributedTo(statusable)
 	if err != nil {
-		return nil, errors.New("attributedTo was empty")
+		return nil, errors.New("ASStatusToStatus: attributedTo was empty")
 	}
 	status.AccountURI = attributedTo.String()
 
 	statusOwner, err := c.db.GetAccountByURI(ctx, attributedTo.String())
 	if err != nil {
-		return nil, fmt.Errorf("couldn't get status owner from db: %s", err)
+		return nil, fmt.Errorf("ASStatusToStatus: couldn't get status owner from db: %s", err)
 	}
 	status.AccountID = statusOwner.ID
 	status.AccountURI = statusOwner.URI
@@ -280,46 +278,9 @@ func (c *converter) ASStatusToStatus(ctx context.Context, statusable ap.Statusab
 	}
 
 	// visibility entry for this status
-	var visibility gtsmodel.Visibility
-
-	to, err := ap.ExtractTos(statusable)
+	visibility, err := ap.ExtractVisibility(statusable, status.Account.FollowersURI)
 	if err != nil {
-		return nil, fmt.Errorf("error extracting TO values: %s", err)
-	}
-
-	cc, err := ap.ExtractCCs(statusable)
-	if err != nil {
-		return nil, fmt.Errorf("error extracting CC values: %s", err)
-	}
-
-	if len(to) == 0 && len(cc) == 0 {
-		return nil, errors.New("message wasn't TO or CC anyone")
-	}
-
-	// for visibility derivation, we start by assuming most restrictive, and work our way to least restrictive
-
-	// if it's a DM then it's addressed to SPECIFIC ACCOUNTS and not followers or public
-	if len(to) != 0 && len(cc) == 0 {
-		visibility = gtsmodel.VisibilityDirect
-	}
-
-	// if it's just got followers in TO and it's not also CC'ed to public, it's followers only
-	if isFollowers(to, statusOwner.FollowersURI) {
-		visibility = gtsmodel.VisibilityFollowersOnly
-	}
-
-	// if it's CC'ed to public, it's public or unlocked
-	// mentioned SPECIFIC ACCOUNTS also get added to CC'es if it's not a direct message
-	if isPublic(cc) {
-		visibility = gtsmodel.VisibilityUnlocked
-	}
-	if isPublic(to) {
-		visibility = gtsmodel.VisibilityPublic
-	}
-
-	// we should have a visibility by now
-	if visibility == "" {
-		return nil, errors.New("couldn't derive visibility")
+		return nil, fmt.Errorf("ASStatusToStatus: error extracting visibility: %s", err)
 	}
 	status.Visibility = visibility
 
@@ -553,55 +514,12 @@ func (c *converter) ASAnnounceToStatus(ctx context.Context, announceable ap.Anno
 	status.MentionIDs = []string{}
 	status.EmojiIDs = []string{}
 
-	// parse the visibility from the To and CC entries
-	var visibility gtsmodel.Visibility
-
-	to, err := ap.ExtractTos(announceable)
+	visibility, err := ap.ExtractVisibility(announceable, boostingAccount.FollowersURI)
 	if err != nil {
-		return nil, isNew, fmt.Errorf("error extracting TO values: %s", err)
-	}
-
-	cc, err := ap.ExtractCCs(announceable)
-	if err != nil {
-		return nil, isNew, fmt.Errorf("error extracting CC values: %s", err)
-	}
-
-	if len(to) == 0 && len(cc) == 0 {
-		return nil, isNew, errors.New("message wasn't TO or CC anyone")
-	}
-
-	// if it's CC'ed to public, it's public or unlocked
-	if isPublic(cc) {
-		visibility = gtsmodel.VisibilityUnlocked
-	}
-	if isPublic(to) {
-		visibility = gtsmodel.VisibilityPublic
-	}
-
-	// we should have a visibility by now
-	if visibility == "" {
-		return nil, isNew, errors.New("couldn't derive visibility")
+		return nil, isNew, fmt.Errorf("ASAnnounceToStatus: error extracting visibility: %s", err)
 	}
 	status.Visibility = visibility
 
 	// the rest of the fields will be taken from the target status, but it's not our job to do the dereferencing here
 	return status, isNew, nil
 }
-
-func isPublic(tos []*url.URL) bool {
-	for _, entry := range tos {
-		if strings.EqualFold(entry.String(), "https://www.w3.org/ns/activitystreams#Public") {
-			return true
-		}
-	}
-	return false
-}
-
-func isFollowers(ccs []*url.URL, followersURI string) bool {
-	for _, entry := range ccs {
-		if strings.EqualFold(entry.String(), followersURI) {
-			return true
-		}
-	}
-	return false
-}

internal/typeutils/converter.go

@@ -32,10 +32,6 @@
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 )
 
-const (
-	asPublicURI = "https://www.w3.org/ns/activitystreams#Public"
-)
-
 // TypeConverter is an interface for the common action of converting between apimodule (frontend, serializable) models,
 // internal gts models used in the database, and activitypub models used in federation.
 //

internal/typeutils/internaltoas.go

@@ -25,6 +25,7 @@
 	"fmt"
 	"net/url"
 
+	"github.com/go-fed/activity/pub"
 	"github.com/go-fed/activity/streams"
 	"github.com/go-fed/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
@@ -444,9 +445,9 @@ func (c *converter) StatusToAS(ctx context.Context, s *gtsmodel.Status) (vocab.A
 		return nil, fmt.Errorf("StatusToAS: error parsing url %s: %s", s.Account.FollowersURI, err)
 	}
 
-	publicURI, err := url.Parse(asPublicURI)
+	publicURI, err := url.Parse(pub.PublicActivityPubIRI)
 	if err != nil {
-		return nil, fmt.Errorf("StatusToAS: error parsing url %s: %s", asPublicURI, err)
+		return nil, fmt.Errorf("StatusToAS: error parsing url %s: %s", pub.PublicActivityPubIRI, err)
 	}
 
 	// to and cc
@@ -795,9 +796,9 @@ func (c *converter) BoostToAS(ctx context.Context, boostWrapperStatus *gtsmodel.
 		return nil, fmt.Errorf("BoostToAS: error parsing uri %s: %s", boostedAccount.URI, err)
 	}
 
-	publicURI, err := url.Parse(asPublicURI)
+	publicURI, err := url.Parse(pub.PublicActivityPubIRI)
 	if err != nil {
-		return nil, fmt.Errorf("BoostToAS: error parsing uri %s: %s", asPublicURI, err)
+		return nil, fmt.Errorf("BoostToAS: error parsing uri %s: %s", pub.PublicActivityPubIRI, err)
 	}
 
 	ccProp := streams.NewActivityStreamsCcProperty()

internal/typeutils/wrap.go

@@ -4,6 +4,7 @@
 	"fmt"
 	"net/url"
 
+	"github.com/go-fed/activity/pub"
 	"github.com/go-fed/activity/streams"
 	"github.com/go-fed/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
@@ -46,9 +47,9 @@ func (c *converter) WrapPersonInUpdate(person vocab.ActivityStreamsPerson, origi
 	update.SetActivityStreamsObject(objectProp)
 
 	// to should be public
-	toURI, err := url.Parse(asPublicURI)
+	toURI, err := url.Parse(pub.PublicActivityPubIRI)
 	if err != nil {
-		return nil, fmt.Errorf("WrapPersonInUpdate: error parsing url %s: %s", asPublicURI, err)
+		return nil, fmt.Errorf("WrapPersonInUpdate: error parsing url %s: %s", pub.PublicActivityPubIRI, err)
 	}
 	toProp := streams.NewActivityStreamsToProperty()
 	toProp.AppendIRI(toURI)

testrig/testmodels.go

@@ -1387,7 +1387,7 @@ func NewTestFediStatuses() map[string]vocab.ActivityStreamsNote {
 			"",
 			URLMustParse("https://unknown-instance.com/users/brand_new_person"),
 			[]*url.URL{
-				URLMustParse("https://www.w3.org/ns/activitystreams#Public"),
+				URLMustParse(pub.PublicActivityPubIRI),
 			},
 			[]*url.URL{},
 			false,
@@ -1401,7 +1401,7 @@ func NewTestFediStatuses() map[string]vocab.ActivityStreamsNote {
 			"",
 			URLMustParse("https://unknown-instance.com/users/brand_new_person"),
 			[]*url.URL{
-				URLMustParse("https://www.w3.org/ns/activitystreams#Public"),
+				URLMustParse(pub.PublicActivityPubIRI),
 			},
 			[]*url.URL{},
 			false,