[MISC] Refactor and address most errcheck linter ignores (#1511)

* [MISC] Refactor and address most errcheck linter ignores

This is mostly a quality of life change.
When we first implemented the errcheck linter we ignored a number of items in our legacy codebase with intent to revisit down the track.

* Handle errors for regulation marks and remove unnecessary logging

cmd/authelia-scripts/cmd_suites.go

@@ -195,10 +195,16 @@ func setupSuite(suiteName string) error {
 
 	if errSetup := runSuiteSetupTeardown("setup", suiteName); errSetup != nil || interrupted {
 		if errSetup == utils.ErrTimeoutReached {
-			runOnSetupTimeout(suiteName) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+			err := runOnSetupTimeout(suiteName)
+			if err != nil {
+				log.Fatal(err)
+			}
 		}
 
-		teardownSuite(suiteName) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+		err := teardownSuite(suiteName)
+		if err != nil {
+			log.Fatal(err)
+		}
 
 		return errSetup
 	}

internal/handlers/handler_configuration.go

@@ -26,5 +26,9 @@ func ConfigurationGet(ctx *middlewares.AutheliaCtx) {
 	ctx.Logger.Tracef("Second factor enabled: %v", body.SecondFactorEnabled)
 
 	ctx.Logger.Tracef("Available methods are %s", body.AvailableMethods)
-	ctx.SetJSONBody(body) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+
+	err := ctx.SetJSONBody(body)
+	if err != nil {
+		ctx.Logger.Errorf("Unable to set configuration response in body: %s", err)
+	}
 }

internal/handlers/handler_firstfactor.go

@@ -95,7 +95,10 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware
 
 		if err != nil {
 			ctx.Logger.Debugf("Mark authentication attempt made by user %s", bodyJSON.Username)
-			ctx.Providers.Regulator.Mark(bodyJSON.Username, false) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+
+			if err := ctx.Providers.Regulator.Mark(bodyJSON.Username, false); err != nil {
+				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()), authenticationFailedMessage)
 
@@ -104,17 +107,16 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware
 
 		if !userPasswordOk {
 			ctx.Logger.Debugf("Mark authentication attempt made by user %s", bodyJSON.Username)
-			ctx.Providers.Regulator.Mark(bodyJSON.Username, false) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
 
-			handleAuthenticationUnauthorized(ctx, fmt.Errorf("Credentials are wrong for user %s", bodyJSON.Username), authenticationFailedMessage)
+			if err := ctx.Providers.Regulator.Mark(bodyJSON.Username, false); err != nil {
+				ctx.Logger.Errorf("Unable to mark authentication: %s", err.Error())
+			}
 
-			ctx.ReplyError(fmt.Errorf("Credentials are wrong for user %s", bodyJSON.Username), authenticationFailedMessage)
+			handleAuthenticationUnauthorized(ctx, fmt.Errorf("Credentials are wrong for user %s", bodyJSON.Username), authenticationFailedMessage)
 
 			return
 		}
 
-		ctx.Logger.Debugf("Credentials validation of user %s is ok", bodyJSON.Username)
-
 		ctx.Logger.Debugf("Mark authentication attempt made by user %s", bodyJSON.Username)
 		err = ctx.Providers.Regulator.Mark(bodyJSON.Username, true)
 
@@ -123,6 +125,8 @@ func FirstFactorPost(msInitialDelay time.Duration, delayEnabled bool) middleware
 			return
 		}
 
+		ctx.Logger.Debugf("Credentials validation of user %s is ok", bodyJSON.Username)
+
 		// Reset all values from previous session before regenerating the cookie.
 		err = ctx.SaveSession(session.NewDefaultUserSession())
 

internal/handlers/handler_logout_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/authelia/authelia/internal/mocks"
@@ -20,7 +21,8 @@ func (s *LogoutSuite) SetupTest() {
 	s.mock = mocks.NewMockAutheliaCtx(s.T())
 	userSession := s.mock.Ctx.GetSession()
 	userSession.Username = testUsername
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 }
 
 func (s *LogoutSuite) TearDownTest() {

internal/handlers/handler_register_totp.go

@@ -60,7 +60,10 @@ func secondFactorTOTPIdentityFinish(ctx *middlewares.AutheliaCtx, username strin
 		Base32Secret: key.Secret(),
 	}
 
-	ctx.SetJSONBody(response) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err = ctx.SetJSONBody(response)
+	if err != nil {
+		ctx.Logger.Errorf("Unable to set TOTP key response in body: %s", err)
+	}
 }
 
 // SecondFactorTOTPIdentityFinish the handler for finishing the identity validation.

internal/handlers/handler_register_u2f_step1.go

@@ -56,7 +56,10 @@ func secondFactorU2FIdentityFinish(ctx *middlewares.AutheliaCtx, username string
 		return
 	}
 
-	ctx.SetJSONBody(u2f.NewWebRegisterRequest(challenge, []u2f.Registration{})) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err = ctx.SetJSONBody(u2f.NewWebRegisterRequest(challenge, []u2f.Registration{}))
+	if err != nil {
+		ctx.Logger.Errorf("Unable to create request to enrol new token: %s", err)
+	}
 }
 
 // SecondFactorU2FIdentityFinish the handler for finishing the identity validation.

internal/handlers/handler_register_u2f_step1_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/dgrijalva/jwt-go"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/authelia/authelia/internal/middlewares"
@@ -25,7 +26,8 @@ func (s *HandlerRegisterU2FStep1Suite) SetupTest() {
 
 	userSession := s.mock.Ctx.GetSession()
 	userSession.Username = testUsername
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 }
 
 func (s *HandlerRegisterU2FStep1Suite) TearDownTest() {

internal/handlers/handler_register_u2f_step2.go

@@ -28,7 +28,11 @@ func SecondFactorU2FRegister(ctx *middlewares.AutheliaCtx) {
 	// Ensure the challenge is cleared if anything goes wrong.
 	defer func() {
 		userSession.U2FChallenge = nil
-		ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+
+		err := ctx.SaveSession(userSession)
+		if err != nil {
+			ctx.Logger.Errorf("Unable to clear U2F challenge in session for user %s: %s", userSession.Username, err)
+		}
 	}()
 
 	registration, err := u2f.Register(responseBody, *userSession.U2FChallenge, u2fConfig)

internal/handlers/handler_reset_password_step1.go

@@ -46,7 +46,11 @@ func resetPasswordIdentityFinish(ctx *middlewares.AutheliaCtx, username string)
 	userSession := ctx.GetSession()
 	// TODO(c.michaud): use JWT tokens to expire the request in only few seconds for better security.
 	userSession.PasswordResetUsername = &username
-	ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+
+	err := ctx.SaveSession(userSession)
+	if err != nil {
+		ctx.Logger.Errorf("Unable to clear password reset flag in session for user %s: %s", userSession.Username, err)
+	}
 
 	ctx.ReplyOK()
 }

internal/handlers/handler_sign_duo_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/authelia/authelia/internal/duo"
@@ -25,7 +26,8 @@ func (s *SecondFactorDuoPostSuite) SetupTest() {
 	s.mock = mocks.NewMockAutheliaCtx(s.T())
 	userSession := s.mock.Ctx.GetSession()
 	userSession.Username = testUsername
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 }
 
 func (s *SecondFactorDuoPostSuite) TearDownTest() {

internal/handlers/handler_sign_totp_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"github.com/tstranex/u2f"
 
@@ -25,7 +26,8 @@ func (s *HandlerSignTOTPSuite) SetupTest() {
 	userSession.Username = testUsername
 	userSession.U2FChallenge = &u2f.Challenge{}
 	userSession.U2FRegistration = &session.U2FRegistration{}
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 }
 
 func (s *HandlerSignTOTPSuite) TearDownTest() {

internal/handlers/handler_sign_u2f_step2_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"github.com/tstranex/u2f"
 
@@ -25,7 +26,8 @@ func (s *HandlerSignU2FStep2Suite) SetupTest() {
 	userSession.Username = testUsername
 	userSession.U2FChallenge = &u2f.Challenge{}
 	userSession.U2FRegistration = &session.U2FRegistration{}
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 }
 
 func (s *HandlerSignU2FStep2Suite) TearDownTest() {

internal/handlers/handler_state.go

@@ -12,5 +12,9 @@ func StateGet(ctx *middlewares.AutheliaCtx) {
 		AuthenticationLevel:   userSession.AuthenticationLevel,
 		DefaultRedirectionURL: ctx.Configuration.DefaultRedirectionURL,
 	}
-	ctx.SetJSONBody(stateResponse) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+
+	err := ctx.SetJSONBody(stateResponse)
+	if err != nil {
+		ctx.Logger.Errorf("Unable to set state response in body: %s", err)
+	}
 }

internal/handlers/handler_state_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/authelia/authelia/internal/authentication"
@@ -28,7 +29,8 @@ func (s *StateGetSuite) TearDownTest() {
 func (s *StateGetSuite) TestShouldReturnUsernameFromSession() {
 	userSession := s.mock.Ctx.GetSession()
 	userSession.Username = "username"
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 
 	StateGet(s.mock.Ctx)
 
@@ -47,7 +49,8 @@ func (s *StateGetSuite) TestShouldReturnUsernameFromSession() {
 	}
 	actualBody := Response{}
 
-	json.Unmarshal(s.mock.Ctx.Response.Body(), &actualBody) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err = json.Unmarshal(s.mock.Ctx.Response.Body(), &actualBody)
+	require.NoError(s.T(), err)
 	assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode())
 	assert.Equal(s.T(), []byte("application/json"), s.mock.Ctx.Response.Header.ContentType())
 	assert.Equal(s.T(), expectedBody, actualBody)
@@ -56,7 +59,8 @@ func (s *StateGetSuite) TestShouldReturnUsernameFromSession() {
 func (s *StateGetSuite) TestShouldReturnAuthenticationLevelFromSession() {
 	userSession := s.mock.Ctx.GetSession()
 	userSession.AuthenticationLevel = authentication.OneFactor
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 
 	StateGet(s.mock.Ctx)
 
@@ -75,7 +79,8 @@ func (s *StateGetSuite) TestShouldReturnAuthenticationLevelFromSession() {
 	}
 	actualBody := Response{}
 
-	json.Unmarshal(s.mock.Ctx.Response.Body(), &actualBody) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err = json.Unmarshal(s.mock.Ctx.Response.Body(), &actualBody)
+	require.NoError(s.T(), err)
 	assert.Equal(s.T(), 200, s.mock.Ctx.Response.StatusCode())
 	assert.Equal(s.T(), []byte("application/json"), s.mock.Ctx.Response.Header.ContentType())
 	assert.Equal(s.T(), expectedBody, actualBody)

internal/handlers/handler_user_info.go

@@ -93,7 +93,10 @@ func UserInfoGet(ctx *middlewares.AutheliaCtx) {
 
 	userInfo.DisplayName = userSession.DisplayName
 
-	ctx.SetJSONBody(userInfo) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := ctx.SetJSONBody(userInfo)
+	if err != nil {
+		ctx.Logger.Errorf("Unable to set user info response in body: %s", err)
+	}
 }
 
 // MethodBody the selected 2FA method.

internal/handlers/handler_user_info_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/authelia/authelia/internal/mocks"
@@ -24,7 +25,8 @@ func (s *FetchSuite) SetupTest() {
 	userSession := s.mock.Ctx.GetSession()
 	userSession.Username = testUsername
 	userSession.AuthenticationLevel = 1
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 }
 
 func (s *FetchSuite) TearDownTest() {
@@ -92,7 +94,8 @@ func TestMethodSetToU2F(t *testing.T) {
 		userSession := mock.Ctx.GetSession()
 		userSession.Username = testUsername
 		userSession.AuthenticationLevel = 1
-		mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+		err := mock.Ctx.SaveSession(userSession)
+		require.NoError(t, err)
 
 		setPreferencesExpectations(expectedPreferences, mock.StorageProviderMock)
 		UserInfoGet(mock.Ctx)
@@ -170,7 +173,8 @@ func (s *SaveSuite) SetupTest() {
 	userSession := s.mock.Ctx.GetSession()
 	userSession.Username = testUsername
 	userSession.AuthenticationLevel = 1
-	s.mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := s.mock.Ctx.SaveSession(userSession)
+	require.NoError(s.T(), err)
 }
 
 func (s *SaveSuite) TearDownTest() {

internal/handlers/response.go

@@ -15,7 +15,10 @@ import (
 func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI string, username string, groups []string) {
 	if targetURI == "" {
 		if !ctx.Providers.Authorizer.IsSecondFactorEnabled() && ctx.Configuration.DefaultRedirectionURL != "" {
-			ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL}) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+			err := ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL})
+			if err != nil {
+				ctx.Logger.Errorf("Unable to set default redirection URL in body: %s", err)
+			}
 		} else {
 			ctx.ReplyOK()
 		}
@@ -48,7 +51,10 @@ func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI string, username
 
 	if !safeRedirection {
 		if !ctx.Providers.Authorizer.IsSecondFactorEnabled() && ctx.Configuration.DefaultRedirectionURL != "" {
-			ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL}) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+			err := ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL})
+			if err != nil {
+				ctx.Logger.Errorf("Unable to set default redirection URL in body: %s", err)
+			}
 		} else {
 			ctx.ReplyOK()
 		}
@@ -59,14 +65,21 @@ func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI string, username
 	ctx.Logger.Debugf("Redirection URL %s is safe", targetURI)
 
 	response := redirectResponse{Redirect: targetURI}
-	ctx.SetJSONBody(response) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+
+	err = ctx.SetJSONBody(response)
+	if err != nil {
+		ctx.Logger.Errorf("Unable to set redirection URL in body: %s", err)
+	}
 }
 
 // Handle2FAResponse handle the redirection upon 2FA authentication.
 func Handle2FAResponse(ctx *middlewares.AutheliaCtx, targetURI string) {
 	if targetURI == "" {
 		if ctx.Configuration.DefaultRedirectionURL != "" {
-			ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL}) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+			err := ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL})
+			if err != nil {
+				ctx.Logger.Errorf("Unable to set default redirection URL in body: %s", err)
+			}
 		} else {
 			ctx.ReplyOK()
 		}
@@ -82,7 +95,10 @@ func Handle2FAResponse(ctx *middlewares.AutheliaCtx, targetURI string) {
 	}
 
 	if targetURL != nil && utils.IsRedirectionSafe(*targetURL, ctx.Configuration.Session.Domain) {
-		ctx.SetJSONBody(redirectResponse{Redirect: targetURI}) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+		err := ctx.SetJSONBody(redirectResponse{Redirect: targetURI})
+		if err != nil {
+			ctx.Logger.Errorf("Unable to set redirection URL in body: %s", err)
+		}
 	} else {
 		ctx.ReplyOK()
 	}

internal/suites/action_2fa_methods.go

@@ -4,10 +4,14 @@ import (
 	"context"
 	"fmt"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func (wds *WebDriverSession) doChangeMethod(ctx context.Context, t *testing.T, method string) {
-	wds.WaitElementLocatedByID(ctx, t, "methods-button").Click() //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err := wds.WaitElementLocatedByID(ctx, t, "methods-button").Click()
+	require.NoError(t, err)
 	wds.WaitElementLocatedByID(ctx, t, "methods-dialog")
-	wds.WaitElementLocatedByID(ctx, t, fmt.Sprintf("%s-option", method)).Click() //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.
+	err = wds.WaitElementLocatedByID(ctx, t, fmt.Sprintf("%s-option", method)).Click()
+	require.NoError(t, err)
 }