diff --git a/controller/model/authenticator_mod_updb.go b/controller/model/authenticator_mod_updb.go index 2c22a73fd..f9013db76 100644 --- a/controller/model/authenticator_mod_updb.go +++ b/controller/model/authenticator_mod_updb.go @@ -21,12 +21,13 @@ import ( "encoding/base64" "errors" "fmt" + "time" + "github.com/michaelquigley/pfxlog" "github.com/openziti/foundation/v2/errorz" "github.com/openziti/ziti/controller/apierror" "github.com/openziti/ziti/controller/db" cmap "github.com/orcaman/concurrent-map/v2" - "time" ) var _ AuthProcessor = &AuthModuleUpdb{} @@ -72,7 +73,6 @@ func (module *AuthModuleUpdb) Process(context AuthContext) (AuthResult, error) { if username == "" || password == "" { reason := "username and password fields are required" failEvent := module.NewAuthEventFailure(context, bundle, reason) - module.DispatchEvent(failEvent) return nil, errorz.NewCouldNotValidate(errors.New(reason)) } @@ -154,30 +154,16 @@ func (module *AuthModuleUpdb) Process(context AuthContext) (AuthResult, error) { } attempts := int64(0) - module.attemptsByAuthenticatorId.Upsert(bundle.Authenticator.Id, 0, func(exist bool, prevAttempts int64, newValue int64) int64 { + module.attemptsByAuthenticatorId.Upsert(bundle.Authenticator.Id, 1, func(exist bool, prevAttempts int64, attemptIncrement int64) int64 { if exist { - attempts = prevAttempts + 1 - return attempts + attempts = prevAttempts + attemptIncrement + } else { + attempts = attemptIncrement } - return 0 + return attempts }) - if bundle.AuthPolicy.Primary.Updb.MaxAttempts != db.UpdbUnlimitedAttemptsLimit && attempts > bundle.AuthPolicy.Primary.Updb.MaxAttempts { - reason := fmt.Sprintf("updb auth failed, max attempts exceeded, attempts: %v, maxAttempts: %v", attempts, bundle.AuthPolicy.Primary.Updb.MaxAttempts) - failEvent := module.NewAuthEventFailure(context, bundle, reason) - - module.DispatchEvent(failEvent) - logger.WithField("attempts", attempts).WithField("maxAttempts", bundle.AuthPolicy.Primary.Updb.MaxAttempts).Error(reason) - - duration := time.Duration(bundle.AuthPolicy.Primary.Updb.LockoutDurationMinutes) * time.Minute - if err = module.env.GetManagers().Identity.Disable(bundle.Authenticator.IdentityId, duration, context.GetChangeContext()); err != nil { - logger.WithError(err).Error("could not lock identity, unhandled error") - } - - return nil, apierror.NewInvalidAuth() - } - updb := bundle.Authenticator.ToUpdb() salt, err := DecodeSalt(updb.Salt) @@ -201,9 +187,25 @@ func (module *AuthModuleUpdb) Process(context AuthContext) (AuthResult, error) { module.DispatchEvent(failEvent) logger.Error(reason) + if bundle.AuthPolicy.Primary.Updb.MaxAttempts != db.UpdbUnlimitedAttemptsLimit && attempts >= bundle.AuthPolicy.Primary.Updb.MaxAttempts { + reason := fmt.Sprintf("updb auth failed, max attempts exceeded, attempts: %v, maxAttempts: %v", attempts, bundle.AuthPolicy.Primary.Updb.MaxAttempts) + failEvent := module.NewAuthEventFailure(context, bundle, reason) + + module.DispatchEvent(failEvent) + logger.WithField("attempts", attempts).WithField("maxAttempts", bundle.AuthPolicy.Primary.Updb.MaxAttempts).Error(reason) + + duration := time.Duration(bundle.AuthPolicy.Primary.Updb.LockoutDurationMinutes) * time.Minute + if err = module.env.GetManagers().Identity.Disable(bundle.Authenticator.IdentityId, duration, context.GetChangeContext()); err != nil { + logger.WithError(err).Error("could not lock identity, unhandled error") + } + + module.attemptsByAuthenticatorId.Remove(bundle.Authenticator.Id) + } + return nil, apierror.NewInvalidAuth() } + module.attemptsByAuthenticatorId.Remove(bundle.Authenticator.Id) successEvent := module.NewAuthEventSuccess(context, bundle) module.DispatchEvent(successEvent) diff --git a/tests/auth_updb_test.go b/tests/auth_updb_test.go index 7a684b8d6..785d3df9f 100644 --- a/tests/auth_updb_test.go +++ b/tests/auth_updb_test.go @@ -21,18 +21,20 @@ package tests import ( "fmt" + "net/http" + "testing" + "time" + "github.com/Jeffail/gabs" "github.com/openziti/ziti/controller/env" "github.com/stretchr/testify/require" - "net/http" - "testing" + "gopkg.in/resty.v1" ) func Test_Authenticate_Updb(t *testing.T) { testContext := NewTestContext(t) defer testContext.Teardown() testContext.StartServer() - tests := &authUpdbTests{ ctx: testContext, } @@ -42,6 +44,43 @@ func Test_Authenticate_Updb(t *testing.T) { t.Run("login with missing password should fail", tests.testAuthenticateUPDBMissingPassword) t.Run("login with missing username should fail", tests.testAuthenticateUPDBMissingUsername) t.Run("admin login should pass", tests.testAuthenticateUPDBDefaultAdminSuccess) + t.Run("test user login should pass", tests.testAuthenticateUpdbTestUserSuccess) +} + +func Test_Lockout_Updb(t *testing.T) { + testContext := NewTestContext(t) + defer testContext.Teardown() + testContext.StartServer() + tests := &authUpdbTests{ + ctx: testContext, + } + + t.Run("identity should not be disabled, when logging in with correct credentials after exceeding maxAttempts", tests.testAuthenticateUpdbNotLockedAfterMaxAttempts) + t.Run("identity is disabled, as soon as failed login attempts exceed maxAttempts", tests.testAuthenticateUpdbLockedAfterMaxAttempts) + t.Run("Disabled identity must not be allowed to login", tests.testAuthenticateUpdbDisabledIdentityCannotLogin) + +} + +func Test_Lockout_Reset_On_Success_Updb(t *testing.T) { + testContext := NewTestContext(t) + defer testContext.Teardown() + testContext.StartServer() + tests := &authUpdbTests{ + ctx: testContext, + } + t.Run("login attempts are reset on successful login", tests.testAuthenticateUpdbAttemptsResetOnSuccess) + +} + +func Test_Lockout_Reset_After_Lockout_Duration_Updb(t *testing.T) { + testContext := NewTestContext(t) + defer testContext.Teardown() + testContext.StartServer() + tests := &authUpdbTests{ + ctx: testContext, + } + t.Run("login should succeed after lockout duration has eleapsed", tests.testAuthenticateUpdbUnlockAfterLockoutDuration) + } type authUpdbTests struct { @@ -187,3 +226,269 @@ func (tests *authUpdbTests) testAuthenticateUPDBDefaultAdminSuccess(t *testing.T r.NoError(err, "session token property in 'data.token' is empty") }) } + +func (tests *authUpdbTests) testAuthenticateUpdbTestUserSuccess(t *testing.T) { + body := gabs.New() + _, _ = body.SetP(tests.ctx.TestUserAuthenticator.Username, "username") + _, _ = body.SetP(tests.ctx.TestUserAuthenticator.Password, "password") + + resp, err := tests.ctx.DefaultClientApiClient().R(). + SetHeader("content-type", "application/json"). + SetBody(body.String()). + Post("authenticate?method=password") + + t.Run("should not have returned an error", func(t *testing.T) { + require.New(t).NoError(err) + }) + + standardJsonResponseTests(resp, http.StatusOK, t) + + t.Run("returns a session token HTTP headers", func(t *testing.T) { + require.New(t).NotEmpty(resp.Header().Get(env.ZitiSession), fmt.Sprintf("HTTP header %s is empty", env.ZitiSession)) + }) + + t.Run("returns a session token in body", func(t *testing.T) { + r := require.New(t) + data, err := gabs.ParseJSON(resp.Body()) + + r.NoError(err) + + r.True(data.ExistsP("data.token"), "session token property in 'data.token' as not found") + r.NotEmpty(data.Path("data.token").String(), "session token property in 'data.token' is empty") + }) + + t.Run("body session token matches HTTP header token", func(t *testing.T) { + r := require.New(t) + data, err := gabs.ParseJSON(resp.Body()) + + r.NoError(err) + + bodyToken := data.Path("data.token").Data().(string) + headerToken := resp.Header().Get(env.ZitiSession) + r.Equal(bodyToken, headerToken) + }) + + t.Run("returns an identity", func(t *testing.T) { + r := require.New(t) + data, err := gabs.ParseJSON(resp.Body()) + + r.NoError(err) + + r.True(data.ExistsP("data.identity"), "session token property in 'data.token' as not found") + + _, err = data.ObjectP("data.identity") + r.NoError(err, "session token property in 'data.token' is empty") + }) +} + +func (tests *authUpdbTests) testAuthenticateUpdbNotLockedAfterMaxAttempts(t *testing.T) { + r := require.New(t) + + username := tests.ctx.TestUserAuthenticator.Username + password := tests.ctx.TestUserAuthenticator.Password + maxAttempts := int(tests.ctx.TestUserAuthPolicy.Primary.Updb.MaxAttempts) + + for attempt := 1; attempt <= (maxAttempts + 1); attempt++ { + + body := gabs.New() + _, _ = body.SetP(username, "username") + _, _ = body.SetP(password, "password") + + resp, err := tests.ctx.DefaultClientApiClient().R(). + SetHeader("content-type", "application/json"). + SetBody(body.String()). + Post("authenticate?method=password") + r.NoError(err) + + testUserIdentity, err := tests.ctx.EdgeController.AppEnv.Managers.Identity.ReadByName(username) + r.NoError(err) + + t.Run("Identity must not be disabled, when logging with correct credentials after exceeding maxAttempts", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusOK) + r.False(testUserIdentity.Disabled) + }) + + } +} + +func (tests *authUpdbTests) testAuthenticateUpdbLockedAfterMaxAttempts(t *testing.T) { + r := require.New(t) + + username := tests.ctx.TestUserAuthenticator.Username + maxAttempts := int(tests.ctx.TestUserAuthPolicy.Primary.Updb.MaxAttempts) + + for attempt := 1; attempt <= maxAttempts; attempt++ { + + body := gabs.New() + _, _ = body.SetP(username, "username") + _, _ = body.SetP("wrong_password", "password") + + resp, err := tests.ctx.DefaultClientApiClient().R(). + SetHeader("content-type", "application/json"). + SetBody(body.String()). + Post("authenticate?method=password") + r.NoError(err) + + testUserIdentity, err := tests.ctx.EdgeController.AppEnv.Managers.Identity.ReadByName(username) + r.NoError(err) + + if attempt < maxAttempts { + t.Run("Identity should not be disabled, as long as maxAttempts are not exceeded", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusUnauthorized) + r.False(testUserIdentity.Disabled) + }) + } else { + t.Run("Identity should be disabled, as soon as last available attempt was not a successful login", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusUnauthorized) + r.True(testUserIdentity.Disabled) + }) + t.Run("Identity should have disabledAt and disabledUntil field set", func(t *testing.T) { + + r.NotEmpty(testUserIdentity.DisabledAt) + r.NotEmpty(testUserIdentity.DisabledUntil) + }) + + t.Run("Identity should disabledUntil field set, according to configured lockoutDuration", func(t *testing.T) { + + lockDuration := testUserIdentity.DisabledUntil.Sub(*testUserIdentity.DisabledAt) + expectedDuration := int(tests.ctx.TestUserAuthPolicy.Primary.Updb.LockoutDurationMinutes) + actualDuration := int(lockDuration.Minutes()) + r.Equal(expectedDuration, actualDuration, "lockout duration does not match configured value") + + }) + + } + } +} + +func (tests *authUpdbTests) testAuthenticateUpdbDisabledIdentityCannotLogin(t *testing.T) { + r := require.New(t) + username := tests.ctx.TestUserAuthenticator.Username + password := tests.ctx.TestUserAuthenticator.Password + + testUserIdentity, err := tests.ctx.EdgeController.AppEnv.Managers.Identity.ReadByName(username) + r.NoError(err) + r.True(testUserIdentity.Disabled) + + body := gabs.New() + _, _ = body.SetP(username, "username") + _, _ = body.SetP(password, "password") + + resp, err := tests.ctx.DefaultClientApiClient().R(). + SetHeader("content-type", "application/json"). + SetBody(body.String()). + Post("authenticate?method=password") + r.NoError(err) + + t.Run("Disabled identities must not be authorized when attempting to login", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusUnauthorized) + }) + t.Run("Identity should have disabledAt and disabledUntil field set", func(t *testing.T) { + + r.NotEmpty(testUserIdentity.DisabledAt) + r.NotEmpty(testUserIdentity.DisabledUntil) + }) +} + +func (tests *authUpdbTests) testAuthenticateUpdbAttemptsResetOnSuccess(t *testing.T) { + r := require.New(t) + + username := tests.ctx.TestUserAuthenticator.Username + password := "wrong_password" + maxAttempts := int(tests.ctx.TestUserAuthPolicy.Primary.Updb.MaxAttempts) + + for attempt := 1; attempt <= (maxAttempts + 1); attempt++ { + + body := gabs.New() + _, _ = body.SetP(username, "username") + _, _ = body.SetP(password, "password") + + resp, err := tests.ctx.DefaultClientApiClient().R(). + SetHeader("content-type", "application/json"). + SetBody(body.String()). + Post("authenticate?method=password") + r.NoError(err) + + testUserIdentity, err := tests.ctx.EdgeController.AppEnv.Managers.Identity.ReadByName(username) + r.NoError(err) + + // Switch to correct password on the third attempt to reset attempts, then back to wrong password + switch attempt { + case 1, 2: + t.Run("Identity must not be disabled until maxAttempts exceeded", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusUnauthorized) + r.False(testUserIdentity.Disabled) + }) + if attempt == 2 { + password = tests.ctx.TestUserAuthenticator.Password + } + case 3: + t.Run("Identity must not be disabled and login should be successful on the last allowed attempt", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusOK) + r.False(testUserIdentity.Disabled) + }) + password = "wrong_password" + default: + t.Run("Identity must not be disabled, as attempts should have been reset on last successful login", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusUnauthorized) + r.False(testUserIdentity.Disabled) + }) + } + } +} + +func (tests *authUpdbTests) testAuthenticateUpdbUnlockAfterLockoutDuration(t *testing.T) { + r := require.New(t) + + username := tests.ctx.TestUserAuthenticator.Username + password := "wrong_password" + maxAttempts := int(tests.ctx.TestUserAuthPolicy.Primary.Updb.MaxAttempts) + lockoutDuration := int(tests.ctx.TestUserAuthPolicy.Primary.Updb.LockoutDurationMinutes) + var resp *resty.Response + + for attempt := 1; attempt <= maxAttempts; attempt++ { + body := gabs.New() + _, _ = body.SetP(username, "username") + _, _ = body.SetP(password, "password") + + var err error + resp, err = tests.ctx.DefaultClientApiClient().R(). + SetHeader("content-type", "application/json"). + SetBody(body.String()). + Post("authenticate?method=password") + r.NoError(err) + } + + testUserIdentity, err := tests.ctx.EdgeController.AppEnv.Managers.Identity.ReadByName(username) + r.NoError(err) + + t.Run("Identity should be disabled", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusUnauthorized) + r.True(testUserIdentity.Disabled) + }) + + // Simulate waiting for lockout duration to expire + time.Sleep((time.Duration(lockoutDuration) * time.Minute) + (5 * time.Second)) + + testUserIdentity, err = tests.ctx.EdgeController.AppEnv.Managers.Identity.ReadByName(username) + r.NoError(err) + + t.Run("Identity should be enabled again", func(t *testing.T) { + r.False(testUserIdentity.Disabled) + }) + + // Try to login with correct credentials after lockout duration + body := gabs.New() + _, _ = body.SetP(username, "username") + _, _ = body.SetP(tests.ctx.TestUserAuthenticator.Password, "password") + + resp, err = tests.ctx.DefaultClientApiClient().R(). + SetHeader("content-type", "application/json"). + SetBody(body.String()). + Post("authenticate?method=password") + r.NoError(err) + + t.Run("UPDB login should succeed", func(t *testing.T) { + r.Equal(resp.StatusCode(), http.StatusOK) + }) +} diff --git a/tests/authenticator_test.go b/tests/authenticator_test.go index 235e0d6a3..885aaede7 100644 --- a/tests/authenticator_test.go +++ b/tests/authenticator_test.go @@ -21,6 +21,10 @@ package tests import ( "fmt" + "net/http" + "testing" + "time" + "github.com/Jeffail/gabs" "github.com/google/uuid" "github.com/openziti/edge-api/rest_model" @@ -28,9 +32,6 @@ import ( "github.com/openziti/ziti/common/eid" "github.com/openziti/ziti/controller/apierror" "github.com/stretchr/testify/require" - "net/http" - "testing" - "time" ) func Test_Authenticators_AdminUsingAdminEndpoints(t *testing.T) { @@ -53,12 +54,12 @@ func Test_Authenticators_AdminUsingAdminEndpoints(t *testing.T) { authenticatorsBody, err := gabs.ParseJSON(resp.Body()) req.NoError(err) - t.Run("can see three authenticators", func(t *testing.T) { + t.Run("can see four authenticators", func(t *testing.T) { req := require.New(t) count, err := authenticatorsBody.ArrayCount("data") req.NoError(err) - req.Equal(3, count, "expected three authenticators") + req.Equal(4, count, "expected four authenticators") }) t.Run("ott listed authenticator has isIssuedByNetwork=true", func(t *testing.T) { diff --git a/tests/context.go b/tests/context.go index fbeafd08c..c37fa4b17 100644 --- a/tests/context.go +++ b/tests/context.go @@ -41,6 +41,8 @@ import ( edge_apis "github.com/openziti/sdk-golang/edge-apis" "github.com/openziti/ziti/controller/config" + "github.com/openziti/ziti/controller/model" + "github.com/openziti/ziti/controller/models" env2 "github.com/openziti/ziti/router/env" "github.com/openziti/ziti/router/xgress_router" "github.com/openziti/ziti/zitirest" @@ -66,6 +68,7 @@ import ( "github.com/openziti/ziti/common" "github.com/openziti/ziti/common/eid" "github.com/openziti/ziti/controller" + "github.com/openziti/ziti/controller/db" "github.com/openziti/ziti/controller/env" "github.com/openziti/ziti/controller/server" "github.com/openziti/ziti/controller/xt_smartrouting" @@ -126,6 +129,8 @@ type TestContext struct { *require.Assertions ApiHost string AdminAuthenticator *updbAuthenticator + TestUserAuthenticator *updbAuthenticator + TestUserAuthPolicy *model.AuthPolicy AdminManagementSession *session AdminClientSession *session RestClients *zitirest.Clients @@ -149,6 +154,24 @@ var defaultTestContext = &TestContext{ Username: eid.New(), Password: eid.New(), }, + TestUserAuthenticator: &updbAuthenticator{ + Username: eid.New(), + Password: eid.New(), + }, + TestUserAuthPolicy: &model.AuthPolicy{ + Name: "test-auth-policy", + Primary: model.AuthPolicyPrimary{ + Updb: model.AuthPolicyUpdb{ + Allowed: true, + MinPasswordLength: 8, + RequireSpecialChar: false, + RequireMixedCase: false, + RequireNumberChar: false, + MaxAttempts: int64(3), + LockoutDurationMinutes: int64(1), + }, + }, + }, } func NewTestContext(t *testing.T) *TestContext { @@ -158,6 +181,24 @@ func NewTestContext(t *testing.T) *TestContext { Username: eid.New(), Password: eid.New(), }, + TestUserAuthenticator: &updbAuthenticator{ + Username: eid.New(), + Password: eid.New(), + }, + TestUserAuthPolicy: &model.AuthPolicy{ + Name: "test-auth-policy", + Primary: model.AuthPolicyPrimary{ + Updb: model.AuthPolicyUpdb{ + Allowed: true, + MinPasswordLength: 8, + RequireSpecialChar: false, + RequireMixedCase: false, + RequireNumberChar: false, + MaxAttempts: int64(3), + LockoutDurationMinutes: int64(1), + }, + }, + }, LogLevel: os.Getenv("ZITI_TEST_LOG_LEVEL"), Req: require.New(t), } @@ -446,8 +487,41 @@ func (ctx *TestContext) StartServerFor(testDb string, clean bool) { log.WithError(err).Warn("error during initialize admin") } - logrus.Infof("username: %v", ctx.AdminAuthenticator.Username) - logrus.Infof("password: %v", ctx.AdminAuthenticator.Password) + logrus.Infof("default admin - username: %v", ctx.AdminAuthenticator.Username) + logrus.Infof("default admin - password: %v", ctx.AdminAuthenticator.Password) + + authPolicyManager := ctx.EdgeController.AppEnv.Managers.AuthPolicy + err = authPolicyManager.Create(ctx.TestUserAuthPolicy, nil) + ctx.Req.NoError(err) + + identityManager := ctx.EdgeController.AppEnv.Managers.Identity + identity := &model.Identity{ + Name: ctx.TestUserAuthenticator.Username, + IdentityTypeId: db.DefaultIdentityType, + IsAdmin: false, + AuthPolicyId: ctx.TestUserAuthPolicy.Id, + } + + err = identityManager.Create(identity, nil) + ctx.Req.NoError(err) + + authenticatorManager := ctx.EdgeController.AppEnv.Managers.Authenticator + testUserAuthenticator := &model.Authenticator{ + BaseEntity: models.BaseEntity{}, + Method: db.MethodAuthenticatorUpdb, + IdentityId: identity.Id, + SubType: &model.AuthenticatorUpdb{ + Username: ctx.TestUserAuthenticator.Username, + Password: ctx.TestUserAuthenticator.Password, + }, + } + + err = authenticatorManager.Create(testUserAuthenticator, nil) + ctx.Req.NoError(err) + + logrus.Infof("Created test user: %v with custom auth policy: %v", ctx.TestUserAuthenticator.Username, ctx.TestUserAuthPolicy.Id) + logrus.Infof("test user - username: %v", ctx.TestUserAuthenticator.Username) + logrus.Infof("test user - password: %v", ctx.TestUserAuthenticator.Password) ctx.EdgeController.Run() go func() {