diff --git a/changelog/unreleased/owner-type-is-optional.md b/changelog/unreleased/owner-type-is-optional.md new file mode 100644 index 0000000000..e45b1dab61 --- /dev/null +++ b/changelog/unreleased/owner-type-is-optional.md @@ -0,0 +1,5 @@ +Bugfix: owner type is optional + +When reading the user from the extended attributes the user type might not be set, in this case we now return a user with an invalid type, which correctly reflects the state on disk. + +https://github.com/cs3org/reva/pull/1978 \ No newline at end of file diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index afb0bc757d..023aefa978 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -270,11 +270,13 @@ func (n *Node) Parent() (p *Node, err error) { // Owner returns the cached owner id or reads it from the extended attributes // TODO can be private as only the AsResourceInfo uses it -func (n *Node) Owner() (o *userpb.UserId, err error) { +func (n *Node) Owner() (*userpb.UserId, error) { if n.owner != nil { return n.owner, nil } + owner := &userpb.UserId{} + // FIXME ... do we return the owner of the reference or the owner of the target? // we don't really know the owner of the target ... and as the reference may point anywhere we cannot really find out // but what are the permissions? all? none? the gateway has to fill in? @@ -282,33 +284,42 @@ func (n *Node) Owner() (o *userpb.UserId, err error) { nodePath := n.InternalPath() // lookup parent id in extended attributes var attrBytes []byte + var err error // lookup ID in extended attributes - if attrBytes, err = xattr.Get(nodePath, xattrs.OwnerIDAttr); err == nil { - if n.owner == nil { - n.owner = &userpb.UserId{} - } - n.owner.OpaqueId = string(attrBytes) - } else { - return + attrBytes, err = xattr.Get(nodePath, xattrs.OwnerIDAttr) + switch { + case err == nil: + owner.OpaqueId = string(attrBytes) + case isNoData(err), isNotFound(err): + fallthrough + default: + return nil, err } + // lookup IDP in extended attributes - if attrBytes, err = xattr.Get(nodePath, xattrs.OwnerIDPAttr); err == nil { - if n.owner == nil { - n.owner = &userpb.UserId{} - } - n.owner.Idp = string(attrBytes) - } else { - return + attrBytes, err = xattr.Get(nodePath, xattrs.OwnerIDPAttr) + switch { + case err == nil: + owner.Idp = string(attrBytes) + case isNoData(err), isNotFound(err): + fallthrough + default: + return nil, err } + // lookup type in extended attributes - if attrBytes, err = xattr.Get(nodePath, xattrs.OwnerTypeAttr); err == nil { - if n.owner == nil { - n.owner = &userpb.UserId{} - } - n.owner.Type = utils.UserTypeMap(string(attrBytes)) - } else { - return + attrBytes, err = xattr.Get(nodePath, xattrs.OwnerTypeAttr) + switch { + case err == nil: + owner.Type = utils.UserTypeMap(string(attrBytes)) + case isNoData(err), isNotFound(err): + fallthrough + default: + // TODO the user type defaults to invalid, which is the case + err = nil } + + n.owner = owner return n.owner, err }