From 49b50538ab0254689930b8c0db58e108e0315e17 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 30 Nov 2014 13:56:09 +0100 Subject: [PATCH 1/7] pylint client, add docs --- letsencrypt/client/client.py | 79 ++++++++++++++++++------------------ letsencrypt/scripts/main.py | 4 +- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 653b4b6c6..7ce8839d2 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -31,54 +31,57 @@ ALLOW_RAW_IPV6_SERVER = False class Client(object): - """ACME protocol client.""" + """ACME protocol client. + + :ivar config: Configurator. + :type config: :class:`letsencrypt.client.configurator.Configurator` + + :ivar str ca_server: Certificate authority server + :ivar str ca_server_url: Full URL of the CSR server + + :ivar csr: Certificate Signing Request + :type csr: :class:`CSR` + + :ivar list names: Domain names (:class:`list` of :class:`str`). + + :ivar privkey: Private key + :type privkey: :class:`Key` + + :ivar redirect: Redirect HTTP to HTTPS? + :type redirect: bool or None + + :ivar bool use_curses: Use curses UI + + """ Key = collections.namedtuple("Key", "file pem") CSR = collections.namedtuple("CSR", "file data type") - def __init__(self, ca_server, cert_signing_request=CSR(None, None, None), - private_key=Key(None, None), use_curses=True): - """ + def __init__(self, ca_server, csr=CSR(None, None, None), + privkey=Key(None, None), redirect=None, use_curses=True): + """Initialize a client.""" + self.ca_server = ca_server + self.ca_server_url = "https://%s/acme/" % self.ca_server + self.names = [] + self.redirect = redirect + self.use_curses = use_curses - :param str ca_server: Certificate authority server - :param str cert_signing_request: Contents of the CSR - :param str private_key: Contents of the private key - :param bool use_curses: Use curses UI - - """ - self.curses = use_curses + self.csr = csr + self.privkey = privkey + self._validate_csr_key_cli() # TODO: catch exceptions # Logger needs to be initialized before Configurator self.init_logger() + # TODO: Can probably figure out which configurator to use # without special packaging based on system info Command # line arg or client function to discover self.config = apache_configurator.ApacheConfigurator( CONFIG.SERVER_ROOT) - self.server = ca_server - # These are CSR/Key namedtuples - self.csr = cert_signing_request - self.privkey = private_key - - # TODO: Figure out all exceptions from this function - try: - self._validate_csr_key_cli() - - except errors.LetsEncryptClientError as e: - # TODO: Something nice here... - logger.fatal(("%s - until the programmers get their act together, " - "we are just going to exit" % str(e))) - sys.exit(1) - self.server_url = "https://%s/acme/" % self.server - - def authenticate(self, domains=None, redirect=None, eula=False): + def authenticate(self, domains=None, eula=False): """ :param list domains: List of domains - - :param redirect: - :type redirect: bool or None - :param bool eula: EULA accepted :raises errors.LetsEncryptClientError: CSR does not contain one of the @@ -91,8 +94,6 @@ class Client(object): if not self.config.config_test(): sys.exit(1) - self.redirect = redirect - # Display preview warning if not eula: with open('EULA') as eula_file: @@ -102,7 +103,7 @@ class Client(object): # Display screen to select domains to validate if domains: - sanity_check_names([self.server] + domains) + sanity_check_names([self.ca_server] + domains) self.names = domains else: # This function adds all names @@ -240,7 +241,7 @@ class Client(object): try: response = requests.post( - self.server_url, + self.ca_server_url, data=json_encoded, headers={"Content-Type": "application/json"}, ) @@ -422,7 +423,7 @@ class Client(object): self.config.enable_site(host) # sites may have been enabled / final cleanup - self.config.restart(quiet=self.curses) + self.config.restart(quiet=self.use_curses) display.success_installation(self.names) @@ -434,7 +435,7 @@ class Client(object): if self.redirect: self.redirect_to_ssl(vhost) - self.config.restart(quiet=self.curses) + self.config.restart(quiet=self.use_curses) # if self.ocsp_stapling is None: # q = ("Would you like to protect the privacy of your users " @@ -703,7 +704,7 @@ class Client(object): return names def init_logger(self): - if self.curses: + if self.use_curses: logger.setLogger(logger.NcursesLogger()) logger.setLogLevel(logger.INFO) else: diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 78afabe4f..220781830 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -93,11 +93,11 @@ def main(): else: csr = client.Client.CSR(args.csr[0], args.csr[1], "pem") - acme = client.Client(server, csr, privkey, args.curses) + acme = client.Client(server, csr, privkey, args.redirect, args.curses) if args.revoke: acme.list_certs_keys() else: - acme.authenticate(args.domains, args.redirect, args.eula) + acme.authenticate(args.domains, args.eula) def read_file(filename): From a88e3f8fc112e4c34287e797873cf2a40ae626d9 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 30 Nov 2014 19:22:07 +0100 Subject: [PATCH 2/7] Add Challenge docs, pep8/docs interactive_challenge --- letsencrypt/client/challenge.py | 1 + letsencrypt/client/interactive_challenge.py | 22 +++++++++++---------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/letsencrypt/client/challenge.py b/letsencrypt/client/challenge.py index 6eaee82f7..bd053ec30 100644 --- a/letsencrypt/client/challenge.py +++ b/letsencrypt/client/challenge.py @@ -6,6 +6,7 @@ from letsencrypt.client import logger class Challenge(object): + """Let's Encrypt challenge.""" def __init__(self, configurator): self.config = configurator diff --git a/letsencrypt/client/interactive_challenge.py b/letsencrypt/client/interactive_challenge.py index 720a0381d..4f93f1e4f 100644 --- a/letsencrypt/client/interactive_challenge.py +++ b/letsencrypt/client/interactive_challenge.py @@ -5,14 +5,15 @@ import dialog from letsencrypt.client import challenge -########################################################### -# Interactive challenge displays the string sent by the CA -# formatted to fit on the screen of the client -# The Challenge also adds proper instructions for how the -# client should continue the letsencrypt process -########################################################### +class InteractiveChallenge(challenge.Challenge): + """Interactive challange. -class Interactive_Challenge(challenge.Challenge): + Interactive challenge displays the string sent by the CA formatted + to fit on the screen of the client. The Challenge also adds proper + instructions for how the client should continue the letsencrypt + process. + + """ BOX_SIZE = 70 def __init__(self, string): @@ -20,16 +21,17 @@ class Interactive_Challenge(challenge.Challenge): def perform(self, quiet=True): if quiet: - dialog.Dialog().msgbox(self.get_display_string(), width=self.BOX_SIZE) + dialog.Dialog().msgbox( + self.get_display_string(), width=self.BOX_SIZE) else: print self.get_display_string() raw_input('') return True - def get_display_string(self): - return textwrap.fill(self.string, width=self.BOX_SIZE) + "\n\nPlease Press Enter to Continue" + return (textwrap.fill(self.string, width=self.BOX_SIZE) + + "\n\nPlease Press Enter to Continue") # def formatted_reasons(self): # return "\n\t* %s\n", self.reason From 06bb6b344b416b56cd96e81b2f1d39edefe91517 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 30 Nov 2014 19:28:03 +0100 Subject: [PATCH 3/7] More docs in challenge --- letsencrypt/client/challenge.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/letsencrypt/client/challenge.py b/letsencrypt/client/challenge.py index bd053ec30..719b04945 100644 --- a/letsencrypt/client/challenge.py +++ b/letsencrypt/client/challenge.py @@ -12,12 +12,19 @@ class Challenge(object): self.config = configurator def perform(self, quiet=True): + """Perform the challange. + + :param bool quiet: TODO + + """ raise NotImplementedError() def generate_response(self): + """Generate response.""" raise NotImplementedError() def cleanup(self): + """Cleanup.""" raise NotImplementedError() @@ -47,10 +54,10 @@ def gen_challenge_path(challenges, combos=None): def _find_smart_path(challenges, combos): - """ - Can be called if combinations is included - Function uses a simple ranking system to choose the combo with the - lowest cost + """Find challenge path with server hints. + + Can be called if combinations is included. Function uses a simple + ranking system to choose the combo with the lowest cost. :param list challenges: A list of challenges from ACME "challenge" server message to be fulfilled by the client in order to prove @@ -94,10 +101,11 @@ def _find_smart_path(challenges, combos): def _find_dumb_path(challenges): - """ - Should be called if the combinations hint is not included by the server - This function returns the best path that does not contain multiple - mutually exclusive challenges + """Find challange path without server hints. + + Should be called if the combinations hint is not included by the + server. This function returns the best path that does not contain + multiple mutually exclusive challenges. :param list challanges: A list of challenges from ACME "challenge" server message to be fulfilled by the client in order to prove From 6886680a6204d75b087e6f00382ea16449f51069 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 1 Dec 2014 20:13:39 +0100 Subject: [PATCH 4/7] Fix redirect var flow --- letsencrypt/client/client.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index a78bd5d2b..db0a85388 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -47,7 +47,7 @@ class Client(object): :ivar privkey: Private key :type privkey: :class:`Key` - :ivar redirect: Redirect HTTP to HTTPS? + :ivar redirect: If traffic should be forwarded from HTTP to HTTPS. :type redirect: bool or None :ivar bool use_curses: Use curses UI @@ -142,7 +142,7 @@ class Client(object): cert_file = self.install_certificate(certificate_dict, vhost) # Perform optimal config changes - self.optimize_config(vhost, redirect) + self.optimize_config(vhost) self.config.save("Completed Let's Encrypt Authentication") @@ -429,19 +429,18 @@ class Client(object): return cert_file - def optimize_config(self, vhost, redirect): + def optimize_config(self, vhost): """Optimize the configuration. :param vhost: vhost to optimize :type vhost: :class:`apache_configurator.VH` - :param bool redirect: If traffic should be forwarded from HTTP to HTTPS - """ - if redirect is None: - redirect = display.redirect_by_default() + # TODO: this should most definitely be moved to __init__ + if self.redirect is None: + self.redirect = display.redirect_by_default() - if redirect: + if self.redirect: self.redirect_to_ssl(vhost) self.config.restart(quiet=self.use_curses) From 5a2143290a5d44a95b430d21c0536470fea6b245 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 1 Dec 2014 22:33:57 +0100 Subject: [PATCH 5/7] ca_server -> server --- letsencrypt/client/client.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index db0a85388..62ed89de5 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -36,8 +36,8 @@ class Client(object): :ivar config: Configurator. :type config: :class:`letsencrypt.client.configurator.Configurator` - :ivar str ca_server: Certificate authority server - :ivar str ca_server_url: Full URL of the CSR server + :ivar str server: Certificate authority server + :ivar str server_url: Full URL of the CSR server :ivar csr: Certificate Signing Request :type csr: :class:`CSR` @@ -56,11 +56,11 @@ class Client(object): Key = collections.namedtuple("Key", "file pem") CSR = collections.namedtuple("CSR", "file data type") - def __init__(self, ca_server, csr=CSR(None, None, None), + def __init__(self, server, csr=CSR(None, None, None), privkey=Key(None, None), redirect=None, use_curses=True): """Initialize a client.""" - self.ca_server = ca_server - self.ca_server_url = "https://%s/acme/" % self.ca_server + self.server = server + self.server_url = "https://%s/acme/" % self.server self.names = [] self.redirect = redirect self.use_curses = use_curses @@ -103,7 +103,7 @@ class Client(object): # Display screen to select domains to validate if domains: - sanity_check_names([self.ca_server] + domains) + sanity_check_names([self.server] + domains) self.names = domains else: # This function adds all names @@ -241,7 +241,7 @@ class Client(object): try: response = requests.post( - self.ca_server_url, + self.server_url, data=json_encoded, headers={"Content-Type": "application/json"}, ) From e0f91b06b2991f9218c4ff0c72188d7745f82019 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 1 Dec 2014 22:37:15 +0100 Subject: [PATCH 6/7] Move redirect to authenticate --- letsencrypt/client/client.py | 24 +++++++++++++----------- letsencrypt/scripts/main.py | 4 ++-- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 62ed89de5..006f6289e 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -47,9 +47,6 @@ class Client(object): :ivar privkey: Private key :type privkey: :class:`Key` - :ivar redirect: If traffic should be forwarded from HTTP to HTTPS. - :type redirect: bool or None - :ivar bool use_curses: Use curses UI """ @@ -57,12 +54,11 @@ class Client(object): CSR = collections.namedtuple("CSR", "file data type") def __init__(self, server, csr=CSR(None, None, None), - privkey=Key(None, None), redirect=None, use_curses=True): + privkey=Key(None, None), use_curses=True): """Initialize a client.""" self.server = server self.server_url = "https://%s/acme/" % self.server self.names = [] - self.redirect = redirect self.use_curses = use_curses self.csr = csr @@ -78,12 +74,15 @@ class Client(object): self.config = apache_configurator.ApacheConfigurator( CONFIG.SERVER_ROOT) - def authenticate(self, domains=None, eula=False): + def authenticate(self, domains=None, eula=False, redirect=None): """ :param list domains: List of domains :param bool eula: EULA accepted + :param redirect: If traffic should be forwarded from HTTP to HTTPS. + :type redirect: bool or None + :raises errors.LetsEncryptClientError: CSR does not contain one of the specified names. @@ -142,7 +141,7 @@ class Client(object): cert_file = self.install_certificate(certificate_dict, vhost) # Perform optimal config changes - self.optimize_config(vhost) + self.optimize_config(vhost, redirect) self.config.save("Completed Let's Encrypt Authentication") @@ -429,18 +428,21 @@ class Client(object): return cert_file - def optimize_config(self, vhost): + def optimize_config(self, vhost, redirect=None): """Optimize the configuration. :param vhost: vhost to optimize :type vhost: :class:`apache_configurator.VH` + :param redirect: If traffic should be forwarded from HTTP to HTTPS. + :type redirect: bool or None + """ # TODO: this should most definitely be moved to __init__ - if self.redirect is None: - self.redirect = display.redirect_by_default() + if redirect is None: + redirect = display.redirect_by_default() - if self.redirect: + if redirect: self.redirect_to_ssl(vhost) self.config.restart(quiet=self.use_curses) diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 499703e94..70fa6302d 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -93,11 +93,11 @@ def main(): else: csr = client.Client.CSR(args.csr[0], args.csr[1], "pem") - acme = client.Client(server, csr, privkey, args.redirect, args.curses) + acme = client.Client(server, csr, privkey, args.curses) if args.revoke: acme.list_certs_keys() else: - acme.authenticate(args.domains, args.eula) + acme.authenticate(args.domains, args.eula, arg.redirect) def read_file(filename): From bcd423aaa37e622e467b196c28223554ddad3434 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 2 Dec 2014 12:19:15 -0800 Subject: [PATCH 7/7] Fixed undefined variable from Kuba-PEP8 --- letsencrypt/scripts/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 70fa6302d..fc63e5b62 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -97,7 +97,7 @@ def main(): if args.revoke: acme.list_certs_keys() else: - acme.authenticate(args.domains, args.eula, arg.redirect) + acme.authenticate(args.domains, args.eula, args.redirect) def read_file(filename):