From c1ac9bb69acf1e94e2cfcc8632bd0f6901aee438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Thu, 17 Sep 2026 10:29:49 +0200 Subject: [PATCH 1/6] refactor: factorize the SCIM name of a field Four places answered the question of the name a field is serialized under, each with its own fallback, and only one of them fell back on the camel-cased field name. BaseModel._scim_name now answers it for all of them, so the name the resolution reads cannot drift from the name the serialization writes. --- scim2_models/attributes.py | 5 +---- scim2_models/base.py | 5 +++++ scim2_models/messages/patch_op.py | 7 +------ scim2_models/path/path.py | 10 ++-------- scim2_models/resources/resource.py | 2 +- 5 files changed, 10 insertions(+), 19 deletions(-) diff --git a/scim2_models/attributes.py b/scim2_models/attributes.py index ee2f5d6..2fc4e72 100644 --- a/scim2_models/attributes.py +++ b/scim2_models/attributes.py @@ -71,10 +71,7 @@ def _get_attribute_urn(self, field_name: str) -> str: See RFC7644 §3.10. """ - alias = ( - self.__class__.model_fields[field_name].serialization_alias or field_name - ) - return f"{self._attribute_urn}.{alias}" + return f"{self._attribute_urn}.{self._scim_name(field_name)}" class MultiValuedComplexAttribute(ComplexAttribute): diff --git a/scim2_models/base.py b/scim2_models/base.py index 9550b79..1c2dc60 100644 --- a/scim2_models/base.py +++ b/scim2_models/base.py @@ -329,6 +329,11 @@ def get_field_multiplicity(cls, attribute_name: str) -> bool: origin = get_origin(attribute_type) return isinstance(origin, type) and issubclass(origin, list) + @classmethod + def _scim_name(cls, field_name: str) -> str: + """Return the name a field is serialized under, ``$ref`` included.""" + return cls.model_fields[field_name].serialization_alias or _to_camel(field_name) + @classmethod def __pydantic_on_complete__(cls) -> None: """Build the per-class SCIM metadata table on ``cls.__scim_info__``. diff --git a/scim2_models/messages/patch_op.py b/scim2_models/messages/patch_op.py index 183df53..de505df 100644 --- a/scim2_models/messages/patch_op.py +++ b/scim2_models/messages/patch_op.py @@ -102,11 +102,6 @@ def _resolved_field(resource_class: type[BaseModel], attr_name: str) -> str | No """Fields that carry the payload rather than the state it describes.""" -def _attribute_name(model: type[BaseModel], field_name: str) -> str: - """Return the SCIM spelling of a field, as a path segment.""" - return model.model_fields[field_name].serialization_alias or field_name - - def _asserted_sub_attributes(entries: Any) -> set[str]: """Return the sub-attributes the entries of a wanted state name.""" asserted: set[str] = set() @@ -212,7 +207,7 @@ def _diff( old = getattr(before, field_name, None) if before is not None else None new = getattr(after, field_name, None) - path = f"{prefix}{_attribute_name(model, field_name)}" + path = f"{prefix}{model._scim_name(field_name)}" if model.get_field_multiplicity(field_name): yield from _diff_multi_valued(path, old, new, mutability) diff --git a/scim2_models/path/path.py b/scim2_models/path/path.py index be46d2d..446c628 100644 --- a/scim2_models/path/path.py +++ b/scim2_models/path/path.py @@ -10,7 +10,6 @@ from ..base import BaseModel from ..urn import URN -from ..utils import _to_camel from .access import _delete_value from .access import _get_value from .access import _set_value @@ -51,11 +50,6 @@ def _node_attr_path(node: PathNode) -> AttrPath: return node.attr_path -def _scim_name(model: type[BaseModel], field_name: str) -> str: - """Return the name a field is serialized under, ``$ref`` included.""" - return model.model_fields[field_name].serialization_alias or _to_camel(field_name) - - class Path(_BoundToModels, _Expression, Generic[ResourceT]): """A SCIM attribute path, as defined at :rfc:`RFC7644 §3.5.2 <7644#section-3.5.2>`. @@ -506,7 +500,7 @@ def iter_model_paths( elif isclass(target_model) and issubclass(target_model, Extension): urn = target_model()._get_attribute_urn(field_name) else: - urn = _scim_name(target_model, field_name) + urn = target_model._scim_name(field_name) yield cls(urn) @@ -519,7 +513,7 @@ def iter_model_paths( for sub_field_name in field_type.model_fields: # type: ignore[union-attr] if not matches_filters(field_type, sub_field_name): # type: ignore[arg-type] continue - sub_urn = f"{urn}.{_scim_name(field_type, sub_field_name)}" # type: ignore[arg-type] + sub_urn = f"{urn}.{field_type._scim_name(sub_field_name)}" # type: ignore[union-attr] yield cls(sub_urn) yield from iter_model_paths(model) # type: ignore[arg-type] diff --git a/scim2_models/resources/resource.py b/scim2_models/resources/resource.py index 1315985..fcfb69e 100644 --- a/scim2_models/resources/resource.py +++ b/scim2_models/resources/resource.py @@ -539,7 +539,7 @@ def _model_attribute_to_scim_attribute( ) kwargs: dict[str, Any] = { - "name": field_info.serialization_alias or attribute_name, + "name": model._scim_name(attribute_name), "type": Attribute.Type(attribute_type), "multi_valued": model.get_field_multiplicity(attribute_name), "description": field_info.description, From 8027e01f937e87fe12631e287a805f48facc701a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Thu, 17 Sep 2026 10:30:42 +0200 Subject: [PATCH 2/6] fix: match attribute names on their case only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC7643 §2.1 makes attribute names case-insensitive, and its nameChar rule makes $, - and _ part of a name, but the normalisation stripped them all, so userName, user_name and u.s.e.r.n.a.m.e reached one field and a field carrying an explicit alias reached none. Every model now carries a table listing, for each of its fields, the spellings a payload may use, the validation hook rewrites each key through it, and pydantic reads the attribute under the SCIM spelling, which an error and a published JSON schema then carry. --- doc/changelog.rst | 16 +++ doc/how-to/define-custom-models.rst | 41 ++++++ scim2_models/attributes.py | 4 +- scim2_models/base.py | 163 +++++++++++++++++----- scim2_models/resources/enterprise_user.py | 2 +- scim2_models/resources/group.py | 2 +- scim2_models/resources/resource.py | 3 +- scim2_models/resources/schema.py | 3 +- scim2_models/resources/user.py | 2 +- scim2_models/utils.py | 26 ++-- tests/test_model_attributes.py | 141 ++++++++++++++++++- tests/test_reference.py | 8 +- 12 files changed, 343 insertions(+), 68 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 86912b6..5dd5bd6 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -65,6 +65,12 @@ Added Changed ^^^^^^^ +- Attribute names are matched case-insensitively, and nothing else. The ``nameChar`` rule of + :rfc:`RFC7643 §2.1 <7643#section-2.1>` makes ``$``, ``-`` and ``_`` part of a name, so + ``{"user-name": "x"}``, which 0.7 read as ``userName``, is now an unknown attribute that + :attr:`~scim2_models.ScimPolicy.unknown` governs. Paths, filters and ``sortBy`` resolve the + same way. A model whose fields answer to one attribute name raises a :class:`TypeError` where + it is defined. :issue:`166` - The bulk models take the resource type their operations carry, as in ``BulkRequest[User]`` or ``BulkRequest[User | Group]``, and raise a :class:`TypeError` when used bare. A payload the type parameter does not cover is now refused, and a bulk response no longer dumps ``path``. @@ -124,6 +130,16 @@ Deprecated Fixed ^^^^^ +- A pydantic error spells the attribute as SCIM does, ``userName`` and ``$ref`` where it used to + report ``username`` and ``ref``, and so does the JSON schema a model publishes, which FastAPI + reads to document a request body. :issue:`166` +- A field declaring its own ``alias`` is read under it, and an unknown attribute is refused under + the spelling the peer used. ``Field(alias="string_field")`` used to answer ``extra_forbidden`` + quoting a spelling nobody had sent. :issue:`166` +- An extension is read under its class name as well as under its URN, so + ``User[EnterpriseUser](EnterpriseUser=extension)`` is accepted and a resource carrying an + extension survives a dump without aliases read back. Both used to answer ``extra_forbidden``. + :issue:`166` - A bulk model indexed with something other than a resource type names itself in the error. The rules of the :class:`~scim2_models.PatchOp` its operations carry used to answer for it, so ``BulkRequest[str]`` told the caller to write ``PatchOp[User]``. diff --git a/doc/how-to/define-custom-models.rst b/doc/how-to/define-custom-models.rst index 9b44df5..ed49d6c 100644 --- a/doc/how-to/define-custom-models.rst +++ b/doc/how-to/define-custom-models.rst @@ -41,6 +41,47 @@ as optional because SCIM can omit an attribute in a valid request or response. >>> pet.model_dump()["details"] {'color': 'ginger', 'weightKg': 4.2} +Name an attribute +----------------- + +A field named in Python takes the camel-case spelling of its name as its SCIM attribute name: +``weight_kg`` becomes ``weightKg``. Reading a payload goes the other way, and +:rfc:`RFC7643 §2.1 <7643#section-2.1>` makes attribute names case-insensitive, so ``weightKg``, +``weightkg`` and ``WEIGHTKG`` all reach ``weight_kg``. Nothing else is folded: the ABNF of that +section makes ``$``, ``-`` and ``_`` part of a name, so ``weight-kg`` is another attribute +altogether, refused as an unknown one. See :doc:`tolerate-a-nonconformant-peer` to accept what a peer +spells its own way. + +When the attribute name is not what camel-casing a Python name yields, declare it with a +``serialization_alias``: + +.. doctest:: + + >>> from pydantic import Field + >>> class Pet(Resource): + ... __schema__ = URN("urn:example:schemas:Pet") + ... vet_ref: str | None = Field(None, serialization_alias="$vetRef") + ... + >>> Pet.model_validate({"$vetRef": "https://example.com/Vets/1"}).vet_ref + 'https://example.com/Vets/1' + +An alias applies to reading as well as to writing, and it wins over the Python name of any other +field. A field whose alias is the Python name of its neighbour therefore takes the +keyword that spells it, in a payload and in the constructor alike: + +.. doctest:: + + >>> class Pet(Resource): + ... __schema__ = URN("urn:example:schemas:Pet") + ... pet_name: str | None = None + ... legacy: str | None = Field(None, serialization_alias="pet_name") + ... + >>> Pet(pet_name="Mochi").legacy + 'Mochi' + +Two fields cannot answer to one attribute name. Such a model raises a :class:`TypeError` where it +is defined, no payload key being able to reach both. + Apply SCIM attribute metadata ----------------------------- diff --git a/scim2_models/attributes.py b/scim2_models/attributes.py index 2fc4e72..eb5aa3c 100644 --- a/scim2_models/attributes.py +++ b/scim2_models/attributes.py @@ -92,7 +92,9 @@ class MultiValuedComplexAttribute(ComplexAttribute): value: Any | None = None """The value of an entitlement.""" - ref: Reference[Any] | None = Field(None, serialization_alias="$ref") + ref: Reference[Any] | None = Field( + None, serialization_alias="$ref", validation_alias="$ref" + ) """The reference URI of a target resource, if the attribute is a reference.""" diff --git a/scim2_models/base.py b/scim2_models/base.py index 1c2dc60..52f1bbc 100644 --- a/scim2_models/base.py +++ b/scim2_models/base.py @@ -11,6 +11,7 @@ from typing import get_args from typing import get_origin +from pydantic import AliasChoices from pydantic import AliasGenerator from pydantic import Base64Bytes from pydantic import BaseModel as PydanticBaseModel @@ -23,6 +24,7 @@ from pydantic import ValidatorFunctionWrapHandler from pydantic import model_serializer from pydantic import model_validator +from pydantic.fields import FieldInfo from pydantic_core import InitErrorDetails from pydantic_core import PydanticCustomError from typing_extensions import Self @@ -118,9 +120,10 @@ class _SCIMClassInfo(NamedTuple): """SCIM metadata for BaseModel.""" alias_to_field: Mapping[str, str] = MappingProxyType({}) - """Alias -> Python field name. + """Serialization alias -> Python field name. - Holds both validation and serialization aliases. + Keyed by the spelling a dump carries, so a serializer can walk back from a + key it produced to the field that holds it. """ attribute_urns: Mapping[str, str] = MappingProxyType({}) @@ -132,12 +135,92 @@ class _SCIMClassInfo(NamedTuple): extensions: frozenset[str] = frozenset() """Field names whose root type is a ``Extension`` subclass.""" - known_keys: frozenset[str] = frozenset() - """Every payload key the class accepts, normalized. + validation_names: Mapping[str, str] = MappingProxyType({}) + """""Python field name -> the name pydantic reads that field under. - Field names and aliases alike: an extension is named by its URN in a - payload and by its class name as a field, and both name the same thing. + An attribute reaches pydantic under its SCIM spelling, which a validation + error and a published JSON schema then carry. + """ "" + + field_by_name: Mapping[str, str] = MappingProxyType({}) + """Lowercased attribute name -> Python field name. + + Every spelling a payload may use for a field: the name it is serialized + under, the aliases it declares for itself, and its Python name. RFC7643 + §2.1 makes attribute names case-insensitive and nothing else — its + ``nameChar`` rule makes ``$``, ``-`` and ``_`` part of a name — so the keys + are lowercased and keep their punctuation. + """ + + +_DECLARED_ALIAS_PRIORITY = 2 +"""The ``alias_priority`` pydantic gives an alias the field itself declares, an +alias generator filling the slots left empty with a priority of 1.""" + + +def _declared_validation_names(field: FieldInfo) -> list[str]: + """Return the names a field declares for itself. + + Only the field itself declares an attribute name: what the alias generator + derived from a Python name is how pydantic reads the field, not a spelling + a peer may use. An AliasChoices holds several spellings of one attribute, + each of them usable. An AliasPath points at a place inside the payload + rather than at an attribute, so it indexes nothing: the key it starts from + is no attribute name, and reaches pydantic as the peer spelled it. + """ + if field.alias_priority != _DECLARED_ALIAS_PRIORITY: + return [] + + alias = field.validation_alias + if isinstance(alias, str): + return [alias] + if isinstance(alias, AliasChoices): + return [choice for choice in alias.choices if isinstance(choice, str)] + return [] + + +def _validation_name(field: FieldInfo, field_name: str) -> str: + """Return the name pydantic reads a field under. + + The alias generator gives every field its SCIM attribute name, which an + error and a published JSON schema then carry. A field declaring + several spellings of its own names none of them in particular, and is read + under its Python name. + """ + alias = field.validation_alias + return alias if isinstance(alias, str) else field_name + + +def _claim_attribute_name( + index: dict[str, str], name: str, field_name: str, owner: type +) -> None: + """Record that a field answers to an attribute name. + + Two fields answering to one name leave a payload key reaching both, which + the class cannot be built with, so such a class is refused where it is + written. + """ + key = name.lower() + claimed = index.get(key) + if claimed is not None and claimed != field_name: + raise TypeError( + f"{owner.__name__} has two fields answering to the SCIM attribute " + f"name {name!r}: {claimed!r} and {field_name!r}. Attribute names are " + f"case-insensitive (RFC7643 §2.1), so one payload key would reach both." + ) + index[key] = field_name + + +def _claim_python_name(index: dict[str, str], field_name: str) -> None: + """Record that a field answers to its own Python name. + + That name is a convenience rather than an attribute name, so two fields + whose names only differ by case take it from each other instead of making + the class impossible to build: the key then designates neither, leaving the + SCIM name of each of them the only way to reach it. """ + key = field_name.lower() + index[key] = "" if key in index and index[key] != field_name else field_name def _holds_reference(model: type["BaseModel"], field_name: str) -> bool: @@ -172,7 +255,7 @@ class BaseModel(PydanticBaseModel): model_config = ConfigDict( alias_generator=AliasGenerator( - validation_alias=_normalize_attribute_name, + validation_alias=_to_camel, serialization_alias=_to_camel, ), validate_assignment=True, @@ -347,6 +430,9 @@ def __pydantic_on_complete__(cls) -> None: attribute_urns: dict[str, str] = {} complex_fields: set[str] = set() extensions: set[str] = set() + scim_names: dict[str, str] = {} + python_names: dict[str, str] = {} + validation_names: dict[str, str] = {} main_schema = getattr(cls, "__schema__", None) extension_cls: type | None = None @@ -357,10 +443,16 @@ def __pydantic_on_complete__(cls) -> None: for field_name, field in cls.model_fields.items(): # Alias -> field name mapping - serialization_alias = field.serialization_alias or field_name + serialization_alias = cls._scim_name(field_name) alias_to_field[serialization_alias] = field_name - if isinstance(field.validation_alias, str): - alias_to_field[field.validation_alias] = field_name + + # The names this field answers to, the SCIM ones winning over the + # Python one, which is only the spelling pydantic offers. + _claim_attribute_name(scim_names, serialization_alias, field_name, cls) + for declared in _declared_validation_names(field): + _claim_attribute_name(scim_names, declared, field_name, cls) + _claim_python_name(python_names, field_name) + validation_names[field_name] = _validation_name(field, field_name) root_type = cls.get_field_root_type(field_name) @@ -390,41 +482,42 @@ def __pydantic_on_complete__(cls) -> None: attribute_urns=attribute_urns, complex_fields=frozenset(complex_fields), extensions=frozenset(extensions), - known_keys=frozenset( - _normalize_attribute_name(key) - for key in (*cls.model_fields, *alias_to_field) - ), + field_by_name={ + **{key: name for key, name in python_names.items() if name}, + **scim_names, + }, + validation_names=validation_names, ) @model_validator(mode="wrap") @classmethod - def _normalize_attribute_names( + def _resolve_attribute_names( cls, value: Any, handler: ValidatorFunctionWrapHandler, info: ValidationInfo ) -> Self: - """Normalize payload attribute names, and set aside the ones no field declares. + """Rewrite each payload key to the field holding it, and set aside the rest. - RFC7643 §2.1 indicate that attribute names should be case-insensitive. - Any attribute name is transformed in lowercase so any case is handled - the same way. + RFC7643 §2.1 makes attribute names case-insensitive, so a key is looked + up folded. What it resolves to is the Python name of the field, which + is the one spelling pydantic accepts for every field. - Unless the policy forbids them, unknown keys are taken out of the - payload with the spelling the peer used. Pydantic never sees them, so - the ``extra="forbid"`` of the class has nothing to refuse. + A key no field answers to is taken out of the payload with the spelling + the peer used, unless the policy forbids unknown attributes: it is then + left in place, so that the error pydantic raises quotes what was sent. """ unknown: dict[str, Any] = {} if isinstance(value, dict): - if _policy(info).unknown == ScimPolicy.Unknown.forbid: - value = {_normalize_attribute_name(k): v for k, v in value.items()} - else: - known = cls.__scim_info__.known_keys - normalized = {} - for key, item in value.items(): - name = _normalize_attribute_name(key) - if name in known: - normalized[name] = item - else: - unknown[key] = item - value = normalized + scim_info = cls.__scim_info__ + tolerated = _policy(info).unknown != ScimPolicy.Unknown.forbid + resolved: dict[Any, Any] = {} + for key, item in value.items(): + field_name = scim_info.field_by_name.get(_normalize_attribute_name(key)) + if field_name is not None: + resolved[scim_info.validation_names[field_name]] = item + elif tolerated: + unknown[key] = item + else: + resolved[key] = item + value = resolved obj = cast(Self, handler(value)) if unknown: @@ -672,7 +765,7 @@ def _set_complex_attribute_urns(self) -> None: ``_attribute_urn`` is later read by _get_attribute_urn. """ - for field_name in self.__class__.__scim_info__.complex_fields: + for field_name in self.__scim_info__.complex_fields: attr_value = getattr(self, field_name) if not attr_value: continue diff --git a/scim2_models/resources/enterprise_user.py b/scim2_models/resources/enterprise_user.py index 227ab09..34a65e5 100644 --- a/scim2_models/resources/enterprise_user.py +++ b/scim2_models/resources/enterprise_user.py @@ -22,7 +22,7 @@ class Manager(ComplexAttribute): ref: Annotated[ # type: ignore[type-arg] Reference["User"] | None, Required.true, - ] = Field(None, serialization_alias="$ref") + ] = Field(None, serialization_alias="$ref", validation_alias="$ref") """The URI of the SCIM resource representing the User's manager.""" display_name: Annotated[str | None, Mutability.read_only] = None diff --git a/scim2_models/resources/group.py b/scim2_models/resources/group.py index 328293c..187a9c3 100644 --- a/scim2_models/resources/group.py +++ b/scim2_models/resources/group.py @@ -25,7 +25,7 @@ class GroupMember(ComplexAttribute): ref: Annotated[ # type: ignore[type-arg] Reference[Union["User", "Group"]] | None, Mutability.immutable, - ] = Field(None, serialization_alias="$ref") + ] = Field(None, serialization_alias="$ref", validation_alias="$ref") """The reference URI of a target resource, if the attribute is a reference.""" diff --git a/scim2_models/resources/resource.py b/scim2_models/resources/resource.py index fcfb69e..f451e4d 100644 --- a/scim2_models/resources/resource.py +++ b/scim2_models/resources/resource.py @@ -38,7 +38,6 @@ from ..policy import _policy from ..scim_object import ScimObject from ..utils import UNION_TYPES -from ..utils import _normalize_attribute_name if TYPE_CHECKING: from .schema import Attribute @@ -248,7 +247,7 @@ def __class_getitem__(cls, item: Any) -> type["Resource[Any]"]: class_attrs[extension.__name__] = Field( default=None, # type: ignore[arg-type] serialization_alias=schema, - validation_alias=_normalize_attribute_name(schema), + validation_alias=schema, ) new_annotations = { diff --git a/scim2_models/resources/schema.py b/scim2_models/resources/schema.py index 6bffeef..771eb9c 100644 --- a/scim2_models/resources/schema.py +++ b/scim2_models/resources/schema.py @@ -30,7 +30,6 @@ from ..reference import External from ..reference import Reference from ..urn import URN -from ..utils import _normalize_attribute_name from .resource import Resource T = TypeVar("T", bound=BaseModel) @@ -236,7 +235,7 @@ def _to_python(self) -> tuple[Any, Any] | None: description=self.description, examples=self.canonical_values, serialization_alias=self.name, - validation_alias=_normalize_attribute_name(self.name), + validation_alias=self.name, default=None, ) diff --git a/scim2_models/resources/user.py b/scim2_models/resources/user.py index fcb7539..4423c74 100644 --- a/scim2_models/resources/user.py +++ b/scim2_models/resources/user.py @@ -200,7 +200,7 @@ class GroupMembership(ComplexAttribute): ref: Annotated[ Reference["Group"] | None, Mutability.read_only, - ] = Field(None, serialization_alias="$ref") + ] = Field(None, serialization_alias="$ref", validation_alias="$ref") """The reference URI of a target resource, if the attribute is a reference.""" diff --git a/scim2_models/utils.py b/scim2_models/utils.py index 953d63d..c757635 100644 --- a/scim2_models/utils.py +++ b/scim2_models/utils.py @@ -1,5 +1,4 @@ import re -from functools import lru_cache from inspect import isclass from typing import TYPE_CHECKING from typing import Any @@ -42,7 +41,6 @@ def _model_union(annotation: Any) -> "tuple[type[BaseModel], ...] | None": _UNDERSCORE_ALPHANUMERIC = re.compile(r"_+([0-9A-Za-z]+)") -_NON_WORD_UNDERSCORE = re.compile(r"[\W_]+") def _int_to_str(status: int | None) -> str | None: @@ -62,28 +60,22 @@ def _to_camel(string: str) -> str: return camel -@lru_cache(maxsize=256) def _normalize_attribute_name(attribute_name: str) -> str: - """Remove all non-alphabetical characters and lowerise a string. + """Fold the case of an attribute name. - This method is used for attribute name validation. + RFC7643 §2.1 makes attribute names case-insensitive, and its ``nameChar`` + rule makes ``$``, ``-`` and ``_`` part of a name, so the case is all there + is to fold. """ - is_extension_attribute = ":" in attribute_name - if not is_extension_attribute: - attribute_name = _NON_WORD_UNDERSCORE.sub("", attribute_name) - return attribute_name.lower() def _find_field_name(model_class: type["BaseModel"], attr_name: str) -> str | None: """Return the field a SCIM attribute name designates, or None. - ``nickName`` designates the ``nick_name`` field. + ``nickName`` designates the ``nick_name`` field, and ``$ref`` the ``ref`` + one. """ - normalized_attr_name = _normalize_attribute_name(attr_name) - - for field_key in model_class.model_fields: - if _normalize_attribute_name(field_key) == normalized_attr_name: - return field_key - - return None + return model_class.__scim_info__.field_by_name.get( + _normalize_attribute_name(attr_name) + ) diff --git a/tests/test_model_attributes.py b/tests/test_model_attributes.py index 79d487e..d144294 100644 --- a/tests/test_model_attributes.py +++ b/tests/test_model_attributes.py @@ -1,9 +1,12 @@ import uuid from typing import Annotated +import pytest from pydantic import AliasChoices +from pydantic import AliasPath from pydantic import Base64Bytes from pydantic import Field +from pydantic import ValidationError from scim2_models import URN from scim2_models import ResponseParameters @@ -457,7 +460,7 @@ def test_extension_excluded_by_full_urn(): def test_field_with_custom_validation_aliases(): - """A field may bring its own validation aliases, which are not indexed as names.""" + """A field is read under every name it declares for itself.""" class AliasedResource(Resource): __schema__ = URN("urn:example:2.0:AliasedResource") @@ -466,9 +469,7 @@ class AliasedResource(Resource): None, validation_alias=AliasChoices("value", "legacyvalue") ) - assert all( - isinstance(alias, str) for alias in AliasedResource.__scim_info__.alias_to_field - ) + assert AliasedResource.__scim_info__.field_by_name["legacyvalue"] == "value" obj = AliasedResource.model_validate({"legacyValue": "x"}) @@ -485,3 +486,135 @@ def test_short_attr_path_with_plain_name(): assert _short_attr_path("userName") == "userName" assert _short_attr_path("name.familyName") == "name.familyName" + + +@pytest.mark.parametrize( + "spelling", ["userName", "username", "USERNAME", "UserName", "user_name"] +) +def test_an_attribute_name_is_read_whatever_its_case(spelling): + """RFC7643 §2.1 makes attribute names case-insensitive.""" + user = User.model_validate({"schemas": [str(User.__schema__)], spelling: "bjensen"}) + + assert user.user_name == "bjensen" + + +@pytest.mark.parametrize( + "spelling", ["user-name", "u.s.e.r.n.a.m.e", "user$name", "username "] +) +def test_a_name_differing_by_punctuation_is_another_attribute(spelling): + """The nameChar rule of RFC7643 §2.1 makes $, - and _ part of a name, so dropping them would merge two attributes into one.""" + with pytest.raises(ValidationError) as exc_info: + User.model_validate({"schemas": [str(User.__schema__)], spelling: "bjensen"}) + + assert exc_info.value.errors()[0]["loc"] == (spelling,) + + +def test_an_unknown_attribute_is_named_as_the_peer_spelled_it(): + """A refusal quotes what was sent, so that the peer can find it in its own payload.""" + with pytest.raises(ValidationError) as exc_info: + User.model_validate({"schemas": [str(User.__schema__)], "usr_Name": "bjensen"}) + + assert exc_info.value.errors()[0]["loc"] == ("usr_Name",) + + +def test_a_field_is_read_under_the_alias_it_declares(): + """An alias naming an attribute the camel-cased field name would not spell is honoured.""" + + class Aliased(Resource): + __schema__ = URN("urn:example:2.0:Aliased") + + string_field: str | None = Field(None, alias="string_field") + + obj = Aliased.model_validate({"string_field": "x"}) + + assert obj.string_field == "x" + + +def test_an_alias_wins_over_the_python_name_of_another_field(): + """The name an attribute is serialized under is the one SCIM names it by, where a Python field name is only the spelling pydantic offers.""" + + class Aliased(Resource): + __schema__ = URN("urn:example:2.0:Aliased") + + user_name: str | None = None + legacy: str | None = Field(None, serialization_alias="user_name") + + obj = Aliased.model_validate( + {"schemas": [str(Aliased.__schema__)], "userName": "a", "user_name": "b"} + ) + + assert obj.user_name == "a" + assert obj.legacy == "b" + + +def test_a_constructor_keyword_reaches_the_field_the_attribute_name_designates(): + """A keyword is resolved as a payload key is, so an alias covering it takes it.""" + + class Aliased(Resource): + __schema__ = URN("urn:example:2.0:Aliased") + + user_name: str | None = None + legacy: str | None = Field(None, serialization_alias="user_name") + + obj = Aliased(user_name="x") + + assert obj.legacy == "x" + assert obj.user_name is None + + +def test_two_fields_cannot_answer_to_one_attribute_name(): + """A model whose fields share an attribute name is refused where it is written, no payload key being able to reach both.""" + with pytest.raises(TypeError, match="two fields answering"): + + class Ambiguous(Resource): + __schema__ = URN("urn:example:2.0:Ambiguous") + + display_name: str | None = None + legacy: str | None = Field(None, serialization_alias="displayName") + + +def test_an_extension_is_named_by_its_field_as_well_as_by_its_urn(): + """An extension answers to its class name, which is what a dump without aliases carries.""" + extended = User[EnterpriseUser]( + user_name="bjensen", EnterpriseUser=EnterpriseUser(department="Sales") + ) + + assert extended[EnterpriseUser].department == "Sales" + + revalidated = User[EnterpriseUser].model_validate( + extended.model_dump(scim_ctx=None) + ) + + assert revalidated[EnterpriseUser].department == "Sales" + + +def test_a_payload_naming_one_attribute_twice_keeps_the_last_spelling(): + """RFC7643 §2.1 makes two cases of one name the same attribute, so the payload assigns it twice.""" + user = User.model_validate( + {"schemas": [str(User.__schema__)], "userName": "first", "USERNAME": "last"} + ) + + assert user.user_name == "last" + + +def test_the_name_a_field_is_serialized_under_falls_back_on_its_camel_case(): + """Every model carries an alias generator, so the fallback answers for a field defined without one.""" + + class Bare(BaseModel): + model_config = {} + + user_name: str | None = None + + assert Bare._scim_name("user_name") == "userName" + + +def test_an_alias_naming_a_place_in_the_payload_names_no_attribute(): + """An AliasPath reaches into a payload rather than naming an attribute, so it adds no name a peer may use.""" + + class Nested(Resource): + __schema__ = URN("urn:example:2.0:Nested") + + value: str | None = Field(None, validation_alias=AliasPath("outer", "inner")) + + assert "outer" not in Nested.__scim_info__.field_by_name + assert Nested.__scim_info__.validation_names["value"] == "value" diff --git a/tests/test_reference.py b/tests/test_reference.py index d346666..bd74165 100644 --- a/tests/test_reference.py +++ b/tests/test_reference.py @@ -171,7 +171,7 @@ def test_reference_json_schema_generation(): """Test that models with Reference fields can generate JSON Schema.""" schema = ReferenceTestModel.model_json_schema() assert schema["type"] == "object" - assert "uriref" in schema["properties"] - assert "extref" in schema["properties"] - assert "resourceref" in schema["properties"] - assert "multiref" in schema["properties"] + assert "uriRef" in schema["properties"] + assert "extRef" in schema["properties"] + assert "resourceRef" in schema["properties"] + assert "multiRef" in schema["properties"] From 7767ec918e00632630e97f18c3c1fb0b3cc0e3dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Thu, 17 Sep 2026 10:31:11 +0200 Subject: [PATCH 3/6] fix: build a field for every attribute a schema declares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two SCIM names may yield one Python name, as employee_id and employeeId both yield employee_id, and the comprehension building the fields let the last one win: a dump then reported one value twice while losing the other. Each of them now gets a field, the attribute already spelled as that name keeping it and the others being held under their SCIM name, which the order of declaration cannot change. A schema declaring two names that only differ by case is still refused, RFC7643 §2.1 making them one attribute. --- doc/changelog.rst | 11 ++ scim2_models/resources/schema.py | 77 ++++++++--- tests/test_dynamic_resources.py | 211 +++++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+), 15 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 5dd5bd6..e7c1e79 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -65,6 +65,11 @@ Added Changed ^^^^^^^ +- :meth:`Resource.from_schema ` and + :meth:`Extension.from_schema ` refuse a schema declaring + two attributes whose names only differ by case, and report both. + :rfc:`RFC7643 §2.1 <7643#section-2.1>` makes them one attribute, so such a schema describes + it twice. :issue:`166` - Attribute names are matched case-insensitively, and nothing else. The ``nameChar`` rule of :rfc:`RFC7643 §2.1 <7643#section-2.1>` makes ``$``, ``-`` and ``_`` part of a name, so ``{"user-name": "x"}``, which 0.7 read as ``userName``, is now an unknown attribute that @@ -130,6 +135,12 @@ Deprecated Fixed ^^^^^ +- A schema declaring several attributes that yield one Python name builds a field for each of + them: the attribute already spelled as that name keeps it, and the others are held under their + SCIM name. ``employee_id`` and ``employeeId`` used to share one field, so a dump reported one + value twice and lost the other. An attribute is read under the name SCIM gives it, as in + ``resource["employeeId"]``, so a field name that is no Python identifier costs nothing. + :issue:`166` - A pydantic error spells the attribute as SCIM does, ``userName`` and ``$ref`` where it used to report ``username`` and ``ref``, and so does the JSON schema a model publishes, which FastAPI reads to document a request body. :issue:`166` diff --git a/scim2_models/resources/schema.py b/scim2_models/resources/schema.py index 771eb9c..378bca1 100644 --- a/scim2_models/resources/schema.py +++ b/scim2_models/resources/schema.py @@ -1,4 +1,5 @@ import re +from collections import Counter from datetime import datetime from enum import Enum from typing import Annotated @@ -46,32 +47,78 @@ def _make_python_identifier(identifier: str) -> str: return sanitized +def _field_names(attributes: "list[Attribute]") -> list[str]: + """Return the Python name each attribute is held under. + + Two SCIM names may yield one Python name, as ``employee_id`` and + ``employeeId`` both yield ``employee_id``. The one already spelled as that + name keeps it and the others are held under their SCIM name, so that no + attribute is dropped and the order the schema declares them in changes + nothing. Such a name is no Python identifier when it carries a dash, which + costs nothing: an attribute is read under the name SCIM gives it, as in + ``resource["employee-Id"]``. + """ + natural = [ + to_snake(_make_python_identifier(attr.name or "")) for attr in attributes + ] + shared = Counter(natural) + return [ + name if shared[name] == 1 or name == attr.name else attr.name or "" + for attr, name in zip(attributes, natural, strict=True) + ] + + +def _python_attributes( + attributes: "list[Attribute] | None", declared_by: str +) -> dict[str, Any]: + """Return the fields a schema or a complex attribute declares. + + ``declared_by`` is what declares them, quoted by the error. Two + attributes whose names only differ by case are refused: RFC7643 §2.1 makes + them one attribute, so a schema declaring both describes it twice. + """ + declared = [] + named: dict[str, str] = {} + for attr in attributes or []: + if not attr.name: + continue + + claimed = named.get(attr.name.lower()) + if claimed is not None: + raise ValueError( + f"{declared_by} declares {claimed!r} and {attr.name!r}, " + f"which name the same attribute" + ) + named[attr.name.lower()] = attr.name + declared.append(attr) + + return { + field_name: attr._to_python() + for field_name, attr in zip(_field_names(declared), declared, strict=True) + } + + def _make_python_model( obj: Union["Schema", "Attribute"], base: type[T], ) -> type[T]: """Build a Python model from a Schema or an Attribute object.""" - if isinstance(obj, Attribute): - pydantic_attributes = { - to_snake(_make_python_identifier(attr.name)): attr._to_python() - for attr in (obj.sub_attributes or []) - if attr.name - } - - else: - pydantic_attributes = { - to_snake(_make_python_identifier(attr.name)): attr._to_python() - for attr in (obj.attributes or []) - if attr.name - } - if not obj.name: raise ValueError("Schema or Attribute 'name' must be defined") + if isinstance(obj, Attribute): + pydantic_attributes = _python_attributes( + obj.sub_attributes, f"the attribute {obj.name!r}" + ) + else: + pydantic_attributes = _python_attributes( + obj.attributes, f"the schema {obj.id or obj.name}" + ) + model_name = to_pascal(to_snake(obj.name)) model = cast( type[T], - create_model(model_name, __base__=base, **pydantic_attributes), # type: ignore[call-overload] + create_model(model_name, __base__=base, **pydantic_attributes), ) if isinstance(obj, Schema) and obj.id: diff --git a/tests/test_dynamic_resources.py b/tests/test_dynamic_resources.py index e00e778..39e0f61 100644 --- a/tests/test_dynamic_resources.py +++ b/tests/test_dynamic_resources.py @@ -3,6 +3,7 @@ import weakref from typing import Union +import pytest from pydantic import Base64Bytes from scim2_models.annotations import CaseExact @@ -2904,3 +2905,213 @@ def test_a_model_built_at_runtime_is_collected_once_it_is_dropped(): gc.collect() assert reference() is None + + +def test_a_schema_declaring_two_attributes_that_differ_by_case_is_refused(): + """RFC7643 §2.1 makes two cases of one name the same attribute, so a schema declaring both describes one attribute twice.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + {"name": "userName", "type": "string", "multiValued": False}, + {"name": "username", "type": "string", "multiValued": False}, + ], + } + ) + + with pytest.raises(ValueError, match="name the same attribute"): + Resource.from_schema(schema) + + +@pytest.mark.parametrize( + "declared", [["employee_id", "employeeId"], ["employeeId", "employee_id"]] +) +def test_two_attributes_yielding_one_python_name_each_get_a_field(declared): + """The attribute already spelled as that name keeps it, whichever order the schema declares them in, and the other is held under its SCIM name.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + {"name": name, "type": "string", "multiValued": False} + for name in declared + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Ambiguous"], + "employee_id": "snake", + "employeeId": "camel", + } + ) + + assert obj["employee_id"] == "snake" + assert obj["employeeId"] == "camel" + assert obj.model_dump() == { + "schemas": ["urn:example:2.0:Ambiguous"], + "employee_id": "snake", + "employeeId": "camel", + } + + +def test_attributes_none_of_which_is_spelled_as_its_python_name_keep_their_scim_name(): + """A SCIM name carrying a dash is no Python identifier, which costs nothing since an attribute is read under the name SCIM gives it.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + {"name": "employeeId", "type": "string", "multiValued": False}, + {"name": "employee-Id", "type": "string", "multiValued": False}, + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Ambiguous"], + "employeeId": "camel", + "employee-Id": "dashed", + } + ) + + assert obj["employeeId"] == "camel" + assert obj["employee-Id"] == "dashed" + + +def test_a_complex_attribute_declaring_two_sub_attributes_that_differ_by_case_is_refused(): + """A sub-attribute answers for the same rules as an attribute of the schema itself.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + { + "name": "address", + "type": "complex", + "multiValued": False, + "subAttributes": [ + {"name": "postalCode", "type": "string"}, + {"name": "postalcode", "type": "string"}, + ], + } + ], + } + ) + + with pytest.raises(ValueError, match="the attribute 'address' declares"): + Resource.from_schema(schema) + + +def test_two_sub_attributes_yielding_one_python_name_each_get_a_field(): + """A sub-attribute is held under its SCIM name where an attribute would be.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + { + "name": "address", + "type": "complex", + "multiValued": False, + "subAttributes": [ + {"name": "postal_code", "type": "string"}, + {"name": "postalCode", "type": "string"}, + ], + } + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Ambiguous"], + "address": {"postal_code": "snake", "postalCode": "camel"}, + } + ) + + assert obj["address.postal_code"] == "snake" + assert obj["address.postalCode"] == "camel" + + +def test_attribute_names_keep_the_punctuation_they_carry(): + """The nameChar rule of RFC7643 §2.1 makes - and $ part of a name, so two names that differ by one are two attributes.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Punctuated", + "name": "Punctuated", + "attributes": [ + {"name": "employee-id", "type": "string", "multiValued": False}, + {"name": "employeeId", "type": "string", "multiValued": False}, + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Punctuated"], + "employee-id": "dashed", + "employeeId": "camel", + } + ) + + assert obj.model_dump() == { + "schemas": ["urn:example:2.0:Punctuated"], + "employee-id": "dashed", + "employeeId": "camel", + } + + +def test_an_attribute_without_a_name_builds_no_field(): + """RFC7643 §7 makes the name mandatory, and nothing can be built from an attribute missing it.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Nameless", + "name": "Nameless", + "attributes": [{"type": "string", "multiValued": False}], + } + ) + Model = Resource.from_schema(schema) + + assert set(Model.model_fields) == set(Resource.model_fields) + + +def test_the_three_spellings_of_one_python_name_each_get_a_field(): + """A schema may declare names that a dash, an underscore and a capital tell apart, and each of them keeps its value.""" + schema = Schema.model_validate( + { + "id": "urn:example:2.0:Ambiguous", + "name": "Ambiguous", + "attributes": [ + {"name": name, "type": "string", "multiValued": False} + for name in ("employee-id", "employee_id", "employeeId") + ], + } + ) + Model = Resource.from_schema(schema) + + obj = Model.model_validate( + { + "schemas": ["urn:example:2.0:Ambiguous"], + "employee-id": "dashed", + "employee_id": "snake", + "employeeId": "camel", + } + ) + + assert obj.model_dump() == { + "schemas": ["urn:example:2.0:Ambiguous"], + "employee-id": "dashed", + "employee_id": "snake", + "employeeId": "camel", + } + # The Python name of the dashed attribute is no attribute name, so the + # spelling that only differs from employeeId by its case reaches that one. + assert obj["EMPLOYEEID"] == "camel" From f2ff3c0196f38ad33dde518ad6befabd08cd5855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Thu, 17 Sep 2026 10:31:37 +0200 Subject: [PATCH 4/6] fix: accept a $ in any position of an attribute name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nameChar rule of RFC7643 §2.1 lists $ among the characters a name is made of, and errata 8924 adds the leading one of $ref, but the pattern the grammar interpolates only accepted it in first position, so an attribute a schema declares as user$name could be read from a payload yet named by no path nor filter. The keyword lookaheads widen accordingly, so an attribute named eq$x is still not read as an operator. --- doc/changelog.rst | 2 ++ scim2_models/path/expressions.py | 4 ++-- scim2_models/path/grammar.py | 16 ++++++++-------- tests/test_filter_grammar.py | 8 +++++++- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index e7c1e79..16d249e 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -135,6 +135,8 @@ Deprecated Fixed ^^^^^ +- A filter or a path accepts a ``$`` anywhere in an attribute name, as ``nameChar`` allows. Only + a leading one went through. :issue:`166` - A schema declaring several attributes that yield one Python name builds a field for each of them: the attribute already spelled as that name keeps it, and the others are held under their SCIM name. ``employee_id`` and ``employeeId`` used to share one field, so a dump reported one diff --git a/scim2_models/path/expressions.py b/scim2_models/path/expressions.py index bde35ac..6546ed0 100644 --- a/scim2_models/path/expressions.py +++ b/scim2_models/path/expressions.py @@ -16,8 +16,8 @@ else: # pragma: no cover Template = None -_ATTR_NAME = r"\$?[A-Za-z][A-Za-z0-9_-]*" -"""The ``ATTRNAME`` rule, with the leading ``$`` of ``$ref`` that errata 8924 adds.""" +_ATTR_NAME = r"\$?[A-Za-z][A-Za-z0-9$_-]*" +"""The ``ATTRNAME`` rule, ``nameChar`` and the leading ``$`` of ``$ref`` that errata 8924 adds.""" _URN = r"urn:[A-Za-z0-9][A-Za-z0-9._-]*(?::[A-Za-z0-9][A-Za-z0-9._-]*)*" """A schema URN, the only ``URI`` an attribute path accepts.""" diff --git a/scim2_models/path/grammar.py b/scim2_models/path/grammar.py index 8cd13c6..4b16060 100644 --- a/scim2_models/path/grammar.py +++ b/scim2_models/path/grammar.py @@ -89,14 +89,14 @@ // Trailing word boundaries are mandatory: without them an attribute named // "never" lexes as the "ne" operator followed by "ver". -COMPARE_OP.2: /(?:eq|ne|co|sw|ew|gt|lt|ge|le)(?![A-Za-z0-9_-])/i -PR.2: /pr(?![A-Za-z0-9_-])/i -TRUE.2: /true(?![A-Za-z0-9_-])/i -FALSE.2: /false(?![A-Za-z0-9_-])/i -NULL.2: /null(?![A-Za-z0-9_-])/i -_AND.3: /and(?![A-Za-z0-9_-])/i -_OR.3: /or(?![A-Za-z0-9_-])/i -_NOT.3: /not(?![A-Za-z0-9_-])/i +COMPARE_OP.2: /(?:eq|ne|co|sw|ew|gt|lt|ge|le)(?![A-Za-z0-9$_-])/i +PR.2: /pr(?![A-Za-z0-9$_-])/i +TRUE.2: /true(?![A-Za-z0-9$_-])/i +FALSE.2: /false(?![A-Za-z0-9$_-])/i +NULL.2: /null(?![A-Za-z0-9$_-])/i +_AND.3: /and(?![A-Za-z0-9$_-])/i +_OR.3: /or(?![A-Za-z0-9$_-])/i +_NOT.3: /not(?![A-Za-z0-9$_-])/i // Both literals follow the JSON rules the ABNF refers to, escapes included, so // that an invalid one is a syntax error located by the lexer rather than a diff --git a/tests/test_filter_grammar.py b/tests/test_filter_grammar.py index 457750b..9540d01 100644 --- a/tests/test_filter_grammar.py +++ b/tests/test_filter_grammar.py @@ -522,7 +522,7 @@ def test_quoting_a_value_yields_a_literal_the_grammar_reads_back(value, literal) @pytest.mark.parametrize( "name", - ['userName eq "admin" or userName', "", "1abc", "name.given", 'a"b', "foo$bar"], + ['userName eq "admin" or userName', "", "1abc", "name.given", 'a"b', "foo bar"], ) def test_an_attribute_path_refuses_what_is_not_a_name(name): """A node built by hand carries a name the grammar reads as one, or nothing.""" @@ -530,6 +530,12 @@ def test_an_attribute_path_refuses_what_is_not_a_name(name): AttrPath(name) +@pytest.mark.parametrize("name", ["$ref", "foo$bar", "foo-bar", "foo_bar", "a$"]) +def test_an_attribute_path_takes_every_name_char(name): + """The nameChar rule of RFC7643 §2.1 makes $, - and _ part of a name.""" + assert AttrPath(name).attr == name + + def test_an_attribute_path_refuses_a_sub_attribute_that_is_not_a_name(): with pytest.raises(ValueError, match="is not an attribute name"): AttrPath("emails", sub_attr='type eq "work"') From bf75f53b250003d2f36f501b47a1df2525850da4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Thu, 17 Sep 2026 10:32:01 +0200 Subject: [PATCH 5/6] fix: check the constraints of an extension a pathless PATCH names The constraint checks of a pathless operation resolved each attribute name themselves and gave up on the ones they could not, which an extension named by its schema URN always was, so an operation unassigning an extension declared required was answered success. The caller now resolves each name once, before refusing the ones the schema does not declare, and hands the checks the field that holds the constraints. --- doc/changelog.rst | 4 ++++ scim2_models/messages/patch_op.py | 37 +++++-------------------------- 2 files changed, 10 insertions(+), 31 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 16d249e..1b15939 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -135,6 +135,10 @@ Deprecated Fixed ^^^^^ +- A PATCH operation carrying no ``path`` that unassigns an extension declared + :attr:`Required.true ` is refused. The extension was named by its + URN, which the constraint checks did not resolve, so they found no constraint to answer for. + :issue:`166` - A filter or a path accepts a ``$`` anywhere in an attribute name, as ``nameChar`` allows. Only a leading one went through. :issue:`166` - A schema declaring several attributes that yield one Python name builds a field for each of diff --git a/scim2_models/messages/patch_op.py b/scim2_models/messages/patch_op.py index de505df..6c8b6e2 100644 --- a/scim2_models/messages/patch_op.py +++ b/scim2_models/messages/patch_op.py @@ -70,24 +70,6 @@ def _targeted_attributes(value: Any) -> dict[str, Any]: return value if isinstance(value, dict) else {} -def _names_a_declared_target( - resource_class: type[Resource[Any]], attr_name: str -) -> bool: - """Whether a pathless operation names something the model declares. - - The ``value`` of a pathless operation names attributes of the resource, and - an extension by its schema URN, under which it names the attributes of that - extension. - """ - if _resolved_field(resource_class, attr_name) is not None: - return True - - lowered = attr_name.lower() - return any( - schema.lower() == lowered for schema in resource_class.get_extension_models() - ) - - def _resolved_field(resource_class: type[BaseModel], attr_name: str) -> str | None: """Return the Python field a SCIM attribute name designates. @@ -254,10 +236,7 @@ def _validate_mutability( resource instance and is enforced at runtime in PatchOp._check_immutable. """ - if (field := _resolved_field(resource_class, field_name)) is None: - return - - mutability = resource_class.get_field_annotation(field, Mutability) + mutability = resource_class.get_field_annotation(field_name, Mutability) if mutability == Mutability.read_only: raise MutabilityException( @@ -288,12 +267,7 @@ def _validate_required_attribute( else: return - # An extension is named by its schema URN, which is no field of the - # resource and carries no annotation of its own to check. - if (field := _resolved_field(resource_class, field_name)) is None: - return - - required = resource_class.get_field_annotation(field, Required) + required = resource_class.get_field_annotation(field_name, Required) # RFC7644 §3.5.2.2 has a server answer "mutability" when a required # attribute is removed or becomes unassigned. @@ -426,7 +400,8 @@ def _validate_operations(self, info: ValidationInfo) -> Self: # the attributes to write. Each of them is a target of its own, # and answers to §3.5.2 as a named path does. for attr_name, written in _targeted_attributes(operation.value).items(): - if not _names_a_declared_target(resource_class, attr_name): + field_name = _resolved_field(resource_class, attr_name) + if field_name is None: # §3.5.2 has an operation that is not compatible with an # attribute's schema return an error, and §3.12 defines # invalidValue for a value "not compatible with [...] the @@ -434,9 +409,9 @@ def _validate_operations(self, info: ValidationInfo) -> Self: raise InvalidValueException( detail=f"attribute '{attr_name}' is not declared by the resource schema" ).as_pydantic_error() - operation._validate_mutability(resource_class, attr_name) + operation._validate_mutability(resource_class, field_name) operation._validate_required_attribute( - resource_class, attr_name, written + resource_class, field_name, written ) continue From 711da2c55b87d998da612045708f145926625289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Sat, 19 Sep 2026 21:07:54 +0200 Subject: [PATCH 6/6] refactor: replace populate_by_name with validate_by_name Pydantic advises against populate_by_name since 2.11 and will deprecate it in v3, the pair of validate_by_name and validate_by_alias telling apart what the single setting conflated. Both are set, which the documentation states to be strictly equivalent to what the models declared until now. --- scim2_models/base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scim2_models/base.py b/scim2_models/base.py index 52f1bbc..e60c9a2 100644 --- a/scim2_models/base.py +++ b/scim2_models/base.py @@ -259,7 +259,8 @@ class BaseModel(PydanticBaseModel): serialization_alias=_to_camel, ), validate_assignment=True, - populate_by_name=True, + validate_by_name=True, + validate_by_alias=True, use_attribute_docstrings=True, extra="forbid", )