refactor(handlers): add more detailed errors for password-change failures (#8899)

Adds some more helpful log information to the change password feature.

Signed-off-by: Brynn Crowley <littlehill723@gmail.com>
Signed-off-by: James Elliott <james-d-elliott@users.noreply.github.com>
Co-authored-by: James Elliott <james-d-elliott@users.noreply.github.com>

1 files changed, 62 insertions, 14 deletions

diff --git a/internal/handlers/handler_change_password.go b/internal/handlers/handler_change_password.go
index cbdd25ddc..cad4de1e5 100644
--- a/internal/handlers/handler_change_password.go
+++ b/internal/handlers/handler_change_password.go
@@ -2,7 +2,6 @@ package handlers
 
 import (
 	"errors"
-	"fmt"
 	"net/http"
 
 	"github.com/authelia/authelia/v4/internal/authentication"
@@ -14,11 +13,25 @@ import (
 func ChangePasswordPOST(ctx *middlewares.AutheliaCtx) {
 	var (
 		userSession session.UserSession
+		provider    *session.Session
 		err         error
 	)
 
-	if userSession, err = ctx.GetSession(); err != nil {
-		ctx.Error(fmt.Errorf("error occurred retrieving session for user: %w", err), messageUnableToChangePassword)
+	if provider, err = ctx.GetSessionProvider(); err != nil {
+		ctx.Logger.WithError(err).
+			Error("Unable to change password for user: error occurred retrieving session provider")
+		ctx.SetJSONError(messageUnableToChangePassword)
+		ctx.SetStatusCode(http.StatusInternalServerError)
+
+		return
+	}
+
+	if userSession, err = provider.GetSession(ctx.RequestCtx); err != nil {
+		ctx.Logger.WithError(err).
+			Error("Unable to change password for user: error occurred retrieving session for user")
+		ctx.SetJSONError(messageUnableToChangePassword)
+		ctx.SetStatusCode(http.StatusInternalServerError)
+
 		return
 	}
 
@@ -27,29 +40,49 @@ func ChangePasswordPOST(ctx *middlewares.AutheliaCtx) {
 	var requestBody changePasswordRequestBody
 
 	if err = ctx.ParseBody(&requestBody); err != nil {
-		ctx.Error(err, messageUnableToChangePassword)
+		ctx.Logger.WithError(err).
+			WithFields(map[string]any{"username": username}).
+			Error("Unable to change password for user: unable to parse request body")
+		ctx.SetJSONError(messageUnableToChangePassword)
+		ctx.SetStatusCode(http.StatusBadRequest)
+
 		return
 	}
 
 	if err = ctx.Providers.PasswordPolicy.Check(requestBody.NewPassword); err != nil {
-		ctx.Error(err, messagePasswordWeak)
+		ctx.Logger.WithError(err).
+			WithFields(map[string]any{"username": username}).
+			Debug("Unable to change password for user as their new password was weak or empty")
+		ctx.SetJSONError(messagePasswordWeak)
+		ctx.SetStatusCode(http.StatusBadRequest)
+
 		return
 	}
 
 	if err = ctx.Providers.UserProvider.ChangePassword(username, requestBody.OldPassword, requestBody.NewPassword); err != nil {
-		ctx.Logger.WithError(err).Debugf("Unable to change password for user '%s'", username)
-
 		switch {
 		case errors.Is(err, authentication.ErrIncorrectPassword):
+			ctx.Logger.WithError(err).
+				WithFields(map[string]any{"username": username}).
+				Debug("Unable to change password for user as their old password was incorrect")
 			ctx.SetJSONError(messageIncorrectPassword)
 			ctx.SetStatusCode(http.StatusUnauthorized)
 		case errors.Is(err, authentication.ErrPasswordWeak):
+			ctx.Logger.WithError(err).
+				WithFields(map[string]any{"username": username}).
+				Debug("Unable to change password for user as their new password was weak or empty")
 			ctx.SetJSONError(messagePasswordWeak)
 			ctx.SetStatusCode(http.StatusBadRequest)
 		case errors.Is(err, authentication.ErrAuthenticationFailed):
+			ctx.Logger.WithError(err).
+				WithFields(map[string]any{"username": username}).
+				Error("Unable to change password for user as authentication failed for the user")
 			ctx.SetJSONError(messageOperationFailed)
 			ctx.SetStatusCode(http.StatusUnauthorized)
 		default:
+			ctx.Logger.WithError(err).
+				WithFields(map[string]any{"username": username}).
+				Error("Unable to change password for user for an unknown reason")
 			ctx.SetJSONError(messageOperationFailed)
 			ctx.SetStatusCode(http.StatusInternalServerError)
 		}
@@ -57,10 +90,16 @@ func ChangePasswordPOST(ctx *middlewares.AutheliaCtx) {
 		return
 	}
 
-	ctx.Logger.Debugf("User %s has changed their password", username)
+	ctx.Logger.
+		WithFields(map[string]any{"username": username}).
+		Debug("User has changed their password")
+
+	if err = provider.SaveSession(ctx.RequestCtx, userSession); err != nil {
+		ctx.Logger.WithError(err).
+			WithFields(map[string]any{"username": username}).
+			Error("Unable to update password change state")
+		ctx.SetJSONError(messageOperationFailed)
 
-	if err = ctx.SaveSession(userSession); err != nil {
-		ctx.Error(fmt.Errorf("unable to update password reset state: %w", err), messageOperationFailed)
 		return
 	}
 
@@ -73,7 +112,8 @@ func ChangePasswordPOST(ctx *middlewares.AutheliaCtx) {
 	}
 
 	if len(userInfo.Emails) == 0 {
-		ctx.Logger.Error(fmt.Errorf("user %s has no email address configured", username))
+		ctx.Logger.WithFields(map[string]any{"username": username}).
+			Debug("user has no email address configured")
 		ctx.ReplyOK()
 
 		return
@@ -93,11 +133,19 @@ func ChangePasswordPOST(ctx *middlewares.AutheliaCtx) {
 
 	addresses := userInfo.Addresses()
 
-	ctx.Logger.Debugf("Sending an email to user %s (%s) to inform that the password has changed.",
-		username, addresses[0].String())
+	ctx.Logger.WithFields(map[string]any{
+		"username": username,
+		"email":    addresses[0].String(),
+	}).
+		Debug("Sending an email to inform user that their password has changed.")
 
 	if err = ctx.Providers.Notifier.Send(ctx, addresses[0], "Password changed successfully", ctx.Providers.Templates.GetEventEmailTemplate(), data); err != nil {
-		ctx.Logger.Error(err)
+		ctx.Logger.WithError(err).
+			WithFields(map[string]any{
+				"username": username,
+				"email":    addresses[0].String(),
+			}).
+			Debug("Unable to notify user of password change")
 		ctx.ReplyOK()
 
 		return