mirror of
https://github.com/certbot/certbot.git
synced 2026-05-28 04:34:11 -04:00
Fix review comments
This commit is contained in:
Jakub Warmuz
parent
4b829603d0
commit
7a4c7acdfb
4 changed files with 105 additions and 50 deletions
|
|
@ -1,6 +1,8 @@
|
|||
:mod:`letsencrypt.acme`
|
||||
=======================
|
||||
|
||||
.. contents::
|
||||
|
||||
.. automodule:: letsencrypt.acme
|
||||
:members:
|
||||
|
||||
|
|
|
|||
|
|
@ -43,7 +43,8 @@ class Error(jose.JSONObjectWithFields, Exception):
|
|||
return without_prefix
|
||||
|
||||
@property
|
||||
def description(self): # pylint: disable=missing-docstring,no-self-argument
|
||||
def description(self):
|
||||
"""Hardcoded error description based on its type."""
|
||||
return self.ERROR_TYPE_DESCRIPTIONS[self.typ]
|
||||
|
||||
|
||||
|
|
@ -91,7 +92,11 @@ IDENTIFIER_FQDN = IdentifierType('dns') # IdentifierDNS in Boulder
|
|||
|
||||
|
||||
class Identifier(jose.JSONObjectWithFields):
|
||||
"""ACME identifier."""
|
||||
"""ACME identifier.
|
||||
|
||||
:ivar letsencrypt.acme.messages2.IdentifierType typ:
|
||||
|
||||
"""
|
||||
typ = jose.Field('type', decoder=IdentifierType.from_json)
|
||||
value = jose.Field('value')
|
||||
|
||||
|
|
@ -99,32 +104,35 @@ class Identifier(jose.JSONObjectWithFields):
|
|||
class Resource(jose.ImmutableMap):
|
||||
"""ACME Resource.
|
||||
|
||||
:param body: Resource body.
|
||||
:type body: Instance of `ResourceBody` (subclass).
|
||||
|
||||
:param str uri: Location of the resource.
|
||||
:ivar letsencrypt.acme.messages2.ResourceBody body: Resource body.
|
||||
:ivar str uri: Location of the resource.
|
||||
|
||||
"""
|
||||
__slots__ = ('body', 'uri')
|
||||
|
||||
|
||||
class ResourceBody(jose.JSONObjectWithFields):
|
||||
"""ACME Resource body."""
|
||||
"""ACME Resource Body."""
|
||||
|
||||
|
||||
class RegistrationResource(Resource):
|
||||
"""Registration resource.
|
||||
"""Registration Resource.
|
||||
|
||||
:ivar body: `Registration`
|
||||
:ivar str uri: URI of the resource.
|
||||
:ivar new_authzr_uri: URI found in the 'next' Link header
|
||||
:ivar letsencrypt.acme.messages2.Registration body:
|
||||
:ivar str new_authzr_uri: URI found in the 'next' ``Link`` header
|
||||
:ivar str terms_of_service: URL for the CA TOS.
|
||||
|
||||
"""
|
||||
__slots__ = ('body', 'uri', 'new_authzr_uri', 'terms_of_service')
|
||||
|
||||
|
||||
class Registration(ResourceBody):
|
||||
"""Registration resource body."""
|
||||
"""Registration Resource Body.
|
||||
|
||||
:ivar letsencrypt.acme.jose.jwk.JWK key: Public key.
|
||||
:ivar tuple contact:
|
||||
|
||||
"""
|
||||
|
||||
# on new-reg key server ignores 'key' and populates it based on
|
||||
# JWS.signature.combined.jwk
|
||||
|
|
@ -135,10 +143,10 @@ class Registration(ResourceBody):
|
|||
|
||||
|
||||
class ChallengeResource(Resource, jose.JSONObjectWithFields):
|
||||
"""Challenge resource.
|
||||
"""Challenge Resource.
|
||||
|
||||
:ivar body: `.challenges.ChallengeBody`
|
||||
:ivar authzr_uri: URI found in the 'up' Link header.
|
||||
:ivar letsencrypt.acme.messages2.ChallengeBody body:
|
||||
:ivar str authzr_uri: URI found in the 'up' ``Link`` header.
|
||||
|
||||
"""
|
||||
__slots__ = ('body', 'authzr_uri')
|
||||
|
|
@ -151,18 +159,21 @@ class ChallengeResource(Resource, jose.JSONObjectWithFields):
|
|||
|
||||
|
||||
class ChallengeBody(ResourceBody):
|
||||
"""Challenge resource body.
|
||||
|
||||
Confusingly, this has a similar name to `.challenges.Challenge`, as
|
||||
well as `.achallanges.AnnotatedChallenge` or
|
||||
`.achallanges.IndexedChallenge`. Use names such as ``challb`` to
|
||||
distinguish instances of this class from ``achall`` or ``ichall``.
|
||||
"""Challenge Resource Body.
|
||||
|
||||
.. todo::
|
||||
This class could be integrated with challenges.Challenge, but
|
||||
this way it would be confusing when compared to acme-spec, where
|
||||
all challenges are presented without 'uri', 'status', or
|
||||
'validated' fields.
|
||||
Confusingly, this has a similar name to `.challenges.Challenge`,
|
||||
as well as `.achallenges.AnnotatedChallenge` or
|
||||
`.achallenges.Indexed`... Once `messages2` and `network2` is
|
||||
integrated with the rest of the client, this class functionality
|
||||
will be merged with `.challenges.Challenge`. Meanwhile,
|
||||
separation allows the ``master`` to be still interoperable with
|
||||
Node.js server (protocol v00). For the time being use names such
|
||||
as ``challb`` to distinguish instances of this class from
|
||||
``achall`` or ``ichall``.
|
||||
|
||||
:ivar letsencrypt.acme.messages2.Status status:
|
||||
:ivar datetime.datetime validated:
|
||||
|
||||
"""
|
||||
|
||||
|
|
@ -184,19 +195,26 @@ class ChallengeBody(ResourceBody):
|
|||
|
||||
|
||||
class AuthorizationResource(Resource):
|
||||
"""Authorization resource.
|
||||
"""Authorization Resource.
|
||||
|
||||
:ivar body: `Authorization`
|
||||
:ivar new_cert_uri: URI found in the 'next' Link header
|
||||
:ivar letsencrypt.acme.messages2.Authorization body:
|
||||
:ivar str new_cert_uri: URI found in the 'next' ``Link`` header
|
||||
|
||||
"""
|
||||
__slots__ = ('body', 'uri', 'new_cert_uri')
|
||||
|
||||
|
||||
class Authorization(ResourceBody):
|
||||
"""Authorization resource body.
|
||||
"""Authorization Resource Body.
|
||||
|
||||
:ivar challenges: `list` of `Challenge`
|
||||
:ivar letsencrypt.acme.messages2.Identifier identifier:
|
||||
:ivar list challenges: `list` of `Challenge`
|
||||
:ivar tuple combinations: Challenge combinations (`tuple` of `tuple`
|
||||
of `int`, as opposed to `list` of `list` from the spec).
|
||||
:ivar letsencrypt.acme.jose.jwk.JWK key: Public key.
|
||||
:ivar tuple contact:
|
||||
:ivar letsencrypt.acme.messages2.Status status:
|
||||
:ivar datetime.datetime expires:
|
||||
|
||||
"""
|
||||
|
||||
|
|
@ -229,7 +247,9 @@ class Authorization(ResourceBody):
|
|||
class CertificateRequest(jose.JSONObjectWithFields):
|
||||
"""ACME new-cert request.
|
||||
|
||||
:ivar csr: `M2Crypto.X509.Request` wrapped in `.ComparableX509`
|
||||
:ivar letsencrypt.acme.jose.util.ComparableX509 csr:
|
||||
`M2Crypto.X509.Request` wrapped in `.ComparableX509`
|
||||
:ivar tuple authorizations: `tuple` of URIs (`str`)
|
||||
|
||||
"""
|
||||
csr = jose.Field('csr', decoder=jose.decode_csr, encoder=jose.encode_csr)
|
||||
|
|
@ -237,11 +257,12 @@ class CertificateRequest(jose.JSONObjectWithFields):
|
|||
|
||||
|
||||
class CertificateResource(Resource):
|
||||
"""Authorization resource.
|
||||
"""Certificate Resource.
|
||||
|
||||
:ivar body: `M2Crypto.X509.X509` wrapped in `.ComparableX509`
|
||||
:ivar cert_chain_uri: URI found in the 'up' Link header
|
||||
:ivar authzrs: `list` of `AuthorizationResource`.
|
||||
:ivar letsencrypt.acme.jose.util.ComparableX509 body:
|
||||
`M2Crypto.X509.X509` wrapped in `.ComparableX509`
|
||||
:ivar str cert_chain_uri: URI found in the 'up' ``Link`` header
|
||||
:ivar tuple authzrs: `tuple` of `AuthorizationResource`.
|
||||
|
||||
"""
|
||||
__slots__ = ('body', 'uri', 'cert_chain_uri', 'authzrs')
|
||||
|
|
@ -250,7 +271,8 @@ class CertificateResource(Resource):
|
|||
class Revocation(jose.JSONObjectWithFields):
|
||||
"""Revocation message.
|
||||
|
||||
:ivar revoke: Either a `datetime.datetime` or `NOW`.
|
||||
:ivar revoke: Either a `datetime.datetime` or `Revocation.NOW`.
|
||||
:ivar tuple authorizations: Same as `CertificateRequest.authorizations`
|
||||
|
||||
"""
|
||||
|
||||
|
|
|
|||
|
|
@ -23,13 +23,17 @@ requests.packages.urllib3.contrib.pyopenssl.inject_into_urllib3()
|
|||
class Network(object):
|
||||
"""ACME networking.
|
||||
|
||||
.. todo::
|
||||
Clean up raised error types hierarchy, document, and handle (wrap)
|
||||
instances of `.DeserializationError` raised in `from_json()``.
|
||||
|
||||
:ivar str new_reg_uri: Location of new-reg
|
||||
:ivar key: `.JWK` (private)
|
||||
:ivar alg: `.JWASignature`
|
||||
|
||||
"""
|
||||
|
||||
DER_CONTENT_TYPE = 'application/plix-cert'
|
||||
DER_CONTENT_TYPE = 'application/pkix-cert'
|
||||
JSON_CONTENT_TYPE = 'application/json'
|
||||
JSON_ERROR_CONTENT_TYPE = 'application/problem+json'
|
||||
|
||||
|
|
@ -58,6 +62,16 @@ class Network(object):
|
|||
HTTP header is ignored if response is an expected JSON object
|
||||
(c.f. Boulder #56).
|
||||
|
||||
:param str content_type: Expected Content-Type response header.
|
||||
If JSON is expected and not present in server response, this
|
||||
function will raise an error. Otherwise, wrong Content-Type
|
||||
is ignored, but logged.
|
||||
|
||||
:raises letsencrypt.messages2.Error: If server response body
|
||||
carries HTTP Problem (draft-ietf-appsawg-http-problem-00).
|
||||
:raises letsencrypt.errors.NetworkError: In case of other
|
||||
networking errors.
|
||||
|
||||
"""
|
||||
response_ct = response.headers.get('Content-Type')
|
||||
|
||||
|
|
@ -222,9 +236,12 @@ class Network(object):
|
|||
:param identifier: Identifier to be challenged.
|
||||
:type identifier: `.messages2.Identifier`
|
||||
|
||||
:pram regr: Registration resource.
|
||||
:param regr: Registration Resource.
|
||||
:type regr: `.RegistrationResource`
|
||||
|
||||
:returns: Authorization Resource.
|
||||
:rtype: `.AuthorizationResource`
|
||||
|
||||
"""
|
||||
new_authz = messages2.Authorization(identifier=identifier)
|
||||
response = self._post(regr.new_authzr_uri, self._wrap_in_jws(new_authz))
|
||||
|
|
@ -232,7 +249,15 @@ class Network(object):
|
|||
return self._authzr_from_response(response, identifier)
|
||||
|
||||
def request_domain_challenges(self, domain, regr):
|
||||
"""Request challenges for domain names."""
|
||||
"""Request challenges for domain names.
|
||||
|
||||
This is simply a convenience function that wraps around
|
||||
`request_challenges`, but works with domain names instead of
|
||||
generic identifiers.
|
||||
|
||||
:param str domain: Domain name to be challenged.
|
||||
|
||||
"""
|
||||
return self.request_challenges(messages2.Identifier(
|
||||
typ=messages2.IDENTIFIER_FQDN, value=domain), regr)
|
||||
|
||||
|
|
@ -245,7 +270,7 @@ class Network(object):
|
|||
:param response: Corresponding Challenge response
|
||||
:type response: `.challenges.ChallengeResponse`
|
||||
|
||||
:returns: Challenge resource with updated body.
|
||||
:returns: Challenge Resource with updated body.
|
||||
:rtype: `.ChallengeResource`
|
||||
|
||||
:raises errors.UnexpectedUpdate:
|
||||
|
|
@ -345,10 +370,7 @@ class Network(object):
|
|||
content_type=content_type,
|
||||
headers={'Accept': content_type})
|
||||
|
||||
try:
|
||||
cert_chain_uri = response.links['up']['url']
|
||||
except KeyError:
|
||||
raise errors.NetworkError('"up" Link missing')
|
||||
cert_chain_uri = response.links.get('up', {}).get('url')
|
||||
|
||||
try:
|
||||
uri = response.headers['Location']
|
||||
|
|
@ -451,6 +473,9 @@ class Network(object):
|
|||
:rtype: `.CertificateResource`
|
||||
|
||||
"""
|
||||
# TODO: If a client sends a refresh request and the server is
|
||||
# not willing to refresh the certificate, the server MUST
|
||||
# respond with status code 403 (Forbidden)
|
||||
return self.check_cert(certr)
|
||||
|
||||
def fetch_chain(self, certr):
|
||||
|
|
@ -459,11 +484,12 @@ class Network(object):
|
|||
:param certr: Certificate Resource
|
||||
:type certr: `.CertificateResource`
|
||||
|
||||
:returns: Certificate chain
|
||||
:returns: Certificate chain, or `None` if no "up" Link was provided.
|
||||
:rtype: `M2Crypto.X509.X509` wrapped in `.ComparableX509`
|
||||
|
||||
"""
|
||||
return self._get_cert(certr.cert_chain_uri)
|
||||
if certr.cert_chain_uri is not None:
|
||||
return self._get_cert(certr.cert_chain_uri)
|
||||
|
||||
def revoke(self, certr, when=messages2.Revocation.NOW):
|
||||
"""Revoke certificate.
|
||||
|
|
|
|||
|
|
@ -317,13 +317,14 @@ class NetworkTest(unittest.TestCase):
|
|||
# TODO: check POST args
|
||||
|
||||
def test_request_issuance_missing_up(self):
|
||||
self.response.content = CERT.as_der()
|
||||
self.response.headers['Location'] = self.certr.uri
|
||||
self._mock_post_get()
|
||||
self.assertRaises(
|
||||
errors.NetworkError, self.net.request_issuance,
|
||||
CSR, (self.authzr,))
|
||||
self.assertEqual(
|
||||
self.certr.update(cert_chain_uri=None),
|
||||
self.net.request_issuance(CSR, (self.authzr,)))
|
||||
|
||||
def test_request_issuance_missing_location(self):
|
||||
self.response.links['up'] = {'url': self.certr.cert_chain_uri}
|
||||
self._mock_post_get()
|
||||
self.assertRaises(
|
||||
errors.NetworkError, self.net.request_issuance,
|
||||
|
|
@ -437,6 +438,10 @@ class NetworkTest(unittest.TestCase):
|
|||
self.assertEqual(self.net._get_cert(self.certr.cert_chain_uri),
|
||||
self.net.fetch_chain(self.certr))
|
||||
|
||||
def test_fetch_chain_no_up_link(self):
|
||||
self.assertTrue(self.net.fetch_chain(self.certr.update(
|
||||
cert_chain_uri=None)) is None)
|
||||
|
||||
def test_revoke(self):
|
||||
self._mock_post_get()
|
||||
self.net.revoke(self.certr, when=messages2.Revocation.NOW)
|
||||
|
|
|
|||
Loading…
Reference in a new issue