From acc86899440e04905d1bfe3f76aa7c9faf0b3d10 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 13 Jul 2022 14:17:37 -0400 Subject: [PATCH] Backport #13530 to 1.2.x (#13552) Apply the same verification process as in job, allocs and scaling policy list endpoints to handle the eval list when using an ACL token with limited namespace support but querying using the `*` wildcard namespace. --- .changelog/13552.txt | 3 + nomad/eval_endpoint.go | 71 +++++++++++++-------- nomad/eval_endpoint_test.go | 120 ++++++++++++++++++++++++------------ 3 files changed, 129 insertions(+), 65 deletions(-) create mode 100644 .changelog/13552.txt diff --git a/.changelog/13552.txt b/.changelog/13552.txt new file mode 100644 index 000000000000..3c71c30655de --- /dev/null +++ b/.changelog/13552.txt @@ -0,0 +1,3 @@ +```release-note:bug +api: Fix listing evaluations with the wildcard namespace and an ACL token +``` diff --git a/nomad/eval_endpoint.go b/nomad/eval_endpoint.go index 8a48e27c1dee..5decaa9c1b70 100644 --- a/nomad/eval_endpoint.go +++ b/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") diff --git a/nomad/eval_endpoint_test.go b/nomad/eval_endpoint_test.go index 0701264590d7..7253573d83f2 100644 --- a/nomad/eval_endpoint_test.go +++ b/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) + } + }) } }