From 2e52deffd449e52dcf9209f09f3af0dccf3677ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Sat, 21 Mar 2026 20:50:36 +0100 Subject: [PATCH 1/3] Add MySQL/PgSQL Authentication unit tests (Phase 2.2, #5474) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comprehensive unit tests for MySQL_Authentication and PgSQL_Authentication covering 58 test cases across 13 test functions. Runs in <0.01s with no infrastructure dependencies. MySQL_Authentication coverage: - Core CRUD: add/exists/lookup/del with frontend and backend users - Duplicate handling: add() overwrites password, hostgroup, max_connections - exists() only checks frontends (not backends) — documented quirk - lookup() returns empty struct for nonexistent users - SHA1 credential storage and retrieval via set_SHA1() - Clear-text password management (PRIMARY and ADDITIONAL slots) - Connection counting: increase/decrease with max_connections enforcement - Bulk operations: set_all_inactive + remove_inactives config reload pattern - reset() clears all frontend and backend users - Runtime checksum: zero when empty, changes on add/modify - Memory tracking: increases on add, returns to baseline on reset - Frontend/backend separation: same username, different credentials PgSQL_Authentication coverage: - Core CRUD with PgSQL-specific lookup() signature (returns char*) - del() and reset() behavior - Connection counting (no PASSWORD_TYPE parameter) - Inactive pattern (set_all_inactive + remove_inactives) --- test/tap/tests/unit/auth_unit-t.cpp | 579 ++++++++++++++++++++++++++++ 1 file changed, 579 insertions(+) create mode 100644 test/tap/tests/unit/auth_unit-t.cpp diff --git a/test/tap/tests/unit/auth_unit-t.cpp b/test/tap/tests/unit/auth_unit-t.cpp new file mode 100644 index 0000000000..839e81c5f8 --- /dev/null +++ b/test/tap/tests/unit/auth_unit-t.cpp @@ -0,0 +1,579 @@ +/** + * @file auth_unit-t.cpp + * @brief Unit tests for MySQL_Authentication and PgSQL_Authentication. + * + * Tests the authentication subsystem in isolation without a running + * ProxySQL instance. Covers: + * - Core CRUD: add, lookup, del, exists + * - Credential management: SHA1, clear-text passwords + * - Connection counting and max_connections enforcement + * - Bulk operations: set_all_inactive, remove_inactives, reset + * - Runtime checksums + * - Memory tracking + * - Frontend vs backend separation + * - PgSQL Authentication parity + * + * @see Phase 2.2 of the Unit Testing Framework (GitHub issue #5474) + */ + +#include "tap.h" +#include "test_globals.h" +#include "test_init.h" + +#include "proxysql.h" +#include "MySQL_Authentication.hpp" +#include "PgSQL_Authentication.h" + +#include +#include + +// Extern declarations for Glo* pointers (defined in test_globals.cpp) +extern MySQL_Authentication *GloMyAuth; +extern PgSQL_Authentication *GloPgAuth; + +// ============================================================================ +// Helper: add a MySQL frontend user with common defaults +// ============================================================================ +static bool mysql_add_frontend(MySQL_Authentication *auth, + const char *user, const char *pass, + int default_hg = 0, int max_conn = 100) +{ + return auth->add( + (char *)user, (char *)pass, USERNAME_FRONTEND, + false, // use_ssl + default_hg, // default_hostgroup + (char *)"", // default_schema + false, // schema_locked + false, // transaction_persistent + false, // fast_forward + max_conn, // max_connections + (char *)"", // attributes + (char *)"" // comment + ); +} + +// ============================================================================ +// Helper: add a MySQL backend user +// ============================================================================ +static bool mysql_add_backend(MySQL_Authentication *auth, + const char *user, const char *pass, + int default_hg = 0, int max_conn = 100) +{ + return auth->add( + (char *)user, (char *)pass, USERNAME_BACKEND, + false, default_hg, (char *)"", false, false, false, + max_conn, (char *)"", (char *)"" + ); +} + +// ============================================================================ +// Helper: add a PgSQL frontend user +// ============================================================================ +static bool pgsql_add_frontend(PgSQL_Authentication *auth, + const char *user, const char *pass, + int default_hg = 0, int max_conn = 100) +{ + return auth->add( + (char *)user, (char *)pass, USERNAME_FRONTEND, + false, // use_ssl + default_hg, // default_hostgroup + false, // transaction_persistent + false, // fast_forward + max_conn, // max_connections + (char *)"", // attributes + (char *)"" // comment + ); +} + +// ============================================================================ +// 1. MySQL_Authentication: Core CRUD +// ============================================================================ + +/** + * @brief Test basic add + exists + lookup cycle. + */ +static void test_mysql_add_exists_lookup() { + mysql_add_frontend(GloMyAuth, "alice", "pass123", 1, 50); + + ok(GloMyAuth->exists((char *)"alice") == true, + "MySQL: exists() returns true for added frontend user"); + + ok(GloMyAuth->exists((char *)"unknown") == false, + "MySQL: exists() returns false for nonexistent user"); + + // Lookup with dup options + dup_account_details_t dup = {true, true, true}; + account_details_t ad = GloMyAuth->lookup( + (char *)"alice", USERNAME_FRONTEND, dup); + + ok(ad.password != nullptr && strcmp(ad.password, "pass123") == 0, + "MySQL: lookup() returns correct password"); + ok(ad.default_hostgroup == 1, + "MySQL: lookup() returns correct default_hostgroup"); + ok(ad.max_connections == 50, + "MySQL: lookup() returns correct max_connections"); + + free_account_details(ad); +} + +/** + * @brief Test that exists() only checks frontends, not backends. + */ +static void test_mysql_exists_frontend_only() { + mysql_add_backend(GloMyAuth, "backend_only", "secret"); + + ok(GloMyAuth->exists((char *)"backend_only") == false, + "MySQL: exists() returns false for backend-only user"); + + // But lookup with USERNAME_BACKEND should find it + dup_account_details_t dup = {false, false, false}; + account_details_t ad = GloMyAuth->lookup( + (char *)"backend_only", USERNAME_BACKEND, dup); + + ok(ad.password != nullptr && strcmp(ad.password, "secret") == 0, + "MySQL: lookup(BACKEND) finds backend user"); + + free_account_details(ad); +} + +/** + * @brief Test that add() overwrites on duplicate username. + */ +static void test_mysql_add_overwrites() { + mysql_add_frontend(GloMyAuth, "bob", "old_pass", 1, 10); + mysql_add_frontend(GloMyAuth, "bob", "new_pass", 2, 20); + + dup_account_details_t dup = {true, false, false}; + account_details_t ad = GloMyAuth->lookup( + (char *)"bob", USERNAME_FRONTEND, dup); + + ok(strcmp(ad.password, "new_pass") == 0, + "MySQL: add() overwrites password on duplicate"); + ok(ad.default_hostgroup == 2, + "MySQL: add() overwrites default_hostgroup on duplicate"); + ok(ad.max_connections == 20, + "MySQL: add() overwrites max_connections on duplicate"); + + free_account_details(ad); +} + +/** + * @brief Test del() removes a user. + */ +static void test_mysql_del() { + mysql_add_frontend(GloMyAuth, "charlie", "pass"); + + ok(GloMyAuth->exists((char *)"charlie") == true, + "MySQL: user exists before del()"); + + bool deleted = GloMyAuth->del((char *)"charlie", USERNAME_FRONTEND); + ok(deleted == true, "MySQL: del() returns true for existing user"); + + ok(GloMyAuth->exists((char *)"charlie") == false, + "MySQL: user gone after del()"); + + bool deleted_again = GloMyAuth->del((char *)"charlie", USERNAME_FRONTEND); + ok(deleted_again == false, + "MySQL: del() returns false for already-deleted user"); +} + +/** + * @brief Test lookup() returns empty struct for nonexistent user. + */ +static void test_mysql_lookup_not_found() { + dup_account_details_t dup = {true, true, true}; + account_details_t ad = GloMyAuth->lookup( + (char *)"nonexistent", USERNAME_FRONTEND, dup); + + ok(ad.password == nullptr, + "MySQL: lookup() returns null password for nonexistent user"); + ok(ad.username == nullptr, + "MySQL: lookup() returns null username for nonexistent user"); +} + +// ============================================================================ +// 2. MySQL_Authentication: Credential Management +// ============================================================================ + +/** + * @brief Test set_SHA1() stores and retrieves SHA1 hash. + */ +static void test_mysql_sha1() { + mysql_add_frontend(GloMyAuth, "sha1user", "pass"); + + unsigned char sha1_hash[SHA_DIGEST_LENGTH]; + SHA1((unsigned char *)"pass", 4, sha1_hash); + + bool set_ok = GloMyAuth->set_SHA1( + (char *)"sha1user", USERNAME_FRONTEND, sha1_hash); + ok(set_ok == true, "MySQL: set_SHA1() returns true"); + + // Lookup with sha1 duplication + dup_account_details_t dup = {false, true, false}; + account_details_t ad = GloMyAuth->lookup( + (char *)"sha1user", USERNAME_FRONTEND, dup); + + ok(ad.sha1_pass != nullptr, "MySQL: SHA1 pass retrieved via lookup()"); + if (ad.sha1_pass != nullptr) { + ok(memcmp(ad.sha1_pass, sha1_hash, SHA_DIGEST_LENGTH) == 0, + "MySQL: SHA1 hash matches what was set"); + } else { + ok(0, "MySQL: SHA1 hash matches (skipped - null)"); + } + + // set_SHA1 on nonexistent user + bool set_fail = GloMyAuth->set_SHA1( + (char *)"nobody", USERNAME_FRONTEND, sha1_hash); + ok(set_fail == false, + "MySQL: set_SHA1() returns false for nonexistent user"); + + free_account_details(ad); +} + +/** + * @brief Test set_clear_text_password() for PRIMARY and ADDITIONAL. + */ +static void test_mysql_clear_text_password() { + mysql_add_frontend(GloMyAuth, "ctpuser", "original"); + + bool set_ok = GloMyAuth->set_clear_text_password( + (char *)"ctpuser", USERNAME_FRONTEND, + "clearpass", PASSWORD_TYPE::PRIMARY); + ok(set_ok == true, + "MySQL: set_clear_text_password(PRIMARY) returns true"); + + set_ok = GloMyAuth->set_clear_text_password( + (char *)"ctpuser", USERNAME_FRONTEND, + "altpass", PASSWORD_TYPE::ADDITIONAL); + ok(set_ok == true, + "MySQL: set_clear_text_password(ADDITIONAL) returns true"); + + bool set_fail = GloMyAuth->set_clear_text_password( + (char *)"nobody", USERNAME_FRONTEND, + "pass", PASSWORD_TYPE::PRIMARY); + ok(set_fail == false, + "MySQL: set_clear_text_password() returns false for nonexistent user"); +} + +// ============================================================================ +// 3. MySQL_Authentication: Connection Counting +// ============================================================================ + +/** + * @brief Test increase/decrease frontend user connections. + */ +static void test_mysql_connection_counting() { + mysql_add_frontend(GloMyAuth, "connuser", "pass", 0, 3); + + // Increase connections until limit + int remaining; + remaining = GloMyAuth->increase_frontend_user_connections( + (char *)"connuser", PASSWORD_TYPE::PRIMARY); + ok(remaining > 0, "MySQL: 1st connection: remaining > 0"); + + remaining = GloMyAuth->increase_frontend_user_connections( + (char *)"connuser", PASSWORD_TYPE::PRIMARY); + ok(remaining > 0, "MySQL: 2nd connection: remaining > 0"); + + remaining = GloMyAuth->increase_frontend_user_connections( + (char *)"connuser", PASSWORD_TYPE::PRIMARY); + ok(remaining > 0, "MySQL: 3rd connection: remaining > 0"); + + // At limit now — next should return 0 + remaining = GloMyAuth->increase_frontend_user_connections( + (char *)"connuser", PASSWORD_TYPE::PRIMARY); + ok(remaining == 0, "MySQL: 4th connection rejected (max_connections=3)"); + + // Decrease and verify we can connect again + GloMyAuth->decrease_frontend_user_connections( + (char *)"connuser", PASSWORD_TYPE::PRIMARY); + + remaining = GloMyAuth->increase_frontend_user_connections( + (char *)"connuser", PASSWORD_TYPE::PRIMARY); + ok(remaining > 0, + "MySQL: connection allowed after decrease"); +} + +// ============================================================================ +// 4. MySQL_Authentication: Bulk Operations +// ============================================================================ + +/** + * @brief Test set_all_inactive + remove_inactives pattern. + */ +static void test_mysql_inactive_pattern() { + // Start fresh + GloMyAuth->reset(); + + mysql_add_frontend(GloMyAuth, "keep_me", "pass1"); + mysql_add_frontend(GloMyAuth, "remove_me", "pass2"); + + // Mark all inactive + GloMyAuth->set_all_inactive(USERNAME_FRONTEND); + + // Re-add the one we want to keep (sets __active = true) + mysql_add_frontend(GloMyAuth, "keep_me", "pass1"); + + // Remove inactive users + GloMyAuth->remove_inactives(USERNAME_FRONTEND); + + ok(GloMyAuth->exists((char *)"keep_me") == true, + "MySQL: re-added user survives remove_inactives()"); + ok(GloMyAuth->exists((char *)"remove_me") == false, + "MySQL: inactive user removed by remove_inactives()"); +} + +/** + * @brief Test reset() clears all users. + */ +static void test_mysql_reset() { + mysql_add_frontend(GloMyAuth, "user1", "p1"); + mysql_add_frontend(GloMyAuth, "user2", "p2"); + mysql_add_backend(GloMyAuth, "user3", "p3"); + + GloMyAuth->reset(); + + ok(GloMyAuth->exists((char *)"user1") == false, + "MySQL: frontend user gone after reset()"); + + dup_account_details_t dup = {false, false, false}; + account_details_t ad = GloMyAuth->lookup( + (char *)"user3", USERNAME_BACKEND, dup); + ok(ad.password == nullptr, + "MySQL: backend user gone after reset()"); +} + +// ============================================================================ +// 5. MySQL_Authentication: Checksums +// ============================================================================ + +/** + * @brief Test runtime checksum behavior. + */ +static void test_mysql_checksums() { + GloMyAuth->reset(); + + uint64_t empty_checksum = GloMyAuth->get_runtime_checksum(); + ok(empty_checksum == 0, + "MySQL: checksum is 0 with no users"); + + mysql_add_frontend(GloMyAuth, "checksumA", "passA", 1); + uint64_t checksum1 = GloMyAuth->get_runtime_checksum(); + ok(checksum1 != 0, + "MySQL: checksum is non-zero with users"); + + mysql_add_frontend(GloMyAuth, "checksumB", "passB", 2); + uint64_t checksum2 = GloMyAuth->get_runtime_checksum(); + ok(checksum2 != checksum1, + "MySQL: checksum changes when users are added"); + + // Modify a user and verify checksum changes + mysql_add_frontend(GloMyAuth, "checksumA", "passA_changed", 1); + uint64_t checksum3 = GloMyAuth->get_runtime_checksum(); + ok(checksum3 != checksum2, + "MySQL: checksum changes when password is modified"); +} + +// ============================================================================ +// 6. MySQL_Authentication: Memory Usage +// ============================================================================ + +/** + * @brief Test memory_usage() tracking. + */ +static void test_mysql_memory() { + GloMyAuth->reset(); + + unsigned int mem_empty = GloMyAuth->memory_usage(); + + mysql_add_frontend(GloMyAuth, "memuser1", "password1"); + unsigned int mem_one = GloMyAuth->memory_usage(); + ok(mem_one > mem_empty, + "MySQL: memory_usage() increases after add()"); + + mysql_add_frontend(GloMyAuth, "memuser2", "password2_longer"); + unsigned int mem_two = GloMyAuth->memory_usage(); + ok(mem_two > mem_one, + "MySQL: memory_usage() increases with more users"); + + GloMyAuth->reset(); + unsigned int mem_after_reset = GloMyAuth->memory_usage(); + ok(mem_after_reset <= mem_empty, + "MySQL: memory_usage() returns to baseline after reset()"); +} + +// ============================================================================ +// 7. MySQL_Authentication: Frontend vs Backend Separation +// ============================================================================ + +/** + * @brief Test that frontend and backend are independent. + */ +static void test_mysql_frontend_backend_separation() { + GloMyAuth->reset(); + + // Same username in both frontend and backend with different passwords + mysql_add_frontend(GloMyAuth, "dualuser", "frontend_pass", 1); + mysql_add_backend(GloMyAuth, "dualuser", "backend_pass", 2); + + dup_account_details_t dup = {false, false, false}; + + account_details_t fe = GloMyAuth->lookup( + (char *)"dualuser", USERNAME_FRONTEND, dup); + ok(strcmp(fe.password, "frontend_pass") == 0, + "MySQL: frontend lookup returns frontend password"); + ok(fe.default_hostgroup == 1, + "MySQL: frontend lookup returns frontend hostgroup"); + + account_details_t be = GloMyAuth->lookup( + (char *)"dualuser", USERNAME_BACKEND, dup); + ok(strcmp(be.password, "backend_pass") == 0, + "MySQL: backend lookup returns backend password"); + ok(be.default_hostgroup == 2, + "MySQL: backend lookup returns backend hostgroup"); + + free_account_details(fe); + free_account_details(be); +} + +// ============================================================================ +// 8. PgSQL_Authentication: Core CRUD +// ============================================================================ + +/** + * @brief Test PgSQL basic add + exists + lookup cycle. + */ +static void test_pgsql_add_exists_lookup() { + pgsql_add_frontend(GloPgAuth, "pg_alice", "pgpass", 1, 50); + + ok(GloPgAuth->exists((char *)"pg_alice") == true, + "PgSQL: exists() returns true for added frontend user"); + ok(GloPgAuth->exists((char *)"pg_unknown") == false, + "PgSQL: exists() returns false for nonexistent user"); + + // PgSQL lookup has different signature — returns password string + bool use_ssl = false; + int default_hg = -1, max_conn = -1; + bool trans_persist = false, fast_fwd = false; + void *sha1 = nullptr; + char *attrs = nullptr; + + char *password = GloPgAuth->lookup( + (char *)"pg_alice", USERNAME_FRONTEND, + &use_ssl, &default_hg, &trans_persist, &fast_fwd, + &max_conn, &sha1, &attrs); + + ok(password != nullptr && strcmp(password, "pgpass") == 0, + "PgSQL: lookup() returns correct password"); + ok(default_hg == 1, + "PgSQL: lookup() returns correct default_hostgroup"); + ok(max_conn == 50, + "PgSQL: lookup() returns correct max_connections"); + + if (password) free(password); + if (attrs) free(attrs); +} + +/** + * @brief Test PgSQL del() and reset(). + */ +static void test_pgsql_del_and_reset() { + pgsql_add_frontend(GloPgAuth, "pg_del", "pass"); + ok(GloPgAuth->exists((char *)"pg_del") == true, + "PgSQL: user exists before del()"); + + bool del_ok = GloPgAuth->del((char *)"pg_del", USERNAME_FRONTEND); + ok(del_ok == true, "PgSQL: del() returns true"); + ok(GloPgAuth->exists((char *)"pg_del") == false, + "PgSQL: user gone after del()"); + + // Test reset + pgsql_add_frontend(GloPgAuth, "pg_r1", "p1"); + pgsql_add_frontend(GloPgAuth, "pg_r2", "p2"); + GloPgAuth->reset(); + ok(GloPgAuth->exists((char *)"pg_r1") == false, + "PgSQL: user gone after reset()"); +} + +/** + * @brief Test PgSQL connection counting. + */ +static void test_pgsql_connection_counting() { + GloPgAuth->reset(); + pgsql_add_frontend(GloPgAuth, "pg_conn", "pass", 0, 2); + + int r1 = GloPgAuth->increase_frontend_user_connections( + (char *)"pg_conn"); + ok(r1 > 0, "PgSQL: 1st connection allowed"); + + int r2 = GloPgAuth->increase_frontend_user_connections( + (char *)"pg_conn"); + ok(r2 > 0, "PgSQL: 2nd connection allowed"); + + int r3 = GloPgAuth->increase_frontend_user_connections( + (char *)"pg_conn"); + ok(r3 == 0, "PgSQL: 3rd connection rejected (max=2)"); + + GloPgAuth->decrease_frontend_user_connections((char *)"pg_conn"); + int r4 = GloPgAuth->increase_frontend_user_connections( + (char *)"pg_conn"); + ok(r4 > 0, "PgSQL: connection allowed after decrease"); +} + +/** + * @brief Test PgSQL inactive pattern. + */ +static void test_pgsql_inactive_pattern() { + GloPgAuth->reset(); + pgsql_add_frontend(GloPgAuth, "pg_keep", "p1"); + pgsql_add_frontend(GloPgAuth, "pg_drop", "p2"); + + GloPgAuth->set_all_inactive(USERNAME_FRONTEND); + pgsql_add_frontend(GloPgAuth, "pg_keep", "p1"); + GloPgAuth->remove_inactives(USERNAME_FRONTEND); + + ok(GloPgAuth->exists((char *)"pg_keep") == true, + "PgSQL: re-added user survives remove_inactives()"); + ok(GloPgAuth->exists((char *)"pg_drop") == false, + "PgSQL: inactive user removed"); +} + + +// ============================================================================ +// Main +// ============================================================================ + +int main() { + plan(58); + + test_init_minimal(); + test_init_auth(); + + // MySQL tests + test_mysql_add_exists_lookup(); // 5 tests + test_mysql_exists_frontend_only(); // 2 tests + test_mysql_add_overwrites(); // 3 tests + test_mysql_del(); // 4 tests + test_mysql_lookup_not_found(); // 2 tests + test_mysql_sha1(); // 4 tests + test_mysql_clear_text_password(); // 3 tests + test_mysql_connection_counting(); // 5 tests + test_mysql_inactive_pattern(); // 2 tests + test_mysql_reset(); // 2 tests + test_mysql_checksums(); // 4 tests + test_mysql_memory(); // 3 tests + test_mysql_frontend_backend_separation();// 4 tests + + // PgSQL tests + test_pgsql_add_exists_lookup(); // 5 tests + test_pgsql_del_and_reset(); // 4 tests (49-52) + test_pgsql_connection_counting(); // 4 tests (53-56) + test_pgsql_inactive_pattern(); // 2 tests (57-58) + // Note: PgSQL does not have set_clear_text_password() + // Note: PgSQL does not have default_schema or schema_locked + + test_cleanup_auth(); + test_cleanup_minimal(); + + return exit_status(); +} From a968b049a230540ff51dc2b18ceb1101eb84b52e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Sat, 21 Mar 2026 20:50:42 +0100 Subject: [PATCH 2/3] Add auth_unit-t to unit test Makefile Registers the Authentication unit test binary in the build system so it is built alongside smoke_test-t via 'make build_tap_tests'. --- test/tap/tests/unit/Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/tap/tests/unit/Makefile b/test/tap/tests/unit/Makefile index c05bb5c778..d0736cfe56 100644 --- a/test/tap/tests/unit/Makefile +++ b/test/tap/tests/unit/Makefile @@ -231,7 +231,7 @@ $(ODIR)/test_init.o: $(TEST_HELPERS_DIR)/test_init.cpp | $(ODIR) # Unit test targets # =========================================================================== -UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t protocol_unit-t +UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t protocol_unit-t auth_unit-t .PHONY: all all: $(UNIT_TESTS) @@ -265,6 +265,11 @@ protocol_unit-t: protocol_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ $(ALLOW_MULTI_DEF) -o $@ +auth_unit-t: auth_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) + $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ + $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ + $(ALLOW_MULTI_DEF) -o $@ + # =========================================================================== # Clean From 051435f810ed227cdbabe50bd0beaccd7d0f26c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Sat, 21 Mar 2026 21:07:44 +0100 Subject: [PATCH 3/3] Address review feedback on auth unit tests (PR #5485) - Add null guards before strcmp() in test_mysql_add_overwrites and test_mysql_frontend_backend_separation to prevent segfaults on unexpected lookup failures - Fix misleading "skipped - null" message in SHA1 test fallback to clearly indicate an unexpected null - Free sha1 pointer in PgSQL lookup test to prevent potential memory leak if sha1_pass is ever set - Enhance test_mysql_clear_text_password to verify stored PRIMARY and ADDITIONAL passwords are actually retrievable via lookup(), not just that set_clear_text_password() returns true (2 new test cases, plan updated from 58 to 60) --- test/tap/tests/unit/auth_unit-t.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/test/tap/tests/unit/auth_unit-t.cpp b/test/tap/tests/unit/auth_unit-t.cpp index 839e81c5f8..89ceef1ee3 100644 --- a/test/tap/tests/unit/auth_unit-t.cpp +++ b/test/tap/tests/unit/auth_unit-t.cpp @@ -147,7 +147,7 @@ static void test_mysql_add_overwrites() { account_details_t ad = GloMyAuth->lookup( (char *)"bob", USERNAME_FRONTEND, dup); - ok(strcmp(ad.password, "new_pass") == 0, + ok(ad.password != nullptr && strcmp(ad.password, "new_pass") == 0, "MySQL: add() overwrites password on duplicate"); ok(ad.default_hostgroup == 2, "MySQL: add() overwrites default_hostgroup on duplicate"); @@ -218,7 +218,7 @@ static void test_mysql_sha1() { ok(memcmp(ad.sha1_pass, sha1_hash, SHA_DIGEST_LENGTH) == 0, "MySQL: SHA1 hash matches what was set"); } else { - ok(0, "MySQL: SHA1 hash matches (skipped - null)"); + ok(0, "MySQL: SHA1 hash was unexpectedly null"); } // set_SHA1 on nonexistent user @@ -231,7 +231,8 @@ static void test_mysql_sha1() { } /** - * @brief Test set_clear_text_password() for PRIMARY and ADDITIONAL. + * @brief Test set_clear_text_password() for PRIMARY and ADDITIONAL, + * and verify stored values are retrievable via lookup(). */ static void test_mysql_clear_text_password() { mysql_add_frontend(GloMyAuth, "ctpuser", "original"); @@ -248,6 +249,18 @@ static void test_mysql_clear_text_password() { ok(set_ok == true, "MySQL: set_clear_text_password(ADDITIONAL) returns true"); + // Verify stored clear-text passwords are retrievable + dup_account_details_t dup = {false, false, false}; + account_details_t ad = GloMyAuth->lookup( + (char *)"ctpuser", USERNAME_FRONTEND, dup); + ok(ad.clear_text_password[PASSWORD_TYPE::PRIMARY] != nullptr + && strcmp(ad.clear_text_password[PASSWORD_TYPE::PRIMARY], "clearpass") == 0, + "MySQL: PRIMARY clear_text_password retrievable via lookup()"); + ok(ad.clear_text_password[PASSWORD_TYPE::ADDITIONAL] != nullptr + && strcmp(ad.clear_text_password[PASSWORD_TYPE::ADDITIONAL], "altpass") == 0, + "MySQL: ADDITIONAL clear_text_password retrievable via lookup()"); + free_account_details(ad); + bool set_fail = GloMyAuth->set_clear_text_password( (char *)"nobody", USERNAME_FRONTEND, "pass", PASSWORD_TYPE::PRIMARY); @@ -420,14 +433,14 @@ static void test_mysql_frontend_backend_separation() { account_details_t fe = GloMyAuth->lookup( (char *)"dualuser", USERNAME_FRONTEND, dup); - ok(strcmp(fe.password, "frontend_pass") == 0, + ok(fe.password != nullptr && strcmp(fe.password, "frontend_pass") == 0, "MySQL: frontend lookup returns frontend password"); ok(fe.default_hostgroup == 1, "MySQL: frontend lookup returns frontend hostgroup"); account_details_t be = GloMyAuth->lookup( (char *)"dualuser", USERNAME_BACKEND, dup); - ok(strcmp(be.password, "backend_pass") == 0, + ok(be.password != nullptr && strcmp(be.password, "backend_pass") == 0, "MySQL: backend lookup returns backend password"); ok(be.default_hostgroup == 2, "MySQL: backend lookup returns backend hostgroup"); @@ -472,6 +485,7 @@ static void test_pgsql_add_exists_lookup() { if (password) free(password); if (attrs) free(attrs); + if (sha1) free(sha1); } /** @@ -544,7 +558,7 @@ static void test_pgsql_inactive_pattern() { // ============================================================================ int main() { - plan(58); + plan(60); test_init_minimal(); test_init_auth();