implement the ability for users without an account to still submit accredited co...

---
type: change
message: implement the ability for users without an account to still submit accredited
  commits
change_hash: AIcRB2u380KAM345cCdexq3RzhDeEuDNT9gwcGDIj8xp
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl6STloACgkQlcRvpqQRSKwMvg//c0JOUTlXNpDA8VSSUdPwHPrUJix223eMOi+SzP4RlCkEh8bHT24D3P1bpMacjgNpcmHGshAwtZkboTbvHNnqVu7HvOM2Vb/lrtkod/0sD4NsO2+GjJBNJmBtv9rEEz9wBGKKhjgZc5+h3d7UlmWHiLO++v2RnjEYd13Fj4/fOHnpAWrXVodtSEFVxUGup980Ug3uvC/vc8+a9w5llafqBMAnastQ/DyulPQeMTE2lxfyGvK1EhQSxbSokO61rgNssPFyfsmheA7T1FIrOzkXqNhIFsfRB4dAOBhhtoqRFEoJ755jK4XSlE5Y8klFuRVIfdnyBlrbia+Pc8u9KMoIBk0hDP+niFqUn9lEZS5D6X7PW/8DsaHa0rlHic9IB7nE563Fm1QVd5GSj7t3/0vPBetxdmXshLuTWtq+gSEJBH2DlC7AHw6gZkSr0w4d2HJlDivfP/cWuyrp0PrOAnEuKCRpnD+EBV5+wa+QlYYIAuTLMwF+/aT/G1VCtCSFkE5JWzZVw6J2oVq25deLpe1TMQ8dHevSlPx/UcofZasO7uFHLc3xDyC8ceK+pGuvRA2SSOIGo7+qR1xh2EhmQ2RZO1AN0NB4NYHQixYqWERen8SDe1jsSy6ercKTE5T/jJeHVPIOm1nutdP+D5gjQGU0JzcNoG/luJv02MWoD7J7RWU=
  account: mediocregopher

ROADMAP.md

@@ -11,9 +11,6 @@ to accept help from people asking to help.
 
 * Fast-forward perms on branches (so they can be deleted)
 * Figure out commit range syntax, use that everywhere.
-* Ability to specify a pgp key manually, even if it's not in the project.
-* Ability to require _any_ signature on a commit, even if it's not in the
-  config.
 * Create a branch which is just a public "welcome thread", which can be part of
   the tutorials.
 * Tutorials

accessctl/access_control.go

@@ -3,10 +3,11 @@
 package accessctl
 
 import (
-	"dehub.dev/src/dehub.git/sigcred"
 	"errors"
 	"fmt"
 
+	"dehub.dev/src/dehub.git/sigcred"
+
 	yaml "gopkg.in/yaml.v2"
 )
 

accessctl/filter_sig.go

@@ -11,17 +11,21 @@ import (
 // FilterSignature represents the configuration of a Filter which requires one
 // or more signature credentials to be present on a commit.
 //
-// Either AccountIDs or AnyAccount must be filled in.
+// Either AccountIDs, AnyAccount, or Any must be filled in; all are mutually
+// exclusive.
 type FilterSignature struct {
 	AccountIDs []string `yaml:"account_ids,omitempty"`
+	Any        bool     `yaml:"any,omitempty"`
 	AnyAccount bool     `yaml:"any_account,omitempty"`
-	Count      string   `yaml:"count"`
+	Count      string   `yaml:"count,omitempty"`
 }
 
 var _ FilterInterface = FilterSignature{}
 
 func (f FilterSignature) targetNum() (int, error) {
-	if !strings.HasSuffix(f.Count, "%") {
+	if f.Count == "" {
+		return 1, nil
+	} else if !strings.HasSuffix(f.Count, "%") {
 		return strconv.Atoi(f.Count)
 	} else if f.AnyAccount {
 		return 0, errors.New("cannot use AnyAccount and a percent Count together")
@@ -56,17 +60,29 @@ func (f FilterSignature) MatchCommit(req CommitRequest) error {
 		return fmt.Errorf("computing target number of accounts: %w", err)
 	}
 
+	var numSigs int
 	credAccountIDs := map[string]struct{}{}
 	for _, cred := range req.Credentials {
 		// TODO support other kinds of signatures
 		if cred.PGPSignature == nil {
 			continue
 		}
-		credAccountIDs[cred.AccountID] = struct{}{}
+		numSigs++
+		if cred.AccountID != "" {
+			credAccountIDs[cred.AccountID] = struct{}{}
+		}
+	}
+
+	if numSigs == 0 {
+		return ErrFilterNoMatch{
+			Err: ErrFilterSignatureUnsatisfied{TargetNumAccounts: targetN},
+		}
 	}
 
 	var n int
-	if f.AnyAccount {
+	if f.Any {
+		return nil
+	} else if f.AnyAccount {
 		// TODO this doesn't actually check that the accounts are defined in the
 		// Config. It works for now as long as the Credentials are valid, since
 		// only an Account defined in the Config could create a valid

accessctl/filter_sig_test.go

@@ -1,8 +1,9 @@
 package accessctl
 
 import (
-	"dehub.dev/src/dehub.git/sigcred"
 	"testing"
+
+	"dehub.dev/src/dehub.git/sigcred"
 )
 
 func TestFilterSignature(t *testing.T) {
@@ -99,5 +100,25 @@ func TestFilterSignature(t *testing.T) {
 				NumAccounts:       1,
 			},
 		},
+		{
+			descr: "any sig at all",
+			filter: FilterSignature{
+				Any: true,
+			},
+			req: CommitRequest{
+				Credentials: []sigcred.Credential{
+					{PGPSignature: new(sigcred.CredentialPGPSignature)},
+				},
+			},
+			match: true,
+		},
+		{
+			descr:  "not any sig at all",
+			filter: FilterSignature{Any: true},
+			req:    CommitRequest{},
+			matchErr: ErrFilterSignatureUnsatisfied{
+				TargetNumAccounts: 1,
+			},
+		},
 	})
 }

cmd/dehub/cmd_commit.go

@@ -7,44 +7,57 @@ import (
 
 	"dehub.dev/src/dehub.git"
 	"dehub.dev/src/dehub.git/cmd/dehub/dcmd"
+	"dehub.dev/src/dehub.git/sigcred"
 
 	"gopkg.in/src-d/go-git.v4/plumbing"
 )
 
 func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) {
 	flag := cmd.FlagSet()
-	accountID := flag.String("account-id", "", "Account to accredit commit with")
+	accountID := flag.String("as", "", "Account to accredit commit with")
+	pgpKeyID := flag.String("anon-pgp-key", "", "ID of pgp key to sign with instead of using an account")
 
 	var repo repo
 	repo.initFlags(flag)
 
 	accreditAndCommit := func(commit dehub.Commit) error {
 
-		cfg, err := repo.LoadConfig()
+		var sigInt sigcred.SignifierInterface
-		if err != nil {
+		if *accountID != "" {
-			return err
+			cfg, err := repo.LoadConfig()
-		}
+			if err != nil {
+				return err
+			}
 
-		var account dehub.Account
+			var account dehub.Account
-		var ok bool
+			var ok bool
-		for _, account = range cfg.Accounts {
+			for _, account = range cfg.Accounts {
-			if account.ID == *accountID {
+				if account.ID == *accountID {
-				ok = true
+					ok = true
-				break
+					break
+				}
+			}
+			if !ok {
+				return fmt.Errorf("account ID %q not found in config", *accountID)
+			} else if l := len(account.Signifiers); l == 0 || l > 1 {
+				return fmt.Errorf("account %q has %d signifiers, only one is supported right now", *accountID, l)
+			}
+
+			sig := account.Signifiers[0]
+			sigInt, err = sig.Interface(*accountID)
+			if err != nil {
+				return fmt.Errorf("casting %#v to SignifierInterface: %w", sig, err)
+
+			}
+		} else {
+			var err error
+			if sigInt, err = sigcred.SignifierPGPFromKeyID(*pgpKeyID, true); err != nil {
+				return fmt.Errorf("loading pgp key %q: %w", *pgpKeyID, err)
 			}
 		}
-		if !ok {
-			return fmt.Errorf("account ID %q not found in config", *accountID)
-		} else if l := len(account.Signifiers); l == 0 || l > 1 {
-			return fmt.Errorf("account %q has %d signifiers, only one is supported right now", *accountID, l)
-		}
 
-		sig := account.Signifiers[0]
+		commit, err := repo.AccreditCommit(commit, sigInt)
-		sigInt, err := sig.Interface(*accountID)
 		if err != nil {
-			return fmt.Errorf("casting %#v to SignifierInterface: %w", sig, err)
-
-		} else if commit, err = repo.AccreditCommit(commit, sigInt); err != nil {
 			return fmt.Errorf("accrediting commit: %w", err)
 		}
 
@@ -59,8 +72,8 @@ func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) {
 
 	var hasStaged bool
 	body := func() (context.Context, error) {
-		if *accountID == "" {
+		if *accountID == "" && *pgpKeyID == "" {
-			return nil, errors.New("-account-id is required")
+			return nil, errors.New("-as or -anon-pgp-key is required")
 		}
 
 		if err := repo.openRepo(); err != nil {

commit.go

@@ -427,11 +427,17 @@ func (r *Repo) verifyCommit(branch plumbing.ReferenceName, gitCommit GitCommit,
 
 	// verify all credentials
 	for _, cred := range gitCommit.Commit.Common.Credentials {
-		sig, err := r.signifierForCredential(sigFS, cred)
+		if cred.AccountID == "" {
-		if err != nil {
+			if err := cred.SelfVerify(expectedCommitHash); err != nil {
-			return fmt.Errorf("finding signifier for credential %+v: %w", cred, err)
+				return fmt.Errorf("verifying credential %+v: %w", cred, err)
-		} else if err := sig.Verify(sigFS, expectedCommitHash, cred); err != nil {
+			}
-			return fmt.Errorf("verifying credential %+v: %w", cred, err)
+		} else {
+			sig, err := r.signifierForCredential(sigFS, cred)
+			if err != nil {
+				return fmt.Errorf("finding signifier for credential %+v: %w", cred, err)
+			} else if err := sig.Verify(sigFS, expectedCommitHash, cred); err != nil {
+				return fmt.Errorf("verifying credential %+v: %w", cred, err)
+			}
 		}
 	}
 

commit_change_test.go

@@ -1,11 +1,12 @@
 package dehub
 
 import (
-	"dehub.dev/src/dehub.git/sigcred"
 	"reflect"
 	"strings"
 	"testing"
 
+	"dehub.dev/src/dehub.git/sigcred"
+
 	"github.com/davecgh/go-spew/spew"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	yaml "gopkg.in/yaml.v2"

commit_test.go

@@ -3,6 +3,7 @@ package dehub
 import (
 	"testing"
 
+	"dehub.dev/src/dehub.git/accessctl"
 	"dehub.dev/src/dehub.git/sigcred"
 
 	"gopkg.in/src-d/go-git.v4/plumbing"
@@ -84,3 +85,27 @@ func TestMainAncestryRequirement(t *testing.T) {
 		h.tryCommit(false, badCommit, h.cfg.Accounts[0].ID, h.sig)
 	})
 }
+
+func TestAnonymousCommits(t *testing.T) {
+	h := newHarness(t)
+	anonSig, anonPubKeyBody := sigcred.SignifierPGPTmp("", h.rand)
+
+	h.cfg.AccessControls = []accessctl.AccessControl{{
+		Action: accessctl.ActionAllow,
+		Filters: []accessctl.Filter{
+			{Signature: &accessctl.FilterSignature{Any: true}},
+		},
+	}}
+	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)
+}

config.go

@@ -1,11 +1,12 @@
 package dehub
 
 import (
+	"errors"
+	"fmt"
+
 	"dehub.dev/src/dehub.git/accessctl"
 	"dehub.dev/src/dehub.git/fs"
 	"dehub.dev/src/dehub.git/sigcred"
-	"errors"
-	"fmt"
 
 	yaml "gopkg.in/yaml.v2"
 )

repo.go

@@ -428,7 +428,6 @@ func (r *Repo) GetGitCommitRange(start, end plumbing.Hash) ([]GitCommit, error)
 }
 
 var (
-	hashLen    = len(plumbing.ZeroHash)
 	hashStrLen = len(plumbing.ZeroHash.String())
 	errNotHex  = errors.New("not a valid hex string")
 )

sigcred/credential.go

@@ -1,6 +1,10 @@
 package sigcred
 
-import "dehub.dev/src/dehub.git/typeobj"
+import (
+	"fmt"
+
+	"dehub.dev/src/dehub.git/typeobj"
+)
 
 // Credential represents a credential which has been attached to a commit which
 // hopefully will allow it to be included in the main. Exactly one field tagged
@@ -8,10 +12,12 @@ import "dehub.dev/src/dehub.git/typeobj"
 type Credential struct {
 	PGPSignature *CredentialPGPSignature `type:"pgp_signature"`
 
-	// AccountID specifies the account which generated this Credential. The
+	// AccountID specifies the account which generated this Credential.
-	// Credentials produced by the Signifier.Sign method do not fill this field
+	//
-	// in.
+	// NOTE that the Credentials produced by the Signifier.Sign method do not
-	AccountID string `yaml:"account"`
+	// fill this field in, and it may be empty in cases where a non-account user
+	// has added a credential to a commit.
+	AccountID string `yaml:"account,omitempty"`
 }
 
 // MarshalYAML implements the yaml.Marshaler interface.
@@ -23,3 +29,44 @@ func (c Credential) MarshalYAML() (interface{}, error) {
 func (c *Credential) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	return typeobj.UnmarshalYAML(c, unmarshal)
 }
+
+// ErrNotSelfVerifying is returned from the SelfVerify method of Credential when
+// the Credential does not implement the SelfVerifyingCredential interface. It
+// may also be returned from the SelfVerify method of the
+// SelfVerifyingCredential itself, if the Credential can only self-verify under
+// certain circumstances.
+type ErrNotSelfVerifying struct {
+	// Subject is a descriptor of the value which could not be verified. It may
+	// be a type name or some other identifying piece of information.
+	Subject string
+}
+
+func (e ErrNotSelfVerifying) Error() string {
+	return fmt.Sprintf("%s cannot verify itself", e.Subject)
+}
+
+// SelfVerify will attempt to cast the Credential as a SelfVerifyingCredential,
+// and returns the result of the SelfVerify method being called on it.
+func (c Credential) SelfVerify(data []byte) error {
+	el, _, err := typeobj.Element(c)
+	if err != nil {
+		return err
+	} else if selfVerifyingCred, ok := el.(SelfVerifyingCredential); !ok {
+		return ErrNotSelfVerifying{Subject: fmt.Sprintf("Credential of type %T", el)}
+	} else if err := selfVerifyingCred.SelfVerify(data); err != nil {
+		return fmt.Errorf("self-verifying Credential of type %T: %w", el, err)
+	}
+	return nil
+}
+
+// SelfVerifyingCredential is one which is able to prove its own authenticity by
+// some means or another. It is not required for a Credential to implement this
+// interface.
+type SelfVerifyingCredential interface {
+	// SelfVerify should return nil if the Credential has successfully verified
+	// that it has accredited the given data, or an error describing why it
+	// could not do so. It may return ErrNotSelfVerifying if the Credential can
+	// only self-verify under certain circumstances, and those circumstances are
+	// not met.
+	SelfVerify(data []byte) error
+}

sigcred/credential_test.go

@@ -0,0 +1,60 @@
+package sigcred
+
+import (
+	"errors"
+	"math/rand"
+	"testing"
+	"time"
+)
+
+func TestSelfVerifyingCredentials(t *testing.T) {
+	seed := time.Now().UnixNano()
+	t.Logf("seed: %d", seed)
+	rand := rand.New(rand.NewSource(seed))
+
+	tests := []struct {
+		descr  string
+		mkCred func(toSign []byte) (Credential, error)
+		expErr bool
+	}{
+		{
+			descr: "pgp sig no body",
+			mkCred: func(toSign []byte) (Credential, error) {
+				privKey, _ := SignifierPGPTmp("", rand)
+				return privKey.Sign(nil, toSign)
+			},
+			expErr: true,
+		},
+		{
+			descr: "pgp sig with body",
+			mkCred: func(toSign []byte) (Credential, error) {
+				privKey, pubKeyBody := SignifierPGPTmp("", rand)
+				cred, err := privKey.Sign(nil, toSign)
+				cred.PGPSignature.PubKeyBody = string(pubKeyBody)
+				return cred, err
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.descr, func(t *testing.T) {
+			data := make([]byte, rand.Intn(1024))
+			if _, err := rand.Read(data); err != nil {
+				t.Fatal(err)
+			}
+
+			cred, err := test.mkCred(data)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			err = cred.SelfVerify(data)
+			isNotSelfVerifying := errors.As(err, new(ErrNotSelfVerifying))
+			if test.expErr && !isNotSelfVerifying {
+				t.Fatalf("expected ErrNotSelfVerifying but got: %v", err)
+			} else if !test.expErr && err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+		})
+	}
+}

sigcred/pgp.go

@@ -6,8 +6,6 @@ import (
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/sha256"
-	"dehub.dev/src/dehub.git/fs"
-	"dehub.dev/src/dehub.git/yamlutil"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -16,6 +14,9 @@ import (
 	"strings"
 	"time"
 
+	"dehub.dev/src/dehub.git/fs"
+	"dehub.dev/src/dehub.git/yamlutil"
+
 	"golang.org/x/crypto/openpgp/armor"
 	"golang.org/x/crypto/openpgp/packet"
 )
@@ -23,8 +24,22 @@ import (
 // CredentialPGPSignature describes a PGP signature which has been used to sign
 // a commit.
 type CredentialPGPSignature struct {
-	PubKeyID string        `yaml:"pub_key_id"`
+	PubKeyID   string        `yaml:"pub_key_id"`
-	Body     yamlutil.Blob `yaml:"body"`
+	PubKeyBody string        `yaml:"pub_key_body,omitempty"`
+	Body       yamlutil.Blob `yaml:"body"`
+}
+
+// SelfVerify will only work if PubKeyBody is filled in. If so, Body will
+// attempt to be verified by that public key.
+func (c *CredentialPGPSignature) SelfVerify(data []byte) error {
+	if c.PubKeyBody == "" {
+		return ErrNotSelfVerifying{
+			Subject: "PGP signature Credential with no pub_key_body field",
+		}
+	}
+
+	sig := SignifierPGP{Body: c.PubKeyBody}
+	return sig.Verify(nil, data, Credential{PGPSignature: c})
 }
 
 type pgpPubKey struct {
@@ -134,10 +149,7 @@ func SignifierPGPTmp(accountID string, randReader io.Reader) (SignifierInterface
 		panic(err)
 	}
 
-	return accountSignifier{
+	return accountSignifier(accountID, privKey), pubKeyBody
-		accountID:          accountID,
-		SignifierInterface: privKey,
-	}, pubKeyBody
 }
 
 func (s pgpPrivKey) Sign(_ fs.FS, data []byte) (Credential, error) {
@@ -164,35 +176,86 @@ func (s pgpPrivKey) Sign(_ fs.FS, data []byte) (Credential, error) {
 }
 
 // SignifierPGP describes a pgp public key whose corresponding private key will
-// be used as a signing key.
+// be used as a signing key. The public key can be described by one of multiple
+// fields, each being a different method of loading the public key. Only one
+// field should be set.
 type SignifierPGP struct {
-	Body string `yaml:"body"`
+	// An armored string encoding of the public key, as exported via
+	// `gpg -a --export <key-id>`
+	Body string `yaml:"body,omitempty"`
+
+	// Path, relative to the root of the repo, of the armored public key file.
+	Path string `yaml:"path,omitempty"`
 }
 
 var _ SignifierInterface = SignifierPGP{}
 
-func (s SignifierPGP) load() (pgpPubKey, error) {
+func cmdGPG(stdin []byte, args ...string) ([]byte, error) {
-	return newPGPPubKey(strings.NewReader(s.Body))
+	args = append([]string{"--openpgp"}, args...)
+	stderr := new(bytes.Buffer)
+	cmd := exec.Command("gpg", args...)
+	cmd.Stdin = bytes.NewBuffer(stdin)
+	cmd.Stderr = stderr
+	out, err := cmd.Output()
+	if err != nil {
+		return nil, fmt.Errorf("calling gpg command (%v): %s", err, stderr.String())
+	}
+	return out, nil
+}
+
+// SignifierPGPFromKeyID 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 SignifierPGPFromKeyID(keyID string, setPubKeyBody 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)
+			},
+		}
+	}
+	return sigInt, nil
+}
+
+func (s SignifierPGP) load(fs fs.FS) (pgpPubKey, error) {
+	if s.Body != "" {
+		return newPGPPubKey(strings.NewReader(s.Body))
+	}
+
+	path := filepath.Clean(s.Path)
+	fr, err := fs.Open(path)
+	if err != nil {
+		return pgpPubKey{}, fmt.Errorf("opening PGP public key file at %q: %w", path, err)
+	}
+	defer fr.Close()
+
+	pubKeyB, err := ioutil.ReadAll(fr)
+	if err != nil {
+		return pgpPubKey{}, fmt.Errorf("reading PGP public key from file at %q: %w", s.Path, err)
+	}
+
+	return SignifierPGP{Body: string(pubKeyB)}.load(fs)
 }
 
 // Sign will sign the given arbitrary bytes using the private key corresponding
 // to the pgp public key embedded in this Signifier.
 func (s SignifierPGP) Sign(fs fs.FS, data []byte) (Credential, error) {
-	sigPGP, err := s.load()
+	sigPGP, err := s.load(fs)
 	if err != nil {
 		return Credential{}, err
 	}
 
-	stderr := new(bytes.Buffer)
+	sig, err := cmdGPG(data, "--detach-sign", "--local-user", sigPGP.pubKey.KeyIdString())
-	cmd := exec.Command("gpg",
-		"--openpgp",
-		"--detach-sign",
-		"--local-user", sigPGP.pubKey.KeyIdString())
-	cmd.Stdin = bytes.NewBuffer(data)
-	cmd.Stderr = stderr
-	sig, err := cmd.Output()
 	if err != nil {
-		return Credential{}, fmt.Errorf("error signing with gpg (%v): %s", err, stderr.String())
+		return Credential{}, fmt.Errorf("signing with pgp key: %w", err)
 	}
 
 	return Credential{
@@ -206,7 +269,7 @@ func (s SignifierPGP) Sign(fs fs.FS, data []byte) (Credential, error) {
 // Signed returns true if the private key corresponding to the pgp public key
 // embedded in this Signifier was used to produce the given Credential.
 func (s SignifierPGP) Signed(fs fs.FS, cred Credential) (bool, error) {
-	sigPGP, err := s.load()
+	sigPGP, err := s.load(fs)
 	if err != nil {
 		return false, err
 	}
@@ -217,67 +280,15 @@ func (s SignifierPGP) Signed(fs fs.FS, cred Credential) (bool, error) {
 // Verify asserts that the given signature was produced by this key signing the
 // given piece of data.
 func (s SignifierPGP) Verify(fs fs.FS, data []byte, cred Credential) error {
-	sigPGP, err := s.load()
+	sigPGP, err := s.load(fs)
 	if err != nil {
 		return err
 	}
 	return sigPGP.Verify(fs, data, cred)
 }
 
-// SignifierPGPFile is the same as SignifierPGP, except that the public key is
+// SignifierPGPFile is deprecated and should not be used, use the Path field of
-// found in the repo rather than encoded into the object.
+// SignifierPGP instead.
 type SignifierPGPFile struct {
 	Path string `yaml:"path"`
 }
-
-var _ SignifierInterface = SignifierPGPFile{}
-
-func (s SignifierPGPFile) load(fs fs.FS) (SignifierPGP, error) {
-	path := filepath.Clean(s.Path)
-	fr, err := fs.Open(path)
-	if err != nil {
-		return SignifierPGP{}, fmt.Errorf("could not open PGP public key file at %q: %w", path, err)
-	}
-	defer fr.Close()
-
-	pubKeyB, err := ioutil.ReadAll(fr)
-	if err != nil {
-		return SignifierPGP{}, fmt.Errorf("could not read PGP public key from file blob at %q: %w", s.Path, err)
-	}
-
-	return SignifierPGP{Body: string(pubKeyB)}, nil
-}
-
-// Sign will sign the given arbitrary bytes using the private key corresponding
-// to the pgp public key located by this Signifier.
-func (s SignifierPGPFile) Sign(fs fs.FS, data []byte) (Credential, error) {
-	sigPGP, err := s.load(fs)
-	if err != nil {
-		return Credential{}, err
-	}
-	return sigPGP.Sign(fs, data)
-}
-
-// Signed returns true if the private key corresponding to the pgp public key
-// located by this Signifier was used to produce the given Credential.
-func (s SignifierPGPFile) Signed(fs fs.FS, cred Credential) (bool, error) {
-	if cred.PGPSignature == nil {
-		return false, nil
-	}
-
-	sigPGP, err := s.load(fs)
-	if err != nil {
-		return false, err
-	}
-	return sigPGP.Signed(fs, cred)
-}
-
-// Verify asserts that the given signature was produced by this key signing the
-// given piece of data.
-func (s SignifierPGPFile) Verify(fs fs.FS, data []byte, cred Credential) error {
-	sigPGP, err := s.load(fs)
-	if err != nil {
-		return err
-	}
-	return sigPGP.Verify(fs, data, cred)
-}

sigcred/pgp_test.go

@@ -1,10 +1,11 @@
 package sigcred
 
 import (
-	"dehub.dev/src/dehub.git/fs"
 	"math/rand"
 	"testing"
 	"time"
+
+	"dehub.dev/src/dehub.git/fs"
 )
 
 // There are not currently tests for testing pgp signature creation, as they
@@ -17,18 +18,17 @@ func TestPGPVerification(t *testing.T) {
 		init  func(pubKeyBody []byte) (SignifierInterface, fs.FS)
 	}{
 		{
-			descr: "SignifierPGP",
+			descr: "SignifierPGP Body",
 			init: func(pubKeyBody []byte) (SignifierInterface, fs.FS) {
 				return SignifierPGP{Body: string(pubKeyBody)}, nil
 			},
 		},
 		{
-			descr: "SignifierPGPFile",
+			descr: "SignifierPGP Path",
 			init: func(pubKeyBody []byte) (SignifierInterface, fs.FS) {
 				pubKeyPath := "some/dir/pubkey.asc"
 				fs := fs.Stub{pubKeyPath: pubKeyBody}
-				sigPGPFile := SignifierPGPFile{Path: pubKeyPath}
+				return SignifierPGP{Path: pubKeyPath}, fs
-				return sigPGPFile, fs
 			},
 		},
 	}
@@ -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 := SignifierPGPTmp("foo", rand)
+			privKey, pubKeyBody := SignifierPGPTmp("", rand)
 
 			sig, fs := test.init(pubKeyBody)
 			data := make([]byte, rand.Intn(1024))

sigcred/signifier.go

@@ -8,7 +8,9 @@ import (
 // Signifier reprsents a single signing method being defined in the Config. Only
 // one field should be set on each Signifier.
 type Signifier struct {
-	PGPPublicKey     *SignifierPGP     `type:"pgp_public_key"`
+	PGPPublicKey *SignifierPGP `type:"pgp_public_key"`
+
+	// PGPPublicKeyFile is deprecated, only PGPPublicKey should be used
 	PGPPublicKeyFile *SignifierPGPFile `type:"pgp_public_key_file"`
 }
 
@@ -19,7 +21,16 @@ func (s Signifier) MarshalYAML() (interface{}, error) {
 
 // UnmarshalYAML implements the yaml.Unmarshaler interface.
 func (s *Signifier) UnmarshalYAML(unmarshal func(interface{}) error) error {
-	return typeobj.UnmarshalYAML(s, unmarshal)
+	if err := typeobj.UnmarshalYAML(s, unmarshal); err != nil {
+		return err
+	}
+
+	// TODO deprecate PGPPublicKeyFile
+	if s.PGPPublicKeyFile != nil {
+		s.PGPPublicKey = &SignifierPGP{Path: s.PGPPublicKeyFile.Path}
+		s.PGPPublicKeyFile = nil
+	}
+	return nil
 }
 
 // Interface returns the SignifierInterface instance encapsulated by this
@@ -33,7 +44,7 @@ func (s Signifier) Interface(accountID string) (SignifierInterface, error) {
 	if err != nil {
 		return nil, err
 	}
-	return accountSignifier{accountID, el.(SignifierInterface)}, nil
+	return accountSignifier(accountID, el.(SignifierInterface)), nil
 }
 
 // SignifierInterface describes the methods that all Signifiers must implement.
@@ -53,19 +64,31 @@ type SignifierInterface interface {
 	Verify(fs fs.FS, data []byte, cred Credential) error
 }
 
+type signifierMiddleware struct {
+	SignifierInterface
+	signCallback func(*Credential)
+}
+
+func (sm signifierMiddleware) Sign(fs fs.FS, data []byte) (Credential, error) {
+	cred, err := sm.SignifierInterface.Sign(fs, data)
+	if err != nil || sm.signCallback == nil {
+		return cred, err
+	}
+	sm.signCallback(&cred)
+	return cred, nil
+}
+
 // accountSignifier wraps a SignifierInterface to always set the accountID field
 // on Credentials it produces via the Sign method.
 //
 // TODO accountSignifier shouldn't be necessary, it's very ugly. Which indicates
 // that Credential probably shouldn't have AccountID on it, which makes sense.
 // Some refactoring is required here.
-type accountSignifier struct {
+func accountSignifier(accountID string, sigInt SignifierInterface) SignifierInterface {
-	accountID string
+	return signifierMiddleware{
-	SignifierInterface
+		SignifierInterface: sigInt,
-}
+		signCallback: func(cred *Credential) {
-
+			cred.AccountID = accountID
-func (as accountSignifier) Sign(fs fs.FS, data []byte) (Credential, error) {
+		},
-	cred, err := as.SignifierInterface.Sign(fs, data)
+	}
-	cred.AccountID = as.accountID
-	return cred, err
 }