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

[Timeline][RBAC] - Add RBAC logic to timeline alerts search strategy #105333

Merged
merged 29 commits into from
Jul 28, 2021
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
00cf943
changes made to search strategy to allow for internal user
yctercero Jul 20, 2021
a2cd92c
adds spaceId filter as option to alerting auth find filter constructor
yctercero Jul 20, 2021
cd764da
updates security solution custom rule script, integration test alerts…
yctercero Jul 20, 2021
7f93d65
terrible commit because it has a lot, but has tests passing for searc…
yctercero Jul 20, 2021
7ff22db
updates search strategy to include a search strategy that queries usi…
yctercero Jul 20, 2021
819b20e
reverted changes to create persistence rule
yctercero Jul 20, 2021
3ae4c47
updated some types
yctercero Jul 20, 2021
b19e93e
tests only work when adding security solution rule registry flag, may…
yctercero Jul 21, 2021
aae42ff
cleanup and updates integration tests to be skipped as they require t…
yctercero Jul 21, 2021
8a4f9de
Merge branch 'master' of github.com:elastic/kibana into rbac_search_s…
yctercero Jul 21, 2021
3453827
made changes to alerting kuery builder per PR comments
yctercero Jul 21, 2021
4523431
updating authorization filter options
yctercero Jul 21, 2021
9fd4b37
updates to use getspaceid, removing async issues
yctercero Jul 21, 2021
e94f321
fixing up type issues
yctercero Jul 21, 2021
c9cfa5b
Removed unnecessary spreads
yctercero Jul 22, 2021
2268c0a
added a flag to determine search cancel logic
yctercero Jul 22, 2021
da04629
moved tests to be under api integration tests specific to auth in ord…
yctercero Jul 22, 2021
6a793b6
Merge branch 'master' of github.com:yctercero/kibana into rbac_search…
yctercero Jul 22, 2021
2125d30
Merge branch 'master' of github.com:elastic/kibana into rbac_search_s…
yctercero Jul 22, 2021
4f8972a
cleaned up some of the auth kuery code per suggestions
yctercero Jul 22, 2021
cc787f6
additional pr feedback around nameing and context comments
yctercero Jul 26, 2021
612b3f7
Merge branch 'master' of github.com:elastic/kibana into rbac_search_s…
yctercero Jul 26, 2021
3591194
additional cleanup and adding some tests
yctercero Jul 26, 2021
a54a480
update security and spaces trial tests
yctercero Jul 26, 2021
775b56f
Merge branch 'master' of github.com:elastic/kibana into rbac_search_s…
yctercero Jul 26, 2021
338a9db
clean up types
yctercero Jul 26, 2021
d87e796
removing new cancel logic to be addressed in follow up pr. updates tests
yctercero Jul 27, 2021
aaa6dad
Merge branch 'master' of github.com:elastic/kibana into rbac_search_s…
yctercero Jul 27, 2021
124ff67
updating one rename from alertTypeRegistry to ruleTypeRegistry
yctercero Jul 28, 2021
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 @@ -17,5 +17,6 @@ export interface ISearchStart<SearchStrategyRequest extends IKibanaSearchRequest
| [aggs](./kibana-plugin-plugins-data-server.isearchstart.aggs.md) | <code>AggsStart</code> | |
| [asScoped](./kibana-plugin-plugins-data-server.isearchstart.asscoped.md) | <code>(request: KibanaRequest) =&gt; IScopedSearchClient</code> | |
| [getSearchStrategy](./kibana-plugin-plugins-data-server.isearchstart.getsearchstrategy.md) | <code>(name?: string) =&gt; ISearchStrategy&lt;SearchStrategyRequest, SearchStrategyResponse&gt;</code> | Get other registered search strategies by name (or, by default, the Elasticsearch strategy). For example, if a new strategy needs to use the already-registered ES search strategy, it can use this function to accomplish that. |
| [searchAsInternalUser](./kibana-plugin-plugins-data-server.isearchstart.searchasinternaluser.md) | <code>ISearchStrategy</code> | Search as the internal Kibana system user. This is not a registered search strategy as we don't want to allow access from the client. |
| [searchSource](./kibana-plugin-plugins-data-server.isearchstart.searchsource.md) | <code>{</code><br/><code> asScoped: (request: KibanaRequest) =&gt; Promise&lt;ISearchStartSearchSource&gt;;</code><br/><code> }</code> | |

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-data-server](./kibana-plugin-plugins-data-server.md) &gt; [ISearchStart](./kibana-plugin-plugins-data-server.isearchstart.md) &gt; [searchAsInternalUser](./kibana-plugin-plugins-data-server.isearchstart.searchasinternaluser.md)

## ISearchStart.searchAsInternalUser property

Search as the internal Kibana system user. This is not a registered search strategy as we don't want to allow access from the client.

<b>Signature:</b>

```typescript
searchAsInternalUser: ISearchStrategy;
```
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

/**
Expand All @@ -13,7 +14,18 @@
* This doesn't work in combination with the `xpack.ruleRegistry.index`
* setting, with which the user can change the index prefix.
*/
export const mapConsumerToIndexName = {

export const ALERTS_CONSUMERS = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for this PR, but I'm wondering if there's a way we can get this dynamically through the registered rule types instead of hard-coding it here. We aren't adding consumers frequently so it's probably not a high priority 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.

Yea, there's been a lot of discussion around this. One of the things that's been suggested is whether or not this state can be registered or maintained within alerting.

#104958

APM: 'apm',
LOGS: 'logs',
INFRASTRUCTURE: 'infrastructure',
OBSERVABILITY: 'observability',
SIEM: 'siem',
SYNTHETICS: 'synthetics',
} as const;
export type ALERTS_CONSUMERS = typeof ALERTS_CONSUMERS[keyof typeof ALERTS_CONSUMERS];

export const mapConsumerToIndexName: Record<ALERTS_CONSUMERS, string | string[]> = {
apm: '.alerts-observability-apm',
logs: '.alerts-observability.logs',
infrastructure: '.alerts-observability.metrics',
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-rule-data-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
*/

export * from './technical_field_names';
export * from './alerts_as_data_rbac';
1 change: 1 addition & 0 deletions src/plugins/data/server/search/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export function createSearchSetupMock(): jest.Mocked<ISearchSetup> {
export function createSearchStartMock(): jest.Mocked<ISearchStart> {
return {
aggs: searchAggsStartMock(),
searchAsInternalUser: createSearchRequestHandlerContext(),
getSearchStrategy: jest.fn(),
asScoped: jest.fn().mockReturnValue(createSearchRequestHandlerContext()),
searchSource: searchSourceMock.createStartContract(),
Expand Down
13 changes: 13 additions & 0 deletions src/plugins/data/server/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
private searchStrategies: StrategyMap = {};
private sessionService: ISearchSessionService;
private asScoped!: ISearchStart['asScoped'];
private searchAsInternalUser!: ISearchStrategy;

constructor(
private initializerContext: PluginInitializerContext<ConfigSchema>,
Expand Down Expand Up @@ -156,6 +157,17 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
)
);

// We don't want to register this because we don't want the client to be able to access this
// strategy, but we do want to expose it to other server-side plugins
// see x-pack/plugins/security_solution/server/search_strategy/timeline/index.ts
// for example use case
this.searchAsInternalUser = enhancedEsSearchStrategyProvider(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lukasolson now that we're exposing this here, should route logic be added to ensure routes throw based on space access? Right now only those that add RBAC explicitly will (which we are doing) hit logic that throws a 403, otherwise, call would go through.

this.initializerContext.config.legacy.globalConfig$,
this.logger,
usage,
true
);

this.registerSearchStrategy(EQL_SEARCH_STRATEGY, eqlSearchStrategyProvider(this.logger));

registerBsearchRoute(
Expand Down Expand Up @@ -220,6 +232,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
uiSettings,
indexPatterns,
}),
searchAsInternalUser: this.searchAsInternalUser,
getSearchStrategy: this.getSearchStrategy,
asScoped: this.asScoped,
searchSource: {
Expand Down
5 changes: 5 additions & 0 deletions src/plugins/data/server/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ export interface ISearchStart<
SearchStrategyResponse extends IKibanaSearchResponse = IEsSearchResponse
> {
aggs: AggsStart;
/**
* Search as the internal Kibana system user. This is not a registered search strategy as we don't
* want to allow access from the client.
*/
searchAsInternalUser: ISearchStrategy;
/**
* Get other registered search strategies by name (or, by default, the Elasticsearch strategy).
* For example, if a new strategy needs to use the already-registered ES search strategy, it can
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/data/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,7 @@ export interface ISearchStart<SearchStrategyRequest extends IKibanaSearchRequest
// (undocumented)
asScoped: (request: KibanaRequest) => IScopedSearchClient;
getSearchStrategy: (name?: string) => ISearchStrategy<SearchStrategyRequest, SearchStrategyResponse>;
searchAsInternalUser: ISearchStrategy;
// (undocumented)
searchSource: {
asScoped: (request: KibanaRequest) => Promise<ISearchStartSearchSource>;
Expand Down Expand Up @@ -1517,7 +1518,7 @@ export function usageProvider(core: CoreSetup_2): SearchUsage;
// src/plugins/data/server/index.ts:280:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:281:1 - (ae-forgotten-export) The symbol "calcAutoIntervalLessThan" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/plugin.ts:81:74 - (ae-forgotten-export) The symbol "DataEnhancements" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/search/types.ts:115:5 - (ae-forgotten-export) The symbol "ISearchStartSearchSource" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/search/types.ts:120:5 - (ae-forgotten-export) The symbol "ISearchStartSearchSource" needs to be exported by the entry point index.d.ts

// (No @packageDocumentation comment for this package)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const securityPluginStart = securityMock.createStart();
const alertingAuthorizationClientFactoryParams: jest.Mocked<AlertingAuthorizationClientFactoryOpts> = {
alertTypeRegistry: alertTypeRegistryMock.create(),
getSpace: jest.fn(),
getSpaceId: jest.fn(),
features,
};

Expand Down Expand Up @@ -73,6 +74,7 @@ test('creates an alerting authorization client with proper constructor arguments
features: alertingAuthorizationClientFactoryParams.features,
auditLogger: expect.any(AlertingAuthorizationAuditLogger),
getSpace: expect.any(Function),
getSpaceId: expect.any(Function),
exemptConsumerIds: [],
});

Expand Down Expand Up @@ -100,6 +102,7 @@ test('creates an alerting authorization client with proper constructor arguments
features: alertingAuthorizationClientFactoryParams.features,
auditLogger: expect.any(AlertingAuthorizationAuditLogger),
getSpace: expect.any(Function),
getSpaceId: expect.any(Function),
exemptConsumerIds: ['exemptConsumerA', 'exemptConsumerB'],
});

Expand All @@ -122,6 +125,7 @@ test('creates an alerting authorization client with proper constructor arguments
features: alertingAuthorizationClientFactoryParams.features,
auditLogger: expect.any(AlertingAuthorizationAuditLogger),
getSpace: expect.any(Function),
getSpaceId: expect.any(Function),
exemptConsumerIds: [],
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface AlertingAuthorizationClientFactoryOpts {
securityPluginSetup?: SecurityPluginSetup;
securityPluginStart?: SecurityPluginStart;
getSpace: (request: KibanaRequest) => Promise<Space | undefined>;
getSpaceId: (request: KibanaRequest) => string | undefined;
features: FeaturesPluginStart;
}

Expand All @@ -29,6 +30,7 @@ export class AlertingAuthorizationClientFactory {
private securityPluginSetup?: SecurityPluginSetup;
private features!: FeaturesPluginStart;
private getSpace!: (request: KibanaRequest) => Promise<Space | undefined>;
private getSpaceId!: (request: KibanaRequest) => string | undefined;

public initialize(options: AlertingAuthorizationClientFactoryOpts) {
if (this.isInitialized) {
Expand All @@ -40,6 +42,7 @@ export class AlertingAuthorizationClientFactory {
this.securityPluginSetup = options.securityPluginSetup;
this.securityPluginStart = options.securityPluginStart;
this.features = options.features;
this.getSpaceId = options.getSpaceId;
}

public create(request: KibanaRequest, exemptConsumerIds: string[] = []): AlertingAuthorization {
Expand All @@ -48,6 +51,7 @@ export class AlertingAuthorizationClientFactory {
authorization: securityPluginStart?.authz,
request,
getSpace: this.getSpace,
getSpaceId: this.getSpaceId,
alertTypeRegistry: this.alertTypeRegistry,
features: features!,
auditLogger: new AlertingAuthorizationAuditLogger(
Expand Down
Loading