Merge pull request #964 from phsmit/access_rewriteserv

Rewrite/simplify gogs serve

cmd/serve.go

@@ -21,6 +21,10 @@ import (
 	"github.com/gogits/gogs/modules/uuid"
 )
 
+const (
+	ACCESS_DENIED_MESSAGE = "Repository does not exist or you do not have access"
+)
+
 var CmdServ = cli.Command{
 	Name:        "serv",
 	Usage:       "This command should only be called by SSH shell",
@@ -51,72 +55,58 @@ func setup(logPath string) {
 }
 
 func parseCmd(cmd string) (string, string) {
+
 	ss := strings.SplitN(cmd, " ", 2)
 	if len(ss) != 2 {
 		return "", ""
 	}
-
+	return ss[0], strings.Replace(ss[1], "'/", "'", 1)
-	verb, args := ss[0], ss[1]
-	if verb == "git" {
-		ss = strings.SplitN(args, " ", 2)
-		args = ss[1]
-		verb = fmt.Sprintf("%s %s", verb, ss[0])
-	}
-	return verb, strings.Replace(args, "'/", "'", 1)
 }
 
 var (
-	COMMANDS_READONLY = map[string]models.AccessMode{
+	COMMANDS = map[string]models.AccessMode{
-		"git-upload-pack":    models.ACCESS_MODE_WRITE,
+		"git-upload-pack":    models.ACCESS_MODE_READ,
-		"git upload-pack":    models.ACCESS_MODE_WRITE,
+		"git-upload-archive": models.ACCESS_MODE_READ,
-		"git-upload-archive": models.ACCESS_MODE_WRITE,
+		"git-receive-pack":   models.ACCESS_MODE_WRITE,
-	}
-
-	COMMANDS_WRITE = map[string]models.AccessMode{
-		"git-receive-pack": models.ACCESS_MODE_READ,
-		"git receive-pack": models.ACCESS_MODE_READ,
 	}
 )
 
-func In(b string, sl map[string]models.AccessMode) bool {
-	_, e := sl[b]
-	return e
-}
-
 func runServ(c *cli.Context) {
 	if c.IsSet("config") {
 		setting.CustomConf = c.String("config")
 	}
 	setup("serv.log")
 
+	fail := func(userMessage, logMessage string, args ...interface{}) {
+		fmt.Fprintln(os.Stderr, "Gogs: ", userMessage)
+		log.GitLogger.Fatal(2, logMessage, args...)
+	}
+
 	if len(c.Args()) < 1 {
-		log.GitLogger.Fatal(2, "Not enough arguments")
+		fail("Not enough arguments", "Not enough arugments")
 	}
+
 	keys := strings.Split(c.Args()[0], "-")
 	if len(keys) != 2 {
-		println("Gogs: auth file format error")
+		fail("key-id format error", "Invalid key id: %s", c.Args()[0])
-		log.GitLogger.Fatal(2, "Invalid auth file format: %s", os.Args[2])
 	}
 
 	keyId, err := com.StrTo(keys[1]).Int64()
 	if err != nil {
-		println("Gogs: auth file format error")
+		fail("key-id format error", "Invalid key id: %s", err)
-		log.GitLogger.Fatal(2, "Invalid auth file format: %v", err)
 	}
+
 	user, err := models.GetUserByKeyId(keyId)
 	if err != nil {
-		if err == models.ErrUserNotKeyOwner {
+		fail("internal error", "Fail to get user by key ID(%d): %v", keyId, err)
-			println("Gogs: you are not the owner of SSH key")
-			log.GitLogger.Fatal(2, "Invalid owner of SSH key: %d", keyId)
-		}
-		println("Gogs: internal error:", err.Error())
-		log.GitLogger.Fatal(2, "Fail to get user by key ID(%d): %v", keyId, err)
 	}
 
 	cmd := os.Getenv("SSH_ORIGINAL_COMMAND")
 	if cmd == "" {
 		println("Hi", user.Name, "! You've successfully authenticated, but Gogs does not provide shell access.")
-		println("If this is what you do not expect, please log in with password and setup Gogs under another user.")
+		if user.IsAdmin {
+			println("If this is unexpected, please log in with password and setup Gogs under another user.")
+		}
 		return
 	}
 
@@ -124,67 +114,47 @@ func runServ(c *cli.Context) {
 	repoPath := strings.Trim(args, "'")
 	rr := strings.SplitN(repoPath, "/", 2)
 	if len(rr) != 2 {
-		println("Gogs: unavailable repository", args)
+		fail("Invalid repository path", "Invalide repository path: %v", args)
-		log.GitLogger.Fatal(2, "Unavailable repository: %v", args)
 	}
 	repoUserName := rr[0]
 	repoName := strings.TrimSuffix(rr[1], ".git")
 
-	isWrite := In(verb, COMMANDS_WRITE)
-	isRead := In(verb, COMMANDS_READONLY)
-
 	repoUser, err := models.GetUserByName(repoUserName)
 	if err != nil {
 		if err == models.ErrUserNotExist {
-			println("Gogs: given repository owner are not registered")
+			fail("Repository owner does not exist", "Unregistered owner: %s", repoUserName)
-			log.GitLogger.Fatal(2, "Unregistered owner: %s", repoUserName)
 		}
-		println("Gogs: internal error:", err.Error())
+		fail("Internal error", "Fail to get repository owner(%s): %v", repoUserName, err)
-		log.GitLogger.Fatal(2, "Fail to get repository owner(%s): %v", repoUserName, err)
 	}
 
-	// Access check.
 	repo, err := models.GetRepositoryByName(repoUser.Id, repoName)
 	if err != nil {
 		if err == models.ErrRepoNotExist {
-			println("Gogs: given repository does not exist")
+			if user.Id == repoUser.Id || repoUser.IsOwnedBy(user.Id) {
-			log.GitLogger.Fatal(2, "Repository does not exist: %s/%s", repoUser.Name, repoName)
+				fail("Repository does not exist", "Repository does not exist: %s/%s", repoUser.Name, repoName)
+			} else {
+				fail(ACCESS_DENIED_MESSAGE, "Repository does not exist: %s/%s", repoUser.Name, repoName)
 			}
-		println("Gogs: internal error:", err.Error())
-		log.GitLogger.Fatal(2, "Fail to get repository: %v", err)
 		}
-
+		fail("Internal error", "Fail to get repository: %v", err)
-	switch {
-	case isWrite:
-		has, err := models.HasAccess(user, repo, models.ACCESS_MODE_WRITE)
-		if err != nil {
-			println("Gogs: internal error:", err.Error())
-			log.GitLogger.Fatal(2, "Fail to check write access:", err)
-		} else if !has {
-			println("You have no right to write this repository")
-			log.GitLogger.Fatal(2, "User %s has no right to write repository %s", user.Name, repoPath)
 	}
 
-		if repo.IsMirror {
+	requestedMode, has := COMMANDS[verb]
-			println("You can't write to a mirror repository")
+	if !has {
-			log.GitLogger.Fatal(2, "User %s tried to write to a mirror repository %s", user.Name, repoPath)
+		fail("Unknown git command", "Unknown git command %s", verb)
-		}
-	case isRead:
-		if !repo.IsPrivate {
-			break
 	}
 
-		has, err := models.HasAccess(user, repo, models.ACCESS_MODE_READ)
+	mode, err := models.AccessLevel(user, repo)
 	if err != nil {
-			println("Gogs: internal error:", err.Error())
+		fail("Internal error", "HasAccess fail: %v", err)
-			log.GitLogger.Fatal(2, "Fail to check read access:", err)
+	} else if mode < requestedMode {
-		} else if !has {
+		clientMessage := ACCESS_DENIED_MESSAGE
-			println("You have no right to access this repository")
+		if mode >= models.ACCESS_MODE_READ {
-			log.GitLogger.Fatal(2, "User %s has no right to read repository %s", user.Name, repoPath)
+			clientMessage = "You do not have sufficient authorization for this action"
-		}
+		}
-	default:
+		fail(clientMessage,
-		println("Unknown command: " + cmd)
+			"User %s does not have level %v access to repository %s",
-		return
+			user.Name, requestedMode, repoPath)
 	}
 
 	uuid := uuid.NewV4().String()
@@ -202,11 +172,10 @@ func runServ(c *cli.Context) {
 	gitcmd.Stdin = os.Stdin
 	gitcmd.Stderr = os.Stderr
 	if err = gitcmd.Run(); err != nil {
-		println("Gogs: internal error:", err.Error())
+		fail("Internal error", "Fail to execute git command: %v", err)
-		log.GitLogger.Fatal(2, "Fail to execute git command: %v", err)
 	}
 
-	if isWrite {
+	if requestedMode == models.ACCESS_MODE_WRITE {
 		tasks, err := models.GetUpdateTasksByUuid(uuid)
 		if err != nil {
 			log.GitLogger.Fatal(2, "GetUpdateTasksByUuid: %v", err)
@@ -228,10 +197,10 @@ func runServ(c *cli.Context) {
 	// Update key activity.
 	key, err := models.GetPublicKeyById(keyId)
 	if err != nil {
-		log.GitLogger.Fatal(2, "GetPublicKeyById: %v", err)
+		fail("Internal error", "GetPublicKeyById: %v", err)
 	}
 	key.Updated = time.Now()
 	if err = models.UpdatePublicKey(key); err != nil {
-		log.GitLogger.Fatal(2, "UpdatePublicKey: %v", err)
+		fail("Internal error", "UpdatePublicKey: %v", err)
 	}
 }

models/user.go

@@ -40,7 +40,7 @@ var (
 	ErrUserHasOrgs           = errors.New("User still have membership of organization")
 	ErrUserAlreadyExist      = errors.New("User already exist")
 	ErrUserNotExist          = errors.New("User does not exist")
-	ErrUserNotKeyOwner       = errors.New("User does not the owner of public key")
+	ErrPublicKeyNotExist     = errors.New("Public key does not exist")
 	ErrEmailAlreadyUsed      = errors.New("E-mail already used")
 	ErrEmailNotExist         = errors.New("E-mail does not exist")
 	ErrEmailNotActivated     = errors.New("E-mail address has not been activated")
@@ -518,7 +518,7 @@ func GetUserByKeyId(keyId int64) (*User, error) {
 	if err != nil {
 		return nil, err
 	} else if !has {
-		return nil, ErrUserNotKeyOwner
+		return nil, ErrPublicKeyNotExist
 	}
 	return user, nil
 }