From 7c34505f544336d8cdbac0b0da95441f8712382e Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Tue, 10 Sep 2024 13:03:51 +0200 Subject: [PATCH] fix: use FromCS3Status error helper --- .../service/v0/api_driveitem_permissions.go | 30 ++++--- services/graph/pkg/service/v0/base.go | 85 +++++++++---------- 2 files changed, 56 insertions(+), 59 deletions(-) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 6f843b4042..9cdc557724 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -16,15 +16,16 @@ import ( ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/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/go-chi/render" + libregraph "github.com/owncloud/libre-graph-api-go" + revactx "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/publicshare" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/v2/pkg/share" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" - "github.com/go-chi/chi/v5" - "github.com/go-chi/render" - libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/ocis-pkg/l10n" l10n_pkg "github.com/owncloud/ocis/v2/services/graph/pkg/l10n" @@ -496,10 +497,10 @@ func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemI return s.removeSpacePermission(ctx, permissionID, sharedResourceID) case OCM: return s.removeOCMPermission(ctx, permissionID) + default: + // This should never be reached + return errorcode.New(errorcode.GeneralException, "failed to delete permission") } - - // This should never be reached - return errorcode.New(errorcode.GeneralException, "failed to delete permission") } // DeleteSpaceRootPermission deletes a permission on the root item of a project space @@ -525,15 +526,15 @@ func (s DriveItemPermissionsService) DeleteSpaceRootPermission(ctx context.Conte // UpdatePermission updates a permission on a drive item func (s DriveItemPermissionsService) UpdatePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string, newPermission libregraph.Permission) (libregraph.Permission, error) { oldPermission, sharedResourceID, err := s.getPermissionByID(ctx, permissionID, itemID) + + // try to get the permission from ocm if the permission was not found first place + if err != nil && s.config.IncludeOCMSharees { + oldPermission, sharedResourceID, err = s.getOCMPermissionByID(ctx, permissionID, itemID) + } + + // if we still can't find the permission, return an error if err != nil { - if s.config.IncludeOCMSharees { - oldPermission, sharedResourceID, err = s.getOCMPermissionByID(ctx, permissionID, itemID) - if err != nil { - return libregraph.Permission{}, err - } - } else { - return libregraph.Permission{}, err - } + return libregraph.Permission{}, err } // The resourceID of the shared resource need to match the item ID from the Request Path @@ -565,6 +566,7 @@ func (s DriveItemPermissionsService) UpdatePermission(ctx context.Context, itemI return *updatePermission, nil } } + return libregraph.Permission{}, err } diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index bf860345ad..dcbb3bf2bd 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -640,13 +640,10 @@ func (g BaseGraphService) getCS3PublicShareByID(ctx context.Context, permissionI }, }, ) - if err != nil { - return nil, err - } - if err := errorcode.FromCS3Status(getPublicShareResp.GetStatus(), err); err != nil { return nil, err } + return getPublicShareResp.GetShare(), nil } @@ -666,14 +663,12 @@ func (g BaseGraphService) removeOCMPermission(ctx context.Context, permissionID }, }, }, - }) - if err != nil { - return err - } - + }, + ) if err := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); err != nil { return err } + // We need to return an untyped nil here otherwise the error==nil check won't work return nil } @@ -694,14 +689,12 @@ func (g BaseGraphService) removePublicShare(ctx context.Context, permissionID st }, }, }, - }) - if err != nil { - return err - } - + }, + ) if err := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); err != nil { return err } + // We need to return an untyped nil here otherwise the error==nil check won't work return nil } @@ -722,14 +715,12 @@ func (g BaseGraphService) removeUserShare(ctx context.Context, permissionID stri }, }, }, - }) - if err != nil { - return err - } - + }, + ) if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil { return err } + // We need to return an untyped nil here otherwise the error==nil check won't work return nil } @@ -755,13 +746,10 @@ func (g BaseGraphService) removeSpacePermission(ctx context.Context, permissionI }, }, }) - if err != nil { - return err - } - if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil { return err } + // We need to return an untyped nil here otherwise the error==nil check won't work return nil } @@ -771,6 +759,7 @@ func (g BaseGraphService) getOCMPermissionResourceID(ctx context.Context, permis if err != nil { return nil, err } + return cs3Share.GetResourceId(), nil } @@ -790,14 +779,12 @@ func (g BaseGraphService) getCS3OCMShareByID(ctx context.Context, permissionID s }, }, }, - }) - if err != nil { - return nil, err - } - + }, + ) if err := errorcode.FromCS3Status(getShareResp.GetStatus(), err); err != nil { return nil, err } + return getShareResp.GetShare(), nil } @@ -806,6 +793,7 @@ func (g BaseGraphService) getUserPermissionResourceID(ctx context.Context, permi if err != nil { return nil, err } + return cs3Share.GetResourceId(), nil } @@ -825,14 +813,12 @@ func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID }, }, }, - }) - if err != nil { - return nil, err - } - + }, + ) if err := errorcode.FromCS3Status(getShareResp.GetStatus(), err); err != nil { return nil, err } + return getShareResp.GetShare(), nil } @@ -842,22 +828,27 @@ func (g BaseGraphService) getOCMPermissionByID(ctx context.Context, permissionID g.logger.Debug().Err(err).Msg("selecting gatewaySelevtor failed") return nil, nil, err } + ocmShare, err := g.getCS3OCMShareByID(ctx, permissionID) if err != nil { return nil, nil, err } + 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.cs3OCMShareToPermission(ctx, ocmShare, condition) if err != nil { return nil, nil, err } + return permission, ocmShare.GetResourceId(), nil } @@ -900,18 +891,22 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st if err != nil { return nil, nil, err } + 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, cs3Share, condition) if err != nil { return nil, nil, err } + return permission, cs3Share.GetResourceId(), nil } @@ -929,10 +924,12 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID if err != nil { return nil, err } + condition, err := roleConditionForResourceType(resourceInfo) if err != nil { return nil, err } + var cs3UpdateOCMShareReq ocm.UpdateOCMShareRequest cs3UpdateOCMShareReq.Ref = &ocm.ShareReference{ Spec: &ocm.ShareReference_Id{ @@ -941,12 +938,15 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID }, }, } + if expiration, ok := newPermission.GetExpirationDateTimeOk(); ok { - cs3UpdateOCMShareReq.Field = append(cs3UpdateOCMShareReq.Field, &ocm.UpdateOCMShareRequest_UpdateField{ - Field: &ocm.UpdateOCMShareRequest_UpdateField_Expiration{ - Expiration: utils.TimeToTS(*expiration), + cs3UpdateOCMShareReq.Field = append( + cs3UpdateOCMShareReq.Field, + &ocm.UpdateOCMShareRequest_UpdateField{ + Field: &ocm.UpdateOCMShareRequest_UpdateField_Expiration{ + Expiration: utils.TimeToTS(*expiration), + }, }, - }, ) } @@ -981,7 +981,6 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID Permissions: unifiedrole.PermissionsToCS3ResourcePermissions( []*libregraph.UnifiedRolePermission{ { - AllowedResourceActions: allowedResourceActions, }, }, @@ -995,9 +994,6 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID } updateOCMShareResp, err := gatewayClient.UpdateOCMShare(ctx, &cs3UpdateOCMShareReq) - if err != nil { - return nil, err - } if err := errorcode.FromCS3Status(updateOCMShareResp.GetStatus(), err); err != nil { return nil, err } @@ -1011,13 +1007,15 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID }, }, }) - if err != nil { + if err := errorcode.FromCS3Status(ocmShareResp.GetStatus(), err); err != nil { return nil, err } + permission, err := g.cs3OCMShareToPermission(ctx, ocmShareResp.GetShare(), condition) if err != nil { return nil, err } + return permission, nil } @@ -1108,9 +1106,6 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri } updateUserShareResp, err := gatewayClient.UpdateShare(ctx, &cs3UpdateShareReq) - if err != nil { - return nil, err - } if err := errorcode.FromCS3Status(updateUserShareResp.GetStatus(), err); err != nil { return nil, err }