Implement GET /users/:uid/keys for admin users

Complements POST operation added in gitlabhq/gitlabhq#3146 Implement DELETE /users/:uid/keys/:id for admin users Fix "Line is too long. [83/80]" Use single quotes as advised Use single quotes as advised Use single quotes as advised Fix missing space around { and } Fix typo in documentation Only catch ActiveRecord::RecordNotFound, let other exceptions propagate Raise a "404 Not found" if key to be deleted cannot be found As requested by @jvanbaarsen in https://github.com/gitlabhq/gitlabhq/pull/6781#discussion_r11735114 Remove tab Unconfigured vim on this box, grrrr./

Implement GET /users/:uid/keys for admin users
Complements POST operation added in gitlabhq/gitlabhq#3146 Implement DELETE /users/:uid/keys/:id for admin users Fix "Line is too long. [83/80]" Use single quotes as advised Use single quotes as advised Use single quotes as advised Fix missing space around { and } Fix typo in documentation Only catch ActiveRecord::RecordNotFound, let other exceptions propagate Raise a "404 Not found" if key to be deleted cannot be found As requested by @jvanbaarsen in https://github.com/gitlabhq/gitlabhq/pull/6781#discussion_r11735114 Remove tab Unconfigured vim on this box, grrrr./
Timm Friebe
1 parent cd623218
Showing 4 changed files with 129 additions and 2 deletions Show diff stats
Gemfile.lock
doc/api/users.md
lib/api/users.rb
spec/requests/api/users_spec.rb
@@ -644,7 +644,7 @@ DEPENDENCIES
   simplecov
   sinatra
   six
-  slack-notifier (~> 0.2.0)
+  slack-notifier (~> 0.3.2)
   slim
   spinach-rails
   spring (= 1.1.1)
@@ -662,4 +662,4 @@ DEPENDENCIES
   unicorn (~> 4.6.3)
   unicorn-worker-killer
   version_sorter
-  webmock
 \ No newline at end of file
+  webmock
@@ -220,6 +220,18 @@ Parameters:
 + **none**
+## List SSH keys for user
+
+Get a list of a specified user's SSH keys. Available only for admin
+
+```
+GET /users/:uid/keys
+```
+
+Parameters:
+
++ `uid` (required) - id of specified user
+
 ## Single SSH key
@@ -286,3 +298,18 @@ Parameters:
 + `id` (required) - SSH key ID
+## Delete SSH key
+
+Deletes key owned by a specified user. Available only for admin.
+
+```
+DELETE /users/:uid/keys/:id
+```
+
+Parameters:
+
++ `uid` (required) - id of specified user
++ `id` (required) - SSH key ID
+
+Will return `200 Ok` on success, or `404 Not found` if either user or key cannot be found.
+
@@ -113,6 +113,45 @@ module API
         end
       end
+      # Get ssh keys of a specified user. Only available to admin users.
+      #
+      # Parameters:
+      # uid (required) - The ID of a user
+      # Example Request:
+      # GET /users/:uid/keys
+      get ':uid/keys' do
+        authenticated_as_admin!
+        user = User.find_by(id: params[:uid])
+        if user
+          present user.keys, with: Entities::SSHKey
+        else
+          not_found!
+        end
+      end
+
+      # Delete existing ssh key of a specified user. Only available to admin
+      # users.
+      #
+      # Parameters:
+      #   uid (required) - The ID of a user
+      #   id (required) - SSH Key ID
+      # Example Request:
+      #   DELETE /users/:uid/keys/:id
+      delete ':uid/keys/:id' do
+        authenticated_as_admin!
+        user = User.find_by(id: params[:uid])
+        if user
+          begin
+            key = user.keys.find params[:id]
+            key.destroy
+          rescue ActiveRecord::RecordNotFound
+            not_found!
+          end
+        else
+          not_found!
+        end
+      end
+
       # Delete user. Available only for admin
       #
       # Example Request:
@@ -242,6 +242,67 @@ describe API::API, api: true  do
     end
   end
+  describe 'GET /user/:uid/keys' do
+    before { admin }
+
+    context 'when unauthenticated' do
+      it 'should return authentication error' do
+        get api("/users/#{user.id}/keys")
+        response.status.should == 401
+      end
+    end
+
+    context 'when authenticated' do
+      it 'should return 404 for non-existing user' do
+        get api('/users/999999/keys', admin)
+        response.status.should == 404
+      end
+
+      it 'should return array of ssh keys' do
+        user.keys << key
+        user.save
+        get api("/users/#{user.id}/keys", admin)
+        response.status.should == 200
+        json_response.should be_an Array
+        json_response.first['title'].should == key.title
+      end
+    end
+  end
+
+  describe 'DELETE /user/:uid/keys/:id' do
+    before { admin }
+
+    context 'when unauthenticated' do
+      it 'should return authentication error' do
+        delete api("/users/#{user.id}/keys/42")
+        response.status.should == 401
+      end
+    end
+
+    context 'when authenticated' do
+      it 'should delete existing key' do
+        user.keys << key
+        user.save
+        expect {
+          delete api("/users/#{user.id}/keys/#{key.id}", admin)
+        }.to change { user.keys.count }.by(-1)
+        response.status.should == 200
+      end
+
+      it 'should return 404 error if user not found' do
+        user.keys << key
+        user.save
+        delete api("/users/999999/keys/#{key.id}", admin)
+        response.status.should == 404
+      end
+
+      it 'should return 404 error if key not foud' do
+        delete api("/users/#{user.id}/keys/42", admin)
+        response.status.should == 404
+      end
+    end
+  end
+
   describe "DELETE /users/:id" do
     before { admin }
	@@ -220,6 +220,18 @@ Parameters:		@@ -220,6 +220,18 @@ Parameters:
220		220
221	+ none	221	+ none
222		222
		223	+## List SSH keys for user
		224	+
		225	+Get a list of a specified user's SSH keys. Available only for admin
		226	+
		227	+```
		228	+GET /users/:uid/keys
		229	+```
		230	+
		231	+Parameters:
		232	+
		233	++ `uid` (required) - id of specified user
		234	+
223		235
224	## Single SSH key	236	## Single SSH key
225		237
	@@ -286,3 +298,18 @@ Parameters:		@@ -286,3 +298,18 @@ Parameters:
286		298
287	+ `id` (required) - SSH key ID	299	+ `id` (required) - SSH key ID
288		300
		301	+## Delete SSH key
		302	+
		303	+Deletes key owned by a specified user. Available only for admin.
		304	+
		305	+```
		306	+DELETE /users/:uid/keys/:id
		307	+```
		308	+
		309	+Parameters:
		310	+
		311	++ `uid` (required) - id of specified user
		312	++ `id` (required) - SSH key ID
		313	+
		314	+Will return `200 Ok` on success, or `404 Not found` if either user or key cannot be found.
		315	+
	@@ -113,6 +113,45 @@ module API		@@ -113,6 +113,45 @@ module API
113	end	113	end
114	end	114	end
115		115
		116	+ # Get ssh keys of a specified user. Only available to admin users.
		117	+ #
		118	+ # Parameters:
		119	+ # uid (required) - The ID of a user
		120	+ # Example Request:
		121	+ # GET /users/:uid/keys
		122	+ get ':uid/keys' do
		123	+ authenticated_as_admin!
		124	+ user = User.find_by(id: params[:uid])
		125	+ if user
		126	+ present user.keys, with: Entities::SSHKey
		127	+ else
		128	+ not_found!
		129	+ end
		130	+ end
		131	+
		132	+ # Delete existing ssh key of a specified user. Only available to admin
		133	+ # users.
		134	+ #
		135	+ # Parameters:
		136	+ # uid (required) - The ID of a user
		137	+ # id (required) - SSH Key ID
		138	+ # Example Request:
		139	+ # DELETE /users/:uid/keys/:id
		140	+ delete ':uid/keys/:id' do
		141	+ authenticated_as_admin!
		142	+ user = User.find_by(id: params[:uid])
		143	+ if user
		144	+ begin
		145	+ key = user.keys.find params[:id]
		146	+ key.destroy
		147	+ rescue ActiveRecord::RecordNotFound
		148	+ not_found!
		149	+ end
		150	+ else
		151	+ not_found!
		152	+ end
		153	+ end
		154	+
116	# Delete user. Available only for admin	155	# Delete user. Available only for admin
117	#	156	#
118	# Example Request:	157	# Example Request:
	@@ -242,6 +242,67 @@ describe API::API, api: true do		@@ -242,6 +242,67 @@ describe API::API, api: true do
242	end	242	end
243	end	243	end
244		244
		245	+ describe 'GET /user/:uid/keys' do
		246	+ before { admin }
		247	+
		248	+ context 'when unauthenticated' do
		249	+ it 'should return authentication error' do
		250	+ get api("/users/#{user.id}/keys")
		251	+ response.status.should == 401
		252	+ end
		253	+ end
		254	+
		255	+ context 'when authenticated' do
		256	+ it 'should return 404 for non-existing user' do
		257	+ get api('/users/999999/keys', admin)
		258	+ response.status.should == 404
		259	+ end
		260	+
		261	+ it 'should return array of ssh keys' do
		262	+ user.keys << key
		263	+ user.save
		264	+ get api("/users/#{user.id}/keys", admin)
		265	+ response.status.should == 200
		266	+ json_response.should be_an Array
		267	+ json_response.first['title'].should == key.title
		268	+ end
		269	+ end
		270	+ end
		271	+
		272	+ describe 'DELETE /user/:uid/keys/:id' do
		273	+ before { admin }
		274	+
		275	+ context 'when unauthenticated' do
		276	+ it 'should return authentication error' do
		277	+ delete api("/users/#{user.id}/keys/42")
		278	+ response.status.should == 401
		279	+ end
		280	+ end
		281	+
		282	+ context 'when authenticated' do
		283	+ it 'should delete existing key' do
		284	+ user.keys << key
		285	+ user.save
		286	+ expect {
		287	+ delete api("/users/#{user.id}/keys/#{key.id}", admin)
		288	+ }.to change { user.keys.count }.by(-1)
		289	+ response.status.should == 200
		290	+ end
		291	+
		292	+ it 'should return 404 error if user not found' do
		293	+ user.keys << key
		294	+ user.save
		295	+ delete api("/users/999999/keys/#{key.id}", admin)
		296	+ response.status.should == 404
		297	+ end
		298	+
		299	+ it 'should return 404 error if key not foud' do
		300	+ delete api("/users/#{user.id}/keys/42", admin)
		301	+ response.status.should == 404
		302	+ end
		303	+ end
		304	+ end
		305	+
245	describe "DELETE /users/:id" do	306	describe "DELETE /users/:id" do
246	before { admin }	307	before { admin }
247		308