From 95294686b7fe8703e7fda632d15e16963ffbdfc2 Mon Sep 17 00:00:00 2001 From: Nikolai Papin Date: Sun, 13 Jul 2025 19:10:34 +0300 Subject: [PATCH] feat: PasswordResetBegin of auth controller; fix: sql query updateLoginInformationByUsername used in-database hashing; refactor: renamed LogOutAccounts into LogOutSessions in models/auth; refactor: added error checks on opening transactions for all auth service methods; refactor: added error checks on commiting transactions likewise; refactor: simplified PasswordResetBegin logic; feat: implemented PasswordResetComplete method of auth service; --- backend/internal/controllers/auth.go | 19 ++- backend/internal/database/query.sql.go | 12 +- backend/internal/models/auth.go | 2 +- backend/internal/services/auth.go | 201 +++++++++++++++++++++++-- sqlc/query.sql | 8 +- 5 files changed, 208 insertions(+), 34 deletions(-) diff --git a/backend/internal/controllers/auth.go b/backend/internal/controllers/auth.go index 0691f6f..e6b6c61 100644 --- a/backend/internal/controllers/auth.go +++ b/backend/internal/controllers/auth.go @@ -89,7 +89,24 @@ func (a *authControllerImpl) Login(c *gin.Context) { // @Success 200 "Reset code sent to the email if it is attached to an account" // @Failure 429 "Too many recent requests for this email" func (a *authControllerImpl) PasswordResetBegin(c *gin.Context) { - c.Status(http.StatusNotImplemented) + request, ok := utils.GetRequest[models.PasswordResetBeginRequest](c) + if !ok { + c.Status(http.StatusBadRequest) + return + } + + response, err := a.auth.PasswordResetBegin(request.Body) + if err != nil { + if errors.Is(err, errs.ErrTooManyRequests) { + c.Status(http.StatusTooManyRequests) + } else { + c.Status(http.StatusInternalServerError) + } + return + } + + c.JSON(http.StatusOK, response) + return } // @Summary Complete password reset via email code diff --git a/backend/internal/database/query.sql.go b/backend/internal/database/query.sql.go index fc7ef43..e36d045 100644 --- a/backend/internal/database/query.sql.go +++ b/backend/internal/database/query.sql.go @@ -865,13 +865,7 @@ const updateLoginInformationByUsername = `-- name: UpdateLoginInformationByUsern UPDATE login_informations SET email = COALESCE($2, email), - password_hash = COALESCE( - CASE - WHEN $3::text IS NOT NULL - THEN crypt($3::text, gen_salt('bf')) - END, - password_hash - ), + password_hash = COALESCE($3::text, password_hash), totp_encrypted = COALESCE($4, totp_encrypted), email_2fa_enabled = COALESCE($5, email_2fa_enabled), password_change_date = COALESCE($6, password_change_date) @@ -882,7 +876,7 @@ WHERE users.username = $1 AND login_informations.user_id = users.id type UpdateLoginInformationByUsernameParams struct { Username string Email *string - Password string + PasswordHash string TotpEncrypted *string Email2faEnabled *bool PasswordChangeDate pgtype.Timestamp @@ -892,7 +886,7 @@ func (q *Queries) UpdateLoginInformationByUsername(ctx context.Context, arg Upda _, err := q.db.Exec(ctx, updateLoginInformationByUsername, arg.Username, arg.Email, - arg.Password, + arg.PasswordHash, arg.TotpEncrypted, arg.Email2faEnabled, arg.PasswordChangeDate, diff --git a/backend/internal/models/auth.go b/backend/internal/models/auth.go index 503840f..7665b29 100644 --- a/backend/internal/models/auth.go +++ b/backend/internal/models/auth.go @@ -66,7 +66,7 @@ type PasswordResetCompleteRequest struct { Email string `json:"email" binding:"required,email"` VerificationCode string `json:"verification_code" binding:"required" validate:"verification_code=reset"` NewPassword string `json:"password" binding:"required" validate:"password"` - LogOutAccounts bool `json:"log_out_accounts"` + LogOutSessions bool `json:"log_out_sessions"` } type PasswordResetCompleteResponse struct { diff --git a/backend/internal/services/auth.go b/backend/internal/services/auth.go index d9aaa8d..81bdc02 100644 --- a/backend/internal/services/auth.go +++ b/backend/internal/services/auth.go @@ -65,7 +65,14 @@ func (a *authServiceImpl) RegistrationBegin(request models.RegistrationBeginRequ var passwordHash string var err error - helper, db, _ := database.NewDbHelperTransaction(a.dbctx) + helper, db, err := database.NewDbHelperTransaction(a.dbctx) + if err != nil { + a.log.Error( + "Failed to open a transaction", + zap.Error(err)) + return false, errs.ErrServerError + } + defer helper.RollbackOnError(err) // TODO: check occupation with redis @@ -184,7 +191,12 @@ func (a *authServiceImpl) RegistrationBegin(request models.RegistrationBeginRequ zap.String("code", generatedCode)) } - helper.Commit() + if err = helper.Commit(); err != nil { + a.log.Error( + "Failed to commit transaction", + zap.Error(err)) + return false, errs.ErrServerError + } return true, nil } @@ -198,7 +210,13 @@ func (a *authServiceImpl) RegistrationComplete(request models.RegistrationComple var accessToken, refreshToken string var err error - helper, db, _ := database.NewDbHelperTransaction(a.dbctx) + helper, db, err := database.NewDbHelperTransaction(a.dbctx) + if err != nil { + a.log.Error( + "Failed to open a transaction", + zap.Error(err)) + return nil, errs.ErrServerError + } defer helper.RollbackOnError(err) user, err = db.TXQueries.GetUserByUsername(db.CTX, request.Username) @@ -316,7 +334,12 @@ func (a *authServiceImpl) RegistrationComplete(request models.RegistrationComple return nil, errs.ErrServerError } - helper.Commit() + if err = helper.Commit(); err != nil { + a.log.Error( + "Failed to commit transaction", + zap.Error(err)) + return nil, errs.ErrServerError + } a.log.Info( "User verified registration", @@ -336,7 +359,13 @@ func (a *authServiceImpl) Login(request models.LoginRequest) (*models.LoginRespo var session database.Session var err error - helper, db, _ := database.NewDbHelperTransaction(a.dbctx) + helper, db, err := database.NewDbHelperTransaction(a.dbctx) + if err != nil { + a.log.Error( + "Failed to open a transaction", + zap.Error(err)) + return nil, errs.ErrServerError + } defer helper.RollbackOnError(err) userRow, err = db.TXQueries.GetValidUserByLoginCredentials(db.CTX, database.GetValidUserByLoginCredentialsParams{ @@ -395,7 +424,12 @@ func (a *authServiceImpl) Login(request models.LoginRequest) (*models.LoginRespo return nil, errs.ErrServerError } - helper.Commit() + if err = helper.Commit(); err != nil { + a.log.Error( + "Failed to commit transaction", + zap.Error(err)) + return nil, errs.ErrServerError + } response := models.LoginResponse{Tokens: models.Tokens{ AccessToken: accessToken, @@ -416,6 +450,12 @@ func (a *authServiceImpl) PasswordResetBegin(request models.PasswordResetBeginRe var err error helper, db, err := database.NewDbHelperTransaction(a.dbctx) + if err != nil { + a.log.Error( + "Failed to open a transaction", + zap.Error(err)) + return false, errs.ErrServerError + } defer helper.RollbackOnError(err) ctx := context.TODO() @@ -427,15 +467,13 @@ func (a *authServiceImpl) PasswordResetBegin(request models.PasswordResetBeginRe zap.String("email", request.Email), zap.Error(redisErr)) return false, errs.ErrServerError + } - } else if err == nil { - current_time := time.Now() - if current_time.Unix() < cooldownTimeUnix { - a.log.Warn( - "Attempted to request a new password reset code for email on active reset cooldown", - zap.String("email", request.Email)) - return false, errs.ErrTooManyRequests - } + if time.Now().Unix() < cooldownTimeUnix { + a.log.Warn( + "Attempted to request a new password reset code for email on active reset cooldown", + zap.String("email", request.Email)) + return false, errs.ErrTooManyRequests } if user, err = db.TXQueries.GetUserByEmail(db.CTX, request.Email); err != nil { @@ -501,13 +539,144 @@ func (a *authServiceImpl) PasswordResetBegin(request models.PasswordResetBeginRe return false, err } - helper.Commit() + if err = helper.Commit(); err != nil { + a.log.Error( + "Failed to commit transaction", + zap.Error(err)) + return false, errs.ErrServerError + } return true, nil } func (a *authServiceImpl) PasswordResetComplete(request models.PasswordResetCompleteRequest) (*models.PasswordResetCompleteResponse, error) { - return nil, errs.ErrNotImplemented + var resetCode database.ConfirmationCode + var user database.User + var session database.Session + var hashedPassword, accessToken, refreshToken string + var err error + + helper, db, err := database.NewDbHelperTransaction(a.dbctx) + if err != nil { + a.log.Error( + "Failed to open a transaction", + zap.Error(err)) + return nil, errs.ErrServerError + } + + if user, err = db.TXQueries.GetUserByEmail(db.CTX, request.Email); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + a.log.Warn( + "Attempted to complete password reset for unregistered email", + zap.String("email", request.Email), + zap.Error(err)) + return nil, errs.ErrForbidden + } + + a.log.Error( + "Failed to look up user of email while trying to complete password reset", + zap.String("email", request.Email), + zap.Error(err)) + return nil, errs.ErrServerError + } + + if resetCode, err = db.TXQueries.GetValidConfirmationCodeByCode(db.CTX, database.GetValidConfirmationCodeByCodeParams{ + UserID: user.ID, + CodeType: int32(enums.PasswordResetCodeType), + Code: request.VerificationCode, + }); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + a.log.Warn( + "Attempted to reset password for user using incorrect confirmation code", + zap.String("email", request.Email), + zap.String("username", user.Username), + zap.String("provided_code", request.VerificationCode), + zap.Error(err)) + return nil, errs.ErrForbidden + } + } + + if err = db.TXQueries.UpdateConfirmationCode(db.CTX, database.UpdateConfirmationCodeParams{ + ID: resetCode.ID, + Used: utils.NewPointer(true), + }); err != nil { + a.log.Error( + "Failed to invalidate password reset code upon use", + zap.String("username", user.Username), + zap.String("email", request.Email), + zap.Int64("code_id", resetCode.ID), + zap.Error(err)) + return nil, errs.ErrServerError + } + + if hashedPassword, err = utils.HashPassword(request.NewPassword); err != nil { + a.log.Error( + "Failed to hash new password as part of user password reset", + zap.String("email", request.Email), + zap.String("username", user.Username), + zap.Error(err)) + return nil, errs.ErrServerError + } + + if err = db.TXQueries.UpdateLoginInformationByUsername(db.CTX, database.UpdateLoginInformationByUsernameParams{ + Username: user.Username, + PasswordHash: hashedPassword, + }); err != nil { + a.log.Error( + "Failed to save new password to database as part of user password reset", + zap.String("username", user.Username), + zap.String("email", request.Email), + zap.Error(err)) + } + + if request.LogOutSessions { + if err = db.TXQueries.TerminateAllSessionsForUserByUsername(db.CTX, user.Username); err != nil { + a.log.Error( + "Failed to log out older sessions as part of user password reset", + zap.String("email", request.Email), + zap.String("username", user.Username), + zap.Error(err)) + return nil, errs.ErrServerError + } + } + + if session, err = db.TXQueries.CreateSession(db.CTX, database.CreateSessionParams{ + UserID: user.ID, + Name: utils.NewPointer("First device"), + Platform: utils.NewPointer("Unknown"), + LatestIp: utils.NewPointer("Unknown"), + }); err != nil { + a.log.Error( + "Failed to create new session for user as part of user password reset", + zap.String("email", request.Email), + zap.String("username", user.Username), + zap.Error(err)) + } + + if accessToken, refreshToken, err = utils.GenerateTokens(user.Username, session.Guid.String()); err != nil { + a.log.Error( + "Failed to generate tokens as part of user password reset", + zap.String("email", request.Email), + zap.String("username", user.Username), + zap.Error(err)) + return nil, errs.ErrServerError + } + + response := models.PasswordResetCompleteResponse{ + Tokens: models.Tokens{ + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + } + + if err = helper.Commit(); err != nil { + a.log.Error( + "Failed to commit transaction", + zap.Error(err)) + return nil, errs.ErrServerError + } + + return &response, nil } diff --git a/sqlc/query.sql b/sqlc/query.sql index b5f0364..b91c5a5 100644 --- a/sqlc/query.sql +++ b/sqlc/query.sql @@ -174,13 +174,7 @@ VALUES ( $1, $2, @password_hash::text ) RETURNING *; UPDATE login_informations SET email = COALESCE($2, email), - password_hash = COALESCE( - CASE - WHEN @password::text IS NOT NULL - THEN crypt(@password::text, gen_salt('bf')) - END, - password_hash - ), + password_hash = COALESCE(@password_hash::text, password_hash), totp_encrypted = COALESCE($4, totp_encrypted), email_2fa_enabled = COALESCE($5, email_2fa_enabled), password_change_date = COALESCE($6, password_change_date)