From 0aac08de0a7c9aeb3fa1e0dc51ff20aa22ac4150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 May 2024 16:27:12 +0200 Subject: [PATCH] Support t and x in ACEs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/support-tx-in-aces.md | 5 ++++ .../fs/nextcloud/nextcloud_server_mock.go | 4 +-- pkg/storage/utils/ace/ace.go | 25 +++++++++++++------ pkg/storage/utils/ace/ace_test.go | 23 +++++++++++++++++ pkg/storage/utils/decomposedfs/grants_test.go | 9 ++++--- .../integration/grpc/storageprovider_test.go | 9 ++++--- 6 files changed, 59 insertions(+), 16 deletions(-) create mode 100644 changelog/unreleased/support-tx-in-aces.md diff --git a/changelog/unreleased/support-tx-in-aces.md b/changelog/unreleased/support-tx-in-aces.md new file mode 100644 index 00000000000..1ad40a6dbf6 --- /dev/null +++ b/changelog/unreleased/support-tx-in-aces.md @@ -0,0 +1,5 @@ +Enhancement: Support t and x in ACEs + +To support view only shares (dowload forbidden) we added t (read attrs) and x (directory traversal) permissions to the decomposed FS ACEs. + +https://github.com/cs3org/reva/pull/4685 diff --git a/pkg/storage/fs/nextcloud/nextcloud_server_mock.go b/pkg/storage/fs/nextcloud/nextcloud_server_mock.go index a23b5533f2e..4998c5a5b73 100644 --- a/pkg/storage/fs/nextcloud/nextcloud_server_mock.go +++ b/pkg/storage/fs/nextcloud/nextcloud_server_mock.go @@ -53,7 +53,7 @@ const serverStateMetadata = "METADATA" var serverState = serverStateEmpty var responses = map[string]Response{ - `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/AddGrant {"ref":{"path":"/subdir"},"g":{"grantee":{"type":1,"Id":{"UserId":{"opaque_id":"4c510ada-c86b-4815-8820-42cdf82c3d51"}}},"permissions":{"move":true,"stat":true}}} EMPTY`: {200, ``, serverStateGrantAdded}, + `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/AddGrant {"ref":{"path":"/subdir"},"g":{"grantee":{"type":1,"Id":{"UserId":{"opaque_id":"4c510ada-c86b-4815-8820-42cdf82c3d51"}}},"permissions":{"initiate_file_download":true,"move":true,"stat":true}}} EMPTY`: {200, ``, serverStateGrantAdded}, `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/CreateDir {"path":"/subdir"} EMPTY`: {200, ``, serverStateSubdir}, `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/CreateDir {"path":"/subdir"} HOME`: {200, ``, serverStateSubdir}, @@ -149,7 +149,7 @@ var responses = map[string]Response{ `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/UnsetArbitraryMetadata {"ref":{"path":"/subdir"},"keys":["foo"]}`: {200, ``, serverStateSubdir}, - `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/UpdateGrant {"ref":{"path":"/subdir"},"g":{"grantee":{"type":1,"Id":{"UserId":{"opaque_id":"4c510ada-c86b-4815-8820-42cdf82c3d51"}}},"permissions":{"delete":true,"move":true,"stat":true}}}`: {200, ``, serverStateGrantUpdated}, + `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/UpdateGrant {"ref":{"path":"/subdir"},"g":{"grantee":{"type":1,"Id":{"UserId":{"opaque_id":"4c510ada-c86b-4815-8820-42cdf82c3d51"}}},"permissions":{"delete":true,"initiate_file_download":true,"move":true,"stat":true}}}`: {200, ``, serverStateGrantUpdated}, `POST /apps/sciencemesh/~tester/api/storage/GetHome `: {200, `yes we are`, serverStateHome}, `POST /apps/sciencemesh/~tester/api/storage/CreateHome `: {201, ``, serverStateEmpty}, diff --git a/pkg/storage/utils/ace/ace.go b/pkg/storage/utils/ace/ace.go index da891255408..e88a98f7f5f 100644 --- a/pkg/storage/utils/ace/ace.go +++ b/pkg/storage/utils/ace/ace.go @@ -316,10 +316,15 @@ func (e *ACE) granteeType() provider.GranteeType { // grantPermissionSet returns the set of CS3 resource permissions representing the ACE func (e *ACE) grantPermissionSet() *provider.ResourcePermissions { p := &provider.ResourcePermissions{} - // r - if strings.Contains(e.permissions, "r") { + // t + if strings.Contains(e.permissions, "t") { p.Stat = true p.GetPath = true + } + // r + if strings.Contains(e.permissions, "r") { + p.Stat = true // currently assumed + p.GetPath = true // currently assumed p.InitiateFileDownload = true p.ListContainer = true } @@ -336,10 +341,9 @@ func (e *ACE) grantPermissionSet() *provider.ResourcePermissions { p.CreateContainer = true } // x - // if strings.Contains(e.Permissions, "x") { - // TODO execute file permission? - // TODO change directory permission? - // } + if strings.Contains(e.permissions, "x") { + p.ListContainer = true + } // d if strings.Contains(e.permissions, "d") { p.Delete = true @@ -436,10 +440,17 @@ func unmarshalKV(s string) (*ACE, error) { return e, nil } +// getACEPerm produces an NFSv4.x inspired permission string from a CS3 resource permissions set func getACEPerm(set *provider.ResourcePermissions) string { var b strings.Builder - if set.Stat || set.InitiateFileDownload || set.ListContainer || set.GetPath { + if set.Stat || set.GetPath { + b.WriteString("t") + } + if set.ListContainer { // we have no dedicated traversal permission, but to listing a container allows traversing it + b.WriteString("x") + } + if set.InitiateFileDownload { b.WriteString("r") } if set.InitiateFileUpload || set.Move { diff --git a/pkg/storage/utils/ace/ace_test.go b/pkg/storage/utils/ace/ace_test.go index 8ff7b07ab80..6424ac48254 100644 --- a/pkg/storage/utils/ace/ace_test.go +++ b/pkg/storage/utils/ace/ace_test.go @@ -100,6 +100,21 @@ var _ = Describe("ACE", func() { }) Describe("converting permissions", func() { + It("converts t", func() { + userGrant.Permissions.Stat = true + newGrant := ace.FromGrant(userGrant).Grant() + userGrant.Permissions.Stat = false + Expect(newGrant.Permissions.Stat).To(BeTrue()) + Expect(newGrant.Permissions.Delete).To(BeFalse()) + + userGrant.Permissions.GetPath = true + newGrant = ace.FromGrant(userGrant).Grant() + fmt.Println(newGrant.Permissions) + userGrant.Permissions.GetPath = false + Expect(newGrant.Permissions.GetPath).To(BeTrue()) + Expect(newGrant.Permissions.Delete).To(BeFalse()) + }) + It("converts r", func() { userGrant.Permissions.Stat = true newGrant := ace.FromGrant(userGrant).Grant() @@ -152,6 +167,14 @@ var _ = Describe("ACE", func() { Expect(newGrant.Permissions.Delete).To(BeFalse()) }) + It("converts x", func() { + userGrant.Permissions.ListContainer = true + newGrant := ace.FromGrant(userGrant).Grant() + userGrant.Permissions.ListContainer = false + Expect(newGrant.Permissions.ListContainer).To(BeTrue()) + Expect(newGrant.Permissions.Delete).To(BeFalse()) + }) + It("converts d", func() { userGrant.Permissions.Delete = true newGrant := ace.FromGrant(userGrant).Grant() diff --git a/pkg/storage/utils/decomposedfs/grants_test.go b/pkg/storage/utils/decomposedfs/grants_test.go index bc8f5e038b4..c70760a9d87 100644 --- a/pkg/storage/utils/decomposedfs/grants_test.go +++ b/pkg/storage/utils/decomposedfs/grants_test.go @@ -50,9 +50,10 @@ var _ = Describe("Grants", func() { }, }, Permissions: &provider.ResourcePermissions{ - Stat: true, - Move: true, - Delete: false, + Stat: true, + Move: true, + Delete: false, + InitiateFileDownload: true, }, Creator: &userpb.UserId{ OpaqueId: helpers.OwnerID, @@ -130,7 +131,7 @@ var _ = Describe("Grants", func() { Expect(err).ToNot(HaveOccurred()) attr, err := n.XattrString(env.Ctx, prefixes.GrantUserAcePrefix+grant.Grantee.GetUserId().OpaqueId) Expect(err).ToNot(HaveOccurred()) - Expect(attr).To(Equal(fmt.Sprintf("\x00t=A:f=:p=rw:c=%s:e=0\n", o.GetOpaqueId()))) // NOTE: this tests ace package + Expect(attr).To(Equal(fmt.Sprintf("\x00t=A:f=:p=trw:c=%s:e=0\n", o.GetOpaqueId()))) // NOTE: this tests ace package }) It("creates a storage space per created grant", func() { diff --git a/tests/integration/grpc/storageprovider_test.go b/tests/integration/grpc/storageprovider_test.go index 4c9854726bd..452cdd530cd 100644 --- a/tests/integration/grpc/storageprovider_test.go +++ b/tests/integration/grpc/storageprovider_test.go @@ -312,9 +312,10 @@ var _ = Describe("storage providers", func() { }, }, Permissions: &storagep.ResourcePermissions{ - Stat: true, - Move: true, - Delete: false, + Stat: true, + Move: true, + Delete: false, + InitiateFileDownload: true, }, } addRes, err := serviceClient.AddGrant(ctx, &storagep.AddGrantRequest{Ref: subdirRef, Grant: grant}) @@ -329,6 +330,7 @@ var _ = Describe("storage providers", func() { Expect(readGrant.Permissions.Stat).To(BeTrue()) Expect(readGrant.Permissions.Move).To(BeTrue()) Expect(readGrant.Permissions.Delete).To(BeFalse()) + Expect(readGrant.Permissions.InitiateFileDownload).To(BeTrue()) By("updating the grant") grant.Permissions.Delete = true @@ -344,6 +346,7 @@ var _ = Describe("storage providers", func() { Expect(readGrant.Permissions.Stat).To(BeTrue()) Expect(readGrant.Permissions.Move).To(BeTrue()) Expect(readGrant.Permissions.Delete).To(BeTrue()) + Expect(readGrant.Permissions.InitiateFileDownload).To(BeTrue()) By("deleting a grant") delRes, err := serviceClient.RemoveGrant(ctx, &storagep.RemoveGrantRequest{Ref: subdirRef, Grant: readGrant})