Merge branch...

Merge branch '14284-allow-all-deploy-keys-to-be-retrieved-via-api-regardless-of-project-affiliation' into 'master'

Add /deploy_keys API to retrieve all deploy keys regardless of project affiliation

## What does this MR do?

Add /deploy_keys API to retrieve all deploy keys regardless of project affiliation

## Are there points in the code the reviewer needs to double check?

Is documentation clear?

## Why was this MR needed?

User request

## What are the relevant issue numbers?

#14284

See merge request !4426

CHANGELOG

@@ -30,6 +30,8 @@ v 8.10.0 (unreleased)
   - Apply the trusted_proxies config to the rack request object for use with rack_attack
   - Upgrade to Rails 4.2.7. !5236
   - Extend exposed environment variables for CI builds
+  - Deprecate APIs "projects/:id/keys/...". Use "projects/:id/deploy_keys/..." instead
+  - Add API "deploy_keys" for admins to get all deploy keys
   - Allow to pull code with deploy key from public projects
   - Use limit parameter rather than hardcoded value in `ldap:check` rake task (Mike Ricketts)
   - Add Sidekiq queue duration to transaction metrics.

doc/api/deploy_key_multiple_projects.md

@@ -24,6 +24,6 @@ With those IDs, add the same deploy key to all:
 ```
 for project_id in 321 456 987; do
     curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -H "Content-Type: application/json" \
-    --data '{"title": "my key", "key": "ssh-rsa AAAA..."}' https://gitlab.example.com/api/v3/projects/${project_id}/keys
+    --data '{"title": "my key", "key": "ssh-rsa AAAA..."}' https://gitlab.example.com/api/v3/projects/${project_id}/deploy_keys
 done
 ```

doc/api/deploy_keys.md

 # Deploy Keys
 
-## List deploy keys
+## List all deploy keys
+
+Get a list of all deploy keys across all projects.
+
+```
+GET /deploy_keys
+```
+
+```bash
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/deploy_keys"
+```
+
+Example response:
+
+```json
+[
+  {
+    "id": 1,
+    "title": "Public key",
+    "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
+    "created_at": "2013-10-02T10:12:29Z"
+  },
+  {
+    "id": 3,
+    "title": "Another Public key",
+    "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",
+    "created_at": "2013-10-02T11:12:29Z"
+  }
+]
+```
+
+## List project deploy keys
 
 Get a list of a project's deploy keys.
 
 ```
-GET /projects/:id/keys
+GET /projects/:id/deploy_keys
 ```
 
 | Attribute | Type | Required | Description |
@@ -13,7 +44,7 @@ GET /projects/:id/keys
 | `id` | integer | yes | The ID of the project |
 
 ```bash
-curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/keys"
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/deploy_keys"
 ```
 
 Example response:
@@ -40,7 +71,7 @@ Example response:
 Get a single key.
 
 ```
-GET /projects/:id/keys/:key_id
+GET /projects/:id/deploy_keys/:key_id
 ```
 
 Parameters:
@@ -51,7 +82,7 @@ Parameters:
 | `key_id`  | integer | yes | The ID of the deploy key |
 
 ```bash
-curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/keys/11"
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/deploy_keys/11"
 ```
 
 Example response:
@@ -73,7 +104,7 @@ If the deploy key already exists in another project, it will be joined to curren
 project only if original one was is accessible by the same user.
 
 ```
-POST /projects/:id/keys
+POST /projects/:id/deploy_keys
 ```
 
 | Attribute | Type | Required | Description |
@@ -83,7 +114,7 @@ POST /projects/:id/keys
 | `key`   | string  | yes | New deploy key |
 
 ```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -H "Content-Type: application/json" --data '{"title": "My deploy key", "key": "ssh-rsa AAAA..."}' "https://gitlab.example.com/api/v3/projects/5/keys/"
+curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -H "Content-Type: application/json" --data '{"title": "My deploy key", "key": "ssh-rsa AAAA..."}' "https://gitlab.example.com/api/v3/projects/5/deploy_keys/"
 ```
 
 Example response:
@@ -102,7 +133,7 @@ Example response:
 Delete a deploy key from a project
 
 ```
-DELETE /projects/:id/keys/:key_id
+DELETE /projects/:id/deploy_keys/:key_id
 ```
 
 | Attribute | Type | Required | Description |
@@ -111,7 +142,7 @@ DELETE /projects/:id/keys/:key_id
 | `key_id`  | integer | yes | The ID of the deploy key |
 
 ```bash
-curl -X DELETE -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/keys/13"
+curl -X DELETE -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/deploy_keys/13"
 ```
 
 Example response:

lib/api/deploy_keys.rb

@@ -2,74 +2,87 @@ module API
   # Projects API
   class DeployKeys < Grape::API
     before { authenticate! }
-    before { authorize_admin_project }
+
+    get "deploy_keys" do
+      authenticated_as_admin!
+
+      keys = DeployKey.all
+      present keys, with: Entities::SSHKey
+    end
 
     resource :projects do
-      # Get a specific project's keys
-      #
-      # Example Request:
-      #   GET /projects/:id/keys
-      get ":id/keys" do
-        present user_project.deploy_keys, with: Entities::SSHKey
-      end
+      before { authorize_admin_project }
 
-      # Get single key owned by currently authenticated user
+      # Routing "projects/:id/keys/..." is DEPRECATED and WILL BE REMOVED in version 9.0
+      # Use "projects/:id/deploy_keys/..." instead.
       #
-      # Example Request:
-      #   GET /projects/:id/keys/:id
-      get ":id/keys/:key_id" do
-        key = user_project.deploy_keys.find params[:key_id]
-        present key, with: Entities::SSHKey
-      end
+      %w(keys deploy_keys).each do |path|
+        # Get a specific project's deploy keys
+        #
+        # Example Request:
+        #   GET /projects/:id/deploy_keys
+        get ":id/#{path}" do
+          present user_project.deploy_keys, with: Entities::SSHKey
+        end
 
-      # Add new ssh key to currently authenticated user
-      # If deploy key already exists - it will be joined to project
-      # but only if original one was is accessible by same user
-      #
-      # Parameters:
-      #   key (required) - New SSH Key
-      #   title (required) - New SSH Key's title
-      # Example Request:
-      #   POST /projects/:id/keys
-      post ":id/keys" do
-        attrs = attributes_for_keys [:title, :key]
+        # Get single deploy key owned by currently authenticated user
+        #
+        # Example Request:
+        #   GET /projects/:id/deploy_keys/:key_id
+        get ":id/#{path}/:key_id" do
+          key = user_project.deploy_keys.find params[:key_id]
+          present key, with: Entities::SSHKey
+        end
 
-        if attrs[:key].present?
-          attrs[:key].strip!
+        # Add new deploy key to currently authenticated user
+        # If deploy key already exists - it will be joined to project
+        # but only if original one was accessible by same user
+        #
+        # Parameters:
+        #   key (required) - New deploy Key
+        #   title (required) - New deploy Key's title
+        # Example Request:
+        #   POST /projects/:id/deploy_keys
+        post ":id/#{path}" do
+          attrs = attributes_for_keys [:title, :key]
 
-          # check if key already exist in project
-          key = user_project.deploy_keys.find_by(key: attrs[:key])
-          if key
-            present key, with: Entities::SSHKey
-            return
+          if attrs[:key].present?
+            attrs[:key].strip!
+
+            # check if key already exist in project
+            key = user_project.deploy_keys.find_by(key: attrs[:key])
+            if key
+              present key, with: Entities::SSHKey
+              next
+            end
+
+            # Check for available deploy keys in other projects
+            key = current_user.accessible_deploy_keys.find_by(key: attrs[:key])
+            if key
+              user_project.deploy_keys << key
+              present key, with: Entities::SSHKey
+              next
+            end
           end
 
-          # Check for available deploy keys in other projects
-          key = current_user.accessible_deploy_keys.find_by(key: attrs[:key])
-          if key
-            user_project.deploy_keys << key
+          key = DeployKey.new attrs
+
+          if key.valid? && user_project.deploy_keys << key
             present key, with: Entities::SSHKey
-            return
+          else
+            render_validation_error!(key)
           end
         end
 
-        key = DeployKey.new attrs
-
-        if key.valid? && user_project.deploy_keys << key
-          present key, with: Entities::SSHKey
-        else
-          render_validation_error!(key)
+        # Delete existing deploy key of currently authenticated user
+        #
+        # Example Request:
+        #   DELETE /projects/:id/deploy_keys/:key_id
+        delete ":id/#{path}/:key_id" do
+          key = user_project.deploy_keys.find params[:key_id]
+          key.destroy
         end
       end
-
-      # Delete existed ssh key of currently authenticated user
-      #
-      # Example Request:
-      #   DELETE /projects/:id/keys/:id
-      delete ":id/keys/:key_id" do
-        key = user_project.deploy_keys.find params[:key_id]
-        key.destroy
-      end
     end
   end
 end

spec/requests/api/deploy_keys.rb

+require 'spec_helper'
+
+describe API::API, api: true  do
+  include ApiHelpers
+
+  let(:user)  { create(:user) }
+  let(:project) { create(:project, creator_id: user.id) }
+  let!(:deploy_keys_project) { create(:deploy_keys_project, project: project) }
+  let(:admin) { create(:admin) }
+
+  describe 'GET /deploy_keys' do
+    before { admin }
+
+    context 'when unauthenticated' do
+      it 'should return authentication error' do
+        get api('/deploy_keys')
+        expect(response.status).to eq(401)
+      end
+    end
+
+    context 'when authenticated as non-admin user' do
+      it 'should return a 403 error' do
+        get api('/deploy_keys', user)
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'when authenticated as admin' do
+      it 'should return all deploy keys' do
+        get api('/deploy_keys', admin)
+        expect(response.status).to eq(200)
+
+        expect(json_response).to be_an Array
+        expect(json_response.first['id']).to eq(deploy_keys_project.deploy_key.id)
+      end
+    end
+  end
+end

spec/requests/api/projects_spec.rb

@@ -647,33 +647,33 @@ describe API::API, api: true  do
     let(:deploy_keys_project) { create(:deploy_keys_project, project: project) }
     let(:deploy_key) { deploy_keys_project.deploy_key }
 
-    describe 'GET /projects/:id/keys' do
+    describe 'GET /projects/:id/deploy_keys' do
       before { deploy_key }
 
       it 'should return array of ssh keys' do
-        get api("/projects/#{project.id}/keys", user)
+        get api("/projects/#{project.id}/deploy_keys", user)
         expect(response).to have_http_status(200)
         expect(json_response).to be_an Array
         expect(json_response.first['title']).to eq(deploy_key.title)
       end
     end
 
-    describe 'GET /projects/:id/keys/:key_id' do
+    describe 'GET /projects/:id/deploy_keys/:key_id' do
       it 'should return a single key' do
-        get api("/projects/#{project.id}/keys/#{deploy_key.id}", user)
+        get api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", user)
         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
-        get api("/projects/#{project.id}/keys/404", user)
+        get api("/projects/#{project.id}/deploy_keys/404", user)
         expect(response).to have_http_status(404)
       end
     end
 
-    describe 'POST /projects/:id/keys' do
+    describe 'POST /projects/:id/deploy_keys' do
       it 'should not create an invalid ssh key' do
-        post api("/projects/#{project.id}/keys", user), { title: 'invalid key' }
+        post api("/projects/#{project.id}/deploy_keys", user), { title: 'invalid key' }
         expect(response).to have_http_status(400)
         expect(json_response['message']['key']).to eq([
           'can\'t be blank',
@@ -683,7 +683,7 @@ describe API::API, api: true  do
       end
 
       it 'should not create a key without title' do
-        post api("/projects/#{project.id}/keys", user), key: 'some key'
+        post api("/projects/#{project.id}/deploy_keys", user), key: 'some key'
         expect(response).to have_http_status(400)
         expect(json_response['message']['title']).to eq([
           'can\'t be blank',
@@ -694,22 +694,22 @@ describe API::API, api: true  do
       it 'should create new ssh key' do
         key_attrs = attributes_for :key
         expect do
-          post api("/projects/#{project.id}/keys", user), key_attrs
+          post api("/projects/#{project.id}/deploy_keys", user), key_attrs
         end.to change{ project.deploy_keys.count }.by(1)
       end
     end
 
-    describe 'DELETE /projects/:id/keys/:key_id' do
+    describe 'DELETE /projects/:id/deploy_keys/:key_id' do
       before { deploy_key }
 
       it 'should delete existing key' do
         expect do
-          delete api("/projects/#{project.id}/keys/#{deploy_key.id}", user)
+          delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", user)
         end.to change{ project.deploy_keys.count }.by(-1)
       end
 
       it 'should return 404 Not Found with invalid ID' do
-        delete api("/projects/#{project.id}/keys/404", user)
+        delete api("/projects/#{project.id}/deploy_keys/404", user)
         expect(response).to have_http_status(404)
       end
     end