diff --git a/entities/identity/identity_test.go b/entities/identity/identity_test.go index ab6a1e2321e0c5efcd95d9bca5a9b56ea7f76261..44bebc1122b02d8f97734e8b57332dec25720bf1 100644 --- a/entities/identity/identity_test.go +++ b/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{ diff --git a/entities/identity/key.go b/entities/identity/key.go index 543fade2dbcf449dbadd443bc5b956a389cb92e6..d6d9e11a999d4ed9d7add4d2cc1c6ea5a15d2dd7 100644 --- a/entities/identity/key.go +++ b/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 } diff --git a/entities/identity/key_test.go b/entities/identity/key_test.go index f94ca92a225c17330a701aba1963a4508c8790f1..09cbc3651fe0b3ba001ead56276d90f96b75f35b 100644 --- a/entities/identity/key_test.go +++ b/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)) } diff --git a/entities/identity/version_test.go b/entities/identity/version_test.go index cf677ce17221fcef339fe2a5c3f9ab964de40693..24559ae6e89fc06546ffcbffa7535ecfab5e458f 100644 --- a/entities/identity/version_test.go +++ b/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[:]) + } } diff --git a/entity/dag/operation_pack_test.go b/entity/dag/operation_pack_test.go index 34470386520c9fc03838c91c26a4799206424dcf..357171a2eadfe44982623d31dbea2110a5cca249 100644 --- a/entity/dag/operation_pack_test.go +++ b/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)