[Cloud Security] Azure integration manual fields

[JordanSh]

Summary

Resolves #165579

Also added all other fields in cooredination with @moukoublen

1. Managed Identity
2. Client Secret (tenant_id, client_id , client_secret)
3. Client Certificate (tenant_id, client_id , client_certificate_path , client_certificate_password)
4. Client Username and Password (tenant_id, client_id ,client_username, client_password)

Screen.Recording.2023-11-12.at.11.20.13.mov

[elasticmachine]

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

[maxcold]

Small UX feedback after looking at the video - The Managed Identity and Manual options would confuse me as a user, as it looks like smth is broken (form not showing up), especially when coming from other manual options. As there are no params to fill, and users just need to read the docs, should we still add some text in addition to our standard Read the documentation... text? Something like For Managed Identity option there is no need to add any credentials here ... then Read the documentation for more details would be the natural addition to that

[maxcold]

code-wise looks good. Tested a bit and had one UX concern that I added as a separate comment. The build is red though

[kfirpeled]

Found couple of issues that need to be fixed

1. Missing migration from 1.6.5 to 1.7.0 (must be fixed)

When I've upgraded my installation from 1.6.5 to 1.7.0-preview03, and went to edit my integration. I expected to see the selected credentials type would be manual instead it was service_principal_with_client_secret

See @animehart example of migration: https://github.com/elastic/security-team/issues/7722

2. When choosing manual I miss the documentation reference that was there previously

The previous reference to the documentation: Ensure the agent is deployed ...

3. The following fields are required, however, I was able to save the integration without them

Client Id
Tenant Id
Client secret
Client certificate path
Client username
Client password

basically all the fields except Client certificate password

[kfirpeled]

checkout my previous comment

[JordanSh]

Screen.Recording.2023-11-23.at.15.13.44.mov

@kfirpeled thanks for the thorough review, I've added a detailed video with the entire upgrade process that is now working properly. The problem was a bad default value determinator that did not took into account the version of the policy, meaning the value of older integrations would get the default value of the newer ones.

managed identity will be the default selected value for new integrations

The following fields are required, however, I was able to save the integration without them

It doesn't look like we support required fields ATM, we manually set a policy to invalid in some specific cases like trying to save a policy in arm_template or cloud_formation but not having the template url link to the azure/aws pages. but i didnt encounter any infra for invalidating the policy over missing fields. because each field is only required when its option is selected, this change is not straight forward and i'll suggest to create a tech debt task to add support for that.

[JordanSh]

Small UX feedback after looking at the video - The Managed Identity and Manual options would confuse me as a user, as it looks like smth is broken (form not showing up), especially when coming from other manual options. As there are no params to fill, and users just need to read the docs, should we still add some text in addition to our standard Read the documentation... text? Something like For Managed Identity option there is no need to add any credentials here ... then Read the documentation for more details would be the natural addition to that

I agree but we kinda backed ourself into a corner with the previous integration version using manual as a valid input, in order to keep support in edit we need to provide a matching option in the drop down. the manual and managed identity options also share the same description which is confusing as well but i used what i had from figma. @tinnytintin10 is aware and there is an ongoing discussion on the topic in slack. for now, i would rather merge this imprefect solution and add fixes or changes later as needed.

The build is red though

MB, added a utility function which i later removed, but i left in its tests. cleaned that up as well.

[kfirpeled]

LGTM, lets add a task on our backlog to make the required fields required

[JordanSh]

@elasticmachine merge upstream

[JordanSh]

@elasticmachine merge upstream

[maxcold]

one nit comment on the latest additions, otherwise looks good!

[maxcold]

nit: I find it a bit confusing that when "package is valid for manual" we return managed identity otherwise manual. without the context, I would think it should be the other way around. Maybe change to isPackageVersionValidForManagedIdentity?

[JordanSh]

umm this is a very good point, thing is that "manual fields" are meant to reference the entire options (fields) under the Manual credential type, managed identity is just one of them and it happens to be the default value out of those options.

do you think something like isPackageVersionValidForManualOption is better? i dont mind sticking with your suggestions, just wanted to give more context and see what do you think

[maxcold]

yeah, I see where the confusion is coming from, that we have Manual as a term that includes multiple setup methods, and inside it we have another Manual as its own setup method, as in "really manual, you need to do everything yourself" kind of thing. changing to isPackageVersionValidForManualOption tbh wouldn't change much for me, in the end starting from 1.7.0 we started to support Manual -> Managed Identity so it became the default, and before it was Manual -> Manual the default one. So isPackageVersionValidForManagedIdentity still makes more sense to me (even though it's not only Managed Identity but some other options we started to support) but I'm also ok with a comment in this function to explain this Manual extravaganza, without changing the name :)

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 3acdeaa

Failed CI Steps

• Investigations - Security Solution Cypress Tests #5

Test Failures

• [job] [logs] Investigations - Security Solution Cypress Tests #5 / Save Timeline Prompts "before each" hook for "Changed & unsaved timeline should NOT prompt when user navigates away within security solution where timelines are enabled" "before each" hook for "Changed & unsaved timeline should NOT prompt when user navigates away within security solution where timelines are enabled"

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
cloudSecurityPosture	427.6KB	432.1KB	+4.6KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
cloudSecurityPosture	15.2KB	15.8KB	+535.0B

History

• 💔 Build #179791 failed 05fab2a
• 💛 Build #178682 was flaky a3b3b64
• 💔 Build #176696 failed 46732af
• 💔 Build #176682 failed b12d7c7
• 💔 Build #176394 failed a25c4a8

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

cc @JordanSh