From d1f77a48ab542e9ef37bf315fe7e58dc3b4a8125 Mon Sep 17 00:00:00 2001
From: James Rasell <jrasell@users.noreply.github.com>
Date: Mon, 23 Jun 2025 07:44:32 +0100
Subject: [PATCH] rpc: Use client only auth for node get client allocs
 endpoint. (#26084)

The RPC is only ever called from a Nomad client which means we
can move it away from the generic Authenticate function to the
tighter AuthenticateClientOnly one. An addition check to ensure
the ACL object allows client operations is performed, mimicking
other endpoints of this nature.
---
 nomad/node_endpoint.go      | 10 +++++++++-
 nomad/node_endpoint_test.go | 35 ++++++++++++++++++++++++++++-------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go
index 6ab97ce7a..e0743379e 100644
--- a/nomad/node_endpoint.go
+++ b/nomad/node_endpoint.go
@@ -1101,7 +1101,11 @@ func (n *Node) GetAllocs(args *structs.NodeSpecificRequest,
 func (n *Node) GetClientAllocs(args *structs.NodeSpecificRequest,
 	reply *structs.NodeClientAllocsResponse) error {
 
-	authErr := n.srv.Authenticate(n.ctx, args)
+	// This RPC is only ever called by Nomad clients, so we can use the tightly
+	// scoped AuthenticateClientOnly method to authenticate and authorize the
+	// request.
+	aclObj, authErr := n.srv.AuthenticateClientOnly(n.ctx, args)
+
 	isForwarded := args.IsForwarded()
 	if done, err := n.srv.forward("Node.GetClientAllocs", args, args, reply); done {
 		// We have a valid node connection since there is no error from the
@@ -1120,6 +1124,10 @@ func (n *Node) GetClientAllocs(args *structs.NodeSpecificRequest,
 	}
 	defer metrics.MeasureSince([]string{"nomad", "client", "get_client_allocs"}, time.Now())
 
+	if !aclObj.AllowClientOp() {
+		return structs.ErrPermissionDenied
+	}
+
 	// Verify the arguments
 	if args.NodeID == "" {
 		return fmt.Errorf("missing node ID")
diff --git a/nomad/node_endpoint_test.go b/nomad/node_endpoint_test.go
index 1d6774ba9..a69ae4470 100644
--- a/nomad/node_endpoint_test.go
+++ b/nomad/node_endpoint_test.go
@@ -2406,6 +2406,19 @@ func TestClientEndpoint_GetClientAllocs(t *testing.T) {
 	// Check that we have no client connections
 	require.Empty(s1.connectedNodes())
 
+	// The RPC is client only, so perform a test using the leader ACL token to
+	// ensure that even this powerful token cannot access the endpoint.
+	leaderACLReq := structs.NodeSpecificRequest{
+		NodeID: uuid.Generate(),
+		QueryOptions: structs.QueryOptions{
+			Region:    "global",
+			AuthToken: s1.leaderAcl,
+		},
+	}
+	var leaderACLResp structs.NodeClientAllocsResponse
+	err := msgpackrpc.CallWithCodec(codec, "Node.GetClientAllocs", &leaderACLReq, &leaderACLResp)
+	must.ErrorContains(t, err, "Permission denied")
+
 	// Create the register request
 	node := mock.Node()
 	state := s1.fsm.State()
@@ -2415,16 +2428,19 @@ func TestClientEndpoint_GetClientAllocs(t *testing.T) {
 	alloc := mock.Alloc()
 	alloc.NodeID = node.ID
 	state.UpsertJobSummary(99, mock.JobSummary(alloc.JobID))
-	err := state.UpsertAllocs(structs.MsgTypeTestSetup, 100, []*structs.Allocation{alloc})
+	err = state.UpsertAllocs(structs.MsgTypeTestSetup, 100, []*structs.Allocation{alloc})
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
 
 	// Lookup the allocs
 	get := &structs.NodeSpecificRequest{
-		NodeID:       node.ID,
-		SecretID:     node.SecretID,
-		QueryOptions: structs.QueryOptions{Region: "global"},
+		NodeID:   node.ID,
+		SecretID: node.SecretID,
+		QueryOptions: structs.QueryOptions{
+			Region:    "global",
+			AuthToken: node.SecretID,
+		},
 	}
 	var resp2 structs.NodeClientAllocsResponse
 	if err := msgpackrpc.CallWithCodec(codec, "Node.GetClientAllocs", get, &resp2); err != nil {
@@ -2517,6 +2533,7 @@ func TestClientEndpoint_GetClientAllocs_Blocking(t *testing.T) {
 		NodeID:   node.ID,
 		SecretID: node.SecretID,
 		QueryOptions: structs.QueryOptions{
+			AuthToken:     node.SecretID,
 			Region:        "global",
 			MinQueryIndex: 50,
 			MaxQueryTime:  time.Second,
@@ -2635,6 +2652,7 @@ func TestClientEndpoint_GetClientAllocs_Blocking_GC(t *testing.T) {
 		NodeID:   node.ID,
 		SecretID: node.SecretID,
 		QueryOptions: structs.QueryOptions{
+			AuthToken:     node.SecretID,
 			Region:        "global",
 			MinQueryIndex: 50,
 			MaxQueryTime:  time.Second,
@@ -2711,9 +2729,12 @@ func TestClientEndpoint_GetClientAllocs_WithoutMigrateTokens(t *testing.T) {
 
 	// Lookup the allocs
 	get := &structs.NodeSpecificRequest{
-		NodeID:       node.ID,
-		SecretID:     node.SecretID,
-		QueryOptions: structs.QueryOptions{Region: "global"},
+		NodeID:   node.ID,
+		SecretID: node.SecretID,
+		QueryOptions: structs.QueryOptions{
+			AuthToken: node.SecretID,
+			Region:    "global",
+		},
 	}
 	var resp2 structs.NodeClientAllocsResponse