From ca1ae8876519bb8e97807b27b8974a759f915887 Mon Sep 17 00:00:00 2001 From: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> Date: Wed, 23 Jun 2021 13:51:09 -0700 Subject: [PATCH 1/8] Common Azure auth logic - Currently implemented on secretstores/azure/keyvault and state/azure/blobstorage - Supports Azure AD via service principal (client credentials, client certificate, MSI) - based on the previous authorizer for AKV - Allows using other Azure clouds (China, Germany, etc) - For Blob Storage state, supports using custom endpoints (like emulators like Azurite) --- authentication/azure/auth.go | 276 ++++++++++++++++++ .../azure/auth_test.go | 142 +++++---- authentication/azure/metadata-properties.go | 22 ++ authentication/azure/storage.go | 58 ++++ secretstores/azure/keyvault/authutils.go | 167 ----------- secretstores/azure/keyvault/keyvault.go | 44 +-- state/azure/blobstorage/blobstorage.go | 28 +- state/azure/blobstorage/blobstorage_test.go | 2 - state/azure/tablestorage/tablestorage.go | 4 +- 9 files changed, 467 insertions(+), 276 deletions(-) create mode 100644 authentication/azure/auth.go rename secretstores/azure/keyvault/authutils_test.go => authentication/azure/auth_test.go (74%) create mode 100644 authentication/azure/metadata-properties.go create mode 100644 authentication/azure/storage.go delete mode 100644 secretstores/azure/keyvault/authutils.go diff --git a/authentication/azure/auth.go b/authentication/azure/auth.go new file mode 100644 index 0000000000..eeae105869 --- /dev/null +++ b/authentication/azure/auth.go @@ -0,0 +1,276 @@ +// ------------------------------------------------------------ +// Copyright (c) Microsoft Corporation and Dapr Contributors. +// Licensed under the MIT License. +// ------------------------------------------------------------ + +package azure + +import ( + "crypto/rsa" + "crypto/x509" + "errors" + "fmt" + + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/adal" + "github.com/Azure/go-autorest/autorest/azure" + "github.com/Azure/go-autorest/autorest/azure/auth" + "golang.org/x/crypto/pkcs12" +) + +// NewEnvironmentSettings returns a new EnvironmentSettings configured for a given Azure resource. +func NewEnvironmentSettings(resourceName string, values map[string]string) (EnvironmentSettings, error) { + es := EnvironmentSettings{ + Values: values, + } + env, err := es.GetEnvironment() + if err != nil { + return es, err + } + es.AzureEnvironment = env + switch resourceName { + case "azure": + // Azure Resource Manager (management plane) + es.Resource = env.TokenAudience + case "keyvault": + // Azure Key Vault (data plane) + es.Resource = env.ResourceIdentifiers.KeyVault + case "storage": + // Azure Storage (data plane) + es.Resource = env.ResourceIdentifiers.Storage + default: + return es, errors.New("invalid resource name: " + resourceName) + } + return es, nil +} + +// EnvironmentSettings hold settings to authenticate with Azure. +type EnvironmentSettings struct { + Values map[string]string + Resource string + AzureEnvironment *azure.Environment +} + +// GetEnvironment returns the Azure environment for a given name. +func (s EnvironmentSettings) GetEnvironment() (*azure.Environment, error) { + envName, ok := s.Values[AzureEnvironmentKey] + if !ok || envName == "" { + envName = DefaultAzureEnvironment + } + env, err := azure.EnvironmentFromName(envName) + if err != nil { + return nil, err + } + return &env, err +} + +// GetAuthorizer creates an Authorizer retrieved from, in order: +// 1. Client credentials +// 2. Client certificate +// 3. MSI +func (s EnvironmentSettings) GetAuthorizer() (autorest.Authorizer, error) { + spt, err := s.GetServicePrincipalToken() + if err != nil { + return nil, err + } + + return autorest.NewBearerAuthorizer(spt), nil +} + +// GetServicePrincipalToken returns a Service Principal Token retrieved from, in order: +// 1. Client credentials +// 2. Client certificate +// 3. MSI +func (s EnvironmentSettings) GetServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + // 1. Client credentials + if c, e := s.GetClientCredentials(); e == nil { + return c.ServicePrincipalToken() + } + + // 2. Client Certificate + if c, e := s.GetClientCert(); e == nil { + return c.ServicePrincipalToken() + } + + // 3. MSI + return s.GetMSI().ServicePrincipalToken() +} + +// GetClientCredentials creates a config object from the available client credentials. +// An error is returned if no certificate credentials are available. +func (s EnvironmentSettings) GetClientCredentials() (CredentialsConfig, error) { + env, err := s.GetEnvironment() + if err != nil { + return CredentialsConfig{}, err + } + + clientID := s.Values[ClientIDKey] + clientSecret := s.Values[ClientSecretKey] + tenantID := s.Values[TenantIDKey] + + if clientID == "" || clientSecret == "" || tenantID == "" { + return CredentialsConfig{}, errors.New("parameters clientId, clientSecret, and tenantId must all be present") + } + + authorizer := NewCredentialsConfig(clientID, tenantID, clientSecret, s.Resource, env) + + return authorizer, nil +} + +// GetClientCert creates a config object from the available certificate credentials. +// An error is returned if no certificate credentials are available. +func (s EnvironmentSettings) GetClientCert() (CertConfig, error) { + env, err := s.GetEnvironment() + if err != nil { + return CertConfig{}, err + } + + certFilePath, certFilePathPresent := s.Values[CertificateFileKey] + certBytes, certBytesPresent := s.Values[CertificateKey] + certPassword := s.Values[CertificatePasswordKey] + clientID := s.Values[ClientIDKey] + tenantID := s.Values[TenantIDKey] + + if !certFilePathPresent && !certBytesPresent { + return CertConfig{}, fmt.Errorf("missing client certificate") + } + + authorizer := NewCertConfig(clientID, tenantID, certFilePath, []byte(certBytes), certPassword, s.Resource, env) + + return authorizer, nil +} + +// GetMSI creates a MSI config object from the available client ID. +func (s EnvironmentSettings) GetMSI() MSIConfig { + config := NewMSIConfig(s.Resource) + config.Resource = azure.PublicCloud.ResourceIdentifiers.KeyVault + config.ClientID = s.Values[ClientIDKey] + + return config +} + +// CredentialsConfig provides the options to get a bearer authorizer from client credentials +type CredentialsConfig struct { + *auth.ClientCredentialsConfig +} + +// NewCredentialsConfig creates an CredentialsConfig object configured to obtain an Authorizer through Client Credentials. +func NewCredentialsConfig(clientID string, tenantID string, clientSecret string, resource string, env *azure.Environment) CredentialsConfig { + return CredentialsConfig{ + &auth.ClientCredentialsConfig{ + ClientSecret: clientSecret, + ClientID: clientID, + TenantID: tenantID, + Resource: resource, + AADEndpoint: env.ActiveDirectoryEndpoint, + }, + } +} + +// ServicePrincipalToken gets a ServicePrincipalToken object from the credentials. +func (c CredentialsConfig) ServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + oauthConfig, err := adal.NewOAuthConfig(c.AADEndpoint, c.TenantID) + if err != nil { + return nil, err + } + return adal.NewServicePrincipalToken(*oauthConfig, c.ClientID, c.ClientSecret, c.Resource) +} + +// CertConfig provides the options to get a bearer authorizer from a client certificate. +type CertConfig struct { + *auth.ClientCertificateConfig + CertificateData []byte +} + +// NewCertConfig creates an CertConfig object configured to obtain an Authorizer through Client Credentials, using a certificate. +func NewCertConfig(clientID string, tenantID string, certificatePath string, certificateBytes []byte, certificatePassword string, resource string, env *azure.Environment) CertConfig { + return CertConfig{ + &auth.ClientCertificateConfig{ + CertificatePath: certificatePath, + CertificatePassword: certificatePassword, + ClientID: clientID, + TenantID: tenantID, + Resource: resource, + AADEndpoint: env.ActiveDirectoryEndpoint, + }, + certificateBytes, + } +} + +// ServicePrincipalToken gets a ServicePrincipalToken object from client certificate. +func (c CertConfig) ServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + if c.ClientCertificateConfig.CertificatePath != "" { + // in standalone mode, component yaml will pass cert path + return c.ClientCertificateConfig.ServicePrincipalToken() + } else if len(c.CertificateData) > 0 { + // in kubernetes mode, runtime will get the secret from K8S secret store and pass byte array + return c.ServicePrincipalTokenByCertBytes() + } + + return nil, fmt.Errorf("certificate is not given") +} + +// ServicePrincipalTokenByCertBytes gets the service principal token by CertificateBytes. +func (c CertConfig) ServicePrincipalTokenByCertBytes() (*adal.ServicePrincipalToken, error) { + oauthConfig, err := adal.NewOAuthConfig(c.AADEndpoint, c.TenantID) + if err != nil { + return nil, err + } + + certificate, rsaPrivateKey, err := c.decodePkcs12(c.CertificateData, c.CertificatePassword) + if err != nil { + return nil, fmt.Errorf("failed to decode pkcs12 certificate while creating spt: %v", err) + } + + return adal.NewServicePrincipalTokenFromCertificate(*oauthConfig, c.ClientID, certificate, rsaPrivateKey, c.Resource) +} + +func (c CertConfig) decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.PrivateKey, error) { + privateKey, certificate, err := pkcs12.Decode(pkcs, password) + if err != nil { + return nil, nil, err + } + + rsaPrivateKey, isRsaKey := privateKey.(*rsa.PrivateKey) + if !isRsaKey { + return nil, nil, fmt.Errorf("PKCS#12 certificate must contain an RSA private key") + } + + return certificate, rsaPrivateKey, nil +} + +// MSIConfig provides the options to get a bearer authorizer through MSI. +type MSIConfig struct { + Resource string + ClientID string +} + +// NewMSIConfig creates an MSIConfig object configured to obtain an Authorizer through MSI. +func NewMSIConfig(resource string) MSIConfig { + return MSIConfig{ + Resource: resource, + } +} + +// ServicePrincipalToken gets the ServicePrincipalToken object from MSI. +func (mc MSIConfig) ServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + msiEndpoint, err := adal.GetMSIEndpoint() + if err != nil { + return nil, err + } + + var spToken *adal.ServicePrincipalToken + if mc.ClientID == "" { + spToken, err = adal.NewServicePrincipalTokenFromMSI(msiEndpoint, mc.Resource) + if err != nil { + return nil, fmt.Errorf("failed to get oauth token from MSI: %v", err) + } + } else { + spToken, err = adal.NewServicePrincipalTokenFromMSIWithUserAssignedID(msiEndpoint, mc.Resource, mc.ClientID) + if err != nil { + return nil, fmt.Errorf("failed to get oauth token from MSI for user assigned identity: %v", err) + } + } + + return spToken, nil +} diff --git a/secretstores/azure/keyvault/authutils_test.go b/authentication/azure/auth_test.go similarity index 74% rename from secretstores/azure/keyvault/authutils_test.go rename to authentication/azure/auth_test.go index fd0a597fda..b3d862a229 100644 --- a/secretstores/azure/keyvault/authutils_test.go +++ b/authentication/azure/auth_test.go @@ -3,7 +3,7 @@ // Licensed under the MIT License. // ------------------------------------------------------------ -package keyvault +package azure import ( "encoding/base64" @@ -23,16 +23,18 @@ const ( ) func TestGetClientCert(t *testing.T) { - settings := EnvironmentSettings{ - Values: map[string]string{ - componentSPNCertificateFile: "testfile", - componentSPNCertificate: "testcert", - componentSPNCertificatePassword: "1234", - componentSPNClientID: fakeClientID, - componentSPNTenantID: fakeTenantID, - componentVaultName: "vaultName", + settings, err := NewEnvironmentSettings( + "keyvault", + map[string]string{ + CertificateFileKey: "testfile", + CertificateKey: "testcert", + CertificatePasswordKey: "1234", + ClientIDKey: fakeClientID, + TenantIDKey: fakeTenantID, + "vaultName": "vaultName", }, - } + ) + assert.NoError(t, err) testCertConfig, _ := settings.GetClientCert() @@ -53,15 +55,17 @@ func TestAuthorizorWithCertFile(t *testing.T) { err := ioutil.WriteFile(testCertFileName, certBytes, 0o644) assert.NoError(t, err) - settings := EnvironmentSettings{ - Values: map[string]string{ - componentSPNCertificateFile: testCertFileName, - componentSPNCertificatePassword: "", - componentSPNClientID: fakeClientID, - componentSPNTenantID: fakeTenantID, - componentVaultName: "vaultName", + settings, err := NewEnvironmentSettings( + "keyvault", + map[string]string{ + CertificateFileKey: testCertFileName, + CertificatePasswordKey: "", + ClientIDKey: fakeClientID, + TenantIDKey: fakeTenantID, + "vaultName": "vaultName", }, - } + ) + assert.NoError(t, err) testCertConfig, _ := settings.GetClientCert() assert.NotNil(t, testCertConfig) @@ -79,55 +83,61 @@ func TestAuthorizorWithCertBytes(t *testing.T) { t.Run("Certificate is valid", func(t *testing.T) { certBytes := getTestCert() - settings := EnvironmentSettings{ - Values: map[string]string{ - componentSPNCertificate: string(certBytes), - componentSPNCertificatePassword: "", - componentSPNClientID: fakeClientID, - componentSPNTenantID: fakeTenantID, - componentVaultName: "vaultName", + settings, err := NewEnvironmentSettings( + "keyvault", + map[string]string{ + CertificateKey: string(certBytes), + CertificatePasswordKey: "", + ClientIDKey: fakeClientID, + TenantIDKey: fakeTenantID, + "vaultName": "vaultName", }, - } + ) + assert.NoError(t, err) testCertConfig, _ := settings.GetClientCert() assert.NotNil(t, testCertConfig) assert.NotNil(t, testCertConfig.ClientCertificateConfig) - authorizer, err := testCertConfig.Authorizer() + spt, err := testCertConfig.ServicePrincipalToken() assert.NoError(t, err) - assert.NotNil(t, authorizer) + assert.NotNil(t, spt) }) t.Run("Certificate is invalid", func(t *testing.T) { certBytes := getTestCert() - settings := EnvironmentSettings{ - Values: map[string]string{ - componentSPNCertificate: string(certBytes[0:20]), - componentSPNCertificatePassword: "", - componentSPNClientID: fakeClientID, - componentSPNTenantID: fakeTenantID, - componentVaultName: "vaultName", + settings, err := NewEnvironmentSettings( + "keyvault", + map[string]string{ + CertificateKey: string(certBytes[0:20]), + CertificatePasswordKey: "", + ClientIDKey: fakeClientID, + TenantIDKey: fakeTenantID, + "vaultName": "vaultName", }, - } + ) + assert.NoError(t, err) testCertConfig, _ := settings.GetClientCert() assert.NotNil(t, testCertConfig) assert.NotNil(t, testCertConfig.ClientCertificateConfig) - _, err := testCertConfig.Authorizer() + _, err = testCertConfig.ServicePrincipalToken() assert.Error(t, err) }) } func TestGetMSI(t *testing.T) { - settings := EnvironmentSettings{ - Values: map[string]string{ - componentSPNClientID: fakeClientID, - componentVaultName: "vaultName", + settings, err := NewEnvironmentSettings( + "keyvault", + map[string]string{ + ClientIDKey: fakeClientID, + "vaultName": "vaultName", }, - } + ) + assert.NoError(t, err) testCertConfig := settings.GetMSI() @@ -136,49 +146,55 @@ func TestGetMSI(t *testing.T) { } func TestFallbackToMSI(t *testing.T) { - settings := EnvironmentSettings{ - Values: map[string]string{ - componentSPNClientID: fakeClientID, - componentVaultName: "vaultName", + settings, err := NewEnvironmentSettings( + "keyvault", + map[string]string{ + ClientIDKey: fakeClientID, + "vaultName": "vaultName", }, - } + ) + assert.NoError(t, err) - authorizer, err := settings.GetAuthorizer() + spt, err := settings.GetServicePrincipalToken() - assert.NotNil(t, authorizer) + assert.NotNil(t, spt) assert.NoError(t, err) } func TestAuthorizorWithMSI(t *testing.T) { - settings := EnvironmentSettings{ - Values: map[string]string{ - componentSPNClientID: fakeClientID, - componentVaultName: "vaultName", + settings, err := NewEnvironmentSettings( + "keyvault", + map[string]string{ + ClientIDKey: fakeClientID, + "vaultName": "vaultName", }, - } + ) + assert.NoError(t, err) testCertConfig := settings.GetMSI() assert.NotNil(t, testCertConfig) - authorizer, err := testCertConfig.Authorizer() + spt, err := testCertConfig.ServicePrincipalToken() assert.NoError(t, err) - assert.NotNil(t, authorizer) + assert.NotNil(t, spt) } func TestAuthorizorWithMSIAndUserAssignedID(t *testing.T) { - settings := EnvironmentSettings{ - Values: map[string]string{ - componentSPNClientID: fakeClientID, - componentVaultName: "vaultName", + settings, err := NewEnvironmentSettings( + "keyvault", + map[string]string{ + ClientIDKey: fakeClientID, + "vaultName": "vaultName", }, - } + ) + assert.NoError(t, err) testCertConfig := settings.GetMSI() assert.NotNil(t, testCertConfig) - authorizer, err := testCertConfig.Authorizer() + spt, err := testCertConfig.ServicePrincipalToken() assert.NoError(t, err) - assert.NotNil(t, authorizer) + assert.NotNil(t, spt) } func getTestCert() []byte { diff --git a/authentication/azure/metadata-properties.go b/authentication/azure/metadata-properties.go new file mode 100644 index 0000000000..bb48b345a5 --- /dev/null +++ b/authentication/azure/metadata-properties.go @@ -0,0 +1,22 @@ +// ------------------------------------------------------------ +// Copyright (c) Microsoft Corporation and Dapr Contributors. +// Licensed under the MIT License. +// ------------------------------------------------------------ + +package azure + +// Keys for all metadata properties +const ( + CertificateKey = "spnCertificate" + CertificateFileKey = "spnCertificateFile" + CertificatePasswordKey = "spnCertificatePassword" + ClientIDKey = "spnClientId" + ClientSecretKey = "spnClientSecret" + TenantIDKey = "spnTenantId" + // Identifier for the Azure environment + // Allowed values (case-insensitive): AZUREPUBLICCLOUD, AZURECHINACLOUD, AZUREGERMANCLOUD, AZUREUSGOVERNMENTCLOUD + AzureEnvironmentKey = "azureEnvironment" +) + +// Default Azure environment +const DefaultAzureEnvironment = "AZUREPUBLICCLOUD" diff --git a/authentication/azure/storage.go b/authentication/azure/storage.go new file mode 100644 index 0000000000..7ff7b74656 --- /dev/null +++ b/authentication/azure/storage.go @@ -0,0 +1,58 @@ +package azure + +import ( + "fmt" + "time" + + "github.com/Azure/azure-storage-blob-go/azblob" + "github.com/Azure/go-autorest/autorest/azure" + + "github.com/dapr/kit/logger" +) + +const ( + storageAccountKeyKey = "accountKey" +) + +// GetAzureStorageCredentials returns a azblob.Credential object that can be used to authenticate an Azure Blob Storage SDK pipeline. +// First it tries to authenticate using shared key credentials (using an account key) if present. It falls back to attempting to use Azure AD (via a service principal or MSI). +func GetAzureStorageCredentials(log logger.Logger, accountName string, metadata map[string]string) (azblob.Credential, *azure.Environment, error) { + settings, err := NewEnvironmentSettings("storage", metadata) + if err != nil { + return nil, nil, err + } + + // Try using shared key credentials first + accountKey, ok := metadata[storageAccountKeyKey] + if ok && accountKey != "" { + credential, err := azblob.NewSharedKeyCredential(accountName, accountKey) + if err != nil { + return nil, nil, fmt.Errorf("invalid credentials with error: %s", err.Error()) + } + return credential, settings.AzureEnvironment, nil + } + + // Fallback to using Azure AD + spt, err := settings.GetServicePrincipalToken() + if err != nil { + return nil, nil, err + } + var tokenRefresher azblob.TokenRefresher = func(credential azblob.TokenCredential) time.Duration { + log.Debug("Refreshing Azure Storage auth token") + fmt.Println("Refreshing Azure Storage auth token") + err := spt.Refresh() + if err != nil { + panic(err) + } + token := spt.Token() + credential.SetToken(token.AccessToken) + + // Make the token expire 2 minutes earlier to get some extra buffer + exp := token.Expires().Sub(time.Now().Add(2 * time.Minute)) + log.Debug("Received new token, valid for", exp) + fmt.Println("Received new token, valid for", exp) + return exp + } + credential := azblob.NewTokenCredential("", tokenRefresher) + return credential, settings.AzureEnvironment, nil +} diff --git a/secretstores/azure/keyvault/authutils.go b/secretstores/azure/keyvault/authutils.go deleted file mode 100644 index 4c976621ed..0000000000 --- a/secretstores/azure/keyvault/authutils.go +++ /dev/null @@ -1,167 +0,0 @@ -// ------------------------------------------------------------ -// Copyright (c) Microsoft Corporation and Dapr Contributors. -// Licensed under the MIT License. -// ------------------------------------------------------------ - -package keyvault - -import ( - "crypto/rsa" - "crypto/x509" - "fmt" - - "github.com/Azure/go-autorest/autorest" - "github.com/Azure/go-autorest/autorest/adal" - "github.com/Azure/go-autorest/autorest/azure" - "github.com/Azure/go-autorest/autorest/azure/auth" - "golang.org/x/crypto/pkcs12" -) - -// EnvironmentSettings hold settings to authenticate with the Key Vault. -type EnvironmentSettings struct { - Values map[string]string -} - -// CertConfig provides the options to get a bearer authorizer from a client certificate. -type CertConfig struct { - *auth.ClientCertificateConfig - CertificateData []byte -} - -// GetClientCert creates a config object from the available certificate credentials. -// An error is returned if no certificate credentials are available. -func (s EnvironmentSettings) GetClientCert() (CertConfig, error) { - certFilePath, certFilePathPresent := s.Values[componentSPNCertificateFile] - certBytes, certBytesPresent := s.Values[componentSPNCertificate] - certPassword := s.Values[componentSPNCertificatePassword] - clientID := s.Values[componentSPNClientID] - tenantID := s.Values[componentSPNTenantID] - - if !certFilePathPresent && !certBytesPresent { - return CertConfig{}, fmt.Errorf("missing client secret") - } - - authorizer := NewCertConfig(certFilePath, []byte(certBytes), certPassword, clientID, tenantID) - - return authorizer, nil -} - -// NewCertConfig creates an ClientAuthorizer object configured to obtain an Authorizer through Client Credentials. -func NewCertConfig(certificatePath string, certificateBytes []byte, certificatePassword string, clientID string, tenantID string) CertConfig { - return CertConfig{ - &auth.ClientCertificateConfig{ - CertificatePath: certificatePath, - CertificatePassword: certificatePassword, - ClientID: clientID, - TenantID: tenantID, - Resource: azure.PublicCloud.ResourceIdentifiers.KeyVault, - AADEndpoint: azure.PublicCloud.ActiveDirectoryEndpoint, - }, - certificateBytes, - } -} - -// Authorizer gets an authorizer object from client certificate. -func (c CertConfig) Authorizer() (autorest.Authorizer, error) { - if c.ClientCertificateConfig.CertificatePath != "" { - // in standalone mode, component yaml will pass cert path - return c.ClientCertificateConfig.Authorizer() - } else if len(c.CertificateData) > 0 { - // in kubernetes mode, runtime will get the secret from K8S secret store and pass byte array - spToken, err := c.ServicePrincipalTokenByCertBytes() - if err != nil { - return nil, fmt.Errorf("failed to get oauth token from certificate auth: %v", err) - } - - return autorest.NewBearerAuthorizer(spToken), nil - } - - return nil, fmt.Errorf("certificate is not given") -} - -// ServicePrincipalTokenByCertBytes gets the service principal token by CertificateBytes. -func (c CertConfig) ServicePrincipalTokenByCertBytes() (*adal.ServicePrincipalToken, error) { - oauthConfig, err := adal.NewOAuthConfig(c.AADEndpoint, c.TenantID) - if err != nil { - return nil, err - } - - certificate, rsaPrivateKey, err := c.decodePkcs12(c.CertificateData, c.CertificatePassword) - if err != nil { - return nil, fmt.Errorf("failed to decode pkcs12 certificate while creating spt: %v", err) - } - - return adal.NewServicePrincipalTokenFromCertificate(*oauthConfig, c.ClientID, certificate, rsaPrivateKey, c.Resource) -} - -// MSIConfig provides the options to get a bearer authorizer through MSI. -type MSIConfig struct { - Resource string - ClientID string -} - -// NewMSIConfig creates an MSIConfig object configured to obtain an Authorizer through MSI. -func NewMSIConfig() MSIConfig { - return MSIConfig{ - Resource: azure.PublicCloud.ResourceManagerEndpoint, - } -} - -// GetMSI creates a MSI config object from the available client ID. -func (s EnvironmentSettings) GetMSI() MSIConfig { - config := NewMSIConfig() - config.Resource = azure.PublicCloud.ResourceIdentifiers.KeyVault - config.ClientID = s.Values[componentSPNClientID] - - return config -} - -// Authorizer gets the authorizer from MSI. -func (mc MSIConfig) Authorizer() (autorest.Authorizer, error) { - msiEndpoint, err := adal.GetMSIEndpoint() - if err != nil { - return nil, err - } - - var spToken *adal.ServicePrincipalToken - if mc.ClientID == "" { - spToken, err = adal.NewServicePrincipalTokenFromMSI(msiEndpoint, mc.Resource) - if err != nil { - return nil, fmt.Errorf("failed to get oauth token from MSI: %v", err) - } - } else { - spToken, err = adal.NewServicePrincipalTokenFromMSIWithUserAssignedID(msiEndpoint, mc.Resource, mc.ClientID) - if err != nil { - return nil, fmt.Errorf("failed to get oauth token from MSI for user assigned identity: %v", err) - } - } - - return autorest.NewBearerAuthorizer(spToken), nil -} - -// GetAuthorizer creates an Authorizer configured from environment variables in the order: -// 1. Client certificate -// 2. MSI -func (s EnvironmentSettings) GetAuthorizer() (autorest.Authorizer, error) { - // 1. Client Certificate - if c, e := s.GetClientCert(); e == nil { - return c.Authorizer() - } - - // 2. MSI - return s.GetMSI().Authorizer() -} - -func (c CertConfig) decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.PrivateKey, error) { - privateKey, certificate, err := pkcs12.Decode(pkcs, password) - if err != nil { - return nil, nil, err - } - - rsaPrivateKey, isRsaKey := privateKey.(*rsa.PrivateKey) - if !isRsaKey { - return nil, nil, fmt.Errorf("PKCS#12 certificate must contain an RSA private key") - } - - return certificate, rsaPrivateKey, nil -} diff --git a/secretstores/azure/keyvault/keyvault.go b/secretstores/azure/keyvault/keyvault.go index 60674e5e8d..f3d1ea613b 100644 --- a/secretstores/azure/keyvault/keyvault.go +++ b/secretstores/azure/keyvault/keyvault.go @@ -12,32 +12,24 @@ import ( "strings" kv "github.com/Azure/azure-sdk-for-go/profiles/latest/keyvault/keyvault" + + azauth "github.com/dapr/components-contrib/authentication/azure" "github.com/dapr/components-contrib/secretstores" "github.com/dapr/kit/logger" ) // Keyvault secret store component metadata properties +// This is in addition to what's defined in authentication/azure const ( - componentSPNCertificate = "spnCertificate" - componentSPNCertificateFile = "spnCertificateFile" - componentSPNCertificatePassword = "spnCertificatePassword" - componentSPNClientID = "spnClientId" - componentSPNTenantID = "spnTenantId" - componentVaultName = "vaultName" - VersionID = "version_id" - secretItemIDPrefix = "/secrets/" - - // AzureCloud urls refer to https://docs.microsoft.com/en-us/azure/key-vault/general/about-keys-secrets-certificates#dns-suffixes-for-base-url - AzureCloud = ".vault.azure.net" - AzureChinaCloud = ".vault.azure.cn" - AzureUSGov = ".vault.usgovcloudapi.net" - AzureGermanCloud = ".vault.microsoftazure.de" - https = "https://" + componentVaultName = "vaultName" + VersionID = "version_id" + secretItemIDPrefix = "/secrets/" ) type keyvaultSecretStore struct { - vaultName string - vaultClient kv.BaseClient + vaultName string + vaultClient kv.BaseClient + vaultDNSSuffix string logger logger.Logger } @@ -53,8 +45,9 @@ func NewAzureKeyvaultSecretStore(logger logger.Logger) secretstores.SecretStore // Init creates a Kubernetes client func (k *keyvaultSecretStore) Init(metadata secretstores.Metadata) error { - settings := EnvironmentSettings{ - Values: metadata.Properties, + settings, err := azauth.NewEnvironmentSettings("keyvault", metadata.Properties) + if err != nil { + return err } authorizer, err := settings.GetAuthorizer() @@ -63,6 +56,7 @@ func (k *keyvaultSecretStore) Init(metadata secretstores.Metadata) error { } k.vaultName = settings.Values[componentVaultName] + k.vaultDNSSuffix = settings.AzureEnvironment.KeyVaultDNSSuffix return err } @@ -138,17 +132,7 @@ func (k *keyvaultSecretStore) BulkGetSecret(req secretstores.BulkGetSecretReques // getVaultURI returns Azure Key Vault URI func (k *keyvaultSecretStore) getVaultURI() string { - for _, suffix := range []string{AzureCloud, AzureChinaCloud, AzureGermanCloud, AzureUSGov} { - if strings.HasSuffix(k.vaultName, suffix) { - if strings.HasPrefix(k.vaultName, https) { - return k.vaultName - } - - return fmt.Sprintf("%s%s", https, k.vaultName) - } - } - - return fmt.Sprintf("%s%s%s", https, k.vaultName, AzureCloud) + return fmt.Sprintf("https://%s.%s", k.vaultName, k.vaultDNSSuffix) } func (k *keyvaultSecretStore) getMaxResultsFromMetadata(metadata map[string]string) (*int32, error) { diff --git a/state/azure/blobstorage/blobstorage.go b/state/azure/blobstorage/blobstorage.go index 286b41aa33..95619850d8 100644 --- a/state/azure/blobstorage/blobstorage.go +++ b/state/azure/blobstorage/blobstorage.go @@ -39,6 +39,7 @@ import ( "github.com/agrea/ptr" jsoniter "github.com/json-iterator/go" + azauth "github.com/dapr/components-contrib/authentication/azure" "github.com/dapr/components-contrib/state" "github.com/dapr/kit/logger" ) @@ -46,8 +47,8 @@ import ( const ( keyDelimiter = "||" accountNameKey = "accountName" - accountKeyKey = "accountKey" containerNameKey = "containerName" + endpointKey = "endpoint" contentType = "ContentType" contentMD5 = "ContentMD5" contentEncoding = "ContentEncoding" @@ -68,7 +69,6 @@ type StateStore struct { type blobStorageMetadata struct { accountName string - accountKey string containerName string } @@ -79,15 +79,25 @@ func (r *StateStore) Init(metadata state.Metadata) error { return err } - credential, err := azblob.NewSharedKeyCredential(meta.accountName, meta.accountKey) + credential, env, err := azauth.GetAzureStorageCredentials(r.logger, meta.accountName, metadata.Properties) if err != nil { return fmt.Errorf("invalid credentials with error: %s", err.Error()) } p := azblob.NewPipeline(credential, azblob.PipelineOptions{}) - URL, _ := url.Parse(fmt.Sprintf("https://%s.blob.core.windows.net/%s", meta.accountName, meta.containerName)) - containerURL := azblob.NewContainerURL(*URL, p) + var containerURL azblob.ContainerURL + customEndpoint, ok := metadata.Properties[endpointKey] + if ok && customEndpoint != "" { + URL, err := url.Parse(fmt.Sprintf("%s/%s/%s", customEndpoint, meta.accountName, meta.containerName)) + if err != nil { + return err + } + containerURL = azblob.NewContainerURL(*URL, p) + } else { + URL, _ := url.Parse(fmt.Sprintf("https://%s.blob.%s/%s", meta.accountName, env.StorageEndpointSuffix, meta.containerName)) + containerURL = azblob.NewContainerURL(*URL, p) + } ctx := context.Background() _, err = containerURL.Create(ctx, azblob.Metadata{}, azblob.PublicAccessNone) @@ -169,16 +179,10 @@ func getBlobStorageMetadata(metadata map[string]string) (*blobStorageMetadata, e return nil, fmt.Errorf("missing or empty %s field from metadata", accountNameKey) } - if val, ok := metadata[accountKeyKey]; ok && val != "" { - meta.accountKey = val - } else { - return nil, fmt.Errorf("missing of empty %s field from metadata", accountKeyKey) - } - if val, ok := metadata[containerNameKey]; ok && val != "" { meta.containerName = val } else { - return nil, fmt.Errorf("missing of empty %s field from metadata", containerNameKey) + return nil, fmt.Errorf("missing or empty %s field from metadata", containerNameKey) } return &meta, nil diff --git a/state/azure/blobstorage/blobstorage_test.go b/state/azure/blobstorage/blobstorage_test.go index 0d45bf1374..fd4128315b 100644 --- a/state/azure/blobstorage/blobstorage_test.go +++ b/state/azure/blobstorage/blobstorage_test.go @@ -50,13 +50,11 @@ func TestGetBlobStorageMetaData(t *testing.T) { t.Run("All parameters passed and parsed", func(t *testing.T) { m := make(map[string]string) m["accountName"] = "acc" - m["accountKey"] = "key" m["containerName"] = "dapr" meta, err := getBlobStorageMetadata(m) assert.Nil(t, err) assert.Equal(t, "acc", meta.accountName) - assert.Equal(t, "key", meta.accountKey) assert.Equal(t, "dapr", meta.containerName) }) } diff --git a/state/azure/tablestorage/tablestorage.go b/state/azure/tablestorage/tablestorage.go index d87a899864..2250917b5c 100644 --- a/state/azure/tablestorage/tablestorage.go +++ b/state/azure/tablestorage/tablestorage.go @@ -170,13 +170,13 @@ func getTablesMetadata(metadata map[string]string) (*tablesMetadata, error) { if val, ok := metadata[accountKeyKey]; ok && val != "" { meta.accountKey = val } else { - return nil, errors.New(fmt.Sprintf("missing of empty %s field from metadata", accountKeyKey)) + return nil, errors.New(fmt.Sprintf("missing or empty %s field from metadata", accountKeyKey)) } if val, ok := metadata[tableNameKey]; ok && val != "" { meta.tableName = val } else { - return nil, errors.New(fmt.Sprintf("missing of empty %s field from metadata", tableNameKey)) + return nil, errors.New(fmt.Sprintf("missing or empty %s field from metadata", tableNameKey)) } return &meta, nil From fe08c74b76f0c8ca7a943113a1c99e01efa7dfa8 Mon Sep 17 00:00:00 2001 From: Bernd Verst Date: Thu, 5 Aug 2021 16:03:39 -0700 Subject: [PATCH 2/8] Add environment variable aliases --- authentication/azure/auth.go | 57 +++++++++++++-------- authentication/azure/metadata-properties.go | 29 ++++++++--- 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/authentication/azure/auth.go b/authentication/azure/auth.go index eeae105869..15dc0f82d6 100644 --- a/authentication/azure/auth.go +++ b/authentication/azure/auth.go @@ -23,21 +23,21 @@ func NewEnvironmentSettings(resourceName string, values map[string]string) (Envi es := EnvironmentSettings{ Values: values, } - env, err := es.GetEnvironment() + azureEnv, err := es.GetAzureEnvironment() if err != nil { return es, err } - es.AzureEnvironment = env + es.AzureEnvironment = azureEnv switch resourceName { case "azure": // Azure Resource Manager (management plane) - es.Resource = env.TokenAudience + es.Resource = azureEnv.TokenAudience case "keyvault": // Azure Key Vault (data plane) - es.Resource = env.ResourceIdentifiers.KeyVault + es.Resource = azureEnv.ResourceIdentifiers.KeyVault case "storage": // Azure Storage (data plane) - es.Resource = env.ResourceIdentifiers.Storage + es.Resource = azureEnv.ResourceIdentifiers.Storage default: return es, errors.New("invalid resource name: " + resourceName) } @@ -51,8 +51,8 @@ type EnvironmentSettings struct { AzureEnvironment *azure.Environment } -// GetEnvironment returns the Azure environment for a given name. -func (s EnvironmentSettings) GetEnvironment() (*azure.Environment, error) { +// GetAzureEnvironment returns the Azure environment for a given name. +func (s EnvironmentSettings) GetAzureEnvironment() (*azure.Environment, error) { envName, ok := s.Values[AzureEnvironmentKey] if !ok || envName == "" { envName = DefaultAzureEnvironment @@ -99,20 +99,20 @@ func (s EnvironmentSettings) GetServicePrincipalToken() (*adal.ServicePrincipalT // GetClientCredentials creates a config object from the available client credentials. // An error is returned if no certificate credentials are available. func (s EnvironmentSettings) GetClientCredentials() (CredentialsConfig, error) { - env, err := s.GetEnvironment() + azureEnv, err := s.GetAzureEnvironment() if err != nil { return CredentialsConfig{}, err } - clientID := s.Values[ClientIDKey] - clientSecret := s.Values[ClientSecretKey] - tenantID := s.Values[TenantIDKey] + clientID, _ := s.GetEnvironmentValueByKeyAndAlias(ClientIDKey) + clientSecret, _ := s.GetEnvironmentValueByKeyAndAlias(ClientSecretKey) + tenantID, _ := s.GetEnvironmentValueByKeyAndAlias(TenantIDKey) if clientID == "" || clientSecret == "" || tenantID == "" { return CredentialsConfig{}, errors.New("parameters clientId, clientSecret, and tenantId must all be present") } - authorizer := NewCredentialsConfig(clientID, tenantID, clientSecret, s.Resource, env) + authorizer := NewCredentialsConfig(clientID, tenantID, clientSecret, s.Resource, azureEnv) return authorizer, nil } @@ -120,22 +120,22 @@ func (s EnvironmentSettings) GetClientCredentials() (CredentialsConfig, error) { // GetClientCert creates a config object from the available certificate credentials. // An error is returned if no certificate credentials are available. func (s EnvironmentSettings) GetClientCert() (CertConfig, error) { - env, err := s.GetEnvironment() + azureEnv, err := s.GetAzureEnvironment() if err != nil { return CertConfig{}, err } - certFilePath, certFilePathPresent := s.Values[CertificateFileKey] - certBytes, certBytesPresent := s.Values[CertificateKey] - certPassword := s.Values[CertificatePasswordKey] - clientID := s.Values[ClientIDKey] - tenantID := s.Values[TenantIDKey] + certFilePath, certFilePathPresent := s.GetEnvironmentValueByKeyAndAlias(CertificateFileKey) + certBytes, certBytesPresent := s.GetEnvironmentValueByKeyAndAlias(CertificateKey) + certPassword, _ := s.GetEnvironmentValueByKeyAndAlias(CertificatePasswordKey) + clientID, _ := s.GetEnvironmentValueByKeyAndAlias(ClientIDKey) + tenantID, _ := s.GetEnvironmentValueByKeyAndAlias(TenantIDKey) if !certFilePathPresent && !certBytesPresent { return CertConfig{}, fmt.Errorf("missing client certificate") } - authorizer := NewCertConfig(clientID, tenantID, certFilePath, []byte(certBytes), certPassword, s.Resource, env) + authorizer := NewCertConfig(clientID, tenantID, certFilePath, []byte(certBytes), certPassword, s.Resource, azureEnv) return authorizer, nil } @@ -143,8 +143,12 @@ func (s EnvironmentSettings) GetClientCert() (CertConfig, error) { // GetMSI creates a MSI config object from the available client ID. func (s EnvironmentSettings) GetMSI() MSIConfig { config := NewMSIConfig(s.Resource) - config.Resource = azure.PublicCloud.ResourceIdentifiers.KeyVault - config.ClientID = s.Values[ClientIDKey] + azureEnv, err := s.GetAzureEnvironment() + if err != nil { + azureEnv = &azure.PublicCloud + } + config.Resource = azureEnv.ResourceIdentifiers.KeyVault + config.ClientID, _ = s.GetEnvironmentValueByKeyAndAlias(ClientIDKey) return config } @@ -274,3 +278,14 @@ func (mc MSIConfig) ServicePrincipalToken() (*adal.ServicePrincipalToken, error) return spToken, nil } + +// GetAzureEnvironment returns the Azure environment for a given name. +func (s EnvironmentSettings) GetEnvironmentValueByKeyAndAlias(key string) (string, bool) { + if val, ok := s.Values[key]; ok { + return val, true + } + if val, ok := s.Values[KeyAliases[key]]; ok { + return val, true + } + return "", false +} diff --git a/authentication/azure/metadata-properties.go b/authentication/azure/metadata-properties.go index bb48b345a5..919660a657 100644 --- a/authentication/azure/metadata-properties.go +++ b/authentication/azure/metadata-properties.go @@ -7,16 +7,31 @@ package azure // Keys for all metadata properties const ( - CertificateKey = "spnCertificate" - CertificateFileKey = "spnCertificateFile" - CertificatePasswordKey = "spnCertificatePassword" - ClientIDKey = "spnClientId" - ClientSecretKey = "spnClientSecret" - TenantIDKey = "spnTenantId" + CertificateKey = "CERTIFICATE" + CertificateKeyAlias = "spnCertificate" + CertificateFileKey = "CERTIFICATE_FILE" + CertificateFileKeyAlias = "spnCertificateFile" + CertificatePasswordKey = "CERTIFICATE_PASSWORD" + CertificatePasswordKeyAlias = "spnCertificatePassword" + ClientIDKey = "AZURE_CLIENT_ID" + ClientIDKeyAlias = "spnClientId" + ClientSecretKey = "AZURE_CLIENT_SECRET" + ClientSecretKeyAlias = "spnClientSecret" + TenantIDKey = "AZURE_TENANT_ID" + TenantIDKeyAlias = "spnTenantId" // Identifier for the Azure environment // Allowed values (case-insensitive): AZUREPUBLICCLOUD, AZURECHINACLOUD, AZUREGERMANCLOUD, AZUREUSGOVERNMENTCLOUD - AzureEnvironmentKey = "azureEnvironment" + AzureEnvironmentKey = "AZURE_ENVIRONMENT" ) // Default Azure environment const DefaultAzureEnvironment = "AZUREPUBLICCLOUD" + +var KeyAliases = map[string]string{ + CertificateKey: CertificateKeyAlias, + CertificateFileKey: CertificateFileKeyAlias, + CertificatePasswordKey: CertificatePasswordKeyAlias, + ClientIDKey: ClientIDKeyAlias, + ClientSecretKey: ClientSecretKeyAlias, + TenantIDKey: TenantIDKeyAlias, +} From ac9a38bfaf1140ecddfd367c2df1293d9449d301 Mon Sep 17 00:00:00 2001 From: Bernd Verst Date: Thu, 5 Aug 2021 17:00:16 -0700 Subject: [PATCH 3/8] Address linter warnings --- authentication/azure/auth.go | 4 ++++ authentication/azure/metadata-properties.go | 4 ++-- authentication/azure/storage.go | 6 ++++-- state/azure/blobstorage/blobstorage.go | 4 ++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/authentication/azure/auth.go b/authentication/azure/auth.go index 15dc0f82d6..0599ce5a31 100644 --- a/authentication/azure/auth.go +++ b/authentication/azure/auth.go @@ -41,6 +41,7 @@ func NewEnvironmentSettings(resourceName string, values map[string]string) (Envi default: return es, errors.New("invalid resource name: " + resourceName) } + return es, nil } @@ -61,6 +62,7 @@ func (s EnvironmentSettings) GetAzureEnvironment() (*azure.Environment, error) { if err != nil { return nil, err } + return &env, err } @@ -177,6 +179,7 @@ func (c CredentialsConfig) ServicePrincipalToken() (*adal.ServicePrincipalToken, if err != nil { return nil, err } + return adal.NewServicePrincipalToken(*oauthConfig, c.ClientID, c.ClientSecret, c.Resource) } @@ -287,5 +290,6 @@ func (s EnvironmentSettings) GetEnvironmentValueByKeyAndAlias(key string) (strin if val, ok := s.Values[KeyAliases[key]]; ok { return val, true } + return "", false } diff --git a/authentication/azure/metadata-properties.go b/authentication/azure/metadata-properties.go index 919660a657..5fa453d25f 100644 --- a/authentication/azure/metadata-properties.go +++ b/authentication/azure/metadata-properties.go @@ -15,7 +15,7 @@ const ( CertificatePasswordKeyAlias = "spnCertificatePassword" ClientIDKey = "AZURE_CLIENT_ID" ClientIDKeyAlias = "spnClientId" - ClientSecretKey = "AZURE_CLIENT_SECRET" + ClientSecretKey = "AZURE_CLIENT_SECRET" // nolint: gosec ClientSecretKeyAlias = "spnClientSecret" TenantIDKey = "AZURE_TENANT_ID" TenantIDKeyAlias = "spnTenantId" @@ -27,7 +27,7 @@ const ( // Default Azure environment const DefaultAzureEnvironment = "AZUREPUBLICCLOUD" -var KeyAliases = map[string]string{ +var KeyAliases = map[string]string{ // nolint: gochecknoglobals CertificateKey: CertificateKeyAlias, CertificateFileKey: CertificateFileKeyAlias, CertificatePasswordKey: CertificatePasswordKeyAlias, diff --git a/authentication/azure/storage.go b/authentication/azure/storage.go index 7ff7b74656..ddee21f429 100644 --- a/authentication/azure/storage.go +++ b/authentication/azure/storage.go @@ -25,10 +25,11 @@ func GetAzureStorageCredentials(log logger.Logger, accountName string, metadata // Try using shared key credentials first accountKey, ok := metadata[storageAccountKeyKey] if ok && accountKey != "" { - credential, err := azblob.NewSharedKeyCredential(accountName, accountKey) + credential, newSharedKeyErr := azblob.NewSharedKeyCredential(accountName, accountKey) if err != nil { - return nil, nil, fmt.Errorf("invalid credentials with error: %s", err.Error()) + return nil, nil, fmt.Errorf("invalid credentials with error: %s", newSharedKeyErr.Error()) } + return credential, settings.AzureEnvironment, nil } @@ -54,5 +55,6 @@ func GetAzureStorageCredentials(log logger.Logger, accountName string, metadata return exp } credential := azblob.NewTokenCredential("", tokenRefresher) + return credential, settings.AzureEnvironment, nil } diff --git a/state/azure/blobstorage/blobstorage.go b/state/azure/blobstorage/blobstorage.go index 95619850d8..6b518a86ae 100644 --- a/state/azure/blobstorage/blobstorage.go +++ b/state/azure/blobstorage/blobstorage.go @@ -89,8 +89,8 @@ func (r *StateStore) Init(metadata state.Metadata) error { var containerURL azblob.ContainerURL customEndpoint, ok := metadata.Properties[endpointKey] if ok && customEndpoint != "" { - URL, err := url.Parse(fmt.Sprintf("%s/%s/%s", customEndpoint, meta.accountName, meta.containerName)) - if err != nil { + URL, parseErr := url.Parse(fmt.Sprintf("%s/%s/%s", customEndpoint, meta.accountName, meta.containerName)) + if parseErr != nil { return err } containerURL = azblob.NewContainerURL(*URL, p) From cdb578a6887ec028537a4fa179e1efb32950ed09 Mon Sep 17 00:00:00 2001 From: Bernd Verst Date: Thu, 5 Aug 2021 17:06:07 -0700 Subject: [PATCH 4/8] another lint thing --- authentication/azure/storage.go | 1 + 1 file changed, 1 insertion(+) diff --git a/authentication/azure/storage.go b/authentication/azure/storage.go index ddee21f429..463ea2dd4e 100644 --- a/authentication/azure/storage.go +++ b/authentication/azure/storage.go @@ -52,6 +52,7 @@ func GetAzureStorageCredentials(log logger.Logger, accountName string, metadata exp := token.Expires().Sub(time.Now().Add(2 * time.Minute)) log.Debug("Received new token, valid for", exp) fmt.Println("Received new token, valid for", exp) + return exp } credential := azblob.NewTokenCredential("", tokenRefresher) From e5e55742628d36bd8b1b8f3bd702e5e6598bdec2 Mon Sep 17 00:00:00 2001 From: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> Date: Fri, 6 Aug 2021 22:24:02 +0200 Subject: [PATCH 5/8] Fixed typo in method description --- secretstores/azure/keyvault/keyvault.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/secretstores/azure/keyvault/keyvault.go b/secretstores/azure/keyvault/keyvault.go index f3d1ea613b..66cff078a4 100644 --- a/secretstores/azure/keyvault/keyvault.go +++ b/secretstores/azure/keyvault/keyvault.go @@ -34,7 +34,7 @@ type keyvaultSecretStore struct { logger logger.Logger } -// NewAzureKeyvaultSecretStore returns a new Kubernetes secret store +// NewAzureKeyvaultSecretStore returns a new Azure Key Vault secret store func NewAzureKeyvaultSecretStore(logger logger.Logger) secretstores.SecretStore { return &keyvaultSecretStore{ vaultName: "", @@ -43,7 +43,7 @@ func NewAzureKeyvaultSecretStore(logger logger.Logger) secretstores.SecretStore } } -// Init creates a Kubernetes client +// Init creates a Azure Key Vault client func (k *keyvaultSecretStore) Init(metadata secretstores.Metadata) error { settings, err := azauth.NewEnvironmentSettings("keyvault", metadata.Properties) if err != nil { From 9e881748dc2ef8be5fdef52bb692d6130d3dd715 Mon Sep 17 00:00:00 2001 From: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> Date: Fri, 6 Aug 2021 23:08:39 +0200 Subject: [PATCH 6/8] Updated metadata key names so they're more consistent --- authentication/azure/auth.go | 46 ++++++++-------- authentication/azure/auth_test.go | 58 ++++++++++----------- authentication/azure/metadata-properties.go | 43 +++++++-------- 3 files changed, 70 insertions(+), 77 deletions(-) diff --git a/authentication/azure/auth.go b/authentication/azure/auth.go index 0599ce5a31..c3238af045 100644 --- a/authentication/azure/auth.go +++ b/authentication/azure/auth.go @@ -54,7 +54,7 @@ type EnvironmentSettings struct { // GetAzureEnvironment returns the Azure environment for a given name. func (s EnvironmentSettings) GetAzureEnvironment() (*azure.Environment, error) { - envName, ok := s.Values[AzureEnvironmentKey] + envName, ok := s.GetEnvironment("AzureEnvironment") if !ok || envName == "" { envName = DefaultAzureEnvironment } @@ -106,9 +106,9 @@ func (s EnvironmentSettings) GetClientCredentials() (CredentialsConfig, error) { return CredentialsConfig{}, err } - clientID, _ := s.GetEnvironmentValueByKeyAndAlias(ClientIDKey) - clientSecret, _ := s.GetEnvironmentValueByKeyAndAlias(ClientSecretKey) - tenantID, _ := s.GetEnvironmentValueByKeyAndAlias(TenantIDKey) + clientID, _ := s.GetEnvironment("ClientID") + clientSecret, _ := s.GetEnvironment("ClientSecret") + tenantID, _ := s.GetEnvironment("TenantID") if clientID == "" || clientSecret == "" || tenantID == "" { return CredentialsConfig{}, errors.New("parameters clientId, clientSecret, and tenantId must all be present") @@ -127,11 +127,11 @@ func (s EnvironmentSettings) GetClientCert() (CertConfig, error) { return CertConfig{}, err } - certFilePath, certFilePathPresent := s.GetEnvironmentValueByKeyAndAlias(CertificateFileKey) - certBytes, certBytesPresent := s.GetEnvironmentValueByKeyAndAlias(CertificateKey) - certPassword, _ := s.GetEnvironmentValueByKeyAndAlias(CertificatePasswordKey) - clientID, _ := s.GetEnvironmentValueByKeyAndAlias(ClientIDKey) - tenantID, _ := s.GetEnvironmentValueByKeyAndAlias(TenantIDKey) + certFilePath, certFilePathPresent := s.GetEnvironment("CertificateFile") + certBytes, certBytesPresent := s.GetEnvironment("Certificate") + certPassword, _ := s.GetEnvironment("CertificatePassword") + clientID, _ := s.GetEnvironment("ClientID") + tenantID, _ := s.GetEnvironment("TenantID") if !certFilePathPresent && !certBytesPresent { return CertConfig{}, fmt.Errorf("missing client certificate") @@ -145,13 +145,8 @@ func (s EnvironmentSettings) GetClientCert() (CertConfig, error) { // GetMSI creates a MSI config object from the available client ID. func (s EnvironmentSettings) GetMSI() MSIConfig { config := NewMSIConfig(s.Resource) - azureEnv, err := s.GetAzureEnvironment() - if err != nil { - azureEnv = &azure.PublicCloud - } - config.Resource = azureEnv.ResourceIdentifiers.KeyVault - config.ClientID, _ = s.GetEnvironmentValueByKeyAndAlias(ClientIDKey) - + // This is optional and it's ok if value is empty + config.ClientID, _ = s.GetEnvironment("ClientID") return config } @@ -282,14 +277,17 @@ func (mc MSIConfig) ServicePrincipalToken() (*adal.ServicePrincipalToken, error) return spToken, nil } -// GetAzureEnvironment returns the Azure environment for a given name. -func (s EnvironmentSettings) GetEnvironmentValueByKeyAndAlias(key string) (string, bool) { - if val, ok := s.Values[key]; ok { - return val, true - } - if val, ok := s.Values[KeyAliases[key]]; ok { - return val, true +// GetAzureEnvironment returns the Azure environment for a given name, supporting aliases too. +func (s EnvironmentSettings) GetEnvironment(key string) (string, bool) { + var ( + val string + ok bool + ) + for _, k := range MetadataKeys[key] { + val, ok = s.Values[k] + if ok { + return val, true + } } - return "", false } diff --git a/authentication/azure/auth_test.go b/authentication/azure/auth_test.go index b3d862a229..aefdc80bd8 100644 --- a/authentication/azure/auth_test.go +++ b/authentication/azure/auth_test.go @@ -26,12 +26,12 @@ func TestGetClientCert(t *testing.T) { settings, err := NewEnvironmentSettings( "keyvault", map[string]string{ - CertificateFileKey: "testfile", - CertificateKey: "testcert", - CertificatePasswordKey: "1234", - ClientIDKey: fakeClientID, - TenantIDKey: fakeTenantID, - "vaultName": "vaultName", + "azureCertificateFile": "testfile", + "azureCertificate": "testcert", + "azureCertificatePassword": "1234", + "azureClientID": fakeClientID, + "azureTenantID": fakeTenantID, + "vaultName": "vaultName", }, ) assert.NoError(t, err) @@ -58,11 +58,11 @@ func TestAuthorizorWithCertFile(t *testing.T) { settings, err := NewEnvironmentSettings( "keyvault", map[string]string{ - CertificateFileKey: testCertFileName, - CertificatePasswordKey: "", - ClientIDKey: fakeClientID, - TenantIDKey: fakeTenantID, - "vaultName": "vaultName", + "azureCertificateFile": testCertFileName, + "azureCertificatePassword": "", + "azureClientId": fakeClientID, + "azureTenantId": fakeTenantID, + "vaultName": "vaultName", }, ) assert.NoError(t, err) @@ -86,11 +86,11 @@ func TestAuthorizorWithCertBytes(t *testing.T) { settings, err := NewEnvironmentSettings( "keyvault", map[string]string{ - CertificateKey: string(certBytes), - CertificatePasswordKey: "", - ClientIDKey: fakeClientID, - TenantIDKey: fakeTenantID, - "vaultName": "vaultName", + "azureCertificate": string(certBytes), + "azureCertificatePassword": "", + "azureClientId": fakeClientID, + "azureTenantId": fakeTenantID, + "vaultName": "vaultName", }, ) assert.NoError(t, err) @@ -111,11 +111,11 @@ func TestAuthorizorWithCertBytes(t *testing.T) { settings, err := NewEnvironmentSettings( "keyvault", map[string]string{ - CertificateKey: string(certBytes[0:20]), - CertificatePasswordKey: "", - ClientIDKey: fakeClientID, - TenantIDKey: fakeTenantID, - "vaultName": "vaultName", + "azureCertificate": string(certBytes[0:20]), + "azureCertificatePassword": "", + "azureClientId": fakeClientID, + "azureTenantId": fakeTenantID, + "vaultName": "vaultName", }, ) assert.NoError(t, err) @@ -133,8 +133,8 @@ func TestGetMSI(t *testing.T) { settings, err := NewEnvironmentSettings( "keyvault", map[string]string{ - ClientIDKey: fakeClientID, - "vaultName": "vaultName", + "azureClientId": fakeClientID, + "vaultName": "vaultName", }, ) assert.NoError(t, err) @@ -149,8 +149,8 @@ func TestFallbackToMSI(t *testing.T) { settings, err := NewEnvironmentSettings( "keyvault", map[string]string{ - ClientIDKey: fakeClientID, - "vaultName": "vaultName", + "azureClientId": fakeClientID, + "vaultName": "vaultName", }, ) assert.NoError(t, err) @@ -165,8 +165,8 @@ func TestAuthorizorWithMSI(t *testing.T) { settings, err := NewEnvironmentSettings( "keyvault", map[string]string{ - ClientIDKey: fakeClientID, - "vaultName": "vaultName", + "azureClientId": fakeClientID, + "vaultName": "vaultName", }, ) assert.NoError(t, err) @@ -183,8 +183,8 @@ func TestAuthorizorWithMSIAndUserAssignedID(t *testing.T) { settings, err := NewEnvironmentSettings( "keyvault", map[string]string{ - ClientIDKey: fakeClientID, - "vaultName": "vaultName", + "azureClientId": fakeClientID, + "vaultName": "vaultName", }, ) assert.NoError(t, err) diff --git a/authentication/azure/metadata-properties.go b/authentication/azure/metadata-properties.go index 5fa453d25f..d4e520e476 100644 --- a/authentication/azure/metadata-properties.go +++ b/authentication/azure/metadata-properties.go @@ -6,32 +6,27 @@ package azure // Keys for all metadata properties -const ( - CertificateKey = "CERTIFICATE" - CertificateKeyAlias = "spnCertificate" - CertificateFileKey = "CERTIFICATE_FILE" - CertificateFileKeyAlias = "spnCertificateFile" - CertificatePasswordKey = "CERTIFICATE_PASSWORD" - CertificatePasswordKeyAlias = "spnCertificatePassword" - ClientIDKey = "AZURE_CLIENT_ID" - ClientIDKeyAlias = "spnClientId" - ClientSecretKey = "AZURE_CLIENT_SECRET" // nolint: gosec - ClientSecretKeyAlias = "spnClientSecret" - TenantIDKey = "AZURE_TENANT_ID" - TenantIDKeyAlias = "spnTenantId" +// clientId, clientSecret, tenantId are supported for backwards-compatibility as they're used by some components, but should be considered deprecated +var MetadataKeys = map[string][]string{ + // Certificate contains the raw certificate data + "Certificate": {"azureCertificate", "spnCertificate"}, + // Path to a certificate + "CertificateFile": {"azureCertificateFile", "spnCertificateFile"}, + // Password for the certificate + "CertificatePassword": {"azureCertificatePassword", "spnCertificatePassword"}, + // Client ID for the Service Principal + // The "clientId" alias is supported for backwards-compatibility as it's used by some components, but should be considered deprecated + "ClientID": {"azureClientId", "spnClientId", "clientId"}, + // Client secret for the Service Principal + // The "clientSecret" alias is supported for backwards-compatibility as it's used by some components, but should be considered deprecated + "ClientSecret": {"azureClientSecret", "spnClientSecret", "clientSecret"}, // nolint: gosec + // Tenant ID for the Service Principal + // The "tenantId" alias is supported for backwards-compatibility as it's used by some components, but should be considered deprecated + "TenantID": {"azureTenantId", "spnTenantId", "tenantId"}, // Identifier for the Azure environment // Allowed values (case-insensitive): AZUREPUBLICCLOUD, AZURECHINACLOUD, AZUREGERMANCLOUD, AZUREUSGOVERNMENTCLOUD - AzureEnvironmentKey = "AZURE_ENVIRONMENT" -) + "AzureEnvironment": {"azureEnvironment"}, +} // Default Azure environment const DefaultAzureEnvironment = "AZUREPUBLICCLOUD" - -var KeyAliases = map[string]string{ // nolint: gochecknoglobals - CertificateKey: CertificateKeyAlias, - CertificateFileKey: CertificateFileKeyAlias, - CertificatePasswordKey: CertificatePasswordKeyAlias, - ClientIDKey: ClientIDKeyAlias, - ClientSecretKey: ClientSecretKeyAlias, - TenantIDKey: TenantIDKeyAlias, -} From a0578ecbac3e984096c9eac84f740a37ad2001bf Mon Sep 17 00:00:00 2001 From: Bernd Verst Date: Fri, 6 Aug 2021 14:51:16 -0700 Subject: [PATCH 7/8] Fix test --- authentication/azure/auth_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/authentication/azure/auth_test.go b/authentication/azure/auth_test.go index aefdc80bd8..41dd1cb344 100644 --- a/authentication/azure/auth_test.go +++ b/authentication/azure/auth_test.go @@ -29,8 +29,8 @@ func TestGetClientCert(t *testing.T) { "azureCertificateFile": "testfile", "azureCertificate": "testcert", "azureCertificatePassword": "1234", - "azureClientID": fakeClientID, - "azureTenantID": fakeTenantID, + "azureClientId": fakeClientID, + "azureTenantId": fakeTenantID, "vaultName": "vaultName", }, ) From 1afc0b79535022ac10f6820c840568fd0b560776 Mon Sep 17 00:00:00 2001 From: Bernd Verst Date: Fri, 6 Aug 2021 14:56:59 -0700 Subject: [PATCH 8/8] Some more linter things --- authentication/azure/auth.go | 2 ++ authentication/azure/metadata-properties.go | 9 +++++---- authentication/azure/storage.go | 2 -- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/authentication/azure/auth.go b/authentication/azure/auth.go index c3238af045..5c1fee6db9 100644 --- a/authentication/azure/auth.go +++ b/authentication/azure/auth.go @@ -147,6 +147,7 @@ func (s EnvironmentSettings) GetMSI() MSIConfig { config := NewMSIConfig(s.Resource) // This is optional and it's ok if value is empty config.ClientID, _ = s.GetEnvironment("ClientID") + return config } @@ -289,5 +290,6 @@ func (s EnvironmentSettings) GetEnvironment(key string) (string, bool) { return val, true } } + return "", false } diff --git a/authentication/azure/metadata-properties.go b/authentication/azure/metadata-properties.go index d4e520e476..306f337b5f 100644 --- a/authentication/azure/metadata-properties.go +++ b/authentication/azure/metadata-properties.go @@ -5,9 +5,10 @@ package azure -// Keys for all metadata properties -// clientId, clientSecret, tenantId are supported for backwards-compatibility as they're used by some components, but should be considered deprecated -var MetadataKeys = map[string][]string{ +// MetadataKeys : Keys for all metadata properties +var MetadataKeys = map[string][]string{ // nolint: gochecknoglobals + // clientId, clientSecret, tenantId are supported for backwards-compatibility as they're used by some components, but should be considered deprecated + // Certificate contains the raw certificate data "Certificate": {"azureCertificate", "spnCertificate"}, // Path to a certificate @@ -19,7 +20,7 @@ var MetadataKeys = map[string][]string{ "ClientID": {"azureClientId", "spnClientId", "clientId"}, // Client secret for the Service Principal // The "clientSecret" alias is supported for backwards-compatibility as it's used by some components, but should be considered deprecated - "ClientSecret": {"azureClientSecret", "spnClientSecret", "clientSecret"}, // nolint: gosec + "ClientSecret": {"azureClientSecret", "spnClientSecret", "clientSecret"}, // Tenant ID for the Service Principal // The "tenantId" alias is supported for backwards-compatibility as it's used by some components, but should be considered deprecated "TenantID": {"azureTenantId", "spnTenantId", "tenantId"}, diff --git a/authentication/azure/storage.go b/authentication/azure/storage.go index 463ea2dd4e..1369fe3e3e 100644 --- a/authentication/azure/storage.go +++ b/authentication/azure/storage.go @@ -40,7 +40,6 @@ func GetAzureStorageCredentials(log logger.Logger, accountName string, metadata } var tokenRefresher azblob.TokenRefresher = func(credential azblob.TokenCredential) time.Duration { log.Debug("Refreshing Azure Storage auth token") - fmt.Println("Refreshing Azure Storage auth token") err := spt.Refresh() if err != nil { panic(err) @@ -51,7 +50,6 @@ func GetAzureStorageCredentials(log logger.Logger, accountName string, metadata // Make the token expire 2 minutes earlier to get some extra buffer exp := token.Expires().Sub(time.Now().Add(2 * time.Minute)) log.Debug("Received new token, valid for", exp) - fmt.Println("Received new token, valid for", exp) return exp }