Support PostgreSQL; add automated tests and CI - #227
Merged
Merged
Conversation
On a Joomla site running on PostgreSQL, the plugin installed "successfully" without creating any tables (only MySQL install SQL existed), and even with the tables in place most queries used MySQL-only SQL. All database errors are caught and turned into 0/false/empty results, so the plugin silently never blocked anything. - Add PostgreSQL install/uninstall SQL and a PostgreSQL update schema path (starting at 2.0.0) - Match IP subnets (CIDR) of block and allow list entries in PHP (IpHelper::isInSubnet) instead of with INET_ATON/INET6_ATON/LOCATE/REGEXP; only the literal address match remains in SQL, compared case-insensitively as MySQL's collations did - Compute fixed time windows (DATE_SUB/DATE_ADD with constant intervals) in PHP; the two column-based "crdate + duration minutes" expressions get a PostgreSQL variant - saveParams: no table alias in UPDATE ... SET (not allowed in PostgreSQL) - updatescript: only try the old whitelist rename on MySQL - insertFailedLogin: catch database exceptions like the other methods - Add unit test for the subnet matching Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsFEnqepXD2eL6yLVZmSB9
- tests/Unit: IpHelper subnet matching (moved from unittests/) - tests/Integration: run against a real Joomla site with bfstop installed: installer result (tables, schema version), every DatabaseHelper query, the component's database access (if COM_BFSTOP_ROOT is set), and the plugin reacting to Joomla events (failed logins leading to a block, requests from blocked IPs/subnets being rejected, allow list). Tests fail if the code under test logged an error, since the plugin turns database exceptions into log entries. - tests/ci: scripts to start a database container and to set up a Joomla site with plugin and component installed from the checkouts - CI workflow: syntax check on PHP 8.1 and 8.4, unit tests, and the integration tests on Joomla 5.4 and 6.1 with MySQL 8.0/8.4, MariaDB 10.11/11.4 and PostgreSQL 12/17 - Remove unittests/cryptotest.php, which referred to no longer existing classes and files; replaced by tests/Integration/TokenHelperTest.php Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsFEnqepXD2eL6yLVZmSB9
v4 runs on the deprecated Node.js 20 runtime, which GitHub warned about; v7 runs on Node.js 24. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsFEnqepXD2eL6yLVZmSB9
Merge the per-username failed login statistics (#136) and the utf8mb4 change from main, and make the statistics work on PostgreSQL too: - install/uninstall.postgresql.utf8.sql: add the #__bfstop_username_stats table and the #__bfstop_failedlogin (username, logtime) index - recordUsernameAttempt: use INSERT ... ON CONFLICT DO UPDATE on PostgreSQL, as ON DUPLICATE KEY UPDATE is MySQL-only - tests: cover the statistics in the plugin (counting, not purged) and the component's UsernamestatsModel Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LsFEnqepXD2eL6yLVZmSB9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #206. Companion PR: codeling/com_bfstop#13 (same branch name).
Problem
On Joomla with PostgreSQL, the plugin installed "successfully" but created no tables, because only MySQL install SQL existed. Even with tables in place, most queries used MySQL-only SQL (
DATE_SUB/DATE_ADD … INTERVAL,LOCATE,REGEXP,INET_ATON,INET6_ATON,CONV,CAST … AS SIGNED, a table alias inUPDATE … SET).DatabaseHelpercatches every database exception and returns 0/false/[], so the plugin failed open: it never blocked anything.Changes: PostgreSQL support
sql/install.postgresql.utf8.sql,sql/uninstall.postgresql.utf8.sqland a PostgreSQL schema pathsql/updates/postgresql/(baseline2.0.0.sql), all wired intobfstop.xml. They include the username statistics table from Username statistics #136 and thefailedlogin (username, logtime)index. Differences from the MySQL schema:handledissmallint, because the code compares it with 0/1, which PostgreSQL doesn't allow forboolean.ipaddressisvarchar(49)inbannedip/allowlist, as on MySQL installs upgraded through1.2.0.sql.IpHelper::isInSubnet) instead of through MySQL-only SQL:LOWER()on both sides so it stays case-insensitive as it was on MySQL.crdate+durationminutes" expressions get a PostgreSQL variant viaaddMinutesSql(). Joomla'sDatabaseQuery::dateAdd()can't be used for these, because on PostgreSQL it doesn't accept a column as the duration.main): the upsert usesINSERT … ON CONFLICT (username) DO UPDATEon PostgreSQL, sinceON DUPLICATE KEY UPDATEis MySQL-only.saveParams(): removed the table alias inUPDATE … SET, which PostgreSQL rejects.updatescript.php: the oldwhitelist→allowlistrename now only runs on MySQL.Changes: automated tests and CI
There was no CI before.
tests/README.mdexplains how to run everything locally.tests/Unit: subnet matching (moved fromunittests/).tests/Integration: runs against a real Joomla site with plugin and component installed through Joomla's own extension installer:InstallTest: the installer created all tables, enabled the plugin and recorded the schema version (Joomla reports success even when it created nothing).DatabaseHelperTest: everyDatabaseHelperquery, checked for correct results: counting, exact/expired/unlimited/unblocked blocks, IPv4/IPv6 subnets, corrupted entries, allow list, tokens, known IPs, DNS cache, username statistics, purging,saveParams.ComponentTest: the component's database access, including saving new blocks and allow-list entries through the admin models, email-token unblocking, the username statistics view and the "admin" user warning.PluginEventsTest: the plugin as Joomla runs it. Failed-login events lead to a block, and a request from the blocked IP or subnet is rejected (run as a separate PHP process, since the pluginexits). The allow list takes precedence, and a successful login records a known IP.tests/ci/:start-database.shstarts a database container.install-joomla.shdownloads Joomla, installs it on that database and installs plugin and component from the checkouts..github/workflows/ci.yml:main. The Joomla versions are pinned in the matrix.actions/checkout@v7, which runs on Node 24.unittests/cryptotest.php: it referred to classes and files that no longer exist.TokenHelperTestreplaces it.Testing
main: all 25 tests pass.postgres:12in Docker: ran both repos' workflow integration jobs locally by extracting theirrun:steps fromci.yml; all pass.InstallTest("array containsjos_bfstop_failedlogin").ON DUPLICATE KEY UPDATEalso used on PostgreSQL, the username statistics tests fail.ComponentTestshowed that saving a new block or allow-list entry from the backend failed on PostgreSQL. The fix is in Support PostgreSQL; add CI com_bfstop#13.actionlintandshellcheckare clean.Notes / not changed
varchar(45)forbannedip/allowlistipaddress, while upgraded MySQL installs havevarchar(49).🤖 Generated with Claude Code
https://claude.ai/code/session_01LsFEnqepXD2eL6yLVZmSB9