Merge pull request #1481 from hashicorp/sdk-migration/07-conditionala…

…ccess

SDK Migration 07: migrate `conditionalaccess` to go-azure-sdk

internal/services/conditionalaccess/client/client.go

@@ -4,24 +4,39 @@
 package client
 
 import (
+	"github.com/hashicorp/go-azure-sdk/microsoft-graph/identity/stable/conditionalaccessnamedlocation"
+	"github.com/hashicorp/go-azure-sdk/microsoft-graph/identity/stable/conditionalaccesspolicy"
 	"github.com/hashicorp/terraform-provider-azuread/internal/common"
-	"github.com/manicminer/hamilton/msgraph"
 )
 
+// CAUTION!
+// The Conditional Access API has compatibility issues between API versions. If you create a policy using the Beta API,
+// or even if you update an existing policy using the Beta API that was originally created with the Stable API, that
+// policy will be irrevocably mutated and can no longer be updated, or even _read_ using the Stable API.
+// For this reason, we are bound to using the Stable API here, as to use the Beta API, even to update a single property
+// for a Conditional Access Policy, will break that policy for users. The only way to go back to the Stable API after
+// breaking a policy in this way, is to delete and recreate it, which is wholly undesirable for a critical security resource.
+
 type Client struct {
-	NamedLocationsClient *msgraph.NamedLocationsClient
-	PoliciesClient       *msgraph.ConditionalAccessPoliciesClient
+	PolicyClient        *conditionalaccesspolicy.ConditionalAccessPolicyClient
+	NamedLocationClient *conditionalaccessnamedlocation.ConditionalAccessNamedLocationClient
 }
 
-func NewClient(o *common.ClientOptions) *Client {
-	namedLocationsClient := msgraph.NewNamedLocationsClient()
-	o.ConfigureClient(&namedLocationsClient.BaseClient)
+func NewClient(o *common.ClientOptions) (*Client, error) {
+	policyClient, err := conditionalaccesspolicy.NewConditionalAccessPolicyClientWithBaseURI(o.Environment.MicrosoftGraph)
+	if err != nil {
+		return nil, err
+	}
+	o.Configure(policyClient.Client)
 
-	policiesClient := msgraph.NewConditionalAccessPoliciesClient()
-	o.ConfigureClient(&policiesClient.BaseClient)
+	namedLocationClient, err := conditionalaccessnamedlocation.NewConditionalAccessNamedLocationClientWithBaseURI(o.Environment.MicrosoftGraph)
+	if err != nil {
+		return nil, err
+	}
+	o.Configure(namedLocationClient.Client)
 
 	return &Client{
-		NamedLocationsClient: namedLocationsClient,
-		PoliciesClient:       policiesClient,
-	}
+		PolicyClient:        policyClient,
+		NamedLocationClient: namedLocationClient,
+	}, nil
 }

internal/services/conditionalaccess/conditional_access_policy_resource_test.go

@@ -6,15 +6,16 @@ package conditionalaccess_test
 import (
 	"context"
 	"fmt"
-	"net/http"
 	"testing"
 
 	"github.com/hashicorp/go-azure-helpers/lang/pointer"
-	"github.com/hashicorp/go-azure-sdk/sdk/odata"
+	"github.com/hashicorp/go-azure-helpers/lang/response"
+	"github.com/hashicorp/go-azure-sdk/microsoft-graph/common-types/stable"
+	"github.com/hashicorp/go-azure-sdk/microsoft-graph/identity/stable/conditionalaccesspolicy"
 	"github.com/hashicorp/terraform-provider-azuread/internal/acceptance"
 	"github.com/hashicorp/terraform-provider-azuread/internal/acceptance/check"
 	"github.com/hashicorp/terraform-provider-azuread/internal/clients"
-	"github.com/hashicorp/terraform-provider-azuread/internal/tf/pluginsdk"
+	"github.com/hashicorp/terraform-provider-azuread/internal/helpers/tf/pluginsdk"
 )
 
 type ConditionalAccessPolicyResource struct{}
@@ -332,23 +333,17 @@ 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
-	}()
+	id := stable.NewIdentityConditionalAccessPolicyID(state.ID)
 
-	var id *string
-
-	app, status, err := clients.ConditionalAccess.PoliciesClient.Get(ctx, state.ID, odata.Query{})
+	resp, err := clients.ConditionalAccess.PolicyClient.GetConditionalAccessPolicy(ctx, id, conditionalaccesspolicy.DefaultGetConditionalAccessPolicyOperationOptions())
 	if err != nil {
-		if status == http.StatusNotFound {
-			return nil, fmt.Errorf("Conditional Access Policy with ID %q does not exist", state.ID)
+		if response.WasNotFound(resp.HttpResponse) {
+			return pointer.To(false), nil
 		}
-		return nil, fmt.Errorf("failed to retrieve Conditional Access Policy with ID %q: %+v", state.ID, err)
+		return nil, fmt.Errorf("failed to retrieve %s: %v", id, err)
 	}
-	id = app.ID
 
-	return pointer.To(id != nil && *id == state.ID), nil
+	return pointer.To(true), nil
 }
 
 func (ConditionalAccessPolicyResource) basic(data acceptance.TestData) string {
@@ -511,6 +506,8 @@ resource "azuread_conditional_access_policy" "test" {
     cloud_app_security_policy                 = "monitorOnly"
     persistent_browser_mode                   = "never"
     sign_in_frequency                         = 10
+    sign_in_frequency_authentication_type     = "primaryAndSecondaryAuthentication"
+    sign_in_frequency_interval                = "timeBased"
     sign_in_frequency_period                  = "hours"
   }
 }
@@ -550,10 +547,6 @@ resource "azuread_conditional_access_policy" "test" {
     operator          = "OR"
     built_in_controls = ["block"]
   }
-
-  session_controls {
-    application_enforced_restrictions_enabled = false
-  }
 }
 `, data.RandomInteger)
 }