diff options
Diffstat (limited to 'internal/handlers')
| -rw-r--r-- | internal/handlers/const.go | 1 | ||||
| -rw-r--r-- | internal/handlers/handler_change_password.go | 76 | ||||
| -rw-r--r-- | internal/handlers/handler_change_password_test.go | 73 |
3 files changed, 111 insertions, 39 deletions
diff --git a/internal/handlers/const.go b/internal/handlers/const.go index 884095d5f..ac7a48523 100644 --- a/internal/handlers/const.go +++ b/internal/handlers/const.go @@ -69,7 +69,6 @@ const ( messageUnableToRegisterSecurityKey = "Unable to register your security key." messageSecurityKeyDuplicateName = "Another one of your security keys is already registered with that display name." messageUnableToResetPassword = "Unable to reset your password." - messageCannotReusePassword = "You cannot reuse your old password." messageUnableToChangePassword = "Unable to change your password." messageIncorrectPassword = "Incorrect Password" messageMFAValidationFailed = "Authentication failed, please retry later." 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 diff --git a/internal/handlers/handler_change_password_test.go b/internal/handlers/handler_change_password_test.go index 3783fef8e..920b2975e 100644 --- a/internal/handlers/handler_change_password_test.go +++ b/internal/handlers/handler_change_password_test.go @@ -3,15 +3,15 @@ package handlers import ( "encoding/json" "fmt" + "regexp" "testing" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/suite" "github.com/valyala/fasthttp" "go.uber.org/mock/gomock" "github.com/authelia/authelia/v4/internal/authentication" - "github.com/authelia/authelia/v4/internal/configuration/schema" "github.com/authelia/authelia/v4/internal/middlewares" "github.com/authelia/authelia/v4/internal/mocks" @@ -22,30 +22,13 @@ const ( testPasswordNew = "new_password456" ) -type ChangePasswordSuite struct { - suite.Suite - mock *mocks.MockAutheliaCtx -} - -func (s *ChangePasswordSuite) SetupTest() { - s.mock = mocks.NewMockAutheliaCtx(s.T()) - userSession, err := s.mock.Ctx.GetSession() - s.Assert().NoError(err) - - userSession.Username = testUsername - userSession.DisplayName = testUsername - userSession.Emails[0] = testEmail - userSession.AuthenticationMethodRefs.UsernameAndPassword = true - s.Assert().NoError(s.mock.Ctx.SaveSession(userSession)) -} - -func (s *ChangePasswordSuite) TearDownTest() { - s.mock.Close() -} - func TestChangePasswordPOST_ShouldSucceedWithValidCredentials(t *testing.T) { mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + mock.Ctx.Logger.Logger.SetLevel(logrus.DebugLevel) + userSession, err := mock.Ctx.GetSession() assert.NoError(t, err) @@ -83,12 +66,18 @@ func TestChangePasswordPOST_ShouldSucceedWithValidCredentials(t *testing.T) { ChangePasswordPOST(mock.Ctx) + mock.AssertLogEntryAdvanced(t, 1, logrus.DebugLevel, "User has changed their password", map[string]any{"username": testUsername}) + assert.Equal(t, fasthttp.StatusOK, mock.Ctx.Response.StatusCode()) } func TestChangePasswordPOST_ShouldFailWhenPasswordPolicyNotMet(t *testing.T) { mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + mock.Ctx.Logger.Logger.SetLevel(logrus.DebugLevel) + userSession, err := mock.Ctx.GetSession() assert.NoError(t, err) @@ -124,6 +113,8 @@ func TestChangePasswordPOST_ShouldFailWhenPasswordPolicyNotMet(t *testing.T) { ChangePasswordPOST(mock.Ctx) + mock.AssertLogEntryAdvanced(t, 0, logrus.DebugLevel, "Unable to change password for user as their new password was weak or empty", map[string]any{"username": testUsername, "error": "the supplied password does not met the security policy"}) + errResponse := mock.GetResponseError(t) assert.Equal(t, "KO", errResponse.Status) @@ -133,6 +124,10 @@ func TestChangePasswordPOST_ShouldFailWhenPasswordPolicyNotMet(t *testing.T) { func TestChangePasswordPOST_ShouldFailWhenRequestBodyIsInvalid(t *testing.T) { mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + mock.Ctx.Logger.Logger.SetLevel(logrus.DebugLevel) + userSession, err := mock.Ctx.GetSession() assert.NoError(t, err) @@ -144,6 +139,8 @@ func TestChangePasswordPOST_ShouldFailWhenRequestBodyIsInvalid(t *testing.T) { ChangePasswordPOST(mock.Ctx) + mock.AssertLogEntryAdvanced(t, 0, logrus.ErrorLevel, "Unable to change password for user: unable to parse request body", map[string]any{"username": testUsername, "error": regexp.MustCompile(`^(unable to parse body: .+|unable to validate body: .+|Body is not valid)$`)}) + errResponse := mock.GetResponseError(t) assert.Equal(t, "KO", errResponse.Status) assert.Equal(t, messageUnableToChangePassword, errResponse.Message) @@ -152,6 +149,10 @@ func TestChangePasswordPOST_ShouldFailWhenRequestBodyIsInvalid(t *testing.T) { func TestChangePasswordPOST_ShouldFailWhenOldPasswordIsIncorrect(t *testing.T) { mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + mock.Ctx.Logger.Logger.SetLevel(logrus.DebugLevel) + userSession, err := mock.Ctx.GetSession() assert.NoError(t, err) @@ -179,6 +180,11 @@ func TestChangePasswordPOST_ShouldFailWhenOldPasswordIsIncorrect(t *testing.T) { ChangePasswordPOST(mock.Ctx) + mock.AssertLogEntryAdvanced(t, 0, logrus.DebugLevel, "Unable to change password for user as their old password was incorrect", map[string]any{"username": testUsername, "error": "incorrect password"}) + + errorField := mock.Hook.LastEntry().Data["error"] + assert.ErrorIs(t, authentication.ErrIncorrectPassword, errorField.(error)) + errResponse := mock.GetResponseError(t) assert.Equal(t, "KO", errResponse.Status) assert.Equal(t, messageIncorrectPassword, errResponse.Message) @@ -187,6 +193,10 @@ func TestChangePasswordPOST_ShouldFailWhenOldPasswordIsIncorrect(t *testing.T) { func TestChangePasswordPOST_ShouldFailWhenPasswordReuseIsNotAllowed(t *testing.T) { mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + mock.Ctx.Logger.Logger.SetLevel(logrus.DebugLevel) + userSession, err := mock.Ctx.GetSession() assert.NoError(t, err) @@ -214,6 +224,11 @@ func TestChangePasswordPOST_ShouldFailWhenPasswordReuseIsNotAllowed(t *testing.T ChangePasswordPOST(mock.Ctx) + mock.AssertLogEntryAdvanced(t, 0, logrus.DebugLevel, "Unable to change password for user as their new password was weak or empty", map[string]any{"username": testUsername, "error": "your supplied password does not meet the password policy requirements"}) + + errorField := mock.Hook.LastEntry().Data["error"] + assert.ErrorIs(t, authentication.ErrPasswordWeak, errorField.(error)) + errResponse := mock.GetResponseError(t) assert.Equal(t, "KO", errResponse.Status) assert.Equal(t, messagePasswordWeak, errResponse.Message) @@ -221,6 +236,9 @@ func TestChangePasswordPOST_ShouldFailWhenPasswordReuseIsNotAllowed(t *testing.T func TestChangePasswordPOST_ShouldSucceedButLogErrorWhenUserHasNoEmail(t *testing.T) { mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + mock.Ctx.Logger.Logger.SetLevel(logrus.DebugLevel) userSession, err := mock.Ctx.GetSession() assert.NoError(t, err) @@ -255,11 +273,16 @@ func TestChangePasswordPOST_ShouldSucceedButLogErrorWhenUserHasNoEmail(t *testin ChangePasswordPOST(mock.Ctx) + mock.AssertLogEntryAdvanced(t, 1, logrus.DebugLevel, "User has changed their password", map[string]any{"username": testUsername}) + assert.Equal(t, fasthttp.StatusOK, mock.Ctx.Response.StatusCode()) } func TestChangePasswordPOST_ShouldSucceedButLogErrorWhenNotificationFails(t *testing.T) { mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + mock.Ctx.Logger.Logger.SetLevel(logrus.DebugLevel) userSession, err := mock.Ctx.GetSession() assert.NoError(t, err) @@ -294,9 +317,11 @@ func TestChangePasswordPOST_ShouldSucceedButLogErrorWhenNotificationFails(t *tes mock.NotifierMock.EXPECT(). Send(mock.Ctx, gomock.Any(), "Password changed successfully", gomock.Any(), gomock.Any()). - Return(fmt.Errorf("failed to send notification")) + Return(fmt.Errorf("notifier: smtp: failed to send message: connection refused")) ChangePasswordPOST(mock.Ctx) + mock.AssertLogEntryAdvanced(t, 0, logrus.DebugLevel, "Unable to notify user of password change", map[string]any{"username": testUsername, "email": nil, "error": regexp.MustCompile(`^notifier: smtp: failed to .*: .+$`)}) + assert.Equal(t, fasthttp.StatusOK, mock.Ctx.Response.StatusCode()) } |
