lifecycle protection rules (#715)

app/api/controller/githook/pre_receive.go

@@ -64,7 +64,7 @@ func (c *Controller) PreReceive(
 
  // TODO: Remove the dummy session and use the real session, once that has been done and the session has a value.
  dummySession := &auth.Session{
- Principal: types.Principal{ID: principalID},
+ Principal: types.Principal{ID: principalID, Admin: false}, // TODO: In the dummySession "Admin" is always false
  Metadata: nil,
  }
 
@@ -94,27 +94,37 @@ func (c *Controller) checkProtectionRules(
  }
 
  var ruleViolations []types.RuleViolations
+ var errCheckAction error
 
- // TODO: protectionRules.CanCreateBranch
- // if len(refUpdates.branches.created) > 0 {}
-
- // TODO: protectionRules.CanDeleteBranch
- // if len(refUpdates.branches.deleted) > 0 {}
+ checkAction := func(refAction protection.RefAction, refType protection.RefType, names []string) {
+ if errCheckAction != nil || len(names) == 0 {
+ return
+ }
 
- if len(refUpdates.branches.updated) > 0 {
- _, violations, err := protectionRules.CanPush(ctx, protection.CanPushInput{
+ violations, err := protectionRules.CanModifyRef(ctx, protection.CanModifyRefInput{
  Actor: &session.Principal,
  IsSpaceOwner: isSpaceOwner,
  Repo: repo,
- BranchNames: refUpdates.branches.updated,
+ RefAction: refAction,
+ RefType: refType,
+ RefNames: names,
  })
  if err != nil {
- return fmt.Errorf("failed to verify protection rules for git push: %w", err)
+ errCheckAction = fmt.Errorf("failed to verify protection rules for git push: %w", err)
+ return
  }
 
  ruleViolations = append(ruleViolations, violations...)
  }
 
+ checkAction(protection.RefActionCreate, protection.RefTypeBranch, refUpdates.branches.created)
+ checkAction(protection.RefActionDelete, protection.RefTypeBranch, refUpdates.branches.deleted)
+ checkAction(protection.RefActionUpdate, protection.RefTypeBranch, refUpdates.branches.updated)
+
+ if errCheckAction != nil {
+ return errCheckAction
+ }
+
  var criticalViolation bool
 
  for _, ruleViolation := range ruleViolations {

app/api/controller/repo/commit.go

@@ -58,31 +58,41 @@ func (c *Controller) CommitFiles(ctx context.Context,
  return types.CommitFilesResponse{}, err
  }
 
- if in.NewBranch == "" {
- isSpaceOwner, err := apiauth.IsSpaceAdmin(ctx, c.authorizer, session, repo)
- if err != nil {
- return types.CommitFilesResponse{}, err
- }
+ isSpaceOwner, err := apiauth.IsSpaceAdmin(ctx, c.authorizer, session, repo)
+ if err != nil {
+ return types.CommitFilesResponse{}, err
+ }
 
-  protectionRules, err := c.protectionManager.ForRepository(ctx, repo.ID)
-  if err != nil {
-  return types.CommitFilesResponse{},
-  fmt.Errorf("failed to fetch protection rules for the repository: %w", err)
-  }
+ protectionRules, err := c.protectionManager.ForRepository(ctx, repo.ID)
+ if err != nil {
+ return types.CommitFilesResponse{},
+ fmt.Errorf("failed to fetch protection rules for the repository: %w", err)
+ }
 
-  _, violations, err := protectionRules.CanPush(ctx, protection.CanPushInput{
-  Actor: &session.Principal,
-  IsSpaceOwner: isSpaceOwner,
-  Repo: repo,
-  BranchNames: []string{in.Branch},
-  })
- if err != nil {
-  return types.CommitFilesResponse{}, fmt.Errorf("failed to verify protection rules for git push: %w", err)
-  }
+ var refAction protection.RefAction
+ var branchName string
+ if in.NewBranch != "" {
+ refAction = protection.RefActionCreate
+ branchName = in.NewBranch
+ } else {
+ refAction = protection.RefActionUpdate
+ branchName = in.Branch
+ }
 
- if protection.IsCritical(violations) {
- return types.CommitFilesResponse{RuleViolations: violations}, nil
- }
+ violations, err := protectionRules.CanModifyRef(ctx, protection.CanModifyRefInput{
+ Actor: &session.Principal,
+ IsSpaceOwner: isSpaceOwner,
+ Repo: repo,
+ RefAction: refAction,
+ RefType: protection.RefTypeBranch,
+ RefNames: []string{branchName},
+ })
+ if err != nil {
+ return types.CommitFilesResponse{}, fmt.Errorf("failed to verify protection rules for git push: %w", err)
+ }
+
+ if protection.IsCritical(violations) {
+ return types.CommitFilesResponse{RuleViolations: violations}, nil
  }
 
  actions := make([]gitrpc.CommitFileAction, len(in.Actions))

app/services/protection/interface.go

@@ -33,10 +33,8 @@ type (
 
  // Protection defines interface for branch protection.
  Protection interface {
- // CanMerge tests if a pull request can be merged.
  CanMerge(ctx context.Context, in CanMergeInput) (CanMergeOutput, []types.RuleViolations, error)
-
- CanPush(ctx context.Context, in CanPushInput) (CanPushOutput, []types.RuleViolations, error)
+ CanModifyRef(ctx context.Context, in CanModifyRefInput) ([]types.RuleViolations, error)
  }
 
  CanMergeInput struct {
@@ -56,15 +54,30 @@ type (
  DeleteSourceBranch bool
  }
 
- CanPushInput struct {
+ CanModifyRefInput struct {
  Actor *types.Principal
  IsSpaceOwner bool
  Repo *types.Repository
- BranchNames []string
+ RefAction RefAction
+ RefType RefType
+ RefNames []string
  }
 
- CanPushOutput struct {
- }
+ RefType int
+
+ RefAction int
+)
+
+const (
+ RefTypeRaw RefType = iota
+ RefTypeBranch
+ RefTypeTag
+)
+
+const (
+ RefActionCreate RefAction = iota
+ RefActionDelete
+ RefActionUpdate
 )
 
 var (

app/services/protection/rule_branch.go

@@ -31,7 +31,6 @@ var TypeBranch types.RuleType = "branch"
 type Branch struct {
  Bypass DefBypass `json:"bypass"`
  PullReq DefPullReq `json:"pullreq"`
- Push DefPush `json:"push"`
  Lifecycle DefLifecycle `json:"lifecycle"`
 }
 
@@ -110,28 +109,38 @@ func (v *Branch) CanMerge(_ context.Context, in CanMergeInput) (CanMergeOutput,
  return out, []types.RuleViolations{violations}, nil
 }
 
-func (v *Branch) CanPush(_ context.Context, in CanPushInput) (CanPushOutput, []types.RuleViolations, error) {
- var out CanPushOutput
+func (v *Branch) CanModifyRef(_ context.Context, in CanModifyRefInput) ([]types.RuleViolations, error) {
  var violations types.RuleViolations
 
- if v.isBypassed(in.Actor, in.IsSpaceOwner) {
- return out, nil, nil
+ if v.isBypassed(in.Actor, in.IsSpaceOwner) || in.RefType != RefTypeBranch || len(in.RefNames) == 0 {
+ return nil, nil
  }
 
- if v.Push.Block {
- violations.Add("pullreq.push.block",
- "Push is not allowed. Please use pull requests.")
+ switch in.RefAction {
+ case RefActionCreate:
+ if v.Lifecycle.CreateForbidden {
+ violations.Addf("pullreq.lifecycle.create",
+ "Creation of branch %q is not allowed.", in.RefNames[0])
+ }
+ case RefActionDelete:
+ if v.Lifecycle.DeleteForbidden {
+ violations.Addf("pullreq.lifecycle.delete",
+ "Delete of branch %q is not allowed.", in.RefNames[0])
+ }
+ case RefActionUpdate:
+ if v.Lifecycle.UpdateForbidden {
+ violations.Addf("pullreq.lifecycle.update",
+ "Push to branch %q is not allowed. Please use pull requests.", in.RefNames[0])
+ }
  }
 
- return out, []types.RuleViolations{violations}, nil
+ return []types.RuleViolations{violations}, nil
 }
 
 func (v *Branch) isBypassed(actor *types.Principal, isSpaceOwner bool) bool {
- if v.Bypass.SpaceOwners && isSpaceOwner {
- return true
- }
-
- return slices.Contains(v.Bypass.UserIDs, actor.ID)
+ return actor.Admin ||
+ v.Bypass.SpaceOwners && isSpaceOwner ||
+ slices.Contains(v.Bypass.UserIDs, actor.ID)
 }
 
 func (v *Branch) Sanitize() error {
@@ -143,10 +152,6 @@ func (v *Branch) Sanitize() error {
  return fmt.Errorf("pull request: %w", err)
  }
 
- if err := v.Push.Validate(); err != nil {
- return fmt.Errorf("push: %w", err)
- }
-
  if err := v.Lifecycle.Validate(); err != nil {
  return fmt.Errorf("lifecycle: %w", err)
  }
@@ -234,6 +239,7 @@ func (v DefPush) Validate() error {
 type DefLifecycle struct {
  CreateForbidden bool `json:"create_forbidden,omitempty"`
  DeleteForbidden bool `json:"delete_forbidden,omitempty"`
+ UpdateForbidden bool `json:"update_forbidden,omitempty"`
 }
 
 func (v DefLifecycle) Validate() error {

app/services/protection/set.go

@@ -59,37 +59,36 @@ func (s ruleSet) CanMerge(ctx context.Context, in CanMergeInput) (CanMergeOutput
  return out, violations, nil
 }
 
-func (s ruleSet) CanPush(ctx context.Context, in CanPushInput) (CanPushOutput, []types.RuleViolations, error) {
- var out CanPushOutput
+func (s ruleSet) CanModifyRef(ctx context.Context, in CanModifyRefInput) ([]types.RuleViolations, error) {
  var violations []types.RuleViolations
 
  for _, r := range s.rules {
- matched, err := matchedNames(r.Pattern, in.Repo.DefaultBranch, in.BranchNames...)
+ matched, err := matchedNames(r.Pattern, in.Repo.DefaultBranch, in.RefNames...)
  if err != nil {
- return out, nil, err
+ return nil, err
  }
  if len(matched) == 0 {
  continue
  }
 
  protection, err := s.manager.FromJSON(r.Type, r.Definition, false)
  if err != nil {
- return out, nil,
+ return nil,
  fmt.Errorf("failed to parse protection definition ID=%d Type=%s: %w", r.ID, r.Type, err)
  }
 
  ruleIn := in
- in.BranchNames = matched
+ ruleIn.RefNames = matched
 
- _, rVs, err := protection.CanPush(ctx, ruleIn)
+ rVs, err := protection.CanModifyRef(ctx, ruleIn)
  if err != nil {
- return out, nil, err
+ return nil, err
  }
 
  violations = append(violations, backFillRule(rVs, r.RuleInfo)...)
  }
 
- return out, violations, nil
+ return violations, nil
 }
 
 func backFillRule(vs []types.RuleViolations, rule types.RuleInfo) []types.RuleViolations {