[Saved Objects] Fixes namespace argument to encryption calls in repository update method (#149074)

Closes #149037.

## Summary

The namespace argument in Saved Object Repository Update method's call
to optionallyEncryptAttributes was being supplied by an unqualified
optional parameter. This means that when the namespace option was not
defined, the current namespace was not being passed to the encrypt
method. This PR updates the argument to the qualified namespace
determined by the optional parameter and the Spaces Extension's
getCurrentNamespace method.

Unit tests have also been updated to catch this case should it occur in
the future. The same consideration will be made for security extension
unit tests in [PR
148165](https://github.com/elastic/kibana/pull/148165).

## Testing
To test, follow the instructions given in [the
issue](https://github.com/elastic/kibana/issues/149037).
This commit is contained in:
Jeramy Soucy 2023-01-19 12:10:31 -05:00 committed by GitHub
parent 2f86985c44
commit c5faa31faf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 204 additions and 3 deletions

View file

@ -31,6 +31,7 @@ import { SavedObject } from '@kbn/core-saved-objects-common';
import { import {
ISavedObjectsSpacesExtension, ISavedObjectsSpacesExtension,
ISavedObjectsSecurityExtension, ISavedObjectsSecurityExtension,
ISavedObjectsEncryptionExtension,
} from '@kbn/core-saved-objects-server'; } from '@kbn/core-saved-objects-server';
import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-utils-server'; import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-utils-server';
import { kibanaMigratorMock } from '../mocks'; import { kibanaMigratorMock } from '../mocks';
@ -56,6 +57,7 @@ import {
setupPerformAuthUnauthorized, setupPerformAuthUnauthorized,
generateIndexPatternSearchResults, generateIndexPatternSearchResults,
bulkDeleteSuccess, bulkDeleteSuccess,
ENCRYPTED_TYPE,
} from '../test_helpers/repository.test.common'; } from '../test_helpers/repository.test.common';
import { savedObjectsExtensionsMock } from '../mocks/saved_objects_extensions.mock'; import { savedObjectsExtensionsMock } from '../mocks/saved_objects_extensions.mock';
@ -72,6 +74,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
let serializer: jest.Mocked<SavedObjectsSerializer>; let serializer: jest.Mocked<SavedObjectsSerializer>;
let mockSpacesExt: jest.Mocked<ISavedObjectsSpacesExtension>; let mockSpacesExt: jest.Mocked<ISavedObjectsSpacesExtension>;
let mockSecurityExt: jest.Mocked<ISavedObjectsSecurityExtension>; let mockSecurityExt: jest.Mocked<ISavedObjectsSecurityExtension>;
let mockEncryptionExt: jest.Mocked<ISavedObjectsEncryptionExtension>;
const registry = createRegistry(); const registry = createRegistry();
const documentMigrator = createDocumentMigrator(registry); const documentMigrator = createDocumentMigrator(registry);
@ -93,7 +96,11 @@ describe('SavedObjectsRepository Spaces Extension', () => {
serializer, serializer,
allowedTypes, allowedTypes,
logger, logger,
extensions: { spacesExtension: mockSpacesExt, securityExtension: mockSecurityExt }, extensions: {
spacesExtension: mockSpacesExt,
securityExtension: mockSecurityExt,
encryptionExtension: mockEncryptionExt,
},
}); });
}; };
@ -922,4 +929,198 @@ describe('SavedObjectsRepository Spaces Extension', () => {
}); });
}); });
}); });
describe(`with encryption extension`, () => {
const currentSpace = 'current_space';
const encryptedSO = {
id: 'encrypted-id',
type: ENCRYPTED_TYPE,
namespaces: ['foo-namespace'],
attributes: {
attrNotSoSecret: '*not-so-secret*',
attrOne: 'one',
attrSecret: '*secret*',
attrThree: 'three',
title: 'Testing',
},
references: [],
};
const decryptedStrippedAttributes = {
attributes: { attrOne: 'one', attrNotSoSecret: 'not-so-secret', attrThree: 'three' },
};
beforeEach(() => {
pointInTimeFinderMock.mockClear();
client = elasticsearchClientMock.createElasticsearchClient();
migrator = kibanaMigratorMock.create();
documentMigrator.prepareMigrations();
migrator.migrateDocument = jest.fn().mockImplementation(documentMigrator.migrate);
migrator.runMigrations = jest.fn().mockResolvedValue([{ status: 'skipped' }]);
logger = loggerMock.create();
serializer = createSpySerializer(registry);
mockSpacesExt = savedObjectsExtensionsMock.createSpacesExtension();
mockEncryptionExt = savedObjectsExtensionsMock.createEncryptionExtension();
mockGetCurrentTime.mockReturnValue(mockTimestamp);
mockGetSearchDsl.mockClear();
repository = instantiateRepository();
mockSpacesExt.getCurrentNamespace.mockImplementation((namespace: string | undefined) => {
if (namespace) {
throw SavedObjectsErrorHelpers.createBadRequestError(ERROR_NAMESPACE_SPECIFIED);
}
return currentSpace;
});
});
describe(`#create`, () => {
test(`calls encryptAttributes with the current namespace by default`, async () => {
mockEncryptionExt.isEncryptableType.mockReturnValue(true);
await repository.create(encryptedSO.type, encryptedSO.attributes);
expect(mockSpacesExt.getCurrentNamespace).toBeCalledTimes(1);
expect(mockSpacesExt.getCurrentNamespace).toHaveBeenCalledWith(undefined);
expect(client.create).toHaveBeenCalledTimes(1);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenCalledTimes(3); // (no upsert) optionallyEncryptAttributes, optionallyDecryptAndRedactSingleResult
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(1, encryptedSO.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(2, encryptedSO.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(3, encryptedSO.type);
expect(mockEncryptionExt.encryptAttributes).toHaveBeenCalledTimes(1);
expect(mockEncryptionExt.encryptAttributes).toHaveBeenCalledWith(
{
id: expect.objectContaining(/[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}/),
namespace: currentSpace,
type: ENCRYPTED_TYPE,
},
encryptedSO.attributes
);
});
});
describe(`#bulkCreate`, () => {
const obj1 = {
type: 'config',
id: '6.0.0-alpha1',
attributes: { title: 'Test One' },
references: [{ name: 'ref_0', type: 'test', id: '1' }],
};
test(`calls encryptAttributes with the current namespace by default`, async () => {
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(false);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(true);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(false);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(true);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(false);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(true);
await bulkCreateSuccess(client, repository, [
obj1,
{ ...encryptedSO, id: undefined }, // Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID
]);
expect(mockSpacesExt.getCurrentNamespace).toBeCalledTimes(1);
expect(mockSpacesExt.getCurrentNamespace).toHaveBeenCalledWith(undefined);
expect(mockSpacesExt.getSearchableNamespaces).not.toHaveBeenCalled();
expect(client.bulk).toHaveBeenCalledTimes(1);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenCalledTimes(6);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(1, obj1.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(2, encryptedSO.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(3, obj1.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(4, encryptedSO.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(5, obj1.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(6, encryptedSO.type);
expect(mockEncryptionExt.encryptAttributes).toHaveBeenCalledTimes(1);
expect(mockEncryptionExt.encryptAttributes).toHaveBeenCalledWith(
{
id: expect.objectContaining(/[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}/),
namespace: currentSpace,
type: ENCRYPTED_TYPE,
},
encryptedSO.attributes
);
});
});
describe(`#update`, () => {
it('calls encryptAttributes with the current namespace by default', async () => {
mockEncryptionExt.isEncryptableType.mockReturnValue(true);
mockEncryptionExt.decryptOrStripResponseAttributes.mockResolvedValue({
...encryptedSO,
...decryptedStrippedAttributes,
});
await updateSuccess(
client,
repository,
registry,
encryptedSO.type,
encryptedSO.id,
encryptedSO.attributes,
{
// no namespace provided
references: encryptedSO.references,
}
);
expect(mockSpacesExt.getCurrentNamespace).toBeCalledTimes(1);
expect(mockSpacesExt.getCurrentNamespace).toHaveBeenCalledWith(undefined);
expect(client.update).toHaveBeenCalledTimes(1);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenCalledTimes(2); // (no upsert) optionallyEncryptAttributes, optionallyDecryptAndRedactSingleResult
expect(mockEncryptionExt.isEncryptableType).toHaveBeenCalledWith(encryptedSO.type);
expect(mockEncryptionExt.encryptAttributes).toHaveBeenCalledTimes(1);
expect(mockEncryptionExt.encryptAttributes).toHaveBeenCalledWith(
{
id: encryptedSO.id,
namespace: currentSpace,
type: ENCRYPTED_TYPE,
},
encryptedSO.attributes
);
});
});
describe(`#bulkUpdate`, () => {
const obj1: SavedObjectsBulkUpdateObject = {
type: 'config',
id: '6.0.0-alpha1',
attributes: { title: 'Test One' },
};
const obj2: SavedObjectsBulkUpdateObject = {
type: 'index-pattern',
id: 'logstash-*',
attributes: { title: 'Test Two' },
};
it(`calls encryptAttributes with the current namespace by default`, async () => {
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(false);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(true);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(false);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(false);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(true);
mockEncryptionExt.isEncryptableType.mockReturnValueOnce(false);
await bulkUpdateSuccess(
client,
repository,
registry,
[obj1, encryptedSO, obj2],
undefined, // No options/namespace specified
undefined,
undefined
);
expect(mockSpacesExt.getCurrentNamespace).toBeCalledTimes(1);
expect(mockSpacesExt.getCurrentNamespace).toHaveBeenCalledWith(undefined);
expect(mockSpacesExt.getSearchableNamespaces).not.toHaveBeenCalled();
expect(mockEncryptionExt.isEncryptableType).toHaveBeenCalledTimes(6); // (no upsert) optionallyEncryptAttributes, optionallyDecryptAndRedactSingleResult
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(1, obj1.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(2, encryptedSO.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(3, obj2.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(4, obj1.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(5, encryptedSO.type);
expect(mockEncryptionExt.isEncryptableType).toHaveBeenNthCalledWith(6, obj2.type);
expect(mockEncryptionExt.encryptAttributes).toHaveBeenCalledTimes(1);
expect(mockEncryptionExt.encryptAttributes).toHaveBeenCalledWith(
{
id: encryptedSO.id,
namespace: currentSpace,
type: ENCRYPTED_TYPE,
},
encryptedSO.attributes
);
});
});
});
}); });

View file

@ -1957,7 +1957,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
...(savedObjectNamespace && { namespace: savedObjectNamespace }), ...(savedObjectNamespace && { namespace: savedObjectNamespace }),
...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }), ...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }),
attributes: { attributes: {
...(await this.optionallyEncryptAttributes(type, id, options.namespace, upsert)), ...(await this.optionallyEncryptAttributes(type, id, namespace, upsert)),
}, },
updated_at: time, updated_at: time,
}); });
@ -1965,7 +1965,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
} }
const doc = { const doc = {
[type]: await this.optionallyEncryptAttributes(type, id, options.namespace, attributes), [type]: await this.optionallyEncryptAttributes(type, id, namespace, attributes),
updated_at: time, updated_at: time,
...(Array.isArray(references) && { references }), ...(Array.isArray(references) && { references }),
}; };