Skip to content

Fix and Improve keyring implementation #799

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix and Improve keyring implementation #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1a32b9
feat: add keyring support and save API token
limonkufu Dec 6, 2024
40e9c0a
fix: keyring support
limonkufu Dec 6, 2024
f6fa190
fix: keyring returns none for the secret
limonkufu Dec 6, 2024
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
22 changes: 19 additions & 3 deletions api/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package api

import (
"time"
"strings"

"github.com/spf13/viper"
"github.com/zalando/go-keyring"
Expand Down Expand Up @@ -30,16 +31,31 @@ func Client(config jira.Config) *jira.Client {
if config.APIToken == "" {
config.APIToken = viper.GetString("api_token")
}

// Try netrc first if no API token provided
if config.APIToken == "" {
netrcConfig, _ := netrc.Read(config.Server, config.Login)
if netrcConfig != nil {
config.APIToken = netrcConfig.Password
}
}
if config.APIToken == "" {
secret, _ := keyring.Get("jira-cli", config.Login)
config.APIToken = secret

// Only try keyring if explicitly enabled in config
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you pls help understand why we want to have it explicitly enabled?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to break anything as I wasn't sure what was the intended default behaviour. I can make it always check if you'd like?

useKeyring := viper.GetBool("use_keyring")
if config.APIToken == "" && useKeyring {
// Try to get API token from keyring
secret, err := keyring.Get("jira-cli", strings.ToLower(config.Login))
if err == nil {
config.APIToken = secret
}
} else if useKeyring && config.APIToken != "" {
// Save valid API token to keyring for future use
if err := keyring.Set("jira-cli", strings.ToLower(config.Login), config.APIToken); err != nil {
// Non-fatal error, just continue without saving to keyring
// The API token will still work for this session
}
}

if config.AuthType == nil {
authType := jira.AuthType(viper.GetString("auth_type"))
config.AuthType = &authType
Expand Down
59 changes: 31 additions & 28 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
module github.com/ankitpokhrel/jira-cli

go 1.21
go 1.22.0

toolchain go1.23.4

require (
github.com/AlecAivazis/survey/v2 v2.3.7
github.com/atotto/clipboard v0.1.4
github.com/briandowns/spinner v1.23.0
github.com/charmbracelet/glamour v0.7.0
github.com/briandowns/spinner v1.23.1
github.com/charmbracelet/glamour v0.8.0
github.com/cli/safeexec v1.0.1
github.com/fatih/color v1.16.0
github.com/fatih/color v1.18.0
github.com/gdamore/tcell/v2 v2.7.4
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
Expand All @@ -18,26 +20,28 @@ require (
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
github.com/mitchellh/go-homedir v1.1.0
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c
github.com/rivo/tview v0.0.0-20240406141410-79d4cc321256
github.com/rivo/tview v0.0.0-20241103174730-c76f7879f592
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we need to update all these deps. Can we only update libs related to this pr? Upgrading libs like tview could have unintended issues and requires proper testing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a go developer so I don't know why all of these got updated. I only initialised the repo locally for testing.

I understand the concern and will push a new commit to address this

github.com/russross/blackfriday/v2 v2.1.0
github.com/spf13/cobra v1.8.0
github.com/spf13/viper v1.18.2
github.com/spf13/cobra v1.8.1
github.com/spf13/viper v1.19.0
github.com/stretchr/testify v1.9.0
github.com/zalando/go-keyring v0.2.4
golang.org/x/term v0.19.0
github.com/zalando/go-keyring v0.2.6
golang.org/x/term v0.27.0
)

require (
github.com/alecthomas/chroma/v2 v2.13.0 // indirect
github.com/alessio/shellescape v1.4.2 // indirect
al.essio.dev/pkg/shellescape v1.5.1 // indirect
github.com/alecthomas/chroma/v2 v2.14.0 // indirect
github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
github.com/aymerick/douceur v0.2.0 // indirect
github.com/cpuguy83/go-md2man/v2 v2.0.3 // indirect
github.com/charmbracelet/lipgloss v1.0.0 // indirect
github.com/charmbracelet/x/ansi v0.5.2 // indirect
github.com/cpuguy83/go-md2man/v2 v2.0.5 // indirect
github.com/creack/pty v1.1.18 // indirect
github.com/danieljoos/wincred v1.2.1 // indirect
github.com/danieljoos/wincred v1.2.2 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/dlclark/regexp2 v1.11.0 // indirect
github.com/fsnotify/fsnotify v1.7.0 // indirect
github.com/dlclark/regexp2 v1.11.4 // indirect
github.com/fsnotify/fsnotify v1.8.0 // indirect
github.com/gdamore/encoding v1.0.1 // indirect
github.com/godbus/dbus/v5 v5.1.0 // indirect
github.com/gorilla/css v1.0.1 // indirect
Expand All @@ -47,29 +51,28 @@ require (
github.com/lucasb-eyer/go-colorful v1.2.0 // indirect
github.com/magiconair/properties v1.8.7 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-runewidth v0.0.15 // indirect
github.com/microcosm-cc/bluemonday v1.0.26 // indirect
github.com/mattn/go-runewidth v0.0.16 // indirect
github.com/microcosm-cc/bluemonday v1.0.27 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/muesli/reflow v0.3.0 // indirect
github.com/muesli/termenv v0.15.2 // indirect
github.com/olekukonko/tablewriter v0.0.5 // indirect
github.com/pelletier/go-toml/v2 v2.2.0 // indirect
github.com/muesli/termenv v0.15.3-0.20240618155329-98d742f6907a // indirect
github.com/pelletier/go-toml/v2 v2.2.3 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/sagikazarmark/locafero v0.4.0 // indirect
github.com/sagikazarmark/locafero v0.6.0 // indirect
github.com/sagikazarmark/slog-shim v0.1.0 // indirect
github.com/sourcegraph/conc v0.3.0 // indirect
github.com/spf13/afero v1.11.0 // indirect
github.com/spf13/cast v1.6.0 // indirect
github.com/spf13/cast v1.7.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/subosito/gotenv v1.6.0 // indirect
github.com/yuin/goldmark v1.7.1 // indirect
github.com/yuin/goldmark-emoji v1.0.2 // indirect
github.com/yuin/goldmark v1.7.8 // indirect
github.com/yuin/goldmark-emoji v1.0.4 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/exp v0.0.0-20240404231335-c0f41cb1a7a0 // indirect
golang.org/x/net v0.24.0 // indirect
golang.org/x/sys v0.19.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/exp v0.0.0-20241204233417-43b7b7cde48d // indirect
golang.org/x/net v0.32.0 // indirect
golang.org/x/sys v0.28.0 // indirect
golang.org/x/text v0.21.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
Loading
Loading