Skip to content

Commit 9c14e94

Browse files
authored
fix: prevent nil pointer dereference in idp_saml, idp_social, and idp_oidc when accountLink.filter.groups is null (#2843)
1 parent a933136 commit 9c14e94

6 files changed

Lines changed: 1068 additions & 3 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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+
}
15+
16+
resource "okta_idp_saml_key" "test" {
17+
x5c = [okta_app_saml.test.certificate]
18+
}
19+
20+
# account_link_action = AUTO without account_link_group_include exercises the code path fixed
21+
# in OKTA-1131393: on some Okta cells the API returns accountLink.filter.groups = null while
22+
# filter itself is non-null, which caused a nil pointer dereference in the Read function.
23+
resource "okta_idp_saml" "test" {
24+
name = "testAcc_replace_with_uuid"
25+
acs_type = "INSTANCE"
26+
sso_url = "https://idp.example.com"
27+
sso_destination = "https://idp.example.com"
28+
sso_binding = "HTTP-POST"
29+
username_template = "idpuser.email"
30+
kid = okta_idp_saml_key.test.id
31+
issuer = "https://idp.example.com"
32+
request_signature_scope = "REQUEST"
33+
response_signature_scope = "ANY"
34+
account_link_action = "AUTO"
35+
}

okta/services/idaas/resource_okta_idp_oidc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ func resourceIdpRead(ctx context.Context, d *schema.ResourceData, meta interface
280280
}
281281
if idp.Policy.AccountLink != nil {
282282
_ = d.Set("account_link_action", idp.Policy.AccountLink.Action)
283-
if idp.Policy.AccountLink.Filter != nil {
283+
if idp.Policy.AccountLink.Filter != nil && idp.Policy.AccountLink.Filter.Groups != nil {
284284
setMap["account_link_group_include"] = utils.ConvertStringSliceToSet(idp.Policy.AccountLink.Filter.Groups.Include)
285285
}
286286
}

okta/services/idaas/resource_okta_idp_saml.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func resourceIdpSamlRead(ctx context.Context, d *schema.ResourceData, meta inter
228228
}
229229
if idp.Policy.AccountLink != nil {
230230
_ = d.Set("account_link_action", idp.Policy.AccountLink.Action)
231-
if idp.Policy.AccountLink.Filter != nil {
231+
if idp.Policy.AccountLink.Filter != nil && idp.Policy.AccountLink.Filter.Groups != nil {
232232
setMap["account_link_group_include"] = utils.ConvertStringSliceToSet(idp.Policy.AccountLink.Filter.Groups.Include)
233233
}
234234
}

okta/services/idaas/resource_okta_idp_saml_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,39 @@ func TestAccResourceOktaIdpSaml_crud(t *testing.T) {
118118
})
119119
}
120120

121+
// TestAccResourceOktaIdpSaml_account_link_auto exercises the Read path when account_link_action
122+
// is set to AUTO without account_link_group_include. Regression test for OKTA-1131393 where
123+
// the provider panicked with a nil pointer dereference when the Okta API returned
124+
// accountLink.filter.groups = null (filter non-null, groups null inside it).
125+
func TestAccResourceOktaIdpSaml_account_link_auto(t *testing.T) {
126+
mgr := newFixtureManager("resources", resources.OktaIDaaSIdpSaml, t.Name())
127+
config := mgr.GetFixtures("account_link_auto.tf", t)
128+
resourceName := fmt.Sprintf("%s.test", resources.OktaIDaaSIdpSaml)
129+
130+
acctest.OktaResourceTest(t, resource.TestCase{
131+
PreCheck: acctest.AccPreCheck(t),
132+
ErrorCheck: testAccErrorChecks(t),
133+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactoriesForTestAcc(t),
134+
CheckDestroy: checkResourceDestroy(resources.OktaIDaaSIdpSaml, createDoesIdpExist),
135+
Steps: []resource.TestStep{
136+
{
137+
Config: config,
138+
Check: resource.ComposeTestCheckFunc(
139+
resource.TestCheckResourceAttr(resourceName, "name", acctest.BuildResourceName(mgr.Seed)),
140+
resource.TestCheckResourceAttr(resourceName, "account_link_action", "AUTO"),
141+
resource.TestCheckNoResourceAttr(resourceName, "account_link_group_include"),
142+
),
143+
},
144+
{
145+
ResourceName: resourceName,
146+
ImportState: true,
147+
ImportStateVerify: true,
148+
ImportStateVerifyIgnore: []string{"issuer_mode"},
149+
},
150+
},
151+
})
152+
}
153+
121154
// TestAccResourceOktaIdpSaml_minimal_example was used to prove that the PR
122155
// https://github.com/okta/terraform-provider-okta/pull/1355 was correct. This
123156
// test would fail if the org was missing the mappings api feature. And pass if

okta/services/idaas/resource_okta_idp_social.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func resourceIdpSocialRead(ctx context.Context, d *schema.ResourceData, meta int
232232
if idp.Policy.AccountLink != nil {
233233
_ = d.Set("account_link_action", idp.Policy.AccountLink.Action)
234234

235-
if idp.Policy.AccountLink.Filter != nil {
235+
if idp.Policy.AccountLink.Filter != nil && idp.Policy.AccountLink.Filter.Groups != nil {
236236
setMap["account_link_group_include"] = utils.ConvertStringSliceToSet(idp.Policy.AccountLink.Filter.Groups.Include)
237237
}
238238
}

test/fixtures/vcr/idaas/TestAccResourceOktaIdpSaml_account_link_auto/oie-00.yaml

Lines changed: 997 additions & 0 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)