Fix UPDB Authentication Lockout Logic and Improve Test Coverage

[jan94]

Hi OpenZiti Devs!

I have noticed that there is bug in the way how UPDB logins are handled when maxAttempts and lockoutDurationMinutes is configured on the auth-policy attached to the respective identity. I have raised that issue in the discourse forum, but it has not yet received a response, so I investigated my self and implemented this fix. 🤓

Please have a look and consider merging it! 🙂

Thank you and have a nice weekend
Jan

Summary of Changes

UPDB Authentication Logic (controller/model/authenticator_mod_updb.go):

• Refactored login attempt counting: Only failed authentication attempts increment the counter; successful logins reset it.
• Lockout logic now disables identities only after the configured number of failed attempts (not on successful attempts).
• Removed attempts tracking after lockout or successful authentication to prevent stale state.

Tests (tests/auth_updb_test.go):

• Updated and expanded test cases to match new authentication logic:
  • Added a test to ensure disabled identities cannot log in.
  • Adjusted test logic to reflect that only failed attempts count toward lockout.
  • Added checks for disabledAt and disabledUntil fields when identity is locked.
  • Verified lockout duration matches configuration.

Test Context (tests/context.go):

• Added fields for test user authenticator and authentication policy to support new and updated tests.
• Configured test user authentication policy with lockout settings.

Impact

• Ensures that only failed login attempts are counted toward lockout.
• Improves reliability and coverage of authentication and lockout tests.
• This PR strengthens UPDB authentication security and test coverage, ensuring correct lockout and reset behavior.

fix #3333

[jan94]

Side note:
I won't be available for the next two weeks, but after that I am happy to do any changes - in case necessary 🙂

[jan94]

Just noticed that the test run has failed. This is most likely due to me adding an additional TestUser to the test context.

The test Test_Authenticators_AdminUsingAdminEndpoints expects to receive three authenticators from the controller, this is now different, because of the added TestUser, including the related authenticator. I have now adjusted the test accordingly and run all tests again.

[jan94]

@qrkourier
just saw that a SmokeTest seemed to fail, I have now rebased onto the main branch - could you please approve the action run again ?
Thank you 🙂

[jan94]

On my fork of the ziti project all tests pass successfully. I cannot follow through, why they should fail here :(

My fork is synced to the newest commit and the dev-branch is rebased on main... 😕

[jan94]

Hi there,
I have rebased my feature/fix branch again, and all tests pass (expect smoke ones, which according to the workflow definition are not meant to be run on my side).

See: https://github.com/jan94/ziti/actions/runs/18552220976

[jan94]

The error at the Fablab Smoketest is:

Run $(go env GOPATH)/bin/ziti-ci configure-git
unable to read ssh key from env var gh_ci_key. Found? true

I did not do any changes regarding the actions nor any ssh keys.
Could you point to sth. this issue may be related with ?

[qrkourier]

The error at the Fablab Smoketest is:

Run $(go env GOPATH)/bin/ziti-ci configure-git
unable to read ssh key from env var gh_ci_key. Found? true

I did not do any changes regarding the actions nor any ssh keys. Could you point to sth. this issue may be related with ?

Hi Jan, I suspect this test will succeed only when the job is running under the upstream organization. The private key setup is a little complicated. I'll let the team know this is ready for review. Thank you for the pull!

[jan94]

You are most welcome 👍

[andrewpmartinez]

I have taken an initial look, but not to the level I needed in order to provide constructive feedback. I am in the middle of delivering another set of features and will circle back to this after I get those features up for review.

[andrewpmartinez]

Some bookkeeping needs to be done no matter what:

1. Log an issue and tie your commits to it. Make sure the issue title clearly describes the problem, and the body of the issue explains what is happening, what is expected, and how to reproduce it.
2. Then ensure your commit messages in git has fix openziti/ziti#<issuer-number> as the first line of the commit message, followed by a blank line, and then whatever detail you want to provide.
3. Update your PR to include the same text fix openziti/ziti#<issue-number> either in the title or body

The above allows github to link the issuer, pr, and commits. Additionally, our change log tooling will add the issue to the fix line should it be released.

[jan94]

All changed. Thank you for the hint(s)!