From 11fad3ac1fa1310580da4d5745119948ba580bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 22 Sep 2020 10:01:19 +0200 Subject: [PATCH 01/13] owncloud driver: enforce permissions 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 | 420 ++++++++++++++++++++++++++-- pkg/storage/fs/owncloud/upload.go | 65 +++++ 2 files changed, 463 insertions(+), 22 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index eda83d9290..0ff04a22c7 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -29,6 +29,7 @@ import ( "path/filepath" "strconv" "strings" + "syscall" "time" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -71,8 +72,34 @@ const ( favPrefix string = "user.oc.fav." // favorite flag, per user etagPrefix string = "user.oc.etag." // allow overriding a calculated etag with one from the extended attributes //checksumPrefix string = "user.oc.cs." // TODO add checksum support + ) +var defaultPermissions *provider.ResourcePermissions = &provider.ResourcePermissions{ + // no permissions +} +var ownerPermissions *provider.ResourcePermissions = &provider.ResourcePermissions{ + // all permissions + AddGrant: true, + CreateContainer: true, + Delete: true, + GetPath: true, + GetQuota: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + ListContainer: true, + ListFileVersions: true, + ListGrants: true, + ListRecycle: true, + Move: true, + PurgeRecycle: true, + RemoveGrant: true, + RestoreFileVersion: true, + RestoreRecycleItem: true, + Stat: true, + UpdateGrant: true, +} + func init() { registry.Register("owncloud", New) } @@ -667,6 +694,19 @@ func (fs *ocfs) GetPathByID(ctx context.Context, id *provider.ResourceId) (strin if err != nil { return "", err } + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.GetPath { + return "", errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return "", errtypes.NotFound(fs.unwrap(ctx, np)) + } + return "", errors.Wrap(err, "ocfs: error reading permissions") + } + return fs.unwrap(ctx, np), nil } @@ -694,6 +734,18 @@ func (fs *ocfs) AddGrant(ctx context.Context, ref *provider.Reference, g *provid return errors.Wrap(err, "ocfs: error resolving reference") } + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.AddGrant { + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, np)) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + e := ace.FromGrant(g) principal, value := e.Marshal() if err := xattr.Set(np, sharePrefix+principal, value); err != nil { @@ -726,12 +778,114 @@ func extractACEsFromAttrs(ctx context.Context, fsfn string, attrs []string) (ent return } +// TODO if user is owner but no acls found he can do everything? +// The owncloud driver does not integrate with the os so, for now, the owner can do everything, see ownerPermissions. +// Should this change we can store an acl for the owner in every node. +// We could also add default acls that can only the admin can set, eg for a read only storage? +// Someone needs to write to provide the content that should be read only, so this would likely be an acl for a group anyway. +func (fs *ocfs) readPermissions(ctx context.Context, np string) (p *provider.ResourcePermissions, err error) { + u, ok := user.ContextGetUser(ctx) + if !ok { + appctx.GetLogger(ctx).Debug().Str("np", np).Msg("no user in context, returning default permissions") + return defaultPermissions, nil + } + // check if the current user is the owner + if fs.getOwner(np) == u.Id.OpaqueId { + appctx.GetLogger(ctx).Debug().Str("np", np).Msg("user is owner, returning owner permissions") + return ownerPermissions, nil + } + aggregatedPermissions := &provider.ResourcePermissions{} + // add default permissions + addPermissions(aggregatedPermissions, defaultPermissions) + var e *ace.ACE + e, err = fs.readACE(ctx, np, "u:"+u.Id.OpaqueId) + if err == nil { + addPermissions(aggregatedPermissions, e.Grant().GetPermissions()) + } else if isNoData(err) { + err = nil + } else { + appctx.GetLogger(ctx).Error().Err(err).Str("np", np).Str("principal", "u:"+u.Id.OpaqueId).Msg("error reading user permissions") + return nil, err + } + // check all groups the user is a member of + for i := range u.Groups { + // groups are just strings ... groupnames ... or group ids ??? AAARGH !!! + e, err = fs.readACE(ctx, np, "g:"+u.Groups[i]) + if err == nil { + addPermissions(aggregatedPermissions, e.Grant().GetPermissions()) + } else if isNoData(err) { + err = nil + } else { + appctx.GetLogger(ctx).Error().Err(err).Str("np", np).Str("principal", "g:"+u.Groups[i]).Msg("error reading group permissions") + return nil, err + } + } + // TODO we need to read all parents ... until we find a matching ace? + appctx.GetLogger(ctx).Debug().Interface("permissions", aggregatedPermissions).Str("np", np).Msg("returning aggregated permissions") + return aggregatedPermissions, nil +} + +func isNoData(err error) bool { + if xerr, ok := err.(*xattr.Error); ok { + if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { + return serr == syscall.ENODATA + } + } + return false +} + +func (fs *ocfs) readACE(ctx context.Context, np string, principal string) (e *ace.ACE, err error) { + var b []byte + if b, err = xattr.Get(np, sharePrefix+principal); err != nil { + return nil, err + } + if e, err = ace.Unmarshal(principal, b); err != nil { + return nil, err + } + return +} + +// additive merging of permissions only +func addPermissions(p1 *provider.ResourcePermissions, p2 *provider.ResourcePermissions) { + p1.AddGrant = p1.AddGrant || p2.AddGrant + p1.CreateContainer = p1.CreateContainer || p2.CreateContainer + p1.Delete = p1.Delete || p2.Delete + p1.GetPath = p1.GetPath || p2.GetPath + p1.GetQuota = p1.GetQuota || p2.GetQuota + p1.InitiateFileDownload = p1.InitiateFileDownload || p2.InitiateFileDownload + p1.InitiateFileUpload = p1.InitiateFileUpload || p2.InitiateFileUpload + p1.ListContainer = p1.ListContainer || p2.ListContainer + p1.ListFileVersions = p1.ListFileVersions || p2.ListFileVersions + p1.ListGrants = p1.ListGrants || p2.ListGrants + p1.ListRecycle = p1.ListRecycle || p2.ListRecycle + p1.Move = p1.Move || p2.Move + p1.PurgeRecycle = p1.PurgeRecycle || p2.PurgeRecycle + p1.RemoveGrant = p1.RemoveGrant || p2.RemoveGrant + p1.RestoreFileVersion = p1.RestoreFileVersion || p2.RestoreFileVersion + p1.RestoreRecycleItem = p1.RestoreRecycleItem || p2.RestoreRecycleItem + p1.Stat = p1.Stat || p2.Stat + p1.UpdateGrant = p1.UpdateGrant || p2.UpdateGrant +} + func (fs *ocfs) ListGrants(ctx context.Context, ref *provider.Reference) (grants []*provider.Grant, err error) { log := appctx.GetLogger(ctx) var np string if np, err = fs.resolve(ctx, ref); err != nil { return nil, errors.Wrap(err, "ocfs: error resolving reference") } + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.ListGrants { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, np)) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + var attrs []string if attrs, err = xattr.List(np); err != nil { log.Error().Err(err).Msg("error listing attributes") @@ -757,6 +911,18 @@ func (fs *ocfs) RemoveGrant(ctx context.Context, ref *provider.Reference, g *pro return errors.Wrap(err, "ocfs: error resolving reference") } + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.ListContainer { + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, np)) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + var attr string if g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP { attr = sharePrefix + "g:" + g.Grantee.Id.OpaqueId @@ -772,7 +938,29 @@ func (fs *ocfs) RemoveGrant(ctx context.Context, ref *provider.Reference, g *pro } func (fs *ocfs) UpdateGrant(ctx context.Context, ref *provider.Reference, g *provider.Grant) error { - return fs.AddGrant(ctx, ref, g) + np, err := fs.resolve(ctx, ref) + if err != nil { + return errors.Wrap(err, "ocfs: error resolving reference") + } + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.UpdateGrant { + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, np)) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + + e := ace.FromGrant(g) + principal, value := e.Marshal() + if err := xattr.Set(np, sharePrefix+principal, value); err != nil { + return err + } + return fs.propagate(ctx, np) } func (fs *ocfs) GetQuota(ctx context.Context) (int, int, error) { @@ -814,6 +1002,19 @@ func (fs *ocfs) GetHome(ctx context.Context) (string, error) { func (fs *ocfs) CreateDir(ctx context.Context, fn string) (err error) { np := fs.wrap(ctx, fn) + + // check permissions of parent dir + if perm, err := fs.readPermissions(ctx, filepath.Dir(np)); err == nil { + if !perm.CreateContainer { + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + if err = os.Mkdir(np, 0700); err != nil { if os.IsNotExist(err) { return errtypes.NotFound(fn) @@ -838,6 +1039,7 @@ func (fs *ocfs) CreateReference(ctx context.Context, p string, targetURI *url.UR } fn := fs.wrapShadow(ctx, p) + // TODO check permission? dir, _ := path.Split(fn) if err := os.MkdirAll(dir, 0700); err != nil { @@ -884,6 +1086,18 @@ func (fs *ocfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Referenc return errors.Wrap(err, "ocfs: error resolving reference") } + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.InitiateFileUpload { // TODO add dedicated permission? + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + var fi os.FileInfo fi, err = os.Stat(np) if err != nil { @@ -1015,6 +1229,18 @@ func (fs *ocfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refere return errors.Wrap(err, "ocfs: error resolving reference") } + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.InitiateFileUpload { // TODO add dedicated permission? + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, np)) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + _, err = os.Stat(np) if err != nil { if os.IsNotExist(err) { @@ -1089,12 +1315,23 @@ func (fs *ocfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refere // We will live with that compromise since this storage driver will be // deprecated soon. func (fs *ocfs) Delete(ctx context.Context, ref *provider.Reference) (err error) { - var np string if np, err = fs.resolve(ctx, ref); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.Delete { + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + _, err = os.Stat(np) if err != nil { if os.IsNotExist(err) { @@ -1176,10 +1413,26 @@ func (fs *ocfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (e if oldName, err = fs.resolve(ctx, oldRef); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } + + // check permissions + if perm, err := fs.readPermissions(ctx, oldName); err == nil { + if !perm.Move { // TODO add dedicated permission? + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(oldName))) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + var newName string if newName, err = fs.resolve(ctx, newRef); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } + + // TODO check target permissions ... if it exists + if err = os.Rename(oldName, newName); err != nil { return errors.Wrap(err, "ocfs: error moving "+oldName+" to "+newName) } @@ -1213,6 +1466,18 @@ func (fs *ocfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []str if strings.HasPrefix(p, fs.c.DataDirectory) { np = p } + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.Stat { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + md, err := os.Stat(np) if err != nil { if os.IsNotExist(err) { @@ -1228,20 +1493,33 @@ func (fs *ocfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []str } func (fs *ocfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string) (*provider.ResourceInfo, error) { - fn := fs.wrapShadow(ctx, p) - md, err := os.Stat(fn) + np := fs.wrapShadow(ctx, p) + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.Stat { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + + md, err := os.Stat(np) if err != nil { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrapShadow(ctx, fn)) + return nil, errtypes.NotFound(fs.unwrapShadow(ctx, np)) } - return nil, errors.Wrapf(err, "ocfs: error stating %s", fn) + return nil, errors.Wrapf(err, "ocfs: error stating %s", np) } c := fs.pool.Get() defer c.Close() - m := fs.convertToResourceInfo(ctx, md, fn, fs.unwrapShadow(ctx, fn), c, mdKeys) + m := fs.convertToResourceInfo(ctx, md, np, fs.unwrapShadow(ctx, np), c, mdKeys) if !fs.isShareFolderRoot(p) { m.Type = provider.ResourceType_RESOURCE_TYPE_REFERENCE - ref, err := xattr.Get(fn, mdPrefix+"target") + ref, err := xattr.Get(np, mdPrefix+"target") if err != nil { return nil, err } @@ -1263,31 +1541,46 @@ func (fs *ocfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys if fs.c.EnableHome { log.Debug().Msg("home enabled") if strings.HasPrefix(p, "/") { + // permissions checked in listWithHome return fs.listWithHome(ctx, "/", p, mdKeys) } } log.Debug().Msg("list with nominal home") + // permissions checked in listWithNominalHome return fs.listWithNominalHome(ctx, p, mdKeys) } func (fs *ocfs) listWithNominalHome(ctx context.Context, p string, mdKeys []string) ([]*provider.ResourceInfo, error) { - fn := p + np := p // If a user wants to list a folder shared with him the path will already // be wrapped with the files directory path of the share owner. // In that case we don't want to wrap the path again. if !strings.HasPrefix(p, fs.c.DataDirectory) { - fn = fs.wrap(ctx, p) + np = fs.wrap(ctx, p) + } + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.ListContainer { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") } - mds, err := ioutil.ReadDir(fn) + + mds, err := ioutil.ReadDir(np) if err != nil { - return nil, errors.Wrapf(err, "ocfs: error listing %s", fn) + return nil, errors.Wrapf(err, "ocfs: error listing %s", np) } c := fs.pool.Get() defer c.Close() finfos := []*provider.ResourceInfo{} for _, md := range mds { - p := path.Join(fn, md.Name()) + p := path.Join(np, md.Name()) m := fs.convertToResourceInfo(ctx, md, p, fs.unwrap(ctx, p), c, mdKeys) finfos = append(finfos, m) } @@ -1316,8 +1609,21 @@ func (fs *ocfs) listWithHome(ctx context.Context, home, p string, mdKeys []strin func (fs *ocfs) listHome(ctx context.Context, home string, mdKeys []string) ([]*provider.ResourceInfo, error) { // list files - fn := fs.wrap(ctx, home) - mds, err := ioutil.ReadDir(fn) + np := fs.wrap(ctx, home) + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.ListContainer { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + + mds, err := ioutil.ReadDir(np) if err != nil { return nil, errors.Wrap(err, "ocfs: error listing files") } @@ -1327,19 +1633,19 @@ func (fs *ocfs) listHome(ctx context.Context, home string, mdKeys []string) ([]* finfos := []*provider.ResourceInfo{} for _, md := range mds { - p := path.Join(fn, md.Name()) + p := path.Join(np, md.Name()) m := fs.convertToResourceInfo(ctx, md, p, fs.unwrap(ctx, p), c, mdKeys) finfos = append(finfos, m) } // list shadow_files - fn = fs.wrapShadow(ctx, home) - mds, err = ioutil.ReadDir(fn) + np = fs.wrapShadow(ctx, home) + mds, err = ioutil.ReadDir(np) if err != nil { return nil, errors.Wrap(err, "ocfs: error listing shadow_files") } for _, md := range mds { - p := path.Join(fn, md.Name()) + p := path.Join(np, md.Name()) m := fs.convertToResourceInfo(ctx, md, p, fs.unwrapShadow(ctx, p), c, mdKeys) finfos = append(finfos, m) } @@ -1347,8 +1653,21 @@ func (fs *ocfs) listHome(ctx context.Context, home string, mdKeys []string) ([]* } func (fs *ocfs) listShareFolderRoot(ctx context.Context, p string, mdKeys []string) ([]*provider.ResourceInfo, error) { - fn := fs.wrapShadow(ctx, p) - mds, err := ioutil.ReadDir(fn) + np := fs.wrapShadow(ctx, p) + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.ListContainer { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + + mds, err := ioutil.ReadDir(np) if err != nil { return nil, errors.Wrap(err, "ocfs: error listing shadow_files") } @@ -1358,7 +1677,7 @@ func (fs *ocfs) listShareFolderRoot(ctx context.Context, p string, mdKeys []stri finfos := []*provider.ResourceInfo{} for _, md := range mds { - p := path.Join(fn, md.Name()) + p := path.Join(np, md.Name()) m := fs.convertToResourceInfo(ctx, md, p, fs.unwrapShadow(ctx, p), c, mdKeys) m.Type = provider.ResourceType_RESOURCE_TYPE_REFERENCE ref, err := xattr.Get(p, mdPrefix+"target") @@ -1411,6 +1730,19 @@ func (fs *ocfs) Download(ctx context.Context, ref *provider.Reference) (io.ReadC if err != nil { return nil, errors.Wrap(err, "ocfs: error resolving reference") } + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.InitiateFileDownload { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + r, err := os.Open(np) if err != nil { if os.IsNotExist(err) { @@ -1426,6 +1758,19 @@ func (fs *ocfs) ListRevisions(ctx context.Context, ref *provider.Reference) ([]* if err != nil { return nil, errors.Wrap(err, "ocfs: error resolving reference") } + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.ListFileVersions { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + vp := fs.getVersionsPath(ctx, np) bn := path.Base(np) @@ -1474,6 +1819,19 @@ func (fs *ocfs) RestoreRevision(ctx context.Context, ref *provider.Reference, re if err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } + + // check permissions + if perm, err := fs.readPermissions(ctx, np); err == nil { + if !perm.RestoreFileVersion { + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + vp := fs.getVersionsPath(ctx, np) rp := vp + ".v" + revisionKey @@ -1521,6 +1879,21 @@ func (fs *ocfs) PurgeRecycleItem(ctx context.Context, key string) error { return errors.Wrap(err, "ocfs: error resolving recycle path") } ip := path.Join(rp, path.Clean(key)) + // TODO check permission? + + // check permissions + /* are they stored in the trash? + if perm, err := fs.readPermissions(ctx, fn); err == nil { + if !perm.ListContainer { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + */ err = os.Remove(ip) if err != nil { @@ -1535,6 +1908,7 @@ func (fs *ocfs) PurgeRecycleItem(ctx context.Context, key string) error { } func (fs *ocfs) EmptyRecycle(ctx context.Context) error { + // TODO check permission? on what? user must be the owner rp, err := fs.getRecyclePath(ctx) if err != nil { return errors.Wrap(err, "ocfs: error resolving recycle path") @@ -1591,6 +1965,7 @@ func (fs *ocfs) convertToRecycleItem(ctx context.Context, rp string, md os.FileI } func (fs *ocfs) ListRecycle(ctx context.Context) ([]*provider.RecycleItem, error) { + // TODO check permission? on what? user must be the owner? rp, err := fs.getRecyclePath(ctx) if err != nil { return nil, errors.Wrap(err, "ocfs: error resolving recycle path") @@ -1617,6 +1992,7 @@ func (fs *ocfs) ListRecycle(ctx context.Context) ([]*provider.RecycleItem, error } func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error { + // TODO check permission? on what? user must be the owner? log := appctx.GetLogger(ctx) rp, err := fs.getRecyclePath(ctx) if err != nil { diff --git a/pkg/storage/fs/owncloud/upload.go b/pkg/storage/fs/owncloud/upload.go index 6b0f73fa29..156f6e7e7f 100644 --- a/pkg/storage/fs/owncloud/upload.go +++ b/pkg/storage/fs/owncloud/upload.go @@ -47,6 +47,27 @@ func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCl return errors.Wrap(err, "ocfs: error resolving reference") } + var perm *provider.ResourcePermissions + var perr error + // if destination exists + if _, err := os.Stat(np); err == nil { + // check permissions of file to be overwritten + perm, perr = fs.readPermissions(ctx, np) + } else { + // check permissions + perm, perr = fs.readPermissions(ctx, filepath.Base(np)) + } + if perr == nil { + if !perm.InitiateFileUpload { + return errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return errors.Wrap(err, "ocfs: error reading permissions") + } + // we cannot rely on /tmp as it can live in another partition and we can // hit invalid cross-device link errors, so we create the tmp file in the same directory // the file is supposed to be written. @@ -88,6 +109,28 @@ func (fs *ocfs) InitiateUpload(ctx context.Context, ref *provider.Reference, upl return "", errors.Wrap(err, "ocfs: error resolving reference") } + // check permissions + var perm *provider.ResourcePermissions + var perr error + // if destination exists + if _, err := os.Stat(np); err == nil { + // check permissions of file to be overwritten + perm, perr = fs.readPermissions(ctx, np) + } else { + // check permissions + perm, perr = fs.readPermissions(ctx, filepath.Base(np)) + } + if perr == nil { + if !perm.InitiateFileUpload { + return "", errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return "", errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return "", errors.Wrap(err, "ocfs: error reading permissions") + } + p := fs.unwrap(ctx, np) info := tusd.FileInfo{ @@ -143,6 +186,28 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd. np := fs.wrap(ctx, filepath.Join(info.MetaData["dir"], info.MetaData["filename"])) + // check permissions + var perm *provider.ResourcePermissions + var perr error + // if destination exists + if _, err := os.Stat(np); err == nil { + // check permissions of file to be overwritten + perm, perr = fs.readPermissions(ctx, np) + } else { + // check permissions + perm, perr = fs.readPermissions(ctx, filepath.Base(np)) + } + if perr == nil { + if !perm.InitiateFileUpload { + return nil, errtypes.PermissionDenied("") + } + } else { + if os.IsNotExist(err) { + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + } + return nil, errors.Wrap(err, "ocfs: error reading permissions") + } + log.Debug().Interface("info", info).Msg("ocfs: resolved filename") info.ID = uuid.New().String() From 30fbf8df25a20485bd7f0b69d715d04597f00a0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 23 Sep 2020 21:34:27 +0200 Subject: [PATCH 02/13] clarify ocfs path vars and wrap functions 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 | 635 ++++++++++++++-------------- pkg/storage/fs/owncloud/upload.go | 71 ++-- 2 files changed, 356 insertions(+), 350 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 0ff04a22c7..346094401b 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -25,7 +25,6 @@ import ( "io/ioutil" "net/url" "os" - "path" "path/filepath" "strconv" "strings" @@ -162,7 +161,7 @@ func New(m map[string]interface{}) (storage.FS, error) { c.init(m) // c.DataDirectory should never end in / unless it is the root? - c.DataDirectory = path.Clean(c.DataDirectory) + c.DataDirectory = filepath.Clean(c.DataDirectory) // create datadir if it does not exist err = os.MkdirAll(c.DataDirectory, 0700) @@ -250,76 +249,76 @@ func (fs *ocfs) scanFiles(ctx context.Context, conn redis.Conn) { // the incoming path starts with /, so we need to insert the files subfolder into the path // and prefix the data directory // TODO the path handed to a storage provider should not contain the username -func (fs *ocfs) wrap(ctx context.Context, fn string) (internal string) { +func (fs *ocfs) toInternalPath(ctx context.Context, sp string) (ip string) { if fs.c.EnableHome { u := user.ContextMustGetUser(ctx) layout := templates.WithUser(u, fs.c.UserLayout) - internal = path.Join(fs.c.DataDirectory, layout, "files", fn) + ip = filepath.Join(fs.c.DataDirectory, layout, "files", sp) } else { // trim all / - fn = strings.Trim(fn, "/") + sp = strings.Trim(sp, "/") // p = "" or // p = or // p = /foo/bar.txt - parts := strings.SplitN(fn, "/", 2) + segments := strings.SplitN(sp, "/", 2) - if len(parts) == 1 && parts[0] == "" { - internal = fs.c.DataDirectory + if len(segments) == 1 && segments[0] == "" { + ip = fs.c.DataDirectory return } // parts[0] contains the username or userid. - u, err := fs.getUser(ctx, parts[0]) + u, err := fs.getUser(ctx, segments[0]) if err != nil { // TODO return invalid internal path? return } layout := templates.WithUser(u, fs.c.UserLayout) - if len(parts) == 1 { + if len(segments) == 1 { // parts = "" - internal = path.Join(fs.c.DataDirectory, layout, "files") + ip = filepath.Join(fs.c.DataDirectory, layout, "files") } else { // parts = "", "foo/bar.txt" - internal = path.Join(fs.c.DataDirectory, layout, "files", parts[1]) + ip = filepath.Join(fs.c.DataDirectory, layout, "files", segments[1]) } } return } -func (fs *ocfs) wrapShadow(ctx context.Context, fn string) (internal string) { +func (fs *ocfs) toInternalShadowPath(ctx context.Context, sp string) (internal string) { if fs.c.EnableHome { u := user.ContextMustGetUser(ctx) layout := templates.WithUser(u, fs.c.UserLayout) - internal = path.Join(fs.c.DataDirectory, layout, "shadow_files", fn) + internal = filepath.Join(fs.c.DataDirectory, layout, "shadow_files", sp) } else { // trim all / - fn = strings.Trim(fn, "/") + sp = strings.Trim(sp, "/") // p = "" or // p = or // p = /foo/bar.txt - parts := strings.SplitN(fn, "/", 2) + segments := strings.SplitN(sp, "/", 2) - if len(parts) == 1 && parts[0] == "" { + if len(segments) == 1 && segments[0] == "" { internal = fs.c.DataDirectory return } // parts[0] contains the username or userid. - u, err := fs.getUser(ctx, parts[0]) + u, err := fs.getUser(ctx, segments[0]) if err != nil { // TODO return invalid internal path? return } layout := templates.WithUser(u, fs.c.UserLayout) - if len(parts) == 1 { + if len(segments) == 1 { // parts = "" - internal = path.Join(fs.c.DataDirectory, layout, "shadow_files") + internal = filepath.Join(fs.c.DataDirectory, layout, "shadow_files") } else { // parts = "", "foo/bar.txt" - internal = path.Join(fs.c.DataDirectory, layout, "shadow_files", parts[1]) + internal = filepath.Join(fs.c.DataDirectory, layout, "shadow_files", segments[1]) } } return @@ -329,15 +328,15 @@ func (fs *ocfs) wrapShadow(ctx context.Context, fn string) (internal string) { // the incoming path starts with /, so we need to insert the files subfolder into the path // and prefix the data directory // TODO the path handed to a storage provider should not contain the username -func (fs *ocfs) getVersionsPath(ctx context.Context, np string) string { - // np = /path/to/data//files/foo/bar.txt +func (fs *ocfs) getVersionsPath(ctx context.Context, ip string) string { + // ip = /path/to/data//files/foo/bar.txt // remove data dir if fs.c.DataDirectory != "/" { // fs.c.DataDirectory is a clean path, so it never ends in / - np = strings.TrimPrefix(np, fs.c.DataDirectory) + ip = strings.TrimPrefix(ip, fs.c.DataDirectory) } - // np = //files/foo/bar.txt - parts := strings.SplitN(np, "/", 4) + // ip = //files/foo/bar.txt + parts := strings.SplitN(ip, "/", 4) // parts[1] contains the username or userid. u, err := fs.getUser(ctx, parts[1]) @@ -350,10 +349,10 @@ func (fs *ocfs) getVersionsPath(ctx context.Context, np string) string { switch len(parts) { case 3: // parts = "", "" - return path.Join(fs.c.DataDirectory, layout, "files_versions") + return filepath.Join(fs.c.DataDirectory, layout, "files_versions") case 4: // parts = "", "", "foo/bar.txt" - return path.Join(fs.c.DataDirectory, layout, "files_versions", parts[3]) + return filepath.Join(fs.c.DataDirectory, layout, "files_versions", parts[3]) default: return "" // TODO Must not happen? } @@ -368,7 +367,7 @@ func (fs *ocfs) getRecyclePath(ctx context.Context) (string, error) { return "", err } layout := templates.WithUser(u, fs.c.UserLayout) - return path.Join(fs.c.DataDirectory, layout, "files_trashbin/files"), nil + return filepath.Join(fs.c.DataDirectory, layout, "files_trashbin/files"), nil } func (fs *ocfs) getVersionRecyclePath(ctx context.Context) (string, error) { @@ -378,81 +377,87 @@ func (fs *ocfs) getVersionRecyclePath(ctx context.Context) (string, error) { return "", err } layout := templates.WithUser(u, fs.c.UserLayout) - return path.Join(fs.c.DataDirectory, layout, "files_trashbin/files_versions"), nil + return filepath.Join(fs.c.DataDirectory, layout, "files_trashbin/files_versions"), nil } -func (fs *ocfs) unwrap(ctx context.Context, internal string) (external string) { +func (fs *ocfs) toStoragePath(ctx context.Context, ip string) (sp string) { if fs.c.EnableHome { u := user.ContextMustGetUser(ctx) layout := templates.WithUser(u, fs.c.UserLayout) - trim := path.Join(fs.c.DataDirectory, layout, "files") - external = strings.TrimPrefix(internal, trim) + trim := filepath.Join(fs.c.DataDirectory, layout, "files") + sp = strings.TrimPrefix(ip, trim) // root directory - if external == "" { - external = "/" + if sp == "" { + sp = "/" } } else { - // np = /data//files/foo/bar.txt + // ip = /data//files/foo/bar.txt // remove data dir if fs.c.DataDirectory != "/" { // fs.c.DataDirectory is a clean path, so it never ends in / - internal = strings.TrimPrefix(internal, fs.c.DataDirectory) - // np = //files/foo/bar.txt + ip = strings.TrimPrefix(ip, fs.c.DataDirectory) + // ip = //files/foo/bar.txt } - parts := strings.SplitN(internal, "/", 4) + segments := strings.SplitN(ip, "/", 4) // parts = "", "", "files", "foo/bar.txt" - switch len(parts) { + switch len(segments) { case 1: - external = "/" + sp = "/" case 2: - external = path.Join("/", parts[1]) + sp = filepath.Join("/", segments[1]) case 3: - external = path.Join("/", parts[1]) + sp = filepath.Join("/", segments[1]) default: - external = path.Join("/", parts[1], parts[3]) + sp = filepath.Join("/", segments[1], segments[3]) } } - appctx.GetLogger(ctx).Debug().Str("internal", internal).Str("external", external).Msg("ocfs: unwrap") + log := appctx.GetLogger(ctx) + log.Debug().Str("driver", "ocfs").Str("ipath", ip).Str("spath", sp).Msg("toStoragePath") return } -func (fs *ocfs) unwrapShadow(ctx context.Context, internal string) (external string) { +func (fs *ocfs) toStorageShadowPath(ctx context.Context, ip string) (sp string) { if fs.c.EnableHome { u := user.ContextMustGetUser(ctx) layout := templates.WithUser(u, fs.c.UserLayout) - trim := path.Join(fs.c.DataDirectory, layout, "shadow_files") - external = strings.TrimPrefix(internal, trim) + trim := filepath.Join(fs.c.DataDirectory, layout, "shadow_files") + sp = strings.TrimPrefix(ip, trim) } else { - // np = /data//shadow_files/foo/bar.txt + // ip = /data//shadow_files/foo/bar.txt // remove data dir if fs.c.DataDirectory != "/" { // fs.c.DataDirectory is a clean path, so it never ends in / - internal = strings.TrimPrefix(internal, fs.c.DataDirectory) - // np = //shadow_files/foo/bar.txt + ip = strings.TrimPrefix(ip, fs.c.DataDirectory) + // ip = //shadow_files/foo/bar.txt } - parts := strings.SplitN(internal, "/", 4) + segments := strings.SplitN(ip, "/", 4) // parts = "", "", "shadow_files", "foo/bar.txt" - switch len(parts) { + switch len(segments) { case 1: - external = "/" + sp = "/" case 2: - external = path.Join("/", parts[1]) + sp = filepath.Join("/", segments[1]) case 3: - external = path.Join("/", parts[1]) + sp = filepath.Join("/", segments[1]) default: - external = path.Join("/", parts[1], parts[3]) + sp = filepath.Join("/", segments[1], segments[3]) } } +<<<<<<< HEAD appctx.GetLogger(ctx).Debug().Str("internal", internal).Str("external", external).Msg("ocfs: unwrapShadow") +======= + log := appctx.GetLogger(ctx) + log.Debug().Str("driver", "ocfs").Str("ipath", ip).Str("spath", sp).Msg("toStorageShadowPath") +>>>>>>> e431462... clarify ocfs path vars and wrap functions return } // TODO the owner needs to come from a different place -func (fs *ocfs) getOwner(internal string) string { - internal = strings.TrimPrefix(internal, fs.c.DataDirectory) - parts := strings.SplitN(internal, "/", 3) +func (fs *ocfs) getOwner(ip string) string { + ip = strings.TrimPrefix(ip, fs.c.DataDirectory) + parts := strings.SplitN(ip, "/", 3) if len(parts) > 1 { return parts[1] } @@ -516,14 +521,14 @@ func (fs *ocfs) getUser(ctx context.Context, usernameOrID string) (id *userpb.Us 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) +func (fs *ocfs) convertToResourceInfo(ctx context.Context, fi os.FileInfo, ip string, sp string, c redis.Conn, mdKeys []string) *provider.ResourceInfo { + id := readOrCreateID(ctx, ip, c) etag := calcEtag(ctx, fi) - if val, err := xattr.Get(np, etagPrefix+etag); err == nil { + if val, err := xattr.Get(ip, etagPrefix+etag); err == nil { appctx.GetLogger(ctx).Debug(). - Str("np", np). + Str("ipath", ip). Str("calcetag", etag). Str("etag", string(val)). Msg("overriding calculated etag") @@ -549,9 +554,9 @@ func (fs *ocfs) convertToResourceInfo(ctx context.Context, fi os.FileInfo, np st // the favorite flag is specific to the user, so we need to incorporate the userid if uid := u.GetId(); uid != nil { fa := fmt.Sprintf("%s%s@%s", favPrefix, uid.GetOpaqueId(), uid.GetIdp()) - if val, err := xattr.Get(np, fa); err == nil { + if val, err := xattr.Get(ip, fa); err == nil { appctx.GetLogger(ctx).Debug(). - Str("np", np). + Str("ipath", ip). Str("favorite", string(val)). Str("username", u.GetUsername()). Msg("found favorite flag") @@ -566,14 +571,14 @@ func (fs *ocfs) convertToResourceInfo(ctx context.Context, fi os.FileInfo, np st metadata[favoriteKey] = favorite } - list, err := xattr.List(np) + list, err := xattr.List(ip) if err == nil { for _, entry := range list { // filter out non-custom properties if !strings.HasPrefix(entry, mdPrefix) { continue } - if val, err := xattr.Get(np, entry); err == nil { + if val, err := xattr.Get(ip, entry); err == nil { k := entry[len(mdPrefix):] if _, ok := mdKeysMap[k]; returnAllKeys || ok { metadata[k] = string(val) @@ -590,10 +595,10 @@ func (fs *ocfs) convertToResourceInfo(ctx context.Context, fi os.FileInfo, np st ri := &provider.ResourceInfo{ Id: &provider.ResourceId{OpaqueId: id}, - Path: fn, + Path: sp, Type: getResourceType(fi.IsDir()), Etag: etag, - MimeType: mime.Detect(fi.IsDir(), fn), + MimeType: mime.Detect(fi.IsDir(), ip), Size: uint64(fi.Size()), PermissionSet: &provider.ResourcePermissions{ListContainer: true, CreateContainer: true}, Mtime: &types.Timestamp{ @@ -605,7 +610,7 @@ func (fs *ocfs) convertToResourceInfo(ctx context.Context, fi os.FileInfo, np st }, } - if owner, err := fs.getUser(ctx, fs.getOwner(np)); err == nil { + if owner, err := fs.getUser(ctx, fs.getOwner(ip)); err == nil { ri.Owner = owner.Id } else { appctx.GetLogger(ctx).Error().Err(err).Msg("error getting owner") @@ -620,30 +625,30 @@ func getResourceType(isDir bool) provider.ResourceType { return provider.ResourceType_RESOURCE_TYPE_FILE } -func readOrCreateID(ctx context.Context, np string, conn redis.Conn) string { +func readOrCreateID(ctx context.Context, ip string, conn redis.Conn) string { log := appctx.GetLogger(ctx) // read extended file attribute for id //generate if not present var id []byte var err error - if id, err = xattr.Get(np, idAttribute); err != nil { - log.Warn().Err(err).Msg("error reading file id") + if id, err = xattr.Get(ip, idAttribute); err != nil { + log.Warn().Err(err).Str("driver", "owncloud").Str("ipath", ip).Msg("error reading file id") // try generating a uuid if uuid, err := uuid.NewV4(); err != nil { - log.Error().Err(err).Msg("error generating fileid") + log.Error().Err(err).Str("driver", "owncloud").Str("ipath", ip).Msg("error generating fileid") } else { // store uuid id = uuid.Bytes() - if err := xattr.Set(np, idAttribute, id); err != nil { - log.Error().Err(err).Msg("error storing file id") + if err := xattr.Set(ip, idAttribute, id); err != nil { + log.Error().Err(err).Str("driver", "owncloud").Str("ipath", ip).Msg("error storing file id") } // TODO cache path for uuid in redis // TODO reuse conn? if conn != nil { - _, err := conn.Do("SET", uuid.String(), np) + _, err := conn.Do("SET", uuid.String(), ip) if err != nil { - log.Error().Str("path", np).Err(err).Msg("error caching id") + log.Error().Err(err).Str("driver", "owncloud").Str("ipath", ip).Msg("error caching id") // continue } } @@ -663,12 +668,12 @@ func (fs *ocfs) getPath(ctx context.Context, id *provider.ResourceId) (string, e c := fs.pool.Get() defer c.Close() fs.scanFiles(ctx, c) - np, err := redis.String(c.Do("GET", id.OpaqueId)) + ip, err := redis.String(c.Do("GET", id.OpaqueId)) if err != nil { return "", errtypes.NotFound(id.OpaqueId) } - idFromXattr, err := xattr.Get(np, idAttribute) + idFromXattr, err := xattr.Get(ip, idAttribute) if err != nil { return "", errtypes.NotFound(id.OpaqueId) } @@ -685,43 +690,43 @@ func (fs *ocfs) getPath(ctx context.Context, id *provider.ResourceId) (string, e return "", errtypes.NotFound(id.OpaqueId) } - return np, nil + return ip, nil } -// GetPathByID returns the fn pointed by the file id, without the internal namespace +// GetPathByID returns the storage relative path for the file id, without the internal namespace func (fs *ocfs) GetPathByID(ctx context.Context, id *provider.ResourceId) (string, error) { - np, err := fs.getPath(ctx, id) + ip, err := fs.getPath(ctx, id) if err != nil { return "", err } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.GetPath { return "", errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return "", errtypes.NotFound(fs.unwrap(ctx, np)) + return "", errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return "", errors.Wrap(err, "ocfs: error reading permissions") } - return fs.unwrap(ctx, np), nil + return fs.toStoragePath(ctx, ip), nil } -// resolve takes in a request path or request id and converts it to a internal path. +// resolve takes in a request path or request id and converts it to an internal path. func (fs *ocfs) resolve(ctx context.Context, ref *provider.Reference) (string, error) { if ref.GetPath() != "" { - return fs.wrap(ctx, ref.GetPath()), nil + return fs.toInternalPath(ctx, ref.GetPath()), nil } if ref.GetId() != nil { - np, err := fs.getPath(ctx, ref.GetId()) + ip, err := fs.getPath(ctx, ref.GetId()) if err != nil { return "", err } - return np, nil + return ip, nil } // reference is invalid @@ -729,40 +734,40 @@ func (fs *ocfs) resolve(ctx context.Context, ref *provider.Reference) (string, e } func (fs *ocfs) AddGrant(ctx context.Context, ref *provider.Reference, g *provider.Grant) error { - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.AddGrant { return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, np)) + return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return errors.Wrap(err, "ocfs: error reading permissions") } e := ace.FromGrant(g) principal, value := e.Marshal() - if err := xattr.Set(np, sharePrefix+principal, value); err != nil { + if err := xattr.Set(ip, sharePrefix+principal, value); err != nil { return err } - return fs.propagate(ctx, np) + return fs.propagate(ctx, ip) } // extractACEsFromAttrs reads ACEs in the list of attrs from the file -func extractACEsFromAttrs(ctx context.Context, fsfn string, attrs []string) (entries []*ace.ACE) { +func extractACEsFromAttrs(ctx context.Context, ip string, attrs []string) (entries []*ace.ACE) { log := appctx.GetLogger(ctx) entries = []*ace.ACE{} for i := range attrs { if strings.HasPrefix(attrs[i], sharePrefix) { var value []byte var err error - if value, err = xattr.Get(fsfn, attrs[i]); err != nil { + if value, err = xattr.Get(ip, attrs[i]); err != nil { log.Error().Err(err).Str("attr", attrs[i]).Msg("could not read attribute") continue } @@ -783,45 +788,47 @@ func extractACEsFromAttrs(ctx context.Context, fsfn string, attrs []string) (ent // Should this change we can store an acl for the owner in every node. // We could also add default acls that can only the admin can set, eg for a read only storage? // Someone needs to write to provide the content that should be read only, so this would likely be an acl for a group anyway. -func (fs *ocfs) readPermissions(ctx context.Context, np string) (p *provider.ResourcePermissions, err error) { +// We need the storage relative path so we can calculate the permissions +// for the node based on all acls in the tree up to the root +func (fs *ocfs) readPermissions(ctx context.Context, ip string) (p *provider.ResourcePermissions, err error) { u, ok := user.ContextGetUser(ctx) if !ok { - appctx.GetLogger(ctx).Debug().Str("np", np).Msg("no user in context, returning default permissions") + appctx.GetLogger(ctx).Debug().Str("ipath", ip).Msg("no user in context, returning default permissions") return defaultPermissions, nil } // check if the current user is the owner - if fs.getOwner(np) == u.Id.OpaqueId { - appctx.GetLogger(ctx).Debug().Str("np", np).Msg("user is owner, returning owner permissions") + if fs.getOwner(ip) == u.Id.OpaqueId { + appctx.GetLogger(ctx).Debug().Str("ipath", ip).Msg("user is owner, returning owner permissions") return ownerPermissions, nil } aggregatedPermissions := &provider.ResourcePermissions{} // add default permissions addPermissions(aggregatedPermissions, defaultPermissions) var e *ace.ACE - e, err = fs.readACE(ctx, np, "u:"+u.Id.OpaqueId) + e, err = fs.readACE(ctx, ip, "u:"+u.Id.OpaqueId) if err == nil { addPermissions(aggregatedPermissions, e.Grant().GetPermissions()) } else if isNoData(err) { err = nil } else { - appctx.GetLogger(ctx).Error().Err(err).Str("np", np).Str("principal", "u:"+u.Id.OpaqueId).Msg("error reading user permissions") + appctx.GetLogger(ctx).Error().Err(err).Str("ipath", ip).Str("principal", "u:"+u.Id.OpaqueId).Msg("error reading user permissions") return nil, err } // check all groups the user is a member of for i := range u.Groups { // groups are just strings ... groupnames ... or group ids ??? AAARGH !!! - e, err = fs.readACE(ctx, np, "g:"+u.Groups[i]) + e, err = fs.readACE(ctx, ip, "g:"+u.Groups[i]) if err == nil { addPermissions(aggregatedPermissions, e.Grant().GetPermissions()) } else if isNoData(err) { err = nil } else { - appctx.GetLogger(ctx).Error().Err(err).Str("np", np).Str("principal", "g:"+u.Groups[i]).Msg("error reading group permissions") + appctx.GetLogger(ctx).Error().Err(err).Str("ipath", ip).Str("principal", "g:"+u.Groups[i]).Msg("error reading group permissions") return nil, err } } // TODO we need to read all parents ... until we find a matching ace? - appctx.GetLogger(ctx).Debug().Interface("permissions", aggregatedPermissions).Str("np", np).Msg("returning aggregated permissions") + appctx.GetLogger(ctx).Debug().Interface("permissions", aggregatedPermissions).Str("ipath", ip).Msg("returning aggregated permissions") return aggregatedPermissions, nil } @@ -834,9 +841,9 @@ func isNoData(err error) bool { return false } -func (fs *ocfs) readACE(ctx context.Context, np string, principal string) (e *ace.ACE, err error) { +func (fs *ocfs) readACE(ctx context.Context, ip string, principal string) (e *ace.ACE, err error) { var b []byte - if b, err = xattr.Get(np, sharePrefix+principal); err != nil { + if b, err = xattr.Get(ip, sharePrefix+principal); err != nil { return nil, err } if e, err = ace.Unmarshal(principal, b); err != nil { @@ -869,32 +876,32 @@ func addPermissions(p1 *provider.ResourcePermissions, p2 *provider.ResourcePermi func (fs *ocfs) ListGrants(ctx context.Context, ref *provider.Reference) (grants []*provider.Grant, err error) { log := appctx.GetLogger(ctx) - var np string - if np, err = fs.resolve(ctx, ref); err != nil { + var ip string + if ip, err = fs.resolve(ctx, ref); err != nil { return nil, errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.ListGrants { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, np)) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } var attrs []string - if attrs, err = xattr.List(np); err != nil { + if attrs, err = xattr.List(ip); err != nil { log.Error().Err(err).Msg("error listing attributes") return nil, err } log.Debug().Interface("attrs", attrs).Msg("read attributes") - aces := extractACEsFromAttrs(ctx, np, attrs) + aces := extractACEsFromAttrs(ctx, ip, attrs) grants = make([]*provider.Grant, 0, len(aces)) for i := range aces { @@ -906,19 +913,19 @@ func (fs *ocfs) ListGrants(ctx context.Context, ref *provider.Reference) (grants func (fs *ocfs) RemoveGrant(ctx context.Context, ref *provider.Reference, g *provider.Grant) (err error) { - var np string - if np, err = fs.resolve(ctx, ref); err != nil { + var ip string + if ip, err = fs.resolve(ctx, ref); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.ListContainer { return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, np)) + return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return errors.Wrap(err, "ocfs: error reading permissions") } @@ -930,37 +937,37 @@ func (fs *ocfs) RemoveGrant(ctx context.Context, ref *provider.Reference, g *pro attr = sharePrefix + "u:" + g.Grantee.Id.OpaqueId } - if err = xattr.Remove(np, attr); err != nil { + if err = xattr.Remove(ip, attr); err != nil { return } - return fs.propagate(ctx, np) + return fs.propagate(ctx, ip) } func (fs *ocfs) UpdateGrant(ctx context.Context, ref *provider.Reference, g *provider.Grant) error { - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.UpdateGrant { return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, np)) + return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return errors.Wrap(err, "ocfs: error reading permissions") } e := ace.FromGrant(g) principal, value := e.Marshal() - if err := xattr.Set(np, sharePrefix+principal, value); err != nil { + if err := xattr.Set(ip, sharePrefix+principal, value); err != nil { return err } - return fs.propagate(ctx, np) + return fs.propagate(ctx, ip) } func (fs *ocfs) GetQuota(ctx context.Context) (int, int, error) { @@ -976,11 +983,11 @@ func (fs *ocfs) CreateHome(ctx context.Context) error { layout := templates.WithUser(u, fs.c.UserLayout) homePaths := []string{ - path.Join(fs.c.DataDirectory, layout, "files"), - path.Join(fs.c.DataDirectory, layout, "files_trashbin"), - path.Join(fs.c.DataDirectory, layout, "files_versions"), - path.Join(fs.c.DataDirectory, layout, "uploads"), - path.Join(fs.c.DataDirectory, layout, "shadow_files"), + filepath.Join(fs.c.DataDirectory, layout, "files"), + filepath.Join(fs.c.DataDirectory, layout, "files_trashbin"), + filepath.Join(fs.c.DataDirectory, layout, "files_versions"), + filepath.Join(fs.c.DataDirectory, layout, "uploads"), + filepath.Join(fs.c.DataDirectory, layout, "shadow_files"), } for _, v := range homePaths { @@ -1000,79 +1007,79 @@ func (fs *ocfs) GetHome(ctx context.Context) (string, error) { return "", nil } -func (fs *ocfs) CreateDir(ctx context.Context, fn string) (err error) { - np := fs.wrap(ctx, fn) +func (fs *ocfs) CreateDir(ctx context.Context, sp string) (err error) { + ip := fs.toInternalPath(ctx, sp) // check permissions of parent dir - if perm, err := fs.readPermissions(ctx, filepath.Dir(np)); err == nil { + if perm, err := fs.readPermissions(ctx, filepath.Dir(ip)); err == nil { if !perm.CreateContainer { return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") } - if err = os.Mkdir(np, 0700); err != nil { + if err = os.Mkdir(ip, 0700); err != nil { if os.IsNotExist(err) { - return errtypes.NotFound(fn) + return errtypes.NotFound(sp) } // FIXME we also need already exists error, webdav expects 405 MethodNotAllowed - return errors.Wrap(err, "ocfs: error creating dir "+np) + return errors.Wrap(err, "ocfs: error creating dir "+ip) } - return fs.propagate(ctx, np) + return fs.propagate(ctx, ip) } -func (fs *ocfs) isShareFolderChild(p string) bool { - return strings.HasPrefix(p, fs.c.ShareFolder) +func (fs *ocfs) isShareFolderChild(sp string) bool { + return strings.HasPrefix(sp, fs.c.ShareFolder) } -func (fs *ocfs) isShareFolderRoot(p string) bool { - return p == fs.c.ShareFolder +func (fs *ocfs) isShareFolderRoot(sp string) bool { + return sp == fs.c.ShareFolder } -func (fs *ocfs) CreateReference(ctx context.Context, p string, targetURI *url.URL) error { - if !fs.isShareFolderChild(p) { - return errtypes.PermissionDenied("ocfs: cannot create references outside the share folder: share_folder=" + "/Shares" + " path=" + p) +func (fs *ocfs) CreateReference(ctx context.Context, sp string, targetURI *url.URL) error { + if !fs.isShareFolderChild(sp) { + return errtypes.PermissionDenied("ocfs: cannot create references outside the share folder: share_folder=" + "/Shares" + " path=" + sp) } - fn := fs.wrapShadow(ctx, p) + ip := fs.toInternalShadowPath(ctx, sp) // TODO check permission? - dir, _ := path.Split(fn) + dir, _ := filepath.Split(ip) if err := os.MkdirAll(dir, 0700); err != nil { return errors.Wrapf(err, "ocfs: error creating shadow path %s", dir) } - f, err := os.Create(fn) + f, err := os.Create(ip) if err != nil { - return errors.Wrapf(err, "ocfs: error creating shadow file %s", fn) + return errors.Wrapf(err, "ocfs: error creating shadow file %s", ip) } err = xattr.FSet(f, mdPrefix+"target", []byte(targetURI.String())) if err != nil { - return errors.Wrapf(err, "ocfs: error setting the target %s on the shadow file %s", targetURI.String(), fn) + return errors.Wrapf(err, "ocfs: error setting the target %s on the shadow file %s", targetURI.String(), ip) } return nil } -func (fs *ocfs) setMtime(ctx context.Context, np string, mtimeString string) error { +func (fs *ocfs) setMtime(ctx context.Context, ip string, mtime string) error { log := appctx.GetLogger(ctx) - if mtime, err := parseMTime(mtimeString); err == nil { + if mt, err := parseMTime(mtime); err == nil { // updating mtime also updates atime - if err := os.Chtimes(np, mtime, mtime); err != nil { + if err := os.Chtimes(ip, mt, mt); err != nil { log.Error().Err(err). - Str("np", np). - Time("mtime", mtime). + Str("ipath", ip). + Time("mtime", mt). Msg("could not set mtime") return errors.Wrap(err, "could not set mtime") } } else { log.Error().Err(err). - Str("np", np). - Str("mtimeString", mtimeString). + Str("ipath", ip). + Str("mtime", mtime). Msg("could not parse mtime") return errors.Wrap(err, "could not parse mtime") } @@ -1081,37 +1088,37 @@ func (fs *ocfs) setMtime(ctx context.Context, np string, mtimeString string) err func (fs *ocfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Reference, md *provider.ArbitraryMetadata) (err error) { log := appctx.GetLogger(ctx) - var np string - if np, err = fs.resolve(ctx, ref); err != nil { + var ip string + if ip, err = fs.resolve(ctx, ref); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.InitiateFileUpload { // TODO add dedicated permission? return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") } var fi os.FileInfo - fi, err = os.Stat(np) + fi, err = os.Stat(ip) if err != nil { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, np)) + return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } - return errors.Wrap(err, "ocfs: error stating "+np) + return errors.Wrap(err, "ocfs: error stating "+ip) } errs := []error{} if md.Metadata != nil { if val, ok := md.Metadata["mtime"]; ok { - err := fs.setMtime(ctx, np, val) + err := fs.setMtime(ctx, ip, val) if err != nil { errs = append(errs, errors.Wrap(err, "could not set mtime")) } @@ -1126,15 +1133,15 @@ func (fs *ocfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Referenc val = fmt.Sprintf("\"%s\"", strings.Trim(val, "\"")) if etag == val { log.Debug(). - Str("np", np). + Str("ipath", ip). Str("etag", val). Msg("ignoring request to update identical etag") } else // etag is only valid until the calculated etag changes // TODO(jfd) cleanup in a batch job - if err := xattr.Set(np, etagPrefix+etag, []byte(val)); err != nil { + if err := xattr.Set(ip, etagPrefix+etag, []byte(val)); err != nil { log.Error().Err(err). - Str("np", np). + Str("ipath", ip). Str("calcetag", etag). Str("etag", val). Msg("could not set etag") @@ -1163,9 +1170,9 @@ func (fs *ocfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Referenc // the favorite flag is specific to the user, so we need to incorporate the userid if uid := u.GetId(); uid != nil { fa := fmt.Sprintf("%s%s@%s", favPrefix, uid.GetOpaqueId(), uid.GetIdp()) - if err := xattr.Set(np, fa, []byte(val)); err != nil { + if err := xattr.Set(ip, fa, []byte(val)); err != nil { log.Error().Err(err). - Str("np", np). + Str("ipath", ip). Interface("user", u). Str("key", fa). Msg("could not set favorite flag") @@ -1173,14 +1180,14 @@ func (fs *ocfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Referenc } } else { log.Error(). - Str("np", np). + Str("ipath", ip). Interface("user", u). Msg("user has no id") errs = append(errs, errors.Wrap(errtypes.UserRequired("userrequired"), "user has no id")) } } else { log.Error(). - Str("np", np). + Str("ipath", ip). Interface("user", u). Msg("error getting user from ctx") errs = append(errs, errors.Wrap(errtypes.UserRequired("userrequired"), "error getting user from ctx")) @@ -1190,9 +1197,9 @@ func (fs *ocfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Referenc } } for k, v := range md.Metadata { - if err := xattr.Set(np, mdPrefix+k, []byte(v)); err != nil { + if err := xattr.Set(ip, mdPrefix+k, []byte(v)); err != nil { log.Error().Err(err). - Str("np", np). + Str("ipath", ip). Str("key", k). Str("val", v). Msg("could not set metadata") @@ -1201,7 +1208,7 @@ func (fs *ocfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Referenc } switch len(errs) { case 0: - return fs.propagate(ctx, np) + return fs.propagate(ctx, ip) case 1: return errs[0] default: @@ -1224,29 +1231,29 @@ func parseMTime(v string) (t time.Time, err error) { func (fs *ocfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Reference, keys []string) (err error) { log := appctx.GetLogger(ctx) - var np string - if np, err = fs.resolve(ctx, ref); err != nil { + var ip string + if ip, err = fs.resolve(ctx, ref); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.InitiateFileUpload { // TODO add dedicated permission? return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, np)) + return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return errors.Wrap(err, "ocfs: error reading permissions") } - _, err = os.Stat(np) + _, err = os.Stat(ip) if err != nil { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, np)) + return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } - return errors.Wrap(err, "ocfs: error stating "+np) + return errors.Wrap(err, "ocfs: error stating "+ip) } errs := []error{} @@ -1257,9 +1264,9 @@ func (fs *ocfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refere // the favorite flag is specific to the user, so we need to incorporate the userid if uid := u.GetId(); uid != nil { fa := fmt.Sprintf("%s%s@%s", favPrefix, uid.GetOpaqueId(), uid.GetIdp()) - if err := xattr.Remove(np, fa); err != nil { + if err := xattr.Remove(ip, fa); err != nil { log.Error().Err(err). - Str("np", np). + Str("ipath", ip). Interface("user", u). Str("key", fa). Msg("could not unset favorite flag") @@ -1267,25 +1274,25 @@ func (fs *ocfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refere } } else { log.Error(). - Str("np", np). + Str("ipath", ip). Interface("user", u). Msg("user has no id") errs = append(errs, errors.Wrap(errtypes.UserRequired("userrequired"), "user has no id")) } } else { log.Error(). - Str("np", np). + Str("ipath", ip). Interface("user", u). Msg("error getting user from ctx") errs = append(errs, errors.Wrap(errtypes.UserRequired("userrequired"), "error getting user from ctx")) } default: - if err = xattr.Remove(np, mdPrefix+k); err != nil { + if err = xattr.Remove(ip, mdPrefix+k); err != nil { // a non-existing attribute will return an error, which we can ignore // (using string compare because the error type is syscall.Errno and not wrapped/recognizable) if e, ok := err.(*xattr.Error); !ok || e.Err.Error() != "no data available" { log.Error().Err(err). - Str("np", np). + Str("ipath", ip). Str("key", k). Msg("could not unset metadata") errs = append(errs, errors.Wrap(err, "could not unset metadata")) @@ -1296,7 +1303,7 @@ func (fs *ocfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refere switch len(errs) { case 0: - return fs.propagate(ctx, np) + return fs.propagate(ctx, ip) case 1: return errs[0] default: @@ -1315,29 +1322,29 @@ func (fs *ocfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refere // We will live with that compromise since this storage driver will be // deprecated soon. func (fs *ocfs) Delete(ctx context.Context, ref *provider.Reference) (err error) { - var np string - if np, err = fs.resolve(ctx, ref); err != nil { + var ip string + if ip, err = fs.resolve(ctx, ref); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.Delete { return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") } - _, err = os.Stat(np) + _, err = os.Stat(ip) if err != nil { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, np)) + return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } - return errors.Wrap(err, "ocfs: error stating "+np) + return errors.Wrap(err, "ocfs: error stating "+ip) } rp, err := fs.getRecyclePath(ctx) @@ -1349,45 +1356,45 @@ func (fs *ocfs) Delete(ctx context.Context, ref *provider.Reference) (err error) return errors.Wrap(err, "ocfs: error creating trashbin dir "+rp) } - // np is the path on disk ... we need only the path relative to root - origin := path.Dir(fs.unwrap(ctx, np)) + // ip is the path on disk ... we need only the path relative to root + origin := filepath.Dir(fs.toStoragePath(ctx, ip)) - err = fs.trash(ctx, np, rp, origin) + err = fs.trash(ctx, ip, rp, origin) if err != nil { - return errors.Wrapf(err, "ocfs: error deleting file %s", np) + return errors.Wrapf(err, "ocfs: error deleting file %s", ip) } - err = fs.trashVersions(ctx, np, origin) + err = fs.trashVersions(ctx, ip, origin) if err != nil { - return errors.Wrapf(err, "ocfs: error deleting versions of file %s", np) + return errors.Wrapf(err, "ocfs: error deleting versions of file %s", ip) } return nil } -func (fs *ocfs) trash(ctx context.Context, np string, rp string, origin string) error { +func (fs *ocfs) trash(ctx context.Context, ip string, rp string, origin string) error { // set origin location in metadata - if err := xattr.Set(np, trashOriginPrefix, []byte(origin)); err != nil { + if err := xattr.Set(ip, trashOriginPrefix, []byte(origin)); err != nil { return err } // move to trash location dtime := time.Now().Unix() - tgt := path.Join(rp, fmt.Sprintf("%s.d%d", path.Base(np), dtime)) - if err := os.Rename(np, tgt); err != nil { + tgt := filepath.Join(rp, fmt.Sprintf("%s.d%d", filepath.Base(ip), dtime)) + if err := os.Rename(ip, tgt); err != nil { if os.IsExist(err) { // timestamp collision, try again with higher value: dtime++ - tgt := path.Join(rp, fmt.Sprintf("%s.d%d", path.Base(np), dtime)) - if err := os.Rename(np, tgt); err != nil { + tgt := filepath.Join(rp, fmt.Sprintf("%s.d%d", filepath.Base(ip), dtime)) + if err := os.Rename(ip, tgt); err != nil { return errors.Wrap(err, "ocfs: could not move item to trash") } } } - return fs.propagate(ctx, path.Dir(np)) + return fs.propagate(ctx, filepath.Dir(ip)) } -func (fs *ocfs) trashVersions(ctx context.Context, np string, origin string) error { - vp := fs.getVersionsPath(ctx, np) +func (fs *ocfs) trashVersions(ctx context.Context, ip string, origin string) error { + vp := fs.getVersionsPath(ctx, ip) vrp, err := fs.getVersionRecyclePath(ctx) if err != nil { return errors.Wrap(err, "error resolving versions recycle path") @@ -1409,51 +1416,51 @@ func (fs *ocfs) trashVersions(ctx context.Context, np string, origin string) err } func (fs *ocfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (err error) { - var oldName string - if oldName, err = fs.resolve(ctx, oldRef); err != nil { + var oldIP string + if oldIP, err = fs.resolve(ctx, oldRef); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, oldName); err == nil { + if perm, err := fs.readPermissions(ctx, oldIP); err == nil { if !perm.Move { // TODO add dedicated permission? return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(oldName))) + return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(oldIP))) } return errors.Wrap(err, "ocfs: error reading permissions") } - var newName string - if newName, err = fs.resolve(ctx, newRef); err != nil { + var newIP string + if newIP, err = fs.resolve(ctx, newRef); err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // TODO check target permissions ... if it exists - if err = os.Rename(oldName, newName); err != nil { - return errors.Wrap(err, "ocfs: error moving "+oldName+" to "+newName) + if err = os.Rename(oldIP, newIP); err != nil { + return errors.Wrap(err, "ocfs: error moving "+oldIP+" to "+newIP) } - if err := fs.propagate(ctx, newName); err != nil { + if err := fs.propagate(ctx, newIP); err != nil { return err } - if err := fs.propagate(ctx, path.Dir(oldName)); err != nil { + if err := fs.propagate(ctx, filepath.Dir(oldIP)); err != nil { return err } return nil } func (fs *ocfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { return nil, err } return nil, errors.Wrap(err, "ocfs: error resolving reference") } - p := fs.unwrap(ctx, np) + p := fs.toStoragePath(ctx, ip) if fs.c.EnableHome { if fs.isShareFolderChild(p) { @@ -1464,62 +1471,62 @@ func (fs *ocfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []str // If GetMD is called for a path shared with the user then the path is // already wrapped. (fs.resolve wraps the path) if strings.HasPrefix(p, fs.c.DataDirectory) { - np = p + ip = p } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.Stat { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } - md, err := os.Stat(np) + md, err := os.Stat(ip) if err != nil { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, np)) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, ip)) } - return nil, errors.Wrap(err, "ocfs: error stating "+np) + return nil, errors.Wrap(err, "ocfs: error stating "+ip) } c := fs.pool.Get() defer c.Close() - m := fs.convertToResourceInfo(ctx, md, np, fs.unwrap(ctx, np), c, mdKeys) + m := fs.convertToResourceInfo(ctx, md, ip, fs.toStoragePath(ctx, ip), c, mdKeys) return m, nil } -func (fs *ocfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string) (*provider.ResourceInfo, error) { - np := fs.wrapShadow(ctx, p) +func (fs *ocfs) getMDShareFolder(ctx context.Context, sp string, mdKeys []string) (*provider.ResourceInfo, error) { + ip := fs.toInternalShadowPath(ctx, sp) // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.Stat { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } - md, err := os.Stat(np) + md, err := os.Stat(ip) if err != nil { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrapShadow(ctx, np)) + return nil, errtypes.NotFound(fs.toStorageShadowPath(ctx, ip)) } - return nil, errors.Wrapf(err, "ocfs: error stating %s", np) + return nil, errors.Wrapf(err, "ocfs: error stating %s", ip) } c := fs.pool.Get() defer c.Close() - m := fs.convertToResourceInfo(ctx, md, np, fs.unwrapShadow(ctx, np), c, mdKeys) - if !fs.isShareFolderRoot(p) { + m := fs.convertToResourceInfo(ctx, md, ip, fs.toStorageShadowPath(ctx, ip), c, mdKeys) + if !fs.isShareFolderRoot(sp) { m.Type = provider.ResourceType_RESOURCE_TYPE_REFERENCE - ref, err := xattr.Get(np, mdPrefix+"target") + ref, err := xattr.Get(ip, mdPrefix+"target") if err != nil { return nil, err } @@ -1532,56 +1539,56 @@ func (fs *ocfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string) func (fs *ocfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { log := appctx.GetLogger(ctx) - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { return nil, errors.Wrap(err, "ocfs: error resolving reference") } - p := fs.unwrap(ctx, np) + sp := fs.toStoragePath(ctx, ip) if fs.c.EnableHome { log.Debug().Msg("home enabled") - if strings.HasPrefix(p, "/") { + if strings.HasPrefix(sp, "/") { // permissions checked in listWithHome - return fs.listWithHome(ctx, "/", p, mdKeys) + return fs.listWithHome(ctx, "/", sp, mdKeys) } } log.Debug().Msg("list with nominal home") // permissions checked in listWithNominalHome - return fs.listWithNominalHome(ctx, p, mdKeys) + return fs.listWithNominalHome(ctx, sp, mdKeys) } -func (fs *ocfs) listWithNominalHome(ctx context.Context, p string, mdKeys []string) ([]*provider.ResourceInfo, error) { - np := p +func (fs *ocfs) listWithNominalHome(ctx context.Context, ip string, mdKeys []string) ([]*provider.ResourceInfo, error) { + // If a user wants to list a folder shared with him the path will already // be wrapped with the files directory path of the share owner. // In that case we don't want to wrap the path again. - if !strings.HasPrefix(p, fs.c.DataDirectory) { - np = fs.wrap(ctx, p) + if !strings.HasPrefix(ip, fs.c.DataDirectory) { + ip = fs.toInternalPath(ctx, ip) } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.ListContainer { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } - mds, err := ioutil.ReadDir(np) + mds, err := ioutil.ReadDir(ip) if err != nil { - return nil, errors.Wrapf(err, "ocfs: error listing %s", np) + return nil, errors.Wrapf(err, "ocfs: error listing %s", ip) } c := fs.pool.Get() defer c.Close() finfos := []*provider.ResourceInfo{} for _, md := range mds { - p := path.Join(np, md.Name()) - m := fs.convertToResourceInfo(ctx, md, p, fs.unwrap(ctx, p), c, mdKeys) + cp := filepath.Join(ip, md.Name()) + m := fs.convertToResourceInfo(ctx, md, cp, fs.toStoragePath(ctx, cp), c, mdKeys) finfos = append(finfos, m) } return finfos, nil @@ -1609,21 +1616,21 @@ func (fs *ocfs) listWithHome(ctx context.Context, home, p string, mdKeys []strin func (fs *ocfs) listHome(ctx context.Context, home string, mdKeys []string) ([]*provider.ResourceInfo, error) { // list files - np := fs.wrap(ctx, home) + ip := fs.toInternalPath(ctx, home) // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.ListContainer { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } - mds, err := ioutil.ReadDir(np) + mds, err := ioutil.ReadDir(ip) if err != nil { return nil, errors.Wrap(err, "ocfs: error listing files") } @@ -1633,41 +1640,41 @@ func (fs *ocfs) listHome(ctx context.Context, home string, mdKeys []string) ([]* finfos := []*provider.ResourceInfo{} for _, md := range mds { - p := path.Join(np, md.Name()) - m := fs.convertToResourceInfo(ctx, md, p, fs.unwrap(ctx, p), c, mdKeys) + cp := filepath.Join(ip, md.Name()) + m := fs.convertToResourceInfo(ctx, md, cp, fs.toStoragePath(ctx, cp), c, mdKeys) finfos = append(finfos, m) } // list shadow_files - np = fs.wrapShadow(ctx, home) - mds, err = ioutil.ReadDir(np) + ip = fs.toInternalShadowPath(ctx, home) + mds, err = ioutil.ReadDir(ip) if err != nil { return nil, errors.Wrap(err, "ocfs: error listing shadow_files") } for _, md := range mds { - p := path.Join(np, md.Name()) - m := fs.convertToResourceInfo(ctx, md, p, fs.unwrapShadow(ctx, p), c, mdKeys) + cp := filepath.Join(ip, md.Name()) + m := fs.convertToResourceInfo(ctx, md, cp, fs.toStorageShadowPath(ctx, cp), c, mdKeys) finfos = append(finfos, m) } return finfos, nil } -func (fs *ocfs) listShareFolderRoot(ctx context.Context, p string, mdKeys []string) ([]*provider.ResourceInfo, error) { - np := fs.wrapShadow(ctx, p) +func (fs *ocfs) listShareFolderRoot(ctx context.Context, sp string, mdKeys []string) ([]*provider.ResourceInfo, error) { + ip := fs.toInternalShadowPath(ctx, sp) // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.ListContainer { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } - mds, err := ioutil.ReadDir(np) + mds, err := ioutil.ReadDir(ip) if err != nil { return nil, errors.Wrap(err, "ocfs: error listing shadow_files") } @@ -1677,10 +1684,10 @@ func (fs *ocfs) listShareFolderRoot(ctx context.Context, p string, mdKeys []stri finfos := []*provider.ResourceInfo{} for _, md := range mds { - p := path.Join(np, md.Name()) - m := fs.convertToResourceInfo(ctx, md, p, fs.unwrapShadow(ctx, p), c, mdKeys) + cp := filepath.Join(ip, md.Name()) + m := fs.convertToResourceInfo(ctx, md, cp, fs.toStorageShadowPath(ctx, cp), c, mdKeys) m.Type = provider.ResourceType_RESOURCE_TYPE_REFERENCE - ref, err := xattr.Get(p, mdPrefix+"target") + ref, err := xattr.Get(cp, mdPrefix+"target") if err != nil { return nil, err } @@ -1691,16 +1698,16 @@ func (fs *ocfs) listShareFolderRoot(ctx context.Context, p string, mdKeys []stri return finfos, nil } -func (fs *ocfs) archiveRevision(ctx context.Context, vbp string, np string) error { +func (fs *ocfs) archiveRevision(ctx context.Context, vbp string, ip string) error { // move existing file to versions dir vp := fmt.Sprintf("%s.v%d", vbp, time.Now().Unix()) - if err := os.MkdirAll(path.Dir(vp), 0700); err != nil { + if err := os.MkdirAll(filepath.Dir(vp), 0700); err != nil { return errors.Wrap(err, "ocfs: error creating versions dir "+vp) } // TODO(jfd): make sure rename is atomic, missing fsync ... - if err := os.Rename(np, vp); err != nil { - return errors.Wrap(err, "ocfs: error renaming from "+np+" to "+vp) + if err := os.Rename(ip, vp); err != nil { + return errors.Wrap(err, "ocfs: error renaming from "+ip+" to "+vp) } return nil @@ -1726,59 +1733,59 @@ func (fs *ocfs) copyMD(s string, t string) (err error) { } func (fs *ocfs) Download(ctx context.Context, ref *provider.Reference) (io.ReadCloser, error) { - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { return nil, errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.InitiateFileDownload { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } - r, err := os.Open(np) + r, err := os.Open(ip) if err != nil { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, np)) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, ip)) } - return nil, errors.Wrap(err, "ocfs: error reading "+np) + return nil, errors.Wrap(err, "ocfs: error reading "+ip) } return r, nil } func (fs *ocfs) ListRevisions(ctx context.Context, ref *provider.Reference) ([]*provider.FileVersion, error) { - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { return nil, errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.ListFileVersions { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } - vp := fs.getVersionsPath(ctx, np) + vp := fs.getVersionsPath(ctx, ip) - bn := path.Base(np) + bn := filepath.Base(ip) revisions := []*provider.FileVersion{} - mds, err := ioutil.ReadDir(path.Dir(vp)) + mds, err := ioutil.ReadDir(filepath.Dir(vp)) if err != nil { - return nil, errors.Wrap(err, "ocfs: error reading"+path.Dir(vp)) + return nil, errors.Wrap(err, "ocfs: error reading"+filepath.Dir(vp)) } for i := range mds { rev := fs.filterAsRevision(ctx, bn, mds[i]) @@ -1815,24 +1822,24 @@ func (fs *ocfs) DownloadRevision(ctx context.Context, ref *provider.Reference, r } func (fs *ocfs) RestoreRevision(ctx context.Context, ref *provider.Reference, revisionKey string) error { - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } // check permissions - if perm, err := fs.readPermissions(ctx, np); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.RestoreFileVersion { return errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") } - vp := fs.getVersionsPath(ctx, np) + vp := fs.getVersionsPath(ctx, ip) rp := vp + ".v" + revisionKey // check revision exists @@ -1852,11 +1859,11 @@ func (fs *ocfs) RestoreRevision(ctx context.Context, ref *provider.Reference, re defer source.Close() // destination should be available, otherwise we could not have navigated to its revisions - if err := fs.archiveRevision(ctx, fs.getVersionsPath(ctx, np), np); err != nil { + if err := fs.archiveRevision(ctx, fs.getVersionsPath(ctx, ip), ip); err != nil { return err } - destination, err := os.Create(np) + destination, err := os.Create(ip) if err != nil { // TODO(jfd) bring back revision in case sth goes wrong? return err @@ -1870,7 +1877,7 @@ func (fs *ocfs) RestoreRevision(ctx context.Context, ref *provider.Reference, re } // TODO(jfd) bring back revision in case sth goes wrong? - return fs.propagate(ctx, np) + return fs.propagate(ctx, ip) } func (fs *ocfs) PurgeRecycleItem(ctx context.Context, key string) error { @@ -1878,18 +1885,18 @@ func (fs *ocfs) PurgeRecycleItem(ctx context.Context, key string) error { if err != nil { return errors.Wrap(err, "ocfs: error resolving recycle path") } - ip := path.Join(rp, path.Clean(key)) + ip := filepath.Join(rp, filepath.Clean(key)) // TODO check permission? // check permissions /* are they stored in the trash? - if perm, err := fs.readPermissions(ctx, fn); err == nil { + if perm, err := fs.readPermissions(ctx, ip); err == nil { if !perm.ListContainer { return nil, errtypes.PermissionDenied("") } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } @@ -1899,7 +1906,7 @@ func (fs *ocfs) PurgeRecycleItem(ctx context.Context, key string) error { if err != nil { return errors.Wrap(err, "ocfs: error deleting recycle item") } - err = os.RemoveAll(path.Join(path.Dir(rp), "versions", path.Clean(key))) + err = os.RemoveAll(filepath.Join(filepath.Dir(rp), "versions", filepath.Clean(key))) if err != nil { return errors.Wrap(err, "ocfs: error deleting recycle item versions") } @@ -1917,7 +1924,7 @@ func (fs *ocfs) EmptyRecycle(ctx context.Context) error { if err != nil { return errors.Wrap(err, "ocfs: error deleting recycle files") } - err = os.RemoveAll(path.Join(path.Dir(rp), "versions")) + err = os.RemoveAll(filepath.Join(filepath.Dir(rp), "versions")) if err != nil { return errors.Wrap(err, "ocfs: error deleting recycle files versions") } @@ -1927,7 +1934,7 @@ func (fs *ocfs) EmptyRecycle(ctx context.Context) error { func (fs *ocfs) convertToRecycleItem(ctx context.Context, rp string, md os.FileInfo) *provider.RecycleItem { // trashbin items have filename.ext.d12345678 - suffix := path.Ext(md.Name()) + suffix := filepath.Ext(md.Name()) if len(suffix) == 0 || !strings.HasPrefix(suffix, ".d") { log := appctx.GetLogger(ctx) log.Error().Str("path", md.Name()).Msg("invalid trash item suffix") @@ -1941,7 +1948,7 @@ func (fs *ocfs) convertToRecycleItem(ctx context.Context, rp string, md os.FileI return nil } var v []byte - if v, err = xattr.Get(path.Join(rp, md.Name()), trashOriginPrefix); err != nil { + if v, err = xattr.Get(filepath.Join(rp, md.Name()), trashOriginPrefix); err != nil { log := appctx.GetLogger(ctx) log.Error().Err(err).Str("path", md.Name()).Msg("could not read origin") return nil @@ -1949,7 +1956,7 @@ func (fs *ocfs) convertToRecycleItem(ctx context.Context, rp string, md os.FileI // ownCloud 10 stores the parent dir of the deleted item as the location in the oc_files_trashbin table // we use extended attributes for original location, but also only the parent location, which is why // we need to join and trim the path when listing it - originalPath := path.Join(string(v), strings.TrimSuffix(path.Base(md.Name()), suffix)) + originalPath := filepath.Join(string(v), strings.TrimSuffix(filepath.Base(md.Name()), suffix)) return &provider.RecycleItem{ Type: getResourceType(md.IsDir()), @@ -1998,9 +2005,9 @@ func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error { if err != nil { return errors.Wrap(err, "ocfs: error resolving recycle path") } - src := path.Join(rp, path.Clean(key)) + src := filepath.Join(rp, filepath.Clean(key)) - suffix := path.Ext(src) + suffix := filepath.Ext(src) if len(suffix) == 0 || !strings.HasPrefix(suffix, ".d") { log.Error().Str("key", key).Str("path", src).Msg("invalid trash item suffix") return nil @@ -2010,9 +2017,9 @@ func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error { if v, err := xattr.Get(src, trashOriginPrefix); err != nil { log.Error().Err(err).Str("key", key).Str("path", src).Msg("could not read origin") } else { - origin = path.Clean(string(v)) + origin = filepath.Clean(string(v)) } - tgt := fs.wrap(ctx, path.Join("/", origin, strings.TrimSuffix(path.Base(src), suffix))) + tgt := fs.toInternalPath(ctx, filepath.Join("/", origin, strings.TrimSuffix(filepath.Base(src), suffix))) // move back to original location if err := os.Rename(src, tgt); err != nil { log.Error().Err(err).Str("key", key).Str("origin", origin).Str("src", src).Str("tgt", tgt).Msg("could not restore item") @@ -2031,10 +2038,10 @@ func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error { func (fs *ocfs) propagate(ctx context.Context, leafPath string) error { var root string if fs.c.EnableHome { - root = fs.wrap(ctx, "/") + root = fs.toInternalPath(ctx, "/") } else { owner := fs.getOwner(leafPath) - root = fs.wrap(ctx, owner) + root = fs.toInternalPath(ctx, owner) } if !strings.HasPrefix(leafPath, root) { err := errors.New("internal path outside root") @@ -2066,7 +2073,7 @@ func (fs *ocfs) propagate(ctx context.Context, leafPath string) error { Int("i", i). Interface("parts", parts). Msg("propagating change") - if err := os.Chtimes(path.Join(root), fi.ModTime(), fi.ModTime()); err != nil { + if err := os.Chtimes(filepath.Join(root), fi.ModTime(), fi.ModTime()); err != nil { appctx.GetLogger(ctx).Error(). Err(err). Str("leafPath", leafPath). @@ -2074,7 +2081,7 @@ func (fs *ocfs) propagate(ctx context.Context, leafPath string) error { Msg("could not propagate change") return err } - root = path.Join(root, parts[i]) + root = filepath.Join(root, parts[i]) } return nil } diff --git a/pkg/storage/fs/owncloud/upload.go b/pkg/storage/fs/owncloud/upload.go index 156f6e7e7f..baf2699262 100644 --- a/pkg/storage/fs/owncloud/upload.go +++ b/pkg/storage/fs/owncloud/upload.go @@ -42,7 +42,7 @@ var defaultFilePerm = os.FileMode(0664) // TODO deprecated ... use tus func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCloser) error { - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { return errors.Wrap(err, "ocfs: error resolving reference") } @@ -50,12 +50,12 @@ func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCl var perm *provider.ResourcePermissions var perr error // if destination exists - if _, err := os.Stat(np); err == nil { + if _, err := os.Stat(ip); err == nil { // check permissions of file to be overwritten - perm, perr = fs.readPermissions(ctx, np) + perm, perr = fs.readPermissions(ctx, ip) } else { // check permissions - perm, perr = fs.readPermissions(ctx, filepath.Base(np)) + perm, perr = fs.readPermissions(ctx, filepath.Base(ip)) } if perr == nil { if !perm.InitiateFileUpload { @@ -63,7 +63,7 @@ func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCl } } else { if os.IsNotExist(err) { - return errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") } @@ -71,9 +71,9 @@ func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCl // we cannot rely on /tmp as it can live in another partition and we can // hit invalid cross-device link errors, so we create the tmp file in the same directory // the file is supposed to be written. - tmp, err := ioutil.TempFile(filepath.Dir(np), "._reva_atomic_upload") + tmp, err := ioutil.TempFile(filepath.Dir(ip), "._reva_atomic_upload") if err != nil { - return errors.Wrap(err, "ocfs: error creating tmp fn at "+filepath.Dir(np)) + return errors.Wrap(err, "ocfs: error creating tmp file at "+filepath.Dir(ip)) } _, err = io.Copy(tmp, r) @@ -82,20 +82,20 @@ func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCl } // if destination exists - if _, err := os.Stat(np); err == nil { + if _, err := os.Stat(ip); err == nil { // copy attributes of existing file to tmp file - if err := fs.copyMD(np, tmp.Name()); err != nil { - return errors.Wrap(err, "ocfs: error copying metadata from "+np+" to "+tmp.Name()) + if err := fs.copyMD(ip, tmp.Name()); err != nil { + return errors.Wrap(err, "ocfs: error copying metadata from "+ip+" to "+tmp.Name()) } // create revision - if err := fs.archiveRevision(ctx, fs.getVersionsPath(ctx, np), np); err != nil { + if err := fs.archiveRevision(ctx, fs.getVersionsPath(ctx, ip), ip); err != nil { return err } } // TODO(jfd): make sure rename is atomic, missing fsync ... - if err := os.Rename(tmp.Name(), np); err != nil { - return errors.Wrap(err, "ocfs: error renaming from "+tmp.Name()+" to "+np) + if err := os.Rename(tmp.Name(), ip); err != nil { + return errors.Wrap(err, "ocfs: error renaming from "+tmp.Name()+" to "+ip) } return nil @@ -104,7 +104,7 @@ func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCl // InitiateUpload returns an upload id that can be used for uploads with tus // TODO read optional content for small files in this request func (fs *ocfs) InitiateUpload(ctx context.Context, ref *provider.Reference, uploadLength int64, metadata map[string]string) (uploadID string, err error) { - np, err := fs.resolve(ctx, ref) + ip, err := fs.resolve(ctx, ref) if err != nil { return "", errors.Wrap(err, "ocfs: error resolving reference") } @@ -113,12 +113,12 @@ func (fs *ocfs) InitiateUpload(ctx context.Context, ref *provider.Reference, upl var perm *provider.ResourcePermissions var perr error // if destination exists - if _, err := os.Stat(np); err == nil { + if _, err := os.Stat(ip); err == nil { // check permissions of file to be overwritten - perm, perr = fs.readPermissions(ctx, np) + perm, perr = fs.readPermissions(ctx, ip) } else { // check permissions - perm, perr = fs.readPermissions(ctx, filepath.Base(np)) + perm, perr = fs.readPermissions(ctx, filepath.Base(ip)) } if perr == nil { if !perm.InitiateFileUpload { @@ -126,12 +126,12 @@ func (fs *ocfs) InitiateUpload(ctx context.Context, ref *provider.Reference, upl } } else { if os.IsNotExist(err) { - return "", errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return "", errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return "", errors.Wrap(err, "ocfs: error reading permissions") } - p := fs.unwrap(ctx, np) + p := fs.toStoragePath(ctx, ip) info := tusd.FileInfo{ MetaData: tusd.MetaData{ @@ -172,8 +172,7 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd. log := appctx.GetLogger(ctx) log.Debug().Interface("info", info).Msg("ocfs: NewUpload") - fn := info.MetaData["filename"] - if fn == "" { + if info.MetaData["filename"] == "" { return nil, errors.New("ocfs: missing filename in metadata") } info.MetaData["filename"] = filepath.Clean(info.MetaData["filename"]) @@ -184,18 +183,18 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd. } info.MetaData["dir"] = filepath.Clean(info.MetaData["dir"]) - np := fs.wrap(ctx, filepath.Join(info.MetaData["dir"], info.MetaData["filename"])) + ip := fs.toInternalPath(ctx, filepath.Join(info.MetaData["dir"], info.MetaData["filename"])) // check permissions var perm *provider.ResourcePermissions var perr error // if destination exists - if _, err := os.Stat(np); err == nil { + if _, err := os.Stat(ip); err == nil { // check permissions of file to be overwritten - perm, perr = fs.readPermissions(ctx, np) + perm, perr = fs.readPermissions(ctx, ip) } else { // check permissions - perm, perr = fs.readPermissions(ctx, filepath.Base(np)) + perm, perr = fs.readPermissions(ctx, filepath.Base(ip)) } if perr == nil { if !perm.InitiateFileUpload { @@ -203,7 +202,7 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd. } } else { if os.IsNotExist(err) { - return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(np))) + return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") } @@ -220,7 +219,7 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd. info.Storage = map[string]string{ "Type": "OwnCloudStore", "BinPath": binPath, - "InternalDestination": np, + "InternalDestination": ip, "Idp": usr.Id.Idp, "UserId": usr.Id.OpaqueId, @@ -396,27 +395,27 @@ func (upload *fileUpload) FinishUpload(ctx context.Context) error { } */ - np := upload.info.Storage["InternalDestination"] + ip := upload.info.Storage["InternalDestination"] // if destination exists // TODO check etag with If-Match header - if _, err := os.Stat(np); err == nil { + if _, err := os.Stat(ip); err == nil { // copy attributes of existing file to tmp file - if err := upload.fs.copyMD(np, upload.binPath); err != nil { - return errors.Wrap(err, "ocfs: error copying metadata from "+np+" to "+upload.binPath) + if err := upload.fs.copyMD(ip, upload.binPath); err != nil { + return errors.Wrap(err, "ocfs: error copying metadata from "+ip+" to "+upload.binPath) } // create revision - if err := upload.fs.archiveRevision(upload.ctx, upload.fs.getVersionsPath(upload.ctx, np), np); err != nil { + if err := upload.fs.archiveRevision(upload.ctx, upload.fs.getVersionsPath(upload.ctx, ip), ip); err != nil { return err } } log := appctx.GetLogger(upload.ctx) - err := os.Rename(upload.binPath, np) + err := os.Rename(upload.binPath, ip) if err != nil { log.Err(err).Interface("info", upload.info). Str("binPath", upload.binPath). - Str("np", np). + Str("ipath", ip). Msg("ocfs: could not rename") return err } @@ -430,14 +429,14 @@ func (upload *fileUpload) FinishUpload(ctx context.Context) error { } if upload.info.MetaData["mtime"] != "" { - err := upload.fs.setMtime(ctx, np, upload.info.MetaData["mtime"]) + err := upload.fs.setMtime(ctx, ip, upload.info.MetaData["mtime"]) if err != nil { log.Err(err).Interface("info", upload.info).Msg("ocfs: could not set mtime metadata") return err } } - return upload.fs.propagate(upload.ctx, np) + return upload.fs.propagate(upload.ctx, ip) } // To implement the termination extension as specified in https://tus.io/protocols/resumable-upload.html#termination From 25c8e4614d0285f308b2c82a8e8c8e45ab34dd76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 25 Sep 2020 16:13:16 +0200 Subject: [PATCH 03/13] think through permissions checks 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 | 92 +++++++++++++++++++++++------ pkg/storage/utils/ace/ace.go | 2 +- 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 346094401b..4bef9ec644 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -791,6 +791,7 @@ func extractACEsFromAttrs(ctx context.Context, ip string, attrs []string) (entri // We need the storage relative path so we can calculate the permissions // for the node based on all acls in the tree up to the root func (fs *ocfs) readPermissions(ctx context.Context, ip string) (p *provider.ResourcePermissions, err error) { + u, ok := user.ContextGetUser(ctx) if !ok { appctx.GetLogger(ctx).Debug().Str("ipath", ip).Msg("no user in context, returning default permissions") @@ -801,32 +802,85 @@ func (fs *ocfs) readPermissions(ctx context.Context, ip string) (p *provider.Res appctx.GetLogger(ctx).Debug().Str("ipath", ip).Msg("user is owner, returning owner permissions") return ownerPermissions, nil } + + // for non owners this is a little more complicated: aggregatedPermissions := &provider.ResourcePermissions{} // add default permissions addPermissions(aggregatedPermissions, defaultPermissions) - var e *ace.ACE - e, err = fs.readACE(ctx, ip, "u:"+u.Id.OpaqueId) - if err == nil { - addPermissions(aggregatedPermissions, e.Grant().GetPermissions()) - } else if isNoData(err) { - err = nil - } else { - appctx.GetLogger(ctx).Error().Err(err).Str("ipath", ip).Str("principal", "u:"+u.Id.OpaqueId).Msg("error reading user permissions") - return nil, err - } - // check all groups the user is a member of + + // determine root + rp := fs.toInternalPath(ctx, "") + // TODO rp will be the datadir ... be we don't want to go up that high. The users home is far enough + np := ip + + // for an efficient group lookup convert the list of groups to a map + // groups are just strings ... groupnames ... or group ids ??? AAARGH !!! + groupsMap := make(map[string]bool, len(u.Groups)) for i := range u.Groups { - // groups are just strings ... groupnames ... or group ids ??? AAARGH !!! - e, err = fs.readACE(ctx, ip, "g:"+u.Groups[i]) - if err == nil { - addPermissions(aggregatedPermissions, e.Grant().GetPermissions()) - } else if isNoData(err) { - err = nil - } else { - appctx.GetLogger(ctx).Error().Err(err).Str("ipath", ip).Str("principal", "g:"+u.Groups[i]).Msg("error reading group permissions") + groupsMap[u.Groups[i]] = true + } + + var e *ace.ACE + // for all segments, starting at the leaf + for np != rp { + + var attrs []string + if attrs, err = xattr.List(ip); err != nil { + appctx.GetLogger(ctx).Error().Err(err).Str("ipath", np).Msg("error listing attributes") return nil, err } + + userace := sharePrefix + "u:" + u.Id.OpaqueId + userFound := false + for i := range attrs { + // we only need the find the user once per node + if !userFound && attrs[i] == userace { + e, err = fs.readACE(ctx, np, "u:"+u.Id.OpaqueId) + } else if strings.HasPrefix(attrs[i], sharePrefix+"g:") { + g := strings.TrimPrefix(attrs[i], sharePrefix+"g:") + if groupsMap[g] { + e, err = fs.readACE(ctx, np, "g:"+g) + } else { + // no need to check attribute + continue + } + } else { + // no need to check attribute + continue + } + if err == nil { + addPermissions(aggregatedPermissions, e.Grant().GetPermissions()) + appctx.GetLogger(ctx).Debug().Str("ipath", np).Str("principal", strings.TrimPrefix(attrs[i], sharePrefix)).Interface("permissions", aggregatedPermissions).Msg("adding permissions") + } else if isNoData(err) { + err = nil + appctx.GetLogger(ctx).Error().Str("ipath", np).Str("principal", strings.TrimPrefix(attrs[i], sharePrefix)).Interface("attrs", attrs).Msg("no permissions found on node, but they were listed") + } else { + appctx.GetLogger(ctx).Error().Err(err).Str("ipath", np).Str("principal", strings.TrimPrefix(attrs[i], sharePrefix)).Msg("error reading permissions") + return nil, err + } + } + + np = filepath.Dir(np) } + + // 3. read user permissions until one is found? + // what if, when checking /a/b/c/d, /a/b has write permission, but /a/b/c has not? + // those are two shares one read only, and a higher one rw, + // should the higher one be used? + // or, since we did find a matching ace in a lower node use that because it matches the principal? + // this would allow ai user to share a folder rm but take away the write capability for eg a docs folder inside it. + // 4. read group permissions until all groups of the user are matched? + // same as for user permission, but we need to keep going further up the tree until all groups of the user were matched. + // what if a user has thousands of groups? + // we will always have to walk to the root. + // but the same problem occurs for a user with 2 groups but where only one group was used to share. + // in any case we need to iterate the aces, not the number of groups of the user. + // listing the aces can be used to match the principals, we do not need to fully real all aces + // what if, when checking /a/b/c/d, /a/b has write permission for group g, but /a/b/c has an ace for another group h the user is also a member of? + // it would allow restricting a users permissions by resharing something with him with lower permission? + // so if you have reshare permissons you could accidentially restrict users access to a subfolder of a rw share to ro by sharing it to another group as ro when they are part of both groups + // it makes more sense to have explicit negative permissions + // TODO we need to read all parents ... until we find a matching ace? appctx.GetLogger(ctx).Debug().Interface("permissions", aggregatedPermissions).Str("ipath", ip).Msg("returning aggregated permissions") return aggregatedPermissions, nil diff --git a/pkg/storage/utils/ace/ace.go b/pkg/storage/utils/ace/ace.go index 14154bf468..02ea3d762d 100644 --- a/pkg/storage/utils/ace/ace.go +++ b/pkg/storage/utils/ace/ace.go @@ -41,7 +41,7 @@ import ( // permissions. // L: aLarm - generate a system alarm at any attempted access by // principal which requires permissions -// D for deny is not recommended +// D: for Deny is not recommended // - *flags* for now empty or g for group, no inheritance yet // - d directory-inherit - newly-created subdirectories will inherit the // ACE. From 5706d9157891b661b03e3e5227186dd2ee712508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 28 Sep 2020 21:01:15 +0200 Subject: [PATCH 04/13] check parent for upload permissions 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 | 3 ++- pkg/storage/fs/owncloud/upload.go | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 4bef9ec644..133973911f 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -825,7 +825,7 @@ func (fs *ocfs) readPermissions(ctx context.Context, ip string) (p *provider.Res for np != rp { var attrs []string - if attrs, err = xattr.List(ip); err != nil { + if attrs, err = xattr.List(np); err != nil { appctx.GetLogger(ctx).Error().Err(err).Str("ipath", np).Msg("error listing attributes") return nil, err } @@ -949,6 +949,7 @@ func (fs *ocfs) ListGrants(ctx context.Context, ref *provider.Reference) (grants var attrs []string if attrs, err = xattr.List(ip); err != nil { + // TODO err might be a not exists log.Error().Err(err).Msg("error listing attributes") return nil, err } diff --git a/pkg/storage/fs/owncloud/upload.go b/pkg/storage/fs/owncloud/upload.go index baf2699262..ec1aad3b7a 100644 --- a/pkg/storage/fs/owncloud/upload.go +++ b/pkg/storage/fs/owncloud/upload.go @@ -55,7 +55,7 @@ func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCl perm, perr = fs.readPermissions(ctx, ip) } else { // check permissions - perm, perr = fs.readPermissions(ctx, filepath.Base(ip)) + perm, perr = fs.readPermissions(ctx, filepath.Dir(ip)) } if perr == nil { if !perm.InitiateFileUpload { @@ -118,7 +118,7 @@ func (fs *ocfs) InitiateUpload(ctx context.Context, ref *provider.Reference, upl perm, perr = fs.readPermissions(ctx, ip) } else { // check permissions - perm, perr = fs.readPermissions(ctx, filepath.Base(ip)) + perm, perr = fs.readPermissions(ctx, filepath.Dir(ip)) } if perr == nil { if !perm.InitiateFileUpload { @@ -193,8 +193,8 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd. // check permissions of file to be overwritten perm, perr = fs.readPermissions(ctx, ip) } else { - // check permissions - perm, perr = fs.readPermissions(ctx, filepath.Base(ip)) + // check permissions of parent folder + perm, perr = fs.readPermissions(ctx, filepath.Dir(ip)) } if perr == nil { if !perm.InitiateFileUpload { From 161dab3b59f1244a65a2c6c9536449af587b464a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 28 Sep 2020 21:20:13 +0200 Subject: [PATCH 05/13] make linter happy 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 | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 133973911f..5719a2e294 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -834,9 +834,10 @@ func (fs *ocfs) readPermissions(ctx context.Context, ip string) (p *provider.Res userFound := false for i := range attrs { // we only need the find the user once per node - if !userFound && attrs[i] == userace { + switch { + case !userFound && attrs[i] == userace: e, err = fs.readACE(ctx, np, "u:"+u.Id.OpaqueId) - } else if strings.HasPrefix(attrs[i], sharePrefix+"g:") { + case strings.HasPrefix(attrs[i], sharePrefix+"g:"): g := strings.TrimPrefix(attrs[i], sharePrefix+"g:") if groupsMap[g] { e, err = fs.readACE(ctx, np, "g:"+g) @@ -844,17 +845,19 @@ func (fs *ocfs) readPermissions(ctx context.Context, ip string) (p *provider.Res // no need to check attribute continue } - } else { + default: // no need to check attribute continue } - if err == nil { + + switch { + case err == nil: addPermissions(aggregatedPermissions, e.Grant().GetPermissions()) appctx.GetLogger(ctx).Debug().Str("ipath", np).Str("principal", strings.TrimPrefix(attrs[i], sharePrefix)).Interface("permissions", aggregatedPermissions).Msg("adding permissions") - } else if isNoData(err) { + case isNoData(err): err = nil appctx.GetLogger(ctx).Error().Str("ipath", np).Str("principal", strings.TrimPrefix(attrs[i], sharePrefix)).Interface("attrs", attrs).Msg("no permissions found on node, but they were listed") - } else { + default: appctx.GetLogger(ctx).Error().Err(err).Str("ipath", np).Str("principal", strings.TrimPrefix(attrs[i], sharePrefix)).Msg("error reading permissions") return nil, err } @@ -878,7 +881,7 @@ func (fs *ocfs) readPermissions(ctx context.Context, ip string) (p *provider.Res // listing the aces can be used to match the principals, we do not need to fully real all aces // what if, when checking /a/b/c/d, /a/b has write permission for group g, but /a/b/c has an ace for another group h the user is also a member of? // it would allow restricting a users permissions by resharing something with him with lower permission? - // so if you have reshare permissons you could accidentially restrict users access to a subfolder of a rw share to ro by sharing it to another group as ro when they are part of both groups + // so if you have reshare permissions you could accidentially restrict users access to a subfolder of a rw share to ro by sharing it to another group as ro when they are part of both groups // it makes more sense to have explicit negative permissions // TODO we need to read all parents ... until we find a matching ace? From 357ab88cc27631f23a1a953c7b48e4feb39897d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 28 Sep 2020 21:29:12 +0200 Subject: [PATCH 06/13] add changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/chheck-permissions-in-owncloud-driver.md | 5 +++++ changelog/unreleased/no-longer-swallow-permissions-errors.md | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/chheck-permissions-in-owncloud-driver.md diff --git a/changelog/unreleased/chheck-permissions-in-owncloud-driver.md b/changelog/unreleased/chheck-permissions-in-owncloud-driver.md new file mode 100644 index 0000000000..d3eb8734de --- /dev/null +++ b/changelog/unreleased/chheck-permissions-in-owncloud-driver.md @@ -0,0 +1,5 @@ +Enhancement: check permissions in owncloud driver + +We are now checking grant permissions in the owncloud storage driver. + +https://github.com/cs3org/reva/pull/1202 diff --git a/changelog/unreleased/no-longer-swallow-permissions-errors.md b/changelog/unreleased/no-longer-swallow-permissions-errors.md index 3084e18cd6..9be023c5a2 100644 --- a/changelog/unreleased/no-longer-swallow-permissions-errors.md +++ b/changelog/unreleased/no-longer-swallow-permissions-errors.md @@ -3,4 +3,4 @@ Bugfix: No longer swallow permissions errors The storageprovider is no longer ignoring permissions errors. It will now report them properly using `status.NewPermissionDenied(...)` instead of `status.NewInternal(...)` -https://github.com/cs3org/reva/pull/1206 \ No newline at end of file +https://github.com/cs3org/reva/pull/1206 From c88ee7d92fa59121b3af8f516bffd59874a5a6c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 29 Sep 2020 15:35:39 +0200 Subject: [PATCH 07/13] drop two ocis specific tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../apiAuthWebDav-webDavPROPFINDAuth.feature | 9 --------- .../apiOcisSpecific/apiWebdavPreviews-previews.feature | 8 -------- 2 files changed, 17 deletions(-) diff --git a/tests/acceptance/features/apiOcisSpecific/apiAuthWebDav-webDavPROPFINDAuth.feature b/tests/acceptance/features/apiOcisSpecific/apiAuthWebDav-webDavPROPFINDAuth.feature index 9a37c0d88b..01930c4d87 100644 --- a/tests/acceptance/features/apiOcisSpecific/apiAuthWebDav-webDavPROPFINDAuth.feature +++ b/tests/acceptance/features/apiOcisSpecific/apiAuthWebDav-webDavPROPFINDAuth.feature @@ -9,12 +9,3 @@ Feature: get file info using PROPFIND And user "Alice" has uploaded file with content "some data" to "/PARENT/parent.txt" And user "Brian" has been created with default attributes and without skeleton files - @issue-ocis-reva-9 @skipOnOcis-EOS-Storage @issue-ocis-reva-303 @skipOnOcis-OCIS-Storage - # after fixing all issues delete this Scenario and use the one from oC10 core - Scenario: send PROPFIND requests to another user's webDav endpoints as normal user - When user "Brian" requests these endpoints with "PROPFIND" to get property "d:getetag" about user "Alice" - | endpoint | - | /remote.php/dav/files/%username%/textfile0.txt | - | /remote.php/dav/files/%username%/PARENT | - | /remote.php/dav/files/%username%/PARENT/parent.txt | - Then the HTTP status code of responses on all endpoints should be "207" diff --git a/tests/acceptance/features/apiOcisSpecific/apiWebdavPreviews-previews.feature b/tests/acceptance/features/apiOcisSpecific/apiWebdavPreviews-previews.feature index bbd431d39c..46be01e018 100644 --- a/tests/acceptance/features/apiOcisSpecific/apiWebdavPreviews-previews.feature +++ b/tests/acceptance/features/apiOcisSpecific/apiWebdavPreviews-previews.feature @@ -69,14 +69,6 @@ Feature: previews of files downloaded through the webdav API Then the HTTP status code should be "200" And the downloaded image should be "1240" pixels wide and "648" pixels high - @issue-ocis-191 @skipOnOcis-EOS-Storage @issue-ocis-reva-308 @skipOnOcis-OCIS-Storage - # after fixing all issues delete this Scenario and use the one from oC10 core - Scenario: download previews of other users files - Given user "Brian" has been created with default attributes and without skeleton files - And user "Alice" has uploaded file "filesForUpload/lorem.txt" to "/parent.txt" - When user "Brian" downloads the preview of "/parent.txt" of "Alice" with width "32" and height "32" using the WebDAV API - Then the HTTP status code should be "200" - @issue-ocis-190 # after fixing all issues delete this Scenario and use the one from oC10 core Scenario: download previews of folders From deb4a5e93c92f2dfbd8e4dd42e44ff47834992e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 29 Sep 2020 15:42:20 +0200 Subject: [PATCH 08/13] fix rebase hiccup 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 | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 5719a2e294..5059c7df91 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -445,12 +445,8 @@ func (fs *ocfs) toStorageShadowPath(ctx context.Context, ip string) (sp string) sp = filepath.Join("/", segments[1], segments[3]) } } -<<<<<<< HEAD - appctx.GetLogger(ctx).Debug().Str("internal", internal).Str("external", external).Msg("ocfs: unwrapShadow") -======= log := appctx.GetLogger(ctx) log.Debug().Str("driver", "ocfs").Str("ipath", ip).Str("spath", sp).Msg("toStorageShadowPath") ->>>>>>> e431462... clarify ocfs path vars and wrap functions return } From 09ca96d3a94591a9efe1cefe703ef6053eec1430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 29 Sep 2020 16:34:43 +0200 Subject: [PATCH 09/13] mark resharing tests as failing again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- tests/acceptance/expected-failures-on-OC-storage.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/acceptance/expected-failures-on-OC-storage.txt b/tests/acceptance/expected-failures-on-OC-storage.txt index 814e9bc4af..166049b341 100644 --- a/tests/acceptance/expected-failures-on-OC-storage.txt +++ b/tests/acceptance/expected-failures-on-OC-storage.txt @@ -242,7 +242,6 @@ apiShareToSharesManagementBasic/deleteShareFromShares.feature:46 apiShareToSharesManagementBasic/deleteShareFromShares.feature:58 apiShareToSharesManagementBasic/deleteShareFromShares.feature:72 apiShareToSharesManagementBasic/deleteShareFromShares.feature:89 -apiShareToSharesManagementBasic/deleteShareFromShares.feature:107 apiShareToSharesManagementBasic/deleteShareFromShares.feature:118 apiShareToSharesManagementBasic/deleteShareFromShares.feature:130 apiShareToSharesManagementBasic/deleteShareFromShares.feature:163 @@ -660,8 +659,14 @@ apiShareReshareToShares2/reShareWhenShareWithOnlyMembershipGroups.feature:41 apiShareReshareToShares2/reShareWhenShareWithOnlyMembershipGroups.feature:42 apiShareReshareToShares3/reShareUpdate.feature:25 apiShareReshareToShares3/reShareUpdate.feature:26 +apiShareReshareToShares3/reShareUpdate.feature:42 +apiShareReshareToShares3/reShareUpdate.feature:43 apiShareReshareToShares3/reShareUpdate.feature:59 apiShareReshareToShares3/reShareUpdate.feature:60 +apiShareReshareToShares3/reShareUpdate.feature:76 +apiShareReshareToShares3/reShareUpdate.feature:77 +apiShareReshareToShares3/reShareUpdate.feature:93 +apiShareReshareToShares3/reShareUpdate.feature:94 apiShareReshareToShares3/reShareUpdate.feature:112 apiShareReshareToShares3/reShareUpdate.feature:113 apiShareReshareToShares3/reShareUpdate.feature:131 From 5dc429a7706d5c9db68bfe3ce6b9114b04f096a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 29 Sep 2020 17:13:45 +0200 Subject: [PATCH 10/13] fix not exists handling 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 | 53 ++++++++++++++++++----------- pkg/storage/fs/owncloud/upload.go | 10 +++--- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 5059c7df91..1b45b220af 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -702,7 +702,7 @@ func (fs *ocfs) GetPathByID(ctx context.Context, id *provider.ResourceId) (strin return "", errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return "", errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return "", errors.Wrap(err, "ocfs: error reading permissions") @@ -741,7 +741,7 @@ func (fs *ocfs) AddGrant(ctx context.Context, ref *provider.Reference, g *provid return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -894,6 +894,17 @@ func isNoData(err error) bool { return false } +// The os not exists error is buried inside the xattr error, +// so we cannot just use os.IsNotExists(). +func isNotFound(err error) bool { + if xerr, ok := err.(*xattr.Error); ok { + if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { + return serr == syscall.ENOENT + } + } + return false +} + func (fs *ocfs) readACE(ctx context.Context, ip string, principal string) (e *ace.ACE, err error) { var b []byte if b, err = xattr.Get(ip, sharePrefix+principal); err != nil { @@ -940,7 +951,7 @@ func (fs *ocfs) ListGrants(ctx context.Context, ref *provider.Reference) (grants return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return nil, errors.Wrap(err, "ocfs: error reading permissions") @@ -978,7 +989,7 @@ func (fs *ocfs) RemoveGrant(ctx context.Context, ref *provider.Reference, g *pro return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -1010,7 +1021,7 @@ func (fs *ocfs) UpdateGrant(ctx context.Context, ref *provider.Reference, g *pro return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -1070,7 +1081,7 @@ func (fs *ocfs) CreateDir(ctx context.Context, sp string) (err error) { return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -1153,7 +1164,7 @@ func (fs *ocfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Referenc return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -1296,7 +1307,7 @@ func (fs *ocfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refere return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, ip)) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -1387,7 +1398,7 @@ func (fs *ocfs) Delete(ctx context.Context, ref *provider.Reference) (err error) return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -1481,7 +1492,7 @@ func (fs *ocfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (e return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(oldIP))) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -1509,6 +1520,7 @@ func (fs *ocfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (e func (fs *ocfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { ip, err := fs.resolve(ctx, ref) if err != nil { + // TODO return correct errtype if _, ok := err.(errtypes.IsNotFound); ok { return nil, err } @@ -1533,7 +1545,7 @@ func (fs *ocfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []str return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") @@ -1562,7 +1574,7 @@ func (fs *ocfs) getMDShareFolder(ctx context.Context, sp string, mdKeys []string return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") @@ -1582,6 +1594,9 @@ func (fs *ocfs) getMDShareFolder(ctx context.Context, sp string, mdKeys []string m.Type = provider.ResourceType_RESOURCE_TYPE_REFERENCE ref, err := xattr.Get(ip, mdPrefix+"target") if err != nil { + if isNotFound(err) { + return nil, errtypes.NotFound(fs.toStorageShadowPath(ctx, ip)) + } return nil, err } m.Target = string(ref) @@ -1627,7 +1642,7 @@ func (fs *ocfs) listWithNominalHome(ctx context.Context, ip string, mdKeys []str return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") @@ -1678,7 +1693,7 @@ func (fs *ocfs) listHome(ctx context.Context, home string, mdKeys []string) ([]* return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") @@ -1722,7 +1737,7 @@ func (fs *ocfs) listShareFolderRoot(ctx context.Context, sp string, mdKeys []str return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") @@ -1798,7 +1813,7 @@ func (fs *ocfs) Download(ctx context.Context, ref *provider.Reference) (io.ReadC return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") @@ -1826,7 +1841,7 @@ func (fs *ocfs) ListRevisions(ctx context.Context, ref *provider.Reference) ([]* return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") @@ -1887,7 +1902,7 @@ func (fs *ocfs) RestoreRevision(ctx context.Context, ref *provider.Reference, re return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } return errors.Wrap(err, "ocfs: error reading permissions") @@ -1949,7 +1964,7 @@ func (fs *ocfs) PurgeRecycleItem(ctx context.Context, key string) error { return nil, errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(err) { return nil, errtypes.NotFound(fs.unwrap(ctx, filepath.Dir(ip))) } return nil, errors.Wrap(err, "ocfs: error reading permissions") diff --git a/pkg/storage/fs/owncloud/upload.go b/pkg/storage/fs/owncloud/upload.go index ec1aad3b7a..41909dc309 100644 --- a/pkg/storage/fs/owncloud/upload.go +++ b/pkg/storage/fs/owncloud/upload.go @@ -62,10 +62,10 @@ func (fs *ocfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCl return errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(perr) { return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } - return errors.Wrap(err, "ocfs: error reading permissions") + return errors.Wrap(perr, "ocfs: error reading permissions") } // we cannot rely on /tmp as it can live in another partition and we can @@ -117,7 +117,7 @@ func (fs *ocfs) InitiateUpload(ctx context.Context, ref *provider.Reference, upl // check permissions of file to be overwritten perm, perr = fs.readPermissions(ctx, ip) } else { - // check permissions + // check permissions of parent perm, perr = fs.readPermissions(ctx, filepath.Dir(ip)) } if perr == nil { @@ -125,10 +125,10 @@ func (fs *ocfs) InitiateUpload(ctx context.Context, ref *provider.Reference, upl return "", errtypes.PermissionDenied("") } } else { - if os.IsNotExist(err) { + if isNotFound(perr) { return "", errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(ip))) } - return "", errors.Wrap(err, "ocfs: error reading permissions") + return "", errors.Wrap(perr, "ocfs: error reading permissions") } p := fs.toStoragePath(ctx, ip) From 310559bfab5c3c4fed32a85de0af62dcdcc8b7ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 30 Sep 2020 12:02:09 +0200 Subject: [PATCH 11/13] add todo 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 1b45b220af..d43cbcaaec 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -65,7 +65,7 @@ const ( idAttribute string = "user.oc.id" // SharePrefix is the prefix for sharing related extended attributes - sharePrefix string = "user.oc.acl." + sharePrefix string = "user.oc.acl." // TODO rename to user.oc.grant. because acl != grant trashOriginPrefix string = "user.oc.o" mdPrefix string = "user.oc.md." // arbitrary metadata favPrefix string = "user.oc.fav." // favorite flag, per user From eeaac827cb1c98b9bd846d0c717667a04f522c9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 30 Sep 2020 16:10:55 +0200 Subject: [PATCH 12/13] newline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/no-longer-swallow-permissions-errors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/no-longer-swallow-permissions-errors.md b/changelog/unreleased/no-longer-swallow-permissions-errors.md index 9be023c5a2..3084e18cd6 100644 --- a/changelog/unreleased/no-longer-swallow-permissions-errors.md +++ b/changelog/unreleased/no-longer-swallow-permissions-errors.md @@ -3,4 +3,4 @@ Bugfix: No longer swallow permissions errors The storageprovider is no longer ignoring permissions errors. It will now report them properly using `status.NewPermissionDenied(...)` instead of `status.NewInternal(...)` -https://github.com/cs3org/reva/pull/1206 +https://github.com/cs3org/reva/pull/1206 \ No newline at end of file From 7bc2eedc6edafdf53f76a1ce9845ac994428a334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 30 Sep 2020 16:41:13 +0200 Subject: [PATCH 13/13] replace user.oc.ace with user.oc.grant in extended attributes 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 | 17 +++++++++-------- pkg/storage/utils/ace/ace.go | 24 ++++++++++++++---------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index d43cbcaaec..2b09a0a636 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -60,17 +60,18 @@ const ( // we will use to store ownCloud specific metadata. To prevent name // collisions with other apps We are going to introduce a sub namespace // "user.oc." + ocPrefix string = "user.oc." // idAttribute is the name of the filesystem extended attribute that is used to store the uuid in - idAttribute string = "user.oc.id" + idAttribute string = ocPrefix + "id" // SharePrefix is the prefix for sharing related extended attributes - sharePrefix string = "user.oc.acl." // TODO rename to user.oc.grant. because acl != grant - trashOriginPrefix string = "user.oc.o" - mdPrefix string = "user.oc.md." // arbitrary metadata - favPrefix string = "user.oc.fav." // favorite flag, per user - etagPrefix string = "user.oc.etag." // allow overriding a calculated etag with one from the extended attributes - //checksumPrefix string = "user.oc.cs." // TODO add checksum support + sharePrefix string = ocPrefix + "grant." // grants are similar to acls, but they are not propagated down the tree when being changed + trashOriginPrefix string = ocPrefix + "o" + mdPrefix string = ocPrefix + "md." // arbitrary metadata + favPrefix string = ocPrefix + "fav." // favorite flag, per user + etagPrefix string = ocPrefix + "etag." // allow overriding a calculated etag with one from the extended attributes + //checksumPrefix string = ocPrefix + "cs." // TODO add checksum support ) @@ -1788,7 +1789,7 @@ func (fs *ocfs) copyMD(s string, t string) (err error) { return err } for i := range attrs { - if strings.HasPrefix(attrs[i], "user.oc.") { + if strings.HasPrefix(attrs[i], ocPrefix) { var d []byte if d, err = xattr.Get(s, attrs[i]); err != nil { return err diff --git a/pkg/storage/utils/ace/ace.go b/pkg/storage/utils/ace/ace.go index 02ea3d762d..3a900fb300 100644 --- a/pkg/storage/utils/ace/ace.go +++ b/pkg/storage/utils/ace/ace.go @@ -29,11 +29,15 @@ import ( ) // ACE represents an Access Control Entry, mimicing NFSv4 ACLs +// The difference is tht grant ACEs are not propagated down the tree when being set on a dir. +// The tradeoff is that every read has to check the permissions of all path segments up to the root, +// to determine the permissions. But reads can be scaled better than writes, so here we are. +// See https://github.com/cs3org/reva/pull/1170#issuecomment-700526118 for more details. // // The following is taken from the nfs4_acl man page, // see https://linux.die.net/man/5/nfs4_acl: // the extended attributes will look like this -// "user.oc.acl.:::" +// "user.oc.grant.:::" // - *type* will be limited to A for now // A: Allow - allow *principal* to perform actions requiring *permissions* // In the future we can use: @@ -73,15 +77,15 @@ import ( // - C write-ACL - write the file/directory NFSv4 ACL. // - o write-owner - change ownership of the file/directory. // - y synchronize - allow clients to use synchronous I/O with the server. -// TODO implement OWNER@ as "user.oc.acl.A::OWNER@:rwaDxtTnNcCy" +// TODO implement OWNER@ as "user.oc.grant.A::OWNER@:rwaDxtTnNcCy" // attribute names are limited to 255 chars by the linux kernel vfs, values to 64 kb // ext3 extended attributes must fit inside a single filesystem block ... 4096 bytes -// that leaves us with "user.oc.acl.A::someonewithaslightlylongersubject@whateverissuer:rwaDxtTnNcCy" ~80 chars +// that leaves us with "user.oc.grant.A::someonewithaslightlylongersubject@whateverissuer:rwaDxtTnNcCy" ~80 chars // 4096/80 = 51 shares ... with luck we might move the actual permissions to the value, saving ~15 chars // 4096/64 = 64 shares ... still meh ... we can do better by using ints instead of strings for principals -// "user.oc.acl.u:100000" is pretty neat, but we can still do better: base64 encode the int -// "user.oc.acl.u:6Jqg" but base64 always has at least 4 chars, maybe hex is better for smaller numbers -// well use 4 chars in addition to the ace: "user.oc.acl.u:////" = 65535 -> 18 chars +// "user.oc.grant.u:100000" is pretty neat, but we can still do better: base64 encode the int +// "user.oc.grant.u:6Jqg" but base64 always has at least 4 chars, maybe hex is better for smaller numbers +// well use 4 chars in addition to the ace: "user.oc.grant.u:////" = 65535 -> 18 chars // 4096/18 = 227 shares // still ... ext attrs for this are not infinite scale ... // so .. attach shares via fileid. @@ -89,9 +93,9 @@ import ( // /metadata//shares/u///A:fdi:rwaDxtTnNcCy permissions as filename to keep them in the stat cache? // // whatever ... 50 shares is good enough. If more is needed we can delegate to the metadata -// if "user.oc.acl.M" is present look inside the metadata app. +// if "user.oc.grant.M" is present look inside the metadata app. // - if we cannot set an ace we might get an io error. -// in that case convert all shares to metadata and try to set "user.oc.acl.m" +// in that case convert all shares to metadata and try to set "user.oc.grant.m" // // what about metadata like share creator, share time, expiry? // - creator is same as owner, but can be set @@ -99,8 +103,8 @@ import ( // - expiry is a unix timestamp // - can be put inside the value // - we need to reorder the fields: -// "user.oc.acl.:" -> "kv:t=:f=:p=:st=:c=:e=:pw=:n=" -// "user.oc.acl.:" -> "v1::::::::" +// "user.oc.grant.:" -> "kv:t=:f=:p=:st=:c=:e=:pw=:n=" +// "user.oc.grant.:" -> "v1::::::::" // or the first byte determines the format // 0x00 = key value // 0x01 = v1 ...