mirror/opencloud
1
0
Fork 0
mirror of https://github.com/opencloud-eu/opencloud.git synced 2026-05-02 00:44:53 -05:00
Code Issues Packages Projects Releases Wiki Activity

fix(sharing-ng): Adapt role conditions to match resource types

Browse Source 
This switches our hardcode unfied role conditions to better reflect what
we're actually using them before. The new conditions also allow to differentiate
between roles elgitible for files, folders or drive roots.
Which means that the `/permissions` endpoint is now able to populate the
`roles.allowedValues` field with the correct roles for type of the resource
it is called for.

Fixes: #8331
This commit is contained in:
Ralf Haferkamp
2024-04-09 16:47:53 +02:00
committed by Ralf Haferkamp
parent fe6986ec79
commit 9ca9b78b2b
10 changed files with 217 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelog/unreleased/fix-graph-allowed-roles.md
+9
View File
@@ -0,0 +1,9 @@
Bugfix: Validate conditions for sharing roles by resource type
We improved the validation of the allowed sharing roles for specific resource type
for various sharing related graph API endpoints. This allows e.g. the web client to
restrict the sharing roles presented to the user based on the type of the resource
that is being shared.
https://github.com/owncloud/ocis/pull/8815
https://github.com/owncloud/ocis/issues/8331
services/graph/pkg/service/v0/api_driveitem_permissions.go
+8 -9
View File
@@ -88,10 +88,9 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId stor
return libregraph.Permission{}, *errCode
}
resourceInfo := statResponse.GetInfo()
condition := unifiedrole.UnifiedRoleConditionGrantee
if IsSpaceRoot(resourceInfo.GetId()) {
condition = unifiedrole.UnifiedRoleConditionOwner
var condition string
if condition, err = roleConditionForResourceType(statResponse.GetInfo()); err != nil {
return libregraph.Permission{}, err
}
unifiedRolePermissions := []*libregraph.UnifiedRolePermission{{AllowedResourceActions: invite.LibreGraphPermissionsActions}}
@@ -116,7 +115,7 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId stor
cs3ResourcePermissions := unifiedrole.PermissionsToCS3ResourcePermissions(unifiedRolePermissions)
createShareRequest := &collaboration.CreateShareRequest{
ResourceInfo: resourceInfo,
ResourceInfo: statResponse.GetInfo(),
Grant: &collaboration.ShareGrant{
Permissions: &collaboration.SharePermissions{
Permissions: cs3ResourcePermissions,
@@ -185,7 +184,7 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId stor
if id := createShareResponse.GetShare().GetId().GetOpaqueId(); id != "" {
permission.Id = conversions.ToPointer(id)
} else if IsSpaceRoot(resourceInfo.GetId()) {
} else if IsSpaceRoot(statResponse.GetInfo().GetId()) {
// permissions on a space root are not handled by a share manager so
// they don't get a share-id
permission.SetId(identitySetToSpacePermissionID(permission.GetGrantedToV2()))
@@ -232,9 +231,9 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
return collectionOfPermissions, err
}
condition := unifiedrole.UnifiedRoleConditionGrantee
if IsSpaceRoot(statResponse.GetInfo().GetId()) {
condition = unifiedrole.UnifiedRoleConditionOwner
var condition string
if condition, err = roleConditionForResourceType(statResponse.GetInfo()); err != nil {
return collectionOfPermissions, err
}
permissionSet := *statResponse.GetInfo().GetPermissionSet()
services/graph/pkg/service/v0/api_driveitem_permissions_test.go
+64 -12
View File
@@ -103,6 +103,10 @@ var _ = Describe("DriveItemPermissionsService", func() {
BeforeEach(func() {
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
statResponse.Info = &provider.ResourceInfo{
Id: &driveItemId,
Type: provider.ResourceType_RESOURCE_TYPE_FILE,
}
getGroupResponse = &grouppb.GetGroupResponse{
Status: status.NewOK(ctx),
@@ -158,7 +162,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(permission.GrantedToV2.Group.GetId()).To(Equal("2"))
})
It("with roles (happy path)", func() {
It("succeeds with file roles (happy path)", func() {
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
gatewayClient.On("CreateShare", mock.Anything, mock.Anything).Return(createShareResponse, nil)
driveItemInvite.Recipients = []libregraph.DriveRecipient{
@@ -173,7 +177,23 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(permission.GetRoles()[0]).To(Equal(unifiedrole.NewViewerUnifiedRole().GetId()))
})
It("fails with wrong role", func() {
It("succeeds with folder roles (happy path)", func() {
statResponse.Info.Type = provider.ResourceType_RESOURCE_TYPE_CONTAINER
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
gatewayClient.On("CreateShare", mock.Anything, mock.Anything).Return(createShareResponse, nil)
driveItemInvite.Recipients = []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
}
driveItemInvite.Roles = []string{unifiedrole.NewEditorUnifiedRole().GetId()}
permission, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
Expect(err).ToNot(HaveOccurred())
Expect(permission.GetRoles()).To(HaveLen(1))
Expect(permission.GetRoles()[0]).To(Equal(unifiedrole.NewEditorUnifiedRole().GetId()))
})
It("fails with when trying to set a space role on a file", func() {
driveItemInvite.Recipients = []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
}
@@ -184,6 +204,29 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(permission).To(BeZero())
})
It("fails with when trying to set a folder role on a file", func() {
driveItemInvite.Recipients = []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
}
driveItemInvite.Roles = []string{unifiedrole.NewEditorUnifiedRole().GetId()}
permission, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
Expect(err).To(MatchError(errorcode.New(errorcode.InvalidRequest, "role not applicable to this resource")))
Expect(permission).To(BeZero())
})
It("fails with when trying to set a file role on a folder", func() {
statResponse.Info.Type = provider.ResourceType_RESOURCE_TYPE_CONTAINER
driveItemInvite.Recipients = []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
}
driveItemInvite.Roles = []string{unifiedrole.NewFileEditorUnifiedRole().GetId()}
permission, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
Expect(err).To(MatchError(errorcode.New(errorcode.InvalidRequest, "role not applicable to this resource")))
Expect(permission).To(BeZero())
})
It("with actions (happy path)", func() {
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
gatewayClient.On("CreateShare", mock.Anything, mock.Anything).Return(createShareResponse, nil)
@@ -245,6 +288,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
createShareResponse = &collaboration.CreateShareResponse{
Status: status.NewOK(ctx),
}
statResponse.Info = &provider.ResourceInfo{}
})
It("adds a user to a space as expected (happy path)", func() {
@@ -254,6 +298,10 @@ var _ = Describe("DriveItemPermissionsService", func() {
SpaceId: "2",
OpaqueId: "3",
}
statResponse.Info.Id = listSpacesResponse.StorageSpaces[0].Root
statResponse.Info.Space = &provider.StorageSpace{
Root: listSpacesResponse.StorageSpaces[0].Root,
}
gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(listSpacesResponse, nil)
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
@@ -307,12 +355,13 @@ var _ = Describe("DriveItemPermissionsService", func() {
Status: status.NewOK(ctx),
Shares: []*collaboration.Share{},
}
})
It("populates allowedValues for files that are not shared", func() {
statResponse.Info = &provider.ResourceInfo{
Id: &itemID,
Type: provider.ResourceType_RESOURCE_TYPE_FILE,
PermissionSet: roleconversions.NewViewerRole().CS3ResourcePermissions(),
}
})
It("populates allowedValues for files that are not shared", func() {
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil)
gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil)
@@ -322,10 +371,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
})
It("returns one permission per share", func() {
statResponse.Info = &provider.ResourceInfo{
Id: &itemID,
PermissionSet: roleconversions.NewEditorRole().CS3ResourcePermissions(),
}
statResponse.Info.PermissionSet = roleconversions.NewEditorRole().CS3ResourcePermissions()
listSharesResponse.Shares = []*collaboration.Share{
{
Id: &collaboration.ShareId{OpaqueId: "1"},
@@ -394,6 +440,10 @@ var _ = Describe("DriveItemPermissionsService", func() {
},
},
}
statResponse.Info = &provider.ResourceInfo{
Type: provider.ResourceType_RESOURCE_TYPE_FILE,
PermissionSet: roleconversions.NewViewerRole().CS3ResourcePermissions(),
}
})
It("adds a user to a space as expected (happy path)", func() {
@@ -406,10 +456,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(listSpacesResponse, nil)
gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil)
statResponse.Info = &provider.ResourceInfo{
Id: listSpacesResponse.StorageSpaces[0].Root,
PermissionSet: roleconversions.NewViewerRole().CS3ResourcePermissions(),
}
statResponse.Info.Id = listSpacesResponse.StorageSpaces[0].Root
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
permissions, err := driveItemPermissionsService.ListSpaceRootPermissions(context.Background(), driveId)
Expect(err).ToNot(HaveOccurred())
@@ -714,6 +761,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(res.Link.GetLibreGraphDisplayName() == TestLinkName)
})
It("updates the expiration date", func() {
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
@@ -743,6 +791,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(res.GetExpirationDateTime().Equal(expiration)).To(BeTrue())
})
It("deletes the expiration date", func() {
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
@@ -770,6 +819,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(ok).To(BeFalse())
})
It("fails to update the share permissions for a file share when setting a space specific role", func() {
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
gatewayClient.On("GetPublicShare", mock.Anything, mock.MatchedBy(func(req *link.GetPublicShareRequest) bool {
@@ -788,6 +838,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(res).To(BeZero())
})
It("fails to update the space permissions for a space share when setting a file specific role", func() {
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(getPublicShareMockResponse, nil)
@@ -809,6 +860,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
Expect(res).To(BeZero())
})
It("updates the share permissions when changing the resource permission actions", func() {
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(getPublicShareMockResponse, nil)
services/graph/pkg/service/v0/base.go
+42 -26
View File
@@ -165,7 +165,7 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s
p.SetExpirationDateTime(time.Unix(int64(exp.GetSeconds()), int64(exp.GetNanos())))
}
if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*perm, unifiedrole.UnifiedRoleConditionOwner); role != nil {
if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*perm, unifiedrole.UnifiedRoleConditionDrive); role != nil {
switch apiVersion {
case APIVersion_1:
if r := unifiedrole.GetLegacyName(*role); r != "" {
@@ -287,7 +287,17 @@ func (g BaseGraphService) cs3UserSharesToDriveItems(ctx context.Context, shares
}
item = *itemptr
}
perm, err := g.cs3UserShareToPermission(ctx, s, false)
var condition string
switch {
case item.Root != nil:
condition = unifiedrole.UnifiedRoleConditionDrive
case item.Folder != nil:
condition = unifiedrole.UnifiedRoleConditionFolder
case item.File != nil:
condition = unifiedrole.UnifiedRoleConditionFile
}
perm, err := g.cs3UserShareToPermission(ctx, s, condition)
var errcode errorcode.Error
switch {
@@ -303,10 +313,10 @@ func (g BaseGraphService) cs3UserSharesToDriveItems(ctx context.Context, shares
return driveItems, nil
}
func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *collaboration.Share, isSpacePermission bool) (*libregraph.Permission, error) {
func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *collaboration.Share, roleCondition string) (*libregraph.Permission, error) {
perm := libregraph.Permission{}
perm.SetRoles([]string{})
if !isSpacePermission {
if roleCondition != unifiedrole.UnifiedRoleConditionDrive {
perm.SetId(share.GetId().GetOpaqueId())
}
grantedTo := libregraph.SharePointIdentitySet{}
@@ -322,7 +332,7 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c
return nil, errorcode.New(errorcode.GeneralException, err.Error())
default:
grantedTo.SetUser(user)
if isSpacePermission {
if roleCondition == unifiedrole.UnifiedRoleConditionDrive {
perm.SetId("u:" + user.GetId())
}
}
@@ -337,7 +347,7 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c
return nil, errorcode.New(errorcode.GeneralException, err.Error())
default:
grantedTo.SetGroup(group)
if isSpacePermission {
if roleCondition == unifiedrole.UnifiedRoleConditionDrive {
perm.SetId("g:" + group.GetId())
}
}
@@ -347,13 +357,9 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c
if share.GetExpiration() != nil {
perm.SetExpirationDateTime(cs3TimestampToTime(share.GetExpiration()))
}
condition := unifiedrole.UnifiedRoleConditionGrantee
if isSpacePermission {
condition = unifiedrole.UnifiedRoleConditionOwner
}
role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(
*share.GetPermissions().GetPermissions(),
condition,
roleCondition,
)
if role != nil {
perm.SetRoles([]string{role.GetId()})
@@ -534,8 +540,13 @@ func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID
}
func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId) (*libregraph.Permission, *storageprovider.ResourceId, error) {
publicShare, err := g.getCS3PublicShareByID(ctx, permissionID)
var errcode errorcode.Error
gatewayClient, err := g.gatewaySelector.Next()
if err != nil {
g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed")
return nil, nil, err
}
publicShare, err := g.getCS3PublicShareByID(ctx, permissionID)
switch {
case err == nil:
// the id is referencing a public share
@@ -546,13 +557,7 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st
return permission, publicShare.GetResourceId(), nil
case IsSpaceRoot(itemID):
// itemID is referencing a spaceroot this is a space permission. Handle
// that next
gatewayClient, err := g.gatewaySelector.Next()
if err != nil {
g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed")
return nil, nil, err
}
// get space id
// that here and get space id
resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient)
if err != nil {
return nil, nil, err
@@ -573,7 +578,15 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st
if err != nil {
return nil, nil, err
}
permission, err := g.cs3UserShareToPermission(ctx, share, false)
resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient)
if err != nil {
return nil, nil, err
}
condition, err := roleConditionForResourceType(resourceInfo)
if err != nil {
return nil, nil, err
}
permission, err := g.cs3UserShareToPermission(ctx, share, condition)
if err != nil {
return nil, nil, err
}
@@ -590,6 +603,14 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri
return nil, err
}
resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient)
if err != nil {
return nil, err
}
condition, err := roleConditionForResourceType(resourceInfo)
if err != nil {
return nil, err
}
var cs3UpdateShareReq collaboration.UpdateShareRequest
// When updating a space root we need to reference the share by resourceId and grantee
if IsSpaceRoot(itemID) {
@@ -632,11 +653,6 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri
return nil, err
}
condition := unifiedrole.UnifiedRoleConditionGrantee
if IsSpaceRoot(itemID) {
condition = unifiedrole.UnifiedRoleConditionOwner
}
// FIXME: When setting permissions on a space, we need to use UnifiedRoleConditionOwner here
allowedResourceActions = unifiedrole.GetAllowedResourceActions(role, condition)
if len(allowedResourceActions) == 0 {
return nil, errorcode.New(errorcode.InvalidRequest, "role not applicable to this resource")
@@ -671,7 +687,7 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri
return nil, *errCode
}
permission, err := g.cs3UserShareToPermission(ctx, updateUserShareResp.GetShare(), IsSpaceRoot(itemID))
permission, err := g.cs3UserShareToPermission(ctx, updateUserShareResp.GetShare(), condition)
if err != nil {
return nil, err
}
services/graph/pkg/service/v0/sharedbyme_test.go
+2 -1
View File
@@ -176,7 +176,8 @@ var _ = Describe("sharedbyme", func() {
Return(&provider.StatResponse{
Status: status.NewOK(ctx),
Info: &provider.ResourceInfo{
Id: userShare.ResourceId,
Id: userShare.ResourceId,
Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER,
},
}, nil)
services/graph/pkg/service/v0/utils.go
+24 -8
View File
@@ -177,7 +177,7 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context,
return errCode
}
driveItem, err := fillDriveItemPropertiesFromReceivedShare(ctx, logger, identityCache, receivedShares)
driveItem, err := fillDriveItemPropertiesFromReceivedShare(ctx, logger, identityCache, receivedShares, shareStat.GetInfo())
if err != nil {
return err
}
@@ -313,7 +313,8 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context,
}
func fillDriveItemPropertiesFromReceivedShare(ctx context.Context, logger *log.Logger,
identityCache identity.IdentityCache, receivedShares []*collaboration.ReceivedShare) (*libregraph.DriveItem, error) {
identityCache identity.IdentityCache, receivedShares []*collaboration.ReceivedShare,
resourceInfo *storageprovider.ResourceInfo) (*libregraph.DriveItem, error) {
driveItem := libregraph.NewDriveItem()
permissions := make([]libregraph.Permission, 0, len(receivedShares))
@@ -327,7 +328,7 @@ func fillDriveItemPropertiesFromReceivedShare(ctx context.Context, logger *log.L
oldestReceivedShare = receivedShare
}
permission, err := cs3ReceivedShareToLibreGraphPermissions(ctx, logger, identityCache, receivedShare)
permission, err := cs3ReceivedShareToLibreGraphPermissions(ctx, logger, identityCache, receivedShare, resourceInfo)
if err != nil {
return driveItem, err
}
@@ -387,7 +388,8 @@ func fillDriveItemPropertiesFromReceivedShare(ctx context.Context, logger *log.L
}
func cs3ReceivedShareToLibreGraphPermissions(ctx context.Context, logger *log.Logger,
identityCache identity.IdentityCache, receivedShare *collaboration.ReceivedShare) (*libregraph.Permission, error) {
identityCache identity.IdentityCache, receivedShare *collaboration.ReceivedShare,
resourceInfo *storageprovider.ResourceInfo) (*libregraph.Permission, error) {
permission := libregraph.NewPermission()
if id := receivedShare.GetShare().GetId().GetOpaqueId(); id != "" {
permission.SetId(id)
@@ -398,10 +400,11 @@ func cs3ReceivedShareToLibreGraphPermissions(ctx context.Context, logger *log.Lo
}
if permissionSet := receivedShare.GetShare().GetPermissions().GetPermissions(); permissionSet != nil {
role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(
*permissionSet,
unifiedrole.UnifiedRoleConditionGrantee,
)
condition, err := roleConditionForResourceType(resourceInfo)
if err != nil {
return nil, err
}
role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*permissionSet, condition)
if role != nil {
permission.SetRoles([]string{role.GetId()})
@@ -433,3 +436,16 @@ func cs3ReceivedShareToLibreGraphPermissions(ctx context.Context, logger *log.Lo
return permission, nil
}
func roleConditionForResourceType(ri *storageprovider.ResourceInfo) (string, error) {
switch {
case utils.ResourceIDEqual(ri.GetSpace().GetRoot(), ri.GetId()):
return unifiedrole.UnifiedRoleConditionDrive, nil
case ri.Type == storageprovider.ResourceType_RESOURCE_TYPE_CONTAINER:
return unifiedrole.UnifiedRoleConditionFolder, nil
case ri.Type == storageprovider.ResourceType_RESOURCE_TYPE_FILE:
return unifiedrole.UnifiedRoleConditionFile, nil
default:
return "", errorcode.New(errorcode.InvalidRequest, "unsupported resource type")
}
}
services/graph/pkg/unifiedrole/unifiedrole.go
+32 -24
View File
@@ -28,12 +28,12 @@ const (
// UnifiedRoleManagerID Unified role manager id.
UnifiedRoleManagerID = "312c0871-5ef7-4b3a-85b6-0e4074c64049"
// UnifiedRoleConditionSelf defines constraint where the principal matches the target resource
UnifiedRoleConditionSelf = "@Subject.objectId == @Resource.objectId"
// UnifiedRoleConditionOwner defines constraints when the principal is the owner of the target resource
UnifiedRoleConditionOwner = "@Subject.objectId Any_of @Resource.owners"
// UnifiedRoleConditionGrantee does not exist in MS Graph, but we use it to express permissions on shared resources
UnifiedRoleConditionGrantee = "@Subject.objectId Any_of @Resource.grantee"
// UnifiedRoleConditionDrive defines constraint that matches a Driveroot/Spaceroot
UnifiedRoleConditionDrive = "exists @Resource.Root"
// UnifiedRoleConditionFolder defines constraints that matches a DriveItem representing a Folder
UnifiedRoleConditionFolder = "exists @Resource.Folder"
// UnifiedRoleConditionFile defines a constraint that matches a DriveItem representing a File
UnifiedRoleConditionFile = "exists @Resource.File"
DriveItemPermissionsCreate = "libre.graph/driveItem/permissions/create"
DriveItemChildrenCreate = "libre.graph/driveItem/children/create"
@@ -78,7 +78,11 @@ func NewViewerUnifiedRole() *libregraph.UnifiedRoleDefinition {
RolePermissions: []libregraph.UnifiedRolePermission{
{
AllowedResourceActions: convert(r),
Condition: proto.String(UnifiedRoleConditionGrantee),
Condition: proto.String(UnifiedRoleConditionFile),
},
{
AllowedResourceActions: convert(r),
Condition: proto.String(UnifiedRoleConditionFolder),
},
},
LibreGraphWeight: proto.Int32(0),
@@ -95,7 +99,7 @@ func NewSpaceViewerUnifiedRole() *libregraph.UnifiedRoleDefinition {
RolePermissions: []libregraph.UnifiedRolePermission{
{
AllowedResourceActions: convert(r),
Condition: proto.String(UnifiedRoleConditionOwner),
Condition: proto.String(UnifiedRoleConditionDrive),
},
},
LibreGraphWeight: proto.Int32(0),
@@ -112,7 +116,7 @@ func NewEditorUnifiedRole() *libregraph.UnifiedRoleDefinition {
RolePermissions: []libregraph.UnifiedRolePermission{
{
AllowedResourceActions: convert(r),
Condition: proto.String(UnifiedRoleConditionGrantee),
Condition: proto.String(UnifiedRoleConditionFolder),
},
},
LibreGraphWeight: proto.Int32(0),
@@ -129,7 +133,7 @@ func NewSpaceEditorUnifiedRole() *libregraph.UnifiedRoleDefinition {
RolePermissions: []libregraph.UnifiedRolePermission{
{
AllowedResourceActions: convert(r),
Condition: proto.String(UnifiedRoleConditionOwner),
Condition: proto.String(UnifiedRoleConditionDrive),
},
},
LibreGraphWeight: proto.Int32(0),
@@ -146,7 +150,7 @@ func NewFileEditorUnifiedRole() *libregraph.UnifiedRoleDefinition {
RolePermissions: []libregraph.UnifiedRolePermission{
{
AllowedResourceActions: convert(r),
Condition: proto.String(UnifiedRoleConditionGrantee),
Condition: proto.String(UnifiedRoleConditionFile),
},
},
LibreGraphWeight: proto.Int32(0),
@@ -163,7 +167,7 @@ func NewUploaderUnifiedRole() *libregraph.UnifiedRoleDefinition {
RolePermissions: []libregraph.UnifiedRolePermission{
{
AllowedResourceActions: convert(r),
Condition: proto.String(UnifiedRoleConditionGrantee),
Condition: proto.String(UnifiedRoleConditionFolder),
},
},
LibreGraphWeight: proto.Int32(0),
@@ -180,7 +184,7 @@ func NewManagerUnifiedRole() *libregraph.UnifiedRoleDefinition {
RolePermissions: []libregraph.UnifiedRolePermission{
{
AllowedResourceActions: convert(r),
Condition: proto.String(UnifiedRoleConditionOwner),
Condition: proto.String(UnifiedRoleConditionDrive),
},
},
LibreGraphWeight: proto.Int32(0),
@@ -219,19 +223,20 @@ func GetApplicableRoleDefinitionsForActions(actions []string, constraints string
definitions := make([]*libregraph.UnifiedRoleDefinition, 0, len(builtin))
for _, definition := range builtin {
definitionMatch := true
var definitionMatch bool
for _, permission := range definition.GetRolePermissions() {
if permission.GetCondition() != constraints {
definitionMatch = false
continue
}
for _, action := range permission.GetAllowedResourceActions() {
for i, action := range permission.GetAllowedResourceActions() {
if !slices.Contains(actions, action) {
definitionMatch = false
break
}
if i == len(permission.GetAllowedResourceActions())-1 {
definitionMatch = true
}
}
if definitionMatch {
@@ -239,30 +244,33 @@ func GetApplicableRoleDefinitionsForActions(actions []string, constraints string
}
}
if !definitionMatch {
continue
if definitionMatch {
definitions = append(definitions, definition)
}
definitions = append(definitions, definition)
}
return WeightRoleDefinitions(definitions, descending)
return WeightRoleDefinitions(definitions, constraints, descending)
}
// WeightRoleDefinitions sorts the provided role definitions by the number of permissions[n].actions they grant,
// the implementation is optimistic and assumes that the weight relies on the number of available actions.
// descending - false - sorts the roles from least to most permissions
// descending - true - sorts the roles from most to least permissions
func WeightRoleDefinitions(roleDefinitions []*libregraph.UnifiedRoleDefinition, descending bool) []*libregraph.UnifiedRoleDefinition {
func WeightRoleDefinitions(roleDefinitions []*libregraph.UnifiedRoleDefinition, constraints string, descending bool) []*libregraph.UnifiedRoleDefinition {
slices.SortFunc(roleDefinitions, func(i, j *libregraph.UnifiedRoleDefinition) int {
var ia []string
for _, rp := range i.GetRolePermissions() {
ia = append(ia, rp.GetAllowedResourceActions()...)
if rp.GetCondition() == constraints {
ia = append(ia, rp.GetAllowedResourceActions()...)
}
}
var ja []string
for _, rp := range j.GetRolePermissions() {
ja = append(ja, rp.GetAllowedResourceActions()...)
if rp.GetCondition() == constraints {
ja = append(ja, rp.GetAllowedResourceActions()...)
}
}
switch descending {
services/graph/pkg/unifiedrole/unifiedrole_test.go
+29 -52
View File
@@ -24,12 +24,13 @@ var _ = Describe("unifiedroles", func() {
Expect(r.GetId()).To(Equal(unifiedRole.GetId()))
},
Entry(rConversions.RoleViewer, rConversions.NewViewerRole(), unifiedrole.NewViewerUnifiedRole(), unifiedrole.UnifiedRoleConditionGrantee),
Entry(rConversions.RoleEditor, rConversions.NewEditorRole(), unifiedrole.NewEditorUnifiedRole(), unifiedrole.UnifiedRoleConditionGrantee),
Entry(rConversions.RoleFileEditor, rConversions.NewFileEditorRole(), unifiedrole.NewFileEditorUnifiedRole(), unifiedrole.UnifiedRoleConditionGrantee),
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),
Entry(rConversions.RoleViewer, rConversions.NewViewerRole(), unifiedrole.NewViewerUnifiedRole(), unifiedrole.UnifiedRoleConditionFile),
Entry(rConversions.RoleViewer, rConversions.NewViewerRole(), unifiedrole.NewViewerUnifiedRole(), unifiedrole.UnifiedRoleConditionFolder),
Entry(rConversions.RoleEditor, rConversions.NewEditorRole(), unifiedrole.NewEditorUnifiedRole(), unifiedrole.UnifiedRoleConditionFolder),
Entry(rConversions.RoleFileEditor, rConversions.NewFileEditorRole(), unifiedrole.NewFileEditorUnifiedRole(), unifiedrole.UnifiedRoleConditionFile),
Entry(rConversions.RoleManager, rConversions.NewManagerRole(), unifiedrole.NewManagerUnifiedRole(), unifiedrole.UnifiedRoleConditionDrive),
Entry(rConversions.RoleSpaceViewer, rConversions.NewSpaceViewerRole(), unifiedrole.NewSpaceViewerUnifiedRole(), unifiedrole.UnifiedRoleConditionDrive),
Entry(rConversions.RoleSpaceEditor, rConversions.NewSpaceEditorRole(), unifiedrole.NewSpaceEditorUnifiedRole(), unifiedrole.UnifiedRoleConditionDrive),
)
DescribeTable("UnifiedRolePermissionsToCS3ResourcePermissions",
@@ -57,9 +58,9 @@ var _ = Describe("unifiedroles", func() {
)
DescribeTable("WeightRoleDefinitions",
func(roleDefinitions []*libregraph.UnifiedRoleDefinition, descending bool, expectedDefinitions []*libregraph.UnifiedRoleDefinition) {
func(roleDefinitions []*libregraph.UnifiedRoleDefinition, constraint string, descending bool, expectedDefinitions []*libregraph.UnifiedRoleDefinition) {
for i, generatedDefinition := range unifiedrole.WeightRoleDefinitions(roleDefinitions, descending) {
for i, generatedDefinition := range unifiedrole.WeightRoleDefinitions(roleDefinitions, constraint, descending) {
Expect(generatedDefinition.Id).To(Equal(expectedDefinitions[i].Id))
}
},
@@ -69,6 +70,7 @@ var _ = Describe("unifiedroles", func() {
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
},
unifiedrole.UnifiedRoleConditionFile,
false,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewViewerUnifiedRole(),
@@ -81,6 +83,7 @@ var _ = Describe("unifiedroles", func() {
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
},
unifiedrole.UnifiedRoleConditionFile,
true,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewFileEditorUnifiedRole(),
@@ -130,7 +133,7 @@ var _ = Describe("unifiedroles", func() {
Entry(
"ViewerUnifiedRole",
rolesToAction(unifiedrole.NewViewerUnifiedRole()),
unifiedrole.UnifiedRoleConditionGrantee,
unifiedrole.UnifiedRoleConditionFolder,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewViewerUnifiedRole(),
},
@@ -139,7 +142,7 @@ var _ = Describe("unifiedroles", func() {
Entry(
"ViewerUnifiedRole | share",
rolesToAction(unifiedrole.NewViewerUnifiedRole()),
unifiedrole.UnifiedRoleConditionGrantee,
unifiedrole.UnifiedRoleConditionFile,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewViewerUnifiedRole(),
},
@@ -148,17 +151,7 @@ var _ = Describe("unifiedroles", func() {
Entry(
"NewFileEditorUnifiedRole",
rolesToAction(unifiedrole.NewFileEditorUnifiedRole()),
unifiedrole.UnifiedRoleConditionGrantee,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
},
),
Entry(
"NewFileEditorUnifiedRole - share",
rolesToAction(unifiedrole.NewFileEditorUnifiedRole()),
unifiedrole.UnifiedRoleConditionGrantee,
unifiedrole.UnifiedRoleConditionFile,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
@@ -168,23 +161,10 @@ var _ = Describe("unifiedroles", func() {
Entry(
"NewEditorUnifiedRole",
rolesToAction(unifiedrole.NewEditorUnifiedRole()),
unifiedrole.UnifiedRoleConditionGrantee,
unifiedrole.UnifiedRoleConditionFolder,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewUploaderUnifiedRole(),
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
unifiedrole.NewEditorUnifiedRole(),
},
),
Entry(
"NewEditorUnifiedRole - share",
rolesToAction(unifiedrole.NewEditorUnifiedRole()),
unifiedrole.UnifiedRoleConditionGrantee,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewUploaderUnifiedRole(),
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
unifiedrole.NewEditorUnifiedRole(),
},
),
@@ -192,11 +172,20 @@ var _ = Describe("unifiedroles", func() {
Entry(
"GetBuiltinRoleDefinitionList",
rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList()...),
unifiedrole.UnifiedRoleConditionGrantee,
unifiedrole.UnifiedRoleConditionFile,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
},
),
Entry(
"GetBuiltinRoleDefinitionList",
rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList()...),
unifiedrole.UnifiedRoleConditionFolder,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewUploaderUnifiedRole(),
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
unifiedrole.NewEditorUnifiedRole(),
},
),
@@ -204,7 +193,7 @@ var _ = Describe("unifiedroles", func() {
Entry(
"GetBuiltinRoleDefinitionList",
rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList()...),
unifiedrole.UnifiedRoleConditionOwner,
unifiedrole.UnifiedRoleConditionDrive,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewSpaceViewerUnifiedRole(),
unifiedrole.NewSpaceEditorUnifiedRole(),
@@ -212,29 +201,17 @@ var _ = Describe("unifiedroles", func() {
},
),
Entry(
"GetBuiltinRoleDefinitionList - share",
rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList()...),
unifiedrole.UnifiedRoleConditionGrantee,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewUploaderUnifiedRole(),
unifiedrole.NewViewerUnifiedRole(),
unifiedrole.NewFileEditorUnifiedRole(),
unifiedrole.NewEditorUnifiedRole(),
},
),
Entry(
"single",
[]string{unifiedrole.DriveItemQuotaRead},
unifiedrole.UnifiedRoleConditionGrantee,
unifiedrole.UnifiedRoleConditionFile,
[]*libregraph.UnifiedRoleDefinition{},
),
Entry(
"mixed",
append(rolesToAction(unifiedrole.NewUploaderUnifiedRole()), unifiedrole.DriveItemQuotaRead),
unifiedrole.UnifiedRoleConditionGrantee,
unifiedrole.UnifiedRoleConditionFolder,
[]*libregraph.UnifiedRoleDefinition{
unifiedrole.NewUploaderUnifiedRole(),
},
tests/acceptance/features/apiSharingNg/listPermissions.feature
+3 -38
View File
@@ -48,8 +48,8 @@ Feature: List a sharing permissions
},
"@libre.graph.permissions.roles.allowedValues": {
"type": "array",
"minItems": 4,
"maxItems": 4,
"minItems": 3,
"maxItems": 3,
"uniqueItems": true,
"items": {
"oneOf": [
@@ -138,41 +138,6 @@ Feature: List a sharing permissions
3
]
},
"description": {
"type": "string",
"enum": [
"View, download and edit."
]
},
"displayName": {
"type": "string",
"enum": [
"Can edit"
]
},
"id": {
"type": "string",
"enum": [
"2d00ce52-1fc2-4dbc-8b95-a73b73395f5a"
]
}
}
},
{
"type": "object",
"required": [
"@libre.graph.weight",
"description",
"displayName",
"id"
],
"properties": {
"@libre.graph.weight": {
"type": "integer",
"enum": [
4
]
},
"description": {
"type": "string",
"enum": [
@@ -594,4 +559,4 @@ Feature: List a sharing permissions
}
}
}
"""
"""
tests/acceptance/features/apiSharingNg/shareInvitations.feature
+4 -4
View File
@@ -1570,7 +1570,7 @@ Feature: Send a sharing invitations
"message": {
"type": "string",
"enum": [
"cannot set the requested permissions on that type of resource"
"role not applicable to this resource"
]
}
}
@@ -2356,9 +2356,9 @@ Feature: Send a sharing invitations
"""
Examples:
| permissions-role | error-message |
| Space Viewer | space type is not eligible for sharing |
| Space Editor | space type is not eligible for sharing |
| Manager | space type is not eligible for sharing |
| Space Viewer | role not applicable to this resource |
| Space Editor | role not applicable to this resource |
| Manager | role not applicable to this resource |
| Co Owner | Key: 'DriveItemInvite.Roles' Error:Field validation for 'Roles' failed on the 'available_role' tag |