Repository: oasis-open/cti-python-stix2
Pull request #513 (2021-06-25 00:33:32)

improv: Check reference prop types for all customs
Previously the requirement that properties ending in `_ref(s)` were
instances of an appropriate type capable of containing a STIX
`identifier` (the current interpretation of section 3.1 of the spec) was
only enforced for custom observables. This change refactors the property
checks for custom objects to enforce this and the STIX 2.1 property name
requirement (also from section 3.1) in a common helper, therefore
extending the enforcement of both requirements to all custom object
types created by downstream projects.

ref: #511 

codecov-commenter: # [Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open) Report
> Merging [#513](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open) (4735cab) into [master](https://codecov.io/gh/oasis-open/cti-python-stix2/commit/e9d417de2592c0c7367c312ca0fd25dc8f8a9818?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open) (e9d417d) will **decrease** coverage by `0.03%`.
> The diff coverage is `78.78%`.

[![Impacted file tree graph](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513/graphs/tree.svg?width=650&height=150&src=pr&token=vzHZ2LBVZq&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open)](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open)

```diff
@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
- Coverage   87.12%   87.09%   -0.04%     
==========================================
  Files         151      151              
  Lines       17702    17697       -5     
==========================================
- Hits        15423    15413      -10     
- Misses       2279     2284       +5     
```


| [Impacted Files](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open) | Coverage Δ | |
|---|---|---|
| [stix2/registration.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open#diff-c3RpeDIvcmVnaXN0cmF0aW9uLnB5) | `73.75% <74.07%> (-7.43%)` | :arrow_down: |
| [stix2/test/v20/test\_custom.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open#diff-c3RpeDIvdGVzdC92MjAvdGVzdF9jdXN0b20ucHk=) | `100.00% <100.00%> (ø)` | |
| [stix2/test/v21/test\_custom.py](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open#diff-c3RpeDIvdGVzdC92MjEvdGVzdF9jdXN0b20ucHk=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open). Last update [e9d417d...4735cab](https://codecov.io/gh/oasis-open/cti-python-stix2/pull/513?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oasis-open).

maybe-sybr: Lint should be gone now
maybe-sybr: I've rebased this on top of master now that #468 is in to resolve the conflict in `_register_extension`. Everything appears to still be behaving as expected.
maybe-sybr: I've de-drafted this and fixed/removed the todos (the threads above can still be responded to though) to make life easier if you're satisfied with merging this prior to releasing 3.0.0.

I'd like to implement one of the options for adding extra valid reference types, and I have an example  of the ABCMeta approach on my staging branch at maybe-sybr/cti-python-stix2@a8fe790 , but if I have to keep a staging branch on top of 3.0.0 just for that one change, I could live with that :)
maybe-sybr: @rpiazza I've rebased this on top of v3.0.0 and resolved a conflict with the new top level property validation done for custom extensions. The tests pass but I'll make a quick comment on the diff for @chisholm to sanity check to make sure I didn't mess the expected behaviour.
chisholm: Overall, I think doing the ref property checking consistently makes sense.  It didn't make much sense to do it only for observables.  And I definitely like getting rid of the mro lookups!  I always thought those were ugly.  It's.... approximately correct I think, but not exactly.  E.g. if you registered a custom 2.0 SDO but used `ObjectReferenceProperty` for its ref properties, I think it would pass the checks.  But it shouldn't, since that property class is intended only for 2.0 SCO ref properties.
maybe-sybr: > Overall, I think doing the ref property checking consistently makes sense. It didn't make much sense to do it only for observables. And I definitely like getting rid of the mro lookups! I always thought those were ugly.

Great, hopefully with the context around my use case for `ReferenceProperty` variants as well, my suggestions in https://github.com/oasis-open/cti-python-stix2/pull/513#pullrequestreview-697757608 will make some more sense. LMK what you think.

> It's.... approximately correct I think, but not exactly. E.g. if you registered a custom 2.0 SDO but used `ObjectReferenceProperty` for its ref properties, I think it would pass the checks. But it shouldn't, since that property class is intended only for 2.0 SCO ref properties.

I've just had a quick skim through the 2.0 spec (specifically part 3 and 4) and I think I understand how this has evolved now. I suppose the most correct thing would be to have a a nested mapping like `VALID_REF_TYPES[stix_version][stix_object_type]`, but that does sound pretty clunky for a single case. Alternatively, we could allow this logic to stand since I assume the 2.0 code will be relatively static in future, and the only risk is for downstream authors doing the "wrong thing". Happy to make any changes at your suggestion.
maybe-sybr: Just marking this as a draft while it needs autosquashing.
chisholm: > > It's.... approximately correct I think, but not exactly. E.g. if you registered a custom 2.0 SDO but used `ObjectReferenceProperty` for its ref properties, I think it would pass the checks. But it shouldn't, since that property class is intended only for 2.0 SCO ref properties.
> 
> I've just had a quick skim through the 2.0 spec (specifically part 3 and 4) and I think I understand how this has evolved now. I suppose the most correct thing would be to have a a nested mapping like `VALID_REF_TYPES[stix_version][stix_object_type]`, but that does sound pretty clunky for a single case. Alternatively, we could allow this logic to stand since I assume the 2.0 code will be relatively static in future, and the only risk is for downstream authors doing the "wrong thing". Happy to make any changes at your suggestion.

Nah, I think treatment would only need to depend on the "class" of object, e.g. SDO vs SCO.  So it would only need to be (notionally) `VALID_REF_TYPES[stix_version][stix_type_class]`.  And maybe as simple as `VALID_REF_TYPES[2.0 SCOs]` and `VALID_REF_TYPES[everything else]`.

I don't know how important it is now to be fanatically correct w.r.t. STIX 2.0, but this *is* a reference implementation... :)
chisholm: If we're going to do this, let's just do it right.  It should be easy to adapt your code to:
- Check for `ObjectReferenceProperty` for 2.0 SCOs
- Check for `ReferenceProperty` for all other objects

Then ensure there are unit tests to:
- ensure a 2.0 SCO with `ReferenceProperty` errors
- ensure a 2.0 non-SCO with `ObjectReferenceProperty` errors
- ensure a 2.1 object errors with `ObjectReferenceProperty`


maybe-sybr: Check out the new diff, @chisholm . I've decided against having a more complex mapping since the only special case we have to deal with is 2.0 observables and it's much more obvious to codify that with a single conditional in the reference property validation helper. I imagine that future iterations of the 2.X spec will aim to maintain coherence for all properties that appear to be references, so building infrastructure to support future deviations is just an exercise in complexity for its own sake. That should be fairly easy to pivot back to in future if the need arises (or in this PR if that's your preference, just LMK).

There are tests for custom objects and observables to ensure that reference vs object reference properties are accepted and rejected appropriately. I don't do any testing for acceptance of subclasses, LMK if you'd like to see that. Since supporting that was the point of #511, I don't mind putting it in, but since the use case is now more dubiously useful, I could also see it being valid to omit them.

Patch:
diff --git a/stix2/registration.py b/stix2/registration.py
index 9fb2f49..85c6e9a 100644
--- a/stix2/registration.py
+++ b/stix2/registration.py
@@ -1,11 +1,71 @@
-import itertools
 import re
 
 from . import registry, version
 from .base import _DomainObject
 from .exceptions import DuplicateRegistrationError
-from .properties import _validate_type
-from .utils import PREFIX_21_REGEX, get_class_hierarchy_names
+from .properties import (
+    ListProperty, ObjectReferenceProperty, ReferenceProperty, _validate_type,
+)
+from .utils import PREFIX_21_REGEX
+
+
+def _validate_ref_props(props_map, is_observable20=False):
+    """
+    Validate that reference properties contain an expected type.
+
+    Properties ending in "_ref/s" need to be instances of specific types to
+    meet the specification requirements. For 2.0 and 2.1 conformance, these
+    properties are expected to be implemented with `ReferenceProperty` (or a
+    subclass thereof), except for the special case of STIX 2.0 observables
+    which must be implemented with `ObjectReferenceProperty`.
+
+    Args:
+        props_map (mapping): A mapping of STIX object properties to be checked.
+        is_observable20 (bool): Flag for the STIX 2.0 observables special case.
+
+    Raises:
+        ValueError: If the properties do not conform.
+    """
+    if is_observable20:
+        ref_prop_type = ObjectReferenceProperty
+    else:
+        ref_prop_type = ReferenceProperty
+    for prop_name, prop_obj in props_map.items():
+        tail = prop_name.rsplit("_", 1)[-1]
+        if tail == "ref" and not isinstance(prop_obj, ref_prop_type):
+            raise ValueError(
+                f"{prop_name!r} is named like a reference property but is not "
+                f"a subclass of {ref_prop_type.__name__!r}.",
+            )
+        elif tail == "refs" and not (
+            isinstance(prop_obj, ListProperty)
+            and isinstance(prop_obj.contained, ref_prop_type)
+        ):
+            raise ValueError(
+                f"{prop_name!r} is named like a reference list property but is not "
+                f"a 'ListProperty' containing a subclass of {ref_prop_type.__name__!r}.",
+            )
+
+
+def _validate_props(props_map, version, **kwargs):
+    """
+    Validate that a map of properties is conformant for this STIX `version`.
+
+    Args:
+        props_map (mapping): A mapping of STIX object properties to be checked.
+        version (str): Which STIX2 version the properties must confirm to.
+        kwargs (mapping): Arguments to pass on to specific property validators.
+
+    Raises:
+        ValueError: If the properties do not conform.
+    """
+    # Confirm conformance with STIX 2.1+ requirements for property names
+    if version != "2.0":
+        for prop_name, prop_value in props_map.items():
+            if not re.match(PREFIX_21_REGEX, prop_name):
+                raise ValueError("Property name '%s' must begin with an alpha character." % prop_name)
+    # Confirm conformance of reference properties
+    _validate_ref_props(props_map, **kwargs)
 
 
 def _register_object(new_type, version=version.DEFAULT_VERSION):
@@ -29,15 +89,10 @@ def _register_object(new_type, version=version.DEFAULT_VERSION):
             new_type.__name__,
         )
 
-    properties = new_type._properties
-
     if not version:
         version = version.DEFAULT_VERSION
 
-    if version == "2.1":
-        for prop_name, prop in properties.items():
-            if not re.match(PREFIX_21_REGEX, prop_name):
-                raise ValueError("Property name '%s' must begin with an alpha character" % prop_name)
+    _validate_props(new_type._properties, version)
 
     OBJ_MAP = registry.STIX2_OBJ_MAPS[version]['objects']
     if new_type._type in OBJ_MAP.keys():
@@ -54,19 +109,12 @@ def _register_marking(new_marking, version=version.DEFAULT_VERSION):
             None, use latest version.
 
     """
-
-    mark_type = new_marking._type
-    properties = new_marking._properties
-
     if not version:
         version = version.DEFAULT_VERSION
 
+    mark_type = new_marking._type
     _validate_type(mark_type, version)
-
-    if version == "2.1":
-        for prop_name, prop_value in properties.items():
-            if not re.match(PREFIX_21_REGEX, prop_name):
-                raise ValueError("Property name '%s' must begin with an alpha character." % prop_name)
+    _validate_props(new_marking._properties, version)
 
     OBJ_MAP_MARKING = registry.STIX2_OBJ_MAPS[version]['markings']
     if mark_type in OBJ_MAP_MARKING.keys():
@@ -83,49 +131,13 @@ def _register_observable(new_observable, version=version.DEFAULT_VERSION):
             None, use latest version.
 
     """
-    properties = new_observable._properties
-
     if not version:
         version = version.DEFAULT_VERSION
 
-    if version == "2.0":
-        # If using STIX2.0, check properties ending in "_ref/s" are ObjectReferenceProperties
-        for prop_name, prop in properties.items():
-            if prop_name.endswith('_ref') and ('ObjectReferenceProperty' not in get_class_hierarchy_names(prop)):
-                raise ValueError(
-                    "'%s' is named like an object reference property but "
-                    "is not an ObjectReferenceProperty." % prop_name,
-                )
-            elif (
-                prop_name.endswith('_refs') and (
-                    'ListProperty' not in get_class_hierarchy_names(prop) or
-                    'ObjectReferenceProperty' not in get_class_hierarchy_names(prop.contained)
-                )
-            ):
-                raise ValueError(
-                    "'%s' is named like an object reference list property but "
-                    "is not a ListProperty containing ObjectReferenceProperty." % prop_name,
-                )
-    else:
-        # If using STIX2.1 (or newer...), check properties ending in "_ref/s" are ReferenceProperties
-        for prop_name, prop in properties.items():
-            if not re.match(PREFIX_21_REGEX, prop_name):
-                raise ValueError("Property name '%s' must begin with an alpha character." % prop_name)
-            elif prop_name.endswith('_ref') and ('ReferenceProperty' not in get_class_hierarchy_names(prop)):
-                raise ValueError(
-                    "'%s' is named like a reference property but "
-                    "is not a ReferenceProperty." % prop_name,
-                )
-            elif (
-                prop_name.endswith('_refs') and (
-                    'ListProperty' not in get_class_hierarchy_names(prop) or
-                    'ReferenceProperty' not in get_class_hierarchy_names(prop.contained)
-                )
-            ):
-                raise ValueError(
-                    "'%s' is named like a reference list property but "
-                    "is not a ListProperty containing ReferenceProperty." % prop_name,
-                )
+    _validate_props(
+        new_observable._properties, version,
+        is_observable20=(version == "2.0"),
+    )
 
     OBJ_MAP_OBSERVABLE = registry.STIX2_OBJ_MAPS[version]['observables']
     if new_observable._type in OBJ_MAP_OBSERVABLE.keys():
@@ -146,20 +158,7 @@ def _register_extension(
     """
     ext_type = new_extension._type
 
-    # Need to check both toplevel and nested properties
-    prop_groups = [new_extension._properties]
-    if hasattr(new_extension, "_toplevel_properties"):
-        prop_groups.append(new_extension._toplevel_properties)
-    prop_names = itertools.chain.from_iterable(prop_groups)
-
     _validate_type(ext_type, version)
-
-    if not new_extension._properties:
-        raise ValueError(
-            "Invalid extension: must define at least one property: " +
-            ext_type,
-        )
-
     if version == "2.1":
         if not (ext_type.endswith('-ext') or ext_type.startswith('extension-definition--')):
             raise ValueError(
@@ -167,9 +166,23 @@ def _register_extension(
                 ext_type,
             )
 
-        for prop_name in prop_names:
-            if not re.match(PREFIX_21_REGEX, prop_name):
-                raise ValueError("Property name '%s' must begin with an alpha character." % prop_name)
+    tl_props = getattr(new_extension, "_toplevel_properties", None)
+    if any((
+        # There must always be at least one property in an extension. This
+        # holds for instances of both custom object extensions which must
+        # contain one or more custom properties, and `extension-definition`s
+        # which must contain an `extension_type` property.
+        not new_extension._properties,
+        # If a top-level properties mapping is provided, it cannot be empty
+        tl_props is not None and not tl_props,
+    )):
+        raise ValueError(
+            "Invalid extension: must define at least one property: " +
+            ext_type,
+        )
+    # We need to validate all properties related to this extension
+    combined_props = dict(new_extension._properties, **(tl_props or dict()))
+    _validate_props(combined_props, version)
 
     EXT_MAP = registry.STIX2_OBJ_MAPS[version]['extensions']
 


Test patch:
diff --git a/stix2/test/v20/test_custom.py b/stix2/test/v20/test_custom.py
index 4647167..3264af9 100644
--- a/stix2/test/v20/test_custom.py
+++ b/stix2/test/v20/test_custom.py
@@ -385,6 +385,84 @@ def test_custom_object_invalid_type_name():
     assert "Invalid type name 'x_new_object':" in str(excinfo.value)
 
 
+def test_custom_object_ref_property_as_identifier():
+    @stix2.v20.CustomObject(
+        'x-new-obj-with-ref', [
+            ('property_ref', stix2.properties.ReferenceProperty(invalid_types=[])),
+        ],
+    )
+    class NewObs():
+        pass
+
+
+def test_custom_object_refs_property_containing_identifiers():
+    @stix2.v20.CustomObject(
+        'x-new-obj-with-refs', [
+            ('property_refs', stix2.properties.ListProperty(stix2.properties.ReferenceProperty(invalid_types=[]))),
+        ],
+    )
+    class NewObs():
+        pass
+
+
+def test_custom_object_ref_property_as_objectref():
+    with pytest.raises(ValueError, match=r"not a subclass of 'ReferenceProperty"):
+        @stix2.v20.CustomObject(
+            'x-new-obj-with-objref', [
+                ('property_ref', stix2.properties.ObjectReferenceProperty()),
+            ],
+        )
+        class NewObs():
+            pass
+
+
+def test_custom_object_refs_property_containing_objectrefs():
+    with pytest.raises(ValueError, match=r"not a 'ListProperty' containing a subclass of 'ReferenceProperty"):
+        @stix2.v20.CustomObject(
+            'x-new-obj-with-objrefs', [
+                ('property_refs', stix2.properties.ListProperty(stix2.properties.ObjectReferenceProperty())),
+            ],
+        )
+        class NewObs():
+            pass
+
+
+def test_custom_object_invalid_ref_property():
+    with pytest.raises(ValueError) as excinfo:
+        @stix2.v20.CustomObject(
+            'x-new-obj', [
+                ('property_ref', stix2.properties.StringProperty()),
+            ],
+        )
+        class NewObs():
+            pass
+    assert "is named like a reference property but is not" in str(excinfo.value)
+
+
+def test_custom_object_invalid_refs_property():
+    with pytest.raises(ValueError) as excinfo:
+        @stix2.v20.CustomObject(
+            'x-new-obj', [
+                ('property_refs', stix2.properties.StringProperty()),
+            ],
+        )
+        class NewObs():
+            pass
+    assert "is named like a reference list property but is not" in str(excinfo.value)
+
+
+def test_custom_object_invalid_refs_list_property():
+    with pytest.raises(ValueError) as excinfo:
+        @stix2.v20.CustomObject(
+            'x-new-obj', [
+                ('property_refs', stix2.properties.ListProperty(stix2.properties.StringProperty)),
+            ],
+        )
+        class NewObs():
+            pass
+    assert "is named like a reference list property but is not" in str(excinfo.value)
+
+
 def test_custom_subobject_dict():
     obj_dict = {
         "type": "bundle",
@@ -562,6 +640,48 @@ def test_custom_observable_object_invalid_type_name():
     assert "Invalid type name 'x_new_obs':" in str(excinfo.value)
 
 
+def test_custom_observable_object_ref_property_as_identifier():
+    with pytest.raises(ValueError, match=r"not a subclass of 'ObjectReferenceProperty"):
+        @stix2.v20.CustomObservable(
+            'x-new-obs-with-ref', [
+                ('property_ref', stix2.properties.ReferenceProperty(invalid_types=[])),
+            ],
+        )
+        class NewObs():
+            pass
+
+
+def test_custom_observable_object_refs_property_containing_identifiers():
+    with pytest.raises(ValueError, match=r"not a 'ListProperty' containing a subclass of 'ObjectReferenceProperty"):
+        @stix2.v20.CustomObservable(
+            'x-new-obs-with-refs', [
+                ('property_refs', stix2.properties.ListProperty(stix2.properties.ReferenceProperty(invalid_types=[]))),
+            ],
+        )
+        class NewObs():
+            pass
+
+
+def test_custom_observable_object_ref_property_as_objectref():
+    @stix2.v20.CustomObservable(
+        'x-new-obs-with-objref', [
+            ('property_ref', stix2.properties.ObjectReferenceProperty()),
+        ],
+    )
+    class NewObs():
+        pass
+
+
+def test_custom_observable_object_refs_property_containing_objectrefs():
+    @stix2.v20.CustomObservable(
+        'x-new-obs-with-objrefs', [
+            ('property_refs', stix2.properties.ListProperty(stix2.properties.ObjectReferenceProperty())),
+        ],
+    )
+    class NewObs():
+        pass
+
+
 def test_custom_observable_object_invalid_ref_property():
     with pytest.raises(ValueError) as excinfo:
         @stix2.v20.CustomObservable(
@@ -571,7 +691,7 @@ def test_custom_observable_object_invalid_ref_property():
         )
         class NewObs():
             pass
-    assert "is named like an object reference property but is not an ObjectReferenceProperty" in str(excinfo.value)
+    assert "is named like a reference property but is not" in str(excinfo.value)
 
 
 def test_custom_observable_object_invalid_refs_property():
@@ -583,7 +703,7 @@ def test_custom_observable_object_invalid_refs_property():
         )
         class NewObs():
             pass
-    assert "is named like an object reference list property but is not a ListProperty containing ObjectReferenceProperty" in str(excinfo.value)
+    assert "is named like a reference list property but is not" in str(excinfo.value)
 
 
 def test_custom_observable_object_invalid_refs_list_property():
@@ -595,7 +715,7 @@ def test_custom_observable_object_invalid_refs_list_property():
         )
         class NewObs():
             pass
-    assert "is named like an object reference list property but is not a ListProperty containing ObjectReferenceProperty" in str(excinfo.value)
+    assert "is named like a reference list property but is not" in str(excinfo.value)
 
 
 def test_custom_observable_object_invalid_valid_refs():
diff --git a/stix2/test/v21/test_custom.py b/stix2/test/v21/test_custom.py
index 13eeba9..03a8357 100644
--- a/stix2/test/v21/test_custom.py
+++ b/stix2/test/v21/test_custom.py
@@ -538,6 +538,84 @@ def test_custom_object_invalid_type_name():
     assert "Invalid type name '7x-new-object':" in str(excinfo.value)
 
 
+def test_custom_object_ref_property_containing_identifier():
+    @stix2.v21.CustomObject(
+        'x-new-obj-with-ref', [
+            ('property_ref', stix2.properties.ReferenceProperty(invalid_types=[])),
+        ],
+    )
+    class NewObs():
+        pass
+
+
+def test_custom_object_refs_property_containing_identifiers():
+    @stix2.v21.CustomObject(
+        'x-new-obj-with-refs', [
+            ('property_refs', stix2.properties.ListProperty(stix2.properties.ReferenceProperty(invalid_types=[]))),
+        ],
+    )
+    class NewObs():
+        pass
+
+
+def test_custom_object_ref_property_containing_objectref():
+    with pytest.raises(ValueError, match=r"not a subclass of 'ReferenceProperty"):
+        @stix2.v21.CustomObject(
+            'x-new-obj-with-objref', [
+                ('property_ref', stix2.properties.ObjectReferenceProperty()),
+            ],
+        )
+        class NewObs():
+            pass
+
+
+def test_custom_object_refs_property_containing_objectrefs():
+    with pytest.raises(ValueError, match=r"not a 'ListProperty' containing a subclass of 'ReferenceProperty"):
+        @stix2.v21.CustomObject(
+            'x-new-obj-with-objrefs', [
+                ('property_refs', stix2.properties.ListProperty(stix2.properties.ObjectReferenceProperty())),
+            ],
+        )
+        class NewObs():
+            pass
+
+
+def test_custom_object_invalid_ref_property():
+    with pytest.raises(ValueError) as excinfo:
+        @stix2.v21.CustomObject(
+            'x-new-obj', [
+                ('property_ref', stix2.properties.StringProperty()),
+            ],
+        )
+        class NewObs():
+            pass
+    assert "is named like a reference property but is not" in str(excinfo.value)
+
+
+def test_custom_object_invalid_refs_property():
+    with pytest.raises(ValueError) as excinfo:
+        @stix2.v21.CustomObject(
+            'x-new-obj', [
+                ('property_refs', stix2.properties.StringProperty()),
+            ],
+        )
+        class NewObs():
+            pass
+    assert "is named like a reference list property but is not" in str(excinfo.value)
+
+
+def test_custom_object_invalid_refs_list_property():
+    with pytest.raises(ValueError) as excinfo:
+        @stix2.v21.CustomObject(
+            'x-new-obj', [
+                ('property_refs', stix2.properties.ListProperty(stix2.properties.StringProperty)),
+            ],
+        )
+        class NewObs():
+            pass
+    assert "is named like a reference list property but is not" in str(excinfo.value)
+
+
 def test_custom_subobject_dict():
     obj_dict = {
         "type": "bundle",
@@ -730,6 +808,48 @@ def test_custom_observable_object_invalid_type_name():
     assert "Invalid type name '7x-new-obs':" in str(excinfo.value)
 
 
+def test_custom_observable_object_ref_property_as_identifier():
+    @stix2.v21.CustomObservable(
+        'x-new-obs-with-ref', [
+            ('property_ref', stix2.properties.ReferenceProperty(invalid_types=[])),
+        ],
+    )
+    class NewObs():
+        pass
+
+
+def test_custom_observable_object_refs_property_containing_identifiers():
+    @stix2.v21.CustomObservable(
+        'x-new-obs-with-refs', [
+            ('property_refs', stix2.properties.ListProperty(stix2.properties.ReferenceProperty(invalid_types=[]))),
+        ],
+    )
+    class NewObs():
+        pass
+
+
+def test_custom_observable_object_ref_property_as_objectref():
+    with pytest.raises(ValueError, match=r"not a subclass of 'ReferenceProperty"):
+        @stix2.v21.CustomObservable(
+            'x-new-obs-with-objref', [
+                ('property_ref', stix2.properties.ObjectReferenceProperty()),
+            ],
+        )
+        class NewObs():
+            pass
+
+
+def test_custom_observable_object_refs_property_containing_objectrefs():
+    with pytest.raises(ValueError, match=r"not a 'ListProperty' containing a subclass of 'ReferenceProperty"):
+        @stix2.v21.CustomObservable(
+            'x-new-obs-with-objrefs', [
+                ('property_refs', stix2.properties.ListProperty(stix2.properties.ObjectReferenceProperty())),
+            ],
+        )
+        class NewObs():
+            pass
+
+
 def test_custom_observable_object_invalid_ref_property():
     with pytest.raises(ValueError) as excinfo:
         @stix2.v21.CustomObservable(
@@ -739,7 +859,7 @@ def test_custom_observable_object_invalid_ref_property():
         )
         class NewObs():
             pass
-    assert "is named like a reference property but is not a ReferenceProperty" in str(excinfo.value)
+    assert "is named like a reference property but is not" in str(excinfo.value)
 
 
 def test_custom_observable_object_invalid_refs_property():
@@ -751,7 +871,7 @@ def test_custom_observable_object_invalid_refs_property():
         )
         class NewObs():
             pass
-    assert "is named like a reference list property but is not a ListProperty containing ReferenceProperty" in str(excinfo.value)
+    assert "is named like a reference list property but is not" in str(excinfo.value)
 
 
 def test_custom_observable_object_invalid_refs_list_property():
@@ -763,7 +883,7 @@ def test_custom_observable_object_invalid_refs_list_property():
         )
         class NewObs():
             pass
-    assert "is named like a reference list property but is not a ListProperty containing ReferenceProperty" in str(excinfo.value)
+    assert "is named like a reference list property but is not" in str(excinfo.value)
 
 
 def test_custom_no_properties_raises_exception():