Amends the Kibana validation schema for cross cluster API keys (elast…

…ic#183704) closes elastic#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.
kapral18 · May 22, 2024 · 685aadc · 685aadc
1 parent 89c4159
commit 685aadc
Show file tree

Hide file tree

Showing 3 changed files with 270 additions and 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
@@ -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
@@ -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<typeof restApiKeySchema>;
-export type UpdateCrossClusterAPIKeyParams = TypeOf<typeof crossClusterApiKeySchema>;
-export type UpdateRestAPIKeyWithKibanaPrivilegesParams = TypeOf<
-  ReturnType<typeof getRestApiKeyWithKibanaPrivilegesSchema>
->;
-
-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<typeof updateRestApiKeySchema>;
+export type UpdateCrossClusterAPIKeyParams = TypeOf<typeof updateCrossClusterApiKeySchema>;
+export type UpdateRestAPIKeyWithKibanaPrivilegesParams = TypeOf<
+  ReturnType<typeof getRestApiKeyWithKibanaPrivilegesSchema>
+>;
+
 const getRestApiKeyWithKibanaPrivilegesSchema = (
   getBasePrivilegeNames: Parameters<typeof getKibanaRoleSchema>[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
@@ -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', () => {