Skip to content

Commit 5f3e3b2

Browse files
BenehikoBenehiko
committed
store/keychain: use common filter function between darwin and windows
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
1 parent 6ce3c07 commit 5f3e3b2

5 files changed

Lines changed: 158 additions & 58 deletions

File tree

‎store/keychain/keychain.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package keychain
22

33
import (
44
"errors"
5+
"strings"
56

67
"github.com/docker/secrets-engine/store"
78
)
@@ -57,3 +58,16 @@ func New[T store.Secret](serviceGroup, serviceName string, factory Factory[T]) (
5758
func (k *keychainStore[T]) itemLabel(id store.ID) string {
5859
return k.serviceGroup + ":" + k.serviceName + ":" + id.String()
5960
}
61+
62+
// matchesFilter takes in two maps, a filter map containing key-value pairs
63+
// that must be found in the secretAttributes map.
64+
// filter may be nil, in which case it returns true as there are no values
65+
// that should be filtered on.
66+
func matchesFilter(filter, secretAttributes map[string]string) bool {
67+
for k, v := range filter {
68+
if vv, ok := secretAttributes[k]; !ok || !strings.EqualFold(v, vv) {
69+
return false
70+
}
71+
}
72+
return true
73+
}

‎store/keychain/keychain_darwin.go‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"slices"
87
"strings"
98

109
kc "github.com/Benehiko/go-keychain/v2"
@@ -183,22 +182,7 @@ func (k *keychainStore[T]) Save(_ context.Context, id store.ID, secret store.Sec
183182
return mapKeychainError(kc.AddItem(item))
184183
}
185184

186-
func hasAttribute(searchAttributes, queryAttributes map[string]string) bool {
187-
if searchAttributes == nil {
188-
return true
189-
}
190-
allMatches := make([]bool, len(searchAttributes))
191-
var i int
192-
for k, v := range queryAttributes {
193-
if needle, ok := searchAttributes[k]; ok && strings.EqualFold(needle, v) {
194-
allMatches[i] = true
195-
i++
196-
}
197-
}
198-
return !slices.Contains(allMatches, false)
199-
}
200-
201-
func (k *keychainStore[T]) Filter(_ context.Context, id store.ID, attributes map[string]string) (map[store.ID]store.Secret, error) {
185+
func (k *keychainStore[T]) Filter(_ context.Context, id store.ID, filter map[string]string) (map[store.ID]store.Secret, error) {
202186
if err := id.Valid(); err != nil {
203187
return nil, err
204188
}
@@ -215,11 +199,11 @@ func (k *keychainStore[T]) Filter(_ context.Context, id store.ID, attributes map
215199
return nil, mapKeychainError(err)
216200
}
217201

218-
if attributes == nil {
219-
attributes = make(map[string]string)
202+
if filter == nil {
203+
filter = make(map[string]string)
220204
}
221205
for p := range strings.SplitSeq(id.String(), "/") {
222-
attributes[p] = p
206+
filter[p] = p
223207
}
224208

225209
creds := make(map[store.ID]store.Secret)
@@ -234,7 +218,7 @@ func (k *keychainStore[T]) Filter(_ context.Context, id store.ID, attributes map
234218
return nil, err
235219
}
236220

237-
if !hasAttribute(attributes, attr) {
221+
if !matchesFilter(filter, attr) {
238222
continue
239223
}
240224

‎store/keychain/keychain_darwin_test.go‎

Lines changed: 67 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"testing"
88

99
"github.com/google/uuid"
10+
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
1112

1213
"github.com/docker/secrets-engine/store"
@@ -23,49 +24,102 @@ func TestMacosKeychain(t *testing.T) {
2324
serviceGroup = "test.testing." + uuid.NewString()
2425
id = store.ID(serviceGroup + "/" + serviceName + "/" + uuid.NewString())
2526
)
26-
store := keychainStore[*mocks.MockCredential]{
27+
keychainStore := keychainStore[*mocks.MockCredential]{
2728
serviceGroup: "test.testing." + uuid.NewString(),
2829
serviceName: uuid.NewString(),
2930
factory: func() *mocks.MockCredential {
3031
return &mocks.MockCredential{}
3132
},
3233
}
3334

35+
ids := []store.ID{
36+
store.ID(serviceGroup + "/" + serviceName + "/" + uuid.NewString()),
37+
store.ID(serviceGroup + "/" + serviceName + "/" + uuid.NewString()),
38+
store.ID(serviceGroup + "/" + serviceName + "/" + uuid.NewString()),
39+
}
40+
t.Cleanup(func() {
41+
for _, id := range ids {
42+
assert.NoError(t, keychainStore.Delete(t.Context(), id))
43+
}
44+
})
45+
for _, id := range ids {
46+
assert.NoError(t, keychainStore.Save(t.Context(), id, &mocks.MockCredential{
47+
Username: uuid.NewString(),
48+
Password: uuid.NewString(),
49+
Attributes: map[string]string{
50+
"color": "purple",
51+
"game": "unknown",
52+
},
53+
}))
54+
}
55+
3456
t.Run("can have no attributes", func(t *testing.T) {
3557
t.Cleanup(func() {
36-
require.NoError(t, store.Delete(t.Context(), id))
58+
assert.NoError(t, keychainStore.Delete(t.Context(), id))
3759
})
38-
require.NoError(t, store.Save(t.Context(), id, secret))
60+
require.NoError(t, keychainStore.Save(t.Context(), id, secret))
3961
})
4062
t.Run("can store large attributes", func(t *testing.T) {
4163
t.Cleanup(func() {
42-
require.NoError(t, store.Delete(t.Context(), id))
64+
assert.NoError(t, keychainStore.Delete(t.Context(), id))
4365
})
4466
large := bytes.Repeat([]byte{'a'}, 1024*1024)
4567
secret.Attributes = map[string]string{
4668
"large": string(large),
4769
"small": "eyy",
4870
}
49-
require.NoError(t, store.Save(t.Context(), id, secret))
71+
require.NoError(t, keychainStore.Save(t.Context(), id, secret))
5072
})
5173
t.Run("filter populates both metadata and secret", func(t *testing.T) {
5274
t.Cleanup(func() {
53-
require.NoError(t, store.Delete(t.Context(), id))
75+
assert.NoError(t, keychainStore.Delete(t.Context(), id))
5476
})
5577
secret.Attributes = map[string]string{
5678
"game": "elden ring",
5779
}
58-
require.NoError(t, store.Save(t.Context(), id, secret))
59-
secrets, err := store.Filter(t.Context(), id, map[string]string{
80+
require.NoError(t, keychainStore.Save(t.Context(), id, secret))
81+
secrets, err := keychainStore.Filter(t.Context(), id, map[string]string{
6082
"game": "elden ring",
6183
})
6284
require.NoError(t, err)
63-
require.Len(t, secrets, 1)
64-
require.Subset(t, secrets[id].Metadata(), map[string]string{
85+
assert.Len(t, secrets, 1)
86+
assert.Subset(t, secrets[id].Metadata(), map[string]string{
6587
"game": "elden ring",
6688
})
67-
mockSecret, ok := secrets[id].(*mocks.MockCredential)
68-
require.Truef(t, ok, "secret from store must be of type *mocks.MockCredential")
69-
require.Equal(t, secret.Password, mockSecret.Password)
89+
assert.IsType(t, &mocks.MockCredential{}, secrets[id], "secret from store must be of type *mocks.MockCredential")
90+
mockSecret := secrets[id].(*mocks.MockCredential)
91+
assert.Equal(t, secret.Password, mockSecret.Password)
92+
})
93+
t.Run("can use partial id without filter", func(t *testing.T) {
94+
t.Cleanup(func() {
95+
assert.NoError(t, keychainStore.Delete(t.Context(), id))
96+
})
97+
secret.Attributes = map[string]string{
98+
"color": "blue",
99+
"game": "elden ring",
100+
}
101+
require.NoError(t, keychainStore.Save(t.Context(), id, secret))
102+
secrets, err := keychainStore.Filter(t.Context(), store.ID(serviceName), nil)
103+
require.NoError(t, err)
104+
assert.Len(t, secrets, 4)
105+
_, ok := secrets[id]
106+
assert.Truef(t, ok, "returned secret must match original id")
107+
})
108+
t.Run("can filter using attributes instead of id", func(t *testing.T) {
109+
t.Cleanup(func() {
110+
assert.NoError(t, keychainStore.Delete(t.Context(), id))
111+
})
112+
secret.Attributes = map[string]string{
113+
"color": "blue",
114+
"game": "elden ring",
115+
}
116+
require.NoError(t, keychainStore.Save(t.Context(), id, secret))
117+
secrets, err := keychainStore.Filter(t.Context(), store.ID(serviceGroup), map[string]string{
118+
"game": "elden ring",
119+
})
120+
require.NoError(t, err)
121+
assert.Len(t, secrets, 1)
122+
_, ok := secrets[id]
123+
assert.Truef(t, ok, "returned secret must match")
70124
})
71125
}

‎store/keychain/keychain_test.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"slices"
66
"testing"
77

8+
"github.com/stretchr/testify/assert"
89
"github.com/stretchr/testify/require"
910

1011
"github.com/docker/secrets-engine/store"
@@ -288,3 +289,62 @@ func TestKeychain(t *testing.T) {
288289
require.ErrorContains(t, err, "i am failing on purpose")
289290
})
290291
}
292+
293+
func TestMatchesFilter(t *testing.T) {
294+
for _, tc := range []struct {
295+
desc string
296+
filter map[string]string
297+
attributes map[string]string
298+
expected bool
299+
}{
300+
{
301+
desc: "nil filter does not filter",
302+
attributes: map[string]string{
303+
"color": "green",
304+
},
305+
filter: nil,
306+
expected: true,
307+
},
308+
{
309+
desc: "multiple attributes can be used to match a filter",
310+
attributes: map[string]string{
311+
"color": "green",
312+
"game": "elden ring",
313+
"extra": "unknown",
314+
},
315+
filter: map[string]string{
316+
"color": "green",
317+
"game": "elden ring",
318+
},
319+
expected: true,
320+
},
321+
{
322+
desc: "attributes cannot partially match",
323+
attributes: map[string]string{
324+
"color": "green",
325+
"game": "elden ring",
326+
},
327+
filter: map[string]string{
328+
"color": "green",
329+
"game": "not elden ring",
330+
},
331+
expected: false,
332+
},
333+
{
334+
desc: "singular filter value can be used",
335+
attributes: map[string]string{
336+
"color": "blue",
337+
"game": "none",
338+
"extra": "unknown",
339+
},
340+
filter: map[string]string{
341+
"color": "blue",
342+
},
343+
expected: true,
344+
},
345+
} {
346+
t.Run(tc.desc, func(t *testing.T) {
347+
assert.EqualValues(t, tc.expected, matchesFilter(tc.filter, tc.attributes))
348+
})
349+
}
350+
}

‎store/keychain/keychain_windows.go‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"errors"
66
"iter"
77
"maps"
8-
"slices"
98
"strings"
109

1110
"github.com/danieljoos/wincred"
@@ -112,34 +111,23 @@ func isServiceCredential[T store.Secret](k *keychainStore[T], attrs []wincred.Cr
112111
return strings.EqualFold(serviceGroup, k.serviceGroup) && strings.EqualFold(serviceName, k.serviceName)
113112
}
114113

115-
// hasAttribute iterates over the credential attributes (attrs) and checks if
116-
// each key exists in the attributes map. If they exist, it compares their values
117-
// each attribute inside attributes must match what is stored in the credential
118-
// attributes (attrs) to return true.
119-
func hasAttribute(attributes map[string]string, attrs []wincred.CredentialAttribute) bool {
120-
if attributes == nil {
121-
return true
122-
}
123-
allMatch := make([]bool, len(attributes))
124-
var i int
125-
for _, attr := range attrs {
126-
if v, ok := attributes[attr.Keyword]; ok && strings.EqualFold(v, string(attr.Value)) {
127-
allMatch[i] = true
128-
i++
129-
}
130-
}
131-
return !slices.Contains(allMatch, false)
132-
}
133-
134114
// findServiceCredentials is an iterator that yields credentials that match the
135115
// service group and service name.
136116
func findServiceCredentials[T store.Secret](k *keychainStore[T], attributes map[string]string, credentials []*wincred.Credential) iter.Seq[*wincred.Credential] {
137117
return func(yield func(cred *wincred.Credential) bool) {
138118
for _, c := range credentials {
139-
if isServiceCredential(k, c.Attributes) && hasAttribute(attributes, c.Attributes) {
140-
if !yield(c) {
141-
return
142-
}
119+
if !isServiceCredential(k, c.Attributes) {
120+
continue
121+
}
122+
attrs := make(map[string]string, len(c.Attributes))
123+
for _, windowAttr := range c.Attributes {
124+
attrs[windowAttr.Keyword] = string(windowAttr.Value)
125+
}
126+
if !matchesFilter(attributes, attrs) {
127+
continue
128+
}
129+
if !yield(c) {
130+
return
143131
}
144132
}
145133
}

0 commit comments

Comments
 (0)