ipaserver.install.adtrust: fix CID 323644

Fix Coverity finding CID 323644: logically dead code path

The code to determine whether NetBIOS name was already set or need to be
set after deriving it from a domain or asking a user for an interactive
input, was refactored at some point to avoid retrieving the whole LDAP
entry. Instead, it was provided with the actual NetBIOS name retrieved.

As result, a part of the code got neglected and was never executed.

Fix this code and provide a test that tries to test predefined,
interactively provided and automatically derived NetBIOS name depending
on how the installer is being run.

We mock up the actual execution so that no access to LDAP or Samba is
needed.

Fixes: https://pagure.io/freeipa/issue/7753
Reviewed-By: Christian Heimes <cheimes@redhat.com>

2 files changed, 55 insertions, 2 deletions

diff --git a/ipaserver/install/adtrust.py b/ipaserver/install/adtrust.py
index e9ae3fa3e..75194eed8 100644
--- a/ipaserver/install/adtrust.py
+++ b/ipaserver/install/adtrust.py
@@ -95,7 +95,6 @@ def set_and_check_netbios_name(netbios_name, unattended, api):
     cur_netbios_name = None
     gen_netbios_name = None
     reset_netbios_name = False
-    entry = None
 
     if api.Backend.ldap2.isconnected():
         cur_netbios_name = retrieve_netbios_name(api)
@@ -133,7 +132,7 @@ def set_and_check_netbios_name(netbios_name, unattended, api):
             gen_netbios_name = adtrustinstance.make_netbios_name(
                 api.env.domain)
 
-        if entry is not None:
+        if gen_netbios_name is not None:
             # Fix existing trust configuration
             print("Trust is configured but no NetBIOS domain name found, "
                   "setting it now.")
diff --git a/ipatests/test_ipaserver/test_adtrust_mockup.py b/ipatests/test_ipaserver/test_adtrust_mockup.py
new file mode 100644
index 000000000..a8248e0f8
--- /dev/null
+++ b/ipatests/test_ipaserver/test_adtrust_mockup.py
@@ -0,0 +1,54 @@
+# Copyright (C) 2018  FreeIPA Project Contributors - see LICENSE file
+
+from __future__ import print_function
+import ipaserver.install.adtrust as adtr
+from ipaserver.install.adtrust import set_and_check_netbios_name
+from collections import namedtuple
+from unittest import TestCase, mock
+from io import StringIO
+
+
+class ApiMockup:
+    Backend = namedtuple('Backend', 'ldap2')
+    Calls = namedtuple('Callbacks', 'retrieve_netbios_name')
+    env = namedtuple('Environment', 'domain')
+
+
+class TestNetbiosName(TestCase):
+    @classmethod
+    def setUpClass(cls):
+        api = ApiMockup()
+        ldap2 = namedtuple('LDAP', 'isconnected')
+        ldap2.isconnected = mock.MagicMock(return_value=True)
+        api.Backend.ldap2 = ldap2
+        api.Calls.retrieve_netbios_name = adtr.retrieve_netbios_name
+        adtr.retrieve_netbios_name = mock.MagicMock(return_value=None)
+        cls.api = api
+
+    @classmethod
+    def tearDownClass(cls):
+        adtr.retrieve_netbios_name = cls.api.Calls.retrieve_netbios_name
+
+    def test_NetbiosName(self):
+        """
+        Test set_and_check_netbios_name() using permutation of two inputs:
+        - predefined and not defined NetBIOS name
+        - unattended and interactive run
+        As result, the function has to return expected NetBIOS name in
+        all cases. For interactive run we override input to force what
+        we expect.
+        """
+        self.api.env.domain = 'example.com'
+        expected_nname = 'EXAMPLE'
+        # NetBIOS name, unattended, should set the name?
+        tests = ((expected_nname, True, False),
+                 (None, True, True),
+                 (None, False, True),
+                 (expected_nname, False, False))
+        with mock.patch('sys.stdin', new_callable=StringIO) as stdin:
+            stdin.write(expected_nname + '\r')
+            for test in tests:
+                nname, setname = set_and_check_netbios_name(
+                    test[0], test[1], self.api)
+                assert expected_nname == nname
+                assert setname == test[2]