Skip to content

Commit

Permalink
[Fleet] Fix error when searching for keys whose names have spaces (el…
Browse files Browse the repository at this point in the history
…astic#100056)

## Summary
fixes elastic#99895

Can reproduce elastic#99895 with something like
```shell
curl 'http://localhost:5601/api/fleet/enrollment-api-keys' \
  -H 'content-type: application/json' \
  -H 'kbn-version: 8.0.0' \
  -u elastic:changeme \
  --data-raw '{"name":"with spaces","policy_id":"d6a93200-b1bd-11eb-90ac-052b474d74cd"}'
```

Kibana logs this stack trace

```
server    log   [10:57:07.863] [error][fleet][plugins] KQLSyntaxError: Expected AND, OR, end of input but "\" found.
policy_id:"d6a93200-b1bd-11eb-90ac-052b474d74cd" AND name:with\ spaces*
--------------------------------------------------------------^
    at Object.fromKueryExpression (/Users/jfsiii/work/kibana/src/plugins/data/common/es_query/kuery/ast/ast.ts:52:13)
    at listEnrollmentApiKeys (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts:37:69)
    at Object.generateEnrollmentAPIKey (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts:160:31)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at postEnrollmentApiKeyHandler (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/routes/enrollment_api_key/handler.ts:53:20)
    at Router.handle (/Users/jfsiii/work/kibana/src/core/server/http/router/router.ts:273:30)
    at handler (/Users/jfsiii/work/kibana/src/core/server/http/router/router.ts:228:11)
    at exports.Manager.execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
    at Object.internals.handler (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/handler.js:46:20)
    at exports.execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/handler.js:31:20)
    at Request._lifecycle (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/request.js:370:32)
    at Request._execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/request.js:279:9) {
  shortMessage: 'Expected AND, OR, end of input but "\\" found.'
```

the `kuery` value which causes the `KQLSyntaxError` is
```
policy_id:\"d6a93200-b1bd-11eb-90ac-052b474d74cd\" AND name:with\\ spaces*
``` 

a value without spaces, e.g. `no_spaces` 

```
policy_id:\"d6a93200-b1bd-11eb-90ac-052b474d74cd\" AND name:no_spaces*
```

is converted to this query object

```
{
  "bool": {
    "filter": [
      {
        "bool": {
          "should": [
            {
              "match_phrase": {
                "policy_id": "d6a93200-b1bd-11eb-90ac-052b474d74cd"
              }
            }
          ],
          "minimum_should_match": 1
        }
      },
      {
        "bool": {
          "should": [
            {
              "query_string": {
                "fields": [
                  "name"
                ],
                "query": "no_spaces*"
              }
            }
          ],
          "minimum_should_match": 1
        }
      }
    ]
  }
```

I tried some other approaches for handling the spaces based on what I saw in the docs like `name:"\"with spaces\"` and `name:(with spaces)*`but they all failed as well, like

```
KQLSyntaxError: Expected AND, OR, end of input but "*" found.
policy_id:"d6a93200-b1bd-11eb-90ac-052b474d74cd" AND name:(with spaces)*
-----------------------------------------------------------------------^
    at Object.fromKueryExpression (/Users/jfsiii/work/kibana/src/plugins/data/common/es_query/kuery/ast/ast.ts:52:13)
    at listEnrollmentApiKeys (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts:37:69)
    at Object.generateEnrollmentAPIKey (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts:166:31)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at postEnrollmentApiKeyHandler (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/routes/enrollment_api_key/handler.ts:53:20)
    at Router.handle (/Users/jfsiii/work/kibana/src/core/server/http/router/router.ts:273:30)
    at handler (/Users/jfsiii/work/kibana/src/core/server/http/router/router.ts:228:11)
    at exports.Manager.execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
    at Object.internals.handler (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/handler.js:46:20)
    at exports.execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/handler.js:31:20)
    at Request._lifecycle (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/request.js:370:32)
    at Request._execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/request.js:279:9) {
  shortMessage: 'Expected AND, OR, end of input but "*" found.'
```

So I logged out the query object for a successful request, and put that in a function

```
{
  "query": {
    "bool": {
      "filter": [
        {
          "bool": {
            "should": [
              {
                "match_phrase": {
                  "policy_id": "d6a93200-b1bd-11eb-90ac-052b474d74cd"
                }
              }
            ],
            "minimum_should_match": 1
          }
        },
        {
          "bool": {
            "should": [
              {
                "query_string": {
                  "fields": [
                    "name"
                  ],
                  "query": "(with spaces) *"
                }
              }
            ],
            "minimum_should_match": 1
          }
        }
      ]
    }
  }
}
```


### Checklist
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
John Schulz authored and kibanamachine committed May 13, 2021
1 parent 11e2bfb commit abb56fc
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type { SavedObjectsClientContract, ElasticsearchClient } from 'src/core/s
import { esKuery } from '../../../../../../src/plugins/data/server';
import type { ESSearchResponse as SearchResponse } from '../../../../../../typings/elasticsearch';
import type { EnrollmentAPIKey, FleetServerEnrollmentAPIKey } from '../../types';
import { IngestManagerError } from '../../errors';
import { ENROLLMENT_API_KEYS_INDEX } from '../../constants';
import { agentPolicyService } from '../agent_policy';
import { escapeSearchQueryPhrase } from '../saved_object';
Expand All @@ -28,10 +29,13 @@ export async function listEnrollmentApiKeys(
page?: number;
perPage?: number;
kuery?: string;
query?: ReturnType<typeof esKuery['toElasticsearchQuery']>;
showInactive?: boolean;
}
): Promise<{ items: EnrollmentAPIKey[]; total: any; page: any; perPage: any }> {
const { page = 1, perPage = 20, kuery } = options;
const query =
options.query ?? (kuery && esKuery.toElasticsearchQuery(esKuery.fromKueryExpression(kuery)));

const res = await esClient.search<SearchResponse<FleetServerEnrollmentAPIKey, {}>>({
index: ENROLLMENT_API_KEYS_INDEX,
Expand All @@ -40,9 +44,7 @@ export async function listEnrollmentApiKeys(
sort: 'created_at:desc',
track_total_hits: true,
ignore_unavailable: true,
body: kuery
? { query: esKuery.toElasticsearchQuery(esKuery.fromKueryExpression(kuery)) }
: undefined,
body: query ? { query } : undefined,
});

// @ts-expect-error @elastic/elasticsearch
Expand Down Expand Up @@ -159,7 +161,7 @@ export async function generateEnrollmentAPIKey(
const { items } = await listEnrollmentApiKeys(esClient, {
page: page++,
perPage: 100,
kuery: `policy_id:"${agentPolicyId}" AND name:${providedKeyName.replace(/ /g, '\\ ')}*`,
query: getQueryForExistingKeyNameOnPolicy(agentPolicyId, providedKeyName),
});
if (items.length === 0) {
hasMore = false;
Expand All @@ -176,7 +178,7 @@ export async function generateEnrollmentAPIKey(
k.name?.replace(providedKeyName, '').trim().match(uuidRegex)
)
) {
throw new Error(
throw new IngestManagerError(
i18n.translate('xpack.fleet.serverError.enrollmentKeyDuplicate', {
defaultMessage:
'An enrollment key named {providedKeyName} already exists for agent policy {agentPolicyId}',
Expand Down Expand Up @@ -254,6 +256,29 @@ export async function generateEnrollmentAPIKey(
};
}

function getQueryForExistingKeyNameOnPolicy(agentPolicyId: string, providedKeyName: string) {
const query = {
bool: {
filter: [
{
bool: {
should: [{ match_phrase: { policy_id: agentPolicyId } }],
minimum_should_match: 1,
},
},
{
bool: {
should: [{ query_string: { fields: ['name'], query: `(${providedKeyName}) *` } }],
minimum_should_match: 1,
},
},
],
},
};

return query;
}

export async function getEnrollmentAPIKeyById(esClient: ElasticsearchClient, apiKeyId: string) {
const res = await esClient.search<FleetServerEnrollmentAPIKey>({
index: ENROLLMENT_API_KEYS_INDEX,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export default function (providerContext: FtrProviderContext) {
.expect(400);
});

it('should allow to create an enrollment api key with an agent policy', async () => {
it('should allow to create an enrollment api key with only an agent policy', async () => {
const { body: apiResponse } = await supertest
.post(`/api/fleet/enrollment-api-keys`)
.set('kbn-xsrf', 'xxx')
Expand All @@ -115,6 +115,55 @@ export default function (providerContext: FtrProviderContext) {
expect(apiResponse.item).to.have.keys('id', 'api_key', 'api_key_id', 'name', 'policy_id');
});

it('should allow to create an enrollment api key with agent policy and unique name', async () => {
const { body: noSpacesRes } = await supertest
.post(`/api/fleet/enrollment-api-keys`)
.set('kbn-xsrf', 'xxx')
.send({
policy_id: 'policy1',
name: 'something',
});
expect(noSpacesRes.item).to.have.keys('id', 'api_key', 'api_key_id', 'name', 'policy_id');

const { body: hasSpacesRes } = await supertest
.post(`/api/fleet/enrollment-api-keys`)
.set('kbn-xsrf', 'xxx')
.send({
policy_id: 'policy1',
name: 'something else',
});
expect(hasSpacesRes.item).to.have.keys('id', 'api_key', 'api_key_id', 'name', 'policy_id');

const { body: noSpacesDupe } = await supertest
.post(`/api/fleet/enrollment-api-keys`)
.set('kbn-xsrf', 'xxx')
.send({
policy_id: 'policy1',
name: 'something',
})
.expect(400);

expect(noSpacesDupe).to.eql({
statusCode: 400,
error: 'Bad Request',
message: 'An enrollment key named something already exists for agent policy policy1',
});

const { body: hasSpacesDupe } = await supertest
.post(`/api/fleet/enrollment-api-keys`)
.set('kbn-xsrf', 'xxx')
.send({
policy_id: 'policy1',
name: 'something else',
})
.expect(400);
expect(hasSpacesDupe).to.eql({
statusCode: 400,
error: 'Bad Request',
message: 'An enrollment key named something else already exists for agent policy policy1',
});
});

it('should create an ES ApiKey with metadata', async () => {
const { body: apiResponse } = await supertest
.post(`/api/fleet/enrollment-api-keys`)
Expand Down

0 comments on commit abb56fc

Please sign in to comment.