From 09dac7178166c5a852e486bae3e2b91599a9815e Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 27 Mar 2024 14:30:58 +0100 Subject: [PATCH] refactor(graph): move DeletePermission deletion to the permissions service --- .../mocks/drive_item_permissions_provider.go | 48 ++++ .../service/v0/api_driveitem_permissions.go | 90 +++++++ .../v0/api_driveitem_permissions_test.go | 163 ++++++++++++- services/graph/pkg/service/v0/base.go | 141 +++++++++++ services/graph/pkg/service/v0/driveitems.go | 221 ------------------ .../graph/pkg/service/v0/driveitems_test.go | 181 +------------- services/graph/pkg/service/v0/service.go | 3 +- 7 files changed, 440 insertions(+), 407 deletions(-) diff --git a/services/graph/mocks/drive_item_permissions_provider.go b/services/graph/mocks/drive_item_permissions_provider.go index 853c3d104c..ad2d453b01 100644 --- a/services/graph/mocks/drive_item_permissions_provider.go +++ b/services/graph/mocks/drive_item_permissions_provider.go @@ -24,6 +24,54 @@ func (_m *DriveItemPermissionsProvider) EXPECT() *DriveItemPermissionsProvider_E return &DriveItemPermissionsProvider_Expecter{mock: &_m.Mock} } +// DeletePermission provides a mock function with given fields: ctx, itemID, permissionID +func (_m *DriveItemPermissionsProvider) DeletePermission(ctx context.Context, itemID providerv1beta1.ResourceId, permissionID string) error { + ret := _m.Called(ctx, itemID, permissionID) + + if len(ret) == 0 { + panic("no return value specified for DeletePermission") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, providerv1beta1.ResourceId, string) error); ok { + r0 = rf(ctx, itemID, permissionID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// DriveItemPermissionsProvider_DeletePermission_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeletePermission' +type DriveItemPermissionsProvider_DeletePermission_Call struct { + *mock.Call +} + +// DeletePermission is a helper method to define mock.On call +// - ctx context.Context +// - itemID providerv1beta1.ResourceId +// - permissionID string +func (_e *DriveItemPermissionsProvider_Expecter) DeletePermission(ctx interface{}, itemID interface{}, permissionID interface{}) *DriveItemPermissionsProvider_DeletePermission_Call { + return &DriveItemPermissionsProvider_DeletePermission_Call{Call: _e.mock.On("DeletePermission", ctx, itemID, permissionID)} +} + +func (_c *DriveItemPermissionsProvider_DeletePermission_Call) Run(run func(ctx context.Context, itemID providerv1beta1.ResourceId, permissionID string)) *DriveItemPermissionsProvider_DeletePermission_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(providerv1beta1.ResourceId), args[2].(string)) + }) + return _c +} + +func (_c *DriveItemPermissionsProvider_DeletePermission_Call) Return(_a0 error) *DriveItemPermissionsProvider_DeletePermission_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *DriveItemPermissionsProvider_DeletePermission_Call) RunAndReturn(run func(context.Context, providerv1beta1.ResourceId, string) error) *DriveItemPermissionsProvider_DeletePermission_Call { + _c.Call.Return(run) + return _c +} + // Invite provides a mock function with given fields: ctx, resourceId, invite func (_m *DriveItemPermissionsProvider) Invite(ctx context.Context, resourceId providerv1beta1.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) { ret := _m.Called(ctx, resourceId, invite) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 82507a00ef..5d3caac4f4 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -3,6 +3,7 @@ package svc import ( "context" "net/http" + "net/url" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" @@ -15,6 +16,7 @@ import ( "github.com/cs3org/reva/v2/pkg/share" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" + "github.com/go-chi/chi/v5" "github.com/go-chi/render" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/ocis-pkg/conversions" @@ -33,6 +35,7 @@ type DriveItemPermissionsProvider interface { SpaceRootInvite(ctx context.Context, driveID storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) ListPermissions(ctx context.Context, itemID storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) ListSpaceRootPermissions(ctx context.Context, driveID storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) + DeletePermission(ctx context.Context, itemID storageprovider.ResourceId, permissionID string) error } // DriveItemPermissionsService contains the production business logic for everything that relates to permissions on drive items. @@ -40,6 +43,15 @@ type DriveItemPermissionsService struct { BaseGraphService } +type permissionType int + +const ( + Unknown permissionType = iota + Public + User + Space +) + // NewDriveItemPermissionsService creates a new DriveItemPermissionsService func NewDriveItemPermissionsService(logger log.Logger, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], identityCache identity.IdentityCache, config *config.Config) (DriveItemPermissionsService, error) { return DriveItemPermissionsService{ @@ -287,6 +299,58 @@ func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Contex return s.ListPermissions(ctx, *rootResourceID) } +func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemID storageprovider.ResourceId, permissionID string) error { + var permissionType permissionType + + sharedResourceID, err := s.getLinkPermissionResourceID(ctx, permissionID) + switch { + // Check if the ID is referring to a public share + case err == nil: + permissionType = Public + // If the item id is referring to a space root and this is not a public share + // we have to deal with space permissions + case IsSpaceRoot(&itemID): + permissionType = Space + sharedResourceID = &itemID + err = nil + // If this is neither a public share nor a space permission, check if this is a + // user share + default: + sharedResourceID, err = s.getUserPermissionResourceID(ctx, permissionID) + if err == nil { + permissionType = User + } + } + + switch { + case err != nil: + return err + case permissionType == Unknown: + return errorcode.New(errorcode.ItemNotFound, "permission not found") + case sharedResourceID == nil: + return errorcode.New(errorcode.ItemNotFound, "failed to resolve resource id for shared resource") + } + + // The resourceID of the shared resource need to match the item ID from the Request Path + // otherwise this is an invalid Request. + if !utils.ResourceIDEqual(sharedResourceID, &itemID) { + s.logger.Debug().Msg("resourceID of shared does not match itemID") + return errorcode.New(errorcode.InvalidRequest, "permissionID and itemID do not match") + } + + switch permissionType { + case User: + return s.removeUserShare(ctx, permissionID) + case Public: + return s.removePublicShare(ctx, permissionID) + case Space: + return s.removeSpacePermission(ctx, permissionID, sharedResourceID) + } + + // This should never be reached + return errorcode.New(errorcode.GeneralException, "failed to delete permission") +} + // DriveItemPermissionsService is the api that registers the http endpoints which expose needed operation to the graph api. // the business logic is delegated to the permissions service and further down to the cs3 client. type DriveItemPermissionsApi struct { @@ -408,3 +472,29 @@ func (api DriveItemPermissionsApi) ListSpaceRootPermissions(w http.ResponseWrite render.Status(r, http.StatusOK) render.JSON(w, r, permissions) } + +func (api DriveItemPermissionsApi) DeletePermission(w http.ResponseWriter, r *http.Request) { + _, itemID, err := GetDriveAndItemIDParam(r, &api.logger) + if err != nil { + api.logger.Debug().Err(err).Msg(invalidIdMsg) + errorcode.RenderError(w, r, err) + return + } + + permissionID, err := url.PathUnescape(chi.URLParam(r, "permissionID")) + if err != nil { + api.logger.Debug().Err(err).Msg("could not parse permissionID") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid permissionID") + return + } + + ctx := r.Context() + err = api.driveItemPermissionsService.DeletePermission(ctx, itemID, permissionID) + if err != nil { + errorcode.RenderError(w, r, err) + return + } + + render.Status(r, http.StatusNoContent) + render.NoContent(w, r) +} 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 676f730f06..b6dd76f7a5 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go @@ -35,6 +35,7 @@ import ( "github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole" "github.com/stretchr/testify/mock" "github.com/tidwall/gjson" + "google.golang.org/grpc" ) var _ = Describe("DriveItemPermissionsService", func() { @@ -43,6 +44,7 @@ var _ = Describe("DriveItemPermissionsService", func() { gatewayClient *cs3mocks.GatewayAPIClient gatewaySelector *mocks.Selectable[gateway.GatewayAPIClient] getUserResponse *userpb.GetUserResponse + listPublicSharesResponse *link.ListPublicSharesResponse currentUser = &userpb.User{ Id: &userpb.UserId{ OpaqueId: "user", @@ -76,6 +78,9 @@ var _ = Describe("DriveItemPermissionsService", func() { DisplayName: "Cem Kaner", }, } + listPublicSharesResponse = &link.ListPublicSharesResponse{ + Status: status.NewOK(ctx), + } }) @@ -287,9 +292,8 @@ var _ = Describe("DriveItemPermissionsService", func() { }) Describe("ListPermissions", func() { var ( - itemID provider.ResourceId - listSharesResponse *collaboration.ListSharesResponse - listPublicSharesResponse *link.ListPublicSharesResponse + itemID provider.ResourceId + listSharesResponse *collaboration.ListSharesResponse ) BeforeEach(func() { itemID = provider.ResourceId{ @@ -301,9 +305,6 @@ var _ = Describe("DriveItemPermissionsService", func() { Status: status.NewOK(ctx), Shares: []*collaboration.Share{}, } - listPublicSharesResponse = &link.ListPublicSharesResponse{ - Status: status.NewOK(ctx), - } }) It("populates allowedValues for files that are not shared", func() { statResponse.Info = &provider.ResourceInfo{ @@ -372,9 +373,8 @@ var _ = Describe("DriveItemPermissionsService", func() { }) Describe("ListSpaceRootPermissions", func() { var ( - listSpacesResponse *provider.ListStorageSpacesResponse - driveId provider.ResourceId - listPublicSharesResponse *link.ListPublicSharesResponse + listSpacesResponse *provider.ListStorageSpacesResponse + driveId provider.ResourceId ) BeforeEach(func() { @@ -393,9 +393,6 @@ var _ = Describe("DriveItemPermissionsService", func() { }, }, } - listPublicSharesResponse = &link.ListPublicSharesResponse{ - Status: status.NewOK(ctx), - } }) It("adds a user to a space as expected (happy path)", func() { @@ -419,6 +416,148 @@ var _ = Describe("DriveItemPermissionsService", func() { }) }) + Describe("DeletePermission", func() { + var ( + getShareResponse collaboration.GetShareResponse + getPublicShareResponse link.GetPublicShareResponse + ) + BeforeEach(func() { + getPublicShareResponse.Status = status.NewOK(context.Background()) + getShareResponse.Status = status.NewOK(context.Background()) + getShareResponse.Share = &collaboration.Share{ + Id: &collaboration.ShareId{ + OpaqueId: "permissionid", + }, + ResourceId: &provider.ResourceId{ + StorageId: "1", + SpaceId: "2", + OpaqueId: "3", + }, + } + }) + It("fails to deletes a public link permission when it can be resolved", func() { + gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(&getPublicShareResponse, nil) + + err := driveItemPermissionsService.DeletePermission(context.Background(), + *getShareResponse.Share.ResourceId, + "permissionid", + ) + Expect(err).To(MatchError(errorcode.New(errorcode.ItemNotFound, "failed to resolve resource id for shared resource"))) + }) + It("deletes a user permission as expected", func() { + getPublicShareResponse.Status = status.NewNotFound(context.Background(), "") + gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(&getPublicShareResponse, nil) + gatewayClient.On("GetShare", + mock.Anything, + mock.MatchedBy(func(req *collaboration.GetShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), + ).Return(&getShareResponse, nil) + + rmShareMockResponse := &collaboration.RemoveShareResponse{ + Status: status.NewOK(ctx), + } + gatewayClient.On("RemoveShare", + mock.Anything, + mock.MatchedBy(func(req *collaboration.RemoveShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), + ).Return(rmShareMockResponse, nil) + + err := driveItemPermissionsService.DeletePermission(context.Background(), + *getShareResponse.Share.ResourceId, + "permissionid", + ) + Expect(err).ToNot(HaveOccurred()) + }) + It("deletes a link permission as expected", func() { + getPublicShareResponse.Share = &link.PublicShare{ + Id: &link.PublicShareId{ + OpaqueId: "linkpermissionid", + }, + ResourceId: &provider.ResourceId{ + StorageId: "1", + SpaceId: "2", + OpaqueId: "3", + }, + } + gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(&getPublicShareResponse, nil) + + gatewayClient.On("RemovePublicShare", + mock.Anything, + mock.MatchedBy(func(req *link.RemovePublicShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "linkpermissionid" + }), + ).Return( + &link.RemovePublicShareResponse{ + Status: status.NewOK(ctx), + }, nil, + ) + + err := driveItemPermissionsService.DeletePermission(context.Background(), + *getShareResponse.Share.ResourceId, + "linkpermissionid", + ) + Expect(err).ToNot(HaveOccurred()) + }) + It("deletes a space permission as expected", func() { + getPublicShareResponse.Status = status.NewNotFound(context.Background(), "") + gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(&getPublicShareResponse, nil) + + 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 + }) + + err := driveItemPermissionsService.DeletePermission(context.Background(), + provider.ResourceId{ + StorageId: "1", + SpaceId: "2", + OpaqueId: "2", + }, + "u:userid", + ) + Expect(err).ToNot(HaveOccurred()) + }) + + It("fails to delete permission when the item id does not match the shared resource's id", func() { + getPublicShareResponse.Status = status.NewNotFound(context.Background(), "") + gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(&getPublicShareResponse, nil) + getShareResponse.Share.ResourceId = &provider.ResourceId{ + StorageId: "3", + SpaceId: "4", + OpaqueId: "5", + } + gatewayClient.On("GetShare", + mock.Anything, + mock.MatchedBy(func(req *collaboration.GetShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), + ).Return(&getShareResponse, nil) + + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "1$2") + rctx.URLParams.Add("itemID", "1$2!3") + rctx.URLParams.Add("permissionID", "permissionid") + + ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx) + + err := driveItemPermissionsService.DeletePermission(context.Background(), + provider.ResourceId{ + StorageId: "1", + SpaceId: "2", + OpaqueId: "3", + }, + "permissionid", + ) + Expect(err).To(MatchError(errorcode.New(errorcode.InvalidRequest, "permissionID and itemID do not match"))) + + }) + }) }) var _ = Describe("DriveItemPermissionsApi", func() { diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index 05ab324664..3c64d76367 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -391,3 +391,144 @@ func (g BaseGraphService) cs3PublicSharesToDriveItems(ctx context.Context, share return driveItems, nil } + +func (g BaseGraphService) getLinkPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { + share, err := g.getCS3PublicShareByID(ctx, permissionID) + if err != nil { + return nil, err + } + return share.GetResourceId(), nil +} + +func (g BaseGraphService) getCS3PublicShareByID(ctx context.Context, permissionID string) (*link.PublicShare, error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return nil, err + } + + getPublicShareResp, err := gatewayClient.GetPublicShare(ctx, + &link.GetPublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: permissionID, + }, + }, + }, + }, + ) + if errCode := errorcode.FromCS3Status(getPublicShareResp.GetStatus(), err); errCode != nil { + return nil, *errCode + } + return getPublicShareResp.GetShare(), nil +} + +func (g BaseGraphService) removePublicShare(ctx context.Context, permissionID string) error { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return err + } + + removePublicShareResp, err := gatewayClient.RemovePublicShare(ctx, + &link.RemovePublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + if errcode := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); errcode != nil { + return *errcode + } + // We need to return an untyped nil here otherwise the error==nil check won't work + return nil +} + +func (g BaseGraphService) removeUserShare(ctx context.Context, permissionID string) error { + 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_Id{ + Id: &collaboration.ShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + + if errCode := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); errCode != nil { + return *errCode + } + // We need to return an untyped nil here otherwise the error==nil check won't work + 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 errCode := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); errCode != nil { + return *errCode + } + // We need to return an untyped nil here otherwise the error==nil check won't work + return nil +} + +func (g BaseGraphService) getUserPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { + share, err := g.getCS3UserShareByID(ctx, permissionID) + if err != nil { + return nil, err + } + return share.GetResourceId(), nil +} + +func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID string) (*collaboration.Share, error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return nil, err + } + + getShareResp, err := gatewayClient.GetShare(ctx, + &collaboration.GetShareRequest{ + Ref: &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Id{ + Id: &collaboration.ShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + if errCode := errorcode.FromCS3Status(getShareResp.GetStatus(), err); errCode != nil { + return nil, *errCode + } + return getShareResp.GetShare(), nil +} diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 1c8d3e5f4d..bf91b4ddbc 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -18,7 +18,6 @@ import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" - link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/go-chi/chi/v5" @@ -37,15 +36,6 @@ import ( "github.com/owncloud/ocis/v2/services/graph/pkg/validate" ) -type PermissionType int - -const ( - Unknown PermissionType = iota - Public - User - Space -) - // CreateUploadSession create an upload session to allow your app to upload files up to the maximum file size. // An upload session allows your app to upload ranges of the file in sequential API requests, which allows the // transfer to be resumed if a connection is dropped while the upload is in progress. @@ -429,76 +419,6 @@ func (g Graph) UpdatePermission(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, &updatedPermission) } -// DeletePermission removes a Permission from a Drive item -func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { - _, itemID, err := GetDriveAndItemIDParam(r, g.logger) - if err != nil { - errorcode.RenderError(w, r, err) - return - } - - permissionID, err := url.PathUnescape(chi.URLParam(r, "permissionID")) - if err != nil { - g.logger.Debug().Err(err).Msg("could not parse permissionID") - errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid permissionID") - return - } - - ctx := r.Context() - var permissionType PermissionType - - sharedResourceID, err := g.getLinkPermissionResourceID(ctx, permissionID) - switch { - // Check if the ID is referring to a public share - case err == nil: - permissionType = Public - // If the item id is referring to a space root and this is not a public share - // we have to deal with space permissions - case IsSpaceRoot(&itemID): - permissionType = Space - sharedResourceID = &itemID - err = nil - // If this is neither a public share nor a space permission, check if this is a - // user share - default: - sharedResourceID, err = g.getUserPermissionResourceID(ctx, permissionID) - if err == nil { - permissionType = User - } - } - - if err != nil { - errorcode.RenderError(w, r, err) - return - } - - // The resourceID of the shared resource need to match the item ID from the Request Path - // otherwise this is an invalid Request. - if !utils.ResourceIDEqual(sharedResourceID, &itemID) { - g.logger.Debug().Msg("resourceID of shared does not match itemID") - errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "permissionID and itemID do not match") - return - } - - switch permissionType { - case User: - err = g.removeUserShare(ctx, permissionID) - case Public: - err = g.removePublicShare(ctx, permissionID) - case Space: - err = g.removeSpacePermission(ctx, permissionID, sharedResourceID) - } - - if err != nil { - errorcode.RenderError(w, r, err) - return - } - - render.Status(r, http.StatusNoContent) - render.NoContent(w, r) - -} - func (g Graph) getPermissionByID(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId) (*libregraph.Permission, *storageprovider.ResourceId, error) { publicShare, err := g.getCS3PublicShareByID(ctx, permissionID) if err == nil { @@ -552,37 +472,6 @@ func (g Graph) getPermissionByID(ctx context.Context, permissionID string, itemI } -func (g Graph) getUserPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { - shareByID, err := g.getCS3UserShareByID(ctx, permissionID) - if err != nil { - return nil, err - } - return shareByID.GetResourceId(), nil -} - -func (g Graph) getCS3UserShareByID(ctx context.Context, permissionID string) (*collaboration.Share, error) { - gatewayClient, err := g.gatewaySelector.Next() - if err != nil { - g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") - return nil, err - } - - getShareResp, err := gatewayClient.GetShare(ctx, - &collaboration.GetShareRequest{ - Ref: &collaboration.ShareReference{ - Spec: &collaboration.ShareReference_Id{ - Id: &collaboration.ShareId{ - OpaqueId: permissionID, - }, - }, - }, - }) - if errCode := errorcode.FromCS3Status(getShareResp.GetStatus(), err); errCode != nil { - return nil, *errCode - } - return getShareResp.GetShare(), nil -} - func (g Graph) updateUserShare(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId, newPermission *libregraph.Permission) (*libregraph.Permission, error) { gatewayClient, err := g.gatewaySelector.Next() if err != nil { @@ -679,60 +568,6 @@ func (g Graph) updateUserShare(ctx context.Context, permissionID string, itemID return permission, nil } -func (g Graph) removeUserShare(ctx context.Context, permissionID string) error { - 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_Id{ - Id: &collaboration.ShareId{ - OpaqueId: permissionID, - }, - }, - }, - }) - - if errCode := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); errCode != nil { - return *errCode - } - // We need to return an untyped nil here otherwise the error==nil check won't work - return nil -} - -func (g Graph) 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 errCode := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); errCode != nil { - return *errCode - } - // We need to return an untyped nil here otherwise the error==nil check won't work - return nil -} - 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 @@ -758,62 +593,6 @@ func spacePermissionIdToCS3Grantee(permissionID string) (storageprovider.Grantee return grantee, nil } -func (g Graph) getLinkPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { - shareByID, err := g.getCS3PublicShareByID(ctx, permissionID) - if err != nil { - return nil, err - } - return shareByID.GetResourceId(), nil -} - -func (g Graph) getCS3PublicShareByID(ctx context.Context, permissionID string) (*link.PublicShare, error) { - gatewayClient, err := g.gatewaySelector.Next() - if err != nil { - g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") - return nil, err - } - - getPublicShareResp, err := gatewayClient.GetPublicShare(ctx, - &link.GetPublicShareRequest{ - Ref: &link.PublicShareReference{ - Spec: &link.PublicShareReference_Id{ - Id: &link.PublicShareId{ - OpaqueId: permissionID, - }, - }, - }, - }, - ) - if errCode := errorcode.FromCS3Status(getPublicShareResp.GetStatus(), err); errCode != nil { - return nil, *errCode - } - return getPublicShareResp.GetShare(), nil -} - -func (g Graph) removePublicShare(ctx context.Context, permissionID string) error { - gatewayClient, err := g.gatewaySelector.Next() - if err != nil { - g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") - return err - } - - removePublicShareResp, err := gatewayClient.RemovePublicShare(ctx, - &link.RemovePublicShareRequest{ - Ref: &link.PublicShareReference{ - Spec: &link.PublicShareReference_Id{ - Id: &link.PublicShareId{ - OpaqueId: permissionID, - }, - }, - }, - }) - if errcode := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); errcode != nil { - return *errcode - } - // We need to return an untyped nil here otherwise the error==nil check won't work - return 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 { diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index 339a2f2572..19c8666da3 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -49,16 +49,14 @@ type itemsList struct { var _ = Describe("Driveitems", func() { var ( - svc service.Service - ctx context.Context - cfg *config.Config - gatewayClient *cs3mocks.GatewayAPIClient - gatewaySelector pool.Selectable[gateway.GatewayAPIClient] - eventsPublisher mocks.Publisher - identityBackend *identitymocks.Backend - getPublicShareResponse *link.GetPublicShareResponse - getShareResponse *collaboration.GetShareResponse - listSpacesResponse *provider.ListStorageSpacesResponse + svc service.Service + ctx context.Context + cfg *config.Config + gatewayClient *cs3mocks.GatewayAPIClient + gatewaySelector pool.Selectable[gateway.GatewayAPIClient] + eventsPublisher mocks.Publisher + identityBackend *identitymocks.Backend + listSpacesResponse *provider.ListStorageSpacesResponse rr *httptest.ResponseRecorder @@ -83,12 +81,6 @@ var _ = Describe("Driveitems", func() { return gatewayClient }, ) - getPublicShareResponse = &link.GetPublicShareResponse{ - Status: status.NewNotFound(ctx, "not found"), - } - getShareResponse = &collaboration.GetShareResponse{ - Status: status.NewNotFound(ctx, "not found"), - } grantMapJSON, _ := json.Marshal( map[string]*provider.ResourcePermissions{ @@ -137,161 +129,6 @@ var _ = Describe("Driveitems", func() { ) }) - Describe("DeletePermission", func() { - It("deletes a user permission as expected", func() { - gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(getPublicShareResponse, nil) - - getShareResponse.Status = status.NewOK(ctx) - getShareResponse.Share = &collaboration.Share{ - Id: &collaboration.ShareId{ - OpaqueId: "permissionid", - }, - ResourceId: &provider.ResourceId{ - StorageId: "1", - SpaceId: "2", - OpaqueId: "3", - }, - } - gatewayClient.On("GetShare", - mock.Anything, - mock.MatchedBy(func(req *collaboration.GetShareRequest) bool { - return req.GetRef().GetId().GetOpaqueId() == "permissionid" - }), - ).Return(getShareResponse, nil) - - rmShareMock := gatewayClient.On("RemoveShare", - mock.Anything, - mock.MatchedBy(func(req *collaboration.RemoveShareRequest) bool { - return req.GetRef().GetId().GetOpaqueId() == "permissionid" - }), - ) - rmShareMockResponse := &collaboration.RemoveShareResponse{ - Status: status.NewOK(ctx), - } - rmShareMock.Return(rmShareMockResponse, nil) - - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "1$2") - rctx.URLParams.Add("itemID", "1$2!3") - rctx.URLParams.Add("permissionID", "permissionid") - - ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx) - - svc.DeletePermission( - rr, - httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx), - ) - - Expect(rr.Code).To(Equal(http.StatusNoContent)) - }) - It("deletes a link permission as expected", func() { - getPublicShareMock := gatewayClient.On("GetPublicShare", - mock.Anything, - mock.MatchedBy(func(req *link.GetPublicShareRequest) bool { - return req.GetRef().GetId().GetOpaqueId() == "linkpermissionid" - }), - ) - getPublicShareMock.Return(&link.GetPublicShareResponse{ - Status: status.NewOK(ctx), - Share: &link.PublicShare{ - Id: &link.PublicShareId{ - OpaqueId: "permissionid", - }, - ResourceId: &provider.ResourceId{ - StorageId: "1", - SpaceId: "2", - OpaqueId: "3", - }, - }, - }, nil) - - rmPublicShareMock := gatewayClient.On("RemovePublicShare", - mock.Anything, - mock.MatchedBy(func(req *link.RemovePublicShareRequest) bool { - return req.GetRef().GetId().GetOpaqueId() == "linkpermissionid" - }), - ) - rmPublicShareMockResponse := &link.RemovePublicShareResponse{ - Status: status.NewOK(ctx), - } - rmPublicShareMock.Return(rmPublicShareMockResponse, nil) - - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "1$2") - rctx.URLParams.Add("itemID", "1$2!3") - rctx.URLParams.Add("permissionID", "linkpermissionid") - - ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx) - - svc.DeletePermission( - rr, - httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx), - ) - - Expect(rr.Code).To(Equal(http.StatusNoContent)) - }) - It("deletes a space permission as expected", func() { - gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(getPublicShareResponse, nil) - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "1$2") - rctx.URLParams.Add("itemID", "1$2!2") - rctx.URLParams.Add("permissionID", "u:userid") - - 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 - }) - - ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx) - - svc.DeletePermission( - rr, - httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx), - ) - - Expect(rr.Code).To(Equal(http.StatusNoContent)) - }) - - It("fails to delete permission when the item id does not match the shared resource's id", func() { - gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(getPublicShareResponse, nil) - getShareResponse.Status = status.NewOK(ctx) - getShareResponse.Share = &collaboration.Share{ - Id: &collaboration.ShareId{ - OpaqueId: "permissionid", - }, - ResourceId: &provider.ResourceId{ - StorageId: "3", - SpaceId: "4", - OpaqueId: "5", - }, - } - gatewayClient.On("GetShare", - mock.Anything, - mock.MatchedBy(func(req *collaboration.GetShareRequest) bool { - return req.GetRef().GetId().GetOpaqueId() == "permissionid" - }), - ).Return(getShareResponse, nil) - - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "1$2") - rctx.URLParams.Add("itemID", "1$2!3") - rctx.URLParams.Add("permissionID", "permissionid") - - ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx) - - svc.DeletePermission( - rr, - httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx), - ) - - Expect(rr.Code).To(Equal(http.StatusBadRequest)) - }) - }) - Describe("UpdatePermission", func() { var ( driveItemPermission *libregraph.Permission @@ -727,7 +564,7 @@ var _ = Describe("Driveitems", func() { Expect(ok).To(BeFalse()) }) // that is resharing test. Please delete after disable resharing feature - + // It("updates the share permissions with changing the role", func() { // getPublicShareMockResponse.Share = nil // getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found") diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index adea52f9b7..c24dc9703d 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -113,7 +113,6 @@ type Service interface { SetLinkPassword(writer http.ResponseWriter, request *http.Request) UpdatePermission(w http.ResponseWriter, r *http.Request) - DeletePermission(w http.ResponseWriter, r *http.Request) CreateUploadSession(w http.ResponseWriter, r *http.Request) @@ -251,7 +250,7 @@ func NewService(opts ...Option) (Graph, error) { r.Route("/permissions", func(r chi.Router) { r.Get("/", driveItemPermissionsApi.ListPermissions) r.Route("/{permissionID}", func(r chi.Router) { - r.Delete("/", svc.DeletePermission) + r.Delete("/", driveItemPermissionsApi.DeletePermission) r.Patch("/", svc.UpdatePermission) r.Post("/setPassword", svc.SetLinkPassword) })