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.
  • Loading branch information
rhafer committed Jan 19, 2024
1 parent 040dad9 commit 920c6f6
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 920c6f6

Please sign in to comment.