refactor: feature-toggled authentication methods (#2199)

* refactor: moving the unused SkipCredentialsValidation field to the Provider * refactor: adding a builder to parse out the config feature toggling support for service principal client secret * refactor: feature-toggling support for msi * refactoring: feature-toggling azure cli parsing/cloudshell auth * Removing the unused validate method * Feature-Toggling on Azure CLI Parsing auth * defining an interface for authentication methods * New Authentication Method: Service Principal Client Secret * removing the unused config from the GetAuthToken method * refactor: azure cli auth into it's own file making a bunch of the methods private tests pass: ``` $ go test -v ./azurerm/helpers/authentication/ === RUN TestAzureCLIParsingAuth_validate --- PASS: TestAzureCLIParsingAuth_validate (0.00s) === RUN TestServicePrincipalClientSecretAuth_builder --- PASS: TestServicePrincipalClientSecretAuth_builder (0.00s) === RUN TestServicePrincipalClientSecretAuth_validate --- PASS: TestServicePrincipalClientSecretAuth_validate (0.00s) === RUN TestManagedServiceIdentity_builder 2018/10/31 23:24:40 [DEBUG] Using MSI endpoint "https://hello-world" --- PASS: TestManagedServiceIdentity_builder (0.00s) === RUN TestManagedServiceIdentity_validate --- PASS: TestManagedServiceIdentity_validate (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_InvalidDate --- PASS: TestAzureFindValidAccessTokenForTenant_InvalidDate (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_Expired 2018/10/31 23:24:40 [DEBUG] Token "7cabcf30-8dca-43f9-91e6-fd56dfb8632f" has expired --- PASS: TestAzureFindValidAccessTokenForTenant_Expired (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_ExpiringIn --- PASS: TestAzureFindValidAccessTokenForTenant_ExpiringIn (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_InvalidManagementDomain 2018/10/31 23:24:40 [DEBUG] Resource "https://portal.azure.com/" isn't a management domain --- PASS: TestAzureFindValidAccessTokenForTenant_InvalidManagementDomain (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_DifferentTenant 2018/10/31 23:24:40 [DEBUG] Resource "https://management.core.windows.net/" isn't for the correct Tenant --- PASS: TestAzureFindValidAccessTokenForTenant_DifferentTenant (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_ValidFromCloudShell --- PASS: TestAzureFindValidAccessTokenForTenant_ValidFromCloudShell (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_ValidFromAzureCLI --- PASS: TestAzureFindValidAccessTokenForTenant_ValidFromAzureCLI (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_NoTokens --- PASS: TestAzureFindValidAccessTokenForTenant_NoTokens (0.00s) === RUN TestAzureCliProfile_populateSubscriptionIdMissing --- PASS: TestAzureCliProfile_populateSubscriptionIdMissing (0.00s) === RUN TestAzureCliProfile_populateSubscriptionIdNoDefault --- PASS: TestAzureCliProfile_populateSubscriptionIdNoDefault (0.00s) === RUN TestAzureCliProfile_populateSubscriptionIdValid --- PASS: TestAzureCliProfile_populateSubscriptionIdValid (0.00s) === RUN TestAzureCliProfile_populateTenantIdEmpty --- PASS: TestAzureCliProfile_populateTenantIdEmpty (0.00s) === RUN TestAzureCliProfile_populateTenantIdMissingSubscription --- PASS: TestAzureCliProfile_populateTenantIdMissingSubscription (0.00s) === RUN TestAzureCliProfile_populateTenantIdValid --- PASS: TestAzureCliProfile_populateTenantIdValid (0.00s) === RUN TestAzureCLIProfileFindDefaultSubscription --- PASS: TestAzureCLIProfileFindDefaultSubscription (0.00s) === RUN TestAzureCLIProfileFindSubscription --- PASS: TestAzureCLIProfileFindSubscription (0.00s) === RUN TestAzureEnvironmentNames --- PASS: TestAzureEnvironmentNames (0.00s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/authentication 0.791s ``` * refactor: removing the unused 'accessToken' field * refactor: removing `SkipProviderRegistration` from the authentication package * Re-enabling Client Secret auth for the acceptance tests * New authentication method: Service Principal Client Certificate NOTE: this is /intentionally/ feature-toggled off for now * vendoring `golang.org/x/crypto/pkcs12` * Code review fixes * refactoring: making the method signatures more consistent * refactor: switching to use a factory pattern Tests pass: ``` $ go test -v ./azurerm/helpers/authentication/ === RUN TestAzureCLIParsingAuth_isApplicable --- PASS: TestAzureCLIParsingAuth_isApplicable (0.00s) === RUN TestAzureCLIParsingAuth_populateConfig --- PASS: TestAzureCLIParsingAuth_populateConfig (0.00s) === RUN TestAzureCLIParsingAuth_validate --- PASS: TestAzureCLIParsingAuth_validate (0.00s) === RUN TestServicePrincipalClientCertAuth_builder --- PASS: TestServicePrincipalClientCertAuth_builder (0.00s) === RUN TestServicePrincipalClientCertAuth_isApplicable --- PASS: TestServicePrincipalClientCertAuth_isApplicable (0.00s) === RUN TestServicePrincipalClientCertAuth_populateConfig --- PASS: TestServicePrincipalClientCertAuth_populateConfig (0.00s) === RUN TestServicePrincipalClientCertAuth_validate --- PASS: TestServicePrincipalClientCertAuth_validate (0.00s) === RUN TestServicePrincipalClientSecretAuth_builder --- PASS: TestServicePrincipalClientSecretAuth_builder (0.00s) === RUN TestServicePrincipalClientSecretAuth_isApplicable --- PASS: TestServicePrincipalClientSecretAuth_isApplicable (0.00s) === RUN TestServicePrincipalClientSecretAuth_populateConfig --- PASS: TestServicePrincipalClientSecretAuth_populateConfig (0.00s) === RUN TestServicePrincipalClientSecretAuth_validate --- PASS: TestServicePrincipalClientSecretAuth_validate (0.00s) === RUN TestManagedServiceIdentity_builder 2018/11/06 14:18:37 [DEBUG] Using MSI endpoint "https://hello-world" --- PASS: TestManagedServiceIdentity_builder (0.00s) === RUN TestManagedServiceIdentity_isApplicable --- PASS: TestManagedServiceIdentity_isApplicable (0.00s) === RUN TestManagedServiceIdentity_populateConfig --- PASS: TestManagedServiceIdentity_populateConfig (0.00s) === RUN TestManagedServiceIdentity_validate --- PASS: TestManagedServiceIdentity_validate (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_InvalidDate --- PASS: TestAzureFindValidAccessTokenForTenant_InvalidDate (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_Expired 2018/11/06 14:18:37 [DEBUG] Token "7cabcf30-8dca-43f9-91e6-fd56dfb8632f" has expired --- PASS: TestAzureFindValidAccessTokenForTenant_Expired (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_ExpiringIn --- PASS: TestAzureFindValidAccessTokenForTenant_ExpiringIn (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_InvalidManagementDomain 2018/11/06 14:18:37 [DEBUG] Resource "https://portal.azure.com/" isn't a management domain --- PASS: TestAzureFindValidAccessTokenForTenant_InvalidManagementDomain (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_DifferentTenant 2018/11/06 14:18:37 [DEBUG] Resource "https://management.core.windows.net/" isn't for the correct Tenant --- PASS: TestAzureFindValidAccessTokenForTenant_DifferentTenant (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_ValidFromCloudShell --- PASS: TestAzureFindValidAccessTokenForTenant_ValidFromCloudShell (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_ValidFromAzureCLI --- PASS: TestAzureFindValidAccessTokenForTenant_ValidFromAzureCLI (0.00s) === RUN TestAzureFindValidAccessTokenForTenant_NoTokens --- PASS: TestAzureFindValidAccessTokenForTenant_NoTokens (0.00s) === RUN TestAzureCliProfile_populateSubscriptionIdMissing --- PASS: TestAzureCliProfile_populateSubscriptionIdMissing (0.00s) === RUN TestAzureCliProfile_populateSubscriptionIdNoDefault --- PASS: TestAzureCliProfile_populateSubscriptionIdNoDefault (0.00s) === RUN TestAzureCliProfile_populateSubscriptionIdValid --- PASS: TestAzureCliProfile_populateSubscriptionIdValid (0.00s) === RUN TestAzureCliProfile_populateTenantIdEmpty --- PASS: TestAzureCliProfile_populateTenantIdEmpty (0.00s) === RUN TestAzureCliProfile_populateTenantIdMissingSubscription --- PASS: TestAzureCliProfile_populateTenantIdMissingSubscription (0.00s) === RUN TestAzureCliProfile_populateTenantIdValid --- PASS: TestAzureCliProfile_populateTenantIdValid (0.00s) === RUN TestAzureCLIProfileFindDefaultSubscription --- PASS: TestAzureCLIProfileFindDefaultSubscription (0.00s) === RUN TestAzureCLIProfileFindSubscription --- PASS: TestAzureCLIProfileFindSubscription (0.00s) === RUN TestAzureEnvironmentNames --- PASS: TestAzureEnvironmentNames (0.00s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/authentication 0.851s ```
hashicorp · Nov 6, 2018 · d7974cf · d7974cf
1 parent 59d8ff9
commit d7974cf
Show file tree

Hide file tree

Showing 42 changed files with 2,548 additions and 820 deletions.
diff --git a/azurerm/azurerm_sweeper_test.go b/azurerm/azurerm_sweeper_test.go
@@ -30,16 +30,24 @@ func buildConfigForSweepers() (*ArmClient, error) {
 		return nil, fmt.Errorf("ARM_SUBSCRIPTION_ID, ARM_CLIENT_ID, ARM_CLIENT_SECRET and ARM_TENANT_ID must be set for acceptance tests")
 	}
 
-	config := &authentication.Config{
-		SubscriptionID:           subscriptionID,
-		ClientID:                 clientID,
-		ClientSecret:             clientSecret,
-		TenantID:                 tenantID,
-		Environment:              environment,
-		SkipProviderRegistration: false,
+	builder := &authentication.Builder{
+		SubscriptionID: subscriptionID,
+		ClientID:       clientID,
+		ClientSecret:   clientSecret,
+		TenantID:       tenantID,
+		Environment:    environment,
+
+		// Feature Toggles
+		SupportsClientSecretAuth: true,
+	}
+
+	config, err := builder.Build()
+	if err != nil {
+		return nil, fmt.Errorf("Error building ARM Client: %+v", err)
 	}
 
-	return getArmClient(config)
+	skipProviderRegistration := false
+	return getArmClient(config, skipProviderRegistration)
 }
 
 func shouldSweepAcceptanceTestResource(name string, resourceLocation string, region string) bool {

diff --git a/azurerm/config.go b/azurerm/config.go
@@ -354,7 +354,7 @@ func setUserAgent(client *autorest.Client) {
 
 // getArmClient is a helper method which returns a fully instantiated
 // *ArmClient based on the Config's current settings.
-func getArmClient(c *authentication.Config) (*ArmClient, error) {
+func getArmClient(c *authentication.Config, skipProviderRegistration bool) (*ArmClient, error) {
 	env, err := authentication.DetermineEnvironment(c.Environment)
 	if err != nil {
 		return nil, err
@@ -366,8 +366,8 @@ func getArmClient(c *authentication.Config) (*ArmClient, error) {
 		tenantId:                 c.TenantID,
 		subscriptionId:           c.SubscriptionID,
 		environment:              *env,
-		usingServicePrincipal:    c.ClientSecret != "",
-		skipProviderRegistration: c.SkipProviderRegistration,
+		usingServicePrincipal:    c.AuthenticatedAsAServicePrincipal,
+		skipProviderRegistration: skipProviderRegistration,
 	}
 
 	oauthConfig, err := adal.NewOAuthConfig(env.ActiveDirectoryEndpoint, c.TenantID)
@@ -382,22 +382,22 @@ func getArmClient(c *authentication.Config) (*ArmClient, error) {
 
 	// Resource Manager endpoints
 	endpoint := env.ResourceManagerEndpoint
-	auth, err := authentication.GetAuthorizationToken(c, oauthConfig, endpoint)
+	auth, err := c.GetAuthorizationToken(oauthConfig, endpoint)
 	if err != nil {
 		return nil, err
 	}
 
 	// Graph Endpoints
 	graphEndpoint := env.GraphEndpoint
-	graphAuth, err := authentication.GetAuthorizationToken(c, oauthConfig, graphEndpoint)
+	graphAuth, err := c.GetAuthorizationToken(oauthConfig, graphEndpoint)
 	if err != nil {
 		return nil, err
 	}
 
 	// Key Vault Endpoints
 	sender := azure.BuildSender()
 	keyVaultAuth := autorest.NewBearerAuthorizerCallback(sender, func(tenantID, resource string) (*autorest.BearerAuthorizer, error) {
-		keyVaultSpt, err := authentication.GetAuthorizationToken(c, oauthConfig, resource)
+		keyVaultSpt, err := c.GetAuthorizationToken(oauthConfig, resource)
 		if err != nil {
 			return nil, err
 		}

diff --git a/azurerm/helpers/authentication/auth_method.go b/azurerm/helpers/authentication/auth_method.go
@@ -0,0 +1,20 @@
+package authentication
+
+import (
+	"github.com/Azure/go-autorest/autorest"
+	"github.com/Azure/go-autorest/autorest/adal"
+)
+
+type authMethod interface {
+	build(b Builder) (authMethod, error)
+
+	isApplicable(b Builder) bool
+
+	getAuthorizationToken(oauthConfig *adal.OAuthConfig, endpoint string) (*autorest.BearerAuthorizer, error)
+
+	name() string
+
+	populateConfig(c *Config) error
+
+	validate() error
+}
diff --git a/azurerm/helpers/authentication/auth_method_azure_cli_parsing.go b/azurerm/helpers/authentication/auth_method_azure_cli_parsing.go
@@ -0,0 +1,116 @@
+package authentication
+
+import (
+	"fmt"
+
+	"github.com/Azure/go-autorest/autorest"
+	"github.com/Azure/go-autorest/autorest/adal"
+	"github.com/Azure/go-autorest/autorest/azure/cli"
+	"github.com/hashicorp/go-multierror"
+)
+
+type azureCliParsingAuth struct {
+	profile *azureCLIProfile
+}
+
+func (a azureCliParsingAuth) build(b Builder) (authMethod, error) {
+	auth := azureCliParsingAuth{
+		profile: &azureCLIProfile{
+			clientId:       b.ClientID,
+			environment:    b.Environment,
+			subscriptionId: b.SubscriptionID,
+			tenantId:       b.TenantID,
+		},
+	}
+	profilePath, err := cli.ProfilePath()
+	if err != nil {
+		return nil, fmt.Errorf("Error loading the Profile Path from the Azure CLI: %+v", err)
+	}
+
+	profile, err := cli.LoadProfile(profilePath)
+	if err != nil {
+		return nil, fmt.Errorf("Azure CLI Authorization Profile was not found. Please ensure the Azure CLI is installed and then log-in with `az login`.")
+	}
+
+	auth.profile.profile = profile
+
+	err = auth.profile.populateFields()
+	if err != nil {
+		return nil, err
+	}
+
+	err = auth.profile.populateClientIdAndAccessToken()
+	if err != nil {
+		return nil, fmt.Errorf("Error populating Access Tokens from the Azure CLI: %+v", err)
+	}
+
+	return auth, nil
+}
+
+func (a azureCliParsingAuth) isApplicable(b Builder) bool {
+	return b.SupportsAzureCliCloudShellParsing
+}
+
+func (a azureCliParsingAuth) getAuthorizationToken(oauthConfig *adal.OAuthConfig, endpoint string) (*autorest.BearerAuthorizer, error) {
+	if a.profile.usingCloudShell {
+		// load the refreshed tokens from the CloudShell Azure CLI credentials
+		err := a.profile.populateClientIdAndAccessToken()
+		if err != nil {
+			return nil, fmt.Errorf("Error loading the refreshed CloudShell tokens: %+v", err)
+		}
+	}
+
+	spt, err := adal.NewServicePrincipalTokenFromManualToken(*oauthConfig, a.profile.clientId, endpoint, *a.profile.accessToken)
+	if err != nil {
+		return nil, err
+	}
+
+	err = spt.Refresh()
+
+	if err != nil {
+		return nil, fmt.Errorf("Error refreshing Service Principal Token: %+v", err)
+	}
+
+	auth := autorest.NewBearerAuthorizer(spt)
+	return auth, nil
+}
+
+func (a azureCliParsingAuth) name() string {
+	return "Parsing credentials from the Azure CLI"
+}
+
+func (a azureCliParsingAuth) populateConfig(c *Config) error {
+	c.ClientID = a.profile.clientId
+	c.Environment = a.profile.environment
+	c.SubscriptionID = a.profile.subscriptionId
+	c.TenantID = a.profile.tenantId
+	return nil
+}
+
+func (a azureCliParsingAuth) validate() error {
+	var err *multierror.Error
+
+	errorMessageFmt := "A %s was not found in your Azure CLI Credentials.\n\nPlease login to the Azure CLI again via `az login`"
+
+	if a.profile == nil {
+		return fmt.Errorf("Azure CLI Profile is nil - this is an internal error and should be reported.")
+	}
+
+	if a.profile.accessToken == nil {
+		err = multierror.Append(err, fmt.Errorf(errorMessageFmt, "Access Token"))
+	}
+
+	if a.profile.clientId == "" {
+		err = multierror.Append(err, fmt.Errorf(errorMessageFmt, "Client ID"))
+	}
+
+	if a.profile.subscriptionId == "" {
+		err = multierror.Append(err, fmt.Errorf(errorMessageFmt, "Subscription ID"))
+	}
+
+	if a.profile.tenantId == "" {
+		err = multierror.Append(err, fmt.Errorf(errorMessageFmt, "Tenant ID"))
+	}
+
+	return err.ErrorOrNil()
+}
diff --git a/azurerm/helpers/authentication/auth_method_azure_cli_parsing_test.go b/azurerm/helpers/authentication/auth_method_azure_cli_parsing_test.go
@@ -0,0 +1,157 @@
+package authentication
+
+import (
+	"testing"
+
+	"github.com/Azure/go-autorest/autorest/adal"
+)
+
+func TestAzureCLIParsingAuth_isApplicable(t *testing.T) {
+	cases := []struct {
+		Description string
+		Builder     Builder
+		Valid       bool
+	}{
+		{
+			Description: "Empty Configuration",
+			Builder:     Builder{},
+			Valid:       false,
+		},
+		{
+			Description: "Feature Toggled off",
+			Builder: Builder{
+				SupportsAzureCliCloudShellParsing: false,
+			},
+			Valid: false,
+		},
+		{
+			Description: "Feature Toggled on",
+			Builder: Builder{
+				SupportsAzureCliCloudShellParsing: true,
+			},
+			Valid: true,
+		},
+	}
+
+	for _, v := range cases {
+		applicable := azureCliParsingAuth{}.isApplicable(v.Builder)
+		if v.Valid != applicable {
+			t.Fatalf("Expected %q to be %t but got %t", v.Description, v.Valid, applicable)
+		}
+	}
+}
+
+func TestAzureCLIParsingAuth_populateConfig(t *testing.T) {
+	config := &Config{}
+	auth := azureCliParsingAuth{
+		profile: &azureCLIProfile{
+			clientId:       "some-subscription-id",
+			environment:    "dimension-c137",
+			subscriptionId: "some-subscription-id",
+			tenantId:       "some-tenant-id",
+		},
+	}
+
+	err := auth.populateConfig(config)
+	if err != nil {
+		t.Fatalf("Error populating config: %s", err)
+	}
+
+	if auth.profile.clientId != config.ClientID {
+		t.Fatalf("Expected Client ID to be %q but got %q", auth.profile.tenantId, config.TenantID)
+	}
+
+	if auth.profile.environment != config.Environment {
+		t.Fatalf("Expected Environment to be %q but got %q", auth.profile.tenantId, config.TenantID)
+	}
+
+	if auth.profile.subscriptionId != config.SubscriptionID {
+		t.Fatalf("Expected Subscription ID to be %q but got %q", auth.profile.tenantId, config.TenantID)
+	}
+
+	if auth.profile.tenantId != config.TenantID {
+		t.Fatalf("Expected Tenant ID to be %q but got %q", auth.profile.tenantId, config.TenantID)
+	}
+}
+
+func TestAzureCLIParsingAuth_validate(t *testing.T) {
+	cases := []struct {
+		Description string
+		Config      azureCliParsingAuth
+		ExpectError bool
+	}{
+		{
+			Description: "Empty Configuration",
+			Config:      azureCliParsingAuth{},
+			ExpectError: true,
+		},
+		{
+			Description: "Missing Access Token",
+			Config: azureCliParsingAuth{
+				profile: &azureCLIProfile{
+					clientId:       "62e73395-5017-43b6-8ebf-d6c30a514cf1",
+					subscriptionId: "8e8b5e02-5c13-4822-b7dc-4232afb7e8c2",
+					tenantId:       "9834f8d0-24b3-41b7-8b8d-c611c461a129",
+				},
+			},
+			ExpectError: true,
+		},
+		{
+			Description: "Missing Client ID",
+			Config: azureCliParsingAuth{
+				profile: &azureCLIProfile{
+					accessToken:    &adal.Token{},
+					subscriptionId: "8e8b5e02-5c13-4822-b7dc-4232afb7e8c2",
+					tenantId:       "9834f8d0-24b3-41b7-8b8d-c611c461a129",
+				},
+			},
+			ExpectError: true,
+		},
+		{
+			Description: "Missing Subscription ID",
+			Config: azureCliParsingAuth{
+				profile: &azureCLIProfile{
+					accessToken: &adal.Token{},
+					clientId:    "62e73395-5017-43b6-8ebf-d6c30a514cf1",
+					tenantId:    "9834f8d0-24b3-41b7-8b8d-c611c461a129",
+				},
+			},
+			ExpectError: true,
+		},
+		{
+			Description: "Missing Tenant ID",
+			Config: azureCliParsingAuth{
+				profile: &azureCLIProfile{
+					accessToken:    &adal.Token{},
+					clientId:       "62e73395-5017-43b6-8ebf-d6c30a514cf1",
+					subscriptionId: "8e8b5e02-5c13-4822-b7dc-4232afb7e8c2",
+				},
+			},
+			ExpectError: true,
+		},
+		{
+			Description: "Valid Configuration",
+			Config: azureCliParsingAuth{
+				profile: &azureCLIProfile{
+					accessToken:    &adal.Token{},
+					clientId:       "62e73395-5017-43b6-8ebf-d6c30a514cf1",
+					subscriptionId: "8e8b5e02-5c13-4822-b7dc-4232afb7e8c2",
+					tenantId:       "9834f8d0-24b3-41b7-8b8d-c611c461a129",
+				},
+			},
+			ExpectError: false,
+		},
+	}
+
+	for _, v := range cases {
+		err := v.Config.validate()
+
+		if v.ExpectError && err == nil {
+			t.Fatalf("Expected an error for %q: didn't get one", v.Description)
+		}
+
+		if !v.ExpectError && err != nil {
+			t.Fatalf("Expected there to be no error for %q - but got: %v", v.Description, err)
+		}
+	}
+}