From e585d00be3a212b62d00493d438bb191b8d372ae Mon Sep 17 00:00:00 2001 From: John Rowley Date: Mon, 8 Jul 2024 13:49:00 -0700 Subject: [PATCH] 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. Signed-off-by John Rowley --- pkg/backends/keepersecretsmanager.go | 6 +- pkg/backends/keepersecretsmanager_test.go | 130 +++++++++++++++++++--- 2 files changed, 118 insertions(+), 18 deletions(-) diff --git a/pkg/backends/keepersecretsmanager.go b/pkg/backends/keepersecretsmanager.go index d2e3d3e3..fdec0f7f 100644 --- a/pkg/backends/keepersecretsmanager.go +++ b/pkg/backends/keepersecretsmanager.go @@ -98,6 +98,8 @@ func buildSecretsMap(secretsMap map[string]interface{}, fieldMap map[string]inte // GetSecrets gets secrets from Keeper Secrets Manager. It does not currently // implement anything related to versions or annotations. func (a *KeeperSecretsManager) GetSecrets(path string, version string, annotations map[string]string) (map[string]interface{}, error) { + // keeper returns a record multiple times if the record is used in multiple folders. + // This means that even when filtering by uid you can recieve many different records. records, err := a.client.GetSecrets([]string{ path, }) @@ -110,10 +112,6 @@ func (a *KeeperSecretsManager) GetSecrets(path string, version string, annotatio return nil, fmt.Errorf("no secrets could be found with the given path: %s", path) } - if len(records) > 1 { - return nil, fmt.Errorf("unexpectedly multiple secrets were found with the given path: %s", path) - } - utils.VerboseToStdErr("Keeper Secrets Manager decoding record %s", records[0].Title()) dict := records[0].RecordDict diff --git a/pkg/backends/keepersecretsmanager_test.go b/pkg/backends/keepersecretsmanager_test.go index b095ed3e..866685d3 100644 --- a/pkg/backends/keepersecretsmanager_test.go +++ b/pkg/backends/keepersecretsmanager_test.go @@ -49,7 +49,7 @@ func recordFromJSON(data string) *ksm.Record { func TestKeeperSecretsManager_GetSecrets(t *testing.T) { type args struct { - data string + records []string } tests := []struct { name string @@ -60,7 +60,8 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should handle a secret of type login", args: args{ - data: `{ + records: []string{` +{ "uid": "some-uid", "title": "some-title", "type": "login", @@ -98,6 +99,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { "custom": [], "files": [] }`, + }, }, want: map[string]interface{}{ "login": "some-secret-username", @@ -107,7 +109,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should handle a secret of type databaseCredentials", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -153,6 +155,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { "custom": [], "files": [] }`, + }, }, want: map[string]interface{}{ "host": map[string]interface{}{ @@ -167,7 +170,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should handle a secret of type encryptedNotes", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -194,6 +197,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { "custom": [], "files": [] }`, + }, }, want: map[string]interface{}{ "note": "some-value", @@ -202,7 +206,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should handle a secret with custom fields", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -237,6 +241,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { ], "files": [] }`, + }, }, want: map[string]interface{}{ "note": "some-value", @@ -246,7 +251,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should not overwrite a built in field when a custom field of the same label exists", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -281,6 +286,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { ], "files": [] }`, + }, }, want: map[string]interface{}{ "note": "some-value", @@ -289,7 +295,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should omit fields that have multiple values", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -324,6 +330,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { "custom": [], "files": [] }`, + }, }, want: map[string]interface{}{ "note": "some-value", @@ -333,7 +340,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should omit fields that don't have a value", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -364,6 +371,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { "custom": [], "files": [] }`, + }, }, want: map[string]interface{}{ "note": "some-value", @@ -372,7 +380,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should omit fields that don't have a type", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -405,6 +413,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { "custom": [], "files": [] }`, + }, }, want: map[string]interface{}{ "note": "some-value", @@ -413,7 +422,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should omit fields that don't have a label or type", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -445,6 +454,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { "custom": [], "files": [] }`, + }, }, want: map[string]interface{}{ "note": "some-value", @@ -453,7 +463,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { { name: "should omit fields that don't have a value that is not a slice", args: args{ - data: ` + records: []string{` { "uid": "some-uid", "title": "some-title", @@ -485,21 +495,113 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) { "custom": [], "files": [] }`, + }, }, want: map[string]interface{}{ "note": "some-value", }, }, + + { + name: "should handle multiple records with the same uid", + args: args{ + records: []string{` +{ + "uid": "some-uid", + "title": "some-title", + "type": "login", + "fields": [ + { + "label": "login", + "type": "login", + "value": [ + "some-secret-username" + ] + }, + { + "label": "password", + "type": "password", + "value": [ + "some-secret-password" + ] + }, + { + "label": "url", + "type": "url", + "value": [] + }, + { + "label": "fileRef", + "type": "fileRef", + "value": [] + }, + { + "label": "oneTimeCode", + "type": "oneTimeCode", + "value": [] + } + ], + "custom": [], + "files": [] +}`, ` +{ + "uid": "some-uid", + "title": "some-title", + "type": "login", + "fields": [ + { + "label": "login", + "type": "login", + "value": [ + "some-secret-username" + ] + }, + { + "label": "password", + "type": "password", + "value": [ + "some-secret-password" + ] + }, + { + "label": "url", + "type": "url", + "value": [] + }, + { + "label": "fileRef", + "type": "fileRef", + "value": [] + }, + { + "label": "oneTimeCode", + "type": "oneTimeCode", + "value": [] + } + ], + "custom": [], + "files": [] +}`, + }, + }, + want: map[string]interface{}{ + "login": "some-secret-username", + "password": "some-secret-password", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + records := make([]*ksm.Record, len(tt.args.records)) + for i, recordStr := range tt.args.records { + records[i] = recordFromJSON(recordStr) + } + a := backends.NewKeeperSecretsManagerBackend( MockKeeperClient{ mocks: map[string]mockResults{ "path": { - Records: []*ksm.Record{ - recordFromJSON(tt.args.data), - }, + Records: records, }, }, },