Tweak token credential composition once again

- `authorityHost` and `clientCertificateSendChain` can now be set where applicable. - AZ CLI fields have been removed. - Fallback to `ChainedTokenCredential` with `EnvironmentCredential` and `ManagedIdentityCredential` with defaults if no Secret is given. Tests have not yet been updated. Signed-off-by: Hidde Beydals <hello@hidde.co>
fluxcd · Mar 4, 2022 · 7940044 · 7940044
1 parent f5d7a37
commit 7940044
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 75 deletions.
diff --git a/pkg/azure/blob.go b/pkg/azure/blob.go
@@ -44,18 +44,15 @@ var (
 )
 
 const (
-	resourceIDField                = "resourceId"
-	clientIDField                  = "clientId"
-	tenantIDField                  = "tenantId"
-	clientSecretField              = "clientSecret"
-	clientCertificateField         = "clientCertificate"
-	clientCertificatePasswordField = "clientCertificatePassword"
-	accountKeyField                = "accountKey"
-
-	// Ref: https://docs.microsoft.com/en-us/azure/aks/kubernetes-service-principal?tabs=azure-cli#manually-create-a-service-principal
-	tenantField   = "tenant"
-	appIDField    = "appId"
-	passwordField = "password"
+	resourceIDField                 = "resourceId"
+	clientIDField                   = "clientId"
+	tenantIDField                   = "tenantId"
+	clientSecretField               = "clientSecret"
+	clientCertificateField          = "clientCertificate"
+	clientCertificatePasswordField  = "clientCertificatePassword"
+	clientCertificateSendChainField = "clientCertificateSendChain"
+	authorityHostField              = "authorityHost"
+	accountKeyField                 = "accountKey"
 )
 
 // BlobClient is a minimal Azure Blob client for fetching objects.
@@ -83,39 +80,63 @@ type BlobClient struct {
 //	- azblob.SharedKeyCredential when an `accountKey` field is found.
 //    The account name is extracted from the endpoint specified on the Bucket
 //    object.
+//  - azidentity.ChainedTokenCredential with azidentity.EnvironmentCredential
+//    and azidentity.ManagedIdentityCredential with defaults if no Secret is
+//    given.
 //
-// If no credentials are found, a simple client without credentials is
-// returned.
+// If no credentials are found, and the azidentity.ChainedTokenCredential can
+// not be established. A simple client without credentials is returned.
 func NewClient(obj *sourcev1.Bucket, secret *corev1.Secret) (c *BlobClient, err error) {
 	c = &BlobClient{}
 
-	// Without a Secret, we can return a simple client.
-	if secret == nil || len(secret.Data) == 0 {
-		c.ServiceClient, err = azblob.NewServiceClientWithNoCredential(obj.Spec.Endpoint, nil)
-		return
+	var token azcore.TokenCredential
+
+	if secret != nil && len(secret.Data) > 0 {
+		// Attempt AAD Token Credential options first.
+		if token, err = tokenCredentialFromSecret(secret); err != nil {
+			err = fmt.Errorf("failed to create token credential from '%s' Secret: %w", secret.Name, err)
+			return
+		}
+		if token != nil {
+			c.ServiceClient, err = azblob.NewServiceClient(obj.Spec.Endpoint, token, nil)
+			return
+		}
+
+		// Fallback to Shared Key Credential.
+		var cred *azblob.SharedKeyCredential
+		if cred, err = sharedCredentialFromSecret(obj.Spec.Endpoint, secret); err != nil {
+			return
+		}
+		if cred != nil {
+			c.ServiceClient, err = azblob.NewServiceClientWithSharedKey(obj.Spec.Endpoint, cred, &azblob.ClientOptions{})
+			return
+		}
 	}
 
-	// Attempt AAD Token Credential options first.
-	var token azcore.TokenCredential
-	if token, err = tokenCredentialFromSecret(secret); err != nil {
-		return
+	// Compose token chain based on environment.
+	// This functions as a replacement for azidentity.NewDefaultAzureCredential
+	// to not shell out.
+	var creds []azcore.TokenCredential
+	credOpts := &azidentity.EnvironmentCredentialOptions{}
+	if authorityHost, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
+		credOpts.AuthorityHost = azidentity.AuthorityHost(authorityHost)
 	}
-	if token != nil {
-		c.ServiceClient, err = azblob.NewServiceClient(obj.Spec.Endpoint, token, nil)
-		return
+	if token, _ = azidentity.NewEnvironmentCredential(credOpts); token != nil {
+		creds = append(creds, token)
 	}
-
-	// Fallback to Shared Key Credential.
-	cred, err := sharedCredentialFromSecret(obj.Spec.Endpoint, secret)
-	if err != nil {
-		return
+	if token, _ = azidentity.NewManagedIdentityCredential(nil); token != nil {
+		creds = append(creds, token)
 	}
-	if cred != nil {
-		c.ServiceClient, err = azblob.NewServiceClientWithSharedKey(obj.Spec.Endpoint, cred, &azblob.ClientOptions{})
+	if len(creds) > 0 {
+		if token, err = azidentity.NewChainedTokenCredential(creds, nil); err != nil {
+			err = fmt.Errorf("failed to create environment credential chain: %w", err)
+			return
+		}
+		c.ServiceClient, err = azblob.NewServiceClient(obj.Spec.Endpoint, token, &azblob.ClientOptions{})
 		return
 	}
 
-	// Secret does not contain a valid set of credentials, fallback to simple client.
+	// Fallback to simple client.
 	c.ServiceClient, err = azblob.NewServiceClientWithNoCredential(obj.Spec.Endpoint, nil)
 	return
 }
@@ -138,26 +159,19 @@ func ValidateSecret(secret *corev1.Secret) error {
 			}
 		}
 	}
-	if _, hasTenant := secret.Data[tenantField]; hasTenant {
-		if _, hasAppID := secret.Data[appIDField]; hasAppID {
-			if _, hasPassword := secret.Data[passwordField]; hasPassword {
-				valid = true
-			}
-		}
-	}
-	if _, hasResourceID := secret.Data[resourceIDField]; hasResourceID {
-		valid = true
-	}
 	if _, hasClientID := secret.Data[clientIDField]; hasClientID {
 		valid = true
 	}
 	if _, hasAccountKey := secret.Data[accountKeyField]; hasAccountKey {
 		valid = true
 	}
+	if _, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
+		valid = true
+	}
 
 	if !valid {
-		return fmt.Errorf("invalid '%s' secret data: requires a '%s', '%s', or '%s' field, a combination of '%s', '%s' and '%s', or '%s', '%s' and '%s'",
-			secret.Name, resourceIDField, clientIDField, accountKeyField, tenantIDField, clientIDField, clientSecretField, tenantIDField, clientIDField, clientCertificateField)
+		return fmt.Errorf("invalid '%s' secret data: requires a '%s' or '%s' field, a combination of '%s', '%s' and '%s', or '%s', '%s' and '%s'",
+			secret.Name, clientIDField, accountKeyField, tenantIDField, clientIDField, clientSecretField, tenantIDField, clientIDField, clientCertificateField)
 	}
 	return nil
 }
@@ -289,33 +303,32 @@ func tokenCredentialFromSecret(secret *corev1.Secret) (azcore.TokenCredential, e
 	clientID, hasClientID := secret.Data[clientIDField]
 	if tenantID, hasTenantID := secret.Data[tenantIDField]; hasTenantID && hasClientID {
 		if clientSecret, hasClientSecret := secret.Data[clientSecretField]; hasClientSecret && len(clientSecret) > 0 {
-			return azidentity.NewClientSecretCredential(string(tenantID), string(clientID), string(clientSecret), nil)
+			opts := &azidentity.ClientSecretCredentialOptions{}
+			if authorityHost, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
+				opts.AuthorityHost = azidentity.AuthorityHost(authorityHost)
+			}
+			return azidentity.NewClientSecretCredential(string(tenantID), string(clientID), string(clientSecret), opts)
 		}
 		if clientCertificate, hasClientCertificate := secret.Data[clientCertificateField]; hasClientCertificate && len(clientCertificate) > 0 {
 			certs, key, err := azidentity.ParseCertificates(clientCertificate, secret.Data[clientCertificatePasswordField])
 			if err != nil {
 				return nil, fmt.Errorf("failed to parse client certificates: %w", err)
 			}
-			return azidentity.NewClientCertificateCredential(string(tenantID), string(clientID), certs, key, nil)
-		}
-	}
-	if tenant, hasTenant := secret.Data[tenantField]; hasTenant {
-		if appId, hasAppID := secret.Data[appIDField]; hasAppID {
-			if password, hasPassword := secret.Data[passwordField]; hasPassword {
-				return azidentity.NewClientSecretCredential(string(tenant), string(appId), string(password), nil)
+			opts := &azidentity.ClientCertificateCredentialOptions{}
+			if authorityHost, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
+				opts.AuthorityHost = azidentity.AuthorityHost(authorityHost)
+			}
+			if v, sendChain := secret.Data[clientCertificateSendChainField]; sendChain {
+				opts.SendCertificateChain = string(v) == "1" || strings.ToLower(string(v)) == "true"
 			}
+			return azidentity.NewClientCertificateCredential(string(tenantID), string(clientID), certs, key, opts)
 		}
 	}
 	if hasClientID {
 		return azidentity.NewManagedIdentityCredential(&azidentity.ManagedIdentityCredentialOptions{
 			ID: azidentity.ClientID(clientID),
 		})
 	}
-	if resourceID, hasResourceID := secret.Data[resourceIDField]; hasResourceID {
-		return azidentity.NewManagedIdentityCredential(&azidentity.ManagedIdentityCredentialOptions{
-			ID: azidentity.ResourceID(resourceID),
-		})
-	}
 	return nil, nil
 }
 

diff --git a/pkg/azure/blob_test.go b/pkg/azure/blob_test.go
@@ -76,16 +76,6 @@ func TestValidateSecret(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "valid ServicePrincipal Secret",
-			secret: &corev1.Secret{
-				Data: map[string][]byte{
-					tenantField:   []byte("some-tenant-id-"),
-					appIDField:    []byte("some-client-id-"),
-					passwordField: []byte("some-client-secret-"),
-				},
-			},
-		},
 		{
 			name: "valid SharedKey Secret",
 			secret: &corev1.Secret{
@@ -242,13 +232,6 @@ func Test_tokenCredentialFromSecret(t *testing.T) {
 		},
 		{
 			name: "with Tenant, AppID and Password fields",
-			secret: &corev1.Secret{
-				Data: map[string][]byte{
-					appIDField:    []byte("client-id"),
-					tenantField:   []byte("tenant-id"),
-					passwordField: []byte("client-secret"),
-				},
-			},
 			want: &azidentity.ClientSecretCredential{},
 		},
 		{