From 3ee5e6f1408553a004bffceb280433b519b7901b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= <jfd@butonic.de>
Date: Tue, 10 Aug 2021 13:30:19 +0000
Subject: [PATCH] incorporate review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
---
 .../publicstorageprovider.go                  |  3 +-
 .../storageprovider/storageprovider.go        | 28 +++----------------
 pkg/auth/scope/publicshare.go                 |  1 -
 pkg/auth/scope/resourceinfo.go                |  1 -
 4 files changed, 6 insertions(+), 27 deletions(-)

diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go
index 892ad76d63..32bf2f485a 100644
--- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go
+++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go
@@ -34,6 +34,7 @@ import (
 	"github.com/cs3org/reva/pkg/rgrpc"
 	"github.com/cs3org/reva/pkg/rgrpc/status"
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/pkg/utils"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 	"go.opencensus.io/trace"
@@ -577,7 +578,7 @@ func (s *service) unwrap(ctx context.Context, ref *provider.Reference) (token st
 		return "", "", errtypes.BadRequest("need absolute path ref: got " + ref.String())
 	}
 
-	if !strings.HasPrefix(ref.GetPath(), "/") {
+	if !utils.IsAbsolutePathReference(ref) {
 		// abort, no valid id nor path
 		return "", "", errtypes.BadRequest("invalid ref: " + ref.String())
 	}
diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go
index e7913e13fc..0f8db73fee 100644
--- a/internal/grpc/services/storageprovider/storageprovider.go
+++ b/internal/grpc/services/storageprovider/storageprovider.go
@@ -500,7 +500,6 @@ func (s *service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteSt
 }
 
 func (s *service) CreateContainer(ctx context.Context, req *provider.CreateContainerRequest) (*provider.CreateContainerResponse, error) {
-	var err error
 	var parentRef *provider.Reference
 	var name string
 	switch {
@@ -521,20 +520,6 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta
 			Status: status.NewInvalidArg(ctx, "invalid reference, name required"),
 		}, nil
 	}
-	var st *rpc.Status
-	if err != nil {
-		switch err.(type) {
-		case errtypes.IsNotFound:
-			st = status.NewNotFound(ctx, "path not found when unwrapping")
-		case errtypes.PermissionDenied:
-			st = status.NewPermissionDenied(ctx, err, "permission denied")
-		default:
-			st = status.NewInternal(ctx, err, "error unwrapping: "+req.String())
-		}
-		return &provider.CreateContainerResponse{
-			Status: st,
-		}, nil
-	}
 	if err := s.storage.CreateDir(ctx, parentRef, name); err != nil {
 		var st *rpc.Status
 		switch err.(type) {
@@ -605,19 +590,14 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide
 	sourceRef, err := s.unwrap(ctx, req.Source)
 	if err != nil {
 		return &provider.MoveResponse{
-			Status: status.NewInternal(ctx, err, "error unwrapping source path"),
+			Status: status.NewInvalid(ctx, err.Error()),
 		}, nil
 	}
 	targetRef, err := s.unwrap(ctx, req.Destination)
 	if err != nil {
-		switch err.(type) {
-		case errtypes.IsNotFound:
-			targetRef = req.Destination
-		default:
-			return &provider.MoveResponse{
-				Status: status.NewInternal(ctx, err, "error unwrapping destination path"),
-			}, nil
-		}
+		return &provider.MoveResponse{
+			Status: status.NewInvalid(ctx, err.Error()),
+		}, nil
 	}
 
 	if err := s.storage.Move(ctx, sourceRef, targetRef); err != nil {
diff --git a/pkg/auth/scope/publicshare.go b/pkg/auth/scope/publicshare.go
index 3f3d7cca5f..6c798366c4 100644
--- a/pkg/auth/scope/publicshare.go
+++ b/pkg/auth/scope/publicshare.go
@@ -71,7 +71,6 @@ func publicshareScope(scope *authpb.Scope, resource interface{}) (bool, error) {
 }
 
 func checkStorageRef(s *link.PublicShare, r *provider.Reference) bool {
-	// TODO @ishank011 con you explain how this is used?
 	// r: <resource_id:<storage_id:$storageID opaque_id:$opaqueID> path:$path > >
 	if r.ResourceId != nil && r.Path == "" { // path must be empty
 		return utils.ResourceIDEqual(s.ResourceId, r.GetResourceId())
diff --git a/pkg/auth/scope/resourceinfo.go b/pkg/auth/scope/resourceinfo.go
index 92d1f02e6f..f6267ff3d1 100644
--- a/pkg/auth/scope/resourceinfo.go
+++ b/pkg/auth/scope/resourceinfo.go
@@ -68,7 +68,6 @@ func resourceinfoScope(scope *authpb.Scope, resource interface{}) (bool, error)
 }
 
 func checkResourceInfo(inf *provider.ResourceInfo, ref *provider.Reference) bool {
-	// TODO @ishank011 con you explain how this is used?
 	// ref: <resource_id:<storage_id:$storageID opaque_id:$opaqueID path:$path> >
 	if ref.ResourceId != nil { // path can be empty or a relative path
 		if inf.Id.StorageId == ref.ResourceId.StorageId && inf.Id.OpaqueId == ref.ResourceId.OpaqueId {