fix: [CODE-2908]: Fix permission checks for space level rule + label operations (#3123)

app/api/controller/repo/controller.go

@@ -227,6 +227,14 @@ func (c *Controller) getRepoCheckAccessForGit(
 	)
 }
 
+func (c *Controller) getSpaceCheckAuthRepoCreation(
+	ctx context.Context,
+	session *auth.Session,
+	parentRef string,
+) (*types.Space, error) {
+	return GetSpaceCheckAuthRepoCreation(ctx, c.spaceStore, c.authorizer, session, parentRef)
+}
+
 func ValidateParentRef(parentRef string) error {
 	parentRefAsID, err := strconv.ParseInt(parentRef, 10, 64)
 	if (err == nil && parentRefAsID <= 0) || (len(strings.TrimSpace(parentRef)) == 0) {

app/api/controller/repo/create.go

@@ -22,7 +22,6 @@ import (
 	"strings"
 	"time"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/api/controller/limiter"
 	"github.com/harness/gitness/app/api/usererror"
 	"github.com/harness/gitness/app/auth"
@@ -189,32 +188,6 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea
 	return repoOutput, nil
 }
 
-func (c *Controller) getSpaceCheckAuthRepoCreation(
-	ctx context.Context,
-	session *auth.Session,
-	parentRef string,
-) (*types.Space, error) {
-	space, err := c.spaceStore.FindByRef(ctx, parentRef)
-	if err != nil {
-		return nil, fmt.Errorf("parent space not found: %w", err)
-	}
-
-	// create is a special case - check permission without specific resource
-	err = apiauth.CheckSpaceScope(
-		ctx,
-		c.authorizer,
-		session,
-		space,
-		enum.ResourceTypeRepo,
-		enum.PermissionRepoCreate,
-	)
-	if err != nil {
-		return nil, fmt.Errorf("auth check failed: %w", err)
-	}
-
-	return space, nil
-}
-
 func (c *Controller) sanitizeCreateInput(in *CreateInput) error {
 	// TODO [CODE-1363]: remove after identifier migration.
 	if in.Identifier == "" {

app/api/controller/repo/helper.go

@@ -78,6 +78,34 @@ func GetRepoCheckAccess(
 	return repo, nil
 }
 
+func GetSpaceCheckAuthRepoCreation(
+	ctx context.Context,
+	spaceStore store.SpaceStore,
+	authorizer authz.Authorizer,
+	session *auth.Session,
+	parentRef string,
+) (*types.Space, error) {
+	space, err := spaceStore.FindByRef(ctx, parentRef)
+	if err != nil {
+		return nil, fmt.Errorf("parent space not found: %w", err)
+	}
+
+	// create is a special case - check permission without specific resource
+	err = apiauth.CheckSpaceScope(
+		ctx,
+		authorizer,
+		session,
+		space,
+		enum.ResourceTypeRepo,
+		enum.PermissionRepoCreate,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("auth check failed: %w", err)
+	}
+
+	return space, nil
+}
+
 func GetRepoOutput(
 	ctx context.Context,
 	publicAccess publicaccess.Service,

app/api/controller/space/controller.go

@@ -15,11 +15,17 @@
 package space
 
 import (
+	"context"
 	"encoding/json"
+	"fmt"
+	"strconv"
+	"strings"
 
+	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/api/controller/limiter"
 	"github.com/harness/gitness/app/api/controller/repo"
 	"github.com/harness/gitness/app/api/usererror"
+	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/app/auth/authz"
 	"github.com/harness/gitness/app/services/exporter"
 	"github.com/harness/gitness/app/services/gitspace"
@@ -36,6 +42,7 @@ import (
 	"github.com/harness/gitness/store/database/dbtx"
 	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/check"
+	"github.com/harness/gitness/types/enum"
 )
 
 var (
@@ -136,3 +143,65 @@ func NewController(config *types.Config, tx dbtx.Transactor, urlProvider url.Pro
 		rulesSvc:            rulesSvc,
 	}
 }
+
+// getSpaceCheckAuth checks whether the user has the requested permission on the provided space and returns the space.
+func (c *Controller) getSpaceCheckAuth(
+	ctx context.Context,
+	session *auth.Session,
+	spaceRef string,
+	permission enum.Permission,
+) (*types.Space, error) {
+	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	if err != nil {
+		return nil, fmt.Errorf("parent space not found: %w", err)
+	}
+
+	err = apiauth.CheckSpace(ctx, c.authorizer, session, space, permission)
+	if err != nil {
+		return nil, fmt.Errorf("auth check failed: %w", err)
+	}
+
+	return space, nil
+}
+
+func (c *Controller) getSpaceCheckAuthRepoCreation(
+	ctx context.Context,
+	session *auth.Session,
+	parentRef string,
+) (*types.Space, error) {
+	return repo.GetSpaceCheckAuthRepoCreation(ctx, c.spaceStore, c.authorizer, session, parentRef)
+}
+
+func (c *Controller) getSpaceCheckAuthSpaceCreation(
+	ctx context.Context,
+	session *auth.Session,
+	parentRef string,
+) (*types.Space, error) {
+	parentRefAsID, err := strconv.ParseInt(parentRef, 10, 64)
+	if (parentRefAsID <= 0 && err == nil) || (len(strings.TrimSpace(parentRef)) == 0) {
+		// TODO: Restrict top level space creation - should be move to authorizer?
+		if auth.IsAnonymousSession(session) {
+			return nil, fmt.Errorf("anonymous user not allowed to create top level spaces: %w", usererror.ErrUnauthorized)
+		}
+
+		return &types.Space{}, nil
+	}
+
+	parentSpace, err := c.spaceStore.FindByRef(ctx, parentRef)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get parent space: %w", err)
+	}
+
+	if err = apiauth.CheckSpaceScope(
+		ctx,
+		c.authorizer,
+		session,
+		parentSpace,
+		enum.ResourceTypeSpace,
+		enum.PermissionSpaceEdit,
+	); err != nil {
+		return nil, fmt.Errorf("authorization failed: %w", err)
+	}
+
+	return parentSpace, nil
+}

app/api/controller/space/create.go

@@ -21,7 +21,6 @@ import (
 	"strings"
 	"time"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/api/usererror"
 	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/app/bootstrap"
@@ -175,40 +174,6 @@ func (c *Controller) createSpaceInnerInTX(
 	return space, nil
 }
 
-func (c *Controller) getSpaceCheckAuthSpaceCreation(
-	ctx context.Context,
-	session *auth.Session,
-	parentRef string,
-) (*types.Space, error) {
-	parentRefAsID, err := strconv.ParseInt(parentRef, 10, 64)
-	if (parentRefAsID <= 0 && err == nil) || (len(strings.TrimSpace(parentRef)) == 0) {
-		// TODO: Restrict top level space creation - should be move to authorizer?
-		if auth.IsAnonymousSession(session) {
-			return nil, fmt.Errorf("anonymous user not allowed to create top level spaces: %w", usererror.ErrUnauthorized)
-		}
-
-		return &types.Space{}, nil
-	}
-
-	parentSpace, err := c.spaceStore.FindByRef(ctx, parentRef)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get parent space: %w", err)
-	}
-
-	if err = apiauth.CheckSpaceScope(
-		ctx,
-		c.authorizer,
-		session,
-		parentSpace,
-		enum.ResourceTypeSpace,
-		enum.PermissionSpaceEdit,
-	); err != nil {
-		return nil, fmt.Errorf("authorization failed: %w", err)
-	}
-
-	return parentSpace, nil
-}
-
 func (c *Controller) sanitizeCreateInput(in *CreateInput) error {
 	// TODO [CODE-1363]: remove after identifier migration.
 	if in.Identifier == "" {

app/api/controller/space/events.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/app/sse"
 	"github.com/harness/gitness/types/enum"
@@ -29,13 +28,9 @@ func (c *Controller) Events(
 	session *auth.Session,
 	spaceRef string,
 ) (<-chan *sse.Event, <-chan error, func(context.Context) error, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceView)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("failed to find space ref: %w", err)
+		return nil, nil, nil, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceView); err != nil {
-		return nil, nil, nil, fmt.Errorf("failed to authorize stream: %w", err)
 	}
 
 	chEvents, chErr, sseCancel := c.sseStreamer.Stream(ctx, space.ID)

app/api/controller/space/export.go

@@ -19,7 +19,6 @@ import (
 	"errors"
 	"fmt"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/api/usererror"
 	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/app/services/exporter"
@@ -36,13 +35,9 @@ type ExportInput struct {
 
 // Export creates a new empty repository in harness code and does git push to it.
 func (c *Controller) Export(ctx context.Context, session *auth.Session, spaceRef string, in *ExportInput) error {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceEdit)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceEdit); err != nil {
-		return err
 	}
 
 	err = c.sanitizeExportInput(in)

app/api/controller/space/export_progress.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"fmt"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/api/usererror"
 	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/app/services/exporter"
@@ -37,13 +36,9 @@ func (c *Controller) ExportProgress(ctx context.Context,
 	session *auth.Session,
 	spaceRef string,
 ) (ExportProgressOutput, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceView)
 	if err != nil {
-		return ExportProgressOutput{}, err
+		return ExportProgressOutput{}, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceView); err != nil {
-		return ExportProgressOutput{}, err
 	}
 
 	progress, err := c.exporter.GetProgressForSpace(ctx, space.ID)

app/api/controller/space/find.go

@@ -16,8 +16,8 @@ package space
 
 import (
 	"context"
+	"fmt"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/types/enum"
 )
@@ -26,13 +26,9 @@ import (
 * Find finds a space.
  */
 func (c *Controller) Find(ctx context.Context, session *auth.Session, spaceRef string) (*SpaceOutput, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceView)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceView); err != nil {
-		return nil, err
 	}
 
 	return GetSpaceOutput(ctx, c.publicAccess, space)

app/api/controller/space/import_repositories.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"time"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/api/controller/limiter"
 	repoctrl "github.com/harness/gitness/app/api/controller/repo"
 	"github.com/harness/gitness/app/api/usererror"
@@ -31,7 +30,6 @@ import (
 	"github.com/harness/gitness/audit"
 	"github.com/harness/gitness/store"
 	"github.com/harness/gitness/types"
-	"github.com/harness/gitness/types/enum"
 
 	"github.com/rs/zerolog/log"
 )
@@ -45,33 +43,6 @@ type ImportRepositoriesOutput struct {
 	DuplicateRepos []*repoctrl.RepositoryOutput `json:"duplicate_repos"` // repos which already exist in the space.
 }
 
-// getSpaceCheckAuth checks whether the user has repo permissions permission.
-func (c *Controller) getSpaceCheckAuth(
-	ctx context.Context,
-	session *auth.Session,
-	spaceRef string,
-	permission enum.Permission,
-) (*types.Space, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
-	if err != nil {
-		return nil, fmt.Errorf("parent space not found: %w", err)
-	}
-
-	// create is a special case - check permission without specific resource
-	scope := &types.Scope{SpacePath: space.Path}
-	resource := &types.Resource{
-		Type:       enum.ResourceTypeRepo,
-		Identifier: "",
-	}
-
-	err = apiauth.Check(ctx, c.authorizer, session, scope, resource, permission)
-	if err != nil {
-		return nil, fmt.Errorf("auth check failed: %w", err)
-	}
-
-	return space, nil
-}
-
 // ImportRepositories imports repositories into an existing space. It ignores and continues on
 // repo naming conflicts.
 //
@@ -82,7 +53,7 @@ func (c *Controller) ImportRepositories(
 	spaceRef string,
 	in *ImportRepositoriesInput,
 ) (ImportRepositoriesOutput, error) {
-	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionRepoCreate)
+	space, err := c.getSpaceCheckAuthRepoCreation(ctx, session, spaceRef)
 	if err != nil {
 		return ImportRepositoriesOutput{}, err
 	}

app/api/controller/space/membership_add.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"time"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/api/usererror"
 	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/store"
@@ -61,13 +60,9 @@ func (c *Controller) MembershipAdd(ctx context.Context,
 	spaceRef string,
 	in *MembershipAddInput,
 ) (*types.MembershipUser, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceEdit)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceEdit); err != nil {
-		return nil, err
 	}
 
 	err = in.Validate()

app/api/controller/space/membership_delete.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"
@@ -30,13 +29,9 @@ func (c *Controller) MembershipDelete(ctx context.Context,
 	spaceRef string,
 	userUID string,
 ) error {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceEdit)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceEdit); err != nil {
-		return err
 	}
 
 	user, err := c.principalStore.FindUserByUID(ctx, userUID)

app/api/controller/space/membership_list.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/store/database/dbtx"
 	"github.com/harness/gitness/types"
@@ -31,13 +30,9 @@ func (c *Controller) MembershipList(ctx context.Context,
 	spaceRef string,
 	filter types.MembershipUserFilter,
 ) ([]types.MembershipUser, int64, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceView)
 	if err != nil {
-		return nil, 0, err
+		return nil, 0, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceView); err != nil {
-		return nil, 0, err
 	}
 
 	var memberships []types.MembershipUser

app/api/controller/space/membership_update.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"fmt"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/api/usererror"
 	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/types"
@@ -53,13 +52,9 @@ func (c *Controller) MembershipUpdate(ctx context.Context,
 	userUID string,
 	in *MembershipUpdateInput,
 ) (*types.MembershipUser, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceEdit)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceEdit); err != nil {
-		return nil, err
 	}
 
 	err = in.Validate()

app/api/controller/space/move.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"time"
 
-	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"
@@ -50,13 +49,9 @@ func (c *Controller) Move(
 	spaceRef string,
 	in *MoveInput,
 ) (*SpaceOutput, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceEdit)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceEdit); err != nil {
-		return nil, err
 	}
 
 	if err = c.sanitizeMoveInput(in, space.ParentID == 0); err != nil {

app/api/controller/space/soft_delete.go

@@ -20,7 +20,6 @@ import (
 	"math"
 	"time"
 
-	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"
@@ -36,19 +35,9 @@ func (c *Controller) SoftDelete(
 	session *auth.Session,
 	spaceRef string,
 ) (*SoftDeleteResponse, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceDelete)
 	if err != nil {
-		return nil, fmt.Errorf("failed to find space for soft delete: %w", err)
+		return nil, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(
-		ctx,
-		c.authorizer,
-		session,
-		space,
-		enum.PermissionSpaceDelete,
-	); err != nil {
-		return nil, fmt.Errorf("failed to check access: %w", err)
 	}
 
 	return c.SoftDeleteNoAuth(ctx, session, space)

app/api/controller/space/update.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"strings"
 
-	apiauth "github.com/harness/gitness/app/api/auth"
 	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/check"
@@ -42,13 +41,9 @@ func (c *Controller) Update(
 	spaceRef string,
 	in *UpdateInput,
 ) (*SpaceOutput, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceEdit)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to acquire access to space: %w", err)
-	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceEdit); err != nil {
-		return nil, err
 	}
 
 	if !in.hasChanges(space) {

app/api/controller/space/update_public_access.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/app/paths"
 	"github.com/harness/gitness/types/enum"
@@ -33,15 +32,10 @@ func (c *Controller) UpdatePublicAccess(ctx context.Context,
 	spaceRef string,
 	in *UpdatePublicAccessInput,
 ) (*SpaceOutput, error) {
-	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
+	space, err := c.getSpaceCheckAuth(ctx, session, spaceRef, enum.PermissionSpaceEdit)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to acquire access to space: %w", err)
 	}
-
-	if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceEdit); err != nil {
-		return nil, err
-	}
-
 	parentPath, _, err := paths.DisectLeaf(space.Path)
 	if err != nil {
 		return nil, fmt.Errorf("failed to disect path %q: %w", space.Path, err)