diff options
author | Toon Claes <toon@gitlab.com> | 2022-08-10 16:23:30 +0300 |
---|---|---|
committer | Toon Claes <toon@gitlab.com> | 2022-08-10 16:23:30 +0300 |
commit | 434f167e9c0c9390b0c6a7e158872e3529f27bf8 (patch) | |
tree | e27b0d94e54cced140b56fbc0aaa6a04100f94cc | |
parent | 141ae2fdc49a6fc0cee87a31fd4241d7a6e5a915 (diff) | |
parent | f7f7c1e657db76905b154ae55ecfeeab3faac407 (diff) |
Merge branch '4257-usermergebranch-does-not-verify-it-got-a-user-name-and-email' into 'master'
usermergebranch: add validation for user.{email, name}
Closes #4257
See merge request gitlab-org/gitaly!4800
-rw-r--r-- | internal/gitaly/service/operations/merge.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/operations/merge_test.go | 102 |
2 files changed, 110 insertions, 0 deletions
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 8237ec034..d54b7b8d4 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -21,6 +21,14 @@ func validateMergeBranchRequest(request *gitalypb.UserMergeBranchRequest) error return fmt.Errorf("empty user") } + if len(request.User.Email) == 0 { + return fmt.Errorf("empty user email") + } + + if len(request.User.Name) == 0 { + return fmt.Errorf("empty user name") + } + if len(request.Branch) == 0 { return fmt.Errorf("empty branch name") } diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index 41c5038c8..22dbc32f8 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -120,6 +120,108 @@ func TestUserMergeBranch_successful(t *testing.T) { } } +func TestUserMergeBranch_failure(t *testing.T) { + t.Parallel() + + ctx := testhelper.Context(t) + + ctx, cfg, client := setupOperationsServiceWithoutRepo(t, ctx) + repoProto, _ := gittest.CreateRepository(ctx, t, cfg) + + _ = localrepo.NewTestRepo(t, cfg, repoProto) + + testCases := []struct { + user *gitalypb.User + repo *gitalypb.Repository + desc string + commitID string + branch []byte + message []byte + expectedErr error + }{ + { + desc: "empty user", + repo: repoProto, + branch: []byte(mergeBranchName), + commitID: commitToMerge, + message: []byte("sample-message"), + expectedErr: helper.ErrInvalidArgumentf("empty user"), + }, + { + desc: "empty user name", + user: &gitalypb.User{ + GlId: gittest.TestUser.GlId, + GlUsername: gittest.TestUser.GlUsername, + Email: gittest.TestUser.Email, + Timezone: gittest.TestUser.Timezone, + }, + repo: repoProto, + branch: []byte(mergeBranchName), + commitID: commitToMerge, + message: []byte("sample-message"), + expectedErr: helper.ErrInvalidArgumentf("empty user name"), + }, + { + desc: "empty user email", + user: &gitalypb.User{ + GlId: gittest.TestUser.GlId, + Name: gittest.TestUser.Name, + GlUsername: gittest.TestUser.GlUsername, + Timezone: gittest.TestUser.Timezone, + }, + repo: repoProto, + branch: []byte(mergeBranchName), + commitID: commitToMerge, + message: []byte("sample-message"), + expectedErr: helper.ErrInvalidArgumentf("empty user email"), + }, + { + desc: "empty commit", + repo: repoProto, + user: gittest.TestUser, + branch: []byte(mergeBranchName), + message: []byte("sample-message"), + expectedErr: helper.ErrInvalidArgumentf("empty commit ID"), + }, + { + desc: "empty branch", + repo: repoProto, + user: gittest.TestUser, + commitID: commitToMerge, + message: []byte("sample-message"), + expectedErr: helper.ErrInvalidArgumentf("empty branch name"), + }, + { + desc: "empty message", + repo: repoProto, + user: gittest.TestUser, + branch: []byte(mergeBranchName), + commitID: commitToMerge, + expectedErr: helper.ErrInvalidArgumentf("empty message"), + }, + } + + for _, testCase := range testCases { + t.Run(testCase.desc, func(t *testing.T) { + request := &gitalypb.UserMergeBranchRequest{ + Repository: testCase.repo, + User: testCase.user, + Branch: testCase.branch, + CommitId: testCase.commitID, + Message: testCase.message, + } + + mergeBidi, err := client.UserMergeBranch(ctx) + require.NoError(t, err) + + require.NoError(t, mergeBidi.Send(request), "apply merge") + _, err = mergeBidi.Recv() + + testhelper.RequireGrpcError(t, testCase.expectedErr, err) + }) + } +} + func TestUserMergeBranch_quarantine(t *testing.T) { t.Parallel() |