diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4c798ee5c..b6181bb64 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -569,7 +569,7 @@ def plugins_cmd(args, config, plugins): # TODO: Use IDisplay rather than print def read_file(filename, mode="rb"): """Returns the given file's contents. - :param str filename: Filename + :param str filename: filename as an absolute path :param str mode: open mode (see `open`) :returns: A tuple of filename and its contents @@ -579,6 +579,7 @@ def read_file(filename, mode="rb"): """ try: + filename = os.path.abspath(filename) return filename, open(filename, mode).read() except IOError as exc: raise argparse.ArgumentTypeError(exc.strerror) @@ -930,26 +931,28 @@ def _paths_parser(helpful): if verb in ("install", "revoke", "certonly"): section = verb if verb == "certonly": - add(section, "--cert-path", default=flag_default("auth_cert_path"), help=cph) + add(section, "--cert-path", type=os.path.abspath, + default=flag_default("auth_cert_path"), help=cph) elif verb == "revoke": add(section, "--cert-path", type=read_file, required=True, help=cph) else: - add(section, "--cert-path", help=cph, required=(verb == "install")) + add(section, "--cert-path", type=os.path.abspath, + help=cph, required=(verb == "install")) section = "paths" if verb in ("install", "revoke"): section = verb # revoke --key-path reads a file, install --key-path takes a string - add(section, "--key-path", type=((verb == "revoke" and read_file) or str), - required=(verb == "install"), + add(section, "--key-path", required=(verb == "install"), + type=((verb == "revoke" and read_file) or os.path.abspath), help="Path to private key for cert creation or revocation (if account key is missing)") default_cp = None if verb == "certonly": default_cp = flag_default("auth_chain_path") - add("paths", "--fullchain-path", default=default_cp, + add("paths", "--fullchain-path", default=default_cp, type=os.path.abspath, help="Accompanying path to a full certificate chain (cert plus chain).") - add("paths", "--chain-path", default=default_cp, + add("paths", "--chain-path", default=default_cp, type=os.path.abspath, help="Accompanying path to a certificate chain.") add("paths", "--config-dir", default=flag_default("config_dir"), help=config_help("config_dir")) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index e70171675..4955655f3 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -37,6 +37,11 @@ class NamespaceConfig(object): def __init__(self, namespace): self.namespace = namespace + + self.namespace.config_dir = os.path.abspath(self.namespace.config_dir) + self.namespace.work_dir = os.path.abspath(self.namespace.work_dir) + self.namespace.logs_dir = os.path.abspath(self.namespace.logs_dir) + # Check command line parameters sanity, and error out in case of problem. check_config_sanity(self) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 509b7eb34..2f729f71d 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -1,4 +1,5 @@ """Tests for letsencrypt.cli.""" +import argparse import itertools import os import shutil @@ -22,7 +23,7 @@ from letsencrypt.tests import test_util CSR = test_util.vector_path('csr.der') -class CLITest(unittest.TestCase): +class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods """Tests for different commands.""" def setUp(self): @@ -115,6 +116,23 @@ class CLITest(unittest.TestCase): from letsencrypt import cli self.assertTrue(cli.usage_strings(plugins)[0] in out) + def test_install_abspath(self): + cert = 'cert' + key = 'key' + chain = 'chain' + fullchain = 'fullchain' + + with MockedVerb('install') as mock_install: + self._call(['install', '--cert-path', cert, '--key-path', 'key', + '--chain-path', 'chain', + '--fullchain-path', 'fullchain']) + + args = mock_install.call_args[0][0] + self.assertEqual(args.cert_path, os.path.abspath(cert)) + self.assertEqual(args.key_path, os.path.abspath(key)) + self.assertEqual(args.chain_path, os.path.abspath(chain)) + self.assertEqual(args.fullchain_path, os.path.abspath(fullchain)) + @mock.patch('letsencrypt.cli.display_ops') def test_installer_selection(self, mock_display_ops): self._call(['install', '--domain', 'foo.bar', '--cert-path', 'cert', @@ -210,6 +228,23 @@ class CLITest(unittest.TestCase): available = verified.available() stdout.write.called_once_with(str(available)) + def test_certonly_abspath(self): + cert = 'cert' + key = 'key' + chain = 'chain' + fullchain = 'fullchain' + + with MockedVerb('certonly') as mock_obtaincert: + self._call(['certonly', '--cert-path', cert, '--key-path', 'key', + '--chain-path', 'chain', + '--fullchain-path', 'fullchain']) + + args = mock_obtaincert.call_args[0][0] + self.assertEqual(args.cert_path, os.path.abspath(cert)) + self.assertEqual(args.key_path, os.path.abspath(key)) + self.assertEqual(args.chain_path, os.path.abspath(chain)) + self.assertEqual(args.fullchain_path, os.path.abspath(fullchain)) + def test_certonly_bad_args(self): ret, _, _, _ = self._call(['-d', 'foo.bar', 'certonly', '--csr', CSR]) self.assertEqual(ret, '--domain and --csr are mutually exclusive') @@ -356,6 +391,20 @@ class CLITest(unittest.TestCase): mock_sys.exit.assert_called_with(''.join( traceback.format_exception_only(KeyboardInterrupt, interrupt))) + def test_read_file(self): + from letsencrypt import cli + rel_test_path = os.path.relpath(os.path.join(self.tmp_dir, 'foo')) + self.assertRaises( + argparse.ArgumentTypeError, cli.read_file, rel_test_path) + + test_contents = 'bar\n' + with open(rel_test_path, 'w') as f: + f.write(test_contents) + + path, contents = cli.read_file(rel_test_path) + self.assertEqual(path, os.path.abspath(path)) + self.assertEqual(contents, test_contents) + class DetermineAccountTest(unittest.TestCase): """Tests for letsencrypt.cli._determine_account.""" diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index 3a8bf40cf..c42b99081 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -59,6 +59,38 @@ class NamespaceConfigTest(unittest.TestCase): self.namespace.http01_port = None self.assertEqual(80, self.config.http01_port) + def test_absolute_paths(self): + from letsencrypt.configuration import NamespaceConfig + + config_base = "foo" + work_base = "bar" + logs_base = "baz" + + mock_namespace = mock.MagicMock(spec=['config_dir', 'work_dir', + 'logs_dir', 'http01_port', + 'tls_sni_01_port', + 'domains', 'server']) + mock_namespace.config_dir = config_base + mock_namespace.work_dir = work_base + mock_namespace.logs_dir = logs_base + config = NamespaceConfig(mock_namespace) + + self.assertTrue(os.path.isabs(config.config_dir)) + self.assertEqual(config.config_dir, + os.path.join(os.getcwd(), config_base)) + self.assertTrue(os.path.isabs(config.work_dir)) + self.assertEqual(config.work_dir, + os.path.join(os.getcwd(), work_base)) + self.assertTrue(os.path.isabs(config.logs_dir)) + self.assertEqual(config.logs_dir, + os.path.join(os.getcwd(), logs_base)) + self.assertTrue(os.path.isabs(config.accounts_dir)) + self.assertTrue(os.path.isabs(config.backup_dir)) + self.assertTrue(os.path.isabs(config.csr_dir)) + self.assertTrue(os.path.isabs(config.in_progress_dir)) + self.assertTrue(os.path.isabs(config.key_dir)) + self.assertTrue(os.path.isabs(config.temp_checkpoint_dir)) + class RenewerConfigurationTest(unittest.TestCase): """Test for letsencrypt.configuration.RenewerConfiguration.""" @@ -81,6 +113,28 @@ class RenewerConfigurationTest(unittest.TestCase): self.config.renewal_configs_dir, '/tmp/config/renewal_configs') self.assertEqual(self.config.renewer_config_file, '/tmp/config/r.conf') + def test_absolute_paths(self): + from letsencrypt.configuration import NamespaceConfig + from letsencrypt.configuration import RenewerConfiguration + + config_base = "foo" + work_base = "bar" + logs_base = "baz" + + mock_namespace = mock.MagicMock(spec=['config_dir', 'work_dir', + 'logs_dir', 'http01_port', + 'tls_sni_01_port', + 'domains', 'server']) + mock_namespace.config_dir = config_base + mock_namespace.work_dir = work_base + mock_namespace.logs_dir = logs_base + config = RenewerConfiguration(NamespaceConfig(mock_namespace)) + + self.assertTrue(os.path.isabs(config.archive_dir)) + self.assertTrue(os.path.isabs(config.live_dir)) + self.assertTrue(os.path.isabs(config.renewal_configs_dir)) + self.assertTrue(os.path.isabs(config.renewer_config_file)) + if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index e76b6eb88..daec9678f 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -692,6 +692,9 @@ class RenewableCertTests(BaseRenewableCertTest): self.test_rc.configfile["renewalparams"]["http01_port"] = "1234" self.test_rc.configfile["renewalparams"]["account"] = "abcde" self.test_rc.configfile["renewalparams"]["domains"] = ["example.com"] + self.test_rc.configfile["renewalparams"]["config_dir"] = "config" + self.test_rc.configfile["renewalparams"]["work_dir"] = "work" + self.test_rc.configfile["renewalparams"]["logs_dir"] = "logs" mock_auth = mock.MagicMock() mock_pd.PluginsRegistry.find_all.return_value = {"apache": mock_auth} # Fails because "fake" != "apache"