Ocfs lookup userid (update) (#1052)

.drone.yml

@@ -298,7 +298,8 @@ steps:
   - name: litmus-oc-new-webdav
     image: owncloud/litmus:latest
     environment:
-      LITMUS_URL: http://revad-services:20080/remote.php/dav/files/einstein
+      # UUID is einstein user, see https://github.com/owncloud/ocis-accounts/blob/8de0530f31ed5ffb0bbb7f7f3471f87f429cb2ea/pkg/service/v0/service.go#L45
+      LITMUS_URL: http://revad-services:20080/remote.php/dav/files/4c510ada-c86b-4815-8820-42cdf82c3d51
       LITMUS_USERNAME: einstein
       LITMUS_PASSWORD: relativity
       TESTS: basic http copymove props

changelog/unreleased/ocfs-user-lookup.md

@@ -0,0 +1,5 @@
+Bugfix: ocfs: Lookup user to render template properly
+
+Currently, the username is used to construct paths, which breaks when mounting the `owncloud` storage driver at `/oc` and then expecting paths that use the username like `/oc/einstein/foo` to work, because they will mismatch the path that is used from propagation which uses `/oc/u-u-i-d` as the root, giving an `internal path outside root` error
+
+https://github.com/cs3org/reva/pull/1052

drone/oc-integration-tests/storage-oc.toml

@@ -23,6 +23,7 @@ data_server_url = "http://localhost:11001/data"
 [grpc.services.storageprovider.drivers.owncloud]
 datadirectory = "/drone/src/tmp/reva/data"
 redis = "redis:6379"
+userprovidersvc = "localhost:18000"
 
 [http]
 address = "0.0.0.0:11001"

examples/oc-phoenix/storage-oc.toml

@@ -22,7 +22,8 @@ data_server_url = "http://localhost:11001/data"
 
 [grpc.services.storageprovider.drivers.owncloud]
 datadirectory = "/var/tmp/reva/data"
-
+redis = "redis:6379"
+userprovidersvc = "localhost:18000"
 
 [http]
 address = "0.0.0.0:11001"

pkg/storage/fs/owncloud/owncloud.go

@@ -33,12 +33,15 @@ import (
 	"time"
 
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
+	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/errtypes"
 	"github.com/cs3org/reva/pkg/logger"
 	"github.com/cs3org/reva/pkg/mime"
+	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/pkg/sharedconf"
 	"github.com/cs3org/reva/pkg/storage"
 	"github.com/cs3org/reva/pkg/storage/fs/registry"
 	"github.com/cs3org/reva/pkg/storage/utils/templates"
@@ -152,13 +155,14 @@ func init() {
 }
 
 type config struct {
-	DataDirectory  string `mapstructure:"datadirectory"`
-	UploadInfoDir  string `mapstructure:"upload_info_dir"`
-	ShareDirectory string `mapstructure:"sharedirectory"`
-	UserLayout     string `mapstructure:"user_layout"`
-	Redis          string `mapstructure:"redis"`
-	EnableHome     bool   `mapstructure:"enable_home"`
-	Scan           bool   `mapstructure:"scan"`
+	DataDirectory        string `mapstructure:"datadirectory"`
+	UploadInfoDir        string `mapstructure:"upload_info_dir"`
+	ShareDirectory       string `mapstructure:"sharedirectory"`
+	UserLayout           string `mapstructure:"user_layout"`
+	Redis                string `mapstructure:"redis"`
+	EnableHome           bool   `mapstructure:"enable_home"`
+	Scan                 bool   `mapstructure:"scan"`
+	UserProviderEndpoint string `mapstructure:"userprovidersvc"`
 }
 
 func parseConfig(m map[string]interface{}) (*config, error) {
@@ -175,7 +179,7 @@ func (c *config) init(m map[string]interface{}) {
 		c.Redis = ":6379"
 	}
 	if c.UserLayout == "" {
-		c.UserLayout = "{{.Username}}"
+		c.UserLayout = "{{.Id.OpaqueId}}"
 	}
 	if c.UploadInfoDir == "" {
 		c.UploadInfoDir = "/var/tmp/reva/uploadinfo"
@@ -187,6 +191,7 @@ func (c *config) init(m map[string]interface{}) {
 	if _, ok := m["scan"]; !ok {
 		c.Scan = true
 	}
+	c.UserProviderEndpoint = sharedconf.GetGatewaySVC(c.UserProviderEndpoint)
 }
 
 // New returns an implementation to of the storage.FS interface that talk to
@@ -300,19 +305,29 @@ func (fs *ocfs) wrap(ctx context.Context, fn string) (internal string) {
 		// p = <username>/foo/bar.txt
 		parts := strings.SplitN(fn, "/", 2)
 
-		switch len(parts) {
-		case 1:
-			// parts = "" or "<username>"
-			if parts[0] == "" {
-				internal = fs.c.DataDirectory
-				return
-			}
+		if len(parts) == 1 && parts[0] == "" {
+			internal = fs.c.DataDirectory
+			return
+		}
+
+		// parts[0] contains the username or userid.
+		u, err := fs.getUser(ctx, parts[0])
+		if err != nil {
+			logger.New().Error().Err(err).
+				Msg("could not get user")
+			// TODO return invalid internal path?
+			return
+		}
+		layout := templates.WithUser(u, fs.c.UserLayout)
+
+		if len(parts) == 1 {
 			// parts = "<username>"
-			internal = path.Join(fs.c.DataDirectory, parts[0], "files")
-		default:
+			internal = path.Join(fs.c.DataDirectory, layout, "files")
+		} else {
 			// parts = "<username>", "foo/bar.txt"
-			internal = path.Join(fs.c.DataDirectory, parts[0], "files", parts[1])
+			internal = path.Join(fs.c.DataDirectory, layout, "files", parts[1])
 		}
+
 	}
 	return
 }
@@ -330,18 +345,27 @@ func (fs *ocfs) wrapShadow(ctx context.Context, fn string) (internal string) {
 		// p = <username>/foo/bar.txt
 		parts := strings.SplitN(fn, "/", 2)
 
-		switch len(parts) {
-		case 1:
-			// parts = "" or "<username>"
-			if parts[0] == "" {
-				internal = fs.c.DataDirectory
-				return
-			}
+		if len(parts) == 1 && parts[0] == "" {
+			internal = fs.c.DataDirectory
+			return
+		}
+
+		// parts[0] contains the username or userid.
+		u, err := fs.getUser(ctx, parts[0])
+		if err != nil {
+			logger.New().Error().Err(err).
+				Msg("could not get user")
+			// TODO return invalid internal path?
+			return
+		}
+		layout := templates.WithUser(u, fs.c.UserLayout)
+
+		if len(parts) == 1 {
 			// parts = "<username>"
-			internal = path.Join(fs.c.DataDirectory, parts[0], "shadow_files")
-		default:
+			internal = path.Join(fs.c.DataDirectory, layout, "shadow_files")
+		} else {
 			// parts = "<username>", "foo/bar.txt"
-			internal = path.Join(fs.c.DataDirectory, parts[0], "shadow_files", parts[1])
+			internal = path.Join(fs.c.DataDirectory, layout, "shadow_files", parts[1])
 		}
 	}
 	return
@@ -361,13 +385,23 @@ func (fs *ocfs) getVersionsPath(ctx context.Context, np string) string {
 	// np = /<username>/files/foo/bar.txt
 	parts := strings.SplitN(np, "/", 4)
 
+	// parts[1] contains the username or userid.
+	u, err := fs.getUser(ctx, parts[1])
+	if err != nil {
+		logger.New().Error().Err(err).
+			Msg("could not get user")
+		// TODO return invalid internal path?
+		return ""
+	}
+	layout := templates.WithUser(u, fs.c.UserLayout)
+
 	switch len(parts) {
 	case 3:
 		// parts = "", "<username>"
-		return path.Join(fs.c.DataDirectory, parts[1], "files_versions")
+		return path.Join(fs.c.DataDirectory, layout, "files_versions")
 	case 4:
 		// parts = "", "<username>", "foo/bar.txt"
-		return path.Join(fs.c.DataDirectory, parts[1], "files_versions", parts[3])
+		return path.Join(fs.c.DataDirectory, layout, "files_versions", parts[3])
 	default:
 		return "" // TODO Must not happen?
 	}
@@ -381,7 +415,8 @@ func (fs *ocfs) getRecyclePath(ctx context.Context) (string, error) {
 		err := errors.Wrap(errtypes.UserRequired("userrequired"), "error getting user from ctx")
 		return "", err
 	}
-	return path.Join(fs.c.DataDirectory, u.GetUsername(), "files_trashbin/files"), nil
+	layout := templates.WithUser(u, fs.c.UserLayout)
+	return path.Join(fs.c.DataDirectory, layout, "files_trashbin/files"), nil
 }
 
 func (fs *ocfs) getVersionRecyclePath(ctx context.Context) (string, error) {
@@ -390,7 +425,8 @@ func (fs *ocfs) getVersionRecyclePath(ctx context.Context) (string, error) {
 		err := errors.Wrap(errtypes.UserRequired("userrequired"), "error getting user from ctx")
 		return "", err
 	}
-	return path.Join(fs.c.DataDirectory, u.GetUsername(), "files_trashbin/files_versions"), nil
+	layout := templates.WithUser(u, fs.c.UserLayout)
+	return path.Join(fs.c.DataDirectory, layout, "files_trashbin/files_versions"), nil
 }
 
 func (fs *ocfs) unwrap(ctx context.Context, internal string) (external string) {
@@ -473,6 +509,42 @@ func (fs *ocfs) getOwner(internal string) string {
 	return ""
 }
 
+func (fs *ocfs) getUser(ctx context.Context, usernameOrID string) (id *userpb.User, err error) {
+	u := user.ContextMustGetUser(ctx)
+	// check if username matches and id is set
+	if u.Username == usernameOrID && u.Id != nil && u.Id.OpaqueId != "" {
+		return u, nil
+	}
+	// check if userid matches and username is set
+	if u.Id != nil && u.Id.OpaqueId == usernameOrID && u.Username != "" {
+		return u, nil
+	}
+	// look up at the userprovider
+
+	// parts[0] contains the username or userid. use  user service to look up id
+	c, err := pool.GetUserProviderServiceClient(fs.c.UserProviderEndpoint)
+	if err != nil {
+		return nil, err
+	}
+	res, err := c.GetUser(ctx, &userpb.GetUserRequest{
+		UserId: &userpb.UserId{OpaqueId: usernameOrID},
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	if res.Status.Code == rpc.Code_CODE_NOT_FOUND {
+		logger.New().Error().Str("code", string(res.Status.Code)).Msg("user not found")
+		return nil, fmt.Errorf("user not found")
+	}
+
+	if res.Status.Code != rpc.Code_CODE_OK {
+		logger.New().Error().Str("code", string(res.Status.Code)).Msg("user lookup failed")
+		return nil, fmt.Errorf("user lookup failed")
+	}
+	return res.User, nil
+}
+
 func (fs *ocfs) convertToResourceInfo(ctx context.Context, fi os.FileInfo, np string, fn string, c redis.Conn, mdKeys []string) *provider.ResourceInfo {
 	id := readOrCreateID(ctx, np, c)
 
@@ -545,10 +617,9 @@ func (fs *ocfs) convertToResourceInfo(ctx context.Context, fi os.FileInfo, np st
 		appctx.GetLogger(ctx).Error().Err(err).Msg("error getting list of extended attributes")
 	}
 
-	return &provider.ResourceInfo{
+	ri := &provider.ResourceInfo{
 		Id:            &provider.ResourceId{OpaqueId: id},
 		Path:          fn,
-		Owner:         &userpb.UserId{OpaqueId: fs.getOwner(np)},
 		Type:          getResourceType(fi.IsDir()),
 		Etag:          etag,
 		MimeType:      mime.Detect(fi.IsDir(), fn),
@@ -562,6 +633,14 @@ func (fs *ocfs) convertToResourceInfo(ctx context.Context, fi os.FileInfo, np st
 			Metadata: metadata,
 		},
 	}
+
+	if owner, err := fs.getUser(ctx, fs.getOwner(np)); err == nil {
+		ri.Owner = owner.Id
+	} else {
+		appctx.GetLogger(ctx).Error().Err(err).Msg("error getting owner")
+	}
+
+	return ri
 }
 func getResourceType(isDir bool) provider.ResourceType {
 	if isDir {
@@ -881,6 +960,7 @@ func (fs *ocfs) ListGrants(ctx context.Context, ref *provider.Reference) (grants
 	grants = make([]*provider.Grant, 0, len(aces))
 	for i := range aces {
 		grantee := &provider.Grantee{
+			// TODO lookup uid from principal
 			Id:   &userpb.UserId{OpaqueId: aces[i].Principal},
 			Type: fs.getGranteeType(aces[i]),
 		}
@@ -1852,7 +1932,8 @@ func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error {
 	} else {
 		origin = path.Clean(string(v))
 	}
-	tgt := path.Join(fs.wrap(ctx, path.Join("/", u.GetUsername(), origin)), strings.TrimSuffix(path.Base(src), suffix))
+	layout := templates.WithUser(u, fs.c.UserLayout)
+	tgt := path.Join(fs.wrap(ctx, path.Join("/", layout, origin)), strings.TrimSuffix(path.Base(src), suffix))
 	// move back to original location
 	if err := os.Rename(src, tgt); err != nil {
 		log.Error().Err(err).Str("path", src).Msg("could not restore item")
@@ -1873,8 +1954,8 @@ func (fs *ocfs) propagate(ctx context.Context, leafPath string) error {
 	if fs.c.EnableHome {
 		root = fs.wrap(ctx, "/")
 	} else {
-		u := user.ContextMustGetUser(ctx)
-		root = fs.wrap(ctx, path.Join("/", u.GetUsername()))
+		owner := fs.getOwner(leafPath)
+		root = fs.wrap(ctx, owner)
 	}
 	if !strings.HasPrefix(leafPath, root) {
 		err := errors.New("internal path outside root")
@@ -1897,7 +1978,7 @@ func (fs *ocfs) propagate(ctx context.Context, leafPath string) error {
 	}
 
 	parts := strings.Split(strings.TrimPrefix(leafPath, root), "/")
-	// root never ents in / so the split returns an empty first element, which we can skip
+	// root never ends in / so the split returns an empty first element, which we can skip
 	// we do not need to chmod the last element because it is the leaf path (< and not <= comparison)
 	for i := 1; i < len(parts); i++ {
 		appctx.GetLogger(ctx).Debug().

pkg/storage/fs/owncloud/upload.go

@@ -31,6 +31,7 @@ import (
 	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/errtypes"
 	"github.com/cs3org/reva/pkg/logger"
+	"github.com/cs3org/reva/pkg/storage/utils/templates"
 	"github.com/cs3org/reva/pkg/user"
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
@@ -203,7 +204,8 @@ func (fs *ocfs) getUploadPath(ctx context.Context, uploadID string) (string, err
 		err := errors.Wrap(errtypes.UserRequired("userrequired"), "error getting user from ctx")
 		return "", err
 	}
-	return filepath.Join(fs.c.DataDirectory, u.Username, "uploads", uploadID), nil
+	layout := templates.WithUser(u, fs.c.UserLayout)
+	return filepath.Join(fs.c.DataDirectory, layout, "uploads", uploadID), nil
 }
 
 // GetUpload returns the Upload for the given upload id