Merge pull request #4491 from dragonchaser/filename-incrementor

Add filename incrementor for secret filedrops.

changelog/unreleased/fileincrementor-for-secret-file-drops.md

@@ -0,0 +1,7 @@
+Enhancement: Add filename incrementor for secret filedrops
+
+We have added a function that appends a number to the filename if the file already exists in a secret filedrop. 
+This is useful if you want to upload a file with the same name multiple times.
+
+https://github.com/cs3org/reva/pull/4491
+https://github.com/owncloud/ocis/issues/8291

internal/http/services/owncloud/ocdav/context.go

@@ -0,0 +1,20 @@
+package ocdav
+
+import (
+	"context"
+
+	cs3storage "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+)
+
+type tokenStatInfoKey struct{}
+
+// ContextWithTokenStatInfo adds the token stat info to the context
+func ContextWithTokenStatInfo(ctx context.Context, info *cs3storage.ResourceInfo) context.Context {
+	return context.WithValue(ctx, tokenStatInfoKey{}, info)
+}
+
+// TokenStatInfoFromContext returns the token stat info from the context
+func TokenStatInfoFromContext(ctx context.Context) (*cs3storage.ResourceInfo, bool) {
+	v, ok := ctx.Value(tokenStatInfoKey{}).(*cs3storage.ResourceInfo)
+	return v, ok
+}

internal/http/services/owncloud/ocdav/dav.go

@@ -44,8 +44,6 @@ const (
 	_trashbinPath = "trash-bin"
 )
 
-type tokenStatInfoKey struct{}
-
 // DavHandler routes to the different sub handlers
 type DavHandler struct {
 	AvatarsHandler      *AvatarsHandler
@@ -318,9 +316,9 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
 			}
 			log.Debug().Interface("statInfo", sRes.Info).Msg("Stat info from public link token path")
 
+			ctx := ContextWithTokenStatInfo(ctx, sRes.Info)
+			r = r.WithContext(ctx)
 			if sRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER {
-				ctx := context.WithValue(ctx, tokenStatInfoKey{}, sRes.Info)
-				r = r.WithContext(ctx)
 				h.PublicFileHandler.Handler(s).ServeHTTP(w, r)
 			} else {
 				h.PublicFolderHandler.Handler(s).ServeHTTP(w, r)

internal/http/services/owncloud/ocdav/errors/error.go

@@ -159,6 +159,8 @@ var (
 	ErrNoSuchLock = errors.New("webdav: no such lock")
 	// ErrNotImplemented is returned when hitting not implemented code paths
 	ErrNotImplemented = errors.New("webdav: not implemented")
+	// ErrTokenNotFound is returned when a token is not found
+	ErrTokenStatInfoMissing = errors.New("webdav: token stat info missing")
 )
 
 // HandleErrorStatus checks the status code, logs a Debug or Error level message

internal/http/services/owncloud/ocdav/filedrop.go

@@ -0,0 +1,47 @@
+package ocdav
+
+import (
+	"context"
+	"errors"
+	"path/filepath"
+	"strconv"
+	"strings"
+
+	gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
+	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
+	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+)
+
+// FindName returns the next filename available when the current
+func FindName(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, name string, parentid *provider.ResourceId) (string, *rpc.Status, error) {
+	lReq := &provider.ListContainerRequest{
+		Ref: &provider.Reference{
+			ResourceId: parentid,
+		},
+	}
+	lRes, err := client.ListContainer(ctx, lReq)
+	if err != nil {
+		return "", nil, err
+	}
+	if lRes.Status.Code != rpc.Code_CODE_OK {
+		return "", lRes.Status, nil
+	}
+	// iterate over the listing to determine next suffix
+	var itemMap = make(map[string]struct{})
+	for _, fi := range lRes.Infos {
+		itemMap[fi.GetName()] = struct{}{}
+	}
+	ext := filepath.Ext(name)
+	fileName := strings.TrimSuffix(name, ext)
+	if strings.HasSuffix(fileName, ".tar") {
+		fileName = strings.TrimSuffix(fileName, ".tar")
+		ext = filepath.Ext(fileName) + "." + ext
+	}
+	// starts with two because "normal" humans begin counting with 1 and we say the existing file is the first one
+	for i := 2; i < len(itemMap)+3; i++ {
+		if _, ok := itemMap[fileName+" ("+strconv.Itoa(i)+")"+ext]; !ok {
+			return fileName + " (" + strconv.Itoa(i) + ")" + ext, lRes.GetStatus(), nil
+		}
+	}
+	return "", nil, errors.New("could not determine new filename")
+}

internal/http/services/owncloud/ocdav/publicfile.go

@@ -26,7 +26,7 @@ import (
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
-	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
+	ocdaverrors "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind"
 	"github.com/cs3org/reva/v2/pkg/appctx"
@@ -96,7 +96,16 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s
 	ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "token_propfind")
 	defer span.End()
 
-	tokenStatInfo := ctx.Value(tokenStatInfoKey{}).(*provider.ResourceInfo)
+	tokenStatInfo, ok := TokenStatInfoFromContext(ctx)
+	if !ok {
+		span.RecordError(ocdaverrors.ErrTokenStatInfoMissing)
+		span.SetStatus(codes.Error, ocdaverrors.ErrTokenStatInfoMissing.Error())
+		span.SetAttributes(semconv.HTTPStatusCodeKey.Int(http.StatusInternalServerError))
+		w.WriteHeader(http.StatusInternalServerError)
+		b, err := ocdaverrors.Marshal(http.StatusInternalServerError, ocdaverrors.ErrTokenStatInfoMissing.Error(), "")
+		ocdaverrors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
+		return
+	}
 	sublog := appctx.GetLogger(ctx).With().Interface("tokenStatInfo", tokenStatInfo).Logger()
 	sublog.Debug().Msg("handlePropfindOnToken")
 
@@ -109,20 +118,20 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s
 		sublog.Debug().Str("depth", dh).Msg(err.Error())
 		w.WriteHeader(http.StatusBadRequest)
 		m := fmt.Sprintf("Invalid Depth header value: %v", dh)
-		b, err := errors.Marshal(http.StatusBadRequest, m, "")
-		errors.HandleWebdavError(&sublog, w, b, err)
+		b, err := ocdaverrors.Marshal(http.StatusBadRequest, m, "")
+		ocdaverrors.HandleWebdavError(&sublog, w, b, err)
 		return
 	}
 
 	if depth == net.DepthInfinity && !s.c.AllowPropfindDepthInfinitiy {
-		span.RecordError(errors.ErrInvalidDepth)
+		span.RecordError(ocdaverrors.ErrInvalidDepth)
 		span.SetStatus(codes.Error, "DEPTH: infinity is not supported")
 		span.SetAttributes(semconv.HTTPStatusCodeKey.Int(http.StatusBadRequest))
-		sublog.Debug().Str("depth", dh).Msg(errors.ErrInvalidDepth.Error())
+		sublog.Debug().Str("depth", dh).Msg(ocdaverrors.ErrInvalidDepth.Error())
 		w.WriteHeader(http.StatusBadRequest)
 		m := fmt.Sprintf("Invalid Depth header value: %v", dh)
-		b, err := errors.Marshal(http.StatusBadRequest, m, "")
-		errors.HandleWebdavError(&sublog, w, b, err)
+		b, err := ocdaverrors.Marshal(http.StatusBadRequest, m, "")
+		ocdaverrors.HandleWebdavError(&sublog, w, b, err)
 		return
 	}
 

internal/http/services/owncloud/ocdav/put.go

@@ -155,6 +155,47 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
+
+	// Test if the target is a secret filedrop
+	tokenStatInfo, ok := TokenStatInfoFromContext(ctx)
+	// We assume that when the uploader can create containers, but is not allowed to list them, it is a secret file drop
+	if ok && tokenStatInfo.GetPermissionSet().CreateContainer && !tokenStatInfo.GetPermissionSet().ListContainer {
+		// TODO we can skip this stat if the tokenStatInfo is the direct parent
+		sReq := &provider.StatRequest{
+			Ref: ref,
+		}
+		sRes, err := client.Stat(ctx, sReq)
+		if err != nil {
+			log.Error().Err(err).Msg("error sending grpc stat request")
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
+
+		// We also need to continue if we are not allowed to stat a resource. We may not have stat permission. That still means it exists and we need to find a new filename.
+		switch sRes.Status.Code {
+		case rpc.Code_CODE_OK, rpc.Code_CODE_PERMISSION_DENIED:
+			// find next filename
+			newName, status, err := FindName(ctx, client, filepath.Base(ref.Path), sRes.GetInfo().GetParentId())
+			if err != nil {
+				log.Error().Err(err).Msg("error sending grpc stat request")
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			if status.Code != rpc.Code_CODE_OK {
+				log.Error().Interface("status", status).Msg("error listing file")
+				errors.HandleErrorStatus(&log, w, status)
+				return
+			}
+			ref.Path = utils.MakeRelativePath(filepath.Join(filepath.Dir(ref.GetPath()), newName))
+		case rpc.Code_CODE_NOT_FOUND:
+			// just continue with normal upload
+		default:
+			log.Error().Interface("status", sRes.Status).Msg("error stating file")
+			errors.HandleErrorStatus(&log, w, sRes.Status)
+			return
+		}
+	}
+
 	opaque := &typespb.Opaque{}
 	if mtime := r.Header.Get(net.HeaderOCMtime); mtime != "" {
 		utils.AppendPlainToOpaque(opaque, net.HeaderOCMtime, mtime)

internal/http/services/owncloud/ocdav/tus.go

@@ -24,6 +24,7 @@ import (
 	"io"
 	"net/http"
 	"path"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
@@ -115,6 +116,15 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http.
 		w.WriteHeader(http.StatusPreconditionFailed)
 		return
 	}
+
+	// Test if the target is a secret filedrop
+	var isSecretFileDrop bool
+	tokenStatInfo, ok := TokenStatInfoFromContext(ctx)
+	// We assume that when the uploader can create containers, but is not allowed to list them, it is a secret file drop
+	if ok && tokenStatInfo.GetPermissionSet().CreateContainer && !tokenStatInfo.GetPermissionSet().ListContainer {
+		isSecretFileDrop = true
+	}
+
 	// r.Header.Get(net.HeaderOCChecksum)
 	// TODO must be SHA1, ADLER32 or MD5 ... in capital letters????
 	// curl -X PUT https://demo.owncloud.com/remote.php/webdav/testcs.bin -u demo:demo -d '123' -v -H 'OC-Checksum: SHA1:40bd001563085fc35165329ea1ff5c5ecbdbbeef'
@@ -158,6 +168,22 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http.
 				return
 			}
 		}
+		if isSecretFileDrop {
+			// find next filename
+			newName, status, err := FindName(ctx, client, filepath.Base(ref.Path), sRes.GetInfo().GetParentId())
+			if err != nil {
+				log.Error().Err(err).Msg("error sending grpc stat request")
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			if status.GetCode() != rpc.Code_CODE_OK {
+				log.Error().Interface("status", status).Msg("error listing file")
+				errors.HandleErrorStatus(&log, w, status)
+				return
+			}
+			ref.Path = filepath.Join(filepath.Dir(ref.GetPath()), newName)
+			sRes.GetInfo().Name = newName
+		}
 	}
 
 	uploadLength, err := strconv.ParseInt(r.Header.Get(net.HeaderUploadLength), 10, 64)

tests/acceptance/expected-failures-on-OCIS-storage.md

@@ -76,7 +76,6 @@ Synchronization features like etag propagation, setting mtime and locking files
 #### [Upload-only shares must not overwrite but create a separate file](https://github.com/owncloud/ocis/issues/1267)
 
 - [coreApiSharePublicLink2/uploadToPublicLinkShare.feature:13](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink2/uploadToPublicLinkShare.feature#L13)
-- [coreApiSharePublicLink2/uploadToPublicLinkShare.feature:121](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink2/uploadToPublicLinkShare.feature#L121)
 
 #### [oc:privatelink property not returned in webdav responses](https://github.com/owncloud/product/issues/262)
 -   [coreApiWebdavProperties/getFileProperties.feature:295](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavProperties/getFileProperties.feature#L295)

tests/acceptance/expected-failures-on-S3NG-storage.md

@@ -75,7 +75,6 @@ Synchronization features like etag propagation, setting mtime and locking files
 #### [Upload-only shares must not overwrite but create a separate file](https://github.com/owncloud/ocis/issues/1267)
 
 - [coreApiSharePublicLink2/uploadToPublicLinkShare.feature:13](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink2/uploadToPublicLinkShare.feature#L13)
-- [coreApiSharePublicLink2/uploadToPublicLinkShare.feature:121](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink2/uploadToPublicLinkShare.feature#L121)
 
 #### [Set quota over settings](https://github.com/owncloud/ocis/issues/1290)
 _requires a [CS3 user provisioning api that can update the quota for a user](https://github.com/cs3org/cs3apis/pull/95#issuecomment-772780683)_