Redirect on changed user and org name (#11649)

* Add redirect for user

* Add redirect for orgs

* Add user redirect test

* Appease linter

* Add comment to DeleteUserRedirect function

* Fix locale changes

* Fix GetUserByParams

* Fix orgAssignment

* Remove debug logging

* Add redirect prompt

* Dont Export DeleteUserRedirect & only use it within a session

* Unexport newUserRedirect

* cleanup

* Fix & Dedub API code

* Format Template

* Add Migration & rm dublicat

* Refactor: unexport newRepoRedirect() & rm dedub del exec

* if this fails we'll need to re-rename the user directory

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

12 files changed, 201 insertions, 18 deletions

diff --git a/models/error.go b/models/error.go
index 7f1eda1b14..fc161ed806 100644
--- a/models/error.go
+++ b/models/error.go
@@ -146,6 +146,21 @@ func (err ErrUserNotExist) Error() string {
 	return fmt.Sprintf("user does not exist [uid: %d, name: %s, keyid: %d]", err.UID, err.Name, err.KeyID)
 }
 
+// ErrUserRedirectNotExist represents a "UserRedirectNotExist" kind of error.
+type ErrUserRedirectNotExist struct {
+	Name string
+}
+
+// IsErrUserRedirectNotExist check if an error is an ErrUserRedirectNotExist.
+func IsErrUserRedirectNotExist(err error) bool {
+	_, ok := err.(ErrUserRedirectNotExist)
+	return ok
+}
+
+func (err ErrUserRedirectNotExist) Error() string {
+	return fmt.Sprintf("user redirect does not exist [name: %s]", err.Name)
+}
+
 // ErrUserProhibitLogin represents a "ErrUserProhibitLogin" kind of error.
 type ErrUserProhibitLogin struct {
 	UID  int64
diff --git a/models/fixtures/user_redirect.yml b/models/fixtures/user_redirect.yml
new file mode 100644
index 0000000000..8ff7993398
--- /dev/null
+++ b/models/fixtures/user_redirect.yml
@@ -0,0 +1,4 @@
+-
+  id: 1
+  lower_name: olduser1
+  redirect_user_id: 1
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index f62bba2a71..3227f6f754 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -279,6 +279,8 @@ var migrations = []Migration{
 	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),
+	// v167 -> v168
+	NewMigration("Add user redirect", addUserRedirect),
 }
 
 // GetCurrentDBVersion returns the current db version
diff --git a/models/migrations/v167.go b/models/migrations/v167.go
new file mode 100644
index 0000000000..fd91f226ab
--- /dev/null
+++ b/models/migrations/v167.go
@@ -0,0 +1,24 @@
+// 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 (
+	"fmt"
+
+	"xorm.io/xorm"
+)
+
+func addUserRedirect(x *xorm.Engine) (err error) {
+	type UserRedirect struct {
+		ID             int64  `xorm:"pk autoincr"`
+		LowerName      string `xorm:"UNIQUE(s) INDEX NOT NULL"`
+		RedirectUserID int64
+	}
+
+	if err := x.Sync2(new(UserRedirect)); err != nil {
+		return fmt.Errorf("Sync2: %v", err)
+	}
+	return nil
+}
diff --git a/models/models.go b/models/models.go
index 2ace1ea6dd..1bfe75b7fc 100644
--- a/models/models.go
+++ b/models/models.go
@@ -128,6 +128,7 @@ func init() {
 		new(Task),
 		new(LanguageStat),
 		new(EmailHash),
+		new(UserRedirect),
 		new(Project),
 		new(ProjectBoard),
 		new(ProjectIssue),
diff --git a/models/org.go b/models/org.go
index f45c9af7a7..ee867eec88 100644
--- a/models/org.go
+++ b/models/org.go
@@ -171,6 +171,10 @@ func CreateOrganization(org, owner *User) (err error) {
 		return err
 	}
 
+	if err = deleteUserRedirect(sess, org.Name); err != nil {
+		return err
+	}
+
 	if _, err = sess.Insert(org); err != nil {
 		return fmt.Errorf("insert organization: %v", err)
 	}
diff --git a/models/repo.go b/models/repo.go
index b11671e1fc..62d64fbee9 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -1312,8 +1312,8 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
 		return fmt.Errorf("delete repo redirect: %v", err)
 	}
 
-	if err := NewRepoRedirect(DBContext{sess}, oldOwner.ID, repo.ID, repo.Name, repo.Name); err != nil {
-		return fmt.Errorf("NewRepoRedirect: %v", err)
+	if err := newRepoRedirect(sess, oldOwner.ID, repo.ID, repo.Name, repo.Name); err != nil {
+		return fmt.Errorf("newRepoRedirect: %v", err)
 	}
 
 	return sess.Commit()
@@ -1361,12 +1361,7 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err
 		return fmt.Errorf("sess.Begin: %v", err)
 	}
 
-	// If there was previously a redirect at this location, remove it.
-	if err = deleteRepoRedirect(sess, repo.OwnerID, newRepoName); err != nil {
-		return fmt.Errorf("delete repo redirect: %v", err)
-	}
-
-	if err := NewRepoRedirect(DBContext{sess}, repo.Owner.ID, repo.ID, oldRepoName, newRepoName); err != nil {
+	if err := newRepoRedirect(sess, repo.Owner.ID, repo.ID, oldRepoName, newRepoName); err != nil {
 		return err
 	}
 
diff --git a/models/repo_redirect.go b/models/repo_redirect.go
index 182b6b41a2..afbfeb499e 100644
--- a/models/repo_redirect.go
+++ b/models/repo_redirect.go
@@ -28,16 +28,16 @@ func LookupRepoRedirect(ownerID int64, repoName string) (int64, error) {
 	return redirect.RedirectRepoID, nil
 }
 
-// NewRepoRedirect create a new repo redirect
-func NewRepoRedirect(ctx DBContext, ownerID, repoID int64, oldRepoName, newRepoName string) error {
+// newRepoRedirect create a new repo redirect
+func newRepoRedirect(e Engine, ownerID, repoID int64, oldRepoName, newRepoName string) error {
 	oldRepoName = strings.ToLower(oldRepoName)
 	newRepoName = strings.ToLower(newRepoName)
 
-	if err := deleteRepoRedirect(ctx.e, ownerID, newRepoName); err != nil {
+	if err := deleteRepoRedirect(e, ownerID, newRepoName); err != nil {
 		return err
 	}
 
-	if _, err := ctx.e.Insert(&RepoRedirect{
+	if _, err := e.Insert(&RepoRedirect{
 		OwnerID:        ownerID,
 		LowerName:      oldRepoName,
 		RedirectRepoID: repoID,
diff --git a/models/repo_redirect_test.go b/models/repo_redirect_test.go
index 44ec2b4e94..4c3184a0fd 100644
--- a/models/repo_redirect_test.go
+++ b/models/repo_redirect_test.go
@@ -26,7 +26,7 @@ func TestNewRepoRedirect(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
 	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
-	assert.NoError(t, NewRepoRedirect(DefaultDBContext(), repo.OwnerID, repo.ID, repo.Name, "newreponame"))
+	assert.NoError(t, newRepoRedirect(x, repo.OwnerID, repo.ID, repo.Name, "newreponame"))
 
 	AssertExistsAndLoadBean(t, &RepoRedirect{
 		OwnerID:        repo.OwnerID,
@@ -45,7 +45,7 @@ func TestNewRepoRedirect2(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
 	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
-	assert.NoError(t, NewRepoRedirect(DefaultDBContext(), repo.OwnerID, repo.ID, repo.Name, "oldrepo1"))
+	assert.NoError(t, newRepoRedirect(x, repo.OwnerID, repo.ID, repo.Name, "oldrepo1"))
 
 	AssertExistsAndLoadBean(t, &RepoRedirect{
 		OwnerID:        repo.OwnerID,
@@ -64,7 +64,7 @@ func TestNewRepoRedirect3(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
 	repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
-	assert.NoError(t, NewRepoRedirect(DefaultDBContext(), repo.OwnerID, repo.ID, repo.Name, "newreponame"))
+	assert.NoError(t, newRepoRedirect(x, repo.OwnerID, repo.ID, repo.Name, "newreponame"))
 
 	AssertExistsAndLoadBean(t, &RepoRedirect{
 		OwnerID:        repo.OwnerID,
diff --git a/models/user.go b/models/user.go
index 746608aaa4..8147c9f626 100644
--- a/models/user.go
+++ b/models/user.go
@@ -863,6 +863,10 @@ func CreateUser(u *User) (err error) {
 		return ErrUserAlreadyExist{u.Name}
 	}
 
+	if err = deleteUserRedirect(sess, u.Name); err != nil {
+		return err
+	}
+
 	u.Email = strings.ToLower(u.Email)
 	isExist, err = sess.
 		Where("email=?", u.Email).
@@ -973,6 +977,7 @@ func VerifyActiveEmailCode(code, email string) *EmailAddress {
 
 // ChangeUserName changes all corresponding setting from old user name to new one.
 func ChangeUserName(u *User, newUserName string) (err error) {
+	oldUserName := u.Name
 	if err = IsUsableUsername(newUserName); err != nil {
 		return err
 	}
@@ -990,16 +995,28 @@ func ChangeUserName(u *User, newUserName string) (err error) {
 		return ErrUserAlreadyExist{newUserName}
 	}
 
-	if _, err = sess.Exec("UPDATE `repository` SET owner_name=? WHERE owner_name=?", newUserName, u.Name); err != nil {
+	if _, err = sess.Exec("UPDATE `repository` SET owner_name=? WHERE owner_name=?", newUserName, oldUserName); err != nil {
 		return fmt.Errorf("Change repo owner name: %v", err)
 	}
 
 	// Do not fail if directory does not exist
-	if err = os.Rename(UserPath(u.Name), UserPath(newUserName)); err != nil && !os.IsNotExist(err) {
+	if err = os.Rename(UserPath(oldUserName), UserPath(newUserName)); err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("Rename user directory: %v", err)
 	}
 
-	return sess.Commit()
+	if err = newUserRedirect(sess, u.ID, oldUserName, newUserName); err != nil {
+		return err
+	}
+
+	if err = sess.Commit(); err != nil {
+		if err2 := os.Rename(UserPath(newUserName), UserPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) {
+			log.Critical("Unable to rollback directory change during failed username change from: %s to: %s. DB Error: %v. Filesystem Error: %v", oldUserName, newUserName, err, err2)
+			return fmt.Errorf("failed to rollback directory change during failed username change from: %s to: %s. DB Error: %w. Filesystem Error: %v", oldUserName, newUserName, err, err2)
+		}
+		return err
+	}
+
+	return nil
 }
 
 // checkDupEmail checks whether there are the same email with the user
diff --git a/models/user_redirect.go b/models/user_redirect.go
new file mode 100644
index 0000000000..1da8b44088
--- /dev/null
+++ b/models/user_redirect.go
@@ -0,0 +1,52 @@
+// Copyright 2020 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 models
+
+import "strings"
+
+// UserRedirect represents that a user name should be redirected to another
+type UserRedirect struct {
+	ID             int64  `xorm:"pk autoincr"`
+	LowerName      string `xorm:"UNIQUE(s) INDEX NOT NULL"`
+	RedirectUserID int64  // userID to redirect to
+}
+
+// LookupUserRedirect look up userID if a user has a redirect name
+func LookupUserRedirect(userName string) (int64, error) {
+	userName = strings.ToLower(userName)
+	redirect := &UserRedirect{LowerName: userName}
+	if has, err := x.Get(redirect); err != nil {
+		return 0, err
+	} else if !has {
+		return 0, ErrUserRedirectNotExist{Name: userName}
+	}
+	return redirect.RedirectUserID, nil
+}
+
+// newUserRedirect create a new user redirect
+func newUserRedirect(e Engine, ID int64, oldUserName, newUserName string) error {
+	oldUserName = strings.ToLower(oldUserName)
+	newUserName = strings.ToLower(newUserName)
+
+	if err := deleteUserRedirect(e, newUserName); err != nil {
+		return err
+	}
+
+	if _, err := e.Insert(&UserRedirect{
+		LowerName:      oldUserName,
+		RedirectUserID: ID,
+	}); err != nil {
+		return err
+	}
+	return nil
+}
+
+// deleteUserRedirect delete any redirect from the specified user name to
+// anything else
+func deleteUserRedirect(e Engine, userName string) error {
+	userName = strings.ToLower(userName)
+	_, err := e.Delete(&UserRedirect{LowerName: userName})
+	return err
+}
diff --git a/models/user_redirect_test.go b/models/user_redirect_test.go
new file mode 100644
index 0000000000..791c920bcf
--- /dev/null
+++ b/models/user_redirect_test.go
@@ -0,0 +1,69 @@
+// Copyright 2020 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 models
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLookupUserRedirect(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	userID, err := LookupUserRedirect("olduser1")
+	assert.NoError(t, err)
+	assert.EqualValues(t, 1, userID)
+
+	_, err = LookupUserRedirect("doesnotexist")
+	assert.True(t, IsErrUserRedirectNotExist(err))
+}
+
+func TestNewUserRedirect(t *testing.T) {
+	// redirect to a completely new name
+	assert.NoError(t, PrepareTestDatabase())
+
+	user := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
+	assert.NoError(t, newUserRedirect(x, user.ID, user.Name, "newusername"))
+
+	AssertExistsAndLoadBean(t, &UserRedirect{
+		LowerName:      user.LowerName,
+		RedirectUserID: user.ID,
+	})
+	AssertExistsAndLoadBean(t, &UserRedirect{
+		LowerName:      "olduser1",
+		RedirectUserID: user.ID,
+	})
+}
+
+func TestNewUserRedirect2(t *testing.T) {
+	// redirect to previously used name
+	assert.NoError(t, PrepareTestDatabase())
+
+	user := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
+	assert.NoError(t, newUserRedirect(x, user.ID, user.Name, "olduser1"))
+
+	AssertExistsAndLoadBean(t, &UserRedirect{
+		LowerName:      user.LowerName,
+		RedirectUserID: user.ID,
+	})
+	AssertNotExistsBean(t, &UserRedirect{
+		LowerName:      "olduser1",
+		RedirectUserID: user.ID,
+	})
+}
+
+func TestNewUserRedirect3(t *testing.T) {
+	// redirect for a previously-unredirected user
+	assert.NoError(t, PrepareTestDatabase())
+
+	user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	assert.NoError(t, newUserRedirect(x, user.ID, user.Name, "newusername"))
+
+	AssertExistsAndLoadBean(t, &UserRedirect{
+		LowerName:      user.LowerName,
+		RedirectUserID: user.ID,
+	})
+}