implement listing shares in spaces

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

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,19 @@ 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) {
+	var key string
+	if ref.ResourceId == nil {
+		// This is a path based reference
+		key = ref.Path
+	} else {
+		var err 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) {

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())

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 error 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 {

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)
+		}
+	}
+}

tests/oc-integration-tests/local/ldap-users.toml

@@ -16,7 +16,7 @@ address = "0.0.0.0:18000"
 auth_manager = "ldap"
 
 [grpc.services.authprovider.auth_managers.ldap]
-hostname="ldap"
+hostname="localhost"
 port=636
 insecure=true
 base_dn="dc=owncloud,dc=com"
@@ -34,7 +34,7 @@ cn="cn"
 driver = "ldap"
 
 [grpc.services.userprovider.drivers.ldap]
-hostname="ldap"
+hostname="localhost"
 port=636
 insecure=true
 base_dn="dc=owncloud,dc=com"
@@ -57,7 +57,7 @@ gid="entryuuid"
 driver = "ldap"
 
 [grpc.services.groupprovider.drivers.ldap]
-hostname="ldap"
+hostname="localhost"
 port=636
 insecure=true
 base_dn="dc=owncloud,dc=com"