Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filename incrementor for secret filedrops. #4491

Merged
merged 7 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
20 changes: 20 additions & 0 deletions internal/http/services/owncloud/ocdav/context.go
Original file line number Diff line number Diff line change
@@ -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 {
dragonchaser marked this conversation as resolved.
Show resolved Hide resolved
return context.WithValue(ctx, tokenStatInfoKey{}, info)
}

// TokenStatInfoFromContext returns the token stat info from the context
func TokenStatInfoFromContext(ctx context.Context) (*cs3storage.ResourceInfo, bool) {
dragonchaser marked this conversation as resolved.
Show resolved Hide resolved
v, ok := ctx.Value(tokenStatInfoKey{}).(*cs3storage.ResourceInfo)
return v, ok
}
6 changes: 2 additions & 4 deletions internal/http/services/owncloud/ocdav/dav.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ const (
_trashbinPath = "trash-bin"
)

type tokenStatInfoKey struct{}

// DavHandler routes to the different sub handlers
type DavHandler struct {
AvatarsHandler *AvatarsHandler
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions internal/http/services/owncloud/ocdav/errors/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 47 additions & 0 deletions internal/http/services/owncloud/ocdav/filedrop.go
Original file line number Diff line number Diff line change
@@ -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")
}
25 changes: 17 additions & 8 deletions internal/http/services/owncloud/ocdav/publicfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")

Expand All @@ -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
}

Expand Down
41 changes: 41 additions & 0 deletions internal/http/services/owncloud/ocdav/put.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 26 additions & 0 deletions internal/http/services/owncloud/ocdav/tus.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"io"
"net/http"
"path"
"path/filepath"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 7 additions & 2 deletions tests/acceptance/expected-failures-on-OCIS-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,13 @@ 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)
- [coreApiSharePublicLink3/uploadToPublicLinkShare.feature:13](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink3/uploadToPublicLinkShare.feature#L13)
butonic marked this conversation as resolved.
Show resolved Hide resolved

#### [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)_

- [coreApiSharePublicLink3/uploadToPublicLinkShare.feature:91](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink3/uploadToPublicLinkShare.feature#L91)
- [coreApiSharePublicLink3/uploadToPublicLinkShare.feature:101](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink3/uploadToPublicLinkShare.feature#L101)
butonic marked this conversation as resolved.
Show resolved Hide resolved

#### [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)
Expand Down
3 changes: 1 addition & 2 deletions tests/acceptance/expected-failures-on-S3NG-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ 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)
- [coreApiSharePublicLink3/uploadToPublicLinkShare.feature:13](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink3/uploadToPublicLinkShare.feature#L13)
butonic marked this conversation as resolved.
Show resolved Hide resolved

#### [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)_
Expand Down
Loading