diff --git a/ROADMAP.md b/ROADMAP.md index 586b960..d60136e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -6,12 +6,18 @@ set, only a sequence of milestones and the requirements to hit them. ## Milestone: Be able to add other developers to the project -* Thread commits (comments, signatures) +* Thread commits (comments) * Coalesce command * Fast-forward perms on branches * Authorship in commit messages * README with a "Getting Started" section +## Milestone: refactor accessctl? + +The current accessctl is cumbersome and difficult to describe. It might be +better if it was constructed as an actual ACL, rather than whatever it is +currently. + ## Milestone: Versions * Tag commits @@ -23,6 +29,8 @@ set, only a sequence of milestones and the requirements to hit them. * Maybe coalesce the `accessctl`, `fs`, and `sigcred` packages back into the root "dehub" package. +* Polish all error messages + * Polish commands - New flag system, some kind of interactivity support (e.g. user doesn't specify required argument, give them a prompt on the CLI to input it diff --git a/SPEC.md b/SPEC.md index 40f0de8..ac7a297 100644 --- a/SPEC.md +++ b/SPEC.md @@ -72,18 +72,30 @@ access_controls: any_account: true # indicates any account defined in accounts is valid count: 1 - # If a branch is not matched by any access control object then the following - # default object is implied: - # - # branch_pattern: ** - # change_access_controls: - # - file_path_pattern: ** - # condition: - # type: signature - # any_account: true - # count: 1 + + # credential_access_control is an object describing under what condition + # credential commits can be added to the branch. + credential_access_control: + # never conditions indicate that a commit will never be allowed. In this + # case, credential commits are not allowed in the main branch under any + # circumstances. + condition: + type: never +``` + +Unless the `config.yml` file in place declares otherwise, the following +condition is applied to all commits on all branches: + +```yaml +condition: + type: signature + any_account: true + count: 1 ``` +The exception is commits for the `main` branch, for which only change commits +are allowed by default (under that condition). + # Change Hash When a change commit (see Commits section) is being signed by a signifier there @@ -194,7 +206,7 @@ credentials: ## Credential Commits Commits of type `credential` contain one or more credentials for some hash -(presumably a change hash, but in the future there may be other types). The +(presumably a change hash, but in the future there may be other types). The commit message head is not spec'd, but should be a human-readable description of "who is crediting what, and how". diff --git a/accessctl/access_control.go b/accessctl/access_control.go index b48a13b..d08b3e7 100644 --- a/accessctl/access_control.go +++ b/accessctl/access_control.go @@ -8,42 +8,56 @@ import ( ) var ( + // DefaultSignatureCondition represents the Condition which is applied for + // default access controls. It requires a single signature credential from + // any account defined in the Config. + DefaultSignatureCondition = Condition{ + Signature: &ConditionSignature{ + AnyAccount: true, + Count: "1", + }, + } + // DefaultChangeAccessControl represents the ChangeAccessControl which is // applied when a changed file's path does not match any defined patterns // within a BranchAccessControl. DefaultChangeAccessControl = ChangeAccessControl{ FilePathPattern: "**", - Condition: Condition{ - Signature: &ConditionSignature{ - AnyAccount: true, - Count: "1", - }, - }, + Condition: DefaultSignatureCondition, + } + + // DefaultCredentialAccessControl represents the CredentialAccessControl + // which is applied when a BranchAccessControl does not have a defined + // CredentialAccessControl. + DefaultCredentialAccessControl = CredentialAccessControl{ + Condition: DefaultSignatureCondition, } // DefaultBranchAccessControls represents the BranchAccessControls which are // applied when the name of a branch being interacted with does not match // any defined patterns within the Config. DefaultBranchAccessControls = []BranchAccessControl{ - // These are currently the same, but they will differ once things like - // comments start being implemented. { - BranchPattern: "main", - ChangeAccessControls: []ChangeAccessControl{DefaultChangeAccessControl}, + BranchPattern: "main", + CredentialAccessControl: &CredentialAccessControl{ + Condition: Condition{Never: new(ConditionNever)}, + }, }, { - BranchPattern: "**", - ChangeAccessControls: []ChangeAccessControl{DefaultChangeAccessControl}, + BranchPattern: "**", }, } ) +// any account can do anything, except main branch can only get change commits + // BranchAccessControl represents an access control object defined for the // purpose of controlling who is able to perform what interactions with a // branch. type BranchAccessControl struct { - BranchPattern string `yaml:"branch_pattern"` - ChangeAccessControls []ChangeAccessControl `yaml:"change_access_controls"` + BranchPattern string `yaml:"branch_pattern"` + ChangeAccessControls []ChangeAccessControl `yaml:"change_access_controls,omitempty"` + CredentialAccessControl *CredentialAccessControl `yaml:"credential_access_control,omitempty"` } // ChangeAccessControl represents an access control object being defined in the @@ -53,6 +67,13 @@ type ChangeAccessControl struct { Condition Condition `yaml:"condition"` } +// CredentialAccessControl represents an access control object being defined in +// the Config for the purpose of controlling who is able to create credential +// commits. +type CredentialAccessControl struct { + Condition Condition `yaml:"condition"` +} + // MatchInteractions is used as an input to Match to describe all // interactions which are being attempted on a particular Branch. type MatchInteractions struct { @@ -63,6 +84,10 @@ type MatchInteractions struct { // FilePathsChanged is the set of file paths (relative to the repo root) // which have been modified in some way. FilePathsChanged []string + + // CredentialAdded indicates a credential commit is being added to the + // Branch. + CredentialAdded bool } // MatchedChangeAccessControl contains information about a ChangeAccessControl @@ -75,6 +100,12 @@ type MatchedChangeAccessControl struct { FilePaths []string } +// MatchedCredentialAccessControl contains information about a +// CredentialAccessControl which was matched in Match. +type MatchedCredentialAccessControl struct { + CredentialAccessControl CredentialAccessControl +} + // MatchResult is the result returned from the Match method. type MatchResult struct { // BranchPattern indicates the BranchPattern field of the @@ -84,6 +115,11 @@ type MatchResult struct { // ChangeAccessControls indicates which ChangeAccessControl objects matched // the files being changed. ChangeAccessControls []MatchedChangeAccessControl + + // CredentialAccessControls indicates which CredentialAccessControl object + // matched for a credential commit. Will be nil if CredentialAdded was + // false. + CredentialAccessControl *MatchedCredentialAccessControl } // Match takes in a set of access controls and a set of interactions taking @@ -102,7 +138,7 @@ func Match(accessControls []BranchAccessControl, interactions MatchInteractions) for i := range accessControls { ok, err = doublestar.Match(accessControls[i].BranchPattern, interactions.Branch) if err != nil { - return res, fmt.Errorf("error matching branch %q to pattern %q: %w", + return res, fmt.Errorf("matching branch %q to branch_pattern %q: %w", accessControls[i].BranchPattern, interactions.Branch, err) } else if ok { branchAC = accessControls[i] @@ -124,7 +160,7 @@ func Match(accessControls []BranchAccessControl, interactions MatchInteractions) var err error for _, ac := range changeACs { if ok, err = doublestar.PathMatch(ac.FilePathPattern, path); err != nil { - return res, fmt.Errorf("error matching path %q to patterrn %q: %w", + return res, fmt.Errorf("matching path %q to file_path_pattern %q: %w", path, ac.FilePathPattern, err) } else if ok { acToPaths[ac] = append(acToPaths[ac], path) @@ -150,5 +186,17 @@ func Match(accessControls []BranchAccessControl, interactions MatchInteractions) }) } + // Handle CredentialAccessControl, if applicable + if interactions.CredentialAdded { + credAC := branchAC.CredentialAccessControl + if credAC == nil { + credAC = &DefaultCredentialAccessControl + } + + res.CredentialAccessControl = &MatchedCredentialAccessControl{ + CredentialAccessControl: *credAC, + } + } + return res, nil } diff --git a/accessctl/access_control_test.go b/accessctl/access_control_test.go index 7a47fc7..2278e06 100644 --- a/accessctl/access_control_test.go +++ b/accessctl/access_control_test.go @@ -161,6 +161,51 @@ func TestMatch(t *testing.T) { }, }, }, + { + descr: "no defined CredentialAccessControl", + branchACs: []BranchAccessControl{{ + BranchPattern: "main", + }}, + interactions: MatchInteractions{ + Branch: "main", + CredentialAdded: true, + }, + result: MatchResult{ + BranchPattern: "main", + CredentialAccessControl: &MatchedCredentialAccessControl{ + CredentialAccessControl: DefaultCredentialAccessControl, + }, + }, + }, + { + descr: "defined CredentialAccessControl", + branchACs: []BranchAccessControl{{ + BranchPattern: "main", + CredentialAccessControl: &CredentialAccessControl{ + Condition: Condition{ + Signature: &ConditionSignature{ + AccountIDs: []string{"foo", "bar", "baz"}, + }, + }, + }, + }}, + interactions: MatchInteractions{ + Branch: "main", + CredentialAdded: true, + }, + result: MatchResult{ + BranchPattern: "main", + CredentialAccessControl: &MatchedCredentialAccessControl{ + CredentialAccessControl: CredentialAccessControl{ + Condition: Condition{ + Signature: &ConditionSignature{ + AccountIDs: []string{"foo", "bar", "baz"}, + }, + }, + }, + }, + }, + }, } for _, test := range tests { diff --git a/accessctl/condition.go b/accessctl/condition.go index 7a56354..a158753 100644 --- a/accessctl/condition.go +++ b/accessctl/condition.go @@ -24,6 +24,7 @@ type ConditionInterface interface { // Condition represents an access control condition being defined in the Config. // Only one of its fields may be filled in at a time. type Condition struct { + Never *ConditionNever `type:"never"` Signature *ConditionSignature `type:"signature"` } @@ -47,10 +48,19 @@ func (c Condition) Interface() (ConditionInterface, error) { return el.(ConditionInterface), nil } +// ConditionNever is a Condition which is never satisfied. +type ConditionNever struct{} + +// Satisfied always returns an error, because ConditionNever cannot be +// satisfied. +func (condNever ConditionNever) Satisfied([]sigcred.Credential) error { + return errors.New("condition of type 'never' cannot be satisfied") +} + // ConditionSignature represents the configuration of an access control // condition which requires one or more signatures to be present on a commit. // -// Either AccountIDs or AccountIDsByMeta must be filled. +// Either AccountIDs or AnyAccount must be filled in. type ConditionSignature struct { AccountIDs []string `yaml:"account_ids,omitempty"` AnyAccount bool `yaml:"any_account,omitempty"` diff --git a/cmd/dehub/cmd_commit.go b/cmd/dehub/cmd_commit.go index d010bca..88e2442 100644 --- a/cmd/dehub/cmd_commit.go +++ b/cmd/dehub/cmd_commit.go @@ -7,41 +7,20 @@ import ( "fmt" "dehub/cmd/dehub/dcmd" + + "gopkg.in/src-d/go-git.v4/plumbing" ) func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) { flag := cmd.FlagSet() - msg := flag.String("msg", "", "Commit message") accountID := flag.String("account-id", "", "Account to sign commit as") - cmd.Run(func() (context.Context, error) { - repo := ctxRepo(ctx) - - // Don't bother checking any of the parameters, especially commit - // message, if there's no staged changes, - hasStaged, err := repo.HasStagedChanges() - if err != nil { - return nil, fmt.Errorf("error determining if any changes have been staged: %w", err) - } else if !hasStaged { - return nil, errors.New("no changes have been staged for commit") - } - - if *accountID == "" { - return nil, errors.New("-account-id is required") - } - if *msg == "" { - var err error - if *msg, err = tmpFileMsg(); err != nil { - return nil, fmt.Errorf("error collecting commit message from user: %w", err) - - } else if *msg == "" { - return nil, errors.New("empty commit message, not doing anything") - } - } + repo := ctxRepo(ctx) + accreditAndCommit := func(commit dehub.Commit) error { cfg, err := repo.LoadConfig() if err != nil { - return nil, err + return err } var account dehub.Account @@ -53,33 +32,115 @@ func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) { } } if !ok { - return nil, fmt.Errorf("account ID %q not found in config", *accountID) + return fmt.Errorf("account ID %q not found in config", *accountID) } else if l := len(account.Signifiers); l == 0 || l > 1 { - return nil, fmt.Errorf("account %q has %d signifiers, only one is supported right now", *accountID, l) + 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 nil, fmt.Errorf("could not cast %+v to SignifierInterface: %w", sig, err) + 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) } - commit, err := repo.NewCommitChange(*msg) + hash, err := repo.Commit(commit, *accountID) if err != nil { - return nil, fmt.Errorf("could not construct change commit: %w", err) + return fmt.Errorf("committing to git: %w", err) } - commit, err = repo.AccreditCommit(commit, sigInt) - if err != nil { - return nil, fmt.Errorf("could not accredit commit: %w", err) + fmt.Printf("committed to HEAD as %s\n", hash) + return nil + } + + var hasStaged bool + body := func() (context.Context, error) { + if *accountID == "" { + return nil, errors.New("-account-id is required") } - hash, err := repo.Commit(commit, *accountID) - if err != nil { - return nil, fmt.Errorf("could not commit change commit: %w", err) + var err error + if hasStaged, err = repo.HasStagedChanges(); err != nil { + return nil, fmt.Errorf("determining if any changes have been staged: %w", err) } + return ctx, nil + } + + cmd.SubCmd("change", "Commit file changes", + func(ctx context.Context, cmd *dcmd.Cmd) { + flag := cmd.FlagSet() + msg := flag.String("msg", "", "Commit message") + cmd.Run(func() (context.Context, error) { + if !hasStaged { + return nil, errors.New("no changes have been staged for commit") + } + + if *msg == "" { + var err error + if *msg, err = tmpFileMsg(); err != nil { + return nil, fmt.Errorf("error collecting commit message from user: %w", err) + + } else if *msg == "" { + return nil, errors.New("empty commit message, not doing anything") + } + } + + commit, err := repo.NewCommitChange(*msg) + if err != nil { + return nil, fmt.Errorf("could not construct change commit: %w", err) + + } else if err := accreditAndCommit(commit); err != nil { + return nil, err + } + return nil, nil + }) + }, + ) + + cmd.SubCmd("credential", "Commit credential of a different commit", + func(ctx context.Context, cmd *dcmd.Cmd) { + flag := cmd.FlagSet() + rev := flag.String("rev", "", "Revision of commit to accredit") + cmd.Run(func() (context.Context, error) { + if *rev == "" { + return nil, errors.New("-rev is required") + } else if hasStaged { + return nil, errors.New("credential commit cannot have any files changed") + } + + // TODO maybe nice to have a helper to do all of this + h, err := repo.GitRepo.ResolveRevision(plumbing.Revision(*rev)) + if err != nil { + return nil, fmt.Errorf("resolving revision: %w", err) + } + + commitObj, err := repo.GitRepo.CommitObject(*h) + if err != nil { + return nil, fmt.Errorf("getting commit object %q: %w", h, err) + } + + var commit dehub.Commit + if err := commit.UnmarshalText([]byte(commitObj.Message)); err != nil { + return nil, fmt.Errorf("unmarshaling commit message: %w", err) + } + + commitInt, err := commit.Interface() + if err != nil { + return nil, fmt.Errorf("casting %#v to CommitInterface: %w", commit, err) + } + + credCommit, err := repo.NewCommitCredential(commitInt.GetHash()) + if err != nil { + return nil, fmt.Errorf("constructing credential commit: %w", err) + } else if err := accreditAndCommit(credCommit); err != nil { + return nil, err + } + return nil, nil + }) + }, + ) - fmt.Printf("changes committed to HEAD as %s\n", hash) - return nil, nil - }) + cmd.Run(body) } diff --git a/cmd/dehub/dcmd/dcmd.go b/cmd/dehub/dcmd/dcmd.go index 24bcf33..7727d34 100644 --- a/cmd/dehub/dcmd/dcmd.go +++ b/cmd/dehub/dcmd/dcmd.go @@ -149,7 +149,7 @@ func (cmd *Cmd) Run(body func() (context.Context, error)) { } // now find that sub-command - subCmdName, args := strings.ToLower(args[0]), args[1:] + subCmdName := strings.ToLower(subArgs[0]) var subCmd subCmd var subCmdOk bool for _, subCmd = range cmd.subCmds { diff --git a/commit.go b/commit.go index 3774c6d..5c6102b 100644 --- a/commit.go +++ b/commit.go @@ -8,6 +8,7 @@ import ( "dehub/typeobj" "encoding" "encoding/base64" + "errors" "fmt" "strings" "time" @@ -122,7 +123,7 @@ func (r *Repo) AccreditCommit(commit Commit, sigInt sigcred.SignifierInterface) cred, err := sigInt.Sign(headFS, commitInt.GetHash()) if err != nil { - return commit, fmt.Errorf("could not accreddit change commit: %w", err) + return commit, fmt.Errorf("could not accredit change commit: %w", err) } commit.Credentials = append(commit.Credentials, cred) return commit, nil @@ -172,13 +173,46 @@ func (r *Repo) HasStagedChanges() (bool, error) { return any, nil } +type verificationCtx struct { + commit *object.Commit + commitTree, parentTree *object.Tree + isRootCommit bool +} + +// non-gophers gonna hate on this method, but I say it's fine +func (r *Repo) verificationCtx(h plumbing.Hash) (vctx verificationCtx, err error) { + if vctx.commit, err = r.GitRepo.CommitObject(h); err != nil { + return vctx, fmt.Errorf("retrieving commit object: %w", err) + + } else if vctx.commitTree, err = r.GitRepo.TreeObject(vctx.commit.TreeHash); err != nil { + return vctx, fmt.Errorf("retrieving commit tree object %q: %w", + vctx.commit.TreeHash, err) + + } else if vctx.isRootCommit = vctx.commit.NumParents() == 0; vctx.isRootCommit { + vctx.parentTree = new(object.Tree) + + } else if parent, err := vctx.commit.Parent(0); err != nil { + return vctx, fmt.Errorf("retrieving commit parent: %w", err) + + } else if vctx.parentTree, err = r.GitRepo.TreeObject(parent.TreeHash); err != nil { + return vctx, fmt.Errorf("retrieving commit parent tree object %q: %w", + parent.Hash, err) + } + + return vctx, nil +} + func (r *Repo) assertAccessControls( - accessCtls []accessctl.BranchAccessControl, creds []sigcred.Credential, - branch plumbing.ReferenceName, from, to *object.Tree, -) error { - filesChanged, err := calcDiff(from, to) + accessCtls []accessctl.BranchAccessControl, + commit Commit, vctx verificationCtx, branch plumbing.ReferenceName, +) (err error) { + filesChanged, err := calcDiff(vctx.parentTree, vctx.commitTree) if err != nil { - return err + return fmt.Errorf("calculating diff from tree %q to tree %q: %w", + vctx.parentTree.Hash, vctx.commitTree.Hash, err) + + } else if len(filesChanged) > 0 && commit.Change == nil { + return errors.New("files changes but commit is not a change commit") } pathsChanged := make([]string, len(filesChanged)) @@ -189,20 +223,35 @@ func (r *Repo) assertAccessControls( matchRes, err := accessctl.Match(accessCtls, accessctl.MatchInteractions{ Branch: branch.Short(), FilePathsChanged: pathsChanged, + CredentialAdded: commit.Credential != nil, }) if err != nil { - return fmt.Errorf("could not determine applicable access controls: %w", err) + return fmt.Errorf("determining applicable access controls: %w", err) } - for _, matchedAC := range matchRes.ChangeAccessControls { - ac := matchedAC.ChangeAccessControl - condInt, err := ac.Condition.Interface() + defer func() { if err != nil { - return fmt.Errorf("could not cast Condition of file path pattern %q to interface: %w", - ac.FilePathPattern, err) - } else if err := condInt.Satisfied(creds); err != nil { - return fmt.Errorf("access control of file path pattern %q not satisfied: %w\nFiles matched:\n%s", - ac.FilePathPattern, err, strings.Join(matchedAC.FilePaths, "\n")) + err = fmt.Errorf("asserting access controls for branch_pattern %q: %w", + matchRes.BranchPattern, err) + } + }() + + for _, matchedChangeAC := range matchRes.ChangeAccessControls { + ac := matchedChangeAC.ChangeAccessControl + if condInt, err := ac.Condition.Interface(); err != nil { + return fmt.Errorf("casting ChangeAccessControl.Condition %#v to interface: %w", ac.Condition, err) + } else if err := condInt.Satisfied(commit.Credentials); err != nil { + return fmt.Errorf("satisfying change_access_control with file_path_pattern %q: %w\nfiles matched:\n%s", + ac.FilePathPattern, err, strings.Join(matchedChangeAC.FilePaths, "\n")) + } + } + + if matchRes.CredentialAccessControl != nil { + cond := matchRes.CredentialAccessControl.CredentialAccessControl.Condition + if condInt, err := cond.Interface(); err != nil { + return fmt.Errorf("casting CredentialAccessControl.Condition %#v to interface: %w", cond, err) + } else if err := condInt.Satisfied(commit.Credentials); err != nil { + return fmt.Errorf("satisfying credential_access_control: %w", err) } } @@ -212,31 +261,20 @@ func (r *Repo) assertAccessControls( // VerifyCommit verifies that the commit at the given hash, which is presumably // on the given branch, is gucci. func (r *Repo) VerifyCommit(branch plumbing.ReferenceName, h plumbing.Hash) error { - commitObj, err := r.GitRepo.CommitObject(h) + vctx, err := r.verificationCtx(h) if err != nil { - return fmt.Errorf("could not retrieve commit object: %w", err) - } - - commitTree, err := r.GitRepo.TreeObject(commitObj.TreeHash) - if err != nil { - return fmt.Errorf("could not retrieve tree object: %w", err) + return err } - sigTree := commitTree // only for root commit - parentTree := &object.Tree{} - if commitObj.NumParents() > 0 { - parent, err := commitObj.Parent(0) - if err != nil { - return fmt.Errorf("could not retrieve parent of commit: %w", err) - } else if parentTree, err = r.GitRepo.TreeObject(parent.TreeHash); err != nil { - return fmt.Errorf("could not retrieve tree object of parent %q: %w", parent.Hash, err) - } - sigTree = parentTree + var sigFS fs.FS + if vctx.isRootCommit { + sigFS = fs.FromTree(vctx.commitTree) + } else { + sigFS = fs.FromTree(vctx.parentTree) } - sigFS := fs.FromTree(sigTree) var commit Commit - if err := commit.UnmarshalText([]byte(commitObj.Message)); err != nil { + if err := commit.UnmarshalText([]byte(vctx.commit.Message)); err != nil { return err } @@ -245,10 +283,7 @@ func (r *Repo) VerifyCommit(branch plumbing.ReferenceName, h plumbing.Hash) erro return fmt.Errorf("error loading config: %w", err) } - err = r.assertAccessControls( - cfg.AccessControls, commit.Credentials, - branch, parentTree, commitTree, - ) + err = r.assertAccessControls(cfg.AccessControls, commit, vctx, branch) if err != nil { return fmt.Errorf("failed to satisfy all access controls: %w", err) } @@ -259,7 +294,7 @@ func (r *Repo) VerifyCommit(branch plumbing.ReferenceName, h plumbing.Hash) erro } changeHash := commitInt.GetHash() - expectedChangeHash, err := commitInt.Hash(parentTree, commitTree) + expectedChangeHash, err := commitInt.Hash(vctx.parentTree, vctx.commitTree) if err != nil { return fmt.Errorf("error calculating expected change hash: %w", err) } else if !bytes.Equal(changeHash, expectedChangeHash) { diff --git a/commit_change_test.go b/commit_change_test.go index 313d11c..c739209 100644 --- a/commit_change_test.go +++ b/commit_change_test.go @@ -77,10 +77,6 @@ func TestChangeCommitVerify(t *testing.T) { account := h.cfg.Accounts[0] commit, hash := h.changeCommit(step.msg, account.ID, h.sig) - if err := h.repo.VerifyCommit(MainRefName, hash); err != nil { - t.Fatalf("could not verify hash %v: %v", hash, err) - } - commitObj, err := h.repo.GitRepo.CommitObject(hash) if err != nil { t.Fatalf("failed to retrieve commit %v: %v", hash, err) diff --git a/commit_credential_test.go b/commit_credential_test.go new file mode 100644 index 0000000..f7dec4a --- /dev/null +++ b/commit_credential_test.go @@ -0,0 +1,79 @@ +package dehub + +import ( + "dehub/accessctl" + "dehub/sigcred" + "testing" + + "gopkg.in/src-d/go-git.v4/plumbing" +) + +func TestCredentialCommitVerify(t *testing.T) { + h := newHarness(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.SignifierPGPTmp("toot", h.rand) + h.cfg.Accounts = append(h.cfg.Accounts, Account{ + ID: "toot", + Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{ + Body: string(tootPubKeyBody), + }}}, + }) + + tootBranch := plumbing.NewBranchReferenceName("toot_branch") + tootBranchCond := accessctl.Condition{ + Signature: &accessctl.ConditionSignature{ + AccountIDs: []string{"root", "toot"}, + Count: "1", + }, + } + allBranchCond := accessctl.Condition{ + Signature: &accessctl.ConditionSignature{ + AccountIDs: []string{"root"}, + Count: "1", + }, + } + h.cfg.AccessControls = []accessctl.BranchAccessControl{ + { + BranchPattern: tootBranch.Short(), + ChangeAccessControls: []accessctl.ChangeAccessControl{ + { + FilePathPattern: "**", + Condition: tootBranchCond, + }, + }, + CredentialAccessControl: &accessctl.CredentialAccessControl{ + Condition: tootBranchCond, + }, + }, + { + BranchPattern: "**", + ChangeAccessControls: []accessctl.ChangeAccessControl{ + { + FilePathPattern: "**", + Condition: allBranchCond, + }, + }, + CredentialAccessControl: &accessctl.CredentialAccessControl{ + Condition: allBranchCond, + }, + }, + } + h.stageCfg() + rootCommit, _ := h.changeCommit("initial commit", h.cfg.Accounts[0].ID, h.sig) + + // toot user wants to create a credential commit for the root commit, for + // whatever reason. + rootChangeHash := rootCommit.Change.ChangeHash + credCommit, err := h.repo.NewCommitCredential(rootChangeHash) + if err != nil { + t.Fatalf("creating credential commit for hash %x: %v", rootChangeHash, err) + + } + h.tryCommit(false, credCommit, "toot", tootSig) + + // toot tries again in their own branch, and should be allowed. + h.checkout(tootBranch) + h.tryCommit(true, credCommit, "toot", tootSig) +} diff --git a/commit_test.go b/commit_test.go index 32457b8..195cbb9 100644 --- a/commit_test.go +++ b/commit_test.go @@ -4,9 +4,7 @@ import ( "dehub/sigcred" "testing" - "gopkg.in/src-d/go-git.v4" "gopkg.in/src-d/go-git.v4/plumbing" - yaml "gopkg.in/yaml.v2" ) func TestConfigChange(t *testing.T) { @@ -31,20 +29,15 @@ func TestConfigChange(t *testing.T) { h.cfg.AccessControls[0].ChangeAccessControls[0].Condition.Signature.AccountIDs = []string{"root", "toot"} h.cfg.AccessControls[0].ChangeAccessControls[0].Condition.Signature.Count = "1" - cfgBody, err := yaml.Marshal(h.cfg) + h.stageCfg() + badCommit, err := h.repo.NewCommitChange("add toot user") if err != nil { - t.Fatal(err) + t.Fatalf("creating CommitChange: %v", err) } - h.stage(map[string]string{ConfigPath: string(cfgBody)}) - _, badHash := h.changeCommit("add toot user", h.cfg.Accounts[1].ID, newSig) - - if err := h.repo.VerifyCommit(MainRefName, badHash); err == nil { - t.Fatal("toot user shouldn't be able to add itself to config") - } - h.reset(hash, git.HardReset) + h.tryCommit(false, badCommit, h.cfg.Accounts[1].ID, newSig) // now add with the root user, this should work. - h.stage(map[string]string{ConfigPath: string(cfgBody)}) + h.stageCfg() _, hash = h.changeCommit("add toot user", h.cfg.Accounts[0].ID, h.sig) hashes = append(hashes, hash) diff --git a/repo_test.go b/repo_test.go index 855cae2..6936279 100644 --- a/repo_test.go +++ b/repo_test.go @@ -4,6 +4,7 @@ import ( "bytes" "dehub/accessctl" "dehub/sigcred" + "errors" "io" "math/rand" "path/filepath" @@ -107,24 +108,43 @@ func (h *harness) stage(tree map[string]string) { } } -func (h *harness) changeCommit(msg, accountID string, sig sigcred.SignifierInterface) (Commit, plumbing.Hash) { - commit, err := h.repo.NewCommitChange(msg) +func (h *harness) stageCfg() { + cfgBody, err := yaml.Marshal(h.cfg) if err != nil { - h.t.Fatalf("failed to create CommitChange: %v", err) + h.t.Fatal(err) } + h.stage(map[string]string{ConfigPath: string(cfgBody)}) +} - if sig != nil { - if commit, err = h.repo.AccreditCommit(commit, sig); err != nil { - h.t.Fatalf("failed to accredit commit: %v", err) - } +func (h *harness) checkout(branch plumbing.ReferenceName) { + w, err := h.repo.GitRepo.Worktree() + if err != nil { + h.t.Fatal(err) } - hash, err := h.repo.Commit(commit, accountID) + head, _, err := h.repo.head() if err != nil { - h.t.Fatalf("failed to commit ChangeCommit: %v", err) + h.t.Fatal(err) } - return commit, hash + _, err = h.repo.GitRepo.Branch(branch.Short()) + if errors.Is(err, git.ErrBranchNotFound) { + err = w.Checkout(&git.CheckoutOptions{ + Hash: head.Hash, + Branch: branch, + Create: true, + }) + } else if err != nil { + h.t.Fatalf("checking if branch already exists: %v", branch) + } else { + err = w.Checkout(&git.CheckoutOptions{ + Branch: branch, + }) + } + + if err != nil { + h.t.Fatalf("checking out branch: %v", err) + } } func (h *harness) reset(to plumbing.Hash, mode git.ResetMode) { @@ -142,6 +162,66 @@ func (h *harness) reset(to plumbing.Hash, mode git.ResetMode) { } } +func (h *harness) tryCommit( + shouldSucceed bool, + commit Commit, + accountID string, accountSig sigcred.SignifierInterface, +) ( + Commit, plumbing.Hash, +) { + if accountSig != nil { + var err error + if commit, err = h.repo.AccreditCommit(commit, accountSig); err != nil { + h.t.Fatalf("accrediting commit: %v", err) + } + } + + hash, err := h.repo.Commit(commit, accountID) + if err != nil { + h.t.Fatalf("failed to commit ChangeCommit: %v", err) + } + + branch, err := h.repo.CheckedOutBranch() + if err != nil { + h.t.Fatalf("determining checked out branch: %v", err) + } + + err = h.repo.VerifyCommit(branch, hash) + if shouldSucceed && err != nil { + h.t.Fatalf("verifying commit %q: %v", hash, err) + } else if shouldSucceed { + return commit, hash + } else if !shouldSucceed && err == nil { + h.t.Fatalf("verifying commit %q should have failed", hash) + } + + // commit verifying didn't succeed, reset it back. first get parent commit + // to reset to + commitObj, err := h.repo.GitRepo.CommitObject(hash) + if err != nil { + h.t.Fatalf("getting commit object of unverifiable hash %q: %v", hash, err) + } else if commitObj.NumParents() == 0 { + h.t.Fatalf("unverifiable commit %q has no parents, but it should", hash) + } + + h.reset(commitObj.ParentHashes[0], git.HardReset) + return commit, hash +} + +func (h *harness) changeCommit( + msg string, + accountID string, + sig sigcred.SignifierInterface, +) ( + Commit, plumbing.Hash, +) { + commit, err := h.repo.NewCommitChange(msg) + if err != nil { + h.t.Fatalf("creating ChangeCommit: %v", err) + } + return h.tryCommit(true, commit, accountID, sig) +} + func TestHasStagedChanges(t *testing.T) { harness := newHarness(t) assertHasStaged := func(expHasStaged bool) { @@ -191,22 +271,18 @@ access_controls: - root count: 0 `}) - _, hash0 := harness.changeCommit("first commit, this is going great", "root", harness.sig) - // even though that access_controls doesn't actually require any signatures, - // it should be used because it's not well formed. - harness.stage(map[string]string{"foo": "no rules!"}) - _, hash1 := harness.changeCommit("ain't no laws", "toot", nil) + // this commit should be created and verify fine + harness.changeCommit("first commit, this is going great", "root", harness.sig) - // verifying the first should work, but not the second. - if err := harness.repo.VerifyCommit(MainRefName, hash0); err != nil { - t.Fatalf("first commit %q should be verifiable, but got: %v", hash0, err) - } else if err := harness.repo.VerifyCommit(MainRefName, hash1); err == nil { - t.Fatalf("second commit %q should not have been verified", hash1) + // this commit should not be verifiable, because toot isn't in accounts and + // the default access controls should be being used + harness.stage(map[string]string{"foo": "no rules!"}) + badCommit, err := harness.repo.NewCommitChange("ain't no laws") + if err != nil { + t.Fatalf("creating CommitChange: %v", err) } - - // reset back to hash0 - harness.reset(hash0, git.HardReset) + harness.tryCommit(false, badCommit, "toot", nil) // make a commit fixing the config. everything should still be fine. harness.stage(map[string]string{ConfigPath: ` @@ -217,10 +293,7 @@ accounts: - type: pgp_public_key_file path: ".dehub/root.asc" `}) - _, hash2 := harness.changeCommit("Fix the config!", "root", harness.sig) - if err := harness.repo.VerifyCommit(MainRefName, hash2); err != nil { - t.Fatalf("config fix commit %q should be verifiable, but got: %v", hash2, err) - } + harness.changeCommit("Fix the config!", "root", harness.sig) } // TestThisRepoStillVerifies opens this actual repository and ensures that all