Patch: Client default_organization import issue on terraform generate

internal/auth0/client/expand.go

@@ -1,7 +1,7 @@
 package client
 
 import (
-	"encoding/json"
+	"fmt"
 
 	"github.com/auth0/go-auth0"
 	"github.com/auth0/go-auth0/management"
@@ -11,12 +11,7 @@ import (
 	"github.com/auth0/terraform-provider-auth0/internal/value"
 )
 
-type nilableClient struct {
-	management.Client
-	DefaultOrganization *management.ClientDefaultOrganization `json:"default_organization"`
-}
-
-func expandClient(data *schema.ResourceData) (interface{}, error) {
+func expandClient(data *schema.ResourceData) (*management.Client, error) {
 	config := data.GetRawConfig()
 
 	client := &management.Client{
@@ -53,6 +48,7 @@ func expandClient(data *schema.ResourceData) (interface{}, error) {
 		Addons:                             expandClientAddons(data),
 		NativeSocialLogin:                  expandClientNativeSocialLogin(data),
 		Mobile:                             expandClientMobile(data),
+		DefaultOrganization:                expandDefaultOrganization(data),
 	}
 
 	if data.IsNewResource() && client.IsTokenEndpointIPHeaderTrusted != nil {
@@ -68,48 +64,75 @@ func expandClient(data *schema.ResourceData) (interface{}, error) {
 		}
 	}
 
-	defaultOrg := config.GetAttr("default_organization")
+	defaultConfig := data.GetRawConfig().GetAttr("default_organization")
 
-	if !defaultOrg.IsNull() && defaultOrg.LengthInt() > 0 {
-		if defaultOrg.AsValueSlice()[0].GetAttr("disable").True() {
-			clientJSON, err := json.Marshal(client)
-			if err != nil {
-				return nil, err
-			}
+	for _, item := range defaultConfig.AsValueSlice() {
+		disable := item.GetAttr("disable")
+		organizationID := item.GetAttr("organization_id")
+		flows := item.GetAttr("flows")
 
-			nilableClient := nilableClient{}
-			if err := json.Unmarshal(clientJSON, &nilableClient); err != nil {
-				return nil, err
+		if !disable.IsNull() && disable.True() {
+			if (!flows.IsNull() && flows.LengthInt() > 0) || (!organizationID.IsNull() && organizationID.AsString() != "") {
+				return nil, fmt.Errorf("cannot set both disable and either flows/organization_id")
 			}
-			nilableClient.DefaultOrganization = nil
-			return nilableClient, nil
 		}
-		client.DefaultOrganization = expandDefaultOrganization(data)
 	}
 
 	return client, nil
 }
 
 func expandDefaultOrganization(data *schema.ResourceData) *management.ClientDefaultOrganization {
-	var defaultOrg management.ClientDefaultOrganization
-
-	defaultOrganizationConfig := data.GetRawConfig().GetAttr("default_organization")
-	if defaultOrganizationConfig.IsNull() {
+	if !data.IsNewResource() && !data.HasChange("default_organization") {
 		return nil
 	}
+	var defaultOrg management.ClientDefaultOrganization
 
-	defaultOrganizationConfig.ForEachElement(func(_ cty.Value, config cty.Value) (stop bool) {
-		defaultOrg.Flows = value.Strings(config.GetAttr("flows"))
-		defaultOrg.OrganizationID = value.String(config.GetAttr("organization_id"))
+	config := data.GetRawConfig().GetAttr("default_organization")
+	if config.IsNull() || config.ForEachElement(func(_ cty.Value, cfg cty.Value) (stop bool) {
+		disable := cfg.GetAttr("disable")
+		if !disable.IsNull() && disable.True() {
+			stop = true
+		} else {
+			defaultOrg.Flows = value.Strings(cfg.GetAttr("flows"))
+			defaultOrg.OrganizationID = value.String(cfg.GetAttr("organization_id"))
+		}
 		return stop
-	})
+	}) {
+		// We forced an early return because it was disabled.
+		return nil
+	}
 	if defaultOrg == (management.ClientDefaultOrganization{}) {
 		return nil
 	}
 
 	return &defaultOrg
 }
 
+func isDefaultOrgNull(data *schema.ResourceData) bool {
+	if !data.IsNewResource() && !data.HasChange("default_organization") {
+		return false
+	}
+	empty := true
+	config := data.GetRawConfig()
+	defaultOrgConfig := config.GetAttr("default_organization")
+	if defaultOrgConfig.IsNull() || defaultOrgConfig.ForEachElement(func(_ cty.Value, cfg cty.Value) (stop bool) {
+		disable := cfg.GetAttr("disable")
+		flows := cfg.GetAttr("flows")
+		organizationID := cfg.GetAttr("organization_id")
+
+		if (!disable.IsNull() && disable.True()) || (flows.IsNull() && organizationID.IsNull()) {
+			stop = true
+		} else {
+			empty = false
+		}
+		return stop
+	}) {
+		// We forced an early return because it was disabled.
+		return true
+	}
+	return empty
+}
+
 func expandOIDCBackchannelLogout(data *schema.ResourceData) *management.OIDCBackchannelLogout {
 	raw := data.GetRawConfig().GetAttr("oidc_backchannel_logout_urls")
 

internal/auth0/client/resource.go

@@ -2,8 +2,8 @@ package client
 
 import (
 	"context"
-	"encoding/json"
 	"net/http"
+	"time"
 
 	"github.com/auth0/go-auth0/management"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -1277,30 +1277,28 @@ func NewResource() *schema.Resource {
 				Type:        schema.TypeList,
 				Optional:    true,
 				MaxItems:    1,
-				Description: "Configure and associate an organization with the Client",
 				Computed:    true,
+				Description: "Configure and associate an organization with the Client",
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"flows": {
-							Type:          schema.TypeList,
-							Optional:      true,
-							Elem:          &schema.Schema{Type: schema.TypeString},
-							RequiredWith:  []string{"default_organization.0.organization_id"},
-							ConflictsWith: []string{"default_organization.0.disable"},
-							Description:   "Definition of the flow that needs to be configured. Eg. client_credentials",
+							Type:        schema.TypeList,
+							Optional:    true,
+							Computed:    true,
+							Elem:        &schema.Schema{Type: schema.TypeString},
+							Description: "Definition of the flow that needs to be configured. Eg. client_credentials",
 						},
 						"organization_id": {
-							Type:          schema.TypeString,
-							Optional:      true,
-							RequiredWith:  []string{"default_organization.0.flows"},
-							ConflictsWith: []string{"default_organization.0.disable"},
-							Description:   "The unique identifier of the organization",
+							Type:        schema.TypeString,
+							Optional:    true,
+							Computed:    true,
+							Description: "The unique identifier of the organization",
 						},
 						"disable": {
-							Type:          schema.TypeBool,
-							Optional:      true,
-							ConflictsWith: []string{"default_organization.0.organization_id", "default_organization.0.flows"},
-							Description:   "If set, the `default_organization` will be removed.",
+							Type:        schema.TypeBool,
+							Optional:    true,
+							Computed:    true,
+							Description: "If set, the `default_organization` will be removed.",
 						},
 					},
 				},
@@ -1313,27 +1311,15 @@ func createClient(ctx context.Context, data *schema.ResourceData, meta interface
 	api := meta.(*config.Config).GetAPI()
 	client, err := expandClient(data)
 
-	if err != nil {
-		return diag.FromErr(err)
-	}
-	err = api.Request(ctx, http.MethodPost, api.URI("clients"), client)
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
-	baseClient := management.Client{}
-	clientJSON, err := json.Marshal(client)
-
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	if err = json.Unmarshal(clientJSON, &baseClient); err != nil {
+	if err := api.Client.Create(ctx, client); err != nil {
 		return diag.FromErr(err)
 	}
 
-	data.SetId(baseClient.GetClientID())
-
+	data.SetId(client.GetClientID())
 	return readClient(ctx, data, meta)
 }
 
@@ -1357,20 +1343,8 @@ func updateClient(ctx context.Context, data *schema.ResourceData, meta interface
 		return diag.FromErr(err)
 	}
 
-	baseClient := management.Client{}
-	clientJSON, err := json.Marshal(client)
-
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
-	if err = json.Unmarshal(clientJSON, &baseClient); err != nil {
-		return diag.FromErr(err)
-	}
-
-	if clientHasChange(&baseClient) {
-		if baseClient.GetAddons() != nil {
-			// In case we are switching addons, we need to be able to clear out the previous config.
+	if clientHasChange(client) {
+		if client.GetAddons() != nil {
 			resetAddons := &management.Client{
 				Addons: &management.ClientAddons{},
 			}
@@ -1379,12 +1353,20 @@ func updateClient(ctx context.Context, data *schema.ResourceData, meta interface
 			}
 		}
 
-		err = api.Request(ctx, http.MethodPatch, api.URI("clients", data.Id()), client)
-		if err != nil {
-			return diag.FromErr(err)
+		if err := api.Client.Update(ctx, data.Id(), client); err != nil {
+			return diag.FromErr(internalError.HandleAPIError(data, err))
 		}
-	}
 
+		time.Sleep(200 * time.Millisecond)
+
+		if isDefaultOrgNull(data) {
+			if err := api.Request(ctx, http.MethodPatch, api.URI("clients", data.Id()), map[string]interface{}{
+				"default_organization": nil,
+			}); err != nil {
+				return diag.FromErr(err)
+			}
+		}
+	}
 	return readClient(ctx, data, meta)
 }
 

internal/auth0/client/resource_test.go

@@ -2388,14 +2388,6 @@ resource "auth0_client" "my_client" {
 func TestAccClientWithDefaultOrganization(t *testing.T) {
 	acctest.Test(t, resource.TestCase{
 		Steps: []resource.TestStep{
-			{
-				Config:      acctest.ParseTestName(testAccUpdateClientDefaultOrganizationFlowsOnly, t.Name()),
-				ExpectError: regexp.MustCompile("Error: Missing required argument"),
-			},
-			{
-				Config:      acctest.ParseTestName(testAccUpdateClientDefaultOrganizationOrgIDOnly, t.Name()),
-				ExpectError: regexp.MustCompile("Error: Missing required argument"),
-			},
 			{
 				Config: acctest.ParseTestName(testAccCreateClientWithDefaultOrganization, t.Name()),
 				Check: resource.ComposeTestCheckFunc(
@@ -2424,6 +2416,14 @@ func TestAccClientWithDefaultOrganization(t *testing.T) {
 					resource.TestCheckResourceAttr("auth0_client.my_client", "default_organization.0.organization_id", ""),
 				),
 			},
+			{
+				Config:      acctest.ParseTestName(testAccUpdateClientDefaultOrganizationFlowsOnly, t.Name()),
+				ExpectError: regexp.MustCompile("400 Bad Request"),
+			},
+			{
+				Config:      acctest.ParseTestName(testAccUpdateClientDefaultOrganizationOrgIDOnly, t.Name()),
+				ExpectError: regexp.MustCompile("400 Bad Request"),
+			},
 		},
 	})
 }