enhancement: make use of unifiedrole from the graph invitation endpoint, applying multiple roles works and result in a merged cs3 permission set (#7751)

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

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,11 +206,6 @@ 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"))
 
@@ -217,6 +213,40 @@ var _ = Describe("Driveitems", func() {
 			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",