From a86d4188a87c06d890dcf9b115e61de5f576cef5 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Tue, 2 Sep 2025 16:38:48 +0200 Subject: [PATCH] [#4070] Addressed comments --- ...ile-option-to-high-availability-parameters | 8 +- doc/sphinx/arm/hooks-ha.rst | 2 +- .../tests/ha_config_unittest.cc | 127 +++++++++++++++++- 3 files changed, 131 insertions(+), 6 deletions(-) diff --git a/changelog_unreleased/4070-add-basic-auth-user-file-option-to-high-availability-parameters b/changelog_unreleased/4070-add-basic-auth-user-file-option-to-high-availability-parameters index 21398549df..736a9c0c4c 100644 --- a/changelog_unreleased/4070-add-basic-auth-user-file-option-to-high-availability-parameters +++ b/changelog_unreleased/4070-add-basic-auth-user-file-option-to-high-availability-parameters @@ -1,6 +1,6 @@ -[func] fdupont +[sec] fdupont Added the "basic-auth-user-file" parameter to the HA - hook library to provide the user ID which is part of - the secret used by the basic HTTP auth from a file - instead in clear in the configuration. + hook library. This allows the basic HTTP auth user + ID to be read from a file rather than specified as + clear text in the configuration. (Gitlab #4070) diff --git a/doc/sphinx/arm/hooks-ha.rst b/doc/sphinx/arm/hooks-ha.rst index bfeda44ba3..e1ec8e5c64 100644 --- a/doc/sphinx/arm/hooks-ha.rst +++ b/doc/sphinx/arm/hooks-ha.rst @@ -915,7 +915,7 @@ list: not specified or specified as an empty string, no authentication header is added to HTTP transactions. It must not contain the colon (:) character. -- ``basic-auth-user-file`` - is an alternatibe to ``basic-auth-user``: +- ``basic-auth-user-file`` - is an alternative to ``basic-auth-user``: instead of presenting the user ID in the configuration file it is specified in the file indicated by this parameter. diff --git a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc index 0ccb6861c4..fb89702075 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc @@ -1368,6 +1368,132 @@ TEST_F(HAConfigTest, invalidUser) { "user 'foo:bar' must not contain a ':' in peer 'server2'"); } +// Test that only one of basic-auth-password and basic-auth-password-file +// is allowed +TEST_F(HAConfigTest, twoBasicAuthPassword) { + std::string expected = "only one of basic-auth-password and "; + expected += "basic-auth-password-file parameter can be "; + expected += "configured in peer 'server2'"; + testInvalidConfig( + "[" + " {" + " \"this-server-name\": \"server1\"," + " \"mode\": \"load-balancing\"," + " \"peers\": [" + " {" + " \"name\": \"server1\"," + " \"url\": \"http://127.0.0.1:8080/\"," + " \"role\": \"primary\"," + " \"auto-failover\": false" + " }," + " {" + " \"name\": \"server2\"," + " \"url\": \"http://127.0.0.1:8080/\"," + " \"basic-auth-user\": \"foobar\"," + " \"basic-auth-password\": \"foobar\"," + " \"basic-auth-password-file\": \"foobar\"," + " \"role\": \"secondary\"," + " \"auto-failover\": true" + " }" + " ]" + " }" + "]", + expected); +} + +// Test that invalid basic-auth-password-file is refused. +TEST_F(HAConfigTest, invalidBasicAuthPasswordFile) { + std::string expected = "bad password file in peer 'server2': "; + expected += "Expected a file at path '/does/not/exist'"; + testInvalidConfig( + "[" + " {" + " \"this-server-name\": \"server1\"," + " \"mode\": \"load-balancing\"," + " \"peers\": [" + " {" + " \"name\": \"server1\"," + " \"url\": \"http://127.0.0.1:8080/\"," + " \"role\": \"primary\"," + " \"auto-failover\": false" + " }," + " {" + " \"name\": \"server2\"," + " \"url\": \"http://127.0.0.1:8080/\"," + " \"basic-auth-user\": \"foobar\"," + " \"basic-auth-password-file\": \"/does/not/exist\"," + " \"role\": \"secondary\"," + " \"auto-failover\": true" + " }" + " ]" + " }" + "]", + expected); +} + +// Test that only one of basic-auth-user and basic-auth-user-file +// is allowed +TEST_F(HAConfigTest, twoBasicAuthUser) { + std::string expected = "only one of basic-auth-user and "; + expected += "basic-auth-user-file parameter can be "; + expected += "configured in peer 'server2'"; + testInvalidConfig( + "[" + " {" + " \"this-server-name\": \"server1\"," + " \"mode\": \"load-balancing\"," + " \"peers\": [" + " {" + " \"name\": \"server1\"," + " \"url\": \"http://127.0.0.1:8080/\"," + " \"role\": \"primary\"," + " \"auto-failover\": false" + " }," + " {" + " \"name\": \"server2\"," + " \"url\": \"http://127.0.0.1:8080/\"," + " \"basic-auth-user\": \"foobar\"," + " \"basic-auth-user-file\": \"foobar\"," + " \"basic-auth-password\": \"foobar\"," + " \"role\": \"secondary\"," + " \"auto-failover\": true" + " }" + " ]" + " }" + "]", + expected); +} + +// Test that invalid basic-auth-user-file is refused. +TEST_F(HAConfigTest, invalidBasicAuthUserFile) { + std::string expected = "bad user file in peer 'server2': "; + expected += "Expected a file at path '/does/not/exist'"; + testInvalidConfig( + "[" + " {" + " \"this-server-name\": \"server1\"," + " \"mode\": \"load-balancing\"," + " \"peers\": [" + " {" + " \"name\": \"server1\"," + " \"url\": \"http://127.0.0.1:8080/\"," + " \"role\": \"primary\"," + " \"auto-failover\": false" + " }," + " {" + " \"name\": \"server2\"," + " \"url\": \"http://127.0.0.1:8080/\"," + " \"basic-auth-user-file\": \"/does/not/exist\"," + " \"basic-auth-password\": \"foobar\"," + " \"role\": \"secondary\"," + " \"auto-failover\": true" + " }" + " ]" + " }" + "]", + expected); +} + // Test that setting delayed-updates-limit is not allowed in hot-standby mode. TEST_F(HAConfigTest, hotStandbyDelayedUpdatesLimit) { testInvalidConfig( @@ -2110,5 +2236,4 @@ TEST_F(HAConfigTest, getSubnetServerNameInvalid) { EXPECT_THROW(HAConfig::getSubnetServerName(subnet6), BadValue); } - } // namespace -- 2.47.3