Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(ksm): handle duplicate secrets with the same uid #652

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(ksm): handle duplicate secrets with the same uid #652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions pkg/backends/keepersecretsmanager.go
View file
Open in desktop
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.
robbert229 marked this conversation as resolved.
Show resolved Hide resolved
// 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 {
robbert229 marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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{`
robbert229 marked this conversation as resolved.
Show resolved Hide resolved
{
"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",
},
robbert229 marked this conversation as resolved.
Show resolved Hide resolved
},

{
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 {
robbert229 marked this conversation as resolved.
Show resolved Hide resolved
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