Merge pull request #321 from MichaelMure/cred-password

auth: refactor and introduce Login and LoginPassword, salt IDs

16 files changed, 376 insertions, 77 deletions

diff --git a/bridge/core/auth/credential.go b/bridge/core/auth/credential.go
index 6dcac09f..86cf737e 100644
--- a/bridge/core/auth/credential.go
+++ b/bridge/core/auth/credential.go
@@ -1,6 +1,8 @@
 package auth
 
 import (
+	"crypto/rand"
+	"encoding/base64"
 	"errors"
 	"fmt"
 	"regexp"
@@ -16,6 +18,7 @@ const (
 	configKeyKind       = "kind"
 	configKeyTarget     = "target"
 	configKeyCreateTime = "createtime"
+	configKeySalt       = "salt"
 	configKeyPrefixMeta = "meta."
 
 	MetaKeyLogin   = "login"
@@ -26,6 +29,7 @@ type CredentialKind string
 
 const (
 	KindToken         CredentialKind = "token"
+	KindLogin         CredentialKind = "login"
 	KindLoginPassword CredentialKind = "login-password"
 )
 
@@ -37,9 +41,10 @@ func NewErrMultipleMatchCredential(matching []entity.Id) *entity.ErrMultipleMatc
 
 type Credential interface {
 	ID() entity.Id
-	Target() string
 	Kind() CredentialKind
+	Target() string
 	CreateTime() time.Time
+	Salt() []byte
 	Validate() error
 
 	Metadata() map[string]string
@@ -47,7 +52,7 @@ type Credential interface {
 	SetMetadata(key string, value string)
 
 	// Return all the specific properties of the credential that need to be saved into the configuration.
-	// This does not include Target, Kind, CreateTime and Metadata.
+	// This does not include Target, Kind, CreateTime, Metadata or Salt.
 	toConfig() map[string]string
 }
 
@@ -108,15 +113,23 @@ func loadFromConfig(rawConfigs map[string]string, id entity.Id) (Credential, err
 	}
 
 	var cred Credential
+	var err error
 
 	switch CredentialKind(configs[configKeyKind]) {
 	case KindToken:
-		cred = NewTokenFromConfig(configs)
+		cred, err = NewTokenFromConfig(configs)
+	case KindLogin:
+		cred, err = NewLoginFromConfig(configs)
 	case KindLoginPassword:
+		cred, err = NewLoginPasswordFromConfig(configs)
 	default:
 		return nil, fmt.Errorf("unknown credential type %s", configs[configKeyKind])
 	}
 
+	if err != nil {
+		return nil, fmt.Errorf("loading credential: %v", err)
+	}
+
 	return cred, nil
 }
 
@@ -134,6 +147,23 @@ func metaFromConfig(configs map[string]string) map[string]string {
 	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 + ".")
@@ -211,6 +241,16 @@ func Store(repo repository.RepoConfig, cred Credential) error {
 		return err
 	}
 
+	// Salt
+	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
 	for key, val := range cred.Metadata() {
 		err := repo.GlobalConfig().StoreString(prefix+configKeyPrefixMeta+key, val)
diff --git a/bridge/core/auth/credential_base.go b/bridge/core/auth/credential_base.go
new file mode 100644
index 00000000..488c223c
--- /dev/null
+++ b/bridge/core/auth/credential_base.go
@@ -0,0 +1,90 @@
+package auth
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/MichaelMure/git-bug/bridge/core"
+	"github.com/MichaelMure/git-bug/repository"
+)
+
+type credentialBase struct {
+	target     string
+	createTime time.Time
+	salt       []byte
+	meta       map[string]string
+}
+
+func newCredentialBase(target string) *credentialBase {
+	return &credentialBase{
+		target:     target,
+		createTime: time.Now(),
+		salt:       makeSalt(),
+	}
+}
+
+func newCredentialBaseFromConfig(conf map[string]string) (*credentialBase, error) {
+	base := &credentialBase{
+		target: conf[configKeyTarget],
+		meta:   metaFromConfig(conf),
+	}
+
+	if createTime, ok := conf[configKeyCreateTime]; ok {
+		t, err := repository.ParseTimestamp(createTime)
+		if err != nil {
+			return nil, err
+		}
+		base.createTime = t
+	} else {
+		return nil, fmt.Errorf("missing create time")
+	}
+
+	salt, err := saltFromConfig(conf)
+	if err != nil {
+		return nil, err
+	}
+	base.salt = salt
+
+	return base, nil
+}
+
+func (cb *credentialBase) Target() string {
+	return cb.target
+}
+
+func (cb *credentialBase) CreateTime() time.Time {
+	return cb.createTime
+}
+
+func (cb *credentialBase) Salt() []byte {
+	return cb.salt
+}
+
+func (cb *credentialBase) validate() error {
+	if cb.target == "" {
+		return fmt.Errorf("missing target")
+	}
+	if cb.createTime.IsZero() || cb.createTime.Equal(time.Time{}) {
+		return fmt.Errorf("missing creation time")
+	}
+	if !core.TargetExist(cb.target) {
+		return fmt.Errorf("unknown target")
+	}
+	return nil
+}
+
+func (cb *credentialBase) Metadata() map[string]string {
+	return cb.meta
+}
+
+func (cb *credentialBase) GetMetadata(key string) (string, bool) {
+	val, ok := cb.meta[key]
+	return val, ok
+}
+
+func (cb *credentialBase) SetMetadata(key string, value string) {
+	if cb.meta == nil {
+		cb.meta = make(map[string]string)
+	}
+	cb.meta[key] = value
+}
diff --git a/bridge/core/auth/credential_test.go b/bridge/core/auth/credential_test.go
index 2f8806c9..60c631d7 100644
--- a/bridge/core/auth/credential_test.go
+++ b/bridge/core/auth/credential_test.go
@@ -14,7 +14,7 @@ func TestCredential(t *testing.T) {
 	repo := repository.NewMockRepoForTest()
 
 	storeToken := func(val string, target string) *Token {
-		token := NewToken(val, target)
+		token := NewToken(target, val)
 		err := Store(repo, token)
 		require.NoError(t, err)
 		return token
@@ -100,3 +100,25 @@ func sameIds(t *testing.T, a []Credential, b []Credential) {
 
 	assert.ElementsMatch(t, ids(a), ids(b))
 }
+
+func testCredentialSerial(t *testing.T, original Credential) Credential {
+	repo := repository.NewMockRepoForTest()
+
+	original.SetMetadata("test", "value")
+
+	assert.NotEmpty(t, original.ID().String())
+	assert.NotEmpty(t, original.Salt())
+	assert.NoError(t, Store(repo, original))
+
+	loaded, err := LoadWithId(repo, original.ID())
+	assert.NoError(t, err)
+
+	assert.Equal(t, original.ID(), loaded.ID())
+	assert.Equal(t, original.Kind(), loaded.Kind())
+	assert.Equal(t, original.Target(), loaded.Target())
+	assert.Equal(t, original.CreateTime().Unix(), loaded.CreateTime().Unix())
+	assert.Equal(t, original.Salt(), loaded.Salt())
+	assert.Equal(t, original.Metadata(), loaded.Metadata())
+
+	return loaded
+}
diff --git a/bridge/core/auth/login.go b/bridge/core/auth/login.go
new file mode 100644
index 00000000..ea74835a
--- /dev/null
+++ b/bridge/core/auth/login.go
@@ -0,0 +1,67 @@
+package auth
+
+import (
+	"crypto/sha256"
+	"fmt"
+
+	"github.com/MichaelMure/git-bug/entity"
+)
+
+const (
+	configKeyLoginLogin = "login"
+)
+
+var _ Credential = &Login{}
+
+type Login struct {
+	*credentialBase
+	Login string
+}
+
+func NewLogin(target, login string) *Login {
+	return &Login{
+		credentialBase: newCredentialBase(target),
+		Login:          login,
+	}
+}
+
+func NewLoginFromConfig(conf map[string]string) (*Login, error) {
+	base, err := newCredentialBaseFromConfig(conf)
+	if err != nil {
+		return nil, err
+	}
+
+	return &Login{
+		credentialBase: base,
+		Login:          conf[configKeyLoginLogin],
+	}, nil
+}
+
+func (lp *Login) ID() entity.Id {
+	h := sha256.New()
+	_, _ = h.Write(lp.salt)
+	_, _ = h.Write([]byte(lp.target))
+	_, _ = h.Write([]byte(lp.Login))
+	return entity.Id(fmt.Sprintf("%x", h.Sum(nil)))
+}
+
+func (lp *Login) Kind() CredentialKind {
+	return KindLogin
+}
+
+func (lp *Login) Validate() error {
+	err := lp.credentialBase.validate()
+	if err != nil {
+		return err
+	}
+	if lp.Login == "" {
+		return fmt.Errorf("missing login")
+	}
+	return nil
+}
+
+func (lp *Login) toConfig() map[string]string {
+	return map[string]string{
+		configKeyLoginLogin: lp.Login,
+	}
+}
diff --git a/bridge/core/auth/login_password.go b/bridge/core/auth/login_password.go
new file mode 100644
index 00000000..1981026a
--- /dev/null
+++ b/bridge/core/auth/login_password.go
@@ -0,0 +1,76 @@
+package auth
+
+import (
+	"crypto/sha256"
+	"fmt"
+
+	"github.com/MichaelMure/git-bug/entity"
+)
+
+const (
+	configKeyLoginPasswordLogin    = "login"
+	configKeyLoginPasswordPassword = "password"
+)
+
+var _ Credential = &LoginPassword{}
+
+type LoginPassword struct {
+	*credentialBase
+	Login    string
+	Password string
+}
+
+func NewLoginPassword(target, login, password string) *LoginPassword {
+	return &LoginPassword{
+		credentialBase: newCredentialBase(target),
+		Login:          login,
+		Password:       password,
+	}
+}
+
+func NewLoginPasswordFromConfig(conf map[string]string) (*LoginPassword, error) {
+	base, err := newCredentialBaseFromConfig(conf)
+	if err != nil {
+		return nil, err
+	}
+
+	return &LoginPassword{
+		credentialBase: base,
+		Login:          conf[configKeyLoginPasswordLogin],
+		Password:       conf[configKeyLoginPasswordPassword],
+	}, nil
+}
+
+func (lp *LoginPassword) ID() entity.Id {
+	h := sha256.New()
+	_, _ = h.Write(lp.salt)
+	_, _ = h.Write([]byte(lp.target))
+	_, _ = h.Write([]byte(lp.Login))
+	_, _ = h.Write([]byte(lp.Password))
+	return entity.Id(fmt.Sprintf("%x", h.Sum(nil)))
+}
+
+func (lp *LoginPassword) Kind() CredentialKind {
+	return KindLoginPassword
+}
+
+func (lp *LoginPassword) Validate() error {
+	err := lp.credentialBase.validate()
+	if err != nil {
+		return err
+	}
+	if lp.Login == "" {
+		return fmt.Errorf("missing login")
+	}
+	if lp.Password == "" {
+		return fmt.Errorf("missing password")
+	}
+	return nil
+}
+
+func (lp *LoginPassword) toConfig() map[string]string {
+	return map[string]string{
+		configKeyLoginPasswordLogin:    lp.Login,
+		configKeyLoginPasswordPassword: lp.Password,
+	}
+}
diff --git a/bridge/core/auth/login_password_test.go b/bridge/core/auth/login_password_test.go
new file mode 100644
index 00000000..d9d82f52
--- /dev/null
+++ b/bridge/core/auth/login_password_test.go
@@ -0,0 +1,14 @@
+package auth
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLoginPasswordSerial(t *testing.T) {
+	original := NewLoginPassword("github", "jean", "jacques")
+	loaded := testCredentialSerial(t, original)
+	assert.Equal(t, original.Login, loaded.(*LoginPassword).Login)
+	assert.Equal(t, original.Password, loaded.(*LoginPassword).Password)
+}
diff --git a/bridge/core/auth/login_test.go b/bridge/core/auth/login_test.go
new file mode 100644
index 00000000..3fc4a391
--- /dev/null
+++ b/bridge/core/auth/login_test.go
@@ -0,0 +1,13 @@
+package auth
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLoginSerial(t *testing.T) {
+	original := NewLogin("github", "jean")
+	loaded := testCredentialSerial(t, original)
+	assert.Equal(t, original.Login, loaded.(*Login).Login)
+}
diff --git a/bridge/core/auth/token.go b/bridge/core/auth/token.go
index 42f960bf..1f019f44 100644
--- a/bridge/core/auth/token.go
+++ b/bridge/core/auth/token.go
@@ -3,104 +3,68 @@ package auth
 import (
 	"crypto/sha256"
 	"fmt"
-	"time"
 
-	"github.com/MichaelMure/git-bug/bridge/core"
 	"github.com/MichaelMure/git-bug/entity"
-	"github.com/MichaelMure/git-bug/repository"
 )
 
 const (
-	tokenValueKey = "value"
+	configKeyTokenValue = "value"
 )
 
 var _ Credential = &Token{}
 
 // Token holds an API access token data
 type Token struct {
-	target     string
-	createTime time.Time
-	Value      string
-	meta       map[string]string
+	*credentialBase
+	Value string
 }
 
 // NewToken instantiate a new token
-func NewToken(value, target string) *Token {
+func NewToken(target, value string) *Token {
 	return &Token{
-		target:     target,
-		createTime: time.Now(),
-		Value:      value,
+		credentialBase: newCredentialBase(target),
+		Value:          value,
 	}
 }
 
-func NewTokenFromConfig(conf map[string]string) *Token {
-	token := &Token{}
-
-	token.target = conf[configKeyTarget]
-	if createTime, ok := conf[configKeyCreateTime]; ok {
-		if t, err := repository.ParseTimestamp(createTime); err == nil {
-			token.createTime = t
-		}
+func NewTokenFromConfig(conf map[string]string) (*Token, error) {
+	base, err := newCredentialBaseFromConfig(conf)
+	if err != nil {
+		return nil, err
 	}
 
-	token.Value = conf[tokenValueKey]
-	token.meta = metaFromConfig(conf)
-
-	return token
+	return &Token{
+		credentialBase: base,
+		Value:          conf[configKeyTokenValue],
+	}, nil
 }
 
 func (t *Token) ID() entity.Id {
-	sum := sha256.Sum256([]byte(t.target + t.Value))
-	return entity.Id(fmt.Sprintf("%x", sum))
-}
-
-func (t *Token) Target() string {
-	return t.target
+	h := sha256.New()
+	_, _ = h.Write(t.salt)
+	_, _ = h.Write([]byte(t.target))
+	_, _ = h.Write([]byte(t.Value))
+	return entity.Id(fmt.Sprintf("%x", h.Sum(nil)))
 }
 
 func (t *Token) Kind() CredentialKind {
 	return KindToken
 }
 
-func (t *Token) CreateTime() time.Time {
-	return t.createTime
-}
-
 // Validate ensure token important fields are valid
 func (t *Token) Validate() error {
+	err := t.credentialBase.validate()
+	if err != nil {
+		return err
+	}
 	if t.Value == "" {
 		return fmt.Errorf("missing value")
 	}
-	if t.target == "" {
-		return fmt.Errorf("missing target")
-	}
-	if t.createTime.IsZero() || t.createTime.Equal(time.Time{}) {
-		return fmt.Errorf("missing creation time")
-	}
-	if !core.TargetExist(t.target) {
-		return fmt.Errorf("unknown target")
-	}
 	return nil
 }
 
-func (t *Token) Metadata() map[string]string {
-	return t.meta
-}
-
-func (t *Token) GetMetadata(key string) (string, bool) {
-	val, ok := t.meta[key]
-	return val, ok
-}
-
-func (t *Token) SetMetadata(key string, value string) {
-	if t.meta == nil {
-		t.meta = make(map[string]string)
-	}
-	t.meta[key] = value
-}
-
 func (t *Token) toConfig() map[string]string {
 	return map[string]string{
-		tokenValueKey: t.Value,
+		configKeyTokenValue: t.Value,
 	}
 }
diff --git a/bridge/core/auth/token_test.go b/bridge/core/auth/token_test.go
new file mode 100644
index 00000000..d8cd6652
--- /dev/null
+++ b/bridge/core/auth/token_test.go
@@ -0,0 +1,13 @@
+package auth
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestTokenSerial(t *testing.T) {
+	original := NewToken("github", "value")
+	loaded := testCredentialSerial(t, original)
+	assert.Equal(t, original.Value, loaded.(*Token).Value)
+}
diff --git a/bridge/github/config.go b/bridge/github/config.go
index cc312230..afb8086c 100644
--- a/bridge/github/config.go
+++ b/bridge/github/config.go
@@ -86,7 +86,7 @@ func (g *Github) Configure(repo *cache.RepoCache, params core.BridgeParams) (cor
 		}
 		login = l
 	case params.TokenRaw != "":
-		token := auth.NewToken(params.TokenRaw, target)
+		token := auth.NewToken(target, params.TokenRaw)
 		login, err = getLoginFromToken(token)
 		if err != nil {
 			return nil, err
@@ -296,7 +296,7 @@ func promptTokenOptions(repo repository.RepoConfig, login, owner, project string
 			if err != nil {
 				return nil, err
 			}
-			token := auth.NewToken(value, target)
+			token := auth.NewToken(target, value)
 			token.SetMetadata(auth.MetaKeyLogin, login)
 			return token, nil
 		default:
@@ -327,7 +327,7 @@ func promptToken() (*auth.Token, error) {
 		if !re.MatchString(value) {
 			return "token has incorrect format", nil
 		}
-		login, err = getLoginFromToken(auth.NewToken(value, target))
+		login, err = getLoginFromToken(auth.NewToken(target, value))
 		if err != nil {
 			return fmt.Sprintf("token is invalid: %v", err), nil
 		}
@@ -339,7 +339,7 @@ func promptToken() (*auth.Token, error) {
 		return nil, err
 	}
 
-	token := auth.NewToken(rawToken, target)
+	token := auth.NewToken(target, rawToken)
 	token.SetMetadata(auth.MetaKeyLogin, login)
 
 	return token, nil
diff --git a/bridge/github/config_test.go b/bridge/github/config_test.go
index d7b1b38d..fe54c209 100644
--- a/bridge/github/config_test.go
+++ b/bridge/github/config_test.go
@@ -154,8 +154,8 @@ func TestValidateProject(t *testing.T) {
 		t.Skip("Env var GITHUB_TOKEN_PUBLIC missing")
 	}
 
-	tokenPrivate := auth.NewToken(envPrivate, target)
-	tokenPublic := auth.NewToken(envPublic, target)
+	tokenPrivate := auth.NewToken(target, envPrivate)
+	tokenPublic := auth.NewToken(target, envPublic)
 
 	type args struct {
 		owner   string
diff --git a/bridge/github/export_test.go b/bridge/github/export_test.go
index 7d6e6fb1..56e29835 100644
--- a/bridge/github/export_test.go
+++ b/bridge/github/export_test.go
@@ -157,7 +157,7 @@ func TestPushPull(t *testing.T) {
 	defer backend.Close()
 	interrupt.RegisterCleaner(backend.Close)
 
-	token := auth.NewToken(envToken, target)
+	token := auth.NewToken(target, envToken)
 	token.SetMetadata(auth.MetaKeyLogin, login)
 	err = auth.Store(repo, token)
 	require.NoError(t, err)
diff --git a/bridge/github/import_test.go b/bridge/github/import_test.go
index a8f8e346..7eb901d3 100644
--- a/bridge/github/import_test.go
+++ b/bridge/github/import_test.go
@@ -144,7 +144,7 @@ func Test_Importer(t *testing.T) {
 	login := "test-identity"
 	author.SetMetadata(metaKeyGithubLogin, login)
 
-	token := auth.NewToken(envToken, target)
+	token := auth.NewToken(target, envToken)
 	token.SetMetadata(auth.MetaKeyLogin, login)
 	err = auth.Store(repo, token)
 	require.NoError(t, err)
diff --git a/bridge/gitlab/config.go b/bridge/gitlab/config.go
index 62d385dc..9bd9c3c7 100644
--- a/bridge/gitlab/config.go
+++ b/bridge/gitlab/config.go
@@ -83,7 +83,7 @@ func (g *Gitlab) Configure(repo *cache.RepoCache, params core.BridgeParams) (cor
 		}
 		login = l
 	case params.TokenRaw != "":
-		token := auth.NewToken(params.TokenRaw, target)
+		token := auth.NewToken(target, params.TokenRaw)
 		login, err = getLoginFromToken(baseUrl, token)
 		if err != nil {
 			return nil, err
@@ -265,7 +265,7 @@ func promptToken(baseUrl string) (*auth.Token, error) {
 		if !re.MatchString(value) {
 			return "token has incorrect format", nil
 		}
-		login, err = getLoginFromToken(baseUrl, auth.NewToken(value, target))
+		login, err = getLoginFromToken(baseUrl, auth.NewToken(target, value))
 		if err != nil {
 			return fmt.Sprintf("token is invalid: %v", err), nil
 		}
@@ -277,7 +277,7 @@ func promptToken(baseUrl string) (*auth.Token, error) {
 		return nil, err
 	}
 
-	token := auth.NewToken(rawToken, target)
+	token := auth.NewToken(target, rawToken)
 	token.SetMetadata(auth.MetaKeyLogin, login)
 	token.SetMetadata(auth.MetaKeyBaseURL, baseUrl)
 
diff --git a/bridge/gitlab/export_test.go b/bridge/gitlab/export_test.go
index c97416d8..768b899c 100644
--- a/bridge/gitlab/export_test.go
+++ b/bridge/gitlab/export_test.go
@@ -162,7 +162,7 @@ func TestPushPull(t *testing.T) {
 	defer backend.Close()
 	interrupt.RegisterCleaner(backend.Close)
 
-	token := auth.NewToken(envToken, target)
+	token := auth.NewToken(target, envToken)
 	token.SetMetadata(auth.MetaKeyLogin, login)
 	token.SetMetadata(auth.MetaKeyBaseURL, defaultBaseURL)
 	err = auth.Store(repo, token)
diff --git a/bridge/gitlab/import_test.go b/bridge/gitlab/import_test.go
index a300acf1..99d0d69e 100644
--- a/bridge/gitlab/import_test.go
+++ b/bridge/gitlab/import_test.go
@@ -98,7 +98,7 @@ func TestImport(t *testing.T) {
 	login := "test-identity"
 	author.SetMetadata(metaKeyGitlabLogin, login)
 
-	token := auth.NewToken(envToken, target)
+	token := auth.NewToken(target, envToken)
 	token.SetMetadata(auth.MetaKeyLogin, login)
 	token.SetMetadata(auth.MetaKeyBaseURL, defaultBaseURL)
 	err = auth.Store(repo, token)