Fix Deploy Key and Public Key Semantics

Author: zeripath

cmd/serv.go

@@ -234,19 +234,20 @@ func runServ(c *cli.Context) error {
 
 		// Check deploy key or user key.
 		if key.Type == models.KeyTypeDeploy {
-			if key.Mode < requestedMode {
-				fail("Key permission denied", "Cannot push with deployment key: %d", key.ID)
-			}
-
-			// Check if this deploy key belongs to current repository.
-			has, err := private.HasDeployKey(key.ID, repo.ID)
+			// Now we have to get the deploy key for this repo
+			deployKey, err := private.GetDeployKey(key.ID, repo.ID)
 			if err != nil {
 				fail("Key access denied", "Failed to access internal api: [key_id: %d, repo_id: %d]", key.ID, repo.ID)
 			}
-			if !has {
+
+			if deployKey == nil {
 				fail("Key access denied", "Deploy key access denied: [key_id: %d, repo_id: %d]", key.ID, repo.ID)
 			}
 
+			if deployKey.Mode < requestedMode {
+				fail("Key permission denied", "Cannot push with deployment key: %d to repo_id: %d", key.ID, repo.ID)
+			}
+
 			// Update deploy key activity.
 			if err = private.UpdateDeployKeyUpdated(key.ID, repo.ID); err != nil {
 				fail("Internal error", "UpdateDeployKey: %v", err)

models/ssh_key.go

@@ -350,7 +350,6 @@ func appendAuthorizedKeysToFile(keys ...*PublicKey) error {
 func checkKeyFingerprint(e Engine, fingerprint string) error {
 	has, err := e.Get(&PublicKey{
 		Fingerprint: fingerprint,
-		Type:        KeyTypeUser,
 	})
 	if err != nil {
 		return err
@@ -401,12 +400,18 @@ func AddPublicKey(ownerID int64, name, content string, LoginSourceID int64) (*Pu
 		return nil, err
 	}
 
-	if err := checkKeyFingerprint(x, fingerprint); err != nil {
+	sess := x.NewSession()
+	defer sess.Close()
+	if err = sess.Begin(); err != nil {
+		return nil, err
+	}
+
+	if err := checkKeyFingerprint(sess, fingerprint); err != nil {
 		return nil, err
 	}
 
 	// Key name of same user cannot be duplicated.
-	has, err := x.
+	has, err := sess.
 		Where("owner_id = ? AND name = ?", ownerID, name).
 		Get(new(PublicKey))
 	if err != nil {
@@ -415,12 +420,6 @@ func AddPublicKey(ownerID int64, name, content string, LoginSourceID int64) (*Pu
 		return nil, ErrKeyNameAlreadyUsed{ownerID, name}
 	}
 
-	sess := x.NewSession()
-	defer sess.Close()
-	if err = sess.Begin(); err != nil {
-		return nil, err
-	}
-
 	key := &PublicKey{
 		OwnerID:       ownerID,
 		Name:          name,
@@ -728,24 +727,28 @@ func AddDeployKey(repoID int64, name, content string, readOnly bool) (*DeployKey
 		accessMode = AccessModeWrite
 	}
 
+	sess := x.NewSession()
+	defer sess.Close()
+	if err = sess.Begin(); err != nil {
+		return nil, err
+	}
+
 	pkey := &PublicKey{
 		Fingerprint: fingerprint,
-		Mode:        accessMode,
-		Type:        KeyTypeDeploy,
 	}
-	has, err := x.Get(pkey)
+	has, err := sess.Get(pkey)
 	if err != nil {
 		return nil, err
 	}
 
-	sess := x.NewSession()
-	defer sess.Close()
-	if err = sess.Begin(); err != nil {
-		return nil, err
-	}
-
-	// First time use this deploy key.
-	if !has {
+	if has {
+		if pkey.Type != KeyTypeDeploy {
+			return nil, ErrKeyAlreadyExist{0, fingerprint, ""}
+		}
+	} else {
+		// First time use this deploy key.
+		pkey.Mode = accessMode
+		pkey.Type = KeyTypeDeploy
 		pkey.Content = content
 		pkey.Name = name
 		if err = addKey(sess, pkey); err != nil {

modules/private/key.go

@@ -32,6 +32,31 @@ func UpdateDeployKeyUpdated(keyID int64, repoID int64) error {
 	return nil
 }
 
+// GetDeployKey check if repo has deploy key
+func GetDeployKey(keyID, repoID int64) (*models.DeployKey, error) {
+	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/repositories/%d/keys/%d", repoID, keyID)
+	log.GitLogger.Trace("GetDeployKey: %s", reqURL)
+
+	resp, err := newInternalRequest(reqURL, "GET").Response()
+	if err != nil {
+		return nil, err
+	}
+	defer resp.Body.Close()
+
+	switch resp.StatusCode {
+	case 404:
+		return nil, nil
+	case 200:
+		var dKey models.DeployKey
+		if err := json.NewDecoder(resp.Body).Decode(&dKey); err != nil {
+			return nil, err
+		}
+		return &dKey, nil
+	default:
+		return nil, fmt.Errorf("Failed to get deploy key: %s", decodeJSONError(resp).Err)
+	}
+}
+
 // HasDeployKey check if repo has deploy key
 func HasDeployKey(keyID, repoID int64) (bool, error) {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/repositories/%d/has-keys/%d", repoID, keyID)

options/locale/locale_en-US.ini

@@ -419,7 +419,7 @@ ssh_helper = <strong>Need help?</strong> Have a look at GitHub's guide to <a hre
 gpg_helper = <strong>Need help?</strong> Have a look at GitHub's guide <a href="%s">about GPG</a>.
 add_new_key = Add SSH Key
 add_new_gpg_key = Add GPG Key
-ssh_key_been_used = This SSH key is already added to your account.
+ssh_key_been_used = This SSH key has already been added to the server.
 ssh_key_name_used = An SSH key with same name is already added to your account.
 gpg_key_id_used = A public GPG key with same ID already exists.
 gpg_no_key_email_found = This GPG key is not usable with any email address associated with your account.

routers/api/v1/repo/key.go

@@ -159,6 +159,8 @@ func HandleCheckKeyStringError(ctx *context.APIContext, err error) {
 // HandleAddKeyError handle add key error
 func HandleAddKeyError(ctx *context.APIContext, err error) {
 	switch {
+	case models.IsErrDeployKeyAlreadyExist(err):
+		ctx.Error(422, "", "This key has already been added to this repository")
 	case models.IsErrKeyAlreadyExist(err):
 		ctx.Error(422, "", "Key content has been used as non-deploy key")
 	case models.IsErrKeyNameAlreadyUsed(err):

routers/private/internal.go

@@ -82,6 +82,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 		m.Post("/repositories/:repoid/keys/:keyid/update", UpdateDeployKey)
 		m.Get("/repositories/:repoid/user/:userid/checkunituser", CheckUnitUser)
 		m.Get("/repositories/:repoid/has-keys/:keyid", HasDeployKey)
+		m.Get("/repositories/:repoid/keys/:keyid", GetDeployKey)
 		m.Get("/repositories/:repoid/wiki/init", InitUncyclo)
 		m.Post("/push/update", PushUpdate)
 		m.Get("/protectedbranch/:pbid/:userid", CanUserPush)

routers/private/key.go

@@ -72,6 +72,24 @@ func GetUserByKeyID(ctx *macaron.Context) {
 	ctx.JSON(200, user)
 }
 
+//GetDeployKey chainload to models.GetDeployKey
+func GetDeployKey(ctx *macaron.Context) {
+	repoID := ctx.ParamsInt64(":repoid")
+	keyID := ctx.ParamsInt64(":keyid")
+	dKey, err := models.GetDeployKeyByRepo(keyID, repoID)
+	if err != nil {
+		if models.IsErrDeployKeyNotExist(err) {
+			ctx.JSON(404, []byte("not found"))
+			return
+		}
+		ctx.JSON(500, map[string]interface{}{
+			"err": err.Error(),
+		})
+		return
+	}
+	ctx.JSON(200, dKey)
+}
+
 //HasDeployKey chainload to models.HasDeployKey
 func HasDeployKey(ctx *macaron.Context) {
 	repoID := ctx.ParamsInt64(":repoid")

routers/repo/setting.go

@@ -622,6 +622,9 @@ func DeployKeysPost(ctx *context.Context, form auth.AddKeyForm) {
 		case models.IsErrDeployKeyAlreadyExist(err):
 			ctx.Data["Err_Content"] = true
 			ctx.RenderWithErr(ctx.Tr("repo.settings.key_been_used"), tplDeployKeys, &form)
+		case models.IsErrKeyAlreadyExist(err):
+			ctx.Data["Err_Content"] = true
+			ctx.RenderWithErr(ctx.Tr("settings.ssh_key_been_used"), tplDeployKeys, &form)
 		case models.IsErrKeyNameAlreadyUsed(err):
 			ctx.Data["Err_Title"] = true
 			ctx.RenderWithErr(ctx.Tr("repo.settings.key_name_used"), tplDeployKeys, &form)