diff options
author | ᴜɴᴋɴᴡᴏɴ <u@gogs.io> | 2020-04-18 12:07:30 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-18 12:07:30 +0800 |
commit | 9d64d222a8c30db5076a591981ad7cfe672ba6f1 (patch) | |
tree | c753eb898a1ead0afbe5b6a7a28b466ab0d2b8ac /internal/db | |
parent | fa497b16332c24bc4d9e788c64bda94e3c1499a7 (diff) |
db: add tests for users (#6116)
* Add new methods
* Use Users.Create to replace previous hack
* Reduce side effect
* Do not clear tables when test failed
* test_users_Authenticate
* Rename constant
* test_users_Create
* test_users_GetByEmail
* test_users_GetByID
* test_users_GetByUsername
Diffstat (limited to 'internal/db')
-rw-r--r-- | internal/db/access_tokens_test.go | 2 | ||||
-rw-r--r-- | internal/db/error.go | 26 | ||||
-rw-r--r-- | internal/db/lfs_test.go | 2 | ||||
-rw-r--r-- | internal/db/login_sources_test.go | 8 | ||||
-rw-r--r-- | internal/db/main_test.go | 6 | ||||
-rw-r--r-- | internal/db/mocks.go | 10 | ||||
-rw-r--r-- | internal/db/org.go | 8 | ||||
-rw-r--r-- | internal/db/perms_test.go | 2 | ||||
-rw-r--r-- | internal/db/repos.go | 2 | ||||
-rw-r--r-- | internal/db/repos_test.go | 2 | ||||
-rw-r--r-- | internal/db/two_factors_test.go | 2 | ||||
-rw-r--r-- | internal/db/user.go | 64 | ||||
-rw-r--r-- | internal/db/user_mail.go | 17 | ||||
-rw-r--r-- | internal/db/users.go | 181 | ||||
-rw-r--r-- | internal/db/users_test.go | 262 |
15 files changed, 494 insertions, 100 deletions
diff --git a/internal/db/access_tokens_test.go b/internal/db/access_tokens_test.go index 0f979e95..92a2bfa2 100644 --- a/internal/db/access_tokens_test.go +++ b/internal/db/access_tokens_test.go @@ -38,7 +38,7 @@ func Test_accessTokens(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Cleanup(func() { - err := clearTables(db.DB, tables...) + err := clearTables(t, db.DB, tables...) if err != nil { t.Fatal(err) } diff --git a/internal/db/error.go b/internal/db/error.go index ba07ca87..d1668d99 100644 --- a/internal/db/error.go +++ b/internal/db/error.go @@ -15,32 +15,6 @@ import ( // |______//____ >\___ >__| // \/ \/ -type ErrUserAlreadyExist struct { - Name string -} - -func IsErrUserAlreadyExist(err error) bool { - _, ok := err.(ErrUserAlreadyExist) - return ok -} - -func (err ErrUserAlreadyExist) Error() string { - return fmt.Sprintf("user already exists [name: %s]", err.Name) -} - -type ErrEmailAlreadyUsed struct { - Email string -} - -func IsErrEmailAlreadyUsed(err error) bool { - _, ok := err.(ErrEmailAlreadyUsed) - return ok -} - -func (err ErrEmailAlreadyUsed) Error() string { - return fmt.Sprintf("e-mail has been used [email: %s]", err.Email) -} - type ErrUserOwnRepos struct { UID int64 } diff --git a/internal/db/lfs_test.go b/internal/db/lfs_test.go index 29c7d665..49d94406 100644 --- a/internal/db/lfs_test.go +++ b/internal/db/lfs_test.go @@ -37,7 +37,7 @@ func Test_lfs(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Cleanup(func() { - err := clearTables(db.DB, tables...) + err := clearTables(t, db.DB, tables...) if err != nil { t.Fatal(err) } diff --git a/internal/db/login_sources_test.go b/internal/db/login_sources_test.go index 8cc0ab19..47bfd6d2 100644 --- a/internal/db/login_sources_test.go +++ b/internal/db/login_sources_test.go @@ -40,7 +40,7 @@ func Test_loginSources(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Cleanup(func() { - err := clearTables(db.DB, tables...) + err := clearTables(t, db.DB, tables...) if err != nil { t.Fatal(err) } @@ -119,10 +119,10 @@ func test_loginSources_DeleteByID(t *testing.T, db *loginSources) { } // Create a user that uses this login source - user := &User{ + _, err = (&users{DB: db.DB}).Create(CreateUserOpts{ + Name: "alice", LoginSource: source.ID, - } - err = db.DB.Create(user).Error + }) if err != nil { t.Fatal(err) } diff --git a/internal/db/main_test.go b/internal/db/main_test.go index 172f00fe..c6cc6f9e 100644 --- a/internal/db/main_test.go +++ b/internal/db/main_test.go @@ -42,7 +42,11 @@ func TestMain(m *testing.M) { } // clearTables removes all rows from given tables. -func clearTables(db *gorm.DB, tables ...interface{}) error { +func clearTables(t *testing.T, db *gorm.DB, tables ...interface{}) error { + if t.Failed() { + return nil + } + for _, t := range tables { err := db.Delete(t).Error if err != nil { diff --git a/internal/db/mocks.go b/internal/db/mocks.go index 06bafc75..41bb4072 100644 --- a/internal/db/mocks.go +++ b/internal/db/mocks.go @@ -209,6 +209,8 @@ var _ UsersStore = (*MockUsersStore)(nil) type MockUsersStore struct { MockAuthenticate func(username, password string, loginSourceID int64) (*User, error) + MockCreate func(opts CreateUserOpts) (*User, error) + MockGetByEmail func(email string) (*User, error) MockGetByID func(id int64) (*User, error) MockGetByUsername func(username string) (*User, error) } @@ -217,6 +219,14 @@ func (m *MockUsersStore) Authenticate(username, password string, loginSourceID i return m.MockAuthenticate(username, password, loginSourceID) } +func (m *MockUsersStore) Create(opts CreateUserOpts) (*User, error) { + return m.MockCreate(opts) +} + +func (m *MockUsersStore) GetByEmail(email string) (*User, error) { + return m.MockGetByEmail(email) +} + func (m *MockUsersStore) GetByID(id int64) (*User, error) { return m.MockGetByID(id) } diff --git a/internal/db/org.go b/internal/db/org.go index b28d4333..f235928b 100644 --- a/internal/db/org.go +++ b/internal/db/org.go @@ -12,6 +12,8 @@ import ( "xorm.io/builder" "xorm.io/xorm" + + "gogs.io/gogs/internal/errutil" ) var ( @@ -99,7 +101,7 @@ func (org *User) RemoveOrgRepo(repoID int64) error { // CreateOrganization creates record of a new organization. func CreateOrganization(org, owner *User) (err error) { - if err = IsUsableUsername(org.Name); err != nil { + if err = isUsernameAllowed(org.Name); err != nil { return err } @@ -107,7 +109,7 @@ func CreateOrganization(org, owner *User) (err error) { if err != nil { return err } else if isExist { - return ErrUserAlreadyExist{org.Name} + return ErrUserAlreadyExist{args: errutil.Args{"name": org.Name}} } org.LowerName = strings.ToLower(org.Name) @@ -177,7 +179,7 @@ func GetOrgByName(name string) (*User, error) { } u := &User{ LowerName: strings.ToLower(name), - Type: USER_TYPE_ORGANIZATION, + Type: UserOrganization, } has, err := x.Get(u) if err != nil { diff --git a/internal/db/perms_test.go b/internal/db/perms_test.go index ea4ed1ea..f11a6d60 100644 --- a/internal/db/perms_test.go +++ b/internal/db/perms_test.go @@ -32,7 +32,7 @@ func Test_perms(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Cleanup(func() { - err := clearTables(db.DB, tables...) + err := clearTables(t, db.DB, tables...) if err != nil { t.Fatal(err) } diff --git a/internal/db/repos.go b/internal/db/repos.go index f97cf28d..08cc6485 100644 --- a/internal/db/repos.go +++ b/internal/db/repos.go @@ -75,7 +75,7 @@ type createRepoOpts struct { // create creates a new repository record in the database. Fields of "repo" will be updated // in place upon insertion. It returns ErrNameNotAllowed when the repository name is not allowed, -// or returns ErrRepoAlreadyExist when a repository with same name already exists for the owner. +// or ErrRepoAlreadyExist when a repository with same name already exists for the owner. func (db *repos) create(ownerID int64, opts createRepoOpts) (*Repository, error) { err := isRepoNameAllowed(opts.Name) if err != nil { diff --git a/internal/db/repos_test.go b/internal/db/repos_test.go index 6a39c4ee..57bcdbab 100644 --- a/internal/db/repos_test.go +++ b/internal/db/repos_test.go @@ -35,7 +35,7 @@ func Test_repos(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Cleanup(func() { - err := clearTables(db.DB, tables...) + err := clearTables(t, db.DB, tables...) if err != nil { t.Fatal(err) } diff --git a/internal/db/two_factors_test.go b/internal/db/two_factors_test.go index e4dea6ca..bbf46f90 100644 --- a/internal/db/two_factors_test.go +++ b/internal/db/two_factors_test.go @@ -36,7 +36,7 @@ func Test_twoFactors(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Cleanup(func() { - err := clearTables(db.DB, tables...) + err := clearTables(t, db.DB, tables...) if err != nil { t.Fatal(err) } diff --git a/internal/db/user.go b/internal/db/user.go index c225bd1a..7abdfc69 100644 --- a/internal/db/user.go +++ b/internal/db/user.go @@ -42,8 +42,8 @@ const USER_AVATAR_URL_PREFIX = "avatars" type UserType int const ( - USER_TYPE_INDIVIDUAL UserType = iota // Historic reason to make it starts at 0. - USER_TYPE_ORGANIZATION + UserIndividual UserType = iota // Historic reason to make it starts at 0. + UserOrganization ) // User represents the object of individual and member of organization. @@ -53,10 +53,10 @@ type User struct { Name string `xorm:"UNIQUE NOT NULL" gorm:"NOT NULL"` FullName string // Email is the primary email address (to be used for communication) - Email string `xorm:"NOT NULL" gorm:"NOT NULL"` - Passwd string `xorm:"NOT NULL" gorm:"NOT NULL"` - LoginType LoginType - LoginSource int64 `xorm:"NOT NULL DEFAULT 0" gorm:"NOT NULL;DEFAULT:0"` + Email string `xorm:"NOT NULL" gorm:"NOT NULL"` + Passwd string `xorm:"NOT NULL" gorm:"NOT NULL"` + LoginType LoginType // TODO: Remove me https://github.com/gogs/gogs/issues/6117. + LoginSource int64 `xorm:"NOT NULL DEFAULT 0" gorm:"NOT NULL;DEFAULT:0"` LoginName string Type UserType OwnedOrgs []*User `xorm:"-" gorm:"-" json:"-"` @@ -321,8 +321,8 @@ func (u *User) NewGitSig() *git.Signature { } } -// EncodePasswd encodes password to safe format. -func (u *User) EncodePasswd() { +// EncodePassword encodes password to safe format. +func (u *User) EncodePassword() { newPasswd := pbkdf2.Key([]byte(u.Passwd), []byte(u.Salt), 10000, 50, sha256.New) u.Passwd = fmt.Sprintf("%x", newPasswd) } @@ -330,7 +330,7 @@ func (u *User) EncodePasswd() { // ValidatePassword checks if given password matches the one belongs to the user. func (u *User) ValidatePassword(passwd string) bool { newUser := &User{Passwd: passwd, Salt: u.Salt} - newUser.EncodePasswd() + newUser.EncodePassword() return subtle.ConstantTimeCompare([]byte(u.Passwd), []byte(newUser.Passwd)) == 1 } @@ -388,7 +388,7 @@ func (u *User) IsWriterOfRepo(repo *Repository) bool { // IsOrganization returns true if user is actually a organization. func (u *User) IsOrganization() bool { - return u.Type == USER_TYPE_ORGANIZATION + return u.Type == UserOrganization } // IsUserOrgOwner returns true if user is in the owner team of given organization. @@ -448,7 +448,7 @@ func (u *User) GetOrganizations(showPrivate bool) error { } u.Orgs = make([]*User, 0, len(orgIDs)) - if err = x.Where("type = ?", USER_TYPE_ORGANIZATION).In("id", orgIDs).Find(&u.Orgs); err != nil { + if err = x.Where("type = ?", UserOrganization).In("id", orgIDs).Find(&u.Orgs); err != nil { return err } return nil @@ -555,13 +555,15 @@ func isNameAllowed(names, patterns []string, name string) error { return nil } -func IsUsableUsername(name string) error { +// isUsernameAllowed return an error if given name is a reserved name or pattern for users. +func isUsernameAllowed(name string) error { return isNameAllowed(reservedUsernames, reservedUserPatterns, name) } // CreateUser creates record of a new user. +// Deprecated: Use Users.Create instead. func CreateUser(u *User) (err error) { - if err = IsUsableUsername(u.Name); err != nil { + if err = isUsernameAllowed(u.Name); err != nil { return err } @@ -569,7 +571,7 @@ func CreateUser(u *User) (err error) { if err != nil { return err } else if isExist { - return ErrUserAlreadyExist{u.Name} + return ErrUserAlreadyExist{args: errutil.Args{"name": u.Name}} } u.Email = strings.ToLower(u.Email) @@ -577,7 +579,7 @@ func CreateUser(u *User) (err error) { if err != nil { return err } else if isExist { - return ErrEmailAlreadyUsed{u.Email} + return ErrEmailAlreadyUsed{args: errutil.Args{"email": u.Email}} } u.LowerName = strings.ToLower(u.Name) @@ -589,7 +591,7 @@ func CreateUser(u *User) (err error) { if u.Salt, err = GetUserSalt(); err != nil { return err } - u.EncodePasswd() + u.EncodePassword() u.MaxRepoCreation = -1 sess := x.NewSession() @@ -680,7 +682,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) { - if err = IsUsableUsername(newUserName); err != nil { + if err = isUsernameAllowed(newUserName); err != nil { return err } @@ -688,7 +690,7 @@ func ChangeUserName(u *User, newUserName string) (err error) { if err != nil { return err } else if isExist { - return ErrUserAlreadyExist{newUserName} + return ErrUserAlreadyExist{args: errutil.Args{"name": newUserName}} } if err = ChangeUsernameInPullRequests(u.Name, newUserName); err != nil { @@ -723,7 +725,7 @@ func updateUser(e Engine, u *User) error { if err != nil { return err } else if has { - return ErrEmailAlreadyUsed{u.Email} + return ErrEmailAlreadyUsed{args: errutil.Args{"email": u.Email}} } if len(u.AvatarEmail) == 0 { @@ -904,8 +906,8 @@ func DeleteInactivateUsers() (err error) { } // UserPath returns the path absolute path of user repositories. -func UserPath(userName string) string { - return filepath.Join(conf.Repository.Root, strings.ToLower(userName)) +func UserPath(username string) string { + return filepath.Join(conf.Repository.Root, strings.ToLower(username)) } func GetUserByKeyID(keyID int64) (*User, error) { @@ -919,25 +921,6 @@ func GetUserByKeyID(keyID int64) (*User, error) { return user, nil } -var _ errutil.NotFound = (*ErrUserNotExist)(nil) - -type ErrUserNotExist struct { - args map[string]interface{} -} - -func IsErrUserNotExist(err error) bool { - _, ok := err.(ErrUserNotExist) - return ok -} - -func (err ErrUserNotExist) Error() string { - return fmt.Sprintf("user does not exist: %v", err.args) -} - -func (ErrUserNotExist) NotFound() bool { - return true -} - func getUserByID(e Engine, id int64) (*User, error) { u := new(User) has, err := e.ID(id).Get(u) @@ -1047,6 +1030,7 @@ func ValidateCommitsWithEmails(oldCommits []*git.Commit) []*UserCommit { } // GetUserByEmail returns the user object by given e-mail if exists. +// Deprecated: Use Users.GetByEmail instead. func GetUserByEmail(email string) (*User, error) { if len(email) == 0 { return nil, ErrUserNotExist{args: map[string]interface{}{"email": email}} diff --git a/internal/db/user_mail.go b/internal/db/user_mail.go index 1608dc19..f73278b6 100644 --- a/internal/db/user_mail.go +++ b/internal/db/user_mail.go @@ -9,16 +9,17 @@ import ( "strings" "gogs.io/gogs/internal/db/errors" + "gogs.io/gogs/internal/errutil" ) -// EmailAdresses is the list of all email addresses of a user. Can contain the +// EmailAddresses is the list of all email addresses of a user. Can contain the // primary email address, but is not obligatory. type EmailAddress struct { ID int64 - UID int64 `xorm:"INDEX NOT NULL"` - Email string `xorm:"UNIQUE NOT NULL"` - IsActivated bool - IsPrimary bool `xorm:"-" json:"-"` + UID int64 `xorm:"INDEX NOT NULL" gorm:"INDEX"` + Email string `xorm:"UNIQUE NOT NULL" gorm:"UNIQUE"` + IsActivated bool `gorm:"NOT NULL;DEFAULT:FALSE"` + IsPrimary bool `xorm:"-" gorm:"-" json:"-"` } // GetEmailAddresses returns all email addresses belongs to given user. @@ -68,7 +69,7 @@ func isEmailUsed(e Engine, email string) (bool, error) { } // We need to check primary email of users as well. - return e.Where("type=?", USER_TYPE_INDIVIDUAL).And("email=?", email).Get(new(User)) + return e.Where("type=?", UserIndividual).And("email=?", email).Get(new(User)) } // IsEmailUsed returns true if the email has been used. @@ -82,7 +83,7 @@ func addEmailAddress(e Engine, email *EmailAddress) error { if err != nil { return err } else if used { - return ErrEmailAlreadyUsed{email.Email} + return ErrEmailAlreadyUsed{args: errutil.Args{"email": email.Email}} } _, err = e.Insert(email) @@ -105,7 +106,7 @@ func AddEmailAddresses(emails []*EmailAddress) error { if err != nil { return err } else if used { - return ErrEmailAlreadyUsed{emails[i].Email} + return ErrEmailAlreadyUsed{args: errutil.Args{"email": emails[i].Email}} } } diff --git a/internal/db/users.go b/internal/db/users.go index 006e2089..10935b4a 100644 --- a/internal/db/users.go +++ b/internal/db/users.go @@ -7,10 +7,12 @@ package db import ( "fmt" "strings" + "time" "github.com/jinzhu/gorm" "github.com/pkg/errors" + "gogs.io/gogs/internal/cryptoutil" "gogs.io/gogs/internal/errutil" ) @@ -30,15 +32,35 @@ type UsersStore interface { // When the "loginSourceID" is positive, it tries to authenticate via given // login source and creates a new user when not yet exists in the database. Authenticate(username, password string, loginSourceID int64) (*User, error) + // Create creates a new user and persist to database. + // It returns ErrUserAlreadyExist when a user with same name already exists, + // or ErrEmailAlreadyUsed if the email has been used by another user. + Create(opts CreateUserOpts) (*User, error) + // GetByEmail returns the user (not organization) with given email. + // It ignores records with unverified emails and returns ErrUserNotExist when not found. + GetByEmail(email string) (*User, error) // GetByID returns the user with given ID. It returns ErrUserNotExist when not found. GetByID(id int64) (*User, error) - // GetByUsername returns the user with given username. It returns ErrUserNotExist - // when not found. + // GetByUsername returns the user with given username. It returns ErrUserNotExist when not found. GetByUsername(username string) (*User, error) } var Users UsersStore +// NOTE: This is a GORM create hook. +func (u *User) BeforeCreate() { + u.CreatedUnix = gorm.NowFunc().Unix() + u.UpdatedUnix = u.CreatedUnix +} + +// NOTE: This is a GORM query hook. +func (u *User) AfterFind() { + u.Created = time.Unix(u.CreatedUnix, 0).Local() + u.Updated = time.Unix(u.UpdatedUnix, 0).Local() +} + +var _ UsersStore = (*users)(nil) + type users struct { *gorm.DB } @@ -51,14 +73,14 @@ func (err ErrLoginSourceMismatch) Error() string { return fmt.Sprintf("login source mismatch: %v", err.args) } -func (db *users) Authenticate(username, password string, loginSourceID int64) (*User, error) { - username = strings.ToLower(username) +func (db *users) Authenticate(login, password string, loginSourceID int64) (*User, error) { + login = strings.ToLower(login) var query *gorm.DB - if strings.Contains(username, "@") { - query = db.Where("email = ?", username) + if strings.Contains(login, "@") { + query = db.Where("email = ?", login) } else { - query = db.Where("lower_name = ?", username) + query = db.Where("lower_name = ?", login) } user := new(User) @@ -89,7 +111,7 @@ func (db *users) Authenticate(username, password string, loginSourceID int64) (* return nil, errors.Wrap(err, "get login source") } - _, err = authenticateViaLoginSource(source, username, password, false) + _, err = authenticateViaLoginSource(source, login, password, false) if err != nil { return nil, errors.Wrap(err, "authenticate via login source") } @@ -98,7 +120,7 @@ func (db *users) Authenticate(username, password string, loginSourceID int64) (* // Non-local login source is always greater than 0. if loginSourceID <= 0 { - return nil, ErrUserNotExist{args: map[string]interface{}{"name": username}} + return nil, ErrUserNotExist{args: map[string]interface{}{"login": login}} } source, err := LoginSources.GetByID(loginSourceID) @@ -106,19 +128,154 @@ func (db *users) Authenticate(username, password string, loginSourceID int64) (* return nil, errors.Wrap(err, "get login source") } - user, err = authenticateViaLoginSource(source, username, password, true) + user, err = authenticateViaLoginSource(source, login, password, true) if err != nil { return nil, errors.Wrap(err, "authenticate via login source") } return user, nil } +type CreateUserOpts struct { + Name string + Email string + Password string + LoginSource int64 + Activated bool +} + +type ErrUserAlreadyExist struct { + args errutil.Args +} + +func IsErrUserAlreadyExist(err error) bool { + _, ok := err.(ErrUserAlreadyExist) + return ok +} + +func (err ErrUserAlreadyExist) Error() string { + return fmt.Sprintf("user already exists: %v", err.args) +} + +type ErrEmailAlreadyUsed struct { + args errutil.Args +} + +func IsErrEmailAlreadyUsed(err error) bool { + _, ok := err.(ErrEmailAlreadyUsed) + return ok +} + +func (err ErrEmailAlreadyUsed) Email() string { + email, ok := err.args["email"].(string) + if ok { + return email + } + return "<email not found>" +} + +func (err ErrEmailAlreadyUsed) Error() string { + return fmt.Sprintf("email has been used: %v", err.args) +} + +func (db *users) Create(opts CreateUserOpts) (*User, error) { + err := isUsernameAllowed(opts.Name) + if err != nil { + return nil, err + } + + _, err = db.GetByUsername(opts.Name) + if err == nil { + return nil, ErrUserAlreadyExist{args: errutil.Args{"name": opts.Name}} + } else if !IsErrUserNotExist(err) { + return nil, err + } + + _, err = db.GetByEmail(opts.Email) + if err == nil { + return nil, ErrEmailAlreadyUsed{args: errutil.Args{"email": opts.Email}} + } else if !IsErrUserNotExist(err) { + return nil, err + } + + user := &User{ + LowerName: strings.ToLower(opts.Name), + Name: opts.Name, + Email: opts.Email, + Passwd: opts.Password, + LoginSource: opts.LoginSource, + MaxRepoCreation: -1, + IsActive: opts.Activated, + Avatar: cryptoutil.MD5(opts.Email), + AvatarEmail: opts.Email, + } + + user.Rands, err = GetUserSalt() + if err != nil { + return nil, err + } + user.Salt, err = GetUserSalt() + if err != nil { + return nil, err + } + user.EncodePassword() + + return user, db.DB.Create(user).Error +} + +var _ errutil.NotFound = (*ErrUserNotExist)(nil) + +type ErrUserNotExist struct { + args errutil.Args +} + +func IsErrUserNotExist(err error) bool { + _, ok := err.(ErrUserNotExist) + return ok +} + +func (err ErrUserNotExist) Error() string { + return fmt.Sprintf("user does not exist: %v", err.args) +} + +func (ErrUserNotExist) NotFound() bool { + return true +} + +func (db *users) GetByEmail(email string) (*User, error) { + email = strings.ToLower(email) + + if len(email) == 0 { + return nil, ErrUserNotExist{args: errutil.Args{"email": email}} + } + + // First try to find the user by primary email + user := new(User) + err := db.Where("email = ? AND type = ? AND is_active = ?", email, UserIndividual, true).First(user).Error + if err == nil { + return user, nil + } else if !gorm.IsRecordNotFoundError(err) { + return nil, err + } + + // Otherwise, check activated email addresses + emailAddress := new(EmailAddress) + err = db.Where("email = ? AND is_activated = ?", email, true).First(emailAddress).Error + if err != nil { + if gorm.IsRecordNotFoundError(err) { + return nil, ErrUserNotExist{args: errutil.Args{"email": email}} + } + return nil, err + } + + return db.GetByID(emailAddress.UID) +} + func (db *users) GetByID(id int64) (*User, error) { user := new(User) err := db.Where("id = ?", id).First(user).Error if err != nil { if gorm.IsRecordNotFoundError(err) { - return nil, ErrUserNotExist{args: map[string]interface{}{"userID": id}} + return nil, ErrUserNotExist{args: errutil.Args{"userID": id}} } return nil, err } @@ -130,7 +287,7 @@ func (db *users) GetByUsername(username string) (*User, error) { err := db.Where("lower_name = ?", strings.ToLower(username)).First(user).Error if err != nil { if gorm.IsRecordNotFoundError(err) { - return nil, ErrUserNotExist{args: map[string]interface{}{"name": username}} + return nil, ErrUserNotExist{args: errutil.Args{"name": username}} } return nil, err } diff --git a/internal/db/users_test.go b/internal/db/users_test.go new file mode 100644 index 00000000..e4d8fb86 --- /dev/null +++ b/internal/db/users_test.go @@ -0,0 +1,262 @@ +// Copyright 2020 The Gogs 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 db + +import ( + "testing" + "time" + + "github.com/jinzhu/gorm" + "github.com/stretchr/testify/assert" + + "gogs.io/gogs/internal/errutil" +) + +func Test_users(t *testing.T) { + if testing.Short() { + t.Skip() + } + + t.Parallel() + + tables := []interface{}{new(User), new(EmailAddress)} + db := &users{ + DB: initTestDB(t, "users", tables...), + } + + for _, tc := range []struct { + name string + test func(*testing.T, *users) + }{ + {"Authenticate", test_users_Authenticate}, + {"Create", test_users_Create}, + {"GetByEmail", test_users_GetByEmail}, + {"GetByID", test_users_GetByID}, + {"GetByUsername", test_users_GetByUsername}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Cleanup(func() { + err := clearTables(t, db.DB, tables...) + if err != nil { + t.Fatal(err) + } + }) + tc.test(t, db) + }) + } +} + +// TODO: Only local account is tested, tests for external account will be added +// along with addressing https://github.com/gogs/gogs/issues/6115. +func test_users_Authenticate(t *testing.T, db *users) { + password := "pa$$word" + alice, err := db.Create(CreateUserOpts{ + Name: "alice", + Email: "alice@example.com", + Password: password, + }) + if err != nil { + t.Fatal(err) + } + + t.Run("user not found", func(t *testing.T) { + _, err := db.Authenticate("bob", password, -1) + expErr := ErrUserNotExist{args: map[string]interface{}{"login": "bob"}} + assert.Equal(t, expErr, err) + }) + + t.Run("invalid password", func(t *testing.T) { + _, err := db.Authenticate(alice.Name, "bad_password", -1) + expErr := ErrUserNotExist{args: map[string]interface{}{"userID": alice.ID, "name": alice.Name}} + assert.Equal(t, expErr, err) + }) + + t.Run("via email and password", func(t *testing.T) { + user, err := db.Authenticate(alice.Email, password, -1) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, alice.Name, user.Name) + }) + + t.Run("via username and password", func(t *testing.T) { + user, err := db.Authenticate(alice.Name, password, -1) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, alice.Name, user.Name) + }) +} + +func test_users_Create(t *testing.T, db *users) { + alice, err := db.Create(CreateUserOpts{ + Name: "alice", + Email: "alice@example.com", + Activated: true, + }) + if err != nil { + t.Fatal(err) + } + + t.Run("name not allowed", func(t *testing.T) { + _, err := db.Create(CreateUserOpts{ + Name: "-", + }) + expErr := ErrNameNotAllowed{args: errutil.Args{"reason": "reserved", "name": "-"}} + assert.Equal(t, expErr, err) + }) + + t.Run("name already exists", func(t *testing.T) { + _, err := db.Create(CreateUserOpts{ + Name: alice.Name, + }) + expErr := ErrUserAlreadyExist{args: errutil.Args{"name": alice.Name}} + assert.Equal(t, expErr, err) + }) + + t.Run("email already exists", func(t *testing.T) { + _, err := db.Create(CreateUserOpts{ + Name: "bob", + Email: alice.Email, + }) + expErr := ErrEmailAlreadyUsed{args: errutil.Args{"email": alice.Email}} + assert.Equal(t, expErr, err) + }) + + user, err := db.GetByUsername(alice.Name) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, gorm.NowFunc().Format(time.RFC3339), user.Created.Format(time.RFC3339)) + assert.Equal(t, gorm.NowFunc().Format(time.RFC3339), user.Updated.Format(time.RFC3339)) +} + +func test_users_GetByEmail(t *testing.T, db *users) { + t.Run("empty email", func(t *testing.T) { + _, err := db.GetByEmail("") + expErr := ErrUserNotExist{args: errutil.Args{"email": ""}} + assert.Equal(t, expErr, err) + }) + + t.Run("ignore organization", func(t *testing.T) { + // TODO: Use Orgs.Create to replace SQL hack when the method is available. + org, err := db.Create(CreateUserOpts{ + Name: "gogs", + Email: "gogs@exmaple.com", + }) + if err != nil { + t.Fatal(err) + } + + err = db.Exec(`UPDATE user SET type = ? WHERE id = ?`, UserOrganization, org.ID).Error + if err != nil { + t.Fatal(err) + } + + _, err = db.GetByEmail(org.Email) + expErr := ErrUserNotExist{args: errutil.Args{"email": org.Email}} + assert.Equal(t, expErr, err) + }) + + t.Run("by primary email", func(t *testing.T) { + alice, err := db.Create(CreateUserOpts{ + Name: "alice", + Email: "alice@exmaple.com", + }) + if err != nil { + t.Fatal(err) + } + + _, err = db.GetByEmail(alice.Email) + expErr := ErrUserNotExist{args: errutil.Args{"email": alice.Email}} + assert.Equal(t, expErr, err) + + // Mark user as activated + // TODO: Use UserEmails.Verify to replace SQL hack when the method is available. + err = db.Exec(`UPDATE user SET is_active = ? WHERE id = ?`, true, alice.ID).Error + if err != nil { + t.Fatal(err) + } + + user, err := db.GetByEmail(alice.Email) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, alice.Name, user.Name) + }) + + t.Run("by secondary email", func(t *testing.T) { + bob, err := db.Create(CreateUserOpts{ + Name: "bob", + Email: "bob@example.com", + }) + if err != nil { + t.Fatal(err) + } + + // TODO: Use UserEmails.Create to replace SQL hack when the method is available. + email2 := "bob2@exmaple.com" + err = db.Exec(`INSERT INTO email_address (uid, email) VALUES (?, ?)`, bob.ID, email2).Error + if err != nil { + t.Fatal(err) + } + + _, err = db.GetByEmail(email2) + expErr := ErrUserNotExist{args: errutil.Args{"email": email2}} + assert.Equal(t, expErr, err) + + // TODO: Use UserEmails.Verify to replace SQL hack when the method is available. + err = db.Exec(`UPDATE email_address SET is_activated = ? WHERE email = ?`, true, email2).Error + if err != nil { + t.Fatal(err) + } + + user, err := db.GetByEmail(email2) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, bob.Name, user.Name) + }) +} + +func test_users_GetByID(t *testing.T, db *users) { + alice, err := db.Create(CreateUserOpts{ + Name: "alice", + Email: "alice@exmaple.com", + }) + if err != nil { + t.Fatal(err) + } + + user, err := db.GetByID(alice.ID) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, alice.Name, user.Name) + + _, err = db.GetByID(404) + expErr := ErrUserNotExist{args: errutil.Args{"userID": int64(404)}} + assert.Equal(t, expErr, err) +} + +func test_users_GetByUsername(t *testing.T, db *users) { + alice, err := db.Create(CreateUserOpts{ + Name: "alice", + Email: "alice@exmaple.com", + }) + if err != nil { + t.Fatal(err) + } + + user, err := db.GetByUsername(alice.Name) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, alice.Name, user.Name) + + _, err = db.GetByUsername("bad_username") + expErr := ErrUserNotExist{args: errutil.Args{"name": "bad_username"}} + assert.Equal(t, expErr, err) +} |