completely refactor accessctl (again)

---
type: change
message: |-
  completely refactor accessctl (again)

  This time it's using an actual access control list system, rather than whatever
  it was doing before. The new system uses a Filter type, rather than Condition,
  to decide which acl element should have its action (allow or deny) applied. This
  makes testing way easier, since all the different matching conditions are now
  individual filters, and so are tested individually.
change_hash: AFgN0hormIlO0VWkLKnAdSDZeVRbh0Wj8LLXOMVQEK+L
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl5yoi8ACgkQlcRvpqQRSKwo/g//QkSA80APnPtwcibNeaoUwduH1Xim8pmi5JScKGsOypYkE0Iy+fO3fRNz4r7Tm6qNn7O04fDsiXlxPojxn7+NDFCQArVgoJk3jVTRDBW7LpahWJsYPP1SBjGtaR9o0bOpclXblTMIcteTkLM94AeASqLaEY8StO+5PX/82AkFRQ8E6m9R2HCmgwbBhqwWp8936x8ekMFbSSi0TMIyV4rpd0wj4mjvjjBwa3ArGmH/ynwabPCFuuqMT6996N1zoDn5EqZA5jGrf+Q7rxsI6t1bOnLmg9NGMQYRaZLAVZrp5P6G5XR3et4Gz/2AphAEgYJM3yLbEjZW6Daa77CgTNHXde7gCaWqyfcKlVGPi29/O+2IXhpjwxHwGpsBgEdX9227zapL+jwSAOdUVj8n6C8I8BGqpT7rTwA53yxlbSwXlkttvAn/lGT5X4lK74YfkzMXMEBZKzsb/dQEPyP2Y+AG6z2D4Bs/4szsCiUXF9aG2Yx1o45lVXTTdPUNLIsnhBjM7usbQRg8i5kC+OC9AVCi8E+lf0/Qgp0cUb6QLH47bHvDTH7UluY1bgSLZy+Zjaisvl3a0aK/UspywWN/fFgOrz2cDw232n8IC+Zi4LSKm7dXDRFbC1JNzrwAPP1ifboOrltwKroOsDNaVGhX8ABahNjmrUO4JgE7gvX+zxXb+/I=
  account: mediocregopher

INTRODUCTION.md

@@ -46,24 +46,32 @@ platforms like IPFS.
 ### Example
 
 MyProject wants to ensure that at least 2 of the 3 maintainers sign off on a
-commit before the commit can be placed into the `main` branch (dehub's
+commit which changes files before the commit can be placed into the `main`
-equivalent of the `master` branch). MyProject's repo would contain a
+branch (dehub's equivalent of the `master` branch). MyProject's repo would
-`.dehub/config.yml` file with the following access controls set:
+contain a `.dehub/config.yml` file with the following access controls set:
 
 ```
 # ...
 access_controls:
-  - branch_pattern: main
+  - action: allow
-    change_access_controls:
+    filters:
-    # matches all files, but could be used for more fine-grained control
+    - type: branch
-    - file_path_pattern: "**"
+      pattern: main
-      condition:
+
-        type: signature
+    - type: commit_type
+      commit_type: change
+
+    - type: signature
       account_ids:
       - alice
       - bob
       - carol
       count: 2
+
+  - action: deny
+    filters:
+    - type: branch
+      branch: main
 ```
 
 A commit in the `main` branch would have a message with the following form:
@@ -197,9 +205,9 @@ _Finally_ the thread branch is ready to be coalesced, which is a step anyone
 can do once all the required credentials are available.
 
 To coalesce, the following is done: All file changes in the branch are squashed
-into a single change commit, using the latest commit message which was pushed by Alice.
+into a single change commit, using the latest commit message which was pushed by
-Bob's signature is added to the change commit message as a credential. The
+Alice.  Bob's signature is added to the change commit message as a credential.
-commit can then be pushed to `main` (because it now has two credentials) and
+The commit can then be pushed to `main` (because it now has two credentials) and
 `featureBranch` can be deleted.
 
 ## Pre-emptively Answered Questions

ROADMAP.md

@@ -12,12 +12,6 @@ set, only a sequence of milestones and the requirements to hit them.
 * 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

SPEC.md

@@ -29,72 +29,36 @@ accounts:
             user: "some_keybase_user_id"
 
 # access_controls define who may do what in the repo. The value is a list of
-# access control objects, each applying to one or more potential branch names.
+# access control objects, each containing an action (allow or deny) and a set of
+# filters. If a commit matches all filters (or if there are no filters) then the
+# action is taken. If not, then the next access control is attempted.
+#
+# If no access controls match a commit, then the default list is used, which
+# will definitely match. The following is the default set, which is enumerated
+# here for informational purposes only; it does not normally need to be defined.
 access_controls:
-
+  - action: allow
-  # branch_pattern is a glob pattern describing what branch names this access
+    filters:
-  # control applies to. The first matching branch_pattern for a branch name
+    - type: not
-  # defines which access controls are applied.
+      filter:
-  - branch_pattern: main
+        type: branch
-
+        pattern: main
-    # change_access_controls is an array of possible access controls applied for
+    - type: signature
-    # files being changed in the branch
-    change_access_controls:
-
-      # file_path_pattern is a glob pattern describing what files this access control
-      # applies to. Single star matches all characters except path separators,
-      # double star matches everything. The first matching file_path_pattern for a
-      # file path (relative to the repo root) defines which access controls are
-      # applied.
-      - file_path_pattern: ".dehub/**"
-
-        # signature conditions indicate that a commit must be signed by one or
-        # more accounts in order to be allowed.
-        condition:
-          type: signature
-
-          # account_ids lists all accounts whose signature will count towards
-          # meeting the condition
-          account_ids:
-              - some_user_id
-
-          # count describes how many signatures are required. It can be either a
-          # contrete integer (e.g. 2, meaning any 2 accounts listed by
-          # account_ids) or a percent.
-          count: 100% # all accounts in account_ids must sign
-
-      # This catch-all pattern for the rest of the repo requires that changes to
-      # any files not under `.dehub/` are signed by at least one of the
-      # defined accounts.
-      - file_path_pattern: "**"
-        condition:
-          type: signature
-          any_account: true # indicates any account defined in accounts is valid
-          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
+  - action: allow
-are allowed by default (under that condition).
+    filters:
+    - type: branch
+      pattern: main
+    - type: commit_type
+      commit_type: change
+    - type: signature
+      any_account: true
+      count: 1
+
+  - action: deny
+```
 
 # Change Hash
 

accessctl/access_control.go

@@ -1,202 +1,151 @@
+// Package accessctl implements functionality related to allowing or denying
+// actions in a repo based on who is taking what actions.
 package accessctl
 
 import (
+	"dehub/sigcred"
+	"errors"
 	"fmt"
-	"sort"
 
-	"github.com/bmatcuk/doublestar"
+	yaml "gopkg.in/yaml.v2"
 )
 
-var (
+// DefaultAccessControlsStr is the encoded form of the default access control
-	// DefaultSignatureCondition represents the Condition which is applied for
+// set which is applied to all CommitRequests if no user-supplied ones match.
-	// default access controls. It requires a single signature credential from
+//
-	// any account defined in the Config.
+// The effect of these AccessControls is to allow all commit types on any branch
-	DefaultSignatureCondition = Condition{
+// (with the exception of the main branch, which only allows change commits), as
-		Signature: &ConditionSignature{
+// long as the commit has one signature from a configured account.
-			AnyAccount: true,
+var DefaultAccessControlsStr = `
-			Count:      "1",
+  - action: allow
-		},
+    filters:
+    - type: not
+      filter:
+        type: branch
+        pattern: main
+    - type: signature
+      any_account: true
+      count: 1
+  
+  - action: allow
+    filters:
+    - type: branch
+      pattern: main
+    - type: commit_type
+      commit_type: change
+    - type: signature
+      any_account: true
+      count: 1
+  
+  - action: deny
+`
+
+// DefaultAccessControls is the decoded form of DefaultAccessControlsStr.
+var DefaultAccessControls = func() []AccessControl {
+	var acl []AccessControl
+	if err := yaml.Unmarshal([]byte(DefaultAccessControlsStr), &acl); err != nil {
+		panic(err)
 	}
+	return acl
+}()
 
-	// DefaultChangeAccessControl represents the ChangeAccessControl which is
+// CommitRequest is used to describe a set of interactions which are being
-	// applied when a changed file's path does not match any defined patterns
+// requested to be performed.
-	// within a BranchAccessControl.
+type CommitRequest struct {
-	DefaultChangeAccessControl = ChangeAccessControl{
+	// Type describes what type of commit is being requested. Possibilities are
-		FilePathPattern: "**",
+	// determined by the requester.
-		Condition:       DefaultSignatureCondition,
+	Type string
-	}
 
-	// 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{
-		{
-			BranchPattern: "main",
-			CredentialAccessControl: &CredentialAccessControl{
-				Condition: Condition{Never: new(ConditionNever)},
-			},
-		},
-		{
-			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,omitempty"`
-	CredentialAccessControl *CredentialAccessControl `yaml:"credential_access_control,omitempty"`
-}
-
-// ChangeAccessControl represents an access control object being defined in the
-// Config for the purpose of controlling who is able to change which files.
-type ChangeAccessControl struct {
-	FilePathPattern string    `yaml:"file_path_pattern"`
-	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 {
 	// Branch is the name of the branch the interactions are being attempted on.
 	// It is required.
 	Branch string
 
-	// FilePathsChanged is the set of file paths (relative to the repo root)
+	// Credentials are the Credential objects attached to the commit.
-	// which have been modified in some way.
+	Credentials []sigcred.Credential
-	FilePathsChanged []string
 
-	// CredentialAdded indicates a credential commit is being added to the
+	// FilesChanged is the set of file paths (relative to the repo root) which
-	// Branch.
+	// have been modified in some way.
-	CredentialAdded bool
+	FilesChanged []string
 }
 
-// MatchedChangeAccessControl contains information about a ChangeAccessControl
+// Action describes what action an AccessControl should perform
-// which was matched in Match
+// when given a CommitRequest.
-type MatchedChangeAccessControl struct {
+type Action string
-	ChangeAccessControl ChangeAccessControl
 
-	// FilePaths contains all FilePaths to which this access control was found
+// Enumerates possible Action values
-	// to be applicable.
+const (
-	FilePaths []string
+	ActionAllow Action = "allow"
+	ActionDeny  Action = "deny"
+
+	// ActionNext is used internally when a request does not match an
+	// AccessControl's filters. It _could_ be used in the Config as well, but it
+	// would be pretty pointless to do so, so we don't talk about it.
+	ActionNext Action = "next"
+)
+
+// AccessControl describes a set of Filters, and the Actions which should be
+// taken on a CommitRequest if those Filters all match on the CommitRequest.
+type AccessControl struct {
+	Action  Action   `yaml:"action"`
+	Filters []Filter `yaml:"filters"`
 }
 
-// MatchedCredentialAccessControl contains information about a
+// ActionForCommit returns what Action this AccessControl says to take for a
-// CredentialAccessControl which was matched in Match.
+// given CommitRequest. It may return ActionNext if the request is not matched
-type MatchedCredentialAccessControl struct {
+// by the AccessControl's Filters.
-	CredentialAccessControl CredentialAccessControl
+func (ac AccessControl) ActionForCommit(req CommitRequest) (Action, error) {
-}
+	for _, filter := range ac.Filters {
-
+		filterI, err := filter.Interface()
-// MatchResult is the result returned from the Match method.
-type MatchResult struct {
-	// BranchPattern indicates the BranchPattern field of the
-	// BranchAccessControl object which matched the inputs.
-	BranchPattern string
-
-	// 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
-// place, and returns a MatchResult describing the access controls which should
-// be applied to the interactions.
-func Match(accessControls []BranchAccessControl, interactions MatchInteractions) (MatchResult, error) {
-	var res MatchResult
-
-	accessControls = append(accessControls, DefaultBranchAccessControls...)
-
-	// find the applicable BranchAccessControl
-	var branchAC BranchAccessControl
-	{
-		var ok bool
-		var err error
-		for i := range accessControls {
-			ok, err = doublestar.Match(accessControls[i].BranchPattern, interactions.Branch)
 		if err != nil {
-				return res, fmt.Errorf("matching branch %q to branch_pattern %q: %w",
+			return "", fmt.Errorf("casting %+v to a FilterInterface: %w", filter, err)
-					accessControls[i].BranchPattern, interactions.Branch, err)
+
-			} else if ok {
+		} else if err := filterI.MatchCommit(req); errors.As(err, new(ErrFilterNoMatch)) {
-				branchAC = accessControls[i]
+			return ActionNext, nil
-				break
+
+		} else if err != nil {
+			// ignore the error here, if we could get the FilterInterface then
+			// we should be able to get the type.
+			filterTypeStr, _ := filter.Type()
+			return "", fmt.Errorf("matching commit using filter of type %q: %w", filterTypeStr, err)
 		}
 	}
-		if !ok {
+	return ac.Action, nil
-			panic(fmt.Sprintf("no patterns matched branch %q, which shouldn't be possible", interactions.Branch))
-		}
-		res.BranchPattern = branchAC.BranchPattern
 }
 
-	// determine ChangeAccessControl for each path in FilesChanged
+// ErrCommitRequestDenied is returned from AssertCanCommit when a particular
-	{
+// AccessControl has explicitly disallowed the CommitRequest.
-		changeACs := append(branchAC.ChangeAccessControls, DefaultChangeAccessControl)
+type ErrCommitRequestDenied struct {
-		acToPaths := map[ChangeAccessControl][]string{}
+	By AccessControl
-		for _, path := range interactions.FilePathsChanged {
-			var ok bool
-			var err error
-			for _, ac := range changeACs {
-				if ok, err = doublestar.PathMatch(ac.FilePathPattern, path); err != nil {
-					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)
-					break
-				}
-			}
-			if !ok {
-				panic(fmt.Sprintf("no patterns matched change path %q, which shouldn't be possible", path))
-			}
-		}
-		for ac, paths := range acToPaths {
-			res.ChangeAccessControls = append(res.ChangeAccessControls, MatchedChangeAccessControl{
-				ChangeAccessControl: ac,
-				FilePaths:           paths,
-			})
 }
 
-		// sort result for determinancy
+func (e ErrCommitRequestDenied) Error() string {
-		sort.Slice(res.ChangeAccessControls, func(i, j int) bool {
+	acB, err := yaml.Marshal(e.By)
-			pi := res.ChangeAccessControls[i].ChangeAccessControl.FilePathPattern
+	if err != nil {
-			pj := res.ChangeAccessControls[j].ChangeAccessControl.FilePathPattern
+		panic(err)
-			return pi < pj
+	}
-		})
+	return fmt.Sprintf("commit matched and denied by this access control:\n%s", string(acB))
 }
 
-	// Handle CredentialAccessControl, if applicable
+// AssertCanCommit asserts that the given CommitRequest is allowed by the given
-	if interactions.CredentialAdded {
+// AccessControls.
-		credAC := branchAC.CredentialAccessControl
+func AssertCanCommit(acl []AccessControl, req CommitRequest) error {
-		if credAC == nil {
+	acl = append(acl, DefaultAccessControls...)
-			credAC = &DefaultCredentialAccessControl
+	for _, ac := range acl {
+		action, err := ac.ActionForCommit(req)
+		if err != nil {
+			return err
 		}
-
+		switch action {
-		res.CredentialAccessControl = &MatchedCredentialAccessControl{
+		case ActionNext:
-			CredentialAccessControl: *credAC,
+			continue
+		case ActionAllow:
+			return nil
+		case ActionDeny:
+			return ErrCommitRequestDenied{By: ac}
+		default:
+			return fmt.Errorf("invalid action %q", action)
 		}
 	}
 
-	return res, nil
+	panic("should not be able to get here")
 }

accessctl/access_control_test.go

@@ -1,222 +1,143 @@
 package accessctl
 
 import (
-	"reflect"
+	"dehub/sigcred"
+	"errors"
 	"testing"
-
-	"github.com/davecgh/go-spew/spew"
 )
 
-func normalizeResult(res MatchResult) MatchResult {
+func TestAssertCanCommit(t *testing.T) {
-	if len(res.ChangeAccessControls) == 0 {
-		res.ChangeAccessControls = nil
-	}
-	return res
-}
-
-func TestMatch(t *testing.T) {
-	secondCond := Condition{
-		Signature: &ConditionSignature{
-			AnyAccount: true,
-			Count:      "2",
-		},
-	}
-
 	tests := []struct {
 		descr   string
-
+		acl     []AccessControl
-		branchACs    []BranchAccessControl
+		req     CommitRequest
-		interactions MatchInteractions
+		allowed bool
-		result       MatchResult
 	}{
 		{
-			descr: "empty input empty result",
+			descr: "first allows",
-			result: MatchResult{
+			acl: []AccessControl{
-				BranchPattern: "**",
+				{
-			},
+					Action: ActionAllow,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "foo"},
+					}},
 				},
 				{
-			descr: "empty access controls",
+					Action: ActionDeny,
-			interactions: MatchInteractions{
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "foo"},
+					}},
+				},
+			},
+			req:     CommitRequest{Type: "foo"},
+			allowed: true,
+		},
+		{
+			descr: "first denies",
+			acl: []AccessControl{
+				{
+					Action: ActionDeny,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "foo"},
+					}},
+				},
+				{
+					Action: ActionAllow,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "foo"},
+					}},
+				},
+			},
+			req:     CommitRequest{Type: "foo"},
+			allowed: false,
+		},
+		{
+			descr: "second allows",
+			acl: []AccessControl{
+				{
+					Action: ActionDeny,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "bar"},
+					}},
+				},
+				{
+					Action: ActionAllow,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "foo"},
+					}},
+				},
+			},
+			req:     CommitRequest{Type: "foo"},
+			allowed: true,
+		},
+		{
+			descr: "second denies",
+			acl: []AccessControl{
+				{
+					Action: ActionDeny,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "bar"},
+					}},
+				},
+				{
+					Action: ActionDeny,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "foo"},
+					}},
+				},
+			},
+			req:     CommitRequest{Type: "foo"},
+			allowed: false,
+		},
+		{
+			descr: "default allows",
+			acl: []AccessControl{
+				{
+					Action: ActionDeny,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "bar"},
+					}},
+				},
+			},
+			req: CommitRequest{
+				Branch: "not_main",
+				Type:   "foo",
+				Credentials: []sigcred.Credential{{
+					PGPSignature: new(sigcred.CredentialPGPSignature),
+					AccountID:    "a",
+				}},
+			},
+			allowed: true,
+		},
+		{
+			descr: "default denies",
+			acl: []AccessControl{
+				{
+					Action: ActionDeny,
+					Filters: []Filter{{
+						CommitType: &FilterCommitType{Type: "bar"},
+					}},
+				},
+			},
+			req: CommitRequest{
 				Branch: "main",
-				FilePathsChanged: []string{"foo", "bar"},
+				Type:   "foo",
-			},
+				Credentials: []sigcred.Credential{{
-			result: MatchResult{
+					PGPSignature: new(sigcred.CredentialPGPSignature),
-				BranchPattern: "main",
+					AccountID:    "a",
-				ChangeAccessControls: []MatchedChangeAccessControl{
-					{
-						ChangeAccessControl: DefaultChangeAccessControl,
-						FilePaths:           []string{"foo", "bar"},
-					},
-				},
-			},
-		},
-		{
-			descr:        "empty filesPathsChanged",
-			branchACs:    DefaultBranchAccessControls,
-			interactions: MatchInteractions{Branch: "main"},
-			result:       MatchResult{BranchPattern: "main"},
-		},
-		{
-			descr: "no matching branch patterns",
-			branchACs: []BranchAccessControl{{
-				BranchPattern: "dunk",
-				ChangeAccessControls: []ChangeAccessControl{{
-					FilePathPattern: "**",
-					Condition:       secondCond,
-				}},
-			}},
-			interactions: MatchInteractions{
-				Branch:           "crunk",
-				FilePathsChanged: []string{"foo"},
-			},
-			result: MatchResult{
-				BranchPattern: "**",
-				ChangeAccessControls: []MatchedChangeAccessControl{{
-					ChangeAccessControl: DefaultChangeAccessControl,
-					FilePaths:           []string{"foo"},
 				}},
 			},
-		},
+			allowed: false,
-		{
-			descr: "no matching files",
-			branchACs: []BranchAccessControl{{
-				BranchPattern: "main",
-				ChangeAccessControls: []ChangeAccessControl{{
-					FilePathPattern: "boo",
-					Condition:       secondCond,
-				}},
-			}},
-			interactions: MatchInteractions{
-				Branch:           "main",
-				FilePathsChanged: []string{"foo"},
-			},
-			result: MatchResult{
-				BranchPattern: "main",
-				ChangeAccessControls: []MatchedChangeAccessControl{{
-					ChangeAccessControl: DefaultChangeAccessControl,
-					FilePaths:           []string{"foo"},
-				}},
-			},
-		},
-		{
-			descr: "branch pattern precedent",
-			branchACs: []BranchAccessControl{
-				{
-					BranchPattern: "main",
-					ChangeAccessControls: []ChangeAccessControl{{
-						FilePathPattern: "foo",
-						Condition:       secondCond,
-					}},
-				},
-				{
-					BranchPattern: "**",
-					ChangeAccessControls: []ChangeAccessControl{
-						DefaultChangeAccessControl,
-					},
-				},
-			},
-			interactions: MatchInteractions{
-				Branch:           "main",
-				FilePathsChanged: []string{"foo"},
-			},
-			result: MatchResult{
-				BranchPattern: "main",
-				ChangeAccessControls: []MatchedChangeAccessControl{{
-					ChangeAccessControl: ChangeAccessControl{
-						FilePathPattern: "foo",
-						Condition:       secondCond,
-					},
-					FilePaths: []string{"foo"},
-				}},
-			},
-		},
-		{
-			descr: "multiple files matching FilePathPatterns",
-			branchACs: []BranchAccessControl{{
-				BranchPattern: "main",
-				ChangeAccessControls: []ChangeAccessControl{{
-					FilePathPattern: "foo*",
-					Condition:       secondCond,
-				}},
-			}},
-			interactions: MatchInteractions{
-				Branch:           "main",
-				FilePathsChanged: []string{"foo_a", "bar", "foo_b"},
-			},
-			result: MatchResult{
-				BranchPattern: "main",
-				ChangeAccessControls: []MatchedChangeAccessControl{
-					{
-						ChangeAccessControl: DefaultChangeAccessControl,
-						FilePaths:           []string{"bar"},
-					},
-					{
-						ChangeAccessControl: ChangeAccessControl{
-							FilePathPattern: "foo*",
-							Condition:       secondCond,
-						},
-						FilePaths: []string{"foo_a", "foo_b"},
-					},
-				},
-			},
-		},
-		{
-			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 {
 		t.Run(test.descr, func(t *testing.T) {
-			res, err := Match(test.branchACs, test.interactions)
+			err := AssertCanCommit(test.acl, test.req)
-			if err != nil {
+			if test.allowed && err != nil {
-				t.Fatalf("error matching: %v", err)
+				t.Fatalf("expected to be allowed but got: %v", err)
-			}
+			} else if !test.allowed && !errors.As(err, new(ErrCommitRequestDenied)) {
-			res, expRes := normalizeResult(res), normalizeResult(test.result)
+				t.Fatalf("expected to be denied but got: %v", err)
-			if !reflect.DeepEqual(res, expRes) {
-				t.Fatalf("expected:%s\ngot:    %s", spew.Sdump(expRes), spew.Sdump(res))
 			}
 		})
 	}

accessctl/condition.go

@@ -1,140 +0,0 @@
-package accessctl
-
-import (
-	"dehub/sigcred"
-	"dehub/typeobj"
-	"errors"
-	"fmt"
-	"math"
-	"strconv"
-	"strings"
-)
-
-// ConditionInterface describes the methods that all Signifiers must implement.
-type ConditionInterface interface {
-
-	// Satisfied asserts that the Condition is satisfied by the given set of
-	// Credentials. If it is not (or something else went wrong) then an error is
-	// returned.
-	//
-	// NOTE that Satisfied assumes the Credential has already been Verify'd.
-	Satisfied([]sigcred.Credential) error
-}
-
-// 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"`
-}
-
-// MarshalYAML implements the yaml.Marshaler interface.
-func (c Condition) MarshalYAML() (interface{}, error) {
-	return typeobj.MarshalYAML(c)
-}
-
-// UnmarshalYAML implements the yaml.Unmarshaler interface.
-func (c *Condition) UnmarshalYAML(unmarshal func(interface{}) error) error {
-	return typeobj.UnmarshalYAML(c, unmarshal)
-}
-
-// Interface returns the ConditionInterface encapsulated by this Condition
-// object.
-func (c Condition) Interface() (ConditionInterface, error) {
-	el, _, err := typeobj.Element(c)
-	if err != nil {
-		return nil, err
-	}
-	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 AnyAccount must be filled in.
-type ConditionSignature struct {
-	AccountIDs []string `yaml:"account_ids,omitempty"`
-	AnyAccount bool     `yaml:"any_account,omitempty"`
-	Count      string   `yaml:"count"`
-}
-
-var _ ConditionInterface = ConditionSignature{}
-
-func (condSig ConditionSignature) targetNum() (int, error) {
-	if !strings.HasSuffix(condSig.Count, "%") {
-		return strconv.Atoi(condSig.Count)
-	} else if condSig.AnyAccount {
-		return 0, errors.New("cannot use AnyAccount and a percent Count together")
-	}
-
-	percentStr := strings.TrimRight(condSig.Count, "%")
-	percent, err := strconv.ParseFloat(percentStr, 64)
-	if err != nil {
-		return 0, fmt.Errorf("could not parse Count as percent %q: %w", condSig.Count, err)
-	}
-	targetF := float64(len(condSig.AccountIDs)) * percent / 100
-	targetF = math.Ceil(targetF)
-	return int(targetF), nil
-}
-
-// ErrConditionSignatureUnsatisfied is returned from ConditionSignature's
-// Satisfied method when the Condition has not been satisfied.
-type ErrConditionSignatureUnsatisfied struct {
-	TargetNumAccounts, NumAccounts int
-}
-
-func (err ErrConditionSignatureUnsatisfied) Error() string {
-	return fmt.Sprintf("not enough valid signature credentials, requires %d but only had %d",
-		err.TargetNumAccounts, err.NumAccounts)
-}
-
-// Satisfied asserts that the given Credentials contains enough signatures to be
-// satisfied.
-func (condSig ConditionSignature) Satisfied(creds []sigcred.Credential) error {
-	targetN, err := condSig.targetNum()
-	if err != nil {
-		return fmt.Errorf("could not compute ConditionSignature target number of accounts: %w", err)
-	}
-
-	credAccountIDs := map[string]struct{}{}
-	for _, cred := range creds {
-		// TODO currently only signature credentials are implemented, so we can
-		// just assume that the given AccountID has provided a sig. In the
-		// future this may not be true.
-		credAccountIDs[cred.AccountID] = struct{}{}
-	}
-
-	var n int
-	if condSig.AnyAccount {
-		// TODO this doesn't actually check that the accounts are defined in the
-		// Config.
-		n = len(credAccountIDs)
-	} else {
-		targetAccountIDs := map[string]struct{}{}
-		for _, accountID := range condSig.AccountIDs {
-			targetAccountIDs[accountID] = struct{}{}
-		}
-		for accountID := range targetAccountIDs {
-			if _, ok := credAccountIDs[accountID]; ok {
-				n++
-			}
-		}
-	}
-
-	if n < targetN {
-		return ErrConditionSignatureUnsatisfied{
-			TargetNumAccounts: targetN,
-			NumAccounts:       n,
-		}
-	}
-	return nil
-}

accessctl/condition_test.go

@@ -1,110 +0,0 @@
-package accessctl
-
-import (
-	"dehub/sigcred"
-	"reflect"
-	"testing"
-)
-
-func TestConditionSignatureSatisfied(t *testing.T) {
-	tests := []struct {
-		descr          string
-		cond           ConditionSignature
-		credAccountIDs []string
-		err            error
-	}{
-		{
-			descr: "no cred accounts",
-			cond: ConditionSignature{
-				AnyAccount: true,
-				Count:      "1",
-			},
-			err: ErrConditionSignatureUnsatisfied{
-				TargetNumAccounts: 1,
-				NumAccounts:       0,
-			},
-		},
-		{
-			descr: "one cred account",
-			cond: ConditionSignature{
-				AnyAccount: true,
-				Count:      "1",
-			},
-			credAccountIDs: []string{"foo"},
-		},
-		{
-			descr: "one matching cred account",
-			cond: ConditionSignature{
-				AccountIDs: []string{"foo", "bar"},
-				Count:      "1",
-			},
-			credAccountIDs: []string{"foo"},
-		},
-		{
-			descr: "no matching cred account",
-			cond: ConditionSignature{
-				AccountIDs: []string{"foo", "bar"},
-				Count:      "1",
-			},
-			credAccountIDs: []string{"baz"},
-			err: ErrConditionSignatureUnsatisfied{
-				TargetNumAccounts: 1,
-				NumAccounts:       0,
-			},
-		},
-		{
-			descr: "two matching cred accounts",
-			cond: ConditionSignature{
-				AccountIDs: []string{"foo", "bar"},
-				Count:      "2",
-			},
-			credAccountIDs: []string{"foo", "bar"},
-		},
-		{
-			descr: "one matching cred account, missing one",
-			cond: ConditionSignature{
-				AccountIDs: []string{"foo", "bar"},
-				Count:      "2",
-			},
-			credAccountIDs: []string{"foo", "baz"},
-			err: ErrConditionSignatureUnsatisfied{
-				TargetNumAccounts: 2,
-				NumAccounts:       1,
-			},
-		},
-		{
-			descr: "50 percent matching cred accounts",
-			cond: ConditionSignature{
-				AccountIDs: []string{"foo", "bar", "baz"},
-				Count:      "50%",
-			},
-			credAccountIDs: []string{"foo", "bar"},
-		},
-		{
-			descr: "not 50 percent matching cred accounts",
-			cond: ConditionSignature{
-				AccountIDs: []string{"foo", "bar", "baz"},
-				Count:      "50%",
-			},
-			credAccountIDs: []string{"foo"},
-			err: ErrConditionSignatureUnsatisfied{
-				TargetNumAccounts: 2,
-				NumAccounts:       1,
-			},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.descr, func(t *testing.T) {
-			creds := make([]sigcred.Credential, len(test.credAccountIDs))
-			for i := range test.credAccountIDs {
-				creds[i].AccountID = test.credAccountIDs[i]
-			}
-
-			err := test.cond.Satisfied(creds)
-			if !reflect.DeepEqual(err, test.err) {
-				t.Fatalf("Satisfied returned %#v\nexpected %#v", err, test.err)
-			}
-		})
-	}
-}

accessctl/filter.go

@@ -0,0 +1,102 @@
+package accessctl
+
+import (
+	"dehub/typeobj"
+	"errors"
+	"fmt"
+)
+
+// ErrFilterNoMatch is returned from a FilterInterface's Match method when the
+// given request was not matched to the filter due to the request itself (as
+// opposed to some error in the filter's definition).
+type ErrFilterNoMatch struct {
+	Err error
+}
+
+func (err ErrFilterNoMatch) Error() string {
+	return fmt.Sprintf("matching with filter: %s", err.Err.Error())
+}
+
+// FilterInterface describes the methods that all Filters must implement.
+type FilterInterface interface {
+	// MatchCommit returns nil if the CommitRequest is matched by the filter,
+	// otherwise it returns an error (ErrFilterNoMatch if the error is due to
+	// the CommitRequest).
+	MatchCommit(CommitRequest) error
+}
+
+// Filter represents an access control filter being defined in the Config. Only
+// one of its fields may be filled at a time.
+type Filter struct {
+	Signature    *FilterSignature    `type:"signature"`
+	Branch       *FilterBranch       `type:"branch"`
+	FilesChanged *FilterFilesChanged `type:"files_changed"`
+	CommitType   *FilterCommitType   `type:"commit_type"`
+	Not          *FilterNot          `type:"not"`
+}
+
+// MarshalYAML implements the yaml.Marshaler interface.
+func (f Filter) MarshalYAML() (interface{}, error) {
+	return typeobj.MarshalYAML(f)
+}
+
+// UnmarshalYAML implements the yaml.Unmarshaler interface.
+func (f *Filter) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	return typeobj.UnmarshalYAML(f, unmarshal)
+}
+
+// Interface returns the FilterInterface encapsulated by this Filter.
+func (f Filter) Interface() (FilterInterface, error) {
+	el, _, err := typeobj.Element(f)
+	if err != nil {
+		return nil, err
+	}
+	return el.(FilterInterface), nil
+}
+
+// Type returns a string describing what type of Filter this object
+// encapsulates, based on which of its fields are filled in.
+func (f Filter) Type() (string, error) {
+	_, typeStr, err := typeobj.Element(f)
+	if err != nil {
+		return "", err
+	}
+	return typeStr, nil
+}
+
+// FilterCommitType filters by what type of commit is being requested. Exactly
+// one of its fields should be filled.
+type FilterCommitType struct {
+	Type  string   `yaml:"commit_type"`
+	Types []string `yaml:"commit_types"`
+}
+
+var _ FilterInterface = FilterCommitType{}
+
+// MatchCommit implements the method for FilterInterface.
+func (f FilterCommitType) MatchCommit(req CommitRequest) error {
+	switch {
+	case f.Type != "":
+		if f.Type != req.Type {
+			return ErrFilterNoMatch{
+				Err: fmt.Errorf("commit type %q does not match filter's type %q",
+					req.Type, f.Type),
+			}
+		}
+		return nil
+
+	case len(f.Types) > 0:
+		for _, typ := range f.Types {
+			if typ == req.Type {
+				return nil
+			}
+		}
+		return ErrFilterNoMatch{
+			Err: fmt.Errorf("commit type %q does not match any of filter's types %+v",
+				req.Type, f.Types),
+		}
+
+	default:
+		return errors.New(`one of the following fields must be set: "commit_type", "commit_types"`)
+	}
+}

accessctl/filter_logical.go

@@ -0,0 +1,31 @@
+package accessctl
+
+import (
+	"errors"
+	"fmt"
+)
+
+// FilterNot wraps another Filter. If that filter matches, FilterNot does not
+// match, and vice-versa.
+type FilterNot struct {
+	Filter Filter `yaml:"filter"`
+}
+
+var _ FilterInterface = FilterNot{}
+
+// MatchCommit implements the method for FilterInterface.
+func (f FilterNot) MatchCommit(req CommitRequest) error {
+	fI, err := f.Filter.Interface()
+	if err != nil {
+		return fmt.Errorf("casting %+v to a FilterInterface: %w", f.Filter, err)
+
+	} else if err := fI.MatchCommit(req); errors.As(err, new(ErrFilterNoMatch)) {
+		return nil
+	} else if err != nil {
+		return err
+	}
+	return ErrFilterNoMatch{Err: errors.New("sub-filter did match")}
+}
+
+// TODO FilterAll
+// TODO FilterAny

accessctl/filter_logical_test.go

@@ -0,0 +1,32 @@
+package accessctl
+
+import "testing"
+
+func TestFilterNot(t *testing.T) {
+	runCommitMatchTests(t, []filterCommitMatchTest{
+		{
+			descr: "sub-filter does match",
+			filter: FilterNot{
+				Filter: Filter{
+					CommitType: &FilterCommitType{Type: "foo"},
+				},
+			},
+			req: CommitRequest{
+				Type: "foo",
+			},
+			match: false,
+		},
+		{
+			descr: "sub-filter does not match",
+			filter: FilterNot{
+				Filter: Filter{
+					CommitType: &FilterCommitType{Type: "foo"},
+				},
+			},
+			req: CommitRequest{
+				Type: "bar",
+			},
+			match: true,
+		},
+	})
+}

accessctl/filter_pattern.go

@@ -0,0 +1,92 @@
+package accessctl
+
+import (
+	"errors"
+	"fmt"
+
+	"github.com/bmatcuk/doublestar"
+)
+
+// StringMatcher is used to match against a string. It can use one of several
+// methods to match. Only one field should be filled at a time.
+type StringMatcher struct {
+	// Pattern, if set, indicates that the Match method should succeed if this
+	// doublestar pattern matches against the string.
+	Pattern string `yaml:"pattern,omitempty"`
+
+	// Patterns, if set, indicates that the Match method should succeed if at
+	// least one of these doublestar patterns matches against the string.
+	Patterns []string `yaml:"patterns,omitempty"`
+}
+
+func doublestarMatch(pattern, str string) (bool, error) {
+	ok, err := doublestar.Match(pattern, str)
+	if err != nil {
+		return false, fmt.Errorf("matching %q on pattern %q: %w",
+			str, pattern, err)
+	}
+	return ok, nil
+}
+
+// Match operates similarly to the Match method of the FilterInterface, except
+// it only takes in strings.
+func (m StringMatcher) Match(str string) error {
+	switch {
+	case m.Pattern != "":
+		if ok, err := doublestarMatch(m.Pattern, str); err != nil {
+			return err
+		} else if !ok {
+			return ErrFilterNoMatch{
+				Err: fmt.Errorf("pattern %q does not match %q", m.Pattern, str),
+			}
+		}
+		return nil
+
+	case len(m.Patterns) > 0:
+		for _, pattern := range m.Patterns {
+			if ok, err := doublestarMatch(pattern, str); err != nil {
+				return err
+			} else if ok {
+				return nil
+			}
+		}
+		return ErrFilterNoMatch{
+			Err: fmt.Errorf("no patterns in %+v match %q", m.Patterns, str),
+		}
+
+	default:
+		return errors.New(`one of the following fields must be set: "pattern", "patterns"`)
+	}
+}
+
+// FilterBranch matches a CommitRequest's Branch field using a double-star
+// pattern.
+type FilterBranch struct {
+	StringMatcher StringMatcher `yaml:",inline"`
+}
+
+var _ FilterInterface = FilterBranch{}
+
+// MatchCommit implements the method for FilterInterface.
+func (f FilterBranch) MatchCommit(req CommitRequest) error {
+	return f.StringMatcher.Match(req.Branch)
+}
+
+// FilterFilesChanged matches a CommitRequest's FilesChanged field using a
+// double-star pattern. It only matches if all of the CommitRequest's
+// FilesChanged match.
+type FilterFilesChanged struct {
+	StringMatcher StringMatcher `yaml:",inline"`
+}
+
+var _ FilterInterface = FilterFilesChanged{}
+
+// MatchCommit implements the method for FilterInterface.
+func (f FilterFilesChanged) MatchCommit(req CommitRequest) error {
+	for _, path := range req.FilesChanged {
+		if err := f.StringMatcher.Match(path); err != nil {
+			return err
+		}
+	}
+	return nil
+}

accessctl/filter_pattern_test.go

@@ -0,0 +1,134 @@
+package accessctl
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestStringMatcher(t *testing.T) {
+	tests := []struct {
+		descr   string
+		matcher StringMatcher
+		str     string
+		match   bool
+	}{
+		// Pattern
+		{
+			descr: "pattern exact match",
+			matcher: StringMatcher{
+				Pattern: "foo",
+			},
+			str:   "foo",
+			match: true,
+		},
+		{
+			descr: "pattern exact no match",
+			matcher: StringMatcher{
+				Pattern: "foo",
+			},
+			str:   "bar",
+			match: false,
+		},
+		{
+			descr: "pattern single star match",
+			matcher: StringMatcher{
+				Pattern: "foo/*",
+			},
+			str:   "foo/bar",
+			match: true,
+		},
+		{
+			descr: "pattern single star no match 1",
+			matcher: StringMatcher{
+				Pattern: "foo/*",
+			},
+			str:   "foo",
+			match: false,
+		},
+		{
+			descr: "pattern single star no match 2",
+			matcher: StringMatcher{
+				Pattern: "foo/*",
+			},
+			str:   "foo/bar/baz",
+			match: false,
+		},
+		{
+			descr: "pattern double star match 1",
+			matcher: StringMatcher{
+				Pattern: "foo/**",
+			},
+			str:   "foo/bar",
+			match: true,
+		},
+		{
+			descr: "pattern double star match 2",
+			matcher: StringMatcher{
+				Pattern: "foo/**",
+			},
+			str:   "foo/bar/baz",
+			match: true,
+		},
+		{
+			descr: "pattern double star no match",
+			matcher: StringMatcher{
+				Pattern: "foo/**",
+			},
+			str:   "foo",
+			match: false,
+		},
+
+		// Patterns, assumes individual pattern matching works correctly
+		{
+			descr: "patterns single match",
+			matcher: StringMatcher{
+				Patterns: []string{"foo"},
+			},
+			str:   "foo",
+			match: true,
+		},
+		{
+			descr: "patterns single no match",
+			matcher: StringMatcher{
+				Patterns: []string{"foo"},
+			},
+			str:   "bar",
+			match: false,
+		},
+		{
+			descr: "patterns multi first match",
+			matcher: StringMatcher{
+				Patterns: []string{"foo", "bar"},
+			},
+			str:   "foo",
+			match: true,
+		},
+		{
+			descr: "patterns multi second match",
+			matcher: StringMatcher{
+				Patterns: []string{"foo", "bar"},
+			},
+			str:   "bar",
+			match: true,
+		},
+		{
+			descr: "patterns multi no match",
+			matcher: StringMatcher{
+				Patterns: []string{"foo", "bar"},
+			},
+			str:   "baz",
+			match: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.descr, func(t *testing.T) {
+			err := test.matcher.Match(test.str)
+			if test.match && err != nil {
+				t.Fatalf("expected to match, got %v", err)
+			} else if !test.match && !errors.As(err, new(ErrFilterNoMatch)) {
+				t.Fatalf("expected ErrFilterNoMatch, got %#v", err)
+			}
+		})
+	}
+}

accessctl/filter_sig.go

@@ -0,0 +1,97 @@
+package accessctl
+
+import (
+	"errors"
+	"fmt"
+	"math"
+	"strconv"
+	"strings"
+)
+
+// 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.
+type FilterSignature struct {
+	AccountIDs []string `yaml:"account_ids,omitempty"`
+	AnyAccount bool     `yaml:"any_account,omitempty"`
+	Count      string   `yaml:"count"`
+}
+
+var _ FilterInterface = FilterSignature{}
+
+func (f FilterSignature) targetNum() (int, error) {
+	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")
+	}
+
+	percentStr := strings.TrimRight(f.Count, "%")
+	percent, err := strconv.ParseFloat(percentStr, 64)
+	if err != nil {
+		return 0, fmt.Errorf("could not parse Count as percent %q: %w", f.Count, err)
+	}
+	target := float64(len(f.AccountIDs)) * percent / 100
+	target = math.Ceil(target)
+	return int(target), nil
+}
+
+// ErrFilterSignatureUnsatisfied is returned from FilterSignature's
+// Match method when the filter has not been satisfied.
+type ErrFilterSignatureUnsatisfied struct {
+	TargetNumAccounts, NumAccounts int
+}
+
+func (err ErrFilterSignatureUnsatisfied) Error() string {
+	return fmt.Sprintf("not enough valid signature credentials, filter requires %d but only had %d",
+		err.TargetNumAccounts, err.NumAccounts)
+}
+
+// MatchCommit returns true if the CommitRequest contains a sufficient number of
+// signature Credentials.
+func (f FilterSignature) MatchCommit(req CommitRequest) error {
+	targetN, err := f.targetNum()
+	if err != nil {
+		return fmt.Errorf("computing target number of accounts: %w", err)
+	}
+
+	credAccountIDs := map[string]struct{}{}
+	for _, cred := range req.Credentials {
+		// TODO support other kinds of signatures
+		if cred.PGPSignature == nil {
+			continue
+		}
+		credAccountIDs[cred.AccountID] = struct{}{}
+	}
+
+	var n int
+	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
+		// Credential, but once that's not the case this will need to be
+		// revisited.
+		n = len(credAccountIDs)
+	} else {
+		targetAccountIDs := map[string]struct{}{}
+		for _, accountID := range f.AccountIDs {
+			targetAccountIDs[accountID] = struct{}{}
+		}
+		for accountID := range targetAccountIDs {
+			if _, ok := credAccountIDs[accountID]; ok {
+				n++
+			}
+		}
+	}
+
+	if n >= targetN {
+		return nil
+	}
+	return ErrFilterNoMatch{
+		Err: ErrFilterSignatureUnsatisfied{
+			NumAccounts:       n,
+			TargetNumAccounts: targetN,
+		},
+	}
+}

accessctl/filter_sig_test.go

@@ -0,0 +1,103 @@
+package accessctl
+
+import (
+	"dehub/sigcred"
+	"testing"
+)
+
+func TestFilterSignature(t *testing.T) {
+	mkReq := func(accountIDs ...string) CommitRequest {
+		creds := make([]sigcred.Credential, len(accountIDs))
+		for i := range accountIDs {
+			creds[i].PGPSignature = new(sigcred.CredentialPGPSignature)
+			creds[i].AccountID = accountIDs[i]
+		}
+		return CommitRequest{Credentials: creds}
+	}
+
+	runCommitMatchTests(t, []filterCommitMatchTest{
+		{
+			descr: "no cred accounts",
+			filter: FilterSignature{
+				AnyAccount: true,
+				Count:      "1",
+			},
+			matchErr: ErrFilterSignatureUnsatisfied{
+				TargetNumAccounts: 1,
+				NumAccounts:       0,
+			},
+		},
+		{
+			descr: "one cred account",
+			filter: FilterSignature{
+				AnyAccount: true,
+				Count:      "1",
+			},
+			req:   mkReq("foo"),
+			match: true,
+		},
+		{
+			descr: "one matching cred account",
+			filter: FilterSignature{
+				AccountIDs: []string{"foo", "bar"},
+				Count:      "1",
+			},
+			req:   mkReq("foo"),
+			match: true,
+		},
+		{
+			descr: "no matching cred account",
+			filter: FilterSignature{
+				AccountIDs: []string{"foo", "bar"},
+				Count:      "1",
+			},
+			req: mkReq("baz"),
+			matchErr: ErrFilterSignatureUnsatisfied{
+				TargetNumAccounts: 1,
+				NumAccounts:       0,
+			},
+		},
+		{
+			descr: "two matching cred accounts",
+			filter: FilterSignature{
+				AccountIDs: []string{"foo", "bar"},
+				Count:      "2",
+			},
+			req:   mkReq("foo", "bar"),
+			match: true,
+		},
+		{
+			descr: "one matching cred account, missing one",
+			filter: FilterSignature{
+				AccountIDs: []string{"foo", "bar"},
+				Count:      "2",
+			},
+			req: mkReq("foo", "baz"),
+			matchErr: ErrFilterSignatureUnsatisfied{
+				TargetNumAccounts: 2,
+				NumAccounts:       1,
+			},
+		},
+		{
+			descr: "50 percent matching cred accounts",
+			filter: FilterSignature{
+				AccountIDs: []string{"foo", "bar", "baz"},
+				Count:      "50%",
+			},
+			req:   mkReq("foo", "bar"),
+			match: true,
+		},
+		{
+			descr: "not 50 percent matching cred accounts",
+			filter: FilterSignature{
+				AccountIDs: []string{"foo", "bar", "baz"},
+				Count:      "50%",
+			},
+			req: mkReq("foo"),
+			matchErr: ErrFilterSignatureUnsatisfied{
+				TargetNumAccounts: 2,
+				NumAccounts:       1,
+			},
+		},
+	})
+}

accessctl/filter_test.go

@@ -0,0 +1,88 @@
+package accessctl
+
+import (
+	"errors"
+	"reflect"
+	"testing"
+)
+
+type filterCommitMatchTest struct {
+	descr  string
+	filter FilterInterface
+	req    CommitRequest
+	match  bool
+
+	// assumes match == true, and will ensure that the returned wrapped error is
+	// this one.
+	matchErr error
+}
+
+func runCommitMatchTests(t *testing.T, tests []filterCommitMatchTest) {
+	for _, test := range tests {
+		t.Run(test.descr, func(t *testing.T) {
+			err := test.filter.MatchCommit(test.req)
+			shouldMatch := test.match && test.matchErr == nil
+			if shouldMatch && err != nil {
+				t.Fatalf("expected to match, got %v", err)
+
+			} else if shouldMatch {
+				return
+
+			} else if fErr := new(ErrFilterNoMatch); !errors.As(err, fErr) {
+				t.Fatalf("expected ErrFilterNoMatch, got %#v", err)
+
+			} else if test.matchErr != nil && !reflect.DeepEqual(fErr.Err, test.matchErr) {
+				t.Fatalf("expected err %#v, not %#v", test.matchErr, fErr.Err)
+			}
+		})
+	}
+}
+
+func TestFilterCommitType(t *testing.T) {
+	mkReq := func(commitType string) CommitRequest {
+		return CommitRequest{Type: commitType}
+	}
+
+	runCommitMatchTests(t, []filterCommitMatchTest{
+		{
+			descr: "single match",
+			filter: FilterCommitType{
+				Type: "foo",
+			},
+			req:   mkReq("foo"),
+			match: true,
+		},
+		{
+			descr: "single no match",
+			filter: FilterCommitType{
+				Type: "foo",
+			},
+			req:   mkReq("bar"),
+			match: false,
+		},
+		{
+			descr: "multi match first",
+			filter: FilterCommitType{
+				Types: []string{"foo", "bar"},
+			},
+			req:   mkReq("foo"),
+			match: true,
+		},
+		{
+			descr: "multi match second",
+			filter: FilterCommitType{
+				Types: []string{"foo", "bar"},
+			},
+			req:   mkReq("bar"),
+			match: true,
+		},
+		{
+			descr: "multi no match",
+			filter: FilterCommitType{
+				Types: []string{"foo", "bar"},
+			},
+			req:   mkReq("baz"),
+			match: false,
+		},
+	})
+}

commit.go

@@ -10,7 +10,6 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"strings"
 	"time"
 
 	"gopkg.in/src-d/go-git.v4"
@@ -68,6 +67,15 @@ func (c Commit) Interface() (CommitInterface, error) {
 	return el.(CommitInterface), nil
 }
 
+// Type returns the Commit's type (as would be used in its YAML "type" field).
+func (c Commit) Type() (string, error) {
+	_, typeStr, err := typeobj.Element(c)
+	if err != nil {
+		return "", err
+	}
+	return typeStr, nil
+}
+
 // MarshalText implements the encoding.TextMarshaler interface by returning the
 // form the Commit object takes in the git commit message.
 func (c Commit) MarshalText() ([]byte, error) {
@@ -208,7 +216,7 @@ func (r *Repo) verificationCtx(h plumbing.Hash) (vctx verificationCtx, err error
 }
 
 func (r *Repo) assertAccessControls(
-	accessCtls []accessctl.BranchAccessControl,
+	acl []accessctl.AccessControl,
 	commit Commit, vctx verificationCtx, branch plumbing.ReferenceName,
 ) (err error) {
 	filesChanged, err := calcDiff(vctx.parentTree, vctx.commitTree)
@@ -225,42 +233,17 @@ func (r *Repo) assertAccessControls(
 		pathsChanged[i] = filesChanged[i].path
 	}
 
-	matchRes, err := accessctl.Match(accessCtls, accessctl.MatchInteractions{
+	commitType, err := commit.Type()
+	if err != nil {
+		return fmt.Errorf("determining type of commit %+v: %w", commit, err)
+	}
+
+	return accessctl.AssertCanCommit(acl, accessctl.CommitRequest{
+		Type:         commitType,
 		Branch:       branch.Short(),
-		FilePathsChanged: pathsChanged,
+		Credentials:  commit.Credentials,
-		CredentialAdded:  commit.Credential != nil,
+		FilesChanged: pathsChanged,
 	})
-	if err != nil {
-		return fmt.Errorf("determining applicable access controls: %w", err)
-	}
-
-	defer func() {
-		if err != nil {
-			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)
-		}
-	}
-
-	return nil
 }
 
 // VerifyCommits verifies that the given commits, which are presumably on the

commit_credential_test.go

@@ -1,11 +1,11 @@
 package dehub
 
 import (
-	"dehub/accessctl"
 	"dehub/sigcred"
 	"testing"
 
 	"gopkg.in/src-d/go-git.v4/plumbing"
+	yaml "gopkg.in/yaml.v2"
 )
 
 func TestCredentialCommitVerify(t *testing.T) {
@@ -22,43 +22,29 @@ func TestCredentialCommitVerify(t *testing.T) {
 	})
 
 	tootBranch := plumbing.NewBranchReferenceName("toot_branch")
-	tootBranchCond := accessctl.Condition{
+
-		Signature: &accessctl.ConditionSignature{
+	err := yaml.Unmarshal([]byte(`
-			AccountIDs: []string{"root", "toot"},
+- action: allow
-			Count:      "1",
+  filters:
-		},
+  - type: branch
-	}
+    pattern: `+tootBranch.Short()+`
-	allBranchCond := accessctl.Condition{
+  - type: signature
-		Signature: &accessctl.ConditionSignature{
+    count: 1
-			AccountIDs: []string{"root"},
+    account_ids:
-			Count:      "1",
+    - root
-		},
+    - toot
-	}
+
-	h.cfg.AccessControls = []accessctl.BranchAccessControl{
+- action: allow
-		{
+  filters:
-			BranchPattern: tootBranch.Short(),
+  - type: signature
-			ChangeAccessControls: []accessctl.ChangeAccessControl{
+    count: 1
-				{
+    account_ids:
-					FilePathPattern: "**",
+    - root
-					Condition:       tootBranchCond,
+
-				},
+- action: deny
-			},
+`), &h.cfg.AccessControls)
-			CredentialAccessControl: &accessctl.CredentialAccessControl{
+	if err != nil {
-				Condition: tootBranchCond,
+		t.Fatal(err)
-			},
-		},
-		{
-			BranchPattern: "**",
-			ChangeAccessControls: []accessctl.ChangeAccessControl{
-				{
-					FilePathPattern: "**",
-					Condition:       allBranchCond,
-				},
-			},
-			CredentialAccessControl: &accessctl.CredentialAccessControl{
-				Condition: allBranchCond,
-			},
-		},
 	}
 	h.stageCfg()
 	rootGitCommit := h.changeCommit("initial commit", h.cfg.Accounts[0].ID, h.sig)

commit_test.go

@@ -24,8 +24,6 @@ func TestConfigChange(t *testing.T) {
 			Body: string(newPubKeyBody),
 		}}},
 	})
-	h.cfg.AccessControls[0].ChangeAccessControls[0].Condition.Signature.AccountIDs = []string{"root", "toot"}
-	h.cfg.AccessControls[0].ChangeAccessControls[0].Condition.Signature.Count = "1"
 
 	h.stageCfg()
 	badCommit, err := h.repo.NewCommitChange("add toot user")

config.go

@@ -21,7 +21,7 @@ type Account struct {
 // used to marshal/unmarshal the yaml file.
 type Config struct {
 	Accounts       []Account                 `yaml:"accounts"`
-	AccessControls []accessctl.BranchAccessControl `yaml:"access_controls"`
+	AccessControls []accessctl.AccessControl `yaml:"access_controls"`
 }
 
 func (r *Repo) loadConfig(fs fs.FS) (Config, error) {
@@ -36,6 +36,17 @@ func (r *Repo) loadConfig(fs fs.FS) (Config, error) {
 		return cfg, fmt.Errorf("could not decode config.yml: %w", err)
 	}
 
+	// older config versions also had access_controls be an array, but not using
+	// the action field. So filter out array elements without the action field.
+	acl := cfg.AccessControls
+	cfg.AccessControls = cfg.AccessControls[:0]
+	for _, ac := range acl {
+		if ac.Action == "" {
+			continue
+		}
+		cfg.AccessControls = append(cfg.AccessControls, ac)
+	}
+
 	// TODO validate Config
 
 	return cfg, nil

repo_test.go

@@ -2,7 +2,6 @@ package dehub
 
 import (
 	"bytes"
-	"dehub/accessctl"
 	"dehub/sigcred"
 	"errors"
 	"io"
@@ -36,22 +35,6 @@ func newHarness(t *testing.T) *harness {
 				Path: pubKeyPath,
 			}}},
 		}},
-		AccessControls: []accessctl.BranchAccessControl{
-			{
-				BranchPattern: "**",
-				ChangeAccessControls: []accessctl.ChangeAccessControl{
-					{
-						FilePathPattern: "**",
-						Condition: accessctl.Condition{
-							Signature: &accessctl.ConditionSignature{
-								AccountIDs: []string{"root"},
-								Count:      "100%",
-							},
-						},
-					},
-				},
-			},
-		},
 	}
 	cfgBody, err := yaml.Marshal(cfg)
 	if err != nil {