fix: adjust code to pass the wopivalidator tests

services/collaboration/pkg/internal/app/fileinfo.go

@@ -13,7 +13,7 @@ type FileInfo struct {
 	// A string value uniquely identifying the user currently accessing the file.
 	UserID string `json:"UserId,omitempty"`
 	// The size of the file in bytes, expressed as a long, a 64-bit signed integer.
-	Size int64 `json:"Size,omitempty"`
+	Size int64 `json:"Size"`
 	// The current version of the file based on the server’s file version schema, as a string. This value must change when the file changes, and version values must never repeat for a given file.
 	Version string `json:"Version,omitempty"`
 	// A string that is the name of the user, suitable for displaying in UI.
@@ -47,7 +47,7 @@ type FileInfo struct {
 	// A Boolean value that indicates that the user has permission to alter the file. Setting this to true tells the WOPI client that it can call PutFile on behalf of the user.
 	UserCanWrite bool `json:"UserCanWrite"`
 	// A Boolean value that indicates the WOPI client should close the window or tab when the user activates any Close UI in the WOPI client.
-	CloseButtonClosesWindow bool `json:"CloseButtonClosesWindow"`
+	CloseButtonClosesWindow bool `json:"CloseButtonClosesWindow,omitempty"`
 	// A string value indicating whether the WOPI client should disable Copy and Paste functionality within the application. The default is to permit all Copy and Paste functionality, i.e. the setting has no effect.
 	CopyPasteRestrictions string `json:"CopyPasteRestrictions,omitempty"`
 	// A Boolean value that indicates the WOPI client should disable all print functionality.
@@ -87,7 +87,7 @@ type FileInfo struct {
 	// A Boolean value that indicates a WOPI client may connect to Microsoft services to provide end-user functionality.
 	AllowAdditionalMicrosoftServices bool `json:"AllowAdditionalMicrosoftServices"`
 	// A Boolean value that indicates that in the event of an error, the WOPI client is permitted to prompt the user for permission to collect a detailed report about their specific error. The information gathered could include the user’s file and other session-specific state.
-	AllowErrorReportPrompt bool `json:"AllowErrorReportPrompt"`
+	AllowErrorReportPrompt bool `json:"AllowErrorReportPrompt,omitempty"`
 	// A Boolean value that indicates a WOPI client may allow connections to external services referenced in the file (for example, a marketplace of embeddable JavaScript apps).
 	AllowExternalMarketplace bool `json:"AllowExternalMarketplace"`
 	// A string value offering guidance to the WOPI client as to how to differentiate client throttling behaviors between the user and documents combinations from the WOPI host.
@@ -107,7 +107,7 @@ type FileInfo struct {
 	// A Boolean value that indicates that the host supports the following WOPI operations: CheckFolderInfo, EnumerateChildren (folders), DeleteFile
 	SupportsFolders bool `json:"SupportsFolders"`
 	// A Boolean value that indicates that the host supports the GetFileWopiSrc (ecosystem) operation.
-	SupportsGetFileWopiSrc bool `json:"SupportsGetFileWopiSrc"`
+	//SupportsGetFileWopiSrc bool `json:"SupportsGetFileWopiSrc"`  // wopivalidator is complaining and the property isn't used for now -> commented
 	// A Boolean value that indicates that the host supports the GetLock operation.
 	SupportsGetLock bool `json:"SupportsGetLock"`
 	// A Boolean value that indicates that the host supports the following WOPI operations: Lock, Unlock, RefreshLock, UnlockAndRelock operations for this file.

services/collaboration/pkg/internal/app/headers.go

@@ -1,5 +1,6 @@
 package app
 
 const (
-	HeaderWopiLock string = "X-WOPI-Lock"
+	HeaderWopiLock    string = "X-WOPI-Lock"
+	HeaderWopiOldLock string = "X-WOPI-OldLock"
 )

services/collaboration/pkg/internal/app/wopifilecontents.go

@@ -3,7 +3,10 @@ package app
 import (
 	"io"
 	"net/http"
+	"strconv"
 
+	rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
+	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/owncloud/ocis/v2/services/collaboration/pkg/internal/helpers"
 )
 
@@ -53,7 +56,6 @@ func GetFile(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 		Str("ViewMode", wopiContext.ViewMode.String()).
 		Str("Requester", wopiContext.User.GetId().String()).
 		Msg("GetFile: success")
-	http.Error(w, "", http.StatusOK)
 }
 
 // PutFile uploads the file to the storage
@@ -65,10 +67,77 @@ func PutFile(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 	// read the file from the body
 	defer r.Body.Close()
 
+	// We need a stat call on the target file in order to get both the lock
+	// (if any) and the current size of the file
+	statRes, err := app.gwc.Stat(ctx, &providerv1beta1.StatRequest{
+		Ref: &wopiContext.FileReference,
+	})
+	if err != nil {
+		app.Logger.Error().
+			Err(err).
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", r.Header.Get(HeaderWopiLock)).
+			Str("UploadLength", strconv.FormatInt(r.ContentLength, 10)).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Msg("PutFile: stat failed")
+		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+		return
+	}
+
+	if statRes.Status.Code != rpcv1beta1.Code_CODE_OK {
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", r.Header.Get(HeaderWopiLock)).
+			Str("UploadLength", strconv.FormatInt(r.ContentLength, 10)).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Str("StatusCode", statRes.Status.Code.String()).
+			Str("StatusMsg", statRes.Status.Message).
+			Msg("PutFile: stat failed with unexpected status")
+		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+		return
+	}
+
+	// If there is a lock and it mismatches, return 409
+	if statRes.Info.Lock != nil && statRes.Info.Lock.LockId != r.Header.Get(HeaderWopiLock) {
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", r.Header.Get(HeaderWopiLock)).
+			Str("UploadLength", strconv.FormatInt(r.ContentLength, 10)).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Str("LockID", statRes.Info.Lock.LockId).
+			Msg("PutFile: wrong lock")
+		// onlyoffice says it's required to send the current lockId, MS doesn't say anything
+		w.Header().Add(HeaderWopiLock, statRes.Info.Lock.LockId)
+		http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
+		return
+	}
+
+	// only unlocked uploads can go through if the target file is empty,
+	// otherwise the X-WOPI-Lock header is required even if there is no lock on the file
+	// This is part of the onlyoffice documentation (https://api.onlyoffice.com/editors/wopi/restapi/putfile)
+	// Wopivalidator fails some tests if we don't also check for the X-WOPI-Lock header.
+	if r.Header.Get(HeaderWopiLock) == "" && statRes.Info.Lock == nil && statRes.Info.Size > 0 {
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", r.Header.Get(HeaderWopiLock)).
+			Str("UploadLength", strconv.FormatInt(r.ContentLength, 10)).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Msg("PutFile: file must be locked first")
+		// onlyoffice says to send an empty string if the file is unlocked, MS doesn't say anything
+		w.Header().Add(HeaderWopiLock, "")
+		http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
+		return
+	}
+
 	// upload the file
-	err := helpers.UploadFile(
+	err = helpers.UploadFile(
 		ctx,
 		r.Body,
+		r.ContentLength,
 		&wopiContext.FileReference,
 		app.gwc,
 		wopiContext.AccessToken,
@@ -81,6 +150,8 @@ func PutFile(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 		app.Logger.Error().
 			Err(err).
 			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", r.Header.Get(HeaderWopiLock)).
+			Str("UploadLength", strconv.FormatInt(r.ContentLength, 10)).
 			Str("ViewMode", wopiContext.ViewMode.String()).
 			Str("Requester", wopiContext.User.GetId().String()).
 			Msg("PutFile: uploading the file failed")
@@ -90,8 +161,9 @@ func PutFile(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 
 	app.Logger.Debug().
 		Str("FileReference", wopiContext.FileReference.String()).
+		Str("RequestedLockID", r.Header.Get(HeaderWopiLock)).
+		Str("UploadLength", strconv.FormatInt(r.ContentLength, 10)).
 		Str("ViewMode", wopiContext.ViewMode.String()).
 		Str("Requester", wopiContext.User.GetId().String()).
 		Msg("PutFile: success")
-	http.Error(w, "", http.StatusOK)
 }

services/collaboration/pkg/internal/app/wopiinfo.go

@@ -64,7 +64,8 @@ func CheckFileInfo(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 		HostViewUrl: wopiContext.ViewAppUrl,
 		HostEditUrl: wopiContext.EditAppUrl,
 
-		EnableOwnerTermination: true,
+		//EnableOwnerTermination: true,  // enable only for collabora? wopivalidator is complaining
+		EnableOwnerTermination: false,
 
 		SupportsExtendedLockLength: true,
 

services/collaboration/pkg/internal/app/wopilocking.go

@@ -64,7 +64,6 @@ func GetLock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 		Msg("GetLock success")
 
 	w.Header().Set(HeaderWopiLock, lockID)
-	http.Error(w, http.StatusText(http.StatusOK), http.StatusOK)
 }
 
 // Lock returns a WOPI lock or performs an unlock and relock
@@ -74,8 +73,7 @@ func Lock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	wopiContext, _ := WopiContextFromCtx(ctx)
 
-	// TODO: handle un- and relock
+	oldLockID := r.Header.Get(HeaderWopiOldLock)
-
 	lockID := r.Header.Get(HeaderWopiLock)
 	if lockID == "" {
 		app.Logger.Error().
@@ -87,7 +85,184 @@ func Lock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	req := &providerv1beta1.SetLockRequest{
+	var setOrRefreshStatus *rpcv1beta1.Status
+	if oldLockID == "" {
+		// If the oldLockID is empty, this is a "LOCK" request
+		req := &providerv1beta1.SetLockRequest{
+			Ref: &wopiContext.FileReference,
+			Lock: &providerv1beta1.Lock{
+				LockId:  lockID,
+				AppName: app.Config.App.LockName,
+				Type:    providerv1beta1.LockType_LOCK_TYPE_WRITE,
+				Expiration: &typesv1beta1.Timestamp{
+					Seconds: uint64(time.Now().Add(lockDuration).Unix()),
+				},
+			},
+		}
+
+		resp, err := app.gwc.SetLock(ctx, req)
+		if err != nil {
+			app.Logger.Error().
+				Err(err).
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Msg("SetLock failed")
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			return
+		}
+		setOrRefreshStatus = resp.Status
+	} else {
+		// If the oldLockID isn't empty, this is a "UnlockAndRelock" request. We'll
+		// do a "RefreshLock" in reva and provide the old lock
+		req := &providerv1beta1.RefreshLockRequest{
+			Ref: &wopiContext.FileReference,
+			Lock: &providerv1beta1.Lock{
+				LockId:  lockID,
+				AppName: app.Config.App.LockName,
+				Type:    providerv1beta1.LockType_LOCK_TYPE_WRITE,
+				Expiration: &typesv1beta1.Timestamp{
+					Seconds: uint64(time.Now().Add(lockDuration).Unix()),
+				},
+			},
+			ExistingLockId: oldLockID,
+		}
+
+		resp, err := app.gwc.RefreshLock(ctx, req)
+		if err != nil {
+			app.Logger.Error().
+				Err(err).
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("RequestedOldLockID", oldLockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Msg("UnlockAndRefresh failed")
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			return
+		}
+		setOrRefreshStatus = resp.Status
+	}
+
+	// we're checking the status of either the "SetLock" or "RefreshLock" operations
+	switch setOrRefreshStatus.Code {
+	case rpcv1beta1.Code_CODE_OK:
+		app.Logger.Debug().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Msg("SetLock successful")
+		return
+
+	case rpcv1beta1.Code_CODE_FAILED_PRECONDITION, rpcv1beta1.Code_CODE_ABORTED:
+		// Code_CODE_FAILED_PRECONDITION -> Lock operation mismatched lock
+		// Code_CODE_ABORTED -> UnlockAndRelock operation mismatched lock
+		// In both cases, we need to get the current lock to return it in a
+		// 409 response if needed
+		req := &providerv1beta1.GetLockRequest{
+			Ref: &wopiContext.FileReference,
+		}
+
+		resp, err := app.gwc.GetLock(ctx, req)
+		if err != nil {
+			app.Logger.Error().
+				Err(err).
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Msg("SetLock failed, fallback to GetLock failed too")
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			return
+		}
+
+		if resp.Status.Code != rpcv1beta1.Code_CODE_OK {
+			app.Logger.Error().
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Str("StatusCode", resp.Status.Code.String()).
+				Str("StatusMsg", resp.Status.Message).
+				Msg("SetLock failed, fallback to GetLock failed with unexpected status")
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+		}
+
+		if resp.Lock != nil {
+			if resp.Lock.LockId != lockID {
+				// lockId is different -> return 409 with the current lockId
+				app.Logger.Warn().
+					Str("FileReference", wopiContext.FileReference.String()).
+					Str("RequestedLockID", lockID).
+					Str("ViewMode", wopiContext.ViewMode.String()).
+					Str("Requester", wopiContext.User.GetId().String()).
+					Str("LockID", resp.Lock.LockId).
+					Msg("SetLock conflict")
+				w.Header().Set(HeaderWopiLock, resp.Lock.LockId)
+				http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
+				return
+			}
+
+			// TODO: according to the spec we need to treat this as a RefreshLock
+			// There was a problem with the lock, but the file has the same lockId now.
+			// This should never happen unless there are race conditions.
+			// Since the lockId matches now, we'll assume success for now.
+			// As said in the todo, we probably should send a "RefreshLock" request here.
+			app.Logger.Warn().
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Str("LockID", resp.Lock.LockId).
+				Msg("SetLock lock refreshed instead")
+			return
+		}
+
+		// TODO: Is this the right error code?
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Msg("SetLock failed and could not refresh")
+		http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
+		return
+
+	default:
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Str("StatusCode", setOrRefreshStatus.Code.String()).
+			Str("StatusMsg", setOrRefreshStatus.Message).
+			Msg("SetLock failed with unexpected status")
+		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+		return
+	}
+
+}
+
+// RefreshLock refreshes a provided lock for 30 minutes
+// https://docs.microsoft.com/en-us/microsoft-365/cloud-storage-partner-program/rest/files/refreshlock
+func RefreshLock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
+	ctx := r.Context()
+	wopiContext, _ := WopiContextFromCtx(ctx)
+
+	lockID := r.Header.Get(HeaderWopiLock)
+	if lockID == "" {
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Msg("RefreshLock failed due to empty lockID")
+		http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
+		return
+	}
+
+	req := &providerv1beta1.RefreshLockRequest{
 		Ref: &wopiContext.FileReference,
 		Lock: &providerv1beta1.Lock{
 			LockId:  lockID,
@@ -99,22 +274,15 @@ func Lock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 		},
 	}
 
-	app.Logger.Debug().
+	resp, err := app.gwc.RefreshLock(ctx, req)
-		Str("FileReference", wopiContext.FileReference.String()).
-		Str("ViewMode", wopiContext.ViewMode.String()).
-		Str("Requester", wopiContext.User.GetId().String()).
-		Str("RequestedLockID", lockID).
-		Msg("Performing SetLock")
-
-	resp, err := app.gwc.SetLock(ctx, req)
 	if err != nil {
 		app.Logger.Error().
 			Err(err).
 			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
 			Str("ViewMode", wopiContext.ViewMode.String()).
 			Str("Requester", wopiContext.User.GetId().String()).
-			Str("RequestedLockID", lockID).
+			Msg("RefreshLock failed")
-			Msg("SetLock failed")
 		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return
 	}
@@ -123,15 +291,36 @@ func Lock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 	case rpcv1beta1.Code_CODE_OK:
 		app.Logger.Debug().
 			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
 			Str("ViewMode", wopiContext.ViewMode.String()).
 			Str("Requester", wopiContext.User.GetId().String()).
-			Str("RequestedLockID", lockID).
+			Msg("RefreshLock successful")
-			Msg("SetLock successful")
-		http.Error(w, http.StatusText(http.StatusOK), http.StatusOK)
 		return
 
-	case rpcv1beta1.Code_CODE_FAILED_PRECONDITION:
+	case rpcv1beta1.Code_CODE_NOT_FOUND:
-		// already locked
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Str("StatusCode", resp.Status.Code.String()).
+			Str("StatusMsg", resp.Status.Message).
+			Msg("RefreshLock failed, file reference not found")
+		http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
+		return
+
+	case rpcv1beta1.Code_CODE_ABORTED:
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Str("StatusCode", resp.Status.Code.String()).
+			Str("StatusMsg", resp.Status.Message).
+			Msg("RefreshLock failed, lock mismatch")
+
+		// Either the file is unlocked or there is no lock
+		// We need to return 409 with the current lock
 		req := &providerv1beta1.GetLockRequest{
 			Ref: &wopiContext.FileReference,
 		}
@@ -141,10 +330,10 @@ func Lock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 			app.Logger.Error().
 				Err(err).
 				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
 				Str("ViewMode", wopiContext.ViewMode.String()).
 				Str("Requester", wopiContext.User.GetId().String()).
-				Str("RequestedLockID", lockID).
+				Msg("RefreshLock failed trying to get the current lock")
-				Msg("SetLock failed, fallback to GetLock failed too")
 			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 			return
 		}
@@ -152,71 +341,55 @@ func Lock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 		if resp.Status.Code != rpcv1beta1.Code_CODE_OK {
 			app.Logger.Error().
 				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
 				Str("ViewMode", wopiContext.ViewMode.String()).
 				Str("Requester", wopiContext.User.GetId().String()).
-				Str("RequestedLockID", lockID).
 				Str("StatusCode", resp.Status.Code.String()).
 				Str("StatusMsg", resp.Status.Message).
-				Msg("SetLock failed, fallback to GetLock failed with unexpected status")
+				Msg("RefreshLock failed, tried to get the current lock failed with unexpected status")
 			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		}
-
-		if resp.Lock != nil {
-			if resp.Lock.LockId != lockID {
-				app.Logger.Warn().
-					Str("FileReference", wopiContext.FileReference.String()).
-					Str("ViewMode", wopiContext.ViewMode.String()).
-					Str("Requester", wopiContext.User.GetId().String()).
-					Str("RequestedLockID", lockID).
-					Str("LockID", resp.Lock.LockId).
-					Msg("SetLock conflict")
-				w.Header().Set(HeaderWopiLock, resp.Lock.LockId)
-				http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
-				return
-			}
-
-			// TODO: according to the spec we need to treat this as a RefreshLock
-
-			app.Logger.Warn().
-				Str("FileReference", wopiContext.FileReference.String()).
-				Str("ViewMode", wopiContext.ViewMode.String()).
-				Str("Requester", wopiContext.User.GetId().String()).
-				Str("RequestedLockID", lockID).
-				Str("LockID", resp.Lock.LockId).
-				Msg("SetLock lock refreshed instead")
-			http.Error(w, http.StatusText(http.StatusOK), http.StatusOK)
 			return
 		}
 
-		app.Logger.Error().
+		if resp.Lock == nil {
-			Str("FileReference", wopiContext.FileReference.String()).
+			app.Logger.Error().
-			Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("FileReference", wopiContext.FileReference.String()).
-			Str("Requester", wopiContext.User.GetId().String()).
+				Str("RequestedLockID", lockID).
-			Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
-			Msg("SetLock failed and could not refresh")
+				Str("Requester", wopiContext.User.GetId().String()).
-		http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
+				Str("StatusCode", resp.Status.Code.String()).
-		return
+				Str("StatusMsg", resp.Status.Message).
-
+				Msg("RefreshLock failed, no lock on file")
+			w.Header().Set(HeaderWopiLock, "")
+			http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
+			return
+		} else {
+			// lock is different than the one requested, otherwise we wouldn't reached this point
+			app.Logger.Error().
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Str("LockID", resp.Lock.LockId).
+				Str("StatusCode", resp.Status.Code.String()).
+				Str("StatusMsg", resp.Status.Message).
+				Msg("RefreshLock failed, lock mismatch")
+			w.Header().Set(HeaderWopiLock, resp.Lock.LockId)
+			http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
+			return
+		}
 	default:
 		app.Logger.Error().
 			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
 			Str("ViewMode", wopiContext.ViewMode.String()).
 			Str("Requester", wopiContext.User.GetId().String()).
-			Str("RequestedLockID", lockID).
 			Str("StatusCode", resp.Status.Code.String()).
 			Str("StatusMsg", resp.Status.Message).
-			Msg("SetLock failed with unexpected status")
+			Msg("RefreshLock failed with unexpected status")
 		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return
 	}
-
-}
-
-// RefreshLock refreshes a provided lock for 30 minutes
-// https://docs.microsoft.com/en-us/microsoft-365/cloud-storage-partner-program/rest/files/refreshlock
-func RefreshLock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
-	// TODO: implement
-	http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented)
 }
 
 // UnLock removes a given lock from a file
@@ -249,32 +422,103 @@ func UnLock(app *DemoApp, w http.ResponseWriter, r *http.Request) {
 		app.Logger.Error().
 			Err(err).
 			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
 			Str("ViewMode", wopiContext.ViewMode.String()).
 			Str("Requester", wopiContext.User.GetId().String()).
-			Str("RequestedLockID", lockID).
 			Msg("Unlock failed")
 		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return
 	}
 
-	if resp.Status.Code != rpcv1beta1.Code_CODE_OK {
+	switch resp.Status.Code {
-		app.Logger.Error().
+	case rpcv1beta1.Code_CODE_OK:
+		app.Logger.Debug().
 			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
 			Str("ViewMode", wopiContext.ViewMode.String()).
 			Str("Requester", wopiContext.User.GetId().String()).
+			Msg("Unlock successful")
+		return
+	case rpcv1beta1.Code_CODE_ABORTED:
+		// File isn't locked. Need to return 409 with empty lock
+		app.Logger.Error().
+			Err(err).
+			Str("FileReference", wopiContext.FileReference.String()).
 			Str("RequestedLockID", lockID).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
+			Msg("Unlock failed, file isn't locked")
+		w.Header().Set(HeaderWopiLock, "")
+		http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
+		return
+	case rpcv1beta1.Code_CODE_LOCKED:
+		// We need to return 409 with the current lock
+		req := &providerv1beta1.GetLockRequest{
+			Ref: &wopiContext.FileReference,
+		}
+
+		resp, err := app.gwc.GetLock(ctx, req)
+		if err != nil {
+			app.Logger.Error().
+				Err(err).
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Msg("Unlock failed trying to get the current lock")
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			return
+		}
+
+		if resp.Status.Code != rpcv1beta1.Code_CODE_OK {
+			app.Logger.Error().
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Str("StatusCode", resp.Status.Code.String()).
+				Str("StatusMsg", resp.Status.Message).
+				Msg("Unlock failed, tried to get the current lock failed with unexpected status")
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			return
+		}
+
+		if resp.Lock == nil {
+			app.Logger.Error().
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Str("StatusCode", resp.Status.Code.String()).
+				Str("StatusMsg", resp.Status.Message).
+				Msg("Unlock failed, no lock on file")
+			w.Header().Set(HeaderWopiLock, "")
+			http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
+		} else {
+			// lock is different than the one requested, otherwise we wouldn't reached this point
+			app.Logger.Error().
+				Str("FileReference", wopiContext.FileReference.String()).
+				Str("RequestedLockID", lockID).
+				Str("ViewMode", wopiContext.ViewMode.String()).
+				Str("Requester", wopiContext.User.GetId().String()).
+				Str("LockID", resp.Lock.LockId).
+				Str("StatusCode", resp.Status.Code.String()).
+				Str("StatusMsg", resp.Status.Message).
+				Msg("Unlock failed, lock mismatch")
+			w.Header().Set(HeaderWopiLock, resp.Lock.LockId)
+			http.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict)
+		}
+		return
+	default:
+		app.Logger.Error().
+			Str("FileReference", wopiContext.FileReference.String()).
+			Str("RequestedLockID", lockID).
+			Str("ViewMode", wopiContext.ViewMode.String()).
+			Str("Requester", wopiContext.User.GetId().String()).
 			Str("StatusCode", resp.Status.Code.String()).
 			Str("StatusMsg", resp.Status.Message).
 			Msg("Unlock failed with unexpected status")
 		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return
 	}
-
-	app.Logger.Debug().
-		Str("FileReference", wopiContext.FileReference.String()).
-		Str("ViewMode", wopiContext.ViewMode.String()).
-		Str("Requester", wopiContext.User.GetId().String()).
-		Str("RequestedLockID", lockID).
-		Msg("Unlock successful")
-	http.Error(w, http.StatusText(http.StatusOK), http.StatusOK)
 }

services/collaboration/pkg/internal/helpers/download.go

@@ -54,6 +54,7 @@ func DownloadFile(
 			downloadEndpoint = proto.DownloadEndpoint
 			downloadToken = proto.Token
 			hasDownloadToken = proto.Token != ""
+			break
 		}
 	}
 
@@ -89,7 +90,7 @@ func DownloadFile(
 		httpReq.Header.Add("X-Reva-Transfer", downloadToken)
 	}
 	// TODO: the access token shouldn't be needed
-	httpReq.Header.Add("x-access-token", token)
+	httpReq.Header.Add("X-Access-Token", token)
 
 	httpResp, err := httpClient.Do(httpReq)
 	if err != nil {

services/collaboration/pkg/internal/helpers/upload.go

@@ -7,16 +7,40 @@ import (
 	"io"
 	"net/http"
 	"strconv"
+	"time"
 
 	gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
 )
 
-func UploadFile(ctx context.Context, content io.ReadCloser, ref *providerv1beta1.Reference, gwc gatewayv1beta1.GatewayAPIClient, token string, lockID string, insecure bool, logger log.Logger) error {
+func UploadFile(
+	ctx context.Context,
+	content io.ReadCloser,
+	contentLength int64,
+	ref *providerv1beta1.Reference,
+	gwc gatewayv1beta1.GatewayAPIClient,
+	token string,
+	lockID string,
+	insecure bool,
+	logger log.Logger,
+) error {
+	opaque := &types.Opaque{
+		Map: make(map[string]*types.OpaqueEntry),
+	}
+
+	strContentLength := strconv.FormatInt(contentLength, 10)
+	if contentLength >= 0 {
+		opaque.Map["Upload-Length"] = &types.OpaqueEntry{
+			Decoder: "plain",
+			Value:   []byte(strContentLength),
+		}
+	}
 
 	req := &providerv1beta1.InitiateFileUploadRequest{
+		Opaque: opaque,
 		Ref:    ref,
 		LockId: lockID,
 		// TODO: if-match
@@ -31,6 +55,7 @@ func UploadFile(ctx context.Context, content io.ReadCloser, ref *providerv1beta1
 			Err(err).
 			Str("FileReference", ref.String()).
 			Str("RequestedLockID", lockID).
+			Str("UploadLength", strContentLength).
 			Msg("UploadHelper: InitiateFileUpload failed")
 		return err
 	}
@@ -39,12 +64,18 @@ func UploadFile(ctx context.Context, content io.ReadCloser, ref *providerv1beta1
 		logger.Error().
 			Str("FileReference", ref.String()).
 			Str("RequestedLockID", lockID).
+			Str("UploadLength", strContentLength).
 			Str("StatusCode", resp.Status.Code.String()).
 			Str("StatusMsg", resp.Status.Message).
 			Msg("UploadHelper: InitiateFileUpload failed with wrong status")
 		return errors.New("InitiateFileUpload failed with status " + resp.Status.Code.String())
 	}
 
+	// if content length is 0, we're done. We don't upload anything to the target endpoint
+	if contentLength == 0 {
+		return nil
+	}
+
 	uploadEndpoint := ""
 	uploadToken := ""
 	hasUploadToken := false
@@ -62,6 +93,7 @@ func UploadFile(ctx context.Context, content io.ReadCloser, ref *providerv1beta1
 		logger.Error().
 			Str("FileReference", ref.String()).
 			Str("RequestedLockID", lockID).
+			Str("UploadLength", strContentLength).
 			Str("Endpoint", uploadEndpoint).
 			Bool("HasUploadToken", hasUploadToken).
 			Msg("UploadHelper: Upload endpoint or token is missing")
@@ -74,6 +106,7 @@ func UploadFile(ctx context.Context, content io.ReadCloser, ref *providerv1beta1
 				InsecureSkipVerify: insecure,
 			},
 		},
+		Timeout: 10 * time.Second,
 	}
 
 	httpReq, err := http.NewRequestWithContext(ctx, http.MethodPut, uploadEndpoint, content)
@@ -82,19 +115,25 @@ func UploadFile(ctx context.Context, content io.ReadCloser, ref *providerv1beta1
 			Err(err).
 			Str("FileReference", ref.String()).
 			Str("RequestedLockID", lockID).
+			Str("UploadLength", strContentLength).
 			Str("Endpoint", uploadEndpoint).
 			Bool("HasUploadToken", hasUploadToken).
 			Msg("UploadHelper: Could not create the request to the endpoint")
 		return err
 	}
+	// "content" is an *http.body and doesn't fill the httpReq.ContentLength automatically
+	// we need to fill the ContentLength ourselves, and must match the stream length in order
+	// to prevent issues
+	httpReq.ContentLength = contentLength
 
 	if uploadToken != "" {
 		// public link uploads have the token in the upload endpoint
 		httpReq.Header.Add("X-Reva-Transfer", uploadToken)
 	}
 	// TODO: the access token shouldn't be needed
-	httpReq.Header.Add("x-access-token", token)
+	httpReq.Header.Add("X-Access-Token", token)
 
+	httpReq.Header.Add("X-Lock-Id", lockID)
 	// TODO: better mechanism for the upload while locked, relies on patch in REVA
 	//if lockID, ok := ctxpkg.ContextGetLockID(ctx); ok {
 	//	httpReq.Header.Add("X-Lock-Id", lockID)
@@ -106,16 +145,19 @@ func UploadFile(ctx context.Context, content io.ReadCloser, ref *providerv1beta1
 			Err(err).
 			Str("FileReference", ref.String()).
 			Str("RequestedLockID", lockID).
+			Str("UploadLength", strContentLength).
 			Str("Endpoint", uploadEndpoint).
 			Bool("HasUploadToken", hasUploadToken).
 			Msg("UploadHelper: Put request to the upload endpoint failed")
 		return err
 	}
+	defer httpResp.Body.Close()
 
 	if httpResp.StatusCode != http.StatusOK {
 		logger.Error().
 			Str("FileReference", ref.String()).
 			Str("RequestedLockID", lockID).
+			Str("UploadLength", strContentLength).
 			Str("Endpoint", uploadEndpoint).
 			Bool("HasUploadToken", hasUploadToken).
 			Int("HttpCode", httpResp.StatusCode).

services/collaboration/pkg/server/http/server.go

@@ -109,6 +109,7 @@ func prepareRoutes(r *chi.Mux, demoapp *app.DemoApp) {
 				switch action {
 
 				case "LOCK":
+					// "UnlockAndRelock" operation goes through here
 					app.Lock(demoapp, w, r)
 				case "GET_LOCK":
 					app.GetLock(demoapp, w, r)