Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: feature-toggled authentication methods #2199

Merged
merged 18 commits into from
Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
b28fdc3
refactor: moving the unused SkipCredentialsValidation field to the Pr…
tombuildsstuff Oct 31, 2018
ca623b6
refactor: adding a builder to parse out the config
tombuildsstuff Oct 31, 2018
9537e8d
refactor: feature-toggling support for msi
tombuildsstuff Oct 31, 2018
36520aa
refactoring: feature-toggling azure cli parsing/cloudshell auth
tombuildsstuff Oct 31, 2018
1d8ea24
Removing the unused validate method
tombuildsstuff Oct 31, 2018
6ad36ad
Feature-Toggling on Azure CLI Parsing auth
tombuildsstuff Oct 31, 2018
e45d2a9
defining an interface for authentication methods
tombuildsstuff Oct 31, 2018
db9f9cd
New Authentication Method: Service Principal Client Secret
tombuildsstuff Oct 31, 2018
7b6e4d3
removing the unused config from the GetAuthToken method
tombuildsstuff Oct 31, 2018
9842b51
refactor: azure cli auth into it's own file
tombuildsstuff Oct 31, 2018
743cecc
refactor: removing the unused 'accessToken' field
tombuildsstuff Oct 31, 2018
afafe5e
refactor: removing `SkipProviderRegistration` from the authentication…
tombuildsstuff Oct 31, 2018
0cc33d4
Re-enabling Client Secret auth for the acceptance tests
tombuildsstuff Oct 31, 2018
e12e5db
New authentication method: Service Principal Client Certificate
tombuildsstuff Nov 1, 2018
64f9ce0
vendoring `golang.org/x/crypto/pkcs12`
tombuildsstuff Nov 1, 2018
60f30af
Code review fixes
tombuildsstuff Nov 1, 2018
98e620a
refactoring: making the method signatures more consistent
tombuildsstuff Nov 6, 2018
89362a4
refactor: switching to use a factory pattern
tombuildsstuff Nov 6, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions azurerm/azurerm_sweeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions azurerm/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
}
Expand Down
20 changes: 20 additions & 0 deletions azurerm/helpers/authentication/auth_method.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package authentication
tombuildsstuff marked this conversation as resolved.
Show resolved Hide resolved

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
}
116 changes: 116 additions & 0 deletions azurerm/helpers/authentication/auth_method_azure_cli_parsing.go
Original file line number Diff line number Diff line change
@@ -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()
}
157 changes: 157 additions & 0 deletions azurerm/helpers/authentication/auth_method_azure_cli_parsing_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
Loading