From 9cf91bf32c2f8e81ac5fecf86ca6072489598f87 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Thu, 15 Feb 2024 23:14:19 +0100 Subject: [PATCH] [full-ci] fix an error when lock/unlock a file --- changelog/unreleased/fix-public-link-lock.md | 6 ++++ .../storageprovider/storageprovider.go | 24 ++++++++++++++ .../http/services/owncloud/ocdav/locks.go | 31 ++++++++++++------- 3 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 changelog/unreleased/fix-public-link-lock.md diff --git a/changelog/unreleased/fix-public-link-lock.md b/changelog/unreleased/fix-public-link-lock.md new file mode 100644 index 00000000000..42a53432446 --- /dev/null +++ b/changelog/unreleased/fix-public-link-lock.md @@ -0,0 +1,6 @@ +Bugfix: Fix an error when lock/unlock a file + +We fixed a bug when anonymous user with viewer role in public link of a folder can lock/unlock a file inside it + +https://github.com/cs3org/reva/pull/4518 +https://github.com/owncloud/ocis/issues/7785 diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index c125698c0b9..3468de59180 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -33,6 +33,7 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" + "github.com/cs3org/reva/v2/pkg/conversions" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/events" @@ -230,6 +231,13 @@ func (s *service) UnsetArbitraryMetadata(ctx context.Context, req *provider.Unse // SetLock puts a lock on the given reference func (s *service) SetLock(ctx context.Context, req *provider.SetLockRequest) (*provider.SetLockResponse, error) { + u := ctxpkg.ContextMustGetUser(ctx) + psr := utils.ReadPlainFromOpaque(u.Opaque, "public-share-role") + if psr != "" && psr != conversions.RoleEditor { + return &provider.SetLockResponse{ + Status: status.NewPermissionDenied(ctx, nil, "no permission to lock the share"), + }, nil + } err := s.storage.SetLock(ctx, req.Ref, req.Lock) return &provider.SetLockResponse{ @@ -249,6 +257,14 @@ func (s *service) GetLock(ctx context.Context, req *provider.GetLockRequest) (*p // RefreshLock refreshes an existing lock on the given reference func (s *service) RefreshLock(ctx context.Context, req *provider.RefreshLockRequest) (*provider.RefreshLockResponse, error) { + u := ctxpkg.ContextMustGetUser(ctx) + psr := utils.ReadPlainFromOpaque(u.Opaque, "public-share-role") + if psr != "" && psr != conversions.RoleEditor { + return &provider.RefreshLockResponse{ + Status: status.NewPermissionDenied(ctx, nil, "no permission to refresh the share lock"), + }, nil + } + err := s.storage.RefreshLock(ctx, req.Ref, req.Lock, req.ExistingLockId) return &provider.RefreshLockResponse{ @@ -258,6 +274,14 @@ func (s *service) RefreshLock(ctx context.Context, req *provider.RefreshLockRequ // Unlock removes an existing lock from the given reference func (s *service) Unlock(ctx context.Context, req *provider.UnlockRequest) (*provider.UnlockResponse, error) { + u := ctxpkg.ContextMustGetUser(ctx) + psr := utils.ReadPlainFromOpaque(u.Opaque, "public-share-role") + if psr != "" && psr != conversions.RoleEditor { + return &provider.UnlockResponse{ + Status: status.NewPermissionDenied(ctx, nil, "no permission to unlock the share"), + }, nil + } + err := s.storage.Unlock(ctx, req.Ref, req.Lock) return &provider.UnlockResponse{ diff --git a/internal/http/services/owncloud/ocdav/locks.go b/internal/http/services/owncloud/ocdav/locks.go index 332b9d22760..e415e6a62d5 100644 --- a/internal/http/services/owncloud/ocdav/locks.go +++ b/internal/http/services/owncloud/ocdav/locks.go @@ -260,11 +260,15 @@ func (cls *cs3LS) Unlock(ctx context.Context, now time.Time, ref *provider.Refer return err } - switch res.Status.Code { + switch res.GetStatus().GetCode() { case rpc.Code_CODE_OK: return nil case rpc.Code_CODE_FAILED_PRECONDITION: return errtypes.Aborted("file is not locked") + case rpc.Code_CODE_LOCKED: + appctx.GetLogger(ctx).Error().Str("token", token).Interface("unlock", ref). + Msg("could not unlock " + res.GetStatus().GetMessage()) + return errtypes.Locked("another token") default: return errtypes.NewErrtypeFromStatus(res.Status) } @@ -639,16 +643,19 @@ func (s *svc) unlockReference(ctx context.Context, _ http.ResponseWriter, r *htt t = t[1 : len(t)-1] } - switch err := s.LockSystem.Unlock(ctx, time.Now(), ref, t); err { - case nil: - return http.StatusNoContent, err - case errors.ErrForbidden: - return http.StatusForbidden, err - case errors.ErrLocked: - return http.StatusLocked, err - case errors.ErrNoSuchLock: - return http.StatusConflict, err - default: - return http.StatusInternalServerError, err + err := s.LockSystem.Unlock(ctx, time.Now(), ref, t) + if err == nil { + return http.StatusNoContent, nil + } else { + switch err.(type) { + case errtypes.Aborted, errtypes.Locked: + return http.StatusLocked, err + case errtypes.PermissionDenied: + return http.StatusForbidden, err + case errtypes.PreconditionFailed: + return http.StatusConflict, err + default: + return http.StatusInternalServerError, err + } } }