Skip to content

Commit 14022c7

Browse files
Improve efficiency of querying API key for user profile retrieval (elastic#260410)
## Summary Implementation of a couple of recent features required querying for API keys to retrieve user profile information. The `getApiKey` API calls now pass the ID of the specific API key in the request header, which eliminates the possibility that a large number of keys will be retrieved regardless of the API key's granted privileges. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [ ] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. This PR was partially created with AI. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
1 parent e905658 commit 14022c7

10 files changed

Lines changed: 159 additions & 7 deletions

File tree

src/core/packages/security/server/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,12 @@ export type { AuditLogger } from './src/audit_logging/audit_logger';
3131
export type * from './src/authentication';
3232

3333
export type { KibanaPrivilegesType, ElasticsearchPrivilegesType } from './src/roles';
34-
export { isCreateRestAPIKeyParams, HTTPAuthorizationHeader } from './src/authentication';
34+
export {
35+
isCreateRestAPIKeyParams,
36+
extractApiKeyIdFromAuthzHeader,
37+
decodeApiKeyId,
38+
HTTPAuthorizationHeader,
39+
} from './src/authentication';
3540
export { isUiamCredential } from './src/uiam';
3641
export type { CoreFipsService } from './src/fips';
3742
export { AuthzDisabled, AuthzOptOutReason, unwindNestedSecurityPrivileges } from './src/authz';

src/core/packages/security/server/src/authentication/api_keys/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export type {
3030
} from './api_keys';
3131
export type { NativeAPIKeysWithContextType } from './api_keys_context';
3232
export { isCreateRestAPIKeyParams } from './api_keys';
33+
export { extractApiKeyIdFromAuthzHeader, decodeApiKeyId } from './utils';
3334

3435
export type {
3536
UiamAPIKeysType,
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
import { extractApiKeyIdFromAuthzHeader, decodeApiKeyId } from './utils';
11+
12+
describe('extractApiKeyIdFromAuthzHeader', () => {
13+
it('returns the API key ID from a valid ApiKey authorization header', () => {
14+
const id = 'my-api-key-id';
15+
const secret = 'my-api-key-secret';
16+
const encoded = Buffer.from(`${id}:${secret}`).toString('base64');
17+
expect(extractApiKeyIdFromAuthzHeader(`ApiKey ${encoded}`)).toBe(id);
18+
});
19+
20+
it('is case-insensitive for the ApiKey prefix', () => {
21+
const id = 'test-id';
22+
const encoded = Buffer.from(`${id}:secret`).toString('base64');
23+
expect(extractApiKeyIdFromAuthzHeader(`apikey ${encoded}`)).toBe(id);
24+
expect(extractApiKeyIdFromAuthzHeader(`APIKEY ${encoded}`)).toBe(id);
25+
expect(extractApiKeyIdFromAuthzHeader(`aPiKeY ${encoded}`)).toBe(id);
26+
});
27+
28+
it('returns undefined when the header is undefined', () => {
29+
expect(extractApiKeyIdFromAuthzHeader(undefined)).toBeUndefined();
30+
});
31+
32+
it('returns undefined when the header is an array', () => {
33+
expect(extractApiKeyIdFromAuthzHeader(['ApiKey abc'])).toBeUndefined();
34+
});
35+
36+
it('returns undefined when the header does not start with ApiKey prefix', () => {
37+
expect(extractApiKeyIdFromAuthzHeader('Bearer some-token')).toBeUndefined();
38+
expect(extractApiKeyIdFromAuthzHeader('Basic dXNlcjpwYXNz')).toBeUndefined();
39+
});
40+
41+
it('returns the ID when there is no colon in the decoded value', () => {
42+
const encoded = Buffer.from('just-an-id').toString('base64');
43+
expect(extractApiKeyIdFromAuthzHeader(`ApiKey ${encoded}`)).toBe('just-an-id');
44+
});
45+
46+
it('returns undefined when the decoded ID is empty', () => {
47+
const encoded = Buffer.from(':secret').toString('base64');
48+
expect(extractApiKeyIdFromAuthzHeader(`ApiKey ${encoded}`)).toBeUndefined();
49+
});
50+
});
51+
52+
describe('decodeApiKeyId', () => {
53+
it('returns the API key ID from a base64-encoded api key', () => {
54+
const id = 'my-api-key-id';
55+
const secret = 'my-api-key-secret';
56+
const encoded = Buffer.from(`${id}:${secret}`).toString('base64');
57+
expect(decodeApiKeyId(encoded)).toBe(id);
58+
});
59+
60+
it('returns the full decoded value when there is no colon', () => {
61+
const encoded = Buffer.from('just-an-id').toString('base64');
62+
expect(decodeApiKeyId(encoded)).toBe('just-an-id');
63+
});
64+
65+
it('returns undefined when the decoded ID is empty', () => {
66+
const encoded = Buffer.from(':secret').toString('base64');
67+
expect(decodeApiKeyId(encoded)).toBeUndefined();
68+
});
69+
70+
it('returns undefined for an empty string', () => {
71+
expect(decodeApiKeyId('')).toBeUndefined();
72+
});
73+
74+
it('returns undefined when the decoded ID is only whitespace', () => {
75+
const encoded = Buffer.from(' :secret').toString('base64');
76+
expect(decodeApiKeyId(encoded)).toBeUndefined();
77+
});
78+
});
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
export const extractApiKeyIdFromAuthzHeader = (
11+
authorizationHeader: string | string[] | undefined
12+
): string | undefined => {
13+
if (typeof authorizationHeader !== 'string') {
14+
return undefined;
15+
}
16+
const prefix = 'apikey ';
17+
if (!authorizationHeader.toLowerCase().startsWith(prefix)) {
18+
return undefined;
19+
}
20+
const encodedApiKey = authorizationHeader.slice(prefix.length);
21+
return decodeApiKeyId(encodedApiKey);
22+
};
23+
24+
export const decodeApiKeyId = (encodedApiKey: string): string | undefined => {
25+
const decoded = Buffer.from(encodedApiKey, 'base64').toString();
26+
const [id] = decoded.split(':');
27+
return id.trim() === '' ? undefined : id;
28+
};

src/core/packages/security/server/src/authentication/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,8 @@ export type {
3737
} from './api_keys';
3838

3939
export { HTTPAuthorizationHeader } from './http_authentication';
40-
export { isCreateRestAPIKeyParams } from './api_keys';
40+
export {
41+
isCreateRestAPIKeyParams,
42+
extractApiKeyIdFromAuthzHeader,
43+
decodeApiKeyId,
44+
} from './api_keys';

x-pack/platform/plugins/shared/actions/moon.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ dependsOn:
6565
- '@kbn/actions-utils'
6666
- '@kbn/encrypted-saved-objects-shared'
6767
- '@kbn/usage-api-plugin'
68+
- '@kbn/core-security-server'
6869
tags:
6970
- plugin
7071
- prod

x-pack/platform/plugins/shared/actions/server/plugin.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* 2.0.
66
*/
77

8+
import { extractApiKeyIdFromAuthzHeader } from '@kbn/core-security-server';
89
import type { PublicMethodsOf, Writable } from '@kbn/utility-types';
910
import type { UsageCollectionSetup, UsageCounter } from '@kbn/usage-collection-plugin/server';
1011
import type {
@@ -662,14 +663,23 @@ export class ActionsPlugin
662663
request: KibanaRequest
663664
): Promise<string | undefined> => {
664665
try {
666+
const id = extractApiKeyIdFromAuthzHeader(request.headers.authorization);
667+
if (!id) {
668+
this.logger.debug(`Failed to decode API key ID from Authorization header.`);
669+
return undefined;
670+
}
671+
665672
const response = await core.elasticsearch.client
666673
.asScoped(request)
667674
.asCurrentUser.security.getApiKey({
668675
with_profile_uid: true,
676+
id,
669677
});
678+
670679
if (response.api_keys && response.api_keys.length > 0) {
671680
return response.api_keys[0].profile_uid;
672681
}
682+
673683
logger.debug(
674684
`No API keys were returned from query, cannot retrieve associated profile id.`
675685
);

x-pack/platform/plugins/shared/actions/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
"@kbn/actions-utils",
6060
"@kbn/encrypted-saved-objects-shared",
6161
"@kbn/usage-api-plugin",
62+
"@kbn/core-security-server",
6263
],
6364
"exclude": [
6465
"target/**/*",

x-pack/platform/plugins/shared/security/server/user_profile/user_profile_service.test.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -445,11 +445,15 @@ describe('UserProfileService', () => {
445445

446446
describe(`with api key`, () => {
447447
const testApiKeyValue = 'some-api-key-value';
448+
const testApiKeyId = 'some-api-key-id';
449+
const testEncodedApiKey = Buffer.from(`${testApiKeyId}:${testApiKeyValue}`).toString(
450+
'base64'
451+
);
448452
let mockApiKeyRequest: ReturnType<typeof httpServerMock.createKibanaRequest>;
449453

450454
beforeEach(() => {
451455
mockApiKeyRequest = httpServerMock.createKibanaRequest({
452-
headers: { authorization: `apikey ${testApiKeyValue}` },
456+
headers: { authorization: `apikey ${testEncodedApiKey}` },
453457
});
454458
});
455459

@@ -469,7 +473,10 @@ describe('UserProfileService', () => {
469473
).toHaveBeenCalledTimes(1);
470474
expect(
471475
mockStartParams.clusterClient.asScoped().asCurrentUser.security.getApiKey
472-
).toHaveBeenCalledWith({ with_profile_uid: true });
476+
).toHaveBeenCalledWith({
477+
with_profile_uid: true,
478+
id: testApiKeyId,
479+
});
473480

474481
expect(
475482
mockStartParams.clusterClient.asInternalUser.security.activateUserProfile
@@ -499,7 +506,10 @@ describe('UserProfileService', () => {
499506
expect(mockStartParams.clusterClient.asScoped).toBeCalledWith(mockApiKeyRequest);
500507
expect(
501508
mockStartParams.clusterClient.asScoped().asCurrentUser.security.getApiKey
502-
).toHaveBeenCalledWith({ with_profile_uid: true });
509+
).toHaveBeenCalledWith({
510+
with_profile_uid: true,
511+
id: testApiKeyId,
512+
});
503513

504514
expect(
505515
mockStartParams.clusterClient.asInternalUser.security.activateUserProfile
@@ -533,7 +543,10 @@ describe('UserProfileService', () => {
533543
expect(mockStartParams.clusterClient.asScoped).toBeCalledWith(mockApiKeyRequest);
534544
expect(
535545
mockStartParams.clusterClient.asScoped().asCurrentUser.security.getApiKey
536-
).toHaveBeenCalledWith({ with_profile_uid: true });
546+
).toHaveBeenCalledWith({
547+
with_profile_uid: true,
548+
id: testApiKeyId,
549+
});
537550

538551
expect(
539552
mockStartParams.clusterClient.asInternalUser.security.activateUserProfile
@@ -596,7 +609,10 @@ describe('UserProfileService', () => {
596609
expect(mockStartParams.clusterClient.asScoped).toBeCalledWith(mockApiKeyRequest);
597610
expect(
598611
mockStartParams.clusterClient.asScoped().asCurrentUser.security.getApiKey
599-
).toHaveBeenCalledWith({ with_profile_uid: true });
612+
).toHaveBeenCalledWith({
613+
with_profile_uid: true,
614+
id: testApiKeyId,
615+
});
600616

601617
expect(
602618
mockStartParams.clusterClient.asInternalUser.security.getUserProfile

x-pack/platform/plugins/shared/security/server/user_profile/user_profile_service.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
import pLimit from 'p-limit';
1313

1414
import type { IClusterClient, Logger } from '@kbn/core/server';
15+
import { extractApiKeyIdFromAuthzHeader } from '@kbn/core-security-server';
1516
import type {
1617
CheckUserProfilesPrivilegesResponse,
1718
UserProfileBulkGetParams,
@@ -288,8 +289,15 @@ export class UserProfileService {
288289
request: UserProfileGetCurrentParams['request']
289290
): Promise<string | undefined> {
290291
try {
292+
const id = extractApiKeyIdFromAuthzHeader(request.headers.authorization);
293+
if (!id) {
294+
this.logger.debug(`Failed to decode API key ID from Authorization header.`);
295+
return undefined;
296+
}
297+
291298
const response = await clusterClient.asScoped(request).asCurrentUser.security.getApiKey({
292299
with_profile_uid: true,
300+
id,
293301
});
294302

295303
if (response.api_keys && response.api_keys.length > 0) {

0 commit comments

Comments
 (0)