diff --git a/app/api/controller/repo/label_value_define.go b/app/api/controller/repo/label_value_define.go index b86bbc6bf..1bc083af8 100644 --- a/app/api/controller/repo/label_value_define.go +++ b/app/api/controller/repo/label_value_define.go @@ -18,7 +18,6 @@ import ( "context" "fmt" - apiauth "github.com/harness/gitness/app/api/auth" "github.com/harness/gitness/app/auth" "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" @@ -32,7 +31,7 @@ func (c *Controller) DefineLabelValue( key string, in *types.DefineValueInput, ) (*types.LabelValue, error) { - repo, err := GetRepo(ctx, c.repoStore, repoRef, []enum.RepoState{enum.RepoStateActive}) + repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoEdit) if err != nil { return nil, fmt.Errorf("failed to acquire access to repo: %w", err) } @@ -46,16 +45,6 @@ func (c *Controller) DefineLabelValue( return nil, fmt.Errorf("failed to find repo label: %w", err) } - permission := enum.PermissionRepoEdit - if label.Type == enum.LabelTypeDynamic { - permission = enum.PermissionRepoReview - } - - if err = apiauth.CheckRepo( - ctx, c.authorizer, session, repo, permission); err != nil { - return nil, fmt.Errorf("access check failed: %w", err) - } - value, err := c.labelSvc.DefineValue(ctx, session.Principal.ID, label.ID, in) if err != nil { return nil, fmt.Errorf("failed to create repo label value: %w", err) diff --git a/app/api/controller/space/label_value_define.go b/app/api/controller/space/label_value_define.go index 951dd01b0..2ae355fde 100644 --- a/app/api/controller/space/label_value_define.go +++ b/app/api/controller/space/label_value_define.go @@ -31,7 +31,6 @@ func (c *Controller) DefineLabelValue( key string, in *types.DefineValueInput, ) (*types.LabelValue, error) { - // TODO: permission check should be based on static vs dynamic label space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionRepoEdit) if err != nil { return nil, fmt.Errorf("failed to acquire access to space: %w", err) diff --git a/app/services/label/label.go b/app/services/label/label.go index db6cb9618..7bbf48e35 100644 --- a/app/services/label/label.go +++ b/app/services/label/label.go @@ -96,7 +96,7 @@ func (s *Service) Save( err = s.tx.WithTx(ctx, func(ctx context.Context) error { label, err = s.labelStore.FindByID(ctx, in.Label.ID) - if err != nil { + if err != nil { //nolint:nestif if !errors.Is(err, store.ErrResourceNotFound) { return err } @@ -105,6 +105,10 @@ func (s *Service) Save( return err } } else { + if err := checkLabelInScope(spaceID, repoID, label); err != nil { + return err + } + label, err = s.update(ctx, principalID, label, &types.UpdateLabelInput{ Key: &in.Label.Key, Type: &in.Label.Type, @@ -338,3 +342,15 @@ func applyChanges(principalID int64, label *types.Label, in *types.UpdateLabelIn return label, hasChanges } + +func checkLabelInScope( + spaceID, repoID *int64, + label *types.Label, +) error { + if (repoID != nil && (label.RepoID == nil || *label.RepoID != *repoID)) || + (spaceID != nil && (label.SpaceID == nil || *label.SpaceID != *spaceID)) { + return errors.InvalidArgument("label is not defined in requested scope") + } + + return nil +} diff --git a/app/services/label/label_pullreq.go b/app/services/label/label_pullreq.go index 1d3dad383..89a19bef0 100644 --- a/app/services/label/label_pullreq.go +++ b/app/services/label/label_pullreq.go @@ -52,13 +52,9 @@ func (s *Service) AssignToPullReq( return nil, fmt.Errorf("failed to find label by id: %w", err) } - if err := s.checkLabelIsInSpace(ctx, repoParentID, label); err != nil { + if err := s.checkPullreqLabelInScope(ctx, repoParentID, repoID, label); err != nil { return nil, err } - if label.RepoID != nil && *label.RepoID != repoID { - return nil, - errors.InvalidArgument("label %d is not defined in current repo", label.ID) - } oldPullreqLabel, err := s.pullReqLabelAssignmentStore.FindByLabelID(ctx, pullreqID, label.ID) if err != nil && !errors.Is(err, store.ErrResourceNotFound) { @@ -149,23 +145,6 @@ func (s *Service) AssignToPullReq( }, nil } -func (s *Service) checkLabelIsInSpace( - ctx context.Context, - repoParentID int64, - label *types.Label, -) error { - if label.SpaceID != nil { - spaceIDs, err := s.spaceStore.GetAncestorIDs(ctx, repoParentID) - if err != nil { - return fmt.Errorf("failed to get parent space ids: %w", err) - } - if ok := slices.Contains(spaceIDs, *label.SpaceID); !ok { - return errors.NotFound("label %d is not defined in current space tree path", label.ID) - } - } - return nil -} - func (s *Service) getOrDefineValue( ctx context.Context, principalID int64, @@ -208,7 +187,7 @@ func (s *Service) UnassignFromPullReq( return nil, nil, fmt.Errorf("failed to find label by id: %w", err) } - if err := s.checkLabelIsInSpace(ctx, repoParentID, label); err != nil { + if err := s.checkPullreqLabelInScope(ctx, repoParentID, repoID, label); err != nil { return nil, nil, err } @@ -217,20 +196,6 @@ func (s *Service) UnassignFromPullReq( return nil, nil, fmt.Errorf("failed to find label value: %w", err) } - if label.RepoID != nil && *label.RepoID != repoID { - return nil, nil, errors.InvalidArgument( - "label %d is not defined in current repo", label.ID) - } else if label.SpaceID != nil { - spaceIDs, err := s.spaceStore.GetAncestorIDs(ctx, repoParentID) - if err != nil { - return nil, nil, fmt.Errorf("failed to get parent space ids: %w", err) - } - if ok := slices.Contains(spaceIDs, *label.SpaceID); !ok { - return nil, nil, errors.NotFound( - "label %d is not defined in current space tree path", label.ID) - } - } - return label, value, s.pullReqLabelAssignmentStore.Unassign(ctx, pullreqID, labelID) } @@ -408,3 +373,25 @@ func newPullReqLabel( UpdatedBy: principalID, } } + +func (s *Service) checkPullreqLabelInScope( + ctx context.Context, + repoParentID, repoID int64, + label *types.Label, +) error { + if label.RepoID != nil && *label.RepoID != repoID { + return errors.InvalidArgument("label %d is not defined in current repo", label.ID) + } + + if label.SpaceID != nil { + spaceIDs, err := s.spaceStore.GetAncestorIDs(ctx, repoParentID) + if err != nil { + return fmt.Errorf("failed to get parent space ids: %w", err) + } + if ok := slices.Contains(spaceIDs, *label.SpaceID); !ok { + return errors.InvalidArgument("label %d is not defined in current space tree path", label.ID) + } + } + + return nil +}