From 31ca4f891f2df54674ab7e44abd31c15f0016ec8 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Mon, 27 Jan 2020 08:42:05 +1100 Subject: [PATCH] [FIX] Disable regulation when max_retries set to 0 (#584) - Only set regulator to enabled if max_retries is not set to 0, default is false (zero value). - Added test for the scenario. - Fixes #584 --- internal/regulation/regulator.go | 3 +- internal/regulation/regulator_test.go | 51 +++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/internal/regulation/regulator.go b/internal/regulation/regulator.go index 2478ab688..5fc6ead18 100644 --- a/internal/regulation/regulator.go +++ b/internal/regulation/regulator.go @@ -18,7 +18,8 @@ func NewRegulator(configuration *schema.RegulationConfiguration, provider storag if configuration.FindTime > configuration.BanTime { panic(fmt.Errorf("find_time cannot be greater than ban_time")) } - regulator.enabled = true + // Set regulator enabled only if MaxRetries is not 0. + regulator.enabled = configuration.MaxRetries > 0 regulator.maxRetries = configuration.MaxRetries regulator.findTime = time.Duration(configuration.FindTime) * time.Second regulator.banTime = time.Duration(configuration.BanTime) * time.Second diff --git a/internal/regulation/regulator_test.go b/internal/regulation/regulator_test.go index 8731dba69..3bc04e718 100644 --- a/internal/regulation/regulator_test.go +++ b/internal/regulation/regulator_test.go @@ -40,7 +40,7 @@ func (s *RegulatorSuite) TearDownTest() { } func (s *RegulatorSuite) TestShouldNotThrowWhenUserIsLegitimate() { - attempts := []models.AuthenticationAttempt{ + attemptsInDB := []models.AuthenticationAttempt{ models.AuthenticationAttempt{ Username: "john", Successful: true, @@ -50,7 +50,7 @@ func (s *RegulatorSuite) TestShouldNotThrowWhenUserIsLegitimate() { s.storageMock.EXPECT(). LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()). - Return(attempts, nil) + Return(attemptsInDB, nil) regulator := regulation.NewRegulator(&s.configuration, s.storageMock, &s.clock) @@ -254,3 +254,50 @@ func TestRunRegulatorSuite(t *testing.T) { s := new(RegulatorSuite) suite.Run(t, s) } + +// This test checks that the regulator is disabled when configuration is set to 0. +func (s *RegulatorSuite) TestShouldHaveRegulatorDisabled() { + attemptsInDB := []models.AuthenticationAttempt{ + models.AuthenticationAttempt{ + Username: "john", + Successful: false, + Time: s.clock.Now().Add(-31 * time.Second), + }, + models.AuthenticationAttempt{ + Username: "john", + Successful: false, + Time: s.clock.Now().Add(-34 * time.Second), + }, + models.AuthenticationAttempt{ + Username: "john", + Successful: false, + Time: s.clock.Now().Add(-36 * time.Second), + }, + } + + s.storageMock.EXPECT(). + LoadLatestAuthenticationLogs(gomock.Eq("john"), gomock.Any()). + Return(attemptsInDB, nil) + + // Check Disabled Functionality + configuration := schema.RegulationConfiguration{ + MaxRetries: 0, + FindTime: 180, + BanTime: 180, + } + + regulator := regulation.NewRegulator(&configuration, s.storageMock, &s.clock) + _, err := regulator.Regulate("john") + assert.NoError(s.T(), err) + + // Check Enabled Functionality + configuration = schema.RegulationConfiguration{ + MaxRetries: 1, + FindTime: 180, + BanTime: 180, + } + + regulator = regulation.NewRegulator(&configuration, s.storageMock, &s.clock) + _, err = regulator.Regulate("john") + assert.Equal(s.T(), regulation.ErrUserIsBanned, err) +}