non personal spaces need virtual owner

changelog/unreleased/space-owner.md

@@ -0,0 +1,5 @@
+Bugfix: Project spaces need no real owner
+
+Make it possible to use a non existing user as a space owner.
+
+https://github.com/cs3org/reva/pull/3091

internal/grpc/services/gateway/publicshareprovider.go

@@ -21,6 +21,7 @@ package gateway
 import (
 	"context"
 
+	userprovider "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/appctx"
@@ -138,6 +139,13 @@ func (s *svc) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicShare
 	if err != nil {
 		return nil, errors.Wrap(err, "error updating share")
 	}
-	s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), res.Share.ResourceId)
+	s.cache.RemoveStat(
+		&userprovider.User{
+			Id: &userprovider.UserId{
+				OpaqueId: res.Share.Owner.GetOpaqueId(),
+			},
+		},
+		res.Share.ResourceId,
+	)
 	return res, nil
 }

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

@@ -1119,30 +1119,33 @@ func (h *Handler) addFileInfo(ctx context.Context, s *conversions.ShareData, inf
 					// TODO log error?
 					s.Path = gpRes.Path
 				}
-
-				// cut off configured home namespace, paths in ocs shares are relative to it
-				identifier := h.mustGetIdentifiers(ctx, client, info.GetOwner().GetOpaqueId(), false)
-				u := &userpb.User{
-					Id:          info.Owner,
-					Username:    identifier.Username,
-					DisplayName: identifier.DisplayName,
-					Mail:        identifier.Mail,
+				// on spaces, we could have no owner set
+				if info.Owner != nil {
+					// cut off configured home namespace, paths in ocs shares are relative to it
+					identifier := h.mustGetIdentifiers(ctx, client, info.GetOwner().GetOpaqueId(), false)
+					u := &userpb.User{
+						Id:          info.Owner,
+						Username:    identifier.Username,
+						DisplayName: identifier.DisplayName,
+						Mail:        identifier.Mail,
+					}
+					s.Path = strings.TrimPrefix(s.Path, h.getHomeNamespace(u))
 				}
-				s.Path = strings.TrimPrefix(s.Path, h.getHomeNamespace(u))
 			}
 		}
 		s.StorageID = storageIDPrefix + s.FileTarget
 		// TODO FileParent:
 		// item type
 		s.ItemType = conversions.ResourceType(info.GetType()).String()
 
+		owner := info.GetOwner()
 		// file owner might not yet be set. Use file info
-		if s.UIDFileOwner == "" {
-			s.UIDFileOwner = info.GetOwner().GetOpaqueId()
+		if s.UIDFileOwner == "" && owner != nil {
+			s.UIDFileOwner = owner.GetOpaqueId()
 		}
 		// share owner might not yet be set. Use file info
-		if s.UIDOwner == "" {
-			s.UIDOwner = info.GetOwner().GetOpaqueId()
+		if s.UIDOwner == "" && owner != nil {
+			s.UIDOwner = owner.GetOpaqueId()
 		}
 	}
 	return nil
@@ -1236,19 +1239,21 @@ func (h *Handler) mapUserIds(ctx context.Context, client gateway.GatewayAPIClien
 		if s.DisplaynameOwner == "" {
 			s.DisplaynameOwner = owner.DisplayName
 		}
-		if s.AdditionalInfoFileOwner == "" {
-			s.AdditionalInfoFileOwner = h.getAdditionalInfoAttribute(ctx, owner)
+		if s.AdditionalInfoOwner == "" {
+			s.AdditionalInfoOwner = h.getAdditionalInfoAttribute(ctx, owner)
 		}
 	}
 
 	if s.UIDFileOwner != "" {
 		fileOwner := h.mustGetIdentifiers(ctx, client, s.UIDFileOwner, false)
-		s.UIDFileOwner = fileOwner.Username
+		if fileOwner.Username != "" {
+			s.UIDFileOwner = fileOwner.Username
+		}
 		if s.DisplaynameFileOwner == "" {
 			s.DisplaynameFileOwner = fileOwner.DisplayName
 		}
-		if s.AdditionalInfoOwner == "" {
-			s.AdditionalInfoOwner = h.getAdditionalInfoAttribute(ctx, fileOwner)
+		if s.AdditionalInfoFileOwner == "" {
+			s.AdditionalInfoFileOwner = h.getAdditionalInfoAttribute(ctx, fileOwner)
 		}
 	}
 

pkg/auth/manager/publicshares/publicshares.go

@@ -127,11 +127,18 @@ func (m *manager) Authenticate(ctx context.Context, token, secret string) (*user
 		return nil, nil, errtypes.InternalError(publicShareResponse.Status.Message)
 	}
 
-	getUserResponse, err := gwConn.GetUser(ctx, &userprovider.GetUserRequest{
-		UserId: publicShareResponse.GetShare().GetCreator(),
-	})
-	if err != nil {
-		return nil, nil, err
+	var owner *user.User
+	// FIXME use new user type SPACE_OWNER
+	if publicShareResponse.GetShare().GetOwner().Type == 8 {
+		owner = &user.User{Id: publicShareResponse.GetShare().GetOwner(), DisplayName: "Public", Username: "public"}
+	} else {
+		getUserResponse, err := gwConn.GetUser(ctx, &userprovider.GetUserRequest{
+			UserId: publicShareResponse.GetShare().GetCreator(),
+		})
+		if err != nil {
+			return nil, nil, err
+		}
+		owner = getUserResponse.GetUser()
 	}
 
 	share := publicShareResponse.GetShare()
@@ -150,8 +157,7 @@ func (m *manager) Authenticate(ctx context.Context, token, secret string) (*user
 		return nil, nil, err
 	}
 
-	u := getUserResponse.GetUser()
-	u.Opaque = &types.Opaque{
+	owner.Opaque = &types.Opaque{
 		Map: map[string]*types.OpaqueEntry{
 			"public-share-role": {
 				Decoder: "plain",
@@ -160,7 +166,7 @@ func (m *manager) Authenticate(ctx context.Context, token, secret string) (*user
 		},
 	}
 
-	return u, scope, nil
+	return owner, scope, nil
 }
 
 // ErrPasswordNotProvided is returned when the public share is password protected, but there was no password on the request

pkg/publicshare/manager/cs3/cs3.go

@@ -347,11 +347,32 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters []
 	}
 
 	log := appctx.GetLogger(ctx)
+	var rID *provider.ResourceId
+	if len(filters) != 0 {
+		grouped := publicshare.GroupFiltersByType(filters)
+		for _, f := range grouped {
+			for _, f := range f {
+				if f.GetResourceId() != nil {
+					rID = f.GetResourceId()
+				}
+			}
+		}
+	}
+	var createdShareTokens []string
+	var err error
+	// in spaces, always use the resourceId
+	if rID != nil {
+		createdShareTokens, err = m.indexer.FindBy(&link.PublicShare{},
+			indexer.NewField("ResourceId", resourceIDToIndex(rID)),
+		)
+	} else {
+		// fallback for legacy use
+		createdShareTokens, err = m.indexer.FindBy(&link.PublicShare{},
+			indexer.NewField("Owner", userIDToIndex(u.Id)),
+			indexer.NewField("Creator", userIDToIndex(u.Id)),
+		)
+	}
 
-	createdShareTokens, err := m.indexer.FindBy(&link.PublicShare{},
-		indexer.NewField("Owner", userIDToIndex(u.Id)),
-		indexer.NewField("Creator", userIDToIndex(u.Id)),
-	)
 	if err != nil {
 		return nil, err
 	}

pkg/publicshare/manager/memory/memory.go

@@ -172,17 +172,13 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters []
 	shares := []*link.PublicShare{}
 	m.shares.Range(func(k, v interface{}) bool {
 		s := v.(*link.PublicShare)
-
-		// Skip if the share isn't created by the current user
-		if s.Creator.GetOpaqueId() == u.Id.OpaqueId && (s.Creator.GetIdp() == "" || u.Id.Idp == s.Creator.GetIdp()) {
-			if len(filters) == 0 {
-				shares = append(shares, s)
-			} else {
-				for _, f := range filters {
-					if f.Type == link.ListPublicSharesRequest_Filter_TYPE_RESOURCE_ID {
-						if utils.ResourceIDEqual(s.ResourceId, f.GetResourceId()) {
-							shares = append(shares, s)
-						}
+		if len(filters) == 0 {
+			shares = append(shares, s)
+		} else {
+			for _, f := range filters {
+				if f.Type == link.ListPublicSharesRequest_Filter_TYPE_RESOURCE_ID {
+					if utils.ResourceIDEqual(s.ResourceId, f.GetResourceId()) {
+						shares = append(shares, s)
 					}
 				}
 			}

pkg/storage/utils/decomposedfs/grants.go

@@ -70,7 +70,7 @@ func (fs *Decomposedfs) AddGrant(ctx context.Context, ref *provider.Reference, g
 	// When the owner is empty but grants are set then we do want to check the grants.
 	// However, if we are trying to edit an existing grant we do not have to check for permission if the user owns the grant
 	// TODO: find a better to check this
-	if !(len(grants) == 0 && (owner == nil || owner.OpaqueId == "")) {
+	if !(len(grants) == 0 && (owner == nil || owner.OpaqueId == "" || (owner.OpaqueId == node.SpaceID && owner.Type == 8))) {
 		ok, err := fs.p.HasPermission(ctx, node, func(rp *provider.ResourcePermissions) bool {
 			return rp.AddGrant
 		})

pkg/storage/utils/decomposedfs/spaces.go

@@ -99,10 +99,14 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr
 	if err := root.WriteAllNodeMetadata(); err != nil {
 		return nil, err
 	}
+	var owner *userv1beta1.UserId
 	if req.GetOwner() != nil && req.GetOwner().GetId() != nil {
-		if err := root.WriteOwner(req.GetOwner().GetId()); err != nil {
-			return nil, err
-		}
+		owner = req.GetOwner().GetId()
+	} else {
+		owner = &userv1beta1.UserId{OpaqueId: spaceID, Type: 8}
+	}
+	if err := root.WriteOwner(owner); err != nil {
+		return nil, err
 	}
 
 	err = fs.updateIndexes(ctx, req.GetOwner().GetId().GetOpaqueId(), req.Type, root.ID)

pkg/storage/utils/decomposedfs/spaces_test.go

@@ -62,6 +62,7 @@ var _ = Describe("Spaces", func() {
 		})
 		Context("when creating a space", func() {
 			It("project space is created", func() {
+				env.Owner = nil
 				resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Mission to Mars", Type: "project"})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
@@ -101,6 +102,7 @@ var _ = Describe("Spaces", func() {
 			})
 			Context("creating a space", func() {
 				It("project space is created with custom alias", func() {
+					env.Owner = nil
 					resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Mission to Venus", Type: "project"})
 					Expect(err).ToNot(HaveOccurred())
 					Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))

pkg/utils/utils.go

@@ -298,6 +298,9 @@ func UserTypeMap(accountType string) userpb.UserType {
 		t = userpb.UserType_USER_TYPE_FEDERATED
 	case "lightweight":
 		t = userpb.UserType_USER_TYPE_LIGHTWEIGHT
+	// FIXME new user type
+	case "spaceowner":
+		t = 8
 	}
 	return t
 }
@@ -320,7 +323,11 @@ func UserTypeToString(accountType userpb.UserType) string {
 		t = "federated"
 	case userpb.UserType_USER_TYPE_LIGHTWEIGHT:
 		t = "lightweight"
+	// FIXME new user type
+	case 8:
+		t = "spaceowner"
 	}
+
 	return t
 }