From bf0f91d9f43f4745514054b4bbbe4e3870fa626a Mon Sep 17 00:00:00 2001 From: Willy Kloucek Date: Thu, 7 Oct 2021 14:27:27 +0200 Subject: [PATCH 1/4] fix tus code --- .../http/services/datagateway/datagateway.go | 5 +++++ .../http/services/owncloud/ocdav/options.go | 14 ++++++------ .../http/services/owncloud/ocdav/propfind.go | 2 +- internal/http/services/owncloud/ocdav/tus.go | 22 +++++++++++++++++++ .../http/services/owncloud/ocdav/webdav.go | 1 + pkg/token/manager/jwt/jwt.go | 3 ++- 6 files changed, 38 insertions(+), 9 deletions(-) diff --git a/internal/http/services/datagateway/datagateway.go b/internal/http/services/datagateway/datagateway.go index 0ba7affd55..fc866157c9 100644 --- a/internal/http/services/datagateway/datagateway.go +++ b/internal/http/services/datagateway/datagateway.go @@ -41,6 +41,8 @@ import ( const ( // TokenTransportHeader holds the header key for the reva transfer token TokenTransportHeader = "X-Reva-Transfer" + // UploadExpiresHeader holds the timestamp for the transport token expiry, defined in https://tus.io/protocols/resumable-upload.html#expiration + UploadExpiresHeader = "Upload-Expires" ) func init() { @@ -197,6 +199,9 @@ func (s *svc) doHead(w http.ResponseWriter, r *http.Request) { copyHeader(w.Header(), httpRes.Header) + // add upload expiry / transfer token expiry header for tus https://tus.io/protocols/resumable-upload.html#expiration + w.Header().Set(UploadExpiresHeader, time.Unix(claims.ExpiresAt, 0).Format(time.RFC1123)) + if httpRes.StatusCode != http.StatusOK { // swallow the body and set content-length to 0 to prevent reverse proxies from trying to read from it w.Header().Set("Content-Length", "0") diff --git a/internal/http/services/owncloud/ocdav/options.go b/internal/http/services/owncloud/ocdav/options.go index d86a5afaa1..c436eaecab 100644 --- a/internal/http/services/owncloud/ocdav/options.go +++ b/internal/http/services/owncloud/ocdav/options.go @@ -30,17 +30,17 @@ func (s *svc) handleOptions(w http.ResponseWriter, r *http.Request, ns string) { isPublic := strings.Contains(r.Context().Value(ctxKeyBaseURI).(string), "public-files") - w.Header().Set("Content-Type", "application/xml") + w.Header().Set(HeaderContentType, "application/xml") w.Header().Set("Allow", allow) w.Header().Set("DAV", "1, 2") w.Header().Set("MS-Author-Via", "DAV") if !isPublic { - w.Header().Add("Access-Control-Allow-Headers", "Tus-Resumable") - w.Header().Add("Access-Control-Expose-Headers", "Tus-Resumable, Tus-Version, Tus-Extension") - w.Header().Set("Tus-Resumable", "1.0.0") // TODO(jfd): only for dirs? - w.Header().Set("Tus-Version", "1.0.0") - w.Header().Set("Tus-Extension", "creation,creation-with-upload,checksum") - w.Header().Set("Tus-Checksum-Algorithm", "md5,sha1,crc32") + w.Header().Add(HeaderAccessControlAllowHeaders, HeaderTusResumable) + w.Header().Add(HeaderAccessControlExposeHeaders, strings.Join([]string{HeaderTusResumable, HeaderTusVersion, HeaderTusExtension}, ",")) + w.Header().Set(HeaderTusResumable, "1.0.0") // TODO(jfd): only for dirs? + w.Header().Set(HeaderTusVersion, "1.0.0") + w.Header().Set(HeaderTusExtension, "creation,creation-with-upload,checksum,expiration") + w.Header().Set(HeaderTusChecksumAlgorithm, "md5,sha1,crc32") } w.WriteHeader(http.StatusNoContent) } diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index e95007abc9..c94ae56e46 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -153,7 +153,7 @@ func (s *svc) propfindResponse(ctx context.Context, w http.ResponseWriter, r *ht w.Header().Add(HeaderAccessControlExposeHeaders, strings.Join([]string{HeaderTusResumable, HeaderTusVersion, HeaderTusExtension}, ", ")) w.Header().Set(HeaderTusResumable, "1.0.0") w.Header().Set(HeaderTusVersion, "1.0.0") - w.Header().Set(HeaderTusExtension, "creation,creation-with-upload") + w.Header().Set(HeaderTusExtension, "creation,creation-with-upload,checksum,expiration") } } w.WriteHeader(http.StatusMultiStatus) diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 0c1eb5c4e5..663e6bd3f7 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -29,6 +29,7 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rhttp" rtrace "github.com/cs3org/reva/pkg/trace" @@ -90,6 +91,7 @@ func (s *svc) handleSpacesTusPost(w http.ResponseWriter, r *http.Request, spaceI func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http.Request, meta map[string]string, ref *provider.Reference, log zerolog.Logger) { w.Header().Add(HeaderAccessControlAllowHeaders, strings.Join([]string{HeaderTusResumable, HeaderUploadLength, HeaderUploadMetadata, HeaderIfMatch}, ", ")) w.Header().Add(HeaderAccessControlExposeHeaders, strings.Join([]string{HeaderTusResumable, HeaderLocation}, ", ")) + w.Header().Set(HeaderTusExtension, "creation,creation-with-upload,checksum,expiration") w.Header().Set(HeaderTusResumable, "1.0.0") @@ -260,6 +262,7 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. // check if upload was fully completed if length == 0 || httpRes.Header.Get(HeaderUploadOffset) == r.Header.Get(HeaderUploadLength) { // get uploaded file metadata + sRes, err := client.Stat(ctx, sReq) if err != nil { log.Error().Err(err).Msg("error sending grpc stat request") @@ -268,6 +271,14 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. } if sRes.Status.Code != rpc.Code_CODE_OK && sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + + if sRes.Status.Code == rpc.Code_CODE_PERMISSION_DENIED { + // the token expired during upload, so the stat failed + // and we can't do anything about it. + // the clients will handle this gracefully by doing a propfind on the file + return + } + HandleErrorStatus(&log, w, sRes.Status) return } @@ -283,10 +294,21 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. w.Header().Set(HeaderOCMtime, httpRes.Header.Get(HeaderOCMtime)) } + // get WebDav permissions for file + role := conversions.RoleFromResourcePermissions(info.PermissionSet) + permissions := role.WebDAVPermissions( + info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER, + false, + false, + false, + ) + w.Header().Set(HeaderContentType, info.MimeType) w.Header().Set(HeaderOCFileID, wrapResourceID(info.Id)) w.Header().Set(HeaderOCETag, info.Etag) w.Header().Set(HeaderETag, info.Etag) + w.Header().Set("permissions-HEADER-TODO", permissions) + t := utils.TSToTime(info.Mtime).UTC() lastModifiedString := t.Format(time.RFC1123Z) w.Header().Set(HeaderLastModified, lastModifiedString) diff --git a/internal/http/services/owncloud/ocdav/webdav.go b/internal/http/services/owncloud/ocdav/webdav.go index 97b61510d7..616c0564f2 100644 --- a/internal/http/services/owncloud/ocdav/webdav.go +++ b/internal/http/services/owncloud/ocdav/webdav.go @@ -63,6 +63,7 @@ const ( HeaderTusResumable = "Tus-Resumable" HeaderTusVersion = "Tus-Version" HeaderTusExtension = "Tus-Extension" + HeaderTusChecksumAlgorithm = "Tus-Checksum-Algorithm" HeaderDestination = "Destination" HeaderOverwrite = "Overwrite" HeaderUploadChecksum = "Upload-Checksum" diff --git a/pkg/token/manager/jwt/jwt.go b/pkg/token/manager/jwt/jwt.go index e54df4dae9..794a4fef52 100644 --- a/pkg/token/manager/jwt/jwt.go +++ b/pkg/token/manager/jwt/jwt.go @@ -86,7 +86,8 @@ func New(value map[string]interface{}) (token.Manager, error) { } func (m *manager) MintToken(ctx context.Context, u *user.User, scope map[string]*auth.Scope) (string, error) { - ttl := time.Duration(m.conf.Expires) * time.Second + ttl := 60 * time.Second + //ttl := time.Duration(m.conf.Expires) * time.Second claims := claims{ StandardClaims: jwt.StandardClaims{ ExpiresAt: time.Now().Add(ttl).Unix(), From 0f08fbf2b8c14f1fecd64249c2171d8d816761de Mon Sep 17 00:00:00 2001 From: Willy Kloucek Date: Fri, 8 Oct 2021 19:59:53 +0200 Subject: [PATCH 2/4] return 200 ok --- internal/http/services/owncloud/ocdav/tus.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 663e6bd3f7..af158c4bcd 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -276,6 +276,7 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. // the token expired during upload, so the stat failed // and we can't do anything about it. // the clients will handle this gracefully by doing a propfind on the file + w.WriteHeader(http.StatusOK) return } From b7ebff9ad427676bebeaca0d8f753169979b349c Mon Sep 17 00:00:00 2001 From: Willy Kloucek Date: Mon, 11 Oct 2021 14:29:43 +0200 Subject: [PATCH 3/4] return OC-Perm and set expiry on post with content --- internal/http/services/owncloud/ocdav/tus.go | 3 ++- internal/http/services/owncloud/ocdav/webdav.go | 2 ++ pkg/token/manager/jwt/jwt.go | 3 +-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index af158c4bcd..bd1cf39b0e 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -251,6 +251,7 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. w.Header().Set(HeaderUploadOffset, httpRes.Header.Get(HeaderUploadOffset)) w.Header().Set(HeaderTusResumable, httpRes.Header.Get(HeaderTusResumable)) + w.Header().Set(HeaderTusUploadExpires, httpRes.Header.Get(HeaderTusUploadExpires)) if httpRes.StatusCode != http.StatusNoContent { w.WriteHeader(httpRes.StatusCode) return @@ -308,7 +309,7 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. w.Header().Set(HeaderOCFileID, wrapResourceID(info.Id)) w.Header().Set(HeaderOCETag, info.Etag) w.Header().Set(HeaderETag, info.Etag) - w.Header().Set("permissions-HEADER-TODO", permissions) + w.Header().Set(HeaderOCPermissions, permissions) t := utils.TSToTime(info.Mtime).UTC() lastModifiedString := t.Format(time.RFC1123Z) diff --git a/internal/http/services/owncloud/ocdav/webdav.go b/internal/http/services/owncloud/ocdav/webdav.go index 616c0564f2..8093424795 100644 --- a/internal/http/services/owncloud/ocdav/webdav.go +++ b/internal/http/services/owncloud/ocdav/webdav.go @@ -58,12 +58,14 @@ const ( HeaderOCFileID = "OC-FileId" HeaderOCETag = "OC-ETag" HeaderOCChecksum = "OC-Checksum" + HeaderOCPermissions = "OC-Perm" HeaderDepth = "Depth" HeaderDav = "DAV" HeaderTusResumable = "Tus-Resumable" HeaderTusVersion = "Tus-Version" HeaderTusExtension = "Tus-Extension" HeaderTusChecksumAlgorithm = "Tus-Checksum-Algorithm" + HeaderTusUploadExpires = "Upload-Expires" HeaderDestination = "Destination" HeaderOverwrite = "Overwrite" HeaderUploadChecksum = "Upload-Checksum" diff --git a/pkg/token/manager/jwt/jwt.go b/pkg/token/manager/jwt/jwt.go index 794a4fef52..e54df4dae9 100644 --- a/pkg/token/manager/jwt/jwt.go +++ b/pkg/token/manager/jwt/jwt.go @@ -86,8 +86,7 @@ func New(value map[string]interface{}) (token.Manager, error) { } func (m *manager) MintToken(ctx context.Context, u *user.User, scope map[string]*auth.Scope) (string, error) { - ttl := 60 * time.Second - //ttl := time.Duration(m.conf.Expires) * time.Second + ttl := time.Duration(m.conf.Expires) * time.Second claims := claims{ StandardClaims: jwt.StandardClaims{ ExpiresAt: time.Now().Add(ttl).Unix(), From b6804bc1e49e883c78969ee06a4a12c66d01cae2 Mon Sep 17 00:00:00 2001 From: Willy Kloucek Date: Mon, 11 Oct 2021 14:36:18 +0200 Subject: [PATCH 4/4] add changelog --- changelog/unreleased/webdav-upload-return-code.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changelog/unreleased/webdav-upload-return-code.md diff --git a/changelog/unreleased/webdav-upload-return-code.md b/changelog/unreleased/webdav-upload-return-code.md new file mode 100644 index 0000000000..96754cc7c0 --- /dev/null +++ b/changelog/unreleased/webdav-upload-return-code.md @@ -0,0 +1,10 @@ +Bugfix: Fix return code for webdav uploads when the token expired + +We've fixed the behavior webdav uploads when the token expired before the final stat. +Previously clients would receive a http 500 error which is wrong, because the file +was successfully uploaded and only the stat couldn't be performed. Now we return a http 200 +ok and the clients will fetch the file info in a separate propfind request. + +Also we introduced the upload expires header on the webdav/TUS and datagateway endpoints, to signal clients how long an upload can be performed. + +https://github.com/cs3org/reva/pull/2151