Skip to content

Commit

Permalink
fix(ksm): handle duplicate secrets with the same uid
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@robbert229
robbert229 committed Jul 8, 2024
1 parent 810f1fe commit e585d00
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 18 deletions.
6 changes: 2 additions & 4 deletions pkg/backends/keepersecretsmanager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand All @@ -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
Expand Down
130 changes: 116 additions & 14 deletions pkg/backends/keepersecretsmanager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand Down Expand Up @@ -98,6 +99,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
"custom": [],
"files": []
}`,
},
},
want: map[string]interface{}{
"login": "some-secret-username",
Expand All @@ -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",
Expand Down Expand Up @@ -153,6 +155,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
"custom": [],
"files": []
}`,
},
},
want: map[string]interface{}{
"host": map[string]interface{}{
Expand All @@ -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",
Expand All @@ -194,6 +197,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
"custom": [],
"files": []
}`,
},
},
want: map[string]interface{}{
"note": "some-value",
Expand All @@ -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",
Expand Down Expand Up @@ -237,6 +241,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
],
"files": []
}`,
},
},
want: map[string]interface{}{
"note": "some-value",
Expand All @@ -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",
Expand Down Expand Up @@ -281,6 +286,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
],
"files": []
}`,
},
},
want: map[string]interface{}{
"note": "some-value",
Expand All @@ -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",
Expand Down Expand Up @@ -324,6 +330,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
"custom": [],
"files": []
}`,
},
},
want: map[string]interface{}{
"note": "some-value",
Expand All @@ -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",
Expand Down Expand Up @@ -364,6 +371,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
"custom": [],
"files": []
}`,
},
},
want: map[string]interface{}{
"note": "some-value",
Expand All @@ -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",
Expand Down Expand Up @@ -405,6 +413,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
"custom": [],
"files": []
}`,
},
},
want: map[string]interface{}{
"note": "some-value",
Expand All @@ -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",
Expand Down Expand Up @@ -445,6 +454,7 @@ func TestKeeperSecretsManager_GetSecrets(t *testing.T) {
"custom": [],
"files": []
}`,
},
},
want: map[string]interface{}{
"note": "some-value",
Expand All @@ -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",
Expand Down Expand Up @@ -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,
},
},
},
Expand Down

0 comments on commit e585d00

Please sign in to comment.