diff --git a/CODEOWNERS b/CODEOWNERS index 7d515836f7..e6ee4e0407 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @labkode @glpatcern @diocas +* @labkode @glpatcern @diocas @jessejeens diff --git a/changelog/unreleased/acl-grpc.md b/changelog/unreleased/acl-grpc.md new file mode 100644 index 0000000000..a2d723c581 --- /dev/null +++ b/changelog/unreleased/acl-grpc.md @@ -0,0 +1,7 @@ +Bugfix: make ACL operations work over gRPC + +This change solves two issues: +* AddACL would fail, because the current implementation of AddACL in the EOS gRPC client always sets msg.Recursive = true. This causes issues on the EOS side, because it will try running a recursive find on a file, which fails. +* RemoveACL would fail, because it tried matching ACL rules with a uid to ACL rules with a username. This PR changes this approach to use an approach similar to what is used in the binary client: just set the rule that you want to have deleted with no permissions. + +https://github.com/cs3org/reva/pull/4898 \ No newline at end of file diff --git a/changelog/unreleased/eos-useracl.md b/changelog/unreleased/eos-useracl.md new file mode 100644 index 0000000000..4fad14d98c --- /dev/null +++ b/changelog/unreleased/eos-useracl.md @@ -0,0 +1,6 @@ +Enhancement: do not read eos user ACLs any longer + +This PR drops the compatibility code to read eos user ACLs +in the eos binary client, and aligns it to the GRPC client. + +https://github.com/cs3org/reva/pull/4892 diff --git a/changelog/unreleased/fav-grpc.md b/changelog/unreleased/fav-grpc.md new file mode 100644 index 0000000000..d460c4c947 --- /dev/null +++ b/changelog/unreleased/fav-grpc.md @@ -0,0 +1,3 @@ +Enhancement: Favourites for eos/grpc + +https://github.com/cs3org/reva/pull/4863 diff --git a/changelog/unreleased/no-certs-eos-http-client.md b/changelog/unreleased/no-certs-eos-http-client.md new file mode 100644 index 0000000000..bb4b81b5d8 --- /dev/null +++ b/changelog/unreleased/no-certs-eos-http-client.md @@ -0,0 +1,8 @@ +Bugfix: no certs in EOS HTTP client + +Omit HTTPS cert in EOS HTTP Client, as this causes authentication issues on EOS < 5.2.28. +When EOS receives a certificate, it will look for this cert in the gridmap file. +If it is not found there, the whole authn flow is aborted and the user is mapped to nobody. + + +https://github.com/cs3org/reva/pull/4894 \ No newline at end of file diff --git a/changelog/unreleased/propfind-perms-grpc.md b/changelog/unreleased/propfind-perms-grpc.md new file mode 100644 index 0000000000..bf8436db8e --- /dev/null +++ b/changelog/unreleased/propfind-perms-grpc.md @@ -0,0 +1,8 @@ +Bugfix: broken PROPFIND perms on gRPC + +When using the EOS gRPC stack, the permissions returned by PROPFIND +on a folder in a project were erroneous because ACL permissions were +being ignored. This stems from a bug in grpcMDResponseToFileInfo, +where the SysACL attribute of the FileInfo struct was not being populated. + +See: https://github.com/cs3org/reva/pull/4901 \ No newline at end of file diff --git a/changelog/unreleased/restorefileversion-nilptr.md b/changelog/unreleased/restorefileversion-nilptr.md new file mode 100644 index 0000000000..970380f3bd --- /dev/null +++ b/changelog/unreleased/restorefileversion-nilptr.md @@ -0,0 +1,3 @@ +Bugfix: fix nilpointer error in RollbackToVersion + +https://github.com/cs3org/reva/pull/4896 diff --git a/changelog/unreleased/trashbin-grpc.md b/changelog/unreleased/trashbin-grpc.md new file mode 100644 index 0000000000..cf28473cb8 --- /dev/null +++ b/changelog/unreleased/trashbin-grpc.md @@ -0,0 +1,5 @@ +Bugfix: PurgeDate in ListDeletedEntries was ignored + +The date range that can be passed to ListDeletedEntries was not taken into account due to a bug in reva: the Purgedate argument was set, which only works for PURGE requests, and not for LIST requests. Instead, the Listflag argument must be used. Additionally, there was a bug in the loop that is used to iterate over all days in the date range. + +https://github.com/cs3org/reva/pull/4905 diff --git a/pkg/eosclient/eosbinary/eosbinary.go b/pkg/eosclient/eosbinary/eosbinary.go index 10189b7daf..3f899b5e75 100644 --- a/pkg/eosclient/eosbinary/eosbinary.go +++ b/pkg/eosclient/eosbinary/eosbinary.go @@ -43,9 +43,8 @@ import ( ) const ( - versionPrefix = ".sys.v#." - userACLEvalKey = "eval.useracl" - favoritesKey = "http://owncloud.org/ns/favorite" + versionPrefix = ".sys.v#." + favoritesKey = "http://owncloud.org/ns/favorite" ) func serializeAttribute(a *eosclient.Attribute) string { @@ -1226,23 +1225,6 @@ func (c *Client) mapToFileInfo(ctx context.Context, kv, attrs map[string]string, return nil, err } - // Temporary until we migrate the user ACLs to sys ACLs on our MGMs - // Read user ACLs if sys.eval.useracl is set - if userACLEval, ok := attrs["sys."+userACLEvalKey]; ok && userACLEval == "1" { - if userACL, ok := attrs["user.acl"]; ok { - userAcls, err := acl.Parse(userACL, acl.ShortTextForm) - if err != nil { - return nil, err - } - for _, e := range userAcls.Entries { - err = sysACL.SetEntry(e.Type, e.Qualifier, e.Permissions) - if err != nil { - return nil, err - } - } - } - } - // Read the favorite attr if parseFavoriteKey { parseAndSetFavoriteAttr(ctx, attrs) diff --git a/pkg/eosclient/eosgrpc/eosgrpc.go b/pkg/eosclient/eosgrpc/eosgrpc.go index 8a86c1b5cd..22f46e590c 100644 --- a/pkg/eosclient/eosgrpc/eosgrpc.go +++ b/pkg/eosclient/eosgrpc/eosgrpc.go @@ -48,6 +48,7 @@ import ( const ( versionPrefix = ".sys.v#." + favoritesKey = "http://owncloud.org/ns/favorite" ) const ( @@ -57,6 +58,29 @@ const ( UserAttr ) +func serializeAttribute(a *eosclient.Attribute) string { + return fmt.Sprintf("%s.%s=%s", attrTypeToString(a.Type), a.Key, a.Val) +} + +func attrTypeToString(at eosclient.AttrType) string { + switch at { + case eosclient.SystemAttr: + return "sys" + case eosclient.UserAttr: + return "user" + default: + return "invalid" + } +} + +func isValidAttribute(a *eosclient.Attribute) bool { + // validate that an attribute is correct. + if (a.Type != eosclient.SystemAttr && a.Type != eosclient.UserAttr) || a.Key == "" { + return false + } + return true +} + // Options to configure the Client. type Options struct { @@ -259,6 +283,13 @@ func (c *Client) AddACL(ctx context.Context, auth, rootAuth eosclient.Authorizat log := appctx.GetLogger(ctx) log.Info().Str("func", "AddACL").Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", path).Msg("") + // First, we need to figure out if the path is a directory + // to know whether our request should be recursive + fileInfo, err := c.GetFileInfoByPath(ctx, auth, path) + if err != nil { + return err + } + // Init a new NSRequest rq, err := c.initNSRequest(ctx, rootAuth, "") if err != nil { @@ -272,7 +303,7 @@ func (c *Client) AddACL(ctx context.Context, auth, rootAuth eosclient.Authorizat msg := new(erpc.NSRequest_AclRequest) msg.Cmd = erpc.NSRequest_AclRequest_ACL_COMMAND(erpc.NSRequest_AclRequest_ACL_COMMAND_value["MODIFY"]) msg.Type = erpc.NSRequest_AclRequest_ACL_TYPE(erpc.NSRequest_AclRequest_ACL_TYPE_value["SYS_ACL"]) - msg.Recursive = true + msg.Recursive = fileInfo.IsDir msg.Rule = a.CitrineSerialize() msg.Id = new(erpc.MDId) @@ -300,48 +331,11 @@ func (c *Client) AddACL(ctx context.Context, auth, rootAuth eosclient.Authorizat // RemoveACL removes the acl from EOS. func (c *Client) RemoveACL(ctx context.Context, auth, rootAuth eosclient.Authorization, path string, a *acl.Entry) error { log := appctx.GetLogger(ctx) - log.Info().Str("func", "RemoveACL").Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", path).Msg("") - - acls, err := c.getACLForPath(ctx, auth, path) - if err != nil { - return err - } - - acls.DeleteEntry(a.Type, a.Qualifier) - sysACL := acls.Serialize() - - // Init a new NSRequest - rq, err := c.initNSRequest(ctx, auth, "") - if err != nil { - return err - } - - msg := new(erpc.NSRequest_AclRequest) - msg.Cmd = erpc.NSRequest_AclRequest_ACL_COMMAND(erpc.NSRequest_AclRequest_ACL_COMMAND_value["MODIFY"]) - msg.Type = erpc.NSRequest_AclRequest_ACL_TYPE(erpc.NSRequest_AclRequest_ACL_TYPE_value["SYS_ACL"]) - msg.Recursive = true - msg.Rule = sysACL - - msg.Id = new(erpc.MDId) - msg.Id.Path = []byte(path) - - rq.Command = &erpc.NSRequest_Acl{Acl: msg} - - // Now send the req and see what happens - resp, err := c.cl.Exec(appctx.ContextGetClean(ctx), rq) - e := c.getRespError(resp, err) - if e != nil { - log.Error().Str("func", "RemoveACL").Str("path", path).Str("err", e.Error()).Msg("") - return e - } - - if resp == nil { - return errtypes.NotFound(fmt.Sprintf("Path: %s", path)) - } - - log.Debug().Str("func", "RemoveACL").Str("path", path).Str("resp:", fmt.Sprintf("%#v", resp)).Msg("grpc response") + log.Info().Str("func", "RemoveACL").Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", path).Str("ACL", a.CitrineSerialize()).Msg("") - return err + // We set permissions to "", so the ACL will serialize to `u:123456=`, which will make EOS delete the entry + a.Permissions = "" + return c.AddACL(ctx, auth, rootAuth, path, eosclient.StartPosition, a) } // UpdateACL updates the EOS acl. @@ -387,6 +381,11 @@ func (c *Client) getACLForPath(ctx context.Context, auth eosclient.Authorization log := appctx.GetLogger(ctx) log.Info().Str("func", "GetACLForPath").Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", path).Msg("") + fileInfo, err := c.GetFileInfoByPath(ctx, auth, path) + if err != nil { + return nil, err + } + // Initialize the common fields of the NSReq rq, err := c.initNSRequest(ctx, auth, "") if err != nil { @@ -396,7 +395,7 @@ func (c *Client) getACLForPath(ctx context.Context, auth eosclient.Authorization msg := new(erpc.NSRequest_AclRequest) msg.Cmd = erpc.NSRequest_AclRequest_ACL_COMMAND(erpc.NSRequest_AclRequest_ACL_COMMAND_value["LIST"]) msg.Type = erpc.NSRequest_AclRequest_ACL_TYPE(erpc.NSRequest_AclRequest_ACL_TYPE_value["SYS_ACL"]) - msg.Recursive = true + msg.Recursive = fileInfo.IsDir msg.Id = new(erpc.MDId) msg.Id.Path = []byte(path) @@ -508,6 +507,22 @@ func (c *Client) fixupACLs(ctx context.Context, auth eosclient.Authorization, in // SetAttr sets an extended attributes on a path. func (c *Client) SetAttr(ctx context.Context, auth eosclient.Authorization, attr *eosclient.Attribute, errorIfExists, recursive bool, path, app string) error { + if !isValidAttribute(attr) { + return errors.New("eos: attr is invalid: " + serializeAttribute(attr)) + } + + // Favorites need to be stored per user so handle these separately + if attr.Type == eosclient.UserAttr && attr.Key == favoritesKey { + info, err := c.GetFileInfoByPath(ctx, auth, path) + if err != nil { + return err + } + return c.handleFavAttr(ctx, auth, attr, recursive, path, info, true) + } + return c.setEOSAttr(ctx, auth, attr, errorIfExists, recursive, path, app) +} + +func (c *Client) setEOSAttr(ctx context.Context, auth eosclient.Authorization, attr *eosclient.Attribute, errorIfExists, recursive bool, path, app string) error { log := appctx.GetLogger(ctx) log.Info().Str("func", "SetAttr").Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", path).Msg("") @@ -556,6 +571,32 @@ func (c *Client) SetAttr(ctx context.Context, auth eosclient.Authorization, attr return err } +func (c *Client) handleFavAttr(ctx context.Context, auth eosclient.Authorization, attr *eosclient.Attribute, recursive bool, path string, info *eosclient.FileInfo, set bool) error { + var err error + u := appctx.ContextMustGetUser(ctx) + if info == nil { + info, err = c.GetFileInfoByPath(ctx, auth, path) + if err != nil { + return err + } + } + favStr := info.Attrs[favoritesKey] + favs, err := acl.Parse(favStr, acl.ShortTextForm) + if err != nil { + return err + } + if set { + err = favs.SetEntry(acl.TypeUser, u.Id.OpaqueId, "1") + if err != nil { + return err + } + } else { + favs.DeleteEntry(acl.TypeUser, u.Id.OpaqueId) + } + attr.Val = favs.Serialize() + return c.setEOSAttr(ctx, auth, attr, false, recursive, path, "") +} + // UnsetAttr unsets an extended attribute on a path. func (c *Client) UnsetAttr(ctx context.Context, auth eosclient.Authorization, attr *eosclient.Attribute, recursive bool, path, app string) error { log := appctx.GetLogger(ctx) @@ -1242,7 +1283,9 @@ func (c *Client) List(ctx context.Context, auth eosclient.Authorization, dpath s if parent != nil && parent.SysACL != nil { if fi.SysACL == nil { log.Warn().Str("func", "List").Str("path", dpath).Str("SysACL is nil, taking parent", "").Msg("grpc response") - fi.SysACL.Entries = parent.SysACL.Entries + fi.SysACL = &acl.ACLs{ + Entries: parent.SysACL.Entries, + } } else { fi.SysACL.Entries = append(fi.SysACL.Entries, parent.SysACL.Entries...) } @@ -1373,14 +1416,13 @@ func (c *Client) ListDeletedEntries(ctx context.Context, auth eosclient.Authoriz ret := make([]*eosclient.DeletedEntry, 0) count := 0 - for d := to; !d.Before(to); d = d.AddDate(0, 0, -1) { + for d := to; !d.Before(from); d = d.AddDate(0, 0, -1) { msg := new(erpc.NSRequest_RecycleRequest) msg.Cmd = erpc.NSRequest_RecycleRequest_RECYCLE_CMD(erpc.NSRequest_RecycleRequest_RECYCLE_CMD_value["LIST"]) - msg.Purgedate = new(erpc.NSRequest_RecycleRequest_PurgeDate) - msg.Purgedate.Day = int32(d.Day()) - msg.Purgedate.Month = int32(d.Month()) - msg.Purgedate.Year = int32(d.Year()) msg.Listflag = new(erpc.NSRequest_RecycleRequest_ListFlags) + msg.Listflag.Day = int32(d.Day()) + msg.Listflag.Month = int32(d.Month()) + msg.Listflag.Year = int32(d.Year()) msg.Listflag.Maxentries = int32(maxentries + 1) rq.Command = &erpc.NSRequest_Recycle{Recycle: msg} @@ -1544,8 +1586,9 @@ func (c *Client) RollbackToVersion(ctx context.Context, auth eosclient.Authoriza return errtypes.InternalError(fmt.Sprintf("nil response for uid: '%s' ", auth.Role.UID)) } - log.Info().Str("func", "RollbackToVersion").Int64("errcode", resp.GetError().Code).Str("errmsg", resp.GetError().Msg).Msg("grpc response") - + if resp.GetError() != nil { + log.Info().Str("func", "RollbackToVersion").Int64("errcode", resp.GetError().Code).Str("errmsg", resp.GetError().Msg).Msg("grpc response") + } return err } @@ -1628,6 +1671,10 @@ func (c *Client) grpcMDResponseToFileInfo(ctx context.Context, st *erpc.MDRespon fi.Attrs[strings.TrimPrefix(k, "user.")] = string(v) } + if fi.Attrs["sys.acl"] != "" { + fi.SysACL = aclAttrToAclStruct(fi.Attrs["sys.acl"]) + } + fi.TreeSize = uint64(st.Cmd.TreeSize) fi.Size = fi.TreeSize // TODO(lopresti) this info is missing in the EOS Protobuf, cf. EOS-5974 @@ -1648,6 +1695,10 @@ func (c *Client) grpcMDResponseToFileInfo(ctx context.Context, st *erpc.MDRespon fi.Attrs[strings.TrimPrefix(k, "user.")] = string(v) } + if fi.Attrs["sys.acl"] != "" { + fi.SysACL = aclAttrToAclStruct(fi.Attrs["sys.acl"]) + } + fi.Size = st.Fmd.Size if st.Fmd.Checksum != nil { @@ -1662,3 +1713,23 @@ func (c *Client) grpcMDResponseToFileInfo(ctx context.Context, st *erpc.MDRespon } return fi, nil } + +func aclAttrToAclStruct(aclAttr string) *acl.ACLs { + entries := strings.Split(aclAttr, ",") + + acl := &acl.ACLs{} + + for _, entry := range entries { + parts := strings.Split(entry, ":") + if len(parts) != 3 { + continue + } + aclType := parts[0] + qualifier := parts[1] + permissions := parts[2] + + acl.SetEntry(aclType, qualifier, permissions) + } + + return acl +} diff --git a/pkg/eosclient/eosgrpc/eoshttp.go b/pkg/eosclient/eosgrpc/eoshttp.go index 4966355335..e447afea13 100644 --- a/pkg/eosclient/eosgrpc/eoshttp.go +++ b/pkg/eosclient/eosgrpc/eoshttp.go @@ -21,8 +21,6 @@ package eosgrpc import ( "bytes" "context" - "crypto/tls" - "errors" "fmt" "io" "net/http" @@ -148,10 +146,6 @@ func NewEOSHTTPClient(opt *HTTPOptions) (*EOSHTTPClient, error) { } opt.init() - baseUrl, err := url.Parse(opt.BaseURL) - if err != nil { - return nil, errors.New("Failed to parse BaseURL") - } t := &http.Transport{ MaxIdleConns: opt.MaxIdleConns, @@ -161,21 +155,6 @@ func NewEOSHTTPClient(opt *HTTPOptions) (*EOSHTTPClient, error) { DisableCompression: true, } - if baseUrl.Scheme == "https" { - cert, err := tls.LoadX509KeyPair(opt.ClientCertFile, opt.ClientKeyFile) - if err != nil { - return nil, err - } - t.TLSClientConfig = &tls.Config{ - Certificates: []tls.Certificate{cert}, - } - } - - // TODO: the error reporting of http.transport is insufficient - // we may want to check manually at least the existence of the certfiles - // The point is that also the error reporting of the context that calls this function - // is weak - cl := &http.Client{ Transport: t, CheckRedirect: func(req *http.Request, via []*http.Request) error {