KIKIMR-21024: Replace required fields in auth.proto with optional

molotkov-and · Feb 13, 2024 · 930f62c · 930f62c
1 parent 57cf0e9
commit 930f62c
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 17 deletions.
diff --git a/ydb/core/protos/auth.proto b/ydb/core/protos/auth.proto
@@ -100,9 +100,9 @@ message TLdapAuthentication {
 
     optional string Host = 1;
     optional uint32 Port = 2;
-    required string BaseDn = 3;
-    required string BindDn = 4;
-    required string BindPassword = 5;
+    optional string BaseDn = 3;
+    optional string BindDn = 4;
+    optional string BindPassword = 5;
     optional string SearchFilter = 6;
     optional string SearchAttribute = 7;
     optional TUseTls UseTls = 8;

diff --git a/ydb/core/security/ldap_auth_provider.cpp b/ydb/core/security/ldap_auth_provider.cpp
@@ -186,13 +186,12 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped<TLdapAuthProvider>
     }
 
     TInitializeLdapConnectionResponse InitializeLDAPConnection(LDAP** ld) {
-        const TString& host = Settings.GetHost();
-        if (host.empty()) {
-            return {{TEvLdapAuthProvider::EStatus::UNAVAILABLE, {.Message = "Ldap server host is empty", .Retryable = false}}};
+        if (TInitializeLdapConnectionResponse response = CheckRequiredSettingsParameters(); response.Status != TEvLdapAuthProvider::EStatus::SUCCESS) {
+            return response;
         }
 
+        const TString& host = Settings.GetHost();
         const ui32 port = Settings.GetPort() != 0 ? Settings.GetPort() : NKikimrLdap::GetPort();
-
         int result = 0;
         if (Settings.GetUseTls().GetEnable()) {
             const TString& caCertificateFile = Settings.GetUseTls().GetCaCertFile();
@@ -290,6 +289,22 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped<TLdapAuthProvider>
         return response;
     }
 
+    TInitializeLdapConnectionResponse CheckRequiredSettingsParameters() const {
+        if (Settings.GetHost().empty()) {
+            return {TEvLdapAuthProvider::EStatus::UNAVAILABLE, {.Message = "Ldap server host is empty", .Retryable = false}};
+        }
+        if (Settings.GetBaseDn().empty()) {
+            return {TEvLdapAuthProvider::EStatus::UNAVAILABLE, {.Message = "Parameter BaseDn is empty", .Retryable = false}};
+        }
+        if (Settings.GetBindDn().empty()) {
+            return {TEvLdapAuthProvider::EStatus::UNAVAILABLE, {.Message = "Parameter BindDn is empty", .Retryable = false}};
+        }
+        if (Settings.GetBindPassword().empty()) {
+            return {TEvLdapAuthProvider::EStatus::UNAVAILABLE, {.Message = "Parameter BindPassword is empty", .Retryable = false}};
+        }
+        return {TEvLdapAuthProvider::EStatus::SUCCESS, {}};
+    }
+
 private:
     const NKikimrProto::TLdapAuthentication Settings;
     const TSearchFilterCreator FilterCreator;

diff --git a/ydb/core/security/ticket_parser_ut.cpp b/ydb/core/security/ticket_parser_ut.cpp
@@ -77,6 +77,26 @@ void InitLdapSettingsWithUnavailableHost(NKikimrProto::TLdapAuthentication* ldap
     ldapSettings->SetHost("unavailablehost");
 }
 
+void InitLdapSettingsWithEmptyHost(NKikimrProto::TLdapAuthentication* ldapSettings, ui16 ldapPort, TTempFileHandle& certificateFile) {
+    InitLdapSettings(ldapSettings, ldapPort, certificateFile);
+    ldapSettings->SetHost("");
+}
+
+void InitLdapSettingsWithEmptyBaseDn(NKikimrProto::TLdapAuthentication* ldapSettings, ui16 ldapPort, TTempFileHandle& certificateFile) {
+    InitLdapSettings(ldapSettings, ldapPort, certificateFile);
+    ldapSettings->SetBaseDn("");
+}
+
+void InitLdapSettingsWithEmptyBindDn(NKikimrProto::TLdapAuthentication* ldapSettings, ui16 ldapPort, TTempFileHandle& certificateFile) {
+    InitLdapSettings(ldapSettings, ldapPort, certificateFile);
+    ldapSettings->SetBindDn("");
+}
+
+void InitLdapSettingsWithEmptyBindPassword(NKikimrProto::TLdapAuthentication* ldapSettings, ui16 ldapPort, TTempFileHandle& certificateFile) {
+    InitLdapSettings(ldapSettings, ldapPort, certificateFile);
+    ldapSettings->SetBindPassword("");
+}
+
 void InitLdapSettingsWithCustomGroupAttribute(NKikimrProto::TLdapAuthentication* ldapSettings, ui16 ldapPort, TTempFileHandle& certificateFile) {
     InitLdapSettings(ldapSettings, ldapPort, certificateFile);
     ldapSettings->SetRequestedGroupAttribute("groupDN");
@@ -190,6 +210,24 @@ LdapMock::TLdapMockResponses TCorrectLdapResponse::GetResponses(const TString& l
     responses.SearchResponses.push_back({fetchGroupsSearchRequestInfo, fetchGroupsSearchResponseInfo});
     return responses;
 }
+
+void CheckRequiredLdapSettings(std::function<void(NKikimrProto::TLdapAuthentication*, ui16, TTempFileHandle&)> initLdapSettings, const TString& expectedErrorMessage) {
+    TLdapKikimrServer server(initLdapSettings);
+
+    LdapMock::TLdapMockResponses responses;
+    LdapMock::TLdapSimpleServer ldapServer(server.GetLdapPort(), responses);
+
+    TString login = "ldapuser";
+    TString password = "ldapUserPassword";
+
+    TAutoPtr<IEventHandle> handle = LdapAuthenticate(server, login, password);
+    TEvTicketParser::TEvAuthorizeTicketResult* ticketParserResult = handle->Get<TEvTicketParser::TEvAuthorizeTicketResult>();
+    UNIT_ASSERT_C(!ticketParserResult->Error.empty(), "Expected return error message");
+    UNIT_ASSERT_STRINGS_EQUAL(ticketParserResult->Error.Message, expectedErrorMessage);
+
+    ldapServer.Stop();
+}
+
 } // namespace
 
 Y_UNIT_TEST_SUITE(TTicketParserTest) {
@@ -711,20 +749,23 @@ Y_UNIT_TEST_SUITE(TTicketParserTest) {
     }
 
     Y_UNIT_TEST(LdapServerIsUnavailable) {
-        TLdapKikimrServer server(InitLdapSettingsWithUnavailableHost);
+        CheckRequiredLdapSettings(InitLdapSettingsWithUnavailableHost, "Could not start TLS\nCan't contact LDAP server");
+    }
 
-        LdapMock::TLdapMockResponses responses;
-        LdapMock::TLdapSimpleServer ldapServer(server.GetLdapPort(), responses);
+    Y_UNIT_TEST(LdapRequestWithEmptyHost) {
+        CheckRequiredLdapSettings(InitLdapSettingsWithEmptyHost, "Ldap server host is empty");
+    }
 
-        TString login = "ldapuser";
-        TString password = "ldapUserPassword";
+    Y_UNIT_TEST(LdapRequestWithEmptyBaseDn) {
+        CheckRequiredLdapSettings(InitLdapSettingsWithEmptyBaseDn, "Parameter BaseDn is empty");
+    }
 
-        TAutoPtr<IEventHandle> handle = LdapAuthenticate(server, login, password);
-        TEvTicketParser::TEvAuthorizeTicketResult* ticketParserResult = handle->Get<TEvTicketParser::TEvAuthorizeTicketResult>();
-        UNIT_ASSERT_C(!ticketParserResult->Error.empty(), "Expected return error message");
-        UNIT_ASSERT_STRINGS_EQUAL(ticketParserResult->Error.Message, "Could not start TLS\nCan't contact LDAP server");
+    Y_UNIT_TEST(LdapRequestWithEmptyBindDn) {
+        CheckRequiredLdapSettings(InitLdapSettingsWithEmptyBindDn, "Parameter BindDn is empty");
+    }
 
-        ldapServer.Stop();
+    Y_UNIT_TEST(LdapRequestWithEmptyBindPassword) {
+        CheckRequiredLdapSettings(InitLdapSettingsWithEmptyBindPassword, "Parameter BindPassword is empty");
     }
 
     Y_UNIT_TEST(LdapRefreshGroupsInfoGood) {