diff --git a/app/models/git_hook.rb b/app/models/git_hook.rb
index e48a399d6009658fa6bfeab49fe18e6d0c9d2373..1de3116b62124a1b3c60a5296a44ecd1b302be95 100644
--- a/app/models/git_hook.rb
+++ b/app/models/git_hook.rb
@@ -2,24 +2,30 @@ class GitHook < ActiveRecord::Base
belongs_to :project
validates :project, presence: true, unless: "is_sample?"
-
+
+ def commit_validation?
+ commit_message_regex.present? ||
+ author_email_regex.present? ||
+ member_check ||
+ file_name_regex.present? ||
+ max_file_size > 0
+ end
+
def commit_message_allowed?(message)
- if commit_message_regex.present?
- if message =~ Regexp.new(commit_message_regex)
- true
- else
- false
- end
+ data_valid?(message, commit_message_regex)
+ end
+
+ def author_email_allowed?(email)
+ data_valid?(email, author_email_regex)
+ end
+
+ private
+
+ def data_valid?(data, regex)
+ if regex.present?
+ !!(data =~ Regexp.new(regex))
else
true
end
end
-
- def commit_validation?
- commit_message_regex.present? ||
- author_email_regex.present? ||
- member_check ||
- file_name_regex.present? ||
- max_file_size > 0
- end
end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index ee89c5052c0ff6b2683e4986070d29a0830896ef..b344ff84503b33eb7c8b7f0d09bc80e85a960f50 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -170,9 +170,9 @@ def forced_push?(oldrev, newrev)
end
def git_hook_check(user, project, ref, oldrev, newrev)
- return build_status_object(true) unless project.git_hook
-
- return build_status_object(true) unless newrev && oldrev
+ unless project.git_hook && newrev && oldrev
+ return build_status_object(true)
+ end
git_hook = project.git_hook
@@ -182,8 +182,9 @@ def git_hook_check(user, project, ref, oldrev, newrev)
return build_status_object(false, "You can not delete tag")
end
else
- return build_status_object(true) unless git_hook.commit_validation?
- return build_status_object(true) if Gitlab::Git.blank_ref?(newrev)
+ if Gitlab::Git.blank_ref?(newrev) || !git_hook.commit_validation?
+ return build_status_object(true)
+ end
oldrev = project.default_branch if Gitlab::Git.blank_ref?(oldrev)
@@ -195,61 +196,76 @@ def git_hook_check(user, project, ref, oldrev, newrev)
end
commits.each do |commit|
- if git_hook.commit_message_regex.present?
- unless commit.safe_message =~ Regexp.new(git_hook.commit_message_regex)
- return build_status_object(false, "Commit message does not follow the pattern '#{git_hook.commit_message_regex}'")
- end
+ if status_object = check_commit(commit, git_hook)
+ return status_object
end
+ end
+ end
- if git_hook.author_email_regex.present?
- unless commit.committer_email =~ Regexp.new(git_hook.author_email_regex)
- return build_status_object(false, "Committer's email '#{commit.committer_email}' does not follow the pattern '#{git_hook.author_email_regex}'")
- end
+ build_status_object(true)
+ end
- unless commit.author_email =~ Regexp.new(git_hook.author_email_regex)
- return build_status_object(false, "Author's email '#{commit.author_email}' does not follow the pattern '#{git_hook.author_email_regex}'")
- end
- end
+ private
+
+ # If commit does not pass git hook validation the whole push should be rejected.
+ # This method should return nil if no error found or status object if there are some errors.
+ # In case of errors - all other checks will be canceled and push will be rejected.
+ def check_commit(commit, git_hook)
+ unless git_hook.commit_message_allowed?(commit.safe_message)
+ return build_status_object(false, "Commit message does not follow the pattern '#{git_hook.commit_message_regex}'")
+ end
+
+ unless git_hook.author_email_allowed?(commit.committer_email)
+ return build_status_object(false, "Committer's email '#{commit.committer_email}' does not follow the pattern '#{git_hook.author_email_regex}'")
+ end
+
+ unless git_hook.author_email_allowed?(commit.author_email)
+ return build_status_object(false, "Author's email '#{commit.author_email}' does not follow the pattern '#{git_hook.author_email_regex}'")
+ end
+
+ # Check whether author is a GitLab member
+ if git_hook.member_check
+ unless User.existing_member?(commit.author_email.downcase)
+ return build_status_object(false, "Author '#{commit.author_email}' is not a member of team")
+ end
- # Check whether author is a GitLab member
- if git_hook.member_check
- unless User.existing_member?(commit.author_email.downcase)
- return build_status_object(false, "Author '#{commit.author_email}' is not a member of team")
- end
-
- if commit.author_email.downcase != commit.committer_email.downcase
- unless User.existing_member?(commit.committer_email.downcase)
- return build_status_object(false, "Committer '#{commit.committer_email}' is not a member of team")
- end
- end
+ if commit.author_email.downcase != commit.committer_email.downcase
+ unless User.existing_member?(commit.committer_email.downcase)
+ return build_status_object(false, "Committer '#{commit.committer_email}' is not a member of team")
end
+ end
+ end
+
+ if status_object = check_commit_diff(commit, git_hook)
+ return status_object
+ end
- if git_hook.file_name_regex.present?
- commit.diffs.each do |diff|
- if (diff.renamed_file || diff.new_file) && diff.new_path =~ Regexp.new(git_hook.file_name_regex)
- return build_status_object(false, "File name #{diff.new_path.inspect} does not follow the pattern '#{git_hook.file_name_regex}'")
- end
- end
+ nil
+ end
+
+ def check_commit_diff(commit, git_hook)
+ if git_hook.file_name_regex.present?
+ commit.diffs.each do |diff|
+ if (diff.renamed_file || diff.new_file) && diff.new_path =~ Regexp.new(git_hook.file_name_regex)
+ return build_status_object(false, "File name #{diff.new_path.inspect} does not follow the pattern '#{git_hook.file_name_regex}'")
end
+ end
+ end
- if git_hook.max_file_size > 0
- commit.diffs.each do |diff|
- next if diff.deleted_file
+ if git_hook.max_file_size > 0
+ commit.diffs.each do |diff|
+ next if diff.deleted_file
- blob = project.repository.blob_at(commit.id, diff.new_path)
- if blob.size > git_hook.max_file_size.megabytes
- return build_status_object(false, "File #{diff.new_path.inspect} is larger than the allowed size of #{git_hook.max_file_size} MB")
- end
- end
+ blob = project.repository.blob_at(commit.id, diff.new_path)
+ if blob.size > git_hook.max_file_size.megabytes
+ return build_status_object(false, "File #{diff.new_path.inspect} is larger than the allowed size of #{git_hook.max_file_size} MB")
end
end
end
- build_status_object(true)
+ nil
end
- private
-
def protected_branch_action(oldrev, newrev, branch_name)
# we dont allow force push to protected branch
if forced_push?(oldrev, newrev)
@@ -290,8 +306,6 @@ def tag_name(ref)
end
end
- protected
-
def build_status_object(status, message = '')
GitAccessStatus.new(status, message)
end