From 8aad8dd4d3f055415ef7c69df17893953daeaa9f Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 6 Dec 2023 21:37:02 +0100 Subject: [PATCH 1/7] feat: add setPassword endpoint --- services/graph/pkg/service/v0/links.go | 158 +++++++++++++++++++++++ services/graph/pkg/service/v0/service.go | 2 + 2 files changed, 160 insertions(+) diff --git a/services/graph/pkg/service/v0/links.go b/services/graph/pkg/service/v0/links.go index 035fb1b650..737b081aab 100644 --- a/services/graph/pkg/service/v0/links.go +++ b/services/graph/pkg/service/v0/links.go @@ -14,6 +14,8 @@ import ( link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "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/services/graph/pkg/errorcode" @@ -55,6 +57,52 @@ func (g Graph) CreateLink(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, *perm) } +// SetLinkPassword sets public link password on the cs3 api +func (g Graph) SetLinkPassword(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + _, itemID, err := g.GetDriveAndItemIDParam(r) + 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 + } + + password := &libregraph.SharingLinkPassword{} + if err := StrictJSONUnmarshal(r.Body, password); err != nil { + g.logger.Debug().Err(err).Interface("Body", r.Body).Msg("failed unmarshalling request body") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid request body") + return + } + + publicShare, err := g.getCS3PublicShareByID(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 !utils.ResourceIDEqual(publicShare.GetResourceId(), &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 + } + + newPermission, err := g.updatePublicLinkPassword(ctx, permissionID, password.GetPassword()) + if err != nil { + errorcode.RenderError(w, r, err) + } + + render.Status(r, http.StatusOK) + render.JSON(w, r, *newPermission) +} + func (g Graph) createLink(ctx context.Context, driveItemID *providerv1beta1.ResourceId, createLink libregraph.DriveItemCreateLink) (*link.PublicShare, error) { gatewayClient, err := g.gatewaySelector.Next() if err != nil { @@ -146,6 +194,9 @@ func (g Graph) libreGraphPermissionFromCS3PublicShare(createdLink *link.PublicSh if createdLink.GetExpiration() != nil { perm.SetExpirationDateTime(cs3TimestampToTime(createdLink.GetExpiration()).UTC()) } + + perm.SetHasPassword(createdLink.GetPasswordProtected()) + return perm, nil } @@ -165,3 +216,110 @@ func parseAndFillUpTime(t *time.Time) *types.Timestamp { Nanos: uint32(final % 1000000000), } } + +func (g Graph) updatePublicLinkPassword(ctx context.Context, permissionID string, password string) (*libregraph.Permission, error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + return nil, err + } + + changeLinkRes, err := gatewayClient.UpdatePublicShare(ctx, &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PASSWORD, + Grant: &link.Grant{ + Password: password, + }, + }, + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + if errCode := errorcode.FromCS3Status(changeLinkRes.GetStatus(), err); errCode != nil { + return nil, *errCode + } + permission, err := g.libreGraphPermissionFromCS3PublicShare(changeLinkRes.GetShare()) + if err != nil { + return nil, err + } + return permission, nil +} + +func (g Graph) updatePublicLinkPermission(ctx context.Context, permissionID string, info *providerv1beta1.ResourceInfo, newPermission *libregraph.Permission) (perm *libregraph.Permission, err error) { + if newPermission.HasExpirationDateTime() { + expirationDate := newPermission.GetExpirationDateTime() + update := &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_EXPIRATION, + Grant: &link.Grant{Expiration: parseAndFillUpTime(&expirationDate)}, + } + perm, err = g.updatePublicLink(ctx, permissionID, update) + if err != nil { + return nil, err + } + } + + if newPermission.HasLink() && newPermission.Link.HasLibreGraphDisplayName() { + changedLink := newPermission.GetLink() + update := &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_DISPLAYNAME, + DisplayName: changedLink.GetLibreGraphDisplayName(), + } + perm, err = g.updatePublicLink(ctx, permissionID, update) + if err != nil { + return nil, err + } + } + + if newPermission.HasLink() && newPermission.Link.HasType() { + changedLink := newPermission.Link.GetType() + permissions, err := linktype.CS3ResourcePermissionsFromSharingLink( + libregraph.DriveItemCreateLink{ + Type: &changedLink, + }, + info.GetType(), + ) + update := &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{Permissions: permissions}, + }, + } + perm, err = g.updatePublicLink(ctx, permissionID, update) + if err != nil { + return nil, err + } + } + + return perm, err +} + +func (g Graph) updatePublicLink(ctx context.Context, permissionID string, update *link.UpdatePublicShareRequest_Update) (*libregraph.Permission, error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + return nil, err + } + + changeLinkRes, err := gatewayClient.UpdatePublicShare(ctx, &link.UpdatePublicShareRequest{ + Update: update, + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + + if errCode := errorcode.FromCS3Status(changeLinkRes.GetStatus(), err); errCode != nil { + return nil, *errCode + } + + permission, err := g.libreGraphPermissionFromCS3PublicShare(changeLinkRes.GetShare()) + if err != nil { + return nil, err + } + return permission, nil +} diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index fd0679bd58..b662ce3bd8 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -110,6 +110,7 @@ type Service interface { GetDriveItem(w http.ResponseWriter, r *http.Request) GetDriveItemChildren(w http.ResponseWriter, r *http.Request) CreateLink(w http.ResponseWriter, r *http.Request) + SetLinkPassword(writer http.ResponseWriter, request *http.Request) Invite(w http.ResponseWriter, r *http.Request) ListPermissions(w http.ResponseWriter, r *http.Request) @@ -218,6 +219,7 @@ func NewService(opts ...Option) (Graph, error) { r.Route("/{permissionID}", func(r chi.Router) { r.Delete("/", svc.DeletePermission) r.Patch("/", svc.UpdatePermission) + r.Post("/setPassword", svc.SetLinkPassword) }) }) r.Post("/createLink", svc.CreateLink) From fe48dc148a62f8596aa6bf4d76815e2882038d76 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Fri, 8 Dec 2023 18:08:23 +0100 Subject: [PATCH 2/7] feat: implement edit public link --- services/graph/pkg/service/v0/driveitems.go | 11 +++++++-- services/graph/pkg/service/v0/links.go | 27 +++++++++++++++++++-- services/graph/pkg/validate/libregraph.go | 4 +++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 83406696f5..4f9432c558 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -654,11 +654,18 @@ func (g Graph) UpdatePermission(w http.ResponseWriter, r *http.Request) { return } - // We don't implement updating link permissions yet + // This is a public link if _, ok := oldPermission.GetLinkOk(); ok { - errorcode.NotSupported.Render(w, r, http.StatusNotImplemented, "not implemented") + updatedPermission, err := g.updatePublicLinkPermission(ctx, permissionID, &itemID, permission) + if err != nil { + errorcode.RenderError(w, r, err) + return + } + render.Status(r, http.StatusOK) + render.JSON(w, r, &updatedPermission) return } + // This is a user share updatedPermission, err := g.updateUserShare(ctx, permissionID, oldPermission, permission) if err != nil { diff --git a/services/graph/pkg/service/v0/links.go b/services/graph/pkg/service/v0/links.go index 737b081aab..d450975ab3 100644 --- a/services/graph/pkg/service/v0/links.go +++ b/services/graph/pkg/service/v0/links.go @@ -248,7 +248,30 @@ func (g Graph) updatePublicLinkPassword(ctx context.Context, permissionID string return permission, nil } -func (g Graph) updatePublicLinkPermission(ctx context.Context, permissionID string, info *providerv1beta1.ResourceInfo, newPermission *libregraph.Permission) (perm *libregraph.Permission, err error) { +func (g Graph) updatePublicLinkPermission(ctx context.Context, permissionID string, itemID *providerv1beta1.ResourceId, newPermission *libregraph.Permission) (perm *libregraph.Permission, err error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Error().Err(err).Msg("could not select next gateway client") + return nil, errorcode.New(errorcode.GeneralException, err.Error()) + } + + statResp, err := gatewayClient.Stat( + ctx, + &providerv1beta1.StatRequest{ + Ref: &providerv1beta1.Reference{ + ResourceId: itemID, + Path: ".", + }, + }) + if err != nil { + g.logger.Error().Err(err).Msg("transport error, could not stat resource") + return nil, errorcode.New(errorcode.GeneralException, err.Error()) + } + if code := statResp.GetStatus().GetCode(); code != rpc.Code_CODE_OK { + g.logger.Debug().Interface("itemID", itemID).Msg(statResp.GetStatus().GetMessage()) + return nil, errorcode.New(cs3StatusToErrCode(code), statResp.GetStatus().GetMessage()) + } + if newPermission.HasExpirationDateTime() { expirationDate := newPermission.GetExpirationDateTime() update := &link.UpdatePublicShareRequest_Update{ @@ -279,7 +302,7 @@ func (g Graph) updatePublicLinkPermission(ctx context.Context, permissionID stri libregraph.DriveItemCreateLink{ Type: &changedLink, }, - info.GetType(), + statResp.GetInfo().GetType(), ) update := &link.UpdatePublicShareRequest_Update{ Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, diff --git a/services/graph/pkg/validate/libregraph.go b/services/graph/pkg/validate/libregraph.go index 3cd523d078..ebfec9724a 100644 --- a/services/graph/pkg/validate/libregraph.go +++ b/services/graph/pkg/validate/libregraph.go @@ -47,6 +47,10 @@ func permission(v *validator.Validate) { sl.ReportError(permission.Id, "Id", "Id", "readonly", "") } + if _, ok := permission.GetHasPasswordOk(); ok { + sl.ReportError(permission.HasPassword, "hasPassword", "HasPassword", "readonly", "") + } + rolesAndActions(sl, permission.Roles, permission.LibreGraphPermissionsActions, true) }, s) } From 3931578d4cdf6cb49dd2e501472fa4f8c2ea1c43 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Sun, 10 Dec 2023 21:49:45 +0100 Subject: [PATCH 3/7] test: fix existing tests --- services/graph/pkg/linktype/linktype.go | 3 + .../graph/pkg/service/v0/driveitems_test.go | 75 +++++++++++++++++-- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/services/graph/pkg/linktype/linktype.go b/services/graph/pkg/linktype/linktype.go index e73cef6688..9b36b6cec7 100644 --- a/services/graph/pkg/linktype/linktype.go +++ b/services/graph/pkg/linktype/linktype.go @@ -31,6 +31,9 @@ func (l *LinkType) GetPermissions() *provider.ResourcePermissions { // SharingLinkTypeFromCS3Permissions creates a libregraph link type // It returns a list of libregraph actions when the conversion is not possible func SharingLinkTypeFromCS3Permissions(permissions *linkv1beta1.PublicSharePermissions) (*libregraph.SharingLinkType, []string) { + if permissions == nil { + return nil, nil + } linkTypes := GetAvailableLinkTypes() for _, linkType := range linkTypes { if grants.PermissionsEqual(linkType.GetPermissions(), permissions.GetPermissions()) { diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index 909d6ed3ff..3c6cc6cb3c 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -22,6 +22,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" libregraph "github.com/owncloud/libre-graph-api-go" + "github.com/owncloud/ocis/v2/services/graph/pkg/linktype" "github.com/stretchr/testify/mock" "github.com/tidwall/gjson" "google.golang.org/grpc" @@ -250,11 +251,12 @@ var _ = Describe("Driveitems", func() { Describe("UpdatePermission", func() { var ( - driveItemPermission *libregraph.Permission - getShareMockResponse *collaboration.GetShareResponse - getPublicShareMockResponse *link.GetPublicShareResponse - getUserMockResponse *user.GetUserResponse - updateShareMockResponse *collaboration.UpdateShareResponse + driveItemPermission *libregraph.Permission + getShareMockResponse *collaboration.GetShareResponse + getPublicShareMockResponse *link.GetPublicShareResponse + getUserMockResponse *user.GetUserResponse + updateShareMockResponse *collaboration.UpdateShareResponse + updatePublicShareMockResponse *link.UpdatePublicShareResponse ) BeforeEach(func() { rr = httptest.NewRecorder() @@ -317,6 +319,11 @@ var _ = Describe("Driveitems", func() { Share: share, } + updatePublicShareMockResponse = &link.UpdatePublicShareResponse{ + Status: status.NewOK(ctx), + Share: &link.PublicShare{DisplayName: "Test Link"}, + } + getPublicShareMock := gatewayClient.On("GetPublicShare", mock.Anything, mock.MatchedBy(func(req *link.GetPublicShareRequest) bool { @@ -335,13 +342,39 @@ var _ = Describe("Driveitems", func() { OpaqueId: "3", }, Permissions: &link.PublicSharePermissions{ - Permissions: roleconversions.NewViewerRole(true).CS3ResourcePermissions(), + Permissions: linktype.NewViewLinkPermissionSet().GetPermissions(), }, Token: "token", }, } getPublicShareMock.Return(getPublicShareMockResponse, nil) + statMock := gatewayClient.On("Stat", + mock.Anything, + mock.MatchedBy(func(req *provider.StatRequest) bool { + return utils.ResourceIDEqual( + req.GetRef().GetResourceId(), + &provider.ResourceId{ + StorageId: "1", + SpaceId: "2", + OpaqueId: "3", + }, + ) + })) + + statResponse := &provider.StatResponse{ + Status: status.NewOK(ctx), + Info: &provider.ResourceInfo{ + Id: &provider.ResourceId{ + StorageId: "1", + SpaceId: "2", + OpaqueId: "3", + }, + Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, + }, + } + + statMock.Return(statResponse, nil) }) It("fails when no share is found", func() { getShareMockResponse.Share = nil @@ -359,10 +392,26 @@ var _ = Describe("Driveitems", func() { Expect(rr.Code).To(Equal(http.StatusNotFound)) }) // Updating a public link will be implemented later - It("fails when trying to update a link permission", func() { + It("succeeds when trying to update a link permission with displayname", func() { getShareMockResponse.Share = nil getShareMockResponse.Status = status.NewNotFound(ctx, "not found") + updatePublicShareMock := gatewayClient.On("UpdatePublicShare", + mock.Anything, + mock.MatchedBy(func(req *link.UpdatePublicShareRequest) bool { + if req.GetRef().GetId().GetOpaqueId() == "permissionid" { + return req.Update.GetDisplayName() == "Test Link" + } + return false + }), + ) + + updatePublicShareMock.Return(updatePublicShareMockResponse, nil) + + link := libregraph.NewSharingLink() + link.SetLibreGraphDisplayName("Test Link") + + driveItemPermission.SetLink(*link) body, err := driveItemPermission.MarshalJSON() Expect(err).To(BeNil()) svc.UpdatePermission( @@ -370,7 +419,17 @@ var _ = Describe("Driveitems", func() { httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))). WithContext(ctx), ) - Expect(rr.Code).To(Equal(http.StatusNotImplemented)) + Expect(rr.Code).To(Equal(http.StatusOK)) + + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.Permission{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + Expect(res.Link).ToNot(BeNil()) + Expect(res.Link.GetLibreGraphDisplayName() == "Test Link") }) It("fails updating the id", func() { driveItemPermission.SetId("permissionid") From 416b518873e89ac8cdb33655b9a5415345607deb Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Mon, 11 Dec 2023 21:23:37 +0100 Subject: [PATCH 4/7] fix: wrong error code for failed precondition --- services/graph/pkg/errorcode/cs3.go | 2 +- services/graph/pkg/errorcode/cs3_test.go | 2 +- services/graph/pkg/errorcode/errorcode.go | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/services/graph/pkg/errorcode/cs3.go b/services/graph/pkg/errorcode/cs3.go index 756cf271a4..12bd7b5c8b 100644 --- a/services/graph/pkg/errorcode/cs3.go +++ b/services/graph/pkg/errorcode/cs3.go @@ -45,7 +45,7 @@ func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) *E case code == cs3rpc.Code_CODE_ALREADY_EXISTS: err.errorCode = NameAlreadyExists case code == cs3rpc.Code_CODE_FAILED_PRECONDITION: - err.errorCode = PreconditionFailed + err.errorCode = InvalidRequest case code == cs3rpc.Code_CODE_OUT_OF_RANGE: err.errorCode = InvalidRange case code == cs3rpc.Code_CODE_UNIMPLEMENTED: diff --git a/services/graph/pkg/errorcode/cs3_test.go b/services/graph/pkg/errorcode/cs3_test.go index 22616342e5..13184531ec 100644 --- a/services/graph/pkg/errorcode/cs3_test.go +++ b/services/graph/pkg/errorcode/cs3_test.go @@ -29,7 +29,7 @@ func TestFromCS3Status(t *testing.T) { {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAUTHENTICATED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.Unauthenticated, "msg"))}, {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID_ARGUMENT, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "msg"))}, {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ALREADY_EXISTS, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.NameAlreadyExists, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.PreconditionFailed, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "msg"))}, {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNIMPLEMENTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.NotSupported, "msg"))}, {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, {&cs3rpc.Status{Code: cs3rpc.Code_CODE_CANCELLED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, diff --git a/services/graph/pkg/errorcode/errorcode.go b/services/graph/pkg/errorcode/errorcode.go index a94fa3de59..14db1bb0a5 100644 --- a/services/graph/pkg/errorcode/errorcode.go +++ b/services/graph/pkg/errorcode/errorcode.go @@ -135,6 +135,8 @@ func (e Error) Render(w http.ResponseWriter, r *http.Request) { status = http.StatusMethodNotAllowed case ItemIsLocked: status = http.StatusLocked + case PreconditionFailed: + status = http.StatusPreconditionFailed default: status = http.StatusInternalServerError } From e7a7b3001de61d5218b97457c211a21131db11d1 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Mon, 11 Dec 2023 21:25:12 +0100 Subject: [PATCH 5/7] style: refactor error handling --- services/graph/pkg/service/v0/links.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/services/graph/pkg/service/v0/links.go b/services/graph/pkg/service/v0/links.go index d450975ab3..96b0619ddf 100644 --- a/services/graph/pkg/service/v0/links.go +++ b/services/graph/pkg/service/v0/links.go @@ -263,13 +263,9 @@ func (g Graph) updatePublicLinkPermission(ctx context.Context, permissionID stri Path: ".", }, }) - if err != nil { - g.logger.Error().Err(err).Msg("transport error, could not stat resource") - return nil, errorcode.New(errorcode.GeneralException, err.Error()) - } - if code := statResp.GetStatus().GetCode(); code != rpc.Code_CODE_OK { - g.logger.Debug().Interface("itemID", itemID).Msg(statResp.GetStatus().GetMessage()) - return nil, errorcode.New(cs3StatusToErrCode(code), statResp.GetStatus().GetMessage()) + + if errCode := errorcode.FromCS3Status(statResp.GetStatus(), err); errCode != nil { + return nil, *errCode } if newPermission.HasExpirationDateTime() { From 33feab88a72d706e7b6bb312a11c9be11676e7b0 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Mon, 11 Dec 2023 21:25:39 +0100 Subject: [PATCH 6/7] test: add more tests --- .../graph/pkg/service/v0/driveitems_test.go | 309 +++++++++++++++++- 1 file changed, 303 insertions(+), 6 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index 3c6cc6cb3c..0fbca67f9d 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -22,6 +22,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" libregraph "github.com/owncloud/libre-graph-api-go" + "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" "github.com/owncloud/ocis/v2/services/graph/pkg/linktype" "github.com/stretchr/testify/mock" "github.com/tidwall/gjson" @@ -258,6 +259,7 @@ var _ = Describe("Driveitems", func() { updateShareMockResponse *collaboration.UpdateShareResponse updatePublicShareMockResponse *link.UpdatePublicShareResponse ) + const TestLinkName = "Test Link" BeforeEach(func() { rr = httptest.NewRecorder() @@ -321,7 +323,7 @@ var _ = Describe("Driveitems", func() { updatePublicShareMockResponse = &link.UpdatePublicShareResponse{ Status: status.NewOK(ctx), - Share: &link.PublicShare{DisplayName: "Test Link"}, + Share: &link.PublicShare{DisplayName: TestLinkName}, } getPublicShareMock := gatewayClient.On("GetPublicShare", @@ -359,7 +361,7 @@ var _ = Describe("Driveitems", func() { SpaceId: "2", OpaqueId: "3", }, - ) + ) && req.GetRef().GetPath() == "." })) statResponse := &provider.StatResponse{ @@ -391,7 +393,144 @@ var _ = Describe("Driveitems", func() { ) Expect(rr.Code).To(Equal(http.StatusNotFound)) }) - // Updating a public link will be implemented later + It("fails to update permission when no request body is sent", func() { + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", nil). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) + It("fails to update password when no request body is sent", func() { + svc.SetLinkPassword( + rr, + httptest.NewRequest(http.MethodPatch, "/", nil). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) + It("fails to update password when itemID mismatches with the driveID", func() { + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "1$2") + rctx.URLParams.Add("itemID", "1$4!3") + rctx.URLParams.Add("permissionID", "permissionid") + + ctx = context.WithValue(ctx, chi.RouteCtxKey, rctx) + ctx = revactx.ContextSetUser(ctx, currentUser) + svc.SetLinkPassword( + rr, + httptest.NewRequest(http.MethodPatch, "/", nil). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusNotFound)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.OdataError{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + Expect(res.Error.Code).To(Equal(errorcode.ItemNotFound.String())) + Expect(res.Error.Message).To(Equal("driveID and itemID do not match")) + }) + It("fails to update permission when itemID mismatches with the driveID", func() { + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "1$2") + rctx.URLParams.Add("itemID", "1$4!3") + rctx.URLParams.Add("permissionID", "permissionid") + + ctx = context.WithValue(ctx, chi.RouteCtxKey, rctx) + ctx = revactx.ContextSetUser(ctx, currentUser) + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", nil). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusNotFound)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.OdataError{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + Expect(res.Error.Code).To(Equal(errorcode.ItemNotFound.String())) + Expect(res.Error.Message).To(Equal("driveID and itemID do not match")) + }) + It("fails to update permission when the resourceID mismatches with the shared resource's id", func() { + getShareMockResponse.Share = nil + getShareMockResponse.Status = status.NewNotFound(ctx, "not found") + getPublicShareMockResponse.Share.ResourceId = &provider.ResourceId{ + StorageId: "1", + SpaceId: "2", + OpaqueId: "4", + } + + driveItemPermission.SetExpirationDateTime(time.Now().Add(time.Hour)) + body, err := driveItemPermission.MarshalJSON() + Expect(err).To(BeNil()) + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.OdataError{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + }) + It("fails to update public link password when the permissionID is not parseable", func() { + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "1$2") + rctx.URLParams.Add("itemID", "1$2!3") + rctx.URLParams.Add("permissionID", "permi%ssionid") + + ctx = context.WithValue(ctx, chi.RouteCtxKey, rctx) + ctx = revactx.ContextSetUser(ctx, currentUser) + svc.SetLinkPassword( + rr, + httptest.NewRequest(http.MethodPatch, "/", nil). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.OdataError{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + Expect(res.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + Expect(res.Error.Message).To(Equal("invalid permissionID")) + }) + It("fails to update permission when the permissionID is not parseable", func() { + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "1$2") + rctx.URLParams.Add("itemID", "1$2!3") + rctx.URLParams.Add("permissionID", "permi%ssionid") + + ctx = context.WithValue(ctx, chi.RouteCtxKey, rctx) + ctx = revactx.ContextSetUser(ctx, currentUser) + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", nil). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.OdataError{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + Expect(res.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + Expect(res.Error.Message).To(Equal("invalid permissionID")) + }) It("succeeds when trying to update a link permission with displayname", func() { getShareMockResponse.Share = nil getShareMockResponse.Status = status.NewNotFound(ctx, "not found") @@ -400,7 +539,7 @@ var _ = Describe("Driveitems", func() { mock.Anything, mock.MatchedBy(func(req *link.UpdatePublicShareRequest) bool { if req.GetRef().GetId().GetOpaqueId() == "permissionid" { - return req.Update.GetDisplayName() == "Test Link" + return req.Update.GetDisplayName() == TestLinkName } return false }), @@ -409,7 +548,7 @@ var _ = Describe("Driveitems", func() { updatePublicShareMock.Return(updatePublicShareMockResponse, nil) link := libregraph.NewSharingLink() - link.SetLibreGraphDisplayName("Test Link") + link.SetLibreGraphDisplayName(TestLinkName) driveItemPermission.SetLink(*link) body, err := driveItemPermission.MarshalJSON() @@ -429,7 +568,7 @@ var _ = Describe("Driveitems", func() { err = json.Unmarshal(data, &res) Expect(err).ToNot(HaveOccurred()) Expect(res.Link).ToNot(BeNil()) - Expect(res.Link.GetLibreGraphDisplayName() == "Test Link") + Expect(res.Link.GetLibreGraphDisplayName() == TestLinkName) }) It("fails updating the id", func() { driveItemPermission.SetId("permissionid") @@ -442,6 +581,17 @@ var _ = Describe("Driveitems", func() { ) Expect(rr.Code).To(Equal(http.StatusBadRequest)) }) + It("fails updating the password flag", func() { + driveItemPermission.SetHasPassword(true) + body, err := driveItemPermission.MarshalJSON() + Expect(err).To(BeNil()) + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) It("updates the expiration date", func() { expiration := time.Now().Add(time.Hour) updateShareMock := gatewayClient.On("UpdateShare", @@ -569,6 +719,153 @@ var _ = Describe("Driveitems", func() { _, ok = res.GetLibreGraphPermissionsActionsOk() Expect(ok).To(BeTrue()) }) + It("updates the expiration date on a public share", func() { + getShareMockResponse.Share = nil + getShareMockResponse.Status = status.NewNotFound(ctx, "not found") + + expiration := time.Now().UTC().Add(time.Hour) + updatePublicShareMock := gatewayClient.On("UpdatePublicShare", + mock.Anything, + mock.MatchedBy(func(req *link.UpdatePublicShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), + ) + + updatePublicShareMockResponse.Share.Expiration = utils.TimeToTS(expiration) + updatePublicShareMock.Return(updatePublicShareMockResponse, nil) + + driveItemPermission.SetExpirationDateTime(expiration) + body, err := driveItemPermission.MarshalJSON() + Expect(err).To(BeNil()) + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusOK)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.Permission{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetExpirationDateTime().Equal(expiration)).To(BeTrue()) + }) + It("updates the permissions on a public share", func() { + getShareMockResponse.Share = nil + getShareMockResponse.Status = status.NewNotFound(ctx, "not found") + + newLink := libregraph.NewSharingLink() + newLinkType, err := libregraph.NewSharingLinkTypeFromValue("edit") + Expect(err).ToNot(HaveOccurred()) + newLink.SetType(*newLinkType) + + updatePublicShareMock := gatewayClient.On("UpdatePublicShare", + mock.Anything, + mock.MatchedBy(func(req *link.UpdatePublicShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), + ) + + updatePublicShareMockResponse.Share.Permissions = &link.PublicSharePermissions{ + Permissions: linktype.NewFolderEditLinkPermissionSet().Permissions, + } + updatePublicShareMock.Return(updatePublicShareMockResponse, nil) + + driveItemPermission.SetLink(*newLink) + body, err := driveItemPermission.MarshalJSON() + Expect(err).To(BeNil()) + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusOK)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.Permission{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + linkType := res.Link.GetType() + Expect(string(linkType)).To(Equal("edit")) + }) + It("updates the password on a public share", func() { + getShareMockResponse.Share = nil + getShareMockResponse.Status = status.NewNotFound(ctx, "not found") + + newLinkPassword := libregraph.NewSharingLinkPassword() + newLinkPassword.SetPassword("OC123!") + + updatePublicShareMock := gatewayClient.On("UpdatePublicShare", + mock.Anything, + mock.MatchedBy(func(req *link.UpdatePublicShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), + ) + + updatePublicShareMockResponse.Share.Permissions = &link.PublicSharePermissions{ + Permissions: linktype.NewViewLinkPermissionSet().Permissions, + } + updatePublicShareMockResponse.Share.PasswordProtected = true + updatePublicShareMock.Return(updatePublicShareMockResponse, nil) + + body, err := newLinkPassword.MarshalJSON() + Expect(err).To(BeNil()) + svc.SetLinkPassword( + rr, + httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusOK)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.Permission{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + linkType := res.Link.GetType() + Expect(string(linkType)).To(Equal("view")) + Expect(*res.HasPassword).To(BeTrue()) + }) + It("fails when updating the expiration date on a public share", func() { + getShareMockResponse.Share = nil + getShareMockResponse.Status = status.NewNotFound(ctx, "not found") + + expiration := time.Now().UTC().AddDate(0, 0, -1) + updatePublicShareMock := gatewayClient.On("UpdatePublicShare", + mock.Anything, + mock.MatchedBy(func(req *link.UpdatePublicShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), + ) + + updatePublicShareMockResponse.Share = nil + updatePublicShareMockResponse.Status = status.NewFailedPrecondition(ctx, nil, "expiration date is in the past") + updatePublicShareMock.Return(updatePublicShareMockResponse, nil) + + driveItemPermission.SetExpirationDateTime(expiration) + body, err := driveItemPermission.MarshalJSON() + Expect(err).To(BeNil()) + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := libregraph.OdataError{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetError().Code).To(Equal(errorcode.InvalidRequest.String())) + Expect(res.GetError().Message).To(Equal("expiration date is in the past")) + }) }) Describe("Invite", func() { From 615cc2071fec54d20f77f41a14065a974e8e4979 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Mon, 11 Dec 2023 22:55:07 +0100 Subject: [PATCH 7/7] docs: add changelog --- changelog/unreleased/add-edit-public-share.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/add-edit-public-share.md diff --git a/changelog/unreleased/add-edit-public-share.md b/changelog/unreleased/add-edit-public-share.md new file mode 100644 index 0000000000..3a751b0d63 --- /dev/null +++ b/changelog/unreleased/add-edit-public-share.md @@ -0,0 +1,6 @@ +Enhancement: Add edit public share to sharing NG + +We added the ability to edit public shares to the sharing NG endpoints. + +https://github.com/owncloud/ocis/pull/7908/ +https://github.com/owncloud/ocis/issues/6993