fix: Register only the wanted Azure credential in the chain (#4125)

* fix: Register only the wanted Azure credential in the chain Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl> * fix style issues Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl> Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
kedacore · Jan 17, 2023 · f70718a · f70718a
1 parent 16bef4e
commit f70718a
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -69,8 +69,8 @@ Here is an overview of all new **experimental** features:
 ### Fixes
 
 - **General**: Prevent a panic that might occur while refreshing a scaler cache ([#4092](https://github.com/kedacore/keda/issues/4092))
+- **Azure Service Bus Scaler:** Use correct auth flows with pod identity ([#4026](https://github.com/kedacore/keda/issues/4026)|[#4123](https://github.com/kedacore/keda/issues/4123))
 - **Prometheus Metrics**: Expose Prometheus Metrics also when getting ScaledObject state ([#4075](https://github.com/kedacore/keda/issues/4075))
-- **Azure Service Bus Scaler:** Use correct auth flows with pod identity ([#4026](https://github.com/kedacore/keda/issues/4026))
 - **CPU Memory Scaler** Store forgotten logger ([#4022](https://github.com/kedacore/keda/issues/4022))
 
 ### Deprecations

diff --git a/pkg/scalers/azure/azure_azidentity_chain.go b/pkg/scalers/azure/azure_azidentity_chain.go
@@ -5,9 +5,11 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
+
+	"github.com/kedacore/keda/v2/apis/keda/v1alpha1"
 )
 
-func NewChainedCredential(identityID string) (*azidentity.ChainedTokenCredential, error) {
+func NewChainedCredential(identityID string, podIdentity v1alpha1.PodIdentityProvider) (*azidentity.ChainedTokenCredential, error) {
 	var creds []azcore.TokenCredential
 
 	// Used for local debug based on az-cli user
@@ -19,15 +21,24 @@ func NewChainedCredential(identityID string) (*azidentity.ChainedTokenCredential
 		}
 	}
 
-	// Used for aad-pod-identity
-	msiCred, err := ManagedIdentityWrapperCredential(identityID)
-	if err == nil {
-		creds = append(creds, msiCred)
-	}
-
-	wiCred, err := NewADWorkloadIdentityCredential(identityID)
-	if err == nil {
-		creds = append(creds, wiCred)
+	// https://github.com/kedacore/keda/issues/4123
+	// We shouldn't register both in the same chain because if both are registered, KEDA will use the first one
+	// which returns a valid token. This could produce an unintended behaviour if end-users use 2 different identities
+	// with 2 different permissions. They could set workload-identity with the identity A, but KEDA would use
+	// aad-pod-identity with the identity B. If both identities are differents or have different permissions, this blocks
+	// workload identity
+	switch podIdentity {
+	case v1alpha1.PodIdentityProviderAzure:
+		// Used for aad-pod-identity
+		msiCred, err := ManagedIdentityWrapperCredential(identityID)
+		if err == nil {
+			creds = append(creds, msiCred)
+		}
+	case v1alpha1.PodIdentityProviderAzureWorkload:
+		wiCred, err := NewADWorkloadIdentityCredential(identityID)
+		if err == nil {
+			creds = append(creds, wiCred)
+		}
 	}
 
 	// Create the chained credential based on the previous 3

diff --git a/pkg/scalers/azure_servicebus_scaler.go b/pkg/scalers/azure_servicebus_scaler.go
@@ -296,7 +296,7 @@ func (s *azureServiceBusScaler) getServiceBusAdminClient() (*admin.Client, error
 	case "", kedav1alpha1.PodIdentityProviderNone:
 		return admin.NewClientFromConnectionString(s.metadata.connection, nil)
 	case kedav1alpha1.PodIdentityProviderAzure, kedav1alpha1.PodIdentityProviderAzureWorkload:
-		creds, err := azure.NewChainedCredential(s.podIdentity.IdentityID)
+		creds, err := azure.NewChainedCredential(s.podIdentity.IdentityID, s.podIdentity.Provider)
 		if err != nil {
 			return nil, err
 		}