Merge branch 'hooks_checkbox' into 'master'

Git hooks for email checking

https://dev.gitlab.org/gitlab/gitlab-ee/issues/165

https://dev.gitlab.org/gitlab/gitlab-ee/issues/167

See merge request !228

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

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

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

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

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"

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

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

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|

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

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

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

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

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