feat(identity): store and retrieve full GPG keys

Steve Moyer created

Change summary

entities/identity/identity_test.go | 154 +++++++++++++++++++++++++++++--
entities/identity/key.go           | 146 ++++++++++++++----------------
entities/identity/key_test.go      |  40 +++++++-
entities/identity/version_test.go  |  42 ++++++++
entity/dag/operation_pack_test.go  |   2 
5 files changed, 283 insertions(+), 101 deletions(-)

Detailed changes

entities/identity/identity_test.go ๐Ÿ”—

@@ -1,6 +1,7 @@
 package identity
 
 import (
+	"bytes"
 	"encoding/json"
 	"testing"
 
@@ -9,8 +10,132 @@ import (
 	"github.com/git-bug/git-bug/entity"
 	"github.com/git-bug/git-bug/repository"
 	"github.com/git-bug/git-bug/util/lamport"
+	"github.com/git-bug/git-bug/util/timestamp"
 )
 
+type testIdentityMock struct {
+	name  string
+	login string
+	email string
+}
+
+func (m *testIdentityMock) Name() string                                        { return m.name }
+func (m *testIdentityMock) Login() string                                      { return m.login }
+func (m *testIdentityMock) Email() string                                     { return m.email }
+func (m *testIdentityMock) DisplayName() string                               { return m.name }
+func (m *testIdentityMock) AvatarUrl() string                                 { return "" }
+func (m *testIdentityMock) Keys() []*Key                                      { return nil }
+func (m *testIdentityMock) SigningKey(repo repository.RepoKeyring) (*Key, error) { return nil, nil }
+func (m *testIdentityMock) ValidKeysAtTime(clockName string, time lamport.Time) []*Key { return nil }
+func (m *testIdentityMock) LastModification() timestamp.Timestamp             { return 0 }
+func (m *testIdentityMock) LastModificationLamports() map[string]lamport.Time { return nil }
+func (m *testIdentityMock) IsProtected() bool                                 { return false }
+func (m *testIdentityMock) Validate() error                                   { return nil }
+func (m *testIdentityMock) NeedCommit() bool                                  { return false }
+func (m *testIdentityMock) Id() entity.Id                                     { return "" }
+
+// identitiesEqual compares two identities by their versions
+func identitiesEqual(left, right *Identity) bool {
+	if left == nil && right == nil {
+		return true
+	}
+	if left == nil || right == nil {
+		return false
+	}
+
+	if len(left.versions) != len(right.versions) {
+		return false
+	}
+
+	for i, lv := range left.versions {
+		rv := right.versions[i]
+		if !versionsEqual(lv, rv) {
+			return false
+		}
+	}
+
+	return true
+}
+
+// versionsEqual compares two versions, comparing keys by their public key fingerprints
+func versionsEqual(left, right *version) bool {
+	if left == nil && right == nil {
+		return true
+	}
+	if left == nil || right == nil {
+		return false
+	}
+
+	// Compare basic fields
+	if left.name != right.name || left.email != right.email ||
+		left.login != right.login || left.avatarURL != right.avatarURL ||
+		left.unixTime != right.unixTime {
+		return false
+	}
+
+	// Compare times
+	if len(left.times) != len(right.times) {
+		return false
+	}
+	for k, tv := range left.times {
+		ov, ok := right.times[k]
+		if !ok || tv != ov {
+			return false
+		}
+	}
+
+	// Compare keys by fingerprint
+	if len(left.keys) != len(right.keys) {
+		return false
+	}
+	for i, k := range left.keys {
+		if !keysEqual(k, right.keys[i]) {
+			return false
+		}
+	}
+
+	// Compare nonce
+	if len(left.nonce) != len(right.nonce) {
+		return false
+	}
+	for i, n := range left.nonce {
+		if n != right.nonce[i] {
+			return false
+		}
+	}
+
+	// Compare metadata
+	if len(left.metadata) != len(right.metadata) {
+		return false
+	}
+	for k, vm := range left.metadata {
+		om, ok := right.metadata[k]
+		if !ok || vm != om {
+			return false
+		}
+	}
+
+	// Don't compare id and commitHash as they're derived fields
+	return true
+}
+
+// keysEqual compares two keys by their public key fingerprint
+func keysEqual(left, right *Key) bool {
+	if left == nil && right == nil {
+		return true
+	}
+	if left == nil || right == nil {
+		return false
+	}
+	if left.public == nil && right.public == nil {
+		return true
+	}
+	if left.public == nil || right.public == nil {
+		return false
+	}
+	return bytes.Equal(left.public.Fingerprint[:], right.public.Fingerprint[:])
+}
+
 // Test the commit and load of an Identity with multiple versions
 func TestIdentityCommitLoad(t *testing.T) {
 	repo := makeIdentityTestRepo(t)
@@ -33,22 +158,22 @@ func TestIdentityCommitLoad(t *testing.T) {
 	loaded, err := ReadLocal(repo, identity.Id())
 	require.NoError(t, err)
 	commitsAreSet(t, loaded)
-	require.Equal(t, identity, loaded)
+	require.True(t, identitiesEqual(identity, loaded), "loaded identity should equal original (comparing by key fingerprint)")
 
 	// multiple versions
-
-	identity, err = NewIdentityFull(repo, "Renรฉ Descartes", "rene.descartes@example.com", "", "", []*Key{generatePublicKey()})
+	testIdentity := &testIdentityMock{name: "Renรฉ Descartes", email: "rene.descartes@example.com"}
+	identity, err = NewIdentityFull(repo, "Renรฉ Descartes", "rene.descartes@example.com", "", "", []*Key{generatePublicKey(testIdentity)})
 	require.NoError(t, err)
 
 	idBeforeCommit = identity.Id()
 
 	err = identity.Mutate(repo, func(orig *Mutator) {
-		orig.Keys = []*Key{generatePublicKey()}
+		orig.Keys = []*Key{generatePublicKey(testIdentity)}
 	})
 	require.NoError(t, err)
 
 	err = identity.Mutate(repo, func(orig *Mutator) {
-		orig.Keys = []*Key{generatePublicKey()}
+		orig.Keys = []*Key{generatePublicKey(testIdentity)}
 	})
 	require.NoError(t, err)
 
@@ -65,19 +190,19 @@ func TestIdentityCommitLoad(t *testing.T) {
 	loaded, err = ReadLocal(repo, identity.Id())
 	require.NoError(t, err)
 	commitsAreSet(t, loaded)
-	require.Equal(t, identity, loaded)
+	require.True(t, identitiesEqual(identity, loaded), "loaded identity should equal original (comparing by key fingerprint)")
 
 	// add more version
 
 	err = identity.Mutate(repo, func(orig *Mutator) {
 		orig.Email = "rene@descartes.com"
-		orig.Keys = []*Key{generatePublicKey()}
+		orig.Keys = []*Key{generatePublicKey(testIdentity)}
 	})
 	require.NoError(t, err)
 
 	err = identity.Mutate(repo, func(orig *Mutator) {
 		orig.Email = "rene@descartes.com"
-		orig.Keys = []*Key{generatePublicKey(), generatePublicKey()}
+		orig.Keys = []*Key{generatePublicKey(testIdentity), generatePublicKey(testIdentity)}
 	})
 	require.NoError(t, err)
 
@@ -92,7 +217,7 @@ func TestIdentityCommitLoad(t *testing.T) {
 	loaded, err = ReadLocal(repo, identity.Id())
 	require.NoError(t, err)
 	commitsAreSet(t, loaded)
-	require.Equal(t, identity, loaded)
+	require.True(t, identitiesEqual(identity, loaded), "loaded identity should equal original (comparing by key fingerprint)")
 }
 
 func TestIdentityMutate(t *testing.T) {
@@ -124,11 +249,12 @@ func commitsAreSet(t *testing.T, identity *Identity) {
 
 // Test that the correct crypto keys are returned for a given lamport time
 func TestIdentity_ValidKeysAtTime(t *testing.T) {
-	pubKeyA := generatePublicKey()
-	pubKeyB := generatePublicKey()
-	pubKeyC := generatePublicKey()
-	pubKeyD := generatePublicKey()
-	pubKeyE := generatePublicKey()
+	testIdentity := &testIdentityMock{name: "Test User", email: "test@example.com"}
+	pubKeyA := generatePublicKey(testIdentity)
+	pubKeyB := generatePublicKey(testIdentity)
+	pubKeyC := generatePublicKey(testIdentity)
+	pubKeyD := generatePublicKey(testIdentity)
+	pubKeyE := generatePublicKey(testIdentity)
 
 	identity := Identity{
 		versions: []*version{

entities/identity/key.go ๐Ÿ”—

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"io"
 	"strings"
 	"time"
 
@@ -19,43 +18,68 @@ import (
 var errNoPrivateKey = fmt.Errorf("no private key")
 
 type Key struct {
-	public  *packet.PublicKey
-	private *packet.PrivateKey
+	public *packet.PublicKey
+	entity *openpgp.Entity
 }
 
-// GenerateKey generate a key pair (public+private)
+type keyConfig struct {
+	time time.Time
+}
+
+type KeyOption func(*keyConfig)
+
+// WithTime sets a specific time for key generation.
+// Useful for testing to ensure consistent, deterministic keys.
+func WithTime(t time.Time) KeyOption {
+	return func(cfg *keyConfig) {
+		cfg.time = t
+	}
+}
+
+// GenerateKey generate a key pair (public+private) with identity metadata.
 // The type and configuration of the key is determined by the default value in go's OpenPGP.
-func GenerateKey() *Key {
-	entity, err := openpgp.NewEntity("", "", "", &packet.Config{
-		// The armored format doesn't include the creation time, which makes the round-trip data not being fully equal.
-		// We don't care about the creation time so we can set it to the zero value.
+func GenerateKey(id Interface, opts ...KeyOption) *Key {
+	cfg := &keyConfig{
+		time: time.Now(),
+	}
+	for _, opt := range opts {
+		opt(cfg)
+	}
+
+	entity, err := openpgp.NewEntity(id.Name(), id.Login(), id.Email(), &packet.Config{
 		Time: func() time.Time {
-			return time.Time{}
+			return cfg.time
 		},
 	})
 	if err != nil {
 		panic(err)
 	}
+
 	return &Key{
-		public:  entity.PrimaryKey,
-		private: entity.PrivateKey,
+		public: entity.PrimaryKey,
+		entity: entity,
 	}
 }
 
 // generatePublicKey generate only a public key (only useful for testing)
 // See GenerateKey for the details.
-func generatePublicKey() *Key {
-	k := GenerateKey()
-	k.private = nil
+func generatePublicKey(id Interface) *Key {
+	k := GenerateKey(id, WithTime(time.Time{}))
+	// k.entity = nil
+	k.entity.PrivateKey = nil
 	return k
 }
 
 func (k *Key) Public() *packet.PublicKey {
-	return k.public
+	return k.entity.PrimaryKey
+}
+
+func (k *Key) Version() string {
+	return "new"
 }
 
 func (k *Key) Private() *packet.PrivateKey {
-	return k.private
+	return k.entity.PrivateKey
 }
 
 func (k *Key) Validate() error {
@@ -66,8 +90,8 @@ func (k *Key) Validate() error {
 		return fmt.Errorf("public key can't sign")
 	}
 
-	if k.private != nil {
-		if !k.private.CanSign() {
+	if k.entity != nil && k.entity.PrivateKey != nil {
+		if !k.entity.PrivateKey.CanSign() {
 			return fmt.Errorf("private key can't sign")
 		}
 	}
@@ -81,9 +105,9 @@ func (k *Key) Clone() *Key {
 	pub := *k.public
 	clone.public = &pub
 
-	if k.private != nil {
-		priv := *k.private
-		clone.private = &priv
+	if k.entity != nil {
+		entity := *k.entity
+		clone.entity = &entity
 	}
 
 	return clone
@@ -97,7 +121,7 @@ func (k *Key) MarshalJSON() ([]byte, error) {
 		return nil, err
 	}
 
-	err = k.public.Serialize(w)
+	err = k.entity.Serialize(w)
 	if err != nil {
 		return nil, err
 	}
@@ -109,40 +133,30 @@ func (k *Key) MarshalJSON() ([]byte, error) {
 }
 
 func (k *Key) UnmarshalJSON(data []byte) error {
-	// De-serialize only the public key, in the armored format.
+	// De-serialize the entity using the armored format.
 	var armored string
 	err := json.Unmarshal(data, &armored)
 	if err != nil {
 		return err
 	}
 
-	block, err := armor.Decode(strings.NewReader(armored))
-	if err == io.EOF {
-		return fmt.Errorf("no armored data found")
-	}
+	entities, err := openpgp.ReadArmoredKeyRing(strings.NewReader(armored))
 	if err != nil {
-		return err
+		return errors.Wrap(err, "failed to read armored key ring")
 	}
 
-	if block.Type != openpgp.PublicKeyType {
-		return fmt.Errorf("invalid key type")
+	if len(entities) != 1 {
+		return fmt.Errorf("exactly one entity should be present - got %d", len(entities))
 	}
 
-	p, err := packet.Read(block.Body)
-	if err != nil {
-		return errors.Wrap(err, "failed to read public key packet")
-	}
-
-	public, ok := p.(*packet.PublicKey)
-	if !ok {
-		return errors.New("got no packet.publicKey")
-	}
+	entity := entities[0]
 
-	// The armored format doesn't include the creation time, which makes the round-trip data not being fully equal.
-	// We don't care about the creation time so we can set it to the zero value.
-	public.CreationTime = time.Time{}
+	// The armored format (RFC 4880) doesn't preserve key creation timestamps.
+	// We don't care about the creation time, so we reset it to the zero value to ensure consistency.
+	entity.PrimaryKey.CreationTime = time.Time{}
 
-	k.public = public
+	k.public = entity.PrimaryKey
+	k.entity = entity
 	return nil
 }
 
@@ -155,40 +169,28 @@ func (k *Key) loadPrivate(repo repository.RepoKeyring) error {
 		return err
 	}
 
-	block, err := armor.Decode(bytes.NewReader(item.Data))
-	if err == io.EOF {
-		return fmt.Errorf("no armored data found")
-	}
+	entities, err := openpgp.ReadArmoredKeyRing(bytes.NewReader(item.Data))
 	if err != nil {
 		return err
 	}
 
-	if block.Type != openpgp.PrivateKeyType {
-		return fmt.Errorf("invalid key type")
-	}
-
-	p, err := packet.Read(block.Body)
-	if err != nil {
-		return errors.Wrap(err, "failed to read private key packet")
-	}
-
-	private, ok := p.(*packet.PrivateKey)
-	if !ok {
-		return errors.New("got no packet.privateKey")
+	if len(entities) != 1 {
+		return fmt.Errorf("examtly one entity should be stored - got %d", len(entities))
 	}
 
-	// The armored format doesn't include the creation time, which makes the round-trip data not being fully equal.
-	// We don't care about the creation time so we can set it to the zero value.
-	private.CreationTime = time.Time{}
+	// The armored format (RFC 4880) doesn't preserve key creation timestamps.
+	// We don't care about the creation time, so we reset it to the zero value to ensure consistency.
+	entities[0].PrivateKey.CreationTime = time.Time{}
+	k.entity = entities[0]
+	k.public = entities[0].PrimaryKey
 
-	k.private = private
 	return nil
 }
 
 // ensurePrivateKey attempt to load the corresponding private key if it is not loaded already.
 // If no private key is found, returns errNoPrivateKey
 func (k *Key) ensurePrivateKey(repo repository.RepoKeyring) error {
-	if k.private != nil {
+	if k.entity != nil && k.entity.PrivateKey != nil {
 		return nil
 	}
 
@@ -201,7 +203,7 @@ func (k *Key) storePrivate(repo repository.RepoKeyring) error {
 	if err != nil {
 		return err
 	}
-	err = k.private.Serialize(w)
+	err = k.entity.SerializePrivate(w, nil)
 	if err != nil {
 		return err
 	}
@@ -211,21 +213,11 @@ func (k *Key) storePrivate(repo repository.RepoKeyring) error {
 	}
 
 	return repo.Keyring().Set(repository.Item{
-		Key:  k.public.KeyIdString(),
+		Key:  k.entity.PrimaryKey.KeyIdString(),
 		Data: buf.Bytes(),
 	})
 }
 
 func (k *Key) PGPEntity() *openpgp.Entity {
-	e := &openpgp.Entity{
-		PrimaryKey: k.public,
-		PrivateKey: k.private,
-		Identities: map[string]*openpgp.Identity{},
-	}
-	// somehow initialize the proper fields with identity, self-signature ...
-	err := e.AddUserId("name", "", "", nil)
-	if err != nil {
-		panic(err)
-	}
-	return e
+	return k.entity
 }

entities/identity/key_test.go ๐Ÿ”—

@@ -4,14 +4,40 @@ import (
 	"crypto/rsa"
 	"encoding/json"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/git-bug/git-bug/entity"
 	"github.com/git-bug/git-bug/repository"
+	"github.com/git-bug/git-bug/util/lamport"
+	"github.com/git-bug/git-bug/util/timestamp"
 )
 
+type mockIdentity struct {
+	name  string
+	login string
+	email string
+}
+
+func (m *mockIdentity) Name() string                                        { return m.name }
+func (m *mockIdentity) Login() string                                      { return m.login }
+func (m *mockIdentity) Email() string                                     { return m.email }
+func (m *mockIdentity) DisplayName() string                               { return m.name }
+func (m *mockIdentity) AvatarUrl() string                                 { return "" }
+func (m *mockIdentity) Keys() []*Key                                      { return nil }
+func (m *mockIdentity) SigningKey(repo repository.RepoKeyring) (*Key, error) { return nil, nil }
+func (m *mockIdentity) ValidKeysAtTime(clockName string, time lamport.Time) []*Key { return nil }
+func (m *mockIdentity) LastModification() timestamp.Timestamp             { return 0 }
+func (m *mockIdentity) LastModificationLamports() map[string]lamport.Time { return nil }
+func (m *mockIdentity) IsProtected() bool                                 { return false }
+func (m *mockIdentity) Validate() error                                   { return nil }
+func (m *mockIdentity) NeedCommit() bool                                  { return false }
+func (m *mockIdentity) Id() entity.Id                                     { return "" }
+
 func TestPublicKeyJSON(t *testing.T) {
-	k := generatePublicKey()
+	id := &mockIdentity{name: "John Smith", email: "jsmith@example.com"}
+	k := generatePublicKey(id)
 
 	dataJSON, err := json.Marshal(k)
 	require.NoError(t, err)
@@ -20,14 +46,16 @@ func TestPublicKeyJSON(t *testing.T) {
 	err = json.Unmarshal(dataJSON, &read)
 	require.NoError(t, err)
 
-	require.Equal(t, k, &read)
+	// Compare public keys since entities may differ in internal structure after deserialization
+	require.Equal(t, k.public.Fingerprint[:], read.public.Fingerprint[:])
 }
 
 func TestStoreLoad(t *testing.T) {
 	repo := repository.NewMockRepoKeyring()
 
 	// public + private
-	k := GenerateKey()
+	id := &mockIdentity{name: "John Smith", email: "jsmith@example.com"}
+	k := GenerateKey(id, WithTime(time.Time{}))
 
 	// Store
 
@@ -48,11 +76,11 @@ func TestStoreLoad(t *testing.T) {
 
 	require.Equal(t, k.public, read.public)
 
-	require.IsType(t, (*rsa.PrivateKey)(nil), k.private.PrivateKey)
+	require.IsType(t, (*rsa.PrivateKey)(nil), k.entity.PrivateKey.PrivateKey)
 
 	// See https://github.com/golang/crypto/pull/175
-	rsaPriv := read.private.PrivateKey.(*rsa.PrivateKey)
+	rsaPriv := read.entity.PrivateKey.PrivateKey.(*rsa.PrivateKey)
 	rsaPriv.Primes[0], rsaPriv.Primes[1] = rsaPriv.Primes[1], rsaPriv.Primes[0]
 
-	require.True(t, k.private.PrivateKey.(*rsa.PrivateKey).Equal(read.private.PrivateKey))
+	require.True(t, k.entity.PrivateKey.PrivateKey.(*rsa.PrivateKey).Equal(read.entity.PrivateKey.PrivateKey))
 }

entities/identity/version_test.go ๐Ÿ”—

@@ -11,8 +11,30 @@ import (
 	"github.com/git-bug/git-bug/entity"
 	"github.com/git-bug/git-bug/repository"
 	"github.com/git-bug/git-bug/util/lamport"
+	"github.com/git-bug/git-bug/util/timestamp"
 )
 
+type testVersionMockIdentity struct {
+	name  string
+	login string
+	email string
+}
+
+func (m *testVersionMockIdentity) Name() string                                        { return m.name }
+func (m *testVersionMockIdentity) Login() string                                      { return m.login }
+func (m *testVersionMockIdentity) Email() string                                     { return m.email }
+func (m *testVersionMockIdentity) DisplayName() string                               { return m.name }
+func (m *testVersionMockIdentity) AvatarUrl() string                                 { return "" }
+func (m *testVersionMockIdentity) Keys() []*Key                                      { return nil }
+func (m *testVersionMockIdentity) SigningKey(repo repository.RepoKeyring) (*Key, error) { return nil, nil }
+func (m *testVersionMockIdentity) ValidKeysAtTime(clockName string, time lamport.Time) []*Key { return nil }
+func (m *testVersionMockIdentity) LastModification() timestamp.Timestamp             { return 0 }
+func (m *testVersionMockIdentity) LastModificationLamports() map[string]lamport.Time { return nil }
+func (m *testVersionMockIdentity) IsProtected() bool                                 { return false }
+func (m *testVersionMockIdentity) Validate() error                                   { return nil }
+func (m *testVersionMockIdentity) NeedCommit() bool                                  { return false }
+func (m *testVersionMockIdentity) Id() entity.Id                                     { return "" }
+
 func makeIdentityTestRepo(t *testing.T) repository.ClockedRepo {
 	repo := repository.NewMockRepo()
 
@@ -32,9 +54,10 @@ func makeIdentityTestRepo(t *testing.T) repository.ClockedRepo {
 func TestVersionJSON(t *testing.T) {
 	repo := makeIdentityTestRepo(t)
 
+	testIdentity := &testVersionMockIdentity{name: "name", email: "email", login: "login"}
 	keys := []*Key{
-		generatePublicKey(),
-		generatePublicKey(),
+		generatePublicKey(testIdentity),
+		generatePublicKey(testIdentity),
 	}
 
 	before, err := newVersion(repo, "name", "email", "login", "avatarUrl", keys)
@@ -74,5 +97,18 @@ func TestVersionJSON(t *testing.T) {
 	// make sure we now have an Id
 	expected.Id()
 
-	assert.Equal(t, expected, &after)
+	// Compare versions without Key objects since entities may differ in internal structure after deserialization
+	assert.Equal(t, expected.name, after.name)
+	assert.Equal(t, expected.email, after.email)
+	assert.Equal(t, expected.login, after.login)
+	assert.Equal(t, expected.avatarURL, after.avatarURL)
+	assert.Equal(t, expected.unixTime, after.unixTime)
+	assert.Equal(t, expected.times, after.times)
+	assert.Equal(t, expected.metadata, after.metadata)
+	assert.Equal(t, expected.id, after.id)
+	assert.Equal(t, expected.commitHash, after.commitHash)
+	assert.Equal(t, len(expected.keys), len(after.keys))
+	for i, key := range expected.keys {
+		assert.Equal(t, key.public.Fingerprint[:], after.keys[i].public.Fingerprint[:])
+	}
 }

entity/dag/operation_pack_test.go ๐Ÿ”—

@@ -52,7 +52,7 @@ func TestOperationPackSignedReadWrite(t *testing.T) {
 		repo, author, _, resolver, def := maker()
 
 		err := author.(*identity.Identity).Mutate(repo, func(orig *identity.Mutator) {
-			orig.Keys = append(orig.Keys, identity.GenerateKey())
+			orig.Keys = append(orig.Keys, identity.GenerateKey(author))
 		})
 		require.NoError(t, err)