Merge pull request #10501 from rhafer/issue/10495

fix(graph): Use the correct opaqueId when Statting OCM shares

changelog/unreleased/fix-ocm-file-preview.md

@@ -0,0 +1,7 @@
+Bugfix: Fixed `sharedWithMe` response for OCM shares
+
+OCM shares returned in the `sharedWithMe` response did not have the `mimeType` property
+populated correctly.
+
+https://github.com/owncloud/ocis/pull/10501
+https://github.com/owncloud/ocis/issues/10495

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

@@ -634,7 +634,7 @@ func (api DriveItemPermissionsApi) Invite(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	permission, err := api.driveItemPermissionsService.Invite(ctx, &itemID, *driveItemInvite)
+	permission, err := api.driveItemPermissionsService.Invite(ctx, itemID, *driveItemInvite)
 	if err != nil {
 		errorcode.RenderError(w, r, err)
 		return
@@ -698,7 +698,7 @@ func (api DriveItemPermissionsApi) ListPermissions(w http.ResponseWriter, r *htt
 
 	ctx := r.Context()
 
-	permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, &itemID, listFederatedRoles, selectRoles)
+	permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, itemID, listFederatedRoles, selectRoles)
 	if err != nil {
 		errorcode.RenderError(w, r, err)
 		return
@@ -774,7 +774,7 @@ func (api DriveItemPermissionsApi) DeletePermission(w http.ResponseWriter, r *ht
 	}
 
 	ctx := r.Context()
-	err = api.driveItemPermissionsService.DeletePermission(ctx, &itemID, permissionID)
+	err = api.driveItemPermissionsService.DeletePermission(ctx, itemID, permissionID)
 	if err != nil {
 		errorcode.RenderError(w, r, err)
 		return
@@ -841,7 +841,7 @@ func (api DriveItemPermissionsApi) UpdatePermission(w http.ResponseWriter, r *ht
 		return
 	}
 
-	updatedPermission, err := api.driveItemPermissionsService.UpdatePermission(ctx, &itemID, permissionID, permission)
+	updatedPermission, err := api.driveItemPermissionsService.UpdatePermission(ctx, itemID, permissionID, permission)
 	if err != nil {
 		errorcode.RenderError(w, r, err)
 		return

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

@@ -166,7 +166,7 @@ func (api DriveItemPermissionsApi) CreateLink(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	perm, err := api.driveItemPermissionsService.CreateLink(r.Context(), &driveItemID, createLink)
+	perm, err := api.driveItemPermissionsService.CreateLink(r.Context(), driveItemID, createLink)
 	if err != nil {
 		errorcode.RenderError(w, r, err)
 		return
@@ -228,7 +228,7 @@ func (api DriveItemPermissionsApi) SetLinkPassword(w http.ResponseWriter, r *htt
 		return
 	}
 
-	newPermission, err := api.driveItemPermissionsService.SetPublicLinkPassword(ctx, &itemID, permissionID, password.GetPassword())
+	newPermission, err := api.driveItemPermissionsService.SetPublicLinkPassword(ctx, itemID, permissionID, password.GetPassword())
 	if err != nil {
 		errorcode.RenderError(w, r, err)
 		return

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

@@ -499,7 +499,7 @@ func (api DrivesDriveItemApi) CreateDriveItem(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	if !IsShareJail(driveID) {
+	if !IsShareJail(&driveID) {
 		api.logger.Debug().Interface("driveID", driveID).Msg(ErrNotAShareJail.Error())
 		ErrNotAShareJail.Render(w, r)
 		return

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

@@ -2,6 +2,7 @@ package svc
 
 import (
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"io"
 	"net/http"
@@ -44,8 +45,8 @@ func IsSpaceRoot(rid *storageprovider.ResourceId) bool {
 
 // GetDriveAndItemIDParam parses the driveID and itemID from the request,
 // validates the common fields and returns the parsed IDs if ok.
-func GetDriveAndItemIDParam(r *http.Request, logger *log.Logger) (storageprovider.ResourceId, storageprovider.ResourceId, error) {
-	empty := storageprovider.ResourceId{}
+func GetDriveAndItemIDParam(r *http.Request, logger *log.Logger) (*storageprovider.ResourceId, *storageprovider.ResourceId, error) {
+	empty := &storageprovider.ResourceId{}
 
 	driveID, err := parseIDParam(r, "driveID")
 	if err != nil {
@@ -60,16 +61,16 @@ func GetDriveAndItemIDParam(r *http.Request, logger *log.Logger) (storageprovide
 	}
 
 	if itemID.GetOpaqueId() == "" {
-		logger.Debug().Interface("driveID", driveID).Interface("itemID", itemID).Msg("empty item opaqueID")
+		logger.Debug().Interface("driveID", &driveID).Interface("itemID", &itemID).Msg("empty item opaqueID")
 		return empty, empty, errorcode.New(errorcode.InvalidRequest, "invalid itemID")
 	}
 
 	if driveID.GetStorageId() != itemID.GetStorageId() || driveID.GetSpaceId() != itemID.GetSpaceId() {
-		logger.Debug().Interface("driveID", driveID).Interface("itemID", itemID).Msg("driveID and itemID do not match")
+		logger.Debug().Interface("driveID", &driveID).Interface("itemID", &itemID).Msg("driveID and itemID do not match")
 		return empty, empty, errorcode.New(errorcode.ItemNotFound, "driveID and itemID do not match")
 	}
 
-	return driveID, itemID, nil
+	return &driveID, &itemID, nil
 }
 
 // GetFilterParam returns the $filter query parameter from the request. If you need to parse the filter use godata.ParseRequest
@@ -95,7 +96,7 @@ func (g Graph) GetGatewayClient(w http.ResponseWriter, r *http.Request) (gateway
 }
 
 // IsShareJail returns true if given id is a share jail id.
-func IsShareJail(id storageprovider.ResourceId) bool {
+func IsShareJail(id *storageprovider.ResourceId) bool {
 	return id.GetStorageId() == utils.ShareStorageProviderID && id.GetSpaceId() == utils.ShareStorageSpaceID
 }
 
@@ -510,7 +511,7 @@ func federatedRoleConditionForResourceType(ri *storageprovider.ResourceInfo) (st
 // ExtractShareIdFromResourceId is a bit of a hack.
 // We should not rely on a specific format of the item id.
 // But currently there is no other way to get the ShareID.
-func ExtractShareIdFromResourceId(rid storageprovider.ResourceId) *collaboration.ShareId {
+func ExtractShareIdFromResourceId(rid *storageprovider.ResourceId) *collaboration.ShareId {
 	return &collaboration.ShareId{
 		OpaqueId: rid.GetOpaqueId(),
 	}
@@ -538,14 +539,21 @@ func cs3ReceivedOCMSharesToDriveItems(ctx context.Context,
 
 		group.Go(func() error {
 			var err error // redeclare
+
+			// for OCM shares the opaqueID is the '/' for shared directories and '/filename' for
+			// file shares
+			resOpaqueID := "/"
+			if receivedShares[0].GetResourceType() == storageprovider.ResourceType_RESOURCE_TYPE_FILE {
+				resOpaqueID += receivedShares[0].GetName()
+			}
+
 			shareStat, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{
 				Ref: &storageprovider.Reference{
 					ResourceId: &storageprovider.ResourceId{
 						// TODO maybe the reference is wrong
 						StorageId: utils.OCMStorageProviderID,
 						SpaceId:   receivedShares[0].GetId().GetOpaqueId(),
-						OpaqueId:  "", // in OCM resources the opaque id is the base64 encoded path
-						//OpaqueId: maybe ? receivedShares[0].GetId().GetOpaqueId(),
+						OpaqueId:  base64.StdEncoding.EncodeToString([]byte(resOpaqueID)),
 					},
 				},
 			})

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

@@ -15,6 +15,7 @@ import (
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/storagespace"
+	"google.golang.org/protobuf/testing/protocmp"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/conversions"
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
@@ -42,10 +43,10 @@ var _ = Describe("Utils", func() {
 			case true:
 				Expect(err).To(BeNil())
 				parsedItemID, _ := storagespace.ParseID(itemID)
-				Expect(extractedItemID).To(Equal(parsedItemID))
+				Expect(extractedItemID).To(BeComparableTo(&parsedItemID, protocmp.Transform()))
 
 				parsedDriveID, _ := storagespace.ParseID(driveID)
-				Expect(extractedDriveID).To(Equal(parsedDriveID))
+				Expect(extractedDriveID).To(BeComparableTo(&parsedDriveID, protocmp.Transform()))
 			default:
 				Expect(err).ToNot(BeNil())
 			}
@@ -82,29 +83,29 @@ var _ = Describe("Utils", func() {
 	)
 
 	DescribeTable("IsShareJail",
-		func(resourceID provider.ResourceId, isShareJail bool) {
+		func(resourceID *provider.ResourceId, isShareJail bool) {
 			Expect(service.IsShareJail(resourceID)).To(Equal(isShareJail))
 		},
-		Entry("valid: share jail", provider.ResourceId{
+		Entry("valid: share jail", &provider.ResourceId{
 			StorageId: utils.ShareStorageProviderID,
 			SpaceId:   utils.ShareStorageSpaceID,
 		}, true),
-		Entry("invalid: empty storageId", provider.ResourceId{
+		Entry("invalid: empty storageId", &provider.ResourceId{
 			SpaceId: utils.ShareStorageSpaceID,
 		}, false),
-		Entry("invalid: empty spaceId", provider.ResourceId{
+		Entry("invalid: empty spaceId", &provider.ResourceId{
 			StorageId: utils.ShareStorageProviderID,
 		}, false),
-		Entry("invalid: empty storageId and spaceId", provider.ResourceId{}, false),
-		Entry("invalid: non share jail storageId", provider.ResourceId{
+		Entry("invalid: empty storageId and spaceId", &provider.ResourceId{}, false),
+		Entry("invalid: non share jail storageId", &provider.ResourceId{
 			StorageId: "123",
 			SpaceId:   utils.ShareStorageSpaceID,
 		}, false),
-		Entry("invalid: non share jail spaceId", provider.ResourceId{
+		Entry("invalid: non share jail spaceId", &provider.ResourceId{
 			StorageId: utils.ShareStorageProviderID,
 			SpaceId:   "123",
 		}, false),
-		Entry("invalid: non share jail storageID and spaceId", provider.ResourceId{
+		Entry("invalid: non share jail storageID and spaceId", &provider.ResourceId{
 			StorageId: "123",
 			SpaceId:   "123",
 		}, false),