Skip to content

Commit 2fa27df

Browse files
Remove keychain storage, store tokens in config file
- Drop internal/secrets/ package and keychain integration - Simplify config manager to use TOML-only token storage - Default host when config is empty, backfill user from server - Fix subcommand help delegation Signed-off-by: Jai Pradeesh <jai@deepsource.io>
1 parent e34608a commit 2fa27df

21 files changed

Lines changed: 85 additions & 17379 deletions

File tree

.github/workflows/CI.yml

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,10 @@ jobs:
3030
- name: Build the binary
3131
run: just build
3232

33-
- name: Setup tests
34-
run: just test-setup
35-
env:
36-
CODE_PATH: /home/runner/code
37-
3833
- name: Run tests
3934
run: just test
40-
env:
41-
CODE_PATH: /home/runner/code
4235

4336
- name: Report test coverage to DeepSource
4437
run: |
45-
curl https://deepsource.io/cli | sh
38+
curl -fsSL https://cli.deepsource.com/install | BINDIR=./bin sh
4639
./bin/deepsource report --analyzer test-coverage --key go --value-file ./coverage.out --use-oidc

buildinfo/version.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ var buildInfo *BuildInfo
99

1010
// App identity variables. Defaults are prod values; overridden in main.go for dev builds.
1111
var (
12-
AppName = "deepsource" // binary name / display name
13-
ConfigDirName = ".deepsource" // ~/<this>/
14-
KeychainSvc = "deepsource-cli" // macOS keychain service
15-
KeychainKey = "deepsource-cli-token" // macOS keychain account
12+
AppName = "deepsource" // binary name / display name
13+
ConfigDirName = ".deepsource" // ~/<this>/
1614
)
1715

1816
// BuildInfo describes the compile time information.

cmd/deepsource/main.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ func mainRun() (exitCode int) {
4747
if buildMode == "dev" {
4848
v.AppName = "deepsource-dev"
4949
v.ConfigDirName = ".deepsource-dev"
50-
v.KeychainSvc = "deepsource-dev-cli"
51-
v.KeychainKey = "deepsource-dev-cli-token"
5250
}
5351

5452
// Init sentry

command/auth/login/login.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,12 @@ func (opts *LoginOptions) Run() (err error) {
100100
opts.User = cfg.User
101101
opts.TokenExpired = cfg.IsExpired()
102102

103+
// Default host so that verifyTokenWithServer can reach the server
104+
// even when no config file exists yet.
105+
if cfg.Host == "" {
106+
cfg.Host = config.DefaultHostName
107+
}
108+
103109
// If local says valid, verify against the server
104110
opts.verifyTokenWithServer(cfg, cfgMgr)
105111

@@ -131,7 +137,12 @@ func (opts *LoginOptions) Run() (err error) {
131137
// Checking for condition 1
132138
if !opts.TokenExpired {
133139
// The user is already logged in, confirm re-authentication.
134-
msg := fmt.Sprintf("You're already logged into DeepSource as %s. Do you want to re-authenticate?", opts.User)
140+
var msg string
141+
if opts.User != "" {
142+
msg = fmt.Sprintf("You're already logged into DeepSource as %s. Do you want to re-authenticate?", opts.User)
143+
} else {
144+
msg = "You're already logged into DeepSource. Do you want to re-authenticate?"
145+
}
135146
response, err := prompt.ConfirmFromUser(msg, "")
136147
if err != nil {
137148
return fmt.Errorf("Error in fetching response. Please try again.")
@@ -160,8 +171,16 @@ func (opts *LoginOptions) verifyTokenWithServer(cfg *config.CLIConfig, cfgMgr *c
160171
if err != nil {
161172
return
162173
}
163-
if _, err := client.GetViewer(context.Background()); err != nil {
174+
viewer, err := client.GetViewer(context.Background())
175+
if err != nil {
164176
opts.TokenExpired = true
177+
return
178+
}
179+
// Backfill user from the server when config file was missing or incomplete.
180+
if cfg.User == "" && viewer.Email != "" {
181+
cfg.User = viewer.Email
182+
opts.User = viewer.Email
183+
_ = cfgMgr.Write(cfg)
165184
}
166185
}
167186

command/auth/status/status.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ func (opts *AuthStatusOptions) Run() error {
7070
return clierrors.ErrNotLoggedIn()
7171
}
7272

73+
// Default host if empty (e.g. env-var-only token case)
74+
if cfg.Host == "" {
75+
cfg.Host = config.DefaultHostName
76+
}
77+
7378
// Fast path: if the local token expiry has passed, no need for a network call.
7479
// Skip for env var tokens since they have no local expiry info.
7580
if !cfg.TokenFromEnv && cfg.IsExpired() {
@@ -88,24 +93,34 @@ func (opts *AuthStatusOptions) Run() error {
8893
OnTokenRefreshed: cfgMgr.TokenRefreshCallback(),
8994
})
9095
if err != nil {
91-
fmt.Fprintf(opts.stdout(), "Logged in to DeepSource as %s (could not verify with server).\n", cfg.User)
96+
style.Warnf(opts.stdout(), "Could not connect to DeepSource to verify authentication")
9297
return nil
9398
}
9499
}
95100

96-
_, verifyErr := client.GetViewer(context.Background())
101+
viewer, verifyErr := client.GetViewer(context.Background())
97102
if verifyErr != nil {
98103
var ce *clierrors.CLIError
99104
if stderrors.As(verifyErr, &ce) && ce.Code.IsAuthError() {
100105
style.Warnf(opts.stdout(), "Authentication expired. Run %q to re-authenticate", "deepsource auth login")
101106
return nil
102107
}
103-
// Network error or other non-auth failure — report as logged in but unverified
104-
fmt.Fprintf(opts.stdout(), "Logged in to DeepSource as %s (could not verify with server).\n", cfg.User)
108+
// Network error or other non-auth failure
109+
style.Warnf(opts.stdout(), "Could not connect to DeepSource to verify authentication")
105110
return nil
106111
}
107112

108-
msg := fmt.Sprintf("Logged in to DeepSource as %s", cfg.User)
113+
// Use viewer email; backfill config if empty
114+
displayUser := viewer.Email
115+
if displayUser == "" {
116+
displayUser = cfg.User
117+
}
118+
if cfg.User == "" && viewer.Email != "" {
119+
cfg.User = viewer.Email
120+
_ = cfgMgr.Write(cfg)
121+
}
122+
123+
msg := fmt.Sprintf("Logged in to DeepSource as %s", displayUser)
109124
if cfg.TokenFromEnv {
110125
msg += " (via DEEPSOURCE_TOKEN)"
111126
}

command/auth/status/tests/auth_status_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"github.com/deepsourcelabs/cli/deepsource/graphqlclient"
1717
"github.com/deepsourcelabs/cli/internal/adapters"
1818
clierrors "github.com/deepsourcelabs/cli/internal/errors"
19-
"github.com/deepsourcelabs/cli/internal/secrets"
2019
"github.com/deepsourcelabs/cli/internal/testutil"
2120
)
2221

@@ -29,9 +28,9 @@ func createConfigManager(t *testing.T, token, host, user string, expiry time.Tim
2928
t.Helper()
3029
tmpDir := t.TempDir()
3130
fs := adapters.NewOSFileSystem()
32-
mgr := config.NewManagerWithSecrets(fs, func() (string, error) {
31+
mgr := config.NewManager(fs, func() (string, error) {
3332
return tmpDir, nil
34-
}, secrets.NoopStore{}, "")
33+
})
3534

3635
cfg := &config.CLIConfig{
3736
Token: token,
@@ -115,9 +114,9 @@ func TestAuthStatusEnvVarToken(t *testing.T) {
115114
// No config file — token comes purely from env var
116115
tmpDir := t.TempDir()
117116
fs := adapters.NewOSFileSystem()
118-
cfgMgr := config.NewManagerWithSecrets(fs, func() (string, error) {
117+
cfgMgr := config.NewManager(fs, func() (string, error) {
119118
return tmpDir, nil
120-
}, secrets.NoopStore{}, "")
119+
})
121120

122121
t.Setenv("DEEPSOURCE_TOKEN", "env-test-token")
123122

command/report/tests/init_test.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,7 @@ func prepareArtifacts() error {
6666
return err
6767
}
6868

69-
defaultRoot := filepath.Clean(filepath.Join(wd, "..", "..", ".."))
70-
rootDir := defaultRoot
71-
if envRoot := os.Getenv("CODE_PATH"); envRoot != "" {
72-
coverageCandidate := filepath.Join(envRoot, "command", "report", "tests", "golden_files", "python_coverage.xml")
73-
if _, err := os.Stat(coverageCandidate); err == nil {
74-
rootDir = envRoot
75-
}
76-
}
69+
rootDir := filepath.Clean(filepath.Join(wd, "..", "..", ".."))
7770
repoRoot = rootDir
7871

7972
tempDir, err := os.MkdirTemp("", "deepsource-report")

command/report/tests/report_host_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,15 @@ import (
77
"github.com/deepsourcelabs/cli/config"
88
"github.com/deepsourcelabs/cli/internal/adapters"
99
"github.com/deepsourcelabs/cli/internal/container"
10-
"github.com/deepsourcelabs/cli/internal/secrets"
1110
)
1211

1312
func TestReportHostFallsBackToConfig(t *testing.T) {
1413
// Create a config manager with an enterprise host
1514
tmpDir := t.TempDir()
1615
fs := adapters.NewOSFileSystem()
17-
cfgMgr := config.NewManagerWithSecrets(fs, func() (string, error) {
16+
cfgMgr := config.NewManager(fs, func() (string, error) {
1817
return tmpDir, nil
19-
}, secrets.NoopStore{}, "")
18+
})
2019

2120
cfg := &config.CLIConfig{
2221
Host: "enterprise.example.com",
@@ -66,9 +65,9 @@ func TestReportHostExplicitFlagOverridesConfig(t *testing.T) {
6665
// Config has enterprise host, but --host flag overrides it
6766
tmpDir := t.TempDir()
6867
fs := adapters.NewOSFileSystem()
69-
cfgMgr := config.NewManagerWithSecrets(fs, func() (string, error) {
68+
cfgMgr := config.NewManager(fs, func() (string, error) {
7069
return tmpDir, nil
71-
}, secrets.NoopStore{}, "")
70+
})
7271

7372
cfg := &config.CLIConfig{
7473
Host: "enterprise.example.com",
@@ -115,9 +114,9 @@ func TestReportHostDefaultWhenNoConfigNoFlag(t *testing.T) {
115114
// No config host, no --host flag: should use default
116115
tmpDir := t.TempDir()
117116
fs := adapters.NewOSFileSystem()
118-
cfgMgr := config.NewManagerWithSecrets(fs, func() (string, error) {
117+
cfgMgr := config.NewManager(fs, func() (string, error) {
119118
return tmpDir, nil
120-
}, secrets.NoopStore{}, "")
119+
})
121120

122121
// Write empty config (no host)
123122
cfg := &config.CLIConfig{}

command/root.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,14 @@ func buildExampleText() string {
125125
}
126126

127127
func rootHelpFunc(cmd *cobra.Command, _ []string) {
128+
if cmd.Parent() != nil {
129+
root := cmd.Root()
130+
root.SetHelpFunc(nil)
131+
cmd.Help()
132+
root.SetHelpFunc(rootHelpFunc)
133+
return
134+
}
135+
128136
out := cmd.OutOrStdout()
129137

130138
// Long description (already colored)

config/manager.go

Lines changed: 6 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,37 +9,23 @@ import (
99
"github.com/deepsourcelabs/cli/internal/adapters"
1010
"github.com/deepsourcelabs/cli/internal/debug"
1111
"github.com/deepsourcelabs/cli/internal/interfaces"
12-
"github.com/deepsourcelabs/cli/internal/secrets"
1312
"github.com/pelletier/go-toml"
1413
)
1514

1615
// Manager handles reading and writing CLI config.
1716
type Manager struct {
18-
fs interfaces.FileSystem
19-
homeDir func() (string, error)
20-
secrets secrets.Store
21-
secretsKey string
17+
fs interfaces.FileSystem
18+
homeDir func() (string, error)
2219
}
2320

2421
// NewManager creates a config manager with injected dependencies.
2522
func NewManager(fs interfaces.FileSystem, homeDir func() (string, error)) *Manager {
26-
return NewManagerWithSecrets(fs, homeDir, secrets.NoopStore{}, "")
27-
}
28-
29-
// NewManagerWithSecrets creates a config manager with a secrets store.
30-
func NewManagerWithSecrets(fs interfaces.FileSystem, homeDir func() (string, error), store secrets.Store, key string) *Manager {
31-
if key == "" {
32-
key = buildinfo.KeychainKey
33-
}
34-
if store == nil {
35-
store = secrets.NoopStore{}
36-
}
37-
return &Manager{fs: fs, homeDir: homeDir, secrets: store, secretsKey: key}
23+
return &Manager{fs: fs, homeDir: homeDir}
3824
}
3925

4026
// DefaultManager returns a manager using OS-backed dependencies.
4127
func DefaultManager() *Manager {
42-
return NewManagerWithSecrets(adapters.NewOSFileSystem(), os.UserHomeDir, secrets.DefaultStore(), "")
28+
return NewManager(adapters.NewOSFileSystem(), os.UserHomeDir)
4329
}
4430

4531
func (m *Manager) configDir() (string, error) {
@@ -80,13 +66,6 @@ func (m *Manager) Load() (*CLIConfig, error) {
8066
}
8167
}
8268

83-
tokenFromKeychain := false
84-
if cfg.Token == "" {
85-
if token, err := m.secrets.Get(m.secretsKey); err == nil {
86-
cfg.Token = token
87-
tokenFromKeychain = true
88-
}
89-
}
9069
if cfg.Token == "" {
9170
if envToken := os.Getenv("DEEPSOURCE_TOKEN"); envToken != "" {
9271
cfg.Token = envToken
@@ -99,21 +78,14 @@ func (m *Manager) Load() (*CLIConfig, error) {
9978
}
10079
}
10180

102-
debug.Log("config: host=%q user=%q token_present=%v keychain=%v env=%v", cfg.Host, cfg.User, cfg.Token != "", tokenFromKeychain, cfg.TokenFromEnv)
81+
debug.Log("config: host=%q user=%q token_present=%v env=%v", cfg.Host, cfg.User, cfg.Token != "", cfg.TokenFromEnv)
10382

10483
return cfg, nil
10584
}
10685

10786
// Write persists the CLI config file.
10887
func (m *Manager) Write(cfg *CLIConfig) error {
109-
cfgToWrite := *cfg
110-
if cfg.Token != "" {
111-
if err := m.secrets.Set(m.secretsKey, cfg.Token); err == nil {
112-
cfgToWrite.Token = ""
113-
}
114-
}
115-
116-
data, err := toml.Marshal(&cfgToWrite)
88+
data, err := toml.Marshal(cfg)
11789
if err != nil {
11890
return err
11991
}
@@ -137,10 +109,6 @@ func (m *Manager) Write(cfg *CLIConfig) error {
137109

138110
// Delete removes the CLI config file if it exists.
139111
func (m *Manager) Delete() error {
140-
if err := m.secrets.Delete(m.secretsKey); err != nil && !errors.Is(err, secrets.ErrNotFound) && !errors.Is(err, secrets.ErrUnavailable) {
141-
return err
142-
}
143-
144112
path, err := m.configPath()
145113
if err != nil {
146114
return err

0 commit comments

Comments
 (0)