Skip to content

Commit

Permalink
Merge pull request #1484 from okta/1481_okta_policy_mfa_default_panic
Browse files Browse the repository at this point in the history
clean up integer pointers
  • Loading branch information
monde authored Mar 10, 2023
2 parents 75645c7 + 8b5c4ea commit 78cd3c0
Show file tree
Hide file tree
Showing 53 changed files with 601 additions and 151 deletions.
6 changes: 5 additions & 1 deletion examples/okta_admin_role_custom_assignments/basic.tf
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
variable "hostname" {
type=string
}

locals {
org_url = "https://terraform-provider-okta.oktapreview.com"
org_url = "https://${var.hostname}"
}

resource "okta_admin_role_custom" "test" {
Expand Down
6 changes: 5 additions & 1 deletion examples/okta_admin_role_custom_assignments/updated.tf
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
variable "hostname" {
type=string
}

locals {
org_url = "https://terraform-provider-okta.oktapreview.com"
org_url = "https://${var.hostname}"
}

resource "okta_admin_role_custom" "test" {
Expand Down
10 changes: 9 additions & 1 deletion examples/okta_app_group_assignment/basic.tf
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ resource "okta_group" "test3" {
}

locals {
group_ids = tolist([okta_group.test.id, okta_group.test2.id, okta_group.test3.id])
group_ids = tolist([okta_group.test.id, okta_group.test2.id])
}

resource "okta_app_group_assignment" "test" {
Expand All @@ -34,3 +34,11 @@ resource "okta_app_group_assignment" "test" {
group_id = local.group_ids[count.index]
priority = count.index
}

resource "okta_app_group_assignment" "test3" {
app_id = okta_app_oauth.test.id
group_id = okta_group.test3.id
priority = 3

depends_on = [okta_app_group_assignment.test.0, okta_app_group_assignment.test.1]
}
11 changes: 10 additions & 1 deletion examples/okta_app_group_assignment/updated.tf
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,21 @@ resource "okta_group" "test3" {
}

locals {
group_ids = tolist([okta_group.test.id, okta_group.test2.id, okta_group.test3.id])
group_ids = tolist([okta_group.test.id, okta_group.test2.id])
}

resource "okta_app_group_assignment" "test" {
count = length(local.group_ids)

app_id = okta_app_oauth.test.id
group_id = local.group_ids[count.index]
priority = count.index
}

resource "okta_app_group_assignment" "test3" {
app_id = okta_app_oauth.test.id
group_id = okta_group.test3.id
priority = 4

depends_on = [okta_app_group_assignment.test.0, okta_app_group_assignment.test.1]
}
4 changes: 3 additions & 1 deletion examples/okta_app_shared_credentials/basic.tf
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ resource "okta_app_shared_credentials" "test" {
shared_username = "sharedusername"
accessibility_self_service = true
accessibility_error_redirect_url = "https://example.com/redirect_url_1"
accessibility_login_redirect_url = "https://example.com/redirect_url_2"
// deprecated in OIE
// https://developer.okta.com/docs/reference/api/apps/#accessibility-object
// accessibility_login_redirect_url = "https://example.com/redirect_url_2"
auto_submit_toolbar = true
hide_ios = true
logo = "../examples/okta_app_basic_auth/terraform_icon.png"
Expand Down
4 changes: 3 additions & 1 deletion examples/okta_app_shared_credentials/updated.tf
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ resource "okta_app_shared_credentials" "test" {
shared_username = "sharedusername22"
accessibility_self_service = true
accessibility_error_redirect_url = "https://example.com/redirect_url_1"
accessibility_login_redirect_url = "https://example.com/redirect_url_2"
// deprecated in OIE
// https://developer.okta.com/docs/reference/api/apps/#accessibility-object
// accessibility_login_redirect_url = "https://example.com/redirect_url_2"
auto_submit_toolbar = true
hide_ios = true
logo = "../examples/okta_app_basic_auth/terraform_icon.png"
Expand Down
2 changes: 1 addition & 1 deletion examples/okta_auth_server_scope/basic.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ resource "okta_auth_server_scope" "test" {
consent = "REQUIRED"
description = "test"
name = "test:something"
display_name = "test"
display_name = "test display name"
auth_server_id = okta_auth_server.test.id
}

Expand Down
2 changes: 1 addition & 1 deletion examples/okta_auth_server_scope/basic_updated.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ resource "okta_auth_server_scope" "test" {
consent = "REQUIRED"
description = "test_updated"
name = "test:something"
display_name = "test_updated"
display_name = "test display name updated"
auth_server_id = okta_auth_server.test.id
}

Expand Down
4 changes: 3 additions & 1 deletion okta/data_source_okta_auth_server_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ func dataSourceAuthServerPolicyRead(ctx context.Context, d *schema.ResourceData,
d.SetId(policy.Id)
_ = d.Set("description", policy.Description)
_ = d.Set("assigned_clients", convertStringSliceToSet(policy.Conditions.Clients.Include))
_ = d.Set("priority", policy.Priority)
if policy.PriorityPtr != nil {
_ = d.Set("priority", *policy.PriorityPtr)
}
return nil
}
return diag.Errorf("auth server policy with name '%s' does not exist", name)
Expand Down
4 changes: 3 additions & 1 deletion okta/data_source_okta_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ func dataSourceAuthenticatorRead(ctx context.Context, d *schema.ResourceData, m

if authenticator.Type == "security_key" {
_ = d.Set("provider_hostname", authenticator.Provider.Configuration.HostName)
_ = d.Set("provider_auth_port", authenticator.Provider.Configuration.AuthPort)
if authenticator.Provider.Configuration.AuthPortPtr != nil {
_ = d.Set("provider_auth_port", *authenticator.Provider.Configuration.AuthPortPtr)
}
_ = d.Set("provider_instance_id", authenticator.Provider.Configuration.InstanceId)
}

Expand Down
4 changes: 3 additions & 1 deletion okta/data_source_okta_idp_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ func dataSourceIdpOidcRead(ctx context.Context, d *schema.ResourceData, m interf
_ = d.Set("client_secret", oidc.Protocol.Credentials.Client.ClientSecret)
_ = d.Set("client_id", oidc.Protocol.Credentials.Client.ClientId)
_ = d.Set("issuer_url", oidc.Protocol.Issuer.Url)
_ = d.Set("max_clock_skew", oidc.Policy.MaxClockSkew)
if oidc.Policy.MaxClockSkewPtr != nil {
_ = d.Set("max_clock_skew", *oidc.Policy.MaxClockSkewPtr)
}
_ = d.Set("scopes", convertStringSliceToSet(oidc.Protocol.Scopes))
if oidc.IssuerMode != "" {
_ = d.Set("issuer_mode", oidc.IssuerMode)
Expand Down
4 changes: 3 additions & 1 deletion okta/data_source_okta_idp_social.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ func dataSourceIdpSocialRead(ctx context.Context, d *schema.ResourceData, m inte
d.SetId(idp.Id)
_ = d.Set("name", idp.Name)
_ = d.Set("status", idp.Status)
_ = d.Set("max_clock_skew", idp.Policy.MaxClockSkew)
if idp.Policy.MaxClockSkewPtr != nil {
_ = d.Set("max_clock_skew", *idp.Policy.MaxClockSkewPtr)
}
_ = d.Set("provisioning_action", idp.Policy.Provisioning.Action)
_ = d.Set("deprovisioned_action", idp.Policy.Provisioning.Conditions.Deprovisioned.Action)
_ = d.Set("suspended_action", idp.Policy.Provisioning.Conditions.Suspended.Action)
Expand Down
16 changes: 12 additions & 4 deletions okta/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ func setDefaultPolicy(ctx context.Context, d *schema.ResourceData, m interface{}
_ = d.Set("name", policy.Name)
_ = d.Set("description", policy.Description)
_ = d.Set("status", policy.Status)
_ = d.Set("priority", policy.Priority)
if policy.PriorityPtr != nil {
_ = d.Set("priority", *policy.PriorityPtr)
}
d.SetId(policy.Id)
return policy, nil
}
Expand Down Expand Up @@ -145,7 +147,9 @@ func createPolicy(ctx context.Context, d *schema.ResourceData, m interface{}, te
}
d.SetId(policy.Id)
// Even if priority is invalid we want to add the policy to Terraform to reflect upstream.
err = validatePriority(template.Priority, policy.Priority)
if template.PriorityPtr != nil && policy.PriorityPtr != nil {
err = validatePriority(*template.PriorityPtr, *policy.PriorityPtr)
}
if err != nil {
return err
}
Expand Down Expand Up @@ -213,7 +217,9 @@ func updatePolicy(ctx context.Context, d *schema.ResourceData, m interface{}, te
return err
}
// avoiding perpetual diffs by erroring when the configured priority is not valid and the API defaults it.
err = validatePriority(template.Priority, policy.Priority)
if template.PriorityPtr != nil && policy.PriorityPtr != nil {
err = validatePriority(*template.PriorityPtr, *policy.PriorityPtr)
}
if err != nil {
return err
}
Expand All @@ -239,7 +245,9 @@ func syncPolicyFromUpstream(d *schema.ResourceData, policy *sdk.Policy) error {
_ = d.Set("name", policy.Name)
_ = d.Set("description", policy.Description)
_ = d.Set("status", policy.Status)
_ = d.Set("priority", policy.Priority)
if policy.PriorityPtr != nil {
_ = d.Set("priority", *policy.PriorityPtr)
}
if policy.Conditions != nil &&
policy.Conditions.People != nil &&
policy.Conditions.People.Groups != nil &&
Expand Down
4 changes: 4 additions & 0 deletions okta/provider_sweeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ var testResourcePrefix = "testAcc"
// TestMain overridden main testing function. Package level BeforeAll and AfterAll.
// It also delineates between acceptance tests and unit tests
func TestMain(m *testing.M) {
// TF_VAR_hostname allows the real hostname to be scripted into the config tests
// see examples/okta_resource_set/basic.tf
os.Setenv("TF_VAR_hostname", fmt.Sprintf("%s.%s", os.Getenv("OKTA_ORG_NAME"), os.Getenv("OKTA_BASE_URL")))

// NOTE: Acceptance test sweepers are necessary to prevent dangling
// resources.
// NOTE: Don't run sweepers if we are playing back VCR as nothing should be
Expand Down
21 changes: 21 additions & 0 deletions okta/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,27 @@ func testOIEOnlyAccPreCheck(t *testing.T) func() {
}
}

// testClassicOnlyAccPreCheck is a resource.test PreCheck function that will place a
// logical skip of classic tests when tests are run against an OIE org.
func testClassicOnlyAccPreCheck(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 != "v1" {
t.Skipf("%q test is for classic orgs only", t.Name())
}
}
}

func testAccPreCheck(t *testing.T) func() {
return func() {
err := accPreCheck()
Expand Down
6 changes: 4 additions & 2 deletions okta/resource_okta_app_group_assignment.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@ func resourceAppGroupAssignmentRead(ctx context.Context, d *schema.ResourceData,
return diag.Errorf("failed to marshal app user profile to JSON: %v", err)
}
_ = d.Set("profile", string(jsonProfile))
_ = d.Set("priority", g.Priority)
if g.PriorityPtr != nil {
_ = d.Set("priority", *g.PriorityPtr)
}
return nil
}

Expand Down Expand Up @@ -161,7 +163,7 @@ func buildAppGroupAssignment(d *schema.ResourceData) okta.ApplicationGroupAssign
}
p, ok := d.GetOk("priority")
if ok {
assignment.Priority = int64(p.(int))
assignment.PriorityPtr = int64Ptr(p.(int))
}
return assignment
}
37 changes: 27 additions & 10 deletions okta/resource_okta_app_group_assignment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ import (

func TestAccAppGroupAssignment_crud(t *testing.T) {
ri := acctest.RandInt()
resourceName := fmt.Sprintf("%s.test", appGroupAssignment)
resourceName0 := fmt.Sprintf("%s.test.0", appGroupAssignment)
resourceName1 := fmt.Sprintf("%s.test.1", appGroupAssignment)
resourceName3 := fmt.Sprintf("%s.test3", appGroupAssignment)
mgr := newFixtureManager(appGroupAssignment)
config := mgr.GetFixtures("basic.tf", ri, t)
updatedConfig := mgr.GetFixtures("updated.tf", ri, t)

// TF concurrency will cause this test to flap if the groups and assigned
// priorities aren't executed in proper order
resource.Test(t, resource.TestCase{
PreCheck: testAccPreCheck(t),
ErrorCheck: testAccErrorChecks(t),
Expand All @@ -32,13 +34,18 @@ func TestAccAppGroupAssignment_crud(t *testing.T) {
ensureAppGroupAssignmentExists(resourceName0),
resource.TestCheckResourceAttrSet(resourceName0, "app_id"),
resource.TestCheckResourceAttrSet(resourceName0, "group_id"),
resource.TestCheckResourceAttr(resourceName0, "priority", "0"),
resource.TestCheckResourceAttrSet(resourceName0, "priority"),
resource.TestCheckResourceAttr(resourceName0, "profile", "{}"),
ensureAppGroupAssignmentExists(resourceName1),
resource.TestCheckResourceAttrSet(resourceName1, "app_id"),
resource.TestCheckResourceAttrSet(resourceName1, "group_id"),
resource.TestCheckResourceAttr(resourceName1, "priority", "1"),
resource.TestCheckResourceAttrSet(resourceName1, "priority"),
resource.TestCheckResourceAttr(resourceName1, "profile", "{}"),
ensureAppGroupAssignmentExists(resourceName3),
resource.TestCheckResourceAttrSet(resourceName3, "app_id"),
resource.TestCheckResourceAttrSet(resourceName3, "group_id"),
resource.TestCheckResourceAttr(resourceName3, "priority", "3"),
resource.TestCheckResourceAttr(resourceName3, "profile", "{}"),
),
},
{
Expand All @@ -47,13 +54,18 @@ func TestAccAppGroupAssignment_crud(t *testing.T) {
ensureAppGroupAssignmentExists(resourceName0),
resource.TestCheckResourceAttrSet(resourceName0, "app_id"),
resource.TestCheckResourceAttrSet(resourceName0, "group_id"),
resource.TestCheckResourceAttr(resourceName0, "priority", "0"),
resource.TestCheckResourceAttrSet(resourceName0, "priority"),
resource.TestCheckResourceAttr(resourceName0, "profile", "{}"),
ensureAppGroupAssignmentExists(resourceName1),
resource.TestCheckResourceAttrSet(resourceName1, "app_id"),
resource.TestCheckResourceAttrSet(resourceName1, "group_id"),
resource.TestCheckResourceAttr(resourceName1, "priority", "1"),
resource.TestCheckResourceAttrSet(resourceName0, "priority"),
resource.TestCheckResourceAttr(resourceName1, "profile", "{}"),
ensureAppGroupAssignmentExists(resourceName3),
resource.TestCheckResourceAttrSet(resourceName3, "app_id"),
resource.TestCheckResourceAttrSet(resourceName3, "group_id"),
resource.TestCheckResourceAttr(resourceName3, "priority", "4"),
resource.TestCheckResourceAttr(resourceName3, "profile", "{}"),
),
},
{
Expand All @@ -62,23 +74,28 @@ func TestAccAppGroupAssignment_crud(t *testing.T) {
ensureAppGroupAssignmentExists(resourceName0),
resource.TestCheckResourceAttrSet(resourceName0, "app_id"),
resource.TestCheckResourceAttrSet(resourceName0, "group_id"),
resource.TestCheckResourceAttr(resourceName0, "priority", "0"),
resource.TestCheckResourceAttrSet(resourceName0, "priority"),
resource.TestCheckResourceAttr(resourceName0, "profile", "{}"),
ensureAppGroupAssignmentExists(resourceName1),
resource.TestCheckResourceAttrSet(resourceName1, "app_id"),
resource.TestCheckResourceAttrSet(resourceName1, "group_id"),
resource.TestCheckResourceAttr(resourceName1, "priority", "1"),
resource.TestCheckResourceAttrSet(resourceName1, "priority"),
resource.TestCheckResourceAttr(resourceName1, "profile", "{}"),
ensureAppGroupAssignmentExists(resourceName3),
resource.TestCheckResourceAttrSet(resourceName3, "app_id"),
resource.TestCheckResourceAttrSet(resourceName3, "group_id"),
resource.TestCheckResourceAttr(resourceName3, "priority", "3"),
resource.TestCheckResourceAttr(resourceName3, "profile", "{}"),
),
},
{
ResourceName: resourceName,
ResourceName: resourceName3,
ImportState: true,
ImportStateVerify: true,
ImportStateIdFunc: func(s *terraform.State) (string, error) {
rs, ok := s.RootModule().Resources[resourceName0]
rs, ok := s.RootModule().Resources[resourceName3]
if !ok {
return "", fmt.Errorf("failed to find %s", resourceName)
return "", fmt.Errorf("failed to find %s", resourceName3)
}
appID := rs.Primary.Attributes["app_id"]
groupID := rs.Primary.Attributes["group_id"]
Expand Down
21 changes: 12 additions & 9 deletions okta/resource_okta_app_group_assignments.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ func syncGroups(d *schema.ResourceData, groups []interface{}, assignments []*okt
for _, assignment := range assignments {
if assignment.Id == d.Get(fmt.Sprintf("group.%d.id", i)).(string) {
present = true
if assignment.Priority >= 0 {
groups[i].(map[string]interface{})["priority"] = assignment.Priority
if assignment.PriorityPtr != nil && *assignment.PriorityPtr >= 0 {
groups[i].(map[string]interface{})["priority"] = *assignment.PriorityPtr
}
groups[i].(map[string]interface{})["profile"] = buildProfile(d, i, assignment)
}
Expand Down Expand Up @@ -257,8 +257,8 @@ func containsAssignment(assignments []*okta.ApplicationGroupAssignment, assignme
func containsEqualAssignment(assignments []*okta.ApplicationGroupAssignment, assignment *okta.ApplicationGroupAssignment) bool {
for i := range assignments {
if assignments[i].Id == assignment.Id && reflect.DeepEqual(assignments[i].Profile, assignment.Profile) {
if assignment.Priority >= 0 {
return reflect.DeepEqual(assignments[i].Priority, assignment.Priority)
if assignments[i].PriorityPtr != nil && assignment.PriorityPtr != nil {
return reflect.DeepEqual(*assignments[i].PriorityPtr, *assignment.PriorityPtr)
}
return true
}
Expand All @@ -272,11 +272,14 @@ func groupAssignmentToTFGroup(assignment *okta.ApplicationGroupAssignment) map[s
if string(jsonProfile) != "" {
profile = string(jsonProfile)
}
return map[string]interface{}{
"id": assignment.Id,
"priority": assignment.Priority,
"profile": profile,
result := map[string]interface{}{
"id": assignment.Id,
"profile": profile,
}
if assignment.PriorityPtr != nil {
result["priority"] = *assignment.PriorityPtr
}
return result
}

func tfGroupsToGroupAssignments(d *schema.ResourceData) []*okta.ApplicationGroupAssignment {
Expand All @@ -291,7 +294,7 @@ func tfGroupsToGroupAssignments(d *schema.ResourceData) []*okta.ApplicationGroup
}
priority, ok := d.GetOk(fmt.Sprintf("group.%d.priority", i))
if ok {
a.Priority = int64(priority.(int))
a.PriorityPtr = int64Ptr(priority.(int))
}
assignments[i] = a
}
Expand Down
Loading

0 comments on commit 78cd3c0

Please sign in to comment.