From 83826aa866723ed3710ea566efb30c710d06adc6 Mon Sep 17 00:00:00 2001
From: Elim Tsiagbey <elim.tsiagbey@hashicorp.com>
Date: Fri, 12 May 2023 10:56:58 -0400
Subject: [PATCH] fix: default auth method with recovery kms (#406)

* add test coverage for authenticating with recovery kms mechanism & oidc primary auth method

* Skip fetching default auth method when recovery_kms_hcl is set or the client token isn't null
---
 internal/provider/provider.go                 |  5 +-
 internal/provider/provider_test.go            | 55 +++++++++++++++++--
 .../resource_auth_method_oidc_test.go         |  4 +-
 3 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/internal/provider/provider.go b/internal/provider/provider.go
index 6c5ab047..8b2442f7 100644
--- a/internal/provider/provider.go
+++ b/internal/provider/provider.go
@@ -156,9 +156,10 @@ func providerAuthenticate(ctx context.Context, d *schema.ResourceData, md *metaD
 		md.client.SetToken(token.(string))
 	}
 
-	// If auth_method_id is not set, get the default auth method ID for the given scope ID
+	// If auth_method_id is not set, get the default auth method ID for the given scope ID.
+	// Skip fetching default auth method when recovery_kms_hcl is set or the client token isn't null.
 	authMethodId, authMethodIdOk := d.GetOk("auth_method_id")
-	if !authMethodIdOk {
+	if !authMethodIdOk && !recoveryKmsHclOk && md.client.Token() == "" {
 		defaultAuthMethodId, err := getDefaultAuthMethodId(ctx, amClient, providerScope, "")
 		if err != nil {
 			return err
diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go
index 3808c57b..314c4b6c 100644
--- a/internal/provider/provider_test.go
+++ b/internal/provider/provider_test.go
@@ -14,6 +14,7 @@ import (
 
 	"github.com/hashicorp/boundary/testing/controller"
 	"github.com/hashicorp/cap/ldap"
+	"github.com/hashicorp/cap/oidc"
 	wrapping "github.com/hashicorp/go-kms-wrapping/v2"
 	"github.com/hashicorp/go-kms-wrapping/v2/aead"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -143,9 +144,6 @@ func testConfigWithRecovery(url string, res ...string) string {
 	provider := fmt.Sprintf(`
 provider "boundary" {
 	addr             = "%s"
-	auth_method_id       = "%s"
-	password_auth_method_login_name = "%s"
-	password_auth_method_password = "%s"
 	recovery_kms_hcl = <<DOC
 	kms "aead" {
 		purpose = ["recovery", "config"]
@@ -154,7 +152,7 @@ provider "boundary" {
 		key_id = "global_recovery"
 	}
 	DOC
-}`, url, tcPAUM, tcLoginName, tcPassword)
+}`, url)
 
 	c := []string{provider}
 	c = append(c, res...)
@@ -294,6 +292,55 @@ func TestConfigWithOIDCAuthMethod(t *testing.T) {
 	})
 }
 
+// Create OIDC auth method and set it as the primary auth method.
+// Attempt to authenticate with recovery to test checks for default auth method
+func TestRecoveryWithOIDCDefaultAuthMethod(t *testing.T) {
+	tp := oidc.StartTestProvider(t)
+	wrapper := testWrapper(context.Background(), t, tcRecoveryKey)
+	tc := controller.NewTestController(t, append(tcConfig, controller.WithRecoveryKms(wrapper))...)
+	defer tc.Shutdown()
+	url := tc.ApiAddrs()[0]
+
+	tpCert := strings.TrimSpace(tp.CACert())
+	createConfig := fmt.Sprintf(fooAuthMethodOidc, fooAuthMethodOidcDesc, tp.Addr(), tpCert)
+	updateConfig := fmt.Sprintf(fooAuthMethodOidcUpdate, fooAuthMethodOidcDescUpdate, fooAuthMethodOidcCaCerts)
+
+	var provider *schema.Provider
+	resource.Test(t, resource.TestCase{
+		ProviderFactories: providerFactories(&provider),
+		CheckDestroy:      testAccCheckAuthMethodResourceDestroy(t, provider, oidcAuthMethodType),
+		Steps: []resource.TestStep{
+			{
+				// create auth method
+				Config: testConfig(url, fooOrg, createConfig),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", "description", fooAuthMethodOidcDesc),
+					resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", "name", "test"),
+					resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcIssuerKey, tp.Addr()),
+				),
+			},
+			importStep("boundary_auth_method_oidc.foo", "client_secret", "is_primary_for_scope"),
+			{
+				// set auth method as primary auth method
+				Config: testConfig(url, fooOrg, updateConfig),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", "name", "test"),
+					testAccIsPrimaryForScope(provider, "boundary_auth_method_oidc.foo", true),
+				),
+			},
+			{
+				// authenticate provider with recovery kms with unsupported OIDC primary auth method
+				Config: testConfigWithRecovery(url, fooOrg, updateConfig),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", "name", "test"),
+					testAccIsPrimaryForScope(provider, "boundary_auth_method_oidc.foo", true),
+				),
+			},
+			importStep("boundary_auth_method_oidc.foo", "client_secret", "is_primary_for_scope", authmethodOidcMaxAgeKey),
+		},
+	})
+}
+
 func testProviderTokenExists(testProvider *schema.Provider) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		md := testProvider.Meta().(*metaData)
diff --git a/internal/provider/resource_auth_method_oidc_test.go b/internal/provider/resource_auth_method_oidc_test.go
index d8473486..fc5ffb16 100644
--- a/internal/provider/resource_auth_method_oidc_test.go
+++ b/internal/provider/resource_auth_method_oidc_test.go
@@ -50,7 +50,7 @@ var (
 resource "boundary_auth_method_oidc" "foo" {
 	name        = "test"
 	description = "%s"
-	scope_id    = boundary_scope.org1.id
+	scope_id    = "global"
 	depends_on  = [boundary_role.org1_admin]
 
   issuer            = "%s"
@@ -73,7 +73,7 @@ EOT
 resource "boundary_auth_method_oidc" "foo" {
 	name                 = "test"
 	description          = "%s"
-	scope_id             = boundary_scope.org1.id
+	scope_id             = "global"
 	is_primary_for_scope = true
 	depends_on           = [boundary_role.org1_admin]