Skip to content

Commit c10e91e

Browse files
authored
fix: normalize chains JSON keys during plan phase (OKTA-1184047) (#2906)
1 parent 126ccba commit c10e91e

4 files changed

Lines changed: 1646 additions & 0 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
resource "okta_app_saml" "test" {
2+
label = "testAcc_replace_with_uuid"
3+
sso_url = "http://google.com"
4+
recipient = "http://here.com"
5+
destination = "http://its-about-the-journey.com"
6+
audience = "http://audience.com"
7+
subject_name_id_template = "$${user.userName}"
8+
subject_name_id_format = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
9+
response_signed = true
10+
signature_algorithm = "RSA_SHA256"
11+
digest_algorithm = "SHA256"
12+
honor_force_authn = false
13+
authn_context_class_ref = "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport"
14+
single_logout_issuer = "https://dunshire.okta.com"
15+
single_logout_url = "https://dunshire.okta.com/logout"
16+
single_logout_certificate = "MIIFnDCCA4QCCQDBSLbiON2T1zANBgkqhkiG9w0BAQsFADCBjzELMAkGA1UEBhMCVVMxDjAMBgNV\r\nBAgMBU1haW5lMRAwDgYDVQQHDAdDYXJpYm91MRcwFQYDVQQKDA5Tbm93bWFrZXJzIEluYzEUMBIG\r\nA1UECwwLRW5naW5lZXJpbmcxDTALBgNVBAMMBFNub3cxIDAeBgkqhkiG9w0BCQEWEWVtYWlsQGV4\r\nYW1wbGUuY29tMB4XDTIwMTIwMzIyNDY0M1oXDTMwMTIwMTIyNDY0M1owgY8xCzAJBgNVBAYTAlVT\r\nMQ4wDAYDVQQIDAVNYWluZTEQMA4GA1UEBwwHQ2FyaWJvdTEXMBUGA1UECgwOU25vd21ha2VycyBJ\r\nbmMxFDASBgNVBAsMC0VuZ2luZWVyaW5nMQ0wCwYDVQQDDARTbm93MSAwHgYJKoZIhvcNAQkBFhFl\r\nbWFpbEBleGFtcGxlLmNvbTCCAiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIBANMmWDjXPdoa\r\nPyzIENqeY9njLan2FqCbQPSestWUUcb6NhDsJVGSQ7XR+ozQA5TaJzbP7cAJUj8vCcbqMZsgOQAu\r\nO/pzYyQEKptLmrGvPn7xkJ1A1xLkp2NY18cpDTeUPueJUoidZ9EJwEuyUZIktzxNNU1pA1lGijiu\r\n2XNxs9d9JR/hm3tCu9Im8qLVB4JtX80YUa6QtlRjWR/H8a373AYCOASdoB3c57fIPD8ATDNy2w/c\r\nfCVGiyKDMFB+GA/WTsZpOP3iohRp8ltAncSuzypcztb2iE+jijtTsiC9kUA2abAJqqpoCJubNShi\r\nVff4822czpziS44MV2guC9wANi8u3Uyl5MKsU95j01jzadKRP5S+2f0K+n8n4UoV9fnqZFyuGAKd\r\nCJi9K6NlSAP+TgPe/JP9FOSuxQOHWJfmdLHdJD+evoKi9E55sr5lRFK0xU1Fj5Ld7zjC0pXPhtJf\r\nsgjEZzD433AsHnRzvRT1KSNCPkLYomznZo5n9rWYgCQ8HcytlQDTesmKE+s05E/VSWNtH84XdDrt\r\nieXwfwhHfaABSu+WjZYxi9CXdFCSvXhsgufUcK4FbYAHl/ga/cJxZc52yFC7Pcq0u9O2BSCjYPdQ\r\nDAHs9dhT1RhwVLM8RmoAzgxyyzau0gxnAlgSBD9FMW6dXqIHIp8yAAg9cRXhYRTNAgMBAAEwDQYJ\r\nKoZIhvcNAQELBQADggIBADofEC1SvG8qa7pmKCjB/E9Sxhk3mvUO9Gq43xzwVb721Ng3VYf4vGU3\r\nwLUwJeLt0wggnj26NJweN5T3q9T8UMxZhHSWvttEU3+S1nArRB0beti716HSlOCDx4wTmBu/D1MG\r\nt/kZYFJw+zuzvAcbYct2pK69AQhD8xAIbQvqADJI7cCK3yRry+aWtppc58P81KYabUlCfFXfhJ9E\r\nP72ffN4jVHpX3lxxYh7FKAdiKbY2FYzjsc7RdgKI1R3iAAZUCGBTvezNzaetGzTUjjl/g1tcVYij\r\nltH9ZOQBPlUMI88lxUxqgRTerpPmAJH00CACx4JFiZrweLM1trZyy06wNDQgLrqHr3EOagBF/O2h\r\nhfTehNdVr6iq3YhKWBo4/+RL0RCzHMh4u86VbDDnDn4Y6HzLuyIAtBFoikoKM6UHTOa0Pqv2bBr5\r\nwbkRkVUxl9yJJw/HmTCdfnsM9dTOJUKzEglnGF2184Gg+qJDZB6fSf0EAO1F6sTqiSswl+uHQZiy\r\nDaZzyU7Gg5seKOZ20zTRaX3Ihj9Zij/ORnrARE7eM/usKMECp+7syUwAUKxDCZkGiUdskmOhhBGL\r\nJtbyK3F2UvoJoLsm3pIcvMak9KwMjSTGJB47ABUP1+w+zGcNk0D5Co3IJ6QekiLfWJyQ+kKsWLKt\r\nzOYQQatrnBagM7MI2/T4\r\n"
17+
18+
attribute_statements {
19+
type = "GROUP"
20+
name = "groups"
21+
filter_type = "REGEX"
22+
filter_value = ".*"
23+
}
24+
}
25+
26+
data "okta_app_signon_policy" "test" {
27+
app_id = okta_app_saml.test.id
28+
}
29+
30+
resource "okta_app_signon_policy_rules" "test_chains_misaligned" {
31+
policy_id = data.okta_app_signon_policy.test.id
32+
33+
rule {
34+
name = "MisalignedKeys-testAcc_replace_with_uuid"
35+
priority = 1
36+
status = "ACTIVE"
37+
access = "ALLOW"
38+
factor_mode = "2FA"
39+
type = "AUTH_METHOD_CHAIN"
40+
chains = [
41+
jsonencode(
42+
{
43+
"authenticationMethods" : [
44+
{
45+
"key" : "google_otp",
46+
"method" : "otp"
47+
},
48+
{
49+
"key" : "okta_verify",
50+
"userVerification" : "OPTIONAL",
51+
"method" : "push"
52+
},
53+
{
54+
"key" : "okta_verify",
55+
"method" : "totp"
56+
},
57+
{
58+
"key" : "okta_verify",
59+
"userVerification" : "OPTIONAL",
60+
"method" : "signed_nonce"
61+
}
62+
],
63+
"reauthenticateIn" : "PT0S",
64+
"next" : [
65+
{
66+
"authenticationMethods" : [
67+
{
68+
"key" : "okta_password",
69+
"method" : "password"
70+
}
71+
],
72+
"reauthenticateIn" : "PT0S"
73+
}
74+
]
75+
})
76+
]
77+
}
78+
}
79+
80+
81+

okta/services/idaas/resource_okta_app_signon_policy_rules.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,48 @@ func (m reauthFrequencyModifier) PlanModifyString(ctx context.Context, req planm
159159
}
160160
}
161161

162+
// ChainsPlanModifier normalizes the JSON key order of each chains element during
163+
// planning so the plan value always matches the post-apply canonical form.
164+
type ChainsPlanModifier struct{}
165+
166+
func (m ChainsPlanModifier) Description(_ context.Context) string {
167+
return "Normalizes JSON key order in chains elements to prevent inconsistent result after apply"
168+
}
169+
170+
func (m ChainsPlanModifier) MarkdownDescription(ctx context.Context) string {
171+
return m.Description(ctx)
172+
}
173+
174+
func (m ChainsPlanModifier) PlanModifyList(ctx context.Context, req planmodifier.ListRequest, resp *planmodifier.ListResponse) {
175+
if req.PlanValue.IsNull() || req.PlanValue.IsUnknown() {
176+
return
177+
}
178+
var chainStrings []string
179+
resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &chainStrings, false)...)
180+
if resp.Diagnostics.HasError() {
181+
return
182+
}
183+
normalized := make([]string, len(chainStrings))
184+
for i, s := range chainStrings {
185+
var raw map[string]interface{}
186+
if err := json.Unmarshal([]byte(s), &raw); err != nil {
187+
resp.Diagnostics.AddAttributeError(
188+
req.Path,
189+
"Invalid chains JSON",
190+
fmt.Sprintf("chains[%d] is not valid JSON: %s", i, err),
191+
)
192+
return
193+
}
194+
b, _ := json.Marshal(raw)
195+
normalized[i] = string(b)
196+
}
197+
listVal, diags := types.ListValueFrom(ctx, types.StringType, normalized)
198+
resp.Diagnostics.Append(diags...)
199+
if !resp.Diagnostics.HasError() {
200+
resp.PlanValue = listVal
201+
}
202+
}
203+
162204
// ruleIndex provides efficient lookups for rules by name and ID.
163205
type ruleIndex struct {
164206
byName map[string]policyRuleModel
@@ -788,6 +830,9 @@ func (r *appSignOnPolicyRulesResource) buildRuleAttributes() map[string]schema.A
788830
Optional: true,
789831
ElementType: types.StringType,
790832
Description: "List of authentication method chain objects as JSON-encoded strings. Use with `type = \"AUTH_METHOD_CHAIN\"` only.",
833+
PlanModifiers: []planmodifier.List{
834+
ChainsPlanModifier{},
835+
},
791836
},
792837
"risk_score": schema.StringAttribute{
793838
Optional: true,

okta/services/idaas/resource_okta_app_signon_policy_rules_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package idaas_test
22

33
import (
4+
"context"
45
"fmt"
56
"testing"
67

8+
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
9+
"github.com/hashicorp/terraform-plugin-framework/types"
710
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
811
"github.com/okta/terraform-provider-okta/okta/acctest"
912
"github.com/okta/terraform-provider-okta/okta/resources"
@@ -227,6 +230,44 @@ func TestAccResourceOktaAppSignOnPolicyRules_chains(t *testing.T) {
227230
})
228231
}
229232

233+
// TestAccResourceOktaAppSignOnPolicyRules_chains_misaligned_keys verifies that
234+
// chains with non-alphabetical JSON key ordering (e.g., userVerification before
235+
// method) are normalized during plan, preventing "Provider produced inconsistent
236+
// result after apply" errors. This regression test covers OKTA-1184047.
237+
func TestAccResourceOktaAppSignOnPolicyRules_chains_misaligned_keys(t *testing.T) {
238+
resourceName := fmt.Sprintf("%s.test_chains_misaligned", resources.OktaIDaaSAppSignOnPolicyRules)
239+
mgr := newFixtureManager("resources", resources.OktaIDaaSAppSignOnPolicyRules, t.Name())
240+
config := mgr.GetFixtures("chains_misaligned_keys.tf", t)
241+
acctest.OktaResourceTest(t, resource.TestCase{
242+
PreCheck: acctest.AccPreCheck(t),
243+
ErrorCheck: testAccErrorChecks(t),
244+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactoriesForTestAcc(t),
245+
CheckDestroy: checkAppSignOnPolicyRuleDestroy,
246+
Steps: []resource.TestStep{
247+
{
248+
Config: config,
249+
Check: resource.ComposeTestCheckFunc(
250+
resource.TestCheckResourceAttrSet(resourceName, "id"),
251+
resource.TestCheckResourceAttrSet(resourceName, "policy_id"),
252+
resource.TestCheckResourceAttr(resourceName, "rule.#", "1"),
253+
resource.TestCheckResourceAttrSet(resourceName, "rule.0.id"),
254+
resource.TestCheckResourceAttr(resourceName, "rule.0.name", fmt.Sprintf("MisalignedKeys-testAcc_%s", mgr.SeedStr())),
255+
resource.TestCheckResourceAttr(resourceName, "rule.0.chains.#", "1"),
256+
// Verify the chain is stored
257+
resource.TestCheckResourceAttrSet(resourceName, "rule.0.chains.0"),
258+
),
259+
},
260+
{
261+
// Idempotency check — this should succeed without "inconsistent result" error.
262+
// Before the fix, this step would fail with:
263+
// "Provider produced inconsistent result after apply"
264+
Config: config,
265+
PlanOnly: true,
266+
},
267+
},
268+
})
269+
}
270+
230271
// TestAccResourceOktaAppSignOnPolicyRules_keep_me_signed_in verifies that the
231272
// keep_me_signed_in (KMSI / "Option to stay signed in") block on the plural
232273
// resource round-trips correctly across multiple rules. The config defines four
@@ -318,3 +359,69 @@ func TestAccResourceOktaAppSignOnPolicyRules_keep_me_signed_in(t *testing.T) {
318359
},
319360
})
320361
}
362+
363+
func TestChainsPlanModifier(t *testing.T) {
364+
modifier := idaas.ChainsPlanModifier{}
365+
366+
tests := []struct {
367+
name string
368+
planValue string
369+
expectedValue string
370+
wantErr bool
371+
}{
372+
{
373+
name: "already alphabetical",
374+
planValue: `{"key":"okta_verify","method":"push","userVerification":"OPTIONAL"}`,
375+
expectedValue: `{"key":"okta_verify","method":"push","userVerification":"OPTIONAL"}`,
376+
},
377+
{
378+
name: "userVerification before method",
379+
planValue: `{"key":"okta_verify","userVerification":"OPTIONAL","method":"push"}`,
380+
expectedValue: `{"key":"okta_verify","method":"push","userVerification":"OPTIONAL"}`,
381+
},
382+
{
383+
name: "complex nested non-alphabetical keys",
384+
planValue: `{"authenticationMethods":[{"userVerification":"OPTIONAL","method":"push","key":"okta_verify"}],"reauthenticateIn":"PT0S","next":[]}`,
385+
expectedValue: `{"authenticationMethods":[{"key":"okta_verify","method":"push","userVerification":"OPTIONAL"}],"next":[],"reauthenticateIn":"PT0S"}`,
386+
},
387+
{
388+
name: "invalid JSON returns error diagnostic",
389+
planValue: `not-valid-json`,
390+
wantErr: true,
391+
},
392+
}
393+
394+
for _, tt := range tests {
395+
t.Run(tt.name, func(t *testing.T) {
396+
ctx := context.Background()
397+
listValue, diags := types.ListValueFrom(ctx, types.StringType, []string{tt.planValue})
398+
if diags.HasError() {
399+
t.Fatalf("failed to create list value: %v", diags)
400+
}
401+
402+
req := planmodifier.ListRequest{PlanValue: listValue}
403+
resp := &planmodifier.ListResponse{PlanValue: listValue}
404+
modifier.PlanModifyList(ctx, req, resp)
405+
406+
if tt.wantErr {
407+
if !resp.Diagnostics.HasError() {
408+
t.Fatal("expected error diagnostic, got none")
409+
}
410+
return
411+
}
412+
413+
if resp.Diagnostics.HasError() {
414+
t.Fatalf("unexpected error: %v", resp.Diagnostics)
415+
}
416+
417+
var result []string
418+
resp.PlanValue.ElementsAs(ctx, &result, false)
419+
if len(result) != 1 {
420+
t.Fatalf("expected 1 element, got %d", len(result))
421+
}
422+
if result[0] != tt.expectedValue {
423+
t.Errorf("got %s, want %s", result[0], tt.expectedValue)
424+
}
425+
})
426+
}
427+
}

0 commit comments

Comments
 (0)