Skip to content

Commit

Permalink
Add created_by to saved objects (#179344)
Browse files Browse the repository at this point in the history
## Summary

This PR adds an optional `created_by` field to root saved object fields.
We're doing for adding a filter by an
author to the
dashboard listing page (and then other listings)


### Implementation 

#### `created_by: {type: keyword}`

In this implementation, I added `created_by` as a simple keyword field
assuming we will store `profile_uid` only

```
created_by: {
  type: 'keyword',
},
```

The `profile_uid` is not always available, as azasypkin described
[here](#175431 (comment))
It is not available for anonymous users, for users authenticated via
proxy, and, in some cases, for API users authenticated with API keys.
But this is the best way to globally identify users and longer term we
might get `profile_uid` for all the users



#### Accessing `getCurrentUser` from saved object repo 

After exploring different options and discussing with pgayvallet we
decided to provide `getCurrentUser` through existing security extensions
as it's a better isolation of concern, and we avoid leaking the request
down to the SOR.
  • Loading branch information
Dosant authored Apr 4, 2024
1 parent d723a1b commit 7d80e4f
Show file tree
Hide file tree
Showing 40 changed files with 235 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export interface SimpleSavedObject<T = unknown> {
updatedAt: SavedObjectType<T>['updated_at'];
/** The date this object was created */
createdAt: SavedObjectType<T>['created_at'];
/** The user that created this object */
createdBy: SavedObjectType<T>['created_by'];
/**
* Space(s) that this saved object exists in. This attribute is not used for "global" saved object types which are registered with
* `namespaceType: 'agnostic'`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const performBulkCreate = async <T>(
preflight: preflightHelper,
serializer: serializerHelper,
migration: migrationHelper,
user: userHelper,
} = helpers;
const { securityExtension } = extensions;
const namespace = commonHelper.getCurrentNamespace(options.namespace);
Expand All @@ -83,6 +84,7 @@ export const performBulkCreate = async <T>(
managed: optionsManaged,
} = options;
const time = getCurrentTime();
const createdBy = userHelper.getCurrentUserProfileUid();

let preflightCheckIndexCounter = 0;
const expectedResults = objects.map<ExpectedResult>((object) => {
Expand Down Expand Up @@ -231,6 +233,7 @@ export const performBulkCreate = async <T>(
managed: setManaged({ optionsManaged, objectManaged: object.managed }),
updated_at: time,
created_at: time,
...(createdBy && { created_by: createdBy }),
references: object.references || [],
originId,
}) as SavedObjectSanitizedDoc<T>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const performCreate = async <T>(
preflight: preflightHelper,
serializer: serializerHelper,
migration: migrationHelper,
user: userHelper,
} = helpers;
const { securityExtension } = extensions;

Expand All @@ -69,6 +70,7 @@ export const performCreate = async <T>(
validationHelper.validateOriginId(type, options);

const time = getCurrentTime();
const createdBy = userHelper.getCurrentUserProfileUid();
let savedObjectNamespace: string | undefined;
let savedObjectNamespaces: string[] | undefined;
let existingOriginId: string | undefined;
Expand Down Expand Up @@ -133,6 +135,7 @@ export const performCreate = async <T>(
managed: setManaged({ optionsManaged: managed }),
created_at: time,
updated_at: time,
...(createdBy && { created_by: createdBy }),
...(Array.isArray(references) && { references }),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ describe('find', () => {
'managed',
'updated_at',
'created_at',
'created_by',
'originId',
],
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import type { IValidationHelper } from './validation';
import type { IPreflightCheckHelper } from './preflight_check';
import type { ISerializerHelper } from './serializer';
import type { IMigrationHelper } from './migration';
import type { UserHelper } from './user';

export { CommonHelper, type ICommonHelper } from './common';
export { EncryptionHelper, type IEncryptionHelper } from './encryption';
export { ValidationHelper, type IValidationHelper } from './validation';
export { SerializerHelper, type ISerializerHelper } from './serializer';
export { MigrationHelper, type IMigrationHelper } from './migration';
export { UserHelper } from './user';
export {
PreflightCheckHelper,
type IPreflightCheckHelper,
Expand All @@ -36,4 +38,5 @@ export interface RepositoryHelpers {
preflight: IPreflightCheckHelper;
serializer: ISerializerHelper;
migration: IMigrationHelper;
user: UserHelper;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { ISavedObjectsSecurityExtension } from '@kbn/core-saved-objects-server';

export class UserHelper {
private securityExtension?: ISavedObjectsSecurityExtension;

constructor({ securityExtension }: { securityExtension?: ISavedObjectsSecurityExtension }) {
this.securityExtension = securityExtension;
}

getCurrentUserProfileUid(): string | undefined {
return this.securityExtension?.getCurrentUser()?.profile_uid;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ describe('#getSavedObjectFromSource', () => {
const originId = 'originId';
// eslint-disable-next-line @typescript-eslint/naming-convention
const updated_at = 'updatedAt';
// eslint-disable-next-line @typescript-eslint/naming-convention
const created_by = 'createdBy';
const managed = false;

function createRawDoc(
Expand All @@ -120,6 +122,7 @@ describe('#getSavedObjectFromSource', () => {
managed,
originId,
updated_at,
created_by,
...namespaceAttrs,
},
};
Expand All @@ -142,6 +145,7 @@ describe('#getSavedObjectFromSource', () => {
references,
type,
updated_at,
created_by,
version: encodeHitVersion(doc),
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export function getSavedObjectFromSource<T>(
originId,
updated_at: updatedAt,
created_at: createdAt,
created_by: createdBy,
coreMigrationVersion,
typeMigrationVersion,
managed,
Expand All @@ -134,6 +135,7 @@ export function getSavedObjectFromSource<T>(
...(originId && { originId }),
...(updatedAt && { updated_at: updatedAt }),
...(createdAt && { created_at: createdAt }),
...(createdBy && { created_by: createdBy }),
version: encodeHitVersion(doc),
attributes: doc._source[type],
references: doc._source.references || [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import {
} from '../test_helpers/repository.test.common';
import { savedObjectsExtensionsMock } from '../mocks/saved_objects_extensions.mock';
import { arrayMapsAreEqual } from '@kbn/core-saved-objects-utils-server';
import { mockAuthenticatedUser } from '@kbn/core-security-common/mocks';

describe('SavedObjectsRepository Security Extension', () => {
let client: ReturnType<typeof elasticsearchClientMock.createElasticsearchClient>;
Expand Down Expand Up @@ -423,6 +424,25 @@ describe('SavedObjectsRepository Security Extension', () => {
})
);
});

test(`adds created_by to the saved object when the current user is available`, async () => {
const profileUid = 'profileUid';
mockSecurityExt.getCurrentUser.mockImplementationOnce(() =>
mockAuthenticatedUser({ profile_uid: profileUid })
);
const response = await repository.create(MULTI_NAMESPACE_CUSTOM_INDEX_TYPE, attributes, {
namespace,
});
expect(response.created_by).toBe(profileUid);
});

test(`keeps created_by empty if the current user is not available`, async () => {
mockSecurityExt.getCurrentUser.mockImplementationOnce(() => null);
const response = await repository.create(MULTI_NAMESPACE_CUSTOM_INDEX_TYPE, attributes, {
namespace,
});
expect(response).not.toHaveProperty('created_by');
});
});

describe('#delete', () => {
Expand Down Expand Up @@ -1324,6 +1344,23 @@ describe('SavedObjectsRepository Security Extension', () => {
expect(typeMap).toBe(authMap);
});
});

test(`adds created_by to the saved object when the current user is available`, async () => {
const profileUid = 'profileUid';
mockSecurityExt.getCurrentUser.mockImplementationOnce(() =>
mockAuthenticatedUser({ profile_uid: profileUid })
);
const response = await bulkCreateSuccess(client, repository, [obj1, obj2], { namespace });
expect(response.saved_objects[0].created_by).toBe(profileUid);
expect(response.saved_objects[1].created_by).toBe(profileUid);
});

test(`keeps created_by empty if the current user is not available`, async () => {
mockSecurityExt.getCurrentUser.mockImplementationOnce(() => null);
const response = await bulkCreateSuccess(client, repository, [obj1, obj2], { namespace });
expect(response.saved_objects[0]).not.toHaveProperty('created_by');
expect(response.saved_objects[1]).not.toHaveProperty('created_by');
});
});

describe('#bulkUpdate', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
PreflightCheckHelper,
SerializerHelper,
MigrationHelper,
UserHelper,
} from '../apis/helpers';
import type { RepositoryEsClient } from '../repository_es_client';
import { CreatePointInTimeFinderFn } from '../point_in_time_finder';
Expand Down Expand Up @@ -80,6 +81,9 @@ export const createRepositoryHelpers = ({
migrator,
encryptionHelper,
});
const userHelper = new UserHelper({
securityExtension: extensions?.securityExtension,
});

const helpers: RepositoryHelpers = {
common: commonHelper,
Expand All @@ -88,6 +92,7 @@ export const createRepositoryHelpers = ({
encryption: encryptionHelper,
serializer: serializerHelper,
migration: migrationHelper,
user: userHelper,
};

return helpers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('getRootFields', () => {
"managed",
"updated_at",
"created_at",
"created_by",
"originId",
]
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const ROOT_FIELDS = [
'managed',
'updated_at',
'created_at',
'created_by',
'originId',
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
PreflightCheckHelper,
SerializerHelper,
MigrationHelper,
UserHelper,
} from '../lib/apis/helpers';

export type MigrationHelperMock = jest.Mocked<PublicMethodsOf<MigrationHelper>>;
Expand Down Expand Up @@ -104,13 +105,24 @@ const createPreflightCheckHelperMock = (): PreflightCheckHelperMock => {
return mock;
};

export type UserHelperMock = jest.Mocked<PublicMethodsOf<UserHelper>>;

const createUserHelperMock = (): UserHelperMock => {
const mock: UserHelperMock = {
getCurrentUserProfileUid: jest.fn(),
};

return mock;
};

export interface RepositoryHelpersMock {
common: CommonHelperMock;
encryption: EncryptionHelperMock;
validation: ValidationHelperMock;
preflight: PreflightCheckHelperMock;
serializer: SerializerHelperMock;
migration: MigrationHelperMock;
user: UserHelperMock;
}

const createRepositoryHelpersMock = (): RepositoryHelpersMock => {
Expand All @@ -121,6 +133,7 @@ const createRepositoryHelpersMock = (): RepositoryHelpersMock => {
preflight: createPreflightCheckHelperMock(),
serializer: createSerializerHelperMock(),
migration: createMigrationHelperMock(),
user: createUserHelperMock(),
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const createSecurityExtension = (): jest.Mocked<ISavedObjectsSecurityExtension>
authorizeUpdateSpaces: jest.fn(),
authorizeDisableLegacyUrlAliases: jest.fn(),
auditObjectsForSpaceDeletion: jest.fn(),
getCurrentUser: jest.fn(),
});

const createSpacesExtension = (): jest.Mocked<ISavedObjectsSpacesExtension> => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"@kbn/core-http-server-mocks",
"@kbn/core-saved-objects-migration-server-internal",
"@kbn/utility-types",
"@kbn/core-security-common",
],
"exclude": [
"target/**/*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const createSecurityExtension = (): jest.Mocked<ISavedObjectsSecurityExtension>
authorizeUpdateSpaces: jest.fn(),
authorizeDisableLegacyUrlAliases: jest.fn(),
auditObjectsForSpaceDeletion: jest.fn(),
getCurrentUser: jest.fn(),
});

const createSpacesExtension = (): jest.Mocked<ISavedObjectsSpacesExtension> => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,18 @@ describe('#rawToSavedObject', () => {
expect(actual).toHaveProperty('created_at', now);
});

test('if specified it copies the _source.created_by property to created_by', () => {
const createdBy = 'elastic';
const actual = singleNamespaceSerializer.rawToSavedObject({
_id: 'foo:bar',
_source: {
type: 'foo',
created_by: createdBy,
},
});
expect(actual).toHaveProperty('created_by', createdBy);
});

test(`if _source.updated_at is unspecified it doesn't set updated_at`, () => {
const actual = singleNamespaceSerializer.rawToSavedObject({
_id: 'foo:bar',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export class SavedObjectsSerializer implements ISavedObjectsSerializer {
...(typeMigrationVersion != null ? { typeMigrationVersion } : {}),
...(_source.updated_at && { updated_at: _source.updated_at }),
...(_source.created_at && { created_at: _source.created_at }),
...(_source.created_by && { created_by: _source.created_by }),
...(version && { version }),
};
}
Expand All @@ -144,6 +145,7 @@ export class SavedObjectsSerializer implements ISavedObjectsSerializer {
// eslint-disable-next-line @typescript-eslint/naming-convention
updated_at,
created_at: createdAt,
created_by: createdBy,
version,
references,
coreMigrationVersion,
Expand All @@ -163,6 +165,7 @@ export class SavedObjectsSerializer implements ISavedObjectsSerializer {
...(typeMigrationVersion != null ? { typeMigrationVersion } : {}),
...(updated_at && { updated_at }),
...(createdAt && { created_at: createdAt }),
...(createdBy && { created_by: createdBy }),
};
return {
_id: this.generateRawId(namespace, type, id),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const baseSchema = schema.object<SavedObjectSanitizedDocSchema>({
typeMigrationVersion: schema.maybe(schema.string()),
updated_at: schema.maybe(schema.string()),
created_at: schema.maybe(schema.string()),
created_by: schema.maybe(schema.string()),
version: schema.maybe(schema.string()),
originId: schema.maybe(schema.string()),
managed: schema.maybe(schema.boolean()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export class SimpleSavedObjectImpl<T = unknown> implements SimpleSavedObject<T>
public references: SavedObjectType<T>['references'];
public updatedAt: SavedObjectType<T>['updated_at'];
public createdAt: SavedObjectType<T>['created_at'];
public createdBy: SavedObjectType<T>['created_by'];
public namespaces: SavedObjectType<T>['namespaces'];

constructor(
Expand All @@ -52,6 +53,7 @@ export class SimpleSavedObjectImpl<T = unknown> implements SimpleSavedObject<T>
namespaces,
updated_at: updatedAt,
created_at: createdAt,
created_by: createdBy,
}: SavedObjectType<T>
) {
this.id = id;
Expand All @@ -66,6 +68,7 @@ export class SimpleSavedObjectImpl<T = unknown> implements SimpleSavedObject<T>
this.namespaces = namespaces;
this.updatedAt = updatedAt;
this.createdAt = createdAt;
this.createdBy = createdBy;
if (error) {
this.error = error;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const createSimpleSavedObjectMock = (
references: savedObject.references,
updatedAt: savedObject.updated_at,
createdAt: savedObject.created_at,
createdBy: savedObject.created_by,
namespaces: savedObject.namespaces,
get: jest.fn(),
set: jest.fn(),
Expand Down
Loading

0 comments on commit 7d80e4f

Please sign in to comment.