status-im/consul
2
0
Fork 0
mirror of https://github.com/status-im/consul.git synced 2025-01-22 03:29:43 +00:00
Code Issues Projects Releases Wiki Activity

resource: Pass resource to Write ACL hook instead of just resource Id [NET-4908] (#18192)

Browse Source
This commit is contained in:
Semir Patel 2023-07-20 12:06:29 -05:00 committed by GitHub
parent 1c7fcdf188
commit ada767fc9f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 26 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
agent/grpc-external/services/resource/delete.go
View File

@ -41,7 +41,23 @@ func (s *Server) Delete(ctx context.Context, req *pbresource.DeleteRequest) (*pb
return nil, err
}
err = reg.ACLs.Write(authz, req.Id)
// Retrieve resource since ACL hook requires it. Furthermore, we'll need the
// read to be strongly consistent if the passed in Version or Uid are empty.
consistency := storage.EventualConsistency
if req.Version == "" || req.Id.Uid == "" {
consistency = storage.StrongConsistency
}
existing, err := s.Backend.Read(ctx, consistency, req.Id)
switch {
case errors.Is(err, storage.ErrNotFound):
// Deletes are idempotent so no-op when not found
return &pbresource.DeleteResponse{}, nil
case err != nil:
return nil, status.Errorf(codes.Internal, "failed read: %v", err)
}
// Check ACLs
err = reg.ACLs.Write(authz, existing)
switch {
case acl.IsErrPermissionDenied(err):
return nil, status.Error(codes.PermissionDenied, err.Error())
@ -49,27 +65,11 @@ func (s *Server) Delete(ctx context.Context, req *pbresource.DeleteRequest) (*pb
return nil, status.Errorf(codes.Internal, "failed write acl: %v", err)
}
// The storage backend requires a Version and Uid to delete a resource based
// on CAS semantics. When either are not provided, the resource must be read
// with a strongly consistent read to retrieve either or both.
//
// n.b.: There is a chance DeleteCAS may fail with a storage.ErrCASFailure
// if an update occurs between the Read and DeleteCAS. Consider refactoring
// to use retryCAS() similar to the Write endpoint to close this gap.
deleteVersion := req.Version
deleteId := req.Id
if deleteVersion == "" || deleteId.Uid == "" {
existing, err := s.Backend.Read(ctx, storage.StrongConsistency, req.Id)
switch {
case err == nil:
deleteVersion = existing.Version
deleteId = existing.Id
case errors.Is(err, storage.ErrNotFound):
// Deletes are idempotent so no-op when not found
return &pbresource.DeleteResponse{}, nil
default:
return nil, status.Errorf(codes.Internal, "failed read: %v", err)
}
deleteVersion = existing.Version
deleteId = existing.Id
}
if err := s.maybeCreateTombstone(ctx, deleteId); err != nil {

2
agent/grpc-external/services/resource/write.go
View File

@ -52,7 +52,7 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre
}
// check acls
err = reg.ACLs.Write(authz, req.Resource.Id)
err = reg.ACLs.Write(authz, req.Resource)
switch {
case acl.IsErrPermissionDenied(err):
return nil, status.Error(codes.PermissionDenied, err.Error())

4
internal/resource/demo/demo.go
View File

@ -77,8 +77,8 @@ func RegisterTypes(r resource.Registry) {
return authz.ToAllowAuthorizer().KeyReadAllowed(key, &acl.AuthorizerContext{})
}
writeACL := func(authz acl.Authorizer, id *pbresource.ID) error {
key := fmt.Sprintf("resource/%s/%s", resource.ToGVK(id.Type), id.Name)
writeACL := func(authz acl.Authorizer, res *pbresource.Resource) error {
key := fmt.Sprintf("resource/%s/%s", resource.ToGVK(res.Id.Type), res.Id.Name)
return authz.ToAllowAuthorizer().KeyWriteAllowed(key, &acl.AuthorizerContext{})
}

4
internal/resource/registry.go
View File

@ -57,7 +57,7 @@ type ACLHooks struct {
// Write is used to authorize Write and Delete RPCs.
//
// If it is omitted, `operator:write` permission is assumed.
Write func(acl.Authorizer, *pbresource.ID) error
Write func(acl.Authorizer, *pbresource.Resource) error
// List is used to authorize List RPCs.
//
@ -119,7 +119,7 @@ func (r *TypeRegistry) Register(registration Registration) {
}
}
if registration.ACLs.Write == nil {
registration.ACLs.Write = func(authz acl.Authorizer, id *pbresource.ID) error {
registration.ACLs.Write = func(authz acl.Authorizer, id *pbresource.Resource) error {
return authz.ToAllowAuthorizer().OperatorWriteAllowed(&acl.AuthorizerContext{})
}
}

4
internal/resource/registry_test.go
View File

@ -47,8 +47,8 @@ func TestRegister_Defaults(t *testing.T) {
require.True(t, acl.IsErrPermissionDenied(reg.ACLs.Read(testutils.ACLNoPermissions(t), artist.Id)))
// verify default write hook requires operator:write
require.NoError(t, reg.ACLs.Write(testutils.ACLOperatorWrite(t), artist.Id))
require.True(t, acl.IsErrPermissionDenied(reg.ACLs.Write(testutils.ACLNoPermissions(t), artist.Id)))
require.NoError(t, reg.ACLs.Write(testutils.ACLOperatorWrite(t), artist))
require.True(t, acl.IsErrPermissionDenied(reg.ACLs.Write(testutils.ACLNoPermissions(t), artist)))
// verify default list hook requires operator:read
require.NoError(t, reg.ACLs.List(testutils.ACLOperatorRead(t), artist.Id.Tenancy))