diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index aa68e8d0fe..f55ad2717d 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/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: diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go index 1c7f703856..4a088664dd 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go +++ b/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()) }) diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index 6433484738..fbbfa1edfe 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/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 { diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index da89e25bad..7a6615173b 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/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 { diff --git a/services/graph/pkg/service/v0/sharedwithme.go b/services/graph/pkg/service/v0/sharedwithme.go index f6e645e193..c9d1ca92cf 100644 --- a/services/graph/pkg/service/v0/sharedwithme.go +++ b/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