From f78f767ae252d19dda99589500f3778b9e1f4d95 Mon Sep 17 00:00:00 2001 From: Atefeh Mohseni Ejiyeh Date: Mon, 13 Jan 2025 19:02:51 +0000 Subject: [PATCH] feat: [CODE-2818]: Add Archive Repo functionality (#3051) --- app/api/auth/repo.go | 56 +++++++++++++++++- app/api/controller/check/controller.go | 13 +++- app/api/controller/execution/cancel.go | 16 ++--- app/api/controller/execution/controller.go | 42 +++++++++++++ app/api/controller/execution/create.go | 11 +--- app/api/controller/execution/delete.go | 16 ++--- app/api/controller/execution/find.go | 16 ++--- app/api/controller/execution/list.go | 16 ++--- app/api/controller/githook/controller.go | 9 ++- app/api/controller/githook/pre_receive.go | 7 ++- app/api/controller/migrate/controller.go | 2 + app/api/controller/pipeline/controller.go | 37 ++++++++++++ app/api/controller/pipeline/create.go | 10 +--- app/api/controller/pipeline/delete.go | 10 +--- app/api/controller/pipeline/find.go | 11 +--- app/api/controller/pipeline/update.go | 10 +--- app/api/controller/pullreq/controller.go | 17 ++++-- app/api/controller/pullreq/reviewer_delete.go | 5 ++ app/api/controller/repo/controller.go | 26 +++----- app/api/controller/repo/helper.go | 59 +++++++++++++++---- app/api/controller/repo/list_pipelines.go | 7 ++- .../controller/repo/list_service_accounts.go | 21 +++---- app/api/controller/repo/move.go | 14 +---- app/api/controller/repo/update.go | 51 ++++++++++++++-- app/api/controller/reposettings/controller.go | 29 +++++---- .../controller/reposettings/general_find.go | 4 +- .../controller/reposettings/general_update.go | 4 +- app/api/controller/trigger/controller.go | 37 ++++++++++++ app/api/controller/trigger/create.go | 12 +--- app/api/controller/trigger/delete.go | 12 +--- app/api/controller/trigger/find.go | 10 +--- app/api/controller/trigger/list.go | 12 +--- app/api/controller/trigger/update.go | 12 +--- app/api/controller/upload/controller.go | 9 ++- app/api/controller/webhook/controller.go | 14 ++++- types/enum/repo.go | 3 + 36 files changed, 429 insertions(+), 211 deletions(-) diff --git a/app/api/auth/repo.go b/app/api/auth/repo.go index bca4cdbfb..0c3ab2a73 100644 --- a/app/api/auth/repo.go +++ b/app/api/auth/repo.go @@ -17,14 +17,14 @@ package auth import ( "context" "fmt" + "slices" "github.com/harness/gitness/app/auth" "github.com/harness/gitness/app/auth/authz" "github.com/harness/gitness/app/paths" + "github.com/harness/gitness/errors" "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" - - "github.com/pkg/errors" ) // CheckRepo checks if a repo specific permission is granted for the current auth session @@ -66,3 +66,55 @@ func IsRepoOwner( return err == nil, nil } + +// CheckRepoState checks if requested permission is allowed given the state of the repository. +func CheckRepoState( + _ context.Context, + _ *auth.Session, + repo *types.Repository, + reqPermission enum.Permission, + additionalAllowedRepoStates ...enum.RepoState, +) error { + permissionsAllowedPerRepoState := map[enum.RepoState][]enum.Permission{ + enum.RepoStateActive: { + enum.PermissionRepoView, + enum.PermissionRepoCreate, + enum.PermissionRepoEdit, + enum.PermissionRepoPush, + enum.PermissionRepoReview, + enum.PermissionRepoDelete, + enum.PermissionRepoReportCommitCheck, + + enum.PermissionPipelineView, + enum.PermissionPipelineExecute, + enum.PermissionPipelineEdit, + enum.PermissionPipelineDelete, + + enum.PermissionServiceAccountView, + }, + + enum.RepoStateArchived: { + enum.PermissionRepoView, + + enum.PermissionPipelineView, + + enum.PermissionServiceAccountView, + }, + + // allowed permissions for repos on transition states during import/migration are handled by their controller. + enum.RepoStateGitImport: {}, + enum.RepoStateMigrateDataImport: {}, + enum.RepoStateMigrateGitPush: {}, + } + + if len(additionalAllowedRepoStates) > 0 && slices.Contains(additionalAllowedRepoStates, repo.State) { + return nil + } + + defaultAllowedPermissions := permissionsAllowedPerRepoState[repo.State] + if !slices.Contains(defaultAllowedPermissions, reqPermission) { + return errors.PreconditionFailed("Operation is not allowed for repository in state %s", repo.State) + } + + return nil +} diff --git a/app/api/controller/check/controller.go b/app/api/controller/check/controller.go index 76afc0aef..f221619de 100644 --- a/app/api/controller/check/controller.go +++ b/app/api/controller/check/controller.go @@ -68,8 +68,13 @@ func NewController( } } -func (c *Controller) getRepoCheckAccess(ctx context.Context, - session *auth.Session, repoRef string, reqPermission enum.Permission, +//nolint:unparam +func (c *Controller) getRepoCheckAccess( + ctx context.Context, + session *auth.Session, + repoRef string, + reqPermission enum.Permission, + allowedRepoStates ...enum.RepoState, ) (*types.Repository, error) { if repoRef == "" { return nil, usererror.BadRequest("A valid repository reference must be provided.") @@ -80,6 +85,10 @@ func (c *Controller) getRepoCheckAccess(ctx context.Context, return nil, fmt.Errorf("failed to find repository: %w", err) } + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, reqPermission); err != nil { return nil, fmt.Errorf("access check failed: %w", err) } diff --git a/app/api/controller/execution/cancel.go b/app/api/controller/execution/cancel.go index 7fd123a58..efa7c0bb6 100644 --- a/app/api/controller/execution/cancel.go +++ b/app/api/controller/execution/cancel.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/pipeline/checks" "github.com/harness/gitness/types" @@ -34,14 +33,15 @@ func (c *Controller) Cancel( pipelineIdentifier string, executionNum int64, ) (*types.Execution, error) { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess( + ctx, + session, + repoRef, + pipelineIdentifier, + enum.PermissionPipelineExecute, + ) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineExecute) - if err != nil { - return nil, fmt.Errorf("failed to authorize: %w", err) + return nil, err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/execution/controller.go b/app/api/controller/execution/controller.go index 7b709d4a5..5ab90ff81 100644 --- a/app/api/controller/execution/controller.go +++ b/app/api/controller/execution/controller.go @@ -15,6 +15,11 @@ package execution import ( + "context" + "fmt" + + apiauth "github.com/harness/gitness/app/api/auth" + "github.com/harness/gitness/app/auth" "github.com/harness/gitness/app/auth/authz" "github.com/harness/gitness/app/pipeline/canceler" "github.com/harness/gitness/app/pipeline/commit" @@ -22,6 +27,8 @@ import ( "github.com/harness/gitness/app/services/refcache" "github.com/harness/gitness/app/store" "github.com/harness/gitness/store/database/dbtx" + "github.com/harness/gitness/types" + "github.com/harness/gitness/types/enum" ) type Controller struct { @@ -62,3 +69,38 @@ func NewController( repoFinder: repoFinder, } } + +// getRepoCheckPipelineAccess fetches a repo, checks if the permission is allowed based on the repo state, +// and checks if the current user has permission to access pipelines belong to it. +// +//nolint:unparam +func (c *Controller) getRepoCheckPipelineAccess( + ctx context.Context, + session *auth.Session, + repoRef string, + pipelineIdentifier string, + reqPermission enum.Permission, + allowedRepoStates ...enum.RepoState, +) (*types.Repository, error) { + repo, err := c.repoFinder.FindByRef(ctx, repoRef) + if err != nil { + return nil, fmt.Errorf("failed to find repo by ref: %w", err) + } + + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + + err = apiauth.CheckPipeline( + ctx, + c.authorizer, + session, + repo.Path, + pipelineIdentifier, + reqPermission) + if err != nil { + return nil, fmt.Errorf("failed to authorize: %w", err) + } + + return repo, nil +} diff --git a/app/api/controller/execution/create.go b/app/api/controller/execution/create.go index c03d3df0c..b7fd3605a 100644 --- a/app/api/controller/execution/create.go +++ b/app/api/controller/execution/create.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/pipeline/triggerer" "github.com/harness/gitness/types" @@ -34,15 +33,9 @@ func (c *Controller) Create( pipelineIdentifier string, branch string, ) (*types.Execution, error) { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, pipelineIdentifier, enum.PermissionPipelineExecute) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, - pipelineIdentifier, enum.PermissionPipelineExecute) - if err != nil { - return nil, fmt.Errorf("failed to authorize: %w", err) + return nil, err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/execution/delete.go b/app/api/controller/execution/delete.go index 58775e5ea..f7fd72f8a 100644 --- a/app/api/controller/execution/delete.go +++ b/app/api/controller/execution/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/enum" ) @@ -30,14 +29,15 @@ func (c *Controller) Delete( pipelineIdentifier string, executionNum int64, ) error { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess( + ctx, + session, + repoRef, + pipelineIdentifier, + enum.PermissionPipelineDelete, + ) if err != nil { - return fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineDelete) - if err != nil { - return fmt.Errorf("failed to authorize: %w", err) + return err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/execution/find.go b/app/api/controller/execution/find.go index 9c216dac8..ef5238947 100644 --- a/app/api/controller/execution/find.go +++ b/app/api/controller/execution/find.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" @@ -31,14 +30,15 @@ func (c *Controller) Find( pipelineIdentifier string, executionNum int64, ) (*types.Execution, error) { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess( + ctx, + session, + repoRef, + pipelineIdentifier, + enum.PermissionPipelineView, + ) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineView) - if err != nil { - return nil, fmt.Errorf("failed to authorize: %w", err) + return nil, err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/execution/list.go b/app/api/controller/execution/list.go index c63c416f6..f763a7046 100644 --- a/app/api/controller/execution/list.go +++ b/app/api/controller/execution/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" @@ -32,14 +31,15 @@ func (c *Controller) List( pipelineIdentifier string, pagination types.Pagination, ) ([]*types.Execution, int64, error) { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess( + ctx, + session, + repoRef, + pipelineIdentifier, + enum.PermissionPipelineView, + ) if err != nil { - return nil, 0, fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineView) - if err != nil { - return nil, 0, fmt.Errorf("failed to authorize: %w", err) + return nil, 0, err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/githook/controller.go b/app/api/controller/githook/controller.go index 07f01788c..53ddf8577 100644 --- a/app/api/controller/githook/controller.go +++ b/app/api/controller/githook/controller.go @@ -92,8 +92,12 @@ func NewController( } } -func (c *Controller) getRepoCheckAccess(ctx context.Context, - _ *auth.Session, repoID int64, _ enum.Permission) (*types.Repository, error) { +func (c *Controller) getRepoCheckAccess( + ctx context.Context, + _ *auth.Session, + repoID int64, + _ enum.Permission, +) (*types.Repository, error) { if repoID < 1 { return nil, usererror.BadRequest("A valid repository reference must be provided.") } @@ -102,6 +106,7 @@ func (c *Controller) getRepoCheckAccess(ctx context.Context, if err != nil { return nil, fmt.Errorf("failed to find repo with id %d: %w", repoID, err) } + // repo state check is done in pre-receive. // TODO: execute permission check. block anything but Harness service? diff --git a/app/api/controller/githook/pre_receive.go b/app/api/controller/githook/pre_receive.go index 6ead216de..d74ca5f6e 100644 --- a/app/api/controller/githook/pre_receive.go +++ b/app/api/controller/githook/pre_receive.go @@ -33,6 +33,9 @@ import ( "golang.org/x/exp/slices" ) +// allowedRepoStatesForPush lists repository states that git push is allowed for internal and external calls. +var allowedRepoStatesForPush = []enum.RepoState{enum.RepoStateActive, enum.RepoStateMigrateGitPush} + // PreReceive executes the pre-receive hook for a git repository. func (c *Controller) PreReceive( ctx context.Context, @@ -47,8 +50,8 @@ func (c *Controller) PreReceive( return hook.Output{}, err } - if !in.Internal && repo.State != enum.RepoStateActive && repo.State != enum.RepoStateMigrateGitPush { - output.Error = ptr.String("Push not allowed in the current repository state") + if !in.Internal && !slices.Contains(allowedRepoStatesForPush, repo.State) { + output.Error = ptr.String(fmt.Sprintf("Push not allowed when repository is in '%s' state", repo.State)) return output, nil } diff --git a/app/api/controller/migrate/controller.go b/app/api/controller/migrate/controller.go index b4a01e59a..5e21acc08 100644 --- a/app/api/controller/migrate/controller.go +++ b/app/api/controller/migrate/controller.go @@ -108,6 +108,8 @@ func (c *Controller) getRepoCheckAccess( return nil, fmt.Errorf("failed to find repo: %w", err) } + // repo state check happens per operation as it varies given the stage of the migration. + if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, reqPermission); err != nil { return nil, fmt.Errorf("failed to verify authorization: %w", err) } diff --git a/app/api/controller/pipeline/controller.go b/app/api/controller/pipeline/controller.go index 23f09729b..7a22c4bbc 100644 --- a/app/api/controller/pipeline/controller.go +++ b/app/api/controller/pipeline/controller.go @@ -15,10 +15,17 @@ package pipeline import ( + "context" + "fmt" + + apiauth "github.com/harness/gitness/app/api/auth" + "github.com/harness/gitness/app/auth" "github.com/harness/gitness/app/auth/authz" events "github.com/harness/gitness/app/events/pipeline" "github.com/harness/gitness/app/services/refcache" "github.com/harness/gitness/app/store" + "github.com/harness/gitness/types" + "github.com/harness/gitness/types/enum" ) type Controller struct { @@ -45,3 +52,33 @@ func NewController( reporter: reporter, } } + +// getRepoCheckPipelineAccess fetches a repo, checks if operation is allowed given the repo state +// and checks if the current user has permission to access pipelines of the repo. +// +//nolint:unparam +func (c *Controller) getRepoCheckPipelineAccess( + ctx context.Context, + session *auth.Session, + repoRef string, + pipelineIdentifier string, + reqPermission enum.Permission, + allowedRepoStates ...enum.RepoState, +) (*types.Repository, error) { + repo, err := c.repoFinder.FindByRef(ctx, repoRef) + if err != nil { + return nil, fmt.Errorf("failed to find repo by ref: %w", err) + } + + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + + err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, + pipelineIdentifier, reqPermission) + if err != nil { + return nil, fmt.Errorf("failed to authorize: %w", err) + } + + return repo, nil +} diff --git a/app/api/controller/pipeline/create.go b/app/api/controller/pipeline/create.go index 55001fb04..8a4ec386f 100644 --- a/app/api/controller/pipeline/create.go +++ b/app/api/controller/pipeline/create.go @@ -20,7 +20,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" events "github.com/harness/gitness/app/events/pipeline" @@ -57,14 +56,9 @@ func (c *Controller) Create( return nil, fmt.Errorf("failed to sanitize input: %w", err) } - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, "", enum.PermissionPipelineEdit) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, "", enum.PermissionPipelineEdit) - if err != nil { - return nil, fmt.Errorf("failed to authorize pipeline: %w", err) + return nil, err } var pipeline *types.Pipeline diff --git a/app/api/controller/pipeline/delete.go b/app/api/controller/pipeline/delete.go index f4fc8adb4..75da92fd4 100644 --- a/app/api/controller/pipeline/delete.go +++ b/app/api/controller/pipeline/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/enum" ) @@ -29,14 +28,9 @@ func (c *Controller) Delete( repoRef string, identifier string, ) error { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, identifier, enum.PermissionPipelineDelete) if err != nil { - return fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, identifier, enum.PermissionPipelineDelete) - if err != nil { - return fmt.Errorf("failed to authorize pipeline: %w", err) + return err } err = c.pipelineStore.DeleteByIdentifier(ctx, repo.ID, identifier) diff --git a/app/api/controller/pipeline/find.go b/app/api/controller/pipeline/find.go index d826abbfb..76d8e7dd3 100644 --- a/app/api/controller/pipeline/find.go +++ b/app/api/controller/pipeline/find.go @@ -16,9 +16,7 @@ package pipeline 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,14 +28,9 @@ func (c *Controller) Find( repoRef string, identifier string, ) (*types.Pipeline, error) { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, identifier, enum.PermissionPipelineView) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, identifier, enum.PermissionPipelineView) - if err != nil { - return nil, fmt.Errorf("failed to authorize pipeline: %w", err) + return nil, err } return c.pipelineStore.FindByIdentifier(ctx, repo.ID, identifier) diff --git a/app/api/controller/pipeline/update.go b/app/api/controller/pipeline/update.go index a6a531dd9..bbf9d9318 100644 --- a/app/api/controller/pipeline/update.go +++ b/app/api/controller/pipeline/update.go @@ -19,7 +19,6 @@ import ( "fmt" "strings" - apiauth "github.com/harness/gitness/app/api/auth" "github.com/harness/gitness/app/auth" events "github.com/harness/gitness/app/events/pipeline" "github.com/harness/gitness/types" @@ -43,14 +42,9 @@ func (c *Controller) Update( identifier string, in *UpdateInput, ) (*types.Pipeline, error) { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, identifier, enum.PermissionPipelineEdit) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, identifier, enum.PermissionPipelineEdit) - if err != nil { - return nil, fmt.Errorf("failed to authorize pipeline: %w", err) + return nil, err } if err = c.sanitizeUpdateInput(in); err != nil { diff --git a/app/api/controller/pullreq/controller.go b/app/api/controller/pullreq/controller.go index d50600d80..1b4743a32 100644 --- a/app/api/controller/pullreq/controller.go +++ b/app/api/controller/pullreq/controller.go @@ -185,21 +185,26 @@ func (c *Controller) getRepo(ctx context.Context, repoRef string) (*types.Reposi return nil, fmt.Errorf("failed to find repository: %w", err) } - if repo.State != enum.RepoStateActive { - return nil, usererror.BadRequest("Repository is not ready to use.") - } - return repo, nil } -func (c *Controller) getRepoCheckAccess(ctx context.Context, - session *auth.Session, repoRef string, reqPermission enum.Permission, +//nolint:unparam +func (c *Controller) getRepoCheckAccess( + ctx context.Context, + session *auth.Session, + repoRef string, + reqPermission enum.Permission, + allowedRepoStates ...enum.RepoState, ) (*types.Repository, error) { repo, err := c.getRepo(ctx, repoRef) if err != nil { return nil, err } + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, reqPermission); err != nil { return nil, fmt.Errorf("access check failed: %w", err) } diff --git a/app/api/controller/pullreq/reviewer_delete.go b/app/api/controller/pullreq/reviewer_delete.go index 9827ac686..9eb1c24fc 100644 --- a/app/api/controller/pullreq/reviewer_delete.go +++ b/app/api/controller/pullreq/reviewer_delete.go @@ -60,6 +60,11 @@ func (c *Controller) ReviewerDelete( reqPermission = enum.PermissionRepoEdit // RepoEdit permission is required to remove a submitted review. } + // Make sure the repository state allows delete reviewer operation. + if err = apiauth.CheckRepoState(ctx, session, repo, reqPermission); err != nil { + return err + } + // Make sure the caller has the right permission even if the PR is merged, so that we can return the correct error. if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, reqPermission); err != nil { return fmt.Errorf("access check failed: %w", err) diff --git a/app/api/controller/repo/controller.go b/app/api/controller/repo/controller.go index 3300ddac8..05fb4d380 100644 --- a/app/api/controller/repo/controller.go +++ b/app/api/controller/repo/controller.go @@ -51,12 +51,13 @@ import ( "github.com/harness/gitness/types/enum" ) -var errPublicRepoCreationDisabled = usererror.BadRequestf("Public repository creation is disabled.") +var errPublicRepoCreationDisabled = usererror.BadRequest("Public repository creation is disabled.") type RepositoryOutput struct { types.Repository IsPublic bool `json:"is_public" yaml:"is_public"` Importing bool `json:"importing" yaml:"-"` + Archived bool `json:"archived" yaml:"-"` } // TODO [CODE-1363]: remove after identifier migration. @@ -187,26 +188,16 @@ func NewController( } } -// getRepo fetches an active repo (not one that is currently being imported). -func (c *Controller) getRepo( - ctx context.Context, - repoRef string, -) (*types.Repository, error) { - return GetRepo( - ctx, - c.repoFinder, - repoRef, - ActiveRepoStates, - ) -} - -// getRepoCheckAccess fetches an active repo (not one that is currently being imported) +// getRepoCheckAccess fetches a repo, checks if repo state allows requested permission // and checks if the current user has permission to access it. +// +//nolint:unparam func (c *Controller) getRepoCheckAccess( ctx context.Context, session *auth.Session, repoRef string, reqPermission enum.Permission, + allowedRepoStates ...enum.RepoState, ) (*types.Repository, error) { return GetRepoCheckAccess( ctx, @@ -215,7 +206,7 @@ func (c *Controller) getRepoCheckAccess( session, repoRef, reqPermission, - ActiveRepoStates, + allowedRepoStates..., ) } @@ -234,7 +225,8 @@ func (c *Controller) getRepoCheckAccessForGit( session, repoRef, reqPermission, - nil, // Any state allowed - we'll block in the pre-receive hook. + // importing/migrating states are allowed - we'll block in the pre-receive hook if needed. + enum.RepoStateGitImport, enum.RepoStateMigrateDataImport, enum.RepoStateMigrateGitPush, ) } diff --git a/app/api/controller/repo/helper.go b/app/api/controller/repo/helper.go index bdf4f3671..6bae52eb7 100644 --- a/app/api/controller/repo/helper.go +++ b/app/api/controller/repo/helper.go @@ -24,20 +24,24 @@ import ( "github.com/harness/gitness/app/auth/authz" "github.com/harness/gitness/app/services/publicaccess" "github.com/harness/gitness/app/services/refcache" + "github.com/harness/gitness/app/store" "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" "golang.org/x/exp/slices" ) -var ActiveRepoStates = []enum.RepoState{enum.RepoStateActive} +var importingStates = []enum.RepoState{ + enum.RepoStateGitImport, + enum.RepoStateMigrateDataImport, + enum.RepoStateMigrateGitPush, +} // GetRepo fetches an repository. func GetRepo( ctx context.Context, repoFinder refcache.RepoFinder, repoRef string, - allowedStates []enum.RepoState, ) (*types.Repository, error) { if repoRef == "" { return nil, usererror.BadRequest("A valid repository reference must be provided.") @@ -48,14 +52,10 @@ func GetRepo( return nil, fmt.Errorf("failed to find repository: %w", err) } - if len(allowedStates) > 0 && !slices.Contains(allowedStates, repo.State) { - return nil, usererror.BadRequest("Repository is not ready to use.") - } - return repo, nil } -// GetRepoCheckAccess fetches an active repo (not one that is currently being imported) +// GetRepoCheckAccess fetches a repo with option to enforce repo state check // and checks if the current user has permission to access it. func GetRepoCheckAccess( ctx context.Context, @@ -64,13 +64,17 @@ func GetRepoCheckAccess( session *auth.Session, repoRef string, reqPermission enum.Permission, - allowedStates []enum.RepoState, + allowedRepoStates ...enum.RepoState, ) (*types.Repository, error) { - repo, err := GetRepo(ctx, repoFinder, repoRef, allowedStates) + repo, err := GetRepo(ctx, repoFinder, repoRef) if err != nil { return nil, fmt.Errorf("failed to find repo: %w", err) } + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + if err = apiauth.CheckRepo(ctx, authorizer, session, repo, reqPermission); err != nil { return nil, fmt.Errorf("access check failed: %w", err) } @@ -119,7 +123,8 @@ func GetRepoOutput( return &RepositoryOutput{ Repository: *repo, IsPublic: isPublic, - Importing: repo.State != enum.RepoStateActive, + Importing: slices.Contains(importingStates, repo.State), + Archived: repo.State == enum.RepoStateArchived, }, nil } @@ -131,6 +136,38 @@ func GetRepoOutputWithAccess( return &RepositoryOutput{ Repository: *repo, IsPublic: isPublic, - Importing: repo.State != enum.RepoStateActive, + Importing: slices.Contains(importingStates, repo.State), + Archived: repo.State == enum.RepoStateArchived, } } + +// GetRepoCheckServiceAccountAccess fetches a repo with option to enforce repo state check +// and checks if the current user has permission to access service accounts within repo. +func GetRepoCheckServiceAccountAccess( + ctx context.Context, + session *auth.Session, + authorizer authz.Authorizer, + repoRef string, + reqPermission enum.Permission, + repoFinder refcache.RepoFinder, + repoStore store.RepoStore, + spaceStore store.SpaceStore, + allowedRepoStates ...enum.RepoState, +) (*types.Repository, error) { + repo, err := GetRepo(ctx, repoFinder, repoRef) + if err != nil { + return nil, fmt.Errorf("failed to find repo: %w", err) + } + + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + + if err := apiauth.CheckServiceAccount(ctx, authorizer, session, spaceStore, repoStore, + enum.ParentResourceTypeRepo, repo.ID, "", reqPermission, + ); err != nil { + return nil, fmt.Errorf("access check failed: %w", err) + } + + return repo, nil +} diff --git a/app/api/controller/repo/list_pipelines.go b/app/api/controller/repo/list_pipelines.go index e9f6ca6aa..b7ae5a562 100644 --- a/app/api/controller/repo/list_pipelines.go +++ b/app/api/controller/repo/list_pipelines.go @@ -32,10 +32,15 @@ func (c *Controller) ListPipelines( repoRef string, filter *types.ListPipelinesFilter, ) ([]*types.Pipeline, int64, error) { - repo, err := c.getRepo(ctx, repoRef) + repo, err := GetRepo(ctx, c.repoFinder, repoRef) if err != nil { return nil, 0, fmt.Errorf("failed to find repo: %w", err) } + + if err := apiauth.CheckRepoState(ctx, session, repo, enum.PermissionPipelineView); err != nil { + return nil, 0, err + } + if err := apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, "", enum.PermissionPipelineView); err != nil { return nil, 0, fmt.Errorf("access check failed: %w", err) } diff --git a/app/api/controller/repo/list_service_accounts.go b/app/api/controller/repo/list_service_accounts.go index 28c945741..dcb03808f 100644 --- a/app/api/controller/repo/list_service_accounts.go +++ b/app/api/controller/repo/list_service_accounts.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,22 +29,16 @@ func (c *Controller) ListServiceAccounts( session *auth.Session, repoRef string, ) ([]*types.ServiceAccount, error) { - repo, err := c.getRepo(ctx, repoRef) - if err != nil { - return nil, fmt.Errorf("failed to find repo: %w", err) - } - - if err := apiauth.CheckServiceAccount( + repo, err := GetRepoCheckServiceAccountAccess( ctx, - c.authorizer, session, - c.spaceStore, - c.repoStore, - enum.ParentResourceTypeRepo, - repo.ID, - "", + c.authorizer, + repoRef, enum.PermissionServiceAccountView, - ); err != nil { + c.repoFinder, + c.repoStore, + c.spaceStore) + if err != nil { return nil, fmt.Errorf("access check failed: %w", err) } diff --git a/app/api/controller/repo/move.go b/app/api/controller/repo/move.go index 239faeda2..39d65084c 100644 --- a/app/api/controller/repo/move.go +++ b/app/api/controller/repo/move.go @@ -18,8 +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" "github.com/harness/gitness/types/enum" @@ -53,17 +51,9 @@ func (c *Controller) Move(ctx context.Context, return nil, fmt.Errorf("failed to sanitize input: %w", err) } - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoEdit) if err != nil { - return nil, err - } - - if repo.State != enum.RepoStateActive { - return nil, usererror.BadRequest("Can't move a repo that isn't ready to use.") - } - - if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, enum.PermissionRepoEdit); err != nil { - return nil, err + return nil, fmt.Errorf("failed to find or acquire access to repo: %w", err) } if !in.hasChanges(repo) { diff --git a/app/api/controller/repo/update.go b/app/api/controller/repo/update.go index a6b1940ee..f6675c2f2 100644 --- a/app/api/controller/repo/update.go +++ b/app/api/controller/repo/update.go @@ -19,6 +19,8 @@ import ( "fmt" "strings" + 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/paths" "github.com/harness/gitness/audit" @@ -27,15 +29,31 @@ import ( "github.com/harness/gitness/types/enum" "github.com/rs/zerolog/log" + "golang.org/x/exp/slices" ) // UpdateInput is used for updating a repo. type UpdateInput struct { - Description *string `json:"description"` + Description *string `json:"description"` + State *enum.RepoState `json:"state"` +} + +var allowedRepoStateTransitions = map[enum.RepoState][]enum.RepoState{ + enum.RepoStateActive: {enum.RepoStateArchived, enum.RepoStateMigrateDataImport}, + enum.RepoStateArchived: {enum.RepoStateActive}, + enum.RepoStateMigrateDataImport: {enum.RepoStateActive}, + enum.RepoStateMigrateGitPush: {enum.RepoStateActive, enum.RepoStateMigrateDataImport}, } func (in *UpdateInput) hasChanges(repo *types.Repository) bool { - return in.Description != nil && *in.Description != repo.Description + if in.Description != nil && *in.Description != repo.Description { + return true + } + if in.State != nil && *in.State != repo.State { + return true + } + + return false } // Update updates a repository. @@ -44,11 +62,27 @@ func (c *Controller) Update(ctx context.Context, repoRef string, in *UpdateInput, ) (*RepositoryOutput, error) { - repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoEdit) + repo, err := GetRepo(ctx, c.repoFinder, repoRef) + if err != nil { + return nil, fmt.Errorf("failed to find repo: %w", err) + } + + var additionalAllowedRepoStates []enum.RepoState + + if in.State != nil { + additionalAllowedRepoStates = []enum.RepoState{ + enum.RepoStateArchived, enum.RepoStateMigrateDataImport, enum.RepoStateMigrateGitPush} + } + + err = apiauth.CheckRepoState(ctx, session, repo, enum.PermissionRepoEdit, additionalAllowedRepoStates...) if err != nil { return nil, err } + if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, enum.PermissionRepoEdit); err != nil { + return nil, fmt.Errorf("access check failed: %w", err) + } + repoClone := repo.Clone() if !in.hasChanges(repo) { @@ -59,16 +93,25 @@ func (c *Controller) Update(ctx context.Context, return nil, fmt.Errorf("failed to sanitize input: %w", err) } + if in.State != nil && + !slices.Contains(allowedRepoStateTransitions[repo.State], *in.State) { + return nil, usererror.BadRequestf("Changing the state of a repository from %s to %s is not allowed.", + repo.State, *in.State) + } + repo, err = c.repoStore.UpdateOptLock(ctx, repo, func(repo *types.Repository) error { // update values only if provided if in.Description != nil { repo.Description = *in.Description } + if in.State != nil { + repo.State = *in.State + } return nil }) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to update the repo: %w", err) } err = c.auditService.Log(ctx, diff --git a/app/api/controller/reposettings/controller.go b/app/api/controller/reposettings/controller.go index 553daeb8d..9750e72a1 100644 --- a/app/api/controller/reposettings/controller.go +++ b/app/api/controller/reposettings/controller.go @@ -16,7 +16,9 @@ package reposettings import ( "context" + "fmt" + apiauth "github.com/harness/gitness/app/api/auth" "github.com/harness/gitness/app/api/controller/repo" "github.com/harness/gitness/app/auth" "github.com/harness/gitness/app/auth/authz" @@ -48,22 +50,27 @@ func NewController( } } -// getRepoCheckAccess fetches an active repo (not one that is currently being imported) +// getRepoCheckAccess fetches a repo, checks if operation is allowed given the repo state // and checks if the current user has permission to access it. func (c *Controller) getRepoCheckAccess( ctx context.Context, session *auth.Session, repoRef string, reqPermission enum.Permission, + allowedRepoStates ...enum.RepoState, ) (*types.Repository, error) { - // migrating repositories need to adjust the repo settings during the import, hence expanding the allowedstates. - return repo.GetRepoCheckAccess( - ctx, - c.repoFinder, - c.authorizer, - session, - repoRef, - reqPermission, - []enum.RepoState{enum.RepoStateActive, enum.RepoStateMigrateGitPush}, - ) + repo, err := repo.GetRepo(ctx, c.repoFinder, repoRef) + if err != nil { + return nil, err + } + + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + + if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, reqPermission); err != nil { + return nil, fmt.Errorf("access check failed: %w", err) + } + + return repo, nil } diff --git a/app/api/controller/reposettings/general_find.go b/app/api/controller/reposettings/general_find.go index d83bbaf81..bcbb915c5 100644 --- a/app/api/controller/reposettings/general_find.go +++ b/app/api/controller/reposettings/general_find.go @@ -28,7 +28,9 @@ func (c *Controller) GeneralFind( session *auth.Session, repoRef string, ) (*GeneralSettings, error) { - repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView) + // migrating repos need to adjust repo settings (like file-size-limit) during the migration. + var additionalAllowedRepoStates = []enum.RepoState{enum.RepoStateMigrateGitPush} + repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView, additionalAllowedRepoStates...) if err != nil { return nil, err } diff --git a/app/api/controller/reposettings/general_update.go b/app/api/controller/reposettings/general_update.go index cd9d704a5..2d66989e4 100644 --- a/app/api/controller/reposettings/general_update.go +++ b/app/api/controller/reposettings/general_update.go @@ -33,7 +33,9 @@ func (c *Controller) GeneralUpdate( repoRef string, in *GeneralSettings, ) (*GeneralSettings, error) { - repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoEdit) + // migrating repos need to adjust repo settings (like file-size-limit) during the migration. + var additionalAllowedRepoStates = []enum.RepoState{enum.RepoStateMigrateGitPush} + repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoEdit, additionalAllowedRepoStates...) if err != nil { return nil, err } diff --git a/app/api/controller/trigger/controller.go b/app/api/controller/trigger/controller.go index 876819e82..22743434f 100644 --- a/app/api/controller/trigger/controller.go +++ b/app/api/controller/trigger/controller.go @@ -15,9 +15,16 @@ package trigger import ( + "context" + "fmt" + + apiauth "github.com/harness/gitness/app/api/auth" + "github.com/harness/gitness/app/auth" "github.com/harness/gitness/app/auth/authz" "github.com/harness/gitness/app/services/refcache" "github.com/harness/gitness/app/store" + "github.com/harness/gitness/types" + "github.com/harness/gitness/types/enum" ) type Controller struct { @@ -40,3 +47,33 @@ func NewController( repoFinder: repoFinder, } } + +// getRepoCheckPipelineAccess fetches a repo, checks if the permission is allowed based on the repo state, +// and checks if the current user has permission to access pipelines of the repo. +// +//nolint:unparam +func (c *Controller) getRepoCheckPipelineAccess( + ctx context.Context, + session *auth.Session, + repoRef string, + pipelineIdentifier string, + reqPermission enum.Permission, + allowedRepoStates ...enum.RepoState, +) (*types.Repository, error) { + repo, err := c.repoFinder.FindByRef(ctx, repoRef) + if err != nil { + return nil, fmt.Errorf("failed to find repo by ref: %w", err) + } + + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + + err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, + pipelineIdentifier, reqPermission) + if err != nil { + return nil, fmt.Errorf("failed to authorize: %w", err) + } + + return repo, nil +} diff --git a/app/api/controller/trigger/create.go b/app/api/controller/trigger/create.go index de5b944aa..6481fd070 100644 --- a/app/api/controller/trigger/create.go +++ b/app/api/controller/trigger/create.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/check" @@ -48,16 +47,9 @@ func (c *Controller) Create( return nil, fmt.Errorf("invalid input: %w", err) } - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, pipelineIdentifier, enum.PermissionPipelineEdit) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - // Trigger permissions are associated with pipeline permissions. If a user has permissions - // to edit the pipeline, they will have permissions to create a trigger as well. - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineEdit) - if err != nil { - return nil, fmt.Errorf("failed to authorize pipeline: %w", err) + return nil, err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/trigger/delete.go b/app/api/controller/trigger/delete.go index 059c4c00c..76287f381 100644 --- a/app/api/controller/trigger/delete.go +++ b/app/api/controller/trigger/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/enum" ) @@ -30,16 +29,9 @@ func (c *Controller) Delete( pipelineIdentifier string, triggerIdentifier string, ) error { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, pipelineIdentifier, enum.PermissionPipelineEdit) if err != nil { - return fmt.Errorf("failed to find repo by ref: %w", err) - } - - // Trigger permissions are associated with pipeline permissions. If a user has permissions - // to edit the pipeline, they will have permissions to remove a trigger as well. - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineEdit) - if err != nil { - return fmt.Errorf("failed to authorize pipeline: %w", err) + return err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/trigger/find.go b/app/api/controller/trigger/find.go index e29371fa5..c6818bf1a 100644 --- a/app/api/controller/trigger/find.go +++ b/app/api/controller/trigger/find.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" @@ -31,14 +30,9 @@ func (c *Controller) Find( pipelineIdentifier string, triggerIdentifier string, ) (*types.Trigger, error) { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, pipelineIdentifier, enum.PermissionPipelineView) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineView) - if err != nil { - return nil, fmt.Errorf("failed to authorize pipeline: %w", err) + return nil, err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/trigger/list.go b/app/api/controller/trigger/list.go index d894e0a0f..47949e7b3 100644 --- a/app/api/controller/trigger/list.go +++ b/app/api/controller/trigger/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/types" "github.com/harness/gitness/types/enum" @@ -31,16 +30,9 @@ func (c *Controller) List( pipelineIdentifier string, filter types.ListQueryFilter, ) ([]*types.Trigger, int64, error) { - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, pipelineIdentifier, enum.PermissionPipelineView) if err != nil { - return nil, 0, fmt.Errorf("failed to find repo by ref: %w", err) - } - - // Trigger permissions are associated with pipeline permissions. If a user has permissions - // to view the pipeline, they will have permissions to list triggers as well. - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineView) - if err != nil { - return nil, 0, fmt.Errorf("failed to authorize pipeline: %w", err) + return nil, 0, err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/trigger/update.go b/app/api/controller/trigger/update.go index c2b92c901..25539153c 100644 --- a/app/api/controller/trigger/update.go +++ b/app/api/controller/trigger/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" @@ -49,16 +48,9 @@ func (c *Controller) Update( return nil, fmt.Errorf("invalid input: %w", err) } - repo, err := c.repoFinder.FindByRef(ctx, repoRef) + repo, err := c.getRepoCheckPipelineAccess(ctx, session, repoRef, pipelineIdentifier, enum.PermissionPipelineEdit) if err != nil { - return nil, fmt.Errorf("failed to find repo by ref: %w", err) - } - - // Trigger permissions are associated with pipeline permissions. If a user has permissions - // to edit the pipeline, they will have permissions to edit the trigger as well. - err = apiauth.CheckPipeline(ctx, c.authorizer, session, repo.Path, pipelineIdentifier, enum.PermissionPipelineEdit) - if err != nil { - return nil, fmt.Errorf("failed to authorize pipeline: %w", err) + return nil, err } pipeline, err := c.pipelineStore.FindByIdentifier(ctx, repo.ID, pipelineIdentifier) diff --git a/app/api/controller/upload/controller.go b/app/api/controller/upload/controller.go index 6cc218219..4551325f9 100644 --- a/app/api/controller/upload/controller.go +++ b/app/api/controller/upload/controller.go @@ -17,7 +17,6 @@ package upload import ( "bufio" "context" - "errors" "fmt" "io" "strings" @@ -28,6 +27,7 @@ import ( "github.com/harness/gitness/app/auth/authz" "github.com/harness/gitness/app/services/refcache" "github.com/harness/gitness/blob" + "github.com/harness/gitness/errors" "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" @@ -61,10 +61,13 @@ func NewController(authorizer authz.Authorizer, blobStore: blobStore, } } + +//nolint:unparam func (c *Controller) getRepoCheckAccess(ctx context.Context, session *auth.Session, repoRef string, permission enum.Permission, + allowedRepoStates ...enum.RepoState, ) (*types.Repository, error) { if repoRef == "" { return nil, usererror.BadRequest("A valid repository reference must be provided.") @@ -75,6 +78,10 @@ func (c *Controller) getRepoCheckAccess(ctx context.Context, return nil, fmt.Errorf("failed to find repo: %w", err) } + if err := apiauth.CheckRepoState(ctx, session, repo, permission, allowedRepoStates...); err != nil { + return nil, err + } + if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, permission); err != nil { return nil, fmt.Errorf("failed to verify authorization: %w", err) } diff --git a/app/api/controller/webhook/controller.go b/app/api/controller/webhook/controller.go index 5e6a78f2e..0ee2b8053 100644 --- a/app/api/controller/webhook/controller.go +++ b/app/api/controller/webhook/controller.go @@ -57,8 +57,14 @@ func NewController( } } -func (c *Controller) getRepoCheckAccess(ctx context.Context, - session *auth.Session, repoRef string, reqPermission enum.Permission) (*types.Repository, error) { +//nolint:unparam +func (c *Controller) getRepoCheckAccess( + ctx context.Context, + session *auth.Session, + repoRef string, + reqPermission enum.Permission, + allowedRepoStates ...enum.RepoState, +) (*types.Repository, error) { if repoRef == "" { return nil, errors.InvalidArgument("A valid repository reference must be provided.") } @@ -68,6 +74,10 @@ func (c *Controller) getRepoCheckAccess(ctx context.Context, return nil, fmt.Errorf("failed to find repo: %w", err) } + if err := apiauth.CheckRepoState(ctx, session, repo, reqPermission, allowedRepoStates...); err != nil { + return nil, err + } + if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, reqPermission); err != nil { return nil, fmt.Errorf("failed to verify authorization: %w", err) } diff --git a/types/enum/repo.go b/types/enum/repo.go index 8177bd03f..0f1b9ba79 100644 --- a/types/enum/repo.go +++ b/types/enum/repo.go @@ -82,6 +82,7 @@ const ( RepoStateGitImport RepoStateMigrateGitPush RepoStateMigrateDataImport + RepoStateArchived ) // String returns the string representation of the RepoState. @@ -95,6 +96,8 @@ func (state RepoState) String() string { return "migrate-git-push" case RepoStateMigrateDataImport: return "migrate-data-import" + case RepoStateArchived: + return "archived" default: return undefined }