mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 17:59:23 -04:00
resolves https://github.com/elastic/kibana/issues/114536
We've seen a couple of situations where the KQL auth filter is generating
some garbage (null entries where KQL AST nodes are expected):
- issuing a _find HTTP API call against a non-existent Kibana space
- user has Connectors and Actions feature enabled, but nothing else
In these cases, there are no consumers available, and the KQL AST for
expressions like `consumer-field:(alerts or myApp or myOtherApp)` would
end up generating a `null` value instead of a KQL AST node.
The code was changed to not generate the null value.
This led to the next problem - the UX hung because it was waiting for a
health check that ended up throwing an authorization error.
We now catch that error and allow the health check to proceed.
(cherry picked from commit cb40727fb3
)
Co-authored-by: Patrick Mueller <patrick.mueller@elastic.co>
This commit is contained in:
parent
dc2562dd86
commit
a9f1f89d65
7 changed files with 164 additions and 15 deletions
|
@ -268,6 +268,35 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
|
|||
)
|
||||
);
|
||||
});
|
||||
|
||||
test('constructs KQL filter for single rule type with no authorized consumer', async () => {
|
||||
const result = asFiltersByRuleTypeAndConsumer(
|
||||
new Set([
|
||||
{
|
||||
actionGroups: [],
|
||||
defaultActionGroupId: 'default',
|
||||
recoveryActionGroup: RecoveredActionGroup,
|
||||
id: 'myAppAlertType',
|
||||
name: 'myAppAlertType',
|
||||
producer: 'myApp',
|
||||
minimumLicenseRequired: 'basic',
|
||||
isExportable: true,
|
||||
authorizedConsumers: {},
|
||||
enabledInLicense: true,
|
||||
},
|
||||
]),
|
||||
{
|
||||
type: AlertingAuthorizationFilterType.KQL,
|
||||
fieldNames: {
|
||||
ruleTypeId: 'path.to.rule_type_id',
|
||||
consumer: 'consumer-field',
|
||||
},
|
||||
},
|
||||
'space1'
|
||||
);
|
||||
|
||||
expect(result).toEqual(fromKueryExpression(`path.to.rule_type_id:myAppAlertType`));
|
||||
});
|
||||
});
|
||||
|
||||
describe('asEsDslFiltersByRuleTypeAndConsumer', () => {
|
||||
|
@ -602,6 +631,48 @@ describe('asEsDslFiltersByRuleTypeAndConsumer', () => {
|
|||
},
|
||||
});
|
||||
});
|
||||
|
||||
test('constructs KQL filter for single rule type with no authorized consumer', async () => {
|
||||
const result = asFiltersByRuleTypeAndConsumer(
|
||||
new Set([
|
||||
{
|
||||
actionGroups: [],
|
||||
defaultActionGroupId: 'default',
|
||||
recoveryActionGroup: RecoveredActionGroup,
|
||||
id: 'myAppAlertType',
|
||||
name: 'myAppAlertType',
|
||||
producer: 'myApp',
|
||||
minimumLicenseRequired: 'basic',
|
||||
isExportable: true,
|
||||
authorizedConsumers: {},
|
||||
enabledInLicense: true,
|
||||
},
|
||||
]),
|
||||
{
|
||||
type: AlertingAuthorizationFilterType.ESDSL,
|
||||
fieldNames: {
|
||||
ruleTypeId: 'path.to.rule_type_id',
|
||||
consumer: 'consumer-field',
|
||||
},
|
||||
},
|
||||
'space1'
|
||||
);
|
||||
|
||||
expect(result).toMatchInlineSnapshot(`
|
||||
Object {
|
||||
"bool": Object {
|
||||
"minimum_should_match": 1,
|
||||
"should": Array [
|
||||
Object {
|
||||
"match": Object {
|
||||
"path.to.rule_type_id": "myAppAlertType",
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
}
|
||||
`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('asFiltersBySpaceId', () => {
|
||||
|
|
|
@ -42,15 +42,20 @@ export function asFiltersByRuleTypeAndConsumer(
|
|||
const kueryNode = nodeBuilder.or(
|
||||
Array.from(ruleTypes).reduce<KueryNode[]>((filters, { id, authorizedConsumers }) => {
|
||||
ensureFieldIsSafeForQuery('ruleTypeId', id);
|
||||
const andNodes = [
|
||||
nodeBuilder.is(opts.fieldNames.ruleTypeId, id),
|
||||
nodeBuilder.or(
|
||||
Object.keys(authorizedConsumers).map((consumer) => {
|
||||
ensureFieldIsSafeForQuery('consumer', consumer);
|
||||
return nodeBuilder.is(opts.fieldNames.consumer, consumer);
|
||||
})
|
||||
),
|
||||
];
|
||||
|
||||
const andNodes: KueryNode[] = [nodeBuilder.is(opts.fieldNames.ruleTypeId, id)];
|
||||
|
||||
const authorizedConsumersKeys = Object.keys(authorizedConsumers);
|
||||
if (authorizedConsumersKeys.length) {
|
||||
andNodes.push(
|
||||
nodeBuilder.or(
|
||||
authorizedConsumersKeys.map((consumer) => {
|
||||
ensureFieldIsSafeForQuery('consumer', consumer);
|
||||
return nodeBuilder.is(opts.fieldNames.consumer, consumer);
|
||||
})
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
if (opts.fieldNames.spaceIds != null && spaceId != null) {
|
||||
andNodes.push(nodeBuilder.is(opts.fieldNames.spaceIds, spaceId));
|
||||
|
|
|
@ -192,4 +192,24 @@ describe('health check', () => {
|
|||
`"https://www.elastic.co/guide/en/kibana/mocked-test-branch/alerting-setup.html#alerting-prerequisites"`
|
||||
);
|
||||
});
|
||||
|
||||
it('renders children and no warnings if error thrown getting alerting health', async () => {
|
||||
useKibanaMock().services.http.get = jest
|
||||
.fn()
|
||||
// result from triggers_actions_ui health
|
||||
.mockResolvedValueOnce({ isAlertsAvailable: true })
|
||||
// result from alerting health
|
||||
.mockRejectedValueOnce(new Error('for example, not authorized for rules / 403 response'));
|
||||
const { queryByText } = render(
|
||||
<HealthContextProvider>
|
||||
<HealthCheck waitForCheck={true}>
|
||||
<p>{'should render'}</p>
|
||||
</HealthCheck>
|
||||
</HealthContextProvider>
|
||||
);
|
||||
await act(async () => {
|
||||
// wait for useEffect to run
|
||||
});
|
||||
expect(queryByText('should render')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
@ -6,7 +6,7 @@
|
|||
*/
|
||||
|
||||
import React from 'react';
|
||||
import { Option, none, some, fold } from 'fp-ts/lib/Option';
|
||||
import { Option, none, some, fold, isSome } from 'fp-ts/lib/Option';
|
||||
import { pipe } from 'fp-ts/lib/pipeable';
|
||||
import { FormattedMessage } from '@kbn/i18n-react';
|
||||
|
||||
|
@ -14,7 +14,8 @@ import { EuiLink, EuiSpacer } from '@elastic/eui';
|
|||
import { i18n } from '@kbn/i18n';
|
||||
|
||||
import { EuiEmptyPrompt } from '@elastic/eui';
|
||||
import { DocLinksStart } from '@kbn/core/public';
|
||||
import { DocLinksStart, HttpSetup } from '@kbn/core/public';
|
||||
import { AlertingFrameworkHealth } from '@kbn/alerting-plugin/common';
|
||||
import './health_check.scss';
|
||||
import { useHealthContext } from '../context/health_context';
|
||||
import { useKibana } from '../../common/lib/kibana';
|
||||
|
@ -52,12 +53,22 @@ export const HealthCheck: React.FunctionComponent<Props> = ({
|
|||
hasPermanentEncryptionKey: false,
|
||||
};
|
||||
if (healthStatus.isRulesAvailable) {
|
||||
const alertingHealthResult = await alertingFrameworkHealth({ http });
|
||||
healthStatus.isSufficientlySecure = alertingHealthResult.isSufficientlySecure;
|
||||
healthStatus.hasPermanentEncryptionKey = alertingHealthResult.hasPermanentEncryptionKey;
|
||||
// Get the framework health, but if not available, do NOT cause the
|
||||
// framework health errors/toasts to appear, since the state is
|
||||
// actually unknown. These also need to be set to clear the busy
|
||||
// indicator.
|
||||
const alertingHealthResult = await getAlertingFrameworkHealth(http);
|
||||
if (isSome(alertingHealthResult)) {
|
||||
healthStatus.isSufficientlySecure = alertingHealthResult.value.isSufficientlySecure;
|
||||
healthStatus.hasPermanentEncryptionKey =
|
||||
alertingHealthResult.value.hasPermanentEncryptionKey;
|
||||
} else {
|
||||
healthStatus.isSufficientlySecure = true;
|
||||
healthStatus.hasPermanentEncryptionKey = true;
|
||||
}
|
||||
setAlertingHealth(some(healthStatus));
|
||||
}
|
||||
|
||||
setAlertingHealth(some(healthStatus));
|
||||
setLoadingHealthCheck(false);
|
||||
})();
|
||||
}, [http, setLoadingHealthCheck]);
|
||||
|
@ -93,6 +104,19 @@ export const HealthCheck: React.FunctionComponent<Props> = ({
|
|||
);
|
||||
};
|
||||
|
||||
// Return as an Option, returning none if error occurred getting health.
|
||||
// Currently, alerting health returns a 403 if the user is not authorized
|
||||
// for rules.
|
||||
async function getAlertingFrameworkHealth(
|
||||
http: HttpSetup
|
||||
): Promise<Option<AlertingFrameworkHealth>> {
|
||||
try {
|
||||
return some(await alertingFrameworkHealth({ http }));
|
||||
} catch (err) {
|
||||
return none;
|
||||
}
|
||||
}
|
||||
|
||||
interface PromptErrorProps {
|
||||
docLinks: DocLinksStart;
|
||||
className?: string;
|
||||
|
|
|
@ -35,6 +35,21 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('Loads the app with actions but not alerting privilege', () => {
|
||||
before(async () => {
|
||||
await security.testUser.setRoles(['only_actions_role']);
|
||||
});
|
||||
after(async () => {
|
||||
await security.testUser.restoreDefaults();
|
||||
});
|
||||
|
||||
it('Loads the Alerts page but with error', async () => {
|
||||
await pageObjects.common.navigateToApp('triggersActions');
|
||||
const headingText = await pageObjects.triggersActionsUI.getRulesListTitle();
|
||||
expect(headingText).to.be('No permissions to create rules');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Loads the app', () => {
|
||||
before(async () => {
|
||||
await pageObjects.common.navigateToApp('triggersActions');
|
||||
|
|
|
@ -119,6 +119,16 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
|
|||
},
|
||||
],
|
||||
},
|
||||
only_actions_role: {
|
||||
kibana: [
|
||||
{
|
||||
feature: {
|
||||
actions: ['all'],
|
||||
},
|
||||
spaces: ['*'],
|
||||
},
|
||||
],
|
||||
},
|
||||
discover_alert: {
|
||||
kibana: [
|
||||
{
|
||||
|
|
|
@ -48,6 +48,10 @@ export function TriggersActionsPageProvider({ getService }: FtrProviderContext)
|
|||
await createBtn.click();
|
||||
}
|
||||
},
|
||||
async getRulesListTitle() {
|
||||
const noPermissionsTitle = await find.byCssSelector('[data-test-subj="rulesList"] .euiTitle');
|
||||
return await noPermissionsTitle.getVisibleText();
|
||||
},
|
||||
async clickCreateConnectorButton() {
|
||||
const createBtn = await testSubjects.find('createActionButton');
|
||||
const createBtnIsVisible = await createBtn.isDisplayed();
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue