diff --git a/go.mod b/go.mod index d3f8109008..2f1de60d88 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/hashicorp/go-uuid v1.0.3 github.com/hashicorp/terraform-plugin-sdk/v2 v2.29.0 github.com/hashicorp/terraform-plugin-testing v1.5.1 - github.com/manicminer/hamilton v0.67.0 + github.com/manicminer/hamilton v0.68.0 golang.org/x/text v0.14.0 ) diff --git a/go.sum b/go.sum index c9861012b7..23761cc43b 100644 --- a/go.sum +++ b/go.sum @@ -111,8 +111,8 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= 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.67.0 h1:hG3tPunQCGcgP2Nx0+lwW+Swu9MXOs4JGospakK79pY= -github.com/manicminer/hamilton v0.67.0/go.mod h1:u80g9rPtJpCG7EC0iayttt8UfeAp6jknClixgZGE950= +github.com/manicminer/hamilton v0.68.0 h1:2ZKQRTegktxJIoLBWxI43HczpTiwXl6brvwTXy75gPk= +github.com/manicminer/hamilton v0.68.0/go.mod h1:u80g9rPtJpCG7EC0iayttt8UfeAp6jknClixgZGE950= github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= diff --git a/internal/services/conditionalaccess/conditional_access_policy_resource.go b/internal/services/conditionalaccess/conditional_access_policy_resource.go index e4c8d9d7f8..3f6e90e3a5 100644 --- a/internal/services/conditionalaccess/conditional_access_policy_resource.go +++ b/internal/services/conditionalaccess/conditional_access_policy_resource.go @@ -582,7 +582,7 @@ func conditionalAccessPolicyResource() *pluginsdk.Resource { "sign_in_frequency_authentication_type": { Type: pluginsdk.TypeString, Optional: true, - Default: msgraph.ConditionalAccessAuthenticationTypePrimaryAndSecondaryAuthentication, + Computed: true, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessAuthenticationTypePrimaryAndSecondaryAuthentication, msgraph.ConditionalAccessAuthenticationTypeSecondaryAuthentication, @@ -592,7 +592,7 @@ func conditionalAccessPolicyResource() *pluginsdk.Resource { "sign_in_frequency_interval": { Type: pluginsdk.TypeString, Optional: true, - Default: msgraph.ConditionalAccessFrequencyIntervalTimeBased, + Computed: true, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessFrequencyIntervalTimeBased, msgraph.ConditionalAccessFrequencyIntervalEveryTime, @@ -637,12 +637,14 @@ func conditionalAccessPolicyCustomizeDiff(_ context.Context, diff *pluginsdk.Res func conditionalAccessPolicyDiffSuppress(k, old, new string, d *pluginsdk.ResourceData) bool { suppress := false + // When ineffectual `session_controls` are specified, you must send `sessionControls: null`, and when policy has ineffectual + // `sessionControls`, the API condenses it to `sessionControls: null` in the response. if 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{}) + + // Suppress by default, but only if all the block properties have a non-default value suppress = true if v, ok := sessionControls["application_enforced_restrictions_enabled"]; ok && v.(bool) { suppress = false @@ -659,10 +661,10 @@ func conditionalAccessPolicyDiffSuppress(k, old, new string, d *pluginsdk.Resour if v, ok := sessionControls["sign_in_frequency"]; ok && v.(int) > 0 { suppress = false } - if v, ok := sessionControls["sign_in_frequency_authentication_type"]; ok && v.(string) != msgraph.ConditionalAccessAuthenticationTypePrimaryAndSecondaryAuthentication { + if v, ok := sessionControls["sign_in_frequency_authentication_type"]; ok && v.(string) != "" { suppress = false } - if v, ok := sessionControls["sign_in_frequency_interval"]; ok && v.(string) != msgraph.ConditionalAccessFrequencyIntervalTimeBased { + if v, ok := sessionControls["sign_in_frequency_interval"]; ok && v.(string) != "" { suppress = false } if v, ok := sessionControls["sign_in_frequency_period"]; ok && v.(string) != "" { diff --git a/internal/services/conditionalaccess/conditional_access_policy_resource_test.go b/internal/services/conditionalaccess/conditional_access_policy_resource_test.go index 07ebc3a21c..15f9bf2cc2 100644 --- a/internal/services/conditionalaccess/conditional_access_policy_resource_test.go +++ b/internal/services/conditionalaccess/conditional_access_policy_resource_test.go @@ -162,16 +162,6 @@ func TestAccConditionalAccessPolicy_sessionControls(t *testing.T) { ), }, data.ImportStep(), - }) -} - -func TestAccConditionalAccessPolicy_sessionControlsDisabled(t *testing.T) { - // This is testing the DiffSuppressFunc for the `session_controls` block - - data := acceptance.BuildTestData(t, "azuread_conditional_access_policy", "test") - r := ConditionalAccessPolicyResource{} - - data.ResourceTest(t, r, []acceptance.TestStep{ { Config: r.sessionControlsDisabled(data), Check: acceptance.ComposeTestCheckFunc( @@ -199,6 +189,46 @@ func TestAccConditionalAccessPolicy_sessionControlsDisabled(t *testing.T) { ), }, data.ImportStep(), + { + Config: r.sessionControlsApplicationEnforcedRestrictions(data), + Check: acceptance.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.sessionControlsCloudAppSecurityPolicy(data), + Check: acceptance.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.sessionControlsPersistentBrowserMode(data), + Check: acceptance.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.sessionControlsDisabled(data), + Check: acceptance.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(), }) } @@ -302,6 +332,11 @@ func TestAccConditionalAccessPolicy_guestsOrExternalUsers(t *testing.T) { } func (r ConditionalAccessPolicyResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { + clients.ConditionalAccess.PoliciesClient.BaseClient.DisableRetries = true + defer func() { + clients.ConditionalAccess.PoliciesClient.BaseClient.DisableRetries = false + }() + var id *string app, status, err := clients.ConditionalAccess.PoliciesClient.Get(ctx, state.ID, odata.Query{}) @@ -523,6 +558,129 @@ resource "azuread_conditional_access_policy" "test" { `, data.RandomInteger) } +func (ConditionalAccessPolicyResource) sessionControlsApplicationEnforcedRestrictions(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azuread" {} + +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 + } +} +`, data.RandomInteger) +} + +func (ConditionalAccessPolicyResource) sessionControlsCloudAppSecurityPolicy(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azuread" {} + +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 { + cloud_app_security_policy = "monitorOnly" + } +} +`, data.RandomInteger) +} + +func (ConditionalAccessPolicyResource) sessionControlsPersistentBrowserMode(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azuread" {} + +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 { + persistent_browser_mode = "always" + } +} +`, data.RandomInteger) +} + func (ConditionalAccessPolicyResource) clientApplicationsIncluded(data acceptance.TestData) string { return fmt.Sprintf(` provider "azuread" {} diff --git a/internal/services/conditionalaccess/conditionalaccess.go b/internal/services/conditionalaccess/conditionalaccess.go index f01a63b8cd..ba082f509c 100644 --- a/internal/services/conditionalaccess/conditionalaccess.go +++ b/internal/services/conditionalaccess/conditionalaccess.go @@ -493,19 +493,24 @@ func expandConditionalAccessSessionControls(in []interface{}) *msgraph.Condition signInFrequency.IsEnabled = pointer.To(true) signInFrequency.Type = pointer.To(config["sign_in_frequency_period"].(string)) signInFrequency.Value = pointer.To(int32(frequencyValue)) + + // AuthenticationType and FrequencyInterval must be set to default values here + signInFrequency.AuthenticationType = pointer.To(msgraph.ConditionalAccessAuthenticationTypePrimaryAndSecondaryAuthentication) + signInFrequency.FrequencyInterval = pointer.To(msgraph.ConditionalAccessFrequencyIntervalTimeBased) } - if authenticationType, ok := config["sign_in_frequency_authentication_type"]; ok { + if authenticationType, ok := config["sign_in_frequency_authentication_type"]; ok && authenticationType.(string) != "" { signInFrequency.AuthenticationType = pointer.To(authenticationType.(string)) } - if interval, ok := config["sign_in_frequency_interval"]; ok { + if interval, ok := config["sign_in_frequency_interval"]; ok && interval.(string) != "" { signInFrequency.FrequencyInterval = pointer.To(interval.(string)) } // API returns 400 error if signInFrequency is set with all default/zero values - if pointer.From(signInFrequency.IsEnabled) || pointer.From(signInFrequency.FrequencyInterval) != msgraph.ConditionalAccessFrequencyIntervalTimeBased || - pointer.From(signInFrequency.AuthenticationType) != msgraph.ConditionalAccessAuthenticationTypePrimaryAndSecondaryAuthentication { + if (signInFrequency.IsEnabled != nil && *signInFrequency.IsEnabled) || + (signInFrequency.FrequencyInterval != nil && *signInFrequency.FrequencyInterval != msgraph.ConditionalAccessFrequencyIntervalTimeBased) || + (signInFrequency.AuthenticationType != nil && *signInFrequency.AuthenticationType != msgraph.ConditionalAccessAuthenticationTypePrimaryAndSecondaryAuthentication) { result.SignInFrequency = &signInFrequency } diff --git a/vendor/github.com/manicminer/hamilton/msgraph/models.go b/vendor/github.com/manicminer/hamilton/msgraph/models.go index 630f4b1e49..66d89bf9e4 100644 --- a/vendor/github.com/manicminer/hamilton/msgraph/models.go +++ b/vendor/github.com/manicminer/hamilton/msgraph/models.go @@ -703,11 +703,11 @@ type ConditionalAccessPolicy struct { } type ConditionalAccessSessionControls struct { - ApplicationEnforcedRestrictions *ApplicationEnforcedRestrictionsSessionControl `json:"applicationEnforcedRestrictions,omitempty"` - CloudAppSecurity *CloudAppSecurityControl `json:"cloudAppSecurity,omitempty"` + ApplicationEnforcedRestrictions *ApplicationEnforcedRestrictionsSessionControl `json:"applicationEnforcedRestrictions"` + CloudAppSecurity *CloudAppSecurityControl `json:"cloudAppSecurity"` DisableResilienceDefaults *bool `json:"disableResilienceDefaults,omitempty"` - PersistentBrowser *PersistentBrowserSessionControl `json:"persistentBrowser,omitempty"` - SignInFrequency *SignInFrequencySessionControl `json:"signInFrequency,omitempty"` + PersistentBrowser *PersistentBrowserSessionControl `json:"persistentBrowser"` + SignInFrequency *SignInFrequencySessionControl `json:"signInFrequency"` } type ConditionalAccessUsers struct { diff --git a/vendor/modules.txt b/vendor/modules.txt index bd507408d1..fb108cc0f6 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -209,7 +209,7 @@ github.com/hashicorp/terraform-svchost # github.com/hashicorp/yamux v0.1.1 ## explicit; go 1.15 github.com/hashicorp/yamux -# github.com/manicminer/hamilton v0.67.0 +# github.com/manicminer/hamilton v0.68.0 ## explicit; go 1.21 github.com/manicminer/hamilton/errors github.com/manicminer/hamilton/internal/utils