Skip to content

Commit 726f6ba

Browse files
azasypkinazasypkin
committed
Review#2: add test to verify that non-superuser cannot rotate key, use proper naming convention for the rotate route URL and more.
1 parent fecc14a commit 726f6ba

5 files changed

Lines changed: 39 additions & 15 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,15 @@ export class EncryptionKeyRotationService {
8585

8686
const result = { total: 0, successful: 0, failed: 0 };
8787
if (registeredSavedObjectTypes.length === 0) {
88-
this.options.logger.debug(
88+
this.options.logger.info(
8989
type
9090
? `Saved Object type "${type}" is not registered, encryption key rotation is not needed.`
9191
: 'There are no registered Saved Object types that can have encrypted attributes, encryption key rotation is not needed.'
9292
);
9393
return result;
9494
}
9595

96-
this.options.logger.debug(
96+
this.options.logger.info(
9797
`Saved Objects with the following types [${registeredSavedObjectTypes}] will be processed.`
9898
);
9999

@@ -201,7 +201,7 @@ export class EncryptionKeyRotationService {
201201
}
202202
}
203203

204-
this.options.logger.debug(
204+
this.options.logger.info(
205205
`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.`
206206
);
207207

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe('Key rotation routes', () => {
3939
let routeConfig: RouteConfig<any, any, any, any>;
4040
beforeEach(() => {
4141
const [rotateRouteConfig, rotateRouteHandler] = router.post.mock.calls.find(
42-
([{ path }]) => path === '/api/encrypted_saved_objects/rotate_key'
42+
([{ path }]) => path === '/api/encrypted_saved_objects/_rotate_key'
4343
)!;
4444

4545
routeConfig = rotateRouteConfig;
@@ -86,7 +86,7 @@ describe('Key rotation routes', () => {
8686
const routeParamsMock = routeDefinitionParamsMock.create();
8787
defineKeyRotationRoutes(routeParamsMock);
8888
const [, rotateRouteHandler] = routeParamsMock.router.post.mock.calls.find(
89-
([{ path }]) => path === '/api/encrypted_saved_objects/rotate_key'
89+
([{ path }]) => path === '/api/encrypted_saved_objects/_rotate_key'
9090
)!;
9191

9292
await expect(
@@ -161,7 +161,7 @@ describe('Key rotation routes', () => {
161161
options: { body: { total: 3, successful: 6, failed: 0 } },
162162
});
163163

164-
// And consequent requests resolve properly too.
164+
// And subsequent requests resolve properly too.
165165
await expect(routeHandler(mockContext, mockRequest, kibanaResponseFactory)).resolves.toEqual({
166166
status: 200,
167167
payload: { total: 3, successful: 6, failed: 0 },

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export function defineKeyRotationRoutes({
2525
let rotationInProgress = false;
2626
router.post(
2727
{
28-
path: '/api/encrypted_saved_objects/rotate_key',
28+
path: '/api/encrypted_saved_objects/_rotate_key',
2929
validate: {
3030
query: schema.object({
3131
batchSize: schema.number({

‎x-pack/test/encrypted_saved_objects_api_integration/services.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,10 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
export { services } from '../api_integration/services';
7+
import { services as commonServices } from '../common/services';
8+
import { services as apiIntegrationServices } from '../api_integration/services';
9+
10+
export const services = {
11+
...commonServices,
12+
supertestWithoutAuth: apiIntegrationServices.supertestWithoutAuth,
13+
};

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export default function ({ getService }: FtrProviderContext) {
1313
const randomness = getService('randomness');
1414
const supertest = getService('supertest');
1515
const esArchiver = getService('esArchiver');
16+
const security = getService('security');
1617

1718
const SAVED_OBJECT_WITH_SECRET_TYPE = 'saved-object-with-secret';
1819
const HIDDEN_SAVED_OBJECT_WITH_SECRET_TYPE = 'hidden-saved-object-with-secret';
@@ -539,6 +540,7 @@ export default function ({ getService }: FtrProviderContext) {
539540
});
540541

541542
describe('key rotation', () => {
543+
const supertestWithoutAuth = getService('supertestWithoutAuth');
542544
const savedObjectsEncryptedWithLegacyKeys: Array<[string, string, string[], boolean]> = [
543545
[SAVED_OBJECT_WITH_SECRET_TYPE, 'cd9272b2-6a15-4295-bb7b-15f6347e267b', ['default'], false],
544546
[
@@ -568,23 +570,30 @@ export default function ({ getService }: FtrProviderContext) {
568570
],
569571
];
570572

573+
const KIBANA_ADMIN_USERNAME = 'admin';
574+
const KIBANA_ADMIN_PASSWORD = 'changeme';
571575
before(async () => {
576+
await security.user.create(KIBANA_ADMIN_USERNAME, {
577+
password: KIBANA_ADMIN_PASSWORD,
578+
roles: ['kibana_admin'],
579+
full_name: 'a kibana admin',
580+
});
572581
await esArchiver.load('key_rotation');
573582
});
574583

575584
after(async () => {
576585
await esArchiver.unload('key_rotation');
586+
await security.user.delete('admin');
577587
});
578588

579589
it('#get can properly retrieve objects encrypted with the legacy keys', async () => {
580590
// Hidden objects cannot be retrieved with standard Saved Objects APIs.
581-
for (const [type, id, namespaces, hidden] of savedObjectsEncryptedWithLegacyKeys.filter(
591+
for (const [type, id, namespaces] of savedObjectsEncryptedWithLegacyKeys.filter(
582592
([, , , hiddenSavedObject]) => !hiddenSavedObject
583593
)) {
584-
const url = hidden
585-
? `/api/saved_objects/${type}/${id}`
586-
: `/api/saved_objects/${type}/${id}`;
587-
const { body: decryptedResponse } = await supertest.get(url).expect(200);
594+
const { body: decryptedResponse } = await supertest
595+
.get(`/api/saved_objects/${type}/${id}`)
596+
.expect(200);
588597

589598
expect(decryptedResponse.namespaces.sort()).to.eql(namespaces);
590599
expect(decryptedResponse.attributes).to.eql({
@@ -612,10 +621,19 @@ export default function ({ getService }: FtrProviderContext) {
612621
}
613622
});
614623

624+
it('non-super user cannot rotate encryption key', async () => {
625+
await supertestWithoutAuth
626+
.post('/api/encrypted_saved_objects/_rotate_key')
627+
.set('kbn-xsrf', 'xxx')
628+
.auth(KIBANA_ADMIN_USERNAME, KIBANA_ADMIN_PASSWORD)
629+
.send()
630+
.expect(404);
631+
});
632+
615633
// Since this test re-encrypts objects it should always go last in this suite.
616-
it('saved objects can be properly re-encrypted', async () => {
634+
it('encryption key can be properly rotated by the superuser', async () => {
617635
await supertest
618-
.post('/api/encrypted_saved_objects/rotate_key')
636+
.post('/api/encrypted_saved_objects/_rotate_key')
619637
.set('kbn-xsrf', 'xxx')
620638
.send()
621639
.expect(200, { total: 6, successful: 6, failed: 0 });

0 commit comments

Comments
 (0)