Delete a user's public key via admin api (closes #3014) (#3059)

* Delete a user's public key via admin api

* Test admin ssh endpoint for creating a new ssh key

* Adapt public ssh key test to also test the delete operation

* Test that deleting a missing key will result in a 404

* Test that a normal user can't delete another user's ssh key

* Make DeletePublicKey return err

* Update swagger doc

integrations/api_admin_test.go

@@ -0,0 +1,73 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package integrations
+
+import (
+	"fmt"
+	"net/http"
+	"testing"
+
+	"code.gitea.io/gitea/models"
+	api "code.gitea.io/sdk/gitea"
+)
+
+func TestAPIAdminCreateAndDeleteSSHKey(t *testing.T) {
+	prepareTestEnv(t)
+	// user1 is an admin user
+	session := loginUser(t, "user1")
+	keyOwner := models.AssertExistsAndLoadBean(t, &models.User{Name: "user2"}).(*models.User)
+
+	urlStr := fmt.Sprintf("/api/v1/admin/users/%s/keys", keyOwner.Name)
+	req := NewRequestWithValues(t, "POST", urlStr, map[string]string{
+		"key":   "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n",
+		"title": "test-key",
+	})
+	resp := session.MakeRequest(t, req, http.StatusCreated)
+
+	var newPublicKey api.PublicKey
+	DecodeJSON(t, resp, &newPublicKey)
+	models.AssertExistsAndLoadBean(t, &models.PublicKey{
+		ID:          newPublicKey.ID,
+		Name:        newPublicKey.Title,
+		Content:     newPublicKey.Key,
+		Fingerprint: newPublicKey.Fingerprint,
+		OwnerID:     keyOwner.ID,
+	})
+
+	req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d",
+		keyOwner.Name, newPublicKey.ID)
+	session.MakeRequest(t, req, http.StatusNoContent)
+	models.AssertNotExistsBean(t, &models.PublicKey{ID: newPublicKey.ID})
+}
+
+func TestAPIAdminDeleteMissingSSHKey(t *testing.T) {
+	prepareTestEnv(t)
+	// user1 is an admin user
+	session := loginUser(t, "user1")
+
+	req := NewRequestf(t, "DELETE", "/api/v1/admin/users/user1/keys/%d", models.NonexistentID)
+	session.MakeRequest(t, req, http.StatusNotFound)
+}
+
+func TestAPIAdminDeleteUnauthorizedKey(t *testing.T) {
+	prepareTestEnv(t)
+	adminUsername := "user1"
+	normalUsername := "user2"
+	session := loginUser(t, adminUsername)
+
+	urlStr := fmt.Sprintf("/api/v1/admin/users/%s/keys", adminUsername)
+	req := NewRequestWithValues(t, "POST", urlStr, map[string]string{
+		"key":   "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n",
+		"title": "test-key",
+	})
+	resp := session.MakeRequest(t, req, http.StatusCreated)
+	var newPublicKey api.PublicKey
+	DecodeJSON(t, resp, &newPublicKey)
+
+	session = loginUser(t, normalUsername)
+	req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d",
+		adminUsername, newPublicKey.ID)
+	session.MakeRequest(t, req, http.StatusForbidden)
+}

models/ssh_key.go

@@ -506,10 +506,7 @@ func deletePublicKeys(e *xorm.Session, keyIDs ...int64) error {
 func DeletePublicKey(doer *User, id int64) (err error) {
 	key, err := GetPublicKeyByID(id)
 	if err != nil {
-		if IsErrKeyNotExist(err) {
+		return err
-			return nil
-		}
-		return fmt.Errorf("GetPublicKeyByID: %v", err)
 	}
 
 	// Check if user has access to delete this key.

routers/api/v1/admin/user.go

@@ -236,3 +236,48 @@ func CreatePublicKey(ctx *context.APIContext, form api.CreateKeyOption) {
 	}
 	user.CreateUserPublicKey(ctx, form, u.ID)
 }
+
+// DeleteUserPublicKey api for deleting a user's public key
+func DeleteUserPublicKey(ctx *context.APIContext) {
+	// swagger:operation DELETE /admin/users/{username}/keys/{id} admin adminDeleteUserPublicKey
+	// ---
+	// summary: Delete a user's public key
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: username
+	//   in: path
+	//   description: username of user
+	//   type: string
+	//   required: true
+	// - name: id
+	//   in: path
+	//   description: id of the key to delete
+	//   type: integer
+	//   required: true
+	// responses:
+	//   "204":
+	//     "$ref": "#/responses/empty"
+	//   "403":
+	//     "$ref": "#/responses/forbidden"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
+	u := user.GetUserByParams(ctx)
+	if ctx.Written() {
+		return
+	}
+
+	if err := models.DeletePublicKey(u, ctx.ParamsInt64(":id")); err != nil {
+		if models.IsErrKeyNotExist(err) {
+			ctx.Status(404)
+		} else if models.IsErrKeyAccessDenied(err) {
+			ctx.Error(403, "", "You do not have access to this key")
+		} else {
+			ctx.Error(500, "DeleteUserPublicKey", err)
+		}
+		return
+	}
+	log.Trace("Key deleted by admin(%s): %s", ctx.User.Name, u.Name)
+
+	ctx.Status(204)
+}

routers/api/v1/api.go

@@ -542,7 +542,10 @@ func RegisterRoutes(m *macaron.Macaron) {
 				m.Group("/:username", func() {
 					m.Combo("").Patch(bind(api.EditUserOption{}), admin.EditUser).
 						Delete(admin.DeleteUser)
-					m.Post("/keys", bind(api.CreateKeyOption{}), admin.CreatePublicKey)
+					m.Group("/keys", func() {
+						m.Post("", bind(api.CreateKeyOption{}), admin.CreatePublicKey)
+						m.Delete("/:id", admin.DeleteUserPublicKey)
+					})
 					m.Post("/orgs", bind(api.CreateOrgOption{}), admin.CreateOrg)
 					m.Post("/repos", bind(api.CreateRepoOption{}), admin.CreateRepo)
 				})

routers/api/v1/user/key.go

@@ -178,8 +178,12 @@ func DeletePublicKey(ctx *context.APIContext) {
 	//     "$ref": "#/responses/empty"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
 	if err := models.DeletePublicKey(ctx.User, ctx.ParamsInt64(":id")); err != nil {
-		if models.IsErrKeyAccessDenied(err) {
+		if models.IsErrKeyNotExist(err) {
+			ctx.Status(404)
+		} else if models.IsErrKeyAccessDenied(err) {
 			ctx.Error(403, "", "You do not have access to this key")
 		} else {
 			ctx.Error(500, "DeletePublicKey", err)