Implement ability to update deploy keys

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
-    it 'creates new deploy key with write access' do
-      fill_deploy_key
-      check "deploy_key_can_push"
-      click_button "Create"
-      expect_renders_new_key
-      expect(page).to have_content('Yes')
+      expect(current_path).to eq admin_deploy_keys_path
+      page.within(find('.deploy-keys-list', match: :first)) do
+        expect(page).to have_content('laptop')
+        expect(page).to have_content('Yes')
+      end
+    end
+  end
+  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
-    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'
+  end
+  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