Watcher: Store username on watch execution by hub-cap · Pull Request #31873 · elastic/elasticsearch

hub-cap · 2018-07-06T18:11:58Z

There is currently no way to see what user executed a watch. This commit
adds the decrypted username to each execution in the watch history, in a
new field executed_by.

Closes #31772

There is currently no way to see what user executed a watch. This commit adds the decrypted username to each execution in the watch history, in a new field executed_by. Closes elastic#31772

elasticmachine · 2018-07-06T18:12:00Z

Pinging @elastic/es-core-infra

hub-cap · 2018-07-06T18:12:13Z

One question here... should this also be backported to 6.3?

spinscale

left one major comment regarding naming and asked for a unit test, but almost there. In general I think this can go into 6.4 and master branches

spinscale · 2018-07-09T08:27:21Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/history/WatchRecord.java

    private static final ParseField METADATA = new ParseField("metadata");
    private static final ParseField EXECUTION_RESULT = new ParseField("result");
    private static final ParseField EXCEPTION = new ParseField("exception");
+    private static final ParseField EXECUTED_BY = new ParseField("executed_by");


how about simply naming this user?

one simple change to affect the whole PR ;)

spinscale · 2018-07-09T08:37:05Z

...core/src/main/java/org/elasticsearch/xpack/core/watcher/execution/WatchExecutionContext.java

    public void ensureWatchExists(CheckedSupplier<Watch, Exception> supplier) throws Exception {
        if (watch == null) {
            watch = supplier.get();
+            // now that the watch exists, extract out the authentication


this can be refactored into a small static method, so you could write also some tests (so many ifs/null checks to check). This could also be done lazily in the getter (not sure yet if that is a good idea though).

This method could also be overwritten by sub classes (both happen to call super.ensureExists() though) - might be a good reason to load it lazily

There looks to be no need to override this method, so ill make it final (and still move that bit to a static helper for testing)

edit: disregard.

after some snooping I noticed we can and should do this #31926

spinscale · 2018-07-09T08:41:30Z

...t/resources/rest-api-spec/test/watcher/watcher_and_security/20_test_run_as_execute_watch.yml

+        id: "my_watch"
+  - match: { watch_record.watch_id: "my_watch" }
+  - match: { watch_record.state: "executed" }
+  - match: { watch_record.executed_by: "x_pack_rest_user" }


hub-cap · 2018-07-10T18:19:13Z

Addressed comments and created a test for the Watch Execution Context

spinscale

the doc tests are failing due to the newly added field, and need to be fixed first. codewise I think we are good.

spinscale · 2018-07-11T08:18:20Z

...n/watcher/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java

+        assertNull(context.getUser());
+
+        Map<String, String> headers = new HashMap<>();
+        headers.put(AuthenticationField.AUTHENTICATION_KEY, encoded);


you can use Collections.singletonMap() here for convenience. Can you move the Authentication code down here, where the map gets created?

spinscale · 2018-07-11T08:21:16Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/history/WatchRecord.java

        if (executionResult != null) {
            builder.field(EXECUTION_RESULT.getPreferredName(), executionResult, params);
        }
+        if (user != null) {


can you maybe move this up to where the node id gets written out? This way, one can see both at once when reading a watch history, otherwise one is at the top and the other at the bottom.

Missed, will do this in another PR.

oh nm, i can do it on this one! I thought i had merged already woop woop

@spinscale can you eyeball 0dc862f just so im sure i did what u asked?

spinscale · 2018-07-16T07:01:12Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/history/WatchRecord.java

        builder.field(NODE.getPreferredName(), nodeId);
        builder.field(STATE.getPreferredName(), state.id());

+        if (user != null) {


* es/master: (21 commits) Tweaked Elasticsearch Service links for SEO Watcher: Store username on watch execution (#31873) Use correct formatting for links (#29460) Painless: Separate PainlessLookup into PainlessLookup and PainlessLookupBuilder (#32054) Scripting: Remove dead code from painless module (#32064) [Rollup] Replace RollupIT with a ESRestTestCase version (#31977) [TEST] Consistent algorithm usage (#32077) [Rollup] Fix duplicate field names in test (#32075) Ensure only parent breaker trips in unit test Unmute field collapsing rest tests Fix BWC check after backport [Tests] Fix failure due to changes exception message (#32036) Remove unused params from SSource and Walker (#31935) [Test] Mute MlJobIT#testDeleteJobAfterMissingAliases Turn off real-mem breaker in REST tests Turn off real-mem breaker in single node tests Fix broken OpenLDAP Vagrant QA test Cleanup Duplication in `PainlessScriptEngine` (#31991) SCRIPTING: Remove unused MultiSearchTemplateRequestBuilder (#32049) Fix compile issues introduced by merge (#32058) ...

There is currently no way to see what user executed a watch. This commit adds the decrypted username to each execution in the watch history, in a new field "user". Closes #31772

* 6.x: Security: revert to old way of merging automata (#32254) Fix a test bug in RangeQueryBuilderTests introduced in the field aliases backport. Introduce Application Privileges with support for Kibana RBAC (#32309) Undo a debugging change that snuck in during the field aliases merge. [test] port linux package packaging tests (#31943) Painless: Update More Methods to New Naming Scheme (#32305) Tribe: Add error with secure settings copied to tribe (#32298) Add V_6_3_3 version constant Add ERR to ranking evaluation documentation (#32314) [DOCS] Added link to 6.3.2 RNs [DOCS] Updates 6.3.2 release notes with PRs from ml-cpp repo (#32334) [Kerberos] Add Kerberos authentication support (#32263) [ML] Extract persistent task methods from MlMetadata (#32319) Backport - Add Snapshots Status API to High Level Rest Client (#32295) Make release notes ignore the `>test-failure` label. (#31309) [DOCS] Adds release highlights for search for 6.4 (#32095) Allow Integ Tests to run in a FIPS-140 JVM (#32316) Add support for field aliases to 6.x. (#32184) Register ERR metric with NamedXContentRegistry (#32320) fixes broken build for third-party-tests (#32315) Relates #31918 / Closes infra/issues/6085 [DOCS] Rollup Caps API incorrectly mentions GET Jobs API (#32280) Rest HL client: Add put watch action (#32026) (#32191) Add WeightedAvg metric aggregation (#31037) Consistent encoder names (#29492) Switch monitoring to new style Requests (#32255) specify subdirs of lib, bin, modules in package (#32253) Rename ranking evaluation `quality_level` to `metric_score` (#32168) Add new permission for JDK11 to load JAAS libraries (#32132) Switch x-pack:core to new style Requests (#32252) Watcher: Store username on watch execution (#31873) Silence SSL reload test that fails on JDK 11 Painless: Clean up add methods in PainlessLookup (#32258) CCE when re-throwing "shard not available" exception in TransportShardMultiGetAction (#32185) Fail shard if IndexShard#storeStats runs into an IOException (#32241) Fix `range` queries on `_type` field for singe type indices (#31756) (#32161) AwaitsFix RecoveryIT#testHistoryUUIDIsGenerated Add new fields to monitoring template for Beats state (#32085) (#32273) [TEST] improve REST high-level client naming conventions check (#32244) Check that client methods match API defined in the REST spec (#31825)

The tests we referring to an older watch history template, that was updated as part of b982e1a (initial PR was elastic#31873) Closes elastic#32307

The tests refer to an old watch history template, that was updated as part of b982e1a (initial PR was #31873) Closes #32307

as per elastic/elasticsearch#31873

as per elastic/elasticsearch#31873 (cherry picked from commit 7f99a91)

Watcher: Store username on watch execution

a5874e0

There is currently no way to see what user executed a watch. This commit adds the decrypted username to each execution in the watch history, in a new field executed_by. Closes elastic#31772

hub-cap added >enhancement v7.0.0 :Distributed/Watcher v6.4.0 labels Jul 6, 2018

hub-cap requested a review from spinscale July 6, 2018 18:11

cjcenizal mentioned this pull request Jul 6, 2018

Display watch execution username elastic/kibana#20530

Closed

spinscale requested changes Jul 9, 2018

View reviewed changes

hub-cap added 4 commits July 9, 2018 11:51

Update executed_by to user

45ad8b7

Merge remote-tracking branch 'upstream/master' into store_username

560111e

Move method and create test for the context

db6d207

Merge remote-tracking branch 'upstream/master' into store_username

0edc33e

spinscale approved these changes Jul 11, 2018

View reviewed changes

hub-cap added 7 commits July 11, 2018 10:05

Merge remote-tracking branch 'upstream/master' into store_username

c2c49d8

Cleanup test

dcd734d

Fix docs test

5a467d0

Cleanup wording

a1ea27d

Merge remote-tracking branch 'upstream/master' into store_username

b49b2b5

Merge remote-tracking branch 'upstream/master' into store_username

d54bcbe

Move username

0dc862f

spinscale approved these changes Jul 16, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into store_username

cf3d82e

hub-cap merged commit 637cac9 into elastic:master Jul 16, 2018

hub-cap added the backport pending label Jul 16, 2018

hub-cap removed the backport pending label Jul 23, 2018

spinscale added a commit to spinscale/elasticsearch that referenced this pull request Jul 25, 2018

Tests: Fix XPack upgrade tests

472e732

The tests we referring to an older watch history template, that was updated as part of b982e1a (initial PR was elastic#31873) Closes elastic#32307

spinscale mentioned this pull request Jul 25, 2018

Tests: Fix XPack upgrade tests #32352

Merged

spinscale added a commit that referenced this pull request Jul 25, 2018

Tests: Fix XPack upgrade tests (#32352)

41b12e2

The tests refer to an old watch history template, that was updated as part of b982e1a (initial PR was #31873) Closes #32307

Mpdreamz mentioned this pull request Sep 25, 2018

[meta] 6.4.0 release elastic/elasticsearch-net#3397

Closed

89 tasks

Mpdreamz added a commit to elastic/elasticsearch-net that referenced this pull request Oct 1, 2018

return user on watch record as per elastic/elasticsearch#31873

46c2549

Mpdreamz mentioned this pull request Oct 1, 2018

return user on watch record elastic/elasticsearch-net#3445

Merged

russcam pushed a commit to elastic/elasticsearch-net that referenced this pull request Oct 17, 2018

return user on watch record (#3445)

7f99a91

as per elastic/elasticsearch#31873

russcam pushed a commit to elastic/elasticsearch-net that referenced this pull request Oct 26, 2018

return user on watch record (#3445)

37cd46a

as per elastic/elasticsearch#31873 (cherry picked from commit 7f99a91)

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Conversation

hub-cap commented Jul 6, 2018

Uh oh!

elasticmachine commented Jul 6, 2018

Uh oh!

hub-cap commented Jul 6, 2018

Uh oh!

spinscale left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hub-cap Jul 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hub-cap commented Jul 10, 2018

Uh oh!

spinscale left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hub-cap Jul 9, 2018 •

edited

Loading