rework BasicConstraints and Extension.

3 files changed, 66 insertions, 32 deletions

diff --git a/docs/x509.rst b/docs/x509.rst
index 80242581..7eb47a31 100644
--- a/docs/x509.rst
+++ b/docs/x509.rst
@@ -280,7 +280,11 @@ X.509 Extensions
 
     .. versionadded:: 0.9
 
-    All X.509 extensions are registered against this interface.
+    .. attribute:: oid
+
+        :type: :class:`ObjectIdentifier`
+
+        The attribute OID.
 
     .. attribute:: critical
 
@@ -288,13 +292,18 @@ X.509 Extensions
 
         Determines whether a given extension is critical or not.
 
+    .. attribute:: value
+
+        Returns an instance of the extension type corresponding to the OID.
+
 .. class:: BasicConstraints
 
     .. versionadded:: 0.9
 
-    Basic constraints is an X.509 extension that defines whether a given
+    Basic constraints is an X.509 extension type that defines whether a given
     certificate is allowed to sign additional certificates and what path
-    length restrictions may exist.
+    length restrictions may exist. It corresponds to
+    :data:`OID_BASIC_CONSTRAINTS`.
 
     .. attribute:: ca
 
diff --git a/src/cryptography/x509.py b/src/cryptography/x509.py
index 510e9c6f..d64d61f0 100644
--- a/src/cryptography/x509.py
+++ b/src/cryptography/x509.py
@@ -42,6 +42,7 @@ _OID_NAMES = {
     "1.2.840.10040.4.3": "dsa-with-sha1",
     "2.16.840.1.101.3.4.3.1": "dsa-with-sha224",
     "2.16.840.1.101.3.4.3.2": "dsa-with-sha256",
+    "2.5.29.19": "basicConstraints",
 }
 
 
@@ -144,26 +145,36 @@ class Name(object):
 OID_BASIC_CONSTRAINTS = ObjectIdentifier("2.5.29.19")
 
 
-@six.add_metaclass(abc.ABCMeta)
 class Extension(object):
-    @abc.abstractproperty
-    def critical(self):
-        """
-        Returns the boolean value of the critical extension field.
-        """
+    def __init__(self, oid, critical, value):
+        if not isinstance(oid, ObjectIdentifier):
+            raise TypeError(
+                "oid argument must be an ObjectIdentifier instance."
+            )
 
+        if not isinstance(critical, bool):
+            raise TypeError("critical must be a boolean value")
 
-@utils.register_interface(Extension)
-class BasicConstraints(object):
-    oid = OID_BASIC_CONSTRAINTS
+        self._oid = oid
+        self._critical = critical
+        self._value = value
 
-    def __init__(self, ca, path_length, critical):
+    oid = utils.read_only_property("_oid")
+    critical = utils.read_only_property("_critical")
+    value = utils.read_only_property("_value")
+
+    def __repr__(self):
+        return ("<Extension(oid={oid}, "
+                "critical={critical}, value={value})>").format(
+            oid=self.oid, critical=self.critical, value=self.value
+        )
+
+
+class BasicConstraints(object):
+    def __init__(self, ca, path_length):
         if not isinstance(ca, bool):
             raise TypeError("ca must be a boolean value")
 
-        if not isinstance(critical, bool):
-            raise TypeError("critical must be a boolean value")
-
         if path_length is not None and ca is False:
             raise ValueError("path_length must be None when ca is False")
 
@@ -175,17 +186,14 @@ class BasicConstraints(object):
 
         self._ca = ca
         self._path_length = path_length
-        self._critical = critical
 
     ca = utils.read_only_property("_ca")
     path_length = utils.read_only_property("_path_length")
-    critical = utils.read_only_property("_critical")
 
     def __repr__(self):
         return ("<BasicConstraints(ca={ca}, "
-                "path_length={path_length}, "
-                "critical={critical})>").format(
-            ca=self.ca, path_length=self.path_length, critical=self.critical
+                "path_length={path_length})>").format(
+            ca=self.ca, path_length=self.path_length
         )
 
 
diff --git a/tests/test_x509_ext.py b/tests/test_x509_ext.py
index 9fde1be1..de3ca312 100644
--- a/tests/test_x509_ext.py
+++ b/tests/test_x509_ext.py
@@ -9,32 +9,49 @@ import pytest
 from cryptography import x509
 
 
-class TestBasicConstraints(object):
-    def test_ca_not_boolean(self):
+class TestExtension(object):
+    def test_not_an_oid(self):
+        bc = x509.BasicConstraints(False, None)
         with pytest.raises(TypeError):
-            x509.BasicConstraints("notbool", None, False)
+            x509.Extension("notanoid", True, bc)
+
+    def test_critical_not_a_bool(self):
+        bc = x509.BasicConstraints(False, None)
+        with pytest.raises(TypeError):
+            x509.Extension(x509.OID_BASIC_CONSTRAINTS, "notabool", bc)
+
+    def test_repr(self):
+        bc = x509.BasicConstraints(False, None)
+        ext = x509.Extension(x509.OID_BASIC_CONSTRAINTS, True, bc)
+        assert repr(ext) == (
+            "<Extension(oid=<ObjectIdentifier(oid=2.5.29.19, name=basicConst"
+            "raints)>, critical=True, value=<BasicConstraints(ca=False, path"
+            "_length=None)>)>"
+        )
+
 
-    def test_critical_not_boolean(self):
+class TestBasicConstraints(object):
+    def test_ca_not_boolean(self):
         with pytest.raises(TypeError):
-            x509.BasicConstraints(False, None, "notbool")
+            x509.BasicConstraints("notbool", None)
 
     def test_path_length_not_ca(self):
         with pytest.raises(ValueError):
-            x509.BasicConstraints(False, 0, True)
+            x509.BasicConstraints(False, 0)
 
     def test_path_length_not_int(self):
         with pytest.raises(TypeError):
-            x509.BasicConstraints(True, 1.1, True)
+            x509.BasicConstraints(True, 1.1)
 
         with pytest.raises(TypeError):
-            x509.BasicConstraints(True, "notint", True)
+            x509.BasicConstraints(True, "notint")
 
     def test_path_length_negative(self):
         with pytest.raises(TypeError):
-            x509.BasicConstraints(True, -1, True)
+            x509.BasicConstraints(True, -1)
 
     def test_repr(self):
-        na = x509.BasicConstraints(True, None, True)
+        na = x509.BasicConstraints(True, None)
         assert repr(na) == (
-            "<BasicConstraints(ca=True, path_length=None, critical=True)>"
+            "<BasicConstraints(ca=True, path_length=None)>"
         )