Merge branch 'master' into issue_13171

Author: eneuschild

MAINTAINERS

@@ -36,7 +36,6 @@ Mura Li <[email protected]> (@typeless)
 6543 <[email protected]> (@6543)
 jaqra <[email protected]> (@jaqra)
 David Svantesson <[email protected]> (@davidsvantesson)
-CirnoT <[email protected]> (@CirnoT)
 a1012112796 <[email protected]> (@a1012112796)
 Karl Heinz Marbaise <[email protected]> (@khmarbaise)
 Norwin Roosen <[email protected]> (@noerw)

cmd/admin.go

@@ -349,12 +349,11 @@ func runChangePassword(c *cli.Context) error {
 	if err != nil {
 		return err
 	}
-	if user.Salt, err = models.GetUserSalt(); err != nil {
+	if err = user.SetPassword(c.String("password")); err != nil {
 		return err
 	}
-	user.HashPassword(c.String("password"))
 
-	if err := models.UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
+	if err = models.UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
 		return err
 	}
 

integrations/admin_user_test.go

@@ -0,0 +1,82 @@
+// Copyright 2021 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 (
+	"net/http"
+	"strconv"
+	"testing"
+
+	"code.gitea.io/gitea/models"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAdminViewUsers(t *testing.T) {
+	prepareTestEnv(t)
+
+	session := loginUser(t, "user1")
+	req := NewRequest(t, "GET", "/admin/users")
+	session.MakeRequest(t, req, http.StatusOK)
+
+	session = loginUser(t, "user2")
+	req = NewRequest(t, "GET", "/admin/users")
+	session.MakeRequest(t, req, http.StatusForbidden)
+}
+
+func TestAdminViewUser(t *testing.T) {
+	prepareTestEnv(t)
+
+	session := loginUser(t, "user1")
+	req := NewRequest(t, "GET", "/admin/users/1")
+	session.MakeRequest(t, req, http.StatusOK)
+
+	session = loginUser(t, "user2")
+	req = NewRequest(t, "GET", "/admin/users/1")
+	session.MakeRequest(t, req, http.StatusForbidden)
+}
+
+func TestAdminEditUser(t *testing.T) {
+	prepareTestEnv(t)
+
+	testSuccessfullEdit(t, models.User{ID: 2, Name: "newusername", LoginName: "otherlogin", Email: "[email protected]"})
+}
+
+func testSuccessfullEdit(t *testing.T, formData models.User) {
+	makeRequest(t, formData, http.StatusFound)
+}
+
+func makeRequest(t *testing.T, formData models.User, headerCode int) {
+	session := loginUser(t, "user1")
+	csrf := GetCSRF(t, session, "/admin/users/"+strconv.Itoa(int(formData.ID)))
+	req := NewRequestWithValues(t, "POST", "/admin/users/"+strconv.Itoa(int(formData.ID)), map[string]string{
+		"_csrf":      csrf,
+		"user_name":  formData.Name,
+		"login_name": formData.LoginName,
+		"login_type": "0-0",
+		"email":      formData.Email,
+	})
+
+	session.MakeRequest(t, req, headerCode)
+	user := models.AssertExistsAndLoadBean(t, &models.User{ID: formData.ID}).(*models.User)
+	assert.Equal(t, formData.Name, user.Name)
+	assert.Equal(t, formData.LoginName, user.LoginName)
+	assert.Equal(t, formData.Email, user.Email)
+}
+
+func TestAdminDeleteUser(t *testing.T) {
+	defer prepareTestEnv(t)()
+
+	session := loginUser(t, "user1")
+
+	csrf := GetCSRF(t, session, "/admin/users/8")
+	req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{
+		"_csrf": csrf,
+	})
+	session.MakeRequest(t, req, http.StatusOK)
+
+	assertUserDeleted(t, 8)
+	models.CheckConsistencyFor(t, &models.User{})
+}

integrations/delete_user_test.go

@@ -24,21 +24,6 @@ func assertUserDeleted(t *testing.T, userID int64) {
 	models.AssertNotExistsBean(t, &models.Star{UID: userID})
 }
 
-func TestAdminDeleteUser(t *testing.T) {
-	defer prepareTestEnv(t)()
-
-	session := loginUser(t, "user1")
-
-	csrf := GetCSRF(t, session, "/admin/users/8")
-	req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{
-		"_csrf": csrf,
-	})
-	session.MakeRequest(t, req, http.StatusOK)
-
-	assertUserDeleted(t, 8)
-	models.CheckConsistencyFor(t, &models.User{})
-}
-
 func TestUserDeleteAccount(t *testing.T) {
 	defer prepareTestEnv(t)()
 

models/issue_comment.go

@@ -979,7 +979,7 @@ func (opts *FindCommentsOptions) toConds() builder.Cond {
 	if opts.Type != CommentTypeUnknown {
 		cond = cond.And(builder.Eq{"comment.type": opts.Type})
 	}
-	if opts.Line > 0 {
+	if opts.Line != 0 {
 		cond = cond.And(builder.Eq{"comment.line": opts.Line})
 	}
 	if len(opts.TreePath) > 0 {
@@ -1078,18 +1078,35 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
 	if review == nil {
 		review = &Review{ID: 0}
 	}
-	//Find comments
 	opts := FindCommentsOptions{
 		Type:     CommentTypeCode,
 		IssueID:  issue.ID,
 		ReviewID: review.ID,
 	}
+
+	comments, err := findCodeComments(e, opts, issue, currentUser, review)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, comment := range comments {
+		if pathToLineToComment[comment.TreePath] == nil {
+			pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment)
+		}
+		pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment)
+	}
+	return pathToLineToComment, nil
+}
+
+func findCodeComments(e Engine, opts FindCommentsOptions, issue *Issue, currentUser *User, review *Review) ([]*Comment, error) {
+	var comments []*Comment
+	if review == nil {
+		review = &Review{ID: 0}
+	}
 	conds := opts.toConds()
 	if review.ID == 0 {
 		conds = conds.And(builder.Eq{"invalidated": false})
 	}
-
-	var comments []*Comment
 	if err := e.Where(conds).
 		Asc("comment.created_unix").
 		Asc("comment.id").
@@ -1117,7 +1134,19 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
 		return nil, err
 	}
 
+	n := 0
 	for _, comment := range comments {
+		if re, ok := reviews[comment.ReviewID]; ok && re != nil {
+			// If the review is pending only the author can see the comments (except if the review is set)
+			if review.ID == 0 && re.Type == ReviewTypePending &&
+				(currentUser == nil || currentUser.ID != re.ReviewerID) {
+				continue
+			}
+			comment.Review = re
+		}
+		comments[n] = comment
+		n++
+
 		if err := comment.LoadResolveDoer(); err != nil {
 			return nil, err
 		}
@@ -1126,25 +1155,21 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
 			return nil, err
 		}
 
-		if re, ok := reviews[comment.ReviewID]; ok && re != nil {
-			// If the review is pending only the author can see the comments (except the review is set)
-			if review.ID == 0 {
-				if re.Type == ReviewTypePending &&
-					(currentUser == nil || currentUser.ID != re.ReviewerID) {
-					continue
-				}
-			}
-			comment.Review = re
-		}
-
 		comment.RenderedContent = string(markdown.Render([]byte(comment.Content), issue.Repo.Link(),
 			issue.Repo.ComposeMetas()))
-		if pathToLineToComment[comment.TreePath] == nil {
-			pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment)
-		}
-		pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment)
 	}
-	return pathToLineToComment, nil
+	return comments[:n], nil
+}
+
+// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number
+func FetchCodeCommentsByLine(issue *Issue, currentUser *User, treePath string, line int64) ([]*Comment, error) {
+	opts := FindCommentsOptions{
+		Type:     CommentTypeCode,
+		IssueID:  issue.ID,
+		TreePath: treePath,
+		Line:     line,
+	}
+	return findCodeComments(x, opts, issue, currentUser, nil)
 }
 
 // FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line

models/login_source.go

@@ -771,8 +771,10 @@ func UserSignIn(username, password string) (*User, error) {
 
 				// Update password hash if server password hash algorithm have changed
 				if user.PasswdHashAlgo != setting.PasswordHashAlgo {
-					user.HashPassword(password)
-					if err := UpdateUserCols(user, "passwd", "passwd_hash_algo"); err != nil {
+					if err = user.SetPassword(password); err != nil {
+						return nil, err
+					}
+					if err = UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
 						return nil, err
 					}
 				}

models/migrations/migrations.go

@@ -277,6 +277,8 @@ var migrations = []Migration{
 	NewMigration("Add scope and nonce columns to oauth2_grant table", addScopeAndNonceColumnsToOAuth2Grant),
 	// v165 -> v166
 	NewMigration("Convert hook task type from char(16) to varchar(16) and trim the column", convertHookTaskTypeToVarcharAndTrim),
+	// v166 -> v167
+	NewMigration("Where Password is Valid with Empty String delete it", recalculateUserEmptyPWD),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v166.go

@@ -0,0 +1,115 @@
+// Copyright 2021 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 migrations
+
+import (
+	"crypto/sha256"
+	"fmt"
+
+	"golang.org/x/crypto/argon2"
+	"golang.org/x/crypto/bcrypt"
+	"golang.org/x/crypto/pbkdf2"
+	"golang.org/x/crypto/scrypt"
+	"xorm.io/builder"
+	"xorm.io/xorm"
+)
+
+func recalculateUserEmptyPWD(x *xorm.Engine) (err error) {
+	const (
+		algoBcrypt = "bcrypt"
+		algoScrypt = "scrypt"
+		algoArgon2 = "argon2"
+		algoPbkdf2 = "pbkdf2"
+	)
+
+	type User struct {
+		ID                 int64  `xorm:"pk autoincr"`
+		Passwd             string `xorm:"NOT NULL"`
+		PasswdHashAlgo     string `xorm:"NOT NULL DEFAULT 'argon2'"`
+		MustChangePassword bool   `xorm:"NOT NULL DEFAULT false"`
+		LoginType          int
+		LoginName          string
+		Type               int
+		Salt               string `xorm:"VARCHAR(10)"`
+	}
+
+	// hashPassword hash password based on algo and salt
+	// state 461406070c
+	hashPassword := func(passwd, salt, algo string) string {
+		var tempPasswd []byte
+
+		switch algo {
+		case algoBcrypt:
+			tempPasswd, _ = bcrypt.GenerateFromPassword([]byte(passwd), bcrypt.DefaultCost)
+			return string(tempPasswd)
+		case algoScrypt:
+			tempPasswd, _ = scrypt.Key([]byte(passwd), []byte(salt), 65536, 16, 2, 50)
+		case algoArgon2:
+			tempPasswd = argon2.IDKey([]byte(passwd), []byte(salt), 2, 65536, 8, 50)
+		case algoPbkdf2:
+			fallthrough
+		default:
+			tempPasswd = pbkdf2.Key([]byte(passwd), []byte(salt), 10000, 50, sha256.New)
+		}
+
+		return fmt.Sprintf("%x", tempPasswd)
+	}
+
+	// ValidatePassword checks if given password matches the one belongs to the user.
+	// state 461406070c, changed since it's not necessary to be time constant
+	ValidatePassword := func(u *User, passwd string) bool {
+		tempHash := hashPassword(passwd, u.Salt, u.PasswdHashAlgo)
+
+		if u.PasswdHashAlgo != algoBcrypt && u.Passwd == tempHash {
+			return true
+		}
+		if u.PasswdHashAlgo == algoBcrypt && bcrypt.CompareHashAndPassword([]byte(u.Passwd), []byte(passwd)) == nil {
+			return true
+		}
+		return false
+	}
+
+	sess := x.NewSession()
+	defer sess.Close()
+
+	const batchSize = 100
+
+	for start := 0; ; start += batchSize {
+		users := make([]*User, 0, batchSize)
+		if err = sess.Limit(batchSize, start).Where(builder.Neq{"passwd": ""}, 0).Find(&users); err != nil {
+			return
+		}
+		if len(users) == 0 {
+			break
+		}
+
+		if err = sess.Begin(); err != nil {
+			return
+		}
+
+		for _, user := range users {
+			if ValidatePassword(user, "") {
+				user.Passwd = ""
+				user.Salt = ""
+				user.PasswdHashAlgo = ""
+				if _, err = sess.ID(user.ID).Cols("passwd", "salt", "passwd_hash_algo").Update(user); err != nil {
+					return err
+				}
+			}
+		}
+
+		if err = sess.Commit(); err != nil {
+			return
+		}
+	}
+
+	// delete salt and algo where password is empty
+	if _, err = sess.Where(builder.Eq{"passwd": ""}.And(builder.Neq{"salt": ""}.Or(builder.Neq{"passwd_hash_algo": ""}))).
+		Cols("salt", "passwd_hash_algo").Update(&User{}); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}