Skip to content

Commit 44df351

Browse files
committed
fix(ksm): handle duplicate secrets with the same uid
Keeper Secrets Manager can return multiple records with the same uid. This is because KSM is now returning a secret for each time a secret is referenced. This breaks previous assumptions.
1 parent 810f1fe commit 44df351

File tree

2 files changed

+118
-18
lines changed

2 files changed

+118
-18
lines changed

pkg/backends/keepersecretsmanager.go

+2-4
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ func buildSecretsMap(secretsMap map[string]interface{}, fieldMap map[string]inte
9898
// GetSecrets gets secrets from Keeper Secrets Manager. It does not currently
9999
// implement anything related to versions or annotations.
100100
func (a *KeeperSecretsManager) GetSecrets(path string, version string, annotations map[string]string) (map[string]interface{}, error) {
101+
// keeper returns a record multiple times if the record is used in multiple folders.
102+
// This means that even when filtering by uid you can recieve many different records.
101103
records, err := a.client.GetSecrets([]string{
102104
path,
103105
})
@@ -110,10 +112,6 @@ func (a *KeeperSecretsManager) GetSecrets(path string, version string, annotatio
110112
return nil, fmt.Errorf("no secrets could be found with the given path: %s", path)
111113
}
112114

113-
if len(records) > 1 {
114-
return nil, fmt.Errorf("unexpectedly multiple secrets were found with the given path: %s", path)
115-
}
116-
117115
utils.VerboseToStdErr("Keeper Secrets Manager decoding record %s", records[0].Title())
118116

119117
dict := records[0].RecordDict

pkg/backends/keepersecretsmanager_test.go

+116-14
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func recordFromJSON(data string) *ksm.Record {
4949

5050
func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
5151
type args struct {
52-
data string
52+
records []string
5353
}
5454
tests := []struct {
5555
name string
@@ -60,7 +60,8 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
6060
{
6161
name: "should handle a secret of type login",
6262
args: args{
63-
data: `{
63+
records: []string{`
64+
{
6465
"uid": "some-uid",
6566
"title": "some-title",
6667
"type": "login",
@@ -98,6 +99,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
9899
"custom": [],
99100
"files": []
100101
}`,
102+
},
101103
},
102104
want: map[string]interface{}{
103105
"login": "some-secret-username",
@@ -107,7 +109,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
107109
{
108110
name: "should handle a secret of type databaseCredentials",
109111
args: args{
110-
data: `
112+
records: []string{`
111113
{
112114
"uid": "some-uid",
113115
"title": "some-title",
@@ -153,6 +155,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
153155
"custom": [],
154156
"files": []
155157
}`,
158+
},
156159
},
157160
want: map[string]interface{}{
158161
"host": map[string]interface{}{
@@ -167,7 +170,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
167170
{
168171
name: "should handle a secret of type encryptedNotes",
169172
args: args{
170-
data: `
173+
records: []string{`
171174
{
172175
"uid": "some-uid",
173176
"title": "some-title",
@@ -194,6 +197,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
194197
"custom": [],
195198
"files": []
196199
}`,
200+
},
197201
},
198202
want: map[string]interface{}{
199203
"note": "some-value",
@@ -202,7 +206,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
202206
{
203207
name: "should handle a secret with custom fields",
204208
args: args{
205-
data: `
209+
records: []string{`
206210
{
207211
"uid": "some-uid",
208212
"title": "some-title",
@@ -237,6 +241,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
237241
],
238242
"files": []
239243
}`,
244+
},
240245
},
241246
want: map[string]interface{}{
242247
"note": "some-value",
@@ -246,7 +251,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
246251
{
247252
name: "should not overwrite a built in field when a custom field of the same label exists",
248253
args: args{
249-
data: `
254+
records: []string{`
250255
{
251256
"uid": "some-uid",
252257
"title": "some-title",
@@ -281,6 +286,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
281286
],
282287
"files": []
283288
}`,
289+
},
284290
},
285291
want: map[string]interface{}{
286292
"note": "some-value",
@@ -289,7 +295,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
289295
{
290296
name: "should omit fields that have multiple values",
291297
args: args{
292-
data: `
298+
records: []string{`
293299
{
294300
"uid": "some-uid",
295301
"title": "some-title",
@@ -324,6 +330,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
324330
"custom": [],
325331
"files": []
326332
}`,
333+
},
327334
},
328335
want: map[string]interface{}{
329336
"note": "some-value",
@@ -333,7 +340,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
333340
{
334341
name: "should omit fields that don't have a value",
335342
args: args{
336-
data: `
343+
records: []string{`
337344
{
338345
"uid": "some-uid",
339346
"title": "some-title",
@@ -364,6 +371,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
364371
"custom": [],
365372
"files": []
366373
}`,
374+
},
367375
},
368376
want: map[string]interface{}{
369377
"note": "some-value",
@@ -372,7 +380,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
372380
{
373381
name: "should omit fields that don't have a type",
374382
args: args{
375-
data: `
383+
records: []string{`
376384
{
377385
"uid": "some-uid",
378386
"title": "some-title",
@@ -405,6 +413,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
405413
"custom": [],
406414
"files": []
407415
}`,
416+
},
408417
},
409418
want: map[string]interface{}{
410419
"note": "some-value",
@@ -413,7 +422,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
413422
{
414423
name: "should omit fields that don't have a label or type",
415424
args: args{
416-
data: `
425+
records: []string{`
417426
{
418427
"uid": "some-uid",
419428
"title": "some-title",
@@ -445,6 +454,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
445454
"custom": [],
446455
"files": []
447456
}`,
457+
},
448458
},
449459
want: map[string]interface{}{
450460
"note": "some-value",
@@ -453,7 +463,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
453463
{
454464
name: "should omit fields that don't have a value that is not a slice",
455465
args: args{
456-
data: `
466+
records: []string{`
457467
{
458468
"uid": "some-uid",
459469
"title": "some-title",
@@ -485,21 +495,113 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
485495
"custom": [],
486496
"files": []
487497
}`,
498+
},
488499
},
489500
want: map[string]interface{}{
490501
"note": "some-value",
491502
},
492503
},
504+
505+
{
506+
name: "should handle multiple records with the same uid",
507+
args: args{
508+
records: []string{`
509+
{
510+
"uid": "some-uid",
511+
"title": "some-title",
512+
"type": "login",
513+
"fields": [
514+
{
515+
"label": "login",
516+
"type": "login",
517+
"value": [
518+
"some-secret-username"
519+
]
520+
},
521+
{
522+
"label": "password",
523+
"type": "password",
524+
"value": [
525+
"some-secret-password"
526+
]
527+
},
528+
{
529+
"label": "url",
530+
"type": "url",
531+
"value": []
532+
},
533+
{
534+
"label": "fileRef",
535+
"type": "fileRef",
536+
"value": []
537+
},
538+
{
539+
"label": "oneTimeCode",
540+
"type": "oneTimeCode",
541+
"value": []
542+
}
543+
],
544+
"custom": [],
545+
"files": []
546+
}`, `
547+
{
548+
"uid": "some-uid",
549+
"title": "some-title",
550+
"type": "login",
551+
"fields": [
552+
{
553+
"label": "login",
554+
"type": "login",
555+
"value": [
556+
"some-secret-username"
557+
]
558+
},
559+
{
560+
"label": "password",
561+
"type": "password",
562+
"value": [
563+
"some-secret-password"
564+
]
565+
},
566+
{
567+
"label": "url",
568+
"type": "url",
569+
"value": []
570+
},
571+
{
572+
"label": "fileRef",
573+
"type": "fileRef",
574+
"value": []
575+
},
576+
{
577+
"label": "oneTimeCode",
578+
"type": "oneTimeCode",
579+
"value": []
580+
}
581+
],
582+
"custom": [],
583+
"files": []
584+
}`,
585+
},
586+
},
587+
want: map[string]interface{}{
588+
"login": "some-secret-username",
589+
"password": "some-secret-password",
590+
},
591+
},
493592
}
494593
for _, tt := range tests {
495594
t.Run(tt.name, func(t *testing.T) {
595+
records := make([]*ksm.Record, len(tt.args.records))
596+
for i, recordStr := range tt.args.records {
597+
records[i] = recordFromJSON(recordStr)
598+
}
599+
496600
a := backends.NewKeeperSecretsManagerBackend(
497601
MockKeeperClient{
498602
mocks: map[string]mockResults{
499603
"path": {
500-
Records: []*ksm.Record{
501-
recordFromJSON(tt.args.data),
502-
},
604+
Records: records,
503605
},
504606
},
505607
},

0 commit comments

Comments
 (0)