From da495fd3060d25c4dc851706d4b02e67478ac7e7 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 23 Jan 2024 17:49:25 +0100 Subject: [PATCH] graph/sharing: Fix role condition for space roots When computing the allowed roles for a spaceroot use the correct conditions. Spaceroots require '@Subject.objectId Any_of @Resource.owners'. Note: Updating or deleting the permissions on a spaceroot via 'v1beta1/drives/{driveid}/items/{itemid}/permissions/{permissionid}' does still not work. --- .../unreleased/sharing-ng-roleconditions.md | 8 +++++ services/graph/pkg/service/v0/driveitems.go | 32 ++++++++++++------- 2 files changed, 29 insertions(+), 11 deletions(-) create mode 100644 changelog/unreleased/sharing-ng-roleconditions.md diff --git a/changelog/unreleased/sharing-ng-roleconditions.md b/changelog/unreleased/sharing-ng-roleconditions.md new file mode 100644 index 0000000000..efad06c104 --- /dev/null +++ b/changelog/unreleased/sharing-ng-roleconditions.md @@ -0,0 +1,8 @@ +Bugfix: apply role constraints when creating shares via the graph API + +We fixed a bug in the graph API for creating and updating shares so that +Spaceroot specific roles like 'Manager' and 'Co-owner' can no longer be +assigned for shares on files or directories. + +https://github.com/owncloud/ocis/pull/8247 +https://github.com/owncloud/ocis/issues/8131 diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 6c40de2a80..ea6fc0ad75 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -378,6 +378,11 @@ func (g Graph) ListPermissions(w http.ResponseWriter, r *http.Request) { return } + condition := unifiedrole.UnifiedRoleConditionGrantee + if IsSpaceRoot(statResponse.GetInfo().GetId()) { + condition = unifiedrole.UnifiedRoleConditionOwner + } + permissionSet := *statResponse.GetInfo().GetPermissionSet() allowedActions := unifiedrole.CS3ResourcePermissionsToLibregraphActions(permissionSet) @@ -386,7 +391,7 @@ func (g Graph) ListPermissions(w http.ResponseWriter, r *http.Request) { LibreGraphPermissionsRolesAllowedValues: conversions.ToValueSlice( unifiedrole.GetApplicableRoleDefinitionsForActions( allowedActions, - unifiedrole.UnifiedRoleConditionGrantee, + condition, g.config.FilesSharing.EnableResharing, false, ), @@ -452,6 +457,18 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) { return } + statResponse, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{Ref: &storageprovider.Reference{ResourceId: &itemID}}) + if errCode := errorcode.FromStat(statResponse, err); errCode != nil { + g.logger.Warn().Err(errCode).Interface("stat.res", statResponse).Msg("stat failed") + errCode.Render(w, r) + return + } + + condition := unifiedrole.UnifiedRoleConditionGrantee + if IsSpaceRoot(statResponse.GetInfo().GetId()) { + condition = unifiedrole.UnifiedRoleConditionOwner + } + unifiedRolePermissions := []*libregraph.UnifiedRolePermission{{AllowedResourceActions: driveItemInvite.LibreGraphPermissionsActions}} for _, roleID := range driveItemInvite.GetRoles() { role, err := unifiedrole.NewUnifiedRoleFromID(roleID, g.config.FilesSharing.EnableResharing) @@ -460,8 +477,8 @@ 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) + + allowedResourceActions := unifiedrole.GetAllowedResourceActions(role, condition) if len(allowedResourceActions) == 0 { errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "role not applicable to this resource") return @@ -470,13 +487,6 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) { unifiedRolePermissions = append(unifiedRolePermissions, conversions.ToPointerSlice(role.GetRolePermissions())...) } - statResponse, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{Ref: &storageprovider.Reference{ResourceId: &itemID}}) - if errCode := errorcode.FromStat(statResponse, err); errCode != nil { - g.logger.Warn().Err(errCode).Interface("stat.res", statResponse).Msg("stat failed") - errCode.Render(w, r) - return - } - driveRecipient := driveItemInvite.GetRecipients()[0] objectID := driveRecipient.GetObjectId() @@ -492,7 +502,7 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) { } permission := &libregraph.Permission{} - if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*cs3ResourcePermissions, unifiedrole.UnifiedRoleConditionGrantee, g.config.FilesSharing.EnableResharing); role != nil { + if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*cs3ResourcePermissions, condition, g.config.FilesSharing.EnableResharing); role != nil { permission.Roles = []string{role.GetId()} }