feat(sharing-ng): Support for updating space permissions

PATCH request on a space's root drive item should work now
This commit is contained in:
Ralf Haferkamp
2024-03-25 12:30:33 +01:00
committed by Ralf Haferkamp
parent dd2d6f4f85
commit 352d60a931
3 changed files with 190 additions and 61 deletions
+74 -27
View File
@@ -619,20 +619,20 @@ func (g Graph) UpdatePermission(w http.ResponseWriter, r *http.Request) {
}
permission := &libregraph.Permission{}
if err := StrictJSONUnmarshal(r.Body, permission); err != nil {
if err = StrictJSONUnmarshal(r.Body, permission); err != nil {
g.logger.Debug().Err(err).Interface("Body", r.Body).Msg("failed unmarshalling request body")
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid request body")
return
}
ctx := r.Context()
if err := validate.StructCtx(ctx, permission); err != nil {
if err = validate.StructCtx(ctx, permission); err != nil {
g.logger.Debug().Err(err).Interface("Body", r.Body).Msg("invalid request body")
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
return
}
oldPermission, sharedResourceID, err := g.getPermissionByID(ctx, permissionID)
oldPermission, sharedResourceID, err := g.getPermissionByID(ctx, permissionID, &itemID)
if err != nil {
errorcode.RenderError(w, r, err)
return
@@ -659,7 +659,7 @@ func (g Graph) UpdatePermission(w http.ResponseWriter, r *http.Request) {
}
// This is a user share
updatedPermission, err := g.updateUserShare(ctx, permissionID, permission)
updatedPermission, err := g.updateUserShare(ctx, permissionID, sharedResourceID, permission)
if err != nil {
errorcode.RenderError(w, r, err)
return
@@ -738,23 +738,9 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) {
}
func (g Graph) getPermissionByID(ctx context.Context, permissionID string) (*libregraph.Permission, *storageprovider.ResourceId, error) {
share, err := g.getCS3UserShareByID(ctx, permissionID)
func (g Graph) getPermissionByID(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId) (*libregraph.Permission, *storageprovider.ResourceId, error) {
publicShare, err := g.getCS3PublicShareByID(ctx, permissionID)
if err == nil {
permission, err := g.cs3UserShareToPermission(ctx, share)
if err != nil {
return nil, nil, err
}
return permission, share.GetResourceId(), nil
}
var errcode errorcode.Error
if errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound {
// there is no user share with that id, check if this is a public link
publicShare, err := g.getCS3PublicShareByID(ctx, permissionID)
if err != nil {
return nil, nil, err
}
permission, err := g.libreGraphPermissionFromCS3PublicShare(publicShare)
if err != nil {
return nil, nil, err
@@ -762,6 +748,45 @@ func (g Graph) getPermissionByID(ctx context.Context, permissionID string) (*lib
return permission, publicShare.GetResourceId(), nil
}
// The id is not referencing a public share, if the itemID is referencing
// a spaceroot this is a space permission. Handle that next
if IsSpaceRoot(itemID) {
gatewayClient, err := g.gatewaySelector.Next()
if err != nil {
g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed")
return nil, nil, err
}
// get space id
resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient)
if err != nil {
return nil, nil, err
}
perms, err := g.getSpaceRootPermissions(ctx, resourceInfo.GetSpace().GetId())
if err != nil {
return nil, nil, err
}
for _, p := range perms {
if p.GetId() == permissionID {
return &p, itemID, nil
}
}
}
var errcode errorcode.Error
if errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound {
// there is no public link with that id, check if this is a user share
share, err := g.getCS3UserShareByID(ctx, permissionID)
if err != nil {
return nil, nil, err
}
permission, err := g.cs3UserShareToPermission(ctx, share, false)
if err != nil {
return nil, nil, err
}
return permission, share.GetResourceId(), nil
}
return nil, nil, err
}
@@ -811,19 +836,37 @@ func (g Graph) getCS3UserShareByID(ctx context.Context, permissionID string) (*c
return getShareResp.GetShare(), nil
}
func (g Graph) updateUserShare(ctx context.Context, permissionID string, newPermission *libregraph.Permission) (*libregraph.Permission, error) {
func (g Graph) updateUserShare(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId, newPermission *libregraph.Permission) (*libregraph.Permission, error) {
gatewayClient, err := g.gatewaySelector.Next()
if err != nil {
g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed")
return nil, err
}
cs3UpdateShareReq := &collaboration.UpdateShareRequest{
Share: &collaboration.Share{
var cs3UpdateShareReq collaboration.UpdateShareRequest
// When updating a space root we need to reference the share by resourceId and grantee
if IsSpaceRoot(itemID) {
grantee, err := spacePermissionIdToCS3Grantee(permissionID)
if err != nil {
g.logger.Debug().Err(err).Str("permissionid", permissionID).Msg("failed to parse space permission id")
return nil, err
}
cs3UpdateShareReq.Share = &collaboration.Share{
ResourceId: itemID,
Grantee: &grantee,
}
cs3UpdateShareReq.Opaque = &types.Opaque{
Map: map[string]*types.OpaqueEntry{
"spacegrant": {},
},
}
cs3UpdateShareReq.Opaque = utils.AppendPlainToOpaque(cs3UpdateShareReq.Opaque, "spacetype", _spaceTypeProject)
} else {
cs3UpdateShareReq.Share = &collaboration.Share{
Id: &collaboration.ShareId{
OpaqueId: permissionID,
},
},
}
}
fieldmask := []string{}
if expiration, ok := newPermission.GetExpirationDateTimeOk(); ok {
@@ -842,8 +885,12 @@ func (g Graph) updateUserShare(ctx context.Context, permissionID string, newPerm
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, unifiedrole.UnifiedRoleConditionGrantee)
allowedResourceActions = unifiedrole.GetAllowedResourceActions(role, condition)
if len(allowedResourceActions) == 0 {
return nil, errorcode.New(errorcode.InvalidRequest, "role not applicable to this resource")
}
@@ -872,12 +919,12 @@ func (g Graph) updateUserShare(ctx context.Context, permissionID string, newPerm
Paths: fieldmask,
}
updateUserShareResp, err := gatewayClient.UpdateShare(ctx, cs3UpdateShareReq)
updateUserShareResp, err := gatewayClient.UpdateShare(ctx, &cs3UpdateShareReq)
if errCode := errorcode.FromCS3Status(updateUserShareResp.GetStatus(), err); errCode != nil {
return nil, *errCode
}
permission, err := g.cs3UserShareToPermission(ctx, updateUserShareResp.GetShare())
permission, err := g.cs3UserShareToPermission(ctx, updateUserShareResp.GetShare(), IsSpaceRoot(itemID))
if err != nil {
return nil, err
}
+100 -30
View File
@@ -12,7 +12,6 @@ import (
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
@@ -64,6 +63,7 @@ var _ = Describe("Driveitems", func() {
identityBackend *identitymocks.Backend
getPublicShareResponse *link.GetPublicShareResponse
getShareResponse *collaboration.GetShareResponse
listSpacesResponse *provider.ListStorageSpacesResponse
rr *httptest.ResponseRecorder
@@ -95,6 +95,31 @@ var _ = Describe("Driveitems", func() {
Status: status.NewNotFound(ctx, "not found"),
}
grantMapJSON, _ := json.Marshal(
map[string]*provider.ResourcePermissions{
"userid": roleconversions.NewSpaceViewerRole().CS3ResourcePermissions(),
},
)
spaceOpaque := &types.Opaque{
Map: map[string]*types.OpaqueEntry{
"grants": {
Decoder: "json",
Value: grantMapJSON,
},
},
}
listSpacesResponse = &provider.ListStorageSpacesResponse{
Status: status.NewOK(ctx),
StorageSpaces: []*provider.StorageSpace{
{
Id: &provider.StorageSpaceId{
OpaqueId: "2",
},
Opaque: spaceOpaque,
},
},
}
identityBackend = &identitymocks.Backend{}
newGroup = libregraph.NewGroup()
newGroup.SetMembersodataBind([]string{"/users/user1"})
@@ -277,7 +302,7 @@ var _ = Describe("Driveitems", func() {
driveItemPermission *libregraph.Permission
getShareMockResponse *collaboration.GetShareResponse
getPublicShareMockResponse *link.GetPublicShareResponse
getUserMockResponse *user.GetUserResponse
getUserMockResponse *userpb.GetUserResponse
updateShareMockResponse *collaboration.UpdateShareResponse
updatePublicShareMockResponse *link.UpdatePublicShareResponse
)
@@ -323,7 +348,7 @@ var _ = Describe("Driveitems", func() {
Grantee: &provider.Grantee{
Type: provider.GranteeType_GRANTEE_TYPE_USER,
Id: &provider.Grantee_UserId{
UserId: &user.UserId{
UserId: &userpb.UserId{
OpaqueId: "userid",
},
},
@@ -397,8 +422,33 @@ var _ = Describe("Driveitems", func() {
Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER,
},
}
statMock.Return(statResponse, nil)
spaceRootStatMock := gatewayClient.On("Stat",
mock.Anything,
mock.MatchedBy(func(req *provider.StatRequest) bool {
return utils.ResourceIDEqual(
req.GetRef().GetResourceId(),
&provider.ResourceId{
StorageId: "1",
SpaceId: "2",
OpaqueId: "2",
},
)
}))
spaceRootStatMock.Return(
&provider.StatResponse{
Status: status.NewOK(ctx),
Info: &provider.ResourceInfo{
Id: &provider.ResourceId{
StorageId: "1",
SpaceId: "2",
OpaqueId: "2",
},
Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER,
},
}, nil)
})
It("fails when no share is found", func() {
getShareMockResponse.Share = nil
@@ -615,6 +665,8 @@ var _ = Describe("Driveitems", func() {
Expect(rr.Code).To(Equal(http.StatusBadRequest))
})
It("updates the expiration date", func() {
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
expiration := time.Now().Add(time.Hour)
updateShareMock := gatewayClient.On("UpdateShare",
mock.Anything,
@@ -647,6 +699,8 @@ var _ = Describe("Driveitems", func() {
Expect(res.GetExpirationDateTime().Equal(expiration)).To(BeTrue())
})
It("deletes the expiration date", func() {
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
updateShareMock := gatewayClient.On("UpdateShare",
mock.Anything,
mock.MatchedBy(func(req *collaboration.UpdateShareRequest) bool {
@@ -678,6 +732,8 @@ var _ = Describe("Driveitems", func() {
Expect(ok).To(BeFalse())
})
It("updates the share permissions with changing the role", func() {
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
updateShareMock := gatewayClient.On("UpdateShare",
mock.Anything,
mock.MatchedBy(func(req *collaboration.UpdateShareRequest) bool {
@@ -706,6 +762,8 @@ var _ = Describe("Driveitems", func() {
Expect(ok).To(BeTrue())
})
It("fails to update the share permissions for a file share when setting a space specific role", func() {
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
updateShareMock := gatewayClient.On("UpdateShare",
mock.Anything,
mock.MatchedBy(func(req *collaboration.UpdateShareRequest) bool {
@@ -724,7 +782,44 @@ var _ = Describe("Driveitems", func() {
)
Expect(rr.Code).To(Equal(http.StatusBadRequest))
})
It("fails to update the space permissions for a space share when setting a file specific role", func() {
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
gatewayClient.On("GetPublicShare",
mock.Anything,
mock.Anything,
).Return(getPublicShareMockResponse, nil)
gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(listSpacesResponse, nil)
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
updateShareMock := gatewayClient.On("UpdateShare",
mock.Anything,
mock.MatchedBy(func(req *collaboration.UpdateShareRequest) bool {
return req.GetShare().GetId().GetOpaqueId() == "permissionid"
}),
)
updateShareMock.Return(updateShareMockResponse, nil)
driveItemPermission.SetRoles([]string{unifiedrole.NewFileEditorUnifiedRole(false).GetId()})
body, err := driveItemPermission.MarshalJSON()
Expect(err).To(BeNil())
rctx := chi.NewRouteContext()
rctx.URLParams.Add("driveID", "1$2")
// This is a space root
rctx.URLParams.Add("itemID", "1$2!2")
rctx.URLParams.Add("permissionID", "u:userid")
ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx)
svc.UpdatePermission(
rr,
httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))).
WithContext(ctx),
)
Expect(rr.Code).To(Equal(http.StatusBadRequest))
})
It("updates the share permissions when changing the resource permission actions", func() {
getPublicShareMockResponse.Share = nil
getPublicShareMockResponse.Status = status.NewNotFound(ctx, "not found")
updateShareMock := gatewayClient.On("UpdateShare",
mock.Anything,
mock.MatchedBy(func(req *collaboration.UpdateShareRequest) bool {
@@ -1324,32 +1419,7 @@ var _ = Describe("Driveitems", func() {
rctx.URLParams.Add("driveID", "1$2")
rctx.URLParams.Add("itemID", "1$2!2")
statResponse.Info.Id.OpaqueId = "2"
grantMap := map[string]*provider.ResourcePermissions{
"userid": roleconversions.NewSpaceViewerRole().CS3ResourcePermissions(),
}
grantMapJSON, _ := json.Marshal(grantMap)
spaceOpaque := &types.Opaque{
Map: map[string]*types.OpaqueEntry{
"grants": {
Decoder: "json",
Value: grantMapJSON,
},
},
}
listSpacesMock := gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything)
listSpacesResponse := &provider.ListStorageSpacesResponse{
Status: status.NewOK(ctx),
StorageSpaces: []*provider.StorageSpace{
{
Id: &provider.StorageSpaceId{
OpaqueId: "2",
},
Opaque: spaceOpaque,
},
},
}
listSpacesMock.Return(listSpacesResponse, nil)
gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(listSpacesResponse, nil)
getUserMock := gatewayClient.On("GetUser", mock.Anything, mock.Anything)
getUserMockResponse := &userpb.GetUserResponse{
+16 -4
View File
@@ -124,7 +124,7 @@ func (g Graph) cs3UserSharesToDriveItems(ctx context.Context, shares []*collabor
}
item = *itemptr
}
perm, err := g.cs3UserShareToPermission(ctx, s)
perm, err := g.cs3UserShareToPermission(ctx, s, false)
var errcode errorcode.Error
switch {
@@ -140,10 +140,12 @@ func (g Graph) cs3UserSharesToDriveItems(ctx context.Context, shares []*collabor
return driveItems, nil
}
func (g Graph) cs3UserShareToPermission(ctx context.Context, share *collaboration.Share) (*libregraph.Permission, error) {
func (g Graph) cs3UserShareToPermission(ctx context.Context, share *collaboration.Share, isSpacePermission bool) (*libregraph.Permission, error) {
perm := libregraph.Permission{}
perm.SetRoles([]string{})
perm.SetId(share.Id.OpaqueId)
if !isSpacePermission {
perm.SetId(share.GetId().GetOpaqueId())
}
grantedTo := libregraph.SharePointIdentitySet{}
switch share.GetGrantee().GetType() {
case storageprovider.GranteeType_GRANTEE_TYPE_USER:
@@ -157,6 +159,9 @@ func (g Graph) cs3UserShareToPermission(ctx context.Context, share *collaboratio
return nil, errorcode.New(errorcode.GeneralException, err.Error())
default:
grantedTo.SetUser(user)
if isSpacePermission {
perm.SetId("u:" + user.GetId())
}
}
case storageprovider.GranteeType_GRANTEE_TYPE_GROUP:
group, err := groupIdToIdentity(ctx, g.identityCache, share.Grantee.GetGroupId().GetOpaqueId())
@@ -169,6 +174,9 @@ func (g Graph) cs3UserShareToPermission(ctx context.Context, share *collaboratio
return nil, errorcode.New(errorcode.GeneralException, err.Error())
default:
grantedTo.SetGroup(group)
if isSpacePermission {
perm.SetId("g:" + group.GetId())
}
}
}
@@ -176,9 +184,13 @@ func (g Graph) cs3UserShareToPermission(ctx context.Context, share *collaboratio
if share.GetExpiration() != nil {
perm.SetExpirationDateTime(cs3TimestampToTime(share.GetExpiration()))
}
condition := unifiedrole.UnifiedRoleConditionGrantee
if isSpacePermission {
condition = unifiedrole.UnifiedRoleConditionOwner
}
role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(
*share.GetPermissions().GetPermissions(),
unifiedrole.UnifiedRoleConditionGrantee,
condition,
g.config.FilesSharing.EnableResharing,
)
if role != nil {