From aa68e86206d7d9fc74de8630acfa07dfd7f1bc95 Mon Sep 17 00:00:00 2001 From: Peter Smit Date: Mon, 16 Feb 2015 16:38:01 +0200 Subject: [PATCH 1/2] Rewrite/simplify gogs serve --- cmd/serve.go | 128 ++++++++++++++++++++----------------------------- models/user.go | 4 +- 2 files changed, 53 insertions(+), 79 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index e8e5c186c..20c9b4587 100644 --- a/cmd/serve.go +++ b/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 "", "" } - - 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) + return ss[0], strings.Replace(ss[1], "'/", "'", 1) } var ( - COMMANDS_READONLY = map[string]models.AccessMode{ - "git-upload-pack": models.ACCESS_MODE_WRITE, - "git upload-pack": models.ACCESS_MODE_WRITE, - "git-upload-archive": models.ACCESS_MODE_WRITE, - } - - COMMANDS_WRITE = map[string]models.AccessMode{ - "git-receive-pack": models.ACCESS_MODE_READ, - "git receive-pack": models.ACCESS_MODE_READ, + COMMANDS = map[string]models.AccessMode{ + "git-upload-pack": models.ACCESS_MODE_READ, + "git-upload-archive": models.ACCESS_MODE_READ, + "git-receive-pack": models.ACCESS_MODE_WRITE, } ) -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") - log.GitLogger.Fatal(2, "Invalid auth file format: %s", os.Args[2]) + fail("key-id format error", "Invalid key id: %s", c.Args()[0]) } keyId, err := com.StrTo(keys[1]).Int64() if err != nil { - println("Gogs: auth file format error") - log.GitLogger.Fatal(2, "Invalid auth file format: %v", err) + fail("key-id format error", "Invalid key id: %s", err) } + user, err := models.GetUserByKeyId(keyId) if err != nil { - if err == models.ErrUserNotKeyOwner { - 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) + fail("internal error", "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,62 +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) - log.GitLogger.Fatal(2, "Unavailable repository: %v", args) + fail("Invalid repository path", "Invalide repository path: %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") - log.GitLogger.Fatal(2, "Unregistered owner: %s", repoUserName) + fail("Repository owner does not exist", "Unregistered owner: %s", repoUserName) } - println("Gogs: internal error:", err.Error()) - log.GitLogger.Fatal(2, "Fail to get repository owner(%s): %v", repoUserName, err) + fail("Internal error", "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") - log.GitLogger.Fatal(2, "Repository does not exist: %s/%s", repoUser.Name, repoName) + if user.Id == repoUser.Id || repoUser.IsOwnedBy(user.Id) { + 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) - } - case isRead: - if !repo.IsPrivate { - break - } + requestedMode, has := COMMANDS[verb] + if !has { + fail("Unknown git command", "Unknown git command %s", verb) + } - has, err := models.HasAccess(user, repo, models.ACCESS_MODE_READ) - if err != nil { - println("Gogs: internal error:", err.Error()) - log.GitLogger.Fatal(2, "Fail to check read access:", err) - } else if !has { - println("You have no right to access this repository") - log.GitLogger.Fatal(2, "User %s has no right to read repository %s", user.Name, repoPath) + mode, err := models.AccessLevel(user, repo) + if err != nil { + fail("Internal error", "HasAccess fail: %v", err) + } else if mode < requestedMode { + clientMessage := ACCESS_DENIED_MESSAGE + if mode >= models.ACCESS_MODE_READ { + clientMessage = "You do not have sufficient authorization for this action" } - default: - println("Unknown command: " + cmd) - return + fail(clientMessage, + "User %s does not have level %v access to repository %s", + user.Name, requestedMode, repoPath) } uuid := uuid.NewV4().String() @@ -197,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()) - log.GitLogger.Fatal(2, "Fail to execute git command: %v", err) + fail("Internal error", "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) @@ -223,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) } } diff --git a/models/user.go b/models/user.go index 37e77dd3b..3c439fff0 100644 --- a/models/user.go +++ b/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") @@ -516,7 +516,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 } From d446be9f5f1e4584c329dc418a9699960cb431e1 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Sat, 28 Feb 2015 22:24:53 -0500 Subject: [PATCH 2/2] models: mirror fix on #964 --- cmd/serve.go | 11 ++++++----- conf/locale/locale_en-US.ini | 2 +- models/publickey.go | 1 - models/user.go | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 20c9b4587..ded4392a0 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -22,7 +22,7 @@ import ( ) const ( - ACCESS_DENIED_MESSAGE = "Repository does not exist or you do not have access" + _ACCESS_DENIED_MESSAGE = "Repository does not exist or you do not have access" ) var CmdServ = cli.Command{ @@ -55,7 +55,6 @@ func setup(logPath string) { } func parseCmd(cmd string) (string, string) { - ss := strings.SplitN(cmd, " ", 2) if len(ss) != 2 { return "", "" @@ -66,8 +65,10 @@ func parseCmd(cmd string) (string, string) { var ( COMMANDS = map[string]models.AccessMode{ "git-upload-pack": models.ACCESS_MODE_READ, + "git upload-pack": models.ACCESS_MODE_READ, "git-upload-archive": models.ACCESS_MODE_READ, "git-receive-pack": models.ACCESS_MODE_WRITE, + "git receive-pack": models.ACCESS_MODE_WRITE, } ) @@ -133,7 +134,7 @@ func runServ(c *cli.Context) { if user.Id == repoUser.Id || repoUser.IsOwnedBy(user.Id) { 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) + fail(_ACCESS_DENIED_MESSAGE, "Repository does not exist: %s/%s", repoUser.Name, repoName) } } fail("Internal error", "Fail to get repository: %v", err) @@ -146,9 +147,9 @@ func runServ(c *cli.Context) { mode, err := models.AccessLevel(user, repo) if err != nil { - fail("Internal error", "HasAccess fail: %v", err) + fail("Internal error", "Fail to check access: %v", err) } else if mode < requestedMode { - clientMessage := ACCESS_DENIED_MESSAGE + clientMessage := _ACCESS_DENIED_MESSAGE if mode >= models.ACCESS_MODE_READ { clientMessage = "You do not have sufficient authorization for this action" } diff --git a/conf/locale/locale_en-US.ini b/conf/locale/locale_en-US.ini index ca076b8bb..09a5b8e88 100644 --- a/conf/locale/locale_en-US.ini +++ b/conf/locale/locale_en-US.ini @@ -149,7 +149,7 @@ repo_name_been_taken = Repository name has been already taken. org_name_been_taken = Organization name has been already taken. team_name_been_taken = Team name has been already taken. email_been_used = E-mail address has been already used. -ssh_key_been_used = Public key name has been used. +ssh_key_been_used = Public key name or content has been used. illegal_username = Your username contains illegal characters. illegal_repo_name = Repository name contains illegal characters. illegal_org_name = Organization name contains illegal characters. diff --git a/models/publickey.go b/models/publickey.go index 383b85b63..6bec1139b 100644 --- a/models/publickey.go +++ b/models/publickey.go @@ -130,7 +130,6 @@ func extractTypeFromBase64Key(key string) (string, error) { // Parse 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) diff --git a/models/user.go b/models/user.go index f5bec2e83..ea5041bdf 100644 --- a/models/user.go +++ b/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") - ErrPublicKeyNotExist = errors.New("Public key does not exist") + ErrUserNotKeyOwner = errors.New("User does not the owner of public key") 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, ErrPublicKeyNotExist + return nil, ErrUserNotKeyOwner } return user, nil }