enhancement: make use of unifiedrole from the graph invitation endpoi…

…nt, applying multiple roles works and result in a merged cs3 permission set
owncloud · Nov 22, 2023 · d3537b3 · d3537b3
1 parent 6ac5ac5
commit d3537b3
Show file tree

Hide file tree

Showing 9 changed files with 364 additions and 52 deletions.
diff --git a/changelog/5.0.0_2023-11-22/enhancement-sharing-ng.md b/changelog/5.0.0_2023-11-22/enhancement-sharing-ng.md
@@ -15,6 +15,7 @@ https://github.com/owncloud/ocis/pull/7684
 https://github.com/owncloud/ocis/pull/7683
 https://github.com/owncloud/ocis/pull/7239
 https://github.com/owncloud/ocis/pull/7687
+https://github.com/owncloud/ocis/pull/7751
 https://github.com/owncloud/libre-graph-api/pull/112
 https://github.com/owncloud/ocis/issues/7436
 https://github.com/owncloud/ocis/issues/6993
diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go
@@ -2,7 +2,6 @@ package svc
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -25,13 +24,13 @@ import (
 	"golang.org/x/crypto/sha3"
 	"golang.org/x/sync/errgroup"
 
-	"github.com/cs3org/reva/v2/pkg/conversions"
 	revactx "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/storagespace"
 	"github.com/cs3org/reva/v2/pkg/utils"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/validate"
 )
 
@@ -298,12 +297,16 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	role := conversions.RoleFromName(driveItemInvite.GetRoles()[0], g.config.FilesSharing.EnableResharing)
-	roleJson, err := json.Marshal(role)
-	if err != nil {
-		g.logger.Debug().Err(err).Interface("role", role).Msg("stat marshaling failed")
-		errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
-		return
+	unifiedRolePermissions := []libregraph.UnifiedRolePermission{{AllowedResourceActions: driveItemInvite.LibreGraphPermissionsActions}}
+	for _, roleId := range driveItemInvite.GetRoles() {
+		role, err := unifiedrole.NewUnifiedRoleFromID(roleId, g.config.FilesSharing.EnableResharing)
+		if err != nil {
+			g.logger.Debug().Err(err).Interface("role", driveItemInvite.GetRoles()[0]).Msg("unable to convert requested role")
+			errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
+			return
+		}
+
+		unifiedRolePermissions = append(unifiedRolePermissions, role.GetRolePermissions()...)
 	}
 
 	createShareErrors := sync.Map{}
@@ -322,25 +325,24 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) {
 				return nil
 			}
 
+			cs3ResourcePermissions := unifiedrole.PermissionsToCS3ResourcePermissions(unifiedRolePermissions)
+
 			createShareRequest := &collaboration.CreateShareRequest{
-				Opaque: &types.Opaque{
-					Map: map[string]*types.OpaqueEntry{
-						"role": {
-							Decoder: "json",
-							Value:   roleJson,
-						},
-					},
-				},
 				ResourceInfo: statResponse.GetInfo(),
 				Grant: &collaboration.ShareGrant{
 					Permissions: &collaboration.SharePermissions{
-						Permissions: role.CS3ResourcePermissions(),
+						Permissions: cs3ResourcePermissions,
 					},
 				},
 			}
 
-			permission := &libregraph.Permission{
-				Roles: []string{role.Name},
+			permission := &libregraph.Permission{}
+			if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*cs3ResourcePermissions, unifiedrole.UnifiedRoleConditionGrantee, g.config.FilesSharing.EnableResharing); role != nil {
+				permission.Roles = []string{role.GetId()}
+			}
+
+			if len(permission.GetRoles()) == 0 {
+				permission.LibreGraphPermissionsActions = unifiedrole.CS3ResourcePermissionsToLibregraphActions(*cs3ResourcePermissions)
 			}
 
 			switch driveRecipient.GetLibreGraphRecipientType() {

diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go
@@ -35,6 +35,7 @@ import (
 	"github.com/owncloud/ocis/v2/services/graph/pkg/config/defaults"
 	identitymocks "github.com/owncloud/ocis/v2/services/graph/pkg/identity/mocks"
 	service "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
 )
 
 type itemsList struct {
@@ -128,7 +129,7 @@ var _ = Describe("Driveitems", func() {
 				Recipients: []libregraph.DriveRecipient{
 					{ObjectId: libregraph.PtrString("1")},
 				},
-				Roles: []string{"viewer"},
+				Roles: []string{unifiedrole.NewViewerUnifiedRole(true).GetId()},
 			}
 
 			statMock = gatewayClient.On("Stat", mock.Anything, mock.Anything)
@@ -205,18 +206,47 @@ var _ = Describe("Driveitems", func() {
 			Expect(jsonData.Get("0.expirationDateTime").Str).To(Equal(driveItemInvite.ExpirationDateTime.Format(time.RFC3339Nano)))
 			Expect(jsonData.Get("1.expirationDateTime").Str).To(Equal(driveItemInvite.ExpirationDateTime.Format(time.RFC3339Nano)))
 
-			Expect(jsonData.Get("0.roles.#").Num).To(Equal(float64(1)))
-			Expect(jsonData.Get("0.roles.0").String()).To(Equal("viewer"))
-			Expect(jsonData.Get("1.roles.#").Num).To(Equal(float64(1)))
-			Expect(jsonData.Get("1.roles.0").String()).To(Equal("viewer"))
-
 			Expect(jsonData.Get("#.grantedToV2.user.displayName").Array()[0].Str).To(Equal(getUserResponse.User.DisplayName))
 			Expect(jsonData.Get("#.grantedToV2.user.id").Array()[0].Str).To(Equal("1"))
 
 			Expect(jsonData.Get("#.grantedToV2.group.displayName").Array()[0].Str).To(Equal(getGroupResponse.Group.GroupName))
 			Expect(jsonData.Get("#.grantedToV2.group.id").Array()[0].Str).To(Equal("2"))
 		})
 
+		It("with roles (happy path)", func() {
+			svc.Invite(
+				rr,
+				httptest.NewRequest(http.MethodPost, "/", toJSONReader(driveItemInvite)).
+					WithContext(ctx),
+			)
+
+			jsonData := gjson.Get(rr.Body.String(), "value")
+
+			Expect(rr.Code).To(Equal(http.StatusCreated))
+
+			Expect(jsonData.Get(`0.@libre\.graph\.permissions\.actions`).Exists()).To(BeFalse())
+			Expect(jsonData.Get("0.roles.#").Num).To(Equal(float64(1)))
+			Expect(jsonData.Get("0.roles.0").String()).To(Equal(unifiedrole.NewViewerUnifiedRole(true).GetId()))
+		})
+
+		It("with actions (happy path)", func() {
+			driveItemInvite.Roles = nil
+			driveItemInvite.LibreGraphPermissionsActions = []string{unifiedrole.DriveItemContentRead}
+			svc.Invite(
+				rr,
+				httptest.NewRequest(http.MethodPost, "/", toJSONReader(driveItemInvite)).
+					WithContext(ctx),
+			)
+
+			jsonData := gjson.Get(rr.Body.String(), "value")
+
+			Expect(rr.Code).To(Equal(http.StatusCreated))
+
+			Expect(jsonData.Get("0.roles").Exists()).To(BeFalse())
+			Expect(jsonData.Get(`0.@libre\.graph\.permissions\.actions.#`).Num).To(Equal(float64(1)))
+			Expect(jsonData.Get(`0.@libre\.graph\.permissions\.actions.0`).String()).To(Equal(unifiedrole.DriveItemContentRead))
+		})
+
 		It("validates the driveID", func() {
 			rctx := chi.NewRouteContext()
 			rctx.URLParams.Add("driveID", "")
@@ -287,22 +317,6 @@ var _ = Describe("Driveitems", func() {
 			Entry("fails on unknown fields", func() *strings.Reader {
 				return strings.NewReader(`{"unknown":"field"}`)
 			}, http.StatusBadRequest),
-			Entry("fails without recipients", func() *strings.Reader {
-				driveItemInvite.Recipients = nil
-				return toJSONReader(driveItemInvite)
-			}, http.StatusBadRequest),
-			Entry("fails without roles", func() *strings.Reader {
-				driveItemInvite.Roles = []string{}
-				return toJSONReader(driveItemInvite)
-			}, http.StatusBadRequest),
-			Entry("fails if more than one role item is present", func() *strings.Reader {
-				driveItemInvite.Roles = []string{"", ""}
-				return toJSONReader(driveItemInvite)
-			}, http.StatusBadRequest),
-			Entry("fails if the ExpirationDateTime is not in the future", func() *strings.Reader {
-				driveItemInvite.ExpirationDateTime = libregraph.PtrTime(time.Now())
-				return toJSONReader(driveItemInvite)
-			}, http.StatusBadRequest),
 		)
 
 		DescribeTable("Stat",

diff --git a/services/graph/pkg/unifiedrole/unifiedrole.go b/services/graph/pkg/unifiedrole/unifiedrole.go
@@ -1,6 +1,8 @@
 package unifiedrole
 
 import (
+	"errors"
+
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/conversions"
 	libregraph "github.com/owncloud/libre-graph-api-go"
@@ -189,6 +191,19 @@ func NewManagerUnifiedRole() *libregraph.UnifiedRoleDefinition {
 	}
 }
 
+// NewUnifiedRoleFromID returns a unified role definition from the provided id
+func NewUnifiedRoleFromID(id string, resharing bool) (*libregraph.UnifiedRoleDefinition, error) {
+	for _, definition := range GetBuiltinRoleDefinitionList(resharing) {
+		if definition.GetId() != id {
+			continue
+		}
+
+		return definition, nil
+	}
+
+	return nil, errors.New("role not found")
+}
+
 func GetBuiltinRoleDefinitionList(resharing bool) []*libregraph.UnifiedRoleDefinition {
 	return []*libregraph.UnifiedRoleDefinition{
 		NewViewerUnifiedRole(resharing),
@@ -202,6 +217,58 @@ func GetBuiltinRoleDefinitionList(resharing bool) []*libregraph.UnifiedRoleDefin
 	}
 }
 
+// PermissionsToCS3ResourcePermissions converts the provided libregraph UnifiedRolePermissions to a cs3 ResourcePermissions
+func PermissionsToCS3ResourcePermissions(unifiedRolePermissions []libregraph.UnifiedRolePermission) *provider.ResourcePermissions {
+	p := &provider.ResourcePermissions{}
+
+	for _, permission := range unifiedRolePermissions {
+		for _, allowedResourceAction := range permission.AllowedResourceActions {
+			switch allowedResourceAction {
+			case DriveItemPermissionsCreate:
+				p.AddGrant = true
+			case DriveItemChildrenCreate:
+				p.CreateContainer = true
+			case DriveItemStandardDelete:
+				p.Delete = true
+			case DriveItemPathRead:
+				p.GetPath = true
+			case DriveItemQuotaRead:
+				p.GetQuota = true
+			case DriveItemContentRead:
+				p.InitiateFileDownload = true
+			case DriveItemUploadCreate:
+				p.InitiateFileUpload = true
+			case DriveItemPermissionsRead:
+				p.ListGrants = true
+			case DriveItemChildrenRead:
+				p.ListContainer = true
+			case DriveItemVersionsRead:
+				p.ListFileVersions = true
+			case DriveItemDeletedRead:
+				p.ListRecycle = true
+			case DriveItemPathUpdate:
+				p.Move = true
+			case DriveItemPermissionsDelete:
+				p.RemoveGrant = true
+			case DriveItemDeletedDelete:
+				p.PurgeRecycle = true
+			case DriveItemVersionsUpdate:
+				p.RestoreFileVersion = true
+			case DriveItemDeletedUpdate:
+				p.RestoreRecycleItem = true
+			case DriveItemBasicRead:
+				p.Stat = true
+			case DriveItemPermissionsUpdate:
+				p.UpdateGrant = true
+			case DriveItemPermissionsDeny:
+				p.DenyGrant = true
+			}
+		}
+	}
+
+	return p
+}
+
 // CS3ResourcePermissionsToLibregraphActions converts the provided cs3 ResourcePermissions to a list of
 // libregraph actions
 func CS3ResourcePermissionsToLibregraphActions(p provider.ResourcePermissions) (actions []string) {

diff --git a/services/graph/pkg/unifiedrole/unifiedrole_test.go b/services/graph/pkg/unifiedrole/unifiedrole_test.go
@@ -1,10 +1,14 @@
 package unifiedrole_test
 
 import (
+	"fmt"
+
 	"github.com/cs3org/reva/v2/pkg/conversions"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/types"
 	libregraph "github.com/owncloud/libre-graph-api-go"
+
 	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
 )
 
@@ -23,4 +27,64 @@ var _ = Describe("unifiedroles", func() {
 		Entry(conversions.RoleCoowner, conversions.NewCoownerRole(), unifiedrole.NewCoownerUnifiedRole()),
 		Entry(conversions.RoleManager, conversions.NewManagerRole(), unifiedrole.NewManagerUnifiedRole()),
 	)
+
+	DescribeTable("UnifiedRolePermissionsToCS3ResourcePermissions",
+		func(cs3Role *conversions.Role, libregraphRole *libregraph.UnifiedRoleDefinition, match bool) {
+			permsFromCS3 := cs3Role.CS3ResourcePermissions()
+			permsFromUnifiedRole := unifiedrole.PermissionsToCS3ResourcePermissions(libregraphRole.RolePermissions)
+
+			var matcher types.GomegaMatcher
+
+			if match {
+				matcher = Equal(permsFromUnifiedRole)
+			} else {
+				matcher = Not(Equal(permsFromUnifiedRole))
+			}
+
+			Expect(permsFromCS3).To(matcher)
+		},
+		Entry(conversions.RoleViewer, conversions.NewViewerRole(true), unifiedrole.NewViewerUnifiedRole(true), true),
+		Entry(conversions.RoleEditor, conversions.NewEditorRole(true), unifiedrole.NewEditorUnifiedRole(true), true),
+		Entry(conversions.RoleFileEditor, conversions.NewFileEditorRole(true), unifiedrole.NewFileEditorUnifiedRole(true), true),
+		Entry(conversions.RoleCoowner, conversions.NewCoownerRole(), unifiedrole.NewCoownerUnifiedRole(), true),
+		Entry(conversions.RoleManager, conversions.NewManagerRole(), unifiedrole.NewManagerUnifiedRole(), true),
+		Entry("no match", conversions.NewFileEditorRole(true), unifiedrole.NewManagerUnifiedRole(), false),
+	)
+
+	{
+		var newUnifiedRoleFromIDEntries []TableEntry
+		for _, resharing := range []bool{true, false} {
+			attachEntry := func(name, id string, definition *libregraph.UnifiedRoleDefinition, errors bool) {
+				e := Entry(
+					fmt.Sprintf("%s - resharing: %t", name, resharing),
+					id,
+					resharing,
+					definition,
+					errors,
+				)
+
+				newUnifiedRoleFromIDEntries = append(newUnifiedRoleFromIDEntries, e)
+			}
+
+			for _, definition := range unifiedrole.GetBuiltinRoleDefinitionList(resharing) {
+				attachEntry(definition.GetDisplayName(), definition.GetId(), definition, false)
+			}
+
+			attachEntry("unknown", "123", nil, true)
+		}
+
+		DescribeTable("NewUnifiedRoleFromID",
+			func(id string, resharing bool, expectedRole *libregraph.UnifiedRoleDefinition, expectError bool) {
+				role, err := unifiedrole.NewUnifiedRoleFromID(id, resharing)
+
+				if expectError {
+					Expect(err).To(HaveOccurred())
+				} else {
+					Expect(err).NotTo(HaveOccurred())
+					Expect(role).To(Equal(expectedRole))
+				}
+			},
+			newUnifiedRoleFromIDEntries,
+		)
+	}
 })