Review#2: add test to verify that non-superuser cannot rotate key, us…

…e proper naming convention for the rotate route URL and more.

x-pack/plugins/encrypted_saved_objects/server/crypto/encryption_key_rotation_service.ts

@@ -85,15 +85,15 @@ export class EncryptionKeyRotationService {
 
     const result = { total: 0, successful: 0, failed: 0 };
     if (registeredSavedObjectTypes.length === 0) {
-      this.options.logger.debug(
+      this.options.logger.info(
         type
           ? `Saved Object type "${type}" is not registered, encryption key rotation is not needed.`
           : 'There are no registered Saved Object types that can have encrypted attributes, encryption key rotation is not needed.'
       );
       return result;
     }
 
-    this.options.logger.debug(
+    this.options.logger.info(
       `Saved Objects with the following types [${registeredSavedObjectTypes}] will be processed.`
     );
 
@@ -201,7 +201,7 @@ export class EncryptionKeyRotationService {
       }
     }
 
-    this.options.logger.debug(
+    this.options.logger.info(
       `Encryption key rotation is completed. ${result.successful} objects out ouf ${result.total} were successfully re-encrypted with the primary encryption key and ${result.failed} objects failed.`
     );
 

x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.test.ts

@@ -39,7 +39,7 @@ describe('Key rotation routes', () => {
     let routeConfig: RouteConfig<any, any, any, any>;
     beforeEach(() => {
       const [rotateRouteConfig, rotateRouteHandler] = router.post.mock.calls.find(
-        ([{ path }]) => path === '/api/encrypted_saved_objects/rotate_key'
+        ([{ path }]) => path === '/api/encrypted_saved_objects/_rotate_key'
       )!;
 
       routeConfig = rotateRouteConfig;
@@ -86,7 +86,7 @@ describe('Key rotation routes', () => {
       const routeParamsMock = routeDefinitionParamsMock.create();
       defineKeyRotationRoutes(routeParamsMock);
       const [, rotateRouteHandler] = routeParamsMock.router.post.mock.calls.find(
-        ([{ path }]) => path === '/api/encrypted_saved_objects/rotate_key'
+        ([{ path }]) => path === '/api/encrypted_saved_objects/_rotate_key'
       )!;
 
       await expect(
@@ -161,7 +161,7 @@ describe('Key rotation routes', () => {
         options: { body: { total: 3, successful: 6, failed: 0 } },
       });
 
-      // And consequent requests resolve properly too.
+      // And subsequent requests resolve properly too.
       await expect(routeHandler(mockContext, mockRequest, kibanaResponseFactory)).resolves.toEqual({
         status: 200,
         payload: { total: 3, successful: 6, failed: 0 },

x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.ts

@@ -25,7 +25,7 @@ export function defineKeyRotationRoutes({
   let rotationInProgress = false;
   router.post(
     {
-      path: '/api/encrypted_saved_objects/rotate_key',
+      path: '/api/encrypted_saved_objects/_rotate_key',
       validate: {
         query: schema.object({
           batchSize: schema.number({

x-pack/test/encrypted_saved_objects_api_integration/services.ts

@@ -4,4 +4,10 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export { services } from '../api_integration/services';
+import { services as commonServices } from '../common/services';
+import { services as apiIntegrationServices } from '../api_integration/services';
+
+export const services = {
+  ...commonServices,
+  supertestWithoutAuth: apiIntegrationServices.supertestWithoutAuth,
+};

x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts

@@ -13,6 +13,7 @@ export default function ({ getService }: FtrProviderContext) {
   const randomness = getService('randomness');
   const supertest = getService('supertest');
   const esArchiver = getService('esArchiver');
+  const security = getService('security');
 
   const SAVED_OBJECT_WITH_SECRET_TYPE = 'saved-object-with-secret';
   const HIDDEN_SAVED_OBJECT_WITH_SECRET_TYPE = 'hidden-saved-object-with-secret';
@@ -539,6 +540,7 @@ export default function ({ getService }: FtrProviderContext) {
     });
 
     describe('key rotation', () => {
+      const supertestWithoutAuth = getService('supertestWithoutAuth');
       const savedObjectsEncryptedWithLegacyKeys: Array<[string, string, string[], boolean]> = [
         [SAVED_OBJECT_WITH_SECRET_TYPE, 'cd9272b2-6a15-4295-bb7b-15f6347e267b', ['default'], false],
         [
@@ -568,23 +570,30 @@ export default function ({ getService }: FtrProviderContext) {
         ],
       ];
 
+      const KIBANA_ADMIN_USERNAME = 'admin';
+      const KIBANA_ADMIN_PASSWORD = 'changeme';
       before(async () => {
+        await security.user.create(KIBANA_ADMIN_USERNAME, {
+          password: KIBANA_ADMIN_PASSWORD,
+          roles: ['kibana_admin'],
+          full_name: 'a kibana admin',
+        });
         await esArchiver.load('key_rotation');
       });
 
       after(async () => {
         await esArchiver.unload('key_rotation');
+        await security.user.delete('admin');
       });
 
       it('#get can properly retrieve objects encrypted with the legacy keys', async () => {
         // Hidden objects cannot be retrieved with standard Saved Objects APIs.
-        for (const [type, id, namespaces, hidden] of savedObjectsEncryptedWithLegacyKeys.filter(
+        for (const [type, id, namespaces] of savedObjectsEncryptedWithLegacyKeys.filter(
           ([, , , hiddenSavedObject]) => !hiddenSavedObject
         )) {
-          const url = hidden
-            ? `/api/saved_objects/${type}/${id}`
-            : `/api/saved_objects/${type}/${id}`;
-          const { body: decryptedResponse } = await supertest.get(url).expect(200);
+          const { body: decryptedResponse } = await supertest
+            .get(`/api/saved_objects/${type}/${id}`)
+            .expect(200);
 
           expect(decryptedResponse.namespaces.sort()).to.eql(namespaces);
           expect(decryptedResponse.attributes).to.eql({
@@ -612,10 +621,19 @@ export default function ({ getService }: FtrProviderContext) {
         }
       });
 
+      it('non-super user cannot rotate encryption key', async () => {
+        await supertestWithoutAuth
+          .post('/api/encrypted_saved_objects/_rotate_key')
+          .set('kbn-xsrf', 'xxx')
+          .auth(KIBANA_ADMIN_USERNAME, KIBANA_ADMIN_PASSWORD)
+          .send()
+          .expect(404);
+      });
+
       // Since this test re-encrypts objects it should always go last in this suite.
-      it('saved objects can be properly re-encrypted', async () => {
+      it('encryption key can be properly rotated by the superuser', async () => {
         await supertest
-          .post('/api/encrypted_saved_objects/rotate_key')
+          .post('/api/encrypted_saved_objects/_rotate_key')
           .set('kbn-xsrf', 'xxx')
           .send()
           .expect(200, { total: 6, successful: 6, failed: 0 });