sevenhill/QSfera
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: implement review

Browse Source
This commit is contained in:
Michael Barz
2024-08-30 16:27:36 +02:00
parent 90b36a4f64
commit 903b513d9c
9 changed files with 59 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files
services/collaboration/pkg/connector/connector.go
+12 -18
View File
@@ -1,8 +1,9 @@
package connector
import (
"strconv"
types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
"github.com/owncloud/ocis/v2/services/collaboration/pkg/helpers"
)
// ConnectorResponse represent a response from the FileConnectorService.
@@ -57,24 +58,11 @@ func NewResponseLockConflict(lockID string, lockFailureReason string) *Connector
// NewResponseWithVersion creates a new ConnectorResponse with the specified status
// and the "X-WOPI-ItemVersion" header having the value in the mtime parameter.
func NewResponseWithVersion(status int, mtime *types.Timestamp) *ConnectorResponse {
func NewResponseWithVersion(mtime *types.Timestamp) *ConnectorResponse {
return &ConnectorResponse{
Status: status,
Status: 200,
Headers: map[string]string{
HeaderWopiVersion: helpers.GetVersion(mtime),
},
}
}
// NewResponseConflictWithVersion creates a new ConnectorResponse with the status 409
// and the "X-WOPI-ItemVersion" header having the value in the mtime parameter.
// The lockFailureReason parameter will be included in the "X-WOPI-LockFailureReason".
func NewResponseConflictWithVersion(mtime *types.Timestamp, lockFailureReason string) *ConnectorResponse {
return &ConnectorResponse{
Status: 409,
Headers: map[string]string{
HeaderWopiVersion: helpers.GetVersion(mtime),
HeaderWopiLockFailureReason: lockFailureReason,
HeaderWopiVersion: getVersion(mtime),
},
}
}
@@ -86,7 +74,7 @@ func NewResponseWithVersionAndLock(status int, mtime *types.Timestamp, lockID st
r := &ConnectorResponse{
Status: status,
Headers: map[string]string{
HeaderWopiVersion: helpers.GetVersion(mtime),
HeaderWopiVersion: getVersion(mtime),
HeaderWopiLock: lockID,
},
}
@@ -196,3 +184,9 @@ func (c *Connector) GetFileConnector() FileConnectorService {
func (c *Connector) GetContentConnector() ContentConnectorService {
return c.contentConnector
}
// getVersion returns a string representation of the timestamp
func getVersion(timestamp *types.Timestamp) string {
return "v" + strconv.FormatUint(timestamp.GetSeconds(), 10) +
strconv.FormatUint(uint64(timestamp.GetNanos()), 10)
}
services/collaboration/pkg/connector/contentconnector.go
+2 -3
View File
@@ -17,7 +17,6 @@ import (
revactx "github.com/cs3org/reva/v2/pkg/ctx"
"github.com/owncloud/ocis/v2/ocis-pkg/tracing"
"github.com/owncloud/ocis/v2/services/collaboration/pkg/config"
"github.com/owncloud/ocis/v2/services/collaboration/pkg/helpers"
"github.com/owncloud/ocis/v2/services/collaboration/pkg/middleware"
"github.com/rs/zerolog"
"go.opentelemetry.io/otel/propagation"
@@ -181,7 +180,7 @@ func (c *ContentConnector) GetFile(ctx context.Context, w http.ResponseWriter) e
return NewConnectorError(500, "GetFile: Downloading the file failed")
}
helpers.SetVersionHeader(w, sResp.GetInfo().GetMtime())
w.Header().Set(HeaderWopiVersion, getVersion(sResp.GetInfo().GetMtime()))
// Copy the download into the writer
_, err = io.Copy(w, httpResp.Body)
@@ -404,5 +403,5 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream
}
logger.Debug().Msg("PutFile: success")
return NewResponseWithVersion(200, mtime), nil
return NewResponseWithVersion(mtime), nil
}
services/collaboration/pkg/connector/fileconnector.go
+6 -6
View File
@@ -259,7 +259,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*Co
switch setOrRefreshStatus.GetCode() {
case rpcv1beta1.Code_CODE_OK:
logger.Debug().Msg("SetLock successful")
return NewResponseWithVersion(200, statResp.GetInfo().GetMtime()), nil
return NewResponseWithVersion(statResp.GetInfo().GetMtime()), nil
case rpcv1beta1.Code_CODE_FAILED_PRECONDITION, rpcv1beta1.Code_CODE_ABORTED:
// Code_CODE_FAILED_PRECONDITION -> Lock operation mismatched lock
@@ -300,7 +300,7 @@ func (f *FileConnector) Lock(ctx context.Context, lockID, oldLockID string) (*Co
logger.Warn().
Str("LockID", resp.GetLock().GetLockId()).
Msg("SetLock lock refreshed instead")
return NewResponseWithVersionAndLock(200, statResp.GetInfo().GetMtime(), resp.GetLock().GetLockId()), nil
return NewResponseWithVersion(statResp.GetInfo().GetMtime()), nil
}
logger.Error().Msg("SetLock failed and could not refresh")
@@ -388,7 +388,7 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (*Connec
logger.Debug().Msg("RefreshLock successful")
// The current lock should not be returned in the headers on success
// https://learn.microsoft.com/en-us/microsoft-365/cloud-storage-partner-program/rest/files/refreshlock#response-headers
return NewResponseWithVersion(200, statResp.GetInfo().GetMtime()), nil
return NewResponseWithVersion(statResp.GetInfo().GetMtime()), nil
case rpcv1beta1.Code_CODE_NOT_FOUND:
logger.Error().
@@ -428,7 +428,7 @@ func (f *FileConnector) RefreshLock(ctx context.Context, lockID string) (*Connec
Str("StatusCode", resp.GetStatus().GetCode().String()).
Str("StatusMsg", resp.GetStatus().GetMessage()).
Msg("RefreshLock failed, no lock on file")
return NewResponseConflictWithVersion(statResp.GetInfo().GetMtime(), "No lock on file"), nil
return NewResponseLockConflict("", "No lock on file"), nil
} else {
// lock is different than the one requested, otherwise we wouldn't reached this point
logger.Error().
@@ -510,7 +510,7 @@ func (f *FileConnector) UnLock(ctx context.Context, lockID string) (*ConnectorRe
switch resp.GetStatus().GetCode() {
case rpcv1beta1.Code_CODE_OK:
logger.Debug().Msg("Unlock successful")
return NewResponseWithVersion(200, statResp.GetInfo().GetMtime()), nil
return NewResponseWithVersion(statResp.GetInfo().GetMtime()), nil
case rpcv1beta1.Code_CODE_ABORTED:
// File isn't locked. Need to return 409 with empty lock
logger.Error().Err(err).Msg("Unlock failed, file isn't locked")
@@ -1114,7 +1114,7 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (*ConnectorResponse,
infoMap := map[string]interface{}{
fileinfo.KeyOwnerID: hexEncodedOwnerId,
fileinfo.KeySize: int64(statRes.GetInfo().GetSize()),
fileinfo.KeyVersion: helpers.GetVersion(statRes.GetInfo().GetMtime()),
fileinfo.KeyVersion: getVersion(statRes.GetInfo().GetMtime()),
fileinfo.KeyBaseFileName: path.Base(statRes.GetInfo().GetPath()),
fileinfo.KeyBreadcrumbDocName: path.Base(statRes.GetInfo().GetPath()),
// to get the folder we actually need to do a GetPath() request
services/collaboration/pkg/connector/fileconnector_test.go
+2 -4
View File
@@ -255,8 +255,7 @@ var _ = Describe("FileConnector", func() {
response, err := fc.Lock(ctx, "abcdef123", "")
Expect(err).ToNot(HaveOccurred())
Expect(response.Status).To(Equal(200))
Expect(response.Headers).To(HaveLen(2))
Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("abcdef123"))
Expect(response.Headers).To(HaveLen(1))
Expect(response.Headers[connector.HeaderWopiVersion]).To(Equal("v123456789"))
})
@@ -437,8 +436,7 @@ var _ = Describe("FileConnector", func() {
response, err := fc.Lock(ctx, "abcdef123", "112233")
Expect(err).ToNot(HaveOccurred())
Expect(response.Status).To(Equal(200))
Expect(response.Headers).To(HaveLen(2))
Expect(response.Headers[connector.HeaderWopiLock]).To(Equal("abcdef123"))
Expect(response.Headers).To(HaveLen(1))
Expect(response.Headers[connector.HeaderWopiVersion]).To(Equal("v123456789"))
})
services/collaboration/pkg/connector/httpadapter.go
+1
View File
@@ -51,6 +51,7 @@ func NewHttpAdapter(gwc gatewayv1beta1.GatewayAPIClient, cfg *config.Config) *Ht
),
}
// TODO: check if we can get rid of custom log parsing completely
httpAdapter.locks = &locks.NoopLockParser{}
return httpAdapter
}
services/collaboration/pkg/connector/httpadapter_test.go
+1 -3
View File
@@ -357,9 +357,7 @@ var _ = Describe("HttpAdapter", func() {
w := httptest.NewRecorder()
fc.On("UnLock", mock.Anything, "abc123").Times(1).Return(
connector.NewResponseWithVersion(200,
&typesv1beta1.Timestamp{Seconds: uint64(1234), Nanos: uint32(567)},
), nil)
connector.NewResponseWithVersion(&typesv1beta1.Timestamp{Seconds: uint64(1234), Nanos: uint32(567)}), nil)
httpAdapter.UnLock(w, req)
resp := w.Result()
services/collaboration/pkg/helpers/path.go
-41
View File
@@ -1,41 +0,0 @@
package helpers
import (
"strings"
"github.com/golang-jwt/jwt/v5"
"github.com/owncloud/ocis/v2/services/collaboration/pkg/config"
)
// ParseWopiFileID extracts the file id from a wopi path
//
// If the file id is a jwt, it will be decoded and the file id will be extracted from the jwt claims.
// If the file id is not a jwt, it will be returned as is.
func ParseWopiFileID(cfg *config.Config, path string) string {
s := strings.Split(path, "/")
if len(s) < 4 || (s[1] != "wopi" && s[2] != "files") {
return path
}
// check if the fileid is a jwt
if strings.Contains(s[3], ".") {
token, err := jwt.Parse(s[3], func(_ *jwt.Token) (interface{}, error) {
return []byte(cfg.Wopi.ProxySecret), nil
})
if err != nil {
return s[3]
}
claims, ok := token.Claims.(jwt.MapClaims)
if !ok {
return s[3]
}
f, ok := claims["f"].(string)
if !ok {
return s[3]
}
return f
}
// fileid is not a jwt
return s[3]
}
services/collaboration/pkg/helpers/version.go
-20
View File
@@ -1,20 +0,0 @@
package helpers
import (
"net/http"
"strconv"
typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
)
// SetVersionHeader sets a WOPI version header on the response writer
func SetVersionHeader(w http.ResponseWriter, t *typesv1beta1.Timestamp) {
// non-canonical headers can only be set directly on the header map
w.Header().Set("X-WOPI-ItemVersion", GetVersion(t))
}
// GetVersion returns a string representation of the timestamp
func GetVersion(timestamp *typesv1beta1.Timestamp) string {
return "v" + strconv.FormatUint(timestamp.GetSeconds(), 10) +
strconv.FormatUint(uint64(timestamp.GetNanos()), 10)
}
services/collaboration/pkg/middleware/wopicontext.go
+35 -1
View File
@@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"net/http"
"strings"
appproviderv1beta1 "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1"
providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
@@ -109,7 +110,7 @@ func WopiContextAuthMiddleware(cfg *config.Config, next http.Handler) http.Handl
ctx = wopiLogger.WithContext(ctx)
hashedRef := helpers.HashResourceId(claims.WopiContext.FileReference.GetResourceId())
fileID := helpers.ParseWopiFileID(cfg, r.URL.Path)
fileID := parseWopiFileID(cfg, r.URL.Path)
if fileID != hashedRef {
wopiLogger.Error().Msg("file reference in the URL doesn't match the one inside the access token")
http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
@@ -167,3 +168,36 @@ func GenerateWopiToken(wopiContext WopiContext, cfg *config.Config) (string, int
return accessToken, claims.ExpiresAt.UnixMilli(), err
}
// parseWopiFileID extracts the file id from a wopi path
//
// If the file id is a jwt, it will be decoded and the file id will be extracted from the jwt claims.
// If the file id is not a jwt, it will be returned as is.
func parseWopiFileID(cfg *config.Config, path string) string {
s := strings.Split(path, "/")
if len(s) < 4 || (s[1] != "wopi" && s[2] != "files") {
return path
}
// check if the fileid is a jwt
if strings.Contains(s[3], ".") {
token, err := jwt.Parse(s[3], func(_ *jwt.Token) (interface{}, error) {
return []byte(cfg.Wopi.ProxySecret), nil
})
if err != nil {
return s[3]
}
claims, ok := token.Claims.(jwt.MapClaims)
if !ok {
return s[3]
}
f, ok := claims["f"].(string)
if !ok {
return s[3]
}
return f
}
// fileid is not a jwt
return s[3]
}