diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index da010478b..6c40de2a8 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -460,6 +460,12 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) { errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) return } + // FIXME: When setting permissions on a space, we need to use UnifiedRoleConditionOwner here + allowedResourceActions := unifiedrole.GetAllowedResourceActions(role, unifiedrole.UnifiedRoleConditionGrantee) + if len(allowedResourceActions) == 0 { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "role not applicable to this resource") + return + } unifiedRolePermissions = append(unifiedRolePermissions, conversions.ToPointerSlice(role.GetRolePermissions())...) } diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index 7513ec290..69cacd7e9 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -685,6 +685,25 @@ var _ = Describe("Driveitems", func() { _, ok := res.GetRolesOk() Expect(ok).To(BeTrue()) }) + It("fails to update the share permissions for a file share when setting a space specific role", func() { + updateShareMock := gatewayClient.On("UpdateShare", + mock.Anything, + mock.MatchedBy(func(req *collaboration.UpdateShareRequest) bool { + return req.GetShare().GetId().GetOpaqueId() == "permissionid" + }), + ) + updateShareMock.Return(updateShareMockResponse, nil) + + driveItemPermission.SetRoles([]string{unifiedrole.NewSpaceViewerUnifiedRole().GetId()}) + body, err := driveItemPermission.MarshalJSON() + Expect(err).To(BeNil()) + svc.UpdatePermission( + rr, + httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))). + WithContext(ctx), + ) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) It("updates the share permissions when changing the resource permission actions", func() { updateShareMock := gatewayClient.On("UpdateShare", mock.Anything, @@ -1007,6 +1026,17 @@ var _ = Describe("Driveitems", func() { Expect(jsonData.Get("0.roles.0").String()).To(Equal(unifiedrole.NewViewerUnifiedRole(true).GetId())) }) + It("fails with wrong role", func() { + driveItemInvite.Roles = []string{unifiedrole.NewCoownerUnifiedRole().GetId()} + svc.Invite( + rr, + httptest.NewRequest(http.MethodPost, "/", toJSONReader(driveItemInvite)). + WithContext(ctx), + ) + + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) + It("with actions (happy path)", func() { driveItemInvite.Roles = nil driveItemInvite.LibreGraphPermissionsActions = []string{unifiedrole.DriveItemContentRead} diff --git a/services/graph/pkg/unifiedrole/unifiedrole_test.go b/services/graph/pkg/unifiedrole/unifiedrole_test.go index f11225a5a..12248a375 100644 --- a/services/graph/pkg/unifiedrole/unifiedrole_test.go +++ b/services/graph/pkg/unifiedrole/unifiedrole_test.go @@ -27,8 +27,7 @@ var _ = Describe("unifiedroles", func() { Entry(rConversions.RoleViewer, rConversions.NewViewerRole(true), unifiedrole.NewViewerUnifiedRole(true), unifiedrole.UnifiedRoleConditionGrantee), Entry(rConversions.RoleEditor, rConversions.NewEditorRole(true), unifiedrole.NewEditorUnifiedRole(true), unifiedrole.UnifiedRoleConditionGrantee), Entry(rConversions.RoleFileEditor, rConversions.NewFileEditorRole(true), unifiedrole.NewFileEditorUnifiedRole(true), unifiedrole.UnifiedRoleConditionGrantee), - Entry(rConversions.RoleCoowner, rConversions.NewCoownerRole(), unifiedrole.NewCoownerUnifiedRole(), unifiedrole.UnifiedRoleConditionGrantee), - Entry(rConversions.RoleManager, rConversions.NewManagerRole(), unifiedrole.NewManagerUnifiedRole(), unifiedrole.UnifiedRoleConditionGrantee), + Entry(rConversions.RoleCoowner, rConversions.NewCoownerRole(), unifiedrole.NewCoownerUnifiedRole(), unifiedrole.UnifiedRoleConditionOwner), Entry(rConversions.RoleManager, rConversions.NewManagerRole(), unifiedrole.NewManagerUnifiedRole(), unifiedrole.UnifiedRoleConditionOwner), Entry(rConversions.RoleSpaceViewer, rConversions.NewSpaceViewerRole(), unifiedrole.NewSpaceViewerUnifiedRole(), unifiedrole.UnifiedRoleConditionOwner), Entry(rConversions.RoleSpaceEditor, rConversions.NewSpaceEditorRole(), unifiedrole.NewSpaceEditorUnifiedRole(), unifiedrole.UnifiedRoleConditionOwner), @@ -208,6 +207,17 @@ var _ = Describe("unifiedroles", func() { unifiedrole.NewViewerUnifiedRole(false), unifiedrole.NewFileEditorUnifiedRole(false), unifiedrole.NewEditorUnifiedRole(false), + }, + ), + + Entry( + "GetBuiltinRoleDefinitionList", + rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList(false)...), + unifiedrole.UnifiedRoleConditionOwner, + false, + []*libregraph.UnifiedRoleDefinition{ + unifiedrole.NewSpaceViewerUnifiedRole(), + unifiedrole.NewSpaceEditorUnifiedRole(), unifiedrole.NewCoownerUnifiedRole(), unifiedrole.NewManagerUnifiedRole(), }, @@ -223,8 +233,6 @@ var _ = Describe("unifiedroles", func() { unifiedrole.NewViewerUnifiedRole(true), unifiedrole.NewFileEditorUnifiedRole(true), unifiedrole.NewEditorUnifiedRole(true), - unifiedrole.NewCoownerUnifiedRole(), - unifiedrole.NewManagerUnifiedRole(), }, ),