From dc355b7f8a104240d511328b83d04d21eaf80667 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Fri, 7 Jun 2024 14:14:13 +0200 Subject: [PATCH] fixed the error translation from the statusCodeError type into a corresponding graph api Error representation --- .../unreleased/fix-error-translation-utils.md | 6 +++ go.mod | 2 +- go.sum | 4 +- services/graph/pkg/errorcode/cs3.go | 10 +++++ .../service/v0/api_driveitem_permissions.go | 8 ++-- .../v0/api_driveitem_permissions_links.go | 4 +- services/graph/pkg/service/v0/base.go | 2 +- .../apiSharingNg/removeAccessToDrive.feature | 6 +-- .../shareSubItemOfSpace.feature | 2 +- .../cs3org/reva/v2/pkg/utils/grpc.go | 44 +++++++++++++------ vendor/modules.txt | 2 +- 11 files changed, 61 insertions(+), 29 deletions(-) create mode 100644 changelog/unreleased/fix-error-translation-utils.md diff --git a/changelog/unreleased/fix-error-translation-utils.md b/changelog/unreleased/fix-error-translation-utils.md new file mode 100644 index 00000000000..0df7050bdc1 --- /dev/null +++ b/changelog/unreleased/fix-error-translation-utils.md @@ -0,0 +1,6 @@ +Bugfix: Fix the error translation from utils + +We've fixed the error translation from the statusCodeError type to CS3 Status because the FromCS3Status function converts a CS3 status code into a corresponding local Error representation. + +https://github.com/owncloud/ocis/pull/9331 +https://github.com/owncloud/ocis/issues/9151 diff --git a/go.mod b/go.mod index 080c0417ca5..7a0cf706ce5 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/cenkalti/backoff v2.2.1+incompatible github.com/coreos/go-oidc/v3 v3.10.0 github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 - github.com/cs3org/reva/v2 v2.19.2-0.20240606075653-a7a1d2d2dace + github.com/cs3org/reva/v2 v2.19.2-0.20240607120909-0a9c7b8bef6a github.com/dhowden/tag v0.0.0-20230630033851-978a0926ee25 github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e github.com/egirna/icap-client v0.1.1 diff --git a/go.sum b/go.sum index fda06c77be8..79351b1b13f 100644 --- a/go.sum +++ b/go.sum @@ -1025,8 +1025,8 @@ github.com/crewjam/saml v0.4.14 h1:g9FBNx62osKusnFzs3QTN5L9CVA/Egfgm+stJShzw/c= github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1nl2mME= github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 h1:BUdwkIlf8IS2FasrrPg8gGPHQPOrQ18MS1Oew2tmGtY= github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= -github.com/cs3org/reva/v2 v2.19.2-0.20240606075653-a7a1d2d2dace h1:zK+0QyrqRBwdRthUbXTyDhxZIMZlNJPzGr0+bmyU++0= -github.com/cs3org/reva/v2 v2.19.2-0.20240606075653-a7a1d2d2dace/go.mod h1:lKqw0VuP1NcZbhj0e6tGoAGq3tgWO/pLafVJyDK0yVI= +github.com/cs3org/reva/v2 v2.19.2-0.20240607120909-0a9c7b8bef6a h1:D1Pg9i+pZhzn/h5397/ukX0XYYPBaUVLh6Zhe83vKUg= +github.com/cs3org/reva/v2 v2.19.2-0.20240607120909-0a9c7b8bef6a/go.mod h1:lKqw0VuP1NcZbhj0e6tGoAGq3tgWO/pLafVJyDK0yVI= github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4= github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= diff --git a/services/graph/pkg/errorcode/cs3.go b/services/graph/pkg/errorcode/cs3.go index 6b3f71be317..9367d93fcbd 100644 --- a/services/graph/pkg/errorcode/cs3.go +++ b/services/graph/pkg/errorcode/cs3.go @@ -5,6 +5,7 @@ import ( cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/v2/pkg/utils" ) // FromCS3Status converts a CS3 status code and an error into a corresponding local Error representation. @@ -69,3 +70,12 @@ func FromStat(stat *provider.StatResponse, err error, ignore ...cs3rpc.Code) err // TODO: look into ResourceInfo to get the postprocessing state and map that to 425 status? return FromCS3Status(stat.GetStatus(), err, ignore...) } + +// FromUtilsStatusCodeError returns original error if `err` does not match to the statusCodeError type +func FromUtilsStatusCodeError(err error, ignore ...cs3rpc.Code) error { + stat := utils.StatusCodeErrorToCS3Status(err) + if stat == nil { + return FromCS3Status(nil, err, ignore...) + } + return FromCS3Status(stat, nil, ignore...) +} diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index ee7916850d2..45ad500834c 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -207,7 +207,7 @@ func (s DriveItemPermissionsService) SpaceRootInvite(ctx context.Context, driveI space, err := utils.GetSpace(ctx, storagespace.FormatResourceID(driveID), gatewayClient) if err != nil { - return libregraph.Permission{}, err + return libregraph.Permission{}, errorcode.FromUtilsStatusCodeError(err) } if space.SpaceType != _spaceTypeProject { @@ -298,7 +298,7 @@ func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Contex space, err := utils.GetSpace(ctx, storagespace.FormatResourceID(driveID), gatewayClient) if err != nil { - return collectionOfPermissions, err + return collectionOfPermissions, errorcode.FromUtilsStatusCodeError(err) } if space.SpaceType != _spaceTypeProject { @@ -371,7 +371,7 @@ func (s DriveItemPermissionsService) DeleteSpaceRootPermission(ctx context.Conte space, err := utils.GetSpace(ctx, storagespace.FormatResourceID(driveID), gatewayClient) if err != nil { - return err + return errorcode.FromUtilsStatusCodeError(err) } if space.SpaceType != _spaceTypeProject { @@ -422,7 +422,7 @@ func (s DriveItemPermissionsService) UpdateSpaceRootPermission(ctx context.Conte space, err := utils.GetSpace(ctx, storagespace.FormatResourceID(driveID), gatewayClient) if err != nil { - return libregraph.Permission{}, err + return libregraph.Permission{}, errorcode.FromUtilsStatusCodeError(err) } if space.SpaceType != _spaceTypeProject { diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions_links.go b/services/graph/pkg/service/v0/api_driveitem_permissions_links.go index 3de834a331d..8675884af2e 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_links.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions_links.go @@ -102,7 +102,7 @@ func (s DriveItemPermissionsService) CreateSpaceRootLink(ctx context.Context, dr } space, err := utils.GetSpace(ctx, storagespace.FormatResourceID(driveID), gatewayClient) if err != nil { - return libregraph.Permission{}, err + return libregraph.Permission{}, errorcode.FromUtilsStatusCodeError(err) } if space.SpaceType != _spaceTypeProject { @@ -140,7 +140,7 @@ func (s DriveItemPermissionsService) SetPublicLinkPasswordOnSpaceRoot(ctx contex } space, err := utils.GetSpace(ctx, storagespace.FormatResourceID(driveID), gatewayClient) if err != nil { - return libregraph.Permission{}, err + return libregraph.Permission{}, errorcode.FromUtilsStatusCodeError(err) } if space.SpaceType != _spaceTypeProject { diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index f78a182c9df..a88bb84e6bf 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -55,7 +55,7 @@ func (g BaseGraphService) getSpaceRootPermissions(ctx context.Context, spaceID * } space, err := utils.GetSpace(ctx, spaceID.GetOpaqueId(), gatewayClient) if err != nil { - return nil, err + return nil, errorcode.FromUtilsStatusCodeError(err) } return g.cs3SpacePermissionsToLibreGraph(ctx, space, APIVersion_1_Beta_1), nil diff --git a/tests/acceptance/features/apiSharingNg/removeAccessToDrive.feature b/tests/acceptance/features/apiSharingNg/removeAccessToDrive.feature index c9549aa033c..8d7df68af11 100644 --- a/tests/acceptance/features/apiSharingNg/removeAccessToDrive.feature +++ b/tests/acceptance/features/apiSharingNg/removeAccessToDrive.feature @@ -197,7 +197,7 @@ Feature: Remove access to a drive | permissionsRole | | | password | %public% | When user "Brian" tries to remove the link from space "NewSpace" owned by "Alice" using root endpoint of the Graph API - Then the HTTP status code should be "500" + Then the HTTP status code should be "404" Examples: | permissions-role | | view | @@ -214,7 +214,7 @@ Feature: Remove access to a drive | space | NewSpace | | permissionsRole | internal | When user "Brian" tries to remove the link from space "NewSpace" owned by "Alice" using root endpoint of the Graph API - Then the HTTP status code should be "500" + Then the HTTP status code should be "404" Scenario: remove link share of a project drive using permissions endpoint @@ -226,4 +226,4 @@ Feature: Remove access to a drive | permissionsRole | view | | password | $heLlo*1234* | When user "Alice" deletes the last link share of space "projectSpace" using permissions endpoint of the Graph API - Then the HTTP status code should be "204" \ No newline at end of file + Then the HTTP status code should be "204" diff --git a/tests/acceptance/features/apiSpacesShares/shareSubItemOfSpace.feature b/tests/acceptance/features/apiSpacesShares/shareSubItemOfSpace.feature index 943c4c2f545..aea731499fa 100644 --- a/tests/acceptance/features/apiSpacesShares/shareSubItemOfSpace.feature +++ b/tests/acceptance/features/apiSpacesShares/shareSubItemOfSpace.feature @@ -168,7 +168,7 @@ Feature: Share a file or folder that is inside a space | expirationDateTime | 2042-01-01T23:59:59.000Z | When user "Alice" changes the last share with settings: | role | | - Then the HTTP status code should be "400" + Then the HTTP status code should be "500" Scenario: check the end of expiration date in user share diff --git a/vendor/github.com/cs3org/reva/v2/pkg/utils/grpc.go b/vendor/github.com/cs3org/reva/v2/pkg/utils/grpc.go index 86d2b065121..e82e9895366 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/utils/grpc.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/utils/grpc.go @@ -14,7 +14,6 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" - "google.golang.org/grpc/metadata" ) @@ -54,7 +53,7 @@ func GetServiceUserContextWithContext(ctx context.Context, gwc gateway.GatewayAP return nil, err } - if err := checkStatusCode("authenticating service user", authRes.GetStatus().GetCode()); err != nil { + if err := checkStatusCode("authenticating service user", authRes.GetStatus().GetMessage(), authRes.GetStatus().GetCode()); err != nil { return nil, err } @@ -74,7 +73,7 @@ func GetUserWithContext(ctx context.Context, userID *user.UserId, gwc gateway.Ga return nil, err } - if err := checkStatusCode("getting user", getUserResponse.GetStatus().GetCode()); err != nil { + if err := checkStatusCode("getting user", getUserResponse.GetStatus().GetMessage(), getUserResponse.GetStatus().GetCode()); err != nil { return nil, err } @@ -88,7 +87,7 @@ func GetAcceptedUserWithContext(ctx context.Context, userID *user.UserId, gwc ga return nil, err } - if err := checkStatusCode("getting accepted user", getAcceptedUserResponse.GetStatus().GetCode()); err != nil { + if err := checkStatusCode("getting accepted user", getAcceptedUserResponse.GetStatus().GetMessage(), getAcceptedUserResponse.GetStatus().GetCode()); err != nil { return nil, err } @@ -102,12 +101,12 @@ func GetSpace(ctx context.Context, spaceID string, gwc gateway.GatewayAPIClient) return nil, err } - if err := checkStatusCode("getting space", res.GetStatus().GetCode()); err != nil { + if err := checkStatusCode("getting space", res.GetStatus().GetMessage(), res.GetStatus().GetCode()); err != nil { return nil, err } if len(res.StorageSpaces) == 0 { - return nil, statusCodeError{"getting space", rpc.Code_CODE_NOT_FOUND} + return nil, statusCodeError{"getting space", "", rpc.Code_CODE_NOT_FOUND} } return res.StorageSpaces[0], nil @@ -120,7 +119,7 @@ func GetGroupMembers(ctx context.Context, groupID string, gwc gateway.GatewayAPI return nil, err } - if err := checkStatusCode("getting group", r.GetStatus().GetCode()); err != nil { + if err := checkStatusCode("getting group", r.GetStatus().GetMessage(), r.GetStatus().GetCode()); err != nil { return nil, err } @@ -184,7 +183,7 @@ func GetResource(ctx context.Context, ref *storageprovider.Reference, gwc gatewa return nil, err } - if err := checkStatusCode("getting resource", res.GetStatus().GetCode()); err != nil { + if err := checkStatusCode("getting resource", res.GetStatus().GetMessage(), res.GetStatus().GetCode()); err != nil { return nil, err } @@ -211,10 +210,23 @@ func IsStatusCodeError(err error, code rpc.Code) bool { if !ok { return false } - return sce.code == code } +// StatusCodeErrorToCS3Status translate the `statusCodeError` type to CS3 Status +// returns nil if `err` does not match to the `statusCodeError` type +func StatusCodeErrorToCS3Status(err error) *rpc.Status { + var sce statusCodeError + ok := errors.As(err, &sce) + if !ok { + return nil + } + if sce.message == "" { + sce.message = sce.reason + } + return &rpc.Status{Message: sce.message, Code: sce.code} +} + // IsSpaceRoot checks if the given resource info is referring to a space root func IsSpaceRoot(ri *storageprovider.ResourceInfo) bool { f := ri.GetId() @@ -222,11 +234,11 @@ func IsSpaceRoot(ri *storageprovider.ResourceInfo) bool { return f.GetOpaqueId() == s.GetOpaqueId() && f.GetSpaceId() == s.GetSpaceId() } -func checkStatusCode(reason string, code rpc.Code) error { +func checkStatusCode(reason, message string, code rpc.Code) error { if code == rpc.Code_CODE_OK { return nil } - return statusCodeError{reason, code} + return statusCodeError{reason, message, code} } func gatherProjectSpaceMembers(ctx context.Context, space *storageprovider.StorageSpace, gwc gateway.GatewayAPIClient, role SpaceRole) ([]string, error) { @@ -293,11 +305,15 @@ func listStorageSpaceRequest(spaceID string) *storageprovider.ListStorageSpacesR // statusCodeError is a helper struct to return errors type statusCodeError struct { - reason string - code rpc.Code + reason string + message string // represents the v1beta11.Status.Message + code rpc.Code } // Error implements error interface func (sce statusCodeError) Error() string { - return fmt.Sprintf(_errStatusCodeTmpl, sce.reason, sce.code) + if sce.reason != "" { + return fmt.Sprintf(_errStatusCodeTmpl, sce.reason, sce.code) + } + return fmt.Sprintf(_errStatusCodeTmpl, sce.message, sce.code) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 04c3ac98fee..d86e52de0dd 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -366,7 +366,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1 github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1 github.com/cs3org/go-cs3apis/cs3/tx/v1beta1 github.com/cs3org/go-cs3apis/cs3/types/v1beta1 -# github.com/cs3org/reva/v2 v2.19.2-0.20240606075653-a7a1d2d2dace +# github.com/cs3org/reva/v2 v2.19.2-0.20240607120909-0a9c7b8bef6a ## explicit; go 1.21 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime