[Azure] Add Identity Protection and Provisioning to Azure AD logs #4149

zmoog · 2022-09-07T08:07:24Z

What does this PR do?

Add two new data streams to the Azure AD logs integration:

Identity Protection logs
Provisioning logs

The new log categories supported by the data streams were known as tenant logs.

Background

The Azure AD Identity Protection service produces two log categories:

RiskyUsers contains the risk status for an AD user.
UserRiskEvents describes the detected events that change a user's risk status.

The Azure AD Provisioning service produces one log category:

ProvisioningLogs — describes the sync operation performed by the Provisioning service to sync Azure AD with an external enterprise application.

Notes for the reviewer

Here are areas I'd love to have some experienced eyes on:

Fields: double-check the field names and type to see if my choices are sound or if there's room for improvements (or issues to fix!)
Naming: the two new data stream names are not short; should we keep them shorter? Any name suggestions?

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.
I have added an entry to my package's changelog.yml file.
I have verified that Kibana version constraints are current according to guidelines.

How to test this PR locally

Related issues

Closes Support for Tenant Logs in Azure integration #3460

Screenshots

Settings

Dashboards

elasticmachine · 2022-09-07T08:22:13Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-09-24T10:06:05.701+0000
Duration: 14 min 50 sec

Test stats 🧪

Test	Results
Failed	0
Passed	119
Skipped	0
Total	119

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

elasticmachine · 2022-09-07T08:22:24Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`9/9`)	💚
Files	85.0% (`17/20`)	👎 -12.127
Classes	85.0% (`17/20`)	👎 -12.127
Methods	82.143% (`138/168`)	👎 -7.23
Lines	85.578% (`2504/2926`)	👎 -5.679
Conditionals	100.0% (`0/0`)	💚

zmoog · 2022-09-12T09:53:32Z

@ravikesarwani, I think having an OOTB dashboard is crucial to use these two new data streams. So, I want to provide basic dashboards for Identity Protection and Provisioning logs.

I plan to update the overview Azure dashboard as soon as we learn more about how users will leverage this data for their use cases.

In the meantime, here are the dashboards with a few comments. Let me know what you think!

zmoog · 2022-09-12T10:16:14Z

Oh, I just noticed I have to make dashboards compatible with 7.16+. Thanks, CI!

zmoog · 2022-09-13T08:27:04Z

Hey @ravikesarwani, I made minor changes to the dashboard while making them compatible with Kibana 7.16+ (I accidentally created them on 8.3.3).

You can find the latest version in the description.

packages/azure/data_stream/identityprotectionlogs/elasticsearch/ingest_pipeline/default.yml

Add tables with the log categories supported by the new data streams and other minor updates.

When hours are in the 1-9 range (for example, 8:35), the timestamps are not strictly ISO8601 compliant. For example, here's a timestamp value sent for the the `riskLastUpdatedDateTime` field: "2022-09-09T9:59:27.958Z" This value is missing leading "0" in the hours part, breaking the pipeline's date-processor. It should have been: "2022-09-09T09:59:27.958Z"

The Identity Protection dashboard shows: - a list of users last risk status - a list of risky sign-ins - a timeline of risky sign-ins detections - pies for detection sources and risk types The Provisioning dashboard shows: - list of provisioning actions - a timeline for provisioning actions - pies for source and target systems

The current minimum Kibana version supported by the Azure integration is 7.16. I don't want to increase this minimum if not necessary. I replaced the Lens-based tables with a Metric visualization because, in 7.16+, the "last value" function is not available for date fields. I used the last value function on date fields to visualize the latest user risk status for Identity Protection logs and the latest sync operation for Provisioning logs.

Remove commented out processors

elasticmachine · 2022-09-21T18:04:18Z

🚀 Benchmarks report

Package `azure` 👍(3) 💚(0) 💔(6)

Expand to view

Data stream	Previous EPS	New EPS	Diff (%)	Result
`activitylogs`	1457.73	554.02	-903.71 (-61.99%)	💔
`auditlogs`	2358.49	1200.48	-1158.01 (-49.1%)	💔
`firewall_logs`	1420.45	1096.49	-323.96 (-22.81%)	💔
`platformlogs`	4016.06	1579.78	-2436.28 (-60.66%)	💔
`signinlogs`	1592.36	968.05	-624.31 (-39.21%)	💔
`springcloudlogs`	3322.26	1824.82	-1497.44 (-45.07%)	💔

To see the full report comment with /test benchmark fullreport

zmoog · 2022-09-22T15:59:03Z

Let's add release: beta in the new datastream manifest.yml

Yep, it is the best option! Updated. Thanks, @kaiyan-sheng!

We can annotate the data stream as a beta in the manifest.yml file, but currently, the integration UI shows no indication of the release type. Besides setting the metadata, this sort of "badge" should give users a a hint about the release type.

zmoog · 2022-09-22T20:58:40Z

@ravikesarwani, we can annotate the data stream as a beta in the manifest.yml file, but currently, the integration UI shows no indication of the release type.

Besides setting the metadata, this sort of "badge" should give users a hint about the release type:

WDYT?

Apply the recommended naming contentions on data stream names.

tommyers-elastic · 2022-09-23T10:54:56Z

@ravikesarwani @zmoog per-data stream release badges have been implemented here for 8.5.0 so we should not have to manually add it

tommyers-elastic · 2022-09-23T11:43:46Z

packages/azure/changelog.yml

@@ -1,3 +1,8 @@
+- version: "1.4.0"


question not really related to the PR: i wonder under what circumstances we bump the major version of an integration?

Yeah, I was wondering the same. IIRC the integration docs don't have a mandating policy for version change (please, correct me if I'm wrong!).

I usually apply a semver-ish approach, bumping the minor version on new features and patch version on fixes.

If there is no such information anywhere, we can add something to make current practices explicit.

packages/azure/data_stream/identity_protection_logs/elasticsearch/ingest_pipeline/default.yml

tommyers-elastic · 2022-09-23T12:30:42Z

packages/azure/data_stream/identity_protection_logs/elasticsearch/ingest_pipeline/default.yml

+     ignore_missing: true
+  - rename:
+     field: azure.identityprotection.properties.userDisplayName
+     target_field: azure.identityprotection.properties.user_display_name


do we have any considerations to make around PII like this coming into elastic cloud?

Oh, this is a good point. I believe we have similar data in the sign-in logs.

I don't know our stance on PII; @ravikesarwani maybe you know more?

@tommyers-elastic Can you specify your specific concerns on the PII here. As far as I understand the customer here is in full control (1) The App decides what to log in the log file, we don't (2) They have full control in terms of what logs to send to Elastic (3) They can delete data from Elastic any point in time they want

no direct concerns from me - just wanted to have this discussion to make sure we have considered it. for example GDPR is strict on transferring personal data outside of the EU, and even though it's in control of the customer, if they are on cloud and have a cluster in the US, we are the ones storing the data. obviously i'm not a lawyer so i don't know if this is even relevant here. but i think it's worth briefly talking about.

tommyers-elastic · 2022-09-23T12:32:50Z

packages/azure/data_stream/identity_protection_logs/fields/agent.yml

@@ -0,0 +1,62 @@
+- name: cloud.account.id


i've never fully understood the convention about where fields should go. i figured all the ecs fields live in ecs.yml, but i could easily be wrong?

Uhm, I started using an existing integration as a template; let me double-check if there's something I should move in the ecs.yml file.

packages/azure/data_stream/identity_protection_logs/fields/fields.yml

packages/azure/data_stream/identity_protection_logs/manifest.yml

tommyers-elastic · 2022-09-23T12:53:01Z

.../azure/data_stream/provisioning_logs/elasticsearch/ingest_pipeline/azure-shared-pipeline.yml

@@ -0,0 +1,26 @@
+---


is this identical to the shared pipeline in the other datastream? is there anyway to move it so it can be shared between the two?

Yeah, it's identical: this pattern repeats in all Azure Logs integrations. I researched a little about how to share pipelines, but I failed.

I added the item "research how to share pipelines between integrations" to my list, probably starting from an email thread.

packages/azure/data_stream/provisioning_logs/elasticsearch/ingest_pipeline/default.yml

ravikesarwani · 2022-09-23T12:56:41Z

@ravikesarwani @zmoog per-data stream release badges have been implemented elastic/kibana#136841 for 8.5.0 so we should not have to manually add it

Agreed. @zmoog its a clever idea but let's not put any one offs (that can create inconsistency in user experience and hard to maintain in longer run).

packages/azure/data_stream/identity_protection_logs/elasticsearch/ingest_pipeline/default.yml

packages/azure/data_stream/provisioning_logs/elasticsearch/ingest_pipeline/default.yml

tommyers-elastic

no blockers from me - great work @zmoog

zmoog · 2022-09-23T13:11:06Z

Agreed. @zmoog its a clever idea but let's not put any one offs (that can create inconsistency in user experience and hard to maintain in longer run).

This is great, I missed the beta badge coming to 8.5.0, thanks @tommyers-elastic!

I will revert the change.

This reverts commit c809577.

Both RiskyUsers and UserRiskEvents have a similar "last updated" field I was collapsing into one. After a brief review, we decided the "path of least confusion" was to keep the two fields independent with a distinct name, description and semantics.

The recommended name convention is to not include 'logs' in the data stream name because it is redundant. Here are the final complete data stream names: - `logs-azure.identity_protection-default` - `logs-azure.provisioning-default`

The `risk_event_type` field contains more detailed information compared to the `risk_type` field. It is a more interesting data point to show in the dashboard.

The `risk_event_type` field is more interesting to display on the timeline than the activity (type).

zmoog changed the title ~~[Azure Logs] Add tenant logs~~ [Azure Logs] Add data streams for Identity Protection and Provisioning (tenant logs) to Azure AD logs Sep 9, 2022

zmoog self-assigned this Sep 9, 2022

zmoog added enhancement New feature or request Team:Cloud-Monitoring Label for the Cloud Monitoring team labels Sep 9, 2022

zmoog marked this pull request as ready for review September 9, 2022 10:29

zmoog requested a review from a team as a code owner September 9, 2022 10:29

zmoog changed the title ~~[Azure Logs] Add data streams for Identity Protection and Provisioning (tenant logs) to Azure AD logs~~ [Azure Logs] Add Identity Protection and Provisioning (aka tenant logs) to Azure AD logs Sep 12, 2022

girodav reviewed Sep 19, 2022

View reviewed changes

packages/azure/data_stream/identityprotectionlogs/elasticsearch/ingest_pipeline/default.yml Outdated Show resolved Hide resolved

girodav approved these changes Sep 20, 2022

View reviewed changes

zmoog mentioned this pull request Sep 21, 2022

Support for Tenant Logs in Azure integration #3460

Closed

zmoog added 13 commits September 21, 2022 19:50

Add adidentityprotectionlogs and provisioninglogs

f9a61d3

Move the new data streams into adlogs

ec20871

Update package version

2b26e67

Update docs

d6487c2

Add tables with the log categories supported by the new data streams and other minor updates.

Update rendered docs

19edfb2

Clean up

14f51dc

Update changelog

fc51c8f

Add descriptions to integration settings.

0b3c44d

Add descriptions to dashboards and saved searches

9bb9f5e

Clean up

bf70629

Remove commented out processors

zmoog force-pushed the azure-tenant-logs branch from 49e53fa to bf70629 Compare September 21, 2022 17:50

Release the new data streams as beta

001c6d7

zmoog changed the title ~~[Azure Logs] Add Identity Protection and Provisioning (aka tenant logs) to Azure AD logs~~ [Azure] Add Identity Protection and Provisioning to Azure AD logs Sep 22, 2022

Add "beta badge" to new data streams

c809577

We can annotate the data stream as a beta in the manifest.yml file, but currently, the integration UI shows no indication of the release type. Besides setting the metadata, this sort of "badge" should give users a a hint about the release type.

zmoog added 2 commits September 23, 2022 00:09

Rename data stream to identity_protection_logs

a8e65e9

Apply the recommended naming contentions on data stream names.

Rename data stream to provisioning_logs

2106679

Apply the recommended naming contentions on data stream names.