resource: mutate and validate before acls on write (#18868)

This commit is contained in:
Semir Patel 2023-09-18 17:04:29 -05:00 committed by GitHub
parent dabbc9627b
commit 62796a1454
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 19 deletions

View File

@ -49,15 +49,6 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre
}
v1EntMetaToV2Tenancy(reg, v1EntMeta, req.Resource.Id.Tenancy)
// ACL check comes before tenancy existence checks to not leak tenancy "existence".
err = reg.ACLs.Write(authz, authzContext, req.Resource)
switch {
case acl.IsErrPermissionDenied(err):
return nil, status.Error(codes.PermissionDenied, err.Error())
case err != nil:
return nil, status.Errorf(codes.Internal, "failed write acl: %v", err)
}
// Check the user sent the correct type of data.
if req.Resource.Data != nil && !req.Resource.Data.MessageIs(reg.Proto) {
got := strings.TrimPrefix(req.Resource.Data.TypeUrl, "type.googleapis.com/")
@ -70,6 +61,23 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre
)
}
if err = reg.Mutate(req.Resource); err != nil {
return nil, status.Errorf(codes.Internal, "failed mutate hook: %v", err.Error())
}
if err = reg.Validate(req.Resource); err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
// ACL check comes before tenancy existence checks to not leak tenancy "existence".
err = reg.ACLs.Write(authz, authzContext, req.Resource)
switch {
case acl.IsErrPermissionDenied(err):
return nil, status.Error(codes.PermissionDenied, err.Error())
case err != nil:
return nil, status.Errorf(codes.Internal, "failed write acl: %v", err)
}
// Check V1 tenancy exists for the V2 resource
if err = v1TenancyExists(reg, s.TenancyBridge, req.Resource.Id.Tenancy, codes.InvalidArgument); err != nil {
return nil, err
@ -80,14 +88,6 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre
return nil, err
}
if err = reg.Mutate(req.Resource); err != nil {
return nil, status.Errorf(codes.Internal, "failed mutate hook: %v", err.Error())
}
if err = reg.Validate(req.Resource); err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
// At the storage backend layer, all writes are CAS operations.
//
// This makes it possible to *safely* do things like keeping the Uid stable

View File

@ -49,14 +49,18 @@ type Registration struct {
Proto proto.Message
// ACLs are hooks called to perform authorization on RPCs.
// The hooks can assume that Validate has been called.
ACLs *ACLHooks
// Validate is called to structurally validate the resource (e.g.
// check for required fields).
// check for required fields). Validate can assume that Mutate
// has been called.
Validate func(*pbresource.Resource) error
// Mutate is called to fill out any autogenerated fields (e.g. UUIDs) or
// apply defaults before validation.
// apply defaults before validation. Mutate can assume that
// Resource.ID is populated and has non-empty tenancy fields. This does
// not mean those tenancy fields actually exist.
Mutate func(*pbresource.Resource) error
// Scope describes the tenancy scope of a resource.