From 3f5585628d63f391ccc5e066cce8c76a8dd545ff Mon Sep 17 00:00:00 2001 From: jkoberg Date: Fri, 2 Sep 2022 10:11:17 +0200 Subject: [PATCH] adjust REPORT to PROPFIND endpoint Signed-off-by: jkoberg --- changelog/unreleased/fix-search-report.md | 12 +++ .../pkg/search/provider/searchprovider.go | 81 +++++++++++++++++-- services/webdav/pkg/service/v0/search.go | 51 +++++++++++- 3 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 changelog/unreleased/fix-search-report.md diff --git a/changelog/unreleased/fix-search-report.md b/changelog/unreleased/fix-search-report.md new file mode 100644 index 0000000000..c6470cd324 --- /dev/null +++ b/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 diff --git a/services/search/pkg/search/provider/searchprovider.go b/services/search/pkg/search/provider/searchprovider.go index 719fa6a7b9..033cd17f83 100644 --- a/services/search/pkg/search/provider/searchprovider.go +++ b/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,7 +139,11 @@ 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": @@ -127,7 +151,7 @@ func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*s 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) +} diff --git a/services/webdav/pkg/service/v0/search.go b/services/webdav/pkg/service/v0/search.go index 64d7cc0a5f..18258de6a2 100644 --- a/services/webdav/pkg/service/v0/search.go +++ b/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) {