diff --git a/app/api/controller/githook/controller.go b/app/api/controller/githook/controller.go index f7bca4c52..6ccf7ca4a 100644 --- a/app/api/controller/githook/controller.go +++ b/app/api/controller/githook/controller.go @@ -117,11 +117,11 @@ func (c *Controller) getRepoCheckAccess( return repo, nil } -// GetBaseSHAForRefUpdate returns the commit sha to which the new sha of the reference -// should be compared against when looking for incoming changes. +// GetBaseSHAForScanningChanges returns the commit sha to which the new sha of the reference +// should be compared against when scanning incoming changes. // NOTE: If no such a sha exists, then (sha.None, false, nil) is returned. // This will happen in case the default branch doesn't exist yet. -func GetBaseSHAForRefUpdate( +func GetBaseSHAForScanningChanges( ctx context.Context, rgit RestrictedGIT, repo *types.RepositoryCore, diff --git a/app/api/controller/githook/pre_receive.go b/app/api/controller/githook/pre_receive.go index 00f261deb..1bb2501cb 100644 --- a/app/api/controller/githook/pre_receive.go +++ b/app/api/controller/githook/pre_receive.go @@ -93,14 +93,6 @@ func (c *Controller) PreReceive( return hook.Output{}, fmt.Errorf("failed to find inner principal with id %d: %w", in.PrincipalID, err) } - err = c.userCommiterMatchCheck(ctx, rgit, repo, in, principal.Email, &output) - if output.Error != nil { - return output, nil - } - if err != nil { - return hook.Output{}, fmt.Errorf("failed to check user commiter match: %w", err) - } - dummySession := &auth.Session{Principal: *principal, Metadata: nil} err = c.checkProtectionRules(ctx, dummySession, repo, refUpdates, &output) diff --git a/app/api/controller/githook/pre_receive_scan_secrets.go b/app/api/controller/githook/pre_receive_scan_secrets.go index f86c2e716..461eeff18 100644 --- a/app/api/controller/githook/pre_receive_scan_secrets.go +++ b/app/api/controller/githook/pre_receive_scan_secrets.go @@ -102,7 +102,7 @@ func scanSecretsInternal(ctx context.Context, //nolint:nestif if refUpdate.Old.IsNil() { if baseRevFallBack == nil { - fallbackSHA, fallbackAvailable, err := GetBaseSHAForRefUpdate( + fallbackSHA, fallbackAvailable, err := GetBaseSHAForScanningChanges( ctx, rgit, repo, diff --git a/app/api/controller/githook/pre_receive_user_commiter.go b/app/api/controller/githook/pre_receive_user_commiter.go deleted file mode 100644 index 14fc5b74c..000000000 --- a/app/api/controller/githook/pre_receive_user_commiter.go +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright 2023 Harness, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package githook - -import ( - "context" - "fmt" - - "github.com/harness/gitness/app/api/usererror" - "github.com/harness/gitness/app/services/settings" - "github.com/harness/gitness/git" - "github.com/harness/gitness/git/hook" - "github.com/harness/gitness/git/sha" - "github.com/harness/gitness/types" - - "github.com/gotidy/ptr" -) - -func (c *Controller) userCommiterMatchCheck( - ctx context.Context, - rgit RestrictedGIT, - repo *types.RepositoryCore, - in types.GithookPreReceiveInput, - principalEmail string, - output *hook.Output, -) error { - userCommiterMatch, err := settings.RepoGet( - ctx, - c.settings, - repo.ID, - settings.KeyUserCommiterMatch, - settings.DefaultUserCommiterMatch, - ) - if err != nil { - return fmt.Errorf("failed to check settings for user commiter match: %w", err) - } - if !userCommiterMatch { - return nil - } - - invalidShaEmailMap := make(map[string]string) - - // block any push that contains commits not committed by the user - for _, refUpdate := range in.RefUpdates { - baseSHA, fallbackAvailable, err := GetBaseSHAForRefUpdate( - ctx, - rgit, - repo, - in.Environment, - in.RefUpdates, - refUpdate, - ) - if err != nil { - return fmt.Errorf("failed to get fallback sha: %w", err) - } - // the default branch doesn't exist yet - if !fallbackAvailable { - baseSHA = sha.None - } - - shaEmailMap, err := c.git.GetBranchCommiterEmails(ctx, &git.GetBranchCommitterEmailsParams{ - ReadParams: git.ReadParams{ - RepoUID: repo.GitUID, - AlternateObjectDirs: in.Environment.AlternateObjectDirs, - }, - BaseSHA: baseSHA, - RevSHA: refUpdate.New, - }) - if err != nil { - return fmt.Errorf("failed to get commiter emails for %s: %w", refUpdate.Ref, err) - } - - for sha, email := range shaEmailMap { - if email != principalEmail { - invalidShaEmailMap[sha] = email - } - } - } - - if len(invalidShaEmailMap) > 0 { - output.Error = ptr.String(usererror.ErrCommiterUserMismatch.Error()) - printUserCommiterMismatch(output, invalidShaEmailMap) - } - - return nil -} diff --git a/app/api/controller/githook/print.go b/app/api/controller/githook/print.go index d738c7dfc..af6689fc4 100644 --- a/app/api/controller/githook/print.go +++ b/app/api/controller/githook/print.go @@ -137,25 +137,6 @@ func printOversizeFiles( ) } -func printUserCommiterMismatch(output *hook.Output, shaEmailMap map[string]string) { - output.Messages = append( - output.Messages, - colorScanHeader.Sprintf( - "Push contains commits where committer is not an authenticated user:", - ), - "", // add empty line for making it visually more consumable - ) - - for sha, email := range shaEmailMap { - output.Messages = append( - output.Messages, - fmt.Sprintf(" %s", sha), - fmt.Sprintf(" Commiter: %s", email), - "", // add empty line for making it visually more consumable - ) - } -} - func singularOrPlural(noun string, plural bool) string { if plural { return noun + "s" diff --git a/app/api/controller/reposettings/general.go b/app/api/controller/reposettings/general.go index 6c6751eec..b1ebe6922 100644 --- a/app/api/controller/reposettings/general.go +++ b/app/api/controller/reposettings/general.go @@ -22,26 +22,23 @@ import ( // GeneralSettings represent the general repository settings as exposed externally. type GeneralSettings struct { - FileSizeLimit *int64 `json:"file_size_limit" yaml:"file_size_limit"` - UserCommiterMatch *bool `json:"user_commiter_match" yaml:"user_commiter_match"` + FileSizeLimit *int64 `json:"file_size_limit" yaml:"file_size_limit"` } func GetDefaultGeneralSettings() *GeneralSettings { return &GeneralSettings{ - FileSizeLimit: ptr.Int64(settings.DefaultFileSizeLimit), - UserCommiterMatch: ptr.Bool(settings.DefaultUserCommiterMatch), + FileSizeLimit: ptr.Int64(settings.DefaultFileSizeLimit), } } func GetGeneralSettingsMappings(s *GeneralSettings) []settings.SettingHandler { return []settings.SettingHandler{ settings.Mapping(settings.KeyFileSizeLimit, s.FileSizeLimit), - settings.Mapping(settings.KeyUserCommiterMatch, s.UserCommiterMatch), } } func GetGeneralSettingsAsKeyValues(s *GeneralSettings) []settings.KeyValue { - kvs := make([]settings.KeyValue, 0, 2) + kvs := make([]settings.KeyValue, 0, 1) if s.FileSizeLimit != nil { kvs = append(kvs, settings.KeyValue{ @@ -49,13 +46,5 @@ func GetGeneralSettingsAsKeyValues(s *GeneralSettings) []settings.KeyValue { Value: s.FileSizeLimit, }) } - - if s.UserCommiterMatch != nil { - kvs = append(kvs, settings.KeyValue{ - Key: settings.KeyUserCommiterMatch, - Value: s.UserCommiterMatch, - }) - } - return kvs } diff --git a/app/api/usererror/usererror.go b/app/api/usererror/usererror.go index 4b9158ead..8fbdd854e 100644 --- a/app/api/usererror/usererror.go +++ b/app/api/usererror/usererror.go @@ -64,13 +64,6 @@ var ( ErrSpaceWithChildsCantBeDeleted = New(http.StatusBadRequest, "Space can't be deleted as it still contains child resources") - // ErrCommiterUserMismatch is returned if the commiter's email - // is not the same as authenticated user's email. - ErrCommiterUserMismatch = New( - http.StatusForbidden, - "Committer verification failed: authenticated user and committer must match", - ) - // ErrDefaultBranchCantBeDeleted is returned if the user tries to delete the default branch of a repository. ErrDefaultBranchCantBeDeleted = New(http.StatusBadRequest, "The default branch of a repository can't be deleted") diff --git a/app/services/settings/settings.go b/app/services/settings/settings.go index f9631c21b..9f91c0dd3 100644 --- a/app/services/settings/settings.go +++ b/app/services/settings/settings.go @@ -24,6 +24,4 @@ var ( DefaultFileSizeLimit = int64(1e+8) // 100 MB KeyInstallID Key = "install_id" DefaultInstallID = string("") - KeyUserCommiterMatch Key = "user_commiter_match" - DefaultUserCommiterMatch = false ) diff --git a/git/api/commit.go b/git/api/commit.go index 8b616c620..b47dc3d21 100644 --- a/git/api/commit.go +++ b/git/api/commit.go @@ -589,44 +589,6 @@ func (g *Git) GetCommits( return getCommits(ctx, repoPath, refs) } -func (g *Git) GetBranchCommiterEmails( - ctx context.Context, - repoPath string, - baseSHA sha.SHA, - revSHA sha.SHA, - alternateObjDirs []string, -) (map[string]string, error) { - cmd := command.New( - "log", - command.WithFlag("--pretty=format:%H %ce"), - ) - if baseSHA.IsEmpty() { - cmd.Add(command.WithArg(revSHA.String())) - } else { - cmd.Add(command.WithArg(baseSHA.String() + ".." + revSHA.String())) - } - if len(alternateObjDirs) > 0 { - cmd.Add(command.WithAlternateObjectDirs(alternateObjDirs...)) - } - - output := &bytes.Buffer{} - err := cmd.Run(ctx, command.WithDir(repoPath), command.WithStdout(output)) - if err != nil { - return nil, fmt.Errorf("failed to run git to get commit data: %w", err) - } - - scanner := bufio.NewScanner(output) - - shaEmailMap := make(map[string]string) - for scanner.Scan() { - // scanned line follows pattern "[commit sha] [commiter's email]" - shaCommiterSlice := strings.Split(scanner.Text(), " ") - shaEmailMap[shaCommiterSlice[0]] = shaCommiterSlice[1] - } - - return shaEmailMap, nil -} - // GetCommitDivergences returns the count of the diverging commits for all branch pairs. // IMPORTANT: If a maxCount is provided it limits the overal count of diverging commits // (maxCount 10 could lead to (0, 10) while it's actually (2, 12)). diff --git a/git/commit.go b/git/commit.go index f1f37a699..f62aec218 100644 --- a/git/commit.go +++ b/git/commit.go @@ -43,12 +43,6 @@ type GetCommitParams struct { IgnoreWhitespace bool } -type GetBranchCommitterEmailsParams struct { - ReadParams - BaseSHA sha.SHA - RevSHA sha.SHA -} - type Commit struct { SHA sha.SHA `json:"sha"` ParentSHAs []sha.SHA `json:"parent_shas,omitempty"` @@ -105,17 +99,6 @@ func (s *Service) GetCommit(ctx context.Context, params *GetCommitParams) (*GetC }, nil } -func (s *Service) GetBranchCommiterEmails( - ctx context.Context, - params *GetBranchCommitterEmailsParams, -) (map[string]string, error) { - repoPath := getFullPathForRepo(s.reposRoot, params.RepoUID) - - return s.git.GetBranchCommiterEmails( - ctx, repoPath, params.BaseSHA, params.RevSHA, params.AlternateObjectDirs, - ) -} - type ListCommitsParams struct { ReadParams // GitREF is a git reference (branch / tag / commit SHA) diff --git a/git/interface.go b/git/interface.go index 958c6f7d1..7626507ee 100644 --- a/git/interface.go +++ b/git/interface.go @@ -65,10 +65,6 @@ type Interface interface { ctx context.Context, params *FindOversizeFilesParams, ) (*FindOversizeFilesOutput, error) - GetBranchCommiterEmails( - ctx context.Context, - params *GetBranchCommitterEmailsParams, - ) (map[string]string, error) /* * Git Cli Service