feat: have parity with 1Password connect service for GetSecretMap (#4895)

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

pkg/provider/onepasswordsdk/client.go

@@ -16,7 +16,6 @@ package onepasswordsdk
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"strings"
@@ -29,6 +28,13 @@ import (
 	"github.com/external-secrets/external-secrets/pkg/utils/metadata"
 )
 
+const (
+	fieldPrefix             = "field"
+	filePrefix              = "file"
+	prefixSplitter          = "/"
+	errExpectedOneFieldMsgF = "found more than 1 fields with title '%s' in '%s', got %d"
+)
+
 // ErrKeyNotFound is returned when a key is not found in the 1Password Vaults.
 var ErrKeyNotFound = errors.New("key not found")
 
@@ -112,34 +118,87 @@ func (p *Provider) GetAllSecrets(_ context.Context, _ esv1.ExternalSecretFind) (
 	return nil, fmt.Errorf(errOnePasswordSdkStore, errors.New(errNotImplemented))
 }
 
-// GetSecretMap implements v1.SecretsClient.
+// GetSecretMap returns multiple k/v pairs from the provider, for dataFrom.extract.
 func (p *Provider) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 	if ref.Version != "" {
 		return nil, errors.New(errVersionNotImplemented)
 	}
 
-	// Gets a secret as normal, expecting secret value to be a json object
-	data, err := p.GetSecret(ctx, ref)
+	item, err := p.findItem(ctx, ref.Key)
 	if err != nil {
-		return nil, fmt.Errorf("error getting secret %s: %w", ref.Key, err)
+		return nil, err
 	}
 
-	// Maps the json data to a string:string map
-	kv := make(map[string]string)
-	err = json.Unmarshal(data, &kv)
-	if err != nil {
-		return nil, fmt.Errorf("failed to unmarshal data: %w", err)
+	propertyType, property := getObjType(item.Category, ref.Property)
+	if propertyType == filePrefix {
+		return p.getFiles(ctx, item, property)
 	}
 
-	// Converts values in K:V pairs into bytes, while leaving keys as strings
+	return p.getFields(item, property)
+}
+
+func (p *Provider) getFields(item onepassword.Item, property string) (map[string][]byte, error) {
 	secretData := make(map[string][]byte)
-	for k, v := range kv {
-		secretData[k] = []byte(v)
+	for _, field := range item.Fields {
+		if property != "" && field.Title != property {
+			continue
+		}
+		if length := countFieldsWithLabel(field.Title, item.Fields); length != 1 {
+			return nil, fmt.Errorf(errExpectedOneFieldMsgF, field.Title, item.Title, length)
+		}
+
+		// caution: do not use client.GetValue here because it has undesirable behavior on keys with a dot in them
+		secretData[field.Title] = []byte(field.Value)
 	}
 
 	return secretData, nil
 }
 
+func (p *Provider) getFiles(ctx context.Context, item onepassword.Item, property string) (map[string][]byte, error) {
+	secretData := make(map[string][]byte)
+	for _, file := range item.Files {
+		if property != "" && file.Attributes.Name != property {
+			continue
+		}
+
+		contents, err := p.client.Items().Files().Read(ctx, p.vaultID, file.FieldID, file.Attributes)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read file: %w", err)
+		}
+
+		secretData[file.Attributes.Name] = contents
+	}
+
+	return secretData, nil
+}
+
+func countFieldsWithLabel(fieldLabel string, fields []onepassword.ItemField) int {
+	count := 0
+	for _, field := range fields {
+		if field.Title == fieldLabel {
+			count++
+		}
+	}
+
+	return count
+}
+
+// Clean property string by removing property prefix if needed.
+func getObjType(documentType onepassword.ItemCategory, property string) (string, string) {
+	if strings.HasPrefix(property, fieldPrefix+prefixSplitter) {
+		return fieldPrefix, property[6:]
+	}
+	if strings.HasPrefix(property, filePrefix+prefixSplitter) {
+		return filePrefix, property[5:]
+	}
+
+	if documentType == onepassword.ItemCategoryDocument {
+		return filePrefix, property
+	}
+
+	return fieldPrefix, property
+}
+
 // createItem creates a new item in the first vault. If no vaults exist, it returns an error.
 func (p *Provider) createItem(ctx context.Context, val []byte, ref esv1.PushSecretData) error {
 	// Get the metadata

pkg/provider/onepasswordsdk/client_test.go

@@ -116,13 +116,40 @@ func TestProviderGetSecretMap(t *testing.T) {
 		client      func() *onepassword.Client
 	}{
 		{
-			name: "get secret successfully",
+			name: "get secret successfully for files",
 			client: func() *onepassword.Client {
-				fc := &fakeClient{
-					resolveResult: `{"key": "value"}`,
+				fc := &fakeClient{}
+				fl := &fakeLister{
+					listAllResult: []onepassword.ItemOverview{
+						{
+							ID:       "test-item-id",
+							Title:    "key",
+							Category: "login",
+							VaultID:  "vault-id",
+						},
+					},
+					getResult: onepassword.Item{
+						ID:       "test-item-id",
+						Title:    "key",
+						Category: "login",
+						VaultID:  "vault-id",
+						Files: []onepassword.ItemFile{
+							{
+								Attributes: onepassword.FileAttributes{
+									Name: "name",
+									ID:   "id",
+								},
+								FieldID: "field-id",
+							},
+						},
+					},
+					fileLister: &fakeFileLister{
+						readContent: []byte("content"),
+					},
 				}
 				return &onepassword.Client{
 					SecretsAPI: fc,
+					ItemsAPI:   fl,
 					VaultsAPI:  fc,
 				}
 			},
@@ -130,10 +157,110 @@ func TestProviderGetSecretMap(t *testing.T) {
 				require.NoError(t, err)
 			},
 			ref: v1.ExternalSecretDataRemoteRef{
-				Key: "secret",
+				Key:      "key",
+				Property: "file/name",
+			},
+			want: map[string][]byte{
+				"name": []byte("content"),
+			},
+		},
+		{
+			name: "get secret successfully for fields",
+			client: func() *onepassword.Client {
+				fc := &fakeClient{}
+				fl := &fakeLister{
+					listAllResult: []onepassword.ItemOverview{
+						{
+							ID:       "test-item-id",
+							Title:    "key",
+							Category: "login",
+							VaultID:  "vault-id",
+						},
+					},
+					getResult: onepassword.Item{
+						ID:       "test-item-id",
+						Title:    "key",
+						Category: "login",
+						VaultID:  "vault-id",
+						Fields: []onepassword.ItemField{
+							{
+								ID:        "field-id",
+								Title:     "name",
+								FieldType: onepassword.ItemFieldTypeConcealed,
+								Value:     "value",
+							},
+						},
+					},
+					fileLister: &fakeFileLister{
+						readContent: []byte("content"),
+					},
+				}
+				return &onepassword.Client{
+					SecretsAPI: fc,
+					ItemsAPI:   fl,
+					VaultsAPI:  fc,
+				}
+			},
+			assertError: func(t *testing.T, err error) {
+				require.NoError(t, err)
+			},
+			ref: v1.ExternalSecretDataRemoteRef{
+				Key:      "key",
+				Property: "field/name",
 			},
 			want: map[string][]byte{
-				"key": []byte("value"),
+				"name": []byte("value"),
+			},
+		},
+		{
+			name: "get secret fails with fields with same title",
+			client: func() *onepassword.Client {
+				fc := &fakeClient{}
+				fl := &fakeLister{
+					listAllResult: []onepassword.ItemOverview{
+						{
+							ID:       "test-item-id",
+							Title:    "key",
+							Category: "login",
+							VaultID:  "vault-id",
+						},
+					},
+					getResult: onepassword.Item{
+						ID:       "test-item-id",
+						Title:    "key",
+						Category: "login",
+						VaultID:  "vault-id",
+						Fields: []onepassword.ItemField{
+							{
+								ID:        "field-id",
+								Title:     "name",
+								FieldType: onepassword.ItemFieldTypeConcealed,
+								Value:     "value",
+							},
+							{
+								ID:        "field-id",
+								Title:     "name",
+								FieldType: onepassword.ItemFieldTypeConcealed,
+								Value:     "value",
+							},
+						},
+					},
+					fileLister: &fakeFileLister{
+						readContent: []byte("content"),
+					},
+				}
+				return &onepassword.Client{
+					SecretsAPI: fc,
+					ItemsAPI:   fl,
+					VaultsAPI:  fc,
+				}
+			},
+			assertError: func(t *testing.T, err error) {
+				require.ErrorContains(t, err, "found more than 1 fields with title 'name' in 'key', got 2")
+			},
+			ref: v1.ExternalSecretDataRemoteRef{
+				Key:      "key",
+				Property: "field/name",
 			},
 		},
 	}
@@ -145,7 +272,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 			}
 			got, err := p.GetSecretMap(context.Background(), tt.ref)
 			tt.assertError(t, err)
-			require.Equal(t, got, tt.want)
+			require.Equal(t, tt.want, got)
 		})
 	}
 }
@@ -471,6 +598,7 @@ type fakeLister struct {
 	putCalled     bool
 	deleteCalled  bool
 	getResult     onepassword.Item
+	fileLister    onepassword.ItemsFilesAPI
 }
 
 func (f *fakeLister) Create(ctx context.Context, params onepassword.ItemCreateParams) (onepassword.Item, error) {
@@ -505,9 +633,31 @@ func (f *fakeLister) Shares() onepassword.ItemsSharesAPI {
 }
 
 func (f *fakeLister) Files() onepassword.ItemsFilesAPI {
-	return nil
+	return f.fileLister
 }
 
+type fakeFileLister struct {
+	readContent []byte
+}
+
+func (f *fakeFileLister) Attach(ctx context.Context, item onepassword.Item, fileParams onepassword.FileCreateParams) (onepassword.Item, error) {
+	return onepassword.Item{}, nil
+}
+
+func (f *fakeFileLister) Read(ctx context.Context, vaultID, itemID string, attr onepassword.FileAttributes) ([]byte, error) {
+	return f.readContent, nil
+}
+
+func (f *fakeFileLister) Delete(ctx context.Context, item onepassword.Item, sectionID, fieldID string) (onepassword.Item, error) {
+	return onepassword.Item{}, nil
+}
+
+func (f *fakeFileLister) ReplaceDocument(ctx context.Context, item onepassword.Item, docParams onepassword.DocumentCreateParams) (onepassword.Item, error) {
+	return onepassword.Item{}, nil
+}
+
+var _ onepassword.ItemsFilesAPI = (*fakeFileLister)(nil)
+
 type fakeClient struct {
 	resolveResult   string
 	resolveError    error