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

[ML] Adding capabilities checks to shared functions #70069

Merged
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/server/lib/helpers/setup_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ function getMlSetup(context: APMRequestHandlerContext, request: KibanaRequest) {
const mlClient = ml.mlClient.asScoped(request).callAsCurrentUser;
return {
mlSystem: ml.mlSystemProvider(mlClient, request),
anomalyDetectors: ml.anomalyDetectorsProvider(mlClient),
anomalyDetectors: ml.anomalyDetectorsProvider(mlClient, request),
mlClient,
};
}
2 changes: 1 addition & 1 deletion x-pack/plugins/infra/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export class InfraServerPlugin {
plugins.ml?.mlSystemProvider(context.ml?.mlClient.callAsCurrentUser, request);
const mlAnomalyDetectors =
context.ml &&
plugins.ml?.anomalyDetectorsProvider(context.ml?.mlClient.callAsCurrentUser);
plugins.ml?.anomalyDetectorsProvider(context.ml?.mlClient.callAsCurrentUser, request);
const spaceId = plugins.spaces?.spacesService.getSpaceId(request) || 'default';

return {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/ml/common/types/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const adminMlCapabilities = {
export type UserMlCapabilities = typeof userMlCapabilities;
export type AdminMlCapabilities = typeof adminMlCapabilities;
export type MlCapabilities = UserMlCapabilities & AdminMlCapabilities;
export type MlCapabilitiesKey = keyof MlCapabilities;

export const basicLicenseMlCapabilities = ['canAccessML', 'canFindFileStructure'] as Array<
keyof MlCapabilities
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { LegacyAPICaller } from 'kibana/server';
import { getAdminCapabilities, getUserCapabilities } from './__mocks__/ml_capabilities';
import { capabilitiesProvider } from './check_capabilities';
import { MlLicense } from '../../../common/license';
Expand All @@ -22,8 +23,12 @@ const mlLicenseBasic = {
const mlIsEnabled = async () => true;
const mlIsNotEnabled = async () => false;

const callWithRequestNonUpgrade = async () => ({ upgrade_mode: false });
const callWithRequestUpgrade = async () => ({ upgrade_mode: true });
const callWithRequestNonUpgrade = ((async () => ({
upgrade_mode: false,
})) as unknown) as LegacyAPICaller;
const callWithRequestUpgrade = ((async () => ({
upgrade_mode: true,
})) as unknown) as LegacyAPICaller;

describe('check_capabilities', () => {
describe('getCapabilities() - right number of capabilities', () => {
Expand Down
31 changes: 29 additions & 2 deletions x-pack/plugins/ml/server/lib/capabilities/check_capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { ILegacyScopedClusterClient } from 'kibana/server';
import { LegacyAPICaller, KibanaRequest } from 'kibana/server';
import { mlLog } from '../../client/log';
import {
MlCapabilities,
adminMlCapabilities,
MlCapabilitiesResponse,
ResolveMlCapabilities,
MlCapabilitiesKey,
} from '../../../common/types/capabilities';
import { upgradeCheckProvider } from './upgrade';
import { MlLicense } from '../../../common/license';

export function capabilitiesProvider(
callAsCurrentUser: ILegacyScopedClusterClient['callAsCurrentUser'],
callAsCurrentUser: LegacyAPICaller,
capabilities: MlCapabilities,
mlLicense: MlLicense,
isMlEnabledInSpace: () => Promise<boolean>
Expand Down Expand Up @@ -47,3 +50,27 @@ function disableAdminPrivileges(capabilities: MlCapabilities) {
capabilities.canCreateAnnotation = false;
capabilities.canDeleteAnnotation = false;
}

export type HasMlCapabilities = (capabilities: MlCapabilitiesKey[]) => Promise<void>;

export function hasMlCapabilitiesProvider(resolveMlCapabilities: ResolveMlCapabilities) {
return (request: KibanaRequest): HasMlCapabilities => {
let mlCapabilities: MlCapabilities | null = null;
return async (capabilities: MlCapabilitiesKey[]) => {
try {
mlCapabilities = await resolveMlCapabilities(request);
} catch (e) {
mlLog.warn('Unable to perform ML capabilities check');
throw Error(e);
}

if (mlCapabilities === null) {
throw Error('ML capabilities have not been initialized');
darnautov marked this conversation as resolved.
Show resolved Hide resolved
}

if (capabilities.every((c) => mlCapabilities![c] === true) === false) {
throw Error('Insufficient privileges to access feature');
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering if we could use a custom Error subclass here to enable the routes to translate it into a 403 instead of 500 error. 🤔

We're doing something like that in the infra plugin here:

export class NoLogAnalysisMlJobError extends Error {
constructor(message?: string) {
super(message);
Object.setPrototypeOf(this, new.target.prototype);
}
}

Alternatively, a custom marker attribute on the Error subclass could also be used for identification.

What do you think?

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, I like this idea.
added in fbac69f

Copy link
Member

Choose a reason for hiding this comment

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

awesome, thank you ❤️

}
};
};
}
6 changes: 5 additions & 1 deletion x-pack/plugins/ml/server/lib/capabilities/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { capabilitiesProvider } from './check_capabilities';
export {
capabilitiesProvider,
hasMlCapabilitiesProvider,
HasMlCapabilities,
} from './check_capabilities';
export { setupCapabilitiesSwitcher } from './capabilities_switcher';
6 changes: 2 additions & 4 deletions x-pack/plugins/ml/server/lib/capabilities/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { ILegacyScopedClusterClient } from 'kibana/server';
import { LegacyAPICaller } from 'kibana/server';
import { mlLog } from '../../client/log';

export function upgradeCheckProvider(
callAsCurrentUser: ILegacyScopedClusterClient['callAsCurrentUser']
) {
export function upgradeCheckProvider(callAsCurrentUser: LegacyAPICaller) {
async function isUpgradeInProgress(): Promise<boolean> {
let upgradeInProgress = false;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { LegacyCallAPIOptions, ILegacyScopedClusterClient } from 'kibana/server';
import { LegacyCallAPIOptions, LegacyAPICaller } from 'kibana/server';
import _ from 'lodash';
import { ML_JOB_FIELD_TYPES } from '../../../common/constants/field_types';
import { getSafeAggregationName } from '../../../common/util/job_utils';
Expand Down Expand Up @@ -113,7 +113,7 @@ export class DataVisualizer {
options?: LegacyCallAPIOptions
) => Promise<any>;

constructor(callAsCurrentUser: ILegacyScopedClusterClient['callAsCurrentUser']) {
constructor(callAsCurrentUser: LegacyAPICaller) {
this.callAsCurrentUser = callAsCurrentUser;
}

Expand Down
25 changes: 12 additions & 13 deletions x-pack/plugins/ml/server/models/filter/filter_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import Boom from 'boom';
import { ILegacyScopedClusterClient } from 'kibana/server';
import { LegacyAPICaller } from 'kibana/server';

import { DetectorRule, DetectorRuleScope } from '../../../common/types/detector_rules';

Expand Down Expand Up @@ -58,18 +58,14 @@ interface PartialJob {
}

export class FilterManager {
private _client: ILegacyScopedClusterClient['callAsCurrentUser'];

constructor(client: ILegacyScopedClusterClient['callAsCurrentUser']) {
this._client = client;
}
constructor(private callAsCurrentUser: LegacyAPICaller) {}

async getFilter(filterId: string) {
try {
const [JOBS, FILTERS] = [0, 1];
const results = await Promise.all([
this._client('ml.jobs'),
this._client('ml.filters', { filterId }),
this.callAsCurrentUser('ml.jobs'),
this.callAsCurrentUser('ml.filters', { filterId }),
]);

if (results[FILTERS] && results[FILTERS].filters.length) {
Expand All @@ -91,7 +87,7 @@ export class FilterManager {

async getAllFilters() {
try {
const filtersResp = await this._client('ml.filters');
const filtersResp = await this.callAsCurrentUser('ml.filters');
return filtersResp.filters;
} catch (error) {
throw Boom.badRequest(error);
Expand All @@ -101,7 +97,10 @@ export class FilterManager {
async getAllFilterStats() {
try {
const [JOBS, FILTERS] = [0, 1];
const results = await Promise.all([this._client('ml.jobs'), this._client('ml.filters')]);
const results = await Promise.all([
this.callAsCurrentUser('ml.jobs'),
this.callAsCurrentUser('ml.filters'),
]);

// Build a map of filter_ids against jobs and detectors using that filter.
let filtersInUse: FiltersInUse = {};
Expand Down Expand Up @@ -138,7 +137,7 @@ export class FilterManager {
delete filter.filterId;
try {
// Returns the newly created filter.
return await this._client('ml.addFilter', { filterId, body: filter });
return await this.callAsCurrentUser('ml.addFilter', { filterId, body: filter });
} catch (error) {
throw Boom.badRequest(error);
}
Expand All @@ -158,7 +157,7 @@ export class FilterManager {
}

// Returns the newly updated filter.
return await this._client('ml.updateFilter', {
return await this.callAsCurrentUser('ml.updateFilter', {
filterId,
body,
});
Expand All @@ -168,7 +167,7 @@ export class FilterManager {
}

async deleteFilter(filterId: string) {
return this._client('ml.deleteFilter', { filterId });
return this.callAsCurrentUser('ml.deleteFilter', { filterId });
}

buildFiltersInUse(jobsList: PartialJob[]) {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/ml/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import { registerKibanaSettings } from './lib/register_settings';

declare module 'kibana/server' {
interface RequestHandlerContext {
ml?: {
[PLUGIN_ID]?: {
mlClient: ILegacyScopedClusterClient;
};
}
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/ml/server/routes/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,6 @@ export function systemRoutes(
{
path: '/api/ml/ml_capabilities',
validate: false,
options: {
tags: ['access:ml:canAccessML'],
},
},
mlLicense.basicLicenseAPIGuard(async (context, request, response) => {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,30 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { LegacyAPICaller } from 'kibana/server';
import { LicenseCheck } from '../license_checks';
import { LegacyAPICaller, KibanaRequest } from 'kibana/server';
import { Job } from '../../../common/types/anomaly_detection_jobs';
import { SharedServicesChecks } from '../shared_services';

export interface AnomalyDetectorsProvider {
anomalyDetectorsProvider(
callAsCurrentUser: LegacyAPICaller
callAsCurrentUser: LegacyAPICaller,
request: KibanaRequest
): {
jobs(jobId?: string): Promise<{ count: number; jobs: Job[] }>;
};
}

export function getAnomalyDetectorsProvider(isFullLicense: LicenseCheck): AnomalyDetectorsProvider {
export function getAnomalyDetectorsProvider({
isFullLicense,
getHasMlCapabilities,
}: SharedServicesChecks): AnomalyDetectorsProvider {
return {
anomalyDetectorsProvider(callAsCurrentUser: LegacyAPICaller) {
anomalyDetectorsProvider(callAsCurrentUser: LegacyAPICaller, request: KibanaRequest) {
const hasMlCapabilities = getHasMlCapabilities(request);
return {
jobs(jobId?: string) {
async jobs(jobId?: string) {
isFullLicense();
await hasMlCapabilities(['canGetJobs']);
walterra marked this conversation as resolved.
Show resolved Hide resolved
return callAsCurrentUser('ml.jobs', jobId !== undefined ? { jobId } : {});
},
};
Expand Down
35 changes: 28 additions & 7 deletions x-pack/plugins/ml/server/shared_services/providers/job_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,40 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { LegacyAPICaller } from 'kibana/server';
import { LicenseCheck } from '../license_checks';
import { LegacyAPICaller, KibanaRequest } from 'kibana/server';
import { jobServiceProvider } from '../../models/job_service';
import { SharedServicesChecks } from '../shared_services';

type OrigJobServiceProvider = ReturnType<typeof jobServiceProvider>;

export interface JobServiceProvider {
jobServiceProvider(callAsCurrentUser: LegacyAPICaller): ReturnType<typeof jobServiceProvider>;
jobServiceProvider(
callAsCurrentUser: LegacyAPICaller,
request: KibanaRequest
): {
jobsSummary: OrigJobServiceProvider['jobsSummary'];
};
}

export function getJobServiceProvider(isFullLicense: LicenseCheck): JobServiceProvider {
export function getJobServiceProvider({
isFullLicense,
getHasMlCapabilities,
}: SharedServicesChecks): JobServiceProvider {
return {
jobServiceProvider(callAsCurrentUser: LegacyAPICaller) {
isFullLicense();
return jobServiceProvider(callAsCurrentUser);
jobServiceProvider(callAsCurrentUser: LegacyAPICaller, request: KibanaRequest) {
// const hasMlCapabilities = getHasMlCapabilities(request);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this commented out line needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's related to the commented out code below

const { jobsSummary } = jobServiceProvider(callAsCurrentUser);
return {
async jobsSummary(...args) {
isFullLicense();
// Removed while https://github.com/elastic/kibana/issues/64588 exists.
// SIEM are calling this endpoint with a dummy request object from their alerting
// integration and currently alerting does not supply a request object.
// await hasMlCapabilities(['canGetJobs']);

return jobsSummary(...args);
},
};
},
};
}
Loading