Skip to content

WIP: remove unused user fields #15398

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

WIP: remove unused user fields #15398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions models/external_login_user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
125 changes: 60 additions & 65 deletions models/login_source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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,
})
}

// __________ _____ _____
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
84 changes: 84 additions & 0 deletions models/migrations/v179.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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()
}
54 changes: 29 additions & 25 deletions models/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Loading