Skip to content

Commit

Permalink
graph/sharing: Add check for role conditions
Browse files Browse the repository at this point in the history
Use the condition from the unifiedrole to check if the requested role
is actually applicable to the selected resource.

Fixes: owncloud#8131
  • Loading branch information
rhafer committed Jan 24, 2024
1 parent fa70d58 commit e0d5708
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
6 changes: 6 additions & 0 deletions services/graph/pkg/service/v0/driveitems.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())...)
}
Expand Down
11 changes: 11 additions & 0 deletions services/graph/pkg/service/v0/driveitems_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,17 @@ var _ = Describe("Driveitems", func() {
Expect(jsonData.Get("0.roles.0").String()).To(Equal(unifiedrole.NewViewerUnifiedRole(true).GetId()))
})

It("failes 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}
Expand Down
16 changes: 12 additions & 4 deletions services/graph/pkg/unifiedrole/unifiedrole_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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(),
},
Expand All @@ -223,8 +233,6 @@ var _ = Describe("unifiedroles", func() {
unifiedrole.NewViewerUnifiedRole(true),
unifiedrole.NewFileEditorUnifiedRole(true),
unifiedrole.NewEditorUnifiedRole(true),
unifiedrole.NewCoownerUnifiedRole(),
unifiedrole.NewManagerUnifiedRole(),
},
),

Expand Down

0 comments on commit e0d5708

Please sign in to comment.