From dec4c6265b943c22889c8d8762752da7d76e3a05 Mon Sep 17 00:00:00 2001 From: Benoit BERAUD Date: Mon, 6 Mar 2023 08:36:25 +0100 Subject: [PATCH 1/3] Issue 985 - Add support for clientApplications/servicePrincipals in conditional access policies --- docs/resources/conditional_access_policy.md | 85 +++++++++++++- .../conditional_access_policy_resource.go | 34 +++++- ...conditional_access_policy_resource_test.go | 111 ++++++++++++++++++ .../conditionalaccess/conditionalaccess.go | 33 ++++++ 4 files changed, 257 insertions(+), 6 deletions(-) diff --git a/docs/resources/conditional_access_policy.md b/docs/resources/conditional_access_policy.md index 448e52f9a3..79bd9d4eaa 100644 --- a/docs/resources/conditional_access_policy.md +++ b/docs/resources/conditional_access_policy.md @@ -6,6 +6,8 @@ subcategory: "Conditional Access" Manages a Conditional Access Policy within Azure Active Directory. +-> **Licensing Requirements** Specifying `client_applications` property requires the activation of Microsoft Entra on your tenant and the availability of sufficient Workload Identities Premium licences (one per service principal managed by a conditional access). + ## API Permissions The following API permissions are required in order to use this resource. @@ -16,6 +18,8 @@ When authenticated with a user principal, this resource requires one of the foll ## Example Usage +### All users except guests or external users + ```terraform resource "azuread_conditional_access_policy" "example" { display_name = "example policy" @@ -69,6 +73,73 @@ resource "azuread_conditional_access_policy" "example" { } ``` +### Included client applications / service principals + +```terraform + +data "azuread_client_config" "current" {} + +resource "azuread_conditional_access_policy" "example" { + display_name = "example policy" + state = "disabled" + + conditions { + client_app_types = ["all"] + + applications { + included_applications = ["All"] + } + + client_applications { + included_service_principals = [data.azuread_client_config.current.object_id] + excluded_service_principals = [] + } + + users { + included_users = ["None"] + } + } + + grant_controls { + operator = "OR" + built_in_controls = ["block"] + } +} +``` + +### Excluded client applications / service principals + +```terraform + +data "azuread_client_config" "current" {} + +resource "azuread_conditional_access_policy" "example" { + display_name = "example policy" + state = "disabled" + + conditions { + client_app_types = ["all"] + + applications { + included_applications = ["All"] + } + + client_applications { + included_service_principals = ["ServicePrincipalsInMyTenant"] + excluded_service_principals = [data.azuread_client_config.current.object_id] + } + + users { + included_users = ["None"] + } + } + + grant_controls { + operator = "OR" + built_in_controls = ["block"] + } +} +``` ## Argument Reference The following arguments are supported: @@ -84,13 +155,14 @@ The following arguments are supported: `conditions` block supports the following: * `applications` - (Required) An `applications` block as documented below, which specifies applications and user actions included in and excluded from the policy. +* `client_applications` - (Optional) An `client_applications` block as documented below, which specifies service principals included in and excluded from the policy. Either `client_applications` or `users` must be specified (and not both). * `client_app_types` - (Required) A list of client application types included in the policy. Possible values are: `all`, `browser`, `mobileAppsAndDesktopClients`, `exchangeActiveSync`, `easSupported` and `other`. * `devices` - (Optional) A `devices` block as documented below, which describes devices to be included in and excluded from the policy. A `devices` block can be added to an existing policy, but removing the `devices` block forces a new resource to be created. -* `locations` - (Required) A `locations` block as documented below, which specifies locations included in and excluded from the policy. -* `platforms` - (Required) A `platforms` block as documented below, which specifies platforms included in and excluded from the policy. +* `locations` - (Optional) A `locations` block as documented below, which specifies locations included in and excluded from the policy. +* `platforms` - (Optional) A `platforms` block as documented below, which specifies platforms included in and excluded from the policy. * `sign_in_risk_levels` - (Optional) A list of sign-in risk levels included in the policy. Possible values are: `low`, `medium`, `high`, `hidden`, `none`, `unknownFutureValue`. * `user_risk_levels` - (Optional) A list of user risk levels included in the policy. Possible values are: `low`, `medium`, `high`, `hidden`, `none`, `unknownFutureValue`. -* `users` - (Required) A `users` block as documented below, which specifies users, groups, and roles included in and excluded from the policy. +* `users` - (Optional) A `users` block as documented below, which specifies users, groups, and roles included in and excluded from the policy. Either `client_applications` or `users` must be specified (and not both). --- @@ -102,6 +174,13 @@ The following arguments are supported: --- +`client_applications` block supports the following: + +* `excluded_service_principals` - (Optional) A list of service principal IDs explicitly excluded in the policy. +* `included_service_principals` - (Optional) A list of service principal IDs explicitly included in the policy. Can be set to `ServicePrincipalsInMyTenant` to include all service principals. This is mandatory value when at least one `excluded_service_principals` is set. + +--- + `devices` block supports the following: * `filter` - (Optional) A `filter` block as described below. A `filter` block can be added to an existing policy, but removing the `filter` block forces a new resource to be created. diff --git a/internal/services/conditionalaccess/conditional_access_policy_resource.go b/internal/services/conditionalaccess/conditional_access_policy_resource.go index abdd2c336a..4fd87f49d4 100644 --- a/internal/services/conditionalaccess/conditional_access_policy_resource.go +++ b/internal/services/conditionalaccess/conditional_access_policy_resource.go @@ -109,6 +109,33 @@ func conditionalAccessPolicyResource() *schema.Resource { }, }, + "client_applications": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "included_service_principals": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateDiagFunc: validate.NoEmptyStrings, + }, + }, + + "excluded_service_principals": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateDiagFunc: validate.NoEmptyStrings, + }, + }, + }, + }, + }, + "users": { Type: schema.TypeList, Required: true, @@ -116,9 +143,10 @@ func conditionalAccessPolicyResource() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "included_users": { - Type: schema.TypeList, - Optional: true, - AtLeastOneOf: []string{"conditions.0.users.0.included_groups", "conditions.0.users.0.included_roles", "conditions.0.users.0.included_users"}, + Type: schema.TypeList, + Optional: true, + AtLeastOneOf: []string{"conditions.0.users.0.included_groups", "conditions.0.users.0.included_roles", "conditions.0.users.0.included_users"}, + DiffSuppressFunc: conditionalAccessPolicyDiffSuppress, Elem: &schema.Schema{ Type: schema.TypeString, ValidateDiagFunc: validate.NoEmptyStrings, diff --git a/internal/services/conditionalaccess/conditional_access_policy_resource_test.go b/internal/services/conditionalaccess/conditional_access_policy_resource_test.go index 2e091ea041..c67ccc415a 100644 --- a/internal/services/conditionalaccess/conditional_access_policy_resource_test.go +++ b/internal/services/conditionalaccess/conditional_access_policy_resource_test.go @@ -275,6 +275,48 @@ func TestAccConditionalAccessPolicy_sessionControlsDisabled(t *testing.T) { }) } +func TestAccConditionalAccessPolicy_clientApplications(t *testing.T) { + // This is a separate test for two reasons: + // - conditional access policies applies either to users/groups or to client applications (workload identities) + // - conditional access policies using client applications requires special licensing (Microsoft Entra Workload Identities) + + data := acceptance.BuildTestData(t, "azuread_conditional_access_policy", "test") + r := ConditionalAccessPolicyResource{} + + data.ResourceTest(t, r, []resource.TestStep{ + { + Config: r.clientApplicationsIncluded(data), + Check: resource.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("id").Exists(), + check.That(data.ResourceName).Key("display_name").HasValue(fmt.Sprintf("acctest-CONPOLICY-%d", data.RandomInteger)), + check.That(data.ResourceName).Key("state").HasValue("disabled"), + ), + }, + data.ImportStep(), + { + Config: r.clientApplicationsExcluded(data), + Check: resource.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("id").Exists(), + check.That(data.ResourceName).Key("display_name").HasValue(fmt.Sprintf("acctest-CONPOLICY-%d", data.RandomInteger)), + check.That(data.ResourceName).Key("state").HasValue("disabled"), + ), + }, + data.ImportStep(), + { + Config: r.clientApplicationsIncluded(data), + Check: resource.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("id").Exists(), + check.That(data.ResourceName).Key("display_name").HasValue(fmt.Sprintf("acctest-CONPOLICY-%d", data.RandomInteger)), + check.That(data.ResourceName).Key("state").HasValue("disabled"), + ), + }, + data.ImportStep(), + }) +} + func (r ConditionalAccessPolicyResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) { var id *string @@ -602,3 +644,72 @@ resource "azuread_conditional_access_policy" "test" { } `, data.RandomInteger) } + +func (ConditionalAccessPolicyResource) clientApplicationsIncluded(data acceptance.TestData) string { + return fmt.Sprintf(` + + +data "azuread_client_config" "test" {} + +resource "azuread_conditional_access_policy" "test" { + display_name = "acctest-CONPOLICY-%[1]d" + state = "disabled" + + conditions { + client_app_types = ["all"] + + applications { + included_applications = ["All"] + } + + client_applications { + included_service_principals = [data.azuread_client_config.test.object_id] + } + + users { + included_users = ["None"] + } + } + + grant_controls { + operator = "OR" + built_in_controls = ["block"] + } +} +`, data.RandomInteger) +} + +func (ConditionalAccessPolicyResource) clientApplicationsExcluded(data acceptance.TestData) string { + return fmt.Sprintf(` + + +data "azuread_client_config" "test" {} + +resource "azuread_conditional_access_policy" "test" { + display_name = "acctest-CONPOLICY-%[1]d" + state = "disabled" + + conditions { + client_app_types = ["all"] + + applications { + included_applications = ["All"] + } + + client_applications { + included_service_principals = ["ServicePrincipalsInMyTenant"] + excluded_service_principals = [data.azuread_client_config.test.object_id] + } + + users { + included_users = ["None"] + } + } + + grant_controls { + operator = "OR" + built_in_controls = ["block"] + } +} +`, data.RandomInteger) +} diff --git a/internal/services/conditionalaccess/conditionalaccess.go b/internal/services/conditionalaccess/conditionalaccess.go index ae8eb51172..1d546f9889 100644 --- a/internal/services/conditionalaccess/conditionalaccess.go +++ b/internal/services/conditionalaccess/conditionalaccess.go @@ -17,6 +17,7 @@ func flattenConditionalAccessConditionSet(in *msgraph.ConditionalAccessCondition return []interface{}{ map[string]interface{}{ "applications": flattenConditionalAccessApplications(in.Applications), + "client_applications": flattenConditionalAccessClientApplications(in.ClientApplications), "users": flattenConditionalAccessUsers(in.Users), "client_app_types": tf.FlattenStringSlicePtr(in.ClientAppTypes), "devices": flattenConditionalAccessDevices(in.Devices), @@ -42,6 +43,19 @@ func flattenConditionalAccessApplications(in *msgraph.ConditionalAccessApplicati } } +func flattenConditionalAccessClientApplications(in *msgraph.ConditionalAccessClientApplications) []interface{} { + if in == nil { + return []interface{}{} + } + + return []interface{}{ + map[string]interface{}{ + "included_service_principals": tf.FlattenStringSlicePtr(in.IncludeServicePrincipals), + "excluded_service_principals": tf.FlattenStringSlicePtr(in.ExcludeServicePrincipals), + }, + } +} + func flattenConditionalAccessUsers(in *msgraph.ConditionalAccessUsers) []interface{} { if in == nil { return []interface{}{} @@ -236,6 +250,7 @@ func expandConditionalAccessConditionSet(in []interface{}) *msgraph.ConditionalA platforms := config["platforms"].([]interface{}) signInRiskLevels := config["sign_in_risk_levels"].([]interface{}) userRiskLevels := config["user_risk_levels"].([]interface{}) + clientApplications := config["client_applications"].([]interface{}) result.Applications = expandConditionalAccessApplications(applications) result.Users = expandConditionalAccessUsers(users) @@ -245,6 +260,24 @@ func expandConditionalAccessConditionSet(in []interface{}) *msgraph.ConditionalA result.Platforms = expandConditionalAccessPlatforms(platforms) result.SignInRiskLevels = tf.ExpandStringSlicePtr(signInRiskLevels) result.UserRiskLevels = tf.ExpandStringSlicePtr(userRiskLevels) + result.ClientApplications = expandConditionalAccessClientApplications(clientApplications) + + return &result +} + +func expandConditionalAccessClientApplications(in []interface{}) *msgraph.ConditionalAccessClientApplications { + if len(in) == 0 || in[0] == nil { + return nil + } + + result := msgraph.ConditionalAccessClientApplications{} + config := in[0].(map[string]interface{}) + + includeServicePrincipals := config["included_service_principals"].([]interface{}) + excludeServicePrincipals := config["excluded_service_principals"].([]interface{}) + + result.IncludeServicePrincipals = tf.ExpandStringSlicePtr(includeServicePrincipals) + result.ExcludeServicePrincipals = tf.ExpandStringSlicePtr(excludeServicePrincipals) return &result } From c0ea22f4eb4bf2c9280156f2c3fa80d1f8e85502 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 13 Jul 2023 01:48:28 +0100 Subject: [PATCH 2/3] use single tenant app for workload identity in conditional access policy --- .../conditional_access_policy_resource_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/services/conditionalaccess/conditional_access_policy_resource_test.go b/internal/services/conditionalaccess/conditional_access_policy_resource_test.go index c67ccc415a..298ed89480 100644 --- a/internal/services/conditionalaccess/conditional_access_policy_resource_test.go +++ b/internal/services/conditionalaccess/conditional_access_policy_resource_test.go @@ -647,9 +647,9 @@ resource "azuread_conditional_access_policy" "test" { func (ConditionalAccessPolicyResource) clientApplicationsIncluded(data acceptance.TestData) string { return fmt.Sprintf(` - - -data "azuread_client_config" "test" {} +data "azuread_service_principal" "test" { + display_name = "Terraform Acceptance Tests (Single Tenant)" +} resource "azuread_conditional_access_policy" "test" { display_name = "acctest-CONPOLICY-%[1]d" @@ -663,7 +663,7 @@ resource "azuread_conditional_access_policy" "test" { } client_applications { - included_service_principals = [data.azuread_client_config.test.object_id] + included_service_principals = [data.azuread_service_principal.test.object_id] } users { @@ -681,9 +681,9 @@ resource "azuread_conditional_access_policy" "test" { func (ConditionalAccessPolicyResource) clientApplicationsExcluded(data acceptance.TestData) string { return fmt.Sprintf(` - - -data "azuread_client_config" "test" {} +data "azuread_service_principal" "test" { + display_name = "Terraform Acceptance Tests (Single Tenant)" +} resource "azuread_conditional_access_policy" "test" { display_name = "acctest-CONPOLICY-%[1]d" @@ -698,7 +698,7 @@ resource "azuread_conditional_access_policy" "test" { client_applications { included_service_principals = ["ServicePrincipalsInMyTenant"] - excluded_service_principals = [data.azuread_client_config.test.object_id] + excluded_service_principals = [data.azuread_service_principal.test.object_id] } users { From 55b7697cf9088554885a9087cf3093c44ecd4052 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 13 Jul 2023 02:36:33 +0100 Subject: [PATCH 3/3] azuread_conditional_access_policy: users block is always required to avoid presistent diff --- docs/resources/conditional_access_policy.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/resources/conditional_access_policy.md b/docs/resources/conditional_access_policy.md index 79bd9d4eaa..d6b630d97f 100644 --- a/docs/resources/conditional_access_policy.md +++ b/docs/resources/conditional_access_policy.md @@ -155,14 +155,14 @@ The following arguments are supported: `conditions` block supports the following: * `applications` - (Required) An `applications` block as documented below, which specifies applications and user actions included in and excluded from the policy. -* `client_applications` - (Optional) An `client_applications` block as documented below, which specifies service principals included in and excluded from the policy. Either `client_applications` or `users` must be specified (and not both). * `client_app_types` - (Required) A list of client application types included in the policy. Possible values are: `all`, `browser`, `mobileAppsAndDesktopClients`, `exchangeActiveSync`, `easSupported` and `other`. +* `client_applications` - (Optional) An `client_applications` block as documented below, which specifies service principals included in and excluded from the policy. * `devices` - (Optional) A `devices` block as documented below, which describes devices to be included in and excluded from the policy. A `devices` block can be added to an existing policy, but removing the `devices` block forces a new resource to be created. * `locations` - (Optional) A `locations` block as documented below, which specifies locations included in and excluded from the policy. * `platforms` - (Optional) A `platforms` block as documented below, which specifies platforms included in and excluded from the policy. * `sign_in_risk_levels` - (Optional) A list of sign-in risk levels included in the policy. Possible values are: `low`, `medium`, `high`, `hidden`, `none`, `unknownFutureValue`. * `user_risk_levels` - (Optional) A list of user risk levels included in the policy. Possible values are: `low`, `medium`, `high`, `hidden`, `none`, `unknownFutureValue`. -* `users` - (Optional) A `users` block as documented below, which specifies users, groups, and roles included in and excluded from the policy. Either `client_applications` or `users` must be specified (and not both). +* `users` - (Required) A `users` block as documented below, which specifies users, groups, and roles included in and excluded from the policy. ---