From e7cc719964d01dbf9672d2a2b7ee93600b86b185 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Thu, 29 Jul 2021 09:03:08 -0400 Subject: [PATCH 01/12] fix: support importing okta_auth_backend resource --- go.mod | 1 + vault/resource_okta_auth_backend.go | 55 ++++++++++++- vault/resource_okta_auth_backend_test.go | 100 ++++++++++++++++++----- 3 files changed, 132 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index c59d860ec..91c8ae925 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( github.com/hashicorp/go-hclog v0.16.1 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-retryablehttp v0.6.8 // indirect + github.com/hashicorp/go-secure-stdlib/parseutil v0.1.1 github.com/hashicorp/terraform-plugin-sdk v1.9.0 github.com/hashicorp/vault v1.2.0 github.com/hashicorp/vault/api v1.1.2-0.20210719211531-6b31c12b0af2 diff --git a/vault/resource_okta_auth_backend.go b/vault/resource_okta_auth_backend.go index fa078cc05..9f16f8b34 100644 --- a/vault/resource_okta_auth_backend.go +++ b/vault/resource_okta_auth_backend.go @@ -4,8 +4,10 @@ import ( "errors" "fmt" "log" + "strconv" "strings" + "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/terraform-plugin-sdk/helper/hashcode" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-provider-vault/util" @@ -20,9 +22,11 @@ func oktaAuthBackendResource() *schema.Resource { Delete: oktaAuthBackendDelete, Read: oktaAuthBackendRead, Update: oktaAuthBackendUpdate, - + Exists: oktaAuthBackendExists, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, Schema: map[string]*schema.Schema{ - "path": { Type: schema.TypeString, Optional: true, @@ -79,7 +83,9 @@ func oktaAuthBackendResource() *schema.Resource { Type: schema.TypeString, Required: false, Optional: true, + Default: "0", Description: "Duration after which authentication will be expired", + StateFunc: parseTTLForState, }, "max_ttl": { @@ -87,6 +93,8 @@ func oktaAuthBackendResource() *schema.Resource { Required: false, Optional: true, Description: "Maximum duration after which authentication will be expired", + Default: "0", + StateFunc: parseTTLForState, }, "group": { @@ -204,6 +212,14 @@ func oktaAuthBackendResource() *schema.Resource { } } +func parseTTLForState(i interface{}) string { + d, err := parseutil.ParseDurationSecond(i) + if err != nil { + return i.(string) + } + return strconv.Itoa(int(d.Seconds())) +} + func oktaAuthBackendWrite(d *schema.ResourceData, meta interface{}) error { client := meta.(*api.Client) @@ -245,6 +261,10 @@ func oktaAuthBackendDelete(d *schema.ResourceData, meta interface{}) error { return nil } +func oktaAuthBackendExists(d *schema.ResourceData, meta interface{}) (bool, error) { + return isOktaAuthBackendPresent(meta.(*api.Client), d.Id()) +} + func oktaAuthBackendRead(d *schema.ResourceData, meta interface{}) error { client := meta.(*api.Client) @@ -263,12 +283,15 @@ func oktaAuthBackendRead(d *schema.ResourceData, meta interface{}) error { return nil } + d.Set("path", path) + mount, err := authMountInfoGet(client, path) if err != nil { return fmt.Errorf("error reading okta oth mount from '%q': %s", path, err) } d.Set("accessor", mount.Accessor) + d.Set("description", mount.Description) log.Printf("[DEBUG] Reading groups for mount %s from Vault", path) groups, err := oktaReadAllGroups(client, path) @@ -288,8 +311,36 @@ func oktaAuthBackendRead(d *schema.ResourceData, meta interface{}) error { return err } + if err := oktaReadAuthConfig(client, path, d); err != nil { + return err + } + return nil +} +func oktaReadAuthConfig(client *api.Client, path string, d *schema.ResourceData) error { + log.Printf("[DEBUG] Reading auth config for mount %s from Vault", path) + config, err := client.Logical().Read(oktaConfigEndpoint(path)) + if err != nil { + return err + } + + // map schema config to okta auth params. + fieldMap := map[string]string{ + "organization": "organization", + "ttl": "token_ttl", + "max_ttl": "token_max_ttl", + } + for k, v := range fieldMap { + if v, ok := config.Data[v]; ok { + s, err := parseutil.ParseString(v) + if err != nil { + return err + } + d.Set(k, s) + } + } + return nil } func oktaAuthBackendUpdate(d *schema.ResourceData, meta interface{}) error { diff --git a/vault/resource_okta_auth_backend_test.go b/vault/resource_okta_auth_backend_test.go index 685da0efe..c0348049e 100644 --- a/vault/resource_okta_auth_backend_test.go +++ b/vault/resource_okta_auth_backend_test.go @@ -3,11 +3,9 @@ package vault import ( "encoding/json" "fmt" - "strconv" "testing" "time" - "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/hashicorp/terraform-provider-vault/util" @@ -15,31 +13,89 @@ import ( ) func TestAccOktaAuthBackend(t *testing.T) { - path := "okta-" + strconv.Itoa(acctest.RandInt()) organization := "example" - resource.Test(t, resource.TestCase{ - Providers: testProviders, - PreCheck: func() { testAccPreCheck(t) }, - CheckDestroy: testAccOktaAuthBackend_Destroyed(path), - Steps: []resource.TestStep{ - { - Config: testAccOktaAuthConfig_basic(path, organization), - Check: resource.ComposeTestCheckFunc( - testAccOktaAuthBackend_InitialCheck, - testAccOktaAuthBackend_GroupsCheck(path, "dummy", []string{"one", "two", "default"}), - testAccOktaAuthBackend_UsersCheck(path, "foo", []string{"dummy"}, []string{}), - ), + tests := []struct { + name string + preCheck func() + steps func(path string) []resource.TestStep + }{ + { + name: "default", + preCheck: func() { util.TestAccPreCheck(t) }, + steps: func(path string) []resource.TestStep { + return []resource.TestStep{ + { + Config: testAccOktaAuthConfig_basic(path, organization), + Check: resource.ComposeTestCheckFunc( + testAccOktaAuthBackend_InitialCheck, + testAccOktaAuthBackend_GroupsCheck(path, "dummy", []string{"one", "two", "default"}), + testAccOktaAuthBackend_UsersCheck(path, "foo", []string{"dummy"}, []string{}), + ), + }, + { + Config: testAccOktaAuthConfig_updated(path, organization), + Check: resource.ComposeTestCheckFunc( + testAccOktaAuthBackend_GroupsCheck(path, "example", []string{"three", "four", "default"}), + testAccOktaAuthBackend_UsersCheck(path, "bar", []string{"example"}, []string{}), + ), + }, + } }, - { - Config: testAccOktaAuthConfig_updated(path, organization), - Check: resource.ComposeTestCheckFunc( - testAccOktaAuthBackend_GroupsCheck(path, "example", []string{"three", "four", "default"}), - testAccOktaAuthBackend_UsersCheck(path, "bar", []string{"example"}, []string{}), - ), + }, + { + name: "import", + preCheck: func() { util.TestAccPreCheck(t) }, + steps: func(path string) []resource.TestStep { + return []resource.TestStep{ + { + Config: testAccOktaAuthConfig_basic(path, organization), + Check: resource.ComposeTestCheckFunc( + testAccOktaAuthBackend_InitialCheck, + testAccOktaAuthBackend_GroupsCheck(path, "dummy", []string{"one", "two", "default"}), + testAccOktaAuthBackend_UsersCheck(path, "foo", []string{"dummy"}, []string{}), + ), + }, + { + ResourceName: "vault_okta_auth_backend.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "token", + }, + }, + { + Config: testAccOktaAuthConfig_updated(path, organization), + Check: resource.ComposeTestCheckFunc( + testAccOktaAuthBackend_GroupsCheck(path, "example", []string{"three", "four", "default"}), + testAccOktaAuthBackend_UsersCheck(path, "bar", []string{"example"}, []string{}), + ), + }, + { + ResourceName: "vault_okta_auth_backend.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "token", + }, + }, + } }, }, - }) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := resource.PrefixedUniqueId("okta-" + tt.name + "-") + resource.Test(t, resource.TestCase{ + Providers: testProviders, + PreCheck: tt.preCheck, + CheckDestroy: testAccOktaAuthBackend_Destroyed(path), + Steps: tt.steps(path), + }) + }, + ) + } } func testAccOktaAuthConfig_basic(path string, organization string) string { From b6e27af53e3d769a52aa1ee3fbb1202c8049062b Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Thu, 29 Jul 2021 09:41:40 -0400 Subject: [PATCH 02/12] fix: document import feature --- website/docs/r/okta_auth_backend.html.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/website/docs/r/okta_auth_backend.html.md b/website/docs/r/okta_auth_backend.html.md index f7eb7e905..b62080146 100644 --- a/website/docs/r/okta_auth_backend.html.md +++ b/website/docs/r/okta_auth_backend.html.md @@ -79,3 +79,11 @@ If this is not supplied only locally configured groups will be enabled. In addition to all arguments above, the following attributes are exported: * `accessor` - The mount accessor related to the auth mount. It is useful for integration with [Identity Secrets Engine](https://www.vaultproject.io/docs/secrets/identity/index.html). + +## Import + +Okta authentication backends can be imported using its `path`, e.g. + +``` +$ terraform import vault_okta_auth_backend.example okta +``` From 8c19db6d726cbbf8156ba2105bd84ef0dd293f6a Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Thu, 29 Jul 2021 11:04:47 -0400 Subject: [PATCH 03/12] fix: ensure error checking on all Set() calls --- vault/resource_okta_auth_backend.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/vault/resource_okta_auth_backend.go b/vault/resource_okta_auth_backend.go index 9f16f8b34..810e2a9df 100644 --- a/vault/resource_okta_auth_backend.go +++ b/vault/resource_okta_auth_backend.go @@ -283,15 +283,21 @@ func oktaAuthBackendRead(d *schema.ResourceData, meta interface{}) error { return nil } - d.Set("path", path) + if err := d.Set("path", path); err != nil { + return err + } mount, err := authMountInfoGet(client, path) if err != nil { return fmt.Errorf("error reading okta oth mount from '%q': %s", path, err) } - d.Set("accessor", mount.Accessor) - d.Set("description", mount.Description) + if err := d.Set("accessor", mount.Accessor); err != nil { + return err + } + if err := d.Set("description", mount.Description); err != nil { + return err + } log.Printf("[DEBUG] Reading groups for mount %s from Vault", path) groups, err := oktaReadAllGroups(client, path) @@ -337,7 +343,9 @@ func oktaReadAuthConfig(client *api.Client, path string, d *schema.ResourceData) if err != nil { return err } - d.Set(k, s) + if err := d.Set(k, s); err != nil { + return err + } } } return nil From 100f898c623a13059c8ab31446faf9f96394fe92 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Thu, 29 Jul 2021 11:59:10 -0400 Subject: [PATCH 04/12] fix: warn when failing to parse the TTL for state. --- vault/resource_okta_auth_backend.go | 1 + 1 file changed, 1 insertion(+) diff --git a/vault/resource_okta_auth_backend.go b/vault/resource_okta_auth_backend.go index 810e2a9df..8c77cb131 100644 --- a/vault/resource_okta_auth_backend.go +++ b/vault/resource_okta_auth_backend.go @@ -215,6 +215,7 @@ func oktaAuthBackendResource() *schema.Resource { func parseTTLForState(i interface{}) string { d, err := parseutil.ParseDurationSecond(i) if err != nil { + log.Printf("[WARN] Failed to parse TTL for state, will use specified value %v, error %s", i, err) return i.(string) } return strconv.Itoa(int(d.Seconds())) From 63a9e23f6ef1cd114cdc98814d551fed0d3ad3d0 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Thu, 29 Jul 2021 12:02:06 -0400 Subject: [PATCH 05/12] fix: ensure that the subtest is passed to preCheck --- vault/resource_okta_auth_backend_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vault/resource_okta_auth_backend_test.go b/vault/resource_okta_auth_backend_test.go index c0348049e..7b0a89599 100644 --- a/vault/resource_okta_auth_backend_test.go +++ b/vault/resource_okta_auth_backend_test.go @@ -17,12 +17,12 @@ func TestAccOktaAuthBackend(t *testing.T) { tests := []struct { name string - preCheck func() + preCheck func(t *testing.T) steps func(path string) []resource.TestStep }{ { name: "default", - preCheck: func() { util.TestAccPreCheck(t) }, + preCheck: util.TestAccPreCheck, steps: func(path string) []resource.TestStep { return []resource.TestStep{ { @@ -45,7 +45,7 @@ func TestAccOktaAuthBackend(t *testing.T) { }, { name: "import", - preCheck: func() { util.TestAccPreCheck(t) }, + preCheck: util.TestAccPreCheck, steps: func(path string) []resource.TestStep { return []resource.TestStep{ { @@ -88,8 +88,8 @@ func TestAccOktaAuthBackend(t *testing.T) { t.Run(tt.name, func(t *testing.T) { path := resource.PrefixedUniqueId("okta-" + tt.name + "-") resource.Test(t, resource.TestCase{ + PreCheck: func() { tt.preCheck(t) }, Providers: testProviders, - PreCheck: tt.preCheck, CheckDestroy: testAccOktaAuthBackend_Destroyed(path), Steps: tt.steps(path), }) From 641159fbcad86ace9bcca6d7d0816a20f46a09fe Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Thu, 29 Jul 2021 17:23:50 -0400 Subject: [PATCH 06/12] fix: add validators for ttl and max_ttl --- vault/resource_okta_auth_backend.go | 52 ++++++++++++++++-------- vault/resource_okta_auth_backend_test.go | 51 +++++++++++++++++++++++ 2 files changed, 87 insertions(+), 16 deletions(-) diff --git a/vault/resource_okta_auth_backend.go b/vault/resource_okta_auth_backend.go index 8c77cb131..545bbb65a 100644 --- a/vault/resource_okta_auth_backend.go +++ b/vault/resource_okta_auth_backend.go @@ -80,21 +80,23 @@ func oktaAuthBackendResource() *schema.Resource { }, "ttl": { - Type: schema.TypeString, - Required: false, - Optional: true, - Default: "0", - Description: "Duration after which authentication will be expired", - StateFunc: parseTTLForState, + Type: schema.TypeString, + Required: false, + Optional: true, + Default: "0", + Description: "Duration after which authentication will be expired", + ValidateFunc: validateOktaTTL, + StateFunc: normalizeOktaTTL, }, "max_ttl": { - Type: schema.TypeString, - Required: false, - Optional: true, - Description: "Maximum duration after which authentication will be expired", - Default: "0", - StateFunc: parseTTLForState, + Type: schema.TypeString, + Required: false, + Optional: true, + Description: "Maximum duration after which authentication will be expired", + Default: "0", + ValidateFunc: validateOktaTTL, + StateFunc: normalizeOktaTTL, }, "group": { @@ -212,13 +214,31 @@ func oktaAuthBackendResource() *schema.Resource { } } -func parseTTLForState(i interface{}) string { - d, err := parseutil.ParseDurationSecond(i) +func normalizeOktaTTL(i interface{}) string { + s, err := parseDurationSeconds(i) if err != nil { - log.Printf("[WARN] Failed to parse TTL for state, will use specified value %v, error %s", i, err) + // validateOktaTTL should prevent ever getting here return i.(string) } - return strconv.Itoa(int(d.Seconds())) + return s +} + +func validateOktaTTL(i interface{}, k string) ([]string, []error) { + var errors []error + s, err := parseDurationSeconds(i) + if err != nil { + errors = append(errors, fmt.Errorf("invalid value for %q, could not parse %q", k, i)) + } + return []string{s}, errors +} + +func parseDurationSeconds(i interface{}) (string, error) { + d, err := parseutil.ParseDurationSecond(i) + if err != nil { + log.Printf("[ERROR] Could not parse %v to seconds, error: %s", i, err) + return "", err + } + return strconv.Itoa(int(d.Seconds())), nil } func oktaAuthBackendWrite(d *schema.ResourceData, meta interface{}) error { diff --git a/vault/resource_okta_auth_backend_test.go b/vault/resource_okta_auth_backend_test.go index 7b0a89599..1f89945ab 100644 --- a/vault/resource_okta_auth_backend_test.go +++ b/vault/resource_okta_auth_backend_test.go @@ -3,6 +3,7 @@ package vault import ( "encoding/json" "fmt" + "regexp" "testing" "time" @@ -82,6 +83,30 @@ func TestAccOktaAuthBackend(t *testing.T) { } }, }, + { + name: "invalid_ttl", + preCheck: util.TestAccPreCheck, + steps: func(path string) []resource.TestStep { + return []resource.TestStep{ + { + Config: testAccOktaAuthConfig_invalid_ttl(path, organization), + ExpectError: regexp.MustCompile(`invalid value for "ttl", could not parse "invalid_ttl"$`), + }, + } + }, + }, + { + name: "invalid_max_ttl", + preCheck: util.TestAccPreCheck, + steps: func(path string) []resource.TestStep { + return []resource.TestStep{ + { + Config: testAccOktaAuthConfig_invalid_max_ttl(path, organization), + ExpectError: regexp.MustCompile(`invalid value for "max_ttl", could not parse "invalid_max_ttl"$`), + }, + } + }, + }, } for _, tt := range tests { @@ -137,6 +162,32 @@ resource "vault_okta_auth_backend" "test" { `, path, organization) } +func testAccOktaAuthConfig_invalid_ttl(path string, organization string) string { + return fmt.Sprintf(` +resource "vault_okta_auth_backend" "test" { + description = "Testing the Terraform okta auth backend" + path = "%s" + organization = "%s" + token = "this must be kept secret" + ttl = "invalid_ttl" + max_ttl = "1h" +} +`, path, organization) +} + +func testAccOktaAuthConfig_invalid_max_ttl(path string, organization string) string { + return fmt.Sprintf(` +resource "vault_okta_auth_backend" "test" { + description = "Testing the Terraform okta auth backend" + path = "%s" + organization = "%s" + token = "this must be kept secret" + ttl = "1h" + max_ttl = "invalid_max_ttl" +} +`, path, organization) +} + func testAccOktaAuthBackend_InitialCheck(s *terraform.State) error { resourceState := s.Modules[0].Resources["vault_okta_auth_backend.test"] if resourceState == nil { From 8caeb9403cc06b84c17d5bae7da664276afe1426 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 30 Jul 2021 08:24:24 -0400 Subject: [PATCH 07/12] fix: move all subtests to single test cases --- vault/resource_okta_auth_backend_test.go | 192 +++++++++++------------ 1 file changed, 94 insertions(+), 98 deletions(-) diff --git a/vault/resource_okta_auth_backend_test.go b/vault/resource_okta_auth_backend_test.go index 1f89945ab..028e59282 100644 --- a/vault/resource_okta_auth_backend_test.go +++ b/vault/resource_okta_auth_backend_test.go @@ -13,114 +13,110 @@ import ( "github.com/hashicorp/vault/api" ) -func TestAccOktaAuthBackend(t *testing.T) { +func TestAccOktaAuthBackend_basic(t *testing.T) { organization := "example" - - tests := []struct { - name string - preCheck func(t *testing.T) - steps func(path string) []resource.TestStep - }{ - { - name: "default", - preCheck: util.TestAccPreCheck, - steps: func(path string) []resource.TestStep { - return []resource.TestStep{ - { - Config: testAccOktaAuthConfig_basic(path, organization), - Check: resource.ComposeTestCheckFunc( - testAccOktaAuthBackend_InitialCheck, - testAccOktaAuthBackend_GroupsCheck(path, "dummy", []string{"one", "two", "default"}), - testAccOktaAuthBackend_UsersCheck(path, "foo", []string{"dummy"}, []string{}), - ), - }, - { - Config: testAccOktaAuthConfig_updated(path, organization), - Check: resource.ComposeTestCheckFunc( - testAccOktaAuthBackend_GroupsCheck(path, "example", []string{"three", "four", "default"}), - testAccOktaAuthBackend_UsersCheck(path, "bar", []string{"example"}, []string{}), - ), - }, - } + path := resource.PrefixedUniqueId("okta-basic-") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { util.TestAccPreCheck(t) }, + Providers: testProviders, + CheckDestroy: testAccOktaAuthBackend_Destroyed(path), + Steps: []resource.TestStep{ + { + Config: testAccOktaAuthConfig_basic(path, organization), + Check: resource.ComposeTestCheckFunc( + testAccOktaAuthBackend_InitialCheck, + testAccOktaAuthBackend_GroupsCheck(path, "dummy", []string{"one", "two", "default"}), + testAccOktaAuthBackend_UsersCheck(path, "foo", []string{"dummy"}, []string{}), + ), }, - }, - { - name: "import", - preCheck: util.TestAccPreCheck, - steps: func(path string) []resource.TestStep { - return []resource.TestStep{ - { - Config: testAccOktaAuthConfig_basic(path, organization), - Check: resource.ComposeTestCheckFunc( - testAccOktaAuthBackend_InitialCheck, - testAccOktaAuthBackend_GroupsCheck(path, "dummy", []string{"one", "two", "default"}), - testAccOktaAuthBackend_UsersCheck(path, "foo", []string{"dummy"}, []string{}), - ), - }, - { - ResourceName: "vault_okta_auth_backend.test", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{ - "token", - }, - }, - { - Config: testAccOktaAuthConfig_updated(path, organization), - Check: resource.ComposeTestCheckFunc( - testAccOktaAuthBackend_GroupsCheck(path, "example", []string{"three", "four", "default"}), - testAccOktaAuthBackend_UsersCheck(path, "bar", []string{"example"}, []string{}), - ), - }, - { - ResourceName: "vault_okta_auth_backend.test", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{ - "token", - }, - }, - } + { + Config: testAccOktaAuthConfig_updated(path, organization), + Check: resource.ComposeTestCheckFunc( + testAccOktaAuthBackend_GroupsCheck(path, "example", []string{"three", "four", "default"}), + testAccOktaAuthBackend_UsersCheck(path, "bar", []string{"example"}, []string{}), + ), }, }, - { - name: "invalid_ttl", - preCheck: util.TestAccPreCheck, - steps: func(path string) []resource.TestStep { - return []resource.TestStep{ - { - Config: testAccOktaAuthConfig_invalid_ttl(path, organization), - ExpectError: regexp.MustCompile(`invalid value for "ttl", could not parse "invalid_ttl"$`), - }, - } + }) +} + +func TestAccOktaAuthBackend_import(t *testing.T) { + organization := "example" + path := resource.PrefixedUniqueId("okta-import-") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { util.TestAccPreCheck(t) }, + Providers: testProviders, + CheckDestroy: testAccOktaAuthBackend_Destroyed(path), + Steps: []resource.TestStep{ + { + Config: testAccOktaAuthConfig_basic(path, organization), + Check: resource.ComposeTestCheckFunc( + testAccOktaAuthBackend_InitialCheck, + testAccOktaAuthBackend_GroupsCheck(path, "dummy", []string{"one", "two", "default"}), + testAccOktaAuthBackend_UsersCheck(path, "foo", []string{"dummy"}, []string{}), + ), + }, + { + ResourceName: "vault_okta_auth_backend.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "token", + }, + }, + { + Config: testAccOktaAuthConfig_updated(path, organization), + Check: resource.ComposeTestCheckFunc( + testAccOktaAuthBackend_GroupsCheck(path, "example", []string{"three", "four", "default"}), + testAccOktaAuthBackend_UsersCheck(path, "bar", []string{"example"}, []string{}), + ), + }, + { + ResourceName: "vault_okta_auth_backend.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "token", + }, }, }, - { - name: "invalid_max_ttl", - preCheck: util.TestAccPreCheck, - steps: func(path string) []resource.TestStep { - return []resource.TestStep{ - { - Config: testAccOktaAuthConfig_invalid_max_ttl(path, organization), - ExpectError: regexp.MustCompile(`invalid value for "max_ttl", could not parse "invalid_max_ttl"$`), - }, - } + }) +} + +func TestAccOktaAuthBackend_invalid_ttl(t *testing.T) { + organization := "example" + path := resource.PrefixedUniqueId("okta-invalid-ttl-") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { util.TestAccPreCheck(t) }, + Providers: testProviders, + CheckDestroy: testAccOktaAuthBackend_Destroyed(path), + Steps: []resource.TestStep{ + { + Config: testAccOktaAuthConfig_invalid_ttl(path, organization), + ExpectError: regexp.MustCompile(`invalid value for "ttl", could not parse "invalid_ttl"$`), }, }, - } + }) +} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - path := resource.PrefixedUniqueId("okta-" + tt.name + "-") - resource.Test(t, resource.TestCase{ - PreCheck: func() { tt.preCheck(t) }, - Providers: testProviders, - CheckDestroy: testAccOktaAuthBackend_Destroyed(path), - Steps: tt.steps(path), - }) +func TestAccOktaAuthBackend_invalid_max_ttl(t *testing.T) { + organization := "example" + path := resource.PrefixedUniqueId("okta-invalid_max_ttl-") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { util.TestAccPreCheck(t) }, + Providers: testProviders, + CheckDestroy: testAccOktaAuthBackend_Destroyed(path), + Steps: []resource.TestStep{ + { + Config: testAccOktaAuthConfig_invalid_max_ttl(path, organization), + ExpectError: regexp.MustCompile(`invalid value for "max_ttl", could not parse "invalid_max_ttl"$`), + }, }, - ) - } + }) } func testAccOktaAuthConfig_basic(path string, organization string) string { From 185d8311c0c99e9fcff665c128f310271bdddaae Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 30 Jul 2021 11:46:32 -0400 Subject: [PATCH 08/12] fix: make ttl and max_ttl computed values. --- vault/resource_okta_auth_backend.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vault/resource_okta_auth_backend.go b/vault/resource_okta_auth_backend.go index 545bbb65a..f9a82013c 100644 --- a/vault/resource_okta_auth_backend.go +++ b/vault/resource_okta_auth_backend.go @@ -81,9 +81,9 @@ func oktaAuthBackendResource() *schema.Resource { "ttl": { Type: schema.TypeString, + Computed: true, Required: false, Optional: true, - Default: "0", Description: "Duration after which authentication will be expired", ValidateFunc: validateOktaTTL, StateFunc: normalizeOktaTTL, @@ -91,10 +91,10 @@ func oktaAuthBackendResource() *schema.Resource { "max_ttl": { Type: schema.TypeString, + Computed: true, Required: false, Optional: true, Description: "Maximum duration after which authentication will be expired", - Default: "0", ValidateFunc: validateOktaTTL, StateFunc: normalizeOktaTTL, }, From c6c84b6cd9e91459d7f2e272375d6fa1ba367437 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 30 Jul 2021 16:45:35 -0400 Subject: [PATCH 09/12] fix: update validateOktaTTL --- repro/gh-734/main.tf | 18 ++++++ repro/gh-734/module/main.tf | 62 +++++++++++++++++++++ repro/gh-734/module/variables.tf | 16 ++++++ repro/gh-796/.terraform.lock.hcl | 20 +++++++ repro/gh-796/main.tf | 86 +++++++++++++++++++++++++++++ vault/resource_okta_auth_backend.go | 4 +- 6 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 repro/gh-734/main.tf create mode 100644 repro/gh-734/module/main.tf create mode 100644 repro/gh-734/module/variables.tf create mode 100644 repro/gh-796/.terraform.lock.hcl create mode 100644 repro/gh-796/main.tf diff --git a/repro/gh-734/main.tf b/repro/gh-734/main.tf new file mode 100644 index 000000000..2dfbf9e17 --- /dev/null +++ b/repro/gh-734/main.tf @@ -0,0 +1,18 @@ +terraform { + required_version = ">= 0.12" +} + +provider "vault" { + version = "2.12.2" + max_retries = 5 +} + +module "approle-module" { + source = "./module" + approle_name = "approle-Name" + role_id = "demo" + policies = [ + "approle-test", + "aws-1234567890-test_role" + ] +} diff --git a/repro/gh-734/module/main.tf b/repro/gh-734/module/main.tf new file mode 100644 index 000000000..068e35f47 --- /dev/null +++ b/repro/gh-734/module/main.tf @@ -0,0 +1,62 @@ +#Get the AppRole auth backend details +data "vault_auth_backend" "approle" { + path = var.mount_point +} + +#Dynamic policies - one will be created for each approle +data "vault_policy_document" "role_id" { + rule { + path = "auth/${data.vault_auth_backend.approle.path}/role/${vault_approle_auth_backend_role.approle.role_name}/role-id" + capabilities = ["read"] + description = "Read role-id for ${vault_approle_auth_backend_role.approle.role_name}" + } +} + +data "vault_policy_document" "secret_id" { + rule { + path = "auth/${data.vault_auth_backend.approle.path}/role/${vault_approle_auth_backend_role.approle.role_name}/secret-id" + capabilities = ["update"] + description = "Generate secret-id for ${vault_approle_auth_backend_role.approle.role_name}" + } +} + +data "vault_approle_auth_backend_role_id" "approle" { + depends_on = [vault_approle_auth_backend_role.approle] + backend = data.vault_auth_backend.approle.path + role_name = var.approle_name +} + + +resource "vault_policy" "role_id" { + name = "approle_${vault_approle_auth_backend_role.approle.role_name}_role_id" + policy = data.vault_policy_document.role_id.hcl +} + +resource "vault_policy" "secret_id" { + name = "approle_${vault_approle_auth_backend_role.approle.role_name}_secret_id" + policy = data.vault_policy_document.secret_id.hcl +} + +resource "vault_approle_auth_backend_role" "approle" { + backend = data.vault_auth_backend.approle.path + role_name = var.approle_name +} + +resource "vault_identity_entity" "approle" { + name = "${data.vault_auth_backend.approle.path}-${var.approle_name}" + external_policies = true +} + +resource "vault_identity_entity_policies" "approle" { + policies = concat(["default"], var.policies) + + exclusive = false + + entity_id = vault_identity_entity.approle.id +} + +resource "vault_identity_entity_alias" "approle" { + canonical_id = vault_identity_entity.approle.id + name = data.vault_approle_auth_backend_role_id.approle.role_id + mount_accessor = data.vault_auth_backend.approle.accessor +} diff --git a/repro/gh-734/module/variables.tf b/repro/gh-734/module/variables.tf new file mode 100644 index 000000000..dcdffd314 --- /dev/null +++ b/repro/gh-734/module/variables.tf @@ -0,0 +1,16 @@ +variable "approle_name" { + type = string + description = "Name for the AppRole" +} + +variable "role_id" { + type = string + description = "Custom role_id, can be any string" + default = null +} + +variable "policies" { + type = list(string) + description = "List of policy names to apply to the role" + default = [] +} diff --git a/repro/gh-796/.terraform.lock.hcl b/repro/gh-796/.terraform.lock.hcl new file mode 100644 index 000000000..aab9a8188 --- /dev/null +++ b/repro/gh-796/.terraform.lock.hcl @@ -0,0 +1,20 @@ +# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/vault" { + version = "2.22.1" + hashes = [ + "h1:JTW2/i6KvsOaZ2XBP2At5tP0GZyENJ+a5W7EcBhrevQ=", + "zh:0354ae21e4b53490c3a532004e817c8becdd0debdcdf4fc954bc70f267f3dfda", + "zh:09477c72b8cbb3ffed58f8666f130685e5f3c99f91467c696174cdf118500d49", + "zh:0f00c6b960e0cab4301cda379818079d2998cc0cc1a7eafd69395d1e6588e618", + "zh:2fac74c205fc1e0c4b1d800fb7fd8091a4bbeaae254b5e98bdeed8757b53a115", + "zh:620bf9b6e2ec0094e564f2668e4faddc9b557314d7725114b0cc33069933e8e3", + "zh:aec176654fd7f30c012dfbed58b20f08286e8499150b80e280e70b33f3a4da58", + "zh:b0ff022d70996170d43e8d1983404df85aea41eda1352453226d1f73f1b68897", + "zh:bbaea8145b302717b3e7be1155820fc5ee486b002f1562ef40fd2f21e4303447", + "zh:c9add1aeebef54418550338308f27926e1baae22f095ca53289f11a173dbf40b", + "zh:cde7538aaf01b83656cf8b1f70ca7259eac41a586f2c2fbcc7bf4a384dcade26", + "zh:f07a9a32366d33a7d2ad763c099563ea547f3e208b77ae3fb9e5894fd92a696a", + ] +} diff --git a/repro/gh-796/main.tf b/repro/gh-796/main.tf new file mode 100644 index 000000000..fa116e9bb --- /dev/null +++ b/repro/gh-796/main.tf @@ -0,0 +1,86 @@ +terraform { +} + +provider "vault" { +} + +variable "address" { + type = string + description = "Origin URL of the Vault server" + default = null +} + +variable "token" { + type = string + description = "Vault token that will be used by Terrafokjrm to authenticate" + default = null +} + +variable "token_name" { + type = string + description = "Token name that will be used by Terraform when creating the child token" + default = null +} + +variable "ca_cert_file" { + type = string + description = "Path to a file on local disk that will be used to validate the certificate presented by the Vault server" + default = null +} + +variable "ca_cert_dir" { + type = string + description = "Path to a directory on local disk that contains one or more certificate files that will be used to validate the certificate presented by the Vault server" + default = null +} + +variable "skip_tls_verify" { + type = bool + description = "Set this to `true` to disable verification of the Vault server's TLS certificate" + default = false +} + +variable "max_lease_ttl_seconds" { + type = number + description = "Used as the duration for the intermediate Vault token Terraform issues itself, which in turn limits the duration of secret leases issued by Vault" + default = 1200 +} + +variable "max_retries" { + type = number + description = "Used as the maximum number of retries when a 5xx error code is encountered" + default = 2 +} + +variable "namespace" { + type = string + description = "Set the namespace to use" + default = null +} + +variable "okta" { + type = object({ + path = string + organization = string + token = string + description = string + }) + description = "Okta auth method configuration" + default = null +} + +resource "vault_okta_auth_backend" "okta" { + description = "Testing the Terraform okta auth backend (baz)" + path = "okta-20210728135830937400000001" + organization = "example" + ttl = "1h" + max_ttl = "2h" + group { + group_name = "dummy" + policies = ["one", "two", "default"] + } + user { + username = "foo" + groups = ["dummy"] + } +} diff --git a/vault/resource_okta_auth_backend.go b/vault/resource_okta_auth_backend.go index f9a82013c..c385875f0 100644 --- a/vault/resource_okta_auth_backend.go +++ b/vault/resource_okta_auth_backend.go @@ -224,12 +224,14 @@ func normalizeOktaTTL(i interface{}) string { } func validateOktaTTL(i interface{}, k string) ([]string, []error) { + var values []string var errors []error s, err := parseDurationSeconds(i) if err != nil { errors = append(errors, fmt.Errorf("invalid value for %q, could not parse %q", k, i)) + values = append(values, s) } - return []string{s}, errors + return values, errors } func parseDurationSeconds(i interface{}) (string, error) { From 9461b65b7f83ec6be2dd2c6f3a2b99676de48bf4 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 30 Jul 2021 16:48:29 -0400 Subject: [PATCH 10/12] fix: remove repro dir --- repro/gh-734/main.tf | 18 ------- repro/gh-734/module/main.tf | 62 ----------------------- repro/gh-734/module/variables.tf | 16 ------ repro/gh-796/.terraform.lock.hcl | 20 -------- repro/gh-796/main.tf | 86 -------------------------------- 5 files changed, 202 deletions(-) delete mode 100644 repro/gh-734/main.tf delete mode 100644 repro/gh-734/module/main.tf delete mode 100644 repro/gh-734/module/variables.tf delete mode 100644 repro/gh-796/.terraform.lock.hcl delete mode 100644 repro/gh-796/main.tf diff --git a/repro/gh-734/main.tf b/repro/gh-734/main.tf deleted file mode 100644 index 2dfbf9e17..000000000 --- a/repro/gh-734/main.tf +++ /dev/null @@ -1,18 +0,0 @@ -terraform { - required_version = ">= 0.12" -} - -provider "vault" { - version = "2.12.2" - max_retries = 5 -} - -module "approle-module" { - source = "./module" - approle_name = "approle-Name" - role_id = "demo" - policies = [ - "approle-test", - "aws-1234567890-test_role" - ] -} diff --git a/repro/gh-734/module/main.tf b/repro/gh-734/module/main.tf deleted file mode 100644 index 068e35f47..000000000 --- a/repro/gh-734/module/main.tf +++ /dev/null @@ -1,62 +0,0 @@ -#Get the AppRole auth backend details -data "vault_auth_backend" "approle" { - path = var.mount_point -} - -#Dynamic policies - one will be created for each approle -data "vault_policy_document" "role_id" { - rule { - path = "auth/${data.vault_auth_backend.approle.path}/role/${vault_approle_auth_backend_role.approle.role_name}/role-id" - capabilities = ["read"] - description = "Read role-id for ${vault_approle_auth_backend_role.approle.role_name}" - } -} - -data "vault_policy_document" "secret_id" { - rule { - path = "auth/${data.vault_auth_backend.approle.path}/role/${vault_approle_auth_backend_role.approle.role_name}/secret-id" - capabilities = ["update"] - description = "Generate secret-id for ${vault_approle_auth_backend_role.approle.role_name}" - } -} - -data "vault_approle_auth_backend_role_id" "approle" { - depends_on = [vault_approle_auth_backend_role.approle] - backend = data.vault_auth_backend.approle.path - role_name = var.approle_name -} - - -resource "vault_policy" "role_id" { - name = "approle_${vault_approle_auth_backend_role.approle.role_name}_role_id" - policy = data.vault_policy_document.role_id.hcl -} - -resource "vault_policy" "secret_id" { - name = "approle_${vault_approle_auth_backend_role.approle.role_name}_secret_id" - policy = data.vault_policy_document.secret_id.hcl -} - -resource "vault_approle_auth_backend_role" "approle" { - backend = data.vault_auth_backend.approle.path - role_name = var.approle_name -} - -resource "vault_identity_entity" "approle" { - name = "${data.vault_auth_backend.approle.path}-${var.approle_name}" - external_policies = true -} - -resource "vault_identity_entity_policies" "approle" { - policies = concat(["default"], var.policies) - - exclusive = false - - entity_id = vault_identity_entity.approle.id -} - -resource "vault_identity_entity_alias" "approle" { - canonical_id = vault_identity_entity.approle.id - name = data.vault_approle_auth_backend_role_id.approle.role_id - mount_accessor = data.vault_auth_backend.approle.accessor -} diff --git a/repro/gh-734/module/variables.tf b/repro/gh-734/module/variables.tf deleted file mode 100644 index dcdffd314..000000000 --- a/repro/gh-734/module/variables.tf +++ /dev/null @@ -1,16 +0,0 @@ -variable "approle_name" { - type = string - description = "Name for the AppRole" -} - -variable "role_id" { - type = string - description = "Custom role_id, can be any string" - default = null -} - -variable "policies" { - type = list(string) - description = "List of policy names to apply to the role" - default = [] -} diff --git a/repro/gh-796/.terraform.lock.hcl b/repro/gh-796/.terraform.lock.hcl deleted file mode 100644 index aab9a8188..000000000 --- a/repro/gh-796/.terraform.lock.hcl +++ /dev/null @@ -1,20 +0,0 @@ -# This file is maintained automatically by "terraform init". -# Manual edits may be lost in future updates. - -provider "registry.terraform.io/hashicorp/vault" { - version = "2.22.1" - hashes = [ - "h1:JTW2/i6KvsOaZ2XBP2At5tP0GZyENJ+a5W7EcBhrevQ=", - "zh:0354ae21e4b53490c3a532004e817c8becdd0debdcdf4fc954bc70f267f3dfda", - "zh:09477c72b8cbb3ffed58f8666f130685e5f3c99f91467c696174cdf118500d49", - "zh:0f00c6b960e0cab4301cda379818079d2998cc0cc1a7eafd69395d1e6588e618", - "zh:2fac74c205fc1e0c4b1d800fb7fd8091a4bbeaae254b5e98bdeed8757b53a115", - "zh:620bf9b6e2ec0094e564f2668e4faddc9b557314d7725114b0cc33069933e8e3", - "zh:aec176654fd7f30c012dfbed58b20f08286e8499150b80e280e70b33f3a4da58", - "zh:b0ff022d70996170d43e8d1983404df85aea41eda1352453226d1f73f1b68897", - "zh:bbaea8145b302717b3e7be1155820fc5ee486b002f1562ef40fd2f21e4303447", - "zh:c9add1aeebef54418550338308f27926e1baae22f095ca53289f11a173dbf40b", - "zh:cde7538aaf01b83656cf8b1f70ca7259eac41a586f2c2fbcc7bf4a384dcade26", - "zh:f07a9a32366d33a7d2ad763c099563ea547f3e208b77ae3fb9e5894fd92a696a", - ] -} diff --git a/repro/gh-796/main.tf b/repro/gh-796/main.tf deleted file mode 100644 index fa116e9bb..000000000 --- a/repro/gh-796/main.tf +++ /dev/null @@ -1,86 +0,0 @@ -terraform { -} - -provider "vault" { -} - -variable "address" { - type = string - description = "Origin URL of the Vault server" - default = null -} - -variable "token" { - type = string - description = "Vault token that will be used by Terrafokjrm to authenticate" - default = null -} - -variable "token_name" { - type = string - description = "Token name that will be used by Terraform when creating the child token" - default = null -} - -variable "ca_cert_file" { - type = string - description = "Path to a file on local disk that will be used to validate the certificate presented by the Vault server" - default = null -} - -variable "ca_cert_dir" { - type = string - description = "Path to a directory on local disk that contains one or more certificate files that will be used to validate the certificate presented by the Vault server" - default = null -} - -variable "skip_tls_verify" { - type = bool - description = "Set this to `true` to disable verification of the Vault server's TLS certificate" - default = false -} - -variable "max_lease_ttl_seconds" { - type = number - description = "Used as the duration for the intermediate Vault token Terraform issues itself, which in turn limits the duration of secret leases issued by Vault" - default = 1200 -} - -variable "max_retries" { - type = number - description = "Used as the maximum number of retries when a 5xx error code is encountered" - default = 2 -} - -variable "namespace" { - type = string - description = "Set the namespace to use" - default = null -} - -variable "okta" { - type = object({ - path = string - organization = string - token = string - description = string - }) - description = "Okta auth method configuration" - default = null -} - -resource "vault_okta_auth_backend" "okta" { - description = "Testing the Terraform okta auth backend (baz)" - path = "okta-20210728135830937400000001" - organization = "example" - ttl = "1h" - max_ttl = "2h" - group { - group_name = "dummy" - policies = ["one", "two", "default"] - } - user { - username = "foo" - groups = ["dummy"] - } -} From 4ce429803284ec3d38a76fcaa1d165ee69ed0e28 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 30 Jul 2021 18:32:50 -0400 Subject: [PATCH 11/12] fix: add some missing config fields. --- vault/resource_okta_auth_backend.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/vault/resource_okta_auth_backend.go b/vault/resource_okta_auth_backend.go index c385875f0..95be72a33 100644 --- a/vault/resource_okta_auth_backend.go +++ b/vault/resource_okta_auth_backend.go @@ -354,13 +354,13 @@ func oktaReadAuthConfig(client *api.Client, path string, d *schema.ResourceData) return err } - // map schema config to okta auth params. - fieldMap := map[string]string{ - "organization": "organization", - "ttl": "token_ttl", - "max_ttl": "token_max_ttl", + // map schema config TTL strings to okta auth TTL params. + // the provider input type of string does not match Vault's API of int64 + ttlFieldMap := map[string]string{ + "ttl": "token_ttl", + "max_ttl": "token_max_ttl", } - for k, v := range fieldMap { + for k, v := range ttlFieldMap { if v, ok := config.Data[v]; ok { s, err := parseutil.ParseString(v) if err != nil { @@ -371,6 +371,18 @@ func oktaReadAuthConfig(client *api.Client, path string, d *schema.ResourceData) } } } + + params := []string{ + "base_url", + "bypass_okta_mfa", + "organization", + } + for _, param := range params { + if err := d.Set(param, config.Data[param]); err != nil { + return err + } + } + return nil } From e1d1a6224c7d6c25a0d2384b29480118ff7b2de0 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Mon, 2 Aug 2021 09:44:40 -0400 Subject: [PATCH 12/12] Revert "fix: make ttl and max_ttl computed values." This reverts commit 185d8311c0c99e9fcff665c128f310271bdddaae. --- vault/resource_okta_auth_backend.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vault/resource_okta_auth_backend.go b/vault/resource_okta_auth_backend.go index 95be72a33..db83218b4 100644 --- a/vault/resource_okta_auth_backend.go +++ b/vault/resource_okta_auth_backend.go @@ -81,9 +81,9 @@ func oktaAuthBackendResource() *schema.Resource { "ttl": { Type: schema.TypeString, - Computed: true, Required: false, Optional: true, + Default: "0", Description: "Duration after which authentication will be expired", ValidateFunc: validateOktaTTL, StateFunc: normalizeOktaTTL, @@ -91,10 +91,10 @@ func oktaAuthBackendResource() *schema.Resource { "max_ttl": { Type: schema.TypeString, - Computed: true, Required: false, Optional: true, Description: "Maximum duration after which authentication will be expired", + Default: "0", ValidateFunc: validateOktaTTL, StateFunc: normalizeOktaTTL, },