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
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index cc8b14cc5babe736edddf273c720abc85e257d45..740f924f8d081279724606f1ac5ab689dac4e3e1 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -83,24 +83,39 @@ def update_permissions
# Update user ssh keys if they changed in LDAP
def update_ssh_keys
- user.keys.ldap.where.not(key: ldap_user.ssh_keys).each do |deleted_key|
- Rails.logger.info "#{self.class.name}: removing LDAP SSH key #{deleted_key.key} from #{user.name} (#{user.id})"
- unless deleted_key.destroy
- Rails.logger.error "#{self.class.name}: failed to remove LDAP SSH key #{key.inspect} from #{user.name} (#{user.id})"
- end
- end
+ remove_old_ssh_keys
+ add_new_ssh_keys
+ end
- (ldap_user.ssh_keys - user.keys.ldap.pluck(:key)).each do |key|
- Rails.logger.info "#{self.class.name}: adding LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})"
+ # Add ssh keys that are in LDAP but not in GitLab
+ def add_new_ssh_keys
+ keys = ldap_user.ssh_keys - user.keys.ldap.pluck(:key)
+
+ keys.each do |key|
+ logger.info "#{self.class.name}: adding LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})"
new_key = LDAPKey.new(title: "LDAP - #{ldap_config.sync_ssh_keys}", key: key)
new_key.user = user
+
unless new_key.save
- Rails.logger.error "#{self.class.name}: failed to add LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})\n"\
+ logger.error "#{self.class.name}: failed to add LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})\n"\
"error messages: #{new_key.errors.messages}"
end
end
end
+ # Remove ssh keys that do not exist in LDAP any more
+ def remove_old_ssh_keys
+ keys = user.keys.ldap.where.not(key: ldap_user.ssh_keys)
+
+ keys.each do |deleted_key|
+ logger.info "#{self.class.name}: removing LDAP SSH key #{deleted_key.key} from #{user.name} (#{user.id})"
+
+ unless deleted_key.destroy
+ logger.error "#{self.class.name}: failed to remove LDAP SSH key #{key.inspect} from #{user.name} (#{user.id})"
+ end
+ end
+ end
+
# Update user email if it changed in LDAP
def update_email
return false unless ldap_user.try(:email)
@@ -142,7 +157,7 @@ def update_ldap_group_links
if active_group_links.any?
group.add_users([user.id], fetch_group_access(group, user, active_group_links), skip_notification: true)
elsif group.last_owner?(user)
- Rails.logger.warn "#{self.class.name}: LDAP group sync cannot remove #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner"
+ logger.warn "#{self.class.name}: LDAP group sync cannot remove #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner"
else
group.users.delete(user)
end
@@ -175,6 +190,7 @@ def admin_group
end
private
+
def gitlab_groups_with_ldap_link
::Group.includes(:ldap_group_links).references(:ldap_group_links).
where.not(ldap_group_links: { id: nil }).
@@ -190,6 +206,10 @@ def fetch_group_access(group, user, active_group_links)
# TODO: Test if nil value of current_access_level in handled properly
[current_access_level, max_group_access_level].compact.max
end
+
+ def logger
+ Rails.logger
+ end
end
end
end