Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix broken EOS ACL operations #4898

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+25 −43
Open

Fix broken EOS ACL operations #4898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5964f8f
Make AddACL only recursive when it is passed a directory
Oct 23, 2024
bb0ac06
Make RemoveACL use AddACL with empty permissions instead of modifying…
Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/unreleased/acl-grpc.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
61 changes: 18 additions & 43 deletions pkg/eosclient/eosgrpc/eosgrpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,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 {
Expand All @@ -272,7 +279,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)
Expand Down Expand Up @@ -300,48 +307,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
}
log.Info().Str("func", "RemoveACL").Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", path).Str("ACL", a.CitrineSerialize()).Msg("")

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")

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.
Expand Down Expand Up @@ -387,6 +357,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 {
Expand All @@ -396,7 +371,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)
Expand Down
Loading