[Gautam-aman]

Fixes timezone-dependent authentication bypass caused by JDBC getTimestamp() interpreting SQL DATETIME values using the JVM default timezone.

On non-UTC JVM deployments, suspended users could authenticate because suspendedUntil timestamps were interpreted incorrectly and compared against the wrong epoch time.

This PR standardizes JDBC timestamp reads using a shared UTC Calendar.

Changes:
Added shared UTC JDBC calendar utility

Suspended users are now correctly rejected during authentication regardless of JVM timezone.

[SeanDuggan]

Fix looks good, minor issue with spotless violation which has been resolved.

[ismisepaul]

Nice fix. The DbTime.UTC approach is correct and both getTimestamp (authUser + authUserSSO) are the only ones in the codebase, so the suspension-bypass bug is properly closed.

There is an open issue #841 which also looks at addressing both this, and CTF schedule gates. However, they're different mechanisms so let's get this merged in and that can be addressed in another pr (keeps this clean)

One thing before merge: this needs a regression test. CI runs on UTC, so this bug is invisible to it and can silently come back.

Suggestion: A unit test that puts the JVM in a non-UTC zone e.g. TimeZone.setDefault(US/Pacific), and assert the suspension timestamp still reads as the correct instant. So if anyone later drops the UTC calendar, CI fails immediately instead of the bug silently returning in production.

You can create it under src/test/java/utils/DbTimeTest.java

[Gautam-aman]

Added a regression test under utils/DbTimeTest that runs with a non-UTC JVM timezone and verifies the UTC calendar behavior remains correct. All tests pass locally.

[ismisepaul]

Thanks @Gautam-aman

One thing before we merge: as written the test doesn't quite guard the bug we care about. It asserts DbTime.UTC.getTimeZone().getID() is "UTC" which only confirms the constant was built with a UTC zone. We want to lock in the behaviour because if someone later drops the DbTime.UTC argument from the read path, this test would still pass as CI won't catch it since it runs in UTC.

Suggestion: instead of checking the DbTime.UTC constant directly, test the actual read. You can do this without a database by using Mockito (already a test dependency) to create a fake ResultSet (a stand-in object that returns whatever value you tell it to)

So something like

• Set the JVM's default timezone to something non-UTC (e.g. US/Pacific) at the start of the test, and restore the original in a finally block - like you're already doing.
• Create the fake ResultSet and tell it that when getTimestamp(col, DbTime.UTC) is called, it should return a timestamp for a known moment in time.
• Call that method and check the result matches the exact moment you expect (compare the epoch-millisecond values, since that's the absolute instant independent of any timezone).

The key is that the expected value is worked out as UTC. If the read still lands on the correct instant even though the JVM is set to US/Pacific, that proves the DbTime.UTC calendar is doing its job and the test will fail the day someone removes it.

[Gautam-aman]

Updated the regression test to exercise the actual timestamp read path using a mocked ResultSet rather than asserting the UTC constant directly. The test now runs under a non-UTC JVM timezone and verifies the returned instant matches the expected UTC instant.

[ismisepaul]

@Gautam-aman thanks for that a couple of things more (there's been a big change merged since your PR #816 so I went ahead and resolved the conflicts)

• CI red (spotless): no actual test failed, just formatting - DbTimeTest.java uses tabs + no trailing newline. If you could run mvn spotless:apply and commit (needs Java 17).
• DbTime.UTC isn't thread-safe: Calendar is mutable and shared across concurrent logins. Use ThreadLocal
• DbTimeTest doesn't test the fix: it stubs the method under test, so it'd pass even if DbTime weren't UTC. Make it exercise real UTC-vs-JVM conversion.

Side note: issue #841 captures some other concerns related to this - schedule gates, non-UTC ITs - not for this PR just noting here.

[Gautam-aman]

Addressed the thread-safety concern by changing the shared UTC calendar to a ThreadLocal<Calendar> and updated the timestamp reads accordingly.
I also rebased onto the latest branch changes and verified the project builds successfully locally.

[ismisepaul]

@Gautam-aman thanks for sticking with this. The DbTime.java ThreadLocal change is spot on. There's one issue left though, and it's why integration-tests is failing.

The Getter.java in the PR has reverted the connection-pooling work from #816 (and the rowset cap from #839). The failing check shows it directly:

Getter.java:87 - Could create get core connection:
SQLTransientConnectionException: CorePool - Connection is not available, request timed out after 5000ms.
That's the exact connection-exhaustion bug #816 fixed, coming back because the Phase 1/2/3 connection handling (and MAX_ROWSET_ROWS / CachedRowSet) got dropped from Getter.java. It builds for you locally because the old code still compiles fine — the connection leak only shows up under the integration-test load, not at compile time.

What happened is the merge took your branch's older Getter.java over dev's version rather than combining them. The fix is to take dev's Getter.java as-is and re-apply just your two timestamp reads on top, I think something like

git fetch upstream
git checkout fix/timezone-auth-bypass
git checkout upstream/dev -- src/main/java/dbProcs/Getter.java

Then in that file re-add only:

• import utils.DbTime;
• in authUser: suspendedUntil = userResult.getTimestamp(8, DbTime.UTC.get());
• in authUserSSO: suspendedUntil = userResult.getTimestamp(7, DbTime.UTC.get());

git add src/main/java/dbProcs/Getter.java
git commit -m "fix: restore dev Getter.java, keep only UTC timestamp reads"
git push

After that the Getter.java diff should be ~5 lines instead of ~1500, and the connection-pool test failure will clear. DbTime.java is good as-is — no change needed there.

[Gautam-aman]

Thanks for the review and apologies for the delay in addressing this. I was tied up with other commitments for a bit.
I've now rebased onto the latest branch changes, restored Getter.java from the current branch state as suggested, and kept only the UTC timestamp handling changes. I also verified the project builds successfully locally after the rebase.
Please let me know if there's anything else you'd like adjusted.

[Gautam-aman]

Addressed the latest feedback and verified all checks are now passing. Please let me know if there's anything else that should be updated; otherwise I believe this is ready for another review.

Thanks again for the guidance throughout the review process.

[Gautam-aman]

@ismisepaul please look at it whenever you have time

[ismisepaul]

Thanks for sticking with this @Gautam-aman — a couple of things before this can go in:

1. suspendedUntil is written by the stored procedures from MariaDB's NOW(), i.e. in the DB server's local time, not UTC. Reading it with a UTC calendar is only correct when the DB runs in UTC. Our default Docker setup runs MariaDB and Tomcat with TZ=Europe/Dublin. During Irish summer time that makes every suspension one hour too long, so a single wrong password locks the user out for an hour. On a US/Pacific deployment it makes suspensions 7–8 hours too short, so the lockout is bypassed. Could you move the comparison into SQL instead (e.g. SELECT ..., suspendedUntil > NOW() AS suspended ...) so the same clock writes and checks the value? That works whatever timezone the JVM or DB is in, and DbTime is no longer needed.
2. src/test/java/utils/DbTimeTest.java was dropped in a77e866c, so there's no regression test anymore. With the SQL-side check, an integration test that suspends a user and then calls authUser while the JVM runs in a non-UTC zone would cover it.

[Gautam-aman]

Thanks for the review. I pushed the SQL-side suspension check and regression test in e2b698d. I could not run the full suite locally because Docker and Maven Central were unavailable; I will follow up on any CI failures.

[Copilot]

🟢 Approval recommended

The suspension checks align with the database clock used for suspension writes, with targeted regression coverage and no identified blocking issues.

No open issues 🎉

What changed in this PR

Fixes timezone-dependent suspension checks in password and SSO authentication. The implementation compares timestamps against the database clock rather than introducing the UTC calendar described in the PR text.

Changes:

• Evaluates suspension status in SQL using suspendedUntil > NOW().
• Adds an integration regression test with differing database and JVM timezones.

File	Description
src/​main/​java/​dbProcs/​Getter.java	Uses database-side suspension checks for password and SSO authentication.
src/​it/​java/​dbProcs/​GetterAuthIT.java	Tests suspension enforcement across mismatched timezones.

_{Review effort: Balanced}

[ismisepaul]

LGTM thanks for the contribution @Gautam-aman