Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] Uninstalltoken saved object namespace agnostic and space aware #190741

Merged
merged 9 commits into from
Aug 28, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@
],
"fleet-space-settings": [],
"fleet-uninstall-tokens": [
"namespaces",
"policy_id",
"token_plain"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1812,6 +1812,9 @@
"fleet-uninstall-tokens": {
"dynamic": false,
"properties": {
"namespaces": {
"type": "keyword"
},
"policy_id": {
"type": "keyword"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"fleet-proxy": "6cb688f0d2dd856400c1dbc998b28704ff70363d",
"fleet-setup-lock": "0dc784792c79b5af5a6e6b5dcac06b0dbaa90bde",
"fleet-space-settings": "b278e82a33978900e53a1253884b5bdbd929c9bb",
"fleet-uninstall-tokens": "ed8aa37e3cdd69e4360709e64944bb81cae0c025",
"fleet-uninstall-tokens": "371a691206845b364bcf6d3693ca7905ffdb71a4",
"graph-workspace": "5cc6bb1455b078fd848c37324672163f09b5e376",
"guided-onboarding-guide-state": "d338972ed887ac480c09a1a7fbf582d6a3827c91",
"guided-onboarding-plugin-state": "bc109e5ef46ca594fdc179eda15f3095ca0a37a4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface UninstallToken {
policy_name: string | null;
token: string;
created_at: string;
namespaces?: string[];
}

export type UninstallTokenMetadata = Omit<UninstallToken, 'token'>;
4 changes: 3 additions & 1 deletion x-pack/plugins/fleet/common/types/rest_spec/agent_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,6 @@ export type FetchAllAgentPoliciesOptions = Pick<
'perPage' | 'kuery' | 'sortField' | 'sortOrder'
> & { fields?: string[] };

export type FetchAllAgentPolicyIdsOptions = Pick<ListWithKuery, 'perPage' | 'kuery'>;
export type FetchAllAgentPolicyIdsOptions = Pick<ListWithKuery, 'perPage' | 'kuery'> & {
spaceId?: string;
};
17 changes: 15 additions & 2 deletions x-pack/plugins/fleet/server/saved_objects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ export const getSavedObjectTypes = (
name: MESSAGE_SIGNING_KEYS_SAVED_OBJECT_TYPE,
indexPattern: INGEST_SAVED_OBJECT_INDEX,
hidden: true,
namespaceType: useSpaceAwareness ? 'single' : 'agnostic',
namespaceType: 'agnostic',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the useSpaceAwareness experimental feature potentially being used anywhere of consequence (by customers, internal deployments, etc)?

Single space ESOs use the object's namespace when constructing AAD. Any instances of Kibana where useSpaceAwareness is active, that then upgrade to a version with this change, would cause existing message signing keys and uninstall token objects to be undecryptable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No useSpaceAwareness is not used in any long standing deployment, the feature is still in development, we used that feature flag more as an integration branch for now than a real feature flag

management: {
importableAndExportable: false,
},
Expand All @@ -999,7 +999,7 @@ export const getSavedObjectTypes = (
name: UNINSTALL_TOKENS_SAVED_OBJECT_TYPE,
indexPattern: INGEST_SAVED_OBJECT_INDEX,
hidden: true,
namespaceType: useSpaceAwareness ? 'single' : 'agnostic',
namespaceType: 'agnostic',
management: {
importableAndExportable: false,
},
Expand All @@ -1008,6 +1008,19 @@ export const getSavedObjectTypes = (
properties: {
policy_id: { type: 'keyword' },
token_plain: { type: 'keyword' },
namespaces: { type: 'keyword' },
},
},
modelVersions: {
'1': {
changes: [
{
type: 'mappings_addition',
addedMappings: {
namespaces: { type: 'keyword' },
},
},
],
},
},
},
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/fleet/server/services/agent_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1615,7 +1615,7 @@ class AgentPolicyService {

public async fetchAllAgentPolicyIds(
soClient: SavedObjectsClientContract,
{ perPage = 1000, kuery = undefined }: FetchAllAgentPolicyIdsOptions = {}
{ perPage = 1000, kuery = undefined, spaceId = undefined }: FetchAllAgentPolicyIdsOptions = {}
): Promise<AsyncIterable<string[]>> {
const savedObjectType = await getAgentPolicySavedObjectType();
return createSoFindIterable<{}>({
Expand All @@ -1627,6 +1627,7 @@ class AgentPolicyService {
sortOrder: 'asc',
fields: ['id'],
filter: kuery ? normalizeKuery(savedObjectType, kuery) : undefined,
namespaces: spaceId ? [spaceId] : undefined,
},
resultsMapper: (data) => {
return data.saved_objects.map((agentPolicySO) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import { createHash } from 'crypto';

import type { KibanaRequest } from '@kbn/core-http-server';

import type { SavedObjectsClientContract } from '@kbn/core/server';
import {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some more tests here? It would be great to cover some of the functionalities added below. It will give us more confidence, for instance in the event of the feature flag clean up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added some in 6aa8e8b

SECURITY_EXTENSION_ID,
SPACES_EXTENSION_ID,
type SavedObjectsClientContract,
} from '@kbn/core/server';
import type { EncryptedSavedObjectsClient } from '@kbn/encrypted-saved-objects-plugin/server';
import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks';

Expand All @@ -29,6 +33,7 @@ import { UNINSTALL_TOKENS_SAVED_OBJECT_TYPE } from '../../../constants';
import { createAppContextStartContractMock, type MockedFleetAppContext } from '../../../mocks';
import { appContextService } from '../../app_context';
import { agentPolicyService } from '../../agent_policy';
import { isSpaceAwarenessEnabled } from '../../spaces/helpers';

import { UninstallTokenService, type UninstallTokenServiceInterface } from '.';

Expand All @@ -42,6 +47,8 @@ interface TokenSO {
created_at: string;
}

jest.mock('../../spaces/helpers');

describe('UninstallTokenService', () => {
const now = new Date().toISOString();
const aDayAgo = new Date(Date.now() - 24 * 3600 * 1000).toISOString();
Expand Down Expand Up @@ -194,7 +201,7 @@ describe('UninstallTokenService', () => {
);
}

function setupMocks(canEncrypt: boolean = true) {
function setupMocks(canEncrypt: boolean = true, scoppedInSpace?: string) {
mockContext = createAppContextStartContractMock();
mockContext.encryptedSavedObjectsSetup = encryptedSavedObjectsMock.createSetup({
canEncrypt,
Expand All @@ -204,13 +211,22 @@ describe('UninstallTokenService', () => {
mockContext.encryptedSavedObjectsStart!.getClient() as jest.Mocked<EncryptedSavedObjectsClient>;
soClientMock = appContextService
.getSavedObjects()
.getScopedClient({} as unknown as KibanaRequest) as jest.Mocked<SavedObjectsClientContract>;
.getScopedClient({} as unknown as KibanaRequest, {
excludedExtensions: [SECURITY_EXTENSION_ID, SPACES_EXTENSION_ID],
}) as jest.Mocked<SavedObjectsClientContract>;
agentPolicyService.deployPolicies = jest.fn();

getAgentPoliciesByIDsMock = jest.fn().mockResolvedValue([]);
agentPolicyService.getByIDs = getAgentPoliciesByIDsMock;

uninstallTokenService = new UninstallTokenService(esoClientMock);
if (scoppedInSpace) {
soClientMock.getCurrentNamespace.mockReturnValue(scoppedInSpace);
}

uninstallTokenService = new UninstallTokenService(
esoClientMock,
scoppedInSpace ? soClientMock : undefined
);
mockFind(canEncrypt);
mockCreatePointInTimeFinder(canEncrypt);
mockCreatePointInTimeFinderAsInternalUser();
Expand Down Expand Up @@ -240,6 +256,7 @@ describe('UninstallTokenService', () => {

beforeEach(() => {
setupMocks(canEncrypt);
jest.mocked(isSpaceAwarenessEnabled).mockResolvedValue(false);
});

describe('get uninstall tokens', () => {
Expand Down Expand Up @@ -277,6 +294,78 @@ describe('UninstallTokenService', () => {
);
});

it('filter namespace with scopped service and space awareneness enabled', async () => {
jest.mocked(isSpaceAwarenessEnabled).mockResolvedValue(true);
setupMocks(canEncrypt, 'test');

const so = getDefaultSO(canEncrypt);
mockCreatePointInTimeFinderAsInternalUser([so]);
getAgentPoliciesByIDsMock.mockResolvedValue([
{ id: so.attributes.policy_id, name: 'cheese' },
] as Array<Partial<AgentPolicy>>);

const token = await uninstallTokenService.getToken(so.id);

const expectedItem: UninstallToken = {
id: so.id,
policy_id: so.attributes.policy_id,
policy_name: 'cheese',
token: getToken(so, canEncrypt),
created_at: so.created_at,
};

expect(token).toEqual(expectedItem);

expect(esoClientMock.createPointInTimeFinderDecryptedAsInternalUser).toHaveBeenCalledWith(
{
type: UNINSTALL_TOKENS_SAVED_OBJECT_TYPE,
filter: `(${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}.attributes.namespaces:test) and (${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}.id: "${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}:${so.id}")`,
perPage: SO_SEARCH_LIMIT,
}
);
expect(getAgentPoliciesByIDsMock).toHaveBeenCalledWith(
soClientMock,
[so.attributes.policy_id],
{ ignoreMissing: true }
);
});

it('do not filter namespace with scopped service and space awareneness disabled', async () => {
jest.mocked(isSpaceAwarenessEnabled).mockResolvedValue(false);
setupMocks(canEncrypt, 'test');

const so = getDefaultSO(canEncrypt);
mockCreatePointInTimeFinderAsInternalUser([so]);
getAgentPoliciesByIDsMock.mockResolvedValue([
{ id: so.attributes.policy_id, name: 'cheese' },
] as Array<Partial<AgentPolicy>>);

const token = await uninstallTokenService.getToken(so.id);

const expectedItem: UninstallToken = {
id: so.id,
policy_id: so.attributes.policy_id,
policy_name: 'cheese',
token: getToken(so, canEncrypt),
created_at: so.created_at,
};

expect(token).toEqual(expectedItem);

expect(esoClientMock.createPointInTimeFinderDecryptedAsInternalUser).toHaveBeenCalledWith(
{
type: UNINSTALL_TOKENS_SAVED_OBJECT_TYPE,
filter: `${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}.id: "${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}:${so.id}"`,
perPage: SO_SEARCH_LIMIT,
}
);
expect(getAgentPoliciesByIDsMock).toHaveBeenCalledWith(
soClientMock,
[so.attributes.policy_id],
{ ignoreMissing: true }
);
});

it('sets `policy_name` to `null` if linked policy does not exist', async () => {
const so = getDefaultSO(canEncrypt);
mockCreatePointInTimeFinderAsInternalUser([so]);
Expand Down Expand Up @@ -341,6 +430,72 @@ describe('UninstallTokenService', () => {
expect(actualItems).toEqual(expectedItems);
});

it('filter by namespace if service is scopped and space awareness is enabled', async () => {
setupMocks(canEncrypt, 'test');
jest.mocked(isSpaceAwarenessEnabled).mockResolvedValue(true);
const so = getDefaultSO(canEncrypt);
const so2 = getDefaultSO2(canEncrypt);
getAgentPoliciesByIDsMock.mockResolvedValue([
{ id: so2.attributes.policy_id, name: 'only I have a name' },
] as Array<Partial<AgentPolicy>>);

const actualItems = (await uninstallTokenService.getTokenMetadata()).items;
const expectedItems: UninstallTokenMetadata[] = [
{
id: so.id,
policy_id: so.attributes.policy_id,
policy_name: null,
created_at: so.created_at,
},
{
id: so2.id,
policy_id: so2.attributes.policy_id,
policy_name: 'only I have a name',
created_at: so2.created_at,
},
];
expect(actualItems).toEqual(expectedItems);

expect(soClientMock.createPointInTimeFinder).toHaveBeenCalledWith(
expect.objectContaining({
filter: `${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}.attributes.namespaces:test`,
})
);
});

it('do not filter by namespace if service is scopped and space awareness is disabled', async () => {
setupMocks(canEncrypt, 'test');
jest.mocked(isSpaceAwarenessEnabled).mockResolvedValue(false);
const so = getDefaultSO(canEncrypt);
const so2 = getDefaultSO2(canEncrypt);
getAgentPoliciesByIDsMock.mockResolvedValue([
{ id: so2.attributes.policy_id, name: 'only I have a name' },
] as Array<Partial<AgentPolicy>>);

const actualItems = (await uninstallTokenService.getTokenMetadata()).items;
const expectedItems: UninstallTokenMetadata[] = [
{
id: so.id,
policy_id: so.attributes.policy_id,
policy_name: null,
created_at: so.created_at,
},
{
id: so2.id,
policy_id: so2.attributes.policy_id,
policy_name: 'only I have a name',
created_at: so2.created_at,
},
];
expect(actualItems).toEqual(expectedItems);

expect(soClientMock.createPointInTimeFinder).toHaveBeenCalledWith(
expect.objectContaining({
filter: undefined,
})
);
});

it('should throw error if created_at is missing', async () => {
const defaultBuckets = getDefaultBuckets(canEncrypt);
defaultBuckets[0].latest.hits.hits[0]._source.created_at = '';
Expand Down
Loading
Loading