From 92e6810c9771fe5bbb8c5e842b537c2137c5321b Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Wed, 14 Feb 2024 09:26:40 +0100 Subject: [PATCH 01/22] Fix deprecated matchers for web/sharings --- web/sharings/replicator_test.go | 18 ++-- web/sharings/sharings.go | 5 + web/sharings/sharings_test.go | 172 ++++++++++++++++---------------- 3 files changed, 100 insertions(+), 95 deletions(-) diff --git a/web/sharings/replicator_test.go b/web/sharings/replicator_test.go index 84957716ffd..973c141fc9c 100644 --- a/web/sharings/replicator_test.go +++ b/web/sharings/replicator_test.go @@ -181,16 +181,16 @@ func TestReplicator(t *testing.T) { obj.NotContainsKey(sid2) // sid3 was updated on the source - obj.Value(sid3).Object().Value("missing").Array().Equal([]string{"5-3b"}) + obj.Value(sid3).Object().Value("missing").Array().IsEqual([]string{"5-3b"}) // sid4 is a conflict - obj.Value(sid4).Object().Value("missing").Array().Equal([]string{"2-4b", "2-4c", "4-4d"}) + obj.Value(sid4).Object().Value("missing").Array().IsEqual([]string{"2-4b", "2-4c", "4-4d"}) // sid5 has been created on the target obj.NotContainsKey(sid5) // sid6 has been created on the source - obj.Value(sid6).Object().Value("missing").Array().Equal([]string{"1-6b"}) + obj.Value(sid6).Object().Value("missing").Array().IsEqual([]string{"1-6b"}) }) t.Run("BulkDocs", func(t *testing.T) { @@ -368,13 +368,13 @@ func TestReplicator(t *testing.T) { Expect().Status(200). JSON().Object() - obj.ValueEqual("_id", xoredID) - obj.ValueEqual("_rev", folder.DocRev) - obj.ValueEqual("type", "directory") - obj.ValueEqual("name", "zorglub") + obj.HasValue("_id", xoredID) + obj.HasValue("_rev", folder.DocRev) + obj.HasValue("type", "directory") + obj.HasValue("name", "zorglub") obj.NotContainsKey("dir_id") - obj.Value("created_at").String().DateTime(time.RFC3339) - obj.Value("updated_at").String().DateTime(time.RFC3339) + obj.Value("created_at").String().AsDateTime(time.RFC3339) + obj.Value("updated_at").String().AsDateTime(time.RFC3339) }) } diff --git a/web/sharings/sharings.go b/web/sharings/sharings.go index 66c6fa4706e..d7f052f067f 100644 --- a/web/sharings/sharings.go +++ b/web/sharings/sharings.go @@ -1,3 +1,8 @@ +// Package sharings is the HTTP routes for the sharing. We have two types of +// routes, some routes are used by the clients to create, list, revoke sharings +// and add/remove recipients, and other routes are reserved for an internal +// usage, mostly to synchronize the documents between the Cozys of the members +// of the sharings. package sharings import ( diff --git a/web/sharings/sharings_test.go b/web/sharings/sharings_test.go index 41294495820..56e4761c309 100644 --- a/web/sharings/sharings_test.go +++ b/web/sharings/sharings_test.go @@ -167,7 +167,7 @@ func TestSharings(t *testing.T) { eA.GET(u.Path). WithQuery("state", state). Expect().Status(200). - ContentType("text/html", "utf-8"). + HasContentType("text/html", "utf-8"). Body(). Contains("Connect to your Cozy"). Contains(``) matches := body.Match(` Date: Wed, 14 Feb 2024 13:46:23 +0100 Subject: [PATCH 02/22] Allow to create a sharing with a group of contacts --- docs/sharing.md | 16 +++++ model/contact/contact_test.go | 83 ++++++++++++++++++++++++ model/contact/contacts.go | 16 +++++ model/contact/group.go | 74 ++++++++++++++++++++++ model/sharing/group.go | 44 +++++++++++++ model/sharing/member.go | 31 ++++++--- model/sharing/sharing.go | 1 + pkg/consts/doctype.go | 2 + pkg/couchdb/index.go | 5 +- web/sharings/sharings.go | 28 +++++++-- web/sharings/sharings_test.go | 115 +++++++++++++++++++++++++++++++--- 11 files changed, 389 insertions(+), 26 deletions(-) create mode 100644 model/contact/contact_test.go create mode 100644 model/contact/group.go create mode 100644 model/sharing/group.go diff --git a/docs/sharing.md b/docs/sharing.md index 578dabb75ee..69e90c87139 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -95,6 +95,10 @@ Content-Type: application/vnd.api+json { "id": "2a31ce0128b5f89e40fd90da3f014087", "type": "io.cozy.contacts" + }, + { + "id": "51bbc980acb0013cb5f618c04daba326", + "type": "io.cozy.contacts.groups" } ] } @@ -136,6 +140,18 @@ Content-Type: application/vnd.api+json "status": "mail-not-sent", "name": "Bob", "email": "bob@example.net" + }, + { + "status": "mail-not-sent", + "name": "Gaby", + "email": "gaby@example.net" + } + ], + "groups": [ + { + "id": "51bbc980acb0013cb5f618c04daba326", + "name": "G. people", + "members": [2] } ], "rules": [ diff --git a/model/contact/contact_test.go b/model/contact/contact_test.go new file mode 100644 index 00000000000..e30d5e87835 --- /dev/null +++ b/model/contact/contact_test.go @@ -0,0 +1,83 @@ +package contact + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/cozy/cozy-stack/pkg/config/config" + "github.com/cozy/cozy-stack/pkg/consts" + "github.com/cozy/cozy-stack/pkg/couchdb" + "github.com/cozy/cozy-stack/pkg/prefixer" + "github.com/gofrs/uuid/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFindContacts(t *testing.T) { + config.UseTestFile(t) + instPrefix := prefixer.NewPrefixer(0, "cozy-test", "cozy-test") + t.Cleanup(func() { _ = couchdb.DeleteDB(instPrefix, consts.Contacts) }) + + g := NewGroup() + g.SetID(uuid.Must(uuid.NewV7()).String()) + + gaby := fmt.Sprintf(`{ + "address": [], + "birthday": "", + "birthplace": "", + "company": "", + "cozy": [], + "cozyMetadata": { + "createdAt": "2024-02-13T15:05:58.917Z", + "createdByApp": "Contacts", + "createdByAppVersion": "1.7.0", + "doctypeVersion": 3, + "metadataVersion": 1, + "updatedAt": "2024-02-13T15:06:21.046Z", + "updatedByApps": [ + { + "date": "2024-02-13T15:06:21.046Z", + "slug": "Contacts", + "version": "1.7.0" + } + ] + }, + "displayName": "Gaby", + "email": [], + "fullname": "Gaby", + "gender": "female", + "indexes": { + "byFamilyNameGivenNameEmailCozyUrl": "gaby" + }, + "jobTitle": "", + "metadata": { + "cozy": true, + "version": 1 + }, + "name": { + "givenName": "Gaby" + }, + "note": "", + "phone": [], + "relationships": { + "groups": { + "data": [ + { + "_id": "%s", + "_type": "io.cozy.contacts.groups" + } + ] + } + } +}`, g.ID()) + + doc := couchdb.JSONDoc{Type: consts.Contacts, M: make(map[string]interface{})} + require.NoError(t, json.Unmarshal([]byte(gaby), &doc.M)) + require.NoError(t, couchdb.CreateDoc(instPrefix, &doc)) + + contacts, err := g.FindContacts(instPrefix) + require.NoError(t, err) + require.Len(t, contacts, 1) + assert.Equal(t, contacts[0].PrimaryName(), "Gaby") +} diff --git a/model/contact/contacts.go b/model/contact/contacts.go index def3c6c9c18..685f0a111b6 100644 --- a/model/contact/contacts.go +++ b/model/contact/contacts.go @@ -1,3 +1,5 @@ +// Package contact is for managing the io.cozy.contacts documents and their +// groups. package contact import ( @@ -82,6 +84,20 @@ func (c *Contact) PrimaryName() string { return primary } +// ByFamilyNameGivenNameEmailCozyURL returns a string that can be used for +// sorting the contacts like in the contacts app. +func (c *Contact) ByFamilyNameGivenNameEmailCozyURL() string { + indexes, ok := c.Get("indexes").(map[string]interface{}) + if !ok { + return c.PrimaryName() + } + str, ok := indexes["byFamilyNameGivenNameEmailCozyUrl"].(string) + if !ok { + return c.PrimaryName() + } + return str +} + // PrimaryPhoneNumber returns the preferred phone number, // or a blank string if the contact has no known phone number. func (c *Contact) PrimaryPhoneNumber() string { diff --git a/model/contact/group.go b/model/contact/group.go new file mode 100644 index 00000000000..9d734a63538 --- /dev/null +++ b/model/contact/group.go @@ -0,0 +1,74 @@ +package contact + +import ( + "sort" + + "github.com/cozy/cozy-stack/pkg/consts" + "github.com/cozy/cozy-stack/pkg/couchdb" + "github.com/cozy/cozy-stack/pkg/couchdb/mango" + "github.com/cozy/cozy-stack/pkg/prefixer" +) + +// Group is a struct for a group of contacts. +type Group struct { + couchdb.JSONDoc +} + +// NewGroup returns a new blank group. +func NewGroup() *Group { + return &Group{ + JSONDoc: couchdb.JSONDoc{ + M: make(map[string]interface{}), + }, + } +} + +// DocType returns the contact document type +func (g *Group) DocType() string { return consts.Groups } + +// Name returns the name of the group +func (g *Group) Name() string { + name, _ := g.Get("name").(string) + return name +} + +// FindGroup returns the group of contacts stored in database from a given ID +func FindGroup(db prefixer.Prefixer, groupID string) (*Group, error) { + doc := &Group{} + err := couchdb.GetDoc(db, consts.Groups, groupID, doc) + return doc, err +} + +// FindContacts returns the list of contacts inside this group. +func (g *Group) FindContacts(db prefixer.Prefixer) ([]*Contact, error) { + var docs []*Contact + req := &couchdb.FindRequest{ + UseIndex: "by-groups", + Selector: mango.Map{ + "relationships": map[string]interface{}{ + "groups": map[string]interface{}{ + "data": map[string]interface{}{ + "$elemMatch": map[string]interface{}{ + "_id": g.ID(), + "_type": consts.Groups, + }, + }, + }, + }, + }, + Limit: 1000, + } + err := couchdb.FindDocs(db, consts.Contacts, req, &docs) + if err != nil { + return nil, err + } + + // XXX I didn't find a way to make a mango request with the correct sort + less := func(i, j int) bool { + a := docs[i].ByFamilyNameGivenNameEmailCozyURL() + b := docs[j].ByFamilyNameGivenNameEmailCozyURL() + return a < b + } + sort.Slice(docs, less) + return docs, nil +} diff --git a/model/sharing/group.go b/model/sharing/group.go new file mode 100644 index 00000000000..c4e331eaa36 --- /dev/null +++ b/model/sharing/group.go @@ -0,0 +1,44 @@ +package sharing + +import ( + "github.com/cozy/cozy-stack/model/contact" + "github.com/cozy/cozy-stack/model/instance" +) + +// Group contains the information about a group of members of the sharing. +type Group struct { + ID string `json:"id,omitempty"` // Only present on the instance where the group was added + Name string `json:"name"` + Members []int `json:"members"` // The indexes of the members (0 is the owner, 1 is the first recipient, etc.) + AddedBy int `json:"addedBy"` // The index of the member who have added the group +} + +// AddGroup adds a group of contacts identified by its ID to the members of the +// sharing. +func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly bool) error { + group, err := contact.FindGroup(inst, groupID) + if err != nil { + return err + } + contacts, err := group.FindContacts(inst) + if err != nil { + return err + } + + var members []int + for _, contact := range contacts { + m, err := buildMemberFromContact(contact, readOnly) + if err != nil { + return err + } + _, idx, err := s.addMember(inst, m) + if err != nil { + return err + } + members = append(members, idx) + } + + g := Group{ID: groupID, Name: group.Name(), Members: members, AddedBy: 0} + s.Groups = append(s.Groups, g) + return nil +} diff --git a/model/sharing/member.go b/model/sharing/member.go index c8403fe7c46..0f8761e96b0 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -136,6 +136,15 @@ func (s *Sharing) AddContact(inst *instance.Instance, contactID string, readOnly if err != nil { return err } + m, err := buildMemberFromContact(c, readOnly) + if err != nil { + return err + } + _, _, err = s.addMember(inst, m) + return err +} + +func buildMemberFromContact(c *contact.Contact, readOnly bool) (Member, error) { var name, email string cozyURL := c.PrimaryCozyURL() addr, err := c.ToMailAddress() @@ -144,22 +153,23 @@ func (s *Sharing) AddContact(inst *instance.Instance, contactID string, readOnly email = addr.Email } else { if cozyURL == "" { - return err + return Member{}, err } name = c.PrimaryName() } - m := Member{ + return Member{ Status: MemberStatusMailNotSent, Name: name, Email: email, Instance: cozyURL, ReadOnly: readOnly, - } - _, err = s.addMember(inst, m) - return err + }, nil } -func (s *Sharing) addMember(inst *instance.Instance, m Member) (string, error) { +// addMember adds a member to the members of the sharing if they are not yet in +// this list, and also adds credentials for them. It returns the state +// parameter of the credentials if added, and the index of the member. +func (s *Sharing) addMember(inst *instance.Instance, m Member) (string, int, error) { idx := -1 for i, member := range s.Members { if i == 0 { @@ -175,7 +185,7 @@ func (s *Sharing) addMember(inst *instance.Instance, m Member) (string, error) { continue } if member.Status == MemberStatusReady { - return "", nil + return "", i, nil } idx = i s.Members[i].Status = m.Status @@ -186,7 +196,7 @@ func (s *Sharing) addMember(inst *instance.Instance, m Member) (string, error) { } if idx < 1 { if len(s.Members) >= maxNumberOfMembers(inst) { - return "", ErrTooManyMembers + return "", -1, ErrTooManyMembers } s.Members = append(s.Members, m) } @@ -197,10 +207,11 @@ func (s *Sharing) addMember(inst *instance.Instance, m Member) (string, error) { } if idx < 1 { s.Credentials = append(s.Credentials, creds) + idx = len(s.Credentials) } else { s.Credentials[idx-1] = creds } - return creds.State, nil + return creds.State, idx, nil } // APIDelegateAddContacts is used to serialize a request to add contacts to @@ -381,7 +392,7 @@ func (s *Sharing) AddDelegatedContact(inst *instance.Instance, email, instanceUR Instance: instanceURL, ReadOnly: readOnly, } - state, err := s.addMember(inst, m) + state, _, err := s.addMember(inst, m) if err != nil { return "", err } diff --git a/model/sharing/sharing.go b/model/sharing/sharing.go index a628691e41d..fa8ef418872 100644 --- a/model/sharing/sharing.go +++ b/model/sharing/sharing.go @@ -69,6 +69,7 @@ type Sharing struct { // Members[0] is the owner, Members[1...] are the recipients Members []Member `json:"members"` + Groups []Group `json:"groups,omitempty"` // On the owner, credentials[i] is associated to members[i+1] // On a recipient, there is only credentials[0] (for the owner) diff --git a/pkg/consts/doctype.go b/pkg/consts/doctype.go index fa235c07c8b..04ca406c9e2 100644 --- a/pkg/consts/doctype.go +++ b/pkg/consts/doctype.go @@ -70,6 +70,8 @@ const ( Permissions = "io.cozy.permissions" // Contacts doc type for sharing Contacts = "io.cozy.contacts" + // Groups of contacts doc type for sharing + Groups = "io.cozy.contacts.groups" // RemoteRequests doc type for logging requests to remote websites RemoteRequests = "io.cozy.remote.requests" // RemoteSecrets doc type for secrets used by remote doctypes diff --git a/pkg/couchdb/index.go b/pkg/couchdb/index.go index 2b044e66882..f698fc9f528 100644 --- a/pkg/couchdb/index.go +++ b/pkg/couchdb/index.go @@ -14,7 +14,7 @@ import ( // IndexViewsVersion is the version of current definition of views & indexes. // This number should be incremented when this file changes. -const IndexViewsVersion int = 36 +const IndexViewsVersion int = 37 // Indexes is the index list required by an instance to run properly. var Indexes = []*mango.Index{ @@ -66,6 +66,9 @@ var Indexes = []*mango.Index{ // Used to lookup the bitwarden ciphers mango.MakeIndex(consts.BitwardenCiphers, "by-folder-id", mango.IndexDef{Fields: []string{"folder_id"}}), mango.MakeIndex(consts.BitwardenCiphers, "by-organization-id", mango.IndexDef{Fields: []string{"organization_id"}}), + + // Used to find the contacts in a group + mango.MakeIndex(consts.Contacts, "by-groups", mango.IndexDef{Fields: []string{"relationships.groups.data"}}), } // DiskUsageView is the view used for computing the disk usage for files diff --git a/web/sharings/sharings.go b/web/sharings/sharings.go index d7f052f067f..e6cfa6a4fe5 100644 --- a/web/sharings/sharings.go +++ b/web/sharings/sharings.go @@ -54,9 +54,17 @@ func CreateSharing(c echo.Context) error { if rel, ok := obj.GetRelationship("recipients"); ok { if data, ok := rel.Data.([]interface{}); ok { for _, ref := range data { - if id, ok := ref.(map[string]interface{})["id"].(string); ok { - if err = s.AddContact(inst, id, false); err != nil { - return err + if t, _ := ref.(map[string]interface{})["type"].(string); t == consts.Groups { + if id, ok := ref.(map[string]interface{})["id"].(string); ok { + if err = s.AddGroup(inst, id, false); err != nil { + return err + } + } + } else { + if id, ok := ref.(map[string]interface{})["id"].(string); ok { + if err = s.AddContact(inst, id, false); err != nil { + return err + } } } } @@ -66,9 +74,17 @@ func CreateSharing(c echo.Context) error { if rel, ok := obj.GetRelationship("read_only_recipients"); ok { if data, ok := rel.Data.([]interface{}); ok { for _, ref := range data { - if id, ok := ref.(map[string]interface{})["id"].(string); ok { - if err = s.AddContact(inst, id, true); err != nil { - return err + if t, _ := ref.(map[string]interface{})["type"].(string); t == consts.Groups { + if id, ok := ref.(map[string]interface{})["id"].(string); ok { + if err = s.AddGroup(inst, id, true); err != nil { + return err + } + } + } else { + if id, ok := ref.(map[string]interface{})["id"].(string); ok { + if err = s.AddContact(inst, id, true); err != nil { + return err + } } } } diff --git a/web/sharings/sharings_test.go b/web/sharings/sharings_test.go index 56e4761c309..c0c40211088 100644 --- a/web/sharings/sharings_test.go +++ b/web/sharings/sharings_test.go @@ -122,10 +122,10 @@ func TestSharings(t *testing.T) { }, "relationships": { "recipients": { - "data": [{"id": "` + bobContact.ID() + `", "doctype": "` + bobContact.DocType() + `"}] + "data": [{"id": "` + bobContact.ID() + `", "type": "` + bobContact.DocType() + `"}] }, "read_only_recipients": { - "data": [{"id": "` + daveContact.ID() + `", "doctype": "` + daveContact.DocType() + `"}] + "data": [{"id": "` + daveContact.ID() + `", "type": "` + daveContact.DocType() + `"}] } } } @@ -252,7 +252,7 @@ func TestSharings(t *testing.T) { "type": "` + consts.Sharings + `", "relationships": { "recipients": { - "data": [{"id": "` + edwardContact.ID() + `", "doctype": "` + edwardContact.DocType() + `"}] + "data": [{"id": "` + edwardContact.ID() + `", "type": "` + edwardContact.DocType() + `"}] } } } @@ -286,6 +286,77 @@ func TestSharings(t *testing.T) { edward.HasValue("email", "edward@example.net") }) + t.Run("CreateSharingWithGroup", func(t *testing.T) { + eA := httpexpect.Default(t, tsA.URL) + require.NotEmpty(t, aliceAppToken) + + group := createGroup(t, aliceInstance, "friends") + require.NotNil(t, group) + fionaContact := addContactToGroup(t, aliceInstance, group, "Fiona") + require.NotNil(t, fionaContact) + gabyContact := addContactToGroup(t, aliceInstance, group, "Gaby") + require.NotNil(t, gabyContact) + + obj := eA.POST("/sharings/"). + WithHeader("Authorization", "Bearer "+aliceAppToken). + WithHeader("Content-Type", "application/vnd.api+json"). + WithBytes([]byte(`{ + "data": { + "type": "` + consts.Sharings + `", + "attributes": { + "description": "this is a test with a group", + "open_sharing": true, + "rules": [{ + "title": "test group", + "doctype": "` + iocozytests + `", + "values": ["000001"], + "add": "sync" + }] + }, + "relationships": { + "recipients": { + "data": [{"id": "` + group.ID() + `", "type": "` + consts.Groups + `"}] + } + } + } + }`)). + Expect().Status(201). + JSON(httpexpect.ContentOpts{MediaType: "application/vnd.api+json"}). + Object() + + data := obj.Value("data").Object() + attrs := data.Value("attributes").Object() + attrs.HasValue("description", "this is a test with a group") + members := attrs.Value("members").Array() + members.Length().IsEqual(3) + + owner := members.Value(0).Object() + owner.HasValue("status", "owner") + owner.HasValue("public_name", "Alice") + owner.HasValue("email", "alice@example.net") + owner.HasValue("instance", "http://"+aliceInstance.Domain) + + recipient := members.Value(1).Object() + recipient.HasValue("status", "pending") + recipient.HasValue("name", "Fiona") + recipient.HasValue("email", "fiona@example.net") + recipient.NotContainsKey("read_only") + + recipient = members.Value(2).Object() + recipient.HasValue("status", "pending") + recipient.HasValue("name", "Gaby") + recipient.HasValue("email", "gaby@example.net") + recipient.NotContainsKey("read_only") + + groups := attrs.Value("groups").Array() + groups.Length().IsEqual(1) + g := groups.Value(0).Object() + g.HasValue("id", group.ID()) + g.HasValue("name", "friends") + g.HasValue("members", []int{1, 2}) + g.HasValue("addedBy", 0) + }) + t.Run("CreateSharingWithPreview", func(t *testing.T) { bobContact := createBobContact(t, aliceInstance) require.NotEmpty(t, aliceAppToken) @@ -310,10 +381,10 @@ func TestSharings(t *testing.T) { }, "relationships": { "recipients": { - "data": [{"id": "` + bobContact.ID() + `", "doctype": "` + bobContact.DocType() + `"}] + "data": [{"id": "` + bobContact.ID() + `", "type": "` + bobContact.DocType() + `"}] }, "read_only_recipients": { - "data": [{"id": "` + daveContact.ID() + `", "doctype": "` + daveContact.DocType() + `"}] + "data": [{"id": "` + daveContact.ID() + `", "type": "` + daveContact.DocType() + `"}] } } } @@ -392,7 +463,7 @@ func TestSharings(t *testing.T) { "type": "` + consts.Sharings + `", "relationships": { "recipients": { - "data": [{"id": "` + charlieContact.ID() + `", "doctype": "` + charlieContact.DocType() + `"}] + "data": [{"id": "` + charlieContact.ID() + `", "type": "` + charlieContact.DocType() + `"}] } } } @@ -530,7 +601,7 @@ func TestSharings(t *testing.T) { }, "relationships": { "recipients": { - "data": [{"id": "` + bobContact.ID() + `", "doctype": "` + bobContact.DocType() + `"}] + "data": [{"id": "` + bobContact.ID() + `", "type": "` + bobContact.DocType() + `"}] } } } @@ -1036,14 +1107,40 @@ func createBobContact(t *testing.T, instance *instance.Instance) *contact.Contac func createContact(t *testing.T, inst *instance.Instance, name, email string) *contact.Contact { t.Helper() - mail := map[string]interface{}{"address": email} c := contact.New() c.M["fullname"] = name c.M["email"] = []interface{}{mail} - require.NoError(t, couchdb.CreateDoc(inst, c)) + return c +} + +func createGroup(t *testing.T, inst *instance.Instance, name string) *contact.Group { + t.Helper() + g := contact.NewGroup() + g.M["name"] = name + require.NoError(t, couchdb.CreateDoc(inst, g)) + return g +} +func addContactToGroup(t *testing.T, inst *instance.Instance, g *contact.Group, contactName string) *contact.Contact { + t.Helper() + email := strings.ToLower(contactName) + "@example.net" + mail := map[string]interface{}{"address": email} + c := contact.New() + c.M["fullname"] = contactName + c.M["email"] = []interface{}{mail} + c.M["relationships"] = map[string]interface{}{ + "groups": map[string]interface{}{ + "data": []interface{}{ + map[string]interface{}{ + "_id": g.ID(), + "_type": consts.Groups, + }, + }, + }, + } + require.NoError(t, couchdb.CreateDoc(inst, c)) return c } From b97bc86c3e05ab2d79e2e4c5f435af9902c14df9 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Wed, 14 Feb 2024 15:30:04 +0100 Subject: [PATCH 03/22] Allow to add a group of contacts to a sharing --- docs/sharing.md | 6 +++--- model/sharing/member.go | 13 +++++++++---- model/sharing/sharing.go | 2 +- web/sharings/sharings.go | 25 +++++++++++++++++------- web/sharings/sharings_test.go | 36 +++++++++++++++++++++++++++++++---- 5 files changed, 63 insertions(+), 19 deletions(-) diff --git a/docs/sharing.md b/docs/sharing.md index 69e90c87139..e43fb8bf60d 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -763,9 +763,9 @@ HTTP/1.1 204 No Content ### POST /sharings/:sharing-id/recipients -This route allows the sharer to add new recipients to a sharing. It can also be -used by a recipient when the sharing has `open_sharing` set to true if the -recipient doesn't have the `read_only` flag +This route allows the sharer to add new recipients (and groups of recipients) +to a sharing. It can also be used by a recipient when the sharing has +`open_sharing` set to true if the recipient doesn't have the `read_only` flag. #### Request diff --git a/model/sharing/member.go b/model/sharing/member.go index 0f8761e96b0..9cbcb2cb8d5 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -107,10 +107,15 @@ type Credentials struct { InboundClientID string `json:"inbound_client_id,omitempty"` } -// AddContacts adds a list of contacts on the sharer cozy -func (s *Sharing) AddContacts(inst *instance.Instance, contactIDs map[string]bool) error { - for id, ro := range contactIDs { - if err := s.AddContact(inst, id, ro); err != nil { +// AddGroupsAndContacts adds a list of contacts on the sharer cozy +func (s *Sharing) AddGroupsAndContacts(inst *instance.Instance, groupIDs, contactIDs []string, readOnly bool) error { + for _, id := range contactIDs { + if err := s.AddContact(inst, id, readOnly); err != nil { + return err + } + } + for _, id := range groupIDs { + if err := s.AddGroup(inst, id, readOnly); err != nil { return err } } diff --git a/model/sharing/sharing.go b/model/sharing/sharing.go index fa8ef418872..3590e0398c4 100644 --- a/model/sharing/sharing.go +++ b/model/sharing/sharing.go @@ -485,7 +485,7 @@ func (s *Sharing) RevokePreviewPermissions(inst *instance.Instance) error { // RevokeRecipient revoke only one recipient on the sharer. After that, if the // sharing has still at least one active member, we keep it as is. Else, we -// desactive the sharing. +// desactivate the sharing. func (s *Sharing) RevokeRecipient(inst *instance.Instance, index int) error { if !s.Owner { return ErrInvalidSharing diff --git a/web/sharings/sharings.go b/web/sharings/sharings.go index e6cfa6a4fe5..6601c978c18 100644 --- a/web/sharings/sharings.go +++ b/web/sharings/sharings.go @@ -307,15 +307,26 @@ func ChangeCozyAddress(c echo.Context) error { func addRecipientsToSharing(inst *instance.Instance, s *sharing.Sharing, rel *jsonapi.Relationship, readOnly bool) error { var err error if data, ok := rel.Data.([]interface{}); ok { - ids := make(map[string]bool) - for _, ref := range data { - if id, ok := ref.(map[string]interface{})["id"].(string); ok { - ids[id] = readOnly - } - } if s.Owner { - err = s.AddContacts(inst, ids) + var contactIDs, groupIDs []string + for _, ref := range data { + if id, ok := ref.(map[string]interface{})["id"].(string); ok { + if t, _ := ref.(map[string]interface{})["type"].(string); t == consts.Groups { + groupIDs = append(groupIDs, id) + } else { + contactIDs = append(contactIDs, id) + } + } + } + err = s.AddGroupsAndContacts(inst, groupIDs, contactIDs, readOnly) } else { + // TODO groups + ids := make(map[string]bool) + for _, ref := range data { + if id, ok := ref.(map[string]interface{})["id"].(string); ok { + ids[id] = readOnly + } + } err = s.DelegateAddContacts(inst, ids) } } diff --git a/web/sharings/sharings_test.go b/web/sharings/sharings_test.go index c0c40211088..58108ff8b6f 100644 --- a/web/sharings/sharings_test.go +++ b/web/sharings/sharings_test.go @@ -450,11 +450,18 @@ func TestSharings(t *testing.T) { }) t.Run("AddRecipient", func(t *testing.T) { - assert.NotEmpty(t, aliceAppToken) - assert.NotNil(t, charlieContact) + require.NotEmpty(t, aliceAppToken) + require.NotNil(t, charlieContact) eA := httpexpect.Default(t, tsA.URL) + brothers := createGroup(t, aliceInstance, "brothers") + require.NotNil(t, brothers) + hugoContact := addContactToGroup(t, aliceInstance, brothers, "Hugo") + require.NotNil(t, hugoContact) + idrisContact := addContactToGroup(t, aliceInstance, brothers, "Idris") + require.NotNil(t, idrisContact) + obj := eA.POST("/sharings/"+sharingID+"/recipients"). WithHeader("Authorization", "Bearer "+aliceAppToken). WithHeader("Content-Type", "application/vnd.api+json"). @@ -463,7 +470,10 @@ func TestSharings(t *testing.T) { "type": "` + consts.Sharings + `", "relationships": { "recipients": { - "data": [{"id": "` + charlieContact.ID() + `", "type": "` + charlieContact.DocType() + `"}] + "data": [ + {"id": "` + charlieContact.ID() + `", "type": "` + consts.Contacts + `"}, + {"id": "` + brothers.ID() + `", "type": "` + consts.Groups + `"} + ] } } } @@ -476,7 +486,7 @@ func TestSharings(t *testing.T) { attrs := data.Value("attributes").Object() members := attrs.Value("members").Array() - members.Length().IsEqual(4) + members.Length().IsEqual(6) owner := members.Value(0).Object() owner.HasValue("status", "owner") owner.HasValue("public_name", "Alice") @@ -498,6 +508,24 @@ func TestSharings(t *testing.T) { charlie.HasValue("status", "pending") charlie.HasValue("name", "Charlie") charlie.HasValue("email", "charlie@example.net") + + hugo := members.Value(4).Object() + hugo.HasValue("status", "pending") + hugo.HasValue("name", "Hugo") + hugo.HasValue("email", "hugo@example.net") + + idris := members.Value(5).Object() + idris.HasValue("status", "pending") + idris.HasValue("name", "Idris") + idris.HasValue("email", "idris@example.net") + + groups := attrs.Value("groups").Array() + groups.Length().IsEqual(1) + g := groups.Value(0).Object() + g.HasValue("id", brothers.ID()) + g.HasValue("name", "brothers") + g.HasValue("members", []int{4, 5}) + g.HasValue("addedBy", 0) }) t.Run("RevokedSharingWithPreview", func(t *testing.T) { From 236fcd78e7ca49647015a72ec568a7ab6cac1fba Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Wed, 14 Feb 2024 17:52:47 +0100 Subject: [PATCH 04/22] Refactor to invert the groups-members relation in sharings --- model/sharing/group.go | 7 +++---- model/sharing/member.go | 13 +++++++++++-- model/sharing/oauth.go | 2 +- web/sharings/sharings_test.go | 6 ++++-- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/model/sharing/group.go b/model/sharing/group.go index c4e331eaa36..cdfa941f564 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -9,7 +9,6 @@ import ( type Group struct { ID string `json:"id,omitempty"` // Only present on the instance where the group was added Name string `json:"name"` - Members []int `json:"members"` // The indexes of the members (0 is the owner, 1 is the first recipient, etc.) AddedBy int `json:"addedBy"` // The index of the member who have added the group } @@ -25,7 +24,7 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo return err } - var members []int + groupIndex := len(s.Groups) for _, contact := range contacts { m, err := buildMemberFromContact(contact, readOnly) if err != nil { @@ -35,10 +34,10 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo if err != nil { return err } - members = append(members, idx) + s.Members[idx].Groups = append(s.Members[idx].Groups, groupIndex) } - g := Group{ID: groupID, Name: group.Name(), Members: members, AddedBy: 0} + g := Group{ID: groupID, Name: group.Name(), AddedBy: 0} s.Groups = append(s.Groups, g) return nil } diff --git a/model/sharing/member.go b/model/sharing/member.go index 9cbcb2cb8d5..7004b87e9e9 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -66,6 +66,7 @@ type Member struct { Email string `json:"email,omitempty"` Instance string `json:"instance,omitempty"` ReadOnly bool `json:"read_only,omitempty"` + Groups []int `json:"groups,omitempty"` // The indexes of the groups } // PrimaryName returns the main name of this member @@ -89,6 +90,14 @@ func (m *Member) InstanceHost() string { return u.Host } +// Same returns true if the two members are the same. +func (m *Member) Same(other Member) bool { + return m.Name == other.Name && + m.PublicName == other.PublicName && + m.Email == other.Email && + m.Instance == other.Instance +} + // Credentials is the struct with the secret stuff used for authentication & // authorization. type Credentials struct { @@ -587,14 +596,14 @@ func (s *Sharing) FindMemberByInboundClientID(clientID string) (*Member, error) func (s *Sharing) FindCredentials(m *Member) *Credentials { if s.Owner { for i, member := range s.Members { - if i > 0 && *m == member { + if i > 0 && m.Same(member) { return &s.Credentials[i-1] } } return nil } - if *m == s.Members[0] { + if m.Same(s.Members[0]) { return &s.Credentials[0] } return nil diff --git a/model/sharing/oauth.go b/model/sharing/oauth.go index ee4541a6342..1544b32b022 100644 --- a/model/sharing/oauth.go +++ b/model/sharing/oauth.go @@ -544,7 +544,7 @@ func (s *Sharing) ChangeMemberAddress(inst *instance.Instance, m *Member, params if i == 0 { continue } - if s.Members[i] == *m { + if m.Same(s.Members[i]) { s.Credentials[i-1].AccessToken.AccessToken = params.AccessToken s.Credentials[i-1].AccessToken.RefreshToken = params.RefreshToken } diff --git a/web/sharings/sharings_test.go b/web/sharings/sharings_test.go index 58108ff8b6f..e5eb00ffe11 100644 --- a/web/sharings/sharings_test.go +++ b/web/sharings/sharings_test.go @@ -340,12 +340,14 @@ func TestSharings(t *testing.T) { recipient.HasValue("status", "pending") recipient.HasValue("name", "Fiona") recipient.HasValue("email", "fiona@example.net") + recipient.HasValue("groups", []int{0}) recipient.NotContainsKey("read_only") recipient = members.Value(2).Object() recipient.HasValue("status", "pending") recipient.HasValue("name", "Gaby") recipient.HasValue("email", "gaby@example.net") + recipient.HasValue("groups", []int{0}) recipient.NotContainsKey("read_only") groups := attrs.Value("groups").Array() @@ -353,7 +355,6 @@ func TestSharings(t *testing.T) { g := groups.Value(0).Object() g.HasValue("id", group.ID()) g.HasValue("name", "friends") - g.HasValue("members", []int{1, 2}) g.HasValue("addedBy", 0) }) @@ -513,18 +514,19 @@ func TestSharings(t *testing.T) { hugo.HasValue("status", "pending") hugo.HasValue("name", "Hugo") hugo.HasValue("email", "hugo@example.net") + hugo.HasValue("groups", []int{0}) idris := members.Value(5).Object() idris.HasValue("status", "pending") idris.HasValue("name", "Idris") idris.HasValue("email", "idris@example.net") + idris.HasValue("groups", []int{0}) groups := attrs.Value("groups").Array() groups.Length().IsEqual(1) g := groups.Value(0).Object() g.HasValue("id", brothers.ID()) g.HasValue("name", "brothers") - g.HasValue("members", []int{4, 5}) g.HasValue("addedBy", 0) }) From 67b1378f73ec490d504df393cddf08aa15c27437 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Wed, 14 Feb 2024 17:59:44 +0100 Subject: [PATCH 05/22] Add a property only_in_groups for sharing members --- docs/sharing.md | 6 ++++-- model/sharing/group.go | 1 + model/sharing/member.go | 15 ++++++++------- web/sharings/sharings_test.go | 7 +++++++ 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/docs/sharing.md b/docs/sharing.md index e43fb8bf60d..769c59ecce6 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -144,14 +144,16 @@ Content-Type: application/vnd.api+json { "status": "mail-not-sent", "name": "Gaby", - "email": "gaby@example.net" + "email": "gaby@example.net", + "only_in_groups": true, + "groups": [0] } ], "groups": [ { "id": "51bbc980acb0013cb5f618c04daba326", "name": "G. people", - "members": [2] + "addedBy": 0 } ], "rules": [ diff --git a/model/sharing/group.go b/model/sharing/group.go index cdfa941f564..37168851996 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -30,6 +30,7 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo if err != nil { return err } + m.OnlyInGroups = true _, idx, err := s.addMember(inst, m) if err != nil { return err diff --git a/model/sharing/member.go b/model/sharing/member.go index 7004b87e9e9..91c9fba356d 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -60,13 +60,14 @@ func maxNumberOfMembers(inst *instance.Instance) int { // Member contains the information about a recipient (or the sharer) for a sharing type Member struct { - Status string `json:"status"` - Name string `json:"name,omitempty"` - PublicName string `json:"public_name,omitempty"` - Email string `json:"email,omitempty"` - Instance string `json:"instance,omitempty"` - ReadOnly bool `json:"read_only,omitempty"` - Groups []int `json:"groups,omitempty"` // The indexes of the groups + Status string `json:"status"` + Name string `json:"name,omitempty"` + PublicName string `json:"public_name,omitempty"` + Email string `json:"email,omitempty"` + Instance string `json:"instance,omitempty"` + ReadOnly bool `json:"read_only,omitempty"` + OnlyInGroups bool `json:"only_in_groups,omitempty"` // False if the member as been added as an io.cozy.contacts + Groups []int `json:"groups,omitempty"` // The indexes of the groups } // PrimaryName returns the main name of this member diff --git a/web/sharings/sharings_test.go b/web/sharings/sharings_test.go index e5eb00ffe11..b57ae891fc1 100644 --- a/web/sharings/sharings_test.go +++ b/web/sharings/sharings_test.go @@ -342,6 +342,7 @@ func TestSharings(t *testing.T) { recipient.HasValue("email", "fiona@example.net") recipient.HasValue("groups", []int{0}) recipient.NotContainsKey("read_only") + recipient.HasValue("only_in_groups", true) recipient = members.Value(2).Object() recipient.HasValue("status", "pending") @@ -349,6 +350,7 @@ func TestSharings(t *testing.T) { recipient.HasValue("email", "gaby@example.net") recipient.HasValue("groups", []int{0}) recipient.NotContainsKey("read_only") + recipient.HasValue("only_in_groups", true) groups := attrs.Value("groups").Array() groups.Length().IsEqual(1) @@ -498,29 +500,34 @@ func TestSharings(t *testing.T) { bob.HasValue("status", "pending") bob.HasValue("name", "Bob") bob.HasValue("email", "bob@example.net") + bob.NotContainsKey("only_in_groups") dave := members.Value(2).Object() dave.HasValue("status", "pending") dave.HasValue("name", "Dave") dave.HasValue("email", "dave@example.net") dave.HasValue("read_only", true) + dave.NotContainsKey("only_in_groups") charlie := members.Value(3).Object() charlie.HasValue("status", "pending") charlie.HasValue("name", "Charlie") charlie.HasValue("email", "charlie@example.net") + charlie.NotContainsKey("only_in_groups") hugo := members.Value(4).Object() hugo.HasValue("status", "pending") hugo.HasValue("name", "Hugo") hugo.HasValue("email", "hugo@example.net") hugo.HasValue("groups", []int{0}) + hugo.HasValue("only_in_groups", true) idris := members.Value(5).Object() idris.HasValue("status", "pending") idris.HasValue("name", "Idris") idris.HasValue("email", "idris@example.net") idris.HasValue("groups", []int{0}) + idris.HasValue("only_in_groups", true) groups := attrs.Value("groups").Array() groups.Length().IsEqual(1) From b758dd4d620830bad6229626dfee12d6653fb1f3 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Wed, 14 Feb 2024 18:40:12 +0100 Subject: [PATCH 06/22] Allow to revoke a group from a sharing --- docs/sharing.md | 21 ++++++++++++++++++++ model/sharing/group.go | 42 ++++++++++++++++++++++++++++++++++++++++ web/sharings/revoke.go | 26 +++++++++++++++++++++++++ web/sharings/sharings.go | 1 + 4 files changed, 90 insertions(+) diff --git a/docs/sharing.md b/docs/sharing.md index 769c59ecce6..0a881b4c9c8 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -1155,6 +1155,27 @@ Host: alice.example.net HTTP/1.1 204 No Content ``` +### DELETE /sharings/:sharing-id/groups/:index + +This route can be only be called on the cozy instance of the sharer to revoke a +group of the sharing. The parameter is the index of this recipient in the +`groups` array of the sharing. The `removed` property for this group will be +set to `true`, and it will revoke the members of this group unless they are +still part of the sharing via another group. + +#### Request + +```http +DELETE /sharings/ce8835a061d0ef68947afe69a0046722/groups/0 HTTP/1.1 +Host: alice.example.net +``` + +#### Response + +```http +HTTP/1.1 204 No Content +``` + ### DELETE /sharings/:sharing-id/recipients/self This route can be used by an application in the cozy of a recipient to remove it diff --git a/model/sharing/group.go b/model/sharing/group.go index 37168851996..b867717fc43 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -3,6 +3,7 @@ package sharing import ( "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/model/instance" + "github.com/cozy/cozy-stack/pkg/couchdb" ) // Group contains the information about a group of members of the sharing. @@ -10,6 +11,7 @@ type Group struct { ID string `json:"id,omitempty"` // Only present on the instance where the group was added Name string `json:"name"` AddedBy int `json:"addedBy"` // The index of the member who have added the group + Removed bool `json:"removed,omitempty"` } // AddGroup adds a group of contacts identified by its ID to the members of the @@ -42,3 +44,43 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo s.Groups = append(s.Groups, g) return nil } + +// RevokeGroup revoke a group of members on the sharer Cozy. After that, the +// sharing is desactivated if there are no longer any active recipient. +func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { + if !s.Owner { + return ErrInvalidSharing + } + + for i, m := range s.Members { + if !m.OnlyInGroups { + continue + } + inGroup := false + for _, idx := range m.Groups { + if idx == index { + inGroup = true + } + } + if !inGroup { + continue + } + if len(m.Groups) == 1 { + s.Members[i].Groups = nil + if err := s.RevokeRecipient(inst, i); err != nil { + return err + } + } else { + var groups []int + for _, idx := range m.Groups { + if idx != index { + groups = append(groups, idx) + } + } + s.Members[i].Groups = groups + } + } + + s.Groups[index].Removed = true + return couchdb.UpdateDoc(inst, s) +} diff --git a/web/sharings/revoke.go b/web/sharings/revoke.go index c8b6310f827..e6d8917b3a9 100644 --- a/web/sharings/revoke.go +++ b/web/sharings/revoke.go @@ -55,6 +55,32 @@ func RevokeRecipient(c echo.Context) error { return c.NoContent(http.StatusNoContent) } +// RevokeGroup is used by the owner to revoke a group +func RevokeGroup(c echo.Context) error { + inst := middlewares.GetInstance(c) + sharingID := c.Param("sharing-id") + s, err := sharing.FindSharing(inst, sharingID) + if err != nil { + return wrapErrors(err) + } + _, err = checkCreatePermissions(c, s) + if err != nil { + return echo.NewHTTPError(http.StatusForbidden) + } + index, err := strconv.Atoi(c.Param("index")) + if err != nil { + return jsonapi.InvalidParameter("index", err) + } + if index >= len(s.Groups) { + return jsonapi.InvalidParameter("index", errors.New("Invalid index")) + } + if err = s.RevokeGroup(inst, index); err != nil { + return wrapErrors(err) + } + go s.NotifyRecipients(inst, nil) + return c.NoContent(http.StatusNoContent) +} + // RevocationRecipientNotif is used to inform a recipient that the sharing is revoked func RevocationRecipientNotif(c echo.Context) error { inst := middlewares.GetInstance(c) diff --git a/web/sharings/sharings.go b/web/sharings/sharings.go index 6601c978c18..5083629b2fb 100644 --- a/web/sharings/sharings.go +++ b/web/sharings/sharings.go @@ -759,6 +759,7 @@ func Routes(router *echo.Group) { router.PUT("/:sharing-id/recipients", PutRecipients) router.DELETE("/:sharing-id/recipients", RevokeSharing) // On the sharer router.DELETE("/:sharing-id/recipients/:index", RevokeRecipient) // On the sharer + router.DELETE("/:sharing-id/groups/:index", RevokeGroup) // On the sharer router.POST("/:sharing-id/recipients/self/moved", ChangeCozyAddress) router.POST("/:sharing-id/recipients/:index/readonly", AddReadOnly) // On the sharer router.POST("/:sharing-id/recipients/self/readonly", DowngradeToReadOnly, checkSharingWritePermissions) // On the recipient From 6cca2c73fc3990a043f3d689cd4897d1f0041e95 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Thu, 15 Feb 2024 14:53:38 +0100 Subject: [PATCH 07/22] Update sharing design doc --- docs/sharing-design.md | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/docs/sharing-design.md b/docs/sharing-design.md index 1d6327cc9e6..bd6bcd10c67 100644 --- a/docs/sharing-design.md +++ b/docs/sharing-design.md @@ -473,20 +473,34 @@ care of it later. ### Description of a sharing - An identifier (the same for all members of the sharing) -- A list of `members`. The first one is the owner. For each member, we have - the URL of the cozy, a contact name, a public name, an email, a status, a - read-only flag, and some credentials to authorize the transfer of data - between the owner and the recipients. The status can be: - - `owner` for the member that has created the sharing - - `mail-not-sent` for a member that has been added, but its invitation - has not yet been sent (often, this status is used only for a few - seconds) - - `pending` for a member with an invitation sent, but who has not clicked - on the link - - `seen` for a member that has clicked on the invitation link, but has not - setup the Cozy to Cozy replication for the sharing - - `ready` for a member where the Cozy to Cozy replication has been set up - - `revoked` for a member who is on longer in the sharing +- A list of `members`. The first one is the owner. For each member, we have: + - `status`, a status that can be: + - `owner` for the member that has created the sharing + - `mail-not-sent` for a member that has been added, but its + invitation has not yet been sent (often, this status is used only + for a few seconds) + - `pending` for a member with an invitation sent, but who has not + clicked on the link + - `seen` for a member that has clicked on the invitation link, but + has not setup the Cozy to Cozy replication for the sharing + - `ready` for a member where the Cozy to Cozy replication has been + set up + - `revoked` for a member who is on longer in the sharing + - `name`, a contact name + - `public_name`, a public name + - `email`, the email address + - `instance`, the URL of the Cozy + - `read_only`, a flag to tell if the contact is restricted to read-only mode + - `only_in_groups`, a flag that will be false if the member has been added + as a single contact + - `groups`, a list of indexes of the groups +- A list of `groups`, with for each one: + - `id`, the identifier of the io.cozy.contacts.groups + - `name`, the name of the group + - `addedBy`, the index of the member that has added the group + - `removed`, a flag set to true when the group is revoked from the sharing +- Some `credentials` to authorize the transfer of data between the owner and + the recipients - A `description` (one sentence that will help people understand what is shared and why) - A flag `active` that says if the sharing is currently active for at least From c53cb41027a038672d4a60516ecea33063c66e5d Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Thu, 15 Feb 2024 16:56:22 +0100 Subject: [PATCH 08/22] Add tests for RevokeGroup --- model/sharing/group.go | 11 ++- model/sharing/group_test.go | 128 ++++++++++++++++++++++++++++++++++ web/sharings/sharings_test.go | 89 +++++++++++++++++++++++ 3 files changed, 222 insertions(+), 6 deletions(-) create mode 100644 model/sharing/group_test.go diff --git a/model/sharing/group.go b/model/sharing/group.go index b867717fc43..322e1eb8a8d 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -53,9 +53,6 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { } for i, m := range s.Members { - if !m.OnlyInGroups { - continue - } inGroup := false for _, idx := range m.Groups { if idx == index { @@ -67,9 +64,6 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { } if len(m.Groups) == 1 { s.Members[i].Groups = nil - if err := s.RevokeRecipient(inst, i); err != nil { - return err - } } else { var groups []int for _, idx := range m.Groups { @@ -79,6 +73,11 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { } s.Members[i].Groups = groups } + if m.OnlyInGroups && len(s.Members[i].Groups) == 0 { + if err := s.RevokeRecipient(inst, i); err != nil { + return err + } + } } s.Groups[index].Removed = true diff --git a/model/sharing/group_test.go b/model/sharing/group_test.go new file mode 100644 index 00000000000..de30e2bf411 --- /dev/null +++ b/model/sharing/group_test.go @@ -0,0 +1,128 @@ +package sharing + +import ( + "strings" + "testing" + "time" + + "github.com/cozy/cozy-stack/model/contact" + "github.com/cozy/cozy-stack/model/instance" + "github.com/cozy/cozy-stack/pkg/config/config" + "github.com/cozy/cozy-stack/pkg/consts" + "github.com/cozy/cozy-stack/pkg/couchdb" + "github.com/cozy/cozy-stack/tests/testutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGroups(t *testing.T) { + if testing.Short() { + t.Skip("an instance is required for this test: test skipped due to the use of --short flag") + } + + config.UseTestFile(t) + testutils.NeedCouchdb(t) + setup := testutils.NewSetup(t, t.Name()) + inst := setup.GetTestInstance() + + t.Run("RevokeGroup", func(t *testing.T) { + now := time.Now() + friends := createGroup(t, inst, "Friends") + football := createGroup(t, inst, "Football") + bob := createContactInGroups(t, inst, "Bob", []string{friends.ID()}) + _ = createContactInGroups(t, inst, "Charlie", []string{friends.ID(), football.ID()}) + _ = createContactInGroups(t, inst, "Dave", []string{football.ID()}) + + s := &Sharing{ + Active: true, + Owner: true, + Description: "Just testing groups", + Members: []Member{ + {Status: MemberStatusOwner, Name: "Alice", Email: "alice@cozy.tools"}, + }, + CreatedAt: now, + UpdatedAt: now, + } + require.NoError(t, couchdb.CreateDoc(inst, s)) + require.NoError(t, s.AddContact(inst, bob.ID(), false)) + require.NoError(t, s.AddGroup(inst, friends.ID(), false)) + require.NoError(t, s.AddGroup(inst, football.ID(), false)) + + require.Len(t, s.Members, 4) + require.Equal(t, s.Members[0].Name, "Alice") + require.Equal(t, s.Members[1].Name, "Bob") + assert.False(t, s.Members[1].OnlyInGroups) + assert.Equal(t, s.Members[1].Groups, []int{0}) + require.Equal(t, s.Members[2].Name, "Charlie") + assert.True(t, s.Members[2].OnlyInGroups) + assert.Equal(t, s.Members[2].Groups, []int{0, 1}) + require.Equal(t, s.Members[3].Name, "Dave") + assert.True(t, s.Members[3].OnlyInGroups) + assert.Equal(t, s.Members[3].Groups, []int{1}) + + require.Len(t, s.Groups, 2) + require.Equal(t, s.Groups[0].Name, "Friends") + assert.False(t, s.Groups[0].Removed) + require.Equal(t, s.Groups[1].Name, "Football") + assert.False(t, s.Groups[1].Removed) + + require.NoError(t, s.RevokeGroup(inst, 1)) // Revoke the football group + + require.Len(t, s.Members, 4) + assert.NotEqual(t, s.Members[1].Status, MemberStatusRevoked) + assert.Equal(t, s.Members[1].Groups, []int{0}) + assert.NotEqual(t, s.Members[2].Status, MemberStatusRevoked) + assert.Equal(t, s.Members[2].Groups, []int{0}) + assert.Equal(t, s.Members[3].Status, MemberStatusRevoked) + assert.Empty(t, s.Members[3].Groups) + + require.Len(t, s.Groups, 2) + assert.False(t, s.Groups[0].Removed) + assert.True(t, s.Groups[1].Removed) + + require.NoError(t, s.RevokeGroup(inst, 0)) // Revoke the fiends group + + require.Len(t, s.Members, 4) + assert.NotEqual(t, s.Members[1].Status, MemberStatusRevoked) + assert.Empty(t, s.Members[1].Groups) + assert.Equal(t, s.Members[2].Status, MemberStatusRevoked) + assert.Empty(t, s.Members[2].Groups) + assert.Equal(t, s.Members[3].Status, MemberStatusRevoked) + assert.Empty(t, s.Members[3].Groups) + + require.Len(t, s.Groups, 2) + assert.True(t, s.Groups[0].Removed) + assert.True(t, s.Groups[1].Removed) + }) +} + +func createGroup(t *testing.T, inst *instance.Instance, name string) *contact.Group { + t.Helper() + g := contact.NewGroup() + g.M["name"] = name + require.NoError(t, couchdb.CreateDoc(inst, g)) + return g +} + +func createContactInGroups(t *testing.T, inst *instance.Instance, contactName string, groupIDs []string) *contact.Contact { + t.Helper() + email := strings.ToLower(contactName) + "@cozy.tools" + mail := map[string]interface{}{"address": email} + + var groups []interface{} + for _, id := range groupIDs { + groups = append(groups, map[string]interface{}{ + "_id": id, + "_type": consts.Groups, + }) + } + + c := contact.New() + c.M["fullname"] = contactName + c.M["email"] = []interface{}{mail} + c.M["relationships"] = map[string]interface{}{ + "groups": map[string]interface{}{"data": groups}, + } + require.NoError(t, couchdb.CreateDoc(inst, c)) + return c +} diff --git a/web/sharings/sharings_test.go b/web/sharings/sharings_test.go index b57ae891fc1..797e176e4e0 100644 --- a/web/sharings/sharings_test.go +++ b/web/sharings/sharings_test.go @@ -853,6 +853,95 @@ func TestSharings(t *testing.T) { assertLastRecipientIsRevoked(t, s, sharedRefs, aliceInstance) }) + t.Run("RevokeGroup", func(t *testing.T) { + sharedDocs := []string{"forgroup1"} + s := createSharing(t, aliceInstance, sharedDocs, tsB.URL) + + group := createGroup(t, aliceInstance, "friends") + require.NotNil(t, group) + fionaContact := addContactToGroup(t, aliceInstance, group, "Fiona") + require.NotNil(t, fionaContact) + gabyContact := addContactToGroup(t, aliceInstance, group, "Gaby") + require.NotNil(t, gabyContact) + + eA := httpexpect.Default(t, tsA.URL) + + obj := eA.POST("/sharings/"+s.ID()+"/recipients"). + WithHeader("Authorization", "Bearer "+aliceAppToken). + WithHeader("Content-Type", "application/vnd.api+json"). + WithBytes([]byte(`{ + "data": { + "type": "` + consts.Sharings + `", + "relationships": { + "recipients": { + "data": [ + {"id": "` + group.ID() + `", "type": "` + consts.Groups + `"} + ] + } + } + } + }`)). + Expect().Status(200). + JSON(httpexpect.ContentOpts{MediaType: "application/vnd.api+json"}). + Object() + + data := obj.Value("data").Object() + attrs := data.Value("attributes").Object() + members := attrs.Value("members").Array() + members.Length().IsEqual(4) + + owner := members.Value(0).Object() + owner.HasValue("status", "owner") + owner.HasValue("public_name", "Alice") + + bob := members.Value(1).Object() + bob.HasValue("name", "Bob") + + fiona := members.Value(2).Object() + fiona.HasValue("status", "pending") + fiona.HasValue("name", "Fiona") + fiona.HasValue("groups", []int{0}) + fiona.HasValue("only_in_groups", true) + + gaby := members.Value(3).Object() + gaby.HasValue("status", "pending") + gaby.HasValue("name", "Gaby") + gaby.HasValue("groups", []int{0}) + gaby.HasValue("only_in_groups", true) + + eA.DELETE("/sharings/"+s.ID()+"/groups/0"). + WithHeader("Authorization", "Bearer "+aliceAppToken). + WithHeader("Content-Type", "application/vnd.api+json"). + Expect().Status(204) + + obj = eA.GET("/sharings/"+s.ID()). + WithHeader("Authorization", "Bearer "+aliceAppToken). + WithHeader("Content-Type", "application/vnd.api+json"). + Expect().Status(200). + JSON(httpexpect.ContentOpts{MediaType: "application/vnd.api+json"}). + Object() + + data = obj.Value("data").Object() + attrs = data.Value("attributes").Object() + members = attrs.Value("members").Array() + members.Length().IsEqual(4) + + owner = members.Value(0).Object() + owner.HasValue("status", "owner") + owner.HasValue("public_name", "Alice") + + bob = members.Value(1).Object() + bob.HasValue("name", "Bob") + + fiona = members.Value(2).Object() + fiona.HasValue("status", "revoked") + fiona.HasValue("name", "Fiona") + + gaby = members.Value(3).Object() + gaby.HasValue("status", "revoked") + gaby.HasValue("name", "Gaby") + }) + t.Run("RevocationFromRecipient", func(t *testing.T) { sharedDocs := []string{"mygreatid5", "mygreatid6"} sharedRefs := []*sharing.SharedRef{} From 4ed53349c80c29625ba71e0590ae15a11efbd73f Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Mon, 19 Feb 2024 12:05:05 +0100 Subject: [PATCH 09/22] Add a share-group worker --- cozy.example.yaml | 3 +- docs/workers.md | 15 +++-- model/sharing/group.go | 124 ++++++++++++++++++++++++++++++++++-- model/sharing/group_test.go | 77 ++++++++++++++++++++++ model/sharing/sharing.go | 15 +++++ pkg/couchdb/index.go | 3 + worker/share/share.go | 22 +++++++ 7 files changed, 249 insertions(+), 10 deletions(-) diff --git a/cozy.example.yaml b/cozy.example.yaml index 8649aa74863..b9d18157ec5 100644 --- a/cozy.example.yaml +++ b/cozy.example.yaml @@ -146,7 +146,8 @@ jobs: # - "push": sending push notifications # - "sms": sending SMS notifications # - "sendmail": sending mails - # - "share-replicate": for cozy to cozy sharing + # - "share-group": for cozy to cozy sharing + # - "share-replicate": idem # - "share-track": idem # - "share-upload": idem # - "thumbnail": creatings and deleting thumbnails for images diff --git a/docs/workers.md b/docs/workers.md index aca6676b7b2..86849cfaa80 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -343,11 +343,18 @@ in the config file, via the `fs.auto_clean_trashed_after` parameter. ## share workers -The stack have 3 workers to power the sharings (internal usage only): +The stack have 4 workers to power the sharings (internal usage only): -1. `share-track`, to update the `io.cozy.shared` database -2. `share-replicate`, to start a replicator for most documents -3. `share-upload`, to upload files +1. `share-group`, to add/remove members to a sharing +2. `share-track`, to update the `io.cozy.shared` database +3. `share-replicate`, to start a replicator for most documents +4. `share-upload`, to upload files + +### Share-group + +When a contact is added or removed to a group, they should be added to the +sharings of this group. The message is composed of the contact ID, the list +of groups added and the list of groups removed. ### Share-track diff --git a/model/sharing/group.go b/model/sharing/group.go index 322e1eb8a8d..003a55fd9cc 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -1,17 +1,28 @@ package sharing import ( + "sort" + "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/model/instance" "github.com/cozy/cozy-stack/pkg/couchdb" + multierror "github.com/hashicorp/go-multierror" ) // Group contains the information about a group of members of the sharing. type Group struct { - ID string `json:"id,omitempty"` // Only present on the instance where the group was added - Name string `json:"name"` - AddedBy int `json:"addedBy"` // The index of the member who have added the group - Removed bool `json:"removed,omitempty"` + ID string `json:"id,omitempty"` // Only present on the instance where the group was added + Name string `json:"name"` + AddedBy int `json:"addedBy"` // The index of the member who have added the group + ReadOnly bool `json:"read_only"` + Removed bool `json:"removed,omitempty"` +} + +// GroupMessage is used for jobs on the share-group worker. +type GroupMessage struct { + ContactID string `json:"contact_id"` + GroupsAdded []string `json:"added"` + GroupsRemoved []string `json:"removed"` } // AddGroup adds a group of contacts identified by its ID to the members of the @@ -38,9 +49,10 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo return err } s.Members[idx].Groups = append(s.Members[idx].Groups, groupIndex) + sort.Ints(s.Members[idx].Groups) } - g := Group{ID: groupID, Name: group.Name(), AddedBy: 0} + g := Group{ID: groupID, Name: group.Name(), AddedBy: 0, ReadOnly: readOnly} s.Groups = append(s.Groups, g) return nil } @@ -83,3 +95,105 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { s.Groups[index].Removed = true return couchdb.UpdateDoc(inst, s) } + +// UpdateGroups is called when a contact is added or removed to a group. It +// finds the sharings for this group, and adds or removes the member to those +// sharings. +func UpdateGroups(inst *instance.Instance, msg GroupMessage) error { + sharings, err := FindActive(inst) + if err != nil { + return err + } + + var errm error + for _, s := range sharings { + for _, added := range msg.GroupsAdded { + for idx, group := range s.Groups { + if group.ID == added { + if err := s.AddMemberToGroup(inst, idx, msg.ContactID); err != nil { + errm = multierror.Append(errm, err) + } + } + } + } + for _, removed := range msg.GroupsRemoved { + for idx, group := range s.Groups { + if group.ID == removed { + if err := s.RemoveMemberFromGroup(inst, idx, msg.ContactID); err != nil { + errm = multierror.Append(errm, err) + } + } + } + } + } + + return errm +} + +// AddMemberToGroup adds a contact to a sharing via a group. +func (s *Sharing) AddMemberToGroup(inst *instance.Instance, groupIndex int, contactID string) error { + contact, err := contact.Find(inst, contactID) + if err != nil { + return err + } + + readOnly := s.Groups[groupIndex].ReadOnly + m, err := buildMemberFromContact(contact, readOnly) + if err != nil { + return err + } + m.OnlyInGroups = true + _, idx, err := s.addMember(inst, m) + if err != nil { + return err + } + s.Members[idx].Groups = append(s.Members[idx].Groups, groupIndex) + sort.Ints(s.Members[idx].Groups) + + return couchdb.UpdateDoc(inst, s) +} + +// RemoveMemberFromGroup removes a member of a group. +func (s *Sharing) RemoveMemberFromGroup(inst *instance.Instance, groupIndex int, contactID string) error { + contact, err := contact.Find(inst, contactID) + if err != nil { + return err + } + var email string + if addr, err := contact.ToMailAddress(); err == nil { + email = addr.Email + } + cozyURL := contact.PrimaryCozyURL() + + matchMember := func(m Member) bool { + if m.Email != "" && m.Email == email { + return true + } + if m.Instance != "" && m.Instance == cozyURL { + return true + } + return false + } + + for i, m := range s.Members { + if !matchMember(m) { + continue + } + + var groups []int + for _, idx := range m.Groups { + if idx != groupIndex { + groups = append(groups, idx) + } + } + s.Members[i].Groups = groups + + if m.OnlyInGroups && len(s.Members[i].Groups) == 0 { + return s.RevokeRecipient(inst, i) + } else { + return couchdb.UpdateDoc(inst, s) + } + } + + return nil +} diff --git a/model/sharing/group_test.go b/model/sharing/group_test.go index de30e2bf411..d004dcfae78 100644 --- a/model/sharing/group_test.go +++ b/model/sharing/group_test.go @@ -94,6 +94,83 @@ func TestGroups(t *testing.T) { assert.True(t, s.Groups[0].Removed) assert.True(t, s.Groups[1].Removed) }) + + t.Run("UpdateGroups", func(t *testing.T) { + now := time.Now() + friends := createGroup(t, inst, "Friends") + football := createGroup(t, inst, "Football") + _ = createContactInGroups(t, inst, "Bob", []string{friends.ID()}) + charlie := createContactInGroups(t, inst, "Charlie", []string{football.ID()}) + + s := &Sharing{ + Active: true, + Owner: true, + Description: "Just testing groups", + Members: []Member{ + {Status: MemberStatusOwner, Name: "Alice", Email: "alice@cozy.tools"}, + }, + CreatedAt: now, + UpdatedAt: now, + } + require.NoError(t, couchdb.CreateDoc(inst, s)) + require.NoError(t, s.AddGroup(inst, friends.ID(), false)) + require.NoError(t, s.AddGroup(inst, football.ID(), false)) + require.NoError(t, couchdb.UpdateDoc(inst, s)) + sid := s.ID() + + require.Len(t, s.Members, 3) + require.Equal(t, s.Members[0].Name, "Alice") + require.Equal(t, s.Members[1].Name, "Bob") + assert.True(t, s.Members[1].OnlyInGroups) + assert.Equal(t, s.Members[1].Groups, []int{0}) + require.Equal(t, s.Members[2].Name, "Charlie") + assert.True(t, s.Members[2].OnlyInGroups) + assert.Equal(t, s.Members[2].Groups, []int{1}) + + require.Len(t, s.Groups, 2) + require.Equal(t, s.Groups[0].Name, "Friends") + assert.False(t, s.Groups[0].Removed) + require.Equal(t, s.Groups[1].Name, "Football") + assert.False(t, s.Groups[1].Removed) + + // Charlie is added to the friends group + msg1 := GroupMessage{ + ContactID: charlie.ID(), + GroupsAdded: []string{friends.ID()}, + } + require.NoError(t, UpdateGroups(inst, msg1)) + + s = &Sharing{} + require.NoError(t, couchdb.GetDoc(inst, consts.Sharings, sid, s)) + + require.Len(t, s.Members, 3) + require.Equal(t, s.Members[0].Name, "Alice") + require.Equal(t, s.Members[1].Name, "Bob") + assert.True(t, s.Members[1].OnlyInGroups) + assert.Equal(t, s.Members[1].Groups, []int{0}) + require.Equal(t, s.Members[2].Name, "Charlie") + assert.True(t, s.Members[2].OnlyInGroups) + assert.Equal(t, s.Members[2].Groups, []int{0, 1}) + + // Charlie is removed of the football group + msg2 := GroupMessage{ + ContactID: charlie.ID(), + GroupsRemoved: []string{football.ID()}, + } + require.NoError(t, UpdateGroups(inst, msg2)) + + s = &Sharing{} + require.NoError(t, couchdb.GetDoc(inst, consts.Sharings, sid, s)) + + require.Len(t, s.Members, 3) + require.Equal(t, s.Members[0].Name, "Alice") + require.Equal(t, s.Members[1].Name, "Bob") + assert.True(t, s.Members[1].OnlyInGroups) + assert.Equal(t, s.Members[1].Groups, []int{0}) + require.Equal(t, s.Members[2].Name, "Charlie") + assert.True(t, s.Members[2].OnlyInGroups) + assert.Equal(t, s.Members[2].Groups, []int{0}) + }) } func createGroup(t *testing.T, inst *instance.Instance, name string) *contact.Group { diff --git a/model/sharing/sharing.go b/model/sharing/sharing.go index 3590e0398c4..8800dc9834a 100644 --- a/model/sharing/sharing.go +++ b/model/sharing/sharing.go @@ -720,6 +720,21 @@ func FindSharings(db prefixer.Prefixer, sharingIDs []string) ([]*Sharing, error) return res, nil } +// FindActive returns the list of active sharings. +func FindActive(db prefixer.Prefixer) ([]*Sharing, error) { + req := &couchdb.FindRequest{ + UseIndex: "active", + Selector: mango.Equal("active", true), + Limit: 1000, + } + var res []*Sharing + err := couchdb.FindDocs(db, consts.Sharings, req, &res) + if err != nil { + return nil, err + } + return res, nil +} + // GetSharingsByDocType returns all the sharings for the given doctype func GetSharingsByDocType(inst *instance.Instance, docType string) (map[string]*Sharing, error) { req := &couchdb.ViewRequest{ diff --git a/pkg/couchdb/index.go b/pkg/couchdb/index.go index f698fc9f528..9c79b96d85e 100644 --- a/pkg/couchdb/index.go +++ b/pkg/couchdb/index.go @@ -69,6 +69,9 @@ var Indexes = []*mango.Index{ // Used to find the contacts in a group mango.MakeIndex(consts.Contacts, "by-groups", mango.IndexDef{Fields: []string{"relationships.groups.data"}}), + + // Used to find the active sharings + mango.MakeIndex(consts.Sharings, "active", mango.IndexDef{Fields: []string{"active"}}), } // DiskUsageView is the view used for computing the disk usage for files diff --git a/worker/share/share.go b/worker/share/share.go index a30b0b8cafd..f5c51f854b7 100644 --- a/worker/share/share.go +++ b/worker/share/share.go @@ -1,3 +1,4 @@ +// Package share is where the workers for Cozy to Cozy sharings are defined. package share import ( @@ -9,6 +10,15 @@ import ( ) func init() { + job.AddWorker(&job.WorkerConfig{ + WorkerType: "share-group", + Concurrency: runtime.NumCPU(), + MaxExecCount: 2, + Reserved: true, + Timeout: 30 * time.Second, + WorkerFunc: WorkerGroup, + }) + job.AddWorker(&job.WorkerConfig{ WorkerType: "share-track", Concurrency: runtime.NumCPU(), @@ -43,6 +53,18 @@ func init() { }) } +// WorkerGroup is used to update the list of members of sharings for a group +// when someone is added or removed to this group. +func WorkerGroup(ctx *job.TaskContext) error { + var msg sharing.GroupMessage + if err := ctx.UnmarshalMessage(&msg); err != nil { + return err + } + ctx.Instance.Logger().WithNamespace("share"). + Debugf("Group %#v", msg) + return sharing.UpdateGroups(ctx.Instance, msg) +} + // WorkerTrack is used to update the io.cozy.shared database when a document // that matches a sharing rule is created/updated/remove func WorkerTrack(ctx *job.TaskContext) error { From 2c20c7428fa1b6b8b8048ec9e54e1cbae572cbbc Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Mon, 19 Feb 2024 14:43:02 +0100 Subject: [PATCH 10/22] Add a trigger for share-group --- model/job/mem_scheduler.go | 4 + model/job/redis_scheduler.go | 4 + model/job/trigger_share_group.go | 138 ++++++++++++++++++++++++++ model/job/trigger_share_group_test.go | 111 +++++++++++++++++++++ model/sharing/group.go | 10 +- model/sharing/group_test.go | 5 +- worker/share/share.go | 2 +- 7 files changed, 263 insertions(+), 11 deletions(-) create mode 100644 model/job/trigger_share_group.go create mode 100644 model/job/trigger_share_group_test.go diff --git a/model/job/mem_scheduler.go b/model/job/mem_scheduler.go index 5e73ff384fe..d6daeeec254 100644 --- a/model/job/mem_scheduler.go +++ b/model/job/mem_scheduler.go @@ -24,6 +24,7 @@ type memScheduler struct { ts map[string]Trigger thumb *ThumbnailTrigger + share *ShareGroupTrigger mu sync.RWMutex log *logger.Entry } @@ -51,6 +52,8 @@ func (s *memScheduler) StartScheduler(b Broker) error { s.thumb = NewThumbnailTrigger(s.broker) go s.thumb.Schedule() + s.share = NewShareGroupTrigger(s.broker) + go s.share.Schedule() // XXX The memory scheduler loads the triggers from CouchDB when the stack // is started. This can cause some stability issues when running system @@ -117,6 +120,7 @@ func (s *memScheduler) ShutdownScheduler(ctx context.Context) error { t.Unschedule() } s.thumb.Unschedule() + s.share.Unschedule() fmt.Println("ok.") return nil } diff --git a/model/job/redis_scheduler.go b/model/job/redis_scheduler.go index 9d66c632883..f8b2ce9213a 100644 --- a/model/job/redis_scheduler.go +++ b/model/job/redis_scheduler.go @@ -59,6 +59,7 @@ type redisScheduler struct { client redis.UniversalClient ctx context.Context thumb *ThumbnailTrigger + share *ShareGroupTrigger closed chan struct{} stopped chan struct{} log *logger.Entry @@ -99,6 +100,8 @@ func (s *redisScheduler) StartScheduler(b Broker) error { s.startEventDispatcher() s.thumb = NewThumbnailTrigger(s.broker) go s.thumb.Schedule() + s.share = NewShareGroupTrigger(s.broker) + go s.share.Schedule() go s.pollLoop() return nil } @@ -252,6 +255,7 @@ func (s *redisScheduler) ShutdownScheduler(ctx context.Context) error { fmt.Print(" shutting down redis scheduler...") close(s.closed) s.thumb.Unschedule() + s.share.Unschedule() select { case <-ctx.Done(): fmt.Println("failed: ", ctx.Err()) diff --git a/model/job/trigger_share_group.go b/model/job/trigger_share_group.go new file mode 100644 index 00000000000..a5c6d2433ae --- /dev/null +++ b/model/job/trigger_share_group.go @@ -0,0 +1,138 @@ +package job + +import ( + "github.com/cozy/cozy-stack/pkg/consts" + "github.com/cozy/cozy-stack/pkg/couchdb" + "github.com/cozy/cozy-stack/pkg/logger" + "github.com/cozy/cozy-stack/pkg/realtime" +) + +type ShareGroupTrigger struct { + broker Broker + log *logger.Entry + unscheduled chan struct{} +} + +// ShareGroupMessage is used for jobs on the share-group worker. +type ShareGroupMessage struct { + ContactID string `json:"contact_id"` + GroupsAdded []string `json:"added"` + GroupsRemoved []string `json:"removed"` +} + +func NewShareGroupTrigger(broker Broker) *ShareGroupTrigger { + return &ShareGroupTrigger{ + broker: broker, + log: logger.WithNamespace("scheduler"), + unscheduled: make(chan struct{}), + } +} + +func (t *ShareGroupTrigger) Schedule() { + sub := realtime.GetHub().SubscribeFirehose() + defer sub.Close() + for { + select { + case e := <-sub.Channel: + if msg := t.match(e); msg != nil { + t.pushJob(e, msg) + } + case <-t.unscheduled: + return + } + } +} + +func (t *ShareGroupTrigger) match(e *realtime.Event) *ShareGroupMessage { + if e.Doc.DocType() != consts.Contacts { + return nil + } + if e.Verb == realtime.EventNotify { + return nil + } + + newdoc, ok := e.Doc.(*couchdb.JSONDoc) + if !ok { + return nil + } + newgroups := extractGroupIDs(newdoc) + + var oldgroups []string + if olddoc, ok := e.OldDoc.(*couchdb.JSONDoc); ok { + oldgroups = extractGroupIDs(olddoc) + } + + added := diffGroupIDs(newgroups, oldgroups) + removed := diffGroupIDs(oldgroups, newgroups) + + if len(added) == 0 && len(removed) == 0 { + return nil + } + + return &ShareGroupMessage{ + ContactID: e.Doc.ID(), + GroupsAdded: added, + GroupsRemoved: removed, + } +} + +func extractGroupIDs(doc *couchdb.JSONDoc) []string { + var groupIDs []string + + if rels, ok := doc.Get("relationships").(map[string]interface{}); ok { + for _, groups := range rels { + if groups, ok := groups.(map[string]interface{}); ok { + if data, ok := groups["data"].([]interface{}); ok { + for _, item := range data { + if item, ok := item.(map[string]interface{}); ok { + if item["_type"] == consts.Groups { + if id, ok := item["_id"].(string); ok { + groupIDs = append(groupIDs, id) + } + } + } + } + } + } + } + } + + return groupIDs +} + +func diffGroupIDs(as, bs []string) []string { + var diff []string + for _, a := range as { + found := false + for _, b := range bs { + if a == b { + found = true + } + } + if !found { + diff = append(diff, a) + } + } + return diff +} + +func (t *ShareGroupTrigger) pushJob(e *realtime.Event, msg *ShareGroupMessage) { + log := t.log.WithField("domain", e.Domain) + m, err := NewMessage(msg) + if err != nil { + log.Infof("trigger share-group: cannot serialize message: %s", err) + return + } + req := &JobRequest{ + WorkerType: "share-group", + Message: m, + } + log.Infof("trigger share-group: Pushing new job for contact %s", msg.ContactID) + if _, err := t.broker.PushJob(e, req); err != nil { + log.Errorf("trigger share-group: Could not schedule a new job: %s", err.Error()) + } +} + +func (t *ShareGroupTrigger) Unschedule() { + close(t.unscheduled) +} diff --git a/model/job/trigger_share_group_test.go b/model/job/trigger_share_group_test.go new file mode 100644 index 00000000000..f8c3f340a9c --- /dev/null +++ b/model/job/trigger_share_group_test.go @@ -0,0 +1,111 @@ +package job + +import ( + "encoding/json" + "testing" + + "github.com/cozy/cozy-stack/pkg/consts" + "github.com/cozy/cozy-stack/pkg/couchdb" + "github.com/cozy/cozy-stack/pkg/realtime" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestShareGroupTrigger(t *testing.T) { + trigger := &ShareGroupTrigger{} + + noGroup := &couchdb.JSONDoc{ + Type: consts.Contacts, + M: map[string]interface{}{ + "_id": "85507320-b157-013c-12d8-18c04daba326", + "_rev": "1-abcdef", + "fullname": "Bob", + }, + } + msg := trigger.match(&realtime.Event{ + Doc: noGroup, + OldDoc: nil, + Verb: realtime.EventCreate, + }) + require.Nil(t, msg) + + updatedName := noGroup.Clone().(*couchdb.JSONDoc) + updatedName.M["fullname"] = "Bobby" + updatedName.M["_rev"] = "2-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: updatedName, + OldDoc: noGroup, + Verb: realtime.EventUpdate, + }) + require.Nil(t, msg) + + var groups map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(`{ + "groups": { + "data": [ + { + "_id": "id-friends", + "_type": "io.cozy.contacts.groups" + }, + { + "_id": "id-football", + "_type": "io.cozy.contacts.groups" + } + ] + } +}`), &groups)) + addedInGroups := updatedName.Clone().(*couchdb.JSONDoc) + addedInGroups.M["relationships"] = groups + addedInGroups.M["_rev"] = "3-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: addedInGroups, + OldDoc: updatedName, + Verb: realtime.EventUpdate, + }) + require.NotNil(t, msg) + assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") + assert.EqualValues(t, msg.GroupsAdded, []string{"id-friends", "id-football"}) + assert.Len(t, msg.GroupsRemoved, 0) + + var groups2 map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(`{ + "groups": { + "data": [ + { + "_id": "id-friends", + "_type": "io.cozy.contacts.groups" + } + ] + } +}`), &groups2)) + removedFromFootball := addedInGroups.Clone().(*couchdb.JSONDoc) + removedFromFootball.M["relationships"] = groups2 + removedFromFootball.M["_rev"] = "4-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: removedFromFootball, + OldDoc: addedInGroups, + Verb: realtime.EventUpdate, + }) + require.NotNil(t, msg) + assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") + assert.Len(t, msg.GroupsAdded, 0) + assert.EqualValues(t, msg.GroupsRemoved, []string{"id-football"}) + + deleted := &couchdb.JSONDoc{ + Type: consts.Contacts, + M: map[string]interface{}{ + "_id": removedFromFootball.ID(), + "_rev": "5-abcdef", + "_deleted": true, + }, + } + msg = trigger.match(&realtime.Event{ + Doc: deleted, + OldDoc: removedFromFootball, + Verb: realtime.EventDelete, + }) + require.NotNil(t, msg) + assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") + assert.Len(t, msg.GroupsAdded, 0) + assert.EqualValues(t, msg.GroupsRemoved, []string{"id-friends"}) +} diff --git a/model/sharing/group.go b/model/sharing/group.go index 003a55fd9cc..72309d5565e 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -5,6 +5,7 @@ import ( "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/model/instance" + "github.com/cozy/cozy-stack/model/job" "github.com/cozy/cozy-stack/pkg/couchdb" multierror "github.com/hashicorp/go-multierror" ) @@ -18,13 +19,6 @@ type Group struct { Removed bool `json:"removed,omitempty"` } -// GroupMessage is used for jobs on the share-group worker. -type GroupMessage struct { - ContactID string `json:"contact_id"` - GroupsAdded []string `json:"added"` - GroupsRemoved []string `json:"removed"` -} - // AddGroup adds a group of contacts identified by its ID to the members of the // sharing. func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly bool) error { @@ -99,7 +93,7 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { // UpdateGroups is called when a contact is added or removed to a group. It // finds the sharings for this group, and adds or removes the member to those // sharings. -func UpdateGroups(inst *instance.Instance, msg GroupMessage) error { +func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { sharings, err := FindActive(inst) if err != nil { return err diff --git a/model/sharing/group_test.go b/model/sharing/group_test.go index d004dcfae78..599d9566a15 100644 --- a/model/sharing/group_test.go +++ b/model/sharing/group_test.go @@ -7,6 +7,7 @@ import ( "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/model/instance" + "github.com/cozy/cozy-stack/model/job" "github.com/cozy/cozy-stack/pkg/config/config" "github.com/cozy/cozy-stack/pkg/consts" "github.com/cozy/cozy-stack/pkg/couchdb" @@ -134,7 +135,7 @@ func TestGroups(t *testing.T) { assert.False(t, s.Groups[1].Removed) // Charlie is added to the friends group - msg1 := GroupMessage{ + msg1 := job.ShareGroupMessage{ ContactID: charlie.ID(), GroupsAdded: []string{friends.ID()}, } @@ -153,7 +154,7 @@ func TestGroups(t *testing.T) { assert.Equal(t, s.Members[2].Groups, []int{0, 1}) // Charlie is removed of the football group - msg2 := GroupMessage{ + msg2 := job.ShareGroupMessage{ ContactID: charlie.ID(), GroupsRemoved: []string{football.ID()}, } diff --git a/worker/share/share.go b/worker/share/share.go index f5c51f854b7..4f211784ee1 100644 --- a/worker/share/share.go +++ b/worker/share/share.go @@ -56,7 +56,7 @@ func init() { // WorkerGroup is used to update the list of members of sharings for a group // when someone is added or removed to this group. func WorkerGroup(ctx *job.TaskContext) error { - var msg sharing.GroupMessage + var msg job.ShareGroupMessage if err := ctx.UnmarshalMessage(&msg); err != nil { return err } From 2301132bb2bbedfcbd1718d39eac39ecb002b8c5 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Tue, 20 Feb 2024 10:09:10 +0100 Subject: [PATCH 11/22] Refactor with Contact.GroupIDs --- model/contact/contacts.go | 28 ++++++++++++++++++++++++++++ model/job/trigger_share_group.go | 31 +++++-------------------------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/model/contact/contacts.go b/model/contact/contacts.go index 685f0a111b6..f1e5914dbe9 100644 --- a/model/contact/contacts.go +++ b/model/contact/contacts.go @@ -155,6 +155,34 @@ func (c *Contact) PrimaryCozyURL() string { return url } +// GroupIDs returns the list of the group identifiers that this contact belongs to. +func (c *Contact) GroupIDs() []string { + rels, ok := c.Get("relationships").(map[string]interface{}) + if !ok { + return nil + } + + var groupIDs []string + + for _, groups := range rels { + if groups, ok := groups.(map[string]interface{}); ok { + if data, ok := groups["data"].([]interface{}); ok { + for _, item := range data { + if item, ok := item.(map[string]interface{}); ok { + if item["_type"] == consts.Groups { + if id, ok := item["_id"].(string); ok { + groupIDs = append(groupIDs, id) + } + } + } + } + } + } + } + + return groupIDs +} + // AddNameIfMissing can be used to add a name if there was none. We need the // email address to ignore it if the displayName was updated with it by a // service of the contacts application. diff --git a/model/job/trigger_share_group.go b/model/job/trigger_share_group.go index a5c6d2433ae..9e9e94602b4 100644 --- a/model/job/trigger_share_group.go +++ b/model/job/trigger_share_group.go @@ -1,6 +1,7 @@ package job import ( + "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/pkg/consts" "github.com/cozy/cozy-stack/pkg/couchdb" "github.com/cozy/cozy-stack/pkg/logger" @@ -55,11 +56,13 @@ func (t *ShareGroupTrigger) match(e *realtime.Event) *ShareGroupMessage { if !ok { return nil } - newgroups := extractGroupIDs(newdoc) + newContact := &contact.Contact{JSONDoc: *newdoc} + newgroups := newContact.GroupIDs() var oldgroups []string if olddoc, ok := e.OldDoc.(*couchdb.JSONDoc); ok { - oldgroups = extractGroupIDs(olddoc) + oldContact := &contact.Contact{JSONDoc: *olddoc} + oldgroups = oldContact.GroupIDs() } added := diffGroupIDs(newgroups, oldgroups) @@ -76,30 +79,6 @@ func (t *ShareGroupTrigger) match(e *realtime.Event) *ShareGroupMessage { } } -func extractGroupIDs(doc *couchdb.JSONDoc) []string { - var groupIDs []string - - if rels, ok := doc.Get("relationships").(map[string]interface{}); ok { - for _, groups := range rels { - if groups, ok := groups.(map[string]interface{}); ok { - if data, ok := groups["data"].([]interface{}); ok { - for _, item := range data { - if item, ok := item.(map[string]interface{}); ok { - if item["_type"] == consts.Groups { - if id, ok := item["_id"].(string); ok { - groupIDs = append(groupIDs, id) - } - } - } - } - } - } - } - } - - return groupIDs -} - func diffGroupIDs(as, bs []string) []string { var diff []string for _, a := range as { From f2b38eeeee5cccc2774f3d7e67287c8a95f75f0c Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Tue, 20 Feb 2024 12:33:06 +0100 Subject: [PATCH 12/22] Update ShareGroupTrigger to detect invitable contact --- model/job/trigger_share_group.go | 34 ++++- model/job/trigger_share_group_test.go | 204 +++++++++++++++++--------- 2 files changed, 165 insertions(+), 73 deletions(-) diff --git a/model/job/trigger_share_group.go b/model/job/trigger_share_group.go index 9e9e94602b4..56c5cde484e 100644 --- a/model/job/trigger_share_group.go +++ b/model/job/trigger_share_group.go @@ -16,9 +16,10 @@ type ShareGroupTrigger struct { // ShareGroupMessage is used for jobs on the share-group worker. type ShareGroupMessage struct { - ContactID string `json:"contact_id"` - GroupsAdded []string `json:"added"` - GroupsRemoved []string `json:"removed"` + ContactID string `json:"contact_id"` + GroupsAdded []string `json:"added"` + GroupsRemoved []string `json:"removed"` + BecomeInvitable bool `json:"invitable"` } func NewShareGroupTrigger(broker Broker) *ShareGroupTrigger { @@ -60,22 +61,25 @@ func (t *ShareGroupTrigger) match(e *realtime.Event) *ShareGroupMessage { newgroups := newContact.GroupIDs() var oldgroups []string + invitable := false if olddoc, ok := e.OldDoc.(*couchdb.JSONDoc); ok { oldContact := &contact.Contact{JSONDoc: *olddoc} oldgroups = oldContact.GroupIDs() + invitable = contactIsNowInvitable(oldContact, newContact) } added := diffGroupIDs(newgroups, oldgroups) removed := diffGroupIDs(oldgroups, newgroups) - if len(added) == 0 && len(removed) == 0 { + if len(added) == 0 && len(removed) == 0 && !invitable { return nil } return &ShareGroupMessage{ - ContactID: e.Doc.ID(), - GroupsAdded: added, - GroupsRemoved: removed, + ContactID: e.Doc.ID(), + GroupsAdded: added, + GroupsRemoved: removed, + BecomeInvitable: invitable, } } @@ -95,6 +99,22 @@ func diffGroupIDs(as, bs []string) []string { return diff } +func contactIsNowInvitable(oldContact, newContact *contact.Contact) bool { + if oldURL := oldContact.PrimaryCozyURL(); oldURL != "" { + return false + } + if oldAddr, err := oldContact.ToMailAddress(); err == nil && oldAddr.Email != "" { + return false + } + if newURL := newContact.PrimaryCozyURL(); newURL != "" { + return true + } + if newAddr, err := newContact.ToMailAddress(); err == nil && newAddr.Email != "" { + return true + } + return false +} + func (t *ShareGroupTrigger) pushJob(e *realtime.Event, msg *ShareGroupMessage) { log := t.log.WithField("domain", e.Domain) m, err := NewMessage(msg) diff --git a/model/job/trigger_share_group_test.go b/model/job/trigger_share_group_test.go index f8c3f340a9c..05f7997dd2a 100644 --- a/model/job/trigger_share_group_test.go +++ b/model/job/trigger_share_group_test.go @@ -14,33 +14,101 @@ import ( func TestShareGroupTrigger(t *testing.T) { trigger := &ShareGroupTrigger{} - noGroup := &couchdb.JSONDoc{ - Type: consts.Contacts, - M: map[string]interface{}{ - "_id": "85507320-b157-013c-12d8-18c04daba326", - "_rev": "1-abcdef", - "fullname": "Bob", - }, - } - msg := trigger.match(&realtime.Event{ - Doc: noGroup, - OldDoc: nil, - Verb: realtime.EventCreate, - }) - require.Nil(t, msg) + t.Run("The contact becomes invitable", func(t *testing.T) { + justName := &couchdb.JSONDoc{ + Type: consts.Contacts, + M: map[string]interface{}{ + "_id": "85507320-b157-013c-12d8-18c04daba325", + "_rev": "1-abcdef", + "fullname": "Bob", + }, + } + msg := trigger.match(&realtime.Event{ + Doc: justName, + OldDoc: nil, + Verb: realtime.EventCreate, + }) + require.Nil(t, msg) + + withAnEmail := justName.Clone().(*couchdb.JSONDoc) + withAnEmail.M["email"] = []interface{}{ + map[string]interface{}{ + "address": "bob@example.net", + }, + } + withAnEmail.M["_rev"] = "2-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: withAnEmail, + OldDoc: justName, + Verb: realtime.EventUpdate, + }) + require.NotNil(t, msg) + assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba325") + assert.Len(t, msg.GroupsAdded, 0) + assert.Len(t, msg.GroupsRemoved, 0) + assert.True(t, msg.BecomeInvitable) - updatedName := noGroup.Clone().(*couchdb.JSONDoc) - updatedName.M["fullname"] = "Bobby" - updatedName.M["_rev"] = "2-abcdef" - msg = trigger.match(&realtime.Event{ - Doc: updatedName, - OldDoc: noGroup, - Verb: realtime.EventUpdate, + withCozyURL := justName.Clone().(*couchdb.JSONDoc) + withCozyURL.M["cozy"] = []interface{}{ + map[string]interface{}{ + "url": "bob.mycozy.cloud", + }, + } + withCozyURL.M["_rev"] = "2-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: withCozyURL, + OldDoc: justName, + Verb: realtime.EventUpdate, + }) + require.NotNil(t, msg) + assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba325") + assert.Len(t, msg.GroupsAdded, 0) + assert.Len(t, msg.GroupsRemoved, 0) + assert.True(t, msg.BecomeInvitable) + + both := withAnEmail.Clone().(*couchdb.JSONDoc) + both.M["cozy"] = []interface{}{ + map[string]interface{}{ + "url": "bob.mycozy.cloud", + }, + } + both.M["_rev"] = "3-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: both, + OldDoc: withAnEmail, + Verb: realtime.EventUpdate, + }) + assert.Nil(t, msg) }) - require.Nil(t, msg) - var groups map[string]interface{} - require.NoError(t, json.Unmarshal([]byte(`{ + t.Run("Groups are added/removed to a contact", func(t *testing.T) { + noGroup := &couchdb.JSONDoc{ + Type: consts.Contacts, + M: map[string]interface{}{ + "_id": "85507320-b157-013c-12d8-18c04daba326", + "_rev": "1-abcdef", + "fullname": "Bob", + }, + } + msg := trigger.match(&realtime.Event{ + Doc: noGroup, + OldDoc: nil, + Verb: realtime.EventCreate, + }) + require.Nil(t, msg) + + updatedName := noGroup.Clone().(*couchdb.JSONDoc) + updatedName.M["fullname"] = "Bobby" + updatedName.M["_rev"] = "2-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: updatedName, + OldDoc: noGroup, + Verb: realtime.EventUpdate, + }) + require.Nil(t, msg) + + var groups map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(`{ "groups": { "data": [ { @@ -54,21 +122,22 @@ func TestShareGroupTrigger(t *testing.T) { ] } }`), &groups)) - addedInGroups := updatedName.Clone().(*couchdb.JSONDoc) - addedInGroups.M["relationships"] = groups - addedInGroups.M["_rev"] = "3-abcdef" - msg = trigger.match(&realtime.Event{ - Doc: addedInGroups, - OldDoc: updatedName, - Verb: realtime.EventUpdate, - }) - require.NotNil(t, msg) - assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") - assert.EqualValues(t, msg.GroupsAdded, []string{"id-friends", "id-football"}) - assert.Len(t, msg.GroupsRemoved, 0) + addedInGroups := updatedName.Clone().(*couchdb.JSONDoc) + addedInGroups.M["relationships"] = groups + addedInGroups.M["_rev"] = "3-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: addedInGroups, + OldDoc: updatedName, + Verb: realtime.EventUpdate, + }) + require.NotNil(t, msg) + assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") + assert.EqualValues(t, msg.GroupsAdded, []string{"id-friends", "id-football"}) + assert.Len(t, msg.GroupsRemoved, 0) + assert.False(t, msg.BecomeInvitable) - var groups2 map[string]interface{} - require.NoError(t, json.Unmarshal([]byte(`{ + var groups2 map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(`{ "groups": { "data": [ { @@ -78,34 +147,37 @@ func TestShareGroupTrigger(t *testing.T) { ] } }`), &groups2)) - removedFromFootball := addedInGroups.Clone().(*couchdb.JSONDoc) - removedFromFootball.M["relationships"] = groups2 - removedFromFootball.M["_rev"] = "4-abcdef" - msg = trigger.match(&realtime.Event{ - Doc: removedFromFootball, - OldDoc: addedInGroups, - Verb: realtime.EventUpdate, - }) - require.NotNil(t, msg) - assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") - assert.Len(t, msg.GroupsAdded, 0) - assert.EqualValues(t, msg.GroupsRemoved, []string{"id-football"}) + removedFromFootball := addedInGroups.Clone().(*couchdb.JSONDoc) + removedFromFootball.M["relationships"] = groups2 + removedFromFootball.M["_rev"] = "4-abcdef" + msg = trigger.match(&realtime.Event{ + Doc: removedFromFootball, + OldDoc: addedInGroups, + Verb: realtime.EventUpdate, + }) + require.NotNil(t, msg) + assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") + assert.Len(t, msg.GroupsAdded, 0) + assert.EqualValues(t, msg.GroupsRemoved, []string{"id-football"}) + assert.False(t, msg.BecomeInvitable) - deleted := &couchdb.JSONDoc{ - Type: consts.Contacts, - M: map[string]interface{}{ - "_id": removedFromFootball.ID(), - "_rev": "5-abcdef", - "_deleted": true, - }, - } - msg = trigger.match(&realtime.Event{ - Doc: deleted, - OldDoc: removedFromFootball, - Verb: realtime.EventDelete, + deleted := &couchdb.JSONDoc{ + Type: consts.Contacts, + M: map[string]interface{}{ + "_id": removedFromFootball.ID(), + "_rev": "5-abcdef", + "_deleted": true, + }, + } + msg = trigger.match(&realtime.Event{ + Doc: deleted, + OldDoc: removedFromFootball, + Verb: realtime.EventDelete, + }) + require.NotNil(t, msg) + assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") + assert.Len(t, msg.GroupsAdded, 0) + assert.EqualValues(t, msg.GroupsRemoved, []string{"id-friends"}) + assert.False(t, msg.BecomeInvitable) }) - require.NotNil(t, msg) - assert.Equal(t, msg.ContactID, "85507320-b157-013c-12d8-18c04daba326") - assert.Len(t, msg.GroupsAdded, 0) - assert.EqualValues(t, msg.GroupsRemoved, []string{"id-friends"}) } From 710bf1672201914ef886127b28802976759b6a5d Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Tue, 20 Feb 2024 15:23:30 +0100 Subject: [PATCH 13/22] Send an invitation when group member has an email When a contact has no email and no Cozy URL when added to a sharing via a group, the stack will send an invitation later, when an email or Cozy URL is added to this contact. --- model/sharing/group.go | 100 ++++++++++++++++++++++++++++++----- model/sharing/group_test.go | 101 +++++++++++++++++++++++++++++++++--- model/sharing/invitation.go | 3 ++ model/sharing/member.go | 12 ++--- 4 files changed, 190 insertions(+), 26 deletions(-) diff --git a/model/sharing/group.go b/model/sharing/group.go index 72309d5565e..d9926f4b84f 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -6,6 +6,7 @@ import ( "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/model/instance" "github.com/cozy/cozy-stack/model/job" + "github.com/cozy/cozy-stack/model/permission" "github.com/cozy/cozy-stack/pkg/couchdb" multierror "github.com/hashicorp/go-multierror" ) @@ -94,6 +95,11 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { // finds the sharings for this group, and adds or removes the member to those // sharings. func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { + contact, err := contact.Find(inst, msg.ContactID) + if err != nil { + return err + } + sharings, err := FindActive(inst) if err != nil { return err @@ -104,7 +110,7 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { for _, added := range msg.GroupsAdded { for idx, group := range s.Groups { if group.ID == added { - if err := s.AddMemberToGroup(inst, idx, msg.ContactID); err != nil { + if err := s.AddMemberToGroup(inst, idx, contact); err != nil { errm = multierror.Append(errm, err) } } @@ -113,24 +119,25 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { for _, removed := range msg.GroupsRemoved { for idx, group := range s.Groups { if group.ID == removed { - if err := s.RemoveMemberFromGroup(inst, idx, msg.ContactID); err != nil { + if err := s.RemoveMemberFromGroup(inst, idx, contact); err != nil { errm = multierror.Append(errm, err) } } } } + + if msg.BecomeInvitable { + if err := s.AddInvitationForContact(inst, contact); err != nil { + errm = multierror.Append(errm, err) + } + } } return errm } // AddMemberToGroup adds a contact to a sharing via a group. -func (s *Sharing) AddMemberToGroup(inst *instance.Instance, groupIndex int, contactID string) error { - contact, err := contact.Find(inst, contactID) - if err != nil { - return err - } - +func (s *Sharing) AddMemberToGroup(inst *instance.Instance, groupIndex int, contact *contact.Contact) error { readOnly := s.Groups[groupIndex].ReadOnly m, err := buildMemberFromContact(contact, readOnly) if err != nil { @@ -144,15 +151,25 @@ func (s *Sharing) AddMemberToGroup(inst *instance.Instance, groupIndex int, cont s.Members[idx].Groups = append(s.Members[idx].Groups, groupIndex) sort.Ints(s.Members[idx].Groups) - return couchdb.UpdateDoc(inst, s) + // We can ignore the error as we will try again to save the sharing + // after sending the invitation. + _ = couchdb.UpdateDoc(inst, s) + var perms *permission.Permission + if s.PreviewPath != "" { + if perms, err = s.CreatePreviewPermissions(inst); err != nil { + return err + } + } + if err = s.SendInvitations(inst, perms); err != nil { + return err + } + cloned := s.Clone().(*Sharing) + go cloned.NotifyRecipients(inst, nil) + return nil } // RemoveMemberFromGroup removes a member of a group. -func (s *Sharing) RemoveMemberFromGroup(inst *instance.Instance, groupIndex int, contactID string) error { - contact, err := contact.Find(inst, contactID) - if err != nil { - return err - } +func (s *Sharing) RemoveMemberFromGroup(inst *instance.Instance, groupIndex int, contact *contact.Contact) error { var email string if addr, err := contact.ToMailAddress(); err == nil { email = addr.Email @@ -191,3 +208,58 @@ func (s *Sharing) RemoveMemberFromGroup(inst *instance.Instance, groupIndex int, return nil } + +func (s *Sharing) AddInvitationForContact(inst *instance.Instance, contact *contact.Contact) error { + var email string + if addr, err := contact.ToMailAddress(); err == nil { + email = addr.Email + } + cozyURL := contact.PrimaryCozyURL() + name := contact.PrimaryName() + groupIDs := contact.GroupIDs() + + matchMember := func(m Member) bool { + if m.Name != name { + return false + } + for _, gid := range groupIDs { + for _, g := range m.Groups { + if s.Groups[g].ID == gid { + return true + } + } + } + return false + } + + for i, m := range s.Members { + if i == 0 || m.Status != MemberStatusMailNotSent { + continue + } + if !matchMember(m) { + continue + } + m.Email = email + m.Instance = cozyURL + s.Members[i] = m + + // We can ignore the error as we will try again to save the sharing + // after sending the invitation. + _ = couchdb.UpdateDoc(inst, s) + var perms *permission.Permission + var err error + if s.PreviewPath != "" { + if perms, err = s.CreatePreviewPermissions(inst); err != nil { + return err + } + } + if err = s.SendInvitations(inst, perms); err != nil { + return err + } + cloned := s.Clone().(*Sharing) + go cloned.NotifyRecipients(inst, nil) + return nil + } + + return nil +} diff --git a/model/sharing/group_test.go b/model/sharing/group_test.go index 599d9566a15..1576029589a 100644 --- a/model/sharing/group_test.go +++ b/model/sharing/group_test.go @@ -7,6 +7,7 @@ import ( "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/model/instance" + "github.com/cozy/cozy-stack/model/instance/lifecycle" "github.com/cozy/cozy-stack/model/job" "github.com/cozy/cozy-stack/pkg/config/config" "github.com/cozy/cozy-stack/pkg/consts" @@ -14,6 +15,8 @@ import ( "github.com/cozy/cozy-stack/tests/testutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + _ "github.com/cozy/cozy-stack/worker/mails" ) func TestGroups(t *testing.T) { @@ -24,7 +27,10 @@ func TestGroups(t *testing.T) { config.UseTestFile(t) testutils.NeedCouchdb(t) setup := testutils.NewSetup(t, t.Name()) - inst := setup.GetTestInstance() + inst := setup.GetTestInstance(&lifecycle.Options{ + Email: "alice@example.net", + PublicName: "Alice", + }) t.Run("RevokeGroup", func(t *testing.T) { now := time.Now() @@ -41,6 +47,13 @@ func TestGroups(t *testing.T) { Members: []Member{ {Status: MemberStatusOwner, Name: "Alice", Email: "alice@cozy.tools"}, }, + Rules: []Rule{ + { + Title: "Just testing groups", + DocType: "io.cozy.tests", + Values: []string{uuidv7()}, + }, + }, CreatedAt: now, UpdatedAt: now, } @@ -102,6 +115,7 @@ func TestGroups(t *testing.T) { football := createGroup(t, inst, "Football") _ = createContactInGroups(t, inst, "Bob", []string{friends.ID()}) charlie := createContactInGroups(t, inst, "Charlie", []string{football.ID()}) + dave := createContactWithoutEmail(t, inst, "Dave", []string{football.ID()}) s := &Sharing{ Active: true, @@ -110,23 +124,37 @@ func TestGroups(t *testing.T) { Members: []Member{ {Status: MemberStatusOwner, Name: "Alice", Email: "alice@cozy.tools"}, }, + Rules: []Rule{ + { + Title: "Just testing groups", + DocType: "io.cozy.tests", + Values: []string{uuidv7()}, + }, + }, CreatedAt: now, UpdatedAt: now, } - require.NoError(t, couchdb.CreateDoc(inst, s)) require.NoError(t, s.AddGroup(inst, friends.ID(), false)) require.NoError(t, s.AddGroup(inst, football.ID(), false)) - require.NoError(t, couchdb.UpdateDoc(inst, s)) + perms, err := s.Create(inst) + require.NoError(t, err) + require.NoError(t, s.SendInvitations(inst, perms)) sid := s.ID() - require.Len(t, s.Members, 3) + require.Len(t, s.Members, 4) require.Equal(t, s.Members[0].Name, "Alice") require.Equal(t, s.Members[1].Name, "Bob") assert.True(t, s.Members[1].OnlyInGroups) assert.Equal(t, s.Members[1].Groups, []int{0}) + assert.Equal(t, s.Members[1].Status, MemberStatusPendingInvitation) require.Equal(t, s.Members[2].Name, "Charlie") assert.True(t, s.Members[2].OnlyInGroups) assert.Equal(t, s.Members[2].Groups, []int{1}) + assert.Equal(t, s.Members[2].Status, MemberStatusPendingInvitation) + require.Equal(t, s.Members[3].Name, "Dave") + assert.True(t, s.Members[3].OnlyInGroups) + assert.Equal(t, s.Members[3].Groups, []int{1}) + assert.Equal(t, s.Members[3].Status, MemberStatusMailNotSent) require.Len(t, s.Groups, 2) require.Equal(t, s.Groups[0].Name, "Friends") @@ -144,7 +172,7 @@ func TestGroups(t *testing.T) { s = &Sharing{} require.NoError(t, couchdb.GetDoc(inst, consts.Sharings, sid, s)) - require.Len(t, s.Members, 3) + require.Len(t, s.Members, 4) require.Equal(t, s.Members[0].Name, "Alice") require.Equal(t, s.Members[1].Name, "Bob") assert.True(t, s.Members[1].OnlyInGroups) @@ -152,6 +180,9 @@ func TestGroups(t *testing.T) { require.Equal(t, s.Members[2].Name, "Charlie") assert.True(t, s.Members[2].OnlyInGroups) assert.Equal(t, s.Members[2].Groups, []int{0, 1}) + require.Equal(t, s.Members[3].Name, "Dave") + assert.True(t, s.Members[3].OnlyInGroups) + assert.Equal(t, s.Members[3].Groups, []int{1}) // Charlie is removed of the football group msg2 := job.ShareGroupMessage{ @@ -163,7 +194,7 @@ func TestGroups(t *testing.T) { s = &Sharing{} require.NoError(t, couchdb.GetDoc(inst, consts.Sharings, sid, s)) - require.Len(t, s.Members, 3) + require.Len(t, s.Members, 4) require.Equal(t, s.Members[0].Name, "Alice") require.Equal(t, s.Members[1].Name, "Bob") assert.True(t, s.Members[1].OnlyInGroups) @@ -171,6 +202,35 @@ func TestGroups(t *testing.T) { require.Equal(t, s.Members[2].Name, "Charlie") assert.True(t, s.Members[2].OnlyInGroups) assert.Equal(t, s.Members[2].Groups, []int{0}) + require.Equal(t, s.Members[3].Name, "Dave") + assert.True(t, s.Members[3].OnlyInGroups) + assert.Equal(t, s.Members[3].Groups, []int{1}) + + // Email address is added for Dave, and an invitation can now be sent + addEmailToContact(t, inst, dave) + msg3 := job.ShareGroupMessage{ + ContactID: dave.ID(), + BecomeInvitable: true, + } + require.NoError(t, UpdateGroups(inst, msg3)) + + s = &Sharing{} + require.NoError(t, couchdb.GetDoc(inst, consts.Sharings, sid, s)) + + require.Len(t, s.Members, 4) + require.Equal(t, s.Members[0].Name, "Alice") + require.Equal(t, s.Members[1].Name, "Bob") + assert.True(t, s.Members[1].OnlyInGroups) + assert.Equal(t, s.Members[1].Groups, []int{0}) + assert.Equal(t, s.Members[1].Status, MemberStatusPendingInvitation) + require.Equal(t, s.Members[2].Name, "Charlie") + assert.True(t, s.Members[2].OnlyInGroups) + assert.Equal(t, s.Members[2].Groups, []int{0}) + assert.Equal(t, s.Members[1].Status, MemberStatusPendingInvitation) + require.Equal(t, s.Members[3].Name, "Dave") + assert.True(t, s.Members[3].OnlyInGroups) + assert.Equal(t, s.Members[3].Groups, []int{1}) + assert.Equal(t, s.Members[3].Status, MemberStatusPendingInvitation) }) } @@ -204,3 +264,32 @@ func createContactInGroups(t *testing.T, inst *instance.Instance, contactName st require.NoError(t, couchdb.CreateDoc(inst, c)) return c } + +func createContactWithoutEmail(t *testing.T, inst *instance.Instance, contactName string, groupIDs []string) *contact.Contact { + t.Helper() + + var groups []interface{} + for _, id := range groupIDs { + groups = append(groups, map[string]interface{}{ + "_id": id, + "_type": consts.Groups, + }) + } + + c := contact.New() + c.M["fullname"] = contactName + c.M["relationships"] = map[string]interface{}{ + "groups": map[string]interface{}{"data": groups}, + } + require.NoError(t, couchdb.CreateDoc(inst, c)) + return c +} + +func addEmailToContact(t *testing.T, inst *instance.Instance, c *contact.Contact) { + t.Helper() + + email := strings.ToLower(c.PrimaryName()) + "@cozy.tools" + mail := map[string]interface{}{"address": email} + c.M["email"] = []interface{}{mail} + require.NoError(t, couchdb.UpdateDoc(inst, c)) +} diff --git a/model/sharing/invitation.go b/model/sharing/invitation.go index a95abebcdb3..7e657bdcc3b 100644 --- a/model/sharing/invitation.go +++ b/model/sharing/invitation.go @@ -46,6 +46,9 @@ func (s *Sharing) SendInvitations(inst *instance.Instance, perms *permission.Per } } if m.Email == "" { + if len(m.Groups) > 0 { + return nil + } return ErrInvitationNotSent } if err := m.SendMail(inst, s, sharer, desc, link); err != nil { diff --git a/model/sharing/member.go b/model/sharing/member.go index 91c9fba356d..8217ac89f25 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -155,6 +155,9 @@ func (s *Sharing) AddContact(inst *instance.Instance, contactID string, readOnly if err != nil { return err } + if m.Email == "" && m.Instance == "" { + return contact.ErrNoMailAddress + } _, _, err = s.addMember(inst, m) return err } @@ -167,9 +170,6 @@ func buildMemberFromContact(c *contact.Contact, readOnly bool) (Member, error) { name = addr.Name email = addr.Email } else { - if cozyURL == "" { - return Member{}, err - } name = c.PrimaryName() } return Member{ @@ -191,10 +191,10 @@ func (s *Sharing) addMember(inst *instance.Instance, m Member) (string, int, err continue // Skip the owner } var found bool - if m.Email == "" { - found = m.Instance == member.Instance - } else { + if m.Email != "" { found = m.Email == member.Email + } else if m.Instance != "" { + found = m.Instance == member.Instance } if !found { continue From 0061d6bf8cb6a7ee0a957475653f7234eb65dc53 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Wed, 21 Feb 2024 14:56:00 +0100 Subject: [PATCH 14/22] Transfer the list of groups --- docs/sharing.md | 16 ++++++++++++---- model/sharing/member.go | 26 +++++++++++++++++--------- web/sharings/sharings.go | 8 +++----- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/docs/sharing.md b/docs/sharing.md index 0a881b4c9c8..eb11294c2b5 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -931,10 +931,11 @@ Content-Type: application/json ### PUT /sharings/:sharing-id/recipients -This internal route is used to update the list of members, their states, emails -and names, on the recipients cozy. The token used for this route can be the -access token for a sharing where synchronization is active, or the sharecode -for a member who has only a shortcut to the sharing on their Cozy instance. +This internal route is used to update the list of members (their states, emails +and names) and the list of groups on the recipients cozy. The token used for +this route can be the access token for a sharing where synchronization is +active, or the sharecode for a member who has only a shortcut to the sharing on +their Cozy instance. #### Request @@ -971,6 +972,13 @@ Content-Type: application/vnd.api+json "email": "dave@example.net", "read_only": true } + ], + "included": [ + { + "id": "51bbc980acb0013cb5f618c04daba326", + "name": "G. people", + "addedBy": 0 + } ] } ``` diff --git a/model/sharing/member.go b/model/sharing/member.go index 8217ac89f25..803340f9202 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -462,9 +462,9 @@ func (s *Sharing) DelegateDiscovery(inst *instance.Instance, state, cozyURL stri return success["redirect"], nil } -// UpdateRecipients updates the list of recipients -func (s *Sharing) UpdateRecipients(inst *instance.Instance, members []Member) error { - for i, m := range members { +// UpdateRecipients updates the lists of members and groups. +func (s *Sharing) UpdateRecipients(inst *instance.Instance, params PutRecipientsParams) error { + for i, m := range params.Members { if i >= len(s.Members) { s.Members = append(s.Members, Member{}) } @@ -478,6 +478,7 @@ func (s *Sharing) UpdateRecipients(inst *instance.Instance, members []Member) er s.Members[i].Status = m.Status s.Members[i].ReadOnly = m.ReadOnly } + s.Groups = params.Groups return couchdb.UpdateDoc(inst, s) } @@ -1016,6 +1017,13 @@ func (s *Sharing) NotifyMemberRevocation(inst *instance.Instance, m *Member, c * return nil } +// PutRecipientsParams is the body of the request for updating the list of +// members and groups on the active recipients of a sharing. +type PutRecipientsParams struct { + Members []Member `json:"data"` + Groups []Group `json:"included"` +} + // NotifyRecipients will push the updated list of members of the sharing to the // active recipients. It is meant to be used in a goroutine, errors are just // logged (nothing critical here). @@ -1057,12 +1065,10 @@ func (s *Sharing) NotifyRecipients(inst *instance.Instance, except *Member) { return } - var members struct { - Members []Member `json:"data"` - } - members.Members = make([]Member, len(s.Members)) + var params PutRecipientsParams + params.Members = make([]Member, len(s.Members)) for i, m := range s.Members { - members.Members[i] = Member{ + params.Members[i] = Member{ Status: m.Status, PublicName: m.PublicName, Email: m.Email, @@ -1070,7 +1076,9 @@ func (s *Sharing) NotifyRecipients(inst *instance.Instance, except *Member) { // Instance and name are private } } - body, err := json.Marshal(members) + params.Groups = make([]Group, len(s.Groups)) + copy(params.Groups, s.Groups) + body, err := json.Marshal(params) if err != nil { inst.Logger().WithNamespace("sharing"). Warnf("Can't serialize the updated members list for %s: %s", s.SID, err) diff --git a/web/sharings/sharings.go b/web/sharings/sharings.go index 5083629b2fb..1908374e31e 100644 --- a/web/sharings/sharings.go +++ b/web/sharings/sharings.go @@ -446,13 +446,11 @@ func PutRecipients(c echo.Context) error { } } - var body struct { - Members []sharing.Member `json:"data"` - } - if err = json.NewDecoder(c.Request().Body).Decode(&body); err != nil { + var params sharing.PutRecipientsParams + if err = json.NewDecoder(c.Request().Body).Decode(¶ms); err != nil { return wrapErrors(err) } - if err = s.UpdateRecipients(inst, body.Members); err != nil { + if err = s.UpdateRecipients(inst, params); err != nil { return wrapErrors(err) } return c.NoContent(http.StatusNoContent) From 5c8b0cb3270546ba11188f5c759300d58f51455a Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Wed, 21 Feb 2024 17:45:48 +0100 Subject: [PATCH 15/22] Delegate adding a group to a sharing --- docs/sharing.md | 15 ++++- model/sharing/member.go | 56 +++++++++++----- web/sharings/sharings.go | 121 +++++++++++++++++++--------------- web/sharings/sharings_test.go | 3 +- 4 files changed, 123 insertions(+), 72 deletions(-) diff --git a/docs/sharing.md b/docs/sharing.md index eb11294c2b5..606e2a7beee 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -886,9 +886,9 @@ Content-Type: application/vnd.api+json ### POST /sharings/:sharing-id/recipients/delegated This is an internal route for the stack. It is called by the recipient cozy on -the owner cozy to add recipients to the sharing (`open_sharing: true` only). It -should send an `email` address, but if the email address is not known, an -`instance` URL can also be used. +the owner cozy to add recipients and groups to the sharing (`open_sharing: +true` only). It should send an `email` address, but if the email address is not +known, an `instance` URL can also be used. #### Request @@ -910,6 +910,15 @@ Content-Type: application/vnd.api+json "email": "dave@example.net" } ] + }, + "groups": { + "data": [ + { + "id": "b57cd790b2f4013c3ced18c04daba326", + "name": "Dance", + "addedBy": 1 + } + ] } } } diff --git a/model/sharing/member.go b/model/sharing/member.go index 803340f9202..5ef3cdcaa18 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -234,6 +234,7 @@ func (s *Sharing) addMember(inst *instance.Instance, m Member) (string, int, err type APIDelegateAddContacts struct { sid string members []Member + groups []Group } // ID returns the sharing qualified identifier @@ -268,18 +269,22 @@ func (a *APIDelegateAddContacts) Relationships() jsonapi.RelationshipMap { "recipients": jsonapi.Relationship{ Data: a.members, }, + "groups": jsonapi.Relationship{ + Data: a.groups, + }, } } var _ jsonapi.Object = (*APIDelegateAddContacts)(nil) -// DelegateAddContacts adds a list of contacts on a recipient cozy. Part of -// the work is delegated to owner cozy, but the invitation mail is still sent -// from the recipient cozy. -func (s *Sharing) DelegateAddContacts(inst *instance.Instance, contactIDs map[string]bool) error { +// DelegateAddContactsAndGroups adds a list of contacts and groups on a +// recipient cozy. Part of the work is delegated to owner cozy, but the +// invitation mail is still sent from the recipient cozy. +func (s *Sharing) DelegateAddContactsAndGroups(inst *instance.Instance, groupIDs, contactIDs []string, readOnly bool) error { api := &APIDelegateAddContacts{} api.sid = s.SID - for id, ro := range contactIDs { + + for _, id := range contactIDs { c, err := contact.Find(inst, id) if err != nil { return err @@ -301,10 +306,35 @@ func (s *Sharing) DelegateAddContacts(inst *instance.Instance, contactIDs map[st Name: name, Email: email, Instance: cozyURL, - ReadOnly: ro, + ReadOnly: readOnly, } api.members = append(api.members, m) } + + for _, groupID := range groupIDs { + group, err := contact.FindGroup(inst, groupID) + if err != nil { + return err + } + g := Group{ID: groupID, Name: group.Name(), ReadOnly: readOnly} + api.groups = append(api.groups, g) + + contacts, err := group.FindContacts(inst) + if err != nil { + return err + } + groupIndex := len(s.Groups) + for _, contact := range contacts { + m, err := buildMemberFromContact(contact, readOnly) + if err != nil { + return err + } + m.Groups = []int{groupIndex} + m.OnlyInGroups = true + api.members = append(api.members, m) + } + } + data, err := jsonapi.MarshalObject(api) if err != nil { return err @@ -396,16 +426,10 @@ func (s *Sharing) DelegateAddContacts(inst *instance.Instance, contactIDs map[st // AddDelegatedContact adds a contact on the owner cozy, but for a contact from // a recipient (open_sharing: true only) -func (s *Sharing) AddDelegatedContact(inst *instance.Instance, email, instanceURL string, readOnly bool) (string, error) { - status := MemberStatusPendingInvitation - if instanceURL != "" { - status = MemberStatusMailNotSent - } - m := Member{ - Status: status, - Email: email, - Instance: instanceURL, - ReadOnly: readOnly, +func (s *Sharing) AddDelegatedContact(inst *instance.Instance, m Member) (string, error) { + m.Status = MemberStatusPendingInvitation + if m.Instance != "" || m.Email == "" { + m.Status = MemberStatusMailNotSent } state, _, err := s.addMember(inst, m) if err != nil { diff --git a/web/sharings/sharings.go b/web/sharings/sharings.go index 1908374e31e..f2c46a6c4a7 100644 --- a/web/sharings/sharings.go +++ b/web/sharings/sharings.go @@ -307,27 +307,20 @@ func ChangeCozyAddress(c echo.Context) error { func addRecipientsToSharing(inst *instance.Instance, s *sharing.Sharing, rel *jsonapi.Relationship, readOnly bool) error { var err error if data, ok := rel.Data.([]interface{}); ok { - if s.Owner { - var contactIDs, groupIDs []string - for _, ref := range data { - if id, ok := ref.(map[string]interface{})["id"].(string); ok { - if t, _ := ref.(map[string]interface{})["type"].(string); t == consts.Groups { - groupIDs = append(groupIDs, id) - } else { - contactIDs = append(contactIDs, id) - } + var contactIDs, groupIDs []string + for _, ref := range data { + if id, ok := ref.(map[string]interface{})["id"].(string); ok { + if t, _ := ref.(map[string]interface{})["type"].(string); t == consts.Groups { + groupIDs = append(groupIDs, id) + } else { + contactIDs = append(contactIDs, id) } } + } + if s.Owner { err = s.AddGroupsAndContacts(inst, groupIDs, contactIDs, readOnly) } else { - // TODO groups - ids := make(map[string]bool) - for _, ref := range data { - if id, ok := ref.(map[string]interface{})["id"].(string); ok { - ids[id] = readOnly - } - } - err = s.DelegateAddContacts(inst, ids) + err = s.DelegateAddContactsAndGroups(inst, groupIDs, contactIDs, readOnly) } } return err @@ -362,8 +355,8 @@ func AddRecipients(c echo.Context) error { return jsonapiSharingWithDocs(c, s) } -// AddRecipientsDelegated is used to add a member to a sharing on the owner's cozy -// when it's the recipient's cozy that sends the mail invitation. +// AddRecipientsDelegated is used to add members and groups to a sharing on the +// owner's cozy when it's the recipient's cozy that sends the mail invitation. func AddRecipientsDelegated(c echo.Context) error { inst := middlewares.GetInstance(c) sharingID := c.Param("sharing-id") @@ -374,50 +367,74 @@ func AddRecipientsDelegated(c echo.Context) error { if !s.Owner || !s.Open { return echo.NewHTTPError(http.StatusForbidden) } - var body sharing.Sharing - obj, err := jsonapi.Bind(c.Request().Body, &body) + member, err := requestMember(c, s) if err != nil { + return wrapErrors(err) + } + memberIndex := -1 + for i, m := range s.Members { + if m.Instance == member.Instance { + memberIndex = i + } + } + if memberIndex == -1 { + return jsonapi.InternalServerError(sharing.ErrInvalidSharing) + } + + var body struct { + Data struct { + Type string `json:"type"` + ID string `json:"id"` + Relationships struct { + Groups struct { + Data []sharing.Group `json:"data"` + } `json:"groups"` + Recipients struct { + Data []sharing.Member `json:"data"` + } `json:"recipients"` + } `json:"relationships"` + } `json:"data"` + } + if err = json.NewDecoder(c.Request().Body).Decode(&body); err != nil { return jsonapi.BadJSON() } + + for _, g := range body.Data.Relationships.Groups.Data { + g.AddedBy = memberIndex + s.Groups = append(s.Groups, g) + } + states := make(map[string]string) - if rel, ok := obj.GetRelationship("recipients"); ok { - if data, ok := rel.Data.([]interface{}); ok { - for _, ref := range data { - contact, _ := ref.(map[string]interface{}) - email, _ := contact["email"].(string) - cozy, _ := contact["instance"].(string) - ro, _ := contact["read_only"].(bool) - state, err := s.AddDelegatedContact(inst, email, cozy, ro) - if err != nil { + for _, m := range body.Data.Relationships.Recipients.Data { + state, err := s.AddDelegatedContact(inst, m) + if err != nil { + if len(m.Groups) > 0 { + continue + } + return wrapErrors(err) + } + // If we have an URL for the Cozy, we can create a shortcut as an invitation + if m.Instance != "" { + states[m.Instance] = state + var perms *permission.Permission + if s.PreviewPath != "" { + if perms, err = s.CreatePreviewPermissions(inst); err != nil { return wrapErrors(err) } - if email == "" { - states[cozy] = state - } else { - states[email] = state - } - - // If we have an URL for the Cozy, we can create a shortcut as an invitation - if cozy != "" { - var perms *permission.Permission - if s.PreviewPath != "" { - if perms, err = s.CreatePreviewPermissions(inst); err != nil { - return wrapErrors(err) - } - } - if err = s.SendInvitations(inst, perms); err != nil { - return wrapErrors(err) - } - } } - - if err := couchdb.UpdateDoc(inst, s); err != nil { + if err = s.SendInvitations(inst, perms); err != nil { return wrapErrors(err) } - cloned := s.Clone().(*sharing.Sharing) - go cloned.NotifyRecipients(inst, nil) + } else if m.Email != "" { + states[m.Email] = state } } + + if err := couchdb.UpdateDoc(inst, s); err != nil { + return wrapErrors(err) + } + cloned := s.Clone().(*sharing.Sharing) + go cloned.NotifyRecipients(inst, nil) return c.JSON(http.StatusOK, states) } diff --git a/web/sharings/sharings_test.go b/web/sharings/sharings_test.go index 797e176e4e0..ba81949c4bb 100644 --- a/web/sharings/sharings_test.go +++ b/web/sharings/sharings_test.go @@ -559,7 +559,8 @@ func TestSharings(t *testing.T) { sharingDoc, err := sharing.FindSharing(aliceInstance, sharingID) require.NoError(t, err) - _, err = sharingDoc.AddDelegatedContact(aliceInstance, newMemberMail, "", true) + m := sharing.Member{Email: newMemberMail, ReadOnly: true} + _, err = sharingDoc.AddDelegatedContact(aliceInstance, m) require.NoError(t, err) perms, err := permission.GetForSharePreview(aliceInstance, sharingID) require.NoError(t, err) From fd3dd1ccc9e39eeaab18090d4acf23d4b52f399d Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Thu, 22 Feb 2024 16:27:01 +0100 Subject: [PATCH 16/22] Delegate adding a member to a group --- model/sharing/group.go | 28 +++++++++++++++++++++++++--- model/sharing/member.go | 6 ++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/model/sharing/group.go b/model/sharing/group.go index d9926f4b84f..408b58a51aa 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -110,8 +110,14 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { for _, added := range msg.GroupsAdded { for idx, group := range s.Groups { if group.ID == added { - if err := s.AddMemberToGroup(inst, idx, contact); err != nil { - errm = multierror.Append(errm, err) + if s.Owner { + if err := s.AddMemberToGroup(inst, idx, contact); err != nil { + errm = multierror.Append(errm, err) + } + } else { + if err := s.DelegateAddMemberToGroup(inst, idx, contact); err != nil { + errm = multierror.Append(errm, err) + } } } } @@ -136,7 +142,7 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { return errm } -// AddMemberToGroup adds a contact to a sharing via a group. +// AddMemberToGroup adds a contact to a sharing via a group (on the owner). func (s *Sharing) AddMemberToGroup(inst *instance.Instance, groupIndex int, contact *contact.Contact) error { readOnly := s.Groups[groupIndex].ReadOnly m, err := buildMemberFromContact(contact, readOnly) @@ -168,6 +174,22 @@ func (s *Sharing) AddMemberToGroup(inst *instance.Instance, groupIndex int, cont return nil } +// DelegateAddMemberToGroup adds a contact to a sharing via a group (on a recipient). +func (s *Sharing) DelegateAddMemberToGroup(inst *instance.Instance, groupIndex int, contact *contact.Contact) error { + readOnly := s.Groups[groupIndex].ReadOnly + m, err := buildMemberFromContact(contact, readOnly) + if err != nil { + return err + } + m.OnlyInGroups = true + m.Groups = []int{groupIndex} + api := &APIDelegateAddContacts{ + sid: s.ID(), + members: []Member{m}, + } + return s.SendDelegated(inst, api) +} + // RemoveMemberFromGroup removes a member of a group. func (s *Sharing) RemoveMemberFromGroup(inst *instance.Instance, groupIndex int, contact *contact.Contact) error { var email string diff --git a/model/sharing/member.go b/model/sharing/member.go index 5ef3cdcaa18..cff16dcd3c3 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -335,6 +335,12 @@ func (s *Sharing) DelegateAddContactsAndGroups(inst *instance.Instance, groupIDs } } + return s.SendDelegated(inst, api) +} + +// SendDelegated calls the delegated endpoint on the sharer to adds +// contacts/groups. +func (s *Sharing) SendDelegated(inst *instance.Instance, api *APIDelegateAddContacts) error { data, err := jsonapi.MarshalObject(api) if err != nil { return err From 733fb39077207bfa65b9c4ee7f4cebe4ba80aee3 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Mon, 26 Feb 2024 15:18:27 +0100 Subject: [PATCH 17/22] Add delegation for removing a member from a sharing group --- docs/sharing.md | 18 ++++++++ model/sharing/group.go | 98 ++++++++++++++++++++++++++++++++++------ model/sharing/member.go | 14 ++---- web/sharings/sharings.go | 46 +++++++++++++++++++ 4 files changed, 152 insertions(+), 24 deletions(-) diff --git a/docs/sharing.md b/docs/sharing.md index 606e2a7beee..a411253afc9 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -938,6 +938,24 @@ Content-Type: application/json } ``` +### DELETE /sharings/:sharings-id/groups/:group-index/:member-index + +This is an internal route for the stack. It is called by the recipient cozy on +the owner cozy to remove a member of a sharing from a group. + +#### Request + +```http +DELETE /sharings/ce8835a061d0ef68947afe69a0046722/groups/0/1 HTTP/1.1 +Host: alice.example.net +``` + +#### Response + +```http +HTTP/1.1 204 No Content +``` + ### PUT /sharings/:sharing-id/recipients This internal route is used to update the list of members (their states, emails diff --git a/model/sharing/group.go b/model/sharing/group.go index 408b58a51aa..0ba4829cb38 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -1,14 +1,19 @@ package sharing import ( + "fmt" + "net/http" + "net/url" "sort" + "github.com/cozy/cozy-stack/client/request" "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/model/instance" "github.com/cozy/cozy-stack/model/job" "github.com/cozy/cozy-stack/model/permission" "github.com/cozy/cozy-stack/pkg/couchdb" multierror "github.com/hashicorp/go-multierror" + "github.com/labstack/echo/v4" ) // Group contains the information about a group of members of the sharing. @@ -34,10 +39,7 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo groupIndex := len(s.Groups) for _, contact := range contacts { - m, err := buildMemberFromContact(contact, readOnly) - if err != nil { - return err - } + m := buildMemberFromContact(contact, readOnly) m.OnlyInGroups = true _, idx, err := s.addMember(inst, m) if err != nil { @@ -125,8 +127,14 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { for _, removed := range msg.GroupsRemoved { for idx, group := range s.Groups { if group.ID == removed { - if err := s.RemoveMemberFromGroup(inst, idx, contact); err != nil { - errm = multierror.Append(errm, err) + if s.Owner { + if err := s.RemoveMemberFromGroup(inst, idx, contact); err != nil { + errm = multierror.Append(errm, err) + } + } else { + if err := s.DelegateRemoveMemberFromGroup(inst, idx, contact); err != nil { + errm = multierror.Append(errm, err) + } } } } @@ -145,10 +153,7 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { // AddMemberToGroup adds a contact to a sharing via a group (on the owner). func (s *Sharing) AddMemberToGroup(inst *instance.Instance, groupIndex int, contact *contact.Contact) error { readOnly := s.Groups[groupIndex].ReadOnly - m, err := buildMemberFromContact(contact, readOnly) - if err != nil { - return err - } + m := buildMemberFromContact(contact, readOnly) m.OnlyInGroups = true _, idx, err := s.addMember(inst, m) if err != nil { @@ -177,10 +182,7 @@ func (s *Sharing) AddMemberToGroup(inst *instance.Instance, groupIndex int, cont // DelegateAddMemberToGroup adds a contact to a sharing via a group (on a recipient). func (s *Sharing) DelegateAddMemberToGroup(inst *instance.Instance, groupIndex int, contact *contact.Contact) error { readOnly := s.Groups[groupIndex].ReadOnly - m, err := buildMemberFromContact(contact, readOnly) - if err != nil { - return err - } + m := buildMemberFromContact(contact, readOnly) m.OnlyInGroups = true m.Groups = []int{groupIndex} api := &APIDelegateAddContacts{ @@ -231,6 +233,74 @@ func (s *Sharing) RemoveMemberFromGroup(inst *instance.Instance, groupIndex int, return nil } +// DelegateRemoveMemberFromGroup removes a member from a sharing group (on a recipient). +func (s *Sharing) DelegateRemoveMemberFromGroup(inst *instance.Instance, groupIndex int, contact *contact.Contact) error { + var email string + if addr, err := contact.ToMailAddress(); err == nil { + email = addr.Email + } + cozyURL := contact.PrimaryCozyURL() + + for i, m := range s.Members { + if m.Email != "" && m.Email == email { + return s.SendRemoveMemberFromGroup(inst, groupIndex, i) + } + if m.Instance != "" && m.Instance == cozyURL { + return s.SendRemoveMemberFromGroup(inst, groupIndex, i) + } + } + return ErrMemberNotFound +} + +func (s *Sharing) SendRemoveMemberFromGroup(inst *instance.Instance, groupIndex, memberIndex int) error { + u, err := url.Parse(s.Members[0].Instance) + if err != nil { + return err + } + c := &s.Credentials[0] + if c.AccessToken == nil { + return ErrInvalidSharing + } + opts := &request.Options{ + Method: http.MethodDelete, + Scheme: u.Scheme, + Domain: u.Host, + Path: fmt.Sprintf("/sharings/%s/groups/%d/%d", s.SID, groupIndex, memberIndex), + Headers: request.Headers{ + echo.HeaderAuthorization: "Bearer " + c.AccessToken.AccessToken, + }, + ParseError: ParseRequestError, + } + res, err := request.Req(opts) + if res != nil && res.StatusCode/100 == 4 { + res, err = RefreshToken(inst, err, s, &s.Members[0], c, opts, nil) + } + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ErrInternalServerError + } + return nil +} + +func (s *Sharing) DelegatedRemoveMemberFromGroup(inst *instance.Instance, groupIndex, memberIndex int) error { + var groups []int + for _, idx := range s.Members[memberIndex].Groups { + if idx != groupIndex { + groups = append(groups, idx) + } + } + s.Members[memberIndex].Groups = groups + + if s.Members[memberIndex].OnlyInGroups && len(s.Members[memberIndex].Groups) == 0 { + return s.RevokeRecipient(inst, memberIndex) + } else { + return couchdb.UpdateDoc(inst, s) + } +} + func (s *Sharing) AddInvitationForContact(inst *instance.Instance, contact *contact.Contact) error { var email string if addr, err := contact.ToMailAddress(); err == nil { diff --git a/model/sharing/member.go b/model/sharing/member.go index cff16dcd3c3..c3daabb4885 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -151,10 +151,7 @@ func (s *Sharing) AddContact(inst *instance.Instance, contactID string, readOnly if err != nil { return err } - m, err := buildMemberFromContact(c, readOnly) - if err != nil { - return err - } + m := buildMemberFromContact(c, readOnly) if m.Email == "" && m.Instance == "" { return contact.ErrNoMailAddress } @@ -162,7 +159,7 @@ func (s *Sharing) AddContact(inst *instance.Instance, contactID string, readOnly return err } -func buildMemberFromContact(c *contact.Contact, readOnly bool) (Member, error) { +func buildMemberFromContact(c *contact.Contact, readOnly bool) Member { var name, email string cozyURL := c.PrimaryCozyURL() addr, err := c.ToMailAddress() @@ -178,7 +175,7 @@ func buildMemberFromContact(c *contact.Contact, readOnly bool) (Member, error) { Email: email, Instance: cozyURL, ReadOnly: readOnly, - }, nil + } } // addMember adds a member to the members of the sharing if they are not yet in @@ -325,10 +322,7 @@ func (s *Sharing) DelegateAddContactsAndGroups(inst *instance.Instance, groupIDs } groupIndex := len(s.Groups) for _, contact := range contacts { - m, err := buildMemberFromContact(contact, readOnly) - if err != nil { - return err - } + m := buildMemberFromContact(contact, readOnly) m.Groups = []int{groupIndex} m.OnlyInGroups = true api.members = append(api.members, m) diff --git a/web/sharings/sharings.go b/web/sharings/sharings.go index f2c46a6c4a7..f5c4d84ae78 100644 --- a/web/sharings/sharings.go +++ b/web/sharings/sharings.go @@ -438,6 +438,51 @@ func AddRecipientsDelegated(c echo.Context) error { return c.JSON(http.StatusOK, states) } +// RemoveMemberFromGroup is used to remove a member from a group (delegated). +func RemoveMemberFromGroup(c echo.Context) error { + inst := middlewares.GetInstance(c) + sharingID := c.Param("sharing-id") + s, err := sharing.FindSharing(inst, sharingID) + if err != nil { + return wrapErrors(err) + } + if !s.Owner || !s.Open { + return echo.NewHTTPError(http.StatusForbidden) + } + + member, err := requestMember(c, s) + if err != nil { + return wrapErrors(err) + } + addedBy := -1 + for i, m := range s.Members { + if m.Instance == member.Instance { + addedBy = i + } + } + if addedBy == -1 { + return jsonapi.InternalServerError(sharing.ErrInvalidSharing) + } + + groupIndex, err := strconv.Atoi(c.Param("group-index")) + if err != nil || groupIndex < 0 || groupIndex >= len(s.Groups) { + return jsonapi.InvalidParameter("group-index", errors.New("invalid group-index parameter")) + } + if s.Groups[groupIndex].AddedBy != addedBy { + return echo.NewHTTPError(http.StatusForbidden) + } + + memberIndex, err := strconv.Atoi(c.Param("member-index")) + if err != nil || memberIndex < 0 || memberIndex >= len(s.Members) { + return jsonapi.InvalidParameter("member-index", errors.New("invalid member-index parameter")) + } + + if err := s.DelegatedRemoveMemberFromGroup(inst, groupIndex, memberIndex); err != nil { + return wrapErrors(err) + } + return c.NoContent(http.StatusNoContent) +} + // PutRecipients is used to update the members list on the recipients cozy func PutRecipients(c echo.Context) error { inst := middlewares.GetInstance(c) @@ -787,6 +832,7 @@ func Routes(router *echo.Group) { // Delegated routes for open sharing router.POST("/:sharing-id/recipients/delegated", AddRecipientsDelegated, checkSharingWritePermissions) + router.DELETE("/:sharing-id/groups/:group-index/:member-index", RemoveMemberFromGroup) // Misc router.GET("/news", CountNewShortcuts) From e159e7752e7287294aed2d618175ff45ac085666 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Mon, 26 Feb 2024 17:10:56 +0100 Subject: [PATCH 18/22] Delegate sending sharing invitation --- docs/sharing.md | 39 +++++++++++++++++++- model/sharing/group.go | 78 ++++++++++++++++++++++++++++++++++++++++ web/sharings/sharings.go | 74 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 188 insertions(+), 3 deletions(-) diff --git a/docs/sharing.md b/docs/sharing.md index a411253afc9..b37f2c2edb7 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -938,7 +938,44 @@ Content-Type: application/json } ``` -### DELETE /sharings/:sharings-id/groups/:group-index/:member-index +### POST /sharings/:sharing-id/members/:index/invitation + +This is an internal route for the stack. It is called by the recipient cozy on +the owner cozy to send an invitation. + +#### Request + +```http +POST /sharings/ce8835a061d0ef68947afe69a0046722/members/4/invitation HTTP/1.1 +Host: alice.example.net +Content-Type: application/vnd.api+json +``` + +```json +{ + "data": { + "type": "io.cozy.sharings.members", + "attributes": { + "email": "diana@example.net" + } + } +} +``` + +#### Response + +```http +HTTP/1.1 200 OK +Content-Type: application/json +``` + +```json +{ + "diana@example.net": "uS6wN7fTYaLZ-GdC_P6UWA" +} +``` + +### DELETE /sharings/:sharing-id/groups/:group-index/:member-index This is an internal route for the stack. It is called by the recipient cozy on the owner cozy to remove a member of a sharing from a group. diff --git a/model/sharing/group.go b/model/sharing/group.go index 0ba4829cb38..b0edc560139 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -1,17 +1,22 @@ package sharing import ( + "bytes" + "encoding/json" "fmt" "net/http" "net/url" "sort" + "time" "github.com/cozy/cozy-stack/client/request" "github.com/cozy/cozy-stack/model/contact" "github.com/cozy/cozy-stack/model/instance" "github.com/cozy/cozy-stack/model/job" "github.com/cozy/cozy-stack/model/permission" + "github.com/cozy/cozy-stack/pkg/consts" "github.com/cozy/cozy-stack/pkg/couchdb" + "github.com/cozy/cozy-stack/pkg/jsonapi" multierror "github.com/hashicorp/go-multierror" "github.com/labstack/echo/v4" ) @@ -335,6 +340,10 @@ func (s *Sharing) AddInvitationForContact(inst *instance.Instance, contact *cont m.Instance = cozyURL s.Members[i] = m + if !s.Owner { + return s.DelegateAddInvitation(inst, i) + } + // We can ignore the error as we will try again to save the sharing // after sending the invitation. _ = couchdb.UpdateDoc(inst, s) @@ -355,3 +364,72 @@ func (s *Sharing) AddInvitationForContact(inst *instance.Instance, contact *cont return nil } + +func (s *Sharing) DelegateAddInvitation(inst *instance.Instance, memberIndex int) error { + body, err := json.Marshal(map[string]interface{}{ + "data": map[string]interface{}{ + "type": consts.SharingsMembers, + "attributes": s.Members[memberIndex], + }, + }) + if err != nil { + return err + } + u, err := url.Parse(s.Members[0].Instance) + if err != nil { + return err + } + c := &s.Credentials[0] + if c.AccessToken == nil { + return ErrInvalidSharing + } + opts := &request.Options{ + Method: http.MethodPost, + Scheme: u.Scheme, + Domain: u.Host, + Path: fmt.Sprintf("/sharings/%s/members/%d/invitation", s.ID(), memberIndex), + Headers: request.Headers{ + echo.HeaderAccept: echo.MIMEApplicationJSON, + echo.HeaderContentType: jsonapi.ContentType, + echo.HeaderAuthorization: "Bearer " + c.AccessToken.AccessToken, + }, + Body: bytes.NewReader(body), + ParseError: ParseRequestError, + } + res, err := request.Req(opts) + if res != nil && res.StatusCode/100 == 4 { + res, err = RefreshToken(inst, err, s, &s.Members[0], c, opts, body) + } + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return ErrInternalServerError + } + var states map[string]string + if err = json.NewDecoder(res.Body).Decode(&states); err != nil { + return err + } + + // We can have conflicts when updating the sharing document, so we are + // retrying when it is the case. + maxRetries := 3 + i := 0 + for { + s.Members[i].Status = MemberStatusReady + if err := couchdb.UpdateDoc(inst, s); err == nil { + break + } + i++ + if i > maxRetries { + return err + } + time.Sleep(1 * time.Second) + s, err = FindSharing(inst, s.SID) + if err != nil { + return err + } + } + return s.SendInvitationsToMembers(inst, []Member{s.Members[memberIndex]}, states) +} diff --git a/web/sharings/sharings.go b/web/sharings/sharings.go index f5c4d84ae78..cf544824e33 100644 --- a/web/sharings/sharings.go +++ b/web/sharings/sharings.go @@ -24,6 +24,7 @@ import ( "github.com/cozy/cozy-stack/pkg/config/config" "github.com/cozy/cozy-stack/pkg/consts" "github.com/cozy/cozy-stack/pkg/couchdb" + "github.com/cozy/cozy-stack/pkg/crypto" "github.com/cozy/cozy-stack/pkg/jsonapi" "github.com/cozy/cozy-stack/pkg/logger" "github.com/cozy/cozy-stack/pkg/safehttp" @@ -438,6 +439,74 @@ func AddRecipientsDelegated(c echo.Context) error { return c.JSON(http.StatusOK, states) } +// AddInvitationDelegated is when a member has been added to a sharing via a +// group, but is invited only later (no email or Cozy instance known when they +// was added). +func AddInvitationDelegated(c echo.Context) error { + inst := middlewares.GetInstance(c) + sharingID := c.Param("sharing-id") + s, err := sharing.FindSharing(inst, sharingID) + if err != nil { + return wrapErrors(err) + } + if !s.Owner || !s.Open { + return echo.NewHTTPError(http.StatusForbidden) + } + + memberIndex, err := strconv.Atoi(c.Param("member-index")) + if err != nil || memberIndex <= 0 || memberIndex >= len(s.Members) { + return jsonapi.InvalidParameter("member-index", errors.New("invalid member-index parameter")) + } + + var body struct { + Data struct { + Type string `json:"type"` + Member sharing.Member `json:"attributes"` + } + } + if err = json.NewDecoder(c.Request().Body).Decode(&body); err != nil { + return jsonapi.BadJSON() + } + + states := make(map[string]string) + m := s.Members[memberIndex] + if m.Status == sharing.MemberStatusMailNotSent { + m.Instance = body.Data.Member.Instance + m.Email = body.Data.Member.Email + state64 := crypto.Base64Encode(crypto.GenerateRandomBytes(sharing.StateLen)) + state := string(state64) + creds := sharing.Credentials{ + State: state, + XorKey: sharing.MakeXorKey(), + } + s.Credentials[memberIndex-1] = creds + s.Members[memberIndex] = m + // If we have an URL for the Cozy, we can create a shortcut as an invitation + if m.Instance != "" { + states[m.Instance] = state + var perms *permission.Permission + if s.PreviewPath != "" { + if perms, err = s.CreatePreviewPermissions(inst); err != nil { + return wrapErrors(err) + } + } + if err = s.SendInvitations(inst, perms); err != nil { + return wrapErrors(err) + } + } else if m.Email != "" { + states[m.Email] = state + s.Members[memberIndex].Status = sharing.MemberStatusReady + } + } + + if err := couchdb.UpdateDoc(inst, s); err != nil { + return wrapErrors(err) + } + cloned := s.Clone().(*sharing.Sharing) + go cloned.NotifyRecipients(inst, nil) + return c.JSON(http.StatusOK, states) +} + // RemoveMemberFromGroup is used to remove a member from a group (delegated). func RemoveMemberFromGroup(c echo.Context) error { inst := middlewares.GetInstance(c) @@ -473,7 +542,7 @@ func RemoveMemberFromGroup(c echo.Context) error { } memberIndex, err := strconv.Atoi(c.Param("member-index")) - if err != nil || memberIndex < 0 || memberIndex >= len(s.Members) { + if err != nil || memberIndex <= 0 || memberIndex >= len(s.Members) { return jsonapi.InvalidParameter("member-index", errors.New("invalid member-index parameter")) } @@ -832,7 +901,8 @@ func Routes(router *echo.Group) { // Delegated routes for open sharing router.POST("/:sharing-id/recipients/delegated", AddRecipientsDelegated, checkSharingWritePermissions) - router.DELETE("/:sharing-id/groups/:group-index/:member-index", RemoveMemberFromGroup) + router.POST("/:sharing-id/members/:index/invitation", AddInvitationDelegated, checkSharingWritePermissions) + router.DELETE("/:sharing-id/groups/:group-index/:member-index", RemoveMemberFromGroup, checkSharingWritePermissions) // Misc router.GET("/news", CountNewShortcuts) From 4fcea3687698c12ac617a55e8735881581349285 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Tue, 27 Feb 2024 10:27:08 +0100 Subject: [PATCH 19/22] Add some system tests for sharing with dynamic groups --- model/sharing/group.go | 2 +- model/sharing/sharing.go | 12 +++-- tests/system/lib/contact.rb | 12 ++++- tests/system/lib/group.rb | 26 ++++++++++ tests/system/lib/model.rb | 2 +- tests/system/lib/sharing.rb | 33 +++++++++++- tests/system/tests/revoke_sharing.rb | 6 +-- tests/system/tests/sharing_several_members.rb | 52 ++++++++++++++++++- 8 files changed, 131 insertions(+), 14 deletions(-) create mode 100644 tests/system/lib/group.rb diff --git a/model/sharing/group.go b/model/sharing/group.go index b0edc560139..510cf2e229c 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -23,7 +23,7 @@ import ( // Group contains the information about a group of members of the sharing. type Group struct { - ID string `json:"id,omitempty"` // Only present on the instance where the group was added + ID string `json:"id,omitempty"` Name string `json:"name"` AddedBy int `json:"addedBy"` // The index of the member who have added the group ReadOnly bool `json:"read_only"` diff --git a/model/sharing/sharing.go b/model/sharing/sharing.go index 8800dc9834a..cb80fd5be22 100644 --- a/model/sharing/sharing.go +++ b/model/sharing/sharing.go @@ -1394,11 +1394,13 @@ func (s *Sharing) checkSharingMembers() (checks []map[string]interface{}, validM for _, m := range s.Members { if m.Status == MemberStatusMailNotSent { - checks = append(checks, map[string]interface{}{ - "id": s.SID, - "type": "mail_not_sent", - "member": m.Instance, - }) + if len(m.Groups) == 0 { + checks = append(checks, map[string]interface{}{ + "id": s.SID, + "type": "mail_not_sent", + "member": m.Instance, + }) + } continue } diff --git a/tests/system/lib/contact.rb b/tests/system/lib/contact.rb index 19732fc95c9..003185bc8df 100644 --- a/tests/system/lib/contact.rb +++ b/tests/system/lib/contact.rb @@ -1,7 +1,7 @@ class Contact include Model - attr_reader :name, :fullname, :emails, :addresses, :phones, :cozy, :me + attr_reader :name, :fullname, :emails, :addresses, :phones, :cozy, :me, :group_ids def self.doctype "io.cozy.contacts" @@ -26,6 +26,7 @@ def initialize(opts = {}) @phones = [{ number: phone }] @cozy = opts[:cozy] @me = opts[:me] || false + @group_ids = opts[:groups] || [] end def self.from_json(j) @@ -54,7 +55,14 @@ def as_json email: @emails, cozy: @cozy, address: @addresses, - phone: @phones + phone: @phones, + relationships: { + groups: { + data: @group_ids.map do |id| + { "_id": id, "_type": Group.doctype } + end + } + } } end end diff --git a/tests/system/lib/group.rb b/tests/system/lib/group.rb new file mode 100644 index 00000000000..f803d06ea58 --- /dev/null +++ b/tests/system/lib/group.rb @@ -0,0 +1,26 @@ +class Group + include Model + + attr_reader :name + + def self.doctype + "io.cozy.contacts.groups" + end + + def initialize(opts = {}) + @name = opts[:name] || Faker::Educator.subject + end + + def self.from_json(j) + group = Group.new(name: j["name"]) + group.couch_id = j["_id"] + group.couch_rev = j["_rev"] + group + end + + def as_json + { + name: @name + } + end +end diff --git a/tests/system/lib/model.rb b/tests/system/lib/model.rb index f387405aeaf..4dffaf2b5a8 100644 --- a/tests/system/lib/model.rb +++ b/tests/system/lib/model.rb @@ -44,7 +44,7 @@ def to_json(*_args) def as_reference { - doctype: doctype, + type: doctype, id: @couch_id } end diff --git a/tests/system/lib/sharing.rb b/tests/system/lib/sharing.rb index 0ccf4543c1e..1266d416d1d 100644 --- a/tests/system/lib/sharing.rb +++ b/tests/system/lib/sharing.rb @@ -49,7 +49,8 @@ def self.get_shared_docs(inst, sharing_id, doctype) j.dig "relationships", "shared_docs", "data" end - def add_members(inst, contacts, doctype) + def add_members(inst, contacts) + doctype = @rules[0].doctype opts = { accept: "application/vnd.api+json", content_type: "application/vnd.api+json", @@ -69,6 +70,36 @@ def add_members(inst, contacts, doctype) res.code end + def add_group(inst, group) + doctype = @rules[0].doctype + opts = { + accept: "application/vnd.api+json", + content_type: "application/vnd.api+json", + authorization: "Bearer #{inst.token_for doctype}" + } + data = { + data: { + relationships: { + recipients: { + data: [group.as_reference] + } + } + } + } + body = JSON.generate data + res = inst.client["/sharings/#{@couch_id}/recipients"].post body, opts + res.code + end + + def remove_group(inst, index) + doctype = @rules[0].doctype + opts = { + authorization: "Bearer #{inst.token_for doctype}" + } + res = inst.client["/sharings/#{@couch_id}/groups/#{index}"].delete opts + res.code + end + def read_only(inst, index) opts = { authorization: "Bearer #{inst.token_for Folder.doctype}" diff --git a/tests/system/tests/revoke_sharing.rb b/tests/system/tests/revoke_sharing.rb index e729c37eb67..fb69b5afd73 100644 --- a/tests/system/tests/revoke_sharing.rb +++ b/tests/system/tests/revoke_sharing.rb @@ -179,14 +179,14 @@ def assert_recipient_revoked(inst, sharing_id, index) sleep 3 # Add Charlie, Dave, and Emily to the sharing - code = sharing.add_members inst_alice, [contact_charlie], Folder.doctype + code = sharing.add_members inst_alice, [contact_charlie] assert_equal 200, code sleep 1 inst_charlie.accept sharing sleep 6 - code = sharing.add_members inst_bob, [contact_dave], Folder.doctype + code = sharing.add_members inst_bob, [contact_dave] assert_equal 200, code - code = sharing.add_members inst_bob, [contact_emily], Folder.doctype + code = sharing.add_members inst_bob, [contact_emily] assert_equal 200, code sleep 1 inst_dave.accept sharing, inst_bob diff --git a/tests/system/tests/sharing_several_members.rb b/tests/system/tests/sharing_several_members.rb index f4116a41f90..b73abfb963a 100644 --- a/tests/system/tests/sharing_several_members.rb +++ b/tests/system/tests/sharing_several_members.rb @@ -62,7 +62,38 @@ sleep 6 f2_bob.remove inst_bob - sleep 21 + sleep 10 + + # Check that we can add a group from the owner + g1 = Group.create inst, name: Faker::Kpop.girl_groups + contact_gaby = Contact.create inst, given_name: "Gaby", groups: [g1.couch_id] + sleep 1 + sharing.add_group inst, g1 + sleep 3 + info = Sharing.get_sharing_info inst, sharing.couch_id, Folder.doctype + members = [contact_bob, contact_charlie, contact_dave, contact_emily, contact_gaby] + revoked = [] + check_sharing_has_groups_and_members info, [g1], members, revoked + + # Check that we can add a group from a recipient + g2 = Group.create inst_bob, name: Faker::Kpop.boy_bands + contact_hugo = Contact.create inst_bob, given_name: "Hugo", groups: [g2.couch_id] + sleep 1 + sharing.add_group inst_bob, g2 + sleep 3 + info = Sharing.get_sharing_info inst, sharing.couch_id, Folder.doctype + members = [contact_bob, contact_charlie, contact_dave, contact_emily, contact_gaby, contact_hugo] + revoked = [] + check_sharing_has_groups_and_members info, [g1, g2], members, revoked + + # Check that we can remove a group + sharing.remove_group inst, 0 + sleep 4 + info = Sharing.get_sharing_info inst, sharing.couch_id, Folder.doctype + members = [contact_bob, contact_charlie, contact_dave, contact_emily, contact_gaby, contact_hugo] + revoked = [5] + assert info.dig("attributes", "groups", 0, "removed") + check_sharing_has_groups_and_members info, [g1, g2], members, revoked # Check that the files are the same on disk da = File.join Helpers.current_dir, inst.domain, folder.name @@ -139,3 +170,22 @@ inst_fred.remove end end + +def check_sharing_has_groups_and_members(info, groups, contacts, revoked) + grps = info.dig("attributes", "groups") || [] + assert_equal grps.length, groups.length + groups.each_with_index do |g, i| + assert_equal grps[i]["name"], g.name + end + + members = info.dig "attributes", "members" + # We have the owner in members but not in contacts + assert_equal members.length, contacts.length + 1 + contacts.each_with_index do |contact, i| + assert_equal members[i+1]["name"], contact.fullname + end + + revoked.each do |i| + assert_equal members[i]["status"], "revoked" + end +end From 854a9850e7c084d9e4e24bcb11a075b5de74b406 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Tue, 27 Feb 2024 10:35:55 +0100 Subject: [PATCH 20/22] Fix share-group worker for deleted contact --- model/job/trigger_share_group.go | 23 +++++++++++++------ model/sharing/group.go | 22 +++++++++++------- tests/system/tests/sharing_several_members.rb | 10 +++++++- 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/model/job/trigger_share_group.go b/model/job/trigger_share_group.go index 56c5cde484e..26ac949e7ea 100644 --- a/model/job/trigger_share_group.go +++ b/model/job/trigger_share_group.go @@ -16,10 +16,11 @@ type ShareGroupTrigger struct { // ShareGroupMessage is used for jobs on the share-group worker. type ShareGroupMessage struct { - ContactID string `json:"contact_id"` - GroupsAdded []string `json:"added"` - GroupsRemoved []string `json:"removed"` - BecomeInvitable bool `json:"invitable"` + ContactID string `json:"contact_id"` + GroupsAdded []string `json:"added"` + GroupsRemoved []string `json:"removed"` + BecomeInvitable bool `json:"invitable"` + DeletedDoc *couchdb.JSONDoc `json:"deleted_doc,omitempty"` } func NewShareGroupTrigger(broker Broker) *ShareGroupTrigger { @@ -58,11 +59,15 @@ func (t *ShareGroupTrigger) match(e *realtime.Event) *ShareGroupMessage { return nil } newContact := &contact.Contact{JSONDoc: *newdoc} - newgroups := newContact.GroupIDs() + var newgroups []string + if e.Verb != realtime.EventDelete { + newgroups = newContact.GroupIDs() + } var oldgroups []string invitable := false - if olddoc, ok := e.OldDoc.(*couchdb.JSONDoc); ok { + olddoc, ok := e.OldDoc.(*couchdb.JSONDoc) + if ok { oldContact := &contact.Contact{JSONDoc: *olddoc} oldgroups = oldContact.GroupIDs() invitable = contactIsNowInvitable(oldContact, newContact) @@ -75,12 +80,16 @@ func (t *ShareGroupTrigger) match(e *realtime.Event) *ShareGroupMessage { return nil } - return &ShareGroupMessage{ + msg := &ShareGroupMessage{ ContactID: e.Doc.ID(), GroupsAdded: added, GroupsRemoved: removed, BecomeInvitable: invitable, } + if e.Verb == realtime.EventDelete { + msg.DeletedDoc = olddoc + } + return msg } func diffGroupIDs(as, bs []string) []string { diff --git a/model/sharing/group.go b/model/sharing/group.go index 510cf2e229c..aea65342b5a 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -102,9 +102,15 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { // finds the sharings for this group, and adds or removes the member to those // sharings. func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { - contact, err := contact.Find(inst, msg.ContactID) - if err != nil { - return err + var c *contact.Contact + if msg.DeletedDoc != nil { + c = &contact.Contact{JSONDoc: *msg.DeletedDoc} + } else { + doc, err := contact.Find(inst, msg.ContactID) + if err != nil { + return err + } + c = doc } sharings, err := FindActive(inst) @@ -118,11 +124,11 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { for idx, group := range s.Groups { if group.ID == added { if s.Owner { - if err := s.AddMemberToGroup(inst, idx, contact); err != nil { + if err := s.AddMemberToGroup(inst, idx, c); err != nil { errm = multierror.Append(errm, err) } } else { - if err := s.DelegateAddMemberToGroup(inst, idx, contact); err != nil { + if err := s.DelegateAddMemberToGroup(inst, idx, c); err != nil { errm = multierror.Append(errm, err) } } @@ -133,11 +139,11 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { for idx, group := range s.Groups { if group.ID == removed { if s.Owner { - if err := s.RemoveMemberFromGroup(inst, idx, contact); err != nil { + if err := s.RemoveMemberFromGroup(inst, idx, c); err != nil { errm = multierror.Append(errm, err) } } else { - if err := s.DelegateRemoveMemberFromGroup(inst, idx, contact); err != nil { + if err := s.DelegateRemoveMemberFromGroup(inst, idx, c); err != nil { errm = multierror.Append(errm, err) } } @@ -146,7 +152,7 @@ func UpdateGroups(inst *instance.Instance, msg job.ShareGroupMessage) error { } if msg.BecomeInvitable { - if err := s.AddInvitationForContact(inst, contact); err != nil { + if err := s.AddInvitationForContact(inst, c); err != nil { errm = multierror.Append(errm, err) } } diff --git a/tests/system/tests/sharing_several_members.rb b/tests/system/tests/sharing_several_members.rb index b73abfb963a..521409487b1 100644 --- a/tests/system/tests/sharing_several_members.rb +++ b/tests/system/tests/sharing_several_members.rb @@ -62,7 +62,7 @@ sleep 6 f2_bob.remove inst_bob - sleep 10 + sleep 6 # Check that we can add a group from the owner g1 = Group.create inst, name: Faker::Kpop.girl_groups @@ -86,6 +86,14 @@ revoked = [] check_sharing_has_groups_and_members info, [g1, g2], members, revoked + # Check that we can remove a member of a group + contact_hugo.delete inst_bob + sleep 4 + info = Sharing.get_sharing_info inst, sharing.couch_id, Folder.doctype + members = [contact_bob, contact_charlie, contact_dave, contact_emily, contact_gaby, contact_hugo] + revoked = [6] + check_sharing_has_groups_and_members info, [g1, g2], members, revoked + # Check that we can remove a group sharing.remove_group inst, 0 sleep 4 From e14c54c686c45b1803df0384e3df0b69d4d884e5 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Thu, 29 Feb 2024 10:31:19 +0100 Subject: [PATCH 21/22] Update code from review --- cmd/instances.go | 2 +- docs/cli/cozy-stack_instances_add.md | 2 +- docs/sharing.md | 6 +++--- docs/workers.md | 6 +++--- model/contact/contact_test.go | 4 ++-- model/contact/contacts.go | 6 +++--- model/contact/group.go | 8 ++++---- model/sharing/group.go | 23 +++++++++++++++-------- model/sharing/member.go | 6 +++--- model/sharing/sharing.go | 2 +- 10 files changed, 36 insertions(+), 29 deletions(-) diff --git a/cmd/instances.go b/cmd/instances.go index 0dd3af6ce63..abf01f18c99 100644 --- a/cmd/instances.go +++ b/cmd/instances.go @@ -152,7 +152,7 @@ cozy-stack instances add allows to create an instance on the cozy for a given domain. If the COZY_DISABLE_INSTANCES_ADD_RM env variable is set, creating and -destroying instances will be desactivated and the content of this variable will +destroying instances will be disabled and the content of this variable will be used as the error message. `, Example: "$ cozy-stack instances add --passphrase cozy --apps drive,photos,settings,home,store cozy.localhost:8080", diff --git a/docs/cli/cozy-stack_instances_add.md b/docs/cli/cozy-stack_instances_add.md index 490d8acd669..119e09a4be7 100644 --- a/docs/cli/cozy-stack_instances_add.md +++ b/docs/cli/cozy-stack_instances_add.md @@ -9,7 +9,7 @@ cozy-stack instances add allows to create an instance on the cozy for a given domain. If the COZY_DISABLE_INSTANCES_ADD_RM env variable is set, creating and -destroying instances will be desactivated and the content of this variable will +destroying instances will be disabled and the content of this variable will be used as the error message. diff --git a/docs/sharing.md b/docs/sharing.md index b37f2c2edb7..c393a4e39bf 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -887,8 +887,8 @@ Content-Type: application/vnd.api+json This is an internal route for the stack. It is called by the recipient cozy on the owner cozy to add recipients and groups to the sharing (`open_sharing: -true` only). It should send an `email` address, but if the email address is not -known, an `instance` URL can also be used. +true` only). Data for direct recipients should contain an email address but if +it is not known, an instance URL can also be provided. #### Request @@ -1233,7 +1233,7 @@ This route can be only be called on the cozy instance of the sharer to revoke a group of the sharing. The parameter is the index of this recipient in the `groups` array of the sharing. The `removed` property for this group will be set to `true`, and it will revoke the members of this group unless they are -still part of the sharing via another group. +still part of the sharing via another group or as direct recipients. #### Request diff --git a/docs/workers.md b/docs/workers.md index 86849cfaa80..91e3e4a0171 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -352,9 +352,9 @@ The stack have 4 workers to power the sharings (internal usage only): ### Share-group -When a contact is added or removed to a group, they should be added to the -sharings of this group. The message is composed of the contact ID, the list -of groups added and the list of groups removed. +When a contact is added to or removed from a group, the change should be +reflected in the group's sharings' recipients. The message is composed of the +contact ID, the list of groups added and the list of groups removed. ### Share-track diff --git a/model/contact/contact_test.go b/model/contact/contact_test.go index e30d5e87835..b1238779ec7 100644 --- a/model/contact/contact_test.go +++ b/model/contact/contact_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestFindContacts(t *testing.T) { +func TestGetAllContacts(t *testing.T) { config.UseTestFile(t) instPrefix := prefixer.NewPrefixer(0, "cozy-test", "cozy-test") t.Cleanup(func() { _ = couchdb.DeleteDB(instPrefix, consts.Contacts) }) @@ -76,7 +76,7 @@ func TestFindContacts(t *testing.T) { require.NoError(t, json.Unmarshal([]byte(gaby), &doc.M)) require.NoError(t, couchdb.CreateDoc(instPrefix, &doc)) - contacts, err := g.FindContacts(instPrefix) + contacts, err := g.GetAllContacts(instPrefix) require.NoError(t, err) require.Len(t, contacts, 1) assert.Equal(t, contacts[0].PrimaryName(), "Gaby") diff --git a/model/contact/contacts.go b/model/contact/contacts.go index f1e5914dbe9..9c27a92d358 100644 --- a/model/contact/contacts.go +++ b/model/contact/contacts.go @@ -84,9 +84,9 @@ func (c *Contact) PrimaryName() string { return primary } -// ByFamilyNameGivenNameEmailCozyURL returns a string that can be used for -// sorting the contacts like in the contacts app. -func (c *Contact) ByFamilyNameGivenNameEmailCozyURL() string { +// SortingKey returns a string that can be used for sorting the contacts like +// in the contacts app. +func (c *Contact) SortingKey() string { indexes, ok := c.Get("indexes").(map[string]interface{}) if !ok { return c.PrimaryName() diff --git a/model/contact/group.go b/model/contact/group.go index 9d734a63538..ee2153d9142 100644 --- a/model/contact/group.go +++ b/model/contact/group.go @@ -39,8 +39,8 @@ func FindGroup(db prefixer.Prefixer, groupID string) (*Group, error) { return doc, err } -// FindContacts returns the list of contacts inside this group. -func (g *Group) FindContacts(db prefixer.Prefixer) ([]*Contact, error) { +// GetAllContacts returns the list of contacts inside this group. +func (g *Group) GetAllContacts(db prefixer.Prefixer) ([]*Contact, error) { var docs []*Contact req := &couchdb.FindRequest{ UseIndex: "by-groups", @@ -65,8 +65,8 @@ func (g *Group) FindContacts(db prefixer.Prefixer) ([]*Contact, error) { // XXX I didn't find a way to make a mango request with the correct sort less := func(i, j int) bool { - a := docs[i].ByFamilyNameGivenNameEmailCozyURL() - b := docs[j].ByFamilyNameGivenNameEmailCozyURL() + a := docs[i].SortingKey() + b := docs[j].SortingKey() return a < b } sort.Slice(docs, less) diff --git a/model/sharing/group.go b/model/sharing/group.go index aea65342b5a..ec6f2b299c4 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -25,7 +25,7 @@ import ( type Group struct { ID string `json:"id,omitempty"` Name string `json:"name"` - AddedBy int `json:"addedBy"` // The index of the member who have added the group + AddedBy int `json:"addedBy"` // The index of the member who added the group ReadOnly bool `json:"read_only"` Removed bool `json:"removed,omitempty"` } @@ -37,7 +37,7 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo if err != nil { return err } - contacts, err := group.FindContacts(inst) + contacts, err := group.GetAllContacts(inst) if err != nil { return err } @@ -50,8 +50,11 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo if err != nil { return err } - s.Members[idx].Groups = append(s.Members[idx].Groups, groupIndex) - sort.Ints(s.Members[idx].Groups) + pos := sort.SearchInts(s.Members[idx].Groups, groupIndex) + if pos == len(s.Members[idx].Groups) || s.Members[idx].Groups[pos] != groupIndex { + s.Members[idx].Groups = append(s.Members[idx].Groups, groupIndex) + sort.Ints(s.Members[idx].Groups) + } } g := Group{ID: groupID, Name: group.Name(), AddedBy: 0, ReadOnly: readOnly} @@ -59,13 +62,14 @@ func (s *Sharing) AddGroup(inst *instance.Instance, groupID string, readOnly boo return nil } -// RevokeGroup revoke a group of members on the sharer Cozy. After that, the -// sharing is desactivated if there are no longer any active recipient. +// RevokeGroup revokes a group of members on the sharer Cozy. After that, the +// sharing is disabled if there are no longer any active recipient. func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { if !s.Owner { return ErrInvalidSharing } + var errm error for i, m := range s.Members { inGroup := false for _, idx := range m.Groups { @@ -89,13 +93,16 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { } if m.OnlyInGroups && len(s.Members[i].Groups) == 0 { if err := s.RevokeRecipient(inst, i); err != nil { - return err + errm = multierror.Append(errm, err) } } } s.Groups[index].Removed = true - return couchdb.UpdateDoc(inst, s) + if err := couchdb.UpdateDoc(inst, s); err != nil { + errm = multierror.Append(errm, err) + } + return errm } // UpdateGroups is called when a contact is added or removed to a group. It diff --git a/model/sharing/member.go b/model/sharing/member.go index c3daabb4885..0fdb0104b84 100644 --- a/model/sharing/member.go +++ b/model/sharing/member.go @@ -66,8 +66,8 @@ type Member struct { Email string `json:"email,omitempty"` Instance string `json:"instance,omitempty"` ReadOnly bool `json:"read_only,omitempty"` - OnlyInGroups bool `json:"only_in_groups,omitempty"` // False if the member as been added as an io.cozy.contacts - Groups []int `json:"groups,omitempty"` // The indexes of the groups + OnlyInGroups bool `json:"only_in_groups,omitempty"` // False if the member has been added as an io.cozy.contacts + Groups []int `json:"groups,omitempty"` // The indexes of the groups a member is part of } // PrimaryName returns the main name of this member @@ -316,7 +316,7 @@ func (s *Sharing) DelegateAddContactsAndGroups(inst *instance.Instance, groupIDs g := Group{ID: groupID, Name: group.Name(), ReadOnly: readOnly} api.groups = append(api.groups, g) - contacts, err := group.FindContacts(inst) + contacts, err := group.GetAllContacts(inst) if err != nil { return err } diff --git a/model/sharing/sharing.go b/model/sharing/sharing.go index cb80fd5be22..fee6de6fef1 100644 --- a/model/sharing/sharing.go +++ b/model/sharing/sharing.go @@ -485,7 +485,7 @@ func (s *Sharing) RevokePreviewPermissions(inst *instance.Instance) error { // RevokeRecipient revoke only one recipient on the sharer. After that, if the // sharing has still at least one active member, we keep it as is. Else, we -// desactivate the sharing. +// disable the sharing. func (s *Sharing) RevokeRecipient(inst *instance.Instance, index int) error { if !s.Owner { return ErrInvalidSharing From b03aa64c81658bea8763a11b0dde27a03a1903dd Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Tue, 5 Mar 2024 14:15:43 +0100 Subject: [PATCH 22/22] Rename the Removed field of sharing.Group to Revoked --- docs/sharing-design.md | 31 ++++++++++--------- model/sharing/group.go | 4 +-- model/sharing/group_test.go | 16 +++++----- tests/system/tests/sharing_several_members.rb | 2 +- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/docs/sharing-design.md b/docs/sharing-design.md index bd6bcd10c67..e82c463e882 100644 --- a/docs/sharing-design.md +++ b/docs/sharing-design.md @@ -475,17 +475,17 @@ care of it later. - An identifier (the same for all members of the sharing) - A list of `members`. The first one is the owner. For each member, we have: - `status`, a status that can be: - - `owner` for the member that has created the sharing - - `mail-not-sent` for a member that has been added, but its - invitation has not yet been sent (often, this status is used only - for a few seconds) - - `pending` for a member with an invitation sent, but who has not - clicked on the link - - `seen` for a member that has clicked on the invitation link, but - has not setup the Cozy to Cozy replication for the sharing - - `ready` for a member where the Cozy to Cozy replication has been - set up - - `revoked` for a member who is on longer in the sharing + - `owner` for the member that has created the sharing + - `mail-not-sent` for a member that has been added, but its + invitation has not yet been sent (often, this status is used only + for a few seconds) + - `pending` for a member with an invitation sent, but who has not + clicked on the link + - `seen` for a member that has clicked on the invitation link, but + has not setup the Cozy to Cozy replication for the sharing + - `ready` for a member where the Cozy to Cozy replication has been + set up + - `revoked` for a member who is on longer in the sharing - `name`, a contact name - `public_name`, a public name - `email`, the email address @@ -495,10 +495,11 @@ care of it later. as a single contact - `groups`, a list of indexes of the groups - A list of `groups`, with for each one: - - `id`, the identifier of the io.cozy.contacts.groups - - `name`, the name of the group - - `addedBy`, the index of the member that has added the group - - `removed`, a flag set to true when the group is revoked from the sharing + - `id`, the identifier of the io.cozy.contacts.groups + - `name`, the name of the group + - `addedBy`, the index of the member that has added the group + - `read_only`, a flag to tell if the group is restricted to read-only mode + - `revoked`, a flag set to true when the group is revoked from the sharing - Some `credentials` to authorize the transfer of data between the owner and the recipients - A `description` (one sentence that will help people understand what is diff --git a/model/sharing/group.go b/model/sharing/group.go index ec6f2b299c4..a0c34b97956 100644 --- a/model/sharing/group.go +++ b/model/sharing/group.go @@ -27,7 +27,7 @@ type Group struct { Name string `json:"name"` AddedBy int `json:"addedBy"` // The index of the member who added the group ReadOnly bool `json:"read_only"` - Removed bool `json:"removed,omitempty"` + Revoked bool `json:"revoked,omitempty"` } // AddGroup adds a group of contacts identified by its ID to the members of the @@ -98,7 +98,7 @@ func (s *Sharing) RevokeGroup(inst *instance.Instance, index int) error { } } - s.Groups[index].Removed = true + s.Groups[index].Revoked = true if err := couchdb.UpdateDoc(inst, s); err != nil { errm = multierror.Append(errm, err) } diff --git a/model/sharing/group_test.go b/model/sharing/group_test.go index 1576029589a..90d1e966aa5 100644 --- a/model/sharing/group_test.go +++ b/model/sharing/group_test.go @@ -76,9 +76,9 @@ func TestGroups(t *testing.T) { require.Len(t, s.Groups, 2) require.Equal(t, s.Groups[0].Name, "Friends") - assert.False(t, s.Groups[0].Removed) + assert.False(t, s.Groups[0].Revoked) require.Equal(t, s.Groups[1].Name, "Football") - assert.False(t, s.Groups[1].Removed) + assert.False(t, s.Groups[1].Revoked) require.NoError(t, s.RevokeGroup(inst, 1)) // Revoke the football group @@ -91,8 +91,8 @@ func TestGroups(t *testing.T) { assert.Empty(t, s.Members[3].Groups) require.Len(t, s.Groups, 2) - assert.False(t, s.Groups[0].Removed) - assert.True(t, s.Groups[1].Removed) + assert.False(t, s.Groups[0].Revoked) + assert.True(t, s.Groups[1].Revoked) require.NoError(t, s.RevokeGroup(inst, 0)) // Revoke the fiends group @@ -105,8 +105,8 @@ func TestGroups(t *testing.T) { assert.Empty(t, s.Members[3].Groups) require.Len(t, s.Groups, 2) - assert.True(t, s.Groups[0].Removed) - assert.True(t, s.Groups[1].Removed) + assert.True(t, s.Groups[0].Revoked) + assert.True(t, s.Groups[1].Revoked) }) t.Run("UpdateGroups", func(t *testing.T) { @@ -158,9 +158,9 @@ func TestGroups(t *testing.T) { require.Len(t, s.Groups, 2) require.Equal(t, s.Groups[0].Name, "Friends") - assert.False(t, s.Groups[0].Removed) + assert.False(t, s.Groups[0].Revoked) require.Equal(t, s.Groups[1].Name, "Football") - assert.False(t, s.Groups[1].Removed) + assert.False(t, s.Groups[1].Revoked) // Charlie is added to the friends group msg1 := job.ShareGroupMessage{ diff --git a/tests/system/tests/sharing_several_members.rb b/tests/system/tests/sharing_several_members.rb index 521409487b1..113a80a1e53 100644 --- a/tests/system/tests/sharing_several_members.rb +++ b/tests/system/tests/sharing_several_members.rb @@ -100,7 +100,7 @@ info = Sharing.get_sharing_info inst, sharing.couch_id, Folder.doctype members = [contact_bob, contact_charlie, contact_dave, contact_emily, contact_gaby, contact_hugo] revoked = [5] - assert info.dig("attributes", "groups", 0, "removed") + assert info.dig("attributes", "groups", 0, "revoked") check_sharing_has_groups_and_members info, [g1, g2], members, revoked # Check that the files are the same on disk