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.

Sign up for GitHub

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

[Synthetics] Make core API key include read_ilm privilege in Stateful only #178897

Merged
merged 9 commits into from
Mar 22, 2024
Merged

[Synthetics] Make core API key include read_ilm privilege in Stateful only #178897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7f62ddf
Make `read_ilm` privilege request depend on Kibana build flavor not b…
justinkambic Mar 18, 2024
aae3693
Reduce sugar consumption.
justinkambic Mar 19, 2024
98581b5
Tighten type definition.
justinkambic Mar 19, 2024
2b9ee96
Fix API test references to updated field.
justinkambic Mar 19, 2024
49acc2d
Fix API tests.
justinkambic Mar 20, 2024
ae958df
Opt to utilize Elasticsearch capabilities API instead of Kibana build…
justinkambic Mar 21, 2024
36cdd2f
Last-minute cleanup.
justinkambic Mar 21, 2024
b5577d2
Remove redundant logger call.
justinkambic Mar 21, 2024
30ddac0
DRY the code off.
justinkambic Mar 22, 2024
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
10 changes: 4 additions & 6 deletions x-pack/plugins/observability_solution/synthetics/server/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,14 @@ import { uptimeRuleTypeFieldMap } from './alert_rules/common';

export class Plugin implements PluginType {
private savedObjectsClient?: SavedObjectsClientContract;
private initContext: PluginInitializerContext;
private logger: Logger;
private readonly logger: Logger;
private server?: SyntheticsServerSetup;
private syntheticsService?: SyntheticsService;
private syntheticsMonitorClient?: SyntheticsMonitorClient;
private readonly telemetryEventsSender: TelemetryEventsSender;

constructor(initializerContext: PluginInitializerContext<UptimeConfig>) {
this.initContext = initializerContext;
this.logger = initializerContext.logger.get();
constructor(private readonly initContext: PluginInitializerContext<UptimeConfig>) {
this.logger = initContext.logger.get();
this.telemetryEventsSender = new TelemetryEventsSender(this.logger);
}

Expand All @@ -52,7 +50,6 @@ export class Plugin implements PluginType {

savedObjectsAdapter.config = config;

this.logger = this.initContext.logger.get();
const { ruleDataService } = plugins.ruleRegistry;

const ruleDataClient = ruleDataService.initializeIndex({
Expand Down Expand Up @@ -110,6 +107,7 @@ export class Plugin implements PluginType {
this.server.encryptedSavedObjects = pluginsStart.encryptedSavedObjects;
this.server.savedObjectsClient = this.savedObjectsClient;
this.server.spaces = pluginsStart.spaces;
this.server.isElasticsearchServerless = coreStart.elasticsearch.getCapabilities().serverless;
}

this.syntheticsService?.start(pluginsStart.taskManager);
Expand Down
15 changes: 8 additions & 7 deletions ...ility_solution/synthetics/server/synthetics_service/authentication/check_has_privilege.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,25 @@ import { SecurityIndexPrivilege } from '@elastic/elasticsearch/lib/api/types';
import { UptimeEsClient } from '../../lib';
import { SyntheticsServerSetup } from '../../types';
import { getFakeKibanaRequest } from '../utils/fake_kibana_request';
import { serviceApiKeyPrivileges, syntheticsIndex } from '../get_api_key';
import { getServiceApiKeyPrivileges, syntheticsIndex } from '../get_api_key';

export const checkHasPrivileges = async (
export const checkHasPrivileges = (
server: SyntheticsServerSetup,
apiKey: { id: string; apiKey: string }
) => {
return await server.coreStart.elasticsearch.client
const { indices: index, cluster } = getServiceApiKeyPrivileges(server.isElasticsearchServerless);
return server.coreStart.elasticsearch.client
.asScoped(getFakeKibanaRequest({ id: apiKey.id, api_key: apiKey.apiKey }))
.asCurrentUser.security.hasPrivileges({
body: {
index: serviceApiKeyPrivileges.indices,
cluster: serviceApiKeyPrivileges.cluster,
index,
cluster,
},
});
};

export const checkIndicesReadPrivileges = async (uptimeEsClient: UptimeEsClient) => {
return await uptimeEsClient.baseESClient.security.hasPrivileges({
export const checkIndicesReadPrivileges = (uptimeEsClient: UptimeEsClient) => {
return uptimeEsClient.baseESClient.security.hasPrivileges({
body: {
index: [
{
Expand Down
18 changes: 17 additions & 1 deletion ...k/plugins/observability_solution/synthetics/server/synthetics_service/get_api_key.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
* 2.0.
*/

import { getAPIKeyForSyntheticsService, syntheticsIndex } from './get_api_key';
import {
getAPIKeyForSyntheticsService,
getServiceApiKeyPrivileges,
syntheticsIndex,
} from './get_api_key';
import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks';
import { securityMock } from '@kbn/security-plugin/server/mocks';
import { coreMock } from '@kbn/core/server/mocks';
Expand Down Expand Up @@ -84,6 +88,18 @@ describe('getAPIKeyTest', function () {
);
});

it.each([
[true, ['monitor', 'read_pipeline']],
[false, ['monitor', 'read_pipeline', 'read_ilm']],
])(
'Includes/excludes `read_ilm` priv when serverless is mode is %s',
(isServerlessEs, expectedClusterPrivs) => {
const { cluster } = getServiceApiKeyPrivileges(isServerlessEs);

expect(cluster).toEqual(expectedClusterPrivs);
}
);

it('invalidates api keys with missing read permissions', async () => {
jest.spyOn(authUtils, 'checkHasPrivileges').mockResolvedValue({
index: {
Expand Down
56 changes: 31 additions & 25 deletions x-pack/plugins/observability_solution/synthetics/server/synthetics_service/get_api_key.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,24 @@ import { checkHasPrivileges } from './authentication/check_has_privilege';

export const syntheticsIndex = 'synthetics-*';

export const serviceApiKeyPrivileges = {
cluster: ['monitor', 'read_ilm', 'read_pipeline'] as SecurityClusterPrivilege[],
indices: [
{
names: [syntheticsIndex],
privileges: [
'view_index_metadata',
'create_doc',
'auto_configure',
'read',
] as SecurityIndexPrivilege[],
},
],
run_as: [],
export const getServiceApiKeyPrivileges = (isServerlessEs: boolean) => {
const cluster: SecurityClusterPrivilege[] = ['monitor', 'read_pipeline'];
if (isServerlessEs === false) cluster.push('read_ilm');
return {
cluster,
indices: [
{
names: [syntheticsIndex],
privileges: [
'view_index_metadata',
'create_doc',
'auto_configure',
'read',
] as SecurityIndexPrivilege[],
},
],
run_as: [],
};
};

export const getAPIKeyForSyntheticsService = async ({
Expand Down Expand Up @@ -84,7 +88,7 @@ export const generateAPIKey = async ({
server: SyntheticsServerSetup;
request: KibanaRequest;
}) => {
const { security } = server;
const { isElasticsearchServerless, security } = server;
const isApiKeysEnabled = await security.authc.apiKeys?.areAPIKeysEnabled();

if (!isApiKeysEnabled) {
Expand All @@ -100,7 +104,7 @@ export const generateAPIKey = async ({
return security.authc.apiKeys?.grantAsInternalUser(request, {
name: 'synthetics-api-key (required for Synthetics App)',
role_descriptors: {
synthetics_writer: serviceApiKeyPrivileges,
synthetics_writer: getServiceApiKeyPrivileges(isElasticsearchServerless),
},
metadata: {
description:
Expand Down Expand Up @@ -202,16 +206,16 @@ export const getSyntheticsEnablement = async ({ server }: { server: SyntheticsSe
};
};

const hasEnablePermissions = async ({ uptimeEsClient }: SyntheticsServerSetup) => {
const hasEnablePermissions = async ({
uptimeEsClient,
isElasticsearchServerless,
}: SyntheticsServerSetup) => {
const { cluster: clusterPrivs, indices: index } =
getServiceApiKeyPrivileges(isElasticsearchServerless);
const hasPrivileges = await uptimeEsClient.baseESClient.security.hasPrivileges({
body: {
cluster: [
'manage_security',
'manage_api_key',
'manage_own_api_key',
...serviceApiKeyPrivileges.cluster,
],
index: serviceApiKeyPrivileges.indices,
cluster: ['manage_security', 'manage_api_key', 'manage_own_api_key', ...clusterPrivs],
index,
},
});

Expand All @@ -221,7 +225,9 @@ const hasEnablePermissions = async ({ uptimeEsClient }: SyntheticsServerSetup) =
manage_api_key: manageApiKey,
manage_own_api_key: manageOwnApiKey,
monitor,
read_ilm: readILM,
// `read_ilm` is going to be `undefined` when ES is in serverless mode,
// so we default it to the ES capabilities value.
read_ilm: readILM = isElasticsearchServerless,
read_pipeline: readPipeline,
} = cluster || {};

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/observability_solution/synthetics/server/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export interface SyntheticsServerSetup {
isDev?: boolean;
coreStart: CoreStart;
pluginsStart: SyntheticsPluginsStartDependencies;
isElasticsearchServerless: boolean;
}

export interface SyntheticsPluginsSetupDependencies {
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/api_integration/apis/security/api_keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import expect from '@kbn/expect';
import { ALL_SPACES_ID } from '@kbn/security-plugin/common/constants';
import { serviceApiKeyPrivileges } from '@kbn/synthetics-plugin/server/synthetics_service/get_api_key';
import { getServiceApiKeyPrivileges } from '@kbn/synthetics-plugin/server/synthetics_service/get_api_key';
import { FtrProviderContext } from '../../ftr_provider_context';

export default function ({ getService }: FtrProviderContext) {
Expand Down Expand Up @@ -116,7 +116,7 @@ export default function ({ getService }: FtrProviderContext) {
expiration: '12d',
kibana_role_descriptors: {
uptime_save: {
elasticsearch: serviceApiKeyPrivileges,
elasticsearch: getServiceApiKeyPrivileges(false),
kibana: [
{
base: [],
Expand Down
6 changes: 3 additions & 3 deletions x-pack/test/api_integration/apis/synthetics/add_monitor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { ALL_SPACES_ID } from '@kbn/security-plugin/common/constants';
import { format as formatUrl } from 'url';

import supertest from 'supertest';
import { serviceApiKeyPrivileges } from '@kbn/synthetics-plugin/server/synthetics_service/get_api_key';
import { getServiceApiKeyPrivileges } from '@kbn/synthetics-plugin/server/synthetics_service/get_api_key';
import { syntheticsMonitorType } from '@kbn/synthetics-plugin/common/types/saved_objects';
import { FtrProviderContext } from '../../ftr_provider_context';
import { getFixtureJson } from './helper/get_fixture_json';
Expand Down Expand Up @@ -217,7 +217,7 @@ export default function ({ getService }: FtrProviderContext) {
expiration: '12d',
kibana_role_descriptors: {
uptime_save: {
elasticsearch: serviceApiKeyPrivileges,
elasticsearch: getServiceApiKeyPrivileges(false),
kibana: [
{
base: [],
Expand Down Expand Up @@ -260,7 +260,7 @@ export default function ({ getService }: FtrProviderContext) {
expiration: '12d',
kibana_role_descriptors: {
uptime_save: {
elasticsearch: serviceApiKeyPrivileges,
elasticsearch: getServiceApiKeyPrivileges(false),
kibana: [
{
base: [],
Expand Down
32 changes: 16 additions & 16 deletions x-pack/test/api_integration/apis/synthetics/synthetics_enablement.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import {
syntheticsApiKeyID,
syntheticsApiKeyObjectType,
} from '@kbn/synthetics-plugin/server/saved_objects/service_api_key';
import { serviceApiKeyPrivileges } from '@kbn/synthetics-plugin/server/synthetics_service/get_api_key';
import { getServiceApiKeyPrivileges } from '@kbn/synthetics-plugin/server/synthetics_service/get_api_key';
import expect from '@kbn/expect';
import { FtrProviderContext } from '../../ftr_provider_context';

export default function ({ getService }: FtrProviderContext) {
const correctPrivileges = {
applications: [],
cluster: ['monitor', 'read_ilm', 'read_pipeline'],
cluster: ['monitor', 'read_pipeline', 'read_ilm'],
indices: [
{
allow_restricted_indices: false,
Expand Down Expand Up @@ -74,7 +74,7 @@ export default function ({ getService }: FtrProviderContext) {
],
elasticsearch: {
cluster: [privilege],
indices: serviceApiKeyPrivileges.indices,
indices: getServiceApiKeyPrivileges(false).indices,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we implement equivalent serverless api integration tests where we pass true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I'm in the process of planning this with the Synthetics-on-Serverless team. I will likely do this in a separate PR that focuses just on implementing this. There will likely be other stuff that needs to change and it won't be a simple lift-and-shift. This will explode the delta for this change to many times its current size and require review burden from some additional folks who aren't on this PR already. Synthetics still isn't live in Prod, so nothing we do will impact existing early adopters, and we will likely include the API tests as acceptance criteria for switching on the plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the second step "As an admin, navigate to Synthetics and wait for the startup flow to display." I am not sure what this means. When I navigate to Synthetics I see this:

Synthetics is designed to work with our managed SaaS testing locations by default. Locally, if you haven't run an instance of this service alongside your Kibana and configured them to talk to each other (requires Minikube and some extra setup), you'll be prompted to create your own "private" location, which involves enrolling an agent in Fleet and creating some config.

I tried creating an agent policy and location, but the API keys generated did not contain the synthetics writer section. How do I generate the synthetics API key?

To see this work locally end-to-end you'd need to also set up Fleet. The way we've done this in the past is via the elastic-package's stack up command. It's not super fun to get that working locally, however. The best way to test things end-to-end would be to deploy this PR to cloud QA. LMK if you'd like me to spin one up so you can see it working. This way you'd avoid having to set these things up manually on your local system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do apologize, the testing steps require some knowledge of running Synthetics locally. I'll spin up a cloud instance of this patch so we can see it work in cloud.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, Justin! I don't think it's necessary to spin up a serverless instance - I trust the other reviewers have given this a thorough testing, I just wanted to understand why I wasn't seeing what I thought I should - I don't have any experience with synthetics. This makes sense to me now. I appreciate the time you took to explain.

And thank you for the follow up re:tests. Is there an open issue for this work already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, I will make the ticket before I merge this though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue is linked. I will probably merge later today, or tomorrow morning.

},
});

Expand Down Expand Up @@ -119,8 +119,8 @@ export default function ({ getService }: FtrProviderContext) {
},
],
elasticsearch: {
cluster: serviceApiKeyPrivileges.cluster,
indices: serviceApiKeyPrivileges.indices,
cluster: getServiceApiKeyPrivileges(false).cluster,
indices: getServiceApiKeyPrivileges(false).indices,
},
});

Expand Down Expand Up @@ -167,8 +167,8 @@ export default function ({ getService }: FtrProviderContext) {
},
],
elasticsearch: {
cluster: serviceApiKeyPrivileges.cluster,
indices: serviceApiKeyPrivileges.indices,
cluster: getServiceApiKeyPrivileges(false).cluster,
indices: getServiceApiKeyPrivileges(false).indices,
},
});

Expand Down Expand Up @@ -233,7 +233,7 @@ export default function ({ getService }: FtrProviderContext) {
expiration: '1d',
role_descriptors: {
'role-a': {
cluster: serviceApiKeyPrivileges.cluster,
cluster: getServiceApiKeyPrivileges(false).cluster,
indices: [
{
names: ['synthetics-*'],
Expand Down Expand Up @@ -269,8 +269,8 @@ export default function ({ getService }: FtrProviderContext) {
},
],
elasticsearch: {
cluster: serviceApiKeyPrivileges.cluster,
indices: serviceApiKeyPrivileges.indices,
cluster: getServiceApiKeyPrivileges(false).cluster,
indices: getServiceApiKeyPrivileges(false).indices,
},
});

Expand Down Expand Up @@ -318,8 +318,8 @@ export default function ({ getService }: FtrProviderContext) {
},
],
elasticsearch: {
cluster: serviceApiKeyPrivileges.cluster,
indices: serviceApiKeyPrivileges.indices,
cluster: getServiceApiKeyPrivileges(false).cluster,
indices: getServiceApiKeyPrivileges(false).indices,
},
});

Expand Down Expand Up @@ -449,8 +449,8 @@ export default function ({ getService }: FtrProviderContext) {
},
],
elasticsearch: {
cluster: serviceApiKeyPrivileges.cluster,
indices: serviceApiKeyPrivileges.indices,
cluster: getServiceApiKeyPrivileges(false).cluster,
indices: getServiceApiKeyPrivileges(false).indices,
},
});

Expand Down Expand Up @@ -562,8 +562,8 @@ export default function ({ getService }: FtrProviderContext) {
},
],
elasticsearch: {
cluster: serviceApiKeyPrivileges.cluster,
indices: serviceApiKeyPrivileges.indices,
cluster: getServiceApiKeyPrivileges(false).cluster,
indices: getServiceApiKeyPrivileges(false).indices,
},
});

Expand Down