diff --git a/doc/changelog.rst b/doc/changelog.rst index 1c90420..cad5792 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -1,6 +1,13 @@ Changelog ========= +[0.4.0] - Unreleased +-------------------- + +Added +^^^^^ +- Proper path validation for :attr:`~scim2_models.SearchRequest.attributes`, :attr:`~scim2_models.SearchRequest.excluded_attributes` and :attr:`~scim2_models.SearchRequest.sort_by`. + [0.3.7] - 2025-07-17 -------------------- diff --git a/scim2_models/rfc7644/search_request.py b/scim2_models/rfc7644/search_request.py index 4ccad29..46ee931 100644 --- a/scim2_models/rfc7644/search_request.py +++ b/scim2_models/rfc7644/search_request.py @@ -6,6 +6,8 @@ from pydantic import model_validator from ..annotations import Required +from ..utils import validate_scim_path_syntax +from .error import Error from .message import Message @@ -24,10 +26,38 @@ class SearchRequest(Message): attributes to return in the response, overriding the set of attributes that would be returned by default.""" + @field_validator("attributes") + @classmethod + def validate_attributes_syntax(cls, v: Optional[list[str]]) -> Optional[list[str]]: + """Validate syntax of attribute paths.""" + if v is None: + return v + + for attr in v: + if not validate_scim_path_syntax(attr): + raise ValueError(Error.make_invalid_path_error().detail) + + return v + excluded_attributes: Optional[list[str]] = None """A multi-valued list of strings indicating the names of resource attributes to be removed from the default set of attributes to return.""" + @field_validator("excluded_attributes") + @classmethod + def validate_excluded_attributes_syntax( + cls, v: Optional[list[str]] + ) -> Optional[list[str]]: + """Validate syntax of excluded attribute paths.""" + if v is None: + return v + + for attr in v: + if not validate_scim_path_syntax(attr): + raise ValueError(Error.make_invalid_path_error().detail) + + return v + filter: Optional[str] = None """The filter string used to request a subset of resources.""" @@ -35,6 +65,25 @@ class SearchRequest(Message): """A string indicating the attribute whose value SHALL be used to order the returned responses.""" + @field_validator("sort_by") + @classmethod + def validate_sort_by_syntax(cls, v: Optional[str]) -> Optional[str]: + """Validate syntax of sort_by attribute path. + + :param v: The sort_by attribute path to validate + :type v: Optional[str] + :return: The validated sort_by attribute path + :rtype: Optional[str] + :raises ValueError: If sort_by attribute path has invalid syntax + """ + if v is None: + return v + + if not validate_scim_path_syntax(v): + raise ValueError(Error.make_invalid_path_error().detail) + + return v + class SortOrder(str, Enum): ascending = "ascending" descending = "descending" @@ -48,7 +97,7 @@ class SortOrder(str, Enum): @field_validator("start_index") @classmethod - def start_index_floor(cls, value: int) -> int: + def start_index_floor(cls, value: Optional[int]) -> Optional[int]: """According to :rfc:`RFC7644 §3.4.2 <7644#section-3.4.2.4>, start_index values less than 1 are interpreted as 1. A value less than 1 SHALL be interpreted as 1. @@ -61,7 +110,7 @@ def start_index_floor(cls, value: int) -> int: @field_validator("count") @classmethod - def count_floor(cls, value: int) -> int: + def count_floor(cls, value: Optional[int]) -> Optional[int]: """According to :rfc:`RFC7644 §3.4.2 <7644#section-3.4.2.4>, count values less than 0 are interpreted as 0. A negative value SHALL be interpreted as 0. diff --git a/scim2_models/utils.py b/scim2_models/utils.py index 0e3d248..567c6ce 100644 --- a/scim2_models/utils.py +++ b/scim2_models/utils.py @@ -97,3 +97,62 @@ def normalize_attribute_name(attribute_name: str) -> str: attribute_name = re.sub(r"[\W_]+", "", attribute_name) return attribute_name.lower() + + +def validate_scim_path_syntax(path: str) -> bool: + """Check if path syntax is valid according to RFC 7644 simplified rules. + + :param path: The path to validate + :type path: str + :return: True if path syntax is valid, False otherwise + :rtype: bool + """ + if not path or not path.strip(): + return False + + # Cannot start with a digit + if path[0].isdigit(): + return False + + # Cannot contain double dots + if ".." in path: + return False + + # Cannot contain invalid characters (basic check) + # Allow alphanumeric, dots, underscores, hyphens, colons (for URNs), brackets + if not re.match(r'^[a-zA-Z][a-zA-Z0-9._:\-\[\]"=\s]*$', path): + return False + + # If it contains a colon, validate it's a proper URN format + if ":" in path: + if not validate_scim_urn_syntax(path): + return False + + return True + + +def validate_scim_urn_syntax(path: str) -> bool: + """Validate URN-based path format. + + :param path: The URN path to validate + :type path: str + :return: True if URN path format is valid, False otherwise + :rtype: bool + """ + # Basic URN validation: should start with urn: + if not path.startswith("urn:"): + return False + + # Split on the last colon to separate URN from attribute + urn_part, attr_part = path.rsplit(":", 1) + + # URN part should have at least 4 parts (urn:namespace:specific:resource) + urn_segments = urn_part.split(":") + if len(urn_segments) < 4: + return False + + # Attribute part should be valid + if not attr_part or attr_part[0].isdigit(): + return False + + return True diff --git a/tests/test_path_validation.py b/tests/test_path_validation.py new file mode 100644 index 0000000..4f1f771 --- /dev/null +++ b/tests/test_path_validation.py @@ -0,0 +1,93 @@ +"""Tests for SCIM path validation utilities.""" + +from scim2_models.utils import validate_scim_path_syntax +from scim2_models.utils import validate_scim_urn_syntax + + +def test_validate_scim_path_syntax_valid_paths(): + """Test that valid SCIM paths are accepted.""" + valid_paths = [ + "userName", + "name.familyName", + "emails.value", + "groups.display", + "urn:ietf:params:scim:schemas:core:2.0:User:userName", + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:employeeNumber", + 'emails[type eq "work"].value', + 'groups[display eq "Admin"]', + "meta.lastModified", + ] + + for path in valid_paths: + assert validate_scim_path_syntax(path), f"Path should be valid: {path}" + + +def test_validate_scim_path_syntax_invalid_paths(): + """Test that invalid SCIM paths are rejected.""" + invalid_paths = [ + "", # Empty string + " ", # Whitespace only + "123invalid", # Starts with digit + "invalid..path", # Double dots + "invalid@path", # Invalid character + "urn:invalid", # Invalid URN format + "urn:too:short", # URN too short + ] + + for path in invalid_paths: + assert not validate_scim_path_syntax(path), f"Path should be invalid: {path}" + + +def test_validate_scim_urn_syntax_valid_urns(): + """Test that valid SCIM URN paths are accepted.""" + valid_urns = [ + "urn:ietf:params:scim:schemas:core:2.0:User:userName", + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:employeeNumber", + "urn:custom:namespace:schema:1.0:Resource:attribute", + "urn:example:extension:v2:MyResource:customField", + ] + + for urn in valid_urns: + assert validate_scim_urn_syntax(urn), f"URN should be valid: {urn}" + + +def test_validate_scim_urn_syntax_invalid_urns(): + """Test that invalid SCIM URN paths are rejected.""" + invalid_urns = [ + "not_an_urn", # Doesn't start with urn: + "urn:too:short", # Not enough segments + "urn:ietf:params:scim:schemas:core:2.0:User:", # Empty attribute + "urn:ietf:params:scim:schemas:core:2.0:User:123invalid", # Attribute starts with digit + "urn:invalid", # Too short + "urn:only:two:attribute", # URN part too short + ] + + for urn in invalid_urns: + assert not validate_scim_urn_syntax(urn), f"URN should be invalid: {urn}" + + +def test_validate_scim_path_syntax_edge_cases(): + """Test edge cases for path validation.""" + # Test None handling (shouldn't happen in practice but defensive) + assert not validate_scim_path_syntax("") + + # Test borderline valid cases + assert validate_scim_path_syntax("a") # Single character + assert validate_scim_path_syntax("a.b") # Simple dotted + assert validate_scim_path_syntax("a_b") # Underscore + assert validate_scim_path_syntax("a-b") # Hyphen + + # Test borderline invalid cases + assert not validate_scim_path_syntax("9invalid") # Starts with digit + assert not validate_scim_path_syntax("a..b") # Double dots + + +def test_validate_scim_urn_syntax_edge_cases(): + """Test edge cases for URN validation.""" + # Test minimal valid URN + assert validate_scim_urn_syntax("urn:a:b:c:d") + + # Test boundary cases + assert not validate_scim_urn_syntax("urn:a:b:c:") # Empty attribute + assert not validate_scim_urn_syntax("urn:a:b:") # Missing resource + assert not validate_scim_urn_syntax("urn:") # Just urn: diff --git a/tests/test_search_request.py b/tests/test_search_request.py index 2f7e58e..27825fb 100644 --- a/tests/test_search_request.py +++ b/tests/test_search_request.py @@ -77,3 +77,167 @@ def test_index_0_properties(): req = SearchRequest(start_index=1, count=10) assert req.start_index_0 == 0 assert req.stop_index_0 == 10 + + +def test_search_request_valid_attributes(): + """Test that valid attribute paths are accepted.""" + valid_data = { + "attributes": ["userName", "name.familyName", "emails.value"], + "excluded_attributes": None, + } + + request = SearchRequest.model_validate(valid_data) + assert request.attributes == ["userName", "name.familyName", "emails.value"] + + +def test_search_request_valid_excluded_attributes(): + """Test that valid excluded attribute paths are accepted.""" + valid_data = { + "attributes": None, + "excluded_attributes": ["password", "meta.version"], + } + + request = SearchRequest.model_validate(valid_data) + assert request.excluded_attributes == ["password", "meta.version"] + + +def test_search_request_valid_sort_by(): + """Test that valid sort_by paths are accepted.""" + valid_data = { + "sort_by": "meta.lastModified", + } + + request = SearchRequest.model_validate(valid_data) + assert request.sort_by == "meta.lastModified" + + +def test_search_request_valid_urn_attributes(): + """Test that URN attribute paths are accepted.""" + valid_data = { + "attributes": [ + "urn:ietf:params:scim:schemas:core:2.0:User:userName", + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:employeeNumber", + ], + } + + request = SearchRequest.model_validate(valid_data) + assert len(request.attributes) == 2 + + +def test_search_request_invalid_attributes(): + """Test that invalid attribute paths are rejected.""" + invalid_cases = [ + { + "attributes": ["123invalid"], # Starts with digit + "error_match": "path.*invalid", + }, + { + "attributes": ["valid", "invalid..path"], # Double dots + "error_match": "path.*invalid", + }, + { + "attributes": ["invalid@character"], # Invalid character + "error_match": "path.*invalid", + }, + ] + + for case in invalid_cases: + with pytest.raises(ValidationError, match=case["error_match"]): + SearchRequest.model_validate(case) + + +def test_search_request_invalid_excluded_attributes(): + """Test that invalid excluded attribute paths are rejected.""" + invalid_data = { + "excluded_attributes": ["valid", "123invalid"], # Second one starts with digit + } + + with pytest.raises(ValidationError, match="path.*invalid"): + SearchRequest.model_validate(invalid_data) + + +def test_search_request_invalid_sort_by(): + """Test that invalid sort_by paths are rejected.""" + invalid_cases = [ + {"sort_by": "123invalid"}, # Starts with digit + {"sort_by": "invalid..path"}, # Double dots + {"sort_by": "invalid@char"}, # Invalid character + {"sort_by": "urn:invalid"}, # Invalid URN + ] + + for case in invalid_cases: + with pytest.raises(ValidationError, match="path.*invalid"): + SearchRequest.model_validate(case) + + +def test_search_request_complex_paths_allowed(): + """Test that complex filter paths are allowed in attributes.""" + # Complex paths with filters should be allowed (for now) + valid_data = { + "attributes": [ + 'emails[type eq "work"].value', + 'groups[display eq "Admin"]', + "name.familyName", + ], + } + + request = SearchRequest.model_validate(valid_data) + assert len(request.attributes) == 3 + + +def test_search_request_empty_lists(): + """Test that empty attribute lists are handled correctly.""" + valid_data = { + "attributes": [], + "excluded_attributes": [], + } + + request = SearchRequest.model_validate(valid_data) + assert request.attributes == [] + assert request.excluded_attributes == [] + + +def test_search_request_none_values(): + """Test that None values are handled correctly.""" + valid_data = { + "attributes": None, + "excluded_attributes": None, + "sort_by": None, + } + + request = SearchRequest.model_validate(valid_data) + assert request.attributes is None + assert request.excluded_attributes is None + assert request.sort_by is None + + +def test_search_request_mutually_exclusive_validation(): + """Test that attributes and excluded_attributes are still mutually exclusive.""" + invalid_data = { + "attributes": ["userName"], + "excluded_attributes": ["password"], + } + + with pytest.raises(ValidationError, match="mutually exclusive"): + SearchRequest.model_validate(invalid_data) + + +def test_search_request_integration_with_existing_validation(): + """Test that new path validation works with existing validation.""" + # Valid path syntax but mutually exclusive + invalid_data = { + "attributes": ["userName", "emails.value"], + "excluded_attributes": ["password"], + } + + with pytest.raises(ValidationError, match="mutually exclusive"): + SearchRequest.model_validate(invalid_data) + + # Invalid path syntax should fail before mutual exclusion check + invalid_data = { + "attributes": ["123invalid"], + "excluded_attributes": ["password"], + } + + with pytest.raises(ValidationError, match="path.*invalid"): + SearchRequest.model_validate(invalid_data)