From a912b333f52a0593b2d44ea5f75c2cb3fa8047e6 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Mon, 10 Jul 2023 10:57:58 +0200 Subject: [PATCH] determine space members --- .../fix-webdav-permissions-manager.md | 5 +++ .../services/owncloud/ocdav/net/context.go | 10 ++++- .../owncloud/ocdav/net/context_test.go | 40 +++++++++++++++++-- .../owncloud/ocdav/propfind/propfind.go | 8 ++-- internal/http/services/owncloud/ocdav/tus.go | 2 +- 5 files changed, 55 insertions(+), 10 deletions(-) create mode 100644 changelog/unreleased/fix-webdav-permissions-manager.md diff --git a/changelog/unreleased/fix-webdav-permissions-manager.md b/changelog/unreleased/fix-webdav-permissions-manager.md new file mode 100644 index 0000000000..4c4189e34c --- /dev/null +++ b/changelog/unreleased/fix-webdav-permissions-manager.md @@ -0,0 +1,5 @@ +Bugfix: Fix WebDAV permissions for space managers + +Sub shares of a space were shown as incoming shares for space manager incorrectly. + +https://github.com/cs3org/reva/pull/4076 diff --git a/internal/http/services/owncloud/ocdav/net/context.go b/internal/http/services/owncloud/ocdav/net/context.go index f5236d995f..cc6abc0af4 100644 --- a/internal/http/services/owncloud/ocdav/net/context.go +++ b/internal/http/services/owncloud/ocdav/net/context.go @@ -22,16 +22,22 @@ import ( "context" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" ) -// IsCurrentUserOwner returns whether the context user is the given owner or not -func IsCurrentUserOwner(ctx context.Context, owner *userv1beta1.UserId) bool { +// IsCurrentUserOwnerOrManager returns whether the context user is the given owner or not +func IsCurrentUserOwnerOrManager(ctx context.Context, owner *userv1beta1.UserId, md *provider.ResourceInfo) bool { contextUser, ok := ctxpkg.ContextGetUser(ctx) + // personal spaces have owners if ok && contextUser.Id != nil && owner != nil && contextUser.Id.Idp == owner.Idp && contextUser.Id.OpaqueId == owner.OpaqueId { return true } + // check if the user is space manager + if md != nil && md.Owner != nil && md.Owner.GetType() == userv1beta1.UserType_USER_TYPE_SPACE_OWNER { + return md.GetPermissionSet().AddGrant + } return false } diff --git a/internal/http/services/owncloud/ocdav/net/context_test.go b/internal/http/services/owncloud/ocdav/net/context_test.go index 0cd6b0af7c..b6c623a405 100644 --- a/internal/http/services/owncloud/ocdav/net/context_test.go +++ b/internal/http/services/owncloud/ocdav/net/context_test.go @@ -22,6 +22,7 @@ import ( "context" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" @@ -43,17 +44,50 @@ var _ = Describe("Net", func() { }, Username: "bob", } + spaceManager = &userpb.User{ + Id: &userpb.UserId{ + OpaqueId: "space-id", + }, + Username: "virtual", + } + mdSpaceManager = &provider.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "user-1", + Type: userpb.UserType_USER_TYPE_SPACE_OWNER, + }, + PermissionSet: &provider.ResourcePermissions{ + AddGrant: true, + }, + } + + mdSpaceViewer = &provider.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "user-1", + Type: userpb.UserType_USER_TYPE_SPACE_OWNER, + }, + PermissionSet: &provider.ResourcePermissions{ + ListContainer: true, + }, + } aliceCtx = ctxpkg.ContextSetUser(context.Background(), alice) bobCtx = ctxpkg.ContextSetUser(context.Background(), bob) ) - Describe("IsCurrentUserOwner", func() { + Describe("IsCurrentUserOwnerOrManager", func() { It("returns true", func() { - Expect(net.IsCurrentUserOwner(aliceCtx, alice.Id)).To(BeTrue()) + Expect(net.IsCurrentUserOwnerOrManager(aliceCtx, alice.Id, nil)).To(BeTrue()) }) It("returns false", func() { - Expect(net.IsCurrentUserOwner(bobCtx, alice.Id)).To(BeFalse()) + Expect(net.IsCurrentUserOwnerOrManager(bobCtx, alice.Id, nil)).To(BeFalse()) + }) + + It("user is space manager", func() { + Expect(net.IsCurrentUserOwnerOrManager(bobCtx, spaceManager.Id, mdSpaceManager)).To(BeTrue()) + }) + + It("user is space viewer", func() { + Expect(net.IsCurrentUserOwnerOrManager(bobCtx, spaceManager.Id, mdSpaceViewer)).To(BeFalse()) }) }) }) diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 07c58174dc..28801a0666 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -1031,7 +1031,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } var wdp string isPublic := ls != nil - isShared := shareTypes != "" && !net.IsCurrentUserOwner(ctx, md.Owner) + isShared := shareTypes != "" && !net.IsCurrentUserOwnerOrManager(ctx, md.Owner, md) if md.PermissionSet != nil { wdp = role.WebDAVPermissions( md.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER, @@ -1253,7 +1253,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } case "public-link-share-owner": if ls != nil && ls.Owner != nil { - if net.IsCurrentUserOwner(ctx, ls.Owner) { + if net.IsCurrentUserOwnerOrManager(ctx, ls.Owner, nil) { u := ctxpkg.ContextMustGetUser(ctx) appendToOK(prop.Escaped("oc:public-link-share-owner", u.Username)) } else { @@ -1285,7 +1285,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } case "owner-id": // phoenix only if md.Owner != nil { - if net.IsCurrentUserOwner(ctx, md.Owner) { + if net.IsCurrentUserOwnerOrManager(ctx, md.Owner, md) { u := ctxpkg.ContextMustGetUser(ctx) appendToOK(prop.Escaped("oc:owner-id", u.Username)) } else { @@ -1375,7 +1375,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } case "owner-display-name": // phoenix only if md.Owner != nil { - if net.IsCurrentUserOwner(ctx, md.Owner) { + if net.IsCurrentUserOwnerOrManager(ctx, md.Owner, md) { u := ctxpkg.ContextMustGetUser(ctx) appendToOK(prop.Escaped("oc:owner-display-name", u.DisplayName)) } else { diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 5371472f71..84a3515edd 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -338,7 +338,7 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. isPublic = ls != nil } } - isShared := !net.IsCurrentUserOwner(ctx, info.Owner) + isShared := !net.IsCurrentUserOwnerOrManager(ctx, info.Owner, info) role := conversions.RoleFromResourcePermissions(info.PermissionSet, isPublic) permissions := role.WebDAVPermissions( info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER,