Introduce telemetry for security features #74530

legrego · 2020-08-06T13:52:59Z

Summary

Adds telemetry for Kibana's stack security features. Specifically, this introduces the following fields under a new security section:

      // Denotes if audit logging is available under the current license, and enabled by the user
      auditLoggingEnabled: {
        type: 'boolean',
      },
      // Denotes if the login selector is enabled
      loginSelectorEnabled: {
        type: 'boolean',
      },
      // Denotes if the access agreement is available under the current license, and enabled by the user for at least one auth provider.
      accessAgreementEnabled: {
        type: 'boolean',
      },
      // The number of enabled auth providers
      authProviderCount: {
        type: 'number',
      },
      // A de-duplicated array of enabled auth provider types. ex: ['basic', 'saml']
      enabledAuthProviders: {
        type: 'keyword',
      },
      // The configured http auth schemes (via `xpack.security.authc.http.schemes')
      httpAuthSchemes: {
        type: 'keyword',
      },

Resolves #50815
Resolves #67747

legrego · 2020-08-06T18:29:01Z

@elastic/kibana-telemetry this PR adds new telemetry fields, but I never ran the script to update the JSON mappings. Should the Schema telemetry check have failed this build?

Edit: I added the mappings. The original commit built via https://kibana-ci.elastic.co/job/elastic+kibana+pipeline-pull-request/66908/ passed without updating the mappings

legrego · 2020-08-06T20:05:20Z

src/plugins/telemetry/schema/oss_plugins.json

@@ -13,6 +13,19 @@
        }
      }
    },
+    "search": {


Not changed as part of this PR, but the script to update the telemetry schema picked this up as something that was already missing.

@Bamieh do you know why this could have happened?

@Bamieh poke 😄

sorry for being dormant. I think it's already been merged to master and we've reviewed to the telemetry_check script to make sure it fails if the JSON file is not up-to-date (if you rebase, it should be all good)

elasticmachine · 2020-08-27T11:04:49Z

Pinging @elastic/kibana-security (Team:Security)

afharo

LGTM! Just added one tiny NIT.

x-pack/plugins/security/server/plugin.ts

…c-telemetry

jportner

Some nits/questions in the comments below. In addition, some higher-level questions on our approach:

Will we ever want to ask the question "how many of each auth provider type is enabled?" Currently, the schema doesn't tell us for instance that 2 saml providers and 1 basic provider is enabled. It will just tell us that saml and basic are each enabled, with 3 providers total.
Will we ever want to ask the question "how many of our users who can enable audit logging have it enabled?" Right now the implementation will just tell us that they don't have it enabled if ES Security is disabled and/or if the license level is < Gold. (should we also be tracking allowRbac, allowAccessAgreement, and allowAuditLogging?)

x-pack/plugins/security/server/usage_collector/security_usage_collector.test.ts

x-pack/plugins/security/server/config.ts

legrego · 2020-09-03T15:36:33Z

In addition, some higher-level questions on our approach:

These are exactly the types of questions I was hoping to have a discussion on before getting this ready for review 😄

Will we ever want to ask the question "how many of each auth provider type is enabled?" Currently, the schema doesn't tell us for instance that 2 saml providers and 1 basic provider is enabled. It will just tell us that saml and basic are each enabled, with 3 providers total.

I did consider this, but didn't see a huge benefit to collecting this information. Happy to revisit this decision if we think it's worthwhile though.

Will we ever want to ask the question "how many of our users who can enable audit logging have it enabled?" Right now the implementation will just tell us that they don't have it enabled if ES Security is disabled and/or if the license level is < Gold.

Elasticsearch exposes available and enabled flags for their features to capture this. I considered this approach, but I was trying to be a good telemetry citizen by being conservative with the number of fields I was adding to the mapping. We can derive this information by also aggregating on the cluster's license level, and the information about security that Elasticsearch is already reporting. We could add our own fields here to make it a little easier to consume, but it would mostly be duplicative information that we can read from other fields.

jportner · 2020-09-03T15:44:44Z

I did consider this, but didn't see a huge benefit to collecting this information. Happy to revisit this decision if we think it's worthwhile though.

OK! I only asked because it would probably involve a schema change, not just an addition. I don't feel strongly either way.

We can derive this information by also aggregating on the cluster's license level, and the information about security that Elasticsearch is already reporting.

I'm not super familiar with the telemetry, kinda figured this might be the case but wanted to ask to be sure. Good enough for me!

…c-telemetry

…collector.test.ts Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>

kibanamachine · 2020-09-10T03:01:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 77fe6f4

Build metrics

page load bundle size

id	value	diff	baseline
security	305.4KB	+270.0B	305.2KB

distributable file count

id	value	diff	baseline
default	45479	+2	45477

History

💚 Build #73495 succeeded d97f19d
💚 Build #72004 succeeded 48db12b
💚 Build #70243 succeeded 8890944
💚 Build #67033 succeeded 9fd1ddca65958697ee643077963f64d85bacd0c9
💚 Build #66908 succeeded fcc81def3f02cc515d8a4a50715366f045d62457

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

legrego · 2020-09-10T10:59:11Z

@jportner this is ready for the next review round

jportner

Code review only of changes since my last review--
LGTM!

Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com> Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>

mindbat · 2020-09-15T22:11:53Z

@afharo Perhaps a silly question, but I'll ask it to be certain: Will this require updates to the Telemetry cluster mappings?

afharo · 2020-09-16T16:33:47Z

@mindbat yes, it does. It'll report the data in the same format as specified in x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json (check out the new fields added in this PR)

mindbat · 2020-09-16T16:46:00Z

@afharo Gotcha, thanks for clarifying!

@legrego In order for these fields to be visible in the Telemetry cluster, we'll need to update the mappings under which this data gets indexed once its received. Would you mind opening an issue in the infra repo for making those updates?

legrego · 2020-09-16T16:48:05Z

@legrego In order for these fields to be visible in the Telemetry cluster, we'll need to update the mappings under which this data gets indexed once its received. Would you mind opening an issue in the infra repo for making those updates?

@mindbat will do. I was under the impression that the schema in x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json automagically took care of that -- thanks for letting me know!

edit: done in https://github.com/elastic/infra/issues/23346

elasticmachine · 2021-10-01T21:38:30Z

Pinging @elastic/kibana-core (Team:Core)

legrego added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Team:KibanaTelemetry labels Aug 6, 2020

legrego commented Aug 6, 2020

View reviewed changes

legrego added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 discuss labels Aug 6, 2020

legrego added 2 commits August 25, 2020 07:04

Introduce telemetry for security features

ede1101

update telemetry schemas

e078af5

legrego force-pushed the security/authc-telemetry branch from 9fd1ddc to e078af5 Compare August 25, 2020 11:14

revert unrelated changes to oss telemetry schema

8890944

legrego removed the discuss label Aug 27, 2020

legrego marked this pull request as ready for review August 27, 2020 11:04

legrego requested review from a team as code owners August 27, 2020 11:04

legrego requested review from azasypkin and arisonl August 27, 2020 11:05

afharo approved these changes Sep 1, 2020

View reviewed changes

x-pack/plugins/security/server/plugin.ts Outdated Show resolved Hide resolved

legrego removed the request for review from azasypkin September 2, 2020 18:38

legrego added 2 commits September 2, 2020 14:42

Address telemetry team review

2bc8d69

Merge branch 'master' of github.com:elastic/kibana into security/auth…

48db12b

…c-telemetry

legrego requested a review from jportner September 3, 2020 00:00

jportner reviewed Sep 3, 2020

View reviewed changes

legrego added 3 commits September 9, 2020 13:03

start addressing feedback

ce340ce

Merge branch 'master' of github.com:elastic/kibana into security/auth…

f0fa4e9

…c-telemetry

Merge branch 'master' of github.com:elastic/kibana into security/auth…

d97f19d

…c-telemetry

Update x-pack/plugins/security/server/usage_collector/security_usage_…

77fe6f4

…collector.test.ts Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>

legrego requested a review from jportner September 10, 2020 10:58

jportner approved these changes Sep 10, 2020

View reviewed changes

legrego merged commit 52fba21 into elastic:master Sep 10, 2020

legrego mentioned this pull request Sep 10, 2020

[7.x] Introduce telemetry for security features (#74530) #77199

Merged

legrego added a commit that referenced this pull request Sep 10, 2020

Introduce telemetry for security features (#74530) (#77199)

d767b03

Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com> Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>

legrego deleted the security/authc-telemetry branch September 16, 2020 16:42

lukeelmers added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Oct 1, 2021

lukeelmers added Feature:Telemetry and removed Team:KibanaTelemetry labels Oct 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce telemetry for security features #74530

Introduce telemetry for security features #74530

legrego commented Aug 6, 2020 •

edited

Loading

legrego commented Aug 6, 2020 •

edited

Loading

legrego Aug 6, 2020

afharo Aug 10, 2020

legrego Aug 24, 2020

afharo Aug 25, 2020

elasticmachine commented Aug 27, 2020

afharo left a comment

jportner left a comment

legrego commented Sep 3, 2020

jportner commented Sep 3, 2020

kibanamachine commented Sep 10, 2020

legrego commented Sep 10, 2020

jportner left a comment

mindbat commented Sep 15, 2020

afharo commented Sep 16, 2020

mindbat commented Sep 16, 2020

legrego commented Sep 16, 2020 •

edited

Loading

elasticmachine commented Oct 1, 2021

Introduce telemetry for security features #74530

Introduce telemetry for security features #74530

Conversation

legrego commented Aug 6, 2020 • edited Loading

Summary

legrego commented Aug 6, 2020 • edited Loading

legrego Aug 6, 2020

Choose a reason for hiding this comment

afharo Aug 10, 2020

Choose a reason for hiding this comment

legrego Aug 24, 2020

Choose a reason for hiding this comment

afharo Aug 25, 2020

Choose a reason for hiding this comment

elasticmachine commented Aug 27, 2020

afharo left a comment

Choose a reason for hiding this comment

jportner left a comment

Choose a reason for hiding this comment

legrego commented Sep 3, 2020

jportner commented Sep 3, 2020

kibanamachine commented Sep 10, 2020

💚 Build Succeeded

Build metrics

page load bundle size

distributable file count

History

legrego commented Sep 10, 2020

jportner left a comment

Choose a reason for hiding this comment

mindbat commented Sep 15, 2020

afharo commented Sep 16, 2020

mindbat commented Sep 16, 2020

legrego commented Sep 16, 2020 • edited Loading

elasticmachine commented Oct 1, 2021

legrego commented Aug 6, 2020 •

edited

Loading

legrego commented Aug 6, 2020 •

edited

Loading

legrego commented Sep 16, 2020 •

edited

Loading