Merge pull request #21681 from ahrtr/20260428_auth_refactor

[release-3.6] Refactor auth check for Put requests in TXN

Author: ahrtr

server/etcdserver/apply/apply_auth.go

@@ -20,7 +20,6 @@ import (
 	pb "go.etcd.io/etcd/api/v3/etcdserverpb"
 	"go.etcd.io/etcd/pkg/v3/traceutil"
 	"go.etcd.io/etcd/server/v3/auth"
-	"go.etcd.io/etcd/server/v3/etcdserver/txn"
 	"go.etcd.io/etcd/server/v3/lease"
 )
 
@@ -63,25 +62,34 @@ func (aa *authApplierV3) Apply(r *pb.InternalRaftRequest, applyFunc applyFunc) *
 }
 
 func (aa *authApplierV3) Put(r *pb.PutRequest) (*pb.PutResponse, *traceutil.Trace, error) {
-	if err := aa.as.IsPutPermitted(&aa.authInfo, r.Key); err != nil {
+	if err := checkPutAuth(aa.as, &aa.authInfo, aa.lessor, r); err != nil {
 		return nil, nil, err
 	}
 
-	if err := aa.checkLeasePuts(lease.LeaseID(r.Lease)); err != nil {
+	return aa.applierV3.Put(r)
+}
+
+func checkPutAuth(as auth.AuthStore, ai *auth.AuthInfo, lessor lease.Lessor, r *pb.PutRequest) error {
+	if err := as.IsPutPermitted(ai, r.Key); err != nil {
+		return err
+	}
+
+	if err := checkLeasePuts(as, ai, lessor, lease.LeaseID(r.Lease)); err != nil {
 		// The specified lease is already attached with a key that cannot
 		// be written by this user. It means the user cannot revoke the
 		// lease so attaching the lease to the newly written key should
 		// be forbidden.
-		return nil, nil, err
+		return err
 	}
 
 	if r.PrevKv {
-		err := aa.as.IsRangePermitted(&aa.authInfo, r.Key, nil)
+		err := as.IsRangePermitted(ai, r.Key, nil)
 		if err != nil {
-			return nil, nil, err
+			return err
 		}
 	}
-	return aa.applierV3.Put(r)
+
+	return nil
 }
 
 func (aa *authApplierV3) Range(r *pb.RangeRequest) (*pb.RangeResponse, *traceutil.Trace, error) {
@@ -106,37 +114,105 @@ func (aa *authApplierV3) DeleteRange(r *pb.DeleteRangeRequest) (*pb.DeleteRangeR
 }
 
 func (aa *authApplierV3) Txn(rt *pb.TxnRequest) (*pb.TxnResponse, *traceutil.Trace, error) {
-	if err := txn.CheckTxnAuth(aa.as, &aa.authInfo, rt); err != nil {
+	if err := CheckTxnAuth(aa.as, &aa.authInfo, rt); err != nil {
 		return nil, nil, err
 	}
 	return aa.applierV3.Txn(rt)
 }
 
+func CheckTxnAuth(as auth.AuthStore, ai *auth.AuthInfo, rt *pb.TxnRequest) error {
+	return checkTxnPermission(as, ai, rt)
+}
+
+func checkTxnPermission(as auth.AuthStore, ai *auth.AuthInfo, rt *pb.TxnRequest) error {
+	for _, c := range rt.Compare {
+		if err := as.IsRangePermitted(ai, c.Key, c.RangeEnd); err != nil {
+			return err
+		}
+	}
+	if err := checkTxnReqsPermission(as, ai, rt.Success); err != nil {
+		return err
+	}
+	return checkTxnReqsPermission(as, ai, rt.Failure)
+}
+
+func checkTxnReqsPermission(as auth.AuthStore, ai *auth.AuthInfo, reqs []*pb.RequestOp) error {
+	for _, requ := range reqs {
+		switch tv := requ.Request.(type) {
+		case *pb.RequestOp_RequestRange:
+			if tv.RequestRange == nil {
+				continue
+			}
+
+			if err := as.IsRangePermitted(ai, tv.RequestRange.Key, tv.RequestRange.RangeEnd); err != nil {
+				return err
+			}
+
+		case *pb.RequestOp_RequestPut:
+			if tv.RequestPut == nil {
+				continue
+			}
+
+			if err := as.IsPutPermitted(ai, tv.RequestPut.Key); err != nil {
+				return err
+			}
+
+		case *pb.RequestOp_RequestDeleteRange:
+			if tv.RequestDeleteRange == nil {
+				continue
+			}
+
+			if tv.RequestDeleteRange.PrevKv {
+				err := as.IsRangePermitted(ai, tv.RequestDeleteRange.Key, tv.RequestDeleteRange.RangeEnd)
+				if err != nil {
+					return err
+				}
+			}
+
+			err := as.IsDeleteRangePermitted(ai, tv.RequestDeleteRange.Key, tv.RequestDeleteRange.RangeEnd)
+			if err != nil {
+				return err
+			}
+		case *pb.RequestOp_RequestTxn:
+			if tv.RequestTxn == nil {
+				continue
+			}
+
+			err := checkTxnPermission(as, ai, tv.RequestTxn)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 func (aa *authApplierV3) LeaseRevoke(lc *pb.LeaseRevokeRequest) (*pb.LeaseRevokeResponse, error) {
-	if err := aa.checkLeasePuts(lease.LeaseID(lc.ID)); err != nil {
+	if err := checkLeasePuts(aa.as, &aa.authInfo, aa.lessor, lease.LeaseID(lc.ID)); err != nil {
 		return nil, err
 	}
 	return aa.applierV3.LeaseRevoke(lc)
 }
 
-func (aa *authApplierV3) checkLeasePuts(leaseID lease.LeaseID) error {
-	l := aa.lessor.Lookup(leaseID)
+func checkLeasePuts(as auth.AuthStore, ai *auth.AuthInfo, lessor lease.Lessor, leaseID lease.LeaseID) error {
+	l := lessor.Lookup(leaseID)
 	if l != nil {
-		return aa.checkLeasePutsKeys(l)
+		return checkLeasePutsKeys(as, ai, l)
 	}
 
 	return nil
 }
 
-func (aa *authApplierV3) checkLeasePutsKeys(l *lease.Lease) error {
+func checkLeasePutsKeys(as auth.AuthStore, ai *auth.AuthInfo, l *lease.Lease) error {
 	// early return for most-common scenario of either disabled auth or admin user.
 	// IsAdminPermitted also checks whether auth is enabled
-	if err := aa.as.IsAdminPermitted(&aa.authInfo); err == nil {
+	if err := as.IsAdminPermitted(ai); err == nil {
 		return nil
 	}
 
 	for _, key := range l.Keys() {
-		if err := aa.as.IsPutPermitted(&aa.authInfo, []byte(key)); err != nil {
+		if err := as.IsPutPermitted(ai, []byte(key)); err != nil {
 			return err
 		}
 	}