Bringing in PR 1210

okta/config.go

@@ -58,15 +58,38 @@ type (
 )
 
 func (c *Config) loadAndValidate(ctx context.Context) error {
+	c.logger = providerLogger(c)
+
+	client, err := oktaSDKClient(c)
+	if err != nil {
+		return err
+	}
+
+	if c.apiToken != "" {
+		if _, _, err := client.User.GetUser(ctx, "me"); err != nil {
+			return err
+		}
+	}
+	c.oktaClient = client
+	c.supplementClient = &sdk.APISupplement{
+		RequestExecutor: client.CloneRequestExecutor(),
+	}
+	return nil
+}
+
+func providerLogger(c *Config) hclog.Logger {
 	logLevel := hclog.Level(c.logLevel)
 	if os.Getenv("TF_LOG") != "" {
 		logLevel = hclog.LevelFromString(os.Getenv("TF_LOG"))
 	}
 
-	c.logger = hclog.New(&hclog.LoggerOptions{
+	return hclog.New(&hclog.LoggerOptions{
 		Level:      logLevel,
 		TimeFormat: "2006/01/02 03:04:05",
 	})
+}
+
+func oktaSDKClient(c *Config) (client *okta.Client, err error) {
 	var httpClient *http.Client
 	if c.backoff {
 		retryableClient := retryablehttp.NewClient()
@@ -90,11 +113,10 @@ func (c *Config) loadAndValidate(ctx context.Context) error {
 		c.logger.Info(fmt.Sprintf("running with experimental max_api_capacity configuration at %d%%", c.maxAPICapacity))
 		apiMutex, err := apimutex.NewAPIMutex(c.maxAPICapacity)
 		if err != nil {
-			return err
+			return nil, err
 		}
 		httpClient.Transport = transport.NewGovernedTransport(httpClient.Transport, apiMutex, c.logger)
 	}
-
 	var orgUrl string
 	var disableHTTPS bool
 	if c.httpProxy != "" {
@@ -137,24 +159,14 @@ func (c *Config) loadAndValidate(ctx context.Context) error {
 	if disableHTTPS {
 		setters = append(setters, okta.WithTestingDisableHttpsCheck(true))
 	}
-	_, client, err := okta.NewClient(
+
+	c.client = httpClient
+
+	_, client, err = okta.NewClient(
 		context.Background(),
 		setters...,
 	)
-	if err != nil {
-		return err
-	}
-	if c.apiToken != "" {
-		if _, _, err := client.User.GetUser(ctx, "me"); err != nil {
-			return err
-		}
-	}
-	c.oktaClient = client
-	c.supplementClient = &sdk.APISupplement{
-		RequestExecutor: client.CloneRequestExecutor(),
-	}
-	c.client = httpClient
-	return nil
+	return
 }
 
 func errHandler(resp *http.Response, err error, numTries int) (*http.Response, error) {

okta/provider_test.go

@@ -15,11 +15,14 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 	"github.com/okta/okta-sdk-golang/v2/okta"
+	"github.com/okta/terraform-provider-okta/sdk"
 )
 
 var (
 	testAccProvidersFactories map[string]func() (*schema.Provider, error)
 	testAccProvider           *schema.Provider
+	testSDKClient             *okta.Client
+	testSupplementClient      *sdk.APISupplement
 )
 
 func init() {
@@ -29,6 +32,24 @@ func init() {
 			return testAccProvider, nil
 		},
 	}
+
+	// We need to be able to query the SDK with an Okta SDK golang client that
+	// is outside of the client that terraform provider creates. This is because
+	// tests may need to query the okta API for status and the Terraform SDK
+	// doesn't expose the provider's meta data where we store the provider's
+	// config until after tests have completed.
+	if os.Getenv("TF_ACC") != "" {
+		// only set up for acceptance tests
+		config := &Config{
+			orgName: os.Getenv("OKTA_ORG_NAME"),
+			domain:  os.Getenv("OKTA_BASE_URL"),
+		}
+		config.logger = providerLogger(config)
+		testSDKClient, _ = oktaSDKClient(config)
+		testSupplementClient = &sdk.APISupplement{
+			RequestExecutor: testSDKClient.CloneRequestExecutor(),
+		}
+	}
 }
 
 func TestProvider(t *testing.T) {
@@ -63,6 +84,27 @@ func oktaConfig() (*Config, error) {
 	return config, nil
 }
 
+// testOIEOnlyAccPreCheck is a resource.test PreCheck function that will place a
+// logical skip of OIE tests when tests are run against a classic org.
+func testOIEOnlyAccPreCheck(t *testing.T) func() {
+	return func() {
+		err := accPreCheck()
+		if err != nil {
+			t.Fatalf("%v", err)
+		}
+
+		org, _, err := testSupplementClient.GetWellKnownOktaOrganization(context.TODO())
+		if err != nil {
+			t.Fatalf("%v", err)
+		}
+
+		// v1 == Classic, idx == OIE
+		if org.Pipeline != "idx" {
+			t.Skipf("%q test is for OIE orgs only", t.Name())
+		}
+	}
+}
+
 func testAccPreCheck(t *testing.T) func() {
 	return func() {
 		err := accPreCheck()

okta/resource_okta_policy_mfa.go

@@ -90,7 +90,7 @@ func buildSettings(d *schema.ResourceData) *sdk.PolicySettings {
 	if d.Get("is_oie") == true {
 		authenticators := []*sdk.PolicyAuthenticator{}
 
-		for _, key := range remove(sdk.AuthenticatorProviders, sdk.OktaPasswordFactor) {
+		for _, key := range sdk.AuthenticatorProviders {
 			rawFactor := d.Get(key).(map[string]interface{})
 			enroll := rawFactor["enroll"]
 			if enroll == nil {
@@ -155,7 +155,7 @@ func syncSettings(d *schema.ResourceData, settings *sdk.PolicySettings) {
 	_ = d.Set("is_oie", settings.Type == "AUTHENTICATORS")
 
 	if settings.Type == "AUTHENTICATORS" {
-		for _, key := range remove(sdk.AuthenticatorProviders, sdk.OktaPasswordFactor) {
+		for _, key := range sdk.AuthenticatorProviders {
 			syncAuthenticator(d, key, settings.Authenticators)
 		}
 	} else {
@@ -189,14 +189,9 @@ func syncFactor(d *schema.ResourceData, k string, f *sdk.PolicyFactor) {
 func syncAuthenticator(d *schema.ResourceData, k string, authenticators []*sdk.PolicyAuthenticator) {
 	for _, authenticator := range authenticators {
 		if authenticator.Key == k {
-			// Skip OktaPassword as this should never be returned for MFA policies using authenticator.
-			// Enrollment policy changes for OIE for password
-			// https://help.okta.com/okta_help.htm?type=oie&id=ext-about-mfa-enrol-policies
-			if k != sdk.OktaPasswordFactor {
-				_ = d.Set(k, map[string]interface{}{
-					"enroll": authenticator.Enroll.Self,
-				})
-			}
+			_ = d.Set(k, map[string]interface{}{
+				"enroll": authenticator.Enroll.Self,
+			})
 			return
 		}
 	}

okta/resource_okta_policy_mfa_test.go

@@ -48,3 +48,58 @@ func TestAccOktaMfaPolicy_crud(t *testing.T) {
 		},
 	})
 }
+
+// TestAccOktaMfaPolicy_PR_1210 deals with testing
+// https://github.com/okta/terraform-provider-okta/pull/1210
+func TestAccOktaMfaPolicy_PR_1210(t *testing.T) {
+	ri := acctest.RandInt()
+	mgr := newFixtureManager(policyMfa)
+	config := `
+data "okta_group" "all" {
+  name = "Everyone"
+}
+resource "okta_policy_mfa" "test" {
+  name        = "testAcc_replace_with_uuid"
+  status = "ACTIVE"
+  description = "Terraform Acceptance Test MFA Policy"
+  priority = 1
+  is_oie  = true
+
+  okta_password = {
+    enroll = "REQUIRED"
+  }
+
+  okta_email = {
+    enroll = "NOT_ALLOWED"
+  }
+
+  fido_webauthn = {
+    enroll = "REQUIRED"
+  }
+
+  groups_included = [data.okta_group.all.id]
+}
+	`
+	resourceName := fmt.Sprintf("%s.test", policyMfa)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:          testOIEOnlyAccPreCheck(t),
+		ErrorCheck:        testAccErrorChecks(t),
+		ProviderFactories: testAccProvidersFactories,
+		CheckDestroy:      createPolicyCheckDestroy(policyMfa),
+		Steps: []resource.TestStep{
+			{
+				Config: mgr.ConfigReplace(config, ri),
+				Check: resource.ComposeTestCheckFunc(
+					ensurePolicyExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "name", buildResourceName(ri)),
+					resource.TestCheckResourceAttr(resourceName, "status", statusActive),
+					resource.TestCheckResourceAttr(resourceName, "description", "Terraform Acceptance Test MFA Policy"),
+					resource.TestCheckResourceAttr(resourceName, "okta_password.enroll", "REQUIRED"),
+					resource.TestCheckResourceAttr(resourceName, "okta_email.enroll", "NOT_ALLOWED"),
+					resource.TestCheckResourceAttr(resourceName, "fido_webauthn.enroll", "REQUIRED"),
+				),
+			},
+		},
+	})
+}