bridge: store credentials in the Keyring instead of the git config

12 files changed, 162 insertions, 172 deletions

diff --git a/bridge/core/auth/credential.go b/bridge/core/auth/credential.go
index d95b23c7..2327a6fc 100644
--- a/bridge/core/auth/credential.go
+++ b/bridge/core/auth/credential.go
@@ -1,11 +1,11 @@
 package auth
 
 import (
-	"crypto/rand"
 	"encoding/base64"
+	"encoding/json"
 	"errors"
 	"fmt"
-	"regexp"
+	"strconv"
 	"strings"
 	"time"
 
@@ -14,12 +14,12 @@ import (
 )
 
 const (
-	configKeyPrefix     = "git-bug.auth"
-	configKeyKind       = "kind"
-	configKeyTarget     = "target"
-	configKeyCreateTime = "createtime"
-	configKeySalt       = "salt"
-	configKeyPrefixMeta = "meta."
+	keyringKeyPrefix     = "auth-"
+	keyringKeyKind       = "kind"
+	keyringKeyTarget     = "target"
+	keyringKeyCreateTime = "createtime"
+	keyringKeySalt       = "salt"
+	keyringKeyPrefixMeta = "meta."
 
 	MetaKeyLogin   = "login"
 	MetaKeyBaseURL = "base-url"
@@ -57,22 +57,23 @@ type Credential interface {
 }
 
 // Load loads a credential from the repo config
-func LoadWithId(repo repository.RepoConfig, id entity.Id) (Credential, error) {
-	keyPrefix := fmt.Sprintf("%s.%s.", configKeyPrefix, id)
+func LoadWithId(repo repository.RepoKeyring, id entity.Id) (Credential, error) {
+	key := fmt.Sprintf("%s%s", keyringKeyPrefix, id)
 
-	// read token config pairs
-	rawconfigs, err := repo.GlobalConfig().ReadAll(keyPrefix)
-	if err != nil {
-		// Not exactly right due to the limitation of ReadAll()
+	item, err := repo.Keyring().Get(key)
+	if err == repository.ErrKeyringKeyNotFound {
 		return nil, ErrCredentialNotExist
 	}
+	if err != nil {
+		return nil, err
+	}
 
-	return loadFromConfig(rawconfigs, id)
+	return decode(item)
 }
 
 // LoadWithPrefix load a credential from the repo config with a prefix
-func LoadWithPrefix(repo repository.RepoConfig, prefix string) (Credential, error) {
-	creds, err := List(repo)
+func LoadWithPrefix(repo repository.RepoKeyring, prefix string) (Credential, error) {
+	keys, err := repo.Keyring().Keys()
 	if err != nil {
 		return nil, err
 	}
@@ -80,10 +81,22 @@ func LoadWithPrefix(repo repository.RepoConfig, prefix string) (Credential, erro
 	// preallocate but empty
 	matching := make([]Credential, 0, 5)
 
-	for _, cred := range creds {
-		if cred.ID().HasPrefix(prefix) {
-			matching = append(matching, cred)
+	for _, key := range keys {
+		if !strings.HasPrefix(key, keyringKeyPrefix+prefix) {
+			continue
+		}
+
+		item, err := repo.Keyring().Get(key)
+		if err != nil {
+			return nil, err
 		}
+
+		cred, err := decode(item)
+		if err != nil {
+			return nil, err
+		}
+
+		matching = append(matching, cred)
 	}
 
 	if len(matching) > 1 {
@@ -101,29 +114,25 @@ func LoadWithPrefix(repo repository.RepoConfig, prefix string) (Credential, erro
 	return matching[0], nil
 }
 
-// loadFromConfig is a helper to construct a Credential from the set of git configs
-func loadFromConfig(rawConfigs map[string]string, id entity.Id) (Credential, error) {
-	keyPrefix := fmt.Sprintf("%s.%s.", configKeyPrefix, id)
+// decode is a helper to construct a Credential from the keyring Item
+func decode(item repository.Item) (Credential, error) {
+	data := make(map[string]string)
 
-	// trim key prefix
-	configs := make(map[string]string)
-	for key, value := range rawConfigs {
-		newKey := strings.TrimPrefix(key, keyPrefix)
-		configs[newKey] = value
+	err := json.Unmarshal(item.Data, &data)
+	if err != nil {
+		return nil, err
 	}
 
 	var cred Credential
-	var err error
-
-	switch CredentialKind(configs[configKeyKind]) {
+	switch CredentialKind(data[keyringKeyKind]) {
 	case KindToken:
-		cred, err = NewTokenFromConfig(configs)
+		cred, err = NewTokenFromConfig(data)
 	case KindLogin:
-		cred, err = NewLoginFromConfig(configs)
+		cred, err = NewLoginFromConfig(data)
 	case KindLoginPassword:
-		cred, err = NewLoginPasswordFromConfig(configs)
+		cred, err = NewLoginPasswordFromConfig(data)
 	default:
-		return nil, fmt.Errorf("unknown credential type \"%s\"", configs[configKeyKind])
+		return nil, fmt.Errorf("unknown credential type \"%s\"", data[keyringKeyKind])
 	}
 
 	if err != nil {
@@ -133,64 +142,27 @@ func loadFromConfig(rawConfigs map[string]string, id entity.Id) (Credential, err
 	return cred, nil
 }
 
-func metaFromConfig(configs map[string]string) map[string]string {
-	result := make(map[string]string)
-	for key, val := range configs {
-		if strings.HasPrefix(key, configKeyPrefixMeta) {
-			key = strings.TrimPrefix(key, configKeyPrefixMeta)
-			result[key] = val
-		}
-	}
-	if len(result) == 0 {
-		return nil
-	}
-	return result
-}
-
-func makeSalt() []byte {
-	result := make([]byte, 16)
-	_, err := rand.Read(result)
-	if err != nil {
-		panic(err)
-	}
-	return result
-}
-
-func saltFromConfig(configs map[string]string) ([]byte, error) {
-	val, ok := configs[configKeySalt]
-	if !ok {
-		return nil, fmt.Errorf("no credential salt found")
-	}
-	return base64.StdEncoding.DecodeString(val)
-}
-
 // List load all existing credentials
-func List(repo repository.RepoConfig, opts ...Option) ([]Credential, error) {
-	rawConfigs, err := repo.GlobalConfig().ReadAll(configKeyPrefix + ".")
+func List(repo repository.RepoKeyring, opts ...ListOption) ([]Credential, error) {
+	keys, err := repo.Keyring().Keys()
 	if err != nil {
 		return nil, err
 	}
 
-	re := regexp.MustCompile(`^` + configKeyPrefix + `\.([^.]+)\.([^.]+(?:\.[^.]+)*)$`)
-
-	mapped := make(map[string]map[string]string)
+	matcher := matcher(opts)
 
-	for key, val := range rawConfigs {
-		res := re.FindStringSubmatch(key)
-		if res == nil {
+	var credentials []Credential
+	for _, key := range keys {
+		if !strings.HasPrefix(key, keyringKeyPrefix) {
 			continue
 		}
-		if mapped[res[1]] == nil {
-			mapped[res[1]] = make(map[string]string)
-		}
-		mapped[res[1]][res[2]] = val
-	}
 
-	matcher := matcher(opts)
+		item, err := repo.Keyring().Get(key)
+		if err != nil {
+			return nil, err
+		}
 
-	var credentials []Credential
-	for id, kvs := range mapped {
-		cred, err := loadFromConfig(kvs, entity.Id(id))
+		cred, err := decode(item)
 		if err != nil {
 			return nil, err
 		}
@@ -203,74 +175,48 @@ func List(repo repository.RepoConfig, opts ...Option) ([]Credential, error) {
 }
 
 // IdExist return whether a credential id exist or not
-func IdExist(repo repository.RepoConfig, id entity.Id) bool {
+func IdExist(repo repository.RepoKeyring, id entity.Id) bool {
 	_, err := LoadWithId(repo, id)
 	return err == nil
 }
 
 // PrefixExist return whether a credential id prefix exist or not
-func PrefixExist(repo repository.RepoConfig, prefix string) bool {
+func PrefixExist(repo repository.RepoKeyring, prefix string) bool {
 	_, err := LoadWithPrefix(repo, prefix)
 	return err == nil
 }
 
 // Store stores a credential in the global git config
-func Store(repo repository.RepoConfig, cred Credential) error {
-	confs := cred.toConfig()
-
-	prefix := fmt.Sprintf("%s.%s.", configKeyPrefix, cred.ID())
-
-	// Kind
-	err := repo.GlobalConfig().StoreString(prefix+configKeyKind, string(cred.Kind()))
-	if err != nil {
-		return err
-	}
-
-	// Target
-	err = repo.GlobalConfig().StoreString(prefix+configKeyTarget, cred.Target())
-	if err != nil {
-		return err
-	}
-
-	// CreateTime
-	err = repo.GlobalConfig().StoreTimestamp(prefix+configKeyCreateTime, cred.CreateTime())
-	if err != nil {
-		return err
-	}
-
-	// Salt
+func Store(repo repository.RepoKeyring, cred Credential) error {
 	if len(cred.Salt()) != 16 {
 		panic("credentials need to be salted")
 	}
-	encoded := base64.StdEncoding.EncodeToString(cred.Salt())
-	err = repo.GlobalConfig().StoreString(prefix+configKeySalt, encoded)
-	if err != nil {
-		return err
-	}
 
-	// Metadata
+	confs := cred.toConfig()
+
+	confs[keyringKeyKind] = string(cred.Kind())
+	confs[keyringKeyTarget] = cred.Target()
+	confs[keyringKeyCreateTime] = strconv.Itoa(int(cred.CreateTime().Unix()))
+	confs[keyringKeySalt] = base64.StdEncoding.EncodeToString(cred.Salt())
+
 	for key, val := range cred.Metadata() {
-		err := repo.GlobalConfig().StoreString(prefix+configKeyPrefixMeta+key, val)
-		if err != nil {
-			return err
-		}
+		confs[keyringKeyPrefixMeta+key] = val
 	}
 
-	// Custom
-	for key, val := range confs {
-		err := repo.GlobalConfig().StoreString(prefix+key, val)
-		if err != nil {
-			return err
-		}
+	data, err := json.Marshal(confs)
+	if err != nil {
+		return err
 	}
 
-	return nil
+	return repo.Keyring().Set(repository.Item{
+		Key:  keyringKeyPrefix + cred.ID().String(),
+		Data: data,
+	})
 }
 
 // Remove removes a credential from the global git config
-func Remove(repo repository.RepoConfig, id entity.Id) error {
-	keyPrefix := fmt.Sprintf("%s.%s", configKeyPrefix, id)
-	return repo.GlobalConfig().RemoveAll(keyPrefix)
+func Remove(repo repository.RepoKeyring, id entity.Id) error {
+	return repo.Keyring().Remove(keyringKeyPrefix + id.String())
 }
 
 /*
diff --git a/bridge/core/auth/credential_base.go b/bridge/core/auth/credential_base.go
index 488c223c..f9d1bf67 100644
--- a/bridge/core/auth/credential_base.go
+++ b/bridge/core/auth/credential_base.go
@@ -1,7 +1,10 @@
 package auth
 
 import (
+	"crypto/rand"
+	"encoding/base64"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/MichaelMure/git-bug/bridge/core"
@@ -23,13 +26,22 @@ func newCredentialBase(target string) *credentialBase {
 	}
 }
 
-func newCredentialBaseFromConfig(conf map[string]string) (*credentialBase, error) {
+func makeSalt() []byte {
+	result := make([]byte, 16)
+	_, err := rand.Read(result)
+	if err != nil {
+		panic(err)
+	}
+	return result
+}
+
+func newCredentialBaseFromData(data map[string]string) (*credentialBase, error) {
 	base := &credentialBase{
-		target: conf[configKeyTarget],
-		meta:   metaFromConfig(conf),
+		target: data[keyringKeyTarget],
+		meta:   metaFromData(data),
 	}
 
-	if createTime, ok := conf[configKeyCreateTime]; ok {
+	if createTime, ok := data[keyringKeyCreateTime]; ok {
 		t, err := repository.ParseTimestamp(createTime)
 		if err != nil {
 			return nil, err
@@ -39,7 +51,7 @@ func newCredentialBaseFromConfig(conf map[string]string) (*credentialBase, error
 		return nil, fmt.Errorf("missing create time")
 	}
 
-	salt, err := saltFromConfig(conf)
+	salt, err := saltFromData(data)
 	if err != nil {
 		return nil, err
 	}
@@ -48,6 +60,28 @@ func newCredentialBaseFromConfig(conf map[string]string) (*credentialBase, error
 	return base, nil
 }
 
+func metaFromData(data map[string]string) map[string]string {
+	result := make(map[string]string)
+	for key, val := range data {
+		if strings.HasPrefix(key, keyringKeyPrefixMeta) {
+			key = strings.TrimPrefix(key, keyringKeyPrefixMeta)
+			result[key] = val
+		}
+	}
+	if len(result) == 0 {
+		return nil
+	}
+	return result
+}
+
+func saltFromData(data map[string]string) ([]byte, error) {
+	val, ok := data[keyringKeySalt]
+	if !ok {
+		return nil, fmt.Errorf("no credential salt found")
+	}
+	return base64.StdEncoding.DecodeString(val)
+}
+
 func (cb *credentialBase) Target() string {
 	return cb.target
 }
diff --git a/bridge/core/auth/login.go b/bridge/core/auth/login.go
index ea74835a..496f2412 100644
--- a/bridge/core/auth/login.go
+++ b/bridge/core/auth/login.go
@@ -8,7 +8,7 @@ import (
 )
 
 const (
-	configKeyLoginLogin = "login"
+	keyringKeyLoginLogin = "login"
 )
 
 var _ Credential = &Login{}
@@ -26,14 +26,14 @@ func NewLogin(target, login string) *Login {
 }
 
 func NewLoginFromConfig(conf map[string]string) (*Login, error) {
-	base, err := newCredentialBaseFromConfig(conf)
+	base, err := newCredentialBaseFromData(conf)
 	if err != nil {
 		return nil, err
 	}
 
 	return &Login{
 		credentialBase: base,
-		Login:          conf[configKeyLoginLogin],
+		Login:          conf[keyringKeyLoginLogin],
 	}, nil
 }
 
@@ -62,6 +62,6 @@ func (lp *Login) Validate() error {
 
 func (lp *Login) toConfig() map[string]string {
 	return map[string]string{
-		configKeyLoginLogin: lp.Login,
+		keyringKeyLoginLogin: lp.Login,
 	}
 }
diff --git a/bridge/core/auth/login_password.go b/bridge/core/auth/login_password.go
index 1981026a..166e37fb 100644
--- a/bridge/core/auth/login_password.go
+++ b/bridge/core/auth/login_password.go
@@ -8,8 +8,8 @@ import (
 )
 
 const (
-	configKeyLoginPasswordLogin    = "login"
-	configKeyLoginPasswordPassword = "password"
+	keyringKeyLoginPasswordLogin    = "login"
+	keyringKeyLoginPasswordPassword = "password"
 )
 
 var _ Credential = &LoginPassword{}
@@ -29,15 +29,15 @@ func NewLoginPassword(target, login, password string) *LoginPassword {
 }
 
 func NewLoginPasswordFromConfig(conf map[string]string) (*LoginPassword, error) {
-	base, err := newCredentialBaseFromConfig(conf)
+	base, err := newCredentialBaseFromData(conf)
 	if err != nil {
 		return nil, err
 	}
 
 	return &LoginPassword{
 		credentialBase: base,
-		Login:          conf[configKeyLoginPasswordLogin],
-		Password:       conf[configKeyLoginPasswordPassword],
+		Login:          conf[keyringKeyLoginPasswordLogin],
+		Password:       conf[keyringKeyLoginPasswordPassword],
 	}, nil
 }
 
@@ -70,7 +70,7 @@ func (lp *LoginPassword) Validate() error {
 
 func (lp *LoginPassword) toConfig() map[string]string {
 	return map[string]string{
-		configKeyLoginPasswordLogin:    lp.Login,
-		configKeyLoginPasswordPassword: lp.Password,
+		keyringKeyLoginPasswordLogin:    lp.Login,
+		keyringKeyLoginPasswordPassword: lp.Password,
 	}
 }
diff --git a/bridge/core/auth/options.go b/bridge/core/auth/options.go
index 1d8c44d1..00c6e3ec 100644
--- a/bridge/core/auth/options.go
+++ b/bridge/core/auth/options.go
@@ -1,22 +1,22 @@
 package auth
 
-type options struct {
+type listOptions struct {
 	target string
 	kind   map[CredentialKind]interface{}
 	meta   map[string]string
 }
 
-type Option func(opts *options)
+type ListOption func(opts *listOptions)
 
-func matcher(opts []Option) *options {
-	result := &options{}
+func matcher(opts []ListOption) *listOptions {
+	result := &listOptions{}
 	for _, opt := range opts {
 		opt(result)
 	}
 	return result
 }
 
-func (opts *options) Match(cred Credential) bool {
+func (opts *listOptions) Match(cred Credential) bool {
 	if opts.target != "" && cred.Target() != opts.target {
 		return false
 	}
@@ -35,15 +35,15 @@ func (opts *options) Match(cred Credential) bool {
 	return true
 }
 
-func WithTarget(target string) Option {
-	return func(opts *options) {
+func WithTarget(target string) ListOption {
+	return func(opts *listOptions) {
 		opts.target = target
 	}
 }
 
 // WithKind match credentials with the given kind. Can be specified multiple times.
-func WithKind(kind CredentialKind) Option {
-	return func(opts *options) {
+func WithKind(kind CredentialKind) ListOption {
+	return func(opts *listOptions) {
 		if opts.kind == nil {
 			opts.kind = make(map[CredentialKind]interface{})
 		}
@@ -51,8 +51,8 @@ func WithKind(kind CredentialKind) Option {
 	}
 }
 
-func WithMeta(key string, val string) Option {
-	return func(opts *options) {
+func WithMeta(key string, val string) ListOption {
+	return func(opts *listOptions) {
 		if opts.meta == nil {
 			opts.meta = make(map[string]string)
 		}
diff --git a/bridge/core/auth/token.go b/bridge/core/auth/token.go
index 1f019f44..84d6ac13 100644
--- a/bridge/core/auth/token.go
+++ b/bridge/core/auth/token.go
@@ -8,7 +8,7 @@ import (
 )
 
 const (
-	configKeyTokenValue = "value"
+	keyringKeyTokenValue = "value"
 )
 
 var _ Credential = &Token{}
@@ -28,14 +28,14 @@ func NewToken(target, value string) *Token {
 }
 
 func NewTokenFromConfig(conf map[string]string) (*Token, error) {
-	base, err := newCredentialBaseFromConfig(conf)
+	base, err := newCredentialBaseFromData(conf)
 	if err != nil {
 		return nil, err
 	}
 
 	return &Token{
 		credentialBase: base,
-		Value:          conf[configKeyTokenValue],
+		Value:          conf[keyringKeyTokenValue],
 	}, nil
 }
 
@@ -65,6 +65,6 @@ func (t *Token) Validate() error {
 
 func (t *Token) toConfig() map[string]string {
 	return map[string]string{
-		configKeyTokenValue: t.Value,
+		keyringKeyTokenValue: t.Value,
 	}
 }
diff --git a/bridge/github/config.go b/bridge/github/config.go
index 61d641a6..130b0ad1 100644
--- a/bridge/github/config.go
+++ b/bridge/github/config.go
@@ -239,7 +239,7 @@ func randomFingerprint() string {
 	return string(b)
 }
 
-func promptTokenOptions(repo repository.RepoConfig, login, owner, project string) (auth.Credential, error) {
+func promptTokenOptions(repo repository.RepoKeyring, login, owner, project string) (auth.Credential, error) {
 	creds, err := auth.List(repo,
 		auth.WithTarget(target),
 		auth.WithKind(auth.KindToken),
diff --git a/bridge/gitlab/config.go b/bridge/gitlab/config.go
index e4e3d8e3..dfac4c54 100644
--- a/bridge/gitlab/config.go
+++ b/bridge/gitlab/config.go
@@ -156,7 +156,7 @@ func (g *Gitlab) ValidateConfig(conf core.Configuration) error {
 	return nil
 }
 
-func promptTokenOptions(repo repository.RepoConfig, login, baseUrl string) (auth.Credential, error) {
+func promptTokenOptions(repo repository.RepoKeyring, login, baseUrl string) (auth.Credential, error) {
 	creds, err := auth.List(repo,
 		auth.WithTarget(target),
 		auth.WithKind(auth.KindToken),
diff --git a/bridge/jira/config.go b/bridge/jira/config.go
index ffd3bdc1..717046e2 100644
--- a/bridge/jira/config.go
+++ b/bridge/jira/config.go
@@ -163,7 +163,7 @@ func (*Jira) ValidateConfig(conf core.Configuration) error {
 	return nil
 }
 
-func promptCredOptions(repo repository.RepoConfig, login, baseUrl string) (auth.Credential, error) {
+func promptCredOptions(repo repository.RepoKeyring, login, baseUrl string) (auth.Credential, error) {
 	creds, err := auth.List(repo,
 		auth.WithTarget(target),
 		auth.WithKind(auth.KindToken),
diff --git a/cache/repo_cache_common.go b/cache/repo_cache_common.go
index a931f2be..1e330a4a 100644
--- a/cache/repo_cache_common.go
+++ b/cache/repo_cache_common.go
@@ -20,9 +20,8 @@ func (c *RepoCache) LocalConfig() repository.Config {
 	return c.repo.LocalConfig()
 }
 
-// GlobalConfig give access to the git global configuration
-func (c *RepoCache) GlobalConfig() repository.Config {
-	return c.repo.GlobalConfig()
+func (c *RepoCache) Keyring() repository.Keyring {
+	return c.repo.Keyring()
 }
 
 // GetPath returns the path to the repo.
diff --git a/repository/git_testing.go b/repository/git_testing.go
index 5ae4ccc9..7d40bf1f 100644
--- a/repository/git_testing.go
+++ b/repository/git_testing.go
@@ -3,6 +3,8 @@ package repository
 import (
 	"io/ioutil"
 	"log"
+
+	"github.com/99designs/keyring"
 )
 
 // This is intended for testing only
@@ -34,7 +36,11 @@ func CreateTestRepo(bare bool) TestedRepo {
 		log.Fatal("failed to set user.email for test repository: ", err)
 	}
 
-	return repo
+	// make sure we use a mock keyring for testing to not interact with the global system
+	return &replaceKeyring{
+		TestedRepo: repo,
+		keyring:    keyring.NewArrayKeyring(nil),
+	}
 }
 
 func SetupReposAndRemote() (repoA, repoB, remote TestedRepo) {
@@ -56,3 +62,13 @@ func SetupReposAndRemote() (repoA, repoB, remote TestedRepo) {
 
 	return repoA, repoB, remote
 }
+
+// replaceKeyring allow to replace the Keyring of the underlying repo
+type replaceKeyring struct {
+	TestedRepo
+	keyring Keyring
+}
+
+func (rk replaceKeyring) Keyring() Keyring {
+	return rk.keyring
+}
diff --git a/repository/repo.go b/repository/repo.go
index 696b032e..7a69a14f 100644
--- a/repository/repo.go
+++ b/repository/repo.go
@@ -20,11 +20,6 @@ var (
 type RepoConfig interface {
 	// LocalConfig give access to the repository scoped configuration
 	LocalConfig() Config
-
-	// GlobalConfig give access to the git global configuration
-	// Deprecated: to remove in favor of Keyring()
-	// TODO: remove
-	GlobalConfig() Config
 }
 
 // RepoKeyring give access to a user-wide storage for secrets