Fix unit tests.

internal/handlers/handler_firstfactor.go

@@ -42,14 +42,6 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
 		return
 	}
 
-	ctx.Logger.Debugf("Mark authentication attempt made by user %s", bodyJSON.Username)
-	err = ctx.Providers.Regulator.Mark(bodyJSON.Username, false)
-
-	if err != nil {
-		ctx.Error(fmt.Errorf("Unable to mark authentication: %s", err), authenticationFailedMessage)
-		return
-	}
-
 	if !userPasswordOk {
 		ctx.ReplyError(fmt.Errorf("Credentials are wrong for user %s", bodyJSON.Username), authenticationFailedMessage)
 		return
@@ -57,6 +49,14 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
 
 	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)
+
+	if err != nil {
+		ctx.Error(fmt.Errorf("Unable to mark authentication: %s", err), authenticationFailedMessage)
+		return
+	}
+
 	// Reset all values from previous session before regenerating the cookie.
 	err = ctx.SaveSession(session.NewDefaultUserSession())
 

internal/handlers/handler_firstfactor_test.go

@@ -3,7 +3,6 @@ package handlers
 import (
 	"fmt"
 	"testing"
-	"time"
 
 	"github.com/clems4ever/authelia/internal/mocks"
 	"github.com/clems4ever/authelia/internal/models"
@@ -61,6 +60,14 @@ func (s *FirstFactorSuite) TestShouldFailIfUserProviderCheckPasswordFail() {
 		CheckUserPassword(gomock.Eq("test"), gomock.Eq("hello")).
 		Return(false, fmt.Errorf("Failed"))
 
+	s.mock.StorageProviderMock.
+		EXPECT().
+		AppendAuthenticationLog(gomock.Eq(models.AuthenticationAttempt{
+			Username:   "test",
+			Successful: false,
+			Time:       s.mock.Clock.Now(),
+		}))
+
 	s.mock.Ctx.Request.SetBodyString(`{
 		"username": "test",
 		"password": "hello",
@@ -73,9 +80,6 @@ func (s *FirstFactorSuite) TestShouldFailIfUserProviderCheckPasswordFail() {
 }
 
 func (s *FirstFactorSuite) TestShouldCheckAuthenticationIsMarkedWhenInvalidCredentials() {
-	t, _ := time.Parse("2006-Jan-02", "2013-Feb-03")
-	s.mock.Clock.Set(t)
-
 	s.mock.UserProviderMock.
 		EXPECT().
 		CheckUserPassword(gomock.Eq("test"), gomock.Eq("hello")).
@@ -86,7 +90,7 @@ func (s *FirstFactorSuite) TestShouldCheckAuthenticationIsMarkedWhenInvalidCrede
 		AppendAuthenticationLog(gomock.Eq(models.AuthenticationAttempt{
 			Username:   "test",
 			Successful: false,
-			Time:       t,
+			Time:       s.mock.Clock.Now(),
 		}))
 
 	s.mock.Ctx.Request.SetBodyString(`{
@@ -104,16 +108,16 @@ func (s *FirstFactorSuite) TestShouldFailIfUserProviderGetDetailsFail() {
 		CheckUserPassword(gomock.Eq("test"), gomock.Eq("hello")).
 		Return(true, nil)
 
-	s.mock.UserProviderMock.
-		EXPECT().
-		GetDetails(gomock.Eq("test")).
-		Return(nil, fmt.Errorf("Failed"))
-
 	s.mock.StorageProviderMock.
 		EXPECT().
 		AppendAuthenticationLog(gomock.Any()).
 		Return(nil)
 
+	s.mock.UserProviderMock.
+		EXPECT().
+		GetDetails(gomock.Eq("test")).
+		Return(nil, fmt.Errorf("Failed"))
+
 	s.mock.Ctx.Request.SetBodyString(`{
 		"username": "test",
 		"password": "hello",
@@ -125,17 +129,12 @@ func (s *FirstFactorSuite) TestShouldFailIfUserProviderGetDetailsFail() {
 	s.mock.Assert200KO(s.T(), "Authentication failed. Check your credentials.")
 }
 
-func (s *FirstFactorSuite) TestShouldFailIfAuthenticationLoggingFail() {
+func (s *FirstFactorSuite) TestShouldFailIfAuthenticationMarkFail() {
 	s.mock.UserProviderMock.
 		EXPECT().
 		CheckUserPassword(gomock.Eq("test"), gomock.Eq("hello")).
 		Return(true, nil)
 
-	s.mock.UserProviderMock.
-		EXPECT().
-		GetDetails(gomock.Eq("test")).
-		Return(nil, nil)
-
 	s.mock.StorageProviderMock.
 		EXPECT().
 		AppendAuthenticationLog(gomock.Any()).

internal/handlers/handler_verify_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/clems4ever/authelia/internal/mocks"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
 )
 
 // Test getOriginalURL
@@ -172,54 +173,143 @@ func (tc TestCase) String() string {
 	return fmt.Sprintf("url=%s, auth=%s, exp_status=%d", tc.URL, tc.Authorization, tc.ExpectedStatusCode)
 }
 
-func TestShouldVerifyAuthorizationsUsingBasicAuth(t *testing.T) {
+type BasicAuthorizationSuite struct {
-	testCases := []TestCase{
+	suite.Suite
-		// Authorization has bad format.
+}
-		TestCase{"https://bypass.example.com", "Basic am9objpaaaaaaaaaaaaaaaa", 401},
 
-		// Correct Authorization
+func NewBasicAuthorizationSuite() *BasicAuthorizationSuite {
-		TestCase{"https://test.example.com", "Basic am9objpwYXNzd29yZA==", 403},
+	return &BasicAuthorizationSuite{}
-		TestCase{"https://bypass.example.com", "Basic am9objpwYXNzd29yZA==", 200},
+}
-		TestCase{"https://one-factor.example.com", "Basic am9objpwYXNzd29yZA==", 200},
-		TestCase{"https://two-factor.example.com", "Basic am9objpwYXNzd29yZA==", 401},
-		TestCase{"https://deny.example.com", "Basic am9objpwYXNzd29yZA==", 403},
-	}
 
-	for _, testCase := range testCases {
+func (s *BasicAuthorizationSuite) TestShouldNotBeAbleToParseBasicAuth() {
-		testCase := testCase
+	mock := mocks.NewMockAutheliaCtx(s.T())
-		t.Run(testCase.String(), func(t *testing.T) {
-			mock := mocks.NewMockAutheliaCtx(t)
 	defer mock.Close()
 
+	mock.Ctx.Request.Header.Set("Proxy-Authorization", "Basic am9objpaaaaaaaaaaaaaaaa")
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://test.example.com")
+
+	VerifyGet(mock.Ctx)
+
+	assert.Equal(s.T(), 401, mock.Ctx.Response.StatusCode())
+}
+
+func (s *BasicAuthorizationSuite) TestShouldApplyDefaultPolicy() {
+	mock := mocks.NewMockAutheliaCtx(s.T())
+	defer mock.Close()
+
+	mock.Ctx.Request.Header.Set("Proxy-Authorization", "Basic am9objpwYXNzd29yZA==")
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://test.example.com")
+
 	mock.UserProviderMock.EXPECT().
 		CheckUserPassword(gomock.Eq("john"), gomock.Eq("password")).
 		Return(true, nil)
 
-			details := authentication.UserDetails{
-				Emails: []string{"john@example.com"},
-				Groups: []string{"dev", "admin"},
-			}
 	mock.UserProviderMock.EXPECT().
 		GetDetails(gomock.Eq("john")).
-				Return(&details, nil)
+		Return(&authentication.UserDetails{
-
+			Emails: []string{"john@example.com"},
-			mock.Ctx.Request.Header.Set("Proxy-Authorization", testCase.Authorization)
+			Groups: []string{"dev", "admin"},
-			mock.Ctx.Request.Header.Set("X-Original-URL", testCase.URL)
+		}, nil)
 
 	VerifyGet(mock.Ctx)
-			expStatus, actualStatus := testCase.ExpectedStatusCode, mock.Ctx.Response.StatusCode()
-			assert.Equal(t, expStatus, actualStatus, "URL=%s -> StatusCode=%d != ExpectedStatusCode=%d",
-				testCase.URL, actualStatus, expStatus)
 
-			if testCase.ExpectedStatusCode == 200 {
+	assert.Equal(s.T(), 403, mock.Ctx.Response.StatusCode())
-				assert.Equal(t, []byte("john"), mock.Ctx.Response.Header.Peek("Remote-User"))
+}
-				assert.Equal(t, []byte("dev,admin"), mock.Ctx.Response.Header.Peek("Remote-Groups"))
+
-			} else {
+func (s *BasicAuthorizationSuite) TestShouldApplyPolicyOfBypassDomain() {
-				assert.Equal(t, []byte(nil), mock.Ctx.Response.Header.Peek("Remote-User"))
+	mock := mocks.NewMockAutheliaCtx(s.T())
-				assert.Equal(t, []byte(nil), mock.Ctx.Response.Header.Peek("Remote-Groups"))
+	defer mock.Close()
-			}
+
-		})
+	mock.Ctx.Request.Header.Set("Proxy-Authorization", "Basic am9objpwYXNzd29yZA==")
-	}
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://bypass.example.com")
+
+	mock.UserProviderMock.EXPECT().
+		CheckUserPassword(gomock.Eq("john"), gomock.Eq("password")).
+		Return(true, nil)
+
+	mock.UserProviderMock.EXPECT().
+		GetDetails(gomock.Eq("john")).
+		Return(&authentication.UserDetails{
+			Emails: []string{"john@example.com"},
+			Groups: []string{"dev", "admin"},
+		}, nil)
+
+	VerifyGet(mock.Ctx)
+
+	assert.Equal(s.T(), 200, mock.Ctx.Response.StatusCode())
+}
+
+func (s *BasicAuthorizationSuite) TestShouldApplyPolicyOfOneFactorDomain() {
+	mock := mocks.NewMockAutheliaCtx(s.T())
+	defer mock.Close()
+
+	mock.Ctx.Request.Header.Set("Proxy-Authorization", "Basic am9objpwYXNzd29yZA==")
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://one-factor.example.com")
+
+	mock.UserProviderMock.EXPECT().
+		CheckUserPassword(gomock.Eq("john"), gomock.Eq("password")).
+		Return(true, nil)
+
+	mock.UserProviderMock.EXPECT().
+		GetDetails(gomock.Eq("john")).
+		Return(&authentication.UserDetails{
+			Emails: []string{"john@example.com"},
+			Groups: []string{"dev", "admin"},
+		}, nil)
+
+	VerifyGet(mock.Ctx)
+
+	assert.Equal(s.T(), 200, mock.Ctx.Response.StatusCode())
+}
+
+func (s *BasicAuthorizationSuite) TestShouldApplyPolicyOfTwoFactorDomain() {
+	mock := mocks.NewMockAutheliaCtx(s.T())
+	defer mock.Close()
+
+	mock.Ctx.Request.Header.Set("Proxy-Authorization", "Basic am9objpwYXNzd29yZA==")
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com")
+
+	mock.UserProviderMock.EXPECT().
+		CheckUserPassword(gomock.Eq("john"), gomock.Eq("password")).
+		Return(true, nil)
+
+	mock.UserProviderMock.EXPECT().
+		GetDetails(gomock.Eq("john")).
+		Return(&authentication.UserDetails{
+			Emails: []string{"john@example.com"},
+			Groups: []string{"dev", "admin"},
+		}, nil)
+
+	VerifyGet(mock.Ctx)
+
+	assert.Equal(s.T(), 401, mock.Ctx.Response.StatusCode())
+}
+
+func (s *BasicAuthorizationSuite) TestShouldApplyPolicyOfDenyDomain() {
+	mock := mocks.NewMockAutheliaCtx(s.T())
+	defer mock.Close()
+
+	mock.Ctx.Request.Header.Set("Proxy-Authorization", "Basic am9objpwYXNzd29yZA==")
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://deny.example.com")
+
+	mock.UserProviderMock.EXPECT().
+		CheckUserPassword(gomock.Eq("john"), gomock.Eq("password")).
+		Return(true, nil)
+
+	mock.UserProviderMock.EXPECT().
+		GetDetails(gomock.Eq("john")).
+		Return(&authentication.UserDetails{
+			Emails: []string{"john@example.com"},
+			Groups: []string{"dev", "admin"},
+		}, nil)
+
+	VerifyGet(mock.Ctx)
+
+	assert.Equal(s.T(), 403, mock.Ctx.Response.StatusCode())
+}
+
+func TestShouldVerifyAuthorizationsUsingBasicAuth(t *testing.T) {
+	suite.Run(t, NewBasicAuthorizationSuite())
 }
 
 func TestShouldVerifyWrongCredentialsInBasicAuth(t *testing.T) {

internal/mocks/mock_authelia_ctx.go

@@ -62,6 +62,9 @@ func NewMockAutheliaCtx(t *testing.T) *MockAutheliaCtx {
 	mockAuthelia := new(MockAutheliaCtx)
 	mockAuthelia.Clock = TestingClock{}
 
+	datetime, _ := time.Parse("2006-Jan-02", "2013-Feb-03")
+	mockAuthelia.Clock.Set(datetime)
+
 	configuration := schema.Configuration{
 		AccessControl: new(schema.AccessControlConfiguration),
 	}

internal/regulation/regulator_test.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/clems4ever/authelia/internal/configuration/schema"
+	"github.com/clems4ever/authelia/internal/mocks"
 	"github.com/clems4ever/authelia/internal/models"
 	"github.com/clems4ever/authelia/internal/regulation"
 	"github.com/clems4ever/authelia/internal/storage"
@@ -19,7 +20,7 @@ type RegulatorSuite struct {
 	ctrl          *gomock.Controller
 	storageMock   *storage.MockProvider
 	configuration schema.RegulationConfiguration
-	now           time.Time
+	clock         mocks.TestingClock
 }
 
 func (s *RegulatorSuite) SetupTest() {
@@ -31,7 +32,7 @@ func (s *RegulatorSuite) SetupTest() {
 		BanTime:    180,
 		FindTime:   30,
 	}
-	s.now = time.Now()
+	s.clock.Set(time.Now())
 }
 
 func (s *RegulatorSuite) TearDownTest() {
@@ -43,7 +44,7 @@ func (s *RegulatorSuite) TestShouldNotThrowWhenUserIsLegitimate() {
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: true,
-			Time:       s.now.Add(-4 * time.Minute),
+			Time:       s.clock.Now().Add(-4 * time.Minute),
 		},
 	}
 
@@ -51,7 +52,7 @@ func (s *RegulatorSuite) TestShouldNotThrowWhenUserIsLegitimate() {
 		LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()).
 		Return(attempts, nil)
 
-	regulator := regulation.NewRegulator(&s.configuration, s.storageMock)
+	regulator := regulation.NewRegulator(&s.configuration, s.storageMock, &s.clock)
 
 	_, err := regulator.Regulate("john")
 	assert.NoError(s.T(), err)
@@ -64,17 +65,17 @@ func (s *RegulatorSuite) TestShouldNotThrowWhenFailedAuthenticationNotInFindTime
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-1 * time.Second),
+			Time:       s.clock.Now().Add(-1 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-90 * time.Second),
+			Time:       s.clock.Now().Add(-90 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-180 * time.Second),
+			Time:       s.clock.Now().Add(-180 * time.Second),
 		},
 	}
 
@@ -82,7 +83,7 @@ func (s *RegulatorSuite) TestShouldNotThrowWhenFailedAuthenticationNotInFindTime
 		LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()).
 		Return(attemptsInDB, nil)
 
-	regulator := regulation.NewRegulator(&s.configuration, s.storageMock)
+	regulator := regulation.NewRegulator(&s.configuration, s.storageMock, &s.clock)
 
 	_, err := regulator.Regulate("john")
 	assert.NoError(s.T(), err)
@@ -95,22 +96,22 @@ func (s *RegulatorSuite) TestShouldBanUserIfLatestAttemptsAreWithinFinTime() {
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-1 * time.Second),
+			Time:       s.clock.Now().Add(-1 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-4 * time.Second),
+			Time:       s.clock.Now().Add(-4 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-6 * time.Second),
+			Time:       s.clock.Now().Add(-6 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-180 * time.Second),
+			Time:       s.clock.Now().Add(-180 * time.Second),
 		},
 	}
 
@@ -118,7 +119,7 @@ func (s *RegulatorSuite) TestShouldBanUserIfLatestAttemptsAreWithinFinTime() {
 		LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()).
 		Return(attemptsInDB, nil)
 
-	regulator := regulation.NewRegulator(&s.configuration, s.storageMock)
+	regulator := regulation.NewRegulator(&s.configuration, s.storageMock, &s.clock)
 
 	_, err := regulator.Regulate("john")
 	assert.Equal(s.T(), regulation.ErrUserIsBanned, err)
@@ -133,17 +134,17 @@ func (s *RegulatorSuite) TestShouldCheckUserIsStillBanned() {
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-31 * time.Second),
+			Time:       s.clock.Now().Add(-31 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-34 * time.Second),
+			Time:       s.clock.Now().Add(-34 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-36 * time.Second),
+			Time:       s.clock.Now().Add(-36 * time.Second),
 		},
 	}
 
@@ -151,7 +152,7 @@ func (s *RegulatorSuite) TestShouldCheckUserIsStillBanned() {
 		LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()).
 		Return(attemptsInDB, nil)
 
-	regulator := regulation.NewRegulator(&s.configuration, s.storageMock)
+	regulator := regulation.NewRegulator(&s.configuration, s.storageMock, &s.clock)
 
 	_, err := regulator.Regulate("john")
 	assert.Equal(s.T(), regulation.ErrUserIsBanned, err)
@@ -162,12 +163,12 @@ func (s *RegulatorSuite) TestShouldCheckUserIsNotYetBanned() {
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-34 * time.Second),
+			Time:       s.clock.Now().Add(-34 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-36 * time.Second),
+			Time:       s.clock.Now().Add(-36 * time.Second),
 		},
 	}
 
@@ -175,7 +176,7 @@ func (s *RegulatorSuite) TestShouldCheckUserIsNotYetBanned() {
 		LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()).
 		Return(attemptsInDB, nil)
 
-	regulator := regulation.NewRegulator(&s.configuration, s.storageMock)
+	regulator := regulation.NewRegulator(&s.configuration, s.storageMock, &s.clock)
 
 	_, err := regulator.Regulate("john")
 	assert.NoError(s.T(), err)
@@ -186,7 +187,7 @@ func (s *RegulatorSuite) TestShouldCheckUserWasAboutToBeBanned() {
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-14 * time.Second),
+			Time:       s.clock.Now().Add(-14 * time.Second),
 		},
 		// more than 30 seconds elapsed between this auth and the preceding one.
 		// In that case we don't need to regulate the user even though the number
@@ -194,12 +195,12 @@ func (s *RegulatorSuite) TestShouldCheckUserWasAboutToBeBanned() {
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-94 * time.Second),
+			Time:       s.clock.Now().Add(-94 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-96 * time.Second),
+			Time:       s.clock.Now().Add(-96 * time.Second),
 		},
 	}
 
@@ -207,7 +208,7 @@ func (s *RegulatorSuite) TestShouldCheckUserWasAboutToBeBanned() {
 		LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()).
 		Return(attemptsInDB, nil)
 
-	regulator := regulation.NewRegulator(&s.configuration, s.storageMock)
+	regulator := regulation.NewRegulator(&s.configuration, s.storageMock, &s.clock)
 
 	_, err := regulator.Regulate("john")
 	assert.NoError(s.T(), err)
@@ -218,24 +219,24 @@ func (s *RegulatorSuite) TestShouldCheckRegulationHasBeenResetOnSuccessfulAttemp
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-90 * time.Second),
+			Time:       s.clock.Now().Add(-90 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: true,
-			Time:       s.now.Add(-93 * time.Second),
+			Time:       s.clock.Now().Add(-93 * time.Second),
 		},
 		// The user was almost banned but he did a successful attempt. Therefore, even if the next
 		// failure happens withing FindTime, he should not be banned.
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-94 * time.Second),
+			Time:       s.clock.Now().Add(-94 * time.Second),
 		},
 		models.AuthenticationAttempt{
 			Username:   "john",
 			Successful: false,
-			Time:       s.now.Add(-96 * time.Second),
+			Time:       s.clock.Now().Add(-96 * time.Second),
 		},
 	}
 
@@ -243,7 +244,7 @@ func (s *RegulatorSuite) TestShouldCheckRegulationHasBeenResetOnSuccessfulAttemp
 		LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()).
 		Return(attemptsInDB, nil)
 
-	regulator := regulation.NewRegulator(&s.configuration, s.storageMock)
+	regulator := regulation.NewRegulator(&s.configuration, s.storageMock, &s.clock)
 
 	_, err := regulator.Regulate("john")
 	assert.NoError(s.T(), err)