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 c62e6ca commit 618001c
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 618001c

Please sign in to comment.