Bugfix: enable setting of session_controls instead of, or in addition to, grant_controls

docs/resources/conditional_access_policy.md

@@ -146,8 +146,11 @@ The following arguments are supported:
 
 * `conditions` - (Required) A `conditions` block as documented below, which specifies the rules that must be met for the policy to apply.
 * `display_name` - (Required) The friendly name for this Conditional Access Policy.
-* `grant_controls` - (Required) A `grant_controls` block as documented below, which specifies the grant controls that must be fulfilled to pass the policy.
+* `grant_controls` - (Optional) A `grant_controls` block as documented below, which specifies the grant controls that must be fulfilled to pass the policy.
 * `session_controls` - (Optional) A `session_controls` block as documented below, which specifies the session controls that are enforced after sign-in.
+
+~> Note: At least one of `grant_controls` and/or `session_controls` blocks must be specified.
+
 * `state` - (Required) Specifies the state of the policy object. Possible values are: `enabled`, `disabled` and `enabledForReportingButNotEnforced`
 
 ---

go.mod

@@ -5,7 +5,7 @@ require (
 	github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
 	github.com/hashicorp/go-uuid v1.0.3
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.26.1
-	github.com/manicminer/hamilton v0.62.0
+	github.com/manicminer/hamilton v0.63.0
 	golang.org/x/text v0.9.0
 )
 

go.sum

@@ -204,8 +204,8 @@ github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
-github.com/manicminer/hamilton v0.62.0 h1:auy910L0VntDUDHMG6K6e6jr5QUk3OHSldt13NHztwY=
-github.com/manicminer/hamilton v0.62.0/go.mod h1:va/X2sztcgQ5+BSxc2eU3FTHYIyxLnHvB4LudlPUZdE=
+github.com/manicminer/hamilton v0.63.0 h1:Pxh+TvuRhGsKl29v3dnzAoNJYUwqn6SNp/TGddg3g7E=
+github.com/manicminer/hamilton v0.63.0/go.mod h1:va/X2sztcgQ5+BSxc2eU3FTHYIyxLnHvB4LudlPUZdE=
 github.com/matryer/is v1.2.0/go.mod h1:2fLPjFQM9rhQ15aVEtbuwhJinnOqrmgXPNdZsdwlWXA=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=

internal/services/conditionalaccess/conditional_access_policy_resource.go

@@ -143,10 +143,9 @@ 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"},
-										DiffSuppressFunc: conditionalAccessPolicyDiffSuppress,
+										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"},
 										Elem: &schema.Schema{
 											Type:             schema.TypeString,
 											ValidateDiagFunc: validate.NoEmptyStrings,
@@ -375,9 +374,10 @@ func conditionalAccessPolicyResource() *schema.Resource {
 			},
 
 			"grant_controls": {
-				Type:     schema.TypeList,
-				Required: true,
-				MaxItems: 1,
+				Type:         schema.TypeList,
+				Optional:     true,
+				AtLeastOneOf: []string{"grant_controls", "session_controls"},
+				MaxItems:     1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"operator": {
@@ -428,6 +428,7 @@ func conditionalAccessPolicyResource() *schema.Resource {
 			"session_controls": {
 				Type:             schema.TypeList,
 				Optional:         true,
+				AtLeastOneOf:     []string{"grant_controls", "session_controls"},
 				MaxItems:         1,
 				DiffSuppressFunc: conditionalAccessPolicyDiffSuppress,
 				Elem: &schema.Resource{
@@ -506,6 +507,8 @@ func conditionalAccessPolicyDiffSuppress(k, old, new string, d *schema.ResourceD
 
 	switch {
 	case k == "session_controls.#" && old == "0" && new == "1":
+		// When an ineffectual `session_controls` block is configured, the API just ignores it and returns
+		// sessionControls: null
 		sessionControlsRaw := d.Get("session_controls").([]interface{})
 		if len(sessionControlsRaw) == 1 && sessionControlsRaw[0] != nil {
 			sessionControls := sessionControlsRaw[0].(map[string]interface{})
@@ -538,11 +541,17 @@ func conditionalAccessPolicyResourceCreate(ctx context.Context, d *schema.Resour
 	client := meta.(*clients.Client).ConditionalAccess.PoliciesClient
 
 	properties := msgraph.ConditionalAccessPolicy{
-		DisplayName:     utils.String(d.Get("display_name").(string)),
-		State:           utils.String(d.Get("state").(string)),
-		Conditions:      expandConditionalAccessConditionSet(d.Get("conditions").([]interface{})),
-		GrantControls:   expandConditionalAccessGrantControls(d.Get("grant_controls").([]interface{})),
-		SessionControls: expandConditionalAccessSessionControls(d.Get("session_controls").([]interface{})),
+		DisplayName: utils.String(d.Get("display_name").(string)),
+		State:       utils.String(d.Get("state").(string)),
+		Conditions:  expandConditionalAccessConditionSet(d.Get("conditions").([]interface{})),
+	}
+
+	if v, ok := d.GetOk("grant_controls"); ok {
+		properties.GrantControls = expandConditionalAccessGrantControls(v.([]interface{}))
+	}
+
+	if v, ok := d.GetOk("session_controls"); ok {
+		properties.SessionControls = expandConditionalAccessSessionControls(v.([]interface{}))
 	}
 
 	policy, _, err := client.Create(ctx, properties)
@@ -563,12 +572,18 @@ func conditionalAccessPolicyResourceUpdate(ctx context.Context, d *schema.Resour
 	client := meta.(*clients.Client).ConditionalAccess.PoliciesClient
 
 	properties := msgraph.ConditionalAccessPolicy{
-		ID:              utils.String(d.Id()),
-		DisplayName:     utils.String(d.Get("display_name").(string)),
-		State:           utils.String(d.Get("state").(string)),
-		Conditions:      expandConditionalAccessConditionSet(d.Get("conditions").([]interface{})),
-		GrantControls:   expandConditionalAccessGrantControls(d.Get("grant_controls").([]interface{})),
-		SessionControls: expandConditionalAccessSessionControls(d.Get("session_controls").([]interface{})),
+		ID:          utils.String(d.Id()),
+		DisplayName: utils.String(d.Get("display_name").(string)),
+		State:       utils.String(d.Get("state").(string)),
+		Conditions:  expandConditionalAccessConditionSet(d.Get("conditions").([]interface{})),
+	}
+
+	if v, ok := d.GetOk("grant_controls"); ok {
+		properties.GrantControls = expandConditionalAccessGrantControls(v.([]interface{}))
+	}
+
+	if v, ok := d.GetOk("session_controls"); ok {
+		properties.SessionControls = expandConditionalAccessSessionControls(v.([]interface{}))
 	}
 
 	if _, err := client.Update(ctx, properties); err != nil {

internal/services/conditionalaccess/conditional_access_policy_resource_test.go

@@ -49,7 +49,7 @@ func TestAccConditionalAccessPolicy_complete(t *testing.T) {
 				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"),
+				check.That(data.ResourceName).Key("state").HasValue("enabledForReportingButNotEnforced"),
 			),
 		},
 		data.ImportStep(),
@@ -164,9 +164,6 @@ func TestAccConditionalAccessPolicy_includedUserActions(t *testing.T) {
 }
 
 func TestAccConditionalAccessPolicy_sessionControls(t *testing.T) {
-	// This is in a separate test to avoid ForceNew in the update test due to https://github.com/microsoftgraph/msgraph-metadata/issues/93
-	// session_controls can be added to the complete config, and this rest removed, when this issue is resolved
-
 	data := acceptance.BuildTestData(t, "azuread_conditional_access_policy", "test")
 	r := ConditionalAccessPolicyResource{}
 
@@ -198,26 +195,6 @@ func TestAccConditionalAccessPolicy_sessionControls(t *testing.T) {
 			),
 		},
 		data.ImportStep(),
-		{
-			Config: r.sessionControlsUpdate(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.sessionControls(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.basic(data),
 			Check: resource.ComposeTestCheckFunc(
@@ -239,7 +216,7 @@ func TestAccConditionalAccessPolicy_sessionControls(t *testing.T) {
 }
 
 func TestAccConditionalAccessPolicy_sessionControlsDisabled(t *testing.T) {
-	// Remove this test when https://github.com/microsoftgraph/msgraph-metadata/issues/93 is resolved
+	// This is testing the DiffSuppressFunc for the `session_controls` block
 
 	data := acceptance.BuildTestData(t, "azuread_conditional_access_policy", "test")
 	r := ConditionalAccessPolicyResource{}
@@ -367,7 +344,7 @@ func (ConditionalAccessPolicyResource) complete(data acceptance.TestData) string
 	return fmt.Sprintf(`
 resource "azuread_conditional_access_policy" "test" {
   display_name = "acctest-CONPOLICY-%[1]d"
-  state        = "disabled"
+  state        = "enabledForReportingButNotEnforced"
 
   conditions {
     client_app_types    = ["all"]
@@ -376,7 +353,7 @@ resource "azuread_conditional_access_policy" "test" {
 
     applications {
       included_applications = ["All"]
-      excluded_applications = ["00000004-0000-0ff1-ce00-000000000000"]
+      excluded_applications = []
     }
 
     locations {
@@ -385,8 +362,8 @@ resource "azuread_conditional_access_policy" "test" {
     }
 
     platforms {
-      included_platforms = ["android"]
-      excluded_platforms = ["iOS"]
+      included_platforms = ["all"]
+      excluded_platforms = ["android", "iOS"]
     }
 
     users {
@@ -399,6 +376,15 @@ resource "azuread_conditional_access_policy" "test" {
     operator          = "OR"
     built_in_controls = ["mfa"]
   }
+
+  session_controls {
+    application_enforced_restrictions_enabled = true
+    disable_resilience_defaults               = false
+    cloud_app_security_policy                 = "blockDownloads"
+    persistent_browser_mode                   = "always"
+    sign_in_frequency                         = 2
+    sign_in_frequency_period                  = "days"
+  }
 }
 `, data.RandomInteger)
 }
@@ -549,11 +535,6 @@ resource "azuread_conditional_access_policy" "test" {
     }
   }
 
-  grant_controls {
-    operator          = "OR"
-    built_in_controls = ["block"]
-  }
-
   session_controls {
     application_enforced_restrictions_enabled = true
     disable_resilience_defaults               = true
@@ -566,50 +547,6 @@ resource "azuread_conditional_access_policy" "test" {
 `, data.RandomInteger)
 }
 
-func (ConditionalAccessPolicyResource) sessionControlsUpdate(data acceptance.TestData) string {
-	return fmt.Sprintf(`
-resource "azuread_conditional_access_policy" "test" {
-  display_name = "acctest-CONPOLICY-%[1]d"
-  state        = "disabled"
-
-  conditions {
-    client_app_types = ["browser"]
-
-    applications {
-      included_applications = ["All"]
-    }
-
-    locations {
-      included_locations = ["All"]
-    }
-
-    platforms {
-      included_platforms = ["all"]
-    }
-
-    users {
-      included_users = ["All"]
-      excluded_users = ["GuestsOrExternalUsers"]
-    }
-  }
-
-  grant_controls {
-    operator          = "OR"
-    built_in_controls = ["block"]
-  }
-
-  session_controls {
-    application_enforced_restrictions_enabled = true
-    disable_resilience_defaults               = false
-    cloud_app_security_policy                 = "blockDownloads"
-    persistent_browser_mode                   = "always"
-    sign_in_frequency                         = 2
-    sign_in_frequency_period                  = "days"
-  }
-}
-`, data.RandomInteger)
-}
-
 func (ConditionalAccessPolicyResource) sessionControlsDisabled(data acceptance.TestData) string {
 	return fmt.Sprintf(`
 resource "azuread_conditional_access_policy" "test" {

internal/services/identitygovernance/access_package_assignment_policy_resource.go

@@ -228,11 +228,11 @@ func accessPackageAssignmentPolicyResource() *schema.Resource {
 							Type:        schema.TypeString,
 							Optional:    true,
 							ValidateFunc: validation.StringInSlice([]string{
-								msgraph.AccessReviewRecurranceTypeAnnual,
-								msgraph.AccessReviewRecurranceTypeHalfYearly,
-								msgraph.AccessReviewRecurranceTypeQuarterly,
-								msgraph.AccessReviewRecurranceTypeMonthly,
-								msgraph.AccessReviewRecurranceTypeWeekly,
+								msgraph.AccessReviewRecurrenceTypeAnnual,
+								msgraph.AccessReviewRecurrenceTypeHalfYearly,
+								msgraph.AccessReviewRecurrenceTypeQuarterly,
+								msgraph.AccessReviewRecurrenceTypeMonthly,
+								msgraph.AccessReviewRecurrenceTypeWeekly,
 							}, false),
 						},