Skip to content

Prevent space admin from listing other tennants spaces #412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dragonchaser merged 1 commit into from
Nov 13, 2025
Merged

Prevent space admin from listing other tennants spaces #412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions internal/grpc/services/storageprovider/storageprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ func (s *Service) CreateStorageSpace(ctx context.Context, req *provider.CreateSt
st = status.NewPermissionDenied(ctx, err, "permission denied")
case errtypes.NotSupported:
// if trying to create a user home fall back to CreateHome
if u, ok := ctxpkg.ContextGetUser(ctx); ok && req.Type == "personal" && utils.UserEqual(req.GetOwner().Id, u.Id) {
if u, ok := ctxpkg.ContextGetUser(ctx); ok && req.Type == "personal" && utils.UserEqual(req.GetOwner().GetId(), u.GetId()) {
if err := s.Storage.CreateHome(ctx); err != nil {
st = status.NewInternal(ctx, "error creating home")
} else {
Expand Down Expand Up @@ -583,7 +583,7 @@ func (s *Service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
}

for _, sp := range spaces {
if sp.Id == nil || sp.Id.OpaqueId == "" {
if sp.GetId() == nil || sp.GetId().GetOpaqueId() == "" {
log.Error().Str("service", "storageprovider").Str("driver", s.conf.Driver).Interface("space", sp).Msg("space is missing space id and root id")
continue
}
Expand Down Expand Up @@ -614,7 +614,7 @@ func (s *Service) UpdateStorageSpace(ctx context.Context, req *provider.UpdateSt
func (s *Service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) (*provider.DeleteStorageSpaceResponse, error) {
// we need to get the space before so we can return critical information
// FIXME: why is this string parsing necessary?
idraw, _ := storagespace.ParseID(req.Id.GetOpaqueId())
idraw, _ := storagespace.ParseID(req.GetId().GetOpaqueId())
idraw.OpaqueId = idraw.GetSpaceId()
id := &provider.StorageSpaceId{OpaqueId: storagespace.FormatResourceID(&idraw)}

Expand All @@ -629,7 +629,7 @@ func (s *Service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteSt
case errtypes.BadRequest:
st = status.NewInvalid(ctx, err.Error())
default:
st = status.NewInternal(ctx, "error deleting space: "+req.Id.String())
st = status.NewInternal(ctx, "error deleting space: "+req.GetId().String())
}
return &provider.DeleteStorageSpaceResponse{
Status: st,
Expand All @@ -650,13 +650,13 @@ func (s *Service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteSt
case errtypes.BadRequest:
st = status.NewInvalid(ctx, err.Error())
default:
st = status.NewInternal(ctx, "error deleting space: "+req.Id.String())
st = status.NewInternal(ctx, "error deleting space: "+req.GetId().String())
}
appctx.GetLogger(ctx).
Error().
Err(err).
Interface("status", st).
Interface("storage_space_id", req.Id).
Interface("storage_space_id", req.GetId()).
Msg("failed to delete storage space")
return &provider.DeleteStorageSpaceResponse{
Status: st,
Expand Down Expand Up @@ -751,7 +751,7 @@ func (s *Service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro
Status: status.NewStatusFromErrType(ctx, "delete", err),
Opaque: &typesv1beta1.Opaque{
Map: map[string]*typesv1beta1.OpaqueEntry{
"opaque_id": {Decoder: "plain", Value: []byte(md.Id.OpaqueId)},
"opaque_id": {Decoder: "plain", Value: []byte(md.GetId().GetOpaqueId())},
},
},
}, nil
Expand Down
45 changes: 25 additions & 20 deletions pkg/storage/pkg/decomposedfs/spaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr
err = fs.updateIndexes(ctx, &provider.Grantee{
Type: provider.GranteeType_GRANTEE_TYPE_USER,
Id: &provider.Grantee_UserId{UserId: req.GetOwner().GetId()},
}, req.Type, root.ID, root.ID)
}, req.Type, root.GetID(), root.GetID())
if err != nil {
return nil, err
}
Expand All @@ -221,7 +221,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr
Grantee: &provider.Grantee{
Type: provider.GranteeType_GRANTEE_TYPE_USER,
Id: &provider.Grantee_UserId{
UserId: u.Id,
UserId: u.GetId(),
},
},
Permissions: ocsconv.NewManagerRole().CS3ResourcePermissions(),
Expand Down Expand Up @@ -295,7 +295,8 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
spaceTypes[spaceTypeAny] = struct{}{}
}

authenticatedUserID := ctxpkg.ContextMustGetUser(ctx).GetId().GetOpaqueId()
authenticatedUser := ctxpkg.ContextMustGetUser(ctx)
authenticatedUserID := authenticatedUser.GetId().GetOpaqueId()

if !fs.p.ListSpacesOfUser(ctx, requestedUserID) {
return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list spaces of other users", authenticatedUserID))
Expand Down Expand Up @@ -500,10 +501,14 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
}()

for r := range results {
if authenticatedUser.GetId().GetTenantId() != r.GetOwner().GetId().GetTenantId() {
appctx.GetLogger(ctx).Warn().Str("id", r.GetId().GetOpaqueId()).Msg("space belongs to a different tenant")
continue
}
r.HasTrashedItems = true
resourceID, err := storagespace.ParseID(r.GetId().GetOpaqueId())
if err != nil {
appctx.GetLogger(ctx).Error().Err(err).Str("id", r.Id.GetOpaqueId()).Msg("could not parse space id")
appctx.GetLogger(ctx).Error().Err(err).Str("id", r.GetId().GetOpaqueId()).Msg("could not parse space id")
r.HasTrashedItems = false
continue
}
Expand Down Expand Up @@ -577,7 +582,7 @@ func (fs *Decomposedfs) UpdateStorageSpace(ctx context.Context, req *provider.Up
}

space := req.StorageSpace
_, spaceID, _, _ := storagespace.SplitID(space.Id.OpaqueId)
_, spaceID, _, _ := storagespace.SplitID(space.GetId().GetOpaqueId())

metadata := make(node.Attributes, 5)
if space.Name != "" {
Expand Down Expand Up @@ -723,7 +728,7 @@ func (fs *Decomposedfs) DeleteStorageSpace(ctx context.Context, req *provider.De
_, purge = opaque.Map["purge"]
}

_, spaceID, _, err := storagespace.SplitID(req.Id.GetOpaqueId())
_, spaceID, _, err := storagespace.SplitID(req.GetId().GetOpaqueId())
if err != nil {
return err
}
Expand All @@ -735,7 +740,7 @@ func (fs *Decomposedfs) DeleteStorageSpace(ctx context.Context, req *provider.De

st, err := n.SpaceRoot.XattrString(ctx, prefixes.SpaceTypeAttr)
if err != nil {
return errtypes.InternalError(fmt.Sprintf("space %s does not have a spacetype, possible corrupt decompsedfs", n.ID))
return errtypes.InternalError(fmt.Sprintf("space %s does not have a spacetype, possible corrupt decompsedfs", n.GetID()))
}

if err := canDeleteSpace(ctx, spaceID, st, purge, n, fs.p); err != nil {
Expand Down Expand Up @@ -877,18 +882,18 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
case err != nil:
return nil, err
case !rp.Stat:
return nil, errtypes.NotFound(fmt.Sprintf("space %s not found", n.ID))
return nil, errtypes.NotFound(fmt.Sprintf("space %s not found", n.GetID()))
}

if n.SpaceRoot.IsDisabled(ctx) {
rp, err := fs.p.AssemblePermissions(ctx, n)
if err != nil || !permissions.IsManager(rp) {
return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list deleted spaces %s", user.Username, n.ID))
return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list deleted spaces %s", user.Username, n.GetID()))
}
}
}

sublog := appctx.GetLogger(ctx).With().Str("spaceid", n.SpaceID).Logger()
sublog := appctx.GetLogger(ctx).With().Str("spaceid", n.GetSpaceID()).Logger()

var err error
// TODO apply more filters
Expand Down Expand Up @@ -939,13 +944,13 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
switch {
case g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER:
// remove from user index
if err := fs.userSpaceIndex.Remove(g.Grantee.GetUserId().GetOpaqueId(), n.SpaceID); err != nil {
if err := fs.userSpaceIndex.Remove(g.Grantee.GetUserId().GetOpaqueId(), n.GetSpaceID()); err != nil {
sublog.Error().Err(err).Str("grantee", id).
Msg("failed to delete expired user space index")
}
case g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP:
// remove from group index
if err := fs.groupSpaceIndex.Remove(g.Grantee.GetGroupId().GetOpaqueId(), n.SpaceID); err != nil {
if err := fs.groupSpaceIndex.Remove(g.Grantee.GetGroupId().GetOpaqueId(), n.GetSpaceID()); err != nil {
sublog.Error().Err(err).Str("grantee", id).
Msg("failed to delete expired group space index")
}
Expand Down Expand Up @@ -977,8 +982,8 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
ssID, err := storagespace.FormatReference(
&provider.Reference{
ResourceId: &provider.ResourceId{
SpaceId: n.SpaceRoot.SpaceID,
OpaqueId: n.SpaceRoot.ID},
SpaceId: n.SpaceRoot.GetSpaceID(),
OpaqueId: n.SpaceRoot.GetID()},
},
)
if err != nil {
Expand All @@ -1003,8 +1008,8 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
},
Id: &provider.StorageSpaceId{OpaqueId: ssID},
Root: &provider.ResourceId{
SpaceId: n.SpaceRoot.SpaceID,
OpaqueId: n.SpaceRoot.ID,
SpaceId: n.SpaceRoot.GetSpaceID(),
OpaqueId: n.SpaceRoot.GetID(),
},
Name: sname,
// SpaceType is read from xattr below
Expand Down Expand Up @@ -1046,7 +1051,7 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
}
}

etag, err := node.CalculateEtag(n.ID, tmtime)
etag, err := node.CalculateEtag(n.GetID(), tmtime)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1075,15 +1080,15 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
}
if si := spaceAttributes.String(prefixes.SpaceImageAttr); si != "" {
space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "image", storagespace.FormatResourceID(
&provider.ResourceId{StorageId: space.Root.StorageId, SpaceId: space.Root.SpaceId, OpaqueId: si},
&provider.ResourceId{StorageId: space.Root.StorageId, SpaceId: space.GetRoot().GetSpaceId(), OpaqueId: si},
))
}
if sd := spaceAttributes.String(prefixes.SpaceDescriptionAttr); sd != "" {
space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "description", sd)
}
if sr := spaceAttributes.String(prefixes.SpaceReadmeAttr); sr != "" {
space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "readme", storagespace.FormatResourceID(
&provider.ResourceId{StorageId: space.Root.StorageId, SpaceId: space.Root.SpaceId, OpaqueId: sr},
&provider.ResourceId{StorageId: space.Root.StorageId, SpaceId: space.GetRoot().GetSpaceId(), OpaqueId: sr},
))
}
if sa := spaceAttributes.String(prefixes.SpaceAliasAttr); sa != "" {
Expand Down Expand Up @@ -1157,5 +1162,5 @@ func canDeleteSpace(ctx context.Context, spaceID string, typ string, purge bool,
return nil
}

return errtypes.PermissionDenied(fmt.Sprintf("user is not allowed to delete space %s", n.ID))
return errtypes.PermissionDenied(fmt.Sprintf("user is not allowed to delete space %s", n.GetID()))
}