From 685aadcc5155fa33656fc1f1e0699399c78169e5 Mon Sep 17 00:00:00 2001 From: Jeramy Soucy Date: Wed, 22 May 2024 12:08:49 +0200 Subject: [PATCH] Amends the Kibana validation schema for cross cluster API keys (#183704) closes #183682 ## Summary The validation schema in Kibana's API key endpoints for cross cluster API keys was missing the optional query, field_security, and allow_restricted_indices fields. These have been added, and the schemas have been unified between the create and update endpoints. ### Testing Updated API integration tests to include checking create and update for cross cluster API keys that contain all search options. - x-pack/test/api_integration/apis/security/api_keys.ts ## Release note Fixes an issue in Kibana cross cluster API key endpoints which kept users from creating cross cluster API keys with all possible search options. --- .../src/authentication/api_keys/api_keys.ts | 3 + .../security/server/routes/api_keys/update.ts | 64 ++--- .../api_integration/apis/security/api_keys.ts | 246 ++++++++++++++++++ 3 files changed, 270 insertions(+), 43 deletions(-) diff --git a/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/api_keys.ts b/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/api_keys.ts index 1cbf13a4ad45f..184f21ce2ddac 100644 --- a/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/api_keys.ts +++ b/x-pack/packages/security/plugin_types_server/src/authentication/api_keys/api_keys.ts @@ -185,6 +185,9 @@ export const crossClusterApiKeySchema = restApiKeySchema.extends({ schema.arrayOf( schema.object({ names: schema.arrayOf(schema.string()), + query: schema.maybe(schema.any()), + field_security: schema.maybe(schema.any()), + allow_restricted_indices: schema.maybe(schema.boolean()), }) ) ), diff --git a/x-pack/plugins/security/server/routes/api_keys/update.ts b/x-pack/plugins/security/server/routes/api_keys/update.ts index ef999820c6cae..076e0448be11d 100644 --- a/x-pack/plugins/security/server/routes/api_keys/update.ts +++ b/x-pack/plugins/security/server/routes/api_keys/update.ts @@ -9,7 +9,12 @@ import type { estypes } from '@elastic/elasticsearch'; import { schema } from '@kbn/config-schema'; import type { TypeOf } from '@kbn/config-schema'; -import { elasticsearchRoleSchema, getKibanaRoleSchema } from '@kbn/security-plugin-types-server'; +import { + crossClusterApiKeySchema, + elasticsearchRoleSchema, + getKibanaRoleSchema, + restApiKeySchema, +} from '@kbn/security-plugin-types-server'; import type { RouteDefinitionParams } from '..'; import { UpdateApiKeyValidationError } from '../../authentication/api_keys/api_keys'; @@ -29,56 +34,29 @@ export type UpdateAPIKeyParams = | UpdateCrossClusterAPIKeyParams | UpdateRestAPIKeyWithKibanaPrivilegesParams; -export type UpdateRestAPIKeyParams = TypeOf; -export type UpdateCrossClusterAPIKeyParams = TypeOf; -export type UpdateRestAPIKeyWithKibanaPrivilegesParams = TypeOf< - ReturnType ->; - -const restApiKeySchema = schema.object({ - type: schema.maybe(schema.literal('rest')), +const updateRestApiKeySchema = restApiKeySchema.extends({ + name: null, id: schema.string(), - role_descriptors: schema.recordOf(schema.string(), schema.object({}, { unknowns: 'allow' }), { - defaultValue: {}, - }), - metadata: schema.maybe(schema.object({}, { unknowns: 'allow' })), }); -const crossClusterApiKeySchema = restApiKeySchema.extends({ - type: schema.literal('cross_cluster'), - role_descriptors: null, - access: schema.object( - { - search: schema.maybe( - schema.arrayOf( - schema.object( - { - names: schema.arrayOf(schema.string()), - }, - { unknowns: 'allow' } - ) - ) - ), - replication: schema.maybe( - schema.arrayOf( - schema.object( - { - names: schema.arrayOf(schema.string()), - }, - { unknowns: 'allow' } - ) - ) - ), - }, - { unknowns: 'allow' } - ), +const updateCrossClusterApiKeySchema = crossClusterApiKeySchema.extends({ + name: null, + id: schema.string(), }); +export type UpdateRestAPIKeyParams = TypeOf; +export type UpdateCrossClusterAPIKeyParams = TypeOf; +export type UpdateRestAPIKeyWithKibanaPrivilegesParams = TypeOf< + ReturnType +>; + const getRestApiKeyWithKibanaPrivilegesSchema = ( getBasePrivilegeNames: Parameters[0] ) => restApiKeySchema.extends({ role_descriptors: null, + name: null, + id: schema.string(), kibana_role_descriptors: schema.recordOf( schema.string(), schema.object({ @@ -106,8 +84,8 @@ export function defineUpdateApiKeyRoutes({ path: '/internal/security/api_key', validate: { body: schema.oneOf([ - restApiKeySchema, - crossClusterApiKeySchema, + updateRestApiKeySchema, + updateCrossClusterApiKeySchema, bodySchemaWithKibanaPrivileges, ]), }, diff --git a/x-pack/test/api_integration/apis/security/api_keys.ts b/x-pack/test/api_integration/apis/security/api_keys.ts index 3a9edb14a3d15..f92d427f0160d 100644 --- a/x-pack/test/api_integration/apis/security/api_keys.ts +++ b/x-pack/test/api_integration/apis/security/api_keys.ts @@ -12,6 +12,8 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); + const config = getService('config'); + const basic = config.get('esTestCluster.license') === 'basic'; describe('API Keys', () => { describe('GET /internal/security/api_key/_enabled', () => { @@ -65,6 +67,81 @@ export default function ({ getService }: FtrProviderContext) { expect(name).to.eql('test_api_key_with_metadata'); }); }); + + it(`${basic ? 'basic' : 'trial'} license should ${ + basic ? 'not allow' : 'allow' + } a cross cluster API Key to be created`, async () => { + const result = await supertest + .post('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send({ + type: 'cross_cluster', + name: 'test_cc_api_key', + metadata: {}, + access: { + search: [ + { + names: ['logs*'], + query: { bool: { must_not: { term: { field2: 'value2' } } } }, + field_security: { grant: ['field2'] }, + allow_restricted_indices: true, + }, + ], + }, + }); + expect(result.status).to.be(basic ? 403 : 200); + if (!basic) { + expect(result.body.name).to.be('test_cc_api_key'); + } + }); + + if (!basic) { + it(`Elasticsearch should reject an invalid cross cluster API Key configuration`, async () => { + await supertest + .post('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send({ + type: 'cross_cluster', + name: 'test_cc_api_key_failure', + metadata: {}, + access: { + search: [ + { + names: ['logs*'], + query: { bool: { must_not: { term: { field2: 'value2' } } } }, + }, + ], + // replication section is not allowed if earch contains query or field_security + replication: { + names: ['logs*'], + }, + }, + }) + .expect(400); + + await supertest + .post('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send({ + type: 'cross_cluster', + name: 'test_cc_api_key_failure', + metadata: {}, + access: { + search: [ + { + names: ['logs*'], + field_security: { grant: ['field2'] }, + }, + ], + // replication section is not allowed if earch contains query or field_security + replication: { + names: ['logs*'], + }, + }, + }) + .expect(400); + }); + } }); describe('PUT /internal/security/api_key', () => { @@ -102,7 +179,176 @@ export default function ({ getService }: FtrProviderContext) { const { updated } = response.body; expect(updated).to.eql(true); }); + + const getResult = await supertest + .get('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send(); + + expect(getResult.body.apiKeys).to.not.be(undefined); + const updatedKey = getResult.body.apiKeys.find( + (apiKey: { id: string }) => apiKey.id === id + ); + expect(updatedKey).to.not.be(undefined); + expect(updatedKey.metadata).to.eql({ foo: 'bar' }); + expect(updatedKey.role_descriptors).to.eql({ + role_1: { + cluster: ['monitor'], + indices: [], + applications: [], + run_as: [], + metadata: {}, + transient_metadata: { + enabled: true, + }, + }, + }); }); + + it(`${basic ? 'basic' : 'trial'} license should ${ + basic ? 'not allow' : 'allow' + } a cross cluster API Key to be updated`, async () => { + let id = '123456'; + + const createResult = await supertest + .post('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send({ + type: 'cross_cluster', + name: 'test_cc_api_key', + metadata: {}, + access: { + search: [ + { + names: ['logs*'], + query: { bool: { must_not: { term: { field2: 'value2' } } } }, + field_security: { grant: ['field2'] }, + allow_restricted_indices: true, + }, + ], + }, + }); + expect(createResult.status).to.be(basic ? 403 : 200); + if (!basic) { + id = createResult.body.id; + } + + const updateResult = await supertest + .put('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send({ + type: 'cross_cluster', + id, + metadata: { + foo: 'bar', + }, + access: { + search: [ + { + names: ['somethingelse*'], + query: { bool: { must_not: { term: { field2: 'value3' } } } }, + field_security: { grant: ['field3'] }, + allow_restricted_indices: false, + }, + ], + }, + }); + expect(updateResult.status).to.be(basic ? 403 : 200); + if (!basic) { + expect(updateResult.body.updated).to.be(true); + + const getResult = await supertest + .get('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send(); + + expect(getResult.body.apiKeys).to.not.be(undefined); + const updatedKey = getResult.body.apiKeys.find( + (apiKey: { id: string }) => apiKey.id === id + ); + expect(updatedKey).to.not.be(undefined); + expect(updatedKey.metadata).to.eql({ foo: 'bar' }); + expect(updatedKey.role_descriptors?.cross_cluster?.indices).to.eql([ + { + names: ['somethingelse*'], + privileges: ['read', 'read_cross_cluster', 'view_index_metadata'], + field_security: { grant: ['field3'] }, + query: '{"bool":{"must_not":{"term":{"field2":"value3"}}}}', + allow_restricted_indices: false, + }, + ]); + } + }); + + if (!basic) { + it(`Elasticsearch should reject an invalid cross cluster API Key configuration`, async () => { + const createResult = await supertest + .post('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send({ + type: 'cross_cluster', + name: 'test_cc_api_key', + metadata: {}, + access: { + search: [ + { + names: ['logs*'], + }, + ], + }, + }); + expect(createResult.status).to.be(200); + const id = createResult.body.id; + + await supertest + .put('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send({ + type: 'cross_cluster', + id, + metadata: { + foo: 'bar', + }, + access: { + search: [ + { + names: ['logs*'], + query: { bool: { must_not: { term: { field2: 'value2' } } } }, + }, + ], + // replication section is not allowed if earch contains query or field_security + replication: { + names: ['logs*'], + }, + }, + }) + .expect(400); + + await supertest + .put('/internal/security/api_key') + .set('kbn-xsrf', 'xxx') + .send({ + type: 'cross_cluster', + id, + metadata: { + foo: 'bar', + }, + access: { + search: [ + { + names: ['logs*'], + field_security: { grant: ['field2'] }, + }, + ], + // replication section is not allowed if earch contains query or field_security + replication: { + names: ['logs*'], + }, + }, + }) + .expect(400); + }); + } }); describe('with kibana privileges', () => {