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.

Sign up for GitHub

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

Jump to bottom

Fix audit logger logging to console even when disabled #80928

Merged
merged 1 commit into from
Oct 16, 2020
Merged

Fix audit logger logging to console even when disabled #80928

merged 1 commit into from
Oct 16, 2020

Conversation

thomheymann
Copy link
Contributor

@thomheymann thomheymann commented Oct 16, 2020 •
edited by jportner
Loading

Fixes bug in audit logger that logged to console even when disabled

Follow-on to #74640

@thomheymann thomheymann requested a review from a team as a code owner October 16, 2020 21:46
@jportner jportner added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0 labels Oct 16, 2020
jportner
jportner approved these changes Oct 16, 2020
View reviewed changes
Copy link
Contributor

@jportner jportner left a comment

Choose a reason for hiding this comment

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

Tested locally with several different license/config combinations. LGTM on green CI!

As discussed offline, it might not be a bad idea to add an integration test for the negative case -- can revisit that in a follow-on.

Separately, I'm wondering if the more appropriate permanent fix to this problem is to change how we're actually sending events to the logger. Maybe we shouldn't send events to the logger at all unless audit logging is enabled. But again, we can revisit that in a follow-on too.

@legrego
Copy link
Member

legrego commented Oct 16, 2020

Tested locally with several different license/config combinations. LGTM on green CI!

As discussed offline, it might not be a bad idea to add an integration test for the negative case -- can revisit that in a follow-on.

Separately, I'm wondering if the more appropriate permanent fix to this problem is to change how we're actually sending events to the logger. Maybe we shouldn't send events to the logger at all unless audit logging is enabled. But again, we can revisit that in a follow-on too.

++ agree with all of the above. Let's investigate next week to see what we can do

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@jportner jportner merged commit 72fa61b into master Oct 16, 2020
@jportner jportner deleted the security/audit-hotfix branch October 16, 2020 23:49
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 19, 2020
@gmmorris
Merge branch 'master' into task-manager/refactor-oversized-class
c30d71b 
* master: (43 commits)
  [ML] Transforms: Fix tab ids for expanded row. (elastic#80666)
  server logs config paths to use for runner (elastic#52980)
  Fix audit logger logging to console even when disabled (elastic#80928)
  skip flaky suite (elastic#80929)
  Added Enterprise Search config to kibana-docker (elastic#80872)
  skip flaky suite (elastic#80914)
  [keystore_cli] parse values as JSON before adding to keystore (elastic#80848)
  [Ingest Manager] Fix for comparing versions with -SNAPSHOT suffix (elastic#80742)
  ECS audit logging (elastic#74640)
  [Uptime] Add client-side unit tests for remaining synthetics code (elastic#80215)
  [Security_Solution][Resolver] Promote z-index on node labels (elastic#80854)
  Move renderHeaderActions back into mount useEffect + update tests (elastic#80861)
  [Reporting] Document Network Policy configuration (elastic#80431)
  [Reporting] Add contextual documentation for CSV Max Bytes setting (elastic#80782)
  Add catch for Enterprise Search sending back a 401 response instead of redirect (elastic#80757)
  [Actions] Back Button on Add Connector Flyout (elastic#80160)
  removing `kibana_datatable` in favor of `datatable`  (elastic#80548)
  [Alerting UI] Updating 'Add new' wording (elastic#80509)
  [Docs] Document Encrypted Saved Objects functionality. (elastic#80183)
  [Discover] fix auto-refresh (elastic#80635)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 19, 2020
@gmmorris
Merge branch 'master' into task-manager/health
12e52e6 
* master: (23 commits)
  [ML] Transforms: Fix tab ids for expanded row. (elastic#80666)
  server logs config paths to use for runner (elastic#52980)
  Fix audit logger logging to console even when disabled (elastic#80928)
  skip flaky suite (elastic#80929)
  Added Enterprise Search config to kibana-docker (elastic#80872)
  skip flaky suite (elastic#80914)
  [keystore_cli] parse values as JSON before adding to keystore (elastic#80848)
  [Ingest Manager] Fix for comparing versions with -SNAPSHOT suffix (elastic#80742)
  ECS audit logging (elastic#74640)
  [Uptime] Add client-side unit tests for remaining synthetics code (elastic#80215)
  [Security_Solution][Resolver] Promote z-index on node labels (elastic#80854)
  Move renderHeaderActions back into mount useEffect + update tests (elastic#80861)
  [Reporting] Document Network Policy configuration (elastic#80431)
  [Reporting] Add contextual documentation for CSV Max Bytes setting (elastic#80782)
  Add catch for Enterprise Search sending back a 401 response instead of redirect (elastic#80757)
  [Actions] Back Button on Add Connector Flyout (elastic#80160)
  removing `kibana_datatable` in favor of `datatable`  (elastic#80548)
  [Alerting UI] Updating 'Add new' wording (elastic#80509)
  [Docs] Document Encrypted Saved Objects functionality. (elastic#80183)
  [Discover] fix auto-refresh (elastic#80635)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 19, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 80928 or prevent reminders by adding the backport:skip label.

@thomheymann thomheymann added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. backport pending labels Oct 20, 2020
@thomheymann
Copy link
Contributor Author

Backport for this PR has been manually included in: #80921

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jportner jportner jportner approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thomheymann @legrego @kibanamachine @jportner