Fix REPORT response from webdav

changelog/unreleased/fix-search-report.md

@@ -0,0 +1,12 @@
+Bugfix: Fix search report
+
+There were multiple issues with REPORT search responses from webdav. Also we want it to be consistent with PROPFIND responses.
+*   the `remote.php` prefix was missing from the href (added even though not neccessary)
+*   the ids were formatted wrong, they should look different for shares and spaces.
+*   the name of the resource was missing
+*   the shareid was missing (for shares)
+*   the prop `shareroot` (containing the name of the share root) was missing
+*   the permissions prop was empty
+
+https://github.com/owncloud/web/issues/7557
+https://github.com/owncloud/ocis/pull/4484

protogen/gen/ocis/services/search/v0/search.swagger.json

@@ -192,6 +192,9 @@
         },
         "deleted": {
           "type": "boolean"
+        },
+        "shareRootName": {
+          "type": "string"
         }
       }
     },

protogen/proto/ocis/messages/search/v0/search.proto

@@ -28,6 +28,7 @@ message Entity {
 	string permissions = 8;
 	uint64 type = 9;
 	bool deleted = 10;
+	string shareRootName = 11;
 }
 
 message Match {

services/search/pkg/search/provider/searchprovider.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"path/filepath"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
@@ -28,6 +29,25 @@ import (
 	searchsvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/search/v0"
 )
 
+// Permissions is copied from reva internal conversion pkg
+type Permissions uint
+
+// consts are copied from reva internal conversion pkg
+const (
+	// PermissionInvalid represents an invalid permission
+	PermissionInvalid Permissions = 0
+	// PermissionRead grants read permissions on a resource
+	PermissionRead Permissions = 1 << (iota - 1)
+	// PermissionWrite grants write permissions on a resource
+	PermissionWrite
+	// PermissionCreate grants create permissions on a resource
+	PermissionCreate
+	// PermissionDelete grants delete permissions on a resource
+	PermissionDelete
+	// PermissionShare grants share permissions on a resource
+	PermissionShare
+)
+
 var ListenEvents = []events.Unmarshaller{
 	events.ItemTrashed{},
 	events.ItemRestored{},
@@ -119,15 +139,19 @@ func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*s
 			SpaceId:   space.Root.SpaceId,
 			OpaqueId:  space.Root.OpaqueId,
 		}
-		var mountpointRootId *searchmsg.ResourceID
+		var (
+			mountpointRootID *searchmsg.ResourceID
+			rootName         string
+			permissions      *provider.ResourcePermissions
+		)
 		mountpointPrefix := ""
 		switch space.SpaceType {
 		case "mountpoint":
 			continue // mountpoint spaces are only "links" to the shared spaces. we have to search the shared "grant" space instead
 		case "grant":
 			// In case of grant spaces we search the root of the outer space and translate the paths to the according mountpoint
 			searchRootId.OpaqueId = space.Root.SpaceId
-			mountpointId, ok := mountpointMap[space.Id.OpaqueId]
+			mountpointID, ok := mountpointMap[space.Id.OpaqueId]
 			if !ok {
 				p.logger.Warn().Interface("space", space).Msg("could not find mountpoint space for grant space")
 				continue
@@ -144,17 +168,19 @@ func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*s
 				continue
 			}
 			mountpointPrefix = utils.MakeRelativePath(gpRes.Path)
-			sid, spid, oid, err := storagespace.SplitID(mountpointId)
+			sid, spid, oid, err := storagespace.SplitID(mountpointID)
 			if err != nil {
-				p.logger.Error().Err(err).Str("space", space.Id.OpaqueId).Str("mountpointId", mountpointId).Msg("invalid mountpoint space id")
+				p.logger.Error().Err(err).Str("space", space.Id.OpaqueId).Str("mountpointId", mountpointID).Msg("invalid mountpoint space id")
 				continue
 			}
-			mountpointRootId = &searchmsg.ResourceID{
+			mountpointRootID = &searchmsg.ResourceID{
 				StorageId: sid,
 				SpaceId:   spid,
 				OpaqueId:  oid,
 			}
-			p.logger.Debug().Interface("grantSpace", space).Interface("mountpointRootId", mountpointRootId).Msg("searching a grant")
+			rootName = space.GetRootInfo().GetPath()
+			permissions = space.GetRootInfo().GetPermissionSet()
+			p.logger.Debug().Interface("grantSpace", space).Interface("mountpointRootId", mountpointRootID).Msg("searching a grant")
 		}
 
 		res, err := p.indexClient.Search(ctx, &searchsvc.SearchIndexRequest{
@@ -176,9 +202,11 @@ func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*s
 			if mountpointPrefix != "" {
 				match.Entity.Ref.Path = utils.MakeRelativePath(strings.TrimPrefix(match.Entity.Ref.Path, mountpointPrefix))
 			}
-			if mountpointRootId != nil {
-				match.Entity.Ref.ResourceId = mountpointRootId
+			if mountpointRootID != nil {
+				match.Entity.Ref.ResourceId = mountpointRootID
 			}
+			match.Entity.ShareRootName = rootName
+			match.Entity.Permissions = convertToOCS(permissions)
 			matches = append(matches, match)
 		}
 	}
@@ -279,3 +307,40 @@ func formatQuery(q string) string {
 	// this is a basic filename search
 	return "Name:*" + strings.ReplaceAll(strings.ToLower(query), " ", `\ `) + "*"
 }
+
+// NOTE: this converts cs3 to ocs permissions
+// since conversions pkg is reva internal we have no other choice than to duplicate the logic
+func convertToOCS(p *provider.ResourcePermissions) string {
+	var ocs Permissions
+	if p == nil {
+		return ""
+	}
+	if p.ListContainer &&
+		p.ListFileVersions &&
+		p.ListRecycle &&
+		p.Stat &&
+		p.GetPath &&
+		p.GetQuota &&
+		p.InitiateFileDownload {
+		ocs |= PermissionRead
+	}
+	if p.InitiateFileUpload &&
+		p.RestoreFileVersion &&
+		p.RestoreRecycleItem {
+		ocs |= PermissionWrite
+	}
+	if p.ListContainer &&
+		p.Stat &&
+		p.CreateContainer &&
+		p.InitiateFileUpload {
+		ocs |= PermissionCreate
+	}
+	if p.Delete &&
+		p.PurgeRecycle {
+		ocs |= PermissionDelete
+	}
+	if p.AddGrant {
+		ocs |= PermissionShare
+	}
+	return strconv.FormatUint(uint64(ocs), 10)
+}

services/webdav/pkg/service/v0/search.go

@@ -12,6 +12,8 @@ import (
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	revactx "github.com/cs3org/reva/v2/pkg/ctx"
+	"github.com/cs3org/reva/v2/pkg/storagespace"
+	"github.com/cs3org/reva/v2/pkg/utils"
 	searchmsg "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/search/v0"
 	searchsvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/search/v0"
 	"github.com/owncloud/ocis/v2/services/webdav/pkg/net"
@@ -102,8 +104,39 @@ func multistatusResponse(ctx context.Context, matches []*searchmsg.Match) ([]byt
 }
 
 func matchToPropResponse(ctx context.Context, match *searchmsg.Match) (*propfind.ResponseXML, error) {
+	// unfortunately search uses own versions of ResourceId and Ref. So we need to assert them here
+	var (
+		ref string
+		err error
+	)
+
+	// to copy PROPFIND behaviour we need to deliver different ids
+	// for shares it needs to be sharestorageproviderid!shareid
+	// for other spaces it needs to be storageproviderid$spaceid
+	switch match.Entity.Ref.ResourceId.StorageId {
+	default:
+		ref, err = storagespace.FormatReference(&provider.Reference{
+			ResourceId: &provider.ResourceId{
+				StorageId: match.Entity.Ref.ResourceId.StorageId,
+				SpaceId:   match.Entity.Ref.ResourceId.SpaceId,
+			},
+			Path: match.Entity.Ref.Path,
+		})
+	case utils.ShareStorageProviderID:
+		ref, err = storagespace.FormatReference(&provider.Reference{
+			ResourceId: &provider.ResourceId{
+				//StorageId: match.Entity.Ref.ResourceId.StorageId,
+				SpaceId:  match.Entity.Ref.ResourceId.SpaceId,
+				OpaqueId: match.Entity.Ref.ResourceId.OpaqueId,
+			},
+			Path: match.Entity.Ref.Path,
+		})
+	}
+	if err != nil {
+		return nil, err
+	}
 	response := propfind.ResponseXML{
-		Href:     net.EncodePath(path.Join("/dav/spaces/", match.Entity.Ref.ResourceId.StorageId+"!"+match.Entity.Ref.ResourceId.OpaqueId, match.Entity.Ref.Path)),
+		Href:     net.EncodePath(path.Join("/remote.php/dav/spaces/", ref)),
 		Propstat: []propfind.PropstatXML{},
 	}
 
@@ -112,10 +145,22 @@ func matchToPropResponse(ctx context.Context, match *searchmsg.Match) (*propfind
 		Prop:   []prop.PropertyXML{},
 	}
 
-	propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:fileid", match.Entity.Id.StorageId+"!"+match.Entity.Id.OpaqueId))
-	propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("d:getetag", match.Entity.Etag))
+	propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:fileid", storagespace.FormatResourceID(provider.ResourceId{
+		StorageId: match.Entity.Id.StorageId,
+		SpaceId:   match.Entity.Id.SpaceId,
+		OpaqueId:  match.Entity.Id.OpaqueId,
+	})))
+	if match.Entity.Ref.ResourceId.StorageId == utils.ShareStorageProviderID {
+		propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:shareid", match.Entity.Ref.ResourceId.OpaqueId))
+		propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:shareroot", match.Entity.ShareRootName))
+	}
+	propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:name", match.Entity.Name))
 	propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("d:getlastmodified", match.Entity.LastModifiedTime.AsTime().Format(time.RFC3339)))
 	propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("d:getcontenttype", match.Entity.MimeType))
+	propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:permissions", match.Entity.Permissions))
+
+	// those seem empty - bug?
+	propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("d:getetag", match.Entity.Etag))
 
 	size := strconv.FormatUint(match.Entity.Size, 10)
 	if match.Entity.Type == uint64(provider.ResourceType_RESOURCE_TYPE_CONTAINER) {

tests/acceptance/expected-failures-API-on-OCIS-storage.md

@@ -869,7 +869,7 @@ _ocdav: api compatibility, return correct status code_
 
 - [apiCapabilities/capabilitiesWithNormalUser.feature:11](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiCapabilities/capabilitiesWithNormalUser.feature#L11) Scenario: getting default capabilities with normal user
 
-#### [/webdav and spaces endpoint does not alloow REPORT requests](https://github.com/owncloud/ocis/issues/4034)
+#### [/webdav and spaces endpoint does not allow REPORT requests](https://github.com/owncloud/ocis/issues/4034)
 
 - [apiWebdavOperations/search.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L48)
 - [apiWebdavOperations/search.feature:70](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L70)
@@ -1301,5 +1301,20 @@ Not everything needs to be implemented for ocis. While the oc10 testsuite covers
 - [apiAuthWebDav/webDavDELETEAuth.feature:188](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavDELETEAuth.feature#L188)
 - [apiAuthWebDav/webDavDELETEAuth.feature:199](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavDELETEAuth.feature#L199)
 
+#### [search tests are parsing id wrongly](https://github.com/owncloud/ocis/pull/4485)
+- [apiWebdavOperations/search.feature:126](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L126)
+- [apiWebdavOperations/search.feature:127](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L127)
+- [apiWebdavOperations/search.feature:150](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L150)
+- [apiWebdavOperations/search.feature:151](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L151)
+- [apiWebdavOperations/search.feature:42](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L42)
+- [apiWebdavOperations/search.feature:43](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L43)
+- [apiWebdavOperations/search.feature:64](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L64)
+- [apiWebdavOperations/search.feature:65](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L65)
+- [apiWebdavOperations/search.feature:87](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L87)
+- [apiWebdavOperations/search.feature:88](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L88)
+- [apiWebdavOperations/search.feature:174](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L174)
+- [apiWebdavOperations/search.feature:175](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L175)
+- [apiWebdavOperations/search.feature:264](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L264)
+- [apiWebdavOperations/search.feature:265](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L265)
 Note: always have an empty line at the end of this file.
 The bash script that processes this file requires that the last line has a newline on the end.