Support t and x in ACEs

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

pkg/conversions/role.go

@@ -48,7 +48,9 @@ const (
 	RoleFileEditor = "file-editor"
 	// RoleCoowner grants co-owner permissions on a resource.
 	RoleCoowner = "coowner"
-	// RoleUploader grants uploader permission to upload onto a resource.
+	// RoleEditorLite grants permission to upload and download to a resource.
+	RoleEditorLite = "editor-lite"
+	// RoleUploader grants uploader permission to upload onto a resource (no download).
 	RoleUploader = "uploader"
 	// RoleManager grants manager permissions on a resource. Semantically equivalent to co-owner.
 	RoleManager = "manager"
@@ -313,7 +315,24 @@ func NewCoownerRole() *Role {
 	}
 }
 
-// NewUploaderRole creates an uploader role
+// NewEditorLiteRole creates an editor-lite role
+func NewEditorLiteRole() *Role {
+	return &Role{
+		Name: RoleEditorLite,
+		cS3ResourcePermissions: &provider.ResourcePermissions{
+			Stat:                 true,
+			GetPath:              true,
+			CreateContainer:      true,
+			InitiateFileUpload:   true,
+			InitiateFileDownload: true,
+			ListContainer:        true,
+			Move:                 true,
+		},
+		ocsPermissions: PermissionCreate,
+	}
+}
+
+// NewUploaderRole creates an uploader role with no download permissions
 func NewUploaderRole() *Role {
 	return &Role{
 		Name: RoleUploader,
@@ -524,6 +543,10 @@ func RoleFromResourcePermissions(rp *provider.ResourcePermissions, islink bool)
 		}
 	}
 	if r.ocsPermissions == PermissionCreate {
+		if rp.GetPath && rp.InitiateFileDownload && rp.ListContainer && rp.Move {
+			r.Name = RoleEditorLite
+			return r
+		}
 		r.Name = RoleUploader
 		return r
 	}

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},

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 {

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()

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() {

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})