models/ssh_key: code cleaning

1 files changed, 57 insertions, 59 deletions

diff --git a/models/ssh_key.go b/models/ssh_key.go
index 92f38b570a..4fbc2e8d47 100644
--- a/models/ssh_key.go
+++ b/models/ssh_key.go
@@ -30,11 +30,10 @@ import (
 )
 
 const (
-	// "### autogenerated by gitgos, DO NOT EDIT\n"
 	_TPL_PUBLICK_KEY = `command="%s serv key-%d --config='%s'",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty %s` + "\n"
 )
 
-var sshOpLocker = sync.Mutex{}
+var sshOpLocker sync.Mutex
 
 type KeyType int
 
@@ -43,7 +42,7 @@ const (
 	KEY_TYPE_DEPLOY
 )
 
-// PublicKey represents a SSH or deploy key.
+// PublicKey represents a user or deploy SSH public key.
 type PublicKey struct {
 	ID          int64      `xorm:"pk autoincr"`
 	OwnerID     int64      `xorm:"INDEX NOT NULL"`
@@ -80,45 +79,44 @@ func (k *PublicKey) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
-// OmitEmail returns content of public key but without e-mail address.
+// OmitEmail returns content of public key without email address.
 func (k *PublicKey) OmitEmail() string {
 	return strings.Join(strings.Split(k.Content, " ")[:2], " ")
 }
 
-// GetAuthorizedString generates and returns formatted public key string for authorized_keys file.
-func (key *PublicKey) GetAuthorizedString() string {
+// AuthorizedString returns formatted public key string for authorized_keys file.
+func (key *PublicKey) AuthorizedString() string {
 	return fmt.Sprintf(_TPL_PUBLICK_KEY, setting.AppPath, key.ID, setting.CustomConf, key.Content)
 }
 
 func extractTypeFromBase64Key(key string) (string, error) {
 	b, err := base64.StdEncoding.DecodeString(key)
 	if err != nil || len(b) < 4 {
-		return "", fmt.Errorf("Invalid key format: %v", err)
+		return "", fmt.Errorf("invalid key format: %v", err)
 	}
 
 	keyLength := int(binary.BigEndian.Uint32(b))
 	if len(b) < 4+keyLength {
-		return "", fmt.Errorf("Invalid key format: not enough length")
+		return "", fmt.Errorf("invalid key format: not enough length %d", keyLength)
 	}
 
 	return string(b[4 : 4+keyLength]), nil
 }
 
-// parseKeyString parses any key string in OpenSSH or SSH2 format to clean OpenSSH string (RFC4253)
+// parseKeyString parses any key string in OpenSSH or SSH2 format to clean OpenSSH string (RFC4253).
 func parseKeyString(content string) (string, error) {
-	// Transform all legal line endings to a single "\n"
-	s := strings.Replace(strings.Replace(strings.TrimSpace(content), "\r\n", "\n", -1), "\r", "\n", -1)
-
-	lines := strings.Split(s, "\n")
+	// Transform all legal line endings to a single "\n".
+	content = strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(content)
+	lines := strings.Split(content, "\n")
 
 	var keyType, keyContent, keyComment string
 
 	if len(lines) == 1 {
-		// Parse openssh format
+		// Parse OpenSSH format.
 		parts := strings.SplitN(lines[0], " ", 3)
 		switch len(parts) {
 		case 0:
-			return "", errors.New("Empty key")
+			return "", errors.New("empty key")
 		case 1:
 			keyContent = parts[0]
 		case 2:
@@ -130,17 +128,15 @@ func parseKeyString(content string) (string, error) {
 			keyComment = parts[2]
 		}
 
-		// If keyType is not given, extract it from content. If given, validate it
+		// If keyType is not given, extract it from content. If given, validate it.
+		t, err := extractTypeFromBase64Key(keyContent)
+		if err != nil {
+			return "", fmt.Errorf("extractTypeFromBase64Key: %v", err)
+		}
 		if len(keyType) == 0 {
-			if t, err := extractTypeFromBase64Key(keyContent); err == nil {
-				keyType = t
-			} else {
-				return "", err
-			}
-		} else {
-			if t, err := extractTypeFromBase64Key(keyContent); err != nil || keyType != t {
-				return "", err
-			}
+			keyType = t
+		} else if keyType != t {
+			return "", fmt.Errorf("key type and content does not match: %s - %s", keyType, t)
 		}
 	} else {
 		// Parse SSH2 file format.
@@ -158,11 +154,11 @@ func parseKeyString(content string) (string, error) {
 			}
 		}
 
-		if t, err := extractTypeFromBase64Key(keyContent); err == nil {
-			keyType = t
-		} else {
-			return "", err
+		t, err := extractTypeFromBase64Key(keyContent)
+		if err != nil {
+			return "", fmt.Errorf("extractTypeFromBase64Key: %v", err)
 		}
+		keyType = t
 	}
 	return keyType + " " + keyContent + " " + keyComment, nil
 }
@@ -177,7 +173,7 @@ func writeTmpKeyFile(content string) (string, error) {
 	defer tmpFile.Close()
 
 	if _, err = tmpFile.WriteString(content); err != nil {
-		return "", fmt.Errorf("tmpFile.WriteString: %v", err)
+		return "", fmt.Errorf("WriteString: %v", err)
 	}
 	return tmpFile.Name(), nil
 }
@@ -197,7 +193,7 @@ func SSHKeyGenParsePublicKey(key string) (string, int, error) {
 
 	stdout, stderr, err := process.Exec("SSHKeyGenParsePublicKey", setting.SSH.KeygenPath, "-lf", tmpName)
 	if err != nil {
-		return "", 0, fmt.Errorf("Fail to parse public key: %s - %s", err, stderr)
+		return "", 0, fmt.Errorf("fail to parse public key: %s - %s", err, stderr)
 	}
 	if strings.Contains(stdout, "is not a public key file") {
 		return "", 0, ErrKeyUnableVerify{stdout}
@@ -205,7 +201,7 @@ func SSHKeyGenParsePublicKey(key string) (string, int, error) {
 
 	fields := strings.Split(stdout, " ")
 	if len(fields) < 4 {
-		return "", 0, fmt.Errorf("Invalid public key line: %s", stdout)
+		return "", 0, fmt.Errorf("invalid public key line: %s", stdout)
 	}
 
 	keyType := strings.Trim(fields[len(fields)-1], "()\r\n")
@@ -230,7 +226,7 @@ func SSHNativeParsePublicKey(keyLine string) (string, int, error) {
 		if strings.Contains(err.Error(), "ssh: unknown key algorithm") {
 			return "", 0, ErrKeyUnableVerify{err.Error()}
 		}
-		return "", 0, fmt.Errorf("ssh.ParsePublicKey: %v", err)
+		return "", 0, fmt.Errorf("ParsePublicKey: %v", err)
 	}
 
 	// The ssh library can parse the key, so next we find out what key exactly we have.
@@ -262,15 +258,14 @@ func SSHNativeParsePublicKey(keyLine string) (string, int, error) {
 		return "ecdsa", 384, nil
 	case ssh.KeyAlgoECDSA521:
 		return "ecdsa", 521, nil
-	case "ssh-ed25519": // TODO replace with ssh constant when available
+	case "ssh-ed25519": // TODO: replace with ssh constant when available
 		return "ed25519", 256, nil
 	}
-	return "", 0, fmt.Errorf("Unsupported key length detection for type: %s", pkey.Type())
+	return "", 0, fmt.Errorf("unsupported key length detection for type: %s", pkey.Type())
 }
 
 // CheckPublicKeyString checks if the given public key string is recognized by SSH.
-//
-// The function returns the actual public key line on success.
+// It returns the actual public key line on success.
 func CheckPublicKeyString(content string) (_ string, err error) {
 	if setting.SSH.Disabled {
 		return "", errors.New("SSH is disabled")
@@ -290,16 +285,19 @@ func CheckPublicKeyString(content string) (_ string, err error) {
 	content = strings.TrimSpace(content)
 
 	var (
+		fnName  string
 		keyType string
 		length  int
 	)
 	if setting.SSH.StartBuiltinServer {
+		fnName = "SSHNativeParsePublicKey"
 		keyType, length, err = SSHNativeParsePublicKey(content)
 	} else {
+		fnName = "SSHKeyGenParsePublicKey"
 		keyType, length, err = SSHKeyGenParsePublicKey(content)
 	}
 	if err != nil {
-		return "", fmt.Errorf("ParsePublicKey: %v", err)
+		return "", fmt.Errorf("%s: %v", fnName, err)
 	}
 	log.Trace("Key info [native: %v]: %s-%d", setting.SSH.StartBuiltinServer, keyType, length)
 
@@ -309,13 +307,13 @@ func CheckPublicKeyString(content string) (_ string, err error) {
 	if minLen, found := setting.SSH.MinimumKeySizes[keyType]; found && length >= minLen {
 		return content, nil
 	} else if found && length < minLen {
-		return "", fmt.Errorf("Key length is not enough: got %d, needs %d", length, minLen)
+		return "", fmt.Errorf("key length is not enough: got %d, needs %d", length, minLen)
 	}
-	return "", fmt.Errorf("Key type is not allowed: %s", keyType)
+	return "", fmt.Errorf("key type is not allowed: %s", keyType)
 }
 
-// saveAuthorizedKeyFile writes SSH key content to authorized_keys file.
-func saveAuthorizedKeyFile(keys ...*PublicKey) error {
+// appendAuthorizedKeysToFile appends new SSH keys' content to authorized_keys file.
+func appendAuthorizedKeysToFile(keys ...*PublicKey) error {
 	sshOpLocker.Lock()
 	defer sshOpLocker.Unlock()
 
@@ -326,13 +324,13 @@ func saveAuthorizedKeyFile(keys ...*PublicKey) error {
 	}
 	defer f.Close()
 
-	fi, err := f.Stat()
-	if err != nil {
-		return err
-	}
-
-	// FIXME: following command does not support in Windows.
+	// Note: chmod command does not support in Windows.
 	if !setting.IsWindows {
+		fi, err := f.Stat()
+		if err != nil {
+			return err
+		}
+
 		// .ssh directory should have mode 700, and authorized_keys file should have mode 600.
 		if fi.Mode().Perm() > 0600 {
 			log.Error(4, "authorized_keys file has unusual permission flags: %s - setting to -rw-------", fi.Mode().Perm().String())
@@ -343,7 +341,7 @@ func saveAuthorizedKeyFile(keys ...*PublicKey) error {
 	}
 
 	for _, key := range keys {
-		if _, err = f.WriteString(key.GetAuthorizedString()); err != nil {
+		if _, err = f.WriteString(key.AuthorizedString()); err != nil {
 			return err
 		}
 	}
@@ -390,7 +388,7 @@ func addKey(e Engine, key *PublicKey) (err error) {
 	if setting.SSH.StartBuiltinServer {
 		return nil
 	}
-	return saveAuthorizedKeyFile(key)
+	return appendAuthorizedKeysToFile(key)
 }
 
 // AddPublicKey adds new public key to database and authorized_keys file.
@@ -401,7 +399,7 @@ func AddPublicKey(ownerID int64, name, content string) (*PublicKey, error) {
 	}
 
 	// Key name of same user cannot be duplicated.
-	has, err := x.Where("owner_id=? AND name=?", ownerID, name).Get(new(PublicKey))
+	has, err := x.Where("owner_id = ? AND name = ?", ownerID, name).Get(new(PublicKey))
 	if err != nil {
 		return nil, err
 	} else if has {
@@ -456,7 +454,7 @@ func SearchPublicKeyByContent(content string) (*PublicKey, error) {
 // ListPublicKeys returns a list of public keys belongs to given user.
 func ListPublicKeys(uid int64) ([]*PublicKey, error) {
 	keys := make([]*PublicKey, 0, 5)
-	return keys, x.Where("owner_id=?", uid).Find(&keys)
+	return keys, x.Where("owner_id = ?", uid).Find(&keys)
 }
 
 // rewriteAuthorizedKeys finds and deletes corresponding line in authorized_keys file.
@@ -593,7 +591,7 @@ func RewriteAllPublicKeys() error {
 	defer os.Remove(tmpPath)
 
 	err = x.Iterate(new(PublicKey), func(idx int, bean interface{}) (err error) {
-		_, err = f.WriteString((bean.(*PublicKey)).GetAuthorizedString())
+		_, err = f.WriteString((bean.(*PublicKey)).AuthorizedString())
 		return err
 	})
 	f.Close()
@@ -668,14 +666,14 @@ func (k *DeployKey) GetContent() error {
 
 func checkDeployKey(e Engine, keyID, repoID int64, name string) error {
 	// Note: We want error detail, not just true or false here.
-	has, err := e.Where("key_id=? AND repo_id=?", keyID, repoID).Get(new(DeployKey))
+	has, err := e.Where("key_id = ? AND repo_id = ?", keyID, repoID).Get(new(DeployKey))
 	if err != nil {
 		return err
 	} else if has {
 		return ErrDeployKeyAlreadyExist{keyID, repoID}
 	}
 
-	has, err = e.Where("repo_id=? AND name=?", repoID, name).Get(new(DeployKey))
+	has, err = e.Where("repo_id = ? AND name = ?", repoID, name).Get(new(DeployKey))
 	if err != nil {
 		return err
 	} else if has {
@@ -703,7 +701,7 @@ func addDeployKey(e *xorm.Session, keyID, repoID int64, name, fingerprint string
 
 // HasDeployKey returns true if public key is a deploy key of given repository.
 func HasDeployKey(keyID, repoID int64) bool {
-	has, _ := x.Where("key_id=? AND repo_id=?", keyID, repoID).Get(new(DeployKey))
+	has, _ := x.Where("key_id = ? AND repo_id = ?", keyID, repoID).Get(new(DeployKey))
 	return has
 }
 
@@ -808,11 +806,11 @@ func DeleteDeployKey(doer *User, id int64) error {
 	}
 
 	if _, err = sess.Id(key.ID).Delete(new(DeployKey)); err != nil {
-		return fmt.Errorf("delete deploy key[%d]: %v", key.ID, err)
+		return fmt.Errorf("delete deploy key [%d]: %v", key.ID, err)
 	}
 
 	// Check if this is the last reference to same key content.
-	has, err := sess.Where("key_id=?", key.KeyID).Get(new(DeployKey))
+	has, err := sess.Where("key_id = ?", key.KeyID).Get(new(DeployKey))
 	if err != nil {
 		return err
 	} else if !has {
@@ -827,5 +825,5 @@ func DeleteDeployKey(doer *User, id int64) error {
 // ListDeployKeys returns all deploy keys by given repository ID.
 func ListDeployKeys(repoID int64) ([]*DeployKey, error) {
 	keys := make([]*DeployKey, 0, 5)
-	return keys, x.Where("repo_id=?", repoID).Find(&keys)
+	return keys, x.Where("repo_id = ?", repoID).Find(&keys)
 }