diff --git a/INTRODUCTION.md b/INTRODUCTION.md index d44e999..f5ee4ba 100644 --- a/INTRODUCTION.md +++ b/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 -equivalent of the `master` branch). MyProject's repo would contain a -`.dehub/config.yml` file with the following access controls set: +commit which changes files before the commit can be placed into the `main` +branch (dehub's equivalent of the `master` branch). MyProject's repo would +contain a `.dehub/config.yml` file with the following access controls set: ``` # ... access_controls: - - branch_pattern: main - change_access_controls: - # matches all files, but could be used for more fine-grained control - - file_path_pattern: "**" - condition: - type: signature - account_ids: - - alice - - bob - - carol - count: 2 + - action: allow + filters: + - type: branch + pattern: main + + - 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. -Bob's signature is added to the change commit message as a credential. The -commit can then be pushed to `main` (because it now has two credentials) and +into a single change commit, using the latest commit message which was pushed by +Alice. Bob's signature is added to the change commit message as a credential. +The commit can then be pushed to `main` (because it now has two credentials) and `featureBranch` can be deleted. ## Pre-emptively Answered Questions diff --git a/ROADMAP.md b/ROADMAP.md index d60136e..725a636 100644 --- a/ROADMAP.md +++ b/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 diff --git a/SPEC.md b/SPEC.md index ac7a297..fea584a 100644 --- a/SPEC.md +++ b/SPEC.md @@ -29,73 +29,37 @@ 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: - - # branch_pattern is a glob pattern describing what branch names this access - # control applies to. The first matching branch_pattern for a branch name - # defines which access controls are applied. - - branch_pattern: main - - # change_access_controls is an array of possible access controls applied for - # 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 + - 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 ``` -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 diff --git a/accessctl/access_control.go b/accessctl/access_control.go index d08b3e7..0ab442a 100644 --- a/accessctl/access_control.go +++ b/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 ( - // 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", - }, +// DefaultAccessControlsStr is the encoded form of the default access control +// set which is applied to all CommitRequests if no user-supplied ones match. +// +// The effect of these AccessControls is to allow all commit types on any branch +// (with the exception of the main branch, which only allows change commits), as +// long as the commit has one signature from a configured account. +var DefaultAccessControlsStr = ` + - 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 - // applied when a changed file's path does not match any defined patterns - // within a BranchAccessControl. - DefaultChangeAccessControl = ChangeAccessControl{ - FilePathPattern: "**", - 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{ - { - 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"` -} +// CommitRequest is used to describe a set of interactions which are being +// requested to be performed. +type CommitRequest struct { + // Type describes what type of commit is being requested. Possibilities are + // determined by the requester. + Type string -// 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) - // which have been modified in some way. - FilePathsChanged []string + // Credentials are the Credential objects attached to the commit. + Credentials []sigcred.Credential - // CredentialAdded indicates a credential commit is being added to the - // Branch. - CredentialAdded bool + // FilesChanged is the set of file paths (relative to the repo root) which + // have been modified in some way. + FilesChanged []string } -// MatchedChangeAccessControl contains information about a ChangeAccessControl -// which was matched in Match -type MatchedChangeAccessControl struct { - ChangeAccessControl ChangeAccessControl +// Action describes what action an AccessControl should perform +// when given a CommitRequest. +type Action string - // FilePaths contains all FilePaths to which this access control was found - // to be applicable. - FilePaths []string -} - -// MatchedCredentialAccessControl contains information about a -// CredentialAccessControl which was matched in Match. -type MatchedCredentialAccessControl struct { - CredentialAccessControl CredentialAccessControl -} +// Enumerates possible Action values +const ( + ActionAllow Action = "allow" + ActionDeny Action = "deny" -// 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 + // 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" +) - // CredentialAccessControls indicates which CredentialAccessControl object - // matched for a credential commit. Will be nil if CredentialAdded was - // false. - CredentialAccessControl *MatchedCredentialAccessControl +// 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"` } -// 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", - accessControls[i].BranchPattern, interactions.Branch, err) - } else if ok { - branchAC = accessControls[i] - break - } - } - if !ok { - panic(fmt.Sprintf("no patterns matched branch %q, which shouldn't be possible", interactions.Branch)) +// ActionForCommit returns what Action this AccessControl says to take for a +// given CommitRequest. It may return ActionNext if the request is not matched +// by the AccessControl's Filters. +func (ac AccessControl) ActionForCommit(req CommitRequest) (Action, error) { + for _, filter := range ac.Filters { + filterI, err := filter.Interface() + if err != nil { + return "", fmt.Errorf("casting %+v to a FilterInterface: %w", filter, err) + + } else if err := filterI.MatchCommit(req); errors.As(err, new(ErrFilterNoMatch)) { + return ActionNext, nil + + } 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) } - res.BranchPattern = branchAC.BranchPattern } + return ac.Action, nil +} - // determine ChangeAccessControl for each path in FilesChanged - { - changeACs := append(branchAC.ChangeAccessControls, DefaultChangeAccessControl) - acToPaths := map[ChangeAccessControl][]string{} - 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, - }) - } +// ErrCommitRequestDenied is returned from AssertCanCommit when a particular +// AccessControl has explicitly disallowed the CommitRequest. +type ErrCommitRequestDenied struct { + By AccessControl +} - // sort result for determinancy - sort.Slice(res.ChangeAccessControls, func(i, j int) bool { - pi := res.ChangeAccessControls[i].ChangeAccessControl.FilePathPattern - pj := res.ChangeAccessControls[j].ChangeAccessControl.FilePathPattern - return pi < pj - }) +func (e ErrCommitRequestDenied) Error() string { + acB, err := yaml.Marshal(e.By) + if err != nil { + panic(err) } + return fmt.Sprintf("commit matched and denied by this access control:\n%s", string(acB)) +} - // Handle CredentialAccessControl, if applicable - if interactions.CredentialAdded { - credAC := branchAC.CredentialAccessControl - if credAC == nil { - credAC = &DefaultCredentialAccessControl +// AssertCanCommit asserts that the given CommitRequest is allowed by the given +// AccessControls. +func AssertCanCommit(acl []AccessControl, req CommitRequest) error { + acl = append(acl, DefaultAccessControls...) + for _, ac := range acl { + action, err := ac.ActionForCommit(req) + if err != nil { + return err } - - res.CredentialAccessControl = &MatchedCredentialAccessControl{ - CredentialAccessControl: *credAC, + switch action { + case ActionNext: + 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") } diff --git a/accessctl/access_control_test.go b/accessctl/access_control_test.go index 2278e06..89f19f4 100644 --- a/accessctl/access_control_test.go +++ b/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 { - if len(res.ChangeAccessControls) == 0 { - res.ChangeAccessControls = nil - } - return res -} - -func TestMatch(t *testing.T) { - secondCond := Condition{ - Signature: &ConditionSignature{ - AnyAccount: true, - Count: "2", - }, - } - +func TestAssertCanCommit(t *testing.T) { tests := []struct { - descr string - - branchACs []BranchAccessControl - interactions MatchInteractions - result MatchResult + descr string + acl []AccessControl + req CommitRequest + allowed bool }{ { - descr: "empty input empty result", - result: MatchResult{ - BranchPattern: "**", - }, - }, - { - descr: "empty access controls", - interactions: MatchInteractions{ - Branch: "main", - FilePathsChanged: []string{"foo", "bar"}, - }, - result: MatchResult{ - BranchPattern: "main", - ChangeAccessControls: []MatchedChangeAccessControl{ - { - ChangeAccessControl: DefaultChangeAccessControl, - FilePaths: []string{"foo", "bar"}, - }, + descr: "first allows", + acl: []AccessControl{ + { + Action: ActionAllow, + Filters: []Filter{{ + CommitType: &FilterCommitType{Type: "foo"}, + }}, + }, + { + Action: ActionDeny, + Filters: []Filter{{ + CommitType: &FilterCommitType{Type: "foo"}, + }}, }, }, + req: CommitRequest{Type: "foo"}, + allowed: true, }, { - 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"}, - }}, - }, - }, - { - 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: "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: "branch pattern precedent", - branchACs: []BranchAccessControl{ + descr: "second allows", + acl: []AccessControl{ { - BranchPattern: "main", - ChangeAccessControls: []ChangeAccessControl{{ - FilePathPattern: "foo", - Condition: secondCond, + Action: ActionDeny, + Filters: []Filter{{ + CommitType: &FilterCommitType{Type: "bar"}, }}, }, { - BranchPattern: "**", - ChangeAccessControls: []ChangeAccessControl{ - DefaultChangeAccessControl, - }, + Action: ActionAllow, + Filters: []Filter{{ + CommitType: &FilterCommitType{Type: "foo"}, + }}, }, }, - interactions: MatchInteractions{ - Branch: "main", - FilePathsChanged: []string{"foo"}, - }, - result: MatchResult{ - BranchPattern: "main", - ChangeAccessControls: []MatchedChangeAccessControl{{ - ChangeAccessControl: ChangeAccessControl{ - FilePathPattern: "foo", - Condition: secondCond, - }, - FilePaths: []string{"foo"}, - }}, - }, + req: CommitRequest{Type: "foo"}, + allowed: true, }, { - 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: "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: "no defined CredentialAccessControl", - branchACs: []BranchAccessControl{{ - BranchPattern: "main", - }}, - interactions: MatchInteractions{ - Branch: "main", - CredentialAdded: true, - }, - result: MatchResult{ - BranchPattern: "main", - CredentialAccessControl: &MatchedCredentialAccessControl{ - CredentialAccessControl: DefaultCredentialAccessControl, + 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: "defined CredentialAccessControl", - branchACs: []BranchAccessControl{{ - BranchPattern: "main", - CredentialAccessControl: &CredentialAccessControl{ - Condition: Condition{ - Signature: &ConditionSignature{ - AccountIDs: []string{"foo", "bar", "baz"}, - }, - }, + descr: "default denies", + acl: []AccessControl{ + { + Action: ActionDeny, + Filters: []Filter{{ + CommitType: &FilterCommitType{Type: "bar"}, + }}, }, - }}, - interactions: MatchInteractions{ - Branch: "main", - CredentialAdded: true, }, - result: MatchResult{ - BranchPattern: "main", - CredentialAccessControl: &MatchedCredentialAccessControl{ - CredentialAccessControl: CredentialAccessControl{ - Condition: Condition{ - Signature: &ConditionSignature{ - AccountIDs: []string{"foo", "bar", "baz"}, - }, - }, - }, - }, + req: CommitRequest{ + Branch: "main", + Type: "foo", + Credentials: []sigcred.Credential{{ + PGPSignature: new(sigcred.CredentialPGPSignature), + AccountID: "a", + }}, }, + allowed: false, }, } for _, test := range tests { t.Run(test.descr, func(t *testing.T) { - res, err := Match(test.branchACs, test.interactions) - if err != nil { - t.Fatalf("error matching: %v", err) - } - res, expRes := normalizeResult(res), normalizeResult(test.result) - if !reflect.DeepEqual(res, expRes) { - t.Fatalf("expected:%s\ngot: %s", spew.Sdump(expRes), spew.Sdump(res)) + err := AssertCanCommit(test.acl, test.req) + if test.allowed && err != nil { + t.Fatalf("expected to be allowed but got: %v", err) + } else if !test.allowed && !errors.As(err, new(ErrCommitRequestDenied)) { + t.Fatalf("expected to be denied but got: %v", err) } }) } diff --git a/accessctl/condition.go b/accessctl/condition.go deleted file mode 100644 index a158753..0000000 --- a/accessctl/condition.go +++ /dev/null @@ -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 -} diff --git a/accessctl/condition_test.go b/accessctl/condition_test.go deleted file mode 100644 index a20fc59..0000000 --- a/accessctl/condition_test.go +++ /dev/null @@ -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) - } - }) - } -} diff --git a/accessctl/filter.go b/accessctl/filter.go new file mode 100644 index 0000000..a0c6265 --- /dev/null +++ b/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"`) + } +} diff --git a/accessctl/filter_logical.go b/accessctl/filter_logical.go new file mode 100644 index 0000000..506d3e1 --- /dev/null +++ b/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 diff --git a/accessctl/filter_logical_test.go b/accessctl/filter_logical_test.go new file mode 100644 index 0000000..2a2b883 --- /dev/null +++ b/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, + }, + }) +} diff --git a/accessctl/filter_pattern.go b/accessctl/filter_pattern.go new file mode 100644 index 0000000..6dc1d89 --- /dev/null +++ b/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 +} diff --git a/accessctl/filter_pattern_test.go b/accessctl/filter_pattern_test.go new file mode 100644 index 0000000..700508f --- /dev/null +++ b/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) + } + }) + } +} diff --git a/accessctl/filter_sig.go b/accessctl/filter_sig.go new file mode 100644 index 0000000..93ed62f --- /dev/null +++ b/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, + }, + } +} diff --git a/accessctl/filter_sig_test.go b/accessctl/filter_sig_test.go new file mode 100644 index 0000000..6982d8c --- /dev/null +++ b/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, + }, + }, + }) +} diff --git a/accessctl/filter_test.go b/accessctl/filter_test.go new file mode 100644 index 0000000..a0a916b --- /dev/null +++ b/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, + }, + }) +} diff --git a/commit.go b/commit.go index 9746dd5..f518bde 100644 --- a/commit.go +++ b/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{ - Branch: branch.Short(), - FilePathsChanged: pathsChanged, - CredentialAdded: commit.Credential != nil, - }) + commitType, err := commit.Type() if err != nil { - return fmt.Errorf("determining applicable access controls: %w", err) + return fmt.Errorf("determining type of commit %+v: %w", commit, 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 + return accessctl.AssertCanCommit(acl, accessctl.CommitRequest{ + Type: commitType, + Branch: branch.Short(), + Credentials: commit.Credentials, + FilesChanged: pathsChanged, + }) } // VerifyCommits verifies that the given commits, which are presumably on the diff --git a/commit_credential_test.go b/commit_credential_test.go index 3e6d7e2..ad994f8 100644 --- a/commit_credential_test.go +++ b/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{ - 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, - }, - }, + + err := yaml.Unmarshal([]byte(` +- action: allow + filters: + - type: branch + pattern: `+tootBranch.Short()+` + - type: signature + count: 1 + account_ids: + - root + - toot + +- action: allow + filters: + - type: signature + count: 1 + account_ids: + - root + +- action: deny +`), &h.cfg.AccessControls) + if err != nil { + t.Fatal(err) } h.stageCfg() rootGitCommit := h.changeCommit("initial commit", h.cfg.Accounts[0].ID, h.sig) diff --git a/commit_test.go b/commit_test.go index 65b995d..bfda7f5 100644 --- a/commit_test.go +++ b/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") diff --git a/config.go b/config.go index ceda7d1..aad4041 100644 --- a/config.go +++ b/config.go @@ -20,8 +20,8 @@ type Account struct { // Config represents the structure of the main dehub configuration file, and is // used to marshal/unmarshal the yaml file. type Config struct { - Accounts []Account `yaml:"accounts"` - AccessControls []accessctl.BranchAccessControl `yaml:"access_controls"` + Accounts []Account `yaml:"accounts"` + 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 diff --git a/repo_test.go b/repo_test.go index e64647d..c0759af 100644 --- a/repo_test.go +++ b/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 {