diff --git a/.drone.yml b/.drone.yml index 64a64eb16d..5f3e254465 100644 --- a/.drone.yml +++ b/.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 diff --git a/changelog/unreleased/ocfs-user-lookup.md b/changelog/unreleased/ocfs-user-lookup.md new file mode 100644 index 0000000000..52dd49ae89 --- /dev/null +++ b/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 diff --git a/drone/oc-integration-tests/storage-oc.toml b/drone/oc-integration-tests/storage-oc.toml index 3f2446b11f..f6e21dd9c0 100644 --- a/drone/oc-integration-tests/storage-oc.toml +++ b/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" diff --git a/examples/oc-phoenix/storage-oc.toml b/examples/oc-phoenix/storage-oc.toml index d9896f9063..c86e835c8a 100644 --- a/examples/oc-phoenix/storage-oc.toml +++ b/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" diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 0daf1996f3..a44f46263c 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/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 = /foo/bar.txt parts := strings.SplitN(fn, "/", 2) - switch len(parts) { - case 1: - // parts = "" or "" - 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 = "" - internal = path.Join(fs.c.DataDirectory, parts[0], "files") - default: + internal = path.Join(fs.c.DataDirectory, layout, "files") + } else { // parts = "", "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 = /foo/bar.txt parts := strings.SplitN(fn, "/", 2) - switch len(parts) { - case 1: - // parts = "" or "" - 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 = "" - internal = path.Join(fs.c.DataDirectory, parts[0], "shadow_files") - default: + internal = path.Join(fs.c.DataDirectory, layout, "shadow_files") + } else { // parts = "", "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 = //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 = "", "" - return path.Join(fs.c.DataDirectory, parts[1], "files_versions") + return path.Join(fs.c.DataDirectory, layout, "files_versions") case 4: // parts = "", "", "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(). diff --git a/pkg/storage/fs/owncloud/upload.go b/pkg/storage/fs/owncloud/upload.go index a13cb9aaa9..6b0f73fa29 100644 --- a/pkg/storage/fs/owncloud/upload.go +++ b/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