Merge pull request #88 from linuxmuster/feat/customShareNameTemplate

Feat: allow user to define custom template for share name

Author: dorianim

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/config.py

@@ -19,6 +19,23 @@ def network():
 
     return True, networkConfig
 
+def shares():
+    """
+    Get the shares configuration in `/etc/linuxmuster-linuxclient7/config.yml`
+
+    :return: Tuple (success, dict of keys)
+    :rtype: tuple
+    """
+    config = _readConfig()
+    sharesConfig = {}
+    if config is not None and "shares" in config:
+        sharesConfig = config["shares"]
+
+    if not "letterTemplate" in sharesConfig:
+        sharesConfig["letterTemplate"] = constants.defaultShareLetterTemplate
+
+    return sharesConfig
+
 def writeNetworkConfig(newNetworkConfig):
     """
     Write the network configuration in `/etc/linuxmuster-linuxclient7/config.yml`.
@@ -49,17 +66,19 @@ def upgrade():
     """
     return _upgrade()
 
-def delete():
+def deleteNetworkConfig():
     """
     Delete the network configuration file.
 
     :return: True or False
     :rtype: bool
     """
-    legacyNetworkConfigFleDeleted = fileHelper.deleteFile(constants.legacyNetworkConfigFilePath)
-    configFileDeleted = fileHelper.deleteFile(constants.configFilePath)
-    return legacyNetworkConfigFleDeleted and configFileDeleted
-
+    config = _readConfig()
+    if config is None or "network" not in config:
+        return True
+    
+    del config["network"]
+    return _writeConfig(config)
 
 # --------------------
 # - Helper functions -

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/constants.py

@@ -10,6 +10,7 @@
 shareMountBasepath = "/home/{}/media"
 hiddenShareMountBasepath = "/srv/samba/{}"
 machineAccountSysvolMountPath = "/var/lib/samba/sysvol"
+defaultShareLetterTemplate = " ({letter}:)"
 
 etcBaseDir = "/etc/linuxmuster-linuxclient7"
 shareBaseDir = "/usr/share/linuxmuster-linuxclient7"

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/gpo.py

@@ -239,7 +239,8 @@ def _processDrivesPolicy(policyBasepath):
 
     for drive in shareList:
         if drive["useLetter"] == "1":
-            shareName = f"{drive['label']} ({drive['letter']}:)"  
+            formattedLetter = config.shares()["letterTemplate"].format(letter=drive['letter'])
+            shareName = f"{drive['label']}{formattedLetter}"
         else:
             shareName = drive["label"]
 

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/setup.py

@@ -210,9 +210,9 @@ def _cleanOldDomainJoins():
     if not fileHelper.deleteFilesWithExtension("/var/lib/samba/private/tls", ".pem"):
         return False
 
-    # remove configuration
-    logging.info(f"Deleting configuration files if exist ...")
-    if not config.delete():
+    # remove network configuration
+    logging.info(f"Deleting network configuration ...")
+    if not config.deleteNetworkConfig():
         return False
 
     return True

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/files/config/config.no-shares.yml

@@ -0,0 +1,4 @@
+network:
+  serverHostname: server.linuxmuster.lan
+  domain: linuxmuster.lan
+  realm: LINUXMUSTER.LAN

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/files/config/config.yml

@@ -1,3 +1,6 @@
+shares:
+  letterTemplate: "_{letter}"
+
 network:
   serverHostname: server.linuxmuster.lan
   domain: linuxmuster.lan

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_config.py

@@ -2,7 +2,9 @@
 from unittest import mock
 from .. import config
 
-@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/network.conf")
+MOCK_FILE_PATH = f"{os.path.dirname(os.path.realpath(__file__))}/files/config"
+
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{MOCK_FILE_PATH}/network.conf")
 @mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/does/not/exist/config.yml")
 def test_network_legacy():
     rc, networkConfig = config.network()
@@ -12,16 +14,16 @@ def test_network_legacy():
     assert networkConfig["realm"] == "LINUXMUSTER.LEGACY"
 
 @mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/config.yml")
-@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/config.yml")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{MOCK_FILE_PATH}/config.yml")
 def test_network():
     rc, networkConfig = config.network()
     assert rc
     assert networkConfig["serverHostname"] == "server.linuxmuster.lan"
     assert networkConfig["domain"] == "linuxmuster.lan"
     assert networkConfig["realm"] == "LINUXMUSTER.LAN"
 
-@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/network.conf")
-@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/config.yml")
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{MOCK_FILE_PATH}/network.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{MOCK_FILE_PATH}/config.yml")
 def test_network_both():
     rc, networkConfig = config.network()
     assert rc
@@ -38,27 +40,44 @@ def test_network_none():
     assert networkConfig is None
 
 @mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/network.conf")
-@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/config.invalid-network.yml")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{MOCK_FILE_PATH}/config.invalid-network.yml")
 def test_network_invalid():
     rc, networkConfig = config.network()
     assert not rc
     assert networkConfig is None
 
 @mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/network.conf")
-@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/config.invalid-syntax.yml")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{MOCK_FILE_PATH}/config.yml")
+def test_shares():
+    sharesConfig = config.shares()
+    assert "letterTemplate" in sharesConfig
+    assert sharesConfig["letterTemplate"] == "_{letter}"
+
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/network.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/does/not/exist/config.yml")
+def test_shares_none():
+    sharesConfig = config.shares()
+    assert "letterTemplate" in sharesConfig
+    assert sharesConfig["letterTemplate"] == config.constants.defaultShareLetterTemplate
+
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/network.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{MOCK_FILE_PATH}/config.no-shares.yml")
+def test_shares_missing():
+    sharesConfig = config.shares()
+    assert "letterTemplate" in sharesConfig
+    assert sharesConfig["letterTemplate"] == config.constants.defaultShareLetterTemplate
+
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/network.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{MOCK_FILE_PATH}/config.invalid-syntax.yml")
 def test_syntax_invalid():
     rc, networkConfig = config.network()
     assert not rc
     assert networkConfig is None
 
 @mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/tmp/config.yml")
 def test_writeNetworkConfig():
-    if os.path.exists("/tmp/config.yml"):
-        os.remove("/tmp/config.yml")
-
-    with open(f"{os.path.dirname(os.path.realpath(__file__))}/files/config/config.yml", "r") as fsrc:
-        with open("/tmp/config.yml", "w") as fdst:
-            fdst.write(fsrc.read())
+    _deleteFile("/tmp/config.yml")
+    _copyFile(f"{MOCK_FILE_PATH}/config.yml", "/tmp/config.yml")
 
     newNetworkConfig = {
         "serverHostname": "server.linuxmuster.new",
@@ -72,17 +91,13 @@ def test_writeNetworkConfig():
     assert networkConfig["serverHostname"] == "server.linuxmuster.new"
     assert networkConfig["domain"] == "linuxmuster.new"
     assert networkConfig["realm"] == "LINUXMUSTER.NEW"
+    assert config.shares() == {"letterTemplate": "_{letter}"}
 
-    # TODO: once there are more config options, test that they are preserved
 
 @mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/tmp/config.yml")
 def test_writeNetworkConfig_invalid():
-    if os.path.exists("/tmp/config.yml"):
-        os.remove("/tmp/config.yml")
-
-    with open(f"{os.path.dirname(os.path.realpath(__file__))}/files/config/config.yml", "r") as fsrc:
-        with open("/tmp/config.yml", "w") as fdst:
-            fdst.write(fsrc.read())
+    _deleteFile("/tmp/config.yml")
+    _copyFile(f"{MOCK_FILE_PATH}/config.yml", "/tmp/config.yml")
 
     newNetworkConfig = {
         "sserverHostname": "server.linuxmuster.new",
@@ -109,8 +124,7 @@ def test_writeNetworkConfig_invalidPath():
 
 @mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/tmp/config.yml")
 def test_writeNetworkConfig_empty():
-    if os.path.exists("/tmp/config.yml"):
-        os.remove("/tmp/config.yml")
+    _deleteFile("/tmp/config.yml")
 
     newNetworkConfig = {
         "serverHostname": "server.linuxmuster.new",
@@ -128,15 +142,9 @@ def test_writeNetworkConfig_empty():
 @mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/tmp/network.conf")
 @mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/tmp/config.yml")
 def test_upgrade():
-    if os.path.exists("/tmp/config.yml"):
-        os.remove("/tmp/config.yml")
-
-    if os.path.exists("/tmp/network.conf"):
-        os.remove("/tmp/network.conf")
-
-    with open(f"{os.path.dirname(os.path.realpath(__file__))}/files/config/network.conf", "r") as fsrc:
-        with open("/tmp/network.conf", "w") as fdst:
-            fdst.write(fsrc.read())
+    _deleteFile("/tmp/config.yml")
+    _deleteFile("/tmp/network.conf")
+    _copyFile(f"{MOCK_FILE_PATH}/network.conf", "/tmp/network.conf")
 
     assert config.upgrade()
     assert not os.path.exists("/tmp/network.conf")
@@ -152,8 +160,8 @@ def test_upgrade():
     assert yamlContent["network"]["realm"] == "LINUXMUSTER.LEGACY"
 
 
-@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/network.conf")
-@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/config.yml")
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{MOCK_FILE_PATH}/network.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"{MOCK_FILE_PATH}/config.yml")
 def test_upgrade_alreadyUpToDate():
     assert config.upgrade()
 
@@ -166,36 +174,54 @@ def test_upgrade_alreadyUpToDate():
     assert yamlContent["network"]["domain"] == "linuxmuster.lan"
     assert yamlContent["network"]["realm"] == "LINUXMUSTER.LAN"
 
-@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{os.path.dirname(os.path.realpath(__file__))}/files/config/network.invalid.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{MOCK_FILE_PATH}/network.invalid.conf")
 @mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/tmp/config.yml")
 def test_upgrade_invalid():
-    if os.path.exists("/tmp/config.yml"):
-        os.remove("/tmp/config.yml")
+    _deleteFile("/tmp/config.yml")
+
+    assert not config.upgrade()
+    assert os.path.exists(f"{MOCK_FILE_PATH}/network.invalid.conf")
+    assert not os.path.exists("/tmp/config.yml")
 
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"{MOCK_FILE_PATH}/network.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/does/not/exist/config.yml")
+def test_upgrade_unwritable():
     assert not config.upgrade()
-    assert os.path.exists(f"{os.path.dirname(os.path.realpath(__file__))}/files/config/network.invalid.conf")
+    assert os.path.exists(f"{MOCK_FILE_PATH}/network.invalid.conf")
     assert not os.path.exists("/tmp/config.yml")
 
 @mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/network.conf")
 @mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/does/not/exist/config.yml")
 def test_upgrade_nonexistent():
     assert not config.upgrade()
 
-@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/tmp/network.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/network.conf")
 @mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/tmp/config.yml")
-def test_delete():
-    if os.path.exists("/tmp/network.conf"):
-        os.remove("/tmp/network.conf")
-    if os.path.exists("/tmp/config.yml"):
-        os.remove("/tmp/config.yml")
-
-    with open(f"{os.path.dirname(os.path.realpath(__file__))}/files/config/network.conf", "r") as fsrc:
-        with open("/tmp/network.conf", "w") as fdst:
-            fdst.write(fsrc.read())
-    with open(f"{os.path.dirname(os.path.realpath(__file__))}/files/config/config.yml", "r") as fsrc:
-        with open("/tmp/config.yml", "w") as fdst:
-            fdst.write(fsrc.read())
-
-    assert config.delete()
-    assert not os.path.exists("/tmp/network.conf")
-    assert not os.path.exists("/tmp/config.yml")
+def test_deleteNetworkConfig():
+    _deleteFile("/tmp/network.conf")
+    _deleteFile("/tmp/config.yml")
+
+    _copyFile(f"{MOCK_FILE_PATH}/config.yml", "/tmp/config.yml")
+
+    assert config.deleteNetworkConfig()
+    assert os.path.exists("/tmp/config.yml")
+    assert config.network() == (False, None)
+    assert config.shares() == {"letterTemplate": "_{letter}"}
+
+@mock.patch("linuxmusterLinuxclient7.config.constants.legacyNetworkConfigFilePath", f"/does/not/exist/network.conf")
+@mock.patch("linuxmusterLinuxclient7.config.constants.configFilePath", f"/does/not/exist/config.yml")
+def test_deleteNetworkConfig_nonexistent():
+    assert config.deleteNetworkConfig()
+
+# --------------------
+# - Helper functions -
+# --------------------
+
+def _deleteFile(path):
+    if os.path.exists(path):
+        os.remove(path)
+
+def _copyFile(src, dst):
+    with open(src, "r") as fsrc:
+        with open(dst, "w") as fdst:
+            fdst.write(fsrc.read())

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_gpo.py

@@ -46,6 +46,47 @@ def test_allOkAllTrue(mockUserSchool, mockLdapHelperSearchOne, mockSharesGetMoun
     assert len(mockPrintersInstallPrinter.call_args_list) == 1
     assert mockPrintersInstallPrinter.call_args_list[0] == mock.call('ipp://SERVER/printers/PRINTER1', 'PRINTER1')
 
+@mock.patch("linuxmusterLinuxclient7.gpo.config.shares")
+@mock.patch("linuxmusterLinuxclient7.gpo.computer.isInGroup")
+@mock.patch("linuxmusterLinuxclient7.gpo.user.isInGroup")
+@mock.patch("linuxmusterLinuxclient7.gpo.printers.installPrinter")
+@mock.patch("linuxmusterLinuxclient7.gpo.shares.mountShare")
+@mock.patch("linuxmusterLinuxclient7.gpo.shares.getMountpointOfRemotePath")
+@mock.patch("linuxmusterLinuxclient7.gpo.ldapHelper.searchOne")
+@mock.patch("linuxmusterLinuxclient7.gpo.user.school")
+def test_customShareLetterTemplate(mockUserSchool, mockLdapHelperSearchOne, mockSharesGetMountpointOfRemotePath, mockSharesmMountShare, mockPrintersInstallPrinter, mockUserIsInGroup, mockComputerIsInGroup, mockConfigShares):
+    mockUserSchool.return_value = (True, "school1")
+    mockLdapHelperSearchOne.return_value = (True, {
+        "distinguishedName": "policy1",
+        "gPCFileSysPath": "\\\\linuxmuster.lan\\sysvol\\linuxmuster.lan\\Policies\\policy1"
+    })
+    mockSharesGetMountpointOfRemotePath.return_value = (True, f"{os.path.dirname(os.path.realpath(__file__))}/files/policy1")
+    mockSharesmMountShare.return_value = (True, "")
+    mockPrintersInstallPrinter.return_value = True
+    mockUserIsInGroup.return_value = True
+    mockComputerIsInGroup.return_value = True
+    mockConfigShares.return_value = {
+        "letterTemplate": "_{letter}"
+    }
+
+    assert gpo.processAllPolicies()
+    assert len(mockSharesmMountShare.call_args_list) == 3
+    assert mockSharesmMountShare.call_args_list[0] == mock.call('\\\\server\\default-school\\program', shareName='Programs_K')
+    # Projects (P:) is disabled and should not be mounted
+    assert mockSharesmMountShare.call_args_list[1] == mock.call('\\\\server\\default-school\\students', shareName='Students-Home_S')
+    assert mockSharesmMountShare.call_args_list[2] == mock.call('\\\\server\\default-school\\share', shareName='Shares')
+
+    # Test without letter
+    mockConfigShares.return_value = {
+        "letterTemplate": ""
+    }
+    assert gpo.processAllPolicies()
+    assert len(mockSharesmMountShare.call_args_list) == 6
+    assert mockSharesmMountShare.call_args_list[3] == mock.call('\\\\server\\default-school\\program', shareName='Programs')
+    # Projects (P:) is disabled and should not be mounted
+    assert mockSharesmMountShare.call_args_list[4] == mock.call('\\\\server\\default-school\\students', shareName='Students-Home')
+    assert mockSharesmMountShare.call_args_list[5] == mock.call('\\\\server\\default-school\\share', shareName='Shares')
+
 @mock.patch("linuxmusterLinuxclient7.gpo.computer.isInGroup")
 @mock.patch("linuxmusterLinuxclient7.gpo.user.isInGroup")
 @mock.patch("linuxmusterLinuxclient7.gpo.printers.installPrinter")

usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_user.py

@@ -135,6 +135,29 @@ def test_getHomeShareMountpoint(mockUsername, mockReadAttributes):
     rc, homeShareMountpoint = user.getHomeShareMountpoint()
     assert rc
     assert homeShareMountpoint == "/home/user1/media/user1 (H:)"
+
+
+@mock.patch("linuxmusterLinuxclient7.gpo.config.shares")
+@mock.patch("linuxmusterLinuxclient7.user.readAttributes")
+@mock.patch("linuxmusterLinuxclient7.user.username")
+def test_getHomeShareMountpointCustomShareLetterTemplate(mockUsername, mockReadAttributes, mockConfigShares):
+    mockUsername.return_value = "user1"
+    mockReadAttributes.return_value = (True, {"homeDrive": "H:"})
+    mockConfigShares.return_value = {
+        "letterTemplate": "_{letter}"
+    }
+
+    rc, homeShareMountpoint = user.getHomeShareMountpoint()
+    assert rc
+    assert homeShareMountpoint == "/home/user1/media/user1_H"
+
+    # Test without letter
+    mockConfigShares.return_value = {
+        "letterTemplate": ""
+    }
+    rc, homeShareMountpoint = user.getHomeShareMountpoint()
+    assert rc
+    assert homeShareMountpoint == "/home/user1/media/user1"
 
 @mock.patch("linuxmusterLinuxclient7.user.shares.mountShare")
 @mock.patch("linuxmusterLinuxclient7.user.readAttributes")