From ce8c7311851dbc81743f0b23fe7fce8b41712795 Mon Sep 17 00:00:00 2001 From: David Christofas Date: Mon, 7 Mar 2022 12:43:34 +0100 Subject: [PATCH] implement listing shares in spaces Instead of the query parameter `path` one can now send `space_ref` to list shares in spaces. The url would look like this: `https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares?reshares=true&space_ref=ae96eb17-1b37-4bfc-8ed6-9c044723a43f/elmo.gif` --- changelog/unreleased/sharing-in-spaces.md | 5 + .../handlers/apps/sharing/shares/shares.go | 61 ++++++------ .../owncloud/ocs/response/response.go | 6 +- pkg/utils/utils.go | 37 +++++++- pkg/utils/utils_test.go | 93 +++++++++++++++++++ 5 files changed, 173 insertions(+), 29 deletions(-) create mode 100644 changelog/unreleased/sharing-in-spaces.md diff --git a/changelog/unreleased/sharing-in-spaces.md b/changelog/unreleased/sharing-in-spaces.md new file mode 100644 index 00000000000..7a39a616508 --- /dev/null +++ b/changelog/unreleased/sharing-in-spaces.md @@ -0,0 +1,5 @@ +Enhancement: Allow listing shares in spaces via the OCS API + +Added a `space_ref` parameter to the list shares endpoints so that one can list shares inside of spaces. + +https://github.com/cs3org/reva/pull/2622 diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index 0b0b29264c7..2ebe439b220 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -68,6 +68,10 @@ const ( storageIDPrefix string = "shared::" ) +var ( + errParsingSpaceReference = errors.New("could not parse space reference") +) + // Handler implements the shares part of the ownCloud sharing API type Handler struct { gatewayAddr string @@ -207,19 +211,16 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) { } ref, err := h.extractReference(r) if err != nil { - response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, "could not parse the reference", fmt.Errorf("could not parse the reference")) + response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, errParsingSpaceReference.Error(), errParsingSpaceReference) return } - statReq := provider.StatRequest{ - Ref: &ref, - } - sublog := appctx.GetLogger(ctx).With().Interface("ref", ref).Logger() + statReq := provider.StatRequest{Ref: &ref} statRes, err := client.Stat(ctx, &statReq) if err != nil { - sublog.Debug().Err(err).Str("createShare", "shares").Msg("error on stat call") + sublog.Debug().Err(err).Msg("CreateShare: error on stat call") response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "missing resource information", fmt.Errorf("error getting resource information")) return } @@ -233,7 +234,7 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) { response.WriteOCSError(w, r, http.StatusNotFound, "No share permission", nil) w.WriteHeader(http.StatusForbidden) default: - log.Error().Interface("status", statRes.Status).Msg("grpc request failed") + log.Error().Interface("status", statRes.Status).Msg("CreateShare: stat failed") w.WriteHeader(http.StatusInternalServerError) } return @@ -738,13 +739,17 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { var pinfo *provider.ResourceInfo p := r.URL.Query().Get("path") + shareRef := r.URL.Query().Get("share_ref") // we need to lookup the resource id so we can filter the list of shares later - if p != "" { - // prefix the path with the owners home, because ocs share requests are relative to the home dir - target := path.Join(h.getHomeNamespace(ctxpkg.ContextMustGetUser(ctx)), r.FormValue("path")) + if p != "" || shareRef != "" { + ref, err := h.extractReference(r) + if err != nil { + response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, errParsingSpaceReference.Error(), errParsingSpaceReference) + return + } var status *rpc.Status - pinfo, status, err = h.getResourceInfoByPath(ctx, client, target) + pinfo, status, err = h.getResourceInfoByReference(ctx, client, &ref) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error sending a grpc stat request", err) return @@ -917,9 +922,14 @@ func (h *Handler) listSharesWithOthers(w http.ResponseWriter, r *http.Request) { // shared with others p := r.URL.Query().Get("path") - if p != "" { - // prefix the path with the owners home, because ocs share requests are relative to the home dir - filters, linkFilters, e = h.addFilters(w, r, h.getHomeNamespace(ctxpkg.ContextMustGetUser(r.Context()))) + spaceRef := r.URL.Query().Get("space_ref") + if p != "" || spaceRef != "" { + ref, err := h.extractReference(r) + if err != nil { + response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, errParsingSpaceReference.Error(), errParsingSpaceReference) + return + } + filters, linkFilters, e = h.addFilters(w, r, &ref) if e != nil { // result has been written as part of addFilters return @@ -990,9 +1000,7 @@ func (h *Handler) logProblems(s *rpc.Status, e error, msg string) { } } -func (h *Handler) addFilters(w http.ResponseWriter, r *http.Request, prefix string) ([]*collaboration.Filter, []*link.ListPublicSharesRequest_Filter, error) { - collaborationFilters := []*collaboration.Filter{} - linkFilters := []*link.ListPublicSharesRequest_Filter{} +func (h *Handler) addFilters(w http.ResponseWriter, r *http.Request, ref *provider.Reference) ([]*collaboration.Filter, []*link.ListPublicSharesRequest_Filter, error) { ctx := r.Context() // first check if the file exists @@ -1002,8 +1010,7 @@ func (h *Handler) addFilters(w http.ResponseWriter, r *http.Request, prefix stri return nil, nil, err } - target := path.Join(prefix, r.FormValue("path")) - info, status, err := h.getResourceInfoByPath(ctx, client, target) + info, status, err := h.getResourceInfoByReference(ctx, client, ref) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error sending a grpc stat request", err) return nil, nil, err @@ -1019,9 +1026,8 @@ func (h *Handler) addFilters(w http.ResponseWriter, r *http.Request, prefix stri return nil, nil, err } - collaborationFilters = append(collaborationFilters, share.ResourceIDFilter(info.Id)) - - linkFilters = append(linkFilters, publicshare.ResourceIDFilter(info.Id)) + collaborationFilters := []*collaboration.Filter{share.ResourceIDFilter(info.Id)} + linkFilters := []*link.ListPublicSharesRequest_Filter{publicshare.ResourceIDFilter(info.Id)} return collaborationFilters, linkFilters, nil } @@ -1232,11 +1238,12 @@ func (h *Handler) getAdditionalInfoAttribute(ctx context.Context, u *userIdentif return buf.String() } -func (h *Handler) getResourceInfoByPath(ctx context.Context, client GatewayClient, path string) (*provider.ResourceInfo, *rpc.Status, error) { - return h.getResourceInfo(ctx, client, path, &provider.Reference{ - // FIXME ResourceId? - Path: path, - }) +func (h *Handler) getResourceInfoByReference(ctx context.Context, client GatewayClient, ref *provider.Reference) (*provider.ResourceInfo, *rpc.Status, error) { + key, err := utils.FormatStorageSpaceReference(ref) + if err != nil { + return nil, nil, err + } + return h.getResourceInfo(ctx, client, key, ref) } func (h *Handler) getResourceInfoByID(ctx context.Context, client GatewayClient, id *provider.ResourceId) (*provider.ResourceInfo, *rpc.Status, error) { diff --git a/internal/http/services/owncloud/ocs/response/response.go b/internal/http/services/owncloud/ocs/response/response.go index 534bf5eb064..97ac1c27738 100644 --- a/internal/http/services/owncloud/ocs/response/response.go +++ b/internal/http/services/owncloud/ocs/response/response.go @@ -150,7 +150,11 @@ func WriteOCSData(w http.ResponseWriter, r *http.Request, m Meta, d interface{}, // WriteOCSResponse handles writing ocs responses in json and xml func WriteOCSResponse(w http.ResponseWriter, r *http.Request, res Response, err error) { if err != nil { - appctx.GetLogger(r.Context()).Error().Err(err).Msg(res.OCS.Meta.Message) + appctx.GetLogger(r.Context()). + Debug(). + Err(err). + Str("ocs_msg", res.OCS.Meta.Message). + Msg("writing ocs error response") } version := APIVersion(r.Context()) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 32b69cfd2e0..c7187a09db7 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -42,6 +42,10 @@ import ( "google.golang.org/protobuf/encoding/protojson" ) +const ( + spaceIDDelimiter = "!" +) + var ( matchFirstCap = regexp.MustCompile("(.)([A-Z][a-z]+)") matchAllCap = regexp.MustCompile("([a-z0-9])([A-Z])") @@ -58,6 +62,8 @@ var ( // SpaceGrant is used to signal the storageprovider that the grant is on a space SpaceGrant struct{} + + errInvalidSpaceReference = errors.New("invalid storage space reference") ) // Skip evaluates whether a source endpoint contains any of the prefixes. @@ -326,7 +332,7 @@ func SplitStorageSpaceID(ssid string) (storageid, nodeid string, err error) { if ssid == "" { return "", "", errors.New("can't split empty StorageSpaceID") } - parts := strings.SplitN(ssid, "!", 2) + parts := strings.SplitN(ssid, spaceIDDelimiter, 2) if len(parts) == 1 { return parts[0], parts[0], nil } @@ -357,6 +363,35 @@ func ParseStorageSpaceReference(sRef string) (provider.Reference, error) { }, nil } +// FormatStorageSpaceReference will format a storage space reference into a string representation. +// If ref or ref.ResourceId are nil an empty string will be returned. +// The function doesn't check if all values are set. +// The resulting format can be: +// +// "storage_id!opaque_id" +// "storage_id!opaque_id/path" +// "storage_id/path" +// "storage_id" +func FormatStorageSpaceReference(ref *provider.Reference) (string, error) { + if ref == nil || ref.ResourceId == nil || ref.ResourceId.StorageId == "" { + return "", errInvalidSpaceReference + } + var ssid string + if ref.ResourceId.OpaqueId == "" { + + ssid = ref.ResourceId.StorageId + } else { + var sb strings.Builder + // ssid == storage_id!opaque_id + sb.Grow(len(ref.ResourceId.StorageId) + len(ref.ResourceId.OpaqueId) + 1) + sb.WriteString(ref.ResourceId.StorageId) + sb.WriteString(spaceIDDelimiter) + sb.WriteString(ref.ResourceId.OpaqueId) + ssid = sb.String() + } + return path.Join(ssid, ref.Path), nil +} + // GetViewMode converts a human-readable string to a view mode for opening a resource in an app. func GetViewMode(viewMode string) gateway.OpenInAppRequest_ViewMode { switch viewMode { diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 29bbf661124..d8caa7dedc8 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -19,6 +19,7 @@ package utils import ( + "errors" "testing" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -207,3 +208,95 @@ func TestParseStorageSpaceReference(t *testing.T) { } } } + +func TestFormatStorageSpaceReference(t *testing.T) { + tests := []struct { + ref *provider.Reference + expected string + expectedErr error + }{ + { + ref: &provider.Reference{}, + expected: "", + expectedErr: errInvalidSpaceReference, + }, + { + ref: &provider.Reference{ResourceId: &provider.ResourceId{}}, + expected: "!", + expectedErr: errInvalidSpaceReference, + }, + { + ref: &provider.Reference{ResourceId: &provider.ResourceId{OpaqueId: "opaqueid"}, Path: "path"}, + expectedErr: errInvalidSpaceReference, + }, + { + ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}, Path: "path"}, + expected: "storageid/path", + }, + { + ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "path"}, + expected: "storageid!opaqueid/path", + }, + { + ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "."}, + expected: "storageid!opaqueid", + }, + { + ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}, Path: "."}, + expected: "storageid", + }, + { + ref: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}}, + expected: "storageid", + }, + } + + for _, tt := range tests { + result, err := FormatStorageSpaceReference(tt.ref) + if err != nil && !errors.Is(err, tt.expectedErr) { + t.Errorf("FormateStorageSpaceRefence returned unexpected error: %v", err) + } + if err == nil && result != tt.expected { + t.Errorf("Reference %v got formatted to %s, expected %s", tt.ref, result, tt.expected) + } + } +} + +func TestFormatAndParseReference(t *testing.T) { + tests := []struct { + orig *provider.Reference + expected *provider.Reference + }{ + { + orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}, Path: "./path"}, + expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "storageid"}, Path: "./path"}, + }, + { + orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "./path"}, + expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "./path"}, + }, + { + orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "."}, + expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, Path: "."}, + }, + { + orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}, Path: "."}, + expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "storageid"}, Path: "."}, + }, + { + orig: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid"}}, + expected: &provider.Reference{ResourceId: &provider.ResourceId{StorageId: "storageid", OpaqueId: "storageid"}, Path: "."}, + }, + } + + for _, tt := range tests { + formatted, _ := FormatStorageSpaceReference(tt.orig) + parsed, err := ParseStorageSpaceReference(formatted) + if err != nil { + t.Errorf("failed to parse space reference: %s error: %s", formatted, err) + } + if !(ResourceIDEqual(parsed.ResourceId, tt.expected.ResourceId) && parsed.Path == tt.expected.Path) { + t.Errorf("Formatted then parsed references don't match the original got: %v expected %v", parsed, tt.expected) + } + } +}