Add attached file support to all onepassword secrets (#3901)

* Add attached file support to all onepassword secrets

Signed-off-by: Thibault Cohen <47721+titilambert@users.noreply.github.com>

* Small clean up

Signed-off-by: Thibault Cohen <47721+titilambert@users.noreply.github.com>

* Fix PR comments

Signed-off-by: Thibault Cohen <47721+titilambert@users.noreply.github.com>

* Fix sonarcloud issues

Signed-off-by: Thibault Cohen <47721+titilambert@users.noreply.github.com>

* Fix PR comments

Signed-off-by: Thibault Cohen <47721+titilambert@users.noreply.github.com>

* Fix PR comments

Signed-off-by: Thibault Cohen <47721+titilambert@users.noreply.github.com>

---------

Signed-off-by: Thibault Cohen <47721+titilambert@users.noreply.github.com>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

pkg/provider/onepassword/onepassword.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net/url"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/1Password/connect-sdk-go/connect"
@@ -63,6 +64,9 @@ const (
 	errExpectedOneFieldMsgF = "%w: '%s' in '%s', got %d"
 
 	documentCategory = "DOCUMENT"
+	fieldPrefix      = "field"
+	filePrefix       = "file"
+	prefixSplitter   = "/"
 )
 
 // Custom Errors //.
@@ -329,6 +333,22 @@ func (provider *ProviderOnePassword) PushSecret(ctx context.Context, secret *cor
 	return nil
 }
 
+// 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 == documentCategory {
+		return filePrefix, property
+	}
+
+	return fieldPrefix, property
+}
+
 // GetSecret returns a single secret from the provider.
 func (provider *ProviderOnePassword) GetSecret(_ context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) {
 	if ref.Version != "" {
@@ -340,14 +360,11 @@ func (provider *ProviderOnePassword) GetSecret(_ context.Context, ref esv1beta1.
 		return nil, err
 	}
 
-	// handle files
-	if item.Category == documentCategory {
-		// default to the first file when ref.Property is empty
-		return provider.getFile(item, ref.Property)
+	propertyType, property := getObjType(item.Category, ref.Property)
+	if propertyType == filePrefix {
+		return provider.getFile(item, property)
 	}
-
-	// handle fields
-	return provider.getField(item, ref.Property)
+	return provider.getField(item, property)
 }
 
 // Validate checks if the client is configured correctly
@@ -374,13 +391,11 @@ func (provider *ProviderOnePassword) GetSecretMap(_ context.Context, ref esv1bet
 		return nil, err
 	}
 
-	// handle files
-	if item.Category == documentCategory {
-		return provider.getFiles(item, ref.Property)
+	propertyType, property := getObjType(item.Category, ref.Property)
+	if propertyType == filePrefix {
+		return provider.getFiles(item, property)
 	}
-
-	// handle fields
-	return provider.getFields(item, ref.Property)
+	return provider.getFields(item, property)
 }
 
 // GetAllSecrets syncs multiple 1Password Items into a single Kubernetes Secret, for dataFrom.find.
@@ -568,13 +583,9 @@ func (provider *ProviderOnePassword) getAllForVault(vaultID string, ref esv1beta
 		}
 
 		// handle files
-		if item.Category == documentCategory {
-			err = provider.getAllFiles(item, ref, secretData)
-			if err != nil {
-				return err
-			}
-
-			continue
+		err = provider.getAllFiles(item, ref, secretData)
+		if err != nil {
+			return err
 		}
 
 		// handle fields

pkg/provider/onepassword/onepassword_test.go

@@ -685,11 +685,26 @@ func TestGetSecret(t *testing.T) {
 				vaults: map[string]int{myVault: 1},
 				client: fake.NewMockClient().
 					AddPredictableVault(myVault).
-					AddPredictableItemWithField(myVault, myItem, key1, value1).
+					AppendItem(myVaultID, onepassword.Item{
+						ID:    myItemID,
+						Title: myItem,
+						Vault: onepassword.ItemVault{ID: myVaultID},
+						Files: []*onepassword.File{
+							{
+								ID:   myFilePNGID,
+								Name: myFilePNG,
+							},
+						},
+					}).
 					AppendItemField(myVaultID, myItemID, onepassword.ItemField{
 						Label: password,
 						Value: value2,
-					}),
+					}).
+					AppendItemField(myVaultID, myItemID, onepassword.ItemField{
+						Label: key1,
+						Value: value1,
+					}).
+					SetFileContents(myFilePNG, []byte(myContents)),
 			},
 			checks: []check{
 				{
@@ -702,6 +717,15 @@ func TestGetSecret(t *testing.T) {
 					expectedErr:   nil,
 				},
 				{
+					checkNote: key1 + " with prefix",
+					ref: esv1beta1.ExternalSecretDataRemoteRef{
+						Key:      myItem,
+						Property: fieldPrefix + prefixSplitter + key1,
+					},
+					expectedValue: value1,
+					expectedErr:   nil,
+				},
+				{
 					checkNote: "'password' (defaulted property)",
 					ref: esv1beta1.ExternalSecretDataRemoteRef{
 						Key: myItem,
@@ -718,6 +742,15 @@ func TestGetSecret(t *testing.T) {
 					},
 					expectedErr: errors.New(errVersionNotImplemented),
 				},
+				{
+					checkNote: "file named my-file.png with prefix",
+					ref: esv1beta1.ExternalSecretDataRemoteRef{
+						Key:      myItem,
+						Property: filePrefix + prefixSplitter + myFilePNG,
+					},
+					expectedValue: myContents,
+					expectedErr:   nil,
+				},
 			},
 		},
 		{
@@ -738,10 +771,23 @@ func TestGetSecret(t *testing.T) {
 							},
 						},
 					}).
+					AppendItemField(myVaultID, myItemID, onepassword.ItemField{
+						Label: key1,
+						Value: value2,
+					}).
 					SetFileContents(myFilePNG, []byte(myContents)),
 			},
 			checks: []check{
 				{
+					checkNote: "field named password",
+					ref: esv1beta1.ExternalSecretDataRemoteRef{
+						Key:      myItem,
+						Property: fieldPrefix + prefixSplitter + key1,
+					},
+					expectedValue: value2,
+					expectedErr:   nil,
+				},
+				{
 					checkNote: "file named my-file.png",
 					ref: esv1beta1.ExternalSecretDataRemoteRef{
 						Key:      myItem,
@@ -751,6 +797,15 @@ func TestGetSecret(t *testing.T) {
 					expectedErr:   nil,
 				},
 				{
+					checkNote: "file named my-file.png with prefix",
+					ref: esv1beta1.ExternalSecretDataRemoteRef{
+						Key:      myItem,
+						Property: filePrefix + prefixSplitter + myFilePNG,
+					},
+					expectedValue: myContents,
+					expectedErr:   nil,
+				},
+				{
 					checkNote: "empty ref.Property",
 					ref: esv1beta1.ExternalSecretDataRemoteRef{
 						Key: myItem,
@@ -766,8 +821,17 @@ func TestGetSecret(t *testing.T) {
 					},
 					expectedErr: fmt.Errorf(errDocumentNotFound, errors.New("'my-item', 'you-cant-find-me.png'")),
 				},
+				{
+					checkNote: "file non existent with prefix",
+					ref: esv1beta1.ExternalSecretDataRemoteRef{
+						Key:      myItem,
+						Property: "file/you-cant-find-me.png",
+					},
+					expectedErr: fmt.Errorf(errDocumentNotFound, errors.New("'my-item', 'you-cant-find-me.png'")),
+				},
 			},
 		},
+
 		{
 			setupNote: "one vault, one item, two fields w/ same Label",
 			provider: &ProviderOnePassword{
@@ -845,11 +909,31 @@ func TestGetSecretMap(t *testing.T) {
 				vaults: map[string]int{myVault: 1},
 				client: fake.NewMockClient().
 					AddPredictableVault(myVault).
-					AddPredictableItemWithField(myVault, myItem, key1, value1).
+					AppendItem(myVaultID, onepassword.Item{
+						ID:    myItemID,
+						Title: myItem,
+						Vault: onepassword.ItemVault{ID: myVaultID},
+						Files: []*onepassword.File{
+							{
+								ID:   myFilePNGID,
+								Name: myFilePNG,
+							},
+							{
+								ID:   myFile2ID,
+								Name: myFile2PNG,
+							},
+						},
+					}).
+					AppendItemField(myVaultID, myItemID, onepassword.ItemField{
+						Label: key1,
+						Value: value1,
+					}).
 					AppendItemField(myVaultID, myItemID, onepassword.ItemField{
 						Label: password,
 						Value: value2,
-					}),
+					}).
+					SetFileContents(myFilePNG, []byte(myContents)).
+					SetFileContents(myFile2PNG, []byte(myContents2)),
 			},
 			checks: []check{
 				{
@@ -883,6 +967,17 @@ func TestGetSecretMap(t *testing.T) {
 					},
 					expectedErr: errors.New(errVersionNotImplemented),
 				},
+				{
+					checkNote: "limit by Property with prefix",
+					ref: esv1beta1.ExternalSecretDataRemoteRef{
+						Key:      myItem,
+						Property: filePrefix + prefixSplitter + myFilePNG,
+					},
+					expectedMap: map[string][]byte{
+						myFilePNG: []byte(myContents),
+					},
+					expectedErr: nil,
+				},
 			},
 		},
 		{
@@ -907,6 +1002,10 @@ func TestGetSecretMap(t *testing.T) {
 							},
 						},
 					}).
+					AppendItemField(myVaultID, myItemID, onepassword.ItemField{
+						Label: key1,
+						Value: value2,
+					}).
 					SetFileContents(myFilePNG, []byte(myContents)).
 					SetFileContents(myFile2PNG, []byte(myContents2)),
 			},
@@ -933,6 +1032,28 @@ func TestGetSecretMap(t *testing.T) {
 					},
 					expectedErr: nil,
 				},
+				{
+					checkNote: "limit by Property with prefix",
+					ref: esv1beta1.ExternalSecretDataRemoteRef{
+						Key:      myItem,
+						Property: filePrefix + prefixSplitter + myFilePNG,
+					},
+					expectedMap: map[string][]byte{
+						myFilePNG: []byte(myContents),
+					},
+					expectedErr: nil,
+				},
+				{
+					checkNote: "get field limit by Property",
+					ref: esv1beta1.ExternalSecretDataRemoteRef{
+						Key:      myItem,
+						Property: fieldPrefix + prefixSplitter + key1,
+					},
+					expectedMap: map[string][]byte{
+						key1: []byte(value2),
+					},
+					expectedErr: nil,
+				},
 			},
 		},
 		{