From 3dc7644ae6bdbfbae2e126896937e322247b5b33 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Wed, 6 Oct 2021 18:18:02 +0200 Subject: [PATCH] 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 | 66 +++++++++++++++- internal/ap/extract_test.go | 101 ++++++++++++++++++++++++ internal/ap/extractvisibility_test.go | 71 +++++++++++++++++ internal/ap/interfaces.go | 6 ++ internal/api/s2s/user/inboxpost_test.go | 5 +- internal/typeutils/astointernal.go | 94 ++-------------------- internal/typeutils/converter.go | 4 - internal/typeutils/internaltoas.go | 9 ++- internal/typeutils/wrap.go | 5 +- testrig/testmodels.go | 4 +- 10 files changed, 262 insertions(+), 103 deletions(-) create mode 100644 internal/ap/extractvisibility_test.go diff --git a/internal/ap/extract.go b/internal/ap/extract.go index 9ad148bcb..8a1d99ce9 100644 --- a/internal/ap/extract.go +++ b/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 +} diff --git a/internal/ap/extract_test.go b/internal/ap/extract_test.go index 8753e8c24..1b5c1f11f 100644 --- a/internal/ap/extract_test.go +++ b/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() } diff --git a/internal/ap/extractvisibility_test.go b/internal/ap/extractvisibility_test.go new file mode 100644 index 000000000..96e0d4c70 --- /dev/null +++ b/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 . +*/ + +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{}) +} diff --git a/internal/ap/interfaces.go b/internal/ap/interfaces.go index a20f39bd2..da7e001db 100644 --- a/internal/ap/interfaces.go +++ b/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 diff --git a/internal/api/s2s/user/inboxpost_test.go b/internal/api/s2s/user/inboxpost_test.go index 7e08ff77d..94b3e6507 100644 --- a/internal/api/s2s/user/inboxpost_test.go +++ b/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 diff --git a/internal/typeutils/astointernal.go b/internal/typeutils/astointernal.go index 9b87e03d3..88dae938b 100644 --- a/internal/typeutils/astointernal.go +++ b/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 -} diff --git a/internal/typeutils/converter.go b/internal/typeutils/converter.go index 4515fcbf0..d702c1ae7 100644 --- a/internal/typeutils/converter.go +++ b/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. // diff --git a/internal/typeutils/internaltoas.go b/internal/typeutils/internaltoas.go index 1689db496..73d274908 100644 --- a/internal/typeutils/internaltoas.go +++ b/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() diff --git a/internal/typeutils/wrap.go b/internal/typeutils/wrap.go index e06da2568..d6aa11cb4 100644 --- a/internal/typeutils/wrap.go +++ b/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) diff --git a/testrig/testmodels.go b/testrig/testmodels.go index 3f32b588d..8ed5054d1 100644 --- a/testrig/testmodels.go +++ b/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,