Add tests for RevokeGroup

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

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: "[email protected]"},
+			},
+			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
+}

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{}