From afb127090d04148ab5841d05e67346328c8838c1 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 28 Nov 2023 17:13:30 +0100 Subject: [PATCH] graph sharing: delete link permission Allow to delete link permissions (i.e. Public Shares) --- services/graph/pkg/errorcode/errorcode.go | 4 + services/graph/pkg/service/v0/driveitems.go | 143 +++++++++++++----- .../graph/pkg/service/v0/driveitems_test.go | 103 +++++++------ 3 files changed, 165 insertions(+), 85 deletions(-) diff --git a/services/graph/pkg/errorcode/errorcode.go b/services/graph/pkg/errorcode/errorcode.go index dfa0129e9b..fdaf94033a 100644 --- a/services/graph/pkg/errorcode/errorcode.go +++ b/services/graph/pkg/errorcode/errorcode.go @@ -154,6 +154,10 @@ func (e Error) Error() string { return errString } +func (e Error) GetCode() ErrorCode { + return e.errorCode +} + // RenderError render the Graph Error based on a code or default one func RenderError(w http.ResponseWriter, r *http.Request, err error) { var errcode Error diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 5318e632b3..bdaf1198cd 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -504,14 +504,7 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) { // DeletePermission removes a Permission from a Drive item func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { - gatewayClient, err := g.gatewaySelector.Next() - if err != nil { - g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) - return - } - - _, itemID, err := g.extractDriveAndDriveItem(r) + _, itemID, err := g.GetDriveAndItemIDParam(r) if err != nil { errorcode.RenderError(w, r, err) return @@ -526,6 +519,56 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { } ctx := r.Context() + isUserPermission := true + + // Check if the id is refering to a User Share + sharedResourceId, err := g.getUserPermissionResourceID(ctx, permissionID) + var errcode *errorcode.Error + if err != nil && errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound { + // there is no user share with that ID, so lets check if it is refering to a public link + isUserPermission = false + sharedResourceId, err = g.getLinkPermissionResourceID(ctx, permissionID) + } + + 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 sharedResourceId.GetStorageId() != itemID.GetStorageId() || + sharedResourceId.GetSpaceId() != itemID.GetSpaceId() || + sharedResourceId.GetOpaqueId() != itemID.GetOpaqueId() { + 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 + } + + if isUserPermission { + err = g.removeUserShare(ctx, permissionID) + } else { + err = g.removePublicShare(ctx, permissionID) + } + + if err != nil { + errorcode.RenderError(w, r, err) + return + } + + render.Status(r, http.StatusNoContent) + render.NoContent(w, r) + + return +} + +func (g Graph) getUserPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, 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{ @@ -537,18 +580,16 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { }, }) if errCode := errorcode.FromCS3Status(getShareResp.GetStatus(), err); errCode != nil { - return nil, err + return nil, errCode } + return getShareResp.Share.GetResourceId(), nil +} - sharedResourceId := getShareResp.GetShare().GetResourceId() - // The resourceID of the shared resource need to matched the item ID from the Request Path - // otherwise this is an invalid Request. - if sharedResourceId.GetStorageId() != itemID.GetStorageId() || - sharedResourceId.GetSpaceId() != itemID.GetSpaceId() || - sharedResourceId.GetOpaqueId() != itemID.GetOpaqueId() { - 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 +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, @@ -561,38 +602,60 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { }, }, }) - switch { - case err != nil: - fallthrough - case removeShareResp.Status.GetCode() != cs3rpc.Code_CODE_OK: - g.logger.Debug().Err(err).Interface("permissionID", permissionID).Interface("GetShare", getShareResp).Msg("GetShare failed") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) - return - } - render.Status(r, http.StatusNoContent) - render.NoContent(w, r) - return + 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) extractDriveAndDriveItem(r *http.Request) (driveID storageprovider.ResourceId, itemID storageprovider.ResourceId, err error) { - driveID, err = parseIDParam(r, "driveID") +func (g Graph) getLinkPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { + gatewayClient, err := g.gatewaySelector.Next() if err != nil { - g.logger.Debug().Err(err).Msg("could not parse driveID") - return driveID, itemID, errorcode.New(errorcode.InvalidRequest, "invalid driveID") + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return nil, err } - itemID, err = parseIDParam(r, "itemID") + 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.Share.GetResourceId(), 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("could not parse itemID") - return driveID, itemID, errorcode.New(errorcode.InvalidRequest, "invalid itemID") + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return err } - if driveID.GetStorageId() != itemID.GetStorageId() || driveID.GetSpaceId() != itemID.GetSpaceId() { - g.logger.Debug().Interface("driveID", driveID).Interface("itemID", itemID).Msg("driveID and itemID do not match") - return driveID, itemID, errorcode.New(errorcode.InvalidRequest, "driveID and itemID do not match") + 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 } - return driveID, itemID, nil + // We need to return an untyped nil here otherwise the error==nil check won't work + return nil } func (g Graph) getDriveItem(ctx context.Context, ref storageprovider.Reference) (*libregraph.DriveItem, error) { diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index e34e6f7845..0eab263318 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -105,51 +105,7 @@ var _ = Describe("Driveitems", func() { }) Describe("DeletePermission", func() { - It("validates the driveID", func() { - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "") - - 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)) - }) - - It("validates the itemID", func() { - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "f0042750-23c5-441c-9f2c-ff7c53e5bd2a$cd621428-dfbe-44c1-9393-65bf0dd440a6!cd621428-dfbe-44c1-9393-65bf0dd440a6") - rctx.URLParams.Add("itemID", "") - - 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)) - }) - - It("checks if the itemID and driveID is compatible to each other", func() { - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "1$2!3") - rctx.URLParams.Add("itemID", "4$5!6") - - 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)) - }) - - It("deletes a permission as expected", func() { + It("deletes a user permission as expected", func() { getShareMock := gatewayClient.On("GetShare", mock.Anything, mock.MatchedBy(func(req *collaboration.GetShareRequest) bool { @@ -196,6 +152,63 @@ var _ = Describe("Driveitems", func() { Expect(rr.Code).To(Equal(http.StatusNoContent)) }) + It("deletes a link permission as expected", func() { + getShareMock := gatewayClient.On("GetShare", + mock.Anything, + mock.MatchedBy(func(req *collaboration.GetShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "linkpermissionid" + }), + ) + getShareMockResponse := &collaboration.GetShareResponse{ + Status: status.NewNotFound(ctx, "not found"), + } + getShareMock.Return(getShareMockResponse, nil) + + 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("fails to delete permission when the item id does not match the shared resource's id", func() { getShareMock := gatewayClient.On("GetShare",