Skip to content

Commit 7ea7583

Browse files
rhaferrhafer
authored
Merge pull request #996 from rhafer/count-members-only
graph: Add $filter to only list (and/or count) member permissions
2 parents 69e25b8 + 23c942f commit 7ea7583

File tree

2 files changed

+147
-27
lines changed

2 files changed

+147
-27
lines changed

‎services/graph/pkg/service/v0/api_driveitem_permissions.go‎

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const (
4646
invalidIdMsg = "invalid driveID or itemID"
4747
parseDriveIDErrMsg = "could not parse driveID"
4848
federatedRolesODataFilter = "@libre.graph.permissions.roles.allowedValues/rolePermissions/any(p:contains(p/condition, '@Subject.UserType==\"Federated\"'))"
49+
noLinksODataFilter = "grantedToV2 ne ''"
4950
)
5051

5152
// DriveItemPermissionsProvider contains the methods related to handling permissions on drive items
@@ -80,10 +81,11 @@ const (
8081
)
8182

8283
type ListPermissionsQueryOptions struct {
83-
count bool
84-
noValues bool
85-
filterFederatedRoles bool
86-
selectedAttrs []string
84+
Count bool
85+
NoValues bool
86+
NoLinkPermissions bool
87+
FilterFederatedRoles bool
88+
SelectedAttrs []string
8789
}
8890

8991
// NewDriveItemPermissionsService creates a new DriveItemPermissionsService
@@ -375,17 +377,17 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
375377

376378
collectionOfPermissions = libregraph.CollectionOfPermissionsWithAllowedValues{}
377379

378-
if len(queryOptions.selectedAttrs) == 0 || slices.Contains(queryOptions.selectedAttrs, "@libre.graph.permissions.actions.allowedValues") {
380+
if len(queryOptions.SelectedAttrs) == 0 || slices.Contains(queryOptions.SelectedAttrs, "@libre.graph.permissions.actions.allowedValues") {
379381
collectionOfPermissions.LibreGraphPermissionsActionsAllowedValues = allowedActions
380382
}
381383

382-
if len(queryOptions.selectedAttrs) == 0 || slices.Contains(queryOptions.selectedAttrs, "@libre.graph.permissions.roles.allowedValues") {
384+
if len(queryOptions.SelectedAttrs) == 0 || slices.Contains(queryOptions.SelectedAttrs, "@libre.graph.permissions.roles.allowedValues") {
383385
collectionOfPermissions.LibreGraphPermissionsRolesAllowedValues = conversions.ToValueSlice(
384386
unifiedrole.GetRolesByPermissions(
385387
unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(s.config.UnifiedRoles.AvailableRoles...)),
386388
allowedActions,
387389
condition,
388-
queryOptions.filterFederatedRoles,
390+
queryOptions.FilterFederatedRoles,
389391
false,
390392
),
391393
)
@@ -397,7 +399,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
397399
collectionOfPermissions.LibreGraphPermissionsRolesAllowedValues[i] = definition
398400
}
399401

400-
if len(queryOptions.selectedAttrs) > 0 {
402+
if len(queryOptions.SelectedAttrs) > 0 {
401403
// no need to fetch shares, we are only interested allowedActions and/or allowedRoles
402404
return collectionOfPermissions, nil
403405
}
@@ -414,7 +416,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
414416

415417
if IsSpaceRoot(statResponse.GetInfo().GetId()) {
416418
var permissions []libregraph.Permission
417-
permissions, permissionsCount, err = s.getSpaceRootPermissions(ctx, statResponse.GetInfo().GetSpace().GetId(), queryOptions.noValues)
419+
permissions, permissionsCount, err = s.getSpaceRootPermissions(ctx, statResponse.GetInfo().GetSpace().GetId(), queryOptions.NoValues)
418420
if err != nil {
419421
return collectionOfPermissions, err
420422
}
@@ -439,22 +441,25 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
439441
}
440442
}
441443
}
442-
// finally get public shares, which are possible for spaceroots and "normal" resources
443-
driveItems, err = s.listPublicShares(ctx, []*link.ListPublicSharesRequest_Filter{
444-
publicshare.ResourceIDFilter(itemID),
445-
}, driveItems)
446-
if err != nil {
447-
return collectionOfPermissions, err
444+
445+
if !queryOptions.NoLinkPermissions {
446+
// finally get public shares, which are possible for spaceroots and "normal" resources
447+
driveItems, err = s.listPublicShares(ctx, []*link.ListPublicSharesRequest_Filter{
448+
publicshare.ResourceIDFilter(itemID),
449+
}, driveItems)
450+
if err != nil {
451+
return collectionOfPermissions, err
452+
}
448453
}
449454

450455
for _, driveItem := range driveItems {
451456
permissionsCount += len(driveItem.Permissions)
452-
if !queryOptions.noValues {
457+
if !queryOptions.NoValues {
453458
collectionOfPermissions.Value = append(collectionOfPermissions.Value, driveItem.Permissions...)
454459
}
455460
}
456461

457-
if queryOptions.count {
462+
if queryOptions.Count {
458463
collectionOfPermissions.SetOdataCount(int32(permissionsCount))
459464
}
460465

@@ -819,10 +824,15 @@ func (api DriveItemPermissionsApi) ListSpaceRootPermissions(w http.ResponseWrite
819824
}
820825

821826
func (api DriveItemPermissionsApi) getListPermissionsQueryOptions(odataReq *godata.GoDataRequest) (ListPermissionsQueryOptions, error) {
822-
var listFederatedRoles bool
827+
queryOptions := ListPermissionsQueryOptions{}
823828
if odataReq.Query.Filter != nil {
824-
if odataReq.Query.Filter.RawValue == federatedRolesODataFilter {
825-
listFederatedRoles = true
829+
switch odataReq.Query.Filter.RawValue {
830+
case federatedRolesODataFilter:
831+
queryOptions.FilterFederatedRoles = true
832+
case noLinksODataFilter:
833+
queryOptions.NoLinkPermissions = true
834+
default:
835+
return ListPermissionsQueryOptions{}, errorcode.New(errorcode.InvalidRequest, "invalid filter value")
826836
}
827837
}
828838

@@ -831,22 +841,19 @@ func (api DriveItemPermissionsApi) getListPermissionsQueryOptions(odataReq *goda
831841
return ListPermissionsQueryOptions{}, err
832842
}
833843

834-
queryOptions := ListPermissionsQueryOptions{
835-
filterFederatedRoles: listFederatedRoles,
836-
selectedAttrs: selectAttrs,
837-
}
844+
queryOptions.SelectedAttrs = selectAttrs
838845
if odataReq.Query.Count != nil {
839-
queryOptions.count = bool(*odataReq.Query.Count)
846+
queryOptions.Count = bool(*odataReq.Query.Count)
840847
}
841848
if odataReq.Query.Top != nil {
842849
top := int(*odataReq.Query.Top)
843850
switch {
844851
case top != 0:
845852
return ListPermissionsQueryOptions{}, err
846-
case top == 0 && !queryOptions.count:
853+
case top == 0 && !queryOptions.Count:
847854
return ListPermissionsQueryOptions{}, err
848855
default:
849-
queryOptions.noValues = true
856+
queryOptions.NoValues = true
850857
}
851858
}
852859
return queryOptions, nil

‎services/graph/pkg/service/v0/api_driveitem_permissions_test.go‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,119 @@ var _ = Describe("DriveItemPermissionsService", func() {
515515
Expect(len(permissions.Value)).To(Equal(1))
516516
Expect(permissions.Value[0].GetLibreGraphPermissionsActions()[0]).To(Equal("none"))
517517
})
518+
It("Does not list public shares when requested so", func() {
519+
opt := svc.ListPermissionsQueryOptions{
520+
NoLinkPermissions: true,
521+
}
522+
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
523+
gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil)
524+
permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, opt)
525+
Expect(err).ToNot(HaveOccurred())
526+
Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
527+
Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
528+
})
529+
It("Does not return permissions when the NoValues option is set", func() {
530+
opt := svc.ListPermissionsQueryOptions{
531+
NoValues: true,
532+
}
533+
listSharesResponse.Shares = []*collaboration.Share{
534+
{
535+
Id: &collaboration.ShareId{OpaqueId: "1"},
536+
Permissions: &collaboration.SharePermissions{
537+
Permissions: roleconversions.NewViewerRole().CS3ResourcePermissions(),
538+
},
539+
ResourceId: &provider.ResourceId{
540+
StorageId: "1",
541+
SpaceId: "2",
542+
OpaqueId: "3",
543+
},
544+
Grantee: &provider.Grantee{
545+
Type: provider.GranteeType_GRANTEE_TYPE_USER,
546+
Id: &provider.Grantee_UserId{
547+
UserId: &userpb.UserId{
548+
OpaqueId: "user-id",
549+
},
550+
},
551+
},
552+
},
553+
}
554+
listPublicSharesResponse.Share = []*link.PublicShare{
555+
{
556+
Id: &link.PublicShareId{
557+
OpaqueId: "public-share-id",
558+
},
559+
Token: "public-share-token",
560+
// the link shares the same resource id
561+
ResourceId: &provider.ResourceId{
562+
StorageId: "1",
563+
SpaceId: "2",
564+
OpaqueId: "3",
565+
},
566+
Permissions: &link.PublicSharePermissions{Permissions: roleconversions.NewViewerRole().CS3ResourcePermissions()},
567+
},
568+
}
569+
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
570+
gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil)
571+
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
572+
gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil)
573+
permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, opt)
574+
Expect(err).ToNot(HaveOccurred())
575+
Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
576+
Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
577+
Expect(len(permissions.Value)).To(BeZero())
578+
})
579+
It("Returns a count when the Count option is set", func() {
580+
opt := svc.ListPermissionsQueryOptions{
581+
Count: true,
582+
}
583+
listSharesResponse.Shares = []*collaboration.Share{
584+
{
585+
Id: &collaboration.ShareId{OpaqueId: "1"},
586+
Permissions: &collaboration.SharePermissions{
587+
Permissions: roleconversions.NewViewerRole().CS3ResourcePermissions(),
588+
},
589+
ResourceId: &provider.ResourceId{
590+
StorageId: "1",
591+
SpaceId: "2",
592+
OpaqueId: "3",
593+
},
594+
Grantee: &provider.Grantee{
595+
Type: provider.GranteeType_GRANTEE_TYPE_USER,
596+
Id: &provider.Grantee_UserId{
597+
UserId: &userpb.UserId{
598+
OpaqueId: "user-id",
599+
},
600+
},
601+
},
602+
},
603+
}
604+
listPublicSharesResponse.Share = []*link.PublicShare{
605+
{
606+
Id: &link.PublicShareId{
607+
OpaqueId: "public-share-id",
608+
},
609+
Token: "public-share-token",
610+
// the link shares the same resource id
611+
ResourceId: &provider.ResourceId{
612+
StorageId: "1",
613+
SpaceId: "2",
614+
OpaqueId: "3",
615+
},
616+
Permissions: &link.PublicSharePermissions{Permissions: roleconversions.NewViewerRole().CS3ResourcePermissions()},
617+
},
618+
}
619+
gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
620+
gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil)
621+
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
622+
gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil)
623+
permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, opt)
624+
Expect(err).ToNot(HaveOccurred())
625+
Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
626+
Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
627+
count := int(permissions.GetOdataCount())
628+
Expect(count).To(Equal(2)) // 1 share + 1 public share
629+
Expect(len(permissions.Value)).To(Equal(count))
630+
})
518631
})
519632
Describe("ListSpaceRootPermissions", func() {
520633
var (

0 commit comments

Comments
 (0)