Merge branch 'feature/1376-allow-write-access-deploy-keys' into 'master'

Allow to add deploy keys with write-access

Closes #1376

See merge request !7383

app/assets/stylesheets/pages/deploy_keys.scss

+.deploy-keys-list {
+  width: 100%;
+  overflow: auto;
+
+  table {
+    border: 1px solid $table-border-color;
+  }
+}
+
+.deploy-keys-title {
+  padding-bottom: 2px;
+  line-height: 2;
+}

app/controllers/admin/deploy_keys_controller.rb

@@ -10,7 +10,7 @@ class Admin::DeployKeysController < Admin::ApplicationController
   end
 
   def create
-    @deploy_key = deploy_keys.new(deploy_key_params)
+    @deploy_key = deploy_keys.new(deploy_key_params.merge(user: current_user))
 
     if @deploy_key.save
       redirect_to admin_deploy_keys_path
@@ -39,6 +39,6 @@ class Admin::DeployKeysController < Admin::ApplicationController
   end
 
   def deploy_key_params
-    params.require(:deploy_key).permit(:key, :title)
+    params.require(:deploy_key).permit(:key, :title, :can_push)
   end
 end

app/controllers/projects/deploy_keys_controller.rb

@@ -16,7 +16,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
   end
 
   def create
-    @key = DeployKey.new(deploy_key_params)
+    @key = DeployKey.new(deploy_key_params.merge(user: current_user))
     set_index_vars
 
     if @key.valid? && @project.deploy_keys << @key
@@ -53,6 +53,6 @@ class Projects::DeployKeysController < Projects::ApplicationController
   end
 
   def deploy_key_params
-    params.require(:deploy_key).permit(:key, :title)
+    params.require(:deploy_key).permit(:key, :title, :can_push)
   end
 end

app/helpers/projects_helper.rb

@@ -90,10 +90,12 @@ module ProjectsHelper
   end
 
   def project_for_deploy_key(deploy_key)
-    if deploy_key.projects.include?(@project)
+    if deploy_key.has_access_to?(@project)
       @project
     else
-      deploy_key.projects.find { |project| can?(current_user, :read_project, project) }
+      deploy_key.projects.find do |project|
+        can?(current_user, :read_project, project)
+      end
     end
   end
 

app/models/deploy_key.rb

@@ -20,4 +20,18 @@ class DeployKey < Key
   def destroyed_when_orphaned?
     self.private?
   end
+
+  def has_access_to?(project)
+    projects.include?(project)
+  end
+
+  def can_push_to?(project)
+    can_push? && has_access_to?(project)
+  end
+
+  private
+
+  # we don't want to notify the user for deploy keys
+  def notify_user
+  end
 end

app/models/key.rb

@@ -57,10 +57,6 @@ class Key < ActiveRecord::Base
     )
   end
 
-  def notify_user
-    run_after_commit { NotificationService.new.new_key(self) }
-  end
-
   def post_create_hook
     SystemHooksService.new.execute_hooks_for(self, :create)
   end
@@ -86,4 +82,8 @@ class Key < ActiveRecord::Base
 
     self.fingerprint = Gitlab::KeyFingerprint.new(self.key).fingerprint
   end
+
+  def notify_user
+    run_after_commit { NotificationService.new.new_key(self) }
+  end
 end

app/views/admin/deploy_keys/index.html.haml

 - page_title "Deploy Keys"
-.panel.panel-default.prepend-top-default
-  .panel-heading
+
+%h3.page-title.deploy-keys-title
   Public deploy keys (#{@deploy_keys.count})
-    .controls
-      = link_to 'New Deploy Key', new_admin_deploy_key_path, class: "btn btn-new btn-sm"
-  - if @deploy_keys.any?
-    .table-holder
+  .pull-right
+    = link_to 'New Deploy Key', new_admin_deploy_key_path, class: 'btn btn-new btn-sm btn-inverted'
+
+- if @deploy_keys.any?
+  .table-holder.deploy-keys-list
     %table.table
-        %thead.panel-heading
+      %thead
         %tr
-            %th Title
-            %th Fingerprint
-            %th Added at
-            %th
+          %th.col-sm-2 Title
+          %th.col-sm-4 Fingerprint
+          %th.col-sm-2 Write access allowed
+          %th.col-sm-2 Added at
+          %th.col-sm-2
       %tbody
         - @deploy_keys.each do |deploy_key|
           %tr
@@ -20,8 +22,13 @@
               %strong= deploy_key.title
             %td
               %code.key-fingerprint= deploy_key.fingerprint
+            %td
+              - if deploy_key.can_push?
+                Yes
+              - else
+                No
             %td
               %span.cgray
                 added #{time_ago_with_tooltip(deploy_key.created_at)}
             %td
-                = link_to 'Remove', admin_deploy_key_path(deploy_key), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-sm btn-remove delete-key pull-right"
+              = link_to 'Remove', admin_deploy_key_path(deploy_key), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-remove delete-key pull-right'

app/views/admin/deploy_keys/new.html.haml

@@ -16,6 +16,14 @@
           Paste a machine public key here. Read more about how to generate it
           = link_to "here", help_page_path("ssh/README")
         = f.text_area :key, class: "form-control thin_area", rows: 5
+    .form-group
+      .control-label
+      .col-sm-10
+        = f.label :can_push do
+          = f.check_box :can_push
+          %strong Write access allowed
+        %p.light.append-bottom-0
+          Allow this key to push to repository as well? (Default only allows pull access.)
 
     .form-actions
       = f.submit 'Create', class: "btn-create btn"

app/views/projects/deploy_keys/_deploy_key.html.haml

@@ -6,6 +6,9 @@
       = deploy_key.title
     .description
       = deploy_key.fingerprint
+    - if deploy_key.can_push?
+      .write-access-allowed
+        Write access allowed
   .deploy-key-content.prepend-left-default.deploy-key-projects
     - deploy_key.projects.each do |project|
       - if can?(current_user, :read_project, project)

app/views/projects/deploy_keys/_form.html.haml

@@ -10,4 +10,13 @@
     %p.light.append-bottom-0
       Paste a machine public key here. Read more about how to generate it
       = link_to "here", help_page_path("ssh/README")
+  .form-group
+    .checkbox
+      = f.label :can_push do
+        = f.check_box :can_push
+        %strong Write access allowed
+  .form-group
+    %p.light.append-bottom-0
+      Allow this key to push to repository as well? (Default only allows pull access.)
+
   = f.submit "Add key", class: "btn-create btn"

changelogs/unreleased/feature-1376-allow-write-access-deploy-keys.yml

+---
+title: Allow to add deploy keys with write-access
+merge_request: 5807
+author: Ali Ibrahim

db/migrate/20160811172945_add_can_push_to_keys.rb

+class AddCanPushToKeys < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    add_column_with_default(:keys, :can_push, :boolean, default: false, allow_null: false)
+  end
+
+  def down
+    remove_column(:keys, :can_push)
+  end
+end

db/schema.rb

@@ -527,6 +527,7 @@ ActiveRecord::Schema.define(version: 20161226122833) do
     t.string "type"
     t.string "fingerprint"
     t.boolean "public", default: false, null: false
+    t.boolean "can_push", default: false, null: false
   end
 
   add_index "keys", ["fingerprint"], name: "index_keys_on_fingerprint", unique: true, using: :btree

doc/api/deploy_keys.md

@@ -20,12 +20,14 @@ Example response:
     "id": 1,
     "title": "Public key",
     "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
+    "can_push": false,
     "created_at": "2013-10-02T10:12:29Z"
   },
   {
     "id": 3,
     "title": "Another Public key",
     "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
+    "can_push": true,
     "created_at": "2013-10-02T11:12:29Z"
   }
 ]
@@ -55,12 +57,14 @@ Example response:
     "id": 1,
     "title": "Public key",
     "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
+    "can_push": false,
     "created_at": "2013-10-02T10:12:29Z"
   },
   {
     "id": 3,
     "title": "Another Public key",
     "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
+    "can_push": false,
     "created_at": "2013-10-02T11:12:29Z"
   }
 ]
@@ -92,6 +96,7 @@ Example response:
   "id": 1,
   "title": "Public key",
   "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
+  "can_push": false,
   "created_at": "2013-10-02T10:12:29Z"
 }
 ```
@@ -112,9 +117,10 @@ POST /projects/:id/deploy_keys
 | `id`       | integer | yes | The ID of the project |
 | `title`    | string  | yes | New deploy key's title |
 | `key`      | string  | yes | New deploy key |
+| `can_push` | boolean | no  | Can deploy key push to the project's repository |
 
 ```bash
-curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --header "Content-Type: application/json" --data '{"title": "My deploy key", "key": "ssh-rsa AAAA..."}' "https://gitlab.example.com/api/v3/projects/5/deploy_keys/"
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --header "Content-Type: application/json" --data '{"title": "My deploy key", "key": "ssh-rsa AAAA...", "can_push": "true"}' "https://gitlab.example.com/api/v3/projects/5/deploy_keys/"
 ```
 
 Example response:
@@ -124,6 +130,7 @@ Example response:
    "key" : "ssh-rsa AAAA...",
    "id" : 12,
    "title" : "My deploy key",
+   "can_push": true,
    "created_at" : "2015-08-29T12:44:31.550Z"
 }
 ```

features/admin/deploy_keys.feature

+@admin
+Feature: Admin Deploy Keys
+  Background:
+    Given I sign in as an admin
+    And there are public deploy keys in system
+
+  Scenario: Deploy Keys list
+    When I visit admin deploy keys page
+    Then I should see all public deploy keys
+
+  Scenario: Deploy Keys new
+    When I visit admin deploy keys page
+    And I click 'New Deploy Key'
+    And I submit new deploy key
+    Then I should be on admin deploy keys page
+    And I should see newly created deploy key without write access
+
+  Scenario: Deploy Keys new with write access
+    When I visit admin deploy keys page
+    And I click 'New Deploy Key'
+    And I submit new deploy key with write access
+    Then I should be on admin deploy keys page
+    And I should see newly created deploy key with write access

features/steps/admin/deploy_keys.rb

+class Spinach::Features::AdminDeployKeys < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedPaths
+  include SharedAdmin
+
+  step 'there are public deploy keys in system' do
+    create(:deploy_key, public: true)
+    create(:another_deploy_key, public: true)
+  end
+
+  step 'I should see all public deploy keys' do
+    DeployKey.are_public.each do |p|
+      expect(page).to have_content p.title
+    end
+  end
+
+  step 'I visit admin deploy key page' do
+    visit admin_deploy_key_path(deploy_key)
+  end
+
+  step 'I visit admin deploy keys page' do
+    visit admin_deploy_keys_path
+  end
+
+  step 'I click \'New Deploy Key\'' do
+    click_link 'New Deploy Key'
+  end
+
+  step 'I submit new deploy key' do
+    fill_in "deploy_key_title", with: "laptop"
+    fill_in "deploy_key_key", with: "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAzrEJUIR6Y03TCE9rIJ+GqTBvgb8t1jI9h5UBzCLuK4VawOmkLornPqLDrGbm6tcwM/wBrrLvVOqi2HwmkKEIecVO0a64A4rIYScVsXIniHRS6w5twyn1MD3sIbN+socBDcaldECQa2u1dI3tnNVcs8wi77fiRe7RSxePsJceGoheRQgC8AZ510UdIlO+9rjIHUdVN7LLyz512auAfYsgx1OfablkQ/XJcdEwDNgi9imI6nAXhmoKUm1IPLT2yKajTIC64AjLOnE0YyCh6+7RFMpiMyu1qiOCpdjYwTgBRiciNRZCH8xIedyCoAmiUgkUT40XYHwLuwiPJICpkAzp7Q== user@laptop"
+    click_button "Create"
+  end
+
+  step 'I submit new deploy key with write access' do
+    fill_in "deploy_key_title", with: "server"
+    fill_in "deploy_key_key", with: "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAzrEJUIR6Y03TCE9rIJ+GqTBvgb8t1jI9h5UBzCLuK4VawOmkLornPqLDrGbm6tcwM/wBrrLvVOqi2HwmkKEIecVO0a64A4rIYScVsXIniHRS6w5twyn1MD3sIbN+socBDcaldECQa2u1dI3tnNVcs8wi77fiRe7RSxePsJceGoheRQgC8AZ510UdIlO+9rjIHUdVN7LLyz512auAfYsgx1OfablkQ/XJcdEwDNgi9imI6nAXhmoKUm1IPLT2yKajTIC64AjLOnE0YyCh6+7RFMpiMyu1qiOCpdjYwTgBRiciNRZCH8xIedyCoAmiUgkUT40XYHwLuwiPJICpkAzp7Q== user@laptop"
+    check "deploy_key_can_push"
+    click_button "Create"
+  end
+
+  step 'I should be on admin deploy keys page' do
+    expect(current_path).to eq admin_deploy_keys_path
+  end
+
+  step 'I should see newly created deploy key without write access' do
+    expect(page).to have_content(deploy_key.title)
+    expect(page).to have_content('No')
+  end
+
+  step 'I should see newly created deploy key with write access' do
+    expect(page).to have_content(deploy_key.title)
+    expect(page).to have_content('Yes')
+  end
+
+  def deploy_key
+    @deploy_key ||= DeployKey.are_public.first
+  end
+end

lib/api/entities.rb

@@ -317,7 +317,7 @@ module API
     end
 
     class SSHKey < Grape::Entity
-      expose :id, :title, :key, :created_at
+      expose :id, :title, :key, :created_at, :can_push
     end
 
     class SSHKeyWithUser < SSHKey

lib/gitlab/auth/result.rb

@@ -9,8 +9,7 @@ module Gitlab
 
       def lfs_deploy_token?(for_project)
         type == :lfs_deploy_token &&
-          actor &&
-          actor.projects.include?(for_project)
+          actor.try(:has_access_to?, for_project)
       end
 
       def success?

lib/gitlab/checks/change_access.rb

 module Gitlab
   module Checks
     class ChangeAccess
-      attr_reader :user_access, :project
+      attr_reader :user_access, :project, :skip_authorization
 
-      def initialize(change, user_access:, project:, env: {})
+      def initialize(
+        change, user_access:, project:, env: {}, skip_authorization: false)
         @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
         @branch_name = Gitlab::Git.branch_name(@ref)
         @user_access = user_access
         @project = project
         @env = env
+        @skip_authorization = skip_authorization
       end
 
       def exec
@@ -24,6 +26,7 @@ module Gitlab
       protected
 
       def protected_branch_checks
+        return if skip_authorization
         return unless @branch_name
         return unless project.protected_branch?(@branch_name)
 
@@ -49,6 +52,8 @@ module Gitlab
       end
 
       def tag_checks
+        return if skip_authorization
+
         tag_ref = Gitlab::Git.tag_name(@ref)
 
         if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project)
@@ -57,6 +62,8 @@ module Gitlab
       end
 
       def push_checks
+        return if skip_authorization
+
         if user_access.cannot_do_action?(:push_code)
           "You are not allowed to push code to this project."
         end

lib/gitlab/git_access.rb

@@ -7,7 +7,8 @@ module Gitlab
     ERROR_MESSAGES = {
       upload: 'You are not allowed to upload code for this project.',
       download: 'You are not allowed to download code from this project.',
-      deploy_key: 'Deploy keys are not allowed to push code.',
+      deploy_key_upload:
+        'This deploy key does not have write access to this project.',
       no_repo: 'A repository for this project does not exist yet.'
     }
 
@@ -31,12 +32,13 @@ module Gitlab
       check_active_user!
       check_project_accessibility!
       check_command_existence!(cmd)
+      check_repository_existence!
 
       case cmd
       when *DOWNLOAD_COMMANDS
-        download_access_check
+        check_download_access!
       when *PUSH_COMMANDS
-        push_access_check(changes)
+        check_push_access!(changes)
       end
 
       build_status_object(true)
@@ -44,32 +46,10 @@ module Gitlab
       build_status_object(false, ex.message)
     end
 
-    def download_access_check
-      if user
-        user_download_access_check
-      elsif deploy_key.nil? && !guest_can_downlod_code?
-        raise UnauthorizedError, ERROR_MESSAGES[:download]
-      end
-    end
-
-    def push_access_check(changes)
-      if user
-        user_push_access_check(changes)
-      else
-        raise UnauthorizedError, ERROR_MESSAGES[deploy_key ? :deploy_key : :upload]
-      end
-    end
-
-    def guest_can_downlod_code?
+    def guest_can_download_code?
       Guest.can?(:download_code, project)
     end
 
-    def user_download_access_check
-      unless user_can_download_code? || build_can_download_code?
-        raise UnauthorizedError, ERROR_MESSAGES[:download]
-      end
-    end
-
     def user_can_download_code?
       authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_code)
     end
@@ -78,35 +58,6 @@ module Gitlab
       authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
     end
 
-    def user_push_access_check(changes)
-      unless authentication_abilities.include?(:push_code)
-        raise UnauthorizedError, ERROR_MESSAGES[:upload]
-      end
-
-      if changes.blank?
-        return # Allow access.
-      end
-
-      unless project.repository.exists?
-        raise UnauthorizedError, ERROR_MESSAGES[:no_repo]
-      end
-
-      changes_list = Gitlab::ChangesList.new(changes)
-
-      # Iterate over all changes to find if user allowed all of them to be applied
-      changes_list.each do |change|
-        status = change_access_check(change)
-        unless status.allowed?
-          # If user does not have access to make at least one change - cancel all push
-          raise UnauthorizedError, status.message
-        end
-      end
-    end
-
-    def change_access_check(change)
-      Checks::ChangeAccess.new(change, user_access: user_access, project: project, env: @env).exec
-    end
-
     def protocol_allowed?
       Gitlab::ProtocolAccess.allowed?(protocol)
     end
@@ -120,6 +71,8 @@ module Gitlab
     end
 
     def check_active_user!
+      return if deploy_key?
+
       if user && !user_access.allowed?
         raise UnauthorizedError, "Your account has been blocked."
       end
@@ -137,31 +90,90 @@ module Gitlab
       end
     end
 
-    def matching_merge_request?(newrev, branch_name)
-      Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
+    def check_repository_existence!
+      unless project.repository.exists?
+        raise UnauthorizedError, ERROR_MESSAGES[:no_repo]
+      end
     end
 
-    def deploy_key
-      actor if actor.is_a?(DeployKey)
+    def check_download_access!
+      return if deploy_key?
+
+      passed = user_can_download_code? ||
+        build_can_download_code? ||
+        guest_can_download_code?
+
+      unless passed
+        raise UnauthorizedError, ERROR_MESSAGES[:download]
+      end
     end
 
-    def deploy_key_can_read_project?
+    def check_push_access!(changes)
       if deploy_key
-        return true if project.public?
-        deploy_key.projects.include?(project)
+        check_deploy_key_push_access!
+      elsif user
+        check_user_push_access!
       else
-        false
+        raise UnauthorizedError, ERROR_MESSAGES[:upload]
       end
+
+      return if changes.blank? # Allow access.
+
+      check_change_access!(changes)
     end
 
-    def can_read_project?
-      if user
-        user_access.can_read_project?
-      elsif deploy_key
-        deploy_key_can_read_project?
-      else
-        Guest.can?(:read_project, project)
+    def check_user_push_access!
+      unless authentication_abilities.include?(:push_code)
+        raise UnauthorizedError, ERROR_MESSAGES[:upload]
+      end
+    end
+
+    def check_deploy_key_push_access!
+      unless deploy_key.can_push_to?(project)
+        raise UnauthorizedError, ERROR_MESSAGES[:deploy_key_upload]
+      end
     end
+
+    def check_change_access!(changes)
+      changes_list = Gitlab::ChangesList.new(changes)
+
+      # Iterate over all changes to find if user allowed all of them to be applied
+      changes_list.each do |change|
+        status = check_single_change_access(change)
+        unless status.allowed?
+          # If user does not have access to make at least one change - cancel all push
+          raise UnauthorizedError, status.message
+        end
+      end
+    end
+
+    def check_single_change_access(change)
+      Checks::ChangeAccess.new(
+        change,
+        user_access: user_access,
+        project: project,
+        env: @env,
+        skip_authorization: deploy_key?).exec
+    end
+
+    def matching_merge_request?(newrev, branch_name)
+      Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
+    end
+
+    def deploy_key
+      actor if deploy_key?
+    end
+
+    def deploy_key?
+      actor.is_a?(DeployKey)
+    end
+
+    def can_read_project?
+      if deploy_key
+        deploy_key.has_access_to?(project)
+      elsif user
+        user.can?(:read_project, project)
+      end || Guest.can?(:read_project, project)
     end
 
     protected

lib/gitlab/git_access_wiki.rb

 module Gitlab
   class GitAccessWiki < GitAccess
-    def guest_can_downlod_code?
+    def guest_can_download_code?
       Guest.can?(:download_wiki_code, project)
     end
 
@@ -8,7 +8,7 @@ module Gitlab
       authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_wiki_code)
     end
 
-    def change_access_check(change)
+    def check_single_change_access(change)
       if user_access.can_do_action?(:create_wiki)
         build_status_object(true)
       else

lib/gitlab/user_access.rb

@@ -8,6 +8,8 @@ module Gitlab
     end
 
     def can_do_action?(action)
+      return false if no_user_or_blocked?
+
       @permission_cache ||= {}
       @permission_cache[action] ||= user.can?(action, project)
     end
@@ -17,7 +19,7 @@ module Gitlab
     end
 
     def allowed?
-      return false if user.blank? || user.blocked?
+      return false if no_user_or_blocked?
 
       if user.requires_ldap_check? && user.try_obtain_ldap_lease
         return false unless Gitlab::LDAP::Access.allowed?(user)
@@ -27,7 +29,7 @@ module Gitlab
     end
 
     def can_push_to_branch?(ref)
-      return false unless user
+      return false if no_user_or_blocked?
 
       if project.protected_branch?(ref)
         return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user)
@@ -40,7 +42,7 @@ module Gitlab
     end
 
     def can_merge_to_branch?(ref)
-      return false unless user
+      return false if no_user_or_blocked?
 
       if project.protected_branch?(ref)
         access_levels = project.protected_branches.matching(ref).map(&:merge_access_levels).flatten
@@ -51,9 +53,15 @@ module Gitlab
     end
 
     def can_read_project?
-      return false unless user
+      return false if no_user_or_blocked?
 
       user.can?(:read_project, project)
     end
+
+    private
+
+    def no_user_or_blocked?
+      user.nil? || user.blocked?
+    end
   end
 end

spec/lib/gitlab/git_access_spec.rb

@@ -50,7 +50,7 @@ describe Gitlab::GitAccess, lib: true do
     end
   end
 
-  describe 'download_access_check' do
+  describe '#check_download_access!' do
     subject { access.check('git-upload-pack', '_any') }
 
     describe 'master permissions' do
@@ -82,7 +82,7 @@ describe Gitlab::GitAccess, lib: true do
       end
     end
 
-    describe 'without acccess to project' do
+    describe 'without access to project' do
       context 'pull code' do
         it { expect(subject.allowed?).to be_falsey }
       end
@@ -112,7 +112,7 @@ describe Gitlab::GitAccess, lib: true do
     end
 
     describe 'deploy key permissions' do
-      let(:key) { create(:deploy_key) }
+      let(:key) { create(:deploy_key, user: user) }
       let(:actor) { key }
 
       context 'pull code' do
@@ -136,7 +136,7 @@ describe Gitlab::GitAccess, lib: true do
           end
 
           context 'from private project' do
-            let(:project) { create(:project, :internal) }
+            let(:project) { create(:project, :private) }
 
             it { expect(subject).not_to be_allowed }
           end
@@ -183,7 +183,7 @@ describe Gitlab::GitAccess, lib: true do
     end
   end
 
-  describe 'push_access_check' do
+  describe '#check_push_access!' do
     before { merge_into_protected_branch }
     let(:unprotected_branch) { FFaker::Internet.user_name }
 
@@ -231,7 +231,7 @@ describe Gitlab::GitAccess, lib: true do
 
             permissions_matrix[role].each do |action, allowed|
               context action do
-                subject { access.push_access_check(changes[action]) }
+                subject { access.send(:check_push_access!, changes[action]) }
                 it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
               end
             end
@@ -353,13 +353,13 @@ describe Gitlab::GitAccess, lib: true do
     end
   end
 
-  shared_examples 'can not push code' do
+  shared_examples 'pushing code' do |can|
     subject { access.check('git-receive-pack', '_any') }
 
     context 'when project is authorized' do
       before { authorize }
 
-      it { expect(subject).not_to be_allowed }
+      it { expect(subject).public_send(can, be_allowed) }
     end
 
     context 'when unauthorized' do
@@ -386,7 +386,7 @@ describe Gitlab::GitAccess, lib: true do
   describe 'build authentication abilities' do
     let(:authentication_abilities) { build_authentication_abilities }
 
-    it_behaves_like 'can not push code' do
+    it_behaves_like 'pushing code', :not_to do
       def authorize
         project.team << [user, :reporter]
       end
@@ -394,16 +394,30 @@ describe Gitlab::GitAccess, lib: true do
   end
 
   describe 'deploy key permissions' do
-    let(:key) { create(:deploy_key) }
+    let(:key) { create(:deploy_key, user: user, can_push: can_push) }
     let(:actor) { key }
 
-    it_behaves_like 'can not push code' do
+    context 'when deploy_key can push' do
+      let(:can_push) { true }
+
+      it_behaves_like 'pushing code', :to do
         def authorize
           key.projects << project
         end
       end
     end
 
+    context 'when deploy_key cannot push' do
+      let(:can_push) { false }
+
+      it_behaves_like 'pushing code', :not_to do
+        def authorize
+          key.projects << project
+        end
+      end
+    end
+  end
+
   private
 
   def build_authentication_abilities

spec/lib/gitlab/git_access_wiki_spec.rb

@@ -27,7 +27,7 @@ describe Gitlab::GitAccessWiki, lib: true do
     ['6f6d7e7ed 570e7b2ab refs/heads/master']
   end
 
-  describe '#download_access_check' do
+  describe '#access_check_download!' do
     subject { access.check('git-upload-pack', '_any') }
 
     before do

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -247,6 +247,7 @@ DeployKey:
 - type
 - fingerprint
 - public
+- can_push
 Service:
 - id
 - type

spec/models/deploy_key_spec.rb

 require 'spec_helper'
 
 describe DeployKey, models: true do
+  include EmailHelpers
+
   describe "Associations" do
     it { is_expected.to have_many(:deploy_keys_projects) }
     it { is_expected.to have_many(:projects) }
   end
+
+  describe 'notification' do
+    let(:user) { create(:user) }
+
+    it 'does not send a notification' do
+      perform_enqueued_jobs do
+        create(:deploy_key, user: user)
+      end
+
+      should_not_email(user)
+    end
+  end
 end

spec/models/key_spec.rb

 require 'spec_helper'
 
 describe Key, models: true do
+  include EmailHelpers
+
   describe "Associations" do
     it { is_expected.to belong_to(:user) }
   end
@@ -96,4 +98,16 @@ describe Key, models: true do
       expect(described_class.new(key: " #{valid_key} ").key).to eq(valid_key)
     end
   end
+
+  describe 'notification' do
+    let(:user) { create(:user) }
+
+    it 'sends a notification' do
+      perform_enqueued_jobs do
+        create(:key, user: user)
+      end
+
+      should_email(user)
+    end
+  end
 end

spec/requests/git_http_spec.rb

@@ -371,12 +371,26 @@ describe 'Git HTTP requests', lib: true do
 
             shared_examples 'can download code only' do
               it 'downloads get status 200' do
-                clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+                allow_any_instance_of(Repository).
+                  to receive(:exists?).and_return(true)
+
+                clone_get "#{project.path_with_namespace}.git",
+                  user: 'gitlab-ci-token', password: build.token
 
                 expect(response).to have_http_status(200)
                 expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
               end
 
+              it 'downloads from non-existing repository and gets 403' do
+                allow_any_instance_of(Repository).
+                  to receive(:exists?).and_return(false)
+
+                clone_get "#{project.path_with_namespace}.git",
+                  user: 'gitlab-ci-token', password: build.token
+
+                expect(response).to have_http_status(403)
+              end
+
               it 'uploads get status 403' do
                 push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token