From 8db14e9c5a3ed2719833e33c9b808f3668115bd3 2020-07-20 23:40:24
From: Branko Majic <branko@majic.rs>
Date: 2020-07-20 23:40:24
Subject: [PATCH] GC-37: The --csr and --key-specification options should be exclusive:

- Updated list of invalid invocations in the unit tests.
- Updated parsers for server and client subcommands.

---

diff --git a/gimmecert/cli.py b/gimmecert/cli.py
index 408c83e9bdf5d187d92d25027041c888556d1172..360244e5e2dacad79d3adf65cbdf0fa4eb42ec70 100644
--- a/gimmecert/cli.py
+++ b/gimmecert/cli.py
@@ -167,10 +167,13 @@ def setup_server_subcommand_parser(parser, subparsers):
     subparser = subparsers.add_parser('server', description='Issues server certificate.')
     subparser.add_argument('entity_name', help='Name of the server entity.')
     subparser.add_argument('dns_name', nargs='*', help='Additional DNS names to include in subject alternative name.')
-    subparser.add_argument('--csr', '-c', type=str, default=None, help='''Do not generate server private key locally, and use the passed-in \
+    key_specification_or_csr_group = subparser.add_mutually_exclusive_group()
+    key_specification_or_csr_group.add_argument('--csr', '-c', type=str, default=None,
+                                                help='''Do not generate server private key locally, and use the passed-in \
     certificate signing request (CSR) instead. Use dash (-) to read from standard input. Only the public key is taken from the CSR.''')
-    subparser.add_argument('--key-specification', '-k', type=key_specification,
-                           help=ArgumentHelp.key_specification_format + " Default is to use same algorithm/parameters as used by CA hierarchy.", default=None)
+    key_specification_or_csr_group.add_argument('--key-specification', '-k', type=key_specification, default=None,
+                                                help=ArgumentHelp.key_specification_format +
+                                                " Default is to use same algorithm/parameters as used by CA hierarchy.")
 
     def server_wrapper(args):
         project_directory = os.getcwd()
@@ -186,10 +189,13 @@ def setup_server_subcommand_parser(parser, subparsers):
 def setup_client_subcommand_parser(parser, subparsers):
     subparser = subparsers.add_parser('client', description='Issue client certificate.')
     subparser.add_argument('entity_name', help='Name of the client entity.')
-    subparser.add_argument('--csr', '-c', type=str, default=None, help='''Do not generate client private key locally, and use the passed-in \
+    key_specification_or_csr_group = subparser.add_mutually_exclusive_group()
+    key_specification_or_csr_group.add_argument('--csr', '-c', type=str, default=None,
+                                                help='''Do not generate client private key locally, and use the passed-in \
     certificate signing request (CSR) instead. Use dash (-) to read from standard input. Only the public key is taken from the CSR.''')
-    subparser.add_argument('--key-specification', '-k', type=key_specification,
-                           help=ArgumentHelp.key_specification_format + " Default is to use same algorithm/parameters as used by CA hierarchy.", default=None)
+    key_specification_or_csr_group.add_argument('--key-specification', '-k', type=key_specification, default=None,
+                                                help=ArgumentHelp.key_specification_format +
+                                                " Default is to use same algorithm/parameters as used by CA hierarchy.")
 
     def client_wrapper(args):
         project_directory = os.getcwd()
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 9dee755e63949619fe01aa6ea5eee77918072ad1..7d6fd5335fdcba9e3a3974186cbef3e03c70012a 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -391,12 +391,18 @@ INVALID_CLI_INVOCATIONS = [
     ("gimmecert.cli.server", ["gimmecert", "server", "-k", "unsupported:algorithm", "myserver"]),
     ("gimmecert.cli.server", ["gimmecert", "server", "-k", "ecdsa:unsupported_curve", "myserver"]),
 
+    # server, both key specification and csr specified at the same time
+    ("gimmecert.cli.server", ["gimmecert", "server", "-k", "rsa:1024", "--csr", "myserver.csr.pem", "myserver"]),
+
     # client, invalid key specification
     ("gimmecert.cli.client", ["gimmecert", "client", "-k", "rsa", "myclient"]),
     ("gimmecert.cli.client", ["gimmecert", "client", "-k", "rsa:not_a_number", "myclient"]),
     ("gimmecert.cli.client", ["gimmecert", "client", "-k", "unsupported:algorithm", "myclient"]),
     ("gimmecert.cli.client", ["gimmecert", "client", "-k", "ecdsa:unsupported_curve", "myserver"]),
 
+    # client, both key specification and csr specified at the same time
+    ("gimmecert.cli.client", ["gimmecert", "client", "-k", "rsa:1024", "--csr", "myclient.csr.pem", "myclient"]),
+
     # renew, key specification without new private key option
     ("gimmecert.cli.renew", ["gimmecert", "renew", "-k", "rsa:1024", "server", "myserver"]),
     ("gimmecert.cli.renew", ["gimmecert", "renew", "-k", "rsa:1024", "client", "myclient"]),
@@ -404,6 +410,10 @@ INVALID_CLI_INVOCATIONS = [
     # renew, both new private key and csr specified at same time
     ("gimmecert.cli.renew", ["gimmecert", "renew", "server", "--new-private-key", "--csr", "myserver.csr.pem", "myserver"]),
     ("gimmecert.cli.renew", ["gimmecert", "renew", "client", "--new-private-key", "--csr", "myclient.csr.pem", "myclient"]),
+
+    # renew, both key specification and csr specified at the same time
+    ("gimmecert.cli.renew", ["gimmecert", "renew", "server", "--key-specification", "rsa:1024", "--csr", "myserver.csr.pem", "myserver"]),
+    ("gimmecert.cli.renew", ["gimmecert", "renew", "client", "--key-specification", "rsa:1024", "--csr", "myclient.csr.pem", "myclient"]),
 ]