Skip to content

Commit

Permalink
squashed commit
Browse files Browse the repository at this point in the history
WIP - trying to fix integration tests, broken authz for observer user / role

updates authz feature builder to what ying had before we messed it up in our branch

fixes integration tests

add rac api access to apm

adds getIndex functionality which requires the asset name to be passed in, same style as in the rule registry data client, adds update integration tests

fix small merge conflict and update shell script

fix merge conflict in alerting test file

fix most type errors

fix the rest of the type failures

fix integration tests

fix integration tests

fix type error with feature registration in apm

fix integration tests in apm and security solution

fix type checker

fix jest tests for apm

remove console.error statements for eslint

fix type check

update security solution jest tests

cleaning up PR and adding basic unit tests

still need to clean up types in tests and update one test file

fixes snapshot for signals template

fix tests

fix type check failures

update cypress test

undo changes in alert authz class, updates alert privilege in apm feature to 'read', utilizes the 'rule' object available in executor params over querying for the rule SO directly

remove verbose logging from detection api integration tests

fix type

fix jest tests, adds missing mocked rule object to alert executor params

[RAC] [RBAC] adds function to get alerts-as-data index name (#6)

* WIP - test script and route in rule registry to pull index name. I need to test out adding this route within the APM and sec sol plugins specifically and see if they spit back the same .alerts index but with the appropriate asset name despite not providing one.

WIP - DO NOT DELETE THIS CODE

minor cleanup

updates client to require passing in index name, which is now available through the alerts as data client function getAlertsIndex

fix types

* remove outdated comment

update README, adds integration test (skipped) for testing authz with search strategy (#8)

* WIP

* update README, adds integration test (skipped) for testing authz with search strategy

* fix rebase issues

* adds typedoc docs

* adds SKIPPED integration test for timeline search strategy to be unskipped once authorization is added to search strategy

* removes unused references to the rule data client within the rule registry
  • Loading branch information
dhurley14 committed Jun 17, 2021
1 parent bdc8740 commit 4584189
Show file tree
Hide file tree
Showing 104 changed files with 3,910 additions and 99 deletions.
3 changes: 3 additions & 0 deletions packages/kbn-rule-data-utils/src/technical_field_names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const RULE_NAME = 'rule.name' as const;
const RULE_CATEGORY = 'rule.category' as const;
const TAGS = 'tags' as const;
const PRODUCER = `${ALERT_NAMESPACE}.producer` as const;
const OWNER = `${ALERT_NAMESPACE}.owner` as const;
const ALERT_ID = `${ALERT_NAMESPACE}.id` as const;
const ALERT_UUID = `${ALERT_NAMESPACE}.uuid` as const;
const ALERT_START = `${ALERT_NAMESPACE}.start` as const;
Expand All @@ -40,6 +41,7 @@ const fields = {
RULE_CATEGORY,
TAGS,
PRODUCER,
OWNER,
ALERT_ID,
ALERT_UUID,
ALERT_START,
Expand All @@ -62,6 +64,7 @@ export {
RULE_CATEGORY,
TAGS,
PRODUCER,
OWNER,
ALERT_ID,
ALERT_UUID,
ALERT_START,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ const createAlertingAuthorizationMock = () => {
ensureAuthorized: jest.fn(),
filterByRuleTypeAuthorization: jest.fn(),
getFindAuthorizationFilter: jest.fn(),
getAuthorizedAlertsIndices: jest.fn(),
};
return mocked;
};

export const alertingAuthorizationMock: {
create: () => AlertingAuthorizationMock;
create: () => jest.Mocked<PublicMethodsOf<AlertingAuthorization>>; // AlertingAuthorizationMock;
} = {
create: createAlertingAuthorizationMock,
};
124 changes: 90 additions & 34 deletions x-pack/plugins/alerting/server/authorization/alerting_authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export class AlertingAuthorization {
private readonly featuresIds: Promise<Set<string>>;
private readonly allPossibleConsumers: Promise<AuthorizedConsumers>;
private readonly exemptConsumerIds: string[];
// to be used when building the alerts as data index name
// private readonly spaceId: Promise<string | undefined>;

constructor({
alertTypeRegistry,
Expand Down Expand Up @@ -124,6 +126,8 @@ export class AlertingAuthorization {
return new Set();
});

// this.spaceId = getSpace(request).then((maybeSpace) => maybeSpace?.id ?? undefined);

this.allPossibleConsumers = this.featuresIds.then((featuresIds) =>
featuresIds.size
? asAuthorizedConsumers([...this.exemptConsumerIds, ...featuresIds], {
Expand All @@ -138,6 +142,41 @@ export class AlertingAuthorization {
return this.authorization?.mode?.useRbacForRequest(this.request) ?? false;
}

public async getAuthorizedAlertsIndices(featureIds: string[]): Promise<string[] | undefined> {
const augmentedRuleTypes = await this.augmentRuleTypesWithAuthorization(
this.alertTypeRegistry.list(),
[ReadOperations.Find, ReadOperations.Get, WriteOperations.Update],
AlertingAuthorizationEntity.Alert,
new Set(featureIds)
);

const arrayOfAuthorizedRuleTypes = Array.from(augmentedRuleTypes.authorizedRuleTypes);

// As long as the user can read a minimum of one type of rule type produced by the provided feature,
// the user should be provided that features' alerts index.
// Limiting which alerts that user can read on that index will be done via the findAuthorizationFilter
const authorizedFeatures = arrayOfAuthorizedRuleTypes.reduce(
(acc, ruleType) => acc.add(ruleType.producer),
new Set<string>()
);

// when we add the spaceId to the index name, uncomment this line
// const spaceName = await this.spaceName;

const toReturn = Array.from(authorizedFeatures).flatMap((feature) => {
switch (feature) {
case 'apm':
return '.alerts-observability-apm';
case 'siem':
return ['.alerts-security-solution', '.siem-signals'];
default:
return [];
}
});

return toReturn;
}

public async ensureAuthorized({ ruleTypeId, consumer, operation, entity }: EnsureAuthorizedOpts) {
const { authorization } = this;

Expand Down Expand Up @@ -339,13 +378,15 @@ export class AlertingAuthorization {
private async augmentRuleTypesWithAuthorization(
ruleTypes: Set<RegistryAlertType>,
operations: Array<ReadOperations | WriteOperations>,
authorizationEntity: AlertingAuthorizationEntity
authorizationEntity: AlertingAuthorizationEntity,
featuresIds?: Set<string>
): Promise<{
username?: string;
hasAllRequested: boolean;
authorizedRuleTypes: Set<RegistryAlertTypeWithAuth>;
unauthorizedRuleTypes: Set<RegistryAlertTypeWithAuth> | undefined;
}> {
const featuresIds = await this.featuresIds;
const fIds = featuresIds ?? (await this.featuresIds);
if (this.authorization && this.shouldCheckAuthorization()) {
const checkPrivileges = this.authorization.checkPrivilegesDynamicallyWithRequest(
this.request
Expand All @@ -363,7 +404,7 @@ export class AlertingAuthorization {
// as we can't ask ES for the user's individual privileges we need to ask for each feature
// and ruleType in the system whether this user has this privilege
for (const ruleType of ruleTypesWithAuthorization) {
for (const feature of featuresIds) {
for (const feature of fIds) {
for (const operation of operations) {
privilegeToRuleType.set(
this.authorization!.actions.alerting.get(
Expand All @@ -382,47 +423,62 @@ export class AlertingAuthorization {
kibana: [...privilegeToRuleType.keys()],
});

let authorizedRuleTypes;
let unauthorizedRuleTypes;
if (hasAllRequested) {
authorizedRuleTypes = this.augmentWithAuthorizedConsumers(
ruleTypes,
await this.allPossibleConsumers
);
} else {
[authorizedRuleTypes, unauthorizedRuleTypes] = privileges.kibana.reduce(
([authzRuleTypes, unauthzRuleTypes], { authorized, privilege }) => {
if (authorized && privilegeToRuleType.has(privilege)) {
const [
ruleType,
feature,
hasPrivileges,
isAuthorizedAtProducerLevel,
] = privilegeToRuleType.get(privilege)!;
ruleType.authorizedConsumers[feature] = mergeHasPrivileges(
hasPrivileges,
ruleType.authorizedConsumers[feature]
);

if (isAuthorizedAtProducerLevel && this.exemptConsumerIds.length > 0) {
// granting privileges under the producer automatically authorized exempt consumer IDs as well
this.exemptConsumerIds.forEach((exemptId: string) => {
ruleType.authorizedConsumers[exemptId] = mergeHasPrivileges(
hasPrivileges,
ruleType.authorizedConsumers[exemptId]
);
});
}
authzRuleTypes.add(ruleType);
} else if (!authorized) {
const [ruleType, , , ,] = privilegeToRuleType.get(privilege)!;
unauthzRuleTypes.add(ruleType);
}
return [authzRuleTypes, unauthzRuleTypes];
},
[new Set<RegistryAlertTypeWithAuth>(), new Set<RegistryAlertTypeWithAuth>()]
);
}

return {
username,
hasAllRequested,
authorizedRuleTypes: hasAllRequested
? // has access to all features
this.augmentWithAuthorizedConsumers(ruleTypes, await this.allPossibleConsumers)
: // only has some of the required privileges
privileges.kibana.reduce((authorizedRuleTypes, { authorized, privilege }) => {
if (authorized && privilegeToRuleType.has(privilege)) {
const [
ruleType,
feature,
hasPrivileges,
isAuthorizedAtProducerLevel,
] = privilegeToRuleType.get(privilege)!;
ruleType.authorizedConsumers[feature] = mergeHasPrivileges(
hasPrivileges,
ruleType.authorizedConsumers[feature]
);

if (isAuthorizedAtProducerLevel && this.exemptConsumerIds.length > 0) {
// granting privileges under the producer automatically authorized exempt consumer IDs as well
this.exemptConsumerIds.forEach((exemptId: string) => {
ruleType.authorizedConsumers[exemptId] = mergeHasPrivileges(
hasPrivileges,
ruleType.authorizedConsumers[exemptId]
);
});
}
authorizedRuleTypes.add(ruleType);
}
return authorizedRuleTypes;
}, new Set<RegistryAlertTypeWithAuth>()),
authorizedRuleTypes,
unauthorizedRuleTypes,
};
} else {
return {
hasAllRequested: true,
authorizedRuleTypes: this.augmentWithAuthorizedConsumers(
new Set([...ruleTypes].filter((ruleType) => featuresIds.has(ruleType.producer))),
new Set([...ruleTypes].filter((ruleType) => fIds.has(ruleType.producer))),
await this.allPossibleConsumers
),
unauthorizedRuleTypes: undefined,
};
}
}
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/alerting/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ export { FindResult } from './alerts_client';
export { PublicAlertInstance as AlertInstance } from './alert_instance';
export { parseDuration } from './lib';
export { getEsErrorMessage } from './lib/errors';
export {
ReadOperations,
AlertingAuthorizationFilterType,
AlertingAuthorization,
WriteOperations,
AlertingAuthorizationEntity,
} from './authorization';

export const plugin = (initContext: PluginInitializerContext) => new AlertingPlugin(initContext);

Expand Down
10 changes: 6 additions & 4 deletions x-pack/plugins/apm/common/alert_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import type { ValuesType } from 'utility-types';
import type { ActionGroup } from '../../alerting/common';
import { ANOMALY_SEVERITY, ANOMALY_THRESHOLD } from './ml_constants';

export const APM_SERVER_FEATURE_ID = 'apm';

export enum AlertType {
ErrorCount = 'apm.error_rate', // ErrorRate was renamed to ErrorCount but the key is kept as `error_rate` for backwards-compat.
TransactionErrorRate = 'apm.transaction_error_rate',
Expand Down Expand Up @@ -43,7 +45,7 @@ export const ALERT_TYPES_CONFIG: Record<
actionGroups: [THRESHOLD_MET_GROUP],
defaultActionGroupId: THRESHOLD_MET_GROUP_ID,
minimumLicenseRequired: 'basic',
producer: 'apm',
producer: APM_SERVER_FEATURE_ID,
},
[AlertType.TransactionDuration]: {
name: i18n.translate('xpack.apm.transactionDurationAlert.name', {
Expand All @@ -52,7 +54,7 @@ export const ALERT_TYPES_CONFIG: Record<
actionGroups: [THRESHOLD_MET_GROUP],
defaultActionGroupId: THRESHOLD_MET_GROUP_ID,
minimumLicenseRequired: 'basic',
producer: 'apm',
producer: APM_SERVER_FEATURE_ID,
},
[AlertType.TransactionDurationAnomaly]: {
name: i18n.translate('xpack.apm.transactionDurationAnomalyAlert.name', {
Expand All @@ -61,7 +63,7 @@ export const ALERT_TYPES_CONFIG: Record<
actionGroups: [THRESHOLD_MET_GROUP],
defaultActionGroupId: THRESHOLD_MET_GROUP_ID,
minimumLicenseRequired: 'basic',
producer: 'apm',
producer: APM_SERVER_FEATURE_ID,
},
[AlertType.TransactionErrorRate]: {
name: i18n.translate('xpack.apm.transactionErrorRateAlert.name', {
Expand All @@ -70,7 +72,7 @@ export const ALERT_TYPES_CONFIG: Record<
actionGroups: [THRESHOLD_MET_GROUP],
defaultActionGroupId: THRESHOLD_MET_GROUP_ID,
minimumLicenseRequired: 'basic',
producer: 'apm',
producer: APM_SERVER_FEATURE_ID,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
import React, { useCallback, useMemo } from 'react';
import { useParams } from 'react-router-dom';
import { useKibana } from '../../../../../../../src/plugins/kibana_react/public';
import { AlertType } from '../../../../common/alert_types';
import {
AlertType,
APM_SERVER_FEATURE_ID,
} from '../../../../common/alert_types';
import { getInitialAlertValues } from '../get_initial_alert_values';
import { ApmPluginStartDeps } from '../../../plugin';
interface Props {
Expand All @@ -31,7 +34,7 @@ export function AlertingFlyout(props: Props) {
() =>
alertType &&
services.triggersActionsUi.getAddAlertFlyout({
consumer: 'apm',
consumer: APM_SERVER_FEATURE_ID,
onClose: onCloseAddFlyout,
alertTypeId: alertType,
canChangeTrigger: false,
Expand Down
Loading

0 comments on commit 4584189

Please sign in to comment.