diff --git a/nomad/eval_endpoint.go b/nomad/eval_endpoint.go index 8a1a8204925..575f7edd939 100644 --- a/nomad/eval_endpoint.go +++ b/nomad/eval_endpoint.go @@ -571,13 +571,14 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon 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(namespace, acl.NamespaceCapabilityReadJob) { - // TODO: a wildcard namespace here (structs.AllNamespacesSentinel) isn't checked until further down, so we throw a 403 even if token has job.read and namespace is * - // Reproducible w/ nomad operator api '/v1/evaluations?filter=&namespace=*&next_token=&per_page=25&reverse=true' + } + if !aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadJob) { return structs.ErrPermissionDenied } + allow := aclObj.AllowNsOpFunc(acl.NamespaceCapabilityReadJob) if args.Filter != "" { // Check for incompatible filtering. @@ -598,58 +599,73 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon var iter memdb.ResultIterator var opts paginator.StructsTokenizerOptions - if prefix := args.QueryOptions.Prefix; prefix != "" { - iter, err = store.EvalsByIDPrefix(ws, namespace, prefix, sort) - opts = paginator.StructsTokenizerOptions{ - WithID: true, - } - } else if namespace != structs.AllNamespacesSentinel { - iter, err = store.EvalsByNamespaceOrdered(ws, namespace, sort) - opts = paginator.StructsTokenizerOptions{ - WithCreateIndex: true, - WithID: true, - } + // 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 { - iter, err = store.Evals(ws, sort) - opts = paginator.StructsTokenizerOptions{ - WithCreateIndex: true, - WithID: true, + if prefix := args.QueryOptions.Prefix; prefix != "" { + iter, err = store.EvalsByIDPrefix(ws, namespace, prefix, sort) + opts = paginator.StructsTokenizerOptions{ + WithID: true, + } + } else if namespace != structs.AllNamespacesSentinel { + iter, err = store.EvalsByNamespaceOrdered(ws, namespace, sort) + opts = paginator.StructsTokenizerOptions{ + WithCreateIndex: true, + WithID: true, + } + } else { + iter, err = store.Evals(ws, sort) + opts = paginator.StructsTokenizerOptions{ + WithCreateIndex: true, + WithID: true, + } + } + if err != nil { + return err } - } - if err != nil { - return err - } - iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool { - if eval := raw.(*structs.Evaluation); eval != nil { - return args.ShouldBeFiltered(eval) + iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool { + if eval := raw.(*structs.Evaluation); eval != nil { + return args.ShouldBeFiltered(eval) + } + return false + }) + + tokenizer := paginator.NewStructsTokenizer(iter, opts) + filters := []paginator.Filter{ + paginator.NamespaceFilter{ + AllowableNamespaces: allowableNamespaces, + }, } - return false - }) - tokenizer := paginator.NewStructsTokenizer(iter, opts) + var evals []*structs.Evaluation + paginator, err := paginator.NewPaginator(iter, tokenizer, filters, args.QueryOptions, + func(raw interface{}) error { + eval := raw.(*structs.Evaluation) + evals = append(evals, eval) + return nil + }) + if err != nil { + return structs.NewErrRPCCodedf( + http.StatusBadRequest, "failed to create result paginator: %v", err) + } - var evals []*structs.Evaluation - paginator, err := paginator.NewPaginator(iter, tokenizer, nil, args.QueryOptions, - func(raw interface{}) error { - eval := raw.(*structs.Evaluation) - evals = append(evals, eval) - return nil - }) - if err != nil { - return structs.NewErrRPCCodedf( - http.StatusBadRequest, "failed to create result paginator: %v", err) - } + nextToken, err := paginator.Page() + if err != nil { + return structs.NewErrRPCCodedf( + http.StatusBadRequest, "failed to read result page: %v", err) + } - nextToken, err := paginator.Page() - if err != nil { - return structs.NewErrRPCCodedf( - http.StatusBadRequest, "failed to read result page: %v", err) + reply.QueryMeta.NextToken = nextToken + reply.Evaluations = evals } - reply.QueryMeta.NextToken = nextToken - reply.Evaluations = evals - // Use the last index that affected the jobs table index, err := store.Index("evals") if err != nil { diff --git a/nomad/eval_endpoint_test.go b/nomad/eval_endpoint_test.go index fe4d1c79c9c..59b82112527 100644 --- a/nomad/eval_endpoint_test.go +++ b/nomad/eval_endpoint_test.go @@ -1244,79 +1244,105 @@ 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})) + devToken := mock.CreatePolicyAndToken(t, state, 1005, "test-dev", + mock.NamespacePolicy("dev", "", []string{acl.NamespaceCapabilityReadJob})) - get := &structs.EvalListRequest{ - QueryOptions: structs.QueryOptions{ - Region: "global", - Namespace: structs.DefaultNamespace, + 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()) - } + 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 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) - } + var resp structs.EvalListResponse + err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp) - // 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) - } + 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) - wildcardGet := &structs.EvalListRequest{ - QueryOptions: structs.QueryOptions{ - Region: "global", - Namespace: "*", - }, - } - // List evals with a valid and a wildcard namespace - { - wildcardGet.AuthToken = validToken.SecretID - var resp structs.EvalListResponse - assert.Nil(msgpackrpc.CallWithCodec(codec, "Eval.List", wildcardGet, &resp)) - assert.Equal(uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000) - assert.Lenf(resp.Evaluations, 2, "bad: %#v", resp.Evaluations) + got := make([]string, len(resp.Evaluations)) + for i, eval := range resp.Evaluations { + got[i] = eval.ID + } + require.ElementsMatch(t, got, tc.expectedEvals) + } + }) } - } func TestEvalEndpoint_List_Blocking(t *testing.T) { @@ -1393,6 +1419,12 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { codec := rpcClient(t, s1) testutil.WaitForLeader(t, s1.RPC) + // Create non-default namespace + nondefaultNS := mock.Namespace() + nondefaultNS.Name = "non-default" + err := s1.fsm.State().UpsertNamespaces(999, []*structs.Namespace{nondefaultNS}) + require.NoError(t, err) + // create a set of evals and field values to filter on. these are // in the order that the state store will return them from the // iterator (sorted by create index), for ease of writing tests @@ -1404,7 +1436,7 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { }{ {ids: []string{"aaaa1111-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example"}, // 0 {ids: []string{"aaaaaa22-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example"}, // 1 - {ids: []string{"aaaaaa33-3350-4b4b-d185-0e1992ed43e9"}, namespace: "non-default"}, // 2 + {ids: []string{"aaaaaa33-3350-4b4b-d185-0e1992ed43e9"}, namespace: nondefaultNS.Name}, // 2 {ids: []string{"aaaaaaaa-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example", status: "blocked"}, // 3 {ids: []string{"aaaaaabb-3350-4b4b-d185-0e1992ed43e9"}}, // 4 {ids: []string{"aaaaaacc-3350-4b4b-d185-0e1992ed43e9"}}, // 5 diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 723865d2d93..07c758a0e98 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -10900,6 +10900,14 @@ func (e *Evaluation) GetID() string { return e.ID } +// GetNamespace implements the NamespaceGetter interface, required for pagination. +func (e *Evaluation) GetNamespace() string { + if e == nil { + return "" + } + return e.Namespace +} + // GetCreateIndex implements the CreateIndexGetter interface, required for // pagination. func (e *Evaluation) GetCreateIndex() uint64 {