Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using JWT credentials to grant API keys. #172444

Merged
merged 6 commits into from Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .buildkite/ftr_configs.yml
Expand Up @@ -363,6 +363,7 @@ enabled:
- x-pack/test/search_sessions_integration/config.ts
- x-pack/test/security_api_integration/anonymous_es_anonymous.config.ts
- x-pack/test/security_api_integration/anonymous.config.ts
- x-pack/test/security_api_integration/api_keys.config.ts
- x-pack/test/security_api_integration/audit.config.ts
- x-pack/test/security_api_integration/http_bearer.config.ts
- x-pack/test/security_api_integration/http_no_auth_providers.config.ts
Expand Down
3 changes: 0 additions & 3 deletions config/serverless.yml
Expand Up @@ -96,9 +96,6 @@ console.autocompleteDefinitions.endpointsAvailability: serverless
# Do not check the ES version when running on Serverless
elasticsearch.ignoreVersionMismatch: true

# Allow authentication via the Elasticsearch JWT realm with the `shared_secret` client authentication type.
elasticsearch.requestHeadersWhitelist: ['authorization', 'es-client-authentication']

# Limit maxSockets to 800 as we do in ESS, which improves reliability under high loads.
elasticsearch.maxSockets: 800

Expand Down
Expand Up @@ -45,6 +45,7 @@ test('set correct defaults', () => {
"pingTimeout": "PT30S",
"requestHeadersWhitelist": Array [
"authorization",
"es-client-authentication",
],
"requestTimeout": "PT30S",
"serviceAccountToken": undefined,
Expand Down
Expand Up @@ -94,7 +94,7 @@ export const configSchema = schema.object({
}),
],
{
defaultValue: ['authorization'],
defaultValue: ['authorization', 'es-client-authentication'],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

azasypkin marked this conversation as resolved.
Show resolved Hide resolved
}
),
customHeaders: schema.recordOf(schema.string(), schema.string(), {
Expand Down
Expand Up @@ -536,6 +536,51 @@ describe('API Keys', () => {
});
});

it('calls `grantApiKey` with proper parameters for the Bearer scheme with client authentication', async () => {
mockLicense.isEnabled.mockReturnValue(true);
mockClusterClient.asInternalUser.security.grantApiKey.mockResponseOnce({
id: '123',
name: 'key-name',
api_key: 'abc123',
encoded: 'utf8',
});
const result = await apiKeys.grantAsInternalUser(
httpServerMock.createKibanaRequest({
headers: {
authorization: `Bearer foo-access-token`,
'es-client-authentication': 'SharedSecret secret',
},
}),
{
name: 'test_api_key',
role_descriptors: { foo: true },
expiration: '1d',
}
);
expect(result).toEqual({
api_key: 'abc123',
id: '123',
name: 'key-name',
encoded: 'utf8',
});
expect(mockValidateKibanaPrivileges).not.toHaveBeenCalled(); // this is only called if kibana_role_descriptors is defined
expect(mockClusterClient.asInternalUser.security.grantApiKey).toHaveBeenCalledWith({
body: {
api_key: {
name: 'test_api_key',
role_descriptors: { foo: true },
expiration: '1d',
},
grant_type: 'access_token',
access_token: 'foo-access-token',
client_authentication: {
scheme: 'SharedSecret',
value: 'secret',
},
},
});
});

it('throw error for other schemes', async () => {
mockLicense.isEnabled.mockReturnValue(true);
await expect(
Expand Down
Expand Up @@ -32,6 +32,8 @@ import {

export type { UpdateAPIKeyParams, UpdateAPIKeyResult };

const ELASTICSEARCH_CLIENT_AUTHENTICATION_HEADER = 'es-client-authentication';

/**
* Represents the options to create an APIKey class instance that will be
* shared between functions (create, invalidate, etc).
Expand Down Expand Up @@ -269,6 +271,13 @@ export class APIKeys implements APIKeysType {
`Unable to grant an API Key, request does not contain an authorization header`
);
}

// Try to extract optional Elasticsearch client credentials (currently only used by JWT).
const clientAuthorizationHeader = HTTPAuthorizationHeader.parseFromRequest(
request,
ELASTICSEARCH_CLIENT_AUTHENTICATION_HEADER
);

const { expiration, metadata, name } = createParams;

const roleDescriptors =
Expand All @@ -281,7 +290,8 @@ export class APIKeys implements APIKeysType {

const params = this.getGrantParams(
{ expiration, metadata, name, role_descriptors: roleDescriptors },
authorizationHeader
authorizationHeader,
clientAuthorizationHeader
);

// User needs `manage_api_key` or `grant_api_key` privilege to use this API
Expand Down Expand Up @@ -399,13 +409,22 @@ export class APIKeys implements APIKeysType {

private getGrantParams(
createParams: CreateRestAPIKeyParams | CreateRestAPIKeyWithKibanaPrivilegesParams,
authorizationHeader: HTTPAuthorizationHeader
authorizationHeader: HTTPAuthorizationHeader,
clientAuthorizationHeader: HTTPAuthorizationHeader | null
): GrantAPIKeyParams {
if (authorizationHeader.scheme.toLowerCase() === 'bearer') {
return {
api_key: createParams,
grant_type: 'access_token',
access_token: authorizationHeader.credentials,
...(clientAuthorizationHeader
? {
client_authentication: {
scheme: clientAuthorizationHeader.scheme,
value: clientAuthorizationHeader.credentials,
},
}
: {}),
};
}

Expand Down
Expand Up @@ -75,6 +75,23 @@ describe('HTTPAuthorizationHeader.parseFromRequest()', () => {
expect(header!.credentials).toBe(credentials);
}
});

it('parses custom headers', () => {
const mockRequest = httpServerMock.createKibanaRequest({
headers: { 'es-client-authentication': 'SharedSecret secret' },
});

// Doesn't parse custom headers by default.
expect(HTTPAuthorizationHeader.parseFromRequest(mockRequest)).toBeNull();

const header = HTTPAuthorizationHeader.parseFromRequest(
mockRequest,
'es-client-authentication'
);
expect(header).not.toBeNull();
expect(header?.scheme).toBe('SharedSecret');
expect(header?.credentials).toBe('secret');
});
});

describe('toString()', () => {
Expand Down
Expand Up @@ -27,9 +27,11 @@ export class HTTPAuthorizationHeader {
/**
* Parses request's `Authorization` HTTP header if present.
* @param request Request instance to extract the authorization header from.
* @param [headerName] Optional name of the HTTP header to extract authentication information from. By default, the
* authentication information is extracted from the `Authorization` HTTP header.
*/
static parseFromRequest(request: KibanaRequest) {
const authorizationHeaderValue = request.headers.authorization;
static parseFromRequest(request: KibanaRequest, headerName = 'authorization') {
const authorizationHeaderValue = request.headers[headerName.toLowerCase()];
if (!authorizationHeaderValue || typeof authorizationHeaderValue !== 'string') {
return null;
}
Expand Down
66 changes: 66 additions & 0 deletions x-pack/test/security_api_integration/api_keys.config.ts
@@ -0,0 +1,66 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { resolve } from 'path';
import { FtrConfigProviderContext } from '@kbn/test';
import { services } from './services';

export default async function ({ readConfigFile }: FtrConfigProviderContext) {
const xPackAPITestsConfig = await readConfigFile(require.resolve('../api_integration/config.ts'));

const testEndpointsPlugin = resolve(__dirname, '../security_functional/plugins/test_endpoints');
const jwksPath = require.resolve('@kbn/security-api-integration-helpers/oidc/jwks.json');

return {
testFiles: [require.resolve('./tests/api_keys')],
servers: xPackAPITestsConfig.get('servers'),
security: { disableTestUser: true },
services,
junit: {
reportName: 'X-Pack Security API Integration Tests (Api Keys)',
},

esTestCluster: {
...xPackAPITestsConfig.get('esTestCluster'),
serverArgs: [
...xPackAPITestsConfig.get('esTestCluster.serverArgs'),
'xpack.security.authc.token.enabled=true',

// JWT WITH shared secret
'xpack.security.authc.realms.jwt.jwt_with_secret.allowed_audiences=elasticsearch',
`xpack.security.authc.realms.jwt.jwt_with_secret.allowed_issuer=https://kibana.elastic.co/jwt/`,
`xpack.security.authc.realms.jwt.jwt_with_secret.allowed_signature_algorithms=[RS256]`,
`xpack.security.authc.realms.jwt.jwt_with_secret.allowed_subjects=elastic-agent`,
`xpack.security.authc.realms.jwt.jwt_with_secret.claims.principal=sub`,
'xpack.security.authc.realms.jwt.jwt_with_secret.client_authentication.type=shared_secret',
`xpack.security.authc.realms.jwt.jwt_with_secret.client_authentication.shared_secret=my_super_secret`,
'xpack.security.authc.realms.jwt.jwt_with_secret.order=0',
`xpack.security.authc.realms.jwt.jwt_with_secret.pkc_jwkset_path=${jwksPath}`,
`xpack.security.authc.realms.jwt.jwt_with_secret.token_type=access_token`,

// JWT WITHOUT shared secret
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏅 thanks for testing both with and without shared secret

'xpack.security.authc.realms.jwt.jwt_without_secret.allowed_audiences=elasticsearch',
`xpack.security.authc.realms.jwt.jwt_without_secret.allowed_issuer=https://kibana.elastic.co/jwt/no-secret`,
`xpack.security.authc.realms.jwt.jwt_without_secret.allowed_signature_algorithms=[RS256]`,
`xpack.security.authc.realms.jwt.jwt_without_secret.allowed_subjects=elastic-agent-no-secret`,
`xpack.security.authc.realms.jwt.jwt_without_secret.claims.principal=sub`,
'xpack.security.authc.realms.jwt.jwt_without_secret.client_authentication.type=none',
'xpack.security.authc.realms.jwt.jwt_without_secret.order=1',
`xpack.security.authc.realms.jwt.jwt_without_secret.pkc_jwkset_path=${jwksPath}`,
`xpack.security.authc.realms.jwt.jwt_without_secret.token_type=access_token`,
],
},

kbnTestServer: {
...xPackAPITestsConfig.get('kbnTestServer'),
serverArgs: [
...xPackAPITestsConfig.get('kbnTestServer.serverArgs'),
`--plugin-path=${testEndpointsPlugin}`,
],
},
};
}
@@ -0,0 +1,99 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import expect from '@kbn/expect';
import { adminTestUser } from '@kbn/test';
import { FtrProviderContext } from '../../ftr_provider_context';

export default function ({ getService }: FtrProviderContext) {
const supertest = getService('supertestWithoutAuth');
const esSupertest = getService('esSupertest');

describe('Grant API keys', () => {
async function validateApiKey(username: string, encodedApiKey: string) {
const { body: user } = await supertest
.get('/internal/security/me')
.set('kbn-xsrf', 'xxx')
.set('Authorization', `ApiKey ${encodedApiKey}`)
.expect(200);

expect(user.username).to.eql(username);
expect(user.authentication_provider).to.eql({ name: '__http__', type: 'http' });
expect(user.authentication_type).to.eql('api_key');
}

it('should properly grant API key with `Basic` credentials', async function () {
const credentials = Buffer.from(
`${adminTestUser.username}:${adminTestUser.password}`
).toString('base64');

const { body: apiKey } = await supertest
.post('/api_keys/_grant')
.set('Authorization', `Basic ${credentials}`)
.set('kbn-xsrf', 'xxx')
.send({ name: 'my-basic-api-key' })
.expect(200);
expect(apiKey.name).to.eql('my-basic-api-key');

await validateApiKey(adminTestUser.username, apiKey.encoded);
});

it('should properly grant API key with `Bearer` credentials', async function () {
const { body: token } = await esSupertest
.post('/_security/oauth2/token')
.send({ grant_type: 'password', ...adminTestUser })
.expect(200);

const { body: apiKey } = await supertest
.post('/api_keys/_grant')
.set('Authorization', `Bearer ${token.access_token}`)
.set('kbn-xsrf', 'xxx')
.send({ name: 'my-bearer-api-key' })
.expect(200);
expect(apiKey.name).to.eql('my-bearer-api-key');

await validateApiKey(adminTestUser.username, apiKey.encoded);
});

describe('with JWT credentials', function () {
// When we run tests on MKI, JWT realm is configured differently, and we cannot handcraft valid JWTs. We create
// separate `describe` since `this.tags` only works on a test suite level.
this.tags(['skipMKI']);

it('should properly grant API key (with client authentication)', async function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question should we include negative tests to ensure that end-to-end error handling works correctly if we:

  • Fail to provide a ES-Client-Authentication header
  • Provide a malformed ES-Client-Authentication header
  • Provide a well-formed, but incorrect ES-Client-Authentication header
  • Provide a valid ES-Client-Authentication header with a valid ES bearer token instead of a JWT

While this should be handled more or less automatically, we've benefited in the past from these types of "redundant" tests.

Copy link
Member Author

@azasypkin azasypkin Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these tests will fail during authentication stage, so we'll effectively be testing authentication, not grantApiKey API, but I agree that these tests will be useful, so I added them to a separate test suite. Done in e795826!

const jsonWebToken =
'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodHRwczovL2tpYmFuYS5lbGFzdGljLmNvL2p3dC8iLCJzdWIiOiJlbGFzdGljLWFnZW50IiwiYXVkIjoiZWxhc3RpY3NlYXJjaCIsIm5hbWUiOiJFbGFzdGljIEFnZW50IiwiaWF0Ijo5NDY2ODQ4MDAsImV4cCI6NDA3MDkwODgwMH0.P7RHKZlLskS5DfVRqoVO4ivoIq9rXl2-GW6hhC9NvTSkwphYivcjpTVcyENZvxTTvJJNqcyx6rF3T-7otTTIHBOZIMhZauc5dob-sqcN_mT2htqm3BpSdlJlz60TBq6diOtlNhV212gQCEJMPZj0MNj7kZRj_GsECrTaU7FU0A3HAzkbdx15vQJMKZiFbbQCVI7-X2J0bZzQKIWfMHD-VgHFwOe6nomT-jbYIXtCBDd6fNj1zTKRl-_uzjVqNK-h8YW1h6tE4xvZmXyHQ1-9yNKZIWC7iEaPkBLaBKQulLU5MvW3AtVDUhzm6--5H1J85JH5QhRrnKYRon7ZW5q1AQ';

const { body: apiKey } = await supertest
.post('/api_keys/_grant')
.set('Authorization', `Bearer ${jsonWebToken}`)
.set('ES-Client-Authentication', 'SharedSecret my_super_secret')
.set('kbn-xsrf', 'xxx')
.send({ name: 'my-jwt-secret-api-key' })
.expect(200);
expect(apiKey.name).to.eql('my-jwt-secret-api-key');

await validateApiKey('elastic-agent', apiKey.encoded);
});

it('should properly grant API key (without client authentication)', async function () {
const jsonWebToken =
'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodHRwczovL2tpYmFuYS5lbGFzdGljLmNvL2p3dC9uby1zZWNyZXQiLCJzdWIiOiJlbGFzdGljLWFnZW50LW5vLXNlY3JldCIsImF1ZCI6ImVsYXN0aWNzZWFyY2giLCJuYW1lIjoiRWxhc3RpYyBBZ2VudCIsImlhdCI6OTQ2Njg0ODAwLCJleHAiOjQwNzA5MDg4MDB9.OZ_XIDqMmoWr8XqbWE9C04l1NYMsbGXG0zGPdztT-7PuZirzbSvm8z9T7SqbvsujUMn78vpeHx1HyBukrzrBXw2PKeVCa6PGPBtJ_m1fpsCffelHGAD3n2Mu3HanQmdmamHG6JbyLGUwWJ9F31M1xWFAtnMTqP0yeaDOw_9t0WVXHAedVNjvJIrz2X09GHpa9RXxSA0hDuzPotw41kzSrCOhsiBXTNUUNiv4BQ6LNmxbIS6XcXab6LxnQEKtu7XbziaokHKjdZpVAWG8GF8fu0i77GGszNE30RBonYUUPbBrBjhEueK7M8HXTwdHCalRMGsXqD8qS0-TGzii6G-4vg';

const { body: apiKey } = await supertest
.post('/api_keys/_grant')
.set('Authorization', `Bearer ${jsonWebToken}`)
.set('kbn-xsrf', 'xxx')
.send({ name: 'my-jwt-api-key' })
.expect(200);
expect(apiKey.name).to.eql('my-jwt-api-key');

await validateApiKey('elastic-agent-no-secret', apiKey.encoded);
});
});
});
}
14 changes: 14 additions & 0 deletions x-pack/test/security_api_integration/tests/api_keys/index.ts
@@ -0,0 +1,14 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { FtrProviderContext } from '../../ftr_provider_context';

export default function ({ loadTestFile }: FtrProviderContext) {
describe('security APIs - Api Keys', function () {
loadTestFile(require.resolve('./grant_api_key'));
});
}
Expand Up @@ -10,13 +10,16 @@ import {
TaskManagerSetupContract,
TaskManagerStartContract,
} from '@kbn/task-manager-plugin/server';
import { SecurityPluginSetup, SecurityPluginStart } from '@kbn/security-plugin/server';
import { initRoutes } from './init_routes';

export interface PluginSetupDependencies {
security: SecurityPluginSetup;
taskManager: TaskManagerSetupContract;
}

export interface PluginStartDependencies {
security: SecurityPluginStart;
taskManager: TaskManagerStartContract;
}

Expand Down