Make extension instances work the same as other objects, with

respect to properties.  Before, properties were declared on
toplevel-property-extension extensions as if they were going
to be used in the normal way (as actual properties on instances
of the extension), but they were not used that way, and there
was some ugly hackage to make it work.  Despite the fact that
property instances were given during extension registration,
they were not used to typecheck, set defaults, etc on toplevel
property extension properties.

I changed how registration and object initialization works with
respect to properties associated with extensions.  Now,
extensions work the same as any other object and code is
cleaner.  Property instances associated with registered toplevel
extensions are used to enforce requirements like any other
object.

Added some unit tests specifically for property cleaning for
extensions.

Property order (for those contexts where it matters) is updated
to be spec-defined, toplevel extension, custom.

stix2/base.py

@@ -1,5 +1,6 @@
 """Base classes for type definitions in the STIX2 library."""
 
+import collections
 import collections.abc
 import copy
 import itertools
@@ -36,14 +37,32 @@ class _STIXBase(collections.abc.Mapping):
     """Base class for STIX object types"""
 
     def object_properties(self):
-        props = set(self._properties.keys())
+        """
-        custom_props = list(set(self._inner.keys()) - props)
+        Get a list of property names in a particular order: spec order for
-        custom_props.sort()
+        spec defined properties, followed by toplevel-property-extension
+        properties (any order), followed by custom properties (any order).
 
-        all_properties = list(self._properties.keys())
+        The returned list doesn't include only defined+extension properties,
-        all_properties.extend(custom_props)  # Any custom properties to the bottom
+        nor does it include only assigned properties (i.e. those this object
+        actually possesses).  It's a mix of both: the spec defined property
+        group and extension group include all of them, regardless of whether
+        they're present on this object; the custom group include only names of
+        properties present on this object.
 
-        return all_properties
+        :return: A list of property names
+        """
+        if self.__property_order is None:
+            custom_props = sorted(
+                self.keys() - self._properties.keys()
+                - self.__ext_property_names
+            )
+
+            # Any custom properties to the bottom
+            self.__property_order = list(self._properties) \
+                + list(self.__ext_property_names) \
+                + custom_props
+
+        return self.__property_order
 
     def _check_property(self, prop_name, prop, kwargs, allow_custom):
         if prop_name not in kwargs:
@@ -131,46 +150,45 @@ class _STIXBase(collections.abc.Mapping):
         if custom_props and not isinstance(custom_props, dict):
             raise ValueError("'custom_properties' must be a dictionary")
 
-        # Detect any keyword arguments not allowed for a specific type.
+        # Detect any keyword arguments representing customization.
         # In STIX 2.1, this is complicated by "toplevel-property-extension"
         # type extensions, which can add extra properties which are *not*
         # considered custom.
-        extra_kwargs = kwargs.keys() - self._properties.keys()
-
         extensions = kwargs.get("extensions")
+        registered_toplevel_extension_props = {}
+        has_unregistered_toplevel_extension = False
         if extensions:
-            has_unregistered_toplevel_extension = False
-            registered_toplevel_extension_props = set()
-
             for ext_id, ext in extensions.items():
                 if ext.get("extension_type") == "toplevel-property-extension":
                     registered_ext_class = class_for_type(
                         ext_id, "2.1", "extensions",
                     )
                     if registered_ext_class:
-                        registered_toplevel_extension_props |= \
+                        registered_toplevel_extension_props.update(
-                            registered_ext_class._properties.keys()
+                            registered_ext_class._toplevel_properties
+                        )
                     else:
                         has_unregistered_toplevel_extension = True
 
-            if has_unregistered_toplevel_extension:
+        if has_unregistered_toplevel_extension:
-                # Must assume all extras are extension properties, not custom.
+            # Must assume all extras are extension properties, not custom.
-                extra_kwargs.clear()
+            custom_kwargs = set()
 
-            else:
+        else:
-                # All toplevel property extensions (if any) have been
+            # All toplevel property extensions (if any) have been
-                # registered.  So we can tell what their properties are and
+            # registered.  So we can tell what their properties are and
-                # treat only those as not custom.
+            # treat only those as not custom.
-                extra_kwargs -= registered_toplevel_extension_props
+            custom_kwargs = kwargs.keys() - self._properties.keys() \
+                - registered_toplevel_extension_props.keys()
 
-        if extra_kwargs and not allow_custom:
+        if custom_kwargs and not allow_custom:
-            raise ExtraPropertiesError(cls, extra_kwargs)
+            raise ExtraPropertiesError(cls, custom_kwargs)
 
         if custom_props:
             # loophole for custom_properties...
             allow_custom = True
 
-        all_custom_prop_names = (extra_kwargs | custom_props.keys()) - \
+        all_custom_prop_names = (custom_kwargs | custom_props.keys()) - \
             self._properties.keys()
         if all_custom_prop_names:
             if not isinstance(self, stix2.v20._STIXBase20):
@@ -181,6 +199,21 @@ class _STIXBase(collections.abc.Mapping):
                             reason="Property name '%s' must begin with an alpha character." % prop_name,
                         )
 
+        # defined_properties = all properties defined on this type, plus all
+        # properties defined on this instance as a result of toplevel property
+        # extensions.
+        defined_properties = collections.ChainMap(
+            self._properties, registered_toplevel_extension_props
+        )
+
+        # object_properties() needs this; cache it here to avoid needing to
+        # recompute.
+        self.__ext_property_names = set(registered_toplevel_extension_props)
+        # object_properties() will compute this on first call, based on
+        # __ext_property_names above.  Maybe it makes sense to not compute this
+        # unless really necessary.
+        self.__property_order = None
+
         # Remove any keyword arguments whose value is None or [] (i.e. empty list)
         setting_kwargs = {
             k: v
@@ -189,22 +222,15 @@ class _STIXBase(collections.abc.Mapping):
         }
 
         # Detect any missing required properties
-        required_properties = set(get_required_properties(self._properties))
+        required_properties = set(
-        missing_kwargs = required_properties - set(setting_kwargs)
+            get_required_properties(defined_properties)
+        )
+        missing_kwargs = required_properties - setting_kwargs.keys()
         if missing_kwargs:
-            # In this scenario, we are inside within the scope of the extension.
+            raise MissingPropertiesError(cls, missing_kwargs)
-            # It is possible to check if this is a new Extension Class by
-            # querying "extension_type". Note: There is an API limitation currently
-            # because a toplevel-property-extension cannot validate its parent properties
-            new_ext_check = (
-                bool(getattr(self, "extension_type", None))
-                and issubclass(cls, stix2.v21._Extension)
-            )
-            if new_ext_check is False:
-                raise MissingPropertiesError(cls, missing_kwargs)
 
         has_custom = bool(all_custom_prop_names)
-        for prop_name, prop_metadata in self._properties.items():
+        for prop_name, prop_metadata in defined_properties.items():
             temp_custom = self._check_property(
                 prop_name, prop_metadata, setting_kwargs, allow_custom,
             )
@@ -213,7 +239,7 @@ class _STIXBase(collections.abc.Mapping):
 
         # Cache defaulted optional properties for serialization
         defaulted = []
-        for name, prop in self._properties.items():
+        for name, prop in defined_properties.items():
             try:
                 if (
                     not prop.required and not hasattr(prop, '_fixed_value') and

stix2/custom.py

@@ -1,6 +1,7 @@
 from collections import OrderedDict
 
 from .base import _cls_init
+from .properties import EnumProperty
 from .registration import (
     _get_extension_class, _register_extension, _register_marking,
     _register_object, _register_observable,
@@ -93,12 +94,46 @@ def _custom_observable_builder(cls, type, properties, version, base_class, id_co
 
 
 def _custom_extension_builder(cls, type, properties, version, base_class):
-    prop_dict = _get_properties_dict(properties)
+
+    properties = _get_properties_dict(properties)
+    toplevel_properties = None
+
+    # Auto-create an "extension_type" property from the class attribute, if
+    # it exists.  How to treat the other properties which were given depends on
+    # the extension type.
+    extension_type = getattr(cls, "extension_type", None)
+    if extension_type:
+        # I suppose I could also go with a plain string property, since the
+        # value is fixed... but an enum property seems more true to the
+        # property's semantics.  Also, I can't import a vocab module for the
+        # enum values without circular import errors. :(
+        extension_type_prop = EnumProperty(
+            [
+                "new-sdo", "new-sco", "new-sro", "property-extension",
+                "toplevel-property-extension"
+            ],
+            required=False,
+            fixed=extension_type,
+        )
+
+        nested_properties = {
+            "extension_type": extension_type_prop
+        }
+
+        if extension_type == "toplevel-property-extension":
+            toplevel_properties = properties
+        else:
+            nested_properties.update(properties)
+
+    else:
+        nested_properties = properties
 
     class _CustomExtension(cls, base_class):
 
         _type = type
-        _properties = prop_dict
+        _properties = nested_properties
+        if extension_type == "toplevel-property-extension":
+            _toplevel_properties = toplevel_properties
 
         def __init__(self, **kwargs):
             base_class.__init__(self, **kwargs)

stix2/test/v21/test_custom.py

@@ -1,3 +1,4 @@
+import contextlib
 import uuid
 
 import pytest
@@ -8,7 +9,9 @@ import stix2.registration
 import stix2.registry
 import stix2.v21
 
-from ...exceptions import DuplicateRegistrationError, InvalidValueError
+from ...exceptions import (
+    DuplicateRegistrationError, InvalidValueError, MissingPropertiesError
+)
 from .constants import FAKE_TIME, IDENTITY_ID, MARKING_DEFINITION_ID
 
 # Custom Properties in SDOs
@@ -1675,6 +1678,194 @@ def test_registered_new_extension_marking_allow_custom_false():
            '{"extension_type": "property-extension", "some_marking_field": "value"}}' in marking_serialized
 
 
+@contextlib.contextmanager
+def _register_extension(ext, props):
+
+    ext_def_id = "extension-definition--" + str(uuid.uuid4())
+
+    stix2.v21.CustomExtension(
+        ext_def_id,
+        props
+    )(ext)
+
+    try:
+        yield ext_def_id
+    finally:
+        # "unregister" the extension
+        del stix2.registry.STIX2_OBJ_MAPS["2.1"]["extensions"][ext_def_id]
+
+
+def test_nested_ext_prop_meta():
+
+    class TestExt:
+        extension_type = "property-extension"
+
+    props = {
+        "intprop": stix2.properties.IntegerProperty(required=True),
+        "strprop": stix2.properties.StringProperty(
+            required=False, default=lambda: "foo"
+        )
+    }
+
+    with _register_extension(TestExt, props) as ext_def_id:
+
+        obj = stix2.v21.Identity(
+            name="test",
+            extensions={
+                ext_def_id: {
+                    "extension_type": "property-extension",
+                    "intprop": "1",
+                    "strprop": 2
+                }
+            }
+        )
+
+        assert obj.extensions[ext_def_id].extension_type == "property-extension"
+        assert obj.extensions[ext_def_id].intprop == 1
+        assert obj.extensions[ext_def_id].strprop == "2"
+
+        obj = stix2.v21.Identity(
+            name="test",
+            extensions={
+                ext_def_id: {
+                    "extension_type": "property-extension",
+                    "intprop": "1",
+                }
+            }
+        )
+
+        # Ensure default kicked in
+        assert obj.extensions[ext_def_id].strprop == "foo"
+
+        with pytest.raises(InvalidValueError):
+            stix2.v21.Identity(
+                name="test",
+                extensions={
+                    ext_def_id: {
+                        "extension_type": "property-extension",
+                        # wrong value type
+                        "intprop": "foo"
+                    }
+                }
+            )
+
+        with pytest.raises(InvalidValueError):
+            stix2.v21.Identity(
+                name="test",
+                extensions={
+                    ext_def_id: {
+                        "extension_type": "property-extension",
+                        # missing required property
+                        "strprop": "foo"
+                    }
+                }
+            )
+
+        with pytest.raises(InvalidValueError):
+            stix2.v21.Identity(
+                name="test",
+                extensions={
+                    ext_def_id: {
+                        "extension_type": "property-extension",
+                        "intprop": 1,
+                        # Use of undefined property
+                        "foo": False,
+                    }
+                }
+            )
+
+        with pytest.raises(InvalidValueError):
+            stix2.v21.Identity(
+                name="test",
+                extensions={
+                    ext_def_id: {
+                        # extension_type doesn't match with registration
+                        "extension_type": "new-sdo",
+                        "intprop": 1,
+                        "strprop": "foo",
+                    }
+                }
+            )
+
+
+def test_toplevel_ext_prop_meta():
+
+    class TestExt:
+        extension_type = "toplevel-property-extension"
+
+    props = {
+        "intprop": stix2.properties.IntegerProperty(required=True),
+        "strprop": stix2.properties.StringProperty(
+            required=False, default=lambda: "foo"
+        )
+    }
+
+    with _register_extension(TestExt, props) as ext_def_id:
+
+        obj = stix2.v21.Identity(
+            name="test",
+            intprop="1",
+            strprop=2,
+            extensions={
+                ext_def_id: {
+                    "extension_type": "toplevel-property-extension"
+                }
+            }
+        )
+
+        assert obj.extensions[ext_def_id].extension_type == "toplevel-property-extension"
+        assert obj.intprop == 1
+        assert obj.strprop == "2"
+
+        obj = stix2.v21.Identity(
+            name="test",
+            intprop=1,
+            extensions={
+                ext_def_id: {
+                    "extension_type": "toplevel-property-extension"
+                }
+            }
+        )
+
+        # Ensure default kicked in
+        assert obj.strprop == "foo"
+
+        with pytest.raises(InvalidValueError):
+            stix2.v21.Identity(
+                name="test",
+                intprop="foo",  # wrong value type
+                extensions={
+                    ext_def_id: {
+                        "extension_type": "toplevel-property-extension"
+                    }
+                }
+            )
+
+        with pytest.raises(InvalidValueError):
+            stix2.v21.Identity(
+                name="test",
+                intprop=1,
+                extensions={
+                    ext_def_id: {
+                        "extension_type": "toplevel-property-extension",
+                        # Use of undefined property
+                        "foo": False,
+                    }
+                }
+            )
+
+        with pytest.raises(MissingPropertiesError):
+            stix2.v21.Identity(
+                name="test",
+                strprop="foo",  # missing required property
+                extensions={
+                    ext_def_id: {
+                        "extension_type": "toplevel-property-extension"
+                    }
+                }
+            )
+
+
 def test_allow_custom_propagation():
     obj_dict = {
         "type": "bundle",

stix2/v21/common.py

@@ -1,7 +1,6 @@
 """STIX 2.1 Common Data Types and Properties."""
 
 from collections import OrderedDict
-from collections.abc import Mapping
 
 from . import _Extension
 from ..custom import _custom_marking_builder, _custom_extension_builder
@@ -145,23 +144,8 @@ def CustomExtension(type='x-custom-ext', properties=None):
     """Custom STIX Object Extension decorator.
     """
     def wrapper(cls):
-
-        # Auto-create an "extension_type" property from the class attribute, if
-        # it exists.
-        extension_type = getattr(cls, "extension_type", None)
-        if extension_type:
-            extension_type_prop = EnumProperty(
-                EXTENSION_TYPE,
-                required=False,
-                fixed=extension_type,
-            )
-
-            if isinstance(properties, Mapping):
-                properties["extension_type"] = extension_type_prop
-            else:
-                properties.append(("extension_type", extension_type_prop))
-
         return _custom_extension_builder(cls, type, properties, '2.1', _Extension)
+
     return wrapper
 
 

stix2/v21/observables.py

@@ -889,7 +889,7 @@ def CustomObservable(type='x-custom-observable', properties=None, id_contrib_pro
             ),
         )
         if extension_name:
-            @CustomExtension(type=extension_name, properties=properties)
+            @CustomExtension(type=extension_name, properties={})
             class NameExtension:
                 extension_type = 'new-sco'
 
@@ -899,5 +899,3 @@ def CustomObservable(type='x-custom-observable', properties=None, id_contrib_pro
             cls.with_extension = extension_name
         return _custom_observable_builder(cls, type, _properties, '2.1', _Observable, id_contrib_props)
     return wrapper
-
-

stix2/v21/sdo.py

@@ -857,7 +857,7 @@ def CustomObject(type='x-custom-type', properties=None, extension_name=None, is_
         )
 
         if extension_name:
-            @CustomExtension(type=extension_name, properties=extension_properties)
+            @CustomExtension(type=extension_name, properties={})
             class NameExtension:
                 if is_sdo:
                     extension_type = 'new-sdo'