fix(keepersecurity): properly handle fields key (#5674)

Signed-off-by: Pedro Parra Ortega <parraortega.pedro@gmail.com>
Co-authored-by: Gergely Brautigam <skarlso777@gmail.com>
Co-authored-by: Gustavo Fernandes de Carvalho <17139678+gusfcarvalho@users.noreply.github.com>

docs/provider/keeper-security.md

@@ -43,7 +43,7 @@ Be sure the `keepersecurity` provider is listed in the `Kind=SecretStore`
 * How a Record is equated to an ExternalSecret:
     * `remoteRef.key` is equated to a Record's ID
     * `remoteRef.property` is equated to one of the following options:
-        * Fields: [Record's field's Type](https://docs.keeper.io/secrets-manager/secrets-manager/about/field-record-types)
+        * Fields: Record's field's Label (if present), otherwise [Record's field's Type](https://docs.keeper.io/secrets-manager/secrets-manager/about/field-record-types)
         * CustomFields: Record's field's Label
         * Files: Record's file's Name
         * If empty, defaults to the complete Record in JSON format
@@ -51,7 +51,7 @@ Be sure the `keepersecurity` provider is listed in the `Kind=SecretStore`
 * `dataFrom`:
     * `find.path` is currently not supported.
     * `find.name.regexp` is equated to one of the following options:
-        * Fields: Record's field's Type
+        * Fields: Record's field's Label (if present), otherwise Record's field's Type
         * CustomFields: Record's field's Label
         * Files: Record's file's Name
     * `find.tags` are not supported at this time.

providers/v1/keepersecurity/client.go

@@ -81,9 +81,10 @@ type SecurityClient interface {
 	Save(record *ksm.Record) error
 }
 
-// Field represents a KeeperSecurity field with its type and value.
+// Field represents a KeeperSecurity field with its type, label (optional), and value.
 type Field struct {
 	Type  string `json:"type"`
+	Label string `json:"label,omitempty"`
 	Value []any  `json:"value"`
 }
 
@@ -384,7 +385,11 @@ func (c *Client) findSecretByName(name string) (*ksm.Record, error) {
 func (s *Secret) validate() error {
 	fields := make(map[string]int)
 	for _, field := range s.Fields {
-		fields[field.Type]++
+		fieldKey := field.Label
+		if fieldKey == "" {
+			fieldKey = field.Type
+		}
+		fields[fieldKey]++
 	}
 
 	for _, customField := range s.Custom {
@@ -471,7 +476,11 @@ func getFieldValue(value []any) []byte {
 
 func (s *Secret) getField(key string) ([]byte, error) {
 	for _, field := range s.Fields {
-		if field.Type == key && field.Type != keeperSecurityFileRef && field.Type != keeperSecurityMfa && len(field.Value) > 0 {
+		fieldKey := field.Label
+		if fieldKey == "" {
+			fieldKey = field.Type
+		}
+		if fieldKey == key && field.Type != keeperSecurityFileRef && field.Type != keeperSecurityMfa && len(field.Value) > 0 {
 			return getFieldValue(field.Value), nil
 		}
 	}
@@ -483,7 +492,11 @@ func (s *Secret) getFields() map[string][]byte {
 	secretData := make(map[string][]byte)
 	for _, field := range s.Fields {
 		if len(field.Value) > 0 {
-			secretData[field.Type] = getFieldValue(field.Value)
+			fieldKey := field.Label
+			if fieldKey == "" {
+				fieldKey = field.Type
+			}
+			secretData[fieldKey] = getFieldValue(field.Value)
 		}
 	}
 

providers/v1/keepersecurity/client_test.go

@@ -39,13 +39,17 @@ const (
 	outputRecord0       = "{\"title\":\"record0\",\"type\":\"login\",\"fields\":[{\"type\":\"login\",\"value\":[\"foo\"]},{\"type\":\"password\",\"value\":[\"bar\"]}],\"custom\":[{\"type\":\"host\",\"label\":\"host0\",\"value\":[{\"hostName\":\"mysql\",\"port\":\"3306\"}]}],\"files\":null}"
 	outputRecord1       = "{\"title\":\"record1\",\"type\":\"login\",\"fields\":[{\"type\":\"login\",\"value\":[\"foo\"]},{\"type\":\"password\",\"value\":[\"bar\"]}],\"custom\":[{\"type\":\"host\",\"label\":\"host1\",\"value\":[{\"hostName\":\"mysql\",\"port\":\"3306\"}]}],\"files\":null}"
 	outputRecord2       = "{\"title\":\"record2\",\"type\":\"login\",\"fields\":[{\"type\":\"login\",\"value\":[\"foo\"]},{\"type\":\"password\",\"value\":[\"bar\"]}],\"custom\":[{\"type\":\"host\",\"label\":\"host2\",\"value\":[{\"hostName\":\"mysql\",\"port\":\"3306\"}]}],\"files\":null}"
-	record0             = "record0"
-	record1             = "record1"
-	record2             = "record2"
-	LoginKey            = "login"
-	PasswordKey         = "password"
-	HostKeyFormat       = "host%d"
-	RecordNameFormat    = "record%d"
+	outputRecordWithLabels = "{\"title\":\"recordWithLabels\",\"type\":\"login\",\"fields\":[{\"type\":\"login\",\"label\":\"username\",\"value\":[\"foo\"]},{\"type\":\"password\",\"label\":\"pass\",\"value\":[\"bar\"]}],\"custom\":[{\"type\":\"host\",\"label\":\"host0\",\"value\":[{\"hostName\":\"mysql\",\"port\":\"3306\"}]}],\"files\":null}"
+	record0                 = "record0"
+	record1                 = "record1"
+	record2                 = "record2"
+	recordWithLabels        = "recordWithLabels"
+	LoginKey                = "login"
+	PasswordKey             = "password"
+	HostKeyFormat           = "host%d"
+	RecordNameFormat        = "record%d"
+	UsernameLabel = "username"
+	PassLabel     = "pass"
 )
 
 func TestClientDeleteSecret(t *testing.T) {
@@ -236,6 +240,29 @@ func TestClientGetAllSecrets(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "Get secrets with labels using matching regex",
+			fields: fields{
+				ksmClient: &fake.MockKeeperClient{
+					GetSecretsFn: func(strings []string) ([]*ksm.Record, error) {
+						return []*ksm.Record{generateRecordWithLabels()}, nil
+					},
+				},
+				folderID: folderID,
+			},
+			args: args{
+				ctx: context.Background(),
+				ref: esv1.ExternalSecretFind{
+					Name: &esv1.FindName{
+						RegExp: recordWithLabels,
+					},
+				},
+			},
+			want: map[string][]byte{
+				recordWithLabels: []byte(outputRecordWithLabels),
+			},
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -272,7 +299,7 @@ func TestClientGetSecret(t *testing.T) {
 		wantErr bool
 	}{
 		{
-			name: "Get Secret with a property",
+			name: "Get Secret with a property (no label)",
 			fields: fields{
 				ksmClient: &fake.MockKeeperClient{
 					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
@@ -292,7 +319,7 @@ func TestClientGetSecret(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "Get Secret without property",
+			name: "Get Secret without property (no label)",
 			fields: fields{
 				ksmClient: &fake.MockKeeperClient{
 					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
@@ -311,6 +338,64 @@ func TestClientGetSecret(t *testing.T) {
 			wantErr: false,
 		},
 		{
+			name: "Get Secret with a property using label",
+			fields: fields{
+				ksmClient: &fake.MockKeeperClient{
+					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
+						return []*ksm.Record{generateRecordWithLabels()}, nil
+					},
+				},
+				folderID: folderID,
+			},
+			args: args{
+				ctx: context.Background(),
+				ref: esv1.ExternalSecretDataRemoteRef{
+					Key:      recordWithLabels,
+					Property: UsernameLabel,
+				},
+			},
+			want:    []byte("foo"),
+			wantErr: false,
+		},
+		{
+			name: "Get Secret with a property using type when label exists",
+			fields: fields{
+				ksmClient: &fake.MockKeeperClient{
+					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
+						return []*ksm.Record{generateRecordWithLabels()}, nil
+					},
+				},
+				folderID: folderID,
+			},
+			args: args{
+				ctx: context.Background(),
+				ref: esv1.ExternalSecretDataRemoteRef{
+					Key:      recordWithLabels,
+					Property: LoginKey, // Try to access by type when label exists
+				},
+			},
+			wantErr: true, // Should fail because label takes precedence
+		},
+		{
+			name: "Get Secret without property (with labels)",
+			fields: fields{
+				ksmClient: &fake.MockKeeperClient{
+					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
+						return []*ksm.Record{generateRecordWithLabels()}, nil
+					},
+				},
+				folderID: folderID,
+			},
+			args: args{
+				ctx: context.Background(),
+				ref: esv1.ExternalSecretDataRemoteRef{
+					Key: recordWithLabels,
+				},
+			},
+			want:    []byte(outputRecordWithLabels),
+			wantErr: false,
+		},
+		{
 			name: "Get secret with multiple matches by ID",
 			fields: fields{
 				ksmClient: &fake.MockKeeperClient{
@@ -402,7 +487,7 @@ func TestClientGetSecretMap(t *testing.T) {
 		wantErr bool
 	}{
 		{
-			name: "Get Secret with valid property",
+			name: "Get Secret with valid property (no label)",
 			fields: fields{
 				ksmClient: &fake.MockKeeperClient{
 					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
@@ -424,7 +509,7 @@ func TestClientGetSecretMap(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "Get Secret without property",
+			name: "Get Secret without property (no label)",
 			fields: fields{
 				ksmClient: &fake.MockKeeperClient{
 					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
@@ -447,6 +532,51 @@ func TestClientGetSecretMap(t *testing.T) {
 			wantErr: false,
 		},
 		{
+			name: "Get Secret with valid property using label",
+			fields: fields{
+				ksmClient: &fake.MockKeeperClient{
+					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
+						return []*ksm.Record{generateRecordWithLabels()}, nil
+					},
+				},
+				folderID: folderID,
+			},
+			args: args{
+				ctx: context.Background(),
+				ref: esv1.ExternalSecretDataRemoteRef{
+					Key:      recordWithLabels,
+					Property: UsernameLabel,
+				},
+			},
+			want: map[string][]byte{
+				UsernameLabel: []byte("foo"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "Get Secret without property (with labels)",
+			fields: fields{
+				ksmClient: &fake.MockKeeperClient{
+					GetSecretsFn: func(filter []string) ([]*ksm.Record, error) {
+						return []*ksm.Record{generateRecordWithLabels()}, nil
+					},
+				},
+				folderID: folderID,
+			},
+			args: args{
+				ctx: context.Background(),
+				ref: esv1.ExternalSecretDataRemoteRef{
+					Key: recordWithLabels,
+				},
+			},
+			want: map[string][]byte{
+				UsernameLabel:                 []byte("foo"),
+				PassLabel:                     []byte("bar"),
+				fmt.Sprintf(HostKeyFormat, 0): []byte("{\"hostName\":\"mysql\",\"port\":\"3306\"}"),
+			},
+			wantErr: false,
+		},
+		{
 			name: "Get non existing secret",
 			fields: fields{
 				ksmClient: &fake.MockKeeperClient{
@@ -689,3 +819,20 @@ func generateRecords() []*ksm.Record {
 
 	return records
 }
+
+func generateRecordWithLabels() *ksm.Record {
+	record := ksm.Record{
+		Uid: recordWithLabels,
+		RecordDict: map[string]any{
+			"type":      externalSecretType,
+			"folderUID": folderID,
+		},
+	}
+	// Fields with labels - using label as key
+	sec := fmt.Sprintf("{\"title\":%q,\"type\":\"login\",\"fields\":[{\"type\":\"login\",\"label\":%q,\"value\":[\"foo\"]},{\"type\":\"password\",\"label\":%q,\"value\":[\"bar\"]}],\"custom\":[{\"type\":\"host\",\"label\":\"host0\",\"value\":[{\"hostName\":\"mysql\",\"port\":\"3306\"}]}]}", recordWithLabels, UsernameLabel, PassLabel)
+	record.SetTitle(recordWithLabels)
+	record.SetStandardFieldValue(LoginKey, "foo")
+	record.SetStandardFieldValue(PasswordKey, "bar")
+	record.RawJson = sec
+	return &record
+}