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

Unify password changing and invalidate auth tokens #27625

Merged
merged 9 commits into from
Feb 4, 2024
6 changes: 2 additions & 4 deletions cmd/admin_user_change_password.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
user_model "code.gitea.io/gitea/models/user"
pwd "code.gitea.io/gitea/modules/auth/password"
"code.gitea.io/gitea/modules/setting"
user_service "code.gitea.io/gitea/services/user"

"github.com/urfave/cli/v2"
)
Expand Down Expand Up @@ -65,11 +66,8 @@ func runChangePassword(c *cli.Context) error {
if err != nil {
return err
}
if err = user.SetPassword(c.String("password")); err != nil {
return err
}

if err = user_model.UpdateUserCols(ctx, user, "passwd", "passwd_hash_algo", "salt"); err != nil {
if err := user_service.ChangePassword(ctx, user, c.String("password"), user.MustChangePassword); err != nil {
return err
}

Expand Down
5 changes: 5 additions & 0 deletions models/auth/auth_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ func DeleteAuthTokenByID(ctx context.Context, id string) error {
return err
}

func DeleteAuthTokensByUserID(ctx context.Context, uid int64) error {
_, err := db.GetEngine(ctx).Where(builder.Eq{"user_id": uid}).Delete(&AuthToken{})
KN4CK3R marked this conversation as resolved.
Show resolved Hide resolved
return err
}

func DeleteExpiredAuthTokens(ctx context.Context) error {
_, err := db.GetEngine(ctx).Where(builder.Lt{"expires_unix": timeutil.TimeStampNow()}).Delete(&AuthToken{})
return err
Expand Down
15 changes: 6 additions & 9 deletions routers/api/v1/admin/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ func EditUser(ctx *context.APIContext) {
return
}

if form.MustChangePassword != nil {
ctx.ContextUser.MustChangePassword = *form.MustChangePassword
}

if len(form.Password) != 0 {
if len(form.Password) < setting.MinPasswordLength {
ctx.Error(http.StatusBadRequest, "PasswordTooShort", fmt.Errorf("password must be at least %d characters", setting.MinPasswordLength))
Expand All @@ -203,20 +207,13 @@ func EditUser(ctx *context.APIContext) {
ctx.Error(http.StatusBadRequest, "PasswordPwned", errors.New("PasswordPwned"))
return
}
if ctx.ContextUser.Salt, err = user_model.GetUserSalt(); err != nil {
ctx.Error(http.StatusInternalServerError, "UpdateUser", err)
return
}
if err = ctx.ContextUser.SetPassword(form.Password); err != nil {

if err := user_service.ChangePassword(ctx, ctx.ContextUser, form.Password, ctx.ContextUser.MustChangePassword); err != nil {
ctx.InternalServerError(err)
return
}
}

if form.MustChangePassword != nil {
ctx.ContextUser.MustChangePassword = *form.MustChangePassword
}

ctx.ContextUser.LoginName = form.LoginName

if form.FullName != nil {
Expand Down
8 changes: 2 additions & 6 deletions routers/web/admin/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,12 +381,8 @@ func EditUserPost(ctx *context.Context) {
return
}

if u.Salt, err = user_model.GetUserSalt(); err != nil {
ctx.ServerError("UpdateUser", err)
return
}
if err = u.SetPassword(form.Password); err != nil {
ctx.ServerError("SetPassword", err)
if err := user_service.ChangePassword(ctx, u, form.Password, u.MustChangePassword); err != nil {
KN4CK3R marked this conversation as resolved.
Show resolved Hide resolved
ctx.ServerError("ChangePassword", err)
return
}
}
Expand Down
21 changes: 5 additions & 16 deletions routers/web/auth/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"code.gitea.io/gitea/routers/utils"
"code.gitea.io/gitea/services/forms"
"code.gitea.io/gitea/services/mailer"
user_service "code.gitea.io/gitea/services/user"
)

var (
Expand Down Expand Up @@ -228,13 +229,8 @@ func ResetPasswdPost(ctx *context.Context) {
ctx.ServerError("UpdateUser", err)
return
}
if err = u.SetPassword(passwd); err != nil {
ctx.ServerError("UpdateUser", err)
return
}
u.MustChangePassword = false
if err := user_model.UpdateUserCols(ctx, u, "must_change_password", "passwd", "passwd_hash_algo", "rands", "salt"); err != nil {
ctx.ServerError("UpdateUser", err)
if err := user_service.ChangePassword(ctx, u, passwd, false); err != nil {
ctx.ServerError("ChangePassword", err)
return
}

Expand Down Expand Up @@ -321,15 +317,8 @@ func MustChangePasswordPost(ctx *context.Context) {
return
}

if err = u.SetPassword(form.Password); err != nil {
ctx.ServerError("UpdateUser", err)
return
}

u.MustChangePassword = false

if err := user_model.UpdateUserCols(ctx, u, "must_change_password", "passwd", "passwd_hash_algo", "salt"); err != nil {
ctx.ServerError("UpdateUser", err)
if err := user_service.ChangePassword(ctx, u, form.Password, false); err != nil {
ctx.ServerError("ChangePassword", err)
return
}

Expand Down
11 changes: 4 additions & 7 deletions routers/web/user/setting/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,13 @@ func AccountPost(ctx *context.Context) {
}
ctx.Flash.Error(errMsg)
} else {
var err error
if err = ctx.Doer.SetPassword(form.Password); err != nil {
ctx.ServerError("UpdateUser", err)
return
}
if err := user_model.UpdateUserCols(ctx, ctx.Doer, "salt", "passwd_hash_algo", "passwd"); err != nil {
ctx.ServerError("UpdateUser", err)
if err := user.ChangePassword(ctx, ctx.Doer, form.Password, false); err != nil {
ctx.ServerError("ChangePassword", err)
return
}

log.Trace("User password updated: %s", ctx.Doer.Name)

ctx.Flash.Success(ctx.Tr("settings.change_password_success"))
}

Expand Down
4 changes: 4 additions & 0 deletions services/user/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
}
// ***** END: ExternalLoginUser *****

if err := auth_model.DeleteAuthTokensByUserID(ctx, u.ID); err != nil {
return fmt.Errorf("DeleteAuthTokensByUserID: %w", err)
}

if _, err = db.DeleteByID(ctx, u.ID, new(user_model.User)); err != nil {
return fmt.Errorf("delete: %w", err)
}
Expand Down
16 changes: 16 additions & 0 deletions services/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"code.gitea.io/gitea/models"
asymkey_model "code.gitea.io/gitea/models/asymkey"
auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
packages_model "code.gitea.io/gitea/models/packages"
Expand Down Expand Up @@ -120,6 +121,21 @@ func RenameUser(ctx context.Context, u *user_model.User, newUserName string) err
return nil
}

// ChangePassword sets the users password and invalidates all existing auth tokens
func ChangePassword(ctx context.Context, u *user_model.User, password string, mustChangePassword bool) error {
if err := u.SetPassword(password); err != nil {
return err
}

u.MustChangePassword = mustChangePassword

if err := user_model.UpdateUserCols(ctx, u, "must_change_password", "passwd", "passwd_hash_algo", "rands", "salt"); err != nil {
return err
}

return auth_model.DeleteAuthTokensByUserID(ctx, u.ID)
}

// DeleteUser completely and permanently deletes everything of a user,
// but issues/comments/pulls will be kept and shown as someone has been deleted,
// unless the user is younger than USER_DELETE_WITH_COMMENTS_MAX_DAYS.
Expand Down