From 1ccfd6df58283fb0fc9af7de6e1ee846abaf2042 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 7 Apr 2021 11:06:33 +0800 Subject: [PATCH] remove unused user fields --- models/external_login_user.go | 6 +- models/login_source.go | 125 +++++++++++++++---------------- models/migrations/migrations.go | 2 + models/migrations/v179.go | 84 +++++++++++++++++++++ models/user.go | 54 ++++++------- modules/auth/sso/reverseproxy.go | 2 +- routers/api/v1/admin/user.go | 42 +++-------- routers/user/setting/profile.go | 6 -- services/externalaccount/user.go | 2 +- 9 files changed, 191 insertions(+), 132 deletions(-) create mode 100644 models/migrations/v179.go diff --git a/models/external_login_user.go b/models/external_login_user.go index aa5da8134a3e3..2ef1c7cc13137 100644 --- a/models/external_login_user.go +++ b/models/external_login_user.go @@ -53,8 +53,8 @@ func ListAccountLinks(user *User) ([]*ExternalLoginUser, error) { } // LinkExternalToUser link the external user to the user -func LinkExternalToUser(user *User, externalLoginUser *ExternalLoginUser) error { - has, err := x.Where("external_id=? AND login_source_id=?", externalLoginUser.ExternalID, externalLoginUser.LoginSourceID). +func LinkExternalToUser(ctx DBContext, user *User, externalLoginUser *ExternalLoginUser) error { + has, err := ctx.e.Where("external_id=? AND login_source_id=?", externalLoginUser.ExternalID, externalLoginUser.LoginSourceID). NoAutoCondition(). Exist(externalLoginUser) if err != nil { @@ -63,7 +63,7 @@ func LinkExternalToUser(user *User, externalLoginUser *ExternalLoginUser) error return ErrExternalLoginUserAlreadyExist{externalLoginUser.ExternalID, user.ID, externalLoginUser.LoginSourceID} } - _, err = x.Insert(externalLoginUser) + _, err = ctx.e.Insert(externalLoginUser) return err } diff --git a/models/login_source.go b/models/login_source.go index fd977e20a5d7b..8b75f825f3a5b 100644 --- a/models/login_source.go +++ b/models/login_source.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/modules/util" jsoniter "github.com/json-iterator/go" + "xorm.io/builder" "xorm.io/xorm" "xorm.io/xorm/convert" ) @@ -420,14 +421,7 @@ func UpdateSource(source *LoginSource) error { // DeleteSource deletes a LoginSource record in DB. func DeleteSource(source *LoginSource) error { - count, err := x.Count(&User{LoginSource: source.ID}) - if err != nil { - return err - } else if count > 0 { - return ErrLoginSourceInUse{source.ID} - } - - count, err = x.Count(&ExternalLoginUser{LoginSourceID: source.ID}) + count, err := x.Count(&ExternalLoginUser{LoginSourceID: source.ID}) if err != nil { return err } else if count > 0 { @@ -532,20 +526,24 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*Use Name: sr.Username, FullName: composeFullName(sr.Name, sr.Surname, sr.Username), Email: sr.Mail, - LoginType: source.Type, - LoginSource: source.ID, - LoginName: login, IsActive: true, IsAdmin: sr.IsAdmin, IsRestricted: sr.IsRestricted, } - err := CreateUser(user) + err := CreateUserAndLinkAccount(user, &ExternalLoginUser{ + ExternalID: login, + LoginSourceID: source.ID, + Email: sr.Mail, + Name: user.Name, + }) + if err != nil { + return nil, err + } - if err == nil && isAttributeSSHPublicKeySet && addLdapSSHPublicKeys(user, source, sr.SSHPublicKey) { + if isAttributeSSHPublicKeySet && addLdapSSHPublicKeys(user, source, sr.SSHPublicKey) { err = RewriteAllPublicKeys() } - return user, err } @@ -660,16 +658,18 @@ func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPC } user = &User{ - LowerName: strings.ToLower(username), - Name: strings.ToLower(username), - Email: login, - Passwd: password, - LoginType: LoginSMTP, - LoginSource: sourceID, - LoginName: login, - IsActive: true, + LowerName: strings.ToLower(username), + Name: strings.ToLower(username), + Email: login, + Passwd: password, + IsActive: true, } - return user, CreateUser(user) + return user, CreateUserAndLinkAccount(user, &ExternalLoginUser{ + ExternalID: login, + LoginSourceID: sourceID, + Email: login, + Name: username, + }) } // __________ _____ _____ @@ -702,16 +702,18 @@ func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMCon } user = &User{ - LowerName: strings.ToLower(username), - Name: username, - Email: pamLogin, - Passwd: password, - LoginType: LoginPAM, - LoginSource: sourceID, - LoginName: login, // This is what the user typed in - IsActive: true, + LowerName: strings.ToLower(username), + Name: username, + Email: pamLogin, + Passwd: password, + IsActive: true, } - return user, CreateUser(user) + return user, CreateUserAndLinkAccount(user, &ExternalLoginUser{ + ExternalID: login, + LoginSourceID: sourceID, + Email: pamLogin, + Name: username, + }) } // ExternalUserLogin attempts a login using external source types. @@ -774,48 +776,41 @@ func UserSignIn(username, password string) (*User, error) { return nil, err } + var sources = make([]*LoginSource, 0, 5) if hasUser { - switch user.LoginType { - case LoginNoType, LoginPlain, LoginOAuth2: - if user.IsPasswordSet() && user.ValidatePassword(password) { - - // Update password hash if server password hash algorithm have changed - if user.PasswdHashAlgo != setting.PasswordHashAlgo { - if err = user.SetPassword(password); err != nil { - return nil, err - } - if err = UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil { - return nil, err - } + if user.IsPasswordSet() && user.ValidatePassword(password) { + // Update password hash if server password hash algorithm have changed + if user.PasswdHashAlgo != setting.PasswordHashAlgo { + if err = user.SetPassword(password); err != nil { + return nil, err } - - // WARN: DON'T check user.IsActive, that will be checked on reqSign so that - // user could be hint to resend confirm email. - if user.ProhibitLogin { - return nil, ErrUserProhibitLogin{user.ID, user.Name} + if err = UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil { + return nil, err } - - return user, nil } - return nil, ErrUserNotExist{user.ID, user.Name, 0} - - default: - var source LoginSource - hasSource, err := x.ID(user.LoginSource).Get(&source) - if err != nil { - return nil, err - } else if !hasSource { - return nil, ErrLoginSourceNotExist{user.LoginSource} + // WARN: DON'T check user.IsActive, that will be checked on reqSign so that + // user could be hint to resend confirm email. + if user.ProhibitLogin { + return nil, ErrUserProhibitLogin{user.ID, user.Name} } - return ExternalUserLogin(user, user.LoginName, password, &source) + return user, nil } - } - sources := make([]*LoginSource, 0, 5) - if err = x.Where("is_actived = ?", true).Find(&sources); err != nil { - return nil, err + if err = x.Where( + builder.In("id", + builder.Select("login_source_id"). + From("external_login_user"). + Where(builder.Eq{"user_id": user.ID}), + ), + ).Find(&sources); err != nil { + return nil, err + } + } else { + if err = x.Where("is_actived = ?", true).Find(&sources); err != nil { + return nil, err + } } for _, source := range sources { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index e9d4927ae6395..23ba86d5072fd 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -304,6 +304,8 @@ var migrations = []Migration{ NewMigration("Delete orphaned IssueLabels", deleteOrphanedIssueLabels), // v178 -> v179 NewMigration("Add LFS columns to Mirror", addLFSMirrorColumns), + // v179 -> v180 + NewMigration("Make user/external_login_user tables clear", changeUserTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v179.go b/models/migrations/v179.go new file mode 100644 index 0000000000000..6493d190d857c --- /dev/null +++ b/models/migrations/v179.go @@ -0,0 +1,84 @@ +// 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 ( + "time" + + "xorm.io/builder" + "xorm.io/xorm" +) + +// changeUserTable looks through the database for issue_labels where the label no longer exists and deletes them. +func changeUserTable(x *xorm.Engine) (err error) { + type User struct { + ID int64 + LoginType int + LoginSource int64 + LoginName string + Name string + Email string + } + + type ExternalLoginUser struct { + ExternalID string `xorm:"pk NOT NULL"` + UserID int64 `xorm:"INDEX NOT NULL"` + LoginSourceID int64 `xorm:"pk NOT NULL"` + RawData map[string]interface{} `xorm:"TEXT JSON"` + Provider string `xorm:"index VARCHAR(25)"` + Email string + Name string + FirstName string + LastName string + NickName string + Description string + AvatarURL string + Location string + AccessToken string `xorm:"TEXT"` + AccessTokenSecret string `xorm:"TEXT"` + RefreshToken string `xorm:"TEXT"` + ExpiresAt time.Time + } + + sess := x.NewSession() + defer sess.Close() + + const batchSize = 100 + + if err = sess.Begin(); err != nil { + return + } + + for start := 0; ; start += batchSize { + users := make([]*User, 0, batchSize) + if err = sess.Limit(batchSize, start). + Where(builder.Neq{"login_type": 0}). + Find(&users); err != nil { + return + } + if len(users) == 0 { + break + } + + for _, user := range users { + if _, err = sess.Insert(&ExternalLoginUser{ + ExternalID: user.LoginName, + UserID: user.ID, + LoginSourceID: user.LoginSource, + Email: user.Email, + Name: user.Name, + NickName: user.LoginName, + }); err != nil { + return + } + } + } + + // drop the columns + if err = dropTableColumns(sess, "user", "login_type", "login_source", "login_name"); err != nil { + return + } + return sess.Commit() +} diff --git a/models/user.go b/models/user.go index 53a8ac34862d1..afa0b910bc776 100644 --- a/models/user.go +++ b/models/user.go @@ -112,9 +112,6 @@ type User struct { // is to change his/her password after registration. MustChangePassword bool `xorm:"NOT NULL DEFAULT false"` - LoginType LoginType - LoginSource int64 `xorm:"NOT NULL DEFAULT 0"` - LoginName string Type UserType OwnedOrgs []*User `xorm:"-"` Orgs []*User `xorm:"-"` @@ -242,16 +239,6 @@ func GetAllUsers() ([]*User, error) { return users, x.OrderBy("id").Where("type = ?", UserTypeIndividual).Find(&users) } -// IsLocal returns true if user login type is LoginPlain. -func (u *User) IsLocal() bool { - return u.LoginType <= LoginPlain -} - -// IsOAuth2 returns true if user login type is LoginOAuth2. -func (u *User) IsOAuth2() bool { - return u.LoginType == LoginOAuth2 -} - // HasForkedRepo checks if user has already forked a repository with given ID. func (u *User) HasForkedRepo(repoID int64) bool { _, has := HasForkedRepo(u.ID, repoID) @@ -848,16 +835,12 @@ func IsUsableUsername(name string) error { } // CreateUser creates record of a new user. -func CreateUser(u *User) (err error) { +func CreateUser(ctx DBContext, u *User) (err error) { if err = IsUsableUsername(u.Name); err != nil { return err } - sess := x.NewSession() - defer sess.Close() - if err = sess.Begin(); err != nil { - return err - } + sess := ctx.e isExist, err := isUserExist(sess, 0, u.Name) if err != nil { @@ -910,7 +893,27 @@ func CreateUser(u *User) (err error) { return err } - return sess.Commit() + return nil +} + +// CreateUserAndLinkAccount creates user with linked account +func CreateUserAndLinkAccount(user *User, account *ExternalLoginUser) error { + ctx, commiter, err := TxDBContext() + if err != nil { + return err + } + defer commiter.Close() + + if err = CreateUser(ctx, user); err != nil { + return err + } + + account.UserID = user.ID + if err = LinkExternalToUser(ctx, user, account); err != nil { + return err + } + + return commiter.Commit() } func countUsers(e Engine) int64 { @@ -1912,17 +1915,18 @@ func SyncExternalUsers(ctx context.Context, updateExisting bool) error { LowerName: strings.ToLower(su.Username), Name: su.Username, FullName: fullName, - LoginType: s.Type, - LoginSource: s.ID, - LoginName: su.Username, Email: su.Mail, IsAdmin: su.IsAdmin, IsRestricted: su.IsRestricted, IsActive: true, } - err = CreateUser(usr) - + err = CreateUserAndLinkAccount(usr, &ExternalLoginUser{ + ExternalID: su.Username, + LoginSourceID: s.ID, + Email: su.Mail, + Name: su.Username, + }) if err != nil { log.Error("SyncExternalUsers[%s]: Error creating user %s: %v", s.Name, su.Username, err) } else if isAttributeSSHPublicKeySet { diff --git a/modules/auth/sso/reverseproxy.go b/modules/auth/sso/reverseproxy.go index ca9450e71429d..88594e581f4ac 100644 --- a/modules/auth/sso/reverseproxy.go +++ b/modules/auth/sso/reverseproxy.go @@ -105,7 +105,7 @@ func (r *ReverseProxy) newUser(req *http.Request) *models.User { Passwd: username, IsActive: true, } - if err := models.CreateUser(user); err != nil { + if err := models.CreateUser(models.DefaultDBContext(), user); err != nil { // FIXME: should I create a system notice? log.Error("CreateUser: %v", err) return nil diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 5a74c6ccd5081..358787712c09e 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -22,26 +22,6 @@ import ( "code.gitea.io/gitea/services/mailer" ) -func parseLoginSource(ctx *context.APIContext, u *models.User, sourceID int64, loginName string) { - if sourceID == 0 { - return - } - - source, err := models.GetLoginSourceByID(sourceID) - if err != nil { - if models.IsErrLoginSourceNotExist(err) { - ctx.Error(http.StatusUnprocessableEntity, "", err) - } else { - ctx.Error(http.StatusInternalServerError, "GetLoginSourceByID", err) - } - return - } - - u.LoginType = source.Type - u.LoginSource = source.ID - u.LoginName = loginName -} - // CreateUser create a user func CreateUser(ctx *context.APIContext) { // swagger:operation POST /admin/users admin adminCreateUser @@ -73,16 +53,11 @@ func CreateUser(ctx *context.APIContext) { Passwd: form.Password, MustChangePassword: true, IsActive: true, - LoginType: models.LoginPlain, } if form.MustChangePassword != nil { u.MustChangePassword = *form.MustChangePassword } - parseLoginSource(ctx, u, form.SourceID, form.LoginName) - if ctx.Written() { - return - } if !password.IsComplexEnough(form.Password) { err := errors.New("PasswordComplexity") ctx.Error(http.StatusBadRequest, "PasswordComplexity", err) @@ -97,7 +72,17 @@ func CreateUser(ctx *context.APIContext) { ctx.Error(http.StatusBadRequest, "PasswordPwned", errors.New("PasswordPwned")) return } - if err := models.CreateUser(u); err != nil { + if form.SourceID == 0 { + err = models.CreateUser(models.DefaultDBContext(), u) + } else { + err = models.CreateUserAndLinkAccount(u, &models.ExternalLoginUser{ + ExternalID: form.Username, + LoginSourceID: form.SourceID, + Email: form.Email, + Name: form.Username, + }) + } + if err != nil { if models.IsErrUserAlreadyExist(err) || models.IsErrEmailAlreadyUsed(err) || models.IsErrNameReserved(err) || @@ -151,11 +136,6 @@ func EditUser(ctx *context.APIContext) { return } - parseLoginSource(ctx, u, form.SourceID, form.LoginName) - if ctx.Written() { - return - } - if len(form.Password) != 0 { if !password.IsComplexEnough(form.Password) { err := errors.New("PasswordComplexity") diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go index 0bc2b4ee36b99..82e757a4f1944 100644 --- a/routers/user/setting/profile.go +++ b/routers/user/setting/profile.go @@ -43,12 +43,6 @@ func Profile(ctx *context.Context) { // HandleUsernameChange handle username changes from user settings and admin interface func HandleUsernameChange(ctx *context.Context, user *models.User, newName string) error { - // Non-local users are not allowed to change their username. - if !user.IsLocal() { - ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user")) - return fmt.Errorf(ctx.Tr("form.username_change_not_local_user")) - } - // Check if user name has been changed if user.LowerName != strings.ToLower(newName) { if err := models.ChangeUserName(user, newName); err != nil { diff --git a/services/externalaccount/user.go b/services/externalaccount/user.go index 45773fdb127d2..b07b4d841c3dd 100644 --- a/services/externalaccount/user.go +++ b/services/externalaccount/user.go @@ -40,7 +40,7 @@ func LinkAccountToUser(user *models.User, gothUser goth.User) error { ExpiresAt: gothUser.ExpiresAt, } - if err := models.LinkExternalToUser(user, externalLoginUser); err != nil { + if err := models.LinkExternalToUser(models.DefaultDBContext(), user, externalLoginUser); err != nil { return err }