From 3d085628433143ca2365932c16526c65efce01d8 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Tue, 13 Sep 2022 13:35:30 +0300 Subject: [PATCH] [#505] Handle access denied from tree service Signed-off-by: Denis Kirillov --- api/handler/util.go | 16 ++++++++++- api/layer/object.go | 23 ++++------------ api/layer/tree_service.go | 3 +++ internal/neofs/tree.go | 53 ++++++++++++++++++++----------------- internal/neofs/tree_test.go | 28 ++++++++++++++++++++ 5 files changed, 79 insertions(+), 44 deletions(-) diff --git a/api/handler/util.go b/api/handler/util.go index 6d069dd..b14fb64 100644 --- a/api/handler/util.go +++ b/api/handler/util.go @@ -2,6 +2,7 @@ package handler import ( "context" + errorsStd "errors" "net/http" "strconv" "strings" @@ -23,7 +24,20 @@ func (h *handler) logAndSendError(w http.ResponseWriter, logText string, reqInfo fields = append(fields, additional...) h.log.Error(logText, fields...) - api.WriteErrorResponse(w, reqInfo, err) + api.WriteErrorResponse(w, reqInfo, transformToS3Error(err)) +} + +func transformToS3Error(err error) error { + if _, ok := err.(errors.Error); ok { + return err + } + + if errorsStd.Is(err, layer.ErrAccessDenied) || + errorsStd.Is(err, layer.ErrNodeAccessDenied) { + return errors.GetAPIError(errors.ErrAccessDenied) + } + + return errors.GetAPIError(errors.ErrInternalError) } func (h *handler) getBucketAndCheckOwner(r *http.Request, bucket string, header ...string) (*data.BucketInfo, error) { diff --git a/api/layer/object.go b/api/layer/object.go index ddf8840..ee60464 100644 --- a/api/layer/object.go +++ b/api/layer/object.go @@ -93,7 +93,7 @@ func (n *layer) objectHead(ctx context.Context, bktInfo *data.BucketInfo, idObj res, err := n.neoFS.ReadObject(ctx, prm) if err != nil { - return nil, n.transformNeofsError(ctx, err) + return nil, err } return res.Head, nil @@ -113,7 +113,7 @@ func (n *layer) initObjectPayloadReader(ctx context.Context, p getParams) (io.Re res, err := n.neoFS.ReadObject(ctx, prm) if err != nil { - return nil, n.transformNeofsError(ctx, err) + return nil, err } return res.Payload, nil @@ -132,7 +132,7 @@ func (n *layer) objectGet(ctx context.Context, bktInfo *data.BucketInfo, objID o res, err := n.neoFS.ReadObject(ctx, prm) if err != nil { - return nil, n.transformNeofsError(ctx, err) + return nil, err } return res.Head, nil @@ -420,7 +420,7 @@ func (n *layer) objectDelete(ctx context.Context, bktInfo *data.BucketInfo, idOb n.objCache.Delete(newAddress(bktInfo.CID, idObj)) - return n.transformNeofsError(ctx, n.neoFS.DeleteObject(ctx, prm)) + return n.neoFS.DeleteObject(ctx, prm) } // objectPutAndHash prepare auth parameters and invoke neofs.CreateObject. @@ -432,7 +432,7 @@ func (n *layer) objectPutAndHash(ctx context.Context, prm PrmObjectCreate, bktIn hash.Write(buf) }) id, err := n.neoFS.CreateObject(ctx, prm) - return id, hash.Sum(nil), n.transformNeofsError(ctx, err) + return id, hash.Sum(nil), err } // ListObjectsV1 returns objects in a bucket for requests of Version 1. @@ -815,19 +815,6 @@ func tryDirectoryName(node *data.NodeVersion, prefix, delimiter string) string { return "" } -func (n *layer) transformNeofsError(ctx context.Context, err error) error { - if err == nil { - return nil - } - - if errors.Is(err, ErrAccessDenied) { - n.log.Debug("error was transformed", zap.String("request_id", api.GetRequestID(ctx)), zap.Error(err)) - return apiErrors.GetAPIError(apiErrors.ErrAccessDenied) - } - - return err -} - func wrapReader(input io.Reader, bufSize int, f func(buf []byte)) io.Reader { if input == nil { return nil diff --git a/api/layer/tree_service.go b/api/layer/tree_service.go index 72529f3..4a30910 100644 --- a/api/layer/tree_service.go +++ b/api/layer/tree_service.go @@ -85,6 +85,9 @@ var ( // ErrNodeNotFound is returned from Tree service in case of not found error. ErrNodeNotFound = errors.New("not found") + // ErrNodeAccessDenied is returned from Tree service in case of access denied error. + ErrNodeAccessDenied = errors.New("access denied") + // ErrNoNodeToRemove is returned from Tree service in case of the lack of node with OID to remove. ErrNoNodeToRemove = errors.New("no node to remove") ) diff --git a/internal/neofs/tree.go b/internal/neofs/tree.go index 705a1c7..baba76e 100644 --- a/internal/neofs/tree.go +++ b/internal/neofs/tree.go @@ -441,9 +441,6 @@ func (c *TreeClient) DeleteObjectTagging(ctx context.Context, bktInfo *data.Buck func (c *TreeClient) GetBucketTagging(ctx context.Context, bktInfo *data.BucketInfo) (map[string]string, error) { node, err := c.getSystemNodeWithAllAttributes(ctx, bktInfo, []string{bucketTaggingFilename}) if err != nil { - if strings.Contains(err.Error(), "not found") { - return nil, layer.ErrNodeNotFound - } return nil, err } @@ -549,7 +546,7 @@ func (c *TreeClient) GetLatestVersion(ctx context.Context, bktInfo *data.BucketI } nodes, err := c.getNodes(ctx, p) if err != nil { - return nil, fmt.Errorf("couldn't get nodes: %w", err) + return nil, err } if len(nodes) == 0 { @@ -1107,10 +1104,10 @@ func (c *TreeClient) getVersions(ctx context.Context, bktInfo *data.BucketInfo, } nodes, err := c.getNodes(ctx, p) if err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.Is(err, layer.ErrNodeNotFound) { return nil, nil } - return nil, fmt.Errorf("couldn't get nodes: %w", err) + return nil, err } result := make([]*data.NodeVersion, 0, len(nodes)) @@ -1152,10 +1149,7 @@ func (c *TreeClient) getSubTree(ctx context.Context, bktInfo *data.BucketInfo, t cli, err := c.service.GetSubTree(ctx, request) if err != nil { - if strings.Contains(err.Error(), "not found") { - return nil, layer.ErrNodeNotFound - } - return nil, fmt.Errorf("failed to get sub tree client: %w", err) + return nil, handleError("failed to get sub tree client", err) } var subtree []*tree.GetSubTreeResponse_Body @@ -1164,10 +1158,7 @@ func (c *TreeClient) getSubTree(ctx context.Context, bktInfo *data.BucketInfo, t if err == io.EOF { break } else if err != nil { - if strings.Contains(err.Error(), "not found") { - return nil, layer.ErrNodeNotFound - } - return nil, fmt.Errorf("failed to get sub tree: %w", err) + return nil, handleError("failed to get sub tree", err) } subtree = append(subtree, resp.Body) } @@ -1213,7 +1204,7 @@ func (c *TreeClient) getNode(ctx context.Context, bktInfo *data.BucketInfo, tree } nodes, err := c.getNodes(ctx, p) if err != nil { - return nil, fmt.Errorf("couldn't get nodes: %w", err) + return nil, err } if len(nodes) == 0 { return nil, layer.ErrNodeNotFound @@ -1250,15 +1241,21 @@ func (c *TreeClient) getNodes(ctx context.Context, p *getNodesParams) ([]*tree.G resp, err := c.service.GetNodeByPath(ctx, request) if err != nil { - if strings.Contains(err.Error(), "not found") { - return nil, layer.ErrNodeNotFound - } - return nil, fmt.Errorf("failed to get node path: %w", err) + return nil, handleError("failed to get node by path", err) } return resp.GetBody().GetNodes(), nil } +func handleError(msg string, err error) error { + if strings.Contains(err.Error(), "not found") { + return fmt.Errorf("%w: %s", layer.ErrNodeNotFound, err.Error()) + } else if strings.Contains(err.Error(), "is denied by") { + return fmt.Errorf("%w: %s", layer.ErrNodeAccessDenied, err.Error()) + } + return fmt.Errorf("%s: %w", msg, err) +} + func getBearer(ctx context.Context, bktInfo *data.BucketInfo) []byte { if bd, ok := ctx.Value(api.BoxData).(*accessbox.Box); ok && bd != nil && bd.Gate != nil { if bd.Gate.BearerToken != nil { @@ -1291,7 +1288,7 @@ func (c *TreeClient) addNode(ctx context.Context, bktInfo *data.BucketInfo, tree resp, err := c.service.Add(ctx, request) if err != nil { - return 0, err + return 0, handleError("failed to add node", err) } return resp.GetBody().GetNodeId(), nil @@ -1320,7 +1317,7 @@ func (c *TreeClient) addNodeByPath(ctx context.Context, bktInfo *data.BucketInfo resp, err := c.service.AddByPath(ctx, request) if err != nil { - return 0, err + return 0, handleError("failed to add node by path", err) } body := resp.GetBody() @@ -1355,8 +1352,11 @@ func (c *TreeClient) moveNode(ctx context.Context, bktInfo *data.BucketInfo, tre return err } - _, err := c.service.Move(ctx, request) - return err + if _, err := c.service.Move(ctx, request); err != nil { + return handleError("failed to move node", err) + } + + return nil } func (c *TreeClient) removeNode(ctx context.Context, bktInfo *data.BucketInfo, treeID string, nodeID uint64) error { @@ -1377,8 +1377,11 @@ func (c *TreeClient) removeNode(ctx context.Context, bktInfo *data.BucketInfo, t return err } - _, err := c.service.Remove(ctx, request) - return err + if _, err := c.service.Remove(ctx, request); err != nil { + return handleError("failed to remove node", err) + } + + return nil } func metaToKV(meta map[string]string) []*tree.KeyValue { diff --git a/internal/neofs/tree_test.go b/internal/neofs/tree_test.go index 1a73452..e419999 100644 --- a/internal/neofs/tree_test.go +++ b/internal/neofs/tree_test.go @@ -1,9 +1,11 @@ package neofs import ( + "errors" "testing" "github.com/nspcc-dev/neofs-s3-gw/api/data" + "github.com/nspcc-dev/neofs-s3-gw/api/layer" "github.com/stretchr/testify/require" ) @@ -93,3 +95,29 @@ func TestLockConfigurationEncoding(t *testing.T) { }) } } + +func TestHandleError(t *testing.T) { + defaultError := errors.New("default error") + for _, tc := range []struct { + err error + expectedError error + }{ + { + err: defaultError, + expectedError: defaultError, + }, + { + err: errors.New("something not found"), + expectedError: layer.ErrNodeNotFound, + }, + { + err: errors.New("something is denied by some acl rule"), + expectedError: layer.ErrNodeAccessDenied, + }, + } { + t.Run("", func(t *testing.T) { + err := handleError("err message", tc.err) + require.True(t, errors.Is(err, tc.expectedError)) + }) + } +}