From c7da81555f18ee0d227e59addf889cbf3ca0ae6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 17 Jun 2021 11:04:37 +0000 Subject: [PATCH] do not show shared resources as spaces for owners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../utils/decomposedfs/decomposedfs.go | 47 +++++++++---------- pkg/storage/utils/decomposedfs/node/node.go | 17 ++----- .../utils/decomposedfs/node/permissions.go | 5 +- pkg/utils/utils.go | 12 +++++ 4 files changed, 41 insertions(+), 40 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 542b4e5ba50..8607590d644 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/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// pointing to ../../nodes/, 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// -> ../../nodes/ 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/ symlinks - // - it iterates over all /nodes/ entries to create /spaces/shares/ symlinks - // - should be good enough to iterate over the /nodes/ 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/ 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,6 +576,10 @@ 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 { @@ -598,8 +587,18 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide // 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 diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 1707d9abc95..7182199d0b3 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/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 diff --git a/pkg/storage/utils/decomposedfs/node/permissions.go b/pkg/storage/utils/decomposedfs/node/permissions.go index ea3e5cae9dc..d66ec190e23 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/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 } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 92e4d6a4f9a..d41f14813c3 100644 --- a/pkg/utils/utils.go +++ b/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 + } +}