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.

Sign up for GitHub

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

migrate from adal to azidentity #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+182 −85
Open

migrate from adal to azidentity #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
85 changes: 61 additions & 24 deletions azure/azure_kv.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,31 @@ import (
"context"
"encoding/json"
"errors"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/keyvault/2016-10-01/keyvault"
"github.com/Azure/go-autorest/autorest/to"
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets"
"github.com/libopenstorage/secrets"
"github.com/portworx/sched-ops/task"
)

const (
Name = secrets.TypeAzure
AzureCloud = "AzurePublicCloud"
Name = secrets.TypeAzure
AzureCloud = "AzurePublicCloud"
AzureGovernment = "AzureUSGovernment"
AzureChina = "AzureChinaCloud"
// AzureTenantID for Azure Active Directory
AzureTenantID = "AZURE_TENANT_ID"
// AzureClientID of service principal account
AzureClientID = "AZURE_CLIENT_ID"
// AzureClientSecret of service principal account
AzureClientSecret = "AZURE_CLIENT_SECRET"
// AzureClientCertPath is path of the client certificate
AzureClientCertPath = "AZURE_CLIENT_CERT_PATH"
// AzureClientCertPassword is the password of the private key
AzureClientCertPassword = "AZURE_CIENT_CERT_PASSWORD"
// AzureEnviornment to connect
AzureEnviornment = "AZURE_ENVIRONMENT"
// AzureVaultURI of azure key vault
Expand All @@ -37,6 +45,7 @@ var (
ErrAzureTenantIDNotSet = errors.New("AZURE_TENANT_ID not set.")
ErrAzureClientIDNotSet = errors.New("AZURE_CLIENT_ID not set.")
ErrAzureSecretIDNotSet = errors.New("AZURE_SECRET_ID not set.")
ErrAzureAuthMedhodNotSet = errors.New("AZURE_SECRET_ID or AZURE_CLIENT_CERT_PATH not set")
ErrAzureVaultURLNotSet = errors.New("AZURE_VAULT_URL not set.")
ErrAzureEnvironmentNotset = errors.New("AZURE_ENVIRONMENT not set.")
ErrAzureConfigMissing = errors.New("AzureConfig is not provided")
Expand All @@ -45,7 +54,7 @@ var (
)

type azureSecrets struct {
kv keyvault.BaseClient
kv azsecrets.Client
baseURL string
}

Expand All @@ -62,26 +71,34 @@ func New(
return nil, ErrAzureClientIDNotSet
}
secretID := getAzureKVParams(secretConfig, AzureClientSecret)
if secretID == "" {
return nil, ErrAzureSecretIDNotSet
}
envName := getAzureKVParams(secretConfig, AzureEnviornment)
if envName == "" {
// we set back to default AzurePublicCloud
envName = AzureCloud
}
clientCertPath := getAzureKVParams(secretConfig, AzureClientCertPath)
clientCertPassword := getAzureKVParams(secretConfig, AzureClientCertPassword)

vaultURL := getAzureKVParams(secretConfig, AzureVaultURL)
if vaultURL == "" {
return nil, ErrAzureVaultURLNotSet
}

client, err := getAzureVaultClient(clientID, secretID, tenantID, envName)
if err != nil {
return nil, err
clientOpts := getAzureClientOptions(secretConfig)

var client *azsecrets.Client
var err error
if secretID != "" {
client, err = getAzureVaultClient(clientID, secretID, tenantID, vaultURL, clientOpts)
if err != nil {
return nil, err
}
} else if clientCertPath != "" {
client, err = getAzureVaultClientWithCert(clientID, tenantID, vaultURL, clientCertPath, clientCertPassword, clientOpts)
if err != nil {
return nil, err
}
sp98 marked this conversation as resolved.
Show resolved Hide resolved
} else {
return nil, ErrAzureAuthMedhodNotSet
}

return &azureSecrets{
kv: client,
kv: *client,
baseURL: vaultURL,
}, nil
}
Expand All @@ -98,8 +115,14 @@ func (az *azureSecrets) GetSecret(
}

t := func() (interface{}, bool, error) {
secretResp, err := az.kv.GetSecret(ctx, az.baseURL, secretID, "")
// passing empty version to always get the latest version of the secret.
secretResp, err := az.kv.GetSecret(ctx, secretID, "", nil)
if err != nil {
// don't retry if the secret key was not found
responseError, ok := err.(*azcore.ResponseError)
if ok && responseError.StatusCode == 404 {
return nil, false, secrets.ErrSecretNotFound
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you return this error instead. - ErrInvalidSecretId since all the other providers are returning it when a secret is not found.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adityadani Sorry for the delay. I'll update the PR this week.

}
return nil, true, err
}
return secretResp, false, nil
Expand All @@ -109,7 +132,7 @@ func (az *azureSecrets) GetSecret(
return nil, secrets.NoVersion, err
}

secretResp, ok := resp.(keyvault.SecretBundle)
secretResp, ok := resp.(azsecrets.GetSecretResponse)
Copy link

@iPraveenParihar iPraveenParihar Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Azure now returns secret version.
can we return version as well?

secretResp.ID.Version()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other KMS in the library is returning the version currently. So I think for now, we can keep it as it is and revisit this later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWS Secrets Manager returns the version.The only reason all providers have not been updated to return the version is lack of bandwidth to check and test if each one of them If we are modifying this provider and you are able to verify its correctness we can modify it

if !ok || secretResp.Value == nil {
return nil, secrets.NoVersion, ErrInvalidSecretResp
}
Expand All @@ -133,7 +156,7 @@ func (az *azureSecrets) PutSecret(
ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout)
defer cancel()

var secretResp keyvault.SecretBundle
var secretResp azsecrets.SetSecretResponse
if secretName == "" {
return secrets.NoVersion, secrets.ErrEmptySecretId
}
Expand All @@ -146,10 +169,10 @@ func (az *azureSecrets) PutSecret(
return secrets.NoVersion, err
}

valueStr := string(value)
t := func() (interface{}, bool, error) {
secretResp, err = az.kv.SetSecret(ctx, az.baseURL, secretName, keyvault.SecretSetParameters{
Value: to.StringPtr(string(value)),
})
params := azsecrets.SetSecretParameters{Value: &valueStr}
secretResp, err = az.kv.SetSecret(ctx, secretName, params, nil)
if err != nil {
return nil, true, err
}
Expand All @@ -169,7 +192,7 @@ func (az *azureSecrets) DeleteSecret(
if secretName == "" {
return secrets.ErrEmptySecretId
}
_, err := az.kv.DeleteSecret(ctx, az.baseURL, secretName)
_, err := az.kv.DeleteSecret(ctx, secretName, nil)

return err
}
Expand Down Expand Up @@ -213,3 +236,17 @@ func init() {
panic(err.Error())
}
}

func getAzureClientOptions(secretConfig map[string]interface{}) azcore.ClientOptions {
opts := azcore.ClientOptions{}
cloudEnv := getAzureKVParams(secretConfig, AzureEnviornment)
if strings.EqualFold(cloudEnv, AzureGovernment) {
opts.Cloud = cloud.AzureGovernment
} else if strings.EqualFold(cloudEnv, AzureChina) {
opts.Cloud = cloud.AzureChina
} else if cloudEnv == AzureCloud || cloudEnv == "" {
opts.Cloud = cloud.AzurePublic
}
return opts

}
61 changes: 38 additions & 23 deletions azure/azure_kv_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package azure

import (
"net/url"
"fmt"
"os"
"strings"

"github.com/Azure/azure-sdk-for-go/services/keyvault/2016-10-01/keyvault"
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/adal"
"github.com/Azure/go-autorest/autorest/azure"
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets"
)

func getAzureKVParams(secretConfig map[string]interface{}, name string) string {
Expand All @@ -19,29 +17,46 @@ func getAzureKVParams(secretConfig map[string]interface{}, name string) string {
}
}

func getAzureVaultClient(clientID, secretID, tenantID, envName string) (keyvault.BaseClient, error) {
var environment *azure.Environment
alternateEndpoint, _ := url.Parse(
"https://login.windows.net/" + tenantID + "/oauth2/token")
func getAzureVaultClient(clientID, secretID, tenantID, vaultURL string, opts azcore.ClientOptions) (*azsecrets.Client, error) {
cred, err := azidentity.NewClientSecretCredential(tenantID, clientID, secretID, &azidentity.ClientSecretCredentialOptions{ClientOptions: opts})
if err != nil {
return nil, fmt.Errorf("failed to get client secret credentials. %v", err)
}
client, err := azsecrets.NewClient(vaultURL, cred, &azsecrets.ClientOptions{ClientOptions: opts})
if err != nil {
return nil, fmt.Errorf("failed to get client to access azure kv secrets. %v", err)
}

return client, nil
}

func getAzureVaultClientWithCert(clientID, tenantID, vaultURL, certPath, certPassword string, opts azcore.ClientOptions) (*azsecrets.Client, error) {
certData, err := os.ReadFile(certPath)
if err != nil {
return nil, fmt.Errorf("failed read certificate from path %q. %v", certPath, err)
}

keyClient := keyvault.New()
env, err := azure.EnvironmentFromName(envName)
var passphrase []byte
if certPassword == "" {
passphrase = nil
} else {
passphrase = []byte(certPassword)
}

certs, key, err := azidentity.ParseCertificates(certData, passphrase)
if err != nil {
return keyClient, err
return nil, fmt.Errorf("failed load certificate and private key. %v", err)
}
environment = &env
oauthconfig, err := adal.NewOAuthConfig(
environment.ActiveDirectoryEndpoint, tenantID)

cred, err := azidentity.NewClientCertificateCredential(tenantID, clientID, certs, key, &azidentity.ClientCertificateCredentialOptions{ClientOptions: opts})
if err != nil {
return keyClient, err
return nil, fmt.Errorf("failed to construct client certificate credentials. %v", err)
}
oauthconfig.AuthorizeEndpoint = *alternateEndpoint

token, err := adal.NewServicePrincipalToken(
*oauthconfig, clientID, secretID, strings.TrimSuffix(environment.KeyVaultEndpoint, "/"))
client, err := azsecrets.NewClient(vaultURL, cred, &azsecrets.ClientOptions{ClientOptions: opts})
if err != nil {
return keyClient, err
return nil, fmt.Errorf("failed to get client to access azure kv secrets. %v", err)
}
keyClient.Authorizer = autorest.NewBearerAuthorizer(token)
return keyClient, nil

return client, nil
}
14 changes: 6 additions & 8 deletions azure/azure_kv_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,24 @@ func TestNew(t *testing.T) {
os.Unsetenv("AZURE_TENANT_ID")
os.Unsetenv("AZURE_CLIENT_ID")
os.Unsetenv("AZURE_CLIENT_SECRET")
os.Unsetenv("AZURE_CLIENT_CERT_PATH")
os.Unsetenv("AZURE_ENVIRONMENT")
os.Unsetenv("AZURE_VAULT_URL")

// nil secret config
_, err := New(nil)
assert.Equal(t, err, ErrAzureTenantIDNotSet)
assert.Equal(t, ErrAzureTenantIDNotSet, err)
os.Setenv("AZURE_TENANT_ID", "invalid_tenant_id")

_, err = New(nil)
assert.Equal(t, err, ErrAzureClientIDNotSet)
assert.Equal(t, ErrAzureClientIDNotSet, err)
os.Setenv("AZURE_CLIENT_ID", "invalid-client-id")

_, err = New(nil)
assert.Equal(t, err, ErrAzureSecretIDNotSet)
os.Setenv("AZURE_CLIENT_SECRET", "invalid-secret-id")

_, err = New(nil)
assert.Equal(t, err, ErrAzureVaultURLNotSet)
assert.Equal(t, ErrAzureVaultURLNotSet, err)
os.Setenv("AZURE_VAULT_URL", "invalid-vault-url")

_, err = New(nil)
assert.NoError(t, err, "Unepxected error on New")
assert.Equal(t, ErrAzureAuthMedhodNotSet, err)
os.Setenv("AZURE_CLIENT_SECRET", "invalid-secret-id")
}
12 changes: 4 additions & 8 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ module github.com/libopenstorage/secrets
go 1.13

require (
github.com/Azure/azure-sdk-for-go v62.0.0+incompatible
github.com/Azure/go-autorest/autorest v0.11.27
github.com/Azure/go-autorest/autorest/adal v0.9.20
github.com/Azure/go-autorest/autorest/to v0.4.0
github.com/IBM/keyprotect-go-client v0.5.1
github.com/aws/aws-sdk-go v1.44.164
github.com/golang/mock v1.6.0
Expand All @@ -18,21 +14,21 @@ require (
github.com/portworx/kvdb v0.0.0-20200929023115-b312c7519467
github.com/portworx/sched-ops v1.20.4-rc1
github.com/sirupsen/logrus v1.9.0
github.com/stretchr/testify v1.8.0
github.com/stretchr/testify v1.8.4
google.golang.org/api v0.83.0
k8s.io/client-go v12.0.0+incompatible
)

require (
github.com/Azure/go-autorest/autorest/validation v0.3.1 // indirect
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.1
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.5.1
github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets v0.12.0
github.com/cenkalti/backoff v2.2.1+incompatible // indirect
github.com/cenkalti/backoff/v3 v3.2.2 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/emicklei/go-restful/v3 v3.9.0 // indirect
github.com/go-openapi/jsonreference v0.20.0 // indirect
github.com/go-test/deep v1.0.8 // indirect
github.com/golang-jwt/jwt/v4 v4.3.0 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/hashicorp/go-retryablehttp v0.7.1 // indirect
github.com/hashicorp/hcl v1.0.1-vault-5 // indirect
github.com/imdario/mergo v0.3.13 // indirect
Expand Down
Loading