graph: use share manager for managing space permissions

This gets us rid of quite a bit of special casing for space permission.
Also provides us with "real" permission IDs instead of those faked
"u:<userid>" ones.

services/graph/pkg/service/v0/api_driveitem_permissions.go

@@ -245,10 +245,6 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 
 	if shareid != "" {
 		permission.Id = conversions.ToPointer(shareid)
-	} else if IsSpaceRoot(statResponse.GetInfo().GetId()) {
-		// permissions on a space root are not handled by a share manager so
-		// they don't get a share-id
-		permission.SetId(identitySetToSpacePermissionID(permission.GetGrantedToV2()))
 	}
 
 	if expiration != nil {
@@ -414,12 +410,12 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
 	var permissionsCount int
 
 	if IsSpaceRoot(statResponse.GetInfo().GetId()) {
-		var permissions []libregraph.Permission
-		permissions, permissionsCount, err = s.getSpaceRootPermissions(ctx, statResponse.GetInfo().GetSpace().GetId(), queryOptions.NoValues)
+		driveItems, err = s.listSpaceRootUserShares(ctx, []*collaboration.Filter{
+			share.ResourceIDFilter(itemID),
+		}, driveItems)
 		if err != nil {
 			return collectionOfPermissions, err
 		}
-		collectionOfPermissions.Value = permissions
 	} else {
 		// "normal" driveItem, populate user  permissions via share providers
 		driveItems, err = s.listUserShares(ctx, []*collaboration.Filter{
@@ -535,12 +531,10 @@ func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemI
 	}
 
 	switch permissionType {
-	case User:
+	case User, Space:
 		return s.removeUserShare(ctx, permissionID)
 	case Public:
 		return s.removePublicShare(ctx, permissionID)
-	case Space:
-		return s.removeSpacePermission(ctx, permissionID, sharedResourceID)
 	case OCM:
 		return s.removeOCMPermission(ctx, permissionID)
 	default:

services/graph/pkg/service/v0/api_driveitem_permissions_test.go

@@ -24,7 +24,6 @@ import (
 	"github.com/opencloud-eu/opencloud/services/graph/pkg/config"
 	"github.com/stretchr/testify/mock"
 	"github.com/tidwall/gjson"
-	"google.golang.org/grpc"
 
 	roleconversions "github.com/opencloud-eu/reva/v2/pkg/conversions"
 	revactx "github.com/opencloud-eu/reva/v2/pkg/ctx"
@@ -461,6 +460,8 @@ var _ = Describe("DriveItemPermissionsService", func() {
 		})
 		It("returns role denied", func() {
 			statResponse.Info.PermissionSet = roleconversions.NewManagerRole().CS3ResourcePermissions()
+			statResponse.Info.Type = provider.ResourceType_RESOURCE_TYPE_CONTAINER
+
 			listSharesResponse.Shares = []*collaboration.Share{
 				{
 					Id: &collaboration.ShareId{OpaqueId: "1"},
@@ -685,6 +686,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			}
 
 			gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(listSpacesResponse, nil)
+			gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(&collaboration.ListSharesResponse{Status: status.NewOK(ctx)}, nil)
 			gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil)
 			statResponse.Info.Id = listSpacesResponse.StorageSpaces[0].Root
 			gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
@@ -784,12 +786,10 @@ var _ = Describe("DriveItemPermissionsService", func() {
 
 			gatewayClient.On("RemoveShare",
 				mock.Anything,
-				mock.Anything,
-			).Return(func(ctx context.Context, in *collaboration.RemoveShareRequest, opts ...grpc.CallOption) (*collaboration.RemoveShareResponse, error) {
-				Expect(in.Ref.GetKey()).ToNot(BeNil())
-				Expect(in.Ref.GetKey().GetGrantee().GetUserId().GetOpaqueId()).To(Equal("userid"))
-				return &collaboration.RemoveShareResponse{Status: status.NewOK(ctx)}, nil
-			})
+				mock.MatchedBy(func(req *collaboration.RemoveShareRequest) bool {
+					return req.GetRef().GetId().GetOpaqueId() == "shareid"
+				}),
+			).Return(&collaboration.RemoveShareResponse{Status: status.NewOK(ctx)}, nil)
 
 			err := driveItemPermissionsService.DeletePermission(context.Background(),
 				&provider.ResourceId{
@@ -797,7 +797,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 					SpaceId:   "2",
 					OpaqueId:  "2",
 				},
-				"u:userid",
+				"shareid",
 			)
 			Expect(err).ToNot(HaveOccurred())
 		})
@@ -1064,24 +1064,40 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			Expect(res).To(BeZero())
 		})
 		It("fails to update the space permissions for a space share when setting a file specific role", func() {
-			gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
 			getPublicShareMockResponse.Share = nil
 			getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
 			gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(getPublicShareMockResponse, nil)
 
-			gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(listSpacesResponse, nil)
-
-			statResponse.Info.Id = listSpacesResponse.StorageSpaces[0].Root
-			gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
-			gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
-
-			driveItemPermission.SetRoles([]string{unifiedrole.UnifiedRoleFileEditorID})
 			spaceId := &provider.ResourceId{
 				StorageId: "1",
 				SpaceId:   "2",
 				OpaqueId:  "2",
 			}
-			res, err := driveItemPermissionsService.UpdatePermission(ctx, spaceId, "u:userid", driveItemPermission)
+			statResponse.Info.Id = spaceId
+			gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
+
+			gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(&collaboration.ListSharesResponse{
+				Status: status.NewOK(ctx),
+				Shares: []*collaboration.Share{
+					{
+						Id:         &collaboration.ShareId{OpaqueId: "spaceShareId"},
+						ResourceId: spaceId,
+						Grantee: &provider.Grantee{
+							Type: provider.GranteeType_GRANTEE_TYPE_USER,
+							Id: &provider.Grantee_UserId{
+								UserId: &userpb.UserId{OpaqueId: "userid"},
+							},
+						},
+						Permissions: &collaboration.SharePermissions{
+							Permissions: roleconversions.NewSpaceViewerRole().CS3ResourcePermissions(),
+						},
+					},
+				},
+			}, nil)
+			gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
+
+			driveItemPermission.SetRoles([]string{unifiedrole.UnifiedRoleFileEditorID})
+			res, err := driveItemPermissionsService.UpdatePermission(ctx, spaceId, "spaceShareId", driveItemPermission)
 			Expect(err).To(MatchError(errorcode.New(errorcode.InvalidRequest, "role not applicable to this resource")))
 			Expect(res).To(BeZero())
 		})

services/graph/pkg/service/v0/base.go

@@ -51,22 +51,6 @@ type BaseGraphService struct {
 	availableRoles  []*libregraph.UnifiedRoleDefinition
 }
 
-func (g BaseGraphService) getSpaceRootPermissions(ctx context.Context, spaceID *storageprovider.StorageSpaceId, countOnly bool) ([]libregraph.Permission, int, error) {
-	gatewayClient, err := g.gatewaySelector.Next()
-
-	if err != nil {
-		g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed")
-		return nil, 0, err
-	}
-	space, err := utils.GetSpace(ctx, spaceID.GetOpaqueId(), gatewayClient)
-	if err != nil {
-		return nil, 0, errorcode.FromUtilsStatusCodeError(err)
-	}
-
-	perm, count := g.cs3SpacePermissionsToLibreGraph(ctx, space, countOnly, APIVersion_1_Beta_1)
-	return perm, count, nil
-}
-
 func (g BaseGraphService) getDriveItem(ctx context.Context, ref *storageprovider.Reference) (*libregraph.DriveItem, error) {
 	gatewayClient, err := g.gatewaySelector.Next()
 	if err != nil {
@@ -269,6 +253,17 @@ func (g BaseGraphService) libreGraphPermissionFromCS3PublicShare(createdLink *li
 }
 
 func (g BaseGraphService) listUserShares(ctx context.Context, filters []*collaboration.Filter, driveItems driveItemsByResourceID) (driveItemsByResourceID, error) {
+	return g.listSharesWithSpaceRootFilter(ctx, false, filters, driveItems)
+}
+
+// listSpaceRootUserShares lists user/group shares whose resource is a space root (i.e. space
+// memberships). It uses SpaceRootFilter(true) so only space-root shares are returned, mirroring
+// how listUserShares works for regular file/folder shares.
+func (g BaseGraphService) listSpaceRootUserShares(ctx context.Context, filters []*collaboration.Filter, driveItems driveItemsByResourceID) (driveItemsByResourceID, error) {
+	return g.listSharesWithSpaceRootFilter(ctx, true, filters, driveItems)
+}
+
+func (g BaseGraphService) listSharesWithSpaceRootFilter(ctx context.Context, spaceRoot bool, filters []*collaboration.Filter, driveItems driveItemsByResourceID) (driveItemsByResourceID, error) {
 	gatewayClient, err := g.gatewaySelector.Next()
 	if err != nil {
 		g.logger.Error().Err(err).Msg("could not select next gateway client")
@@ -278,7 +273,7 @@ func (g BaseGraphService) listUserShares(ctx context.Context, filters []*collabo
 	concreteFilters := []*collaboration.Filter{
 		share.UserGranteeFilter(),
 		share.GroupGranteeFilter(),
-		share.SpaceRootFilter(false),
+		share.SpaceRootFilter(spaceRoot),
 	}
 	concreteFilters = append(concreteFilters, filters...)
 
@@ -563,9 +558,7 @@ func (g BaseGraphService) cs3OCMSharesToDriveItems(ctx context.Context, shares [
 func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *collaboration.Share, roleCondition string) (*libregraph.Permission, error) {
 	perm := libregraph.Permission{}
 	perm.SetRoles([]string{})
-	if roleCondition != unifiedrole.UnifiedRoleConditionDrive {
-		perm.SetId(share.GetId().GetOpaqueId())
-	}
+	perm.SetId(share.GetId().GetOpaqueId())
 	grantedTo := libregraph.SharePointIdentitySet{}
 	switch share.GetGrantee().GetType() {
 	case storageprovider.GranteeType_GRANTEE_TYPE_USER:
@@ -579,9 +572,6 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c
 			return nil, errorcode.New(errorcode.GeneralException, err.Error())
 		default:
 			grantedTo.SetUser(user)
-			if roleCondition == unifiedrole.UnifiedRoleConditionDrive {
-				perm.SetId("u:" + user.GetId())
-			}
 		}
 	case storageprovider.GranteeType_GRANTEE_TYPE_GROUP:
 		group, err := groupIdToIdentity(ctx, g.identityCache, share.Grantee.GetGroupId().GetOpaqueId())
@@ -594,9 +584,6 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c
 			return nil, errorcode.New(errorcode.GeneralException, err.Error())
 		default:
 			grantedTo.SetGroup(group)
-			if roleCondition == unifiedrole.UnifiedRoleConditionDrive {
-				perm.SetId("g:" + group.GetId())
-			}
 		}
 	}
 
@@ -927,35 +914,6 @@ func (g BaseGraphService) removeUserShare(ctx context.Context, permissionID stri
 	return nil
 }
 
-func (g BaseGraphService) removeSpacePermission(ctx context.Context, permissionID string, resourceId *storageprovider.ResourceId) error {
-	grantee, err := spacePermissionIdToCS3Grantee(permissionID)
-	if err != nil {
-		return err
-	}
-
-	gatewayClient, err := g.gatewaySelector.Next()
-	if err != nil {
-		g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed")
-		return err
-	}
-	removeShareResp, err := gatewayClient.RemoveShare(ctx, &collaboration.RemoveShareRequest{
-		Ref: &collaboration.ShareReference{
-			Spec: &collaboration.ShareReference_Key{
-				Key: &collaboration.ShareKey{
-					ResourceId: resourceId,
-					Grantee:    &grantee,
-				},
-			},
-		},
-	})
-	if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil {
-		return err
-	}
-
-	// We need to return an untyped nil here otherwise the error==nil check won't work
-	return nil
-}
-
 func (g BaseGraphService) getOCMPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) {
 	cs3Share, err := g.getCS3OCMShareByID(ctx, permissionID)
 	if err != nil {
@@ -1071,20 +1029,19 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st
 		}
 		return permission, publicShare.GetResourceId(), nil
 	case IsSpaceRoot(itemID):
-		// itemID is referencing a spaceroot this is a space permission. Handle
-		// that here and get space id
-		resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient)
+		// itemID is referencing a space root — use the share manager to look up the permission
+		driveItems := make(driveItemsByResourceID)
+		driveItems, err = g.listSpaceRootUserShares(ctx, []*collaboration.Filter{
+			share.ResourceIDFilter(itemID),
+		}, driveItems)
 		if err != nil {
 			return nil, nil, err
 		}
-
-		perms, _, err := g.getSpaceRootPermissions(ctx, resourceInfo.GetSpace().GetId(), false)
-		if err != nil {
-			return nil, nil, err
-		}
-		for _, p := range perms {
-			if p.GetId() == permissionID {
-				return &p, itemID, nil
+		for _, item := range driveItems {
+			for i := range item.Permissions {
+				if item.Permissions[i].GetId() == permissionID {
+					return &item.Permissions[i], itemID, nil
+				}
 			}
 		}
 	case errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound:
@@ -1239,29 +1196,10 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri
 	}
 
 	var cs3UpdateShareReq collaboration.UpdateShareRequest
-	// When updating a space root we need to reference the share by resourceId and grantee
-	if IsSpaceRoot(itemID) {
-		grantee, err := spacePermissionIdToCS3Grantee(permissionID)
-		if err != nil {
-			g.logger.Debug().Err(err).Str("permissionid", permissionID).Msg("failed to parse space permission id")
-			return nil, err
-		}
-		cs3UpdateShareReq.Share = &collaboration.Share{
-			ResourceId: itemID,
-			Grantee:    &grantee,
-		}
-		cs3UpdateShareReq.Opaque = &types.Opaque{
-			Map: map[string]*types.OpaqueEntry{
-				"spacegrant": {},
-			},
-		}
-		cs3UpdateShareReq.Opaque = utils.AppendPlainToOpaque(cs3UpdateShareReq.Opaque, "spacetype", _spaceTypeProject)
-	} else {
-		cs3UpdateShareReq.Share = &collaboration.Share{
-			Id: &collaboration.ShareId{
-				OpaqueId: permissionID,
-			},
-		}
+	cs3UpdateShareReq.Share = &collaboration.Share{
+		Id: &collaboration.ShareId{
+			OpaqueId: permissionID,
+		},
 	}
 	fieldmask := []string{}
 	if expiration, ok := newPermission.GetExpirationDateTimeOk(); ok {

services/graph/pkg/service/v0/driveitems.go

@@ -15,8 +15,6 @@ import (
 	"time"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
-	grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
-	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
@@ -349,35 +347,6 @@ func (g Graph) GetDriveItemChildren(w http.ResponseWriter, r *http.Request) {
 	render.JSON(w, r, &ListResponse{Value: files})
 }
 
-func spacePermissionIdToCS3Grantee(permissionID string) (storageprovider.Grantee, error) {
-	// the permission ID for space permission is made of two parts
-	// the grantee type ('u' or user, 'g' for group) and the user or group id
-	var grantee storageprovider.Grantee
-	parts := strings.SplitN(permissionID, ":", 2)
-	if len(parts) != 2 {
-		return grantee, errorcode.New(errorcode.InvalidRequest, "invalid space permission id")
-	}
-	switch parts[0] {
-	case "u":
-		grantee.Type = storageprovider.GranteeType_GRANTEE_TYPE_USER
-		grantee.Id = &storageprovider.Grantee_UserId{
-			UserId: &userpb.UserId{
-				OpaqueId: parts[1],
-			},
-		}
-	case "g":
-		grantee.Type = storageprovider.GranteeType_GRANTEE_TYPE_GROUP
-		grantee.Id = &storageprovider.Grantee_GroupId{
-			GroupId: &grouppb.GroupId{
-				OpaqueId: parts[1],
-			},
-		}
-	default:
-		return grantee, errorcode.New(errorcode.InvalidRequest, "invalid space permission id")
-	}
-	return grantee, nil
-}
-
 func (g Graph) getRemoteItem(ctx context.Context, root *storageprovider.ResourceId, baseURL *url.URL) (*libregraph.RemoteItem, error) {
 	gatewayClient, err := g.gatewaySelector.Next()
 	if err != nil {
@@ -461,14 +430,22 @@ func cs3ResourceToDriveItem(logger *log.Logger, res *storageprovider.ResourceInf
 		parentRef.SetPath(path.Dir(res.GetPath()))
 		driveItem.ParentReference = parentRef
 	}
-	if res.GetType() == storageprovider.ResourceType_RESOURCE_TYPE_FILE && res.GetMimeType() != "" {
+	switch res.GetType() {
+	case storageprovider.ResourceType_RESOURCE_TYPE_FILE:
+		mimeType := res.GetMimeType()
+		if mimeType == "" {
+			mimeType = "application/octet-stream"
+		}
 		// We cannot use a libregraph.File here because the openapi codegenerator autodetects 'File' as a go type ...
 		driveItem.File = &libregraph.OpenGraphFile{
-			MimeType: &res.MimeType,
+			MimeType: &mimeType,
+		}
+	case storageprovider.ResourceType_RESOURCE_TYPE_CONTAINER:
+		if IsSpaceRoot(res.GetId()) {
+			driveItem.SetRoot(map[string]any{})
+		} else {
+			driveItem.SetFolder(libregraph.Folder{})
 		}
-	}
-	if res.GetType() == storageprovider.ResourceType_RESOURCE_TYPE_CONTAINER {
-		driveItem.Folder = &libregraph.Folder{}
 	}
 
 	if res.GetArbitraryMetadata() != nil {

services/graph/pkg/service/v0/sharedwithme.go

@@ -10,6 +10,7 @@ import (
 	ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1"
 	"github.com/go-chi/render"
 	libregraph "github.com/opencloud-eu/libre-graph-api-go"
+	"github.com/opencloud-eu/reva/v2/pkg/share"
 
 	"github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode"
 	"github.com/opencloud-eu/opencloud/services/thumbnails/pkg/thumbnail"
@@ -41,7 +42,11 @@ func (g Graph) listSharedWithMe(ctx context.Context, expandThumbnails bool) ([]l
 		return nil, err
 	}
 
-	listReceivedSharesResponse, err := gatewayClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{})
+	listReceivedSharesResponse, err := gatewayClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{
+		Filters: []*collaboration.Filter{
+			share.SpaceRootFilter(false),
+		},
+	})
 	if err := errorcode.FromCS3Status(listReceivedSharesResponse.GetStatus(), err); err != nil {
 		g.logger.Error().Err(err).Msg("listing shares failed")
 		return nil, err