Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow querying federated sharing roles #9765

Merged
merged 3 commits into from
Aug 13, 2024
Merged

allow querying federated sharing roles #9765

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
966b4e1
allow querying federated sharing roles
butonic Aug 8, 2024
8da4e26
Update services/graph/pkg/unifiedrole/unifiedrole.go
butonic Aug 13, 2024
99c6b66
no need to introduce dedicated roles
butonic Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/federated-user-roles.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Enhancement: allow querying federated user roles for sharing

When listing permissions clients can now fetch the list of available federated sharing roles by sending a `GET /graph/v1beta1/drives/{driveid}/items/{itemid}/[email protected]/rolePermissions/any(p:contains(p/condition, '@Subject.UserType=="Federated"'))` request. Note that this is the only supported filter expression. Federated sharing roles will be omitted from requests without this filter.

https://github.com/owncloud/ocis/pull/9765
30 changes: 16 additions & 14 deletions services/graph/mocks/drive_item_permissions_provider.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 23 additions & 5 deletions services/graph/pkg/service/v0/api_driveitem_permissions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const (
type DriveItemPermissionsProvider interface {
Invite(ctx context.Context, resourceId *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error)
SpaceRootInvite(ctx context.Context, driveID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error)
ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error)
ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error)
ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error)
DeletePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string) error
DeleteSpaceRootPermission(ctx context.Context, driveID *storageprovider.ResourceId, permissionID string) error
Expand Down Expand Up @@ -283,7 +283,7 @@ func createShareRequestToFederatedUser(user libregraph.User, resourceId *storage
Id: &storageprovider.Grantee_UserId{UserId: &userpb.UserId{
Type: userpb.UserType_USER_TYPE_FEDERATED,
OpaqueId: user.GetId(),
Idp: providerInfo.Domain, // the domain is persisted in the grant as u:{opaqueid}:{domain}
Idp: *user.GetIdentities()[0].Issuer, // the domain is persisted in the grant as u:{opaqueid}:{domain}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it guaranteed that
A) len(user.GetIdentities()) > 0
B) user.GetIdentities()[0] != nil
? Otherwise we need checks for this because it would panic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes there is a check above the only line making a call to this function:

if len(user.Identities) < 1 {
    return libregraph.Permission{}, errorcode.New(errorcode.InvalidRequest, "user has no federated identity")
}
// [...]
createShareRequest := createShareRequestToFederatedUser(user, statResponse.GetInfo().GetId(), providerInfoResp.ProviderInfo, cs3ResourcePermissions)

}},
},
RecipientMeshProvider: providerInfo,
Expand Down Expand Up @@ -320,7 +320,7 @@ func (s DriveItemPermissionsService) SpaceRootInvite(ctx context.Context, driveI
}

// ListPermissions lists the permissions of a driveItem
func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
collectionOfPermissions := libregraph.CollectionOfPermissionsWithAllowedValues{}
gatewayClient, err := s.gatewaySelector.Next()
if err != nil {
Expand All @@ -347,6 +347,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
unifiedrole.GetApplicableRoleDefinitionsForActions(
allowedActions,
condition,
listFederatedRoles,
false,
),
),
Expand All @@ -358,6 +359,13 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
collectionOfPermissions.LibreGraphPermissionsRolesAllowedValues[i] = definition
}

if selectRoles {
// drop the actions
collectionOfPermissions.LibreGraphPermissionsActionsAllowedValues = nil
// no need to fetch shares, we are only interested in the roles
return collectionOfPermissions, nil
}

driveItems := make(driveItemsByResourceID)
if IsSpaceRoot(statResponse.GetInfo().GetId()) {
permissions, err := s.getSpaceRootPermissions(ctx, statResponse.GetInfo().GetSpace().GetId())
Expand Down Expand Up @@ -408,7 +416,7 @@ func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Contex
}

rootResourceID := space.GetRoot()
return s.ListPermissions(ctx, rootResourceID)
return s.ListPermissions(ctx, rootResourceID, false, false) // federated roles are not supported for spaces
}

// DeletePermission deletes a permission from a drive item
Expand Down Expand Up @@ -626,9 +634,19 @@ func (api DriveItemPermissionsApi) ListPermissions(w http.ResponseWriter, r *htt
return
}

var listFederatedRoles bool
if GetFilterParam(r) == "@libre.graph.permissions.roles.allowedValues/rolePermissions/any(p:contains(p/condition, '@Subject.UserType==\"Federated\"'))" {
listFederatedRoles = true
}

var selectRoles bool
if GetSelectParam(r) == "@libre.graph.permissions.roles.allowedValues" {
selectRoles = true
}

ctx := r.Context()

permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, &itemID)
permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, &itemID, listFederatedRoles, selectRoles)
if err != nil {
errorcode.RenderError(w, r, err)
return
Expand Down
21 changes: 9 additions & 12 deletions services/graph/pkg/service/v0/api_driveitem_permissions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
"github.com/go-chi/chi/v5"
. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -367,7 +366,7 @@ var _ = Describe("DriveItemPermissionsService", 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)
permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID)
permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, false)
Expect(err).ToNot(HaveOccurred())
Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
Expand Down Expand Up @@ -414,7 +413,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil)
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil)
permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID)
permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, false)
Expect(err).ToNot(HaveOccurred())
Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero())
Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero())
Expand Down Expand Up @@ -806,10 +805,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
gatewayClient.On("UpdateShare",
mock.Anything,
mock.MatchedBy(func(req *collaboration.UpdateShareRequest) bool {
if req.GetShare().GetId().GetOpaqueId() == "permissionid" {
return true
}
return false
return req.GetShare().GetId().GetOpaqueId() == "permissionid"
}),
).Return(updateShareMockResponse, nil)

Expand Down Expand Up @@ -1072,7 +1068,7 @@ var _ = Describe("DriveItemPermissionsApi", func() {

onInvite := mockProvider.On("Invite", mock.Anything, mock.Anything, mock.Anything)

onInvite.Return(func(ctx context.Context, resourceID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) {
onInvite.Return(func(ctx context.Context, resourceID *provider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) {
return libregraph.Permission{}, errors.New("any")
}).Once()

Expand All @@ -1088,7 +1084,7 @@ var _ = Describe("DriveItemPermissionsApi", func() {
Expect(err).ToNot(HaveOccurred())

onInvite := mockProvider.On("Invite", mock.Anything, mock.Anything, mock.Anything)
onInvite.Return(func(ctx context.Context, resourceID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) {
onInvite.Return(func(ctx context.Context, resourceID *provider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) {
Expect(storagespace.FormatResourceID(resourceID)).To(Equal("1$2!3"))
return libregraph.Permission{}, nil
}).Once()
Expand All @@ -1109,7 +1105,7 @@ var _ = Describe("DriveItemPermissionsApi", func() {
Expect(err).ToNot(HaveOccurred())

onInvite := mockProvider.On("SpaceRootInvite", mock.Anything, mock.Anything, mock.Anything)
onInvite.Return(func(ctx context.Context, driveID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) {
onInvite.Return(func(ctx context.Context, driveID *provider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) {
Expect(storagespace.FormatResourceID(driveID)).To(Equal("1$2"))
return libregraph.Permission{}, nil
}).Once()
Expand Down Expand Up @@ -1144,8 +1140,9 @@ var _ = Describe("DriveItemPermissionsApi", func() {
inviteJson, err := json.Marshal(invite)
Expect(err).ToNot(HaveOccurred())

mockProvider.On("ListPermissions", mock.Anything, mock.Anything, mock.Anything).
Return(func(ctx context.Context, itemid *storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
mockProvider.On("ListPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(func(ctx context.Context, itemid *provider.ResourceId, listFederatedRoles, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) {
Expect(listFederatedRoles).To(Equal(false))
Expect(storagespace.FormatResourceID(itemid)).To(Equal("1$2!3"))
return libregraph.CollectionOfPermissionsWithAllowedValues{}, nil
}).Once()
Expand Down
9 changes: 5 additions & 4 deletions services/graph/pkg/service/v0/sharedwithme_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/stretchr/testify/mock"
"github.com/tidwall/gjson"
"google.golang.org/grpc"
"google.golang.org/protobuf/proto"

libregraph "github.com/owncloud/libre-graph-api-go"
"github.com/owncloud/ocis/v2/ocis-pkg/shared"
Expand Down Expand Up @@ -182,16 +183,16 @@ var _ = Describe("SharedWithMe", func() {
}

gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(func(_ context.Context, r *providerv1beta1.StatRequest, _ ...grpc.CallOption) (*providerv1beta1.StatResponse, error) {
// copy the response to avoid side effects
res := proto.Clone(statResponse).(*providerv1beta1.StatResponse)
for _, share := range listReceivedSharesResponse.Shares {
if share.Share.ResourceId != r.Ref.ResourceId {
continue
}

if statResponse.Info.Id == nil {
statResponse.Info.Id = share.Share.ResourceId
}
res.Info.Id = share.Share.ResourceId

return statResponse, nil
return res, nil
}

return nil, nil
Expand Down
10 changes: 10 additions & 0 deletions services/graph/pkg/service/v0/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ func GetDriveAndItemIDParam(r *http.Request, logger *log.Logger) (storageprovide
return driveID, itemID, nil
}

// GetFilterParam returns the $filter query parameter from the request. If you need to parse the filter use godata.ParseRequest
func GetFilterParam(r *http.Request) string {
return r.URL.Query().Get("$filter")
}

// GetSelectParam returns the $select query parameter from the request. If you need to parse the select filter use godata.ParseRequest
func GetSelectParam(r *http.Request) string {
return r.URL.Query().Get("$select")
}

// GetGatewayClient returns a gateway client from the gatewaySelector.
func (g Graph) GetGatewayClient(w http.ResponseWriter, r *http.Request) (gateway.GatewayAPIClient, bool) {
gatewayClient, err := g.gatewaySelector.Next()
Expand Down
Loading