Merge pull request #1888 from okta/tgoodsell-tempus_1852

Tgoodsell tempus 1852

examples/resources/okta_app_oauth/federation_broker_off.tf

@@ -3,12 +3,11 @@ resource "okta_group" "group" {
 }
 
 resource "okta_user" "user" {
-  admin_roles = ["APP_ADMIN", "USER_ADMIN"]
-  first_name  = "TestAcc"
-  last_name   = "blah"
-  login       = "test-acc-replace_with_uuid@example.com"
-  email       = "test-acc-replace_with_uuid@example.com"
-  status      = "ACTIVE"
+  first_name = "TestAcc"
+  last_name  = "blah"
+  login      = "test-acc-replace_with_uuid@example.com"
+  email      = "test-acc-replace_with_uuid@example.com"
+  status     = "ACTIVE"
 }
 
 resource "okta_app_oauth" "test" {
@@ -21,11 +20,4 @@ resource "okta_app_oauth" "test" {
   response_types            = ["code", "token", "id_token"]
   consent_method            = "TRUSTED"
   implicit_assignment       = false
-
-  users {
-    id       = okta_user.user.id
-    username = okta_user.user.email
-  }
-
-  groups = [okta_group.group.id]
 }

examples/resources/okta_app_oauth/groups_and_users.tf

@@ -3,12 +3,11 @@ resource "okta_group" "group" {
 }
 
 resource "okta_user" "user" {
-  admin_roles = ["APP_ADMIN", "USER_ADMIN"]
-  first_name  = "TestAcc"
-  last_name   = "blah"
-  login       = "testAcc-replace_with_uuid@example.com"
-  email       = "testAcc-replace_with_uuid@example.com"
-  status      = "ACTIVE"
+  first_name = "TestAcc"
+  last_name  = "blah"
+  login      = "testAcc-replace_with_uuid@example.com"
+  email      = "testAcc-replace_with_uuid@example.com"
+  status     = "ACTIVE"
 }
 
 resource "okta_app_oauth" "test" {
@@ -19,11 +18,4 @@ resource "okta_app_oauth" "test" {
   post_logout_redirect_uris = ["http://d.com/post"]
   login_uri                 = "http://test.com"
   response_types            = ["code", "token", "id_token"]
-
-  users {
-    id       = okta_user.user.id
-    username = okta_user.user.email
-  }
-
-  groups = [okta_group.group.id]
 }

examples/resources/okta_app_oauth/omit_secret_off.tf

@@ -0,0 +1,9 @@
+resource "okta_app_oauth" "test" {
+  label                      = "testAcc_replace_with_uuid"
+  type                       = "web"
+  grant_types                = ["authorization_code"]
+  redirect_uris              = ["http://d.com/"]
+  response_types             = ["code"]
+  token_endpoint_auth_method = "client_secret_basic"
+  consent_method             = "TRUSTED"
+}

examples/resources/okta_app_oauth/omit_secret_on.tf

@@ -0,0 +1,10 @@
+resource "okta_app_oauth" "test" {
+  label                      = "testAcc_replace_with_uuid"
+  type                       = "web"
+  grant_types                = ["authorization_code"]
+  redirect_uris              = ["http://d.com/"]
+  response_types             = ["code"]
+  token_endpoint_auth_method = "client_secret_basic"
+  consent_method             = "TRUSTED"
+  omit_secret                = true
+}

okta/resource_okta_app_oauth.go

@@ -137,20 +137,20 @@ func resourceAppOAuth() *schema.Resource {
 				Type:     schema.TypeBool,
 				Optional: true,
 				// No ForceNew to avoid recreating when going from false => true
-				Description: "This tells the provider not to persist the application's secret to state. If this is ever changes from true => false your app will be recreated.",
+				Description: "This tells the provider not manage the client_secret value in state. When this is false (the default), it will cause the auto-generated client_secret to be persisted in the client_secret attribute in state. This also means that every time an update to this app is run, this value is also set on the API. If this changes from false => true, the `client_secret` is dropped from state and the secret at the time of the apply is what remains. If this is ever changes from true => false your app will be recreated, due to the need to regenerate a secret we can store in state.",
 				Default:     false,
 			},
 			"client_secret": {
 				Type:        schema.TypeString,
 				Computed:    true,
 				Sensitive:   true,
-				Description: "OAuth client secret key. This will be in plain text in your statefile unless you set omit_secret above.",
+				Description: "OAuth client secret value, this is output only. This will be in plain text in your statefile unless you set omit_secret above.",
 			},
 			"client_basic_secret": {
 				Type:        schema.TypeString,
 				Optional:    true,
 				Sensitive:   true,
-				Description: "OAuth client secret key, this can be set when token_endpoint_auth_method is client_secret_basic.",
+				Description: "The user provided OAuth client secret key value, this can be set when token_endpoint_auth_method is client_secret_basic. This does nothing when `omit_secret is set to true.",
 			},
 			"token_endpoint_auth_method": {
 				Type:        schema.TypeString,
@@ -414,10 +414,14 @@ func resourceAppOAuthCreate(ctx context.Context, d *schema.ResourceData, m inter
 	app := buildAppOAuth(d, true)
 	activate := d.Get("status").(string) == statusActive
 	params := &query.Params{Activate: &activate}
-	_, _, err := client.Application.CreateApplication(ctx, app, params)
+	appResp, _, err := client.Application.CreateApplication(ctx, app, params)
 	if err != nil {
 		return diag.Errorf("failed to create OAuth application: %v", err)
 	}
+	app, err = verifyOidcAppType(appResp)
+	if err != nil {
+		return diag.FromErr(err)
+	}
 	d.SetId(app.Id)
 	if !d.Get("omit_secret").(bool) {
 		_ = d.Set("client_secret", app.Credentials.OauthClient.ClientSecret)
@@ -529,10 +533,6 @@ func resourceAppOAuthRead(ctx context.Context, d *schema.ResourceData, m interfa
 	} else {
 		_ = d.Set("implicit_assignment", false)
 	}
-	// If this is ever changed omit it.
-	if d.Get("omit_secret").(bool) {
-		_ = d.Set("client_secret", "")
-	}
 
 	c := m.(*Config)
 	if c.IsOAuth20Auth() {
@@ -663,12 +663,32 @@ func resourceAppOAuthUpdate(ctx context.Context, d *schema.ResourceData, m inter
 	if err := validateAppOAuth(d, m); err != nil {
 		return diag.Errorf("failed to create OAuth application: %v", err)
 	}
+
 	app := buildAppOAuth(d, false)
-	_, _, err = client.Application.UpdateApplication(ctx, d.Id(), app)
+	// When omit_secret is true on update, we make sure that do not include
+	// the client secret value in the api call.
+	// This is to ensure that when this is "toggled on", the apply which this occurs also does
+	// not do a final "reset" of the client secret value to the original stored in state.
+	if d.Get("omit_secret").(bool) {
+		app.Credentials.OauthClient.ClientSecret = ""
+	}
+	appResp, _, err := client.Application.UpdateApplication(ctx, d.Id(), app)
 	if err != nil {
 		return diag.Errorf("failed to update OAuth application: %v", err)
 	}
-	if !d.Get("omit_secret").(bool) {
+	app, err = verifyOidcAppType(appResp)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	// The `client_secret` value is always returned from the API on update
+	// regardless if we pass a value or not.
+	// We need to make sure that we set the value in state based upon the `omit_secret` behavior
+	// When `true`: We blank out the secret value
+	// When `false`: We set the secret value to the value returned from the API
+	if d.Get("omit_secret").(bool) {
+		_ = d.Set("client_secret", "")
+	} else {
 		_ = d.Set("client_secret", app.Credentials.OauthClient.ClientSecret)
 	}
 	if d.HasChange("logo") {
@@ -933,3 +953,11 @@ func validateAppOAuth(d *schema.ResourceData, m interface{}) error {
 	}
 	return nil
 }
+
+func verifyOidcAppType(app sdk.App) (*sdk.OpenIdConnectApplication, error) {
+	oidcApp, ok := app.(*sdk.OpenIdConnectApplication)
+	if !ok {
+		return nil, fmt.Errorf("unexpected application response return from Okta: %v", app)
+	}
+	return oidcApp, nil
+}

okta/resource_okta_app_oauth_test.go

@@ -875,3 +875,35 @@ func TestAccResourceOktaAppOauth_config_combinations(t *testing.T) {
 		})
 	}
 }
+
+func TestAccResourceOktaAppOauth_omitSecretSafeEnable(t *testing.T) {
+	mgr := newFixtureManager("resources", appOAuth, t.Name())
+	omit_secret_off := mgr.GetFixtures("omit_secret_off.tf", t)
+	omit_secret_on := mgr.GetFixtures("omit_secret_on.tf", t)
+	resourceName := fmt.Sprintf("%s.test", appOAuth)
+
+	oktaResourceTest(t, resource.TestCase{
+		PreCheck:          testAccPreCheck(t),
+		ErrorCheck:        testAccErrorChecks(t),
+		ProviderFactories: testAccProvidersFactories,
+		CheckDestroy:      checkResourceDestroy(appOAuth, createDoesAppExist(sdk.NewOpenIdConnectApplication())),
+		Steps: []resource.TestStep{
+			{
+				Config: omit_secret_off,
+				Check: resource.ComposeTestCheckFunc(
+					ensureResourceExists(resourceName, createDoesAppExist(sdk.NewOpenIdConnectApplication())),
+					resource.TestCheckResourceAttrSet(resourceName, "client_id"),
+					resource.TestCheckResourceAttrSet(resourceName, "client_secret"),
+				),
+			},
+			{
+				Config: omit_secret_on,
+				Check: resource.ComposeTestCheckFunc(
+					ensureResourceExists(resourceName, createDoesAppExist(sdk.NewOpenIdConnectApplication())),
+					resource.TestCheckResourceAttrSet(resourceName, "client_id"),
+					resource.TestCheckResourceAttr(resourceName, "client_secret", ""),
+				),
+			},
+		},
+	})
+}

website/docs/r/app_oauth.html.markdown

@@ -80,7 +80,7 @@ The following arguments are supported:
 
 - `auto_submit_toolbar` - (Optional) Display auto submit toolbar.
 
-- `client_basic_secret` - (Optional) OAuth client secret key, this can be set when `token_endpoint_auth_method` is `"client_secret_basic"`.
+- `client_basic_secret` - (Optional) The user provided OAuth client secret key value, this can be set when `token_endpoint_auth_method` is `"client_secret_basic"`. This does nothing when `omit_secret` is set to true.
 
 - `client_id` - (Optional) OAuth client ID. If set during creation, app is created with this id. See: https://developer.okta.com/docs/reference/api/apps/#oauth-credential-object
 
@@ -130,7 +130,7 @@ Valid values: `"CUSTOM_URL"`,`"ORG_URL"` or `"DYNAMIC"`. Default is `"ORG_URL"`.
 
 - `logo_uri` - (Optional) URI that references a logo for the client.
 
-- `omit_secret` - (Optional) This tells the provider not to persist the application's secret to state. Your app's `client_secret` will be recreated if this ever changes from true => false.
+- `omit_secret` - (Optional) This tells the provider not manage the `client_secret` value in state. When this is false (the default), it will cause the auto-generated `client_secret` to be persisted in the `client_secret` attribute in state. This also means that every time an update to this app is run, this value is also set on the API. If this changes from false => true, the `client_secret` is dropped from state and the secret at the time of the apply is what remains. If this is ever changes from true => false your app will be recreated, due to the need to regenerate a secret we can store in state.
 
 - `pkce_required` - (Optional) Require Proof Key for Code Exchange (PKCE) for
     additional verification.  If `pkce_required` isn't specified when adding a new
@@ -191,7 +191,7 @@ Valid values: `"CUSTOM_URL"`,`"ORG_URL"` or `"DYNAMIC"`. Default is `"ORG_URL"`.
 
 - `client_id` - The client ID of the application.
 
-- `client_secret` - The client secret of the application. See: https://developer.okta.com/docs/reference/api/apps/#oauth-credential-object
+- `client_secret` - OAuth client secret value, this is output only. This will be in plain text in your statefile unless you set omit_secret above. See: https://developer.okta.com/docs/reference/api/apps/#oauth-credential-object
 
 - `id` - ID of the application.