diff --git a/changelog/unreleased/fix-wopi-on-edge.md b/changelog/unreleased/fix-wopi-on-edge.md new file mode 100644 index 0000000000..d3c9149b4d --- /dev/null +++ b/changelog/unreleased/fix-wopi-on-edge.md @@ -0,0 +1,8 @@ +Bugfix: Fix locking and public link scope checker to make the WOPI server work + +We've fixed the locking implementation to use the CS3api instead of the temporary opaque values. +We've fixed the scope checker on public links to allow the OpenInApp actions. + +These fixes have been done to use the cs3org/wopiserver with REVA edge. + +https://github.com/cs3org/reva/pull/2721 diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 75fbcab4d0..91868a7b5a 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -165,7 +165,6 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s } } } - return errtypes.PermissionDenied("access to resource not allowed within the assigned scope") } @@ -257,9 +256,9 @@ func extractRef(req interface{}, hasEditorRole bool) (*provider.Reference, bool) // App provider requests case *appregistry.GetAppProvidersRequest: - return &provider.Reference{ResourceId: v.ResourceInfo.Id, Path: v.ResourceInfo.Path}, true + return &provider.Reference{ResourceId: v.ResourceInfo.Id}, true case *appprovider.OpenInAppRequest: - return &provider.Reference{ResourceId: v.ResourceInfo.Id, Path: v.ResourceInfo.Path}, true + return &provider.Reference{ResourceId: v.ResourceInfo.Id}, true case *gateway.OpenInAppRequest: return v.GetRef(), true } diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index 4a4c10d181..da3f291e18 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -187,12 +187,7 @@ func registerMimeTypes(mimes map[string]string) { } func (s *service) SetArbitraryMetadata(ctx context.Context, req *provider.SetArbitraryMetadataRequest) (*provider.SetArbitraryMetadataResponse, error) { - // FIXME these should be part of the SetArbitraryMetadataRequest object - if req.Opaque != nil { - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } - } + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) err := s.storage.SetArbitraryMetadata(ctx, req.Ref, req.ArbitraryMetadata) @@ -202,12 +197,7 @@ func (s *service) SetArbitraryMetadata(ctx context.Context, req *provider.SetArb } func (s *service) UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArbitraryMetadataRequest) (*provider.UnsetArbitraryMetadataResponse, error) { - // FIXME these should be part of the UnsetArbitraryMetadataRequest object - if req.Opaque != nil { - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } - } + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) err := s.storage.UnsetArbitraryMetadata(ctx, req.Ref, req.ArbitraryMetadataKeys) @@ -317,12 +307,7 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate metadata["if-match"] = ifMatch } - // FIXME these should be part of the InitiateFileUploadRequest object - if req.Opaque != nil { - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } - } + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) var uploadLength int64 if req.Opaque != nil && req.Opaque.Map != nil { @@ -589,6 +574,8 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro }, nil } + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) + // check DeleteRequest for any known opaque properties. // FIXME these should be part of the DeleteRequest object if req.Opaque != nil { @@ -596,9 +583,6 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro // it is a binary key; its existence signals true. Although, do not assume. ctx = context.WithValue(ctx, appctx.DeletingSharedResource, true) } - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } } err := s.storage.Delete(ctx, req.Ref) @@ -609,12 +593,8 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro } func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provider.MoveResponse, error) { - // FIXME these should be part of the MoveRequest object - if req.Opaque != nil { - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } - } + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) + err := s.storage.Move(ctx, req.Source, req.Destination) return &provider.MoveResponse{ @@ -705,12 +685,8 @@ func (s *service) ListFileVersions(ctx context.Context, req *provider.ListFileVe } func (s *service) RestoreFileVersion(ctx context.Context, req *provider.RestoreFileVersionRequest) (*provider.RestoreFileVersionResponse, error) { - // FIXME these should be part of the RestoreFileVersionRequest object - if req.Opaque != nil { - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } - } + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) + err := s.storage.RestoreRevision(ctx, req.Ref, req.Key) return &provider.RestoreFileVersionResponse{ @@ -797,12 +773,7 @@ func (s *service) ListRecycle(ctx context.Context, req *provider.ListRecycleRequ } func (s *service) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecycleItemRequest) (*provider.RestoreRecycleItemResponse, error) { - // FIXME these should be part of the RestoreRecycleItemRequest object - if req.Opaque != nil { - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } - } + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) // TODO(labkode): CRITICAL: fill recycle info with storage provider. key, itemPath := router.ShiftPath(req.Key) @@ -931,6 +902,8 @@ func (s *service) DenyGrant(ctx context.Context, req *provider.DenyGrantRequest) } func (s *service) AddGrant(ctx context.Context, req *provider.AddGrantRequest) (*provider.AddGrantResponse, error) { + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) + // TODO: update CS3 APIs // FIXME these should be part of the AddGrantRequest object if req.Opaque != nil { @@ -938,9 +911,6 @@ func (s *service) AddGrant(ctx context.Context, req *provider.AddGrantRequest) ( if spacegrant { ctx = context.WithValue(ctx, utils.SpaceGrant, struct{}{}) } - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } } // check grantee type is valid @@ -980,12 +950,7 @@ func (s *service) UpdateGrant(ctx context.Context, req *provider.UpdateGrantRequ } func (s *service) RemoveGrant(ctx context.Context, req *provider.RemoveGrantRequest) (*provider.RemoveGrantResponse, error) { - // FIXME these should be part of the RemoveGrantRequest object - if req.Opaque != nil { - if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { - ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) - } - } + ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) // check targetType is valid if req.Grant.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_INVALID { diff --git a/tests/integration/grpc/storageprovider_test.go b/tests/integration/grpc/storageprovider_test.go index eaf4d8a57a..6b8b71b5e4 100644 --- a/tests/integration/grpc/storageprovider_test.go +++ b/tests/integration/grpc/storageprovider_test.go @@ -33,7 +33,6 @@ import ( "github.com/cs3org/reva/v2/pkg/storage/fs/nextcloud" "github.com/cs3org/reva/v2/pkg/storage/fs/ocis" jwt "github.com/cs3org/reva/v2/pkg/token/manager/jwt" - "github.com/cs3org/reva/v2/pkg/utils" "github.com/cs3org/reva/v2/tests/helpers" "github.com/google/uuid" @@ -620,8 +619,8 @@ var _ = Describe("storage providers", func() { Context("with the owner holding the lock", func() { It("can initiate an upload", func() { ulRes, err := serviceClient.InitiateFileUpload(ctx, &storagep.InitiateFileUploadRequest{ - Opaque: utils.AppendPlainToOpaque(nil, "lockid", lock.LockId), Ref: subdirRef, + LockId: lock.LockId, }) Expect(err).ToNot(HaveOccurred()) Expect(ulRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) @@ -629,8 +628,8 @@ var _ = Describe("storage providers", func() { It("can delete the file", func() { delRes, err := serviceClient.Delete(ctx, &storagep.DeleteRequest{ - Opaque: utils.AppendPlainToOpaque(nil, "lockid", lock.LockId), Ref: subdirRef, + LockId: lock.LockId, }) Expect(err).ToNot(HaveOccurred()) Expect(delRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))