Skip to content

Commit 9925dae

Browse files
gtsiolisgtsiolis
authored
Avoid duplicate keychain prompts during auth token lookup (#139)
1 parent cd3ed4b commit 9925dae

File tree

3 files changed

+228
-20
lines changed

3 files changed

+228
-20
lines changed

‎internal/auth/mock_token_storage.go‎

Lines changed: 67 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/auth/token_storage.go‎

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,29 +23,51 @@ type AuthTokenStorage interface {
2323
DeleteAuthToken() error
2424
}
2525

26-
type systemTokenStorage struct{}
26+
type keyringer interface {
27+
Get(service, user string) (string, error)
28+
Set(service, user, password string) error
29+
Delete(service, user string) error
30+
}
31+
32+
type osKeyringer struct{}
33+
34+
func (osKeyringer) Get(service, user string) (string, error) { return keyring.Get(service, user) }
35+
func (osKeyringer) Set(service, user, password string) error { return keyring.Set(service, user, password) }
36+
func (osKeyringer) Delete(service, user string) error { return keyring.Delete(service, user) }
37+
38+
type systemTokenStorage struct {
39+
keyring keyringer
40+
file AuthTokenStorage
41+
logger log.Logger
42+
}
2743

2844
func (s *systemTokenStorage) GetAuthToken() (string, error) {
29-
token, err := keyring.Get(keyringService, keyringAuthTokenKey)
30-
if err != nil {
31-
if errors.Is(err, keyring.ErrNotFound) {
32-
return "", ErrTokenNotFound
33-
}
34-
return "", err
45+
token, err := s.keyring.Get(keyringService, keyringAuthTokenKey)
46+
if err == nil {
47+
return token, nil
48+
}
49+
if errors.Is(err, keyring.ErrNotFound) {
50+
return "", ErrTokenNotFound
3551
}
36-
return token, nil
52+
s.logger.Info("system keyring unavailable (%v), falling back to file-based storage", err)
53+
return s.file.GetAuthToken()
3754
}
3855

3956
func (s *systemTokenStorage) SetAuthToken(token string) error {
40-
return keyring.Set(keyringService, keyringAuthTokenKey, token)
57+
if err := s.keyring.Set(keyringService, keyringAuthTokenKey, token); err != nil {
58+
s.logger.Info("system keyring unavailable (%v), falling back to file-based storage", err)
59+
return s.file.SetAuthToken(token)
60+
}
61+
return nil
4162
}
4263

4364
func (s *systemTokenStorage) DeleteAuthToken() error {
44-
err := keyring.Delete(keyringService, keyringAuthTokenKey)
45-
if errors.Is(err, keyring.ErrNotFound) {
65+
err := s.keyring.Delete(keyringService, keyringAuthTokenKey)
66+
if err == nil || errors.Is(err, keyring.ErrNotFound) {
4667
return nil
4768
}
48-
return err
69+
s.logger.Info("system keyring unavailable (%v), falling back to file-based storage", err)
70+
return s.file.DeleteAuthToken()
4971
}
5072

5173
func NewTokenStorage(forceFileKeyring bool, logger log.Logger) (AuthTokenStorage, error) {
@@ -62,12 +84,9 @@ func NewTokenStorage(forceFileKeyring bool, logger log.Logger) (AuthTokenStorage
6284
return newFileTokenStorage(configDir), nil
6385
}
6486

65-
// Probe whether the system keyring is functional.
66-
_, err = keyring.Get(keyringService, keyringAuthTokenKey)
67-
if err == nil || errors.Is(err, keyring.ErrNotFound) {
68-
return &systemTokenStorage{}, nil
69-
}
70-
71-
logger.Info("system keyring unavailable (%v), falling back to file-based storage", err)
72-
return newFileTokenStorage(configDir), nil
87+
return &systemTokenStorage{
88+
keyring: osKeyringer{},
89+
file: newFileTokenStorage(configDir),
90+
logger: logger,
91+
}, nil
7392
}

‎internal/auth/token_storage_test.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package auth
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
"github.com/localstack/lstk/internal/log"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/zalando/go-keyring"
10+
"go.uber.org/mock/gomock"
11+
)
12+
13+
func newTestStorage(t *testing.T, kr keyringer, file AuthTokenStorage) *systemTokenStorage {
14+
t.Helper()
15+
return &systemTokenStorage{
16+
keyring: kr,
17+
file: file,
18+
logger: log.Nop(),
19+
}
20+
}
21+
22+
func TestSystemTokenStorage_GetReturnsTokenFromKeyring(t *testing.T) {
23+
ctrl := gomock.NewController(t)
24+
kr := NewMockkeyringer(ctrl)
25+
file := NewMockAuthTokenStorage(ctrl)
26+
27+
kr.EXPECT().Get(keyringService, keyringAuthTokenKey).Return("system-token", nil)
28+
29+
token, err := newTestStorage(t, kr, file).GetAuthToken()
30+
31+
assert.NoError(t, err)
32+
assert.Equal(t, "system-token", token)
33+
}
34+
35+
func TestSystemTokenStorage_GetReturnsErrTokenNotFoundWhenKeyringEmpty(t *testing.T) {
36+
ctrl := gomock.NewController(t)
37+
kr := NewMockkeyringer(ctrl)
38+
file := NewMockAuthTokenStorage(ctrl)
39+
40+
kr.EXPECT().Get(keyringService, keyringAuthTokenKey).Return("", keyring.ErrNotFound)
41+
42+
token, err := newTestStorage(t, kr, file).GetAuthToken()
43+
44+
assert.Empty(t, token)
45+
assert.ErrorIs(t, err, ErrTokenNotFound)
46+
}
47+
48+
func TestSystemTokenStorage_GetFallsBackToFileWhenKeyringUnavailable(t *testing.T) {
49+
ctrl := gomock.NewController(t)
50+
kr := NewMockkeyringer(ctrl)
51+
file := NewMockAuthTokenStorage(ctrl)
52+
53+
kr.EXPECT().Get(keyringService, keyringAuthTokenKey).Return("", errors.New("keychain unavailable"))
54+
file.EXPECT().GetAuthToken().Return("file-token", nil)
55+
56+
token, err := newTestStorage(t, kr, file).GetAuthToken()
57+
58+
assert.NoError(t, err)
59+
assert.Equal(t, "file-token", token)
60+
}
61+
62+
func TestSystemTokenStorage_SetStoresInKeyring(t *testing.T) {
63+
ctrl := gomock.NewController(t)
64+
kr := NewMockkeyringer(ctrl)
65+
file := NewMockAuthTokenStorage(ctrl)
66+
67+
kr.EXPECT().Set(keyringService, keyringAuthTokenKey, "token").Return(nil)
68+
69+
err := newTestStorage(t, kr, file).SetAuthToken("token")
70+
71+
assert.NoError(t, err)
72+
}
73+
74+
func TestSystemTokenStorage_SetFallsBackToFileWhenKeyringUnavailable(t *testing.T) {
75+
ctrl := gomock.NewController(t)
76+
kr := NewMockkeyringer(ctrl)
77+
file := NewMockAuthTokenStorage(ctrl)
78+
79+
kr.EXPECT().Set(keyringService, keyringAuthTokenKey, "token").Return(errors.New("keychain unavailable"))
80+
file.EXPECT().SetAuthToken("token").Return(nil)
81+
82+
err := newTestStorage(t, kr, file).SetAuthToken("token")
83+
84+
assert.NoError(t, err)
85+
}
86+
87+
func TestSystemTokenStorage_DeleteRemovesFromKeyring(t *testing.T) {
88+
ctrl := gomock.NewController(t)
89+
kr := NewMockkeyringer(ctrl)
90+
file := NewMockAuthTokenStorage(ctrl)
91+
92+
kr.EXPECT().Delete(keyringService, keyringAuthTokenKey).Return(nil)
93+
94+
err := newTestStorage(t, kr, file).DeleteAuthToken()
95+
96+
assert.NoError(t, err)
97+
}
98+
99+
func TestSystemTokenStorage_DeleteSucceedsWhenKeyringTokenMissing(t *testing.T) {
100+
ctrl := gomock.NewController(t)
101+
kr := NewMockkeyringer(ctrl)
102+
file := NewMockAuthTokenStorage(ctrl)
103+
104+
kr.EXPECT().Delete(keyringService, keyringAuthTokenKey).Return(keyring.ErrNotFound)
105+
106+
err := newTestStorage(t, kr, file).DeleteAuthToken()
107+
108+
assert.NoError(t, err)
109+
}
110+
111+
func TestSystemTokenStorage_DeleteFallsBackToFileWhenKeyringUnavailable(t *testing.T) {
112+
ctrl := gomock.NewController(t)
113+
kr := NewMockkeyringer(ctrl)
114+
file := NewMockAuthTokenStorage(ctrl)
115+
116+
kr.EXPECT().Delete(keyringService, keyringAuthTokenKey).Return(errors.New("keychain unavailable"))
117+
file.EXPECT().DeleteAuthToken().Return(nil)
118+
119+
err := newTestStorage(t, kr, file).DeleteAuthToken()
120+
121+
assert.NoError(t, err)
122+
}

0 commit comments

Comments
 (0)