From 8bbbd7856bacf1860c75470b9ebbd83a1107cb0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 30 Jul 2020 12:18:59 +0200 Subject: [PATCH 1/3] ocfs: lookup user to render template properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/fs/owncloud/owncloud.go | 102 ++++++++++++++++++++++------ pkg/storage/fs/owncloud/upload.go | 4 +- 2 files changed, 85 insertions(+), 21 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 0daf1996f3..2579353631 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -33,12 +33,14 @@ 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/storage" "github.com/cs3org/reva/pkg/storage/fs/registry" "github.com/cs3org/reva/pkg/storage/utils/templates" @@ -300,19 +302,49 @@ 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. use user service to look up id + c, err := pool.GetUserProviderServiceClient("localhost:9144") + if err != nil { + logger.New().Error().Err(err). + Msg("could not get user provider client") + // TODO return invalid internal path? + return + } + res, err := c.GetUser(ctx, &userpb.GetUserRequest{ + UserId: &userpb.UserId{OpaqueId: parts[0]}, + }) + if err != nil { + logger.New().Error().Err(err).Msg("error performing delete grpc request") + // TODO return invalid internal path? + return + } + + if res.Status.Code == rpc.Code_CODE_NOT_FOUND { + logger.New().Error().Str("code", string(res.Status.Code)).Msg("user not found") + // TODO return invalid internal path? + return + } + + if res.Status.Code != rpc.Code_CODE_OK { + logger.New().Error().Str("code", string(res.Status.Code)).Msg("grpc request failed") + // TODO return invalid internal path? + return + } + layout := templates.WithUser(res.User, 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 +362,47 @@ 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. use user service to look up id + c, err := pool.GetUserProviderServiceClient("localhost:9144") + if err != nil { + logger.New().Error().Err(err). + Msg("could not get user provider client") + // TODO return invalid internal path? + return + } + res, err := c.GetUser(ctx, &userpb.GetUserRequest{ + UserId: &userpb.UserId{OpaqueId: parts[0]}, + }) + if err != nil { + logger.New().Error().Err(err).Msg("error performing delete grpc request") + // TODO return invalid internal path? + return + } + + if res.Status.Code == rpc.Code_CODE_NOT_FOUND { + logger.New().Error().Str("code", string(res.Status.Code)).Msg("user not found") + // TODO return invalid internal path? + return + } + + if res.Status.Code != rpc.Code_CODE_OK { + logger.New().Error().Str("code", string(res.Status.Code)).Msg("grpc request failed") + // TODO return invalid internal path? + return + } + layout := templates.WithUser(res.User, 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 @@ -881,6 +942,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]), } 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 From d4a46c5a5bf428e6c849b4954ab90c2d8a033e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 30 Jul 2020 14:46:01 +0200 Subject: [PATCH 2/3] refactor, configurable addr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/ocfs-user-lookup.md | 5 + pkg/storage/fs/owncloud/owncloud.go | 149 +++++++++++++---------- 2 files changed, 89 insertions(+), 65 deletions(-) create mode 100644 changelog/unreleased/ocfs-user-lookup.md diff --git a/changelog/unreleased/ocfs-user-lookup.md b/changelog/unreleased/ocfs-user-lookup.md new file mode 100644 index 0000000000..6e0a36e24f --- /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/1033 diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 2579353631..aeb0372855 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -41,6 +41,7 @@ import ( "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" @@ -154,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"` + UserShareProviderEndpoint string `mapstructure:"usershareprovidersvc"` } func parseConfig(m map[string]interface{}) (*config, error) { @@ -189,6 +191,7 @@ func (c *config) init(m map[string]interface{}) { if _, ok := m["scan"]; !ok { c.Scan = true } + c.UserShareProviderEndpoint = sharedconf.GetGatewaySVC(c.UserShareProviderEndpoint) } // New returns an implementation to of the storage.FS interface that talk to @@ -307,35 +310,15 @@ func (fs *ocfs) wrap(ctx context.Context, fn string) (internal string) { return } - // parts[0] contains the username or userid. use user service to look up id - c, err := pool.GetUserProviderServiceClient("localhost:9144") + // 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 provider client") + Msg("could not get user") // TODO return invalid internal path? return } - res, err := c.GetUser(ctx, &userpb.GetUserRequest{ - UserId: &userpb.UserId{OpaqueId: parts[0]}, - }) - if err != nil { - logger.New().Error().Err(err).Msg("error performing delete grpc request") - // TODO return invalid internal path? - return - } - - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - logger.New().Error().Str("code", string(res.Status.Code)).Msg("user not found") - // TODO return invalid internal path? - return - } - - if res.Status.Code != rpc.Code_CODE_OK { - logger.New().Error().Str("code", string(res.Status.Code)).Msg("grpc request failed") - // TODO return invalid internal path? - return - } - layout := templates.WithUser(res.User, fs.c.UserLayout) + layout := templates.WithUser(u, fs.c.UserLayout) if len(parts) == 1 { // parts = "" @@ -367,35 +350,15 @@ func (fs *ocfs) wrapShadow(ctx context.Context, fn string) (internal string) { return } - // parts[0] contains the username or userid. use user service to look up id - c, err := pool.GetUserProviderServiceClient("localhost:9144") + // 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 provider client") + Msg("could not get user") // TODO return invalid internal path? return } - res, err := c.GetUser(ctx, &userpb.GetUserRequest{ - UserId: &userpb.UserId{OpaqueId: parts[0]}, - }) - if err != nil { - logger.New().Error().Err(err).Msg("error performing delete grpc request") - // TODO return invalid internal path? - return - } - - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - logger.New().Error().Str("code", string(res.Status.Code)).Msg("user not found") - // TODO return invalid internal path? - return - } - - if res.Status.Code != rpc.Code_CODE_OK { - logger.New().Error().Str("code", string(res.Status.Code)).Msg("grpc request failed") - // TODO return invalid internal path? - return - } - layout := templates.WithUser(res.User, fs.c.UserLayout) + layout := templates.WithUser(u, fs.c.UserLayout) if len(parts) == 1 { // parts = "" @@ -422,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? } @@ -442,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) { @@ -451,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) { @@ -534,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.UserShareProviderEndpoint) + 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) @@ -606,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), @@ -623,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 { @@ -1914,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") @@ -1935,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") @@ -1959,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(). From e8b185b55f7067d64e0ef92ad773207e0b23d8be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 31 Jul 2020 15:46:18 +0200 Subject: [PATCH 3/3] use correct userprovider service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/fs/owncloud/owncloud.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index aeb0372855..ef1976de6f 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -155,14 +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"` - UserShareProviderEndpoint string `mapstructure:"usershareprovidersvc"` + 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) { @@ -191,7 +191,7 @@ func (c *config) init(m map[string]interface{}) { if _, ok := m["scan"]; !ok { c.Scan = true } - c.UserShareProviderEndpoint = sharedconf.GetGatewaySVC(c.UserShareProviderEndpoint) + c.UserProviderEndpoint = sharedconf.GetGatewaySVC(c.UserProviderEndpoint) } // New returns an implementation to of the storage.FS interface that talk to @@ -522,7 +522,7 @@ func (fs *ocfs) getUser(ctx context.Context, usernameOrID string) (id *userpb.Us // 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.UserShareProviderEndpoint) + c, err := pool.GetUserProviderServiceClient(fs.c.UserProviderEndpoint) if err != nil { return nil, err }