diff --git a/.changelog/13552.txt b/.changelog/13552.txt new file mode 100644 index 00000000000..3c71c30655d --- /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 8a48e27c1de..5decaa9c1b7 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 0701264590d..7253573d83f 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) + } + }) } }