Shares improvements

changelog/unreleased/spaces-shares.md

@@ -0,0 +1,7 @@
+Enhancement: Enable space members to list shares inside the space
+
+If there are shared resources in a space then all members are allowed to see those shares.
+The json share manager was enhanced to check if the user is allowed to see a share by checking the grants on a resource.
+
+https://github.com/owncloud/ocis/issues/3370
+https://github.com/cs3org/reva/pull/2674

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go

@@ -95,11 +95,16 @@ func (h *Handler) addSpaceMember(w http.ResponseWriter, r *http.Request, info *p
 		return
 	}
 
+	permissions := role.CS3ResourcePermissions()
+	// All members of a space should be able to list shares inside that space.
+	// The viewer role doesn't have the ListGrants permission so we set it here.
+	permissions.ListGrants = true
+
 	createShareRes, err := client.CreateShare(ctx, &collaborationv1beta1.CreateShareRequest{
 		ResourceInfo: info,
 		Grant: &collaborationv1beta1.ShareGrant{
 			Permissions: &collaborationv1beta1.SharePermissions{
-				Permissions: role.CS3ResourcePermissions(),
+				Permissions: permissions,
 			},
 			Grantee: &grantee,
 		},

pkg/publicshare/manager/cs3/cs3.go

@@ -267,7 +267,9 @@ func (m *Manager) getWithPassword(ctx context.Context, ref *link.PublicShareRefe
 }
 
 func (m *Manager) getByID(ctx context.Context, id string) (*PublicShareWithPassword, error) {
-	tokens, err := m.indexer.FindBy(&link.PublicShare{}, "Id.OpaqueId", id)
+	tokens, err := m.indexer.FindBy(&link.PublicShare{},
+		indexer.NewField("Id.OpaqueId", id),
+	)
 	if err != nil {
 		return nil, err
 	}
@@ -298,7 +300,9 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters []
 		return nil, err
 	}
 
-	tokens, err := m.indexer.FindBy(&link.PublicShare{}, "Owner", userIDToIndex(u.Id))
+	tokens, err := m.indexer.FindBy(&link.PublicShare{},
+		indexer.NewField("Owner", userIDToIndex(u.Id)),
+	)
 	if err != nil {
 		return nil, err
 	}

pkg/share/manager/cs3/cs3.go

@@ -24,14 +24,18 @@ import (
 	"fmt"
 	"net/url"
 	"path"
+	"strings"
 	"sync"
 
+	gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	groupv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
+	rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
+	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/v2/pkg/share"
 	"github.com/cs3org/reva/v2/pkg/share/manager/registry"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/indexer"
@@ -46,8 +50,9 @@ import (
 
 // Manager implements a share manager using a cs3 storage backend
 type Manager struct {
-	sync.RWMutex
+	gatewayClient gatewayv1beta1.GatewayAPIClient
 
+	sync.RWMutex
 	storage metadata.Storage
 	indexer indexer.Indexer
 
@@ -86,15 +91,21 @@ func NewDefault(m map[string]interface{}) (share.Manager, error) {
 	}
 	indexer := indexer.CreateIndexer(s)
 
-	return New(s, indexer)
+	client, err := pool.GetGatewayServiceClient(c.GatewayAddr)
+	if err != nil {
+		return nil, err
+	}
+
+	return New(client, s, indexer)
 }
 
 // New returns a new manager instance
-func New(s metadata.Storage, indexer indexer.Indexer) (*Manager, error) {
+func New(gatewayClient gatewayv1beta1.GatewayAPIClient, s metadata.Storage, indexer indexer.Indexer) (*Manager, error) {
 	return &Manager{
-		storage:     s,
-		indexer:     indexer,
-		initialized: false,
+		gatewayClient: gatewayClient,
+		storage:       s,
+		indexer:       indexer,
+		initialized:   false,
 	}, nil
 }
 
@@ -127,13 +138,27 @@ func (m *Manager) initialize() error {
 	if err != nil {
 		return err
 	}
+	err = m.indexer.AddIndex(&collaboration.Share{}, option.IndexByFunc{
+		Name: "CreatorId",
+		Func: indexCreatorFunc,
+	}, "Id.OpaqueId", "shares", "non_unique", nil, true)
+	if err != nil {
+		return err
+	}
 	err = m.indexer.AddIndex(&collaboration.Share{}, option.IndexByFunc{
 		Name: "GranteeId",
 		Func: indexGranteeFunc,
 	}, "Id.OpaqueId", "shares", "non_unique", nil, true)
 	if err != nil {
 		return err
 	}
+	err = m.indexer.AddIndex(&collaboration.Share{}, option.IndexByFunc{
+		Name: "ResourceId",
+		Func: indexResourceIDFunc,
+	}, "Id.OpaqueId", "shares", "non_unique", nil, true)
+	if err != nil {
+		return err
+	}
 	m.initialized = true
 	return nil
 }
@@ -227,20 +252,90 @@ func (m *Manager) ListShares(ctx context.Context, filters []*collaboration.Filte
 		return nil, errtypes.UserRequired("error getting user from context")
 	}
 
-	allShareIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIDToIndex(user.GetId()))
+	createdShareIds, err := m.indexer.FindBy(&collaboration.Share{},
+		indexer.NewField("OwnerId", userIDToIndex(user.Id)),
+		indexer.NewField("CreatorId", userIDToIndex(user.Id)),
+	)
 	if err != nil {
 		return nil, err
 	}
+
+	// We use shareMem as a temporary lookup store to check which shares were
+	// already added. This is to prevent duplicates.
+	shareMem := make(map[string]struct{})
 	result := []*collaboration.Share{}
-	for _, id := range allShareIds {
+	for _, id := range createdShareIds {
 		s, err := m.getShareByID(ctx, id)
 		if err != nil {
 			return nil, err
 		}
 		if share.MatchesFilters(s, filters) {
 			result = append(result, s)
+			shareMem[s.Id.OpaqueId] = struct{}{}
+		}
+	}
+
+	// If a user requests to list shares which have not been created by them
+	// we have to explicitly fetch these shares and check if the user is
+	// allowed to list the shares.
+	// Only then can we add these shares to the result.
+	grouped := share.GroupFiltersByType(filters)
+	idFilter, ok := grouped[collaboration.Filter_TYPE_RESOURCE_ID]
+	if !ok {
+		return result, nil
+	}
+
+	shareIDsByResourceID := make(map[string]*provider.ResourceId)
+	for _, filter := range idFilter {
+		resourceID := filter.GetResourceId()
+		shareIDs, err := m.indexer.FindBy(&collaboration.Share{},
+			indexer.NewField("ResourceId", resourceIDToIndex(resourceID)),
+		)
+		if err != nil {
+			continue
+		}
+		for _, shareID := range shareIDs {
+			shareIDsByResourceID[shareID] = resourceID
+		}
+	}
+
+	// statMem is used as a local cache to prevent statting resources which
+	// already have been checked.
+	statMem := make(map[string]struct{})
+	for shareID, resourceID := range shareIDsByResourceID {
+		if _, handled := shareMem[shareID]; handled {
+			// We don't want to add a share multiple times when we added it
+			// already.
+			continue
+		}
+
+		if _, checked := statMem[resourceIDToIndex(resourceID)]; !checked {
+			sReq := &provider.StatRequest{
+				Ref: &provider.Reference{ResourceId: resourceID},
+			}
+			sRes, err := m.gatewayClient.Stat(ctx, sReq)
+			if err != nil {
+				continue
+			}
+			if sRes.Status.Code != rpcv1beta1.Code_CODE_OK {
+				continue
+			}
+			if !sRes.Info.PermissionSet.ListGrants {
+				continue
+			}
+			statMem[resourceIDToIndex(resourceID)] = struct{}{}
+		}
+
+		s, err := m.getShareByID(ctx, shareID)
+		if err != nil {
+			return nil, err
+		}
+		if share.MatchesFilters(s, filters) {
+			result = append(result, s)
+			shareMem[s.Id.OpaqueId] = struct{}{}
 		}
 	}
+
 	return result, nil
 }
 
@@ -285,7 +380,9 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati
 	if err != nil {
 		return nil, err
 	}
-	receivedIds, err := m.indexer.FindBy(&collaboration.Share{}, "GranteeId", ids)
+	receivedIds, err := m.indexer.FindBy(&collaboration.Share{},
+		indexer.NewField("GranteeId", ids),
+	)
 	if err != nil {
 		return nil, err
 	}
@@ -297,7 +394,9 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati
 		if err != nil {
 			return nil, err
 		}
-		groupIds, err := m.indexer.FindBy(&collaboration.Share{}, "GranteeId", index)
+		groupIds, err := m.indexer.FindBy(&collaboration.Share{},
+			indexer.NewField("GranteeId", index),
+		)
 		if err != nil {
 			return nil, err
 		}
@@ -448,15 +547,19 @@ func (m *Manager) getShareByID(ctx context.Context, id string) (*collaboration.S
 }
 
 func (m *Manager) getShareByKey(ctx context.Context, key *collaboration.ShareKey) (*collaboration.Share, error) {
-	ownerIds, err := m.indexer.FindBy(&collaboration.Share{}, "OwnerId", userIDToIndex(key.Owner))
+	ownerIds, err := m.indexer.FindBy(&collaboration.Share{},
+		indexer.NewField("OwnerId", userIDToIndex(key.Owner)),
+	)
 	if err != nil {
 		return nil, err
 	}
 	granteeIndex, err := granteeToIndex(key.Grantee)
 	if err != nil {
 		return nil, err
 	}
-	granteeIds, err := m.indexer.FindBy(&collaboration.Share{}, "GranteeId", granteeIndex)
+	granteeIds, err := m.indexer.FindBy(&collaboration.Share{},
+		indexer.NewField("GranteeId", granteeIndex),
+	)
 	if err != nil {
 		return nil, err
 	}
@@ -501,6 +604,14 @@ func indexOwnerFunc(v interface{}) (string, error) {
 	return userIDToIndex(share.Owner), nil
 }
 
+func indexCreatorFunc(v interface{}) (string, error) {
+	share, ok := v.(*collaboration.Share)
+	if !ok {
+		return "", fmt.Errorf("given entity is not a share")
+	}
+	return userIDToIndex(share.Creator), nil
+}
+
 func userIDToIndex(id *userpb.UserId) string {
 	return url.QueryEscape(id.Idp + ":" + id.OpaqueId)
 }
@@ -513,6 +624,18 @@ func indexGranteeFunc(v interface{}) (string, error) {
 	return granteeToIndex(share.Grantee)
 }
 
+func indexResourceIDFunc(v interface{}) (string, error) {
+	share, ok := v.(*collaboration.Share)
+	if !ok {
+		return "", fmt.Errorf("given entity is not a share")
+	}
+	return resourceIDToIndex(share.ResourceId), nil
+}
+
+func resourceIDToIndex(id *provider.ResourceId) string {
+	return strings.Join([]string{id.StorageId, id.OpaqueId}, "!")
+}
+
 func granteeToIndex(grantee *provider.Grantee) (string, error) {
 	switch {
 	case grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER: