diff --git a/changelog/unreleased/prevent-editing-disabled-spaces.md b/changelog/unreleased/prevent-editing-disabled-spaces.md new file mode 100644 index 0000000000..78cf5ad9ce --- /dev/null +++ b/changelog/unreleased/prevent-editing-disabled-spaces.md @@ -0,0 +1,5 @@ +Change: Do not allow to edit disabled spaces + +Previously managers could still upload to disabled spaces. This is now forbidden + +https://github.com/cs3org/reva/pull/2856 diff --git a/pkg/storage/utils/decomposedfs/lookup/lookup.go b/pkg/storage/utils/decomposedfs/lookup/lookup.go index c233d7f36f..9acd721e61 100644 --- a/pkg/storage/utils/decomposedfs/lookup/lookup.go +++ b/pkg/storage/utils/decomposedfs/lookup/lookup.go @@ -74,7 +74,7 @@ func (lu *Lookup) NodeFromID(ctx context.Context, id *provider.ResourceId) (n *n // The Resource references the root of a space return lu.NodeFromSpaceID(ctx, id) } - return node.ReadNode(ctx, lu, id.StorageId, id.OpaqueId) + return node.ReadNode(ctx, lu, id.StorageId, id.OpaqueId, false) } // Pathify segments the beginning of a string into depth segments of width length @@ -95,7 +95,7 @@ func Pathify(id string, depth, width int) string { // NodeFromSpaceID converts a resource id without an opaque id into a Node func (lu *Lookup) NodeFromSpaceID(ctx context.Context, id *provider.ResourceId) (n *node.Node, err error) { - node, err := node.ReadNode(ctx, lu, id.StorageId, id.StorageId) + node, err := node.ReadNode(ctx, lu, id.StorageId, id.StorageId, false) if err != nil { return nil, err } diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 137f029a99..20f6bda094 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -183,8 +183,7 @@ func (n *Node) WriteOwner(owner *userpb.UserId) error { } // ReadNode creates a new instance from an id and checks if it exists -// FIXME check if user is allowed to access disabled spaces -func ReadNode(ctx context.Context, lu PathLookup, spaceID, nodeID string) (n *Node, err error) { +func ReadNode(ctx context.Context, lu PathLookup, spaceID, nodeID string, canListDisabledSpace bool) (n *Node, err error) { // read space root r := &Node{ @@ -202,6 +201,11 @@ func ReadNode(ctx context.Context, lu PathLookup, spaceID, nodeID string) (n *No } r.Exists = true + if !canListDisabledSpace && r.IsDisabled() { + // no permission = not found + return nil, errtypes.NotFound(spaceID) + } + // check if this is a space root if spaceID == nodeID { return r, nil @@ -336,7 +340,7 @@ func (n *Node) Child(ctx context.Context, name string) (*Node, error) { } var c *Node - c, err = ReadNode(ctx, n.lu, spaceID, nodeID) + c, err = ReadNode(ctx, n.lu, spaceID, nodeID, false) if err != nil { return nil, errors.Wrap(err, "could not read child node") } diff --git a/pkg/storage/utils/decomposedfs/node/node_test.go b/pkg/storage/utils/decomposedfs/node/node_test.go index c653ba6b26..cfae71aadb 100644 --- a/pkg/storage/utils/decomposedfs/node/node_test.go +++ b/pkg/storage/utils/decomposedfs/node/node_test.go @@ -71,7 +71,7 @@ var _ = Describe("Node", func() { }) Expect(err).ToNot(HaveOccurred()) - n, err := node.ReadNode(env.Ctx, env.Lookup, lookupNode.SpaceID, lookupNode.ID) + n, err := node.ReadNode(env.Ctx, env.Lookup, lookupNode.SpaceID, lookupNode.ID, false) Expect(err).ToNot(HaveOccurred()) Expect(n.BlobID).To(Equal("file1-blobid")) }) diff --git a/pkg/storage/utils/decomposedfs/revisions.go b/pkg/storage/utils/decomposedfs/revisions.go index b95427b58d..37cd90fa56 100644 --- a/pkg/storage/utils/decomposedfs/revisions.go +++ b/pkg/storage/utils/decomposedfs/revisions.go @@ -109,7 +109,7 @@ func (fs *Decomposedfs) DownloadRevision(ctx context.Context, ref *provider.Refe spaceID := ref.ResourceId.OpaqueId // check if the node is available and has not been deleted - n, err := node.ReadNode(ctx, fs.lu, spaceID, kp[0]) + n, err := node.ReadNode(ctx, fs.lu, spaceID, kp[0], false) if err != nil { return nil, err } @@ -154,7 +154,7 @@ func (fs *Decomposedfs) RestoreRevision(ctx context.Context, ref *provider.Refer spaceID := ref.ResourceId.StorageId // check if the node is available and has not been deleted - n, err := node.ReadNode(ctx, fs.lu, spaceID, kp[0]) + n, err := node.ReadNode(ctx, fs.lu, spaceID, kp[0], false) if err != nil { return err } diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index 51d0de47b0..99e4a71e9e 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -81,7 +81,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr alias = templates.WithSpacePropertiesAndUser(u, req.Type, req.Name, fs.o.PersonalSpaceAliasTemplate) } - root, err := node.ReadNode(ctx, fs.lu, spaceID, spaceID) + root, err := node.ReadNode(ctx, fs.lu, spaceID, spaceID, true) // will fall into `Exists` case below if err == nil && root.Exists { return nil, errtypes.AlreadyExists("decomposedfs: spaces: space already exists") } @@ -286,7 +286,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide if spaceID != spaceIDAny && nodeID != spaceIDAny { // try directly reading the node - n, err := node.ReadNode(ctx, fs.lu, spaceID, nodeID) + n, err := node.ReadNode(ctx, fs.lu, spaceID, nodeID, true) // permission to read disabled space is checked later if err != nil { appctx.GetLogger(ctx).Error().Err(err).Str("id", nodeID).Msg("could not read node") return nil, err @@ -345,7 +345,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide continue } - n, err := node.ReadNode(ctx, fs.lu, spaceID, nodeID) + n, err := node.ReadNode(ctx, fs.lu, spaceID, nodeID, true) if err != nil { appctx.GetLogger(ctx).Error().Err(err).Str("id", nodeID).Msg("could not read node, skipping") continue @@ -378,7 +378,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide // if there are no matches (or they happened to be spaces for the owner) and the node is a child return a space if len(matches) <= numShares && nodeID != spaceID { // try node id - n, err := node.ReadNode(ctx, fs.lu, spaceID, nodeID) + n, err := node.ReadNode(ctx, fs.lu, spaceID, nodeID, true) // permission to read disabled space is checked in storageSpaceFromNode if err != nil { return nil, err } @@ -405,7 +405,7 @@ func (fs *Decomposedfs) UpdateStorageSpace(ctx context.Context, req *provider.Up space := req.StorageSpace spaceID, _, _ := storagespace.SplitID(space.Id.OpaqueId) - node, err := node.ReadNode(ctx, fs.lu, spaceID, spaceID) + node, err := node.ReadNode(ctx, fs.lu, spaceID, spaceID, true) // permission to read disabled space will be checked later if err != nil { return nil, err } @@ -506,7 +506,7 @@ func (fs *Decomposedfs) DeleteStorageSpace(ctx context.Context, req *provider.De spaceID := req.Id.OpaqueId - n, err := node.ReadNode(ctx, fs.lu, spaceID, spaceID) + n, err := node.ReadNode(ctx, fs.lu, spaceID, spaceID, true) // permission to read disabled space is checked later if err != nil { return err } diff --git a/pkg/storage/utils/decomposedfs/testhelpers/helpers.go b/pkg/storage/utils/decomposedfs/testhelpers/helpers.go index 9581766619..3a23cb1536 100644 --- a/pkg/storage/utils/decomposedfs/testhelpers/helpers.go +++ b/pkg/storage/utils/decomposedfs/testhelpers/helpers.go @@ -205,7 +205,7 @@ func (t *TestEnv) CreateTestStorageSpace(typ string, quota *providerv1beta1.Quot ref := buildRef(space.StorageSpace.Id.OpaqueId, "") // the space name attribute is the stop condition in the lookup - h, err := node.ReadNode(t.Ctx, t.Lookup, space.StorageSpace.Id.OpaqueId, space.StorageSpace.Id.OpaqueId) + h, err := node.ReadNode(t.Ctx, t.Lookup, space.StorageSpace.Id.OpaqueId, space.StorageSpace.Id.OpaqueId, false) if err != nil { return nil, err } diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 6ca62d0927..d239275ce1 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -371,7 +371,7 @@ func (t *Tree) ListFolder(ctx context.Context, n *node.Node) ([]*node.Node, erro continue } - child, err := node.ReadNode(ctx, t.lookup, n.SpaceID, nodeID) + child, err := node.ReadNode(ctx, t.lookup, n.SpaceID, nodeID, false) if err != nil { // TODO log continue @@ -826,7 +826,7 @@ func (t *Tree) readRecycleItem(ctx context.Context, spaceID, key, path string) ( } recycleNode = node.New(spaceID, nodeID, "", "", 0, "", nil, t.lookup) - recycleNode.SpaceRoot, err = node.ReadNode(ctx, t.lookup, spaceID, spaceID) + recycleNode.SpaceRoot, err = node.ReadNode(ctx, t.lookup, spaceID, spaceID, false) if err != nil { return }