Backport #13530 to 1.2.x

.changelog/13552.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+api: Fix listing evaluations with the wildcard namespace and an ACL token
+```

nomad/eval_endpoint.go

@@ -389,12 +389,17 @@ func (e *Eval) List(args *structs.EvalListRequest,
 	}
 	defer metrics.MeasureSince([]string{"nomad", "eval", "list"}, time.Now())
 
+	namespace := args.RequestNamespace()
+
 	// Check for read-job permissions
-	if aclObj, err := e.srv.ResolveToken(args.AuthToken); err != nil {
+	aclObj, err := e.srv.ResolveToken(args.AuthToken)
+	if err != nil {
 		return err
-	} else if aclObj != nil && !aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilityReadJob) {
+	}
+	if !aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadJob) {
 		return structs.ErrPermissionDenied
 	}
+	allow := aclObj.AllowNsOpFunc(acl.NamespaceCapabilityReadJob)
 
 	// Setup the blocking query
 	opts := blockingOptions{
@@ -404,34 +409,48 @@ func (e *Eval) List(args *structs.EvalListRequest,
 			// Scan all the evaluations
 			var err error
 			var iter memdb.ResultIterator
-			if args.RequestNamespace() == structs.AllNamespacesSentinel {
-				iter, err = store.Evals(ws)
-			} else if prefix := args.QueryOptions.Prefix; prefix != "" {
-				iter, err = store.EvalsByIDPrefix(ws, args.RequestNamespace(), prefix)
-			} else {
-				iter, err = store.EvalsByNamespace(ws, args.RequestNamespace())
-			}
-			if err != nil {
-				return err
-			}
 
-			iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool {
-				if eval := raw.(*structs.Evaluation); eval != nil {
-					return args.ShouldBeFiltered(eval)
+			// Get the namespaces the user is allowed to access.
+			allowableNamespaces, err := allowedNSes(aclObj, store, allow)
+			if err == structs.ErrPermissionDenied {
+				// return empty evals if token isn't authorized for any
+				// namespace, matching other endpoints
+				reply.Evaluations = make([]*structs.Evaluation, 0)
+			} else if err != nil {
+				return err
+			} else {
+				if args.RequestNamespace() == structs.AllNamespacesSentinel {
+					iter, err = store.Evals(ws)
+				} else if prefix := args.QueryOptions.Prefix; prefix != "" {
+					iter, err = store.EvalsByIDPrefix(ws, args.RequestNamespace(), prefix)
+				} else {
+					iter, err = store.EvalsByNamespace(ws, args.RequestNamespace())
+				}
+				if err != nil {
+					return err
 				}
-				return false
-			})
-
-			var evals []*structs.Evaluation
-			paginator := state.NewPaginator(iter, args.QueryOptions,
-				func(raw interface{}) {
-					eval := raw.(*structs.Evaluation)
-					evals = append(evals, eval)
+
+				iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool {
+					if eval := raw.(*structs.Evaluation); eval != nil {
+						nsAllowed := allowableNamespaces == nil ||
+							allowableNamespaces[eval.Namespace]
+
+						return !nsAllowed || args.ShouldBeFiltered(eval)
+					}
+					return false
 				})
 
-			nextToken := paginator.Page()
-			reply.QueryMeta.NextToken = nextToken
-			reply.Evaluations = evals
+				var evals []*structs.Evaluation
+				paginator := state.NewPaginator(iter, args.QueryOptions,
+					func(raw interface{}) {
+						eval := raw.(*structs.Evaluation)
+						evals = append(evals, eval)
+					})
+
+				nextToken := paginator.Page()
+				reply.QueryMeta.NextToken = nextToken
+				reply.Evaluations = evals
+			}
 
 			// Use the last index that affected the jobs table
 			index, err := store.Index("evals")

nomad/eval_endpoint_test.go

@@ -761,62 +761,104 @@ func TestEvalEndpoint_List_ACL(t *testing.T) {
 	defer cleanupS1()
 	codec := rpcClient(t, s1)
 	testutil.WaitForLeader(t, s1.RPC)
-	assert := assert.New(t)
+
+	// Create dev namespace
+	devNS := mock.Namespace()
+	devNS.Name = "dev"
+	err := s1.fsm.State().UpsertNamespaces(999, []*structs.Namespace{devNS})
+	require.NoError(t, err)
 
 	// Create the register request
 	eval1 := mock.Eval()
 	eval1.ID = "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9"
 	eval2 := mock.Eval()
 	eval2.ID = "aaaabbbb-3350-4b4b-d185-0e1992ed43e9"
+	eval3 := mock.Eval()
+	eval3.ID = "aaaacccc-3350-4b4b-d185-0e1992ed43e9"
+	eval3.Namespace = devNS.Name
 	state := s1.fsm.State()
-	assert.Nil(state.UpsertEvals(structs.MsgTypeTestSetup, 1000, []*structs.Evaluation{eval1, eval2}))
+	err = state.UpsertEvals(structs.MsgTypeTestSetup, 1000, []*structs.Evaluation{eval1, eval2, eval3})
+	require.NoError(t, err)
 
 	// Create ACL tokens
 	validToken := mock.CreatePolicyAndToken(t, state, 1003, "test-valid",
 		mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob}))
 	invalidToken := mock.CreatePolicyAndToken(t, state, 1001, "test-invalid",
 		mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityListJobs}))
-
-	get := &structs.EvalListRequest{
-		QueryOptions: structs.QueryOptions{
-			Region:    "global",
-			Namespace: structs.DefaultNamespace,
+	devToken := mock.CreatePolicyAndToken(t, state, 1005, "test-dev",
+		mock.NamespacePolicy("dev", "", []string{acl.NamespaceCapabilityReadJob}))
+
+	testCases := []struct {
+		name          string
+		namespace     string
+		token         string
+		expectedEvals []string
+		expectedError string
+	}{
+		{
+			name:          "no token",
+			token:         "",
+			namespace:     structs.DefaultNamespace,
+			expectedError: structs.ErrPermissionDenied.Error(),
+		},
+		{
+			name:          "invalid token",
+			token:         invalidToken.SecretID,
+			namespace:     structs.DefaultNamespace,
+			expectedError: structs.ErrPermissionDenied.Error(),
+		},
+		{
+			name:          "valid token",
+			token:         validToken.SecretID,
+			namespace:     structs.DefaultNamespace,
+			expectedEvals: []string{eval1.ID, eval2.ID},
+		},
+		{
+			name:          "root token default namespace",
+			token:         root.SecretID,
+			namespace:     structs.DefaultNamespace,
+			expectedEvals: []string{eval1.ID, eval2.ID},
+		},
+		{
+			name:          "root token all namespaces",
+			token:         root.SecretID,
+			namespace:     structs.AllNamespacesSentinel,
+			expectedEvals: []string{eval1.ID, eval2.ID, eval3.ID},
+		},
+		{
+			name:          "dev token all namespaces",
+			token:         devToken.SecretID,
+			namespace:     structs.AllNamespacesSentinel,
+			expectedEvals: []string{eval3.ID},
 		},
 	}
 
-	// Try without a token and expect permission denied
-	{
-		var resp structs.EvalListResponse
-		err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)
-		assert.NotNil(err)
-		assert.Contains(err.Error(), structs.ErrPermissionDenied.Error())
-	}
-
-	// Try with an invalid token and expect permission denied
-	{
-		get.AuthToken = invalidToken.SecretID
-		var resp structs.EvalListResponse
-		err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)
-		assert.NotNil(err)
-		assert.Contains(err.Error(), structs.ErrPermissionDenied.Error())
-	}
-
-	// List evals with a valid token
-	{
-		get.AuthToken = validToken.SecretID
-		var resp structs.EvalListResponse
-		assert.Nil(msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp))
-		assert.Equal(uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000)
-		assert.Lenf(resp.Evaluations, 2, "bad: %#v", resp.Evaluations)
-	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			get := &structs.EvalListRequest{
+				QueryOptions: structs.QueryOptions{
+					AuthToken: tc.token,
+					Region:    "global",
+					Namespace: tc.namespace,
+				},
+			}
 
-	// List evals with a root token
-	{
-		get.AuthToken = root.SecretID
-		var resp structs.EvalListResponse
-		assert.Nil(msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp))
-		assert.Equal(uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000)
-		assert.Lenf(resp.Evaluations, 2, "bad: %#v", resp.Evaluations)
+			var resp structs.EvalListResponse
+			err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)
+
+			if tc.expectedError != "" {
+				require.Contains(t, err.Error(), tc.expectedError)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000)
+
+				got := make([]string, len(resp.Evaluations))
+				for i, eval := range resp.Evaluations {
+					got[i] = eval.ID
+				}
+				require.ElementsMatch(t, got, tc.expectedEvals)
+			}
+		})
 	}
 }