[API Key] Invalid API Keys are no longer cached

[simitt]

With the latest changes in how an API Key is validated, invalid API Keys do no longer get cached.

When calling ES via the go-elasticsearch client an error is returned whenever resp.IsError() is true, which is the case for statusCode > 299. For existing API Keys the hasPrivileges request returns a 200 and the body holds the information for which requested privileges the API Key is valid. For non-existing API Keys the hasPrivileges request returns a 401.

With the latest changes in how the requests response is returned, an error is returned now for such cases.
When checking if an API Key is authorized, API Key requests resulting in an error, are not cached though. Initially these responses were not cached, as ES could e.g. be temporarily unavailable in which case a potentially valid API Key should not be cached as invalid, and 401 from ES were not returned as errors.

We need to find a way now to differentiate between server side errors and other errors on the consumer level of the Elasticsearch package, e.g. by also returning a statuscode for es.HasPrivileges, to ensure all API Keys are properly cached, while not caching false negatives on Elasticsearch server errors.

It is unclear to me why the system test testing the caching behavior for non-existing API Keys only started failing now.
Failing Test:

apm-server/tests/system/test_access.py

Line 167 in 4824cbb

class TestAPIKeyCache(BaseAPIKey):

Needs to be backported to 7.6.

[axw]

The fix for this should ideally also resolve this known issue:

apm-server/cmd/apikey.go

Lines 461 to 465 in 4824cbb

	// TODO(axw) either allow 404 in DeletePrivileges, and don't
	// return an error, or check for 404 here and ignore that
	// specifically. The request could failure for other reasons
	// and we shouldn't ignore them.
	continue

[axw]

It is unclear to me why the system test testing the caching behavior for non-existing API Keys only started failing now.

I think it's because of elastic/elasticsearch#51042. Running the test with BC1, it passes - because I'm getting an _anonymous user response back.

[simitt]

Tested by ensuring the cache fills up with invalid API Keys, then using a valid API Key and ensure it is not authorized as valid key.

(1) Decrease limit in APM Server configuration for easier testing:

apm-server:  api_key:
    enabled: true
    limit: 2

(2) Create 2 valid API Keys via .apm-server apikey create
(3) Send request with one valid API Key and check that response is as expected:
curl -i -k -H "Content-type: application/x-ndjson" -H "Authorization: ApiKey validApiKey1" --data-binary @events.ndjson http://localhost:8200/intake/v2/events results in 202:

HTTP/1.1 202 Accepted
X-Content-Type-Options: nosniff
Date: Thu, 23 Jan 2020 13:08:42 GMT
Content-Length: 0

(4) Send enough requests with invalid API Keys to fill up cache and check response is 401

HTTP/1.1 401 Unauthorized
Content-Type: application/json
X-Content-Type-Options: nosniff
Date: Thu, 23 Jan 2020 13:07:22 GMT
Content-Length: 30
Connection: close

{
  "error": "unauthorized"
}

(5) Send request with second valid API Key and check response is 401:
curl -i -k -H "Content-type: application/x-ndjson" -H "Authorization: ApiKey validApiKey2" --data-binary @events.ndjson http://localhost:8200/intake/v2/events

HTTP/1.1 401 Unauthorized
Content-Type: application/json
X-Content-Type-Options: nosniff
Date: Thu, 23 Jan 2020 13:11:08 GMT
Content-Length: 30
Connection: close

{
  "error": "unauthorized"
}

Caching of invalid API Keys works as expected, but found another issue #3237 while testing.