Merge branch '3191-deploy-keys-update' into 'master'

Implement ability to update deploy keys

Closes #3191

See merge request !10383

app/assets/javascripts/deploy_keys/components/app.vue

@@ -80,21 +80,27 @@
       v-if="isLoading && !hasKeys"
       size="2"
       label="Loading deploy keys"
-      />
+    />
     <div v-else-if="hasKeys">
       <keys-panel
         title="Enabled deploy keys for this project"
         :keys="keys.enabled_keys"
-        :store="store" />
+        :store="store"
+        :endpoint="endpoint"
+      />
       <keys-panel
         title="Deploy keys from projects you have access to"
         :keys="keys.available_project_keys"
-        :store="store" />
+        :store="store"
+        :endpoint="endpoint"
+      />
       <keys-panel
         v-if="keys.public_keys.length"
         title="Public deploy keys available to any project"
         :keys="keys.public_keys"
-        :store="store" />
+        :store="store"
+        :endpoint="endpoint"
+      />
     </div>
   </div>
 </template>

app/assets/javascripts/deploy_keys/components/key.vue

@@ -11,6 +11,10 @@
         type: Object,
         required: true,
       },
+      endpoint: {
+        type: String,
+        required: true,
+      },
     },
     components: {
       actionBtn,
@@ -19,6 +23,9 @@
       timeagoDate() {
         return gl.utils.getTimeago().format(this.deployKey.created_at);
       },
+      editDeployKeyPath() {
+        return `${this.endpoint}/${this.deployKey.id}/edit`;
+      },
     },
     methods: {
       isEnabled(id) {
@@ -33,7 +40,8 @@
     <div class="pull-left append-right-10 hidden-xs">
       <i
         aria-hidden="true"
-        class="fa fa-key key-icon">
+        class="fa fa-key key-icon"
+      >
       </i>
     </div>
     <div class="deploy-key-content key-list-item-info">
@@ -45,7 +53,8 @@
       </div>
       <div
         v-if="deployKey.can_push"
-        class="write-access-allowed">
+        class="write-access-allowed"
+      >
         Write access allowed
       </div>
     </div>
@@ -53,7 +62,8 @@
       <a
         v-for="project in deployKey.projects"
         class="label deploy-project-label"
-        :href="project.full_path">
+        :href="project.full_path"
+      >
         {{ project.full_name }}
       </a>
     </div>
@@ -61,20 +71,30 @@
       <span class="key-created-at">
         created {{ timeagoDate }}
       </span>
+      <a
+        v-if="deployKey.can_edit"
+        class="btn btn-small"
+        :href="editDeployKeyPath"
+      >
+        Edit
+      </a>
       <action-btn
         v-if="!isEnabled(deployKey.id)"
         :deploy-key="deployKey"
-        type="enable"/>
+        type="enable"
+      />
       <action-btn
         v-else-if="deployKey.destroyed_when_orphaned && deployKey.almost_orphaned"
         :deploy-key="deployKey"
         btn-css-class="btn-warning"
-        type="remove" />
+        type="remove"
+      />
       <action-btn
         v-else
         :deploy-key="deployKey"
         btn-css-class="btn-warning"
-        type="disable" />
+        type="disable"
+      />
     </div>
   </div>
 </template>

app/assets/javascripts/deploy_keys/components/keys_panel.vue

@@ -20,6 +20,10 @@
         type: Object,
         required: true,
       },
+      endpoint: {
+        type: String,
+        required: true,
+      },
     },
     components: {
       key,
@@ -34,18 +38,22 @@
       ({{ keys.length }})
     </h5>
     <ul class="well-list"
-      v-if="keys.length">
+      v-if="keys.length"
+    >
       <li
         v-for="deployKey in keys"
         :key="deployKey.id">
         <key
           :deploy-key="deployKey"
-          :store="store" />
+          :store="store"
+          :endpoint="endpoint"
+        />
       </li>
     </ul>
     <div
       class="settings-message text-center"
-      v-else-if="showHelpBox">
+      v-else-if="showHelpBox"
+    >
       No deploy keys found. Create one with the form above.
     </div>
   </div>

app/controllers/admin/deploy_keys_controller.rb

 class Admin::DeployKeysController < Admin::ApplicationController
   before_action :deploy_keys, only: [:index]
-  before_action :deploy_key, only: [:destroy]
+  before_action :deploy_key, only: [:destroy, :edit, :update]
 
   def index
   end
@@ -10,12 +10,24 @@ class Admin::DeployKeysController < Admin::ApplicationController
   end
 
   def create
-    @deploy_key = deploy_keys.new(deploy_key_params.merge(user: current_user))
+    @deploy_key = deploy_keys.new(create_params.merge(user: current_user))
 
     if @deploy_key.save
       redirect_to admin_deploy_keys_path
     else
-      render "new"
+      render 'new'
+    end
+  end
+
+  def edit
+  end
+
+  def update
+    if deploy_key.update_attributes(update_params)
+      flash[:notice] = 'Deploy key was successfully updated.'
+      redirect_to admin_deploy_keys_path
+    else
+      render 'edit'
     end
   end
 
@@ -38,7 +50,11 @@ class Admin::DeployKeysController < Admin::ApplicationController
     @deploy_keys ||= DeployKey.are_public
   end
 
-  def deploy_key_params
+  def create_params
     params.require(:deploy_key).permit(:key, :title, :can_push)
   end
+
+  def update_params
+    params.require(:deploy_key).permit(:title, :can_push)
+  end
 end

app/controllers/projects/deploy_keys_controller.rb

@@ -4,6 +4,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
 
   # Authorize
   before_action :authorize_admin_project!
+  before_action :authorize_update_deploy_key!, only: [:edit, :update]
 
   layout "project_settings"
 
@@ -21,7 +22,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
   end
 
   def create
-    @key = DeployKey.new(deploy_key_params.merge(user: current_user))
+    @key = DeployKey.new(create_params.merge(user: current_user))
 
     unless @key.valid? && @project.deploy_keys << @key
       flash[:alert] = @key.errors.full_messages.join(', ').html_safe
@@ -29,6 +30,18 @@ class Projects::DeployKeysController < Projects::ApplicationController
     redirect_to_repository_settings(@project)
   end
 
+  def edit
+  end
+
+  def update
+    if deploy_key.update_attributes(update_params)
+      flash[:notice] = 'Deploy key was successfully updated.'
+      redirect_to_repository_settings(@project)
+    else
+      render 'edit'
+    end
+  end
+
   def enable
     Projects::EnableDeployKeyService.new(@project, current_user, params).execute
 
@@ -52,7 +65,19 @@ class Projects::DeployKeysController < Projects::ApplicationController
 
   protected
 
-  def deploy_key_params
+  def deploy_key
+    @deploy_key ||= @project.deploy_keys.find(params[:id])
+  end
+
+  def create_params
     params.require(:deploy_key).permit(:key, :title, :can_push)
   end
+
+  def update_params
+    params.require(:deploy_key).permit(:title, :can_push)
+  end
+
+  def authorize_update_deploy_key!
+    access_denied! unless can?(current_user, :update_deploy_key, deploy_key)
+  end
 end

app/policies/deploy_key_policy.rb

+class DeployKeyPolicy < BasePolicy
+  def rules
+    return unless @user
+
+    can! :update_deploy_key if @user.admin?
+
+    if @subject.private? && @user.project_deploy_keys.exists?(id: @subject.id)
+      can! :update_deploy_key
+    end
+  end
+end

app/presenters/projects/settings/deploy_keys_presenter.rb

@@ -11,7 +11,7 @@ module Projects
       end
 
       def enabled_keys
-        @enabled_keys ||= project.deploy_keys
+        @enabled_keys ||= project.deploy_keys.includes(:projects)
       end
 
       def any_keys_enabled?
@@ -23,11 +23,7 @@ module Projects
       end
 
       def available_project_keys
-        @available_project_keys ||= current_user.project_deploy_keys - enabled_keys
-      end
-
-      def any_available_project_keys_enabled?
-        available_project_keys.any?
+        @available_project_keys ||= current_user.project_deploy_keys.includes(:projects) - enabled_keys
       end
 
       def key_available?(deploy_key)
@@ -37,17 +33,13 @@ module Projects
       def available_public_keys
         return @available_public_keys if defined?(@available_public_keys)
 
-        @available_public_keys ||= DeployKey.are_public - enabled_keys
+        @available_public_keys ||= DeployKey.are_public.includes(:projects) - enabled_keys
 
         # Public keys that are already used by another accessible project are already
         # in @available_project_keys.
         @available_public_keys -= available_project_keys
       end
 
-      def any_available_public_keys_enabled?
-        available_public_keys.any?
-      end
-
       def as_json
         serializer = DeployKeySerializer.new
         opts = { user: current_user }

app/serializers/deploy_key_entity.rb

@@ -11,4 +11,11 @@ class DeployKeyEntity < Grape::Entity
   expose :projects, using: ProjectEntity do |deploy_key|
     deploy_key.projects.select { |project| options[:user].can?(:read_project, project) }
   end
+  expose :can_edit
+
+  private
+
+  def can_edit
+    options[:user].can?(:update_deploy_key, object)
+  end
 end

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

+- page_title 'Edit Deploy Key'
+%h3.page-title Edit public deploy key
+%hr
+
+%div
+  = form_for [:admin, @deploy_key], html: { class: 'deploy-key-form form-horizontal' } do |f|
+    = render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key }
+    .form-actions
+      = f.submit 'Save changes', class: 'btn-save btn'
+      = link_to 'Cancel', admin_deploy_keys_path, class: 'btn btn-cancel'

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

@@ -31,4 +31,6 @@
               %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'
+              .pull-right
+                = link_to 'Edit', edit_admin_deploy_key_path(deploy_key), class: 'btn btn-sm'
+                = link_to 'Remove', admin_deploy_key_path(deploy_key), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-remove delete-key'

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

-- page_title "New Deploy Key"
+- page_title 'New Deploy Key'
 %h3.page-title New public deploy key
 %hr
 
 %div
   = form_for [:admin, @deploy_key], html: { class: 'deploy-key-form form-horizontal' } do |f|
-    = form_errors(@deploy_key)
-
-    .form-group
-      = f.label :title, class: "control-label"
-      .col-sm-10= f.text_field :title, class: 'form-control'
-    .form-group
-      = f.label :key, class: "control-label"
-      .col-sm-10
-        %p.light
-          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.)
-
+    = render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key }
     .form-actions
-      = f.submit 'Create', class: "btn-create btn"
-      = link_to "Cancel", admin_deploy_keys_path, class: "btn btn-cancel"
-
+      = f.submit 'Create', class: 'btn-create btn'
+      = link_to 'Cancel', admin_deploy_keys_path, class: 'btn btn-cancel'

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

-%li
-  .pull-left.append-right-10.hidden-xs
-    = icon "key", class: "key-icon"
-  .deploy-key-content.key-list-item-info
-    %strong.title
-      = 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)
-        = link_to namespace_project_path(project.namespace, project), class: "label deploy-project-label" do
-          = project.name_with_namespace
-  .deploy-key-content
-    %span.key-created-at
-      created #{time_ago_with_tooltip(deploy_key.created_at)}
-    .visible-xs-block.visible-sm-block
-    - if @deploy_keys.key_available?(deploy_key)
-      = link_to enable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), class: "btn btn-sm prepend-left-10", method: :put do
-        Enable
-    - else
-      - if deploy_key.destroyed_when_orphaned? && deploy_key.almost_orphaned?
-        = link_to disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), data: { confirm: "You are going to remove deploy key. Are you sure?" }, method: :put, class: "btn btn-warning btn-sm prepend-left-10" do
-          Remove
-      - else
-        = link_to disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), class: "btn btn-warning btn-sm prepend-left-10", method: :put do
-          Disable

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

+- page_title 'Edit Deploy Key'
+%h3.page-title Edit Deploy Key
+%hr
+
+%div
+  = form_for [@project.namespace.becomes(Namespace), @project, @deploy_key], html: { class: 'form-horizontal js-requires-input' } do |f|
+    = render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key }
+    .form-actions
+      = f.submit 'Save changes', class: 'btn-save btn'
+      = link_to 'Cancel', namespace_project_settings_repository_path(@project.namespace, @project), class: 'btn btn-cancel'

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

-- page_title "New Deploy Key"
-%h3.page-title New Deploy Key
-%hr
-
-= render 'form'

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

+- form = local_assigns.fetch(:form)
+- deploy_key = local_assigns.fetch(:deploy_key)
+
+= form_errors(deploy_key)
+
+.form-group
+  = form.label :title, class: 'control-label'
+  .col-sm-10= form.text_field :title, class: 'form-control'
+
+.form-group
+  - if deploy_key.new_record?
+    = form.label :key, class: 'control-label'
+    .col-sm-10
+      %p.light
+        Paste a machine public key here. Read more about how to generate it
+        = link_to 'here', help_page_path('ssh/README')
+      = form.text_area :key, class: 'form-control thin_area', rows: 5
+  - else
+    = form.label :fingerprint, class: 'control-label'
+    .col-sm-10
+      = form.text_field :fingerprint, class: 'form-control', readonly: 'readonly'
+
+.form-group
+  .control-label
+  .col-sm-10
+    = form.label :can_push do
+      = form.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.)

changelogs/unreleased/3191-deploy-keys-update.yml

+---
+title: Implement ability to update deploy keys
+merge_request: 10383
+author: Alexander Randa

config/routes/admin.rb

@@ -48,7 +48,7 @@ namespace :admin do
     end
   end
 
-  resources :deploy_keys, only: [:index, :new, :create, :destroy]
+  resources :deploy_keys, only: [:index, :new, :create, :edit, :update, :destroy]
 
   resources :hooks, only: [:index, :create, :edit, :update, :destroy] do
     member do

config/routes/project.rb

@@ -73,7 +73,7 @@ constraints(ProjectUrlConstrainer.new) do
 
       resource :mattermost, only: [:new, :create]
 
-      resources :deploy_keys, constraints: { id: /\d+/ }, only: [:index, :new, :create] do
+      resources :deploy_keys, constraints: { id: /\d+/ }, only: [:index, :new, :create, :edit, :update] do
         member do
           put :enable
           put :disable

lib/api/deploy_keys.rb

@@ -76,6 +76,27 @@ module API
         end
       end
 
+      desc 'Update an existing deploy key for a project' do
+        success Entities::SSHKey
+      end
+      params do
+        requires :key_id, type: Integer, desc: 'The ID of the deploy key'
+        optional :title, type: String, desc: 'The name of the deploy key'
+        optional :can_push, type: Boolean, desc: "Can deploy key push to the project's repository"
+        at_least_one_of :title, :can_push
+      end
+      put ":id/deploy_keys/:key_id" do
+        key = user_project.deploy_keys.find(params.delete(:key_id))
+
+        authorize!(:update_deploy_key, key)
+
+        if key.update_attributes(declared_params(include_missing: false))
+          present key, with: Entities::SSHKey
+        else
+          render_validation_error!(key)
+        end
+      end
+
       desc 'Enable a deploy key for a project' do
         detail 'This feature was added in GitLab 8.11'
         success Entities::SSHKey

spec/features/admin/admin_deploy_keys_spec.rb

@@ -11,40 +11,67 @@ RSpec.describe 'admin deploy keys', type: :feature do
   it 'show all public deploy keys' do
     visit admin_deploy_keys_path
 
-    expect(page).to have_content(deploy_key.title)
-    expect(page).to have_content(another_deploy_key.title)
+    page.within(find('.deploy-keys-list', match: :first)) do
+      expect(page).to have_content(deploy_key.title)
+      expect(page).to have_content(another_deploy_key.title)
+    end
   end
 
-  describe 'create new deploy key' do
+  describe 'create a new deploy key' do
+    let(:new_ssh_key) { attributes_for(:key)[:key] }
+
     before do
       visit admin_deploy_keys_path
       click_link 'New deploy key'
     end
 
-    it 'creates new deploy key' do
-      fill_deploy_key
+    it 'creates a new deploy key' do
+      fill_in 'deploy_key_title', with: 'laptop'
+      fill_in 'deploy_key_key', with: new_ssh_key
+      check 'deploy_key_can_push'
       click_button 'Create'
 
-      expect_renders_new_key
-    end
+      expect(current_path).to eq admin_deploy_keys_path
 
-    it 'creates new deploy key with write access' do
-      fill_deploy_key
-      check "deploy_key_can_push"
-      click_button "Create"
+      page.within(find('.deploy-keys-list', match: :first)) do
+        expect(page).to have_content('laptop')
+        expect(page).to have_content('Yes')
+      end
+    end
+  end
 
-      expect_renders_new_key
-      expect(page).to have_content('Yes')
+  describe 'update an existing deploy key' do
+    before do
+      visit admin_deploy_keys_path
+      find('tr', text: deploy_key.title).click_link('Edit')
     end
 
-    def expect_renders_new_key
+    it 'updates an existing deploy key' do
+      fill_in 'deploy_key_title', with: 'new-title'
+      check 'deploy_key_can_push'
+      click_button 'Save changes'
+
       expect(current_path).to eq admin_deploy_keys_path
-      expect(page).to have_content('laptop')
+
+      page.within(find('.deploy-keys-list', match: :first)) do
+        expect(page).to have_content('new-title')
+        expect(page).to have_content('Yes')
+      end
     end
+  end
 
-    def fill_deploy_key
-      fill_in 'deploy_key_title', with: 'laptop'
-      fill_in 'deploy_key_key', with: 'ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAzrEJUIR6Y03TCE9rIJ+GqTBvgb8t1jI9h5UBzCLuK4VawOmkLornPqLDrGbm6tcwM/wBrrLvVOqi2HwmkKEIecVO0a64A4rIYScVsXIniHRS6w5twyn1MD3sIbN+socBDcaldECQa2u1dI3tnNVcs8wi77fiRe7RSxePsJceGoheRQgC8AZ510UdIlO+9rjIHUdVN7LLyz512auAfYsgx1OfablkQ/XJcdEwDNgi9imI6nAXhmoKUm1IPLT2yKajTIC64AjLOnE0YyCh6+7RFMpiMyu1qiOCpdjYwTgBRiciNRZCH8xIedyCoAmiUgkUT40XYHwLuwiPJICpkAzp7Q== user@laptop'
+  describe 'remove an existing deploy key' do
+    before do
+      visit admin_deploy_keys_path
+    end
+
+    it 'removes an existing deploy key' do
+      find('tr', text: deploy_key.title).click_link('Remove')
+
+      expect(current_path).to eq admin_deploy_keys_path
+      page.within(find('.deploy-keys-list', match: :first)) do
+        expect(page).not_to have_content(deploy_key.title)
+      end
     end
   end
 end

spec/features/projects/settings/repository_settings_spec.rb

+require 'spec_helper'
+
+feature 'Repository settings', feature: true do
+  let(:project) { create(:project_empty_repo) }
+  let(:user) { create(:user) }
+  let(:role) { :developer }
+
+  background do
+    project.team << [user, role]
+    login_as(user)
+  end
+
+  context 'for developer' do
+    given(:role) { :developer }
+
+    scenario 'is not allowed to view' do
+      visit namespace_project_settings_repository_path(project.namespace, project)
+
+      expect(page.status_code).to eq(404)
+    end
+  end
+
+  context 'for master' do
+    given(:role) { :master }
+
+    context 'Deploy Keys', js: true do
+      let(:private_deploy_key) { create(:deploy_key, title: 'private_deploy_key', public: false) }
+      let(:public_deploy_key) { create(:another_deploy_key, title: 'public_deploy_key', public: true) }
+      let(:new_ssh_key) { attributes_for(:key)[:key] }
+
+      scenario 'get list of keys' do
+        project.deploy_keys << private_deploy_key
+        project.deploy_keys << public_deploy_key
+
+        visit namespace_project_settings_repository_path(project.namespace, project)
+
+        expect(page.status_code).to eq(200)
+        expect(page).to have_content('private_deploy_key')
+        expect(page).to have_content('public_deploy_key')
+      end
+
+      scenario 'add a new deploy key' do
+        visit namespace_project_settings_repository_path(project.namespace, project)
+
+        fill_in 'deploy_key_title', with: 'new_deploy_key'
+        fill_in 'deploy_key_key', with: new_ssh_key
+        check 'deploy_key_can_push'
+        click_button 'Add key'
+
+        expect(page).to have_content('new_deploy_key')
+        expect(page).to have_content('Write access allowed')
+      end
+
+      scenario 'edit an existing deploy key' do
+        project.deploy_keys << private_deploy_key
+        visit namespace_project_settings_repository_path(project.namespace, project)
+
+        find('li', text: private_deploy_key.title).click_link('Edit')
+
+        fill_in 'deploy_key_title', with: 'updated_deploy_key'
+        check 'deploy_key_can_push'
+        click_button 'Save changes'
+
+        expect(page).to have_content('updated_deploy_key')
+        expect(page).to have_content('Write access allowed')
+      end
+
+      scenario 'remove an existing deploy key' do
+        project.deploy_keys << private_deploy_key
+        visit namespace_project_settings_repository_path(project.namespace, project)
+
+        find('li', text: private_deploy_key.title).click_button('Remove')
+
+        expect(page).not_to have_content(private_deploy_key.title)
+      end
+    end
+  end
+end

spec/javascripts/deploy_keys/components/key_spec.js

@@ -39,9 +39,15 @@ describe('Deploy keys key', () => {
       ).toBe(`created ${gl.utils.getTimeago().format(deployKey.created_at)}`);
     });
 
+    it('shows edit button', () => {
+      expect(
+        vm.$el.querySelectorAll('.btn')[0].textContent.trim(),
+      ).toBe('Edit');
+    });
+
     it('shows remove button', () => {
       expect(
-        vm.$el.querySelector('.btn').textContent.trim(),
+        vm.$el.querySelectorAll('.btn')[1].textContent.trim(),
       ).toBe('Remove');
     });
 
@@ -71,9 +77,15 @@ describe('Deploy keys key', () => {
       setTimeout(done);
     });
 
+    it('shows edit button', () => {
+      expect(
+        vm.$el.querySelectorAll('.btn')[0].textContent.trim(),
+      ).toBe('Edit');
+    });
+
     it('shows enable button', () => {
       expect(
-        vm.$el.querySelector('.btn').textContent.trim(),
+        vm.$el.querySelectorAll('.btn')[1].textContent.trim(),
       ).toBe('Enable');
     });
 
@@ -82,7 +94,7 @@ describe('Deploy keys key', () => {
 
       Vue.nextTick(() => {
         expect(
-          vm.$el.querySelector('.btn').textContent.trim(),
+          vm.$el.querySelectorAll('.btn')[1].textContent.trim(),
         ).toBe('Disable');
 
         done();

spec/policies/deploy_key_policy_spec.rb

+require 'spec_helper'
+
+describe DeployKeyPolicy, models: true do
+  subject { described_class.abilities(current_user, deploy_key).to_set }
+
+  describe 'updating a deploy_key' do
+    context 'when a regular user' do
+      let(:current_user) { create(:user) }
+
+      context 'tries to update private deploy key attached to project' do
+        let(:deploy_key) { create(:deploy_key, public: false) }
+        let(:project) { create(:project_empty_repo) }
+
+        before do
+          project.add_master(current_user)
+          project.deploy_keys << deploy_key
+        end
+
+        it { is_expected.to include(:update_deploy_key) }
+      end
+
+      context 'tries to update private deploy key attached to other project' do
+        let(:deploy_key) { create(:deploy_key, public: false) }
+        let(:other_project) { create(:project_empty_repo) }
+
+        before do
+          other_project.deploy_keys << deploy_key
+        end
+
+        it { is_expected.not_to include(:update_deploy_key) }
+      end
+
+      context 'tries to update public deploy key' do
+        let(:deploy_key) { create(:another_deploy_key, public: true) }
+
+        it { is_expected.not_to include(:update_deploy_key) }
+      end
+    end
+
+    context 'when an admin user' do
+      let(:current_user) { create(:user, :admin) }
+
+      context ' tries to update private deploy key' do
+        let(:deploy_key) { create(:deploy_key, public: false) }
+
+        it { is_expected.to include(:update_deploy_key) }
+      end
+
+      context 'when an admin user tries to update public deploy key' do
+        let(:deploy_key) { create(:another_deploy_key, public: true) }
+
+        it { is_expected.to include(:update_deploy_key) }
+      end
+    end
+  end
+end

spec/presenters/projects/settings/deploy_keys_presenter_spec.rb

@@ -51,10 +51,6 @@ describe Projects::Settings::DeployKeysPresenter do
       expect(presenter.available_project_keys).not_to be_empty
     end
 
-    it 'returns false if any available_project_keys are enabled' do
-      expect(presenter.any_available_project_keys_enabled?).to eq(true)
-    end
-
     it 'returns the available_project_keys size' do
       expect(presenter.available_project_keys_size).to eq(1)
     end

spec/requests/api/deploy_keys_spec.rb

@@ -13,7 +13,7 @@ describe API::DeployKeys do
 
   describe 'GET /deploy_keys' do
     context 'when unauthenticated' do
-      it 'should return authentication error' do
+      it 'returns authentication error' do
         get api('/deploy_keys')
 
         expect(response.status).to eq(401)
@@ -21,7 +21,7 @@ describe API::DeployKeys do
     end
 
     context 'when authenticated as non-admin user' do
-      it 'should return a 403 error' do
+      it 'returns a 403 error' do
         get api('/deploy_keys', user)
 
         expect(response.status).to eq(403)
@@ -29,7 +29,7 @@ describe API::DeployKeys do
     end
 
     context 'when authenticated as admin' do
-      it 'should return all deploy keys' do
+      it 'returns all deploy keys' do
         get api('/deploy_keys', admin)
 
         expect(response.status).to eq(200)
@@ -43,7 +43,7 @@ describe API::DeployKeys do
   describe 'GET /projects/:id/deploy_keys' do
     before { deploy_key }
 
-    it 'should return array of ssh keys' do
+    it 'returns array of ssh keys' do
       get api("/projects/#{project.id}/deploy_keys", admin)
 
       expect(response).to have_http_status(200)
@@ -54,14 +54,14 @@ describe API::DeployKeys do
   end
 
   describe 'GET /projects/:id/deploy_keys/:key_id' do
-    it 'should return a single key' do
+    it 'returns a single key' do
       get api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin)
 
       expect(response).to have_http_status(200)
       expect(json_response['title']).to eq(deploy_key.title)
     end
 
-    it 'should return 404 Not Found with invalid ID' do
+    it 'returns 404 Not Found with invalid ID' do
       get api("/projects/#{project.id}/deploy_keys/404", admin)
 
       expect(response).to have_http_status(404)
@@ -69,26 +69,26 @@ describe API::DeployKeys do
   end
 
   describe 'POST /projects/:id/deploy_keys' do
-    it 'should not create an invalid ssh key' do
+    it 'does not create an invalid ssh key' do
       post api("/projects/#{project.id}/deploy_keys", admin), { title: 'invalid key' }
 
       expect(response).to have_http_status(400)
       expect(json_response['error']).to eq('key is missing')
     end
 
-    it 'should not create a key without title' do
+    it 'does not create a key without title' do
       post api("/projects/#{project.id}/deploy_keys", admin), key: 'some key'
 
       expect(response).to have_http_status(400)
       expect(json_response['error']).to eq('title is missing')
     end
 
-    it 'should create new ssh key' do
+    it 'creates new ssh key' do
       key_attrs = attributes_for :another_key
 
       expect do
         post api("/projects/#{project.id}/deploy_keys", admin), key_attrs
-      end.to change{ project.deploy_keys.count }.by(1)
+      end.to change { project.deploy_keys.count }.by(1)
     end
 
     it 'returns an existing ssh key when attempting to add a duplicate' do
@@ -117,10 +117,53 @@ describe API::DeployKeys do
     end
   end
 
+  describe 'PUT /projects/:id/deploy_keys/:key_id' do
+    let(:private_deploy_key) { create(:another_deploy_key, public: false) }
+    let(:project_private_deploy_key) do
+      create(:deploy_keys_project, project: project, deploy_key: private_deploy_key)
+    end
+
+    it 'updates a public deploy key as admin' do
+      expect do
+        put api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin), { title: 'new title' }
+      end.not_to change(deploy_key, :title)
+
+      expect(response).to have_http_status(200)
+    end
+
+    it 'does not update a public deploy key as non admin' do
+      expect do
+        put api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", user), { title: 'new title' }
+      end.not_to change(deploy_key, :title)
+
+      expect(response).to have_http_status(404)
+    end
+
+    it 'does not update a private key with invalid title' do
+      project_private_deploy_key
+
+      expect do
+        put api("/projects/#{project.id}/deploy_keys/#{private_deploy_key.id}", admin), { title: '' }
+      end.not_to change(deploy_key, :title)
+
+      expect(response).to have_http_status(400)
+    end
+
+    it 'updates a private ssh key with correct attributes' do
+      project_private_deploy_key
+
+      put api("/projects/#{project.id}/deploy_keys/#{private_deploy_key.id}", admin), { title: 'new title', can_push: true }
+
+      expect(json_response['id']).to eq(private_deploy_key.id)
+      expect(json_response['title']).to eq('new title')
+      expect(json_response['can_push']).to eq(true)
+    end
+  end
+
   describe 'DELETE /projects/:id/deploy_keys/:key_id' do
     before { deploy_key }
 
-    it 'should delete existing key' do
+    it 'deletes existing key' do
       expect do
         delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin)
 
@@ -128,7 +171,7 @@ describe API::DeployKeys do
       end.to change{ project.deploy_keys.count }.by(-1)
     end
 
-    it 'should return 404 Not Found with invalid ID' do
+    it 'returns 404 Not Found with invalid ID' do
       delete api("/projects/#{project.id}/deploy_keys/404", admin)
 
       expect(response).to have_http_status(404)
@@ -150,7 +193,7 @@ describe API::DeployKeys do
     end
 
     context 'when authenticated as non-admin user' do
-      it 'should return a 404 error' do
+      it 'returns a 404 error' do
         post api("/projects/#{project2.id}/deploy_keys/#{deploy_key.id}/enable", user)
 
         expect(response).to have_http_status(404)

spec/routing/project_routing_spec.rb

@@ -201,10 +201,12 @@ describe 'project routing' do
   #                         POST   /:project_id/deploy_keys(.:format)          deploy_keys#create
   #  new_project_deploy_key GET    /:project_id/deploy_keys/new(.:format)      deploy_keys#new
   #      project_deploy_key GET    /:project_id/deploy_keys/:id(.:format)      deploy_keys#show
+  # edit_project_deploy_key GET    /:project_id/deploy_keys/:id/edit(.:format) deploy_keys#edit
+  #      project_deploy_key PATCH  /:project_id/deploy_keys/:id(.:format)      deploy_keys#update
   #                         DELETE /:project_id/deploy_keys/:id(.:format)      deploy_keys#destroy
   describe Projects::DeployKeysController, 'routing' do
     it_behaves_like 'RESTful project resources' do
-      let(:actions)    { [:index, :new, :create] }
+      let(:actions)    { [:index, :new, :create, :edit, :update] }
       let(:controller) { 'deploy_keys' }
     end
   end

spec/serializers/deploy_key_entity_spec.rb

@@ -12,27 +12,44 @@ describe DeployKeyEntity do
 
   let(:entity) { described_class.new(deploy_key, user: user) }
 
-  it 'returns deploy keys with projects a user can read' do
-    expected_result = {
-      id: deploy_key.id,
-      user_id: deploy_key.user_id,
-      title: deploy_key.title,
-      fingerprint: deploy_key.fingerprint,
-      can_push: deploy_key.can_push,
-      destroyed_when_orphaned: true,
-      almost_orphaned: false,
-      created_at: deploy_key.created_at,
-      updated_at: deploy_key.updated_at,
-      projects: [
-        {
-          id: project.id,
-          name: project.name,
-          full_path: namespace_project_path(project.namespace, project),
-          full_name: project.full_name
-        }
-      ]
-    }
-
-    expect(entity.as_json).to eq(expected_result)
+  describe 'returns deploy keys with projects a user can read' do
+    let(:expected_result) do
+      {
+        id: deploy_key.id,
+        user_id: deploy_key.user_id,
+        title: deploy_key.title,
+        fingerprint: deploy_key.fingerprint,
+        can_push: deploy_key.can_push,
+        destroyed_when_orphaned: true,
+        almost_orphaned: false,
+        created_at: deploy_key.created_at,
+        updated_at: deploy_key.updated_at,
+        can_edit: false,
+        projects: [
+          {
+            id: project.id,
+            name: project.name,
+            full_path: namespace_project_path(project.namespace, project),
+            full_name: project.full_name
+          }
+        ]
+      }
+    end
+
+    it { expect(entity.as_json).to eq(expected_result) }
+  end
+
+  describe 'returns can_edit true if user is a master of project' do
+    before do
+      project.add_master(user)
+    end
+
+    it { expect(entity.as_json).to include(can_edit: true) }
+  end
+
+  describe 'returns can_edit true if a user admin' do
+    let(:user) { create(:user, :admin) }
+
+    it { expect(entity.as_json).to include(can_edit: true) }
   end
 end