-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[KP] Integrate Audit trail into new es client #71228
Conversation
Pinging @elastic/kibana-platform (Team:Platform) |
}); | ||
|
||
describe('Auditor', () => { | ||
it('logs elasticsearch requests done on behalf on the current & internal user', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be moved to functional tests when the new ES client exposed to the plugins.
await client.callAsInternalUser('ping'); | ||
expect(auditor.add).toHaveBeenCalledTimes(1); | ||
expect(auditor.add).toHaveBeenCalledWith({ | ||
message: 'ping', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the difference in audit log record format between the legacy & new clients:
- in the legacy client, it's not possible to access request method and path unless we auditor is integrated into
ElasticsearchClientLogging
class - in the new client
RequestEvent
doesn't contain client method name
I'm inclining towards keeping this as is until we have the format requirements.
@@ -69,7 +69,7 @@ export class LegacyScopedClusterClient implements ILegacyScopedClusterClient { | |||
if (this.auditor) { | |||
this.auditor.add({ | |||
message: endpoint, | |||
type: 'elasticsearch.call.internalUser', | |||
type: 'legacy.elasticsearch.call.internalUser', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should actually use the same type as the new client. From a user's perspective, I would consider it a bug if they filtered their audit logs on type: elasticsearch.call.*
and they didn't get these events in their results. I know there are slight differences in the data we can expose depending on the client (https://github.com/elastic/kibana/pull/71228/files#r461405740), but it seems we should try to keep the events as similar as possible.
@jportner thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to agree, but I think @thomheymann should weigh in too.
💛 Build succeeded, but was flaky
Test FailuresPlugin Functional Tests.x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration·js.task_manager scheduling and running tasks should return a task run error result when trying to run a task now which is already runningStandard Out
Stack Trace
Build metrics
History
To update your PR or re-run it, just comment with: |
not needed anymore #74640 (comment) |
Summary
Part of #35508
Integrates audit trail into new es client.
Checklist
For maintainers