From 92d328926d96b8d20c44042675a7e5243fb6aeed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Michaud?= Date: Fri, 17 Sep 2021 07:53:40 +0200 Subject: [PATCH] refactor(handlers): lower case error messages (#2289) * refactor(handlers): lower case error messages also refactor verifyAuth function to detect malicious activity both with session cookie and authorization header. * refacto(handlers): simplify error construction * fix(handlers): check prefix in authorization header to determine auth method * fix(handlers): determining the method should be done with headers instead of query arg * refacto(handlers): rollback changes of verifyAuth * don't lowercase log messages * Apply suggestions from code review Make sure logger errors are not lowercased. * fix: uppercase logger errors and remove unused param * Do not lowercase logger errors * Remove unused param targetURL * Rename url variable to not conflict with imported package Co-authored-by: Amir Zarrinkafsh --- internal/handlers/errors.go | 4 +- .../handler_checks_safe_redirection.go | 6 +- internal/handlers/handler_configuration.go | 1 - internal/handlers/handler_firstfactor.go | 22 +++---- internal/handlers/handler_firstfactor_test.go | 6 +- internal/handlers/handler_logout.go | 10 +-- .../handlers/handler_oidc_authorization.go | 8 +-- internal/handlers/handler_oidc_consent.go | 10 +-- internal/handlers/handler_oidc_userinfo.go | 2 +- internal/handlers/handler_register_totp.go | 6 +- .../handlers/handler_register_u2f_step1.go | 4 +- .../handler_register_u2f_step1_test.go | 4 +- .../handlers/handler_register_u2f_step2.go | 6 +- .../handlers/handler_reset_password_step1.go | 2 +- .../handlers/handler_reset_password_step2.go | 8 +-- internal/handlers/handler_sign_duo.go | 4 +- internal/handlers/handler_sign_totp.go | 10 +-- internal/handlers/handler_sign_u2f_step1.go | 10 +-- .../handlers/handler_sign_u2f_step1_test.go | 4 +- internal/handlers/handler_sign_u2f_step2.go | 4 +- internal/handlers/handler_user_info.go | 6 +- internal/handlers/handler_user_info_test.go | 6 +- internal/handlers/handler_verify.go | 64 +++++++++---------- internal/handlers/handler_verify_test.go | 9 ++- internal/handlers/response.go | 12 ++-- .../http_to_authelia_handler_adaptor.go | 2 +- 26 files changed, 112 insertions(+), 118 deletions(-) diff --git a/internal/handlers/errors.go b/internal/handlers/errors.go index aa7dbbc81..cd499ce3c 100644 --- a/internal/handlers/errors.go +++ b/internal/handlers/errors.go @@ -9,5 +9,5 @@ const InternalError = "Internal error." // UnauthorizedError is the error message sent when the user is not authorized. const UnauthorizedError = "You're not authorized." -var errMissingXForwardedHost = errors.New("Missing header X-Forwarded-Host") -var errMissingXForwardedProto = errors.New("Missing header X-Forwarded-Proto") +var errMissingXForwardedHost = errors.New("missing header X-Forwarded-Host") +var errMissingXForwardedProto = errors.New("missing header X-Forwarded-Proto") diff --git a/internal/handlers/handler_checks_safe_redirection.go b/internal/handlers/handler_checks_safe_redirection.go index 49d33190e..a9fe93605 100644 --- a/internal/handlers/handler_checks_safe_redirection.go +++ b/internal/handlers/handler_checks_safe_redirection.go @@ -21,13 +21,13 @@ func CheckSafeRedirection(ctx *middlewares.AutheliaCtx) { err := ctx.ParseBody(&reqBody) if err != nil { - ctx.Error(fmt.Errorf("Unable to parse request body: %w", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to parse request body: %w", err), messageOperationFailed) return } safe, err := utils.IsRedirectionURISafe(reqBody.URI, ctx.Configuration.Session.Domain) if err != nil { - ctx.Error(fmt.Errorf("Unable to determine if uri %s is safe to redirect to: %w", reqBody.URI, err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to determine if uri %s is safe to redirect to: %w", reqBody.URI, err), messageOperationFailed) return } @@ -35,7 +35,7 @@ func CheckSafeRedirection(ctx *middlewares.AutheliaCtx) { OK: safe, }) if err != nil { - ctx.Error(fmt.Errorf("Unable to create response body: %w", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to create response body: %w", err), messageOperationFailed) return } } diff --git a/internal/handlers/handler_configuration.go b/internal/handlers/handler_configuration.go index 7d5e35ba1..8ad2dd3f2 100644 --- a/internal/handlers/handler_configuration.go +++ b/internal/handlers/handler_configuration.go @@ -25,7 +25,6 @@ func ConfigurationGet(ctx *middlewares.AutheliaCtx) { body.SecondFactorEnabled = ctx.Providers.Authorizer.IsSecondFactorEnabled() ctx.Logger.Tracef("Second factor enabled: %v", body.SecondFactorEnabled) - ctx.Logger.Tracef("Available methods are %s", body.AvailableMethods) err := ctx.SetJSONBody(body) diff --git a/internal/handlers/handler_firstfactor.go b/internal/handlers/handler_firstfactor.go index 01dcc368c..b131c41dc 100644 --- a/internal/handlers/handler_firstfactor.go +++ b/internal/handlers/handler_firstfactor.go @@ -33,7 +33,7 @@ func calculateActualDelay(ctx *middlewares.AutheliaCtx, execDuration time.Durati randomDelayMs := float64(rand.Int63n(loginDelayMaximumRandomDelayMilliseconds)) //nolint:gosec // TODO: Consider use of crypto/rand, this should be benchmarked and measured first. totalDelayMs := math.Max(avgExecDurationMs, loginDelayMinimumDelayMilliseconds) + randomDelayMs actualDelayMs := math.Max(totalDelayMs-float64(execDuration.Milliseconds()), 1.0) - ctx.Logger.Tracef("attempt successful: %t, exec duration: %d, avg execution duration: %d, random delay ms: %d, total delay ms: %d, actual delay ms: %d", *successful, execDuration.Milliseconds(), int64(avgExecDurationMs), int64(randomDelayMs), int64(totalDelayMs), int64(actualDelayMs)) + ctx.Logger.Tracef("Attempt successful: %t, exec duration: %d, avg execution duration: %d, random delay ms: %d, total delay ms: %d, actual delay ms: %d", *successful, execDuration.Milliseconds(), int64(avgExecDurationMs), int64(randomDelayMs), int64(totalDelayMs), int64(actualDelayMs)) return actualDelayMs } @@ -81,11 +81,11 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware if err != nil { if err == regulation.ErrUserIsBanned { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("User %s is banned until %s", bodyJSON.Username, bannedUntil), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("user %s is banned until %s", bodyJSON.Username, bannedUntil), messageAuthenticationFailed) return } - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regulate authentication: %s", err.Error()), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to regulate authentication: %s", err.Error()), messageAuthenticationFailed) return } @@ -99,7 +99,7 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware ctx.Logger.Errorf("Unable to mark authentication: %s", err.Error()) } - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Error while checking password for user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("error while checking password for user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) return } @@ -111,7 +111,7 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware ctx.Logger.Errorf("Unable to mark authentication: %s", err.Error()) } - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Credentials are wrong for user %s", bodyJSON.Username), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("credentials are wrong for user %s", bodyJSON.Username), messageAuthenticationFailed) return } @@ -120,7 +120,7 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware err = ctx.Providers.Regulator.Mark(bodyJSON.Username, true) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to mark authentication: %s", err.Error()), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to mark authentication: %s", err.Error()), messageAuthenticationFailed) return } @@ -134,14 +134,14 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware err = ctx.SaveSession(newSession) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to reset the session for user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to reset the session for user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) return } err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regenerate session for user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to regenerate session for user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) return } @@ -152,7 +152,7 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware if keepMeLoggedIn { err = ctx.Providers.SessionProvider.UpdateExpiration(ctx.RequestCtx, ctx.Providers.SessionProvider.RememberMe) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to update expiration timer for user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to update expiration timer for user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) return } } @@ -161,7 +161,7 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware userDetails, err := ctx.Providers.UserProvider.GetDetails(bodyJSON.Username) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Error while retrieving details from user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("error while retrieving details from user %s: %s", bodyJSON.Username, err.Error()), messageAuthenticationFailed) return } @@ -175,7 +175,7 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware err = ctx.SaveSession(userSession) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to save session of user %s", bodyJSON.Username), messageAuthenticationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to save session of user %s", bodyJSON.Username), messageAuthenticationFailed) return } diff --git a/internal/handlers/handler_firstfactor_test.go b/internal/handlers/handler_firstfactor_test.go index fab193e97..4902ebbbc 100644 --- a/internal/handlers/handler_firstfactor_test.go +++ b/internal/handlers/handler_firstfactor_test.go @@ -71,7 +71,7 @@ func (s *FirstFactorSuite) TestShouldFailIfUserProviderCheckPasswordFail() { }`) FirstFactorPost(0, false)(s.mock.Ctx) - assert.Equal(s.T(), "Error while checking password for user test: Failed", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "error while checking password for user test: Failed", s.mock.Hook.LastEntry().Message) s.mock.Assert401KO(s.T(), "Authentication failed. Check your credentials.") } @@ -121,7 +121,7 @@ func (s *FirstFactorSuite) TestShouldFailIfUserProviderGetDetailsFail() { }`) FirstFactorPost(0, false)(s.mock.Ctx) - assert.Equal(s.T(), "Error while retrieving details from user test: Failed", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "error while retrieving details from user test: Failed", s.mock.Hook.LastEntry().Message) s.mock.Assert401KO(s.T(), "Authentication failed. Check your credentials.") } @@ -143,7 +143,7 @@ func (s *FirstFactorSuite) TestShouldFailIfAuthenticationMarkFail() { }`) FirstFactorPost(0, false)(s.mock.Ctx) - assert.Equal(s.T(), "Unable to mark authentication: failed", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "unable to mark authentication: failed", s.mock.Hook.LastEntry().Message) s.mock.Assert401KO(s.T(), "Authentication failed. Check your credentials.") } diff --git a/internal/handlers/handler_logout.go b/internal/handlers/handler_logout.go index 5b5579439..007ef0244 100644 --- a/internal/handlers/handler_logout.go +++ b/internal/handlers/handler_logout.go @@ -21,18 +21,14 @@ func LogoutPost(ctx *middlewares.AutheliaCtx) { body := logoutBody{} responseBody := logoutResponseBody{SafeTargetURL: false} - ctx.Logger.Tracef("Attempting to decode body") - err := ctx.ParseBody(&body) if err != nil { - ctx.Error(fmt.Errorf("Unable to parse body during logout: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to parse body during logout: %s", err), messageOperationFailed) } - ctx.Logger.Tracef("Attempting to destroy session") - err = ctx.Providers.SessionProvider.DestroySession(ctx.RequestCtx) if err != nil { - ctx.Error(fmt.Errorf("Unable to destroy session during logout: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to destroy session during logout: %s", err), messageOperationFailed) } redirectionURL, err := url.Parse(body.TargetURL) @@ -46,6 +42,6 @@ func LogoutPost(ctx *middlewares.AutheliaCtx) { err = ctx.SetJSONBody(responseBody) if err != nil { - ctx.Error(fmt.Errorf("Unable to set body during logout: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to set body during logout: %s", err), messageOperationFailed) } } diff --git a/internal/handlers/handler_oidc_authorization.go b/internal/handlers/handler_oidc_authorization.go index 9c04a52ad..83501b1e8 100644 --- a/internal/handlers/handler_oidc_authorization.go +++ b/internal/handlers/handler_oidc_authorization.go @@ -30,7 +30,7 @@ func oidcAuthorization(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r * client, err := ctx.Providers.OpenIDConnect.Store.GetInternalClient(clientID) if err != nil { - err := fmt.Errorf("Unable to find related client configuration with name '%s': %v", ar.GetID(), err) + err := fmt.Errorf("unable to find related client configuration with name '%s': %v", ar.GetID(), err) ctx.Logger.Error(err) ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, ar, err) @@ -56,7 +56,7 @@ func oidcAuthorization(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r * userSession.OIDCWorkflowSession = nil if err := ctx.SaveSession(userSession); err != nil { - ctx.Logger.Errorf("%v", err) + ctx.Logger.Error(err) http.Error(rw, err.Error(), http.StatusInternalServerError) return @@ -147,7 +147,7 @@ func oidcAuthorizeHandleAuthorizationOrConsentInsufficient( ar fosite.AuthorizeRequester) { issuer, err := ctx.ExternalRootURL() if err != nil { - ctx.Logger.Errorf("%v", err) + ctx.Logger.Error(err) http.Error(rw, err.Error(), http.StatusBadRequest) return @@ -169,7 +169,7 @@ func oidcAuthorizeHandleAuthorizationOrConsentInsufficient( } if err := ctx.SaveSession(userSession); err != nil { - ctx.Logger.Errorf("%v", err) + ctx.Logger.Errorf("Unable to save session: %v", err) http.Error(rw, err.Error(), http.StatusInternalServerError) return diff --git a/internal/handlers/handler_oidc_consent.go b/internal/handlers/handler_oidc_consent.go index 7d84493ff..2cf604c90 100644 --- a/internal/handlers/handler_oidc_consent.go +++ b/internal/handlers/handler_oidc_consent.go @@ -35,7 +35,7 @@ func oidcConsent(ctx *middlewares.AutheliaCtx) { } if err := ctx.SetJSONBody(client.GetConsentResponseBody(userSession.OIDCWorkflowSession)); err != nil { - ctx.Error(fmt.Errorf("Unable to set JSON body: %v", err), "Operation failed") + ctx.Error(fmt.Errorf("unable to set JSON body: %v", err), "Operation failed") } } @@ -69,7 +69,7 @@ func oidcConsentPOST(ctx *middlewares.AutheliaCtx) { err = json.Unmarshal(ctx.Request.Body(), &body) if err != nil { - ctx.Error(fmt.Errorf("Unable to unmarshal body: %v", err), "Operation failed") + ctx.Error(fmt.Errorf("unable to unmarshal body: %v", err), "Operation failed") return } @@ -96,7 +96,7 @@ func oidcConsentPOST(ctx *middlewares.AutheliaCtx) { userSession.OIDCWorkflowSession.GrantedAudience = userSession.OIDCWorkflowSession.RequestedAudience if err := ctx.SaveSession(userSession); err != nil { - ctx.Error(fmt.Errorf("Unable to write session: %v", err), "Operation failed") + ctx.Error(fmt.Errorf("unable to write session: %v", err), "Operation failed") return } } else if body.AcceptOrReject == reject { @@ -105,7 +105,7 @@ func oidcConsentPOST(ctx *middlewares.AutheliaCtx) { userSession.OIDCWorkflowSession = nil if err := ctx.SaveSession(userSession); err != nil { - ctx.Error(fmt.Errorf("Unable to write session: %v", err), "Operation failed") + ctx.Error(fmt.Errorf("unable to write session: %v", err), "Operation failed") return } } @@ -113,6 +113,6 @@ func oidcConsentPOST(ctx *middlewares.AutheliaCtx) { response := ConsentPostResponseBody{RedirectURI: redirectionURL} if err := ctx.SetJSONBody(response); err != nil { - ctx.Error(fmt.Errorf("Unable to set JSON body in response"), "Operation failed") + ctx.Error(fmt.Errorf("unable to set JSON body in response"), "Operation failed") } } diff --git a/internal/handlers/handler_oidc_userinfo.go b/internal/handlers/handler_oidc_userinfo.go index ff176f60b..84a8a0470 100644 --- a/internal/handlers/handler_oidc_userinfo.go +++ b/internal/handlers/handler_oidc_userinfo.go @@ -30,7 +30,7 @@ func oidcUserinfo(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, req *htt } if tokenType != fosite.AccessToken { - errStr := "Authorization header must contain an OAuth access token." + errStr := "authorization header must contain an OAuth access token." rw.Header().Set("WWW-Authenticate", fmt.Sprintf("error_description=\"%s\"", errStr)) ctx.Providers.OpenIDConnect.WriteErrorCode(rw, req, http.StatusUnauthorized, errors.New(errStr)) diff --git a/internal/handlers/handler_register_totp.go b/internal/handlers/handler_register_totp.go index 8ce19274e..784ae7378 100644 --- a/internal/handlers/handler_register_totp.go +++ b/internal/handlers/handler_register_totp.go @@ -14,7 +14,7 @@ func identityRetrieverFromSession(ctx *middlewares.AutheliaCtx) (*session.Identi userSession := ctx.GetSession() if len(userSession.Emails) == 0 { - return nil, fmt.Errorf("User %s does not have any email address", userSession.Username) + return nil, fmt.Errorf("user %s does not have any email address", userSession.Username) } return &session.Identity{ @@ -45,13 +45,13 @@ func secondFactorTOTPIdentityFinish(ctx *middlewares.AutheliaCtx, username strin }) if err != nil { - ctx.Error(fmt.Errorf("Unable to generate TOTP key: %s", err), messageUnableToRegisterOneTimePassword) + ctx.Error(fmt.Errorf("unable to generate TOTP key: %s", err), messageUnableToRegisterOneTimePassword) return } err = ctx.Providers.StorageProvider.SaveTOTPSecret(username, key.Secret()) if err != nil { - ctx.Error(fmt.Errorf("Unable to save TOTP secret in DB: %s", err), messageUnableToRegisterOneTimePassword) + ctx.Error(fmt.Errorf("unable to save TOTP secret in DB: %s", err), messageUnableToRegisterOneTimePassword) return } diff --git a/internal/handlers/handler_register_u2f_step1.go b/internal/handlers/handler_register_u2f_step1.go index b0adfd543..b779cf919 100644 --- a/internal/handlers/handler_register_u2f_step1.go +++ b/internal/handlers/handler_register_u2f_step1.go @@ -42,7 +42,7 @@ func secondFactorU2FIdentityFinish(ctx *middlewares.AutheliaCtx, username string challenge, err := u2f.NewChallenge(appID, trustedFacets) if err != nil { - ctx.Error(fmt.Errorf("Unable to generate new U2F challenge for registration: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to generate new U2F challenge for registration: %s", err), messageOperationFailed) return } @@ -52,7 +52,7 @@ func secondFactorU2FIdentityFinish(ctx *middlewares.AutheliaCtx, username string err = ctx.SaveSession(userSession) if err != nil { - ctx.Error(fmt.Errorf("Unable to save U2F challenge in session: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to save U2F challenge in session: %s", err), messageOperationFailed) return } diff --git a/internal/handlers/handler_register_u2f_step1_test.go b/internal/handlers/handler_register_u2f_step1_test.go index ba8f214a5..28d89c56c 100644 --- a/internal/handlers/handler_register_u2f_step1_test.go +++ b/internal/handlers/handler_register_u2f_step1_test.go @@ -65,7 +65,7 @@ func (s *HandlerRegisterU2FStep1Suite) TestShouldRaiseWhenXForwardedProtoIsMissi SecondFactorU2FIdentityFinish(s.mock.Ctx) assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode()) - assert.Equal(s.T(), "Missing header X-Forwarded-Proto", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "missing header X-Forwarded-Proto", s.mock.Hook.LastEntry().Message) } func (s *HandlerRegisterU2FStep1Suite) TestShouldRaiseWhenXForwardedHostIsMissing() { @@ -85,7 +85,7 @@ func (s *HandlerRegisterU2FStep1Suite) TestShouldRaiseWhenXForwardedHostIsMissin SecondFactorU2FIdentityFinish(s.mock.Ctx) assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode()) - assert.Equal(s.T(), "Missing header X-Forwarded-Host", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "missing header X-Forwarded-Host", s.mock.Hook.LastEntry().Message) } func TestShouldRunHandlerRegisterU2FStep1Suite(t *testing.T) { diff --git a/internal/handlers/handler_register_u2f_step2.go b/internal/handlers/handler_register_u2f_step2.go index c3efc08d4..d36090a76 100644 --- a/internal/handlers/handler_register_u2f_step2.go +++ b/internal/handlers/handler_register_u2f_step2.go @@ -16,7 +16,7 @@ func SecondFactorU2FRegister(ctx *middlewares.AutheliaCtx) { err := ctx.ParseBody(&responseBody) if err != nil { - ctx.Error(fmt.Errorf("Unable to parse response body: %v", err), messageUnableToRegisterSecurityKey) + ctx.Error(fmt.Errorf("unable to parse response body: %v", err), messageUnableToRegisterSecurityKey) } userSession := ctx.GetSession() @@ -38,7 +38,7 @@ func SecondFactorU2FRegister(ctx *middlewares.AutheliaCtx) { registration, err := u2f.Register(responseBody, *userSession.U2FChallenge, u2fConfig) if err != nil { - ctx.Error(fmt.Errorf("Unable to verify U2F registration: %v", err), messageUnableToRegisterSecurityKey) + ctx.Error(fmt.Errorf("unable to verify U2F registration: %v", err), messageUnableToRegisterSecurityKey) return } @@ -48,7 +48,7 @@ func SecondFactorU2FRegister(ctx *middlewares.AutheliaCtx) { err = ctx.Providers.StorageProvider.SaveU2FDeviceHandle(userSession.Username, registration.KeyHandle, publicKey) if err != nil { - ctx.Error(fmt.Errorf("Unable to register U2F device for user %s: %v", userSession.Username, err), messageUnableToRegisterSecurityKey) + ctx.Error(fmt.Errorf("unable to register U2F device for user %s: %v", userSession.Username, err), messageUnableToRegisterSecurityKey) return } diff --git a/internal/handlers/handler_reset_password_step1.go b/internal/handlers/handler_reset_password_step1.go index 3c3926543..7f460bd8c 100644 --- a/internal/handlers/handler_reset_password_step1.go +++ b/internal/handlers/handler_reset_password_step1.go @@ -23,7 +23,7 @@ func identityRetrieverFromStorage(ctx *middlewares.AutheliaCtx) (*session.Identi } if len(details.Emails) == 0 { - return nil, fmt.Errorf("User %s has no email address configured", requestBody.Username) + return nil, fmt.Errorf("user %s has no email address configured", requestBody.Username) } return &session.Identity{ diff --git a/internal/handlers/handler_reset_password_step2.go b/internal/handlers/handler_reset_password_step2.go index 751ca3b29..45689b29d 100644 --- a/internal/handlers/handler_reset_password_step2.go +++ b/internal/handlers/handler_reset_password_step2.go @@ -15,7 +15,7 @@ func ResetPasswordPost(ctx *middlewares.AutheliaCtx) { // otherwise PasswordReset would not be set to true. We can improve the security of this check by making the // request expire at some point because here it only expires when the cookie expires. if userSession.PasswordResetUsername == nil { - ctx.Error(fmt.Errorf("No identity verification process has been initiated"), messageUnableToResetPassword) + ctx.Error(fmt.Errorf("no identity verification process has been initiated"), messageUnableToResetPassword) return } @@ -33,9 +33,9 @@ func ResetPasswordPost(ctx *middlewares.AutheliaCtx) { switch { case utils.IsStringInSliceContains(err.Error(), ldapPasswordComplexityCodes), utils.IsStringInSliceContains(err.Error(), ldapPasswordComplexityErrors): - ctx.Error(fmt.Errorf("%s", err), ldapPasswordComplexityCode) + ctx.Error(err, ldapPasswordComplexityCode) default: - ctx.Error(fmt.Errorf("%s", err), messageUnableToResetPassword) + ctx.Error(err, messageUnableToResetPassword) } return @@ -48,7 +48,7 @@ func ResetPasswordPost(ctx *middlewares.AutheliaCtx) { err = ctx.SaveSession(userSession) if err != nil { - ctx.Error(fmt.Errorf("Unable to update password reset state: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to update password reset state: %s", err), messageOperationFailed) return } diff --git a/internal/handlers/handler_sign_duo.go b/internal/handlers/handler_sign_duo.go index 5ae5b15cd..2b8f6ec80 100644 --- a/internal/handlers/handler_sign_duo.go +++ b/internal/handlers/handler_sign_duo.go @@ -60,7 +60,7 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler { err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to regenerate session for user %s: %s", userSession.Username, err), messageMFAValidationFailed) return } @@ -68,7 +68,7 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler { err = ctx.SaveSession(userSession) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to update authentication level with Duo: %s", err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to update authentication level with Duo: %s", err), messageMFAValidationFailed) return } diff --git a/internal/handlers/handler_sign_totp.go b/internal/handlers/handler_sign_totp.go index 0cecb4fbf..daf69e61a 100644 --- a/internal/handlers/handler_sign_totp.go +++ b/internal/handlers/handler_sign_totp.go @@ -21,25 +21,25 @@ func SecondFactorTOTPPost(totpVerifier TOTPVerifier) middlewares.RequestHandler secret, err := ctx.Providers.StorageProvider.LoadTOTPSecret(userSession.Username) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to load TOTP secret: %s", err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to load TOTP secret: %s", err), messageMFAValidationFailed) return } isValid, err := totpVerifier.Verify(requestBody.Token, secret) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Error occurred during OTP validation for user %s: %s", userSession.Username, err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("error occurred during OTP validation for user %s: %s", userSession.Username, err), messageMFAValidationFailed) return } if !isValid { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Wrong passcode during TOTP validation for user %s", userSession.Username), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("wrong passcode during TOTP validation for user %s", userSession.Username), messageMFAValidationFailed) return } err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to regenerate session for user %s: %s", userSession.Username, err), messageMFAValidationFailed) return } @@ -47,7 +47,7 @@ func SecondFactorTOTPPost(totpVerifier TOTPVerifier) middlewares.RequestHandler err = ctx.SaveSession(userSession) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to update the authentication level with TOTP: %s", err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to update the authentication level with TOTP: %s", err), messageMFAValidationFailed) return } diff --git a/internal/handlers/handler_sign_u2f_step1.go b/internal/handlers/handler_sign_u2f_step1.go index 70c458809..a96b7ec83 100644 --- a/internal/handlers/handler_sign_u2f_step1.go +++ b/internal/handlers/handler_sign_u2f_step1.go @@ -29,7 +29,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) { challenge, err := u2f.NewChallenge(appID, trustedFacets) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to create U2F challenge: %s", err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to create U2F challenge: %s", err), messageMFAValidationFailed) return } @@ -38,11 +38,11 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) { if err != nil { if err == storage.ErrNoU2FDeviceHandle { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("No device handle found for user %s", userSession.Username), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("no device handle found for user %s", userSession.Username), messageMFAValidationFailed) return } - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to retrieve U2F device handle: %s", err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to retrieve U2F device handle: %s", err), messageMFAValidationFailed) return } @@ -63,7 +63,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) { err = ctx.SaveSession(userSession) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to save U2F challenge and registration in session: %s", err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to save U2F challenge and registration in session: %s", err), messageMFAValidationFailed) return } @@ -71,7 +71,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) { err = ctx.SetJSONBody(signRequest) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to set sign request in body: %s", err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to set sign request in body: %s", err), messageMFAValidationFailed) return } } diff --git a/internal/handlers/handler_sign_u2f_step1_test.go b/internal/handlers/handler_sign_u2f_step1_test.go index 909feaa5d..5483c8bc4 100644 --- a/internal/handlers/handler_sign_u2f_step1_test.go +++ b/internal/handlers/handler_sign_u2f_step1_test.go @@ -27,7 +27,7 @@ func (s *HandlerSignU2FStep1Suite) TestShouldRaiseWhenXForwardedProtoIsMissing() SecondFactorU2FSignGet(s.mock.Ctx) assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode()) - assert.Equal(s.T(), "Missing header X-Forwarded-Proto", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "missing header X-Forwarded-Proto", s.mock.Hook.LastEntry().Message) } func (s *HandlerSignU2FStep1Suite) TestShouldRaiseWhenXForwardedHostIsMissing() { @@ -35,7 +35,7 @@ func (s *HandlerSignU2FStep1Suite) TestShouldRaiseWhenXForwardedHostIsMissing() SecondFactorU2FSignGet(s.mock.Ctx) assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode()) - assert.Equal(s.T(), "Missing header X-Forwarded-Host", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "missing header X-Forwarded-Host", s.mock.Hook.LastEntry().Message) } func TestShouldRunHandlerSignU2FStep1Suite(t *testing.T) { diff --git a/internal/handlers/handler_sign_u2f_step2.go b/internal/handlers/handler_sign_u2f_step2.go index f7e8cad71..85c4a5207 100644 --- a/internal/handlers/handler_sign_u2f_step2.go +++ b/internal/handlers/handler_sign_u2f_step2.go @@ -42,7 +42,7 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to regenerate session for user %s: %s", userSession.Username, err), messageMFAValidationFailed) return } @@ -50,7 +50,7 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler err = ctx.SaveSession(userSession) if err != nil { - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to update authentication level with U2F: %s", err), messageMFAValidationFailed) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to update authentication level with U2F: %s", err), messageMFAValidationFailed) return } diff --git a/internal/handlers/handler_user_info.go b/internal/handlers/handler_user_info.go index d9f2e43c5..f45f3300e 100644 --- a/internal/handlers/handler_user_info.go +++ b/internal/handlers/handler_user_info.go @@ -87,7 +87,7 @@ func UserInfoGet(ctx *middlewares.AutheliaCtx) { errors := loadInfo(userSession.Username, ctx.Providers.StorageProvider, &userInfo, ctx.Logger) if len(errors) > 0 { - ctx.Error(fmt.Errorf("Unable to load user information"), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to load user information"), messageOperationFailed) return } @@ -115,7 +115,7 @@ func MethodPreferencePost(ctx *middlewares.AutheliaCtx) { } if !utils.IsStringInSlice(bodyJSON.Method, authentication.PossibleMethods) { - ctx.Error(fmt.Errorf("Unknown method '%s', it should be one of %s", bodyJSON.Method, strings.Join(authentication.PossibleMethods, ", ")), messageOperationFailed) + ctx.Error(fmt.Errorf("unknown method '%s', it should be one of %s", bodyJSON.Method, strings.Join(authentication.PossibleMethods, ", ")), messageOperationFailed) return } @@ -124,7 +124,7 @@ func MethodPreferencePost(ctx *middlewares.AutheliaCtx) { err = ctx.Providers.StorageProvider.SavePreferred2FAMethod(userSession.Username, bodyJSON.Method) if err != nil { - ctx.Error(fmt.Errorf("Unable to save new preferred 2FA method: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to save new preferred 2FA method: %s", err), messageOperationFailed) return } diff --git a/internal/handlers/handler_user_info_test.go b/internal/handlers/handler_user_info_test.go index 6e4e60a6f..db5db2e52 100644 --- a/internal/handlers/handler_user_info_test.go +++ b/internal/handlers/handler_user_info_test.go @@ -154,7 +154,7 @@ func (s *FetchSuite) TestShouldReturnError500WhenStorageFailsToLoad() { UserInfoGet(s.mock.Ctx) s.mock.Assert200KO(s.T(), "Operation failed.") - assert.Equal(s.T(), "Unable to load user information", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "unable to load user information", s.mock.Hook.LastEntry().Message) assert.Equal(s.T(), logrus.ErrorLevel, s.mock.Hook.LastEntry().Level) } @@ -213,7 +213,7 @@ func (s *SaveSuite) TestShouldReturnError500WhenBadMethodProvided() { MethodPreferencePost(s.mock.Ctx) s.mock.Assert200KO(s.T(), "Operation failed.") - assert.Equal(s.T(), "Unknown method 'abc', it should be one of totp, u2f, mobile_push", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "unknown method 'abc', it should be one of totp, u2f, mobile_push", s.mock.Hook.LastEntry().Message) assert.Equal(s.T(), logrus.ErrorLevel, s.mock.Hook.LastEntry().Level) } @@ -226,7 +226,7 @@ func (s *SaveSuite) TestShouldReturnError500WhenDatabaseFailsToSave() { MethodPreferencePost(s.mock.Ctx) s.mock.Assert200KO(s.T(), "Operation failed.") - assert.Equal(s.T(), "Unable to save new preferred 2FA method: Failure", s.mock.Hook.LastEntry().Message) + assert.Equal(s.T(), "unable to save new preferred 2FA method: Failure", s.mock.Hook.LastEntry().Message) assert.Equal(s.T(), logrus.ErrorLevel, s.mock.Hook.LastEntry().Level) } diff --git a/internal/handlers/handler_verify.go b/internal/handlers/handler_verify.go index d8b8106e0..6fce44741 100644 --- a/internal/handlers/handler_verify.go +++ b/internal/handlers/handler_verify.go @@ -47,7 +47,7 @@ func parseBasicAuth(header, auth string) (username, password string, err error) s := strings.IndexByte(cs, ':') if s < 0 { - return "", "", fmt.Errorf("Format of %s header must be user:password", header) + return "", "", fmt.Errorf("format of %s header must be user:password", header) } return cs[:s], cs[s+1:], nil @@ -85,29 +85,29 @@ func isTargetURLAuthorized(authorizer *authorization.Authorizer, targetURL url.U // verifyBasicAuth verify that the provided username and password are correct and // that the user is authorized to target the resource. -func verifyBasicAuth(header string, auth []byte, targetURL url.URL, ctx *middlewares.AutheliaCtx) (username, name string, groups, emails []string, authLevel authentication.Level, err error) { //nolint:unparam +func verifyBasicAuth(header string, auth []byte, ctx *middlewares.AutheliaCtx) (username, name string, groups, emails []string, authLevel authentication.Level, err error) { username, password, err := parseBasicAuth(header, string(auth)) if err != nil { - return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("Unable to parse content of %s header: %s", header, err) + return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("unable to parse content of %s header: %s", header, err) } authenticated, err := ctx.Providers.UserProvider.CheckUserPassword(username, password) if err != nil { - return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("Unable to check credentials extracted from %s header: %s", header, err) + return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("unable to check credentials extracted from %s header: %s", header, err) } // If the user is not correctly authenticated, send a 401. if !authenticated { // Request Basic Authentication otherwise - return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("User %s is not authenticated", username) + return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("user %s is not authenticated", username) } details, err := ctx.Providers.UserProvider.GetDetails(username) if err != nil { - return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("Unable to retrieve details of user %s: %s", username, err) + return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("unable to retrieve details of user %s: %s", username, err) } return username, details.DisplayName, details.Groups, details.Emails, authentication.OneFactor, nil @@ -155,20 +155,20 @@ func verifySessionCookie(ctx *middlewares.AutheliaCtx, targetURL *url.URL, userS isUserAnonymous := userSession.Username == "" if isUserAnonymous && userSession.AuthenticationLevel != authentication.NotAuthenticated { - return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("An anonymous user cannot be authenticated. That might be the sign of a compromise") + return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("an anonymous user cannot be authenticated. That might be the sign of a compromise") } if !userSession.KeepMeLoggedIn && !isUserAnonymous { inactiveLongEnough, err := hasUserBeenInactiveTooLong(ctx) if err != nil { - return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("Unable to check if user has been inactive for a long time: %s", err) + return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("unable to check if user has been inactive for a long time: %s", err) } if inactiveLongEnough { // Destroy the session a new one will be regenerated on next request. err := ctx.Providers.SessionProvider.DestroySession(ctx.RequestCtx) if err != nil { - return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("Unable to destroy user session after long inactivity: %s", err) + return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("unable to destroy user session after long inactivity: %s", err) } return userSession.Username, userSession.DisplayName, userSession.Groups, userSession.Emails, authentication.NotAuthenticated, fmt.Errorf("User %s has been inactive for too long", userSession.Username) @@ -180,13 +180,15 @@ func verifySessionCookie(ctx *middlewares.AutheliaCtx, targetURL *url.URL, userS if err == authentication.ErrUserNotFound { err = ctx.Providers.SessionProvider.DestroySession(ctx.RequestCtx) if err != nil { - ctx.Logger.Error(fmt.Errorf("Unable to destroy user session after provider refresh didn't find the user: %s", err)) + ctx.Logger.Errorf("Unable to destroy user session after provider refresh didn't find the user: %s", err) } return userSession.Username, userSession.DisplayName, userSession.Groups, userSession.Emails, authentication.NotAuthenticated, err } - ctx.Logger.Warnf("Error occurred while attempting to update user details from LDAP: %s", err) + ctx.Logger.Errorf("Error occurred while attempting to update user details from LDAP: %s", err) + + return "", "", nil, nil, authentication.NotAuthenticated, err } return userSession.Username, userSession.DisplayName, userSession.Groups, userSession.Emails, userSession.AuthenticationLevel, nil @@ -286,11 +288,11 @@ func generateVerifySessionHasUpToDateProfileTraceLogs(ctx *middlewares.AutheliaC // Check Groups. var groupsDelta []string if len(groupsAdded) != 0 { - groupsDelta = append(groupsDelta, fmt.Sprintf("Added: %s.", strings.Join(groupsAdded, ", "))) + groupsDelta = append(groupsDelta, fmt.Sprintf("added: %s.", strings.Join(groupsAdded, ", "))) } if len(groupsRemoved) != 0 { - groupsDelta = append(groupsDelta, fmt.Sprintf("Removed: %s.", strings.Join(groupsRemoved, ", "))) + groupsDelta = append(groupsDelta, fmt.Sprintf("removed: %s.", strings.Join(groupsRemoved, ", "))) } if len(groupsDelta) != 0 { @@ -302,11 +304,11 @@ func generateVerifySessionHasUpToDateProfileTraceLogs(ctx *middlewares.AutheliaC // Check Emails. var emailsDelta []string if len(emailsAdded) != 0 { - emailsDelta = append(emailsDelta, fmt.Sprintf("Added: %s.", strings.Join(emailsAdded, ", "))) + emailsDelta = append(emailsDelta, fmt.Sprintf("added: %s.", strings.Join(emailsAdded, ", "))) } if len(emailsRemoved) != 0 { - emailsDelta = append(emailsDelta, fmt.Sprintf("Removed: %s.", strings.Join(emailsRemoved, ", "))) + emailsDelta = append(emailsDelta, fmt.Sprintf("removed: %s.", strings.Join(emailsRemoved, ", "))) } if len(emailsDelta) != 0 { @@ -350,7 +352,7 @@ func verifySessionHasUpToDateProfile(ctx *middlewares.AutheliaCtx, targetURL *ur if !groupsDiff && !emailsDiff && !nameDiff { ctx.Logger.Tracef("Updated profile not detected for %s.", userSession.Username) - // Only update TTL if the user has a interval set. + // Only update TTL if the user has an interval set. // We get to this check when there were no changes. // Also make sure to update the session even if no difference was found. // This is so that we don't check every subsequent request after this one. @@ -411,12 +413,12 @@ func verifyAuth(ctx *middlewares.AutheliaCtx, targetURL *url.URL, refreshProfile if authValue != nil { isBasicAuth = true } else if isBasicAuth { - err = fmt.Errorf("Basic auth requested via query arg, but no value provided via %s header", authHeader) + err = fmt.Errorf("basic auth requested via query arg, but no value provided via %s header", authHeader) return } if isBasicAuth { - username, name, groups, emails, authLevel, err = verifyBasicAuth(authHeader, authValue, *targetURL, ctx) + username, name, groups, emails, authLevel, err = verifyBasicAuth(authHeader, authValue, ctx) return } @@ -429,13 +431,10 @@ func verifyAuth(ctx *middlewares.AutheliaCtx, targetURL *url.URL, refreshProfile err = ctx.Providers.SessionProvider.DestroySession(ctx.RequestCtx) if err != nil { - ctx.Logger.Error( - fmt.Errorf( - "Unable to destroy user session after handler could not match them to their %s header: %s", - HeaderSessionUsername, err)) + ctx.Logger.Errorf("Unable to destroy user session after handler could not match them to their %s header: %s", HeaderSessionUsername, err) } - err = fmt.Errorf("Could not match user %s to their %s header with a value of %s when visiting %s", username, HeaderSessionUsername, sessionUsername, targetURL.String()) + err = fmt.Errorf("could not match user %s to their %s header with a value of %s when visiting %s", username, HeaderSessionUsername, sessionUsername, targetURL.String()) } return @@ -450,37 +449,36 @@ func VerifyGet(cfg schema.AuthenticationBackendConfiguration) middlewares.Reques targetURL, err := ctx.GetOriginalURL() if err != nil { - ctx.Logger.Error(fmt.Errorf("Unable to parse target URL: %s", err)) + ctx.Logger.Errorf("Unable to parse target URL: %s", err) ctx.ReplyUnauthorized() return } if !isSchemeHTTPS(targetURL) && !isSchemeWSS(targetURL) { - ctx.Logger.Error(fmt.Errorf("Scheme of target URL %s must be secure since cookies are "+ - "only transported over a secure connection for security reasons", targetURL.String())) + ctx.Logger.Errorf("Scheme of target URL %s must be secure since cookies are "+ + "only transported over a secure connection for security reasons", targetURL.String()) ctx.ReplyUnauthorized() return } if !isURLUnderProtectedDomain(targetURL, ctx.Configuration.Session.Domain) { - ctx.Logger.Error(fmt.Errorf("The target URL %s is not under the protected domain %s", - targetURL.String(), ctx.Configuration.Session.Domain)) + ctx.Logger.Errorf("Target URL %s is not under the protected domain %s", + targetURL.String(), ctx.Configuration.Session.Domain) ctx.ReplyUnauthorized() return } + method := ctx.XForwardedMethod() isBasicAuth, username, name, groups, emails, authLevel, err := verifyAuth(ctx, targetURL, refreshProfile, refreshProfileInterval) - method := ctx.XForwardedMethod() - if err != nil { - ctx.Logger.Error(fmt.Sprintf("Error caught when verifying user authorization: %s", err)) + ctx.Logger.Errorf("Error caught when verifying user authorization: %s", err) if err := updateActivityTimestamp(ctx, isBasicAuth, username); err != nil { - ctx.Error(fmt.Errorf("Unable to update last activity: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to update last activity: %s", err), messageOperationFailed) return } @@ -503,7 +501,7 @@ func VerifyGet(cfg schema.AuthenticationBackendConfiguration) middlewares.Reques } if err := updateActivityTimestamp(ctx, isBasicAuth, username); err != nil { - ctx.Error(fmt.Errorf("Unable to update last activity: %s", err), messageOperationFailed) + ctx.Error(fmt.Errorf("unable to update last activity: %s", err), messageOperationFailed) } } } diff --git a/internal/handlers/handler_verify_test.go b/internal/handlers/handler_verify_test.go index c1c7e876d..cf1faa17f 100644 --- a/internal/handlers/handler_verify_test.go +++ b/internal/handlers/handler_verify_test.go @@ -100,7 +100,7 @@ func TestShouldRaiseWhenCredentialsAreNotInCorrectForm(t *testing.T) { // The decoded format should be user:password. _, _, err := parseBasicAuth(HeaderProxyAuthorization, "Basic am9obiBwYXNzd29yZA==") assert.Error(t, err) - assert.Equal(t, "Format of Proxy-Authorization header must be user:password", err.Error()) + assert.Equal(t, "format of Proxy-Authorization header must be user:password", err.Error()) } func TestShouldUseProvidedHeaderName(t *testing.T) { @@ -144,7 +144,7 @@ func TestShouldCheckAuthorizationMatching(t *testing.T) { {"deny", authentication.TwoFactor, Forbidden}, } - url, _ := url.ParseRequestURI("https://test.example.com") + u, _ := url.ParseRequestURI("https://test.example.com") for _, rule := range rules { authorizer := authorization.NewAuthorizer(&schema.Configuration{ @@ -161,7 +161,7 @@ func TestShouldCheckAuthorizationMatching(t *testing.T) { username = testUsername } - matching := isTargetURLAuthorized(authorizer, *url, username, []string{}, net.ParseIP("127.0.0.1"), []byte("GET"), rule.AuthLevel) + matching := isTargetURLAuthorized(authorizer, *u, username, []string{}, net.ParseIP("127.0.0.1"), []byte("GET"), rule.AuthLevel) assert.Equal(t, rule.ExpectedMatching, matching, "policy=%s, authLevel=%v, expected=%v, actual=%v", rule.Policy, rule.AuthLevel, rule.ExpectedMatching, matching) } @@ -176,8 +176,7 @@ func TestShouldVerifyWrongCredentials(t *testing.T) { CheckUserPassword(gomock.Eq("john"), gomock.Eq("password")). Return(false, nil) - url, _ := url.ParseRequestURI("https://test.example.com") - _, _, _, _, _, err := verifyBasicAuth(HeaderProxyAuthorization, []byte("Basic am9objpwYXNzd29yZA=="), *url, mock.Ctx) + _, _, _, _, _, err := verifyBasicAuth(HeaderProxyAuthorization, []byte("Basic am9objpwYXNzd29yZA=="), mock.Ctx) assert.Error(t, err) } diff --git a/internal/handlers/response.go b/internal/handlers/response.go index a3f55b9c6..c86808a79 100644 --- a/internal/handlers/response.go +++ b/internal/handlers/response.go @@ -16,7 +16,7 @@ func handleOIDCWorkflowResponse(ctx *middlewares.AutheliaCtx) { userSession := ctx.GetSession() if !authorization.IsAuthLevelSufficient(userSession.AuthenticationLevel, userSession.OIDCWorkflowSession.RequiredAuthorizationLevel) { - ctx.Logger.Warn("OIDC requires 2FA, cannot be redirected yet") + ctx.Logger.Warn("OpenID Connect requires 2FA, cannot be redirected yet") ctx.ReplyOK() return @@ -24,8 +24,8 @@ func handleOIDCWorkflowResponse(ctx *middlewares.AutheliaCtx) { uri, err := ctx.ExternalRootURL() if err != nil { - ctx.Logger.Errorf("%v", err) - handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to get forward facing URI"), messageAuthenticationFailed) + ctx.Logger.Errorf("Unable to extract external root URL: %v", err) + handleAuthenticationUnauthorized(ctx, fmt.Errorf("unable to get forward facing URI"), messageAuthenticationFailed) return } @@ -64,7 +64,7 @@ func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI, requestMethod st targetURL, err := url.ParseRequestURI(targetURI) if err != nil { - ctx.Error(fmt.Errorf("Unable to parse target URL %s: %s", targetURI, err), messageAuthenticationFailed) + ctx.Error(fmt.Errorf("unable to parse target URL %s: %s", targetURI, err), messageAuthenticationFailed) return } @@ -88,6 +88,8 @@ func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI, requestMethod st safeRedirection := utils.IsRedirectionSafe(*targetURL, ctx.Configuration.Session.Domain) if !safeRedirection { + ctx.Logger.Debugf("Redirection URL %s is not safe", targetURI) + if !ctx.Providers.Authorizer.IsSecondFactorEnabled() && ctx.Configuration.DefaultRedirectionURL != "" { err := ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL}) if err != nil { @@ -126,7 +128,7 @@ func Handle2FAResponse(ctx *middlewares.AutheliaCtx, targetURI string) { safe, err := utils.IsRedirectionURISafe(targetURI, ctx.Configuration.Session.Domain) if err != nil { - ctx.Error(fmt.Errorf("Unable to check target URL: %s", err), messageMFAValidationFailed) + ctx.Error(fmt.Errorf("unable to check target URL: %s", err), messageMFAValidationFailed) return } diff --git a/internal/middlewares/http_to_authelia_handler_adaptor.go b/internal/middlewares/http_to_authelia_handler_adaptor.go index 281e75a43..c06c0d925 100644 --- a/internal/middlewares/http_to_authelia_handler_adaptor.go +++ b/internal/middlewares/http_to_authelia_handler_adaptor.go @@ -100,7 +100,7 @@ func NewHTTPToAutheliaHandlerAdaptor(h AutheliaHandlerFunc) RequestHandler { rURL, err := url.ParseRequestURI(r.RequestURI) if err != nil { - ctx.Logger.Errorf("cannot parse requestURI %q: %s", r.RequestURI, err) + ctx.Logger.Errorf("Cannot parse requestURI %q: %s", r.RequestURI, err) ctx.RequestCtx.Error("Internal Server Error", fasthttp.StatusInternalServerError) return