diff --git a/doc/changelog.rst b/doc/changelog.rst index cad5792..ed97224 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -8,6 +8,11 @@ Added ^^^^^ - Proper path validation for :attr:`~scim2_models.SearchRequest.attributes`, :attr:`~scim2_models.SearchRequest.excluded_attributes` and :attr:`~scim2_models.SearchRequest.sort_by`. +Fixed +^^^^^ +- When using ``model_dump``, ignore invalid ``attributes`` and ``excluded_attributes`` + as suggested by RFC7644. + [0.3.7] - 2025-07-17 -------------------- diff --git a/scim2_models/rfc7643/resource.py b/scim2_models/rfc7643/resource.py index c2ebdc6..3d33d57 100644 --- a/scim2_models/rfc7643/resource.py +++ b/scim2_models/rfc7643/resource.py @@ -28,7 +28,7 @@ from ..context import Context from ..reference import Reference from ..scim_object import ScimObject -from ..scim_object import validate_attribute_urn +from ..urn import validate_attribute_urn from ..utils import UNION_TYPES from ..utils import normalize_attribute_name @@ -305,13 +305,19 @@ def _prepare_model_dump( **kwargs: Any, ) -> dict[str, Any]: kwargs = super()._prepare_model_dump(scim_ctx, **kwargs) + + # RFC 7644: "SHOULD ignore any query parameters they do not recognize" kwargs["context"]["scim_attributes"] = [ - validate_attribute_urn(attribute, self.__class__) + valid_attr for attribute in (attributes or []) + if (valid_attr := validate_attribute_urn(attribute, self.__class__)) + is not None ] kwargs["context"]["scim_excluded_attributes"] = [ - validate_attribute_urn(attribute, self.__class__) + valid_attr for attribute in (excluded_attributes or []) + if (valid_attr := validate_attribute_urn(attribute, self.__class__)) + is not None ] return kwargs diff --git a/scim2_models/scim_object.py b/scim2_models/scim_object.py index b53c28c..1002518 100644 --- a/scim2_models/scim_object.py +++ b/scim2_models/scim_object.py @@ -8,86 +8,9 @@ from .annotations import Required from .base import BaseModel from .context import Context -from .utils import normalize_attribute_name if TYPE_CHECKING: - from .rfc7643.resource import Resource - - -def validate_model_attribute(model: type["BaseModel"], attribute_base: str) -> None: - """Validate that an attribute name or a sub-attribute path exist for a given model.""" - attribute_name, *sub_attribute_blocks = attribute_base.split(".") - sub_attribute_base = ".".join(sub_attribute_blocks) - - aliases = {field.validation_alias for field in model.model_fields.values()} - - if normalize_attribute_name(attribute_name) not in aliases: - raise ValueError( - f"Model '{model.__name__}' has no attribute named '{attribute_name}'" - ) - - if sub_attribute_base: - attribute_type = model.get_field_root_type(attribute_name) - - if not attribute_type or not issubclass(attribute_type, BaseModel): - raise ValueError( - f"Attribute '{attribute_name}' is not a complex attribute, and cannot have a '{sub_attribute_base}' sub-attribute" - ) - - validate_model_attribute(attribute_type, sub_attribute_base) - - -def extract_schema_and_attribute_base(attribute_urn: str) -> tuple[str, str]: - """Extract the schema urn part and the attribute name part from attribute name. - - As defined in :rfc:`RFC7644 §3.10 <7644#section-3.10>`. - """ - *urn_blocks, attribute_base = attribute_urn.split(":") - schema = ":".join(urn_blocks) - return schema, attribute_base - - -def validate_attribute_urn( - attribute_name: str, - default_resource: Optional[type["Resource"]] = None, - resource_types: Optional[list[type["Resource"]]] = None, -) -> str: - """Validate that an attribute urn is valid or not. - - :param attribute_name: The attribute urn to check. - :default_resource: The default resource if `attribute_name` is not an absolute urn. - :resource_types: The available resources in which to look for the attribute. - :return: The normalized attribute URN. - """ - from .rfc7643.resource import Resource - - if not resource_types: - resource_types = [] - - if default_resource and default_resource not in resource_types: - resource_types.append(default_resource) - - default_schema = ( - default_resource.model_fields["schemas"].default[0] - if default_resource - else None - ) - - schema: Optional[Any] - schema, attribute_base = extract_schema_and_attribute_base(attribute_name) - if not schema: - schema = default_schema - - if not schema: - raise ValueError("No default schema and relative URN") - - resource = Resource.get_by_schema(resource_types, schema) - if not resource: - raise ValueError(f"No resource matching schema '{schema}'") - - validate_model_attribute(resource, attribute_base) - - return f"{schema}:{attribute_base}" + pass class ScimObject(BaseModel): diff --git a/scim2_models/urn.py b/scim2_models/urn.py new file mode 100644 index 0000000..7ba2a20 --- /dev/null +++ b/scim2_models/urn.py @@ -0,0 +1,69 @@ +from typing import TYPE_CHECKING +from typing import Any +from typing import Optional + +from .base import BaseModel +from .utils import normalize_attribute_name + +if TYPE_CHECKING: + from .base import BaseModel + from .rfc7643.resource import Resource + + +def normalize_path(model: type["BaseModel"], path: str) -> tuple[str, str]: + """Resolve a path to (schema_urn, attribute_path).""" + # Absolute URN + if ":" in path: + parts = path.rsplit(":", 1) + return parts[0], parts[1] + + schemas_field = model.model_fields.get("schemas") + return schemas_field.default[0], path # type: ignore + + +def validate_model_attribute(model: type["BaseModel"], attribute_base: str) -> None: + """Validate that an attribute name or a sub-attribute path exist for a given model.""" + attribute_name, *sub_attribute_blocks = attribute_base.split(".") + sub_attribute_base = ".".join(sub_attribute_blocks) + + aliases = {field.validation_alias for field in model.model_fields.values()} + + if normalize_attribute_name(attribute_name) not in aliases: + raise ValueError( + f"Model '{model.__name__}' has no attribute named '{attribute_name}'" + ) + + if sub_attribute_base: + attribute_type = model.get_field_root_type(attribute_name) + + if not attribute_type or not issubclass(attribute_type, BaseModel): + raise ValueError( + f"Attribute '{attribute_name}' is not a complex attribute, and cannot have a '{sub_attribute_base}' sub-attribute" + ) + + validate_model_attribute(attribute_type, sub_attribute_base) + + +def validate_attribute_urn( + attribute_name: str, resource: type["Resource"] +) -> Optional[str]: + """Validate that an attribute urn is valid or not. + + :param attribute_name: The attribute urn to check. + :return: The normalized attribute URN. + """ + from .rfc7643.resource import Resource + + schema: Optional[Any] + schema, attribute_base = normalize_path(resource, attribute_name) + + validated_resource = Resource.get_by_schema([resource], schema) + if not validated_resource: + return None + + try: + validate_model_attribute(validated_resource, attribute_base) + except ValueError: + return None + + return f"{schema}:{attribute_base}" diff --git a/tests/test_model_attributes.py b/tests/test_model_attributes.py index 9d25d00..0a77a72 100644 --- a/tests/test_model_attributes.py +++ b/tests/test_model_attributes.py @@ -2,8 +2,6 @@ from typing import Annotated from typing import Optional -import pytest - from scim2_models.annotations import Required from scim2_models.annotations import Returned from scim2_models.attributes import ComplexAttribute @@ -15,7 +13,7 @@ from scim2_models.rfc7643.resource import Resource from scim2_models.rfc7643.user import User from scim2_models.rfc7644.error import Error -from scim2_models.scim_object import validate_attribute_urn +from scim2_models.urn import validate_attribute_urn class Sub(ComplexAttribute): @@ -74,32 +72,18 @@ def test_validate_attribute_urn(): validate_attribute_urn("urn:example:2.0:Foo:bar", Foo) == "urn:example:2.0:Foo:bar" ) - assert ( - validate_attribute_urn("urn:example:2.0:Foo:bar", User, resource_types=[Foo]) - == "urn:example:2.0:Foo:bar" - ) assert validate_attribute_urn("sub", Foo) == "urn:example:2.0:Foo:sub" assert ( validate_attribute_urn("urn:example:2.0:Foo:sub", Foo) == "urn:example:2.0:Foo:sub" ) - assert ( - validate_attribute_urn("urn:example:2.0:Foo:sub", User, resource_types=[Foo]) - == "urn:example:2.0:Foo:sub" - ) assert validate_attribute_urn("sub.always", Foo) == "urn:example:2.0:Foo:sub.always" assert ( validate_attribute_urn("urn:example:2.0:Foo:sub.always", Foo) == "urn:example:2.0:Foo:sub.always" ) - assert ( - validate_attribute_urn( - "urn:example:2.0:Foo:sub.always", User, resource_types=[Foo] - ) - == "urn:example:2.0:Foo:sub.always" - ) assert validate_attribute_urn("snakeCase", Foo) == "urn:example:2.0:Foo:snakeCase" assert ( @@ -111,37 +95,18 @@ def test_validate_attribute_urn(): validate_attribute_urn("urn:example:2.0:MyExtension:baz", Foo[MyExtension]) == "urn:example:2.0:MyExtension:baz" ) + + assert validate_attribute_urn("urn:InvalidResource:bar", Foo) is None + + assert validate_attribute_urn("urn:example:2.0:Foo:invalid", Foo) is None + + assert validate_attribute_urn("bar.invalid", Foo) is None + assert ( - validate_attribute_urn( - "urn:example:2.0:MyExtension:baz", resource_types=[Foo[MyExtension]] - ) - == "urn:example:2.0:MyExtension:baz" + validate_attribute_urn("urn:example:2.0:MyExtension:invalid", Foo[MyExtension]) + is None ) - with pytest.raises(ValueError, match="No default schema and relative URN"): - validate_attribute_urn("bar", resource_types=[Foo]) - - with pytest.raises( - ValueError, match="No resource matching schema 'urn:InvalidResource'" - ): - validate_attribute_urn("urn:InvalidResource:bar", Foo) - - with pytest.raises( - ValueError, match="No resource matching schema 'urn:example:2.0:Foo'" - ): - validate_attribute_urn("urn:example:2.0:Foo:bar") - - with pytest.raises( - ValueError, match="Model 'Foo' has no attribute named 'invalid'" - ): - validate_attribute_urn("urn:example:2.0:Foo:invalid", Foo) - - with pytest.raises( - ValueError, - match="Attribute 'bar' is not a complex attribute, and cannot have a 'invalid' sub-attribute", - ): - validate_attribute_urn("bar.invalid", Foo) - def test_payload_attribute_case_sensitivity(): """RFC7643 §2.1 indicates that attribute names should be case insensitive. diff --git a/tests/test_model_serialization.py b/tests/test_model_serialization.py index ab9e0a0..596e3f7 100644 --- a/tests/test_model_serialization.py +++ b/tests/test_model_serialization.py @@ -192,48 +192,82 @@ def test_dump_default_response(ret_resource): def test_invalid_attributes(): - """Test error handling for invalid attributes parameter.""" + """Test that invalid attributes are ignored per RFC 7644 recommendation.""" resource = SupRetResource(id="id", always_returned="x", default_returned="x") - with pytest.raises(ValueError): - resource.model_dump( - scim_ctx=Context.RESOURCE_QUERY_RESPONSE, attributes={"invalidAttribute"} - ) + # Invalid attributes should be ignored, not raise errors + result = resource.model_dump( + scim_ctx=Context.RESOURCE_QUERY_RESPONSE, attributes={"invalidAttribute"} + ) + # Should return default response (alwaysReturned attributes) + assert result == { + "schemas": ["org:example:SupRetResource"], + "id": "id", + "alwaysReturned": "x", + "defaultReturned": "x", + } - with pytest.raises(ValueError): - resource.model_dump( - scim_ctx=Context.RESOURCE_QUERY_RESPONSE, - attributes={"org:example:SupRetResource:invalidAttribute"}, - ) + result = resource.model_dump( + scim_ctx=Context.RESOURCE_QUERY_RESPONSE, + attributes={"org:example:SupRetResource:invalidAttribute"}, + ) + assert result == { + "schemas": ["org:example:SupRetResource"], + "id": "id", + "alwaysReturned": "x", + "defaultReturned": "x", + } - with pytest.raises(ValueError): - resource.model_dump( - scim_ctx=Context.RESOURCE_QUERY_RESPONSE, - attributes={"urn:invalid:schema:invalidAttribute"}, - ) + result = resource.model_dump( + scim_ctx=Context.RESOURCE_QUERY_RESPONSE, + attributes={"urn:invalid:schema:invalidAttribute"}, + ) + assert result == { + "schemas": ["org:example:SupRetResource"], + "id": "id", + "alwaysReturned": "x", + "defaultReturned": "x", + } def test_invalid_excluded_attributes(): - """Test error handling for invalid excluded_attributes parameter.""" + """Test that invalid excluded_attributes are ignored per RFC 7644 recommendation.""" resource = SupRetResource(id="id", always_returned="x", default_returned="x") - with pytest.raises(ValueError): - resource.model_dump( - scim_ctx=Context.RESOURCE_QUERY_RESPONSE, - excluded_attributes={"invalidAttribute"}, - ) - - with pytest.raises(ValueError): - resource.model_dump( - scim_ctx=Context.RESOURCE_QUERY_RESPONSE, - excluded_attributes={"org:example:SupRetResource:invalidAttribute"}, - ) - - with pytest.raises(ValueError): - resource.model_dump( - scim_ctx=Context.RESOURCE_QUERY_RESPONSE, - excluded_attributes={"urn:invalid:schema:invalidAttribute"}, - ) + # Invalid excluded_attributes should be ignored, not raise errors + result = resource.model_dump( + scim_ctx=Context.RESOURCE_QUERY_RESPONSE, + excluded_attributes={"invalidAttribute"}, + ) + # Should return default response (nothing excluded) + assert result == { + "schemas": ["org:example:SupRetResource"], + "id": "id", + "alwaysReturned": "x", + "defaultReturned": "x", + } + + result = resource.model_dump( + scim_ctx=Context.RESOURCE_QUERY_RESPONSE, + excluded_attributes={"org:example:SupRetResource:invalidAttribute"}, + ) + assert result == { + "schemas": ["org:example:SupRetResource"], + "id": "id", + "alwaysReturned": "x", + "defaultReturned": "x", + } + + result = resource.model_dump( + scim_ctx=Context.RESOURCE_QUERY_RESPONSE, + excluded_attributes={"urn:invalid:schema:invalidAttribute"}, + ) + assert result == { + "schemas": ["org:example:SupRetResource"], + "id": "id", + "alwaysReturned": "x", + "defaultReturned": "x", + } @pytest.mark.parametrize(