netcmd: domain: claims: move claim value type lookup by attribute to model

Also, there was no need for the cached property previously in the
command, as the command only calls this once.

Fetching all value types seems excessive now with the new model layer,
we just fetch the one we need and get a model object back.

Use the method lookup, it's consistent with the rest, and raise either
LookupError or ValueError.

Signed-off-by: Rob van der Linde <rob@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>

diff --git a/python/samba/netcmd/domain/claim/claim_type.py b/python/samba/netcmd/domain/claim/claim_type.py
index d48ae86a9827a2c52c0e2001cff6672d022f8772..31aa11be24844cf38b90e249fb74622ea26b679e 100644 (file)
--- a/python/samba/netcmd/domain/claim/claim_type.py
+++ b/python/samba/netcmd/domain/claim/claim_type.py
@@ -31,18 +31,6 @@ from samba.netcmd.domain.models import AttributeSchema, ClassSchema,\
 
 from .base import ClaimCommand
 
-# LDAP Syntax to Claim Type CN lookup table.
-# These are the ones actively used by AD claim type attributes.
-SYNTAX_TO_CLAIM_TYPE_CN = {
-    "2.5.5.1": "MS-DS-Text",     # Object(DS-DN)
-    "2.5.5.2": "MS-DS-Text",     # String(Object-Identifier)
-    "2.5.5.8": "MS-DS-YesNo",    # Boolean
-    "2.5.5.9": "MS-DS-Number",   # Integer
-    "2.5.5.12": "MS-DS-Text",    # String(Unicode)
-    "2.5.5.15": "MS-DS-Text",    # String(NT-Sec-Desc)
-    "2.5.5.16": "MS-DS-Number",  # LargeInteger
-}
-
 
 class cmd_domain_claim_claim_type_create(ClaimCommand):
     """Create claim types on the domain."""
@@ -78,27 +66,6 @@ class cmd_domain_claim_claim_type_create(ClaimCommand):
                dest="unprotect", action="store_true")
     ]
 
-    @property
-    def claim_value_types(self):
-        """Property that returns a dict of claim value types keyed by CN.
-
-        NOTE: Can be replaced with @cached_property when the minimum Python
-        version becomes 3.8
-        """
-        value_types = getattr(self, "_claim_value_types", None)
-        if value_types is None:
-            value_types = {v.cn: v for v in ValueType.query(self.ldb)}
-            setattr(self, "_claim_value_types", value_types)
-        return value_types
-
-    def get_claim_value_type(self, attribute):
-        """Returns the correct claim value type for the given attribute.
-
-        Uses the LDAP attribute syntax to find the matching claim value type.
-        """
-        claim_type_cn = SYNTAX_TO_CLAIM_TYPE_CN[attribute.attribute_syntax]
-        return self.claim_value_types[claim_type_cn].claim_value_type
-
     def run(self, ldap_url=None, sambaopts=None, credopts=None, name=None,
             attribute_name=None, class_names=None, description=None,
             disable=None, enable=None, protect=None, unprotect=None):
@@ -130,6 +97,7 @@ class cmd_domain_claim_claim_type_create(ClaimCommand):
             applies_to = [ClassSchema.lookup(self.ldb, name)
                           for name in class_names]
             attribute = AttributeSchema.lookup(self.ldb, attribute_name)
+            value_type = ValueType.lookup(self.ldb, attribute)
         except (LookupError, ValueError) as e:
             raise CommandError(e)
 
@@ -155,7 +123,7 @@ class cmd_domain_claim_claim_type_create(ClaimCommand):
             claim_is_value_space_restricted=False,
             claim_source_type="AD",
             claim_type_applies_to_class=[obj.dn for obj in applies_to],
-            claim_value_type=self.get_claim_value_type(attribute),
+            claim_value_type=value_type.claim_value_type,
         )
 
         # Either --enable will be set or --disable but never both.

diff --git a/python/samba/netcmd/domain/models/value_type.py b/python/samba/netcmd/domain/models/value_type.py
index 731cc17c102aff24f94fd9676b9ee3748abd3169..c03b394b19a7b51ede34ef5f6caa7e9e0bac26b1 100644 (file)
--- a/python/samba/netcmd/domain/models/value_type.py
+++ b/python/samba/netcmd/domain/models/value_type.py
@@ -23,6 +23,18 @@
 from .fields import BooleanField, DnField, IntegerField, StringField
 from .model import Model
 
+# LDAP Syntax to Value Type CN lookup table.
+# These are the lookups used by known AD attributes, add new ones as required.
+SYNTAX_TO_VALUE_TYPE_CN = {
+    "2.5.5.1": "MS-DS-Text",     # Object(DS-DN)
+    "2.5.5.2": "MS-DS-Text",     # String(Object-Identifier)
+    "2.5.5.8": "MS-DS-YesNo",    # Boolean
+    "2.5.5.9": "MS-DS-Number",   # Integer
+    "2.5.5.12": "MS-DS-Text",    # String(Unicode)
+    "2.5.5.15": "MS-DS-Text",    # String(NT-Sec-Desc)
+    "2.5.5.16": "MS-DS-Number",  # LargeInteger
+}
+
 
 class ValueType(Model):
     description = StringField("description")
@@ -50,5 +62,32 @@ class ValueType(Model):
     def get_object_class():
         return "msDS-ValueType"
 
+    @classmethod
+    def lookup(cls, ldb, attribute):
+        """Helper function to get ValueType by attribute or raise LookupError.
+
+        :param ldb: Ldb connection
+        :param attribute: AttributeSchema object
+        :raises: LookupError if not found
+        :raises: ValueError for unknown attribute syntax
+        """
+        # If attribute is None.
+        if not attribute:
+            raise ValueError("Attribute is required for value type lookup.")
+
+        # Unknown attribute syntax as it isn't in the lookup table.
+        syntax = attribute.attribute_syntax
+        cn = SYNTAX_TO_VALUE_TYPE_CN.get(syntax)
+        if not cn:
+            raise ValueError(f"Unable to process attribute syntax {syntax}")
+
+        # This should always return something but should still be handled.
+        value_type = cls.get(ldb, cn=cn)
+        if value_type is None:
+            raise LookupError(
+                f"Could not find claim value type for {attribute}.")
+
+        return value_type
+
     def __str__(self):
         return str(self.display_name)