Merge pull request #1257 from external-secrets/bug-1137

Azure KeyVault decoding bugs

pkg/provider/azure/keyvault/keyvault.go

@@ -246,6 +246,20 @@ func getSecretTag(tags map[string]*string, property string) ([]byte, error) {
 	if val, exist := tags[property]; exist {
 		return []byte(*val), nil
 	}
+
+	idx := strings.Index(property, ".")
+	if idx < 0 {
+		return nil, fmt.Errorf(errTagNotExist, property)
+	}
+
+	if idx > 0 {
+		tagName := property[0:idx]
+		if val, exist := tags[tagName]; exist {
+			key := strings.Replace(property, tagName+".", "", 1)
+			return getProperty(*val, key, property)
+		}
+	}
+
 	return nil, fmt.Errorf(errTagNotExist, property)
 }
 
@@ -256,6 +270,15 @@ func getProperty(secret, property, key string) ([]byte, error) {
 	}
 	res := gjson.Get(secret, property)
 	if !res.Exists() {
+		idx := strings.Index(property, ".")
+		if idx < 0 {
+			return nil, fmt.Errorf(errPropNotExist, property, key)
+		}
+		escaped := strings.ReplaceAll(property, ".", "\\.")
+		jValue := gjson.Get(secret, escaped)
+		if jValue.Exists() {
+			return []byte(jValue.String()), nil
+		}
 		return nil, fmt.Errorf(errPropNotExist, property, key)
 	}
 	return []byte(res.String()), nil
@@ -308,7 +331,7 @@ func (a *Azure) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataR
 }
 
 // returns a SecretBundle with the tags values.
-func (a *Azure) getSecretTags(ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
+func (a *Azure) getSecretTags(ref esv1beta1.ExternalSecretDataRemoteRef) (map[string]*string, error) {
 	_, secretName := getObjType(ref)
 	secretResp, err := a.baseClient.GetSecret(context.Background(), *a.provider.VaultURL, secretName, ref.Version)
 
@@ -316,7 +339,7 @@ func (a *Azure) getSecretTags(ref esv1beta1.ExternalSecretDataRemoteRef) (map[st
 		return nil, err
 	}
 
-	secretTagsData := make(map[string][]byte)
+	secretTagsData := make(map[string]*string)
 
 	for tagname, tagval := range secretResp.Tags {
 		name := secretName + "_" + tagname
@@ -324,11 +347,11 @@ func (a *Azure) getSecretTags(ref esv1beta1.ExternalSecretDataRemoteRef) (map[st
 		err = json.Unmarshal([]byte(*tagval), &kv)
 		// if the tagvalue is not in JSON format then we added to secretTagsData we added as it is
 		if err != nil {
-			secretTagsData[name] = []byte(*tagval)
+			secretTagsData[name] = tagval
 		} else {
 			for k, v := range kv {
-				keyName := name + "_" + k
-				secretTagsData[keyName] = []byte(v)
+				value := v
+				secretTagsData[name+"_"+k] = &value
 			}
 		}
 	}
@@ -348,21 +371,12 @@ func (a *Azure) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDa
 		}
 
 		if ref.MetadataPolicy == esv1beta1.ExternalSecretMetadataPolicyFetch {
-			return a.getSecretTags(ref)
+			tags, _ := a.getSecretTags(ref)
+			return getSecretMapProperties(tags, ref.Key, ref.Property), nil
 		}
 
-		kv := make(map[string]string)
-		err = json.Unmarshal(data, &kv)
-		if err != nil {
-			return nil, fmt.Errorf(errUnmarshalJSONData, err)
-		}
+		return getSecretMapMap(data)
 
-		secretData := make(map[string][]byte)
-		for k, v := range kv {
-			secretData[k] = []byte(v)
-		}
-
-		return secretData, nil
 	case objectTypeCert:
 		return nil, fmt.Errorf(errDataFromCert)
 	case objectTypeKey:
@@ -371,6 +385,43 @@ func (a *Azure) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDa
 	return nil, fmt.Errorf(errUnknownObjectType, secretName)
 }
 
+func getSecretMapMap(data []byte) (map[string][]byte, error) {
+	kv := make(map[string]json.RawMessage)
+	err := json.Unmarshal(data, &kv)
+	if err != nil {
+		return nil, fmt.Errorf(errUnmarshalJSONData, err)
+	}
+
+	secretData := make(map[string][]byte)
+	for k, v := range kv {
+		var strVal string
+		err = json.Unmarshal(v, &strVal)
+		if err == nil {
+			secretData[k] = []byte(strVal)
+		} else {
+			secretData[k] = v
+		}
+	}
+	return secretData, nil
+}
+
+func getSecretMapProperties(tags map[string]*string, key, property string) map[string][]byte {
+	tagByteArray := make(map[string][]byte)
+	if property != "" {
+		keyPropertyName := key + "_" + property
+		singleTag, _ := getSecretTag(tags, keyPropertyName)
+		tagByteArray[keyPropertyName] = singleTag
+
+		return tagByteArray
+	}
+
+	for k, v := range tags {
+		tagByteArray[k] = []byte(*v)
+	}
+
+	return tagByteArray
+}
+
 func (a *Azure) authorizerForWorkloadIdentity(ctx context.Context, tokenProvider tokenProviderFunc) (autorest.Authorizer, error) {
 	// if no serviceAccountRef was provided
 	// we expect certain env vars to be present.

pkg/provider/azure/keyvault/keyvault_test.go

@@ -104,6 +104,8 @@ const (
 	secretName           = "example-1"
 	testsecret           = "test-secret"
 	fakeURL              = "noop"
+	foo                  = "foo"
+	bar                  = "bar"
 	errStore             = "Azure.ValidateStore() error = %v, wantErr %v"
 )
 
@@ -339,6 +341,99 @@ func TestAzureKeyVaultSecretManagerGetSecret(t *testing.T) {
 		smtc.apiErr = errors.New(smtc.expectError)
 	}
 
+	fetchSingleTag := func(smtc *secretManagerTestCase) {
+		jsonString := jsonTestString
+		smtc.expectedSecret = bar
+		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
+		secretTags := map[string]*string{}
+		tagValue := bar
+		secretTags[foo] = &tagValue
+		smtc.secretOutput = keyvault.SecretBundle{
+			Value: &jsonString,
+			Tags:  secretTags,
+		}
+		smtc.ref.Property = foo
+	}
+
+	fetchJSONTag := func(smtc *secretManagerTestCase) {
+		jsonString := jsonTestString
+		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
+		secretTags := map[string]*string{}
+		tagValue := "{\"key\":\"value\"}"
+		secretTags[foo] = &tagValue
+		smtc.secretOutput = keyvault.SecretBundle{
+			Value: &jsonString,
+			Tags:  secretTags,
+		}
+		smtc.ref.Property = foo
+		smtc.expectedSecret = tagValue
+	}
+
+	fetchDottedJSONTag := func(smtc *secretManagerTestCase) {
+		jsonString := jsonTestString
+		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
+		secretTags := map[string]*string{}
+		tagValue := "{\"key\":\"value\"}"
+		secretTags[foo] = &tagValue
+		smtc.secretOutput = keyvault.SecretBundle{
+			Value: &jsonString,
+			Tags:  secretTags,
+		}
+		smtc.ref.Property = "foo.key"
+		smtc.expectedSecret = "value"
+	}
+
+	fetchNestedJSONTag := func(smtc *secretManagerTestCase) {
+		jsonString := jsonTestString
+		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
+		secretTags := map[string]*string{}
+		tagValue := "{\"key\":\"value\", \"nested\": {\"foo\":\"bar\"}}"
+		secretTags["foo"] = &tagValue
+		smtc.secretOutput = keyvault.SecretBundle{
+			Value: &jsonString,
+			Tags:  secretTags,
+		}
+		smtc.ref.Property = "foo.nested"
+		smtc.expectedSecret = "{\"foo\":\"bar\"}"
+	}
+
+	fetchNestedDottedJSONTag := func(smtc *secretManagerTestCase) {
+		jsonString := jsonTestString
+		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
+		secretTags := map[string]*string{}
+		tagValue := "{\"key\":\"value\", \"nested\": {\"foo\":\"bar\"}}"
+		secretTags[foo] = &tagValue
+		smtc.secretOutput = keyvault.SecretBundle{
+			Value: &jsonString,
+			Tags:  secretTags,
+		}
+		smtc.ref.Property = "foo.nested.foo"
+		smtc.expectedSecret = bar
+	}
+
+	fetchDottedKeyJSONTag := func(smtc *secretManagerTestCase) {
+		jsonString := jsonTestString
+		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
+		secretTags := map[string]*string{}
+		tagValue := "{\"foo.json\":\"bar\"}"
+		secretTags[foo] = &tagValue
+		smtc.secretOutput = keyvault.SecretBundle{
+			Value: &jsonString,
+			Tags:  secretTags,
+		}
+		smtc.ref.Property = "foo.foo.json"
+		smtc.expectedSecret = bar
+	}
+
+	fetchDottedSecretJSONTag := func(smtc *secretManagerTestCase) {
+		jsonString := "{\"foo.json\":\"bar\"}"
+		smtc.secretOutput = keyvault.SecretBundle{
+			Value: &jsonString,
+		}
+		smtc.ref.Property = "foo.json"
+		smtc.expectedSecret = bar
+	}
+
 	successCases := []*secretManagerTestCase{
 		makeValidSecretManagerTestCase(),
 		makeValidSecretManagerTestCaseCustom(setSecretString),
@@ -362,6 +457,13 @@ func TestAzureKeyVaultSecretManagerGetSecret(t *testing.T) {
 		makeValidSecretManagerTestCaseCustom(setKeyWithNoSpecificTag),
 		makeValidSecretManagerTestCaseCustom(setKeyWithNoTags),
 		makeValidSecretManagerTestCaseCustom(badPropertyTag),
+		makeValidSecretManagerTestCaseCustom(fetchSingleTag),
+		makeValidSecretManagerTestCaseCustom(fetchJSONTag),
+		makeValidSecretManagerTestCaseCustom(fetchDottedJSONTag),
+		makeValidSecretManagerTestCaseCustom(fetchNestedJSONTag),
+		makeValidSecretManagerTestCaseCustom(fetchNestedDottedJSONTag),
+		makeValidSecretManagerTestCaseCustom(fetchDottedKeyJSONTag),
+		makeValidSecretManagerTestCaseCustom(fetchDottedSecretJSONTag),
 	}
 
 	sm := Azure{
@@ -481,6 +583,33 @@ func TestAzureKeyVaultSecretManagerGetSecretMap(t *testing.T) {
 		smtc.expectedSecret = ""
 	}
 
+	nestedJSONNoProperty := func(smtc *secretManagerTestCase) {
+		jsonString := jsonTestString
+		smtc.expectedSecret = ""
+		smtc.secretOutput = keyvault.SecretBundle{
+			Value: &jsonString,
+		}
+		smtc.ref.Property = ""
+		smtc.expectedData["Name"] = []byte("External")
+		smtc.expectedData["LastName"] = []byte("Secret")
+		smtc.expectedData["Address"] = []byte(`{ "Street": "Myroad st.", "CP": "J4K4T4" }`)
+	}
+
+	setNestedJSONTag := func(smtc *secretManagerTestCase) {
+		secretTags := map[string]*string{}
+		tagValue := `{"foo":"bar","nested.tag":{"foo":"bar"}}`
+		bug := "1137"
+		secretTags["dev"] = &tagValue
+		secretTags["bug"] = &bug
+
+		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
+		smtc.secretOutput = keyvault.SecretBundle{
+			Tags: secretTags,
+		}
+		smtc.ref.Property = "dev"
+		smtc.expectedData[testsecret+"_dev"] = []byte(tagValue)
+	}
+
 	successCases := []*secretManagerTestCase{
 		makeValidSecretManagerTestCaseCustom(badSecretString),
 		makeValidSecretManagerTestCaseCustom(setSecretJSON),
@@ -492,6 +621,8 @@ func TestAzureKeyVaultSecretManagerGetSecretMap(t *testing.T) {
 		makeValidSecretManagerTestCaseCustom(setSecretTags),
 		makeValidSecretManagerTestCaseCustom(setSecretWithJSONTag),
 		makeValidSecretManagerTestCaseCustom(setSecretWithNoTags),
+		makeValidSecretManagerTestCaseCustom(nestedJSONNoProperty),
+		makeValidSecretManagerTestCaseCustom(setNestedJSONTag),
 	}
 
 	sm := Azure{