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.

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

Azure network security groups add user auth #20779

Merged
merged 9 commits into from Aug 31, 2022

Conversation

ostolero
Copy link
Contributor

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: link to the issue

Description

Added the ability to configure an instance using user authentication flow.

Screenshots

Paste here any images that will help the reviewer

Minimum version of Cortex XSOAR

  • 6.0.0
  • 6.1.0
  • 6.2.0
  • 6.5.0

Does it break backward compatibility?

  • Yes
    • Further details:
  • No

Must have

  • Tests
  • Documentation

Copy link
Contributor

@JudahSchwartz JudahSchwartz left a comment

Choose a reason for hiding this comment

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

Looks good!
Lets just check up on the documentation

@@ -323,11 +307,11 @@ Update a security rule. If one does not exist, it will be created.
| direction | The direction of the rule. Possible values are: "Inbound" and "Outbound". Possible values are: Inbound, Outbound. | Optional |
| action | Whether to allow the traffic. Possible values are "Allow" and "Deny". Possible values are: Allow, Deny. | Optional |
| protocol | The protocol on which to apply the rule. Possible values are: "Any", "TCP", "UDP", and "ICMP". Possible values are: Any, TCP, UDP, ICMP. | Optional |
| source | The source IP address range from which incoming traffic will be allowed or denied by this rule. Possible values are "Any", an IP address range, an application security group, or a default tag. Default is "Any". | Optional |
| priority | The priority by which the rules will be processed. The lower the number, the higher the priority. We recommend leaving gaps between rules - 100, 200, 300, etc. - so that it is easier to add new rules without having to edit existing rules. Default is "4096". | Optional |
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we taking out these "default is x" lines on purpose? Looks like it theyre missing since they werent in the yml. Please check if these defaults make sense according to the api docs, and if they do then lets add them to the yml

Copy link
Contributor

@JudahSchwartz JudahSchwartz left a comment

Choose a reason for hiding this comment

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

Great thanks!!

@xsoar-bot
Copy link
Contributor

@ostolero
Copy link
Contributor Author

force merge is required as we added a new param auth_type with required: true, which fails validation but should be presented in this way.
Default is assigned in yml and in the code for bc.

@ShahafBenYakir ShahafBenYakir merged commit 2490a88 into master Aug 31, 2022
@ShahafBenYakir ShahafBenYakir deleted the Azure_Network_Security_Groups_Add_user_auth branch August 31, 2022 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants