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] Create enrollment API keys as current user #96464

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,9 @@ export const postEnrollmentApiKeyHandler: RequestHandler<
export const deleteEnrollmentApiKeyHandler: RequestHandler<
TypeOf<typeof DeleteEnrollmentAPIKeyRequestSchema.params>
> = async (context, request, response) => {
const soClient = context.core.savedObjects.client;
const esClient = context.core.elasticsearch.client.asCurrentUser;
try {
await APIKeyService.deleteEnrollmentApiKey(soClient, esClient, request.params.keyId);
await APIKeyService.deleteEnrollmentApiKey(esClient, request.params.keyId);

const body: DeleteEnrollmentAPIKeyResponse = { action: 'deleted' };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ export async function agentPolicyUpdateEventHandler(

if (action === 'deleted') {
await unenrollForAgentPolicyId(soClient, esClient, agentPolicyId);
await deleteEnrollmentApiKeyForAgentPolicyId(soClient, esClient, agentPolicyId);
await deleteEnrollmentApiKeyForAgentPolicyId(esClient, agentPolicyId);
}
}
54 changes: 33 additions & 21 deletions x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { ENROLLMENT_API_KEYS_INDEX } from '../../constants';
import { agentPolicyService } from '../agent_policy';
import { escapeSearchQueryPhrase } from '../saved_object';

import { createAPIKey, invalidateAPIKeys } from './security';
import { invalidateAPIKeys } from './security';

const uuidRegex = /^\([0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}\)$/;

Expand Down Expand Up @@ -77,14 +77,9 @@ export async function getEnrollmentAPIKey(

/**
* Invalidate an api key and mark it as inactive
* @param soClient
* @param id
*/
export async function deleteEnrollmentApiKey(
soClient: SavedObjectsClientContract,
esClient: ElasticsearchClient,
id: string
) {
export async function deleteEnrollmentApiKey(esClient: ElasticsearchClient, id: string) {
const enrollmentApiKey = await getEnrollmentAPIKey(esClient, id);

await invalidateAPIKeys([enrollmentApiKey.api_key_id]);
Expand All @@ -102,7 +97,6 @@ export async function deleteEnrollmentApiKey(
}

export async function deleteEnrollmentApiKeyForAgentPolicyId(
soClient: SavedObjectsClientContract,
esClient: ElasticsearchClient,
agentPolicyId: string
) {
Expand All @@ -120,7 +114,7 @@ export async function deleteEnrollmentApiKeyForAgentPolicyId(
}

for (const apiKey of items) {
await deleteEnrollmentApiKey(soClient, esClient, apiKey.id);
await deleteEnrollmentApiKey(esClient, apiKey.id);
}
}
}
Expand Down Expand Up @@ -182,19 +176,37 @@ export async function generateEnrollmentAPIKey(
}

const name = providedKeyName ? `${providedKeyName} (${id})` : id;
const key = await createAPIKey(soClient, name, {
// Useless role to avoid to have the privilege of the user that created the key
'fleet-apikey-enroll': {
cluster: [],
applications: [
{
application: '.fleet',
privileges: ['no-privileges'],
resources: ['*'],

const { body: key } = await esClient.security
.createApiKey({
Copy link
Member

Choose a reason for hiding this comment

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

Should we add some metadata to these enrollment keys?

Maybe it would be nice to have this defined as a constant outside which makes it easier to document and reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes we could add some metadata, what do you have in mind here?

Copy link
Member

Choose a reason for hiding this comment

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

In elastic/fleet-server#195 we have:

"metadata" : {
        "agent_id" : "e4dede19-759e-45d5-b08f-0e78dec888e5",
        "managed_by" : "fleet-server",
        "managed" : true,
        "type" : "output"
      }

We could do something similar here:

"metadata" : {
        "managed_by" : "fleet",
        "managed" : true,
        "type" : "enroll" (or similar)
        "policy_id": "1234"
      }

Copy link
Member Author

Choose a reason for hiding this comment

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

Great add this here 9833563

body: {
name,
// @ts-expect-error Metadata in api keys
metadata: {
Copy link
Member

Choose a reason for hiding this comment

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

@nchaulet Do you know if metadata is the key that has to be used here or could we just use any key?

Copy link
Member Author

Choose a reason for hiding this comment

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

managed_by: 'fleet',
managed: true,
type: 'enroll',
policy_id: data.agentPolicyId,
},
],
},
});
role_descriptors: {
// Useless role to avoid to have the privilege of the user that created the key
'fleet-apikey-enroll': {
cluster: [],
index: [],
applications: [
{
application: '.fleet',
privileges: ['no-privileges'],
resources: ['*'],
},
],
},
},
},
})
.catch((err) => {
throw new Error(`Impossible to create an api key: ${err.message}`);
});

if (!key) {
throw new Error(
Expand Down
49 changes: 22 additions & 27 deletions x-pack/test/fleet_api_integration/apis/enrollment_api_keys/crud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,28 @@ export default function (providerContext: FtrProviderContext) {
expect(apiResponse.item).to.have.keys('id', 'api_key', 'api_key_id', 'name', 'policy_id');
});

it('should create an ES ApiKey with metadata', async () => {
const { body: apiResponse } = await supertest
.post(`/api/fleet/enrollment-api-keys`)
.set('kbn-xsrf', 'xxx')
.send({
policy_id: 'policy1',
})
.expect(200);

const { body: apiKeyRes } = await es.security.getApiKey({
id: apiResponse.item.api_key_id,
});

// @ts-expect-error Metadata not yet in the client type
expect(apiKeyRes.api_keys[0].metadata).eql({
policy_id: 'policy1',
managed_by: 'fleet',
managed: true,
type: 'enroll',
});
});

it('should create an ES ApiKey with limited privileges', async () => {
const { body: apiResponse } = await supertest
.post(`/api/fleet/enrollment-api-keys`)
Expand Down Expand Up @@ -162,33 +184,6 @@ export default function (providerContext: FtrProviderContext) {
},
});
});

describe('It should handle error when the Fleet user is invalid', () => {
before(async () => {});
after(async () => {
await getService('supertest')
.post(`/api/fleet/agents/setup`)
.set('kbn-xsrf', 'xxx')
.send({ forceRecreate: true });
});

it('should not allow to create an enrollment api key if the Fleet admin user is invalid', async () => {
await es.security.changePassword({
username: 'fleet_enroll',
body: {
password: Buffer.from((Math.random() * 10000000).toString()).toString('base64'),
},
});
const res = await supertest
.post(`/api/fleet/enrollment-api-keys`)
.set('kbn-xsrf', 'xxx')
.send({
policy_id: 'policy1',
})
.expect(400);
expect(res.body.message).match(/Fleet Admin user is invalid/);
});
});
});
});
}