Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect on changed user and org name #11649

Merged
merged 26 commits into from
Jan 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6772e89
Add redirect for user
AndrewBezold May 28, 2020
1524fef
Add redirect for orgs
AndrewBezold May 28, 2020
b4ecccf
Add user redirect test
AndrewBezold May 28, 2020
4156980
Appease linter
AndrewBezold May 28, 2020
9112703
Add comment to DeleteUserRedirect function
AndrewBezold May 28, 2020
aecbb0d
Fix locale changes
AndrewBezold May 28, 2020
b4eea72
Fix GetUserByParams
AndrewBezold May 28, 2020
14fa02b
Fix orgAssignment
AndrewBezold May 28, 2020
387ddec
Remove debug logging
AndrewBezold Jun 9, 2020
0167007
Add redirect prompt
AndrewBezold Jun 10, 2020
cd4c6eb
Merge branch 'master' into Redirect-user-and-org-#9531
6543 Jan 23, 2021
b1c38d1
Merge branch 'master' into pr-11649
6543 Jan 23, 2021
b4ac0d6
Dont Export DeleteUserRedirect & only use it within a session
6543 Jan 23, 2021
988075b
Unexport newUserRedirect
6543 Jan 23, 2021
adb0bca
cleanup
6543 Jan 23, 2021
6909535
Fix & Dedub API code
6543 Jan 23, 2021
68c7c78
Format Template
6543 Jan 23, 2021
51f1194
Add Migration & rm dublicat
6543 Jan 23, 2021
10d3ec9
Refactor: unexport newRepoRedirect() & rm dedub del exec
6543 Jan 23, 2021
b47f66f
Merge branch 'master' into Redirect-user-and-org-#9531
6543 Jan 23, 2021
4c7f0f0
if this fails we'll need to re-rename the user directory
6543 Jan 23, 2021
7d3849f
fmt
6543 Jan 24, 2021
6d1b5fd
Merge branch 'master' into Redirect-user-and-org-#9531
6543 Jan 24, 2021
f11bb7e
Update models/user.go
6543 Jan 24, 2021
aaef772
Merge branch 'master' into Redirect-user-and-org-#9531
6543 Jan 24, 2021
df25bf1
Merge branch 'master' into Redirect-user-and-org-#9531
lunny Jan 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions models/fixtures/user_redirect.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-
id: 1
lower_name: olduser1
redirect_user_id: 1
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions models/migrations/v167.go
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ func init() {
new(Task),
new(LanguageStat),
new(EmailHash),
new(UserRedirect),
new(Project),
new(ProjectBoard),
new(ProjectIssue),
Expand Down
4 changes: 4 additions & 0 deletions models/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
11 changes: 3 additions & 8 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}

Expand Down
8 changes: 4 additions & 4 deletions models/repo_redirect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions models/repo_redirect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
23 changes: 20 additions & 3 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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
6543 marked this conversation as resolved.
Show resolved Hide resolved
if err = IsUsableUsername(newUserName); err != nil {
return err
}
Expand All @@ -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
Expand Down
52 changes: 52 additions & 0 deletions models/user_redirect.go
Original file line number Diff line number Diff line change
@@ -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
}
69 changes: 69 additions & 0 deletions models/user_redirect_test.go
Original file line number Diff line number Diff line change
@@ -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,
})
}
20 changes: 20 additions & 0 deletions modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,26 @@ func (ctx *Context) IsUserRepoReaderAny() bool {
return ctx.Repo.HasAccess()
}

// RedirectToUser redirect to a differently-named user
func RedirectToUser(ctx *Context, userName string, redirectUserID int64) {
user, err := models.GetUserByID(redirectUserID)
if err != nil {
ctx.ServerError("GetUserByID", err)
return
}

redirectPath := strings.Replace(
ctx.Req.URL.Path,
userName,
user.Name,
1,
)
if ctx.Req.URL.RawQuery != "" {
redirectPath += "?" + ctx.Req.URL.RawQuery
}
ctx.Redirect(path.Join(setting.AppSubURL, redirectPath))
}

// HasAPIError returns true if error occurs in form validation.
func (ctx *Context) HasAPIError() bool {
hasErr, ok := ctx.Data["HasError"]
Expand Down
9 changes: 8 additions & 1 deletion modules/context/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,14 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
ctx.Org.Organization, err = models.GetUserByName(orgName)
if err != nil {
if models.IsErrUserNotExist(err) {
ctx.NotFound("GetUserByName", err)
redirectUserID, err := models.LookupUserRedirect(orgName)
if err == nil {
RedirectToUser(ctx, orgName, redirectUserID)
} else if models.IsErrUserRedirectNotExist(err) {
ctx.NotFound("GetUserByName", err)
} else {
ctx.ServerError("LookupUserRedirect", err)
}
} else {
ctx.ServerError("GetUserByName", err)
}
Expand Down
15 changes: 11 additions & 4 deletions modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,11 +411,18 @@ func RepoAssignment() macaron.Handler {
owner, err = models.GetUserByName(userName)
if err != nil {
if models.IsErrUserNotExist(err) {
if ctx.Query("go-get") == "1" {
EarlyResponseForGoGetMeta(ctx)
return
redirectUserID, err := models.LookupUserRedirect(userName)
if err == nil {
RedirectToUser(ctx, userName, redirectUserID)
} else if models.IsErrUserRedirectNotExist(err) {
if ctx.Query("go-get") == "1" {
EarlyResponseForGoGetMeta(ctx)
return
}
ctx.NotFound("GetUserByName", nil)
} else {
ctx.ServerError("LookupUserRedirect", err)
}
ctx.NotFound("GetUserByName", nil)
} else {
ctx.ServerError("GetUserByName", err)
}
Expand Down
Loading