do not show shared resources as spaces for owners

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -497,21 +497,15 @@ func (fs *Decomposedfs) Download(ctx context.Context, ref *provider.Reference) (
 
 // ListStorageSpaces returns a list of StorageSpaces.
 // The list can be filtered by space type or space id.
+// Spaces are persisted with symlinks in /spaces/<type>/<spaceid> pointing to ../../nodes/<nodeid>, the root node of the space
+// The spaceid is a concatenation of storageid + "!" + nodeid
 func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) {
 	// TODO check filters
 
-	// for, now list all user homes
-	// TODO make a dedicated /spaces subfolder in the storage root, next to /nodes, /blobs and /trash
-	// it should follow /spaces/<type>/<spaceid> -> ../../nodes/<nodeid> and point to the root node of the space
-	// needs a migration step that checks if the /spaces folder exists, if not
-	// - it iterates over the /nodes/root folder to create /spaces/personal/<spaceid> symlinks
-	// - it iterates over all /nodes/<uuid> entries to create /spaces/shares/<spaceid> symlinks
-	//  - should be good enough to iterate over the /nodes/<uuid> entries, because the ext attrs should indicate personal spaces or share spaces
-	// when the space symlink is broken delete the space? yes
+	// TODO when a space symlink is broken delete the space for cleanup
 	// read permissions are deduced from the node?
-	// the spaceid can be the nodeid
 
-	// this actually requires us to move all user homes into a subfolder of /nodes/root,
+	// TODO for absolute references this actually requires us to move all user homes into a subfolder of /nodes/root,
 	// e.g. /nodes/root/<space type> otherwise storage space names might collide even though they are of different types
 	// /nodes/root/personal/foo and /nodes/root/shares/foo might be two very different spaces, a /nodes/root/foo is not expressive enough
 	// we would not need /nodes/root if access always happened via spaceid+relative path
@@ -531,8 +525,9 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 		}
 	}
 
-	// /var/lib/ocis/storage/users/spaces/personal/nodeid
-	// /var/lib/ocis/storage/users/spaces/shared/nodeid
+	// build the glob path, eg.
+	// /path/to/root/spaces/personal/nodeid
+	// /path/to/root/spaces/shared/nodeid
 	matches, err := filepath.Glob(filepath.Join(fs.o.Root, "spaces", spaceType, spaceID))
 	if err != nil {
 		return nil, err
@@ -563,17 +558,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 				continue
 			}
 
-			// filter out spaces user cannot access (currently based on stat permission)
-			p, err := n.ReadUserPermissions(ctx, u)
-			if err != nil {
-				appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not read permissions, skipping")
-				continue
-			}
-			if !p.Stat {
-				continue
-			}
-
-			// TODO apply filter
+			// TODO apply more filters
 
 			// build return value
 
@@ -591,15 +576,29 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 			}
 
 			if space.SpaceType == "share" {
+				if utils.IsSameUserID(u.Id, owner) {
+					// do not list shares as spaces for the owner
+					continue
+				}
 				// return folder name?
 				space.Name = n.Name
 			} else {
 				space.Name = "root" // do not expose the id as name, this is the root of a space
 				// TODO read from extended attribute for project / group spaces
 			}
 
+			// filter out spaces user cannot access (currently based on stat permission)
+			p, err := n.ReadUserPermissions(ctx, u)
+			if err != nil {
+				appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not read permissions, skipping")
+				continue
+			}
+			if !p.Stat {
+				continue
+			}
+
 			// fill in user object if the current user is the owner
-			if owner.Idp == u.Id.Idp && owner.OpaqueId == u.Id.OpaqueId {
+			if utils.IsSameUserID(u.Id, owner) {
 				space.Owner = u
 			} else {
 				space.Owner = &userv1beta1.User{ // FIXME only return a UserID, not a full blown user object

pkg/storage/utils/decomposedfs/node/node.go

@@ -47,6 +47,7 @@ import (
 	"github.com/cs3org/reva/pkg/storage/utils/ace"
 	"github.com/cs3org/reva/pkg/storage/utils/decomposedfs/xattrs"
 	"github.com/cs3org/reva/pkg/user"
+	"github.com/cs3org/reva/pkg/utils"
 )
 
 // Define keys and values used in the node metadata
@@ -303,7 +304,7 @@ func (n *Node) PermissionSet(ctx context.Context) *provider.ResourcePermissions
 		appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("no user in context, returning default permissions")
 		return NoPermissions
 	}
-	if o, _ := n.Owner(); isSameUserID(u.Id, o) {
+	if o, _ := n.Owner(); utils.IsSameUserID(u.Id, o) {
 		return OwnerPermissions
 	}
 	// read the permissions for the current user from the acls of the current node
@@ -732,7 +733,7 @@ func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap *pro
 		// TODO what if no owner is set but grants are present?
 		return NoOwnerPermissions, nil
 	}
-	if isSameUserID(u.Id, o) {
+	if utils.IsSameUserID(u.Id, o) {
 		appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("user is owner, returning owner permissions")
 		return OwnerPermissions, nil
 	}
@@ -855,18 +856,6 @@ func (n *Node) hasUserShares(ctx context.Context) bool {
 	return false
 }
 
-// TODO make public in a comparison package
-func isSameUserID(i *userpb.UserId, j *userpb.UserId) bool {
-	switch {
-	case i == nil, j == nil:
-		return false
-	case i.OpaqueId == j.OpaqueId && i.Idp == j.Idp:
-		return true
-	default:
-		return false
-	}
-}
-
 func parseMTime(v string) (t time.Time, err error) {
 	p := strings.SplitN(v, ".", 2)
 	var sec, nsec int64

pkg/storage/utils/decomposedfs/node/permissions.go

@@ -28,6 +28,7 @@ import (
 	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/storage/utils/decomposedfs/xattrs"
 	"github.com/cs3org/reva/pkg/user"
+	"github.com/cs3org/reva/pkg/utils"
 	"github.com/pkg/errors"
 	"github.com/pkg/xattr"
 )
@@ -94,7 +95,7 @@ func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap *pro
 		// TODO what if no owner is set but grants are present?
 		return NoOwnerPermissions, nil
 	}
-	if isSameUserID(u.Id, o) {
+	if utils.IsSameUserID(u.Id, o) {
 		appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("user is owner, returning owner permissions")
 		return OwnerPermissions, nil
 	}
@@ -253,7 +254,7 @@ func (p *Permissions) getUserAndPermissions(ctx context.Context, n *Node) (*user
 		// TODO what if no owner is set but grants are present?
 		return nil, NoOwnerPermissions
 	}
-	if isSameUserID(u.Id, o) {
+	if utils.IsSameUserID(u.Id, o) {
 		appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("user is owner, returning owner permissions")
 		return u, OwnerPermissions
 	}

pkg/utils/utils.go

@@ -222,3 +222,15 @@ func MakeRelativePath(p string) string {
 		return "./" + p
 	}
 }
+
+// TODO move to a comparison package
+func IsSameUserID(i *userpb.UserId, j *userpb.UserId) bool {
+	switch {
+	case i == nil, j == nil:
+		return false
+	case i.OpaqueId == j.OpaqueId && i.Idp == j.Idp:
+		return true
+	default:
+		return false
+	}
+}