mediocregopher/dehub
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Refactor how commit authorship is formatted

Browse Source 
---
type: change
message: |-
  Refactor how commit authorship is formatted

  This ended up being a loose thread that, when pulled, untangled a bunch of other
  stuff. Notably the account argument to Repo.Commit is no longer needed, and I
  added an anon argument to TestSignifierPGP which simplified a number of tests.
change_hash: AHMeFpSJb/AoLiELW5pImUiQ+PS0PWibliqcQuFTjC3o
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl6TVu8ACgkQlcRvpqQRSKwExw/+OQa++h0ZkFuguU3593D+9P7l8bElMWu+Je40RrDHz4fD0i51UJGcnkk/9fryUWGmWpbCnDv9k7OGztipH/P2HDhuBn+/Dc7X8CcRsRC7/n+o1gd6f1Sc9q4zTgPKkE+ZrE226e8GNVgpZRspbZHa1IwSs/fIkboavnWFowV6SiLColtwYqCfCJXvEP52D+OjKvi4iatRUzoVIOYNHGI4uOufuQRYPZIQRGxgcvUUB/VhjyBB39BV5cHO8oTFmmXH6+eFj4bHWjHsRzp5ferUmsRCdvo2lkoxXkeqN0okyUcwpXQXI7l6BL9OyCxHifIK9G2BaOAsp7A6piwNzaUGk1RIHZpJ69dTfTre1jolOhkGY9lXGAMdSo+ifsFqKj3sXZNjSEJ49riYP98ERnhF1APHN+xL1dkUd8eTTMRh9+C8Bi7twWkUJ2wH5CL1brkpkHIwXOa7jszdeliMK9aZRT7lyxvjCx0uVFTeXbq0RSRb9Oeo+TJhRIu7kLpMKmzX9y/fRaGiPcjr8OD2cfWhACsaVGuU+oXmJXk4uJ+ADfm4IZy7IOEQdr+3Cg33y2mxRq2APwLaGjvA6UFLfar1/nAKK+uTQDF3DssHdLgEfsH2Lu5orc3+FtAblhBiwrN5a732hLceEMkUvQXwOHbZqddkbuqQ6FqHMDIsvBdK6gI=
  account: mediocregopher
This commit is contained in:
mediocregopher 2020-04-12 11:59:23 -06:00
parent 43b564e711
commit 3d89fe5fd9
12 changed files with 128 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cmd/dehub/cmd_commit.go
View File

@ -61,7 +61,7 @@ func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) {
return fmt.Errorf("accrediting commit: %w", err)
}
gitCommit, err := repo.Commit(commit, *accountID)
gitCommit, err := repo.Commit(commit)
if err != nil {
return fmt.Errorf("committing to git: %w", err)
}

21
commit.go
View File

@ -48,14 +48,18 @@ type CommitCommon struct {
Credentials []sigcred.Credential `yaml:"credentials"`
}
func (cc CommitCommon) credAccountIDs() []string {
func (cc CommitCommon) credIDs() []string {
m := map[string]struct{}{}
for _, cred := range cc.Credentials {
m[cred.AccountID] = struct{}{}
if cred.AccountID != "" {
m[cred.AccountID] = struct{}{}
} else if cred.AnonID != "" {
m[cred.AnonID] = struct{}{}
}
}
s := make([]string, 0, len(m))
for accountID := range m {
s = append(s, accountID)
for id := range m {
s = append(s, id)
}
sort.Strings(s)
return s
@ -222,10 +226,9 @@ func (r *Repo) CommitBare(params CommitBareParams) (GitCommit, error) {
return r.GetGitCommit(commitHash)
}
// Commit uses the given Commit to create a git commit object (with the
// specified accountID as the author) and commits it to the current HEAD,
// returning the full GitCommit.
func (r *Repo) Commit(commit Commit, accountID string) (GitCommit, error) {
// Commit uses the given Commit to create a git commit object and commits it to
// the current HEAD, returning the full GitCommit.
func (r *Repo) Commit(commit Commit) (GitCommit, error) {
headRef, err := r.TraverseReferenceChain(plumbing.HEAD, func(ref *plumbing.Reference) bool {
return ref.Type() == plumbing.HashReference
})
@ -248,7 +251,7 @@ func (r *Repo) Commit(commit Commit, accountID string) (GitCommit, error) {
gitCommit, err := r.CommitBare(CommitBareParams{
Commit: commit,
Author: accountID,
Author: strings.Join(commit.Common.credIDs(), ", "),
ParentHash: headHash,
GitTree: stagedTree,
})

13
commit_change_test.go
View File

@ -78,9 +78,8 @@ func TestChangeCommitVerify(t *testing.T) {
h := newHarness(t)
for _, step := range test.steps {
h.stage(step.tree)
account := h.cfg.Accounts[0]
gitCommit := h.changeCommit(step.msg, account.ID, h.sig)
gitCommit := h.changeCommit(step.msg, h.sig)
if step.msgHead == "" {
step.msgHead = strings.TrimSpace(step.msg) + "\n\n"
}
@ -106,11 +105,11 @@ func TestCombineCommitChanges(t *testing.T) {
h := newHarness(t)
// commit initial config, so the root user can modify it in the next commit
h.changeCommit("initial commit", h.cfg.Accounts[0].ID, h.sig)
h.changeCommit("initial commit", h.sig)
// add a toot user and modify the access controls such that both accounts
// are required for the main branch
tootSig, tootPubKeyBody := sigcred.TestSignifierPGP("toot", h.rand)
tootSig, tootPubKeyBody := sigcred.TestSignifierPGP("toot", false, h.rand)
h.cfg.Accounts = append(h.cfg.Accounts, Account{
ID: "toot",
Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
@ -145,21 +144,21 @@ func TestCombineCommitChanges(t *testing.T) {
}
h.stageCfg()
tootCommit := h.changeCommit("add toot", h.cfg.Accounts[0].ID, h.sig)
tootCommit := h.changeCommit("add toot", h.sig)
// make a single change commit in another branch using root. Then add a
// credential using toot, and combine them onto main.
otherBranch := plumbing.NewBranchReferenceName("other")
h.checkout(otherBranch)
h.stage(map[string]string{"foo": "bar"})
fooCommit := h.changeCommit("add foo file", h.cfg.Accounts[0].ID, h.sig)
fooCommit := h.changeCommit("add foo file", h.sig)
// now adding a credential commit from toot should work
credCommitObj, err := h.repo.NewCommitCredential(fooCommit.Interface.GetHash())
if err != nil {
t.Fatal(err)
}
credCommit := h.tryCommit(true, credCommitObj, h.cfg.Accounts[1].ID, tootSig)
credCommit := h.tryCommit(true, credCommitObj, tootSig)
allCommits, err := h.repo.GetGitCommitRange(
tootCommit.GitCommit.Hash,

7
commit_comment.go
View File

@ -1,10 +1,11 @@
package dehub
import (
"dehub.dev/src/dehub.git/yamlutil"
"fmt"
"strings"
"dehub.dev/src/dehub.git/yamlutil"
"gopkg.in/src-d/go-git.v4/plumbing/object"
)
@ -31,8 +32,8 @@ func (r *Repo) NewCommitComment(msg string) (Commit, error) {
// MessageHead implements the method for the CommitInterface interface.
func (cc CommitComment) MessageHead(common CommitCommon) (string, error) {
msgAbbrev := abbrevCommitMessage(cc.Message)
credAccounts := strings.Join(common.credAccountIDs(), ", ")
return fmt.Sprintf("Comment by %s: %s", credAccounts, msgAbbrev), nil
credIDs := strings.Join(common.credIDs(), ", ")
return fmt.Sprintf("Comment by %s: %s", credIDs, msgAbbrev), nil
}
// Hash implements the method for the CommitInterface.

7
commit_credential.go
View File

@ -1,11 +1,12 @@
package dehub
import (
"dehub.dev/src/dehub.git/yamlutil"
"encoding/base64"
"fmt"
"strings"
"dehub.dev/src/dehub.git/yamlutil"
"gopkg.in/src-d/go-git.v4/plumbing/object"
)
@ -63,8 +64,8 @@ func (cc CommitCredential) MessageHead(common CommitCommon) (string, error) {
hash64 = hash64[:6] + "..."
}
credAccounts := strings.Join(common.credAccountIDs(), ", ")
return fmt.Sprintf("Credential of hash %s by %s", hash64, credAccounts), nil
credIDs := strings.Join(common.credIDs(), ", ")
return fmt.Sprintf("Credential of hash %s by %s", hash64, credIDs), nil
}
// Hash implements the method for the CommitInterface.

11
commit_credential_test.go
View File

@ -1,9 +1,10 @@
package dehub
import (
"dehub.dev/src/dehub.git/sigcred"
"testing"
"dehub.dev/src/dehub.git/sigcred"
"gopkg.in/src-d/go-git.v4/plumbing"
yaml "gopkg.in/yaml.v2"
)
@ -13,7 +14,7 @@ func TestCredentialCommitVerify(t *testing.T) {
// create a new account and modify the config so that that account is only
// allowed to add verifications to a single branch
tootSig, tootPubKeyBody := sigcred.TestSignifierPGP("toot", h.rand)
tootSig, tootPubKeyBody := sigcred.TestSignifierPGP("toot", false, h.rand)
h.cfg.Accounts = append(h.cfg.Accounts, Account{
ID: "toot",
Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
@ -47,7 +48,7 @@ func TestCredentialCommitVerify(t *testing.T) {
t.Fatal(err)
}
h.stageCfg()
rootGitCommit := h.changeCommit("initial commit", h.cfg.Accounts[0].ID, h.sig)
rootGitCommit := h.changeCommit("initial commit", h.sig)
// toot user wants to create a credential commit for the root commit, for
// whatever reason.
@ -57,9 +58,9 @@ func TestCredentialCommitVerify(t *testing.T) {
t.Fatalf("creating credential commit for hash %x: %v", rootChangeHash, err)
}
h.tryCommit(false, credCommit, "toot", tootSig)
h.tryCommit(false, credCommit, tootSig)
// toot tries again in their own branch, and should be allowed.
h.checkout(tootBranch)
h.tryCommit(true, credCommit, "toot", tootSig)
h.tryCommit(true, credCommit, tootSig)
}

30
commit_test.go
View File

@ -16,12 +16,12 @@ func TestConfigChange(t *testing.T) {
// commit the initial staged changes, which merely include the config and
// public key
gitCommit := h.changeCommit("commit configuration", h.cfg.Accounts[0].ID, h.sig)
gitCommit := h.changeCommit("commit configuration", h.sig)
gitCommits = append(gitCommits, gitCommit)
// create a new account and add it to the configuration. That commit should
// not be verifiable, though
newSig, newPubKeyBody := sigcred.TestSignifierPGP("toot", h.rand)
newSig, newPubKeyBody := sigcred.TestSignifierPGP("toot", false, h.rand)
h.cfg.Accounts = append(h.cfg.Accounts, Account{
ID: "toot",
Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
@ -34,16 +34,16 @@ func TestConfigChange(t *testing.T) {
if err != nil {
t.Fatalf("creating CommitChange: %v", err)
}
h.tryCommit(false, badCommit, h.cfg.Accounts[1].ID, newSig)
h.tryCommit(false, badCommit, newSig)
// now add with the root user, this should work.
h.stageCfg()
gitCommit = h.changeCommit("add toot user", h.cfg.Accounts[0].ID, h.sig)
gitCommit = h.changeCommit("add toot user", h.sig)
gitCommits = append(gitCommits, gitCommit)
// _now_ the toot user should be able to do things.
h.stage(map[string]string{"foo/bar": "what a cool file"})
gitCommit = h.changeCommit("add a cool file", h.cfg.Accounts[1].ID, newSig)
gitCommit = h.changeCommit("add a cool file", newSig)
gitCommits = append(gitCommits, gitCommit)
if err := h.repo.VerifyCommits(MainRefName, gitCommits); err != nil {
@ -63,13 +63,13 @@ func TestMainAncestryRequirement(t *testing.T) {
if err != nil {
t.Fatalf("creating CommitChange: %v", err)
}
h.tryCommit(false, badCommit, h.cfg.Accounts[0].ID, h.sig)
h.tryCommit(false, badCommit, h.sig)
})
t.Run("new branch, single commit", func(t *testing.T) {
h := newHarness(t)
h.stageCfg()
h.changeCommit("add cfg", h.cfg.Accounts[0].ID, h.sig)
h.changeCommit("add cfg", h.sig)
// set HEAD to this other branch which doesn't really exist
ref := plumbing.NewSymbolicReference(plumbing.HEAD, otherBranch)
@ -82,13 +82,13 @@ func TestMainAncestryRequirement(t *testing.T) {
if err != nil {
t.Fatalf("creating CommitChange: %v", err)
}
h.tryCommit(false, badCommit, h.cfg.Accounts[0].ID, h.sig)
h.tryCommit(false, badCommit, h.sig)
})
}
func TestAnonymousCommits(t *testing.T) {
h := newHarness(t)
anonSig, anonPubKeyBody := sigcred.TestSignifierPGP("", h.rand)
anonSig, _ := sigcred.TestSignifierPGP("anon", true, h.rand)
h.cfg.AccessControls = []accessctl.AccessControl{{
Action: accessctl.ActionAllow,
@ -97,15 +97,5 @@ func TestAnonymousCommits(t *testing.T) {
},
}}
h.stageCfg()
// manually accredit the commit this time
goodCommit, err := h.repo.NewCommitChange("this will work")
if err != nil {
t.Fatalf("creating CommitChange: %v", err)
} else if goodCommit, err = h.repo.AccreditCommit(goodCommit, anonSig); err != nil {
t.Fatalf("accreditting CommitChange: %v", err)
}
// There is, unfortunately, not a prettier way to do this
goodCommit.Common.Credentials[0].PGPSignature.PubKeyBody = string(anonPubKeyBody)
h.tryCommit(true, goodCommit, "", nil)
h.changeCommit("this will work", anonSig)
}

21
repo_test.go
View File

@ -26,7 +26,7 @@ type harness struct {
func newHarness(t *testing.T) *harness {
rand := rand.New(rand.NewSource(0xb4eadb01))
sig, pubKeyBody := sigcred.TestSignifierPGP("root", rand)
sig, pubKeyBody := sigcred.TestSignifierPGP("root", false, rand)
pubKeyPath := filepath.Join(DehubDir, "root.asc")
cfg := &Config{
@ -158,7 +158,7 @@ func (h *harness) reset(to plumbing.Hash, mode git.ResetMode) {
func (h *harness) tryCommit(
shouldSucceed bool,
commit Commit,
accountID string, accountSig sigcred.SignifierInterface,
accountSig sigcred.SignifierInterface,
) GitCommit {
if accountSig != nil {
var err error
@ -167,7 +167,7 @@ func (h *harness) tryCommit(
}
}
gitCommit, err := h.repo.Commit(commit, accountID)
gitCommit, err := h.repo.Commit(commit)
if err != nil {
h.t.Fatalf("failed to commit ChangeCommit: %v", err)
}
@ -197,14 +197,13 @@ func (h *harness) tryCommit(
func (h *harness) changeCommit(
msg string,
accountID string,
sig sigcred.SignifierInterface,
) GitCommit {
commit, err := h.repo.NewCommitChange(msg)
if err != nil {
h.t.Fatalf("creating ChangeCommit: %v", err)
}
return h.tryCommit(true, commit, accountID, sig)
return h.tryCommit(true, commit, sig)
}
func TestHasStagedChanges(t *testing.T) {
@ -225,12 +224,12 @@ func TestHasStagedChanges(t *testing.T) {
harness.stage(map[string]string{"foo": "bar"})
assertHasStaged(true)
harness.changeCommit("first commit", "root", harness.sig)
harness.changeCommit("first commit", harness.sig)
assertHasStaged(false)
harness.stage(map[string]string{"foo": ""}) // delete foo
assertHasStaged(true)
harness.changeCommit("second commit", "root", harness.sig)
harness.changeCommit("second commit", harness.sig)
assertHasStaged(false)
}
@ -258,7 +257,7 @@ access_controls:
`})
// this commit should be created and verify fine
harness.changeCommit("first commit, this is going great", "root", harness.sig)
harness.changeCommit("first commit, this is going great", harness.sig)
// this commit should not be verifiable, because toot isn't in accounts and
// the default access controls should be being used
@ -267,7 +266,7 @@ access_controls:
if err != nil {
t.Fatalf("creating CommitChange: %v", err)
}
harness.tryCommit(false, badCommit, "toot", nil)
harness.tryCommit(false, badCommit, nil)
// make a commit fixing the config. everything should still be fine.
harness.stage(map[string]string{ConfigPath: `
@ -278,7 +277,7 @@ accounts:
- type: pgp_public_key_file
path: ".dehub/root.asc"
`})
harness.changeCommit("Fix the config!", "root", harness.sig)
harness.changeCommit("Fix the config!", harness.sig)
}
// TestThisRepoStillVerifies opens this actual repository and ensures that all
@ -314,7 +313,7 @@ func TestShortHashResolving(t *testing.T) {
// TODO ideally this test would test the conflicting hashes are noticed, but
// that's hard...
h := newHarness(t)
hash := h.changeCommit("first commit", h.cfg.Accounts[0].ID, h.sig).GitCommit.Hash
hash := h.changeCommit("first commit", h.sig).GitCommit.Hash
hashStr := hash.String()
t.Log(hashStr)

12
sigcred/credential.go
View File

@ -14,10 +14,16 @@ type Credential struct {
// AccountID specifies the account which generated this Credential.
//
// NOTE that the Credentials produced by the Signifier.Sign method do not
// fill this field in, and it may be empty in cases where a non-account user
// has added a credential to a commit.
// NOTE that Credentials produced by the direct implementations of
// SignifierInterface won't fill in this field, unless specifically
// documented. The SignifierInterface produced by the Interface() method of
// Signifier _will_ fill this field in, however.
AccountID string `yaml:"account,omitempty"`
// AnonID specifies an identifier for the anonymous user which produced this
// credential. This field is mutually exclusive with AccountID, and won't be
// set by any SignifierInterface unless specifically documented.
AnonID string `yaml:"-"`
}
// MarshalYAML implements the yaml.Marshaler interface.

8
sigcred/credential_test.go
View File

@ -20,7 +20,7 @@ func TestSelfVerifyingCredentials(t *testing.T) {
{
descr: "pgp sig no body",
mkCred: func(toSign []byte) (Credential, error) {
privKey, _ := TestSignifierPGP("", rand)
privKey, _ := TestSignifierPGP("", false, rand)
return privKey.Sign(nil, toSign)
},
expErr: true,
@ -28,10 +28,8 @@ func TestSelfVerifyingCredentials(t *testing.T) {
{
descr: "pgp sig with body",
mkCred: func(toSign []byte) (Credential, error) {
privKey, pubKeyBody := TestSignifierPGP("", rand)
cred, err := privKey.Sign(nil, toSign)
cred.PGPSignature.PubKeyBody = string(pubKeyBody)
return cred, err
privKey, _ := TestSignifierPGP("", true, rand)
return privKey.Sign(nil, toSign)
},
},
}

80
sigcred/pgp.go
View File

@ -122,22 +122,57 @@ func (s pgpKey) MarshalBinary() ([]byte, error) {
body := new(bytes.Buffer)
armorEncoder, err := armor.Encode(body, "PGP PUBLIC KEY", nil)
if err != nil {
return nil, fmt.Errorf("error initializing armor encoder: %w", err)
return nil, fmt.Errorf("initializing armor encoder: %w", err)
} else if err := s.entity.Serialize(armorEncoder); err != nil {
return nil, fmt.Errorf("error encoding public key: %w", err)
return nil, fmt.Errorf("encoding public key: %w", err)
} else if err := armorEncoder.Close(); err != nil {
return nil, fmt.Errorf("error closing armor encoder: %w", err)
return nil, fmt.Errorf("closing armor encoder: %w", err)
}
return body.Bytes(), nil
}
func (s pgpKey) userID() (*packet.UserId, error) {
if l := len(s.entity.Identities); l == 0 {
return nil, errors.New("pgp key has no identity information")
} else if l > 1 {
return nil, errors.New("multiple identities on a single pgp key is unsupported")
}
var identity *openpgp.Identity
for _, identity = range s.entity.Identities {
break
}
return identity.UserId, nil
}
func anonPGPSignifier(pgpKey pgpKey, sigInt SignifierInterface) (SignifierInterface, error) {
keyID := pgpKey.entity.PrimaryKey.KeyIdString()
userID, err := pgpKey.userID()
if err != nil {
return nil, err
}
pubKeyBody, err := pgpKey.MarshalBinary()
if err != nil {
return nil, err
}
return signifierMiddleware{
SignifierInterface: sigInt,
signCallback: func(cred *Credential) {
cred.PGPSignature.PubKeyBody = string(pubKeyBody)
cred.AnonID = fmt.Sprintf("%s %q", keyID, userID.Email)
},
}, nil
}
// TestSignifierPGP returns a direct implementation of the SignifierInterface
// which uses a random private key generated in memory, as well as an armored
// version of its public key.
//
// NOTE that the key returned is very weak, and should only be used for tests.
func TestSignifierPGP(accountID string, randReader io.Reader) (SignifierInterface, []byte) {
entity, err := openpgp.NewEntity(accountID, "", accountID+"@example.com", &packet.Config{
func TestSignifierPGP(name string, anon bool, randReader io.Reader) (SignifierInterface, []byte) {
entity, err := openpgp.NewEntity(name, "", name+"@example.com", &packet.Config{
Rand: randReader,
RSABits: 512,
})
@ -151,7 +186,14 @@ func TestSignifierPGP(accountID string, randReader io.Reader) (SignifierInterfac
panic(err)
}
return accountSignifier(accountID, pgpKey), pubKeyBody
if anon {
sigInt, err := anonPGPSignifier(pgpKey, pgpKey)
if err != nil {
panic(err)
}
return sigInt, pubKeyBody
}
return accountSignifier(name, pgpKey), pubKeyBody
}
// SignifierPGP describes a pgp public key whose corresponding private key will
@ -185,23 +227,25 @@ func cmdGPG(stdin []byte, args ...string) ([]byte, error) {
// LoadSignifierPGP loads a pgp key using the given identifier. The key is
// assumed to be stored in the client's keyring already.
//
// If setPubKeyBody is true, then CredentialPGPSignature instances produced by
// the returned Signifier will have their PubKeyBody field set.
func LoadSignifierPGP(keyID string, setPubKeyBody bool) (SignifierInterface, error) {
// If this is being called for an anonymous user to use, then anon can be set to
// true. This will have the effect of setting the PubKeyBody and AnonID of all
// produced Credentials.
func LoadSignifierPGP(keyID string, anon bool) (SignifierInterface, error) {
pubKey, err := cmdGPG(nil, "-a", "--export", keyID)
if err != nil {
return nil, fmt.Errorf("loading public key: %w", err)
}
var sigInt SignifierInterface = &SignifierPGP{Body: string(pubKey)}
if setPubKeyBody {
sigInt = signifierMiddleware{
SignifierInterface: sigInt,
signCallback: func(cred *Credential) {
cred.PGPSignature.PubKeyBody = string(pubKey)
},
}
sig := &SignifierPGP{Body: string(pubKey)}
if !anon {
return sig, nil
}
return sigInt, nil
pgpKey, err := sig.load(nil)
if err != nil {
return nil, err
}
return anonPGPSignifier(pgpKey, sig)
}
func (s SignifierPGP) load(fs fs.FS) (pgpKey, error) {

2
sigcred/pgp_test.go
View File

@ -38,7 +38,7 @@ func TestPGPVerification(t *testing.T) {
seed := time.Now().UnixNano()
t.Logf("seed: %d", seed)
rand := rand.New(rand.NewSource(seed))
privKey, pubKeyBody := TestSignifierPGP("", rand)
privKey, pubKeyBody := TestSignifierPGP("", false, rand)
sig, fs := test.init(pubKeyBody)
data := make([]byte, rand.Intn(1024))