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

fix webdav upload return code when token expired #2151

Merged
merged 4 commits into from
Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions changelog/unreleased/webdav-upload-return-code.md
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions internal/http/services/datagateway/datagateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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))
ishank011 marked this conversation as resolved.
Show resolved Hide resolved

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")
Expand Down
14 changes: 7 additions & 7 deletions internal/http/services/owncloud/ocdav/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion internal/http/services/owncloud/ocdav/propfind.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
24 changes: 24 additions & 0 deletions internal/http/services/owncloud/ocdav/tus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -249,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
Expand All @@ -260,6 +263,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")
Expand All @@ -268,6 +272,15 @@ 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
w.WriteHeader(http.StatusOK)
return
}

HandleErrorStatus(&log, w, sRes.Status)
return
}
Expand All @@ -283,10 +296,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(HeaderOCPermissions, permissions)

t := utils.TSToTime(info.Mtime).UTC()
lastModifiedString := t.Format(time.RFC1123Z)
w.Header().Set(HeaderLastModified, lastModifiedString)
Expand Down
3 changes: 3 additions & 0 deletions internal/http/services/owncloud/ocdav/webdav.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +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"
Expand Down