diff --git a/CHANGELOG-EE b/CHANGELOG-EE index 06e03abc4b..6b322d3677 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -1,3 +1,7 @@ +v 7.5.0 + - Added an ability to check each author commit's email by regex + - Added an abulity to restrict commit authors to existing Gitlab users + v 7.4.0 - Support for multiple LDAP servers - Skip AD specific LDAP checks diff --git a/app/controllers/projects/git_hooks_controller.rb b/app/controllers/projects/git_hooks_controller.rb index c774b4233a..71c8faf0d0 100644 --- a/app/controllers/projects/git_hooks_controller.rb +++ b/app/controllers/projects/git_hooks_controller.rb @@ -27,6 +27,7 @@ class Projects::GitHooksController < Projects::ApplicationController # Only allow a trusted parameter "white list" through. def git_hook_params - params.require(:git_hook).permit(:deny_delete_tag, :delete_branch_regex, :commit_message_regex, :force_push_regex) + params.require(:git_hook).permit(:deny_delete_tag, :delete_branch_regex, + :commit_message_regex, :force_push_regex, :author_email_regex, :member_check) end end diff --git a/app/models/git_hook.rb b/app/models/git_hook.rb index f45f57aa99..0be8337856 100644 --- a/app/models/git_hook.rb +++ b/app/models/git_hook.rb @@ -13,4 +13,8 @@ class GitHook < ActiveRecord::Base true end end + + def commit_validation? + commit_message_regex.present? || author_email_regex.present? || member_check + end end diff --git a/app/models/user.rb b/app/models/user.rb index f4215ceef8..d9b0033fc4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -213,6 +213,10 @@ class User < ActiveRecord::Base User.where(name: name).first end + def existing_member?(email) + User.where(email: email).any? || Email.where(email: email).any? + end + def filter(filter_name) case filter_name when "admins"; self.admins diff --git a/app/views/projects/git_hooks/index.html.haml b/app/views/projects/git_hooks/index.html.haml index dedb6b6c2f..45a66a98cd 100644 --- a/app/views/projects/git_hooks/index.html.haml +++ b/app/views/projects/git_hooks/index.html.haml @@ -13,13 +13,23 @@ .form-group = f.label :deny_delete_tag, "Prevent tag removal", class: 'control-label' .col-sm-10 - .checkbox - = f.check_box :deny_delete_tag - %span.descr - Do not allow users to remove git tags with - = succeed '.' do - %code git push - Tags can still be deleted through the web UI. + %label + .checkbox + = f.check_box :deny_delete_tag + %span.descr + Do not allow users to remove git tags with + = succeed '.' do + %code git push + Tags can still be deleted through the web UI. + + .form-group + = f.label :member_check, "Restrict commit authors to existing Gitlab users", class: 'control-label' + .col-sm-10 + %label + .checkbox + = f.check_box :member_check + %span.descr + Check whether author is a GitLab member -#.form-group = f.label :force_push_regex, "Force push", class: 'control-label' @@ -44,5 +54,15 @@ If this field is empty it allows any commit message. For example you can require that an issue number is always mentioned in the commit message. + .form-group + = f.label :author_email_regex, "Commit author's email", class: 'control-label' + .col-sm-10 + = f.text_field :author_email_regex, class: "form-control", placeholder: 'Example: Fixes @my-company.com$' + %p.hint + All commit author's email must match this + = link_to 'Ruby regular expression', 'http://www.ruby-doc.org/core-2.1.1/Regexp.html' + to be pushed. + If this field is empty it allows any email. + .form-actions = f.submit "Save Git hooks", class: "btn btn-create" diff --git a/db/migrate/20141027173526_add_author_email_regex_to_git_hook.rb b/db/migrate/20141027173526_add_author_email_regex_to_git_hook.rb new file mode 100644 index 0000000000..bf58a13e81 --- /dev/null +++ b/db/migrate/20141027173526_add_author_email_regex_to_git_hook.rb @@ -0,0 +1,5 @@ +class AddAuthorEmailRegexToGitHook < ActiveRecord::Migration + def change + add_column :git_hooks, :author_email_regex, :string + end +end diff --git a/db/migrate/20141030133853_add_member_check_to_git_hooks.rb b/db/migrate/20141030133853_add_member_check_to_git_hooks.rb new file mode 100644 index 0000000000..15ce6eec89 --- /dev/null +++ b/db/migrate/20141030133853_add_member_check_to_git_hooks.rb @@ -0,0 +1,5 @@ +class AddMemberCheckToGitHooks < ActiveRecord::Migration + def change + add_column :git_hooks, :member_check, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 1d728f5c5b..8ae23b7d69 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20141010132608) do +ActiveRecord::Schema.define(version: 20141030133853) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -91,6 +91,8 @@ ActiveRecord::Schema.define(version: 20141010132608) do t.integer "project_id" t.datetime "created_at" t.datetime "updated_at" + t.string "author_email_regex" + t.boolean "member_check", default: false, null: false end create_table "issues", force: true do |t| diff --git a/features/project/git_hooks.feature b/features/project/git_hooks.feature new file mode 100644 index 0000000000..b340b4a6c3 --- /dev/null +++ b/features/project/git_hooks.feature @@ -0,0 +1,8 @@ +Feature: Git Hooks + Background: + Given I sign in as a user + And I own project "Shop" + + Scenario: I should see git hook form + When I visit project git hooks page + Then I should see git hook form \ No newline at end of file diff --git a/features/steps/project/git_hooks.rb b/features/steps/project/git_hooks.rb new file mode 100644 index 0000000000..025bbb3e41 --- /dev/null +++ b/features/steps/project/git_hooks.rb @@ -0,0 +1,19 @@ +require 'webmock' + +class Spinach::Features::GitHooks < Spinach::FeatureSteps + include SharedAuthentication + include SharedProject + include SharedPaths + include RSpec::Matchers + include RSpec::Mocks::ExampleMethods + include WebMock::API + + + step 'I should see git hook form' do + page.should have_selector('input#git_hook_commit_message_regex') + page.should have_content "Commit message" + page.should have_content "Commit author's email" + end + + +end diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index cf51a9d369..de356d9577 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -228,6 +228,10 @@ module SharedPaths visit project_hooks_path(@project) end + step 'I visit project git hooks page' do + visit project_git_hooks_path(@project) + end + step 'I visit project deploy keys page' do visit project_deploy_keys_path(@project) end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 893f413dc5..88e5f3642b 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -109,11 +109,24 @@ module Gitlab end # Check commit messages unless its branch removal - if git_hook.commit_message_regex.present? && newrev !~ /00000000/ + if git_hook.commit_validation? && newrev !~ /00000000/ commits = project.repository.commits_between(oldrev, newrev) commits.each do |commit| - unless commit.safe_message =~ Regexp.new(git_hook.commit_message_regex) - return false + if git_hook.commit_message_regex.present? + return false unless commit.safe_message =~ Regexp.new(git_hook.commit_message_regex) + end + + if git_hook.author_email_regex.present? + return false unless commit.committer_email =~ Regexp.new(git_hook.author_email_regex) + return false unless commit.author_email =~ Regexp.new(git_hook.author_email_regex) + end + + # Check whether author is a GitLab member + if git_hook.member_check + return false unless User.existing_member?(commit.author_email) + if commit.author_email != commit.committer_email + return false unless User.existing_member?(commit.committer_email) + end end end end @@ -144,5 +157,6 @@ module Gitlab nil end end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6ad57b06e0..0ff84b8375 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -449,6 +449,27 @@ describe User do end end + describe "#existing_member?" do + it "returns true for exisitng user" do + create :user, email: "bruno@example.com" + + expect(User.existing_member?("bruno@example.com")).to be_true + end + + it "returns false for unknown exisitng user" do + create :user, email: "bruno@example.com" + + expect(User.existing_member?("rendom@example.com")).to be_false + end + + it "returns true if additional email exists" do + user = create :user + user.emails.create(email: "bruno@example.com") + + expect(User.existing_member?("bruno@example.com")).to be_true + end + end + describe "#sort" do before do User.delete_all