diff --git a/doc/changelog.rst b/doc/changelog.rst index 051b5f26..e2e06d20 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -4,6 +4,12 @@ Changelog [Unreleased] ------------ +Added +^^^^^ +- :attr:`ScimPolicy.unmatched_path_filter ` can + make a PATCH ``add`` or ``replace`` create the entry its path filter describes when no entry + matches, as Microsoft Entra ID expects. By default, the operation still fails with ``noTarget``. + Changed ^^^^^^^ - Python 3.11 is now the minimum supported version. @@ -11,6 +17,60 @@ Changed :class:`str` and f-strings give their value, ``readOnly`` rather than ``Mutability.read_only``. - In a model built from a schema, an attribute named after a member of the model, such as ``copy``, is held as ``copy_``. Its SCIM name is unchanged. +- A PATCH ``add`` whose path filter matches no entry, such as ``emails[type eq "work"].value`` + on a user without a work email, now fails with ``noTarget`` instead of silently doing nothing. + :meth:`Path.set ` raises :class:`~scim2_models.NoTargetException` in + that case when strict. +- A PATCH ``add`` or ``replace`` on a filtered path, such as ``emails[type eq "work"]``, merges + its value into the matching entries instead of replacing them + (:rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>`). The entries are updated in place, so the + immutable sub-attributes of a group member cannot be changed this way. + +Fixed +^^^^^ +- A :class:`~scim2_models.PatchOperation` with a null value keeps it when dumped. A ``replace`` + that clears its target used to be sent without a value. +- Setting a null value under an unset complex attribute or extension no longer creates an empty + one, and no longer reports the resource as modified. +- A :class:`~scim2_models.PatchOp` with no operation, or with an operation other than ``add``, + ``remove`` and ``replace``, fails with ``invalidValue`` instead of a validation error without + ``scimType``. +- A PATCH ``add`` or ``replace`` on a complex attribute keeps the sub-attributes its value leaves + out, instead of replacing the whole attribute (:rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>`). +- A PATCH ``add`` without a path adds to the multi-valued attributes in its value, like an + ``add`` with a path, instead of replacing their values. +- A key of a PATCH value can be an attribute path, such as ``name.givenName`` or + ``urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:employeeNumber``, as Microsoft + Entra ID and its SCIM Validator send. It used to be rejected as an undeclared attribute. +- Undeclared attributes and sub-attributes in a PATCH value follow + :attr:`ScimPolicy.unknown `. +- PATCH checks immutable attributes at every level and in every operation, including operations + with no path, an empty path or a schema URN, and the removal of an extension. For example, the + ``value`` of a group member can be added and removed, but not changed + (:rfc:`RFC7643 §4.2 <7643#section-4.2>`). Setting a first value with ``replace``, or writing + back the current value, is accepted. +- PATCH checks read-only attributes at every level, such as the ``displayName`` of the + enterprise ``manager``. A path to one is rejected. A value that contains one is + rejected only if it changes it, so an attribute sent back as it was read is accepted. Okta, for + example, sends back the ``id`` of a group it renames. +- PATCH rejects a change to a required attribute only when it leaves the attribute unset + (:rfc:`RFC7644 §3.5.2.2 <7644#section-3.5.2.2>`). Removing some values of a required + multi-valued attribute, removing a sub-attribute of a required complex attribute, or adding an + empty list is now accepted. Required sub-attributes and extensions are checked too. +- A PATCH ``replace`` without a value fails with ``invalidValue`` instead of clearing its + target. So does an operation whose path targets the resource or an extension with a value that + is not an object, which used to be ignored. +- A PATCH ``replace`` that sets several ``primary`` entries fails with ``invalidValue``, like + ``add`` already did, instead of keeping one of them at random. +- :meth:`PatchOp.patch ` raises + :class:`~scim2_models.InvalidValueException` when the attribute rejects a value, instead of a + pydantic :class:`~pydantic.ValidationError` without ``scimType``. +- :meth:`PatchOp.patch ` no longer reports a resource as modified + when an operation writes a complex or multi-valued value it already has. +- A PATCH path to an undeclared attribute follows + :attr:`ScimPolicy.unknown `, like a value does. With ``ignore`` + or ``keep``, the operation changes nothing instead of failing the whole patch with + ``invalidPath``. An undeclared sub-attribute in a filter still fails with ``invalidFilter``. Security ^^^^^^^^ @@ -21,7 +81,6 @@ Security - :func:`~scim2_models.get_model_by_payload` matches no model when ``schemas`` is not a list of strings. - [0.8.2] - 2026-09-25 -------------------- diff --git a/doc/explanation/patch.rst b/doc/explanation/patch.rst index 7761ce56..549bd824 100644 --- a/doc/explanation/patch.rst +++ b/doc/explanation/patch.rst @@ -13,19 +13,82 @@ Validation and application Validating a PATCH message in :attr:`~scim2_models.Context.RESOURCE_PATCH_REQUEST` rejects what the message alone settles: a missing operation value, a ``remove`` without a path or carrying a -value, a read-only target, and an operation that would unassign a required attribute. +value, a path to a read-only attribute, and an operation that removes a required +attribute or sets it to an empty value. :meth:`~scim2_models.PatchOp.patch` then applies the message to the stored resource. Operations run in their listed order. This is where an immutable value can be compared with the value it -replaces, and where the method reports whether any operation changed the resource. Splitting the -two keeps a parsed :class:`~scim2_models.PatchOp` useful before the resource is loaded. +replaces, and where the method reports whether any operation changed the resource. It also +rejects an operation that unassigns a required attribute in another way, such as removing its +last entry through a filter. Read-only attributes in the value are checked there too. A client +that sends back the ``id`` or ``meta`` it read is accepted, since +:rfc:`RFC7643 §3.1 <7643#section-3.1>` says to ignore them. A client that changes them gets a +``mutability`` error. Splitting the two keeps a parsed :class:`~scim2_models.PatchOp` useful +before the resource is loaded. Operation outcomes ------------------ ``add`` appends a value to a multi-valued attribute rather than replacing its list. ``replace`` -replaces its target, creating an unassigned single-valued complex parent when necessary. -``remove`` removes a selected list entry or unassigns the targeted attribute. +replaces the value of a simple or multi-valued target. ``remove`` removes a selected list entry +or unassigns the targeted attribute. + +A path to a sub-attribute of an absent complex attribute, such as ``name.givenName`` on a user +without a name, creates that attribute. + +A complex attribute is merged rather than replaced. ``add`` and ``replace`` set the +sub-attributes in their value and keep the others, as +:rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>` requires: + +.. doctest:: + + >>> from scim2_models import PatchOp, PatchOperation, User + + >>> user = User(user_name="bjensen", name={"family_name": "Jensen", "given_name": "Barbara"}) + >>> patch = PatchOp[User]( + ... operations=[ + ... PatchOperation( + ... op=PatchOperation.Op.replace_, path="name", value={"givenName": "Babs"} + ... ) + ... ] + ... ) + >>> patch.patch(user) + True + >>> user.name.given_name, user.name.family_name + ('Babs', 'Jensen') + +Operations without a path +------------------------- + +The value of an ``add`` or ``replace`` without a path holds the attributes to write. Each +attribute is handled like an operation with that attribute as its path. A key can also be an +attribute path, such as ``name.givenName`` or the full URN of an extension attribute. Microsoft +Entra ID and its SCIM Validator send such keys. :rfc:`RFC7643 §2.1 <7643#section-2.1>` forbids +dots and colons in attribute names, so such a key cannot be confused with a name: + +.. doctest:: + + >>> from scim2_models import EnterpriseUser, PatchOp, PatchOperation, User + + >>> user = User[EnterpriseUser](user_name="bjensen", name={"family_name": "Jensen"}) + >>> patch = PatchOp[User[EnterpriseUser]]( + ... operations=[ + ... PatchOperation( + ... op=PatchOperation.Op.replace_, + ... value={ + ... "name.givenName": "Barbara", + ... "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:employeeNumber": "42", + ... }, + ... ) + ... ] + ... ) + >>> patch.patch(user) + True + >>> user.name.given_name, user.name.family_name, user[EnterpriseUser].employee_number + ('Barbara', 'Jensen', '42') + +A key with a filter is not read as a path, and neither is a key that matches no declared +attribute. Both follow :attr:`ScimPolicy.unknown `. What a path selects ------------------- @@ -60,6 +123,32 @@ of a multi-valued attribute an operation applies to: >>> [email.value for email in user.emails] ['new@example.com', 'home@example.com'] +When a filter selects whole entries, ``add`` and ``replace`` merge their value into each selected +entry, as they do for a complex attribute. :rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>` says +"all matching record values" are replaced, and keeps the sub-attributes the value does not +specify: + +.. doctest:: + + >>> patch = PatchOp[User]( + ... operations=[ + ... PatchOperation( + ... op=PatchOperation.Op.replace_, + ... path='emails[type eq "home"]', + ... value={"display": "Home"}, + ... ) + ... ] + ... ) + >>> patch.patch(user) + True + >>> user.emails[1].type.value, user.emails[1].value, user.emails[1].display + ('home', 'home@example.com', 'Home') + +Each selected entry is updated in place, not replaced by a new one. So a client can change the +``display`` of a group member without repeating its immutable ``value``. A different ``value`` is +rejected with ``mutability``. To unassign a sub-attribute, give it a null value, or remove it with +a path such as ``emails[type eq "home"].display``. + A selection matching nothing ---------------------------- @@ -100,16 +189,27 @@ nothing changed: >>> patch.patch(user) False -``add`` behaves the same way. :rfc:`RFC7644 §3.5.2.1 <7644#section-3.5.2.1>` does not say what a -selection matching nothing means for it, so the operation is a no-op and not a failure. -`Errata 8097 `_ asks the RFC to say whether ``add`` -accepts a value selection at all, since implementations differ on it. - -.. note:: - - Microsoft Entra ID sends ``add`` operations whose selection matches nothing, and expects the - selected entry to be created. scim2-models does not create it, so an integration serving that - client handles the case before applying the operation. +``add`` fails like ``replace``. :rfc:`RFC7644 §3.5.2.1 <7644#section-3.5.2.1>` does not say what +a selection matching nothing means for it, and Table 9 of :rfc:`RFC7644 §3.12 <7644#section-3.12>` +defines ``noTarget`` for a filter that "yields no match". The filter selects the entries to +write. It does not describe an entry to create. So the operation fails instead of silently doing +nothing. + +Implementations differ here. UnboundID's SCIM 2 SDK, Apache SCIMple and scim-patch create the +entry on ``add``. SCIM-SDK returns ``noTarget``, and so does WSO2 Charon unless the attribute is +unassigned. `Errata 8097 `_ describes the creation +Microsoft Entra ID expects. It is held, with no corrected text. The editor of :rfc:`7644` +`replied `_ that +such an implementation is "going outside the spec at the cost of their interoperability". + +Entra sends such operations to fill an attribute it has not set yet. It sends ``add`` by +default, and ``replace`` when its ``aadOptscim062020`` flag is set. For example, it targets +``emails[type eq "work"].value`` on a user without a work email, and expects a new entry +``{"type": "work", "value": ...}``. +:attr:`ScimPolicy.UnmatchedPathFilter.create ` +creates that entry for both operations. It only works with ``eq`` comparisons joined by ``and``. +The new entry must still match the filter after the value is written, so that later operations +with the same filter find it. See :doc:`../how-to/tolerate-a-nonconformant-peer`. What a remove selects --------------------- @@ -164,9 +264,12 @@ Rejected paths Which ``scimType`` a rejected path answers follows Table 9 of :rfc:`RFC7644 §3.12 <7644#section-3.12>`. ``invalidPath`` covers "the ``path`` attribute was invalid or malformed (see Figure 7)". It answers a path the grammar refuses, and an attribute the -model does not declare. Table 9 lists ``invalidFilter`` as applying to a "PATCH (Path Filter)", +model does not declare, unless :attr:`ScimPolicy.unknown ` drops +it. With ``ignore`` or ``keep``, an operation on an undeclared attribute changes +nothing. Table 9 lists ``invalidFilter`` as applying to a "PATCH (Path Filter)", so it answers what goes wrong between the brackets. That covers an unknown sub-attribute, a -comparison the attribute cannot take, and a selection over an attribute holding a single value. +comparison the attribute cannot take, and a selection over an attribute holding a single value, +under any policy. Building a patch rather than applying one ----------------------------------------- diff --git a/doc/explanation/policies.rst b/doc/explanation/policies.rst index d314f2e6..759304df 100644 --- a/doc/explanation/policies.rst +++ b/doc/explanation/policies.rst @@ -43,16 +43,30 @@ Telling the two apart would take a notion of role, and a model has none — a re by the client that wrote it as readily as by the server that received it, so the context cannot stand in for one. +Tolerances that need no setting +------------------------------- + +A setting is worth its cost when a tolerance could confuse one payload with another. Some cases +the specification does not cover carry no such risk, and scim2-models accepts them without a +setting. + +A key of a PATCH value that is an attribute path, such as ``name.givenName``, is read as that +path. Microsoft Entra ID and its SCIM Validator send such keys. A strict reading would reject them +as unknown attributes. But :rfc:`RFC7643 §2.1 <7643#section-2.1>` forbids dots and colons in +attribute names, so the key cannot mean anything else. A setting that rejects it by default would +only break that client. :doc:`patch` describes how such a key is read. + What a policy leaves alone -------------------------- -**PATCH paths stay strict.** An operation whose ``path`` names an attribute no model declares is -refused with ``invalidPath``, whatever the policy says. Path resolution and unknown attributes are -two separate mechanisms, and making them uniform would take a third. The default that would come -out of it is the wrong one: a server would answer 200 to a modification it never applied, where -:rfc:`RFC7644 §3.5.2 <7644#section-3.5.2>` asks for an error. Inside the body of a resource the -trade is different, since dropping one unknown attribute still lands everything the peer and the -model both knew. +**PATCH filters stay strict.** Under :attr:`~scim2_models.ScimPolicy.Unknown.ignore` or +:attr:`~scim2_models.ScimPolicy.Unknown.keep`, a PATCH operation on an attribute no model +declares changes nothing, like an undeclared attribute in its value. A filter that +compares such an attribute is still rejected with ``invalidFilter``, and a malformed path with +``invalidPath``. In both cases, there is no attribute the policy could drop. Dropping an +operation has a cost: the server returns 200 for a change it never applied, where +:rfc:`RFC7644 §3.5.2 <7644#section-3.5.2>` asks for an error. A tolerant policy already accepts +that cost for the body of a resource, and the strict default keeps the error. **Building a model in Python stays strict.** ``User(bogus=1)`` and ``user.bogus = 1`` raise under every policy. Pydantic only offers a hook for extra keys during validation, so a keyword argument diff --git a/doc/explanation/scim-contexts.rst b/doc/explanation/scim-contexts.rst index 7fa15002..f9287df4 100644 --- a/doc/explanation/scim-contexts.rst +++ b/doc/explanation/scim-contexts.rst @@ -17,12 +17,13 @@ SCIM declares rules on attributes rather than on whole Python models: ``required`` Requires a value in creation and replacement requests. A partial PATCH request does not have - to repeat every required attribute. + to repeat every required attribute, but it cannot leave one unassigned. ``mutability`` Controls whether a client may write an attribute. ``readOnly`` is removed from creation and - replacement payloads, while a PATCH targeting it is rejected. ``immutable`` needs the current - resource value, so replacement and PATCH enforce it as the change is applied. + replacement payloads. A PATCH path to it is rejected. A PATCH value that contains it + is rejected only if it changes the current value. ``immutable`` needs the current resource + value, so replacement and PATCH enforce it as the change is applied. ``returned`` Controls response projection. ``always`` cannot be excluded, ``never`` cannot be returned, @@ -55,8 +56,8 @@ Where rules are enforced Creation and replacement validation check required values. Response serialization applies ``returned`` and attribute projection. The replacement workflow calls :meth:`~scim2_models.Resource.replace` after parsing so it can compare immutable values with the -stored resource. PATCH first validates the operation message, then checks immutable values while -applying it. +stored resource. PATCH first validates the operation message, then checks immutable, read-only and +required values on the result of each operation. :doc:`../how-to/validate-and-serialize` presents the validation and serialization procedure for each resource operation. diff --git a/doc/how-to/build-a-patch.rst b/doc/how-to/build-a-patch.rst index 34c4ddd9..11eb83c6 100644 --- a/doc/how-to/build-a-patch.rst +++ b/doc/how-to/build-a-patch.rst @@ -20,8 +20,9 @@ Pass the state the peer holds first, then the state it should hold: [{'op': 'replace', 'path': 'name.givenName', 'value': 'Babs'}] A complex attribute is compared sub-attribute by sub-attribute, and each one gets its own path. -Targeting ``name`` as a whole would replace it entirely and drop what the operation does not -carry. +Per :rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>`, a ``replace`` on ``name`` keeps the +sub-attributes it does not carry. But some peers replace the whole attribute and drop them. One +path per sub-attribute gives the same result on every peer. Leave alone what the wanted state does not name ----------------------------------------------- diff --git a/doc/how-to/tolerate-a-nonconformant-peer.rst b/doc/how-to/tolerate-a-nonconformant-peer.rst index 3a983dd4..6bd52eed 100644 --- a/doc/how-to/tolerate-a-nonconformant-peer.rst +++ b/doc/how-to/tolerate-a-nonconformant-peer.rst @@ -45,6 +45,10 @@ from: An unmodelled extension takes the same route, since a payload names one with a root key whose name is a URN. +Under that policy, a PATCH operation on an undeclared attribute changes +nothing, and the other operations still apply. Pass the policy to +:meth:`PatchOp.patch ` as well as to the validation of the message. + Write unknown attributes back ----------------------------- @@ -111,6 +115,38 @@ which :rfc:`RFC7644 §3.5.2.2 <7644#section-3.5.2.2>` asks for a membership that Entra documents this form as non-conformant, and its ``aadOptscim062020`` tenant flag makes it send a filter path instead. Setting that flag is the other way out. +Create the entry an Entra filter describes +------------------------------------------ + +Microsoft Entra ID fills an attribute it has not set yet through a filter, such as +``emails[type eq "work"].value`` on a user without a work email. The filter matches nothing, and +scim2-models returns ``noTarget``. Set +:attr:`~scim2_models.ScimPolicy.UnmatchedPathFilter.create` to add the entry the filter describes +instead: + +.. doctest:: + + >>> patch = PatchOp[User]( + ... operations=[ + ... PatchOperation( + ... op=PatchOperation.Op.add, + ... path='emails[type eq "work"].value', + ... value="bjensen@example.com", + ... ) + ... ] + ... ) + >>> creating = ScimPolicy(unmatched_path_filter=ScimPolicy.UnmatchedPathFilter.create) + >>> user = User(user_name="bjensen") + >>> patch.patch(user, scim_policy=creating) + True + >>> [(email.type.value, email.value) for email in user.emails] + [('work', 'bjensen@example.com')] + +The setting covers ``add`` and ``replace``, since Entra sends ``replace`` once its +``aadOptscim062020`` tenant flag is set. It only works with ``eq`` comparisons joined by ``and``. +Any other filter still returns ``noTarget``, and so does a filter on an attribute without +sub-attributes. :doc:`../explanation/patch` explains why this is not the default. + State a policy once per request ------------------------------- diff --git a/scim2_models/messages/patch_op.py b/scim2_models/messages/patch_op.py index e3d2a40c..6817dfc0 100644 --- a/scim2_models/messages/patch_op.py +++ b/scim2_models/messages/patch_op.py @@ -1,4 +1,6 @@ +import copy from collections.abc import Iterator +from dataclasses import replace from enum import StrEnum from inspect import isclass from typing import Annotated @@ -9,10 +11,13 @@ from typing import cast from typing import get_origin -from pydantic import BaseModel as PydanticBaseModel from pydantic import Field +from pydantic import SerializationInfo +from pydantic import SerializerFunctionWrapHandler +from pydantic import ValidationError from pydantic import ValidationInfo from pydantic import field_validator +from pydantic import model_serializer from pydantic import model_validator from ..annotations import Mutability @@ -20,20 +25,26 @@ from ..attributes import ComplexAttribute from ..base import BaseModel from ..context import Context +from ..exceptions import InvalidPathException from ..exceptions import InvalidValueException from ..exceptions import MutabilityException from ..exceptions import NoTargetException -from ..exceptions import PathNotFoundException +from ..exceptions import SCIMException +from ..path import CompareOperator +from ..path import Comparison +from ..path import FilterNode +from ..path import LogicalExpr +from ..path import LogicalOperator from ..path import Path from ..path import ScimFilter -from ..path import attribute_host +from ..path.access import _select +from ..path.access import _set_values from ..policy import ScimPolicy from ..policy import _effective_policy from ..policy import _policy from ..resources.resource import Resource from ..urn import URN from ..utils import UNION_TYPES -from ..utils import _find_field_name from .message import Message from .message import _get_resource_class from .message import _ResourceParameterized @@ -41,170 +52,6 @@ ResourceT = TypeVar("ResourceT", bound=Resource[Any]) -def _commit(resource: Any, working: Any) -> None: - """Write a patched copy back onto the resource the caller holds. - - Assignment is bypassed on purpose: ``working`` was built by the very passes - ``validate_assignment`` would run again. - """ - resource.__dict__.clear() - resource.__dict__.update(working.__dict__) - resource.__pydantic_fields_set__.clear() - resource.__pydantic_fields_set__.update(working.__pydantic_fields_set__) - resource.__pydantic_private__ = working.__pydantic_private__ - - -def _targeted_attributes(value: Any) -> dict[str, Any]: - """Return the attributes an operation without a path writes. - - RFC7644 §3.5.2.3 has the ``value`` name them when the path is omitted. A - client building its payload in Python passes a resource, where a server - parses a mapping, and both name the same attributes. Anything else names - none. - """ - if isinstance(value, BaseModel): - # Dumped out of context on purpose: a payload dumped in the PATCH - # context already leaves read-only attributes out, and an operation - # naming one is to be refused rather than quietly trimmed. - return value.model_dump(exclude_unset=True) - return value if isinstance(value, dict) else {} - - -def _resolved_field(resource_class: type[BaseModel], attr_name: str) -> str | None: - """Return the Python field a SCIM attribute name designates. - - Attribute names are case-insensitive per RFC7643 §2.1 and differ from the - field names of the model, so the constraint checks resolve the name instead - of matching it against ``model_fields``. - """ - return _find_field_name(resource_class, attr_name) - - -_ENVELOPE_FIELDS = frozenset({"schemas"}) -"""Fields that carry the payload rather than the state it describes.""" - - -def _asserted_sub_attributes(entries: Any) -> set[str]: - """Return the sub-attributes the entries of a wanted state name.""" - asserted: set[str] = set() - for entry in entries or []: - if isinstance(entry, BaseModel): - asserted |= entry.model_fields_set - return asserted - - -def _projection(entries: Any, asserted: set[str]) -> list[Any]: - """Reduce the entries of a multi-valued attribute to what is worth comparing. - - RFC7643 §2.4 gives no significance to the order of a multi-valued - attribute, so the projections are sorted before comparison. - """ - projected = [ - tuple(sorted((name, getattr(entry, name, None)) for name in asserted)) - if isinstance(entry, BaseModel) - else entry - for entry in entries or [] - ] - return sorted(projected, key=repr) - - -def _operation( - path: str, old: Any, new: Any, mutability: Mutability | None -) -> tuple["PatchOperation.Op", str, Any]: - """Return the operation writing *new* where the current state holds *old*. - - Called once a difference is established. RFC7644 §3.5.2.3 has a service - provider treat a ``replace`` on an unset target as an ``add``, so a single - operation covers both. An immutable attribute is the exception: RFC7644 - §3.5.2 lets a client add a value to one that had none, and nothing else. - """ - if mutability == Mutability.immutable: - if old is not None: - raise MutabilityException( - attribute=path, mutability="immutable", operation="replace" - ) - return PatchOperation.Op.add, path, new - - if new is None or new == []: - return PatchOperation.Op.remove, path, None - - return PatchOperation.Op.replace_, path, new - - -def _diff_multi_valued( - path: str, old: Any, new: Any, mutability: Mutability | None -) -> Iterator[tuple["PatchOperation.Op", str, Any]]: - """Diff a multi-valued attribute, which is replaced as a whole. - - Only the sub-attributes the wanted entries name take part in the - comparison, so the sub-attributes the peer alone maintains do not read as a - difference. When the collection does change it is replaced entirely: - RFC7643 §2.4 gives the entries no identity, so an entry that changed cannot - be told from a removed one and an added one. - """ - asserted = _asserted_sub_attributes(new) - if _projection(old, asserted) == _projection(new, asserted): - return - - yield _operation(path, old, new, mutability) - - -def _diff_sub_object( - prefix: str, - path: str, - old: Any, - new: Any, - mutability: Mutability | None, -) -> Iterator[tuple["PatchOperation.Op", str, Any]]: - """Diff a complex attribute or an extension, one sub-attribute at a time.""" - if new is not None: - yield from _diff(old, new, prefix) - return - - if old is not None: - yield _operation(path, old, None, mutability) - - -def _diff( - before: Any, after: Any, prefix: str = "" -) -> Iterator[tuple["PatchOperation.Op", str, Any]]: - """Yield the operations turning *before* into *after*. - - Only the attributes *after* names are candidates: what a wanted state never - mentions is left to the peer. Attributes are visited in declaration order, - so a diff is reproducible. - """ - model = type(after) - info = model.__scim_info__ - for field_name in model.model_fields: - if field_name not in after.model_fields_set: - continue - - if field_name in _ENVELOPE_FIELDS: - continue - - mutability = model.get_field_annotation(field_name, Mutability) - if mutability == Mutability.read_only: - continue - - old = getattr(before, field_name, None) if before is not None else None - new = getattr(after, field_name, None) - path = f"{prefix}{model._scim_name(field_name)}" - - if model.get_field_multiplicity(field_name): - yield from _diff_multi_valued(path, old, new, mutability) - - elif field_name in info.extensions: - urn = info.attribute_urns[field_name] - yield from _diff_sub_object(f"{urn}:", urn, old, new, mutability) - - elif field_name in info.complex_fields: - yield from _diff_sub_object(f"{path}.", path, old, new, mutability) - - elif old != new: - yield _operation(path, old, new, mutability) - - class PatchOperation(ComplexAttribute, Generic[ResourceT]): class Op(StrEnum): replace_ = "replace" @@ -226,95 +73,47 @@ class Op(StrEnum): """The "path" attribute value is a String containing an attribute path describing the target of the operation.""" - def _validate_mutability( - self, resource_class: type[BaseModel], field_name: str - ) -> None: - """Validate mutability constraints at parse-time. - - Only scim2_models.Mutability.read_only is validated here. - scim2_models.Mutability.immutable validation requires access to the - resource instance and is enforced at runtime in - PatchOp._check_immutable. - """ - mutability = resource_class.get_field_annotation(field_name, Mutability) - - if mutability == Mutability.read_only: - raise MutabilityException( - attribute=field_name, mutability="readOnly", operation=self.op.value - ).as_pydantic_error() + value: Any | None = None - def _validate_required_attribute( - self, - resource_class: type[BaseModel], - field_name: str, - written: Any = None, - ) -> None: - """Refuse an operation that would leave a required attribute unassigned. + @model_validator(mode="after") + def _validate_request_operation(self, info: ValidationInfo) -> Self: + """Reject an operation that lacks a member RFC7644 §3.5.2 requires. - ``written`` is the value the operation writes to that attribute, which - an operation without a path takes from its ``value``. + A remove needs a path. An add needs a value, and null is not one. A + replace needs a value, and null unassigns the target. """ - # RFC7643 §2.5 makes a null value, an empty array and an unassigned - # attribute equivalent in state, so writing one of those unassigns the - # attribute as surely as a remove does. - if self.op == PatchOperation.Op.remove: - detail = "required attribute cannot be removed" - elif self.op in ( - PatchOperation.Op.replace_, - PatchOperation.Op.add, - ) and written in (None, []): - detail = "required attribute cannot be unassigned" - else: - return - - 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. - if required == Required.true: - raise MutabilityException( - detail=detail, attribute=field_name, operation=self.op.value - ).as_pydantic_error() - - @model_validator(mode="after") - def _validate_operation_requirements(self, info: ValidationInfo) -> Self: - """Validate operation requirements according to RFC 7644.""" - # Only validate in PATCH request context - scim_ctx = info.context.get("scim") if info.context else None - if scim_ctx != Context.RESOURCE_PATCH_REQUEST: + if not _in_patch_request(info): return self - - # RFC 7644 Section 3.5.2.2: "If 'path' is unspecified, the operation - # fails with HTTP status code 400 and a 'scimType' error of 'noTarget'" - if self.path is None and self.op == PatchOperation.Op.remove: + if self.op == PatchOperation.Op.remove and self.path is None: raise NoTargetException( detail="Remove operation requires a path" ).as_pydantic_error() - - # RFC 7644 Section 3.5.2.2 defines a remove by its path alone: the four - # target locations it lists all read off "path", and a selection is - # spelled as a filter there. An operation carrying a value is thus - # incompatible with the schema of the attribute it targets, which - # Section 3.5.2 answers with an error. - if ( - self.op == PatchOperation.Op.remove - and self.value is not None - and _policy(info).remove_value_as_filter != ScimPolicy.RemoveValue.apply + if (self.op == PatchOperation.Op.add and self.value is None) or ( + self.op == PatchOperation.Op.replace_ + and "value" not in self.model_fields_set ): raise InvalidValueException( - detail="a remove operation carries no value, " - "a filter in the path selects what to remove" - ).as_pydantic_error() - - # RFC 7644 Section 3.5.2.1: "Value is required for add operations" - if self.op == PatchOperation.Op.add and self.value is None: - raise InvalidValueException( - detail="value is required for add operations" + detail=f"value is required for {self.op.value} operations" ).as_pydantic_error() - return self - value: Any | None = None + @model_serializer(mode="wrap") + def _scim_serializer( + self, handler: SerializerFunctionWrapHandler, info: SerializationInfo + ) -> dict[str, Any]: + """Keep a null value the operation was given. + + SCIM dumps drop null values, so a replace that clears its target would + be sent without a value. + """ + serialized = super()._scim_serializer(handler, info) + if ( + self.op != PatchOperation.Op.remove + and "value" in self.model_fields_set + and self.value is None + ): + serialized["value"] = None + return serialized @field_validator("op", mode="before") @classmethod @@ -329,8 +128,16 @@ def _normalize_op(cls, v: Any) -> Any: Microsoft Entra ID emits the values of op as Add, Replace, and Remove. """ if isinstance(v, str): - return v.lower() - return v + v = v.lower() + try: + return cls.Op(v) + except ValueError: + # RFC7644 §3.5.2 defines no scimType for an unknown operation, and + # §3.12 defines invalidValue for a value that does not fit the + # operation. + raise InvalidValueException( + detail=f"{v!r} is not a PATCH operation: add, remove or replace" + ).as_pydantic_error() from None class PatchOp(_ResourceParameterized, Message, Generic[ResourceT]): @@ -369,70 +176,33 @@ def __class_getitem__(cls, item: Any) -> Any: __schema__ = URN("urn:ietf:params:scim:api:messages:2.0:PatchOp") operations: Annotated[list[PatchOperation[ResourceT]] | None, Required.true] = ( - Field(None, serialization_alias="Operations", min_length=1) + Field(None, serialization_alias="Operations") ) """The body of an HTTP PATCH request MUST contain the attribute "Operations", whose value is an array of one or more PATCH operations.""" @model_validator(mode="after") def _validate_operations(self, info: ValidationInfo) -> Self: - """Validate operations against resource type metadata if available. - - When PatchOp is used with a specific resource type (e.g., - PatchOp[User]), this validator will automatically check mutability and - required constraints. - """ - # RFC 7644: The body of an HTTP PATCH request MUST contain the attribute "Operations" - scim_ctx = info.context.get("scim") if info.context else None - if scim_ctx == Context.RESOURCE_PATCH_REQUEST and self.operations is None: + """Reject the errors the operations have on any resource of the type.""" + if self.operations is None: + if _in_patch_request(info): + raise InvalidValueException( + detail="operations attribute is required" + ).as_pydantic_error() + return self + if not self.operations: raise InvalidValueException( - detail="operations attribute is required" + detail="operations holds one or more operations" ).as_pydantic_error() - resource_class = _get_resource_class(self) - if resource_class is None or not self.operations: - return self - - for operation in self.operations: - if operation.path is None: - # §3.5.2.1 and §3.5.2.3: "If the path parameter is omitted, the - # target is assumed to be the resource itself", the value naming - # 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(): - 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 - # resource schema". There is no path here to call invalid. - raise InvalidValueException( - detail=f"attribute '{attr_name}' is not declared by the resource schema" - ).as_pydantic_error() - operation._validate_mutability(resource_class, field_name) - operation._validate_required_attribute( - resource_class, field_name, written - ) - continue - - # The attribute a qualified path applies to is declared by the - # extension the URN designates, not by the resource, so the checks - # resolve the path instead of reading its first segment. They read - # the attribute the path applies to rather than the sub-attribute it - # targets, as a constraint on a complex attribute governs everything - # written under it: "meta" is read-only where "meta.version" is not. - if (resolved := operation.path.resolve()) is None: - if operation.path.model is None: - raise PathNotFoundException( - path=str(operation.path), - detail=f"path '{operation.path}' is not declared by the resource schema", - ).as_pydantic_error() - continue - operation._validate_mutability(resolved.model, resolved.field_name) - operation._validate_required_attribute( - resolved.model, resolved.field_name, operation.value - ) - + model = _get_resource_class(self) + try: + for operation in self.operations: + _check_operation( + model, operation, _policy(info), in_request=_in_patch_request(info) + ) + except SCIMException as exc: + raise exc.as_pydantic_error() from exc return self @classmethod @@ -474,16 +244,18 @@ def build_from( # index of a generic as a type, not as a value. model = type(after) operation_class: Any = PatchOperation.__class_getitem__(model) - path_class = Path.__class_getitem__(model) operations = [ - operation_class(op=op, path=path_class(path), value=value) - for op, path, value in _diff(before, after) + operation_class(op=op, path=path, value=value) + for op, path, value in _differences(before, after, "") ] if not operations: return None patch_class = PatchOp.__class_getitem__(model) - return cast("PatchOp[ResourceT]", patch_class(operations=operations)) + patch = cast("PatchOp[ResourceT]", patch_class(operations=operations)) + # Apply to a copy, so that a patch the peer would reject fails here. + patch.patch(before.model_copy(deep=True)) + return patch def patch(self, resource: ResourceT, scim_policy: ScimPolicy | None = None) -> bool: """Apply all PATCH operations to the given SCIM resource in sequence. @@ -501,259 +273,559 @@ def patch(self, resource: ResourceT, scim_policy: ScimPolicy | None = None) -> b The operations are applied as a whole: when one fails, the resource is left as it was. The resource object itself is kept, but the values it - holds are replaced, so a reference taken on one of them beforehand no - longer reflects the resource. + holds are then copies of the ones it held, so a reference taken on one + of them beforehand no longer reflects the resource. :param resource: The SCIM resource to patch. This object is modified in-place. :param scim_policy: The :class:`~scim2_models.ScimPolicy` the patch is applied under. Defaults to the strict reading of the specification. :return: True if the resource was modified by any operation, False otherwise. - :raises InvalidValueException: If multiple values are marked as primary in a single - operation, or if multiple primary values already exist before the patch. + :raises InvalidValueException: If a value is not compatible with the type of + the attribute it is written to, if multiple values are marked as primary + in a single operation, or if multiple primary values already exist + before the patch. + :raises MutabilityException: If an operation changes an immutable value, + changes a read-only value through its value, or leaves a required + attribute unassigned. + :raises NoTargetException: If the path filter of an ``add`` or + ``replace`` matches no value, and + :attr:`~scim2_models.ScimPolicy.unmatched_path_filter` creates no + entry for it. """ - if not self.operations: - return False - - modified = False - # §3.5.2 has a failing operation leave the resource as it was, and an - # operation only fails once tried: a filter selecting nothing is known - # from the state, not from the payload. - working = resource.model_copy(deep=True) - - # The policy is made ambient for the whole application: the passes it - # governs below are revalidations that start from no call of ours. - with _effective_policy(scim_policy): - # RFC 7644 Section 3.5.2: "Apply each operation in sequence" - for operation in self.operations: - if self._apply_operation(working, operation): - modified = True + snapshot = resource.model_copy(deep=True) + try: + # The policy is made ambient for the whole application: the passes + # it governs below are revalidations that start from no call of ours. + with _effective_policy(scim_policy): + modified = [ + _apply_operation(resource, operation, _effective_policy()) + for operation in self.operations or [] + ] + except Exception: + _restore(resource, snapshot) + raise + return any(modified) - _commit(resource, working) - return modified - def _apply_operation( - self, resource: Resource[Any], operation: PatchOperation[ResourceT] - ) -> bool: - """Apply a single patch operation, and say whether the resource changed. +def _in_patch_request(info: ValidationInfo) -> bool: + """Whether a validation reads the payload of a PATCH request.""" + return (info.context or {}).get("scim") == Context.RESOURCE_PATCH_REQUEST - An operation modifying an immutable attribute raises - MutabilityException. - """ - if operation.path is not None: - self._check_immutable(resource, operation) - if operation.op in (PatchOperation.Op.add, PatchOperation.Op.replace_): - return self._apply_add_replace(resource, operation) - if operation.op == PatchOperation.Op.remove: - return self._apply_remove(resource, operation) +def _is_model(type_: Any) -> bool: + """Whether a type is a model, whose values hold attributes.""" + return isclass(type_) and issubclass(type_, BaseModel) + + +def _as_payload(value: Any) -> Any: + """Return the payload a value built in Python would be sent as.""" + if isinstance(value, BaseModel): + return value.model_dump(scim_ctx=Context.DEFAULT) + return value - raise InvalidValueException(detail=f"unsupported operation: {operation.op}") - def _check_immutable( - self, resource: Resource[Any], operation: PatchOperation[ResourceT] - ) -> None: - """Validate immutable constraints at runtime. +def _dropped(path: Path[Any], policy: ScimPolicy) -> bool: + """Whether the policy drops the undeclared attribute of a path.""" + return ( + path.model is None + and path.resolve() is None + and policy.unknown != ScimPolicy.Unknown.forbid + ) - RFC 7644 §3.5.2: - *"A client MUST NOT modify an attribute that has mutability - "readOnly" or "immutable". However, a client MAY "add" a value - to an "immutable" attribute if the attribute had no previous - value."* +def _check_operation( + model: type[Resource[Any]], + operation: PatchOperation[Any], + policy: ScimPolicy, + *, + in_request: bool, +) -> None: + """Reject the errors an operation has on any resource. - An operation is considered a no-op (and thus allowed) when it would not - effectively change the resource state: ``remove`` on an unset field, or - ``replace`` with the current value. - """ - assert operation.path is not None - if (resolved := operation.path.resolve()) is None: - return - field_name = resolved.field_name + The path must target a declared attribute that is not read-only, and must + not remove or unassign a required attribute. The attributes in a value are + checked against the resource when the patch is applied. + """ + path = Path.__class_getitem__(model)(operation.path or "") + removal = operation.op == PatchOperation.Op.remove + if removal and in_request: + _removal_path(path, operation.value, policy) + if _dropped(path, policy): + return - mutability = resolved.model.get_field_annotation(field_name, Mutability) - if mutability != Mutability.immutable: - return + if path.model is None: + binding = path.resolve() + if binding is None: + raise InvalidPathException( + path=str(path), detail=f"'{path}' is not an attribute of the schema" + ) + mutabilities = ( + binding.model.get_field_annotation(binding.field_name, Mutability), + binding.get_annotation(Mutability), + ) + if Mutability.read_only in mutabilities: + raise MutabilityException( + attribute=str(path), detail=f"'{path}' is read-only" + ) - host = attribute_host(resource, resolved) - current_value = getattr(host, field_name, None) + if not removal: + for write_path, value in _writes(path, operation.value, policy): + if value is None or (value == [] and operation.op != PatchOperation.Op.add): + _refuse_unassigning(write_path, "unassigned") + return - if operation.op == PatchOperation.Op.add and current_value is None: - return + if operation.path is not None: + _refuse_unassigning(path, "removed") - if operation.op == PatchOperation.Op.remove and current_value is None: - return - if ( - operation.op == PatchOperation.Op.replace_ - and operation.value == current_value - ): - return +def _refuse_unassigning(path: Path[Any], verb: str) -> None: + """Reject an operation that unassigns a required attribute (RFC7644 §3.5.2.2). + A sub-attribute is only required when its parent is assigned, and a + selection may leave values behind, so both are checked when the patch is + applied. + """ + binding = path.resolve() + if binding is None or binding.sub_field_name or path.value_filter is not None: + return + if ( + binding.model.get_field_annotation(binding.field_name, Required) + == Required.true + ): raise MutabilityException( - attribute=field_name, - mutability="immutable", - operation=operation.op.value, + attribute=str(path), detail=f"required attribute cannot be {verb}" ) - def _apply_add_replace( - self, resource: Resource[Any], operation: PatchOperation[ResourceT] - ) -> bool: - """Apply an add or replace operation.""" - before_state = self._capture_primary_state(resource) - - value = operation.value - if operation.path is None and isinstance(value, BaseModel): - # Path("").set writes the attributes a mapping names, so a resource - # given as a value is dumped to the payload it stands for, and to - # the very attributes the operation was checked against. - value = _targeted_attributes(value) - - path = operation.path if operation.path is not None else Path("") - modified = path.set( - resource, # type: ignore[arg-type] - value, - is_add=operation.op == PatchOperation.Op.add, + +def _removal_path(path: Path[Any], value: Any, policy: ScimPolicy) -> Path[Any] | None: + """Return the path a remove deletes, or None when its value selects nothing. + + RFC7644 §3.5.2.2 reads the target of a remove from its path only. Under the + apply policy, the value lists the entries to remove by their sub- + attributes, as Microsoft Entra sends it. + """ + if value is None: + return path + if policy.remove_value_as_filter == ScimPolicy.RemoveValue.forbid: + raise InvalidValueException( + detail="a remove operation carries no value, " + "a filter in the path selects what to remove" + ) + if path.model is not None: + raise InvalidPathException( + path=str(path), + detail="a remove selecting values by its value needs " + "a path to the attribute holding them", ) - if modified: - self._normalize_primary_after_patch(resource, before_state) - - return modified - - def _capture_primary_state(self, resource: Resource[Any]) -> dict[str, set[int]]: - """Capture indices of elements with primary=True for each multi-valued attribute.""" - state: dict[str, set[int]] = {} - for field_name in type(resource).model_fields: - if not resource.get_field_multiplicity(field_name): - continue - - field_value = getattr(resource, field_name, None) - if not field_value: - continue - - element_type = resource.get_field_root_type(field_name) - if ( - not element_type - or not isclass(element_type) - or not issubclass(element_type, PydanticBaseModel) - or "primary" not in element_type.model_fields - ): - continue - - primary_indices = { - i - for i, item in enumerate(field_value) - if getattr(item, "primary", None) is True - } - state[field_name] = primary_indices - - return state - - def _normalize_primary_after_patch( - self, resource: Resource[Any], before_state: dict[str, set[int]] - ) -> None: - """Normalize primary attributes after a patch operation. - - Per RFC 7644 §3.5.2: a PATCH operation that sets a value's "primary" - sub-attribute to "true" SHALL cause the server to automatically set - "primary" to "false" for any other values. - """ - for field_name in type(resource).model_fields: - if not resource.get_field_multiplicity(field_name): - continue - - field_value = getattr(resource, field_name, None) - if not field_value: - continue - - element_type = resource.get_field_root_type(field_name) - if ( - not element_type - or not isclass(element_type) - or not issubclass(element_type, PydanticBaseModel) - or "primary" not in element_type.model_fields - ): - continue - - current_primary_indices = { - i - for i, item in enumerate(field_value) - if getattr(item, "primary", None) is True - } - - if len(current_primary_indices) <= 1: - continue - - before_primaries = before_state.get(field_name, set()) - new_primaries = current_primary_indices - before_primaries - - if len(new_primaries) > 1: - raise InvalidValueException( - detail=f"Multiple values marked as primary in field '{field_name}'" - ) + entries = value if isinstance(value, list) else [value] + if path.value_filter is not None or not all( + isinstance(entry, dict) and entry for entry in entries + ): + raise InvalidValueException( + detail="the value of a remove must be non-empty objects, " + "on a path without a filter" + ) + if not entries: + return None + selection = " or ".join( + " and ".join( + f"{name} eq {ScimFilter.quote(item)}" for name, item in entry.items() + ) + for entry in entries + ) + return type(path)(f"{path}[{selection}]") - if not new_primaries: - raise InvalidValueException( - detail=f"Multiple primary values already exist in field '{field_name}'" - ) - keep_index = next(iter(new_primaries)) - for i in current_primary_indices: - if i != keep_index: - field_value[i].primary = False +def _apply_operation( + resource: Resource[Any], operation: PatchOperation[Any], policy: ScimPolicy +) -> bool: + """Apply one operation as RFC7644 §3.5.2 defines it, then check the result.""" + if not isinstance(operation.op, PatchOperation.Op): + raise InvalidValueException(detail=f"{operation.op!r} is not a PATCH operation") + path = Path.__class_getitem__(type(resource))(operation.path or "") + if _dropped(path, policy): + return False - def _apply_remove( - self, resource: Resource[Any], operation: PatchOperation[ResourceT] - ) -> bool: - """Apply a remove operation.""" + removal = None + if operation.op == PatchOperation.Op.remove: if operation.path is None: raise NoTargetException(detail="Remove operation requires a path") + removal = _removal_path(path, operation.value, policy) + writes = [] + touched = [] if removal is None else [removal] + else: + writes = list(_writes(path, operation.value, policy)) + touched = [path, *(write_path for write_path, _ in writes)] + + memo: dict[int, Any] = {} + before = _snapshot(resource, touched, memo) + try: + if operation.op == PatchOperation.Op.remove: + if removal is not None: + removal.delete(resource) + elif not _create_described_entry(resource, path, operation.value, policy): + _write(resource, path, writes, is_add=operation.op == PatchOperation.Op.add) + except ValidationError as exc: + raise InvalidValueException(detail=str(exc)) from exc + _settle(before, resource, memo) + return not all( + _same(getattr(before, name), getattr(resource, name)) + for name in type(resource).model_fields + ) - # Checked again here, a PatchOp built in Python reaching no validator. - if operation.value is not None: - if ( - _effective_policy().remove_value_as_filter - != ScimPolicy.RemoveValue.apply - ): - raise InvalidValueException( - detail="a remove operation carries no value, " - "a filter in the path selects what to remove" - ) - return self._remove_selected_values( - resource, operation.path, operation.value + +def _write( + resource: Resource[Any], + path: Path[Any], + writes: list[tuple[Path[Any], Any]], + *, + is_add: bool, +) -> None: + """Write the value of an add or a replace. + + The path filter is resolved even when the value writes nothing, so a filter + that matches nothing is always reported. + """ + if not writes: + selection = _select(path, resource) + if selection is not None and not selection.matched: + raise NoTargetException( + detail=f"no value of '{selection.field_name}' matches the path filter" ) + _set_values(resource, writes, is_add=is_add) - return operation.path.delete(resource) # type: ignore[arg-type] - def _remove_selected_values( - self, resource: Resource[Any], path: Path[ResourceT], value: Any - ) -> bool: - """Remove the entries the value of a remove operation selects. +def _writes( + path: Path[Any], value: Any, policy: ScimPolicy +) -> Iterator[tuple[Path[Any], Any]]: + """Split the value of an add or a replace into single writes. - Microsoft Entra puts the selection in ``value`` where RFC7644 §3.5.2.2 - puts it in ``path``. Each entry becomes a filter on the sub-attributes - it names, which is the path the operation should have carried. - """ - if path.value_filter is not None: - raise InvalidValueException( - detail="a remove operation carrying a value cannot also " - "select values in its path" - ) + A value for a model or a complex attribute is written key by key, so the + attributes it leaves out are kept, as RFC7644 §3.5.2.3 requires. A key can + be an attribute path. Any other value is written at the path. + """ + value = _as_payload(value) + prefix = _key_prefix(path) + if prefix is None or (path.model is None and not isinstance(value, dict)): + yield path, value + return + if not isinstance(value, dict): + raise InvalidValueException( + detail=f"the value written to '{path}' must be an object" + ) - entries = value if isinstance(value, list) else [value] - if not all(isinstance(entry, dict) and entry for entry in entries): - raise InvalidValueException( - detail="the value of a remove operation names the " - "sub-attributes selecting what to remove" + for key, item in value.items(): + if key.lower() == "schemas": + continue + key_path = _declared(path, prefix + key, policy) + if key_path is not None and key_path.model is not None and item is None: + yield key_path, None + elif key_path is not None: + yield from _writes(key_path, item, policy) + + +def _key_prefix(path: Path[Any]) -> str | None: + """Return the path the keys of a value are relative to. + + The keys of a value for a model are attribute names or paths. The keys of a + value for a complex attribute, or for the entries a filter selects, are + sub-attribute names. Any other target takes a plain value, and gets None. + """ + if path.model is not None: + return "" if path.model is path.models[0] else f"{path}:" + + binding = path.resolve() + if ( + binding is None + or binding.sub_field_name is not None + or not _is_model(binding.field_type) + or (binding.is_multivalued and path.value_filter is None) + ): + return None + return f"{path._as_value_path() or path}." + + +def _declared(parent: Path[Any], text: str, policy: ScimPolicy) -> Path[Any] | None: + """Return the path a key of a value stands for. + + The key must match a declared attribute, without a filter. Otherwise it is + dropped under a tolerant policy, or rejected with invalidValue, which + RFC7644 §3.12 defines for a value that does not fit the schema. + """ + try: + key_path = type(parent)(text) + except InvalidPathException: + key_path = None + if ( + key_path is not None + and key_path.value_filter == parent.value_filter + and (key_path.model or key_path.resolve()) + ): + return key_path + if policy.unknown != ScimPolicy.Unknown.forbid: + return None + raise InvalidValueException( + detail=f"'{text}' is not declared by the resource schema" + ) + + +def _create_described_entry( + resource: Resource[Any], path: Path[Any], value: Any, policy: ScimPolicy +) -> bool: + """Add the entry an unmatched path filter describes, under the create policy. + + The entry gets the values the eq comparisons of the filter compare, then + the value of the operation. It must still match the filter, so that later + operations find it. + """ + value_path = path._as_value_path() + if ( + value_path is None + or policy.unmatched_path_filter != ScimPolicy.UnmatchedPathFilter.create + ): + return False + + selection = type(path)(str(replace(value_path, sub_attr=None))) + described = _described_entry(value_path.val_filter) + binding = selection.resolve() + written = ( + {value_path.sub_attr: value} if value_path.sub_attr else _as_payload(value) + ) + if ( + selection.get(resource) is not None + or described is None + or binding is None + or not _is_model(binding.field_type) + or not isinstance(written, dict) + ): + return False + + type(path)(str(value_path.attr_path)).set( + resource, {**described, **written}, is_add=True + ) + if selection.get(resource) is None: + raise InvalidValueException( + detail=f"the value contradicts the filter of '{path}'" + ) + return True + + +def _described_entry(val_filter: FilterNode) -> dict[str, Any] | None: + """Return the sub-attribute values set by eq comparisons joined by and.""" + terms = ( + val_filter.terms + if isinstance(val_filter, LogicalExpr) and val_filter.op == LogicalOperator.and_ + else (val_filter,) + ) + comparisons = [ + term + for term in terms + if isinstance(term, Comparison) + and term.op == CompareOperator.eq + and term.attr_path.sub_attr is None + and term.attr_path.uri is None + ] + if len(comparisons) != len(terms): + return None + return {comparison.attr_path.attr: comparison.value for comparison in comparisons} + + +def _snapshot( + resource: Resource[Any], paths: list[Path[Any]], memo: dict[int, Any] +) -> Resource[Any]: + """Copy the attributes an operation may change, and share the others. + + The memo maps each object to its copy, which pairs each entry of a multi- + valued attribute with its earlier state. When no path reaches into the + entries of a multi-valued attribute, through a filter or a sub-attribute, + entries can only be added or removed, so they are shared instead of copied. + """ + reached: dict[str, bool] = {} + for path in paths: + binding = None if path.model is not None else path.resolve() + model = path.model if binding is None else binding.model + if model is None or model is type(resource): + if binding is not None: + reaches = binding.sub_field_name is not None or bool(path.value_filter) + name = binding.field_name + reached[name] = reached.get(name, False) or reaches + else: + reached[model.__name__] = True + + before = copy.copy(resource) + for name, reaches in reached.items(): + value = getattr(resource, name) + if isinstance(value, list) and not reaches: + memo.update({id(entry): entry for entry in value}) + before.__dict__[name] = list(value) + else: + before.__dict__[name] = copy.deepcopy(value, memo) + return before + + +def _settle(before: Any, after: Any, memo: dict[int, Any]) -> None: + """Compare the state after an operation with the state before it. + + Each attribute is compared with its earlier value, and each entry of a + multi-valued attribute with its earlier state, as the memo pairs them. Per + RFC7644 §3.5.2, a read-only attribute cannot change, an immutable one + cannot change once assigned, and a required one cannot become unassigned. + Removing an extension or a complex attribute may remove its read-only and + required attributes, but not its assigned immutable ones. A value marked + primary unmarks the others. + """ + if before is after: + return + removed = after is None + model = type(before if removed else after) + for field_name in model.model_fields: + old = getattr(before, field_name, None) + new = getattr(after, field_name, None) + if _same(old, new): + continue + + name = model._scim_name(field_name) + mutability = model.get_field_annotation(field_name, Mutability) + if mutability == Mutability.immutable and _assigned(old): + raise MutabilityException(attribute=name, detail=f"'{name}' is immutable") + if not removed and mutability == Mutability.read_only: + raise MutabilityException(attribute=name, detail=f"'{name}' is read-only") + if ( + not removed + and model.get_field_annotation(field_name, Required) == Required.true + and _assigned(old) + and not _assigned(new) + ): + raise MutabilityException( + attribute=name, detail=f"required attribute '{name}' became unassigned" ) - removed = False - for entry in entries: - conditions = " and ".join( - f"{name} eq {ScimFilter.quote(item)}" for name, item in entry.items() + for old_value, new_value in _counterparts(old, new, memo): + _settle(old_value, new_value, memo) + if isinstance(new, list): + _settle_primary(new, memo) + + +def _counterparts(old: Any, new: Any, memo: dict[int, Any]) -> list[tuple[Any, Any]]: + """Pair the objects an attribute holds with their earlier state. + + Entries of a multi-valued attribute have no identity other than the object, + so a removed entry is not visited, and an added entry has no pair. + """ + if isinstance(new, list): + return [ + (memo.get(id(entry)), entry) + for entry in new + if isinstance(entry, BaseModel) + ] + if isinstance(old, BaseModel) or isinstance(new, BaseModel): + return [(old, new)] + return [] + + +def _settle_primary(entries: list[Any], memo: dict[int, Any]) -> None: + """Keep the value just marked primary as the only primary one. + + Per RFC7644 §3.5.2, a value marked primary unmarks the others, and per + RFC7643 §2.4, only one value can be primary. + """ + primary = [ + entry + for entry in entries + if "primary" in getattr(type(entry), "model_fields", {}) + and entry.primary is True + ] + marked = [ + entry + for entry in primary + if getattr(memo.get(id(entry)), "primary", None) is not True + ] + if len(marked) > 1: + raise InvalidValueException(detail="Multiple values marked as primary") + if not marked and len(primary) > 1: + raise InvalidValueException(detail="Multiple primary values already exist") + for entry in primary: + if marked and entry is not marked[0]: + entry.primary = False + + +def _same(old: Any, new: Any) -> bool: + """Whether a value is unchanged, the order of entries aside (RFC7643 §2.4).""" + if old == new: + return True + if not (isinstance(old, list) and isinstance(new, list)) or len(old) != len(new): + return False + return bool(_comparable(old) == _comparable(new)) + + +def _comparable(value: Any, include: set[str] | None = None) -> Any: + """Reduce a value to a form that compares equal regardless of entry order.""" + if isinstance(value, list): + return sorted((_comparable(item, include) for item in value), key=repr) + if isinstance(value, BaseModel): + return value.model_dump(include=include) + return value + + +def _assigned(value: Any) -> bool: + """Whether a value is assigned, as RFC7643 §2.5 defines it.""" + if isinstance(value, BaseModel): + return any(_assigned(getattr(value, name)) for name in type(value).model_fields) + return value is not None and value != [] + + +def _restore(resource: BaseModel, snapshot: BaseModel) -> None: + """Restore a resource from a copy, keeping the same object.""" + for attribute in ("__dict__", "__pydantic_fields_set__", "__pydantic_private__"): + object.__setattr__(resource, attribute, getattr(snapshot, attribute)) + + +def _differences( + before: Any, after: BaseModel, prefix: str +) -> Iterator[tuple[PatchOperation.Op, str, Any]]: + """Yield the operations that set the attributes of the wanted state. + + A complex attribute or an extension is compared attribute by attribute. A + multi-valued attribute is compared on the sub-attributes its wanted entries + name. + """ + model = type(after) + for field_name in model.model_fields: + mutability = model.get_field_annotation(field_name, Mutability) + if ( + field_name not in after.model_fields_set + or field_name == "schemas" + or mutability == Mutability.read_only + ): + continue + + old = getattr(before, field_name, None) + new = getattr(after, field_name) + path = f"{prefix}{model._scim_name(field_name)}" + include = _named_sub_attributes(new) + if isinstance(new, BaseModel): + separator = ":" if field_name in model.__scim_info__.extensions else "." + yield from _differences(old, new, path + separator) + elif not _assigned(new): + if _assigned(old): + yield PatchOperation.Op.remove, path, None + elif _comparable(old, include) != _comparable(new, include): + op = ( + PatchOperation.Op.add + if mutability == Mutability.immutable and old is None + else PatchOperation.Op.replace_ ) - # Subscripted through the call the syntax stands for: mypy reads - # the index of a generic as a type, not as a value. - selection = Path.__class_getitem__(type(resource))(f"{path}[{conditions}]") - removed = selection.delete(resource) or removed - return removed + yield op, path, new + + +def _named_sub_attributes(value: Any) -> set[str] | None: + """Return the sub-attributes the entries of a wanted collection name.""" + if not isinstance(value, list): + return None + return { + name + for entry in value + if isinstance(entry, BaseModel) + for name in entry.model_fields_set + } diff --git a/scim2_models/path/access.py b/scim2_models/path/access.py index aa1c8406..bef940b1 100644 --- a/scim2_models/path/access.py +++ b/scim2_models/path/access.py @@ -255,42 +255,41 @@ def _get_value(path: "Path[Any]", resource: BaseModel) -> Any: return values if target.multivalued else values[0] -def _set_selected( - path: "Path[Any]", selection: "_Selection", value: Any, *, is_add: bool = False -) -> bool: +def _set_selected(path: "Path[Any]", selection: "_Selection", value: Any) -> bool: """Apply a value to every entry matched by a value selection. - A replacement selection matching nothing raises NoTargetException, per - RFC7644 §3.5.2.3. That failure is defined for ``replace`` only: §3.5.2.1 - says nothing of a selection that matches nothing for ``add``, so the - operation is a no-op instead. Errata 8097 - (https://errata.rfc-editor.org/eid8097/) asks for value selections in - ``add`` to be clarified at all, implementations differing on whether they - are allowed. + A selection matching nothing raises NoTargetException. RFC7644 §3.5.2.3 + requires it for replace, and Table 9 of §3.12 defines noTarget for a filter + that "yields no match", which applies to add too. + + Without a sub-attribute, each matched entry is replaced in place, so it + stays the same object. Entries have no identity other than the object. """ host, field_name, matched, sub_attr = selection if not matched: - if is_add: - return False raise NoTargetException( detail=f"no value of '{field_name}' matches the path filter" ) if sub_attr is None: - # Without a sub-attribute the matched entries are replaced wholesale. - current = getattr(host, field_name) - replacement = list(current) new_value = _as_entry(type(host), field_name, value) - modified = False - for index, item in enumerate(replacement): - if any(item is candidate for candidate in matched): - if not _values_match(item, new_value): - replacement[index] = new_value - modified = True - if modified: - setattr(host, field_name, replacement) - return modified + changed = [item for item in matched if not _values_match(item, new_value)] + if not changed: + return False + if isinstance(new_value, BaseModel): + for item in changed: + item.__dict__.update(new_value.__dict__) + object.__setattr__( + item, "__pydantic_fields_set__", set(new_value.model_fields_set) + ) + return True + replacement = [ + new_value if any(item is entry for entry in changed) else item + for item in getattr(host, field_name) + ] + setattr(host, field_name, replacement) + return True modified = False for item in matched: @@ -346,9 +345,9 @@ def _set_value( ) -> bool: """Write a value where a path designates on a resource.""" if (selection := _select(path, resource)) is not None: - return _set_selected(path, selection, value, is_add=is_add) + return _set_selected(path, selection, value) - target = _walk(path, resource, create=True) + target = _walk(path, resource, create=value is not None) if target is None: return False if isinstance(target, _Root): @@ -361,6 +360,22 @@ def _set_value( return any(changed) +def _set_values( + resource: BaseModel, writes: list[tuple["Path[Any]", Any]], *, is_add: bool = False +) -> None: + """Write several values, resolving every selection before writing anything. + + A value may write the sub-attributes a filter compares. Resolving first + keeps the rest of the value writing to the same entries. + """ + selections = [_select(path, resource) for path, _ in writes] + for (path, value), selection in zip(writes, selections, strict=True): + if selection is None: + _set_value(path, resource, value, is_add=is_add) + else: + _set_selected(path, selection, value) + + def _merge(path: "Path[Any]", obj: BaseModel, value: Any, *, explicit: bool) -> bool: """Write the attributes a mapping names onto the object the path designates. @@ -391,7 +406,7 @@ def _set_field_value(obj: BaseModel, field_name: str, value: Any, is_add: bool) """Set or add a value to a field.""" is_multivalued = obj.get_field_multiplicity(field_name) - if is_add and is_multivalued: + if is_add and is_multivalued and value is not None: current_list = getattr(obj, field_name) or [] entries = [ _as_entry(type(obj), field_name, item) diff --git a/scim2_models/path/path.py b/scim2_models/path/path.py index 446c628d..d467af6e 100644 --- a/scim2_models/path/path.py +++ b/scim2_models/path/path.py @@ -389,8 +389,8 @@ def set( :raises InvalidPathException: If strict and the path does not exist or is invalid. :raises InvalidFilterException: If strict and a value selection does not apply to the attribute it selects from. - :raises NoTargetException: If strict, ``is_add`` is false and a value - selection matches nothing. + :raises NoTargetException: If strict and a value selection matches + nothing. """ try: return _set_value(self, resource, value, is_add=is_add) diff --git a/scim2_models/policy.py b/scim2_models/policy.py index 49e4bd1e..fc004ba8 100644 --- a/scim2_models/policy.py +++ b/scim2_models/policy.py @@ -24,7 +24,9 @@ class ScimPolicy(BaseModel): on the wire, symmetric, and defined by the specification; the second is local, and nothing on the wire announces it. - Every setting defaults to the strict reading of the specification. + Every setting defaults to the strict reading of the specification. A + tolerance that cannot confuse one payload with another, such as a PATCH + value key that is an attribute path, needs no setting. >>> from scim2_models import ScimPolicy >>> ScimPolicy().unknown is ScimPolicy.Unknown.forbid @@ -70,14 +72,17 @@ class Unknown(StrEnum): It stays readable on :attr:`~scim2_models.BaseModel.unknown_attributes`, and no dump - restores it. + restores it. A PATCH operation on such an attribute changes + nothing. """ keep = "keep" """The payload is accepted and the attribute is dumped back. The original spelling is preserved. No attribute characteristic is - declared for it, so no context filters it out. + declared for it, so no context filters it out. In the value or the + path of a PATCH operation, it has no field to write to, so it is + dropped as with ``ignore``. """ class RemoveValue(StrEnum): @@ -93,12 +98,35 @@ class RemoveValue(StrEnum): apply = "apply" """The ``value`` selects what to remove, as Microsoft Entra sends it.""" + class UnmatchedPathFilter(StrEnum): + """What becomes of a PATCH operation whose path filter matches no entry.""" + + forbid = "forbid" + """The ``add`` or ``replace`` operation is rejected with ``noTarget``. + + :rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>` requires it for + ``replace``, and Table 9 of :rfc:`RFC7644 §3.12 <7644#section-3.12>` + defines ``noTarget`` for a filter that "yields no match". + """ + + create = "create" + """The entry the filter describes is added, as Microsoft Entra expects. + + Only ``eq`` comparisons on sub-attributes, joined by ``and``, describe + an entry. The entry gets the compared values, then the operation + writes its value. Any other filter, and any ``remove``, is still + rejected with ``noTarget``. + """ + unknown: Unknown = Unknown.forbid """What becomes of an attribute no model declares.""" remove_value_as_filter: RemoveValue = RemoveValue.forbid """What becomes of a PATCH ``remove`` operation carrying a ``value``.""" + unmatched_path_filter: UnmatchedPathFilter = UnmatchedPathFilter.forbid + """What becomes of a PATCH operation whose path filter matches no entry.""" + def __enter__(self) -> "ScimPolicy": """Make this policy the one every call in the block runs under.""" _AMBIENT_POLICIES.set(_AMBIENT_POLICIES.get() + (self,)) diff --git a/tests/test_patch_op_add.py b/tests/test_patch_op_add.py index a13142f1..313ba162 100644 --- a/tests/test_patch_op_add.py +++ b/tests/test_patch_op_add.py @@ -3,6 +3,8 @@ from scim2_models import Group from scim2_models import GroupMember +from scim2_models import InvalidValueException +from scim2_models import NoTargetException from scim2_models import PatchOp from scim2_models import PatchOperation from scim2_models import User @@ -304,18 +306,6 @@ def test_add_operation_no_path_with_invalid_attribute(): assert user.nick_name == "Test" -def test_add_operation_with_non_dict_value_no_path(): - """Test add operation with no path and non-dict value should return False.""" - user = User() - patch = PatchOp[User]( - operations=[ - PatchOperation[User](op=PatchOperation.Op.add, value="invalid_value") - ] - ) - result = patch.patch(user) - assert result is False - - def test_add_a_subattribute_to_every_entry(): """An unfiltered path designates the sub-attribute of each entry.""" user = User( @@ -350,7 +340,7 @@ def test_a_rejected_addition_leaves_the_attribute_untouched(): ) ] ) - with pytest.raises(ValidationError): + with pytest.raises(InvalidValueException): patch.patch(user) assert user.emails == [User.Emails(value="bjensen@example.com")] @@ -376,3 +366,99 @@ def test_an_operation_without_a_path_marks_the_attributes_it_wrote(): assert user.display_name == "Barbara" assert "display_name" in user.model_fields_set assert user.model_dump(exclude_unset=True)["displayName"] == "Barbara" + + +@pytest.mark.parametrize( + "path", [None, "", "urn:ietf:params:scim:schemas:core:2.0:User"] +) +def test_an_add_with_a_multi_valued_attribute_in_its_value_adds_to_it(path): + """RFC7644 §3.5.2.1 adds an email to the resource in its example of an add without a path.""" + user = User(user_name="bjensen", emails=[{"value": "bjensen@example.com"}]) + operation = {"op": "add", "value": {"emails": [{"value": "babs@example.com"}]}} + if path is not None: + operation["path"] = path + + PatchOp[User].model_validate({"Operations": [operation]}).patch(user) + + assert [email.value for email in user.emails] == [ + "bjensen@example.com", + "babs@example.com", + ] + + +def test_an_add_with_an_empty_list_in_its_value_leaves_the_attribute(): + """An add of an empty list adds nothing, with or without a path.""" + user = User(user_name="bjensen", emails=[{"value": "bjensen@example.com"}]) + patch = PatchOp[User].model_validate( + {"Operations": [{"op": "add", "value": {"emails": []}}]} + ) + + assert patch.patch(user) is False + assert [email.value for email in user.emails] == ["bjensen@example.com"] + + +def test_an_add_with_schemas_in_its_value_leaves_them_to_the_resource(): + """The schemas of a resource follow the attributes it holds, not the payload writing them.""" + user = User(user_name="bjensen") + patch = PatchOp[User].model_validate( + { + "Operations": [ + { + "op": "add", + "value": {"schemas": ["urn:example:foo"], "nickName": "Babs"}, + } + ] + } + ) + + patch.patch(user) + + assert user.schemas == ["urn:ietf:params:scim:schemas:core:2.0:User"] + assert user.nick_name == "Babs" + + +def test_an_add_setting_a_multi_valued_attribute_to_null_unassigns_it(): + """RFC7643 §2.5 makes null the state of an attribute holding no value.""" + user = User(user_name="bjensen", emails=[{"value": "bjensen@example.com"}]) + patch = PatchOp[User].model_validate( + {"Operations": [{"op": "add", "value": {"emails": None}}]} + ) + + assert patch.patch(user) + assert user.emails is None + + +def test_add_through_a_filter_matching_nothing_has_no_target(): + """Entra expects the entry to be created, but the filter only selects entries, so noTarget.""" + user = User( + user_name="bjensen", emails=[{"value": "b@example.com", "type": "home"}] + ) + patch = PatchOp[User]( + operations=[ + { + "op": "add", + "path": 'emails[type eq "work"].value', + "value": "w@example.com", + } + ] + ) + with pytest.raises(NoTargetException): + patch.patch(user) + assert [email.value for email in user.emails] == ["b@example.com"] + + +def test_add_an_entry_through_a_filter_on_an_unassigned_attribute_has_no_target(): + """An unassigned attribute holds no entry for the filter to match.""" + user = User(user_name="bjensen") + patch = PatchOp[User]( + operations=[ + { + "op": "add", + "path": 'emails[type eq "work"]', + "value": {"value": "w@example.com"}, + } + ] + ) + with pytest.raises(NoTargetException): + patch.patch(user) + assert user.emails is None diff --git a/tests/test_patch_op_build.py b/tests/test_patch_op_build.py index db78232c..b2ca0cd3 100644 --- a/tests/test_patch_op_build.py +++ b/tests/test_patch_op_build.py @@ -63,7 +63,7 @@ def test_an_attribute_the_wanted_state_does_not_name_is_left_alone(): def test_a_changed_sub_attribute_is_targeted_by_its_own_path(): - """Targeting name as a whole would replace it entirely and drop the sub-attributes the operation does not carry.""" + """A peer that replaces name as a whole would drop the sub-attributes the operation leaves out.""" before = User(name=Name(given_name="Barbara", family_name="Jensen")) after = User(name=Name(given_name="Babs", family_name="Jensen")) diff --git a/tests/test_patch_op_extensions.py b/tests/test_patch_op_extensions.py index e3aba465..8dd18009 100644 --- a/tests/test_patch_op_extensions.py +++ b/tests/test_patch_op_extensions.py @@ -5,10 +5,12 @@ from scim2_models import URN from scim2_models import Group -from scim2_models import InvalidPathException +from scim2_models import MutabilityException from scim2_models import PatchOp from scim2_models import PatchOperation +from scim2_models import ScimPolicy from scim2_models import User +from scim2_models.context import Context from scim2_models.resources.enterprise_user import EnterpriseUser from scim2_models.resources.resource import Resource @@ -97,7 +99,7 @@ def test_patch_operation_extension_complex_attribute(): path="urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:manager", value={ "value": "super-manager-789", - "displayName": "Alice Johnson", + "displayName": "John Smith", "$ref": "https://example.com/Users/super-manager-789", }, ) @@ -106,7 +108,7 @@ def test_patch_operation_extension_complex_attribute(): result = patch2.patch(user) assert result is True assert user[EnterpriseUser].manager.value == "super-manager-789" - assert user[EnterpriseUser].manager.display_name == "Alice Johnson" + assert user[EnterpriseUser].manager.display_name == "John Smith" patch3 = PatchOp[User[EnterpriseUser]]( operations=[ @@ -304,24 +306,6 @@ def test_patch_main_schema_path_without_attribute(): assert user.title == "Manager" -def test_patch_schema_path_with_invalid_value_type(): - """Test PATCH with schema URN path and invalid value type (non-dict).""" - user = User(user_name="test") - - patch = PatchOp[User]( - operations=[ - PatchOperation[User]( - op=PatchOperation.Op.add, - path="urn:ietf:params:scim:schemas:core:2.0:User", - value="invalid string value", - ) - ] - ) - - with pytest.raises(InvalidPathException): - patch.patch(user) - - def test_patch_delete_extension_root(): """Test PATCH remove operation targeting the root of an extension.""" user = User[EnterpriseUser].model_validate( @@ -352,3 +336,201 @@ def test_patch_delete_extension_root(): result = patch.patch(user) assert result is True assert user[EnterpriseUser] is None + + +MANAGER = "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:manager" + + +def _managed_user(): + return User[EnterpriseUser].model_validate( + { + "userName": "jane.doe", + MANAGER.rsplit(":", 1)[0]: { + "manager": {"value": "manager-123", "displayName": "John Smith"} + }, + } + ) + + +def test_a_path_to_a_read_only_sub_attribute_is_refused(): + """RFC7644 §3.5.2 forbids changing a read-only attribute, and this path points to one.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User[EnterpriseUser]].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": f"{MANAGER}.displayName", + "value": "Alice", + } + ] + } + ) + + assert raised.value.errors()[0]["type"] == "scim_mutability" + + +@pytest.mark.parametrize( + ("path", "value"), + [ + (MANAGER, {"value": "manager-456", "displayName": "Alice"}), + (None, {MANAGER.rsplit(":", 1)[0]: {"manager": {"displayName": "Alice"}}}), + ], +) +def test_a_value_changing_a_read_only_sub_attribute_is_refused(path, value): + """A read-only sub-attribute in a value is rejected when it would change.""" + user = _managed_user() + operation = {"op": "replace", "value": value} + if path is not None: + operation["path"] = path + patch = PatchOp[User[EnterpriseUser]].model_validate({"Operations": [operation]}) + + with pytest.raises(MutabilityException): + patch.patch(user) + + assert user[EnterpriseUser].manager.display_name == "John Smith" + + +def test_a_complex_attribute_holding_a_read_only_sub_attribute_may_be_removed(): + """Removing an attribute also removes its read-only sub-attributes.""" + user = _managed_user() + patch = PatchOp[User[EnterpriseUser]].model_validate( + {"Operations": [{"op": "remove", "path": MANAGER}]} + ) + + assert patch.patch(user) + assert user[EnterpriseUser].manager is None + + +def test_a_new_complex_value_cannot_assign_a_read_only_sub_attribute(): + """A client cannot set a read-only sub-attribute, even on an attribute that had no value.""" + user = User[EnterpriseUser](user_name="jane.doe") + patch = PatchOp[User[EnterpriseUser]].model_validate( + { + "Operations": [ + { + "op": "add", + "path": MANAGER, + "value": {"value": "manager-123", "displayName": "John Smith"}, + } + ] + } + ) + + with pytest.raises(MutabilityException): + patch.patch(user) + + +ENTERPRISE_URN = "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User" + + +def _keyed_patch(op, value, **kwargs): + return PatchOp[User[EnterpriseUser]].model_validate( + {"Operations": [{"op": op, "value": value}]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + **kwargs, + ) + + +def _keyed_user(): + user = User[EnterpriseUser](user_name="bjensen", name={"familyName": "Jensen"}) + user[EnterpriseUser] = EnterpriseUser( + manager={"value": "m1", "displayName": "Boss"} + ) + return user + + +@pytest.mark.parametrize("op", ["add", "replace"]) +def test_a_dotted_key_writes_its_sub_attribute(op): + """Entra uses paths as keys in a value without a path, and no attribute name looks like a path.""" + user = _keyed_user() + + assert _keyed_patch(op, {"name.givenName": "Barbara"}).patch(user) + assert user.name.given_name == "Barbara" + assert user.name.family_name == "Jensen" + + +@pytest.mark.parametrize("op", ["add", "replace"]) +def test_a_urn_qualified_key_writes_its_extension_attribute(op): + """The Microsoft SCIM Validator uses the full URN of extension attributes as keys.""" + user = _keyed_user() + + assert _keyed_patch(op, {f"{ENTERPRISE_URN}:employeeNumber": "42"}).patch(user) + assert user[EnterpriseUser].employee_number == "42" + + +def test_a_urn_qualified_key_merges_into_its_complex_attribute(): + """A complex attribute written through a URN key keeps the sub-attributes its value leaves out.""" + user = _keyed_user() + + assert _keyed_patch( + "replace", {f"{ENTERPRISE_URN}:manager": {"value": "m2"}} + ).patch(user) + assert user[EnterpriseUser].manager.value == "m2" + assert user[EnterpriseUser].manager.display_name == "Boss" + + +def test_a_dotted_key_inside_an_extension_is_read_from_the_extension(): + """A key of the value an extension takes is a path relative to that extension.""" + user = _keyed_user() + + assert _keyed_patch("replace", {ENTERPRISE_URN: {"manager.value": "m2"}}).patch( + user + ) + assert user[EnterpriseUser].manager.value == "m2" + + +def test_a_key_qualified_by_the_resource_urn_writes_the_core_attribute(): + """The core schema URN qualifies the attributes of the resource as well.""" + user = _keyed_user() + + assert _keyed_patch( + "replace", {"urn:ietf:params:scim:schemas:core:2.0:User:nickName": "Babs"} + ).patch(user) + assert user.nick_name == "Babs" + + +@pytest.mark.parametrize( + "key", + [ + pytest.param("name.nickName", id="undeclared sub-attribute"), + pytest.param("urn:example:2.0:Unknown:attr", id="undeclared extension"), + pytest.param('emails[type eq "work"].value', id="filter"), + pytest.param("display name", id="malformed"), + ], +) +def test_a_key_spelling_no_declared_attribute_path_is_refused(key): + """A key that is neither an attribute name nor a declared path is undeclared.""" + with pytest.raises(ValidationError) as raised: + _keyed_patch("replace", {key: "x"}) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +def test_a_key_spelling_no_declared_attribute_path_is_dropped_under_a_tolerant_policy(): + """The unknown policy handles such a key like any undeclared attribute.""" + user = _keyed_user() + tolerant = ScimPolicy(unknown=ScimPolicy.Unknown.ignore) + patch = _keyed_patch("replace", {"name.nickName": "x"}, scim_policy=tolerant) + + assert not patch.patch(user, scim_policy=tolerant) + + +def test_a_dotted_key_cannot_change_a_read_only_sub_attribute(): + """A path used as a key has the same constraints as in the path field.""" + user = _keyed_user() + patch = _keyed_patch("replace", {f"{ENTERPRISE_URN}:manager.displayName": "Other"}) + + with pytest.raises(MutabilityException): + patch.patch(user) + assert user[EnterpriseUser].manager.display_name == "Boss" + + +def test_a_qualified_key_cannot_unassign_a_required_attribute(): + """Unassigning a required attribute is rejected however the key spells it.""" + with pytest.raises(ValidationError) as raised: + _keyed_patch( + "replace", {"urn:ietf:params:scim:schemas:core:2.0:User:userName": None} + ) + + assert raised.value.errors()[0]["type"] == "scim_mutability" diff --git a/tests/test_patch_op_replace.py b/tests/test_patch_op_replace.py index af278dbd..9eb570ee 100644 --- a/tests/test_patch_op_replace.py +++ b/tests/test_patch_op_replace.py @@ -1,11 +1,19 @@ from typing import Annotated import pytest +from pydantic import ValidationError from scim2_models import URN +from scim2_models import EnterpriseUser +from scim2_models import Group +from scim2_models import InvalidFilterException +from scim2_models import InvalidValueException from scim2_models import MutabilityException +from scim2_models import Name +from scim2_models import NoTargetException from scim2_models import PatchOp from scim2_models import PatchOperation +from scim2_models import ScimPolicy from scim2_models import User from scim2_models.annotations import Mutability from scim2_models.resources.resource import Resource @@ -191,19 +199,6 @@ def test_replace_operation_no_path_same_attributes(): assert user.display_name == "Display" -def test_replace_operation_with_non_dict_value_no_path(): - """Test replace operation with no path and non-dict value should return False.""" - user = User(nick_name="Test") - patch = PatchOp[User]( - operations=[ - PatchOperation[User](op=PatchOperation.Op.replace_, value="invalid_value") - ] - ) - result = patch.patch(user) - assert result is False - assert user.nick_name == "Test" - - def test_immutable_field(): """Test that replace operations on immutable fields raise mutability errors.""" @@ -395,3 +390,419 @@ def test_replace_a_subattribute_of_every_entry(): "new@example.com", "new@example.com", ] + + +@pytest.mark.parametrize( + "path", + [ + "name.givenName", + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:costCenter", + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:manager.value", + ], +) +def test_unassigning_under_an_unassigned_container_leaves_the_resource(path): + """Nothing is created to hold a null value.""" + user = User[EnterpriseUser](user_name="bjensen") + patch = PatchOp[User[EnterpriseUser]].model_validate( + {"Operations": [{"op": "replace", "path": path, "value": None}]} + ) + + assert patch.patch(user) is False + assert user.name is None + assert user[EnterpriseUser] is None + + +def _jensen(): + return User( + user_name="bjensen", name=Name(given_name="Barbara", family_name="Jensen") + ) + + +COMPLEX_WRITES = [ + pytest.param("name", {"givenName": "Babs"}, id="attribute path"), + pytest.param( + "urn:ietf:params:scim:schemas:core:2.0:User:name", + {"givenName": "Babs"}, + id="qualified path", + ), + pytest.param(None, {"name": {"givenName": "Babs"}}, id="no path"), + pytest.param("", {"name": {"givenName": "Babs"}}, id="empty path"), +] + + +@pytest.mark.parametrize("op", ["add", "replace"]) +@pytest.mark.parametrize(("path", "value"), COMPLEX_WRITES) +def test_a_partial_complex_value_leaves_the_other_sub_attributes(op, path, value): + """RFC7644 §3.5.2.3 keeps the sub-attributes the value does not specify.""" + user = _jensen() + operation = {"op": op, "value": value} + if path is not None: + operation["path"] = path + + assert PatchOp[User].model_validate({"Operations": [operation]}).patch(user) + + assert user.name == Name(given_name="Babs", family_name="Jensen") + + +def test_a_null_sub_attribute_in_a_complex_value_unassigns_it_alone(): + """A null value unassigns only the sub-attribute it is given to.""" + user = _jensen() + patch = PatchOp[User].model_validate( + { + "Operations": [ + {"op": "replace", "path": "name", "value": {"givenName": None}} + ] + } + ) + + patch.patch(user) + + assert user.name == Name(family_name="Jensen") + + +def test_a_complex_value_on_an_unassigned_attribute_assigns_it(): + """Per RFC7644 §3.5.2.3, a replace on a missing attribute adds it.""" + user = User(user_name="bjensen") + patch = PatchOp[User].model_validate( + { + "Operations": [ + {"op": "replace", "path": "name", "value": {"givenName": "Babs"}} + ] + } + ) + + assert patch.patch(user) + assert user.name == Name(given_name="Babs") + + +def test_an_undeclared_sub_attribute_in_a_complex_value_is_refused(): + """RFC7644 §3.12 defines invalidValue for a value that does not fit the resource schema.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User].model_validate( + {"Operations": [{"op": "add", "path": "name", "value": {"bogus": 1}}]} + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +def test_a_tolerant_policy_drops_an_undeclared_sub_attribute_alone(): + """The sub-attributes the value leaves out are kept when the undeclared one is dropped.""" + user = _jensen() + tolerant = ScimPolicy(unknown=ScimPolicy.Unknown.ignore) + patch = PatchOp[User].model_validate( + {"Operations": [{"op": "add", "path": "name", "value": {"bogus": 1}}]}, + scim_policy=tolerant, + ) + + assert patch.patch(user, scim_policy=tolerant) is False + assert user.name == Name(given_name="Barbara", family_name="Jensen") + + +def _user_with_work_email(): + return User( + user_name="bjensen", + emails=[ + {"value": "bjensen@example.com", "type": "work", "display": "Work"}, + {"value": "babs@example.com", "type": "home"}, + ], + ) + + +def test_a_replace_selecting_entries_merges_the_value_into_them(): + """RFC7644 §3.5.2.3 keeps the sub-attributes a value does not specify.""" + user = _user_with_work_email() + patch = PatchOp[User].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": 'emails[type eq "work"]', + "value": {"value": "barbara@example.com"}, + } + ] + } + ) + + assert patch.patch(user) + + assert user.emails[0].value == "barbara@example.com" + assert user.emails[0].type == "work" + assert user.emails[0].display == "Work" + assert user.emails[1].value == "babs@example.com" + + +def test_a_replace_selecting_entries_unassigns_a_sub_attribute_given_null(): + """Per RFC7643 §2.5, a null value unassigns the sub-attribute it is given to.""" + user = _user_with_work_email() + patch = PatchOp[User].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": 'emails[type eq "work"]', + "value": {"display": None}, + } + ] + } + ) + + assert patch.patch(user) + + assert user.emails[0].display is None + assert user.emails[0].value == "bjensen@example.com" + + +def test_a_replace_selecting_a_member_changes_it_without_repeating_its_value(): + """A client does not have to repeat the immutable value its filter used to select the member.""" + group = Group( + display_name="Tour Guides", + members=[{"value": "2819c223", "display": "Babs Jensen"}], + ) + patch = PatchOp[Group].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": 'members[value eq "2819c223"]', + "value": {"display": "Barbara Jensen"}, + } + ] + } + ) + + assert patch.patch(group) + assert group.members[0].value == "2819c223" + assert group.members[0].display == "Barbara Jensen" + + +def test_an_entry_replaced_through_a_selection_keeps_its_identity(): + """Entries are only told apart by identity, so a replaced entry stays the same object.""" + user = _user_with_work_email() + entry = user.emails[0] + patch = PatchOp[User].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": 'emails[type eq "work"]', + "value": {"value": "barbara@example.com"}, + } + ] + } + ) + + patch.patch(user) + + assert user.emails[0] is entry + + +def test_an_add_selecting_entries_merges_the_value_into_them(): + """An add writes the sub-attributes in its value and leaves the others.""" + user = _user_with_work_email() + patch = PatchOp[User].model_validate( + { + "Operations": [ + { + "op": "add", + "path": 'emails[type eq "work"]', + "value": {"display": "Office"}, + } + ] + } + ) + + assert patch.patch(user) + + assert user.emails[0].value == "bjensen@example.com" + assert user.emails[0].type == "work" + assert user.emails[0].display == "Office" + + +def test_an_add_selecting_entries_with_the_values_they_hold_changes_nothing(): + """Per RFC7644 §3.5.2.1, adding a value already held changes nothing.""" + user = _user_with_work_email() + patch = PatchOp[User].model_validate( + { + "Operations": [ + { + "op": "add", + "path": 'emails[type eq "work"]', + "value": {"display": "Work"}, + } + ] + } + ) + + assert patch.patch(user) is False + + +def test_a_replace_selecting_a_member_cannot_change_its_immutable_value(): + """RFC7643 §4.2 makes member sub-attributes immutable, even when the whole entry is written.""" + group = Group(display_name="Tour Guides", members=[{"value": "2819c223"}]) + patch = PatchOp[Group].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": 'members[value eq "2819c223"]', + "value": {"value": "c3a26dd3"}, + } + ] + } + ) + + with pytest.raises(MutabilityException): + patch.patch(group) + + +class Tagged(Resource): + __schema__ = URN("urn:example:2.0:Tagged") + + tags: list[str] | None = None + + +def test_a_replace_selecting_simple_values_replaces_them(): + """The values of a multi-valued attribute without sub-attributes are selected by their value.""" + resource = Tagged(tags=["red", "blue"]) + patch = PatchOp[Tagged].model_validate( + { + "Operations": [ + {"op": "replace", "path": 'tags[value eq "red"]', "value": "green"} + ] + } + ) + + assert patch.patch(resource) + assert resource.tags == ["green", "blue"] + + +def test_a_replace_selecting_entries_refuses_a_value_that_is_no_entry(): + """RFC7644 §3.12 returns invalidValue for a value that does not fit the attribute type.""" + user = _user_with_work_email() + patch = PatchOp[User].model_validate( + { + "Operations": [ + {"op": "replace", "path": 'emails[type eq "work"]', "value": "x"} + ] + } + ) + + with pytest.raises(InvalidValueException): + patch.patch(user) + + +def test_a_replace_selecting_simple_values_with_the_one_they_hold_changes_nothing(): + """Writing the value already held modifies nothing.""" + resource = Tagged(tags=["red", "blue"]) + patch = PatchOp[Tagged].model_validate( + { + "Operations": [ + {"op": "replace", "path": 'tags[value eq "red"]', "value": "red"} + ] + } + ) + + assert patch.patch(resource) is False + + +@pytest.mark.parametrize("path", [None, "emails"]) +def test_a_replace_writing_the_entries_held_reports_no_change(path): + """A patch leaving the resource as it was reports no modification.""" + user = _user_with_work_email() + entries = [ + {"value": "bjensen@example.com", "type": "work", "display": "Work"}, + {"value": "babs@example.com", "type": "home"}, + ] + operation = ( + {"op": "replace", "value": {"emails": entries}} + if path is None + else {"op": "replace", "path": path, "value": entries} + ) + + assert ( + PatchOp[User].model_validate({"Operations": [operation]}).patch(user) is False + ) + + +def test_a_replace_bringing_two_primary_entries_is_refused(): + """RFC7643 §2.4 allows primary to be true on one value at most.""" + user = User( + user_name="bjensen", + emails=[{"value": "bjensen@example.com", "primary": True}], + ) + patch = PatchOp[User].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": "emails", + "value": [ + {"value": "babs@example.com", "primary": True}, + {"value": "barbara@example.com", "primary": True}, + ], + } + ] + } + ) + + with pytest.raises(InvalidValueException): + patch.patch(user) + + assert [email.value for email in user.emails] == ["bjensen@example.com"] + + +def test_a_replace_bringing_one_primary_entry_keeps_it(): + """A replaced list holding a single primary entry keeps it primary.""" + user = User( + user_name="bjensen", + emails=[{"value": "bjensen@example.com", "primary": True}], + ) + patch = PatchOp[User].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": "emails", + "value": [ + {"value": "babs@example.com"}, + {"value": "barbara@example.com", "primary": True}, + ], + } + ] + } + ) + + assert patch.patch(user) + assert [email.primary for email in user.emails] == [None, True] + + +@pytest.mark.parametrize("op", ["add", "replace"]) +@pytest.mark.parametrize( + "value", [{}, {"whatever": "x"}], ids=["empty", "only-undeclared"] +) +def test_a_value_writing_nothing_still_needs_a_target(op, value): + """The filter is resolved even when the value writes nothing to the entries.""" + user = User( + user_name="bjensen", emails=[{"value": "b@example.com", "type": "home"}] + ) + patch = PatchOp[User].model_validate( + {"Operations": [{"op": op, "path": 'emails[type eq "work"]', "value": value}]}, + scim_policy=ScimPolicy(unknown=ScimPolicy.Unknown.ignore), + ) + + with pytest.raises(NoTargetException): + patch.patch(user, scim_policy=ScimPolicy(unknown=ScimPolicy.Unknown.ignore)) + + +@pytest.mark.parametrize("op", ["add", "replace"]) +def test_a_value_writing_nothing_still_needs_a_valid_filter(op): + """A filter on an undeclared sub-attribute is rejected, whatever the value.""" + user = User( + user_name="bjensen", emails=[{"value": "b@example.com", "type": "home"}] + ) + patch = PatchOp[User].model_validate( + {"Operations": [{"op": op, "path": 'emails[whatever eq "x"]', "value": {}}]} + ) + + with pytest.raises(InvalidFilterException): + patch.patch(user) diff --git a/tests/test_patch_op_validation.py b/tests/test_patch_op_validation.py index 1a0a1dc0..f078840c 100644 --- a/tests/test_patch_op_validation.py +++ b/tests/test_patch_op_validation.py @@ -1,3 +1,5 @@ +from datetime import UTC +from datetime import datetime from typing import Annotated from typing import TypeVar @@ -5,6 +7,7 @@ from pydantic import ValidationError from scim2_models import URN +from scim2_models import ComplexAttribute from scim2_models import Extension from scim2_models import Group from scim2_models import InvalidFilterException @@ -12,9 +15,13 @@ from scim2_models import InvalidValueException from scim2_models import Mutability from scim2_models import MutabilityException +from scim2_models import NoTargetException from scim2_models import PatchOp from scim2_models import PatchOperation +from scim2_models import Path +from scim2_models import PathNotFoundException from scim2_models import Required +from scim2_models import ScimPolicy from scim2_models import User from scim2_models.base import Context from scim2_models.resources.resource import Resource @@ -24,6 +31,24 @@ class ImmutableFieldResource(Resource): locked: Annotated[str | None, Mutability.immutable] = None +class ConstrainedComplex(ComplexAttribute): + label: str | None = None + + +class ConstrainedResource(Resource): + __schema__ = URN("urn:example:2.0:ConstrainedResource") + + read_only_attr: Annotated[str | None, Mutability.read_only] = None + immutable_attr: Annotated[str | None, Mutability.immutable] = None + immutable_complex: Annotated[ConstrainedComplex | None, Mutability.immutable] = None + immutable_date: Annotated[datetime | None, Mutability.immutable] = None + immutable_list: Annotated[list[ConstrainedComplex] | None, Mutability.immutable] = ( + None + ) + required_attr: Annotated[str | None, Required.true] = None + plain_attr: str | None = None + + class ConstrainedExtension(Extension): __schema__ = URN("urn:example:2.0:Constrained") @@ -152,7 +177,7 @@ def test_path_required_for_remove_operations(): { "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "operations": [ - {"op": "replace", "value": "foobar"}, + {"op": "replace", "value": {"nickName": "foobar"}}, ], }, context={"scim": Context.RESOURCE_PATCH_REQUEST}, @@ -161,7 +186,7 @@ def test_path_required_for_remove_operations(): { "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "operations": [ - {"op": "add", "value": "foobar"}, + {"op": "add", "value": {"nickName": "foobar"}}, ], }, context={"scim": Context.RESOURCE_PATCH_REQUEST}, @@ -186,15 +211,6 @@ def test_value_required_for_add_operations(): :rfc:`RFC7644 §3.5.2.1 <7644#section-3.5.2.1>`: "The operation MUST contain a 'value' member whose content specifies the value to be added." """ - PatchOp[User].model_validate( - { - "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], - "operations": [ - {"op": "replace", "path": "nickName"}, - ], - }, - context={"scim": Context.RESOURCE_PATCH_REQUEST}, - ) with pytest.raises(ValidationError): PatchOp[User].model_validate( { @@ -406,45 +422,6 @@ def test_remove_operation_on_non_required_field_allowed(): ) -def test_patch_operations_with_none_path_skipped(): - """Test that patch operations with None path are skipped during validation.""" - # Create a patch operation with None path (bypassing normal validation) - patch_op = PatchOp[User]( - operations=[ - PatchOperation.model_construct( - op=PatchOperation.Op.add, path=None, value="test" - ) - ] - ) - - # The _validate_operations method should skip operations with None path - # This should not raise an error - user = User(user_name="test") - result = patch_op.patch(user) - assert result is False # Should return False because operation was skipped - - -def test_patch_operation_with_schema_only_urn_path(): - """Test patch operation with URN path that contains only schema.""" - # Test edge case where extract_field_name returns None for schema-only URNs - user = User(user_name="test") - - # This URN resolves to just the schema without an attribute name - patch = PatchOp[User]( - operations=[ - PatchOperation[User]( - op=PatchOperation.Op.add, - path="urn:ietf:params:scim:schemas:core:2.0:User", - value="test", - ) - ] - ) - - # This should trigger the path where extract_field_name returns None - with pytest.raises(InvalidPathException): - patch.patch(user) - - def test_add_remove_operations_on_group_members_allowed(): """Test that add/remove operations work on group collections.""" # Test operations on group collection (not the immutable value field) @@ -475,7 +452,7 @@ def test_patch_error_handling_no_operations(): def test_patch_error_handling_type_mismatch(): - """Test error handling when patch value type doesn't match field type.""" + """RFC7644 §3.12 returns invalidValue for a value that does not fit the attribute type.""" user = User(user_name="test") # Try to set active (boolean) to a string @@ -487,9 +464,11 @@ def test_patch_error_handling_type_mismatch(): ] ) - with pytest.raises(ValidationError): + with pytest.raises(InvalidValueException) as raised: patch.patch(user) + assert raised.value.scim_type == "invalidValue" + T = TypeVar("T", bound=Resource) UserT = TypeVar("UserT", bound=User) @@ -729,32 +708,79 @@ def test_a_replace_may_unassign_an_optional_attribute(): assert user.display_name is None -def test_an_operation_without_path_cannot_write_a_read_only_attribute(): - """:rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>` has the value name the attributes to write when the path is omitted, so each of them answers to §3.5.2 like a named path.""" - with pytest.raises(ValidationError, match="mutability"): - PatchOp[User].model_validate( - { - "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], - "Operations": [ - {"op": "replace", "value": {"id": "chosen-by-the-client"}} - ], - }, - scim_ctx=Context.RESOURCE_PATCH_REQUEST, - ) +def test_an_operation_without_path_cannot_change_a_read_only_attribute(): + """RFC7644 §3.5.2 forbids changing a read-only attribute, here set in the value.""" + patch = PatchOp[User].model_validate( + { + "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations": [{"op": "replace", "value": {"id": "chosen-by-the-client"}}], + }, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + user = User(id="srv-1", user_name="bjensen") + with pytest.raises(MutabilityException): + patch.patch(user) + assert user.id == "srv-1" -def test_an_operation_without_path_cannot_write_a_read_only_complex_attribute(): - """Naming a complex attribute is enough to refuse it, without descending into its sub-attributes.""" - with pytest.raises(ValidationError, match="mutability"): - PatchOp[User].model_validate( - { - "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], - "Operations": [ - {"op": "replace", "value": {"meta": {"resourceType": "Group"}}} - ], - }, - scim_ctx=Context.RESOURCE_PATCH_REQUEST, - ) + +def test_an_operation_without_path_cannot_assign_a_read_only_complex_attribute(): + """A client cannot set a read-only complex attribute that has no value.""" + patch = PatchOp[User].model_validate( + { + "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations": [ + {"op": "replace", "value": {"meta": {"resourceType": "Group"}}} + ], + }, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + user = User(id="srv-1", user_name="bjensen") + + with pytest.raises(MutabilityException): + patch.patch(user) + assert user.meta is None + + +def test_an_operation_without_path_ignores_a_read_only_attribute_sent_back_unchanged(): + """Okta sends back the id of a group it renames, and RFC7643 §3.1 says to ignore it.""" + patch = PatchOp[Group].model_validate( + { + "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations": [ + {"op": "replace", "value": {"id": "grp-1", "displayName": "Admins"}} + ], + }, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + group = Group(id="grp-1", display_name="Staff") + + assert patch.patch(group) + assert group.id == "grp-1" + assert group.display_name == "Admins" + + +def test_an_operation_without_path_ignores_a_resource_sent_back_as_it_was_read(): + """The read-only meta of a dumped resource compares equal once parsed, whatever its spelling.""" + user = User( + id="srv-1", + user_name="bjensen", + meta={ + "resourceType": "User", + "created": datetime(2020, 1, 1, tzinfo=UTC), + "version": 'W/"1"', + }, + ) + value = user.model_dump(scim_ctx=Context.RESOURCE_QUERY_RESPONSE) + value["nickName"] = "Babs" + patch = PatchOp[User].model_validate( + {"Operations": [{"op": "replace", "value": value}]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + assert patch.patch(user) + assert user.nick_name == "Babs" + assert user.meta.version == 'W/"1"' def test_an_operation_without_path_cannot_unassign_a_required_attribute(): @@ -771,17 +797,32 @@ def test_an_operation_without_path_cannot_unassign_a_required_attribute(): ) -def test_an_add_without_path_cannot_write_a_read_only_attribute(): - """:rfc:`RFC7644 §3.5.2.1 <7644#section-3.5.2.1>` gives ``add`` the same omitted path as ``replace``.""" - with pytest.raises(ValidationError, match="mutability"): +def test_an_add_without_path_cannot_change_a_read_only_attribute(): + """RFC7644 §3.5.2.1 lets add omit the path, as replace does.""" + patch = PatchOp[User].model_validate( + { + "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations": [{"op": "add", "value": {"id": "chosen-by-the-client"}}], + }, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + user = User(id="srv-1", user_name="bjensen") + + with pytest.raises(MutabilityException): + patch.patch(user) + assert user.id == "srv-1" + + +def test_a_path_to_a_read_only_attribute_is_refused_even_unchanged(): + """A path to a read-only attribute targets it on purpose, whatever the value.""" + with pytest.raises(ValidationError) as raised: PatchOp[User].model_validate( - { - "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], - "Operations": [{"op": "add", "value": {"id": "chosen-by-the-client"}}], - }, + {"Operations": [{"op": "replace", "path": "id", "value": "srv-1"}]}, scim_ctx=Context.RESOURCE_PATCH_REQUEST, ) + assert raised.value.errors()[0]["type"] == "scim_mutability" + def test_an_operation_without_path_refuses_an_undeclared_attribute(): """§3.5.2 has an operation incompatible with an attribute's schema return an error. @@ -804,7 +845,7 @@ def test_an_operation_without_path_refuses_an_undeclared_attribute(): def test_an_operation_without_path_may_unassign_an_extension(): - """An extension is no attribute of the resource, so none of it is required.""" + """An extension is not an attribute of the resource, so it is not required.""" patch = PatchOp[User[ConstrainedExtension]].model_validate( { "Operations": [ @@ -891,15 +932,19 @@ def test_an_operation_without_path_takes_a_model_as_value(): def test_a_model_value_answers_to_the_same_constraints_as_a_dict_one(): """The form the value takes says nothing about what the operation may write.""" - with pytest.raises(ValidationError, match="mutability"): - PatchOp[User]( - operations=[ - PatchOperation[User]( - op=PatchOperation.Op.replace_, - value=User(id="chosen-by-the-client"), - ) - ] - ) + patch = PatchOp[User]( + operations=[ + PatchOperation[User]( + op=PatchOperation.Op.replace_, + value=User(id="chosen-by-the-client"), + ) + ] + ) + user = User(id="srv-1", user_name="bjensen") + + with pytest.raises(MutabilityException): + patch.patch(user) + assert user.id == "srv-1" def test_a_model_value_carries_the_attributes_its_payload_would(): @@ -1109,3 +1154,1047 @@ def test_a_path_designating_the_resource_itself_is_accepted(): user = User(user_name="bjensen") patch.patch(user) assert user.nick_name == "Babs" + + +CONSTRAINED_RESOURCE_URN = "urn:example:2.0:ConstrainedResource" +CONSTRAINED_EXTENSION_URN = "urn:example:2.0:Constrained" +CORE_USER_URN = "urn:ietf:params:scim:schemas:core:2.0:User" + + +def _constrained_resource(): + return ConstrainedResource( + read_only_attr="original", + immutable_attr="original", + immutable_complex=ConstrainedComplex(label="original"), + immutable_date=datetime(2020, 1, 1, tzinfo=UTC), + immutable_list=[ConstrainedComplex(label="original")], + required_attr="original", + ) + + +def _operation(op, path, value): + operation = {"op": op, "value": value} + if path is not None: + operation["path"] = path + return operation + + +RESOURCE_PATHS = [ + pytest.param(None, id="no path"), + pytest.param("", id="empty path"), + pytest.param(CONSTRAINED_RESOURCE_URN, id="schema URN"), + pytest.param(CONSTRAINED_RESOURCE_URN.upper(), id="schema URN in capitals"), +] + +USER_PATHS = [ + pytest.param(None, id="no path"), + pytest.param("", id="empty path"), + pytest.param(CORE_USER_URN, id="schema URN"), + pytest.param(CORE_USER_URN.upper(), id="schema URN in capitals"), +] + +VALIDATION_VIOLATIONS = [ + pytest.param("replace", "requiredAttr", None, id="required unassigned"), +] + +APPLICATION_VIOLATIONS = [ + pytest.param("replace", "readOnlyAttr", "hijacked", id="read-only replaced"), + pytest.param("add", "readOnlyAttr", "hijacked", id="read-only added"), + pytest.param("replace", "immutableAttr", "hijacked", id="immutable replaced"), + pytest.param("add", "immutableAttr", "hijacked", id="immutable added over a value"), +] + + +def _assert_constraints_kept(holder): + assert holder.read_only_attr == "original" + assert holder.immutable_attr == "original" + assert holder.required_attr == "original" + + +def _assert_refused_for_mutability(raised): + assert raised.value.errors()[0]["type"] == "scim_mutability" + + +@pytest.mark.parametrize("path", RESOURCE_PATHS) +@pytest.mark.parametrize(("op", "attribute", "value"), VALIDATION_VIOLATIONS) +def test_a_path_designating_the_resource_refuses_the_value_at_validation( + path, op, attribute, value +): + """When the path targets the resource, the value holds the attributes (RFC7644 §3.5.2.3).""" + with pytest.raises(ValidationError) as raised: + PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation(op, path, {attribute: value})]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + _assert_refused_for_mutability(raised) + + +@pytest.mark.parametrize("path", RESOURCE_PATHS) +@pytest.mark.parametrize(("op", "attribute", "value"), APPLICATION_VIOLATIONS) +def test_a_path_designating_the_resource_refuses_a_constrained_change_when_applied( + path, op, attribute, value +): + """Only the resource tells whether the value changes a read-only or immutable attribute, so this is checked when the patch is applied.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation(op, path, {attribute: value})]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + with pytest.raises(MutabilityException): + patch.patch(resource) + + _assert_constraints_kept(resource) + + +@pytest.mark.parametrize("path", USER_PATHS) +@pytest.mark.parametrize( + "value", + [ + pytest.param({"id": "other-id"}, id="id"), + pytest.param({"groups": [{"value": "admins"}]}, id="groups"), + pytest.param({"meta": {"version": 'W/"999"'}}, id="meta"), + ], +) +def test_a_path_designating_the_resource_cannot_rewrite_its_identity(path, value): + """The read-only attributes of a user are rejected, whatever path targets the user.""" + user = User(id="srv-1", user_name="bjensen") + patch = PatchOp[User].model_validate( + {"Operations": [_operation("replace", path, value)]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + with pytest.raises(MutabilityException): + patch.patch(user) + assert user.id == "srv-1" + assert user.groups is None + assert user.meta is None + + +@pytest.mark.parametrize("path", RESOURCE_PATHS) +@pytest.mark.parametrize( + ("op", "attribute", "value"), + [ + pytest.param("replace", "plainAttr", "written", id="plain replaced"), + pytest.param("replace", "immutableAttr", "original", id="immutable kept"), + pytest.param("replace", "readOnlyAttr", "original", id="read-only kept"), + pytest.param("add", "readOnlyAttr", "original", id="read-only added unchanged"), + ], +) +def test_a_path_designating_the_resource_writes_what_the_schema_allows( + path, op, attribute, value +): + """The checks reject what the constraints forbid, and nothing else.""" + resource = _constrained_resource() + + PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation(op, path, {attribute: value})]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ).patch(resource) + + assert resource.model_dump(by_alias=True)[attribute] == value + + +IMMUTABLE_VALUES_AS_SPELLED = [ + pytest.param("immutableAttr", "original", id="string"), + pytest.param("immutableComplex", {"LABEL": "original"}, id="complex"), + pytest.param("immutableDate", "2020-01-01T00:00:00Z", id="datetime"), + pytest.param("immutableList", [{"label": "original"}], id="multi-valued"), +] + + +def test_an_immutable_multi_valued_attribute_takes_back_its_entries_in_another_order(): + """RFC7643 §2.4 gives no significance to the order of a multi-valued attribute.""" + resource = _constrained_resource() + resource.immutable_list = [ + ConstrainedComplex(label="first"), + ConstrainedComplex(label="second"), + ] + patch = PatchOp[ConstrainedResource].model_validate( + { + "Operations": [ + _operation( + "replace", + None, + {"immutableList": [{"label": "second"}, {"label": "first"}]}, + ) + ] + }, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + patch.patch(resource) + + assert {entry.label for entry in resource.immutable_list} == {"first", "second"} + + +@pytest.mark.parametrize( + "value", + [ + pytest.param([{"label": "original"}, {"label": "other"}], id="entry added"), + pytest.param([{"label": "other"}], id="entry replaced"), + ], +) +def test_an_immutable_multi_valued_attribute_refuses_other_entries(value): + """Comparing entries regardless of order still detects a different collection.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation("replace", None, {"immutableList": value})]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + with pytest.raises(MutabilityException): + patch.patch(resource) + + assert resource.immutable_list == [ConstrainedComplex(label="original")] + + +@pytest.mark.parametrize("op", ["add", "replace"]) +@pytest.mark.parametrize(("attribute", "value"), IMMUTABLE_VALUES_AS_SPELLED) +@pytest.mark.parametrize("path", RESOURCE_PATHS) +def test_an_immutable_attribute_takes_back_the_value_it_holds_through_the_value( + path, attribute, value, op +): + """A client that sends the whole state sends the current value of an immutable attribute as JSON.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation(op, path, {attribute: value})]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + patch.patch(resource) + + assert resource.model_dump() == _constrained_resource().model_dump() + + +@pytest.mark.parametrize("op", ["add", "replace"]) +@pytest.mark.parametrize(("attribute", "value"), IMMUTABLE_VALUES_AS_SPELLED) +def test_an_immutable_attribute_takes_back_the_value_it_holds_through_an_attribute_path( + attribute, value, op +): + """RFC7644 §3.5.2.1 makes no change when the target already holds the value.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation(op, attribute, value)]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + patch.patch(resource) + + assert resource.model_dump() == _constrained_resource().model_dump() + + +def test_an_immutable_attribute_refuses_a_value_its_type_does_not_accept(): + """A value the attribute rejects is reported as invalid before any mutability check.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation("replace", None, {"immutableDate": "not a date"})]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + with pytest.raises(InvalidValueException): + patch.patch(resource) + + assert resource.immutable_date == datetime(2020, 1, 1, tzinfo=UTC) + + +@pytest.mark.parametrize("op", ["add", "replace"]) +@pytest.mark.parametrize( + "path", [*RESOURCE_PATHS, pytest.param("immutableAttr", id="attribute path")] +) +def test_an_immutable_attribute_without_value_takes_its_first_value(path, op): + """§3.5.2 lets a client assign an immutable attribute with no value, and §3.5.2.3 treats a replace on it as an add.""" + resource = ConstrainedResource(required_attr="original") + value = "first" if path == "immutableAttr" else {"immutableAttr": "first"} + + PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation(op, path, value)]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ).patch(resource) + + assert resource.immutable_attr == "first" + + +def test_a_path_designating_the_resource_refuses_an_undeclared_attribute(): + """An attribute the schema URN does not declare is incompatible with the resource schema.""" + with pytest.raises(ValidationError) as raised: + PatchOp[ConstrainedResource].model_validate( + { + "Operations": [ + _operation("replace", CONSTRAINED_RESOURCE_URN, {"whatever": "x"}) + ] + }, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +@pytest.mark.parametrize("path", ["", CORE_USER_URN]) +def test_a_path_designating_the_resource_writes_a_resource_given_as_value(path): + """A resource given as the value writes the attributes it was checked against.""" + user = User(user_name="bjensen") + patch = PatchOp[User]( + operations=[ + PatchOperation[User]( + op=PatchOperation.Op.replace_, path=path, value=User(nick_name="Babs") + ) + ] + ) + + assert patch.patch(user) + assert user.nick_name == "Babs" + + +@pytest.mark.parametrize( + "path", + [ + pytest.param("", id="empty path"), + pytest.param(CORE_USER_URN, id="resource schema URN"), + pytest.param(CONSTRAINED_EXTENSION_URN, id="extension schema URN"), + ], +) +def test_a_remove_selecting_by_value_needs_a_path_to_an_attribute(path): + """Under the apply policy the value of a remove selects entries of the attribute in its path.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User[ConstrainedExtension]].model_validate( + {"Operations": [_operation("remove", path, [{"plainAttr": "x"}])]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + scim_policy=ScimPolicy(remove_value_as_filter=ScimPolicy.RemoveValue.apply), + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidPath" + + +def test_a_remove_on_the_schema_urn_of_an_extension_unassigns_it(): + """An extension is not an attribute of the resource, so it is not required.""" + user = User[ConstrainedExtension](user_name="bjensen") + user[ConstrainedExtension] = ConstrainedExtension( + required_attr="original", plain_attr="original" + ) + + PatchOp[User[ConstrainedExtension]].model_validate( + {"Operations": [{"op": "remove", "path": CONSTRAINED_EXTENSION_URN}]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ).patch(user) + + assert user[ConstrainedExtension] is None + + +EXTENSION_TARGETS = [ + pytest.param(None, CONSTRAINED_EXTENSION_URN, id="no path"), + pytest.param( + None, CONSTRAINED_EXTENSION_URN.upper(), id="no path, key in capitals" + ), + pytest.param("", CONSTRAINED_EXTENSION_URN, id="empty path"), + pytest.param(CORE_USER_URN, CONSTRAINED_EXTENSION_URN, id="resource schema URN"), + pytest.param(CONSTRAINED_EXTENSION_URN, None, id="extension schema URN"), + pytest.param( + CONSTRAINED_EXTENSION_URN.upper(), None, id="extension schema URN in capitals" + ), +] + + +def _extension_value(key, attribute, value): + written = {attribute: value} + return {key: written} if key is not None else written + + +@pytest.mark.parametrize(("path", "key"), EXTENSION_TARGETS) +@pytest.mark.parametrize(("op", "attribute", "value"), VALIDATION_VIOLATIONS) +def test_an_extension_in_the_value_refuses_its_constrained_attributes_at_validation( + path, key, op, attribute, value +): + """The attributes of an extension in the value follow the constraints the extension declares.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User[ConstrainedExtension]].model_validate( + { + "Operations": [ + _operation(op, path, _extension_value(key, attribute, value)) + ] + }, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + _assert_refused_for_mutability(raised) + + +@pytest.mark.parametrize(("path", "key"), EXTENSION_TARGETS) +@pytest.mark.parametrize(("op", "attribute", "value"), APPLICATION_VIOLATIONS) +def test_an_extension_in_the_value_refuses_a_constrained_change_when_applied( + path, key, op, attribute, value +): + """A read-only or immutable attribute of an extension in the value is protected once it has a value.""" + user = _constrained_user() + patch = PatchOp[User[ConstrainedExtension]].model_validate( + {"Operations": [_operation(op, path, _extension_value(key, attribute, value))]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + with pytest.raises(MutabilityException): + patch.patch(user) + + _assert_constraints_kept(user[ConstrainedExtension]) + + +@pytest.mark.parametrize(("path", "key"), EXTENSION_TARGETS) +def test_an_extension_in_the_value_refuses_an_attribute_it_does_not_declare(path, key): + """An attribute outside the extension schema is incompatible with it.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User[ConstrainedExtension]].model_validate( + { + "Operations": [ + _operation("replace", path, _extension_value(key, "whatever", "x")) + ] + }, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +def _user_with_hijacked_extension(): + user = User[ConstrainedExtension]() + user[ConstrainedExtension] = ConstrainedExtension(read_only_attr="hijacked") + return user + + +@pytest.mark.parametrize( + ("path", "value"), + [ + pytest.param( + CONSTRAINED_EXTENSION_URN, + ConstrainedExtension(read_only_attr="hijacked"), + id="extension on its schema URN", + ), + pytest.param(None, _user_with_hijacked_extension(), id="resource without path"), + ], +) +def test_an_extension_given_as_a_model_answers_for_its_own_constraints(path, value): + """A value built in Python is checked like the same value in a payload.""" + user = _constrained_user() + patch = PatchOp[User[ConstrainedExtension]]( + operations=[ + PatchOperation[User[ConstrainedExtension]]( + op=PatchOperation.Op.replace_, path=path, value=value + ) + ] + ) + + with pytest.raises(MutabilityException): + patch.patch(user) + _assert_constraints_kept(user[ConstrainedExtension]) + + +def _unvalidated_patch_writing_an_undeclared_attribute(): + return PatchOp[ConstrainedResource].model_construct( + operations=[ + PatchOperation[ConstrainedResource].model_construct( + op=PatchOperation.Op.replace_, path=None, value={"whatever": "x"} + ) + ] + ) + + +def test_an_unvalidated_patch_refuses_an_undeclared_attribute_when_applied(): + """The policy passed to patch decides what happens to undeclared attributes.""" + patch = _unvalidated_patch_writing_an_undeclared_attribute() + + with pytest.raises(InvalidValueException): + patch.patch(_constrained_resource()) + + +def test_an_unvalidated_patch_drops_an_undeclared_attribute_under_a_tolerant_policy(): + """A tolerant policy drops the attribute, and the operation writes nothing.""" + patch = _unvalidated_patch_writing_an_undeclared_attribute() + tolerant = ScimPolicy(unknown=ScimPolicy.Unknown.ignore) + + assert patch.patch(_constrained_resource(), scim_policy=tolerant) is False + + +def test_an_unvalidated_patch_leaves_an_undeclared_path_to_the_write(): + """An undeclared path has no immutability constraint, and fails when the write finds no field.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_construct( + operations=[ + PatchOperation[ConstrainedResource].model_construct( + op=PatchOperation.Op.replace_, + path=Path[ConstrainedResource]("whatever"), + value="x", + ) + ] + ) + + with pytest.raises(PathNotFoundException): + patch.patch(resource) + + +def test_a_remove_without_path_carrying_a_value_has_no_target_outside_a_request(): + """RFC7644 §3.5.2.2 returns noTarget for a remove without a path, whatever its value.""" + patch = PatchOp[User].model_validate( + {"Operations": [{"op": "remove", "value": {"nickName": "x"}}]} + ) + + with pytest.raises(NoTargetException): + patch.patch(User(user_name="bjensen")) + + +def test_a_remove_carrying_a_value_on_a_schema_urn_is_refused_by_default_outside_a_request(): + """Without the apply policy, the value of a remove is rejected, as on any path.""" + patch = PatchOp[User].model_validate( + { + "Operations": [ + {"op": "remove", "path": CORE_USER_URN, "value": [{"nickName": "x"}]} + ] + } + ) + + with pytest.raises(InvalidValueException): + patch.patch(User(user_name="bjensen")) + + +@pytest.mark.parametrize( + "unknown", [ScimPolicy.Unknown.ignore, ScimPolicy.Unknown.keep], ids=str +) +@pytest.mark.parametrize(("path", "key"), EXTENSION_TARGETS) +def test_a_tolerant_policy_writes_the_declared_attributes_beside_an_undeclared_one( + path, key, unknown +): + """The unknown policy applies to the value of a patch as it does to a resource.""" + policy = ScimPolicy(unknown=unknown) + user = _constrained_user() + value = _extension_value(key, "plainAttr", "written") + if key is not None: + value[key]["whatever"] = "x" + else: + value["whatever"] = "x" + + PatchOp[User[ConstrainedExtension]].model_validate( + {"Operations": [_operation("replace", path, value)]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + scim_policy=policy, + ).patch(user, scim_policy=policy) + + dumped = user.model_dump(scim_ctx=Context.RESOURCE_QUERY_RESPONSE) + assert dumped[CONSTRAINED_EXTENSION_URN]["plainAttr"] == "written" + assert "whatever" not in dumped[CONSTRAINED_EXTENSION_URN] + + +@pytest.mark.parametrize( + "unknown", [ScimPolicy.Unknown.ignore, ScimPolicy.Unknown.keep], ids=str +) +@pytest.mark.parametrize("path", RESOURCE_PATHS) +def test_a_tolerant_policy_drops_an_undeclared_attribute_of_the_resource(path, unknown): + """An undeclared attribute has no field to write to.""" + policy = ScimPolicy(unknown=unknown) + resource = _constrained_resource() + + PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation("replace", path, {"plainAttr": "w", "zzz": "x"})]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + scim_policy=policy, + ).patch(resource, scim_policy=policy) + + assert resource.plain_attr == "w" + assert "zzz" not in resource.model_dump() + + +EXTENSION_UNASSIGNMENTS = [ + pytest.param( + {"op": "remove", "path": CONSTRAINED_EXTENSION_URN}, id="removed by URN" + ), + pytest.param( + {"op": "replace", "value": {CONSTRAINED_EXTENSION_URN: None}}, + id="null key without path", + ), + pytest.param( + {"op": "replace", "path": "", "value": {CONSTRAINED_EXTENSION_URN: None}}, + id="null key on the empty path", + ), + pytest.param( + { + "op": "replace", + "path": CORE_USER_URN, + "value": {CONSTRAINED_EXTENSION_URN: None}, + }, + id="null key on the resource URN", + ), +] + + +@pytest.mark.parametrize("operation", EXTENSION_UNASSIGNMENTS) +def test_an_extension_holding_an_immutable_value_cannot_be_unassigned(operation): + """Unassigning an extension changes its immutable attributes, which RFC7644 §3.5.2 forbids once they hold a value.""" + user = _constrained_user() + patch = PatchOp[User[ConstrainedExtension]].model_validate( + {"Operations": [operation]}, scim_ctx=Context.RESOURCE_PATCH_REQUEST + ) + + with pytest.raises(MutabilityException): + patch.patch(user) + + _assert_constraints_kept(user[ConstrainedExtension]) + + +@pytest.mark.parametrize("operation", EXTENSION_UNASSIGNMENTS) +def test_an_extension_without_immutable_value_can_be_unassigned(operation): + """Unassigning an extension does not change an immutable attribute that has no value yet.""" + user = User[ConstrainedExtension](user_name="bjensen") + user[ConstrainedExtension] = ConstrainedExtension( + read_only_attr="original", required_attr="original" + ) + + PatchOp[User[ConstrainedExtension]].model_validate( + {"Operations": [operation]}, scim_ctx=Context.RESOURCE_PATCH_REQUEST + ).patch(user) + + assert user[ConstrainedExtension] is None + + +@pytest.mark.parametrize("path", ["", CORE_USER_URN]) +def test_a_remove_designating_the_resource_itself_is_refused(path): + """A remove on the resource root has no attribute to unassign.""" + patch = PatchOp[User].model_validate( + {"Operations": [{"op": "remove", "path": path}]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + with pytest.raises(InvalidPathException): + patch.patch(User(user_name="bjensen")) + + +def test_building_the_unassignment_of_an_extension_holding_an_immutable_value_is_refused(): + """The peer would reject the patch, so building it fails.""" + after = User[ConstrainedExtension](user_name="bjensen") + after.ConstrainedExtension = None + + with pytest.raises(MutabilityException): + PatchOp.build_from(_constrained_user(), after) + + +def test_the_unassignment_built_for_an_extension_without_immutable_value_applies(): + """A patch built from two states is one the resource accepts.""" + before = User[ConstrainedExtension](user_name="bjensen") + before[ConstrainedExtension] = ConstrainedExtension(plain_attr="original") + after = User[ConstrainedExtension](user_name="bjensen") + after.ConstrainedExtension = None + + patch = PatchOp.build_from(before, after) + + assert patch.patch(before) + assert before[ConstrainedExtension] is None + + +def test_a_replace_unassigning_its_target_keeps_its_null_value_when_dumped(): + """A replace dumped without its value would be read back as missing its value.""" + patch = PatchOp[User]( + operations=[PatchOperation[User](op="replace", path="title", value=None)] + ) + + payload = patch.model_dump(scim_ctx=Context.RESOURCE_PATCH_REQUEST) + + assert payload["Operations"] == [{"op": "replace", "path": "title", "value": None}] + user = User(user_name="bjensen", title="CEO") + PatchOp[User].model_validate( + payload, scim_ctx=Context.RESOURCE_PATCH_REQUEST + ).patch(user) + assert user.title is None + + +def test_an_operation_given_no_value_is_dumped_without_one(): + """Only a null value the operation was given is kept in the dump.""" + operation = PatchOperation[User](op="replace", path="title") + + assert operation.model_dump(scim_ctx=Context.RESOURCE_PATCH_REQUEST) == { + "op": "replace", + "path": "title", + } + + +def test_a_remove_given_a_null_value_is_dumped_without_one(): + """RFC7644 §3.5.2.2 reads a remove from its path only, so a null value is left out.""" + operation = PatchOperation[User](op="remove", path="title", value=None) + + assert operation.model_dump(scim_ctx=Context.RESOURCE_PATCH_REQUEST) == { + "op": "remove", + "path": "title", + } + + +@pytest.mark.parametrize("path", [None, "nickName"]) +def test_a_replace_needs_a_value(path): + """RFC7644 §3.5.2.3 replaces the target with the value, and a missing one would read as null.""" + operation = {"op": "replace"} if path is None else {"op": "replace", "path": path} + + with pytest.raises(ValidationError) as raised: + PatchOp[User].model_validate( + {"Operations": [operation]}, scim_ctx=Context.RESOURCE_PATCH_REQUEST + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +@pytest.mark.parametrize("op", ["add", "replace"]) +@pytest.mark.parametrize("value", ["Babs", ["Babs"], None]) +@pytest.mark.parametrize("path", [*USER_PATHS, CONSTRAINED_EXTENSION_URN]) +def test_a_path_designating_a_model_needs_an_object_value(op, value, path): + """RFC7644 §3.5.2.1 and §3.5.2.3 require that value to be a set of attributes.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User[ConstrainedExtension]].model_validate( + {"Operations": [_operation(op, path, value)]} + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +@pytest.mark.parametrize("path", [None, CONSTRAINED_EXTENSION_URN]) +def test_an_unvalidated_non_object_value_is_refused_when_applied(path): + """A patch built in Python reaches no validator, and is refused when applied.""" + patch = PatchOp[User[ConstrainedExtension]].model_construct( + operations=[ + PatchOperation[User[ConstrainedExtension]].model_construct( + op=PatchOperation.Op.add, + path=None if path is None else Path[User[ConstrainedExtension]](path), + value="Babs", + ) + ] + ) + + with pytest.raises(InvalidValueException): + patch.patch(User[ConstrainedExtension](user_name="bjensen")) + + +@pytest.mark.parametrize("path", USER_PATHS) +def test_an_extension_in_a_value_needs_an_object(path): + """An extension is a container of attributes, not a value of its own.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User[ConstrainedExtension]].model_validate( + {"Operations": [_operation("add", path, {CONSTRAINED_EXTENSION_URN: "x"})]} + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +def _group_with_member(): + return Group( + display_name="Tour Guides", members=[{"value": "2819c223", "display": "Babs"}] + ) + + +@pytest.mark.parametrize( + "path", ['members[value eq "2819c223"].value', "members.value"] +) +def test_an_immutable_sub_attribute_of_an_entry_cannot_change(path): + """RFC7643 §4.2 lets members be added and removed, and makes their sub-attributes immutable.""" + group = _group_with_member() + patch = PatchOp[Group].model_validate( + {"Operations": [{"op": "replace", "path": path, "value": "c3a26dd3"}]} + ) + + with pytest.raises(MutabilityException): + patch.patch(group) + + assert group.members[0].value == "2819c223" + + +def test_a_mutable_sub_attribute_of_an_entry_may_change(): + """Only the sub-attributes declared immutable must keep their value.""" + group = _group_with_member() + patch = PatchOp[Group].model_validate( + { + "Operations": [ + { + "op": "replace", + "path": 'members[value eq "2819c223"].display', + "value": "Barbara", + } + ] + } + ) + + assert patch.patch(group) + assert group.members[0].display == "Barbara" + + +def test_an_entry_holding_immutable_sub_attributes_may_be_removed(): + """Removing a member changes none of the sub-attributes of the ones left.""" + group = Group( + display_name="Tour Guides", + members=[{"value": "2819c223"}, {"value": "c3a26dd3"}], + ) + patch = PatchOp[Group].model_validate( + {"Operations": [{"op": "remove", "path": 'members[value eq "2819c223"]'}]} + ) + + assert patch.patch(group) + assert [member.value for member in group.members] == ["c3a26dd3"] + + +def test_an_entry_kept_through_an_assignment_is_the_same_object(): + """Entries are told apart by identity, which assigning a list must keep.""" + group = _group_with_member() + member = group.members[0] + + group.members = [*group.members, Group.Members(value="c3a26dd3")] + + assert group.members[0] is member + + +@pytest.mark.parametrize("op", ["add", "replace"]) +def test_an_immutable_complex_takes_back_the_sub_attribute_it_holds(op): + """Writing the value a sub-attribute holds modifies nothing.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation(op, "immutableComplex.label", "original")]} + ) + + assert patch.patch(resource) is False + + +def test_an_immutable_complex_refuses_a_new_sub_attribute_value(): + """A complex attribute holding a value is immutable down to its sub-attributes.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_validate( + {"Operations": [_operation("replace", "immutableComplex.label", "changed")]} + ) + + with pytest.raises(MutabilityException): + patch.patch(resource) + + +def test_a_remove_selecting_nothing_in_an_immutable_attribute_is_accepted(): + """Per RFC7644 §3.5.2.2, a remove that selects nothing changes nothing.""" + resource = _constrained_resource() + patch = PatchOp[ConstrainedResource].model_validate( + {"Operations": [{"op": "remove", "path": 'immutableList[label eq "none"]'}]} + ) + + assert patch.patch(resource) is False + + +class StampedEntry(ComplexAttribute): + value: str | None = None + stamp: Annotated[str | None, Mutability.read_only] = None + + +class StampedResource(Resource): + __schema__ = URN("urn:example:2.0:StampedResource") + + entries: list[StampedEntry] | None = None + + +def _stamped(): + return StampedResource(entries=[StampedEntry(value="a", stamp="server")]) + + +def _replace_entry(value): + return PatchOp[StampedResource].model_validate( + { + "Operations": [ + {"op": "replace", "path": 'entries[value eq "a"]', "value": value} + ] + } + ) + + +def test_an_entry_written_back_with_its_read_only_sub_attribute_is_accepted(): + """An entry sent as it was read changes none of its read-only sub-attributes.""" + resource = _stamped() + + assert _replace_entry({"value": "a", "stamp": "server"}).patch(resource) is False + + +def test_an_entry_cannot_change_its_read_only_sub_attribute(): + """RFC7644 §3.5.2 forbids changing a read-only attribute, including a sub-attribute of an entry.""" + resource = _stamped() + + with pytest.raises(MutabilityException): + _replace_entry({"value": "a", "stamp": "client"}).patch(resource) + + +def test_an_added_entry_cannot_assign_a_read_only_sub_attribute(): + """A client cannot set a read-only sub-attribute, even on an entry it adds.""" + resource = _stamped() + patch = PatchOp[StampedResource].model_validate( + { + "Operations": [ + { + "op": "add", + "path": "entries", + "value": [{"value": "b", "stamp": "client"}], + } + ] + } + ) + + with pytest.raises(MutabilityException): + patch.patch(resource) + + +class Tag(ComplexAttribute): + value: str | None = None + + +class Parts(ComplexAttribute): + first: str | None = None + last: str | None = None + + +class Contact(ComplexAttribute): + value: Annotated[str | None, Required.true] = None + label: str | None = None + + +class RequiringResource(Resource): + __schema__ = URN("urn:example:2.0:RequiringResource") + + tags: Annotated[list[Tag] | None, Required.true] = None + parts: Annotated[Parts | None, Required.true] = None + contact: Contact | None = None + + +def _requiring(): + return RequiringResource( + tags=[Tag(value="a"), Tag(value="b")], + parts=Parts(first="Barbara", last="Jensen"), + contact=Contact(value="555", label="work"), + ) + + +def _apply(operation): + patch = PatchOp[RequiringResource].model_validate({"Operations": [operation]}) + resource = _requiring() + return patch.patch(resource), resource + + +@pytest.mark.parametrize( + "operation", + [ + pytest.param( + {"op": "remove", "path": 'tags[value eq "a"]'}, id="some entries removed" + ), + pytest.param( + {"op": "remove", "path": 'tags[value eq "none"]'}, id="no entry selected" + ), + pytest.param( + {"op": "remove", "path": "parts.first"}, id="a sub-attribute removed" + ), + pytest.param({"op": "add", "path": "tags", "value": []}, id="no value added"), + pytest.param( + {"op": "remove", "path": "contact"}, + id="a required sub-attribute removed with its parent", + ), + ], +) +def test_a_required_attribute_left_assigned_is_accepted(operation): + """RFC7644 §3.5.2.2 only rejects a required attribute that is removed or becomes unassigned.""" + _, resource = _apply(operation) + + assert resource.tags + assert resource.parts + + +@pytest.mark.parametrize( + "operation", + [ + pytest.param( + {"op": "remove", "path": 'tags[value eq "a" or value eq "b"]'}, + id="every entry removed", + ), + pytest.param( + {"op": "replace", "path": "parts", "value": {"first": None, "last": None}}, + id="every sub-attribute unassigned", + ), + pytest.param( + {"op": "remove", "path": "contact.value"}, + id="required sub-attribute removed", + ), + ], +) +def test_a_required_attribute_becoming_unassigned_is_refused_when_applied(operation): + """Only the result of the patch shows that a required attribute became unassigned.""" + patch = PatchOp[RequiringResource].model_validate({"Operations": [operation]}) + resource = _requiring() + + with pytest.raises(MutabilityException): + patch.patch(resource) + + assert resource == _requiring() + + +def test_a_required_extension_cannot_be_removed(): + """Per RFC7643 §6, a resource includes the extensions its resource type requires.""" + model = User[Annotated[ConstrainedExtension, Required.true]] + user = model(user_name="bjensen") + user[ConstrainedExtension] = ConstrainedExtension(plain_attr="x") + patch = PatchOp[model].model_validate( + {"Operations": [{"op": "remove", "path": CONSTRAINED_EXTENSION_URN}]} + ) + + with pytest.raises(MutabilityException): + patch.patch(user) + + +def test_a_patch_without_any_operation_is_refused(): + """RFC7644 §3.5.2 requires "Operations" to hold at least one operation.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User].model_validate({"Operations": []}) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +@pytest.mark.parametrize("op", ["move", "copy", 1]) +def test_an_unknown_operation_is_refused(op): + """RFC7644 §3.5.2 defines add, remove and replace, and §3.12 gives invalidValue for anything else.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User].model_validate( + {"Operations": [{"op": op, "path": "nickName", "value": "Babs"}]} + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +def test_an_add_with_a_null_value_is_refused(): + """RFC7644 §3.5.2.1 requires an add to carry a value, and null is not one.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User].model_validate( + {"Operations": [{"op": "add", "path": "nickName", "value": None}]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" + + +@pytest.mark.parametrize("op", ["add", "replace"]) +def test_an_empty_value_on_a_read_only_attribute_is_refused(op): + """The path points to a read-only attribute, whatever the value writes under it.""" + with pytest.raises(ValidationError) as raised: + PatchOp[Group].model_validate( + {"Operations": [{"op": op, "path": "meta", "value": {}}]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + ) + + assert raised.value.errors()[0]["type"] == "scim_mutability" + + +@pytest.mark.parametrize( + "policy", + [ + ScimPolicy(unknown=ScimPolicy.Unknown.ignore), + ScimPolicy(unknown=ScimPolicy.Unknown.keep), + ], + ids=["ignore", "keep"], +) +def test_a_remove_value_on_an_undeclared_path_is_refused_under_a_tolerant_policy( + policy, +): + """The value of a remove is rejected before the path is dropped.""" + with pytest.raises(ValidationError) as raised: + PatchOp[User].model_validate( + {"Operations": [{"op": "remove", "path": "whatever", "value": "x"}]}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + scim_policy=policy, + ) + + assert raised.value.errors()[0]["type"] == "scim_invalidValue" diff --git a/tests/test_path_value_selection.py b/tests/test_path_value_selection.py index 72d28134..eb123da9 100644 --- a/tests/test_path_value_selection.py +++ b/tests/test_path_value_selection.py @@ -163,11 +163,14 @@ def test_writing_the_same_value_changes_nothing(user): def test_writing_replaces_the_matching_values_wholesale(user): - """§3.5.2.3: all matching record values are replaced.""" + """Path.set assigns, so unlike PATCH it unassigns the sub-attributes the value leaves out.""" + entry = user.emails[1] assert Path[User]('emails[type eq "home"]').set( user, {"type": "other", "value": "other@example.com"} ) assert [email.type for email in user.emails] == ["work", "other", "work"] + assert user.emails[1].primary is None + assert user.emails[1] is entry def test_writing_the_same_whole_value_changes_nothing(user): @@ -347,22 +350,27 @@ def test_a_selection_on_a_multivalued_attribute_of_an_unset_extension(): assert not path.delete(user) -def test_adding_to_a_selection_that_matches_nothing_changes_nothing(user): - """§3.5.2.1 does not define this case, and errata 8097 leaves it open. +def test_adding_to_a_selection_that_matches_nothing_has_no_target(user): + """A filter yielding no match is noTarget per §3.12, and no entry is created.""" + before = user.model_dump() + with pytest.raises(NoTargetException): + Path[User]('emails[type eq "other"].value').set( + user, "other@example.com", is_add=True + ) + assert user.model_dump() == before - Microsoft Entra ID emits exactly this operation expecting the entry to be - created, which is not what the published text says, so the operation is a - no-op rather than a failure. - """ + +def test_adding_to_a_selection_that_matches_nothing_leniently_changes_nothing(user): + """A non-strict write ignores the missing target and creates no entry.""" before = user.model_dump() assert not Path[User]('emails[type eq "other"].value').set( - user, "other@example.com", is_add=True + user, "other@example.com", is_add=True, strict=False ) assert user.model_dump() == before def test_replacing_a_selection_that_matches_nothing_has_no_target(user): - """§3.5.2.3 is the only operation the RFC requires ``noTarget`` for.""" + """§3.5.2.3 requires noTarget when the filter matches no value.""" with pytest.raises(NoTargetException): Path[User]('emails[type eq "other"].value').set(user, "x") diff --git a/tests/test_policy.py b/tests/test_policy.py index 44e0c76e..88b5de5e 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -2,6 +2,7 @@ import asyncio import threading +from typing import Annotated from typing import Any import pytest @@ -18,10 +19,16 @@ from scim2_models import CreationRequestContext from scim2_models import CreationResponseContext from scim2_models import EnterpriseUser +from scim2_models import Extension from scim2_models import Group +from scim2_models import InvalidFilterException from scim2_models import InvalidValueException +from scim2_models import Mutability +from scim2_models import MutabilityException +from scim2_models import NoTargetException from scim2_models import PatchOp from scim2_models import PatchOperation +from scim2_models import Path from scim2_models import Resource from scim2_models import ScimPolicy from scim2_models import ScimProvider @@ -388,6 +395,92 @@ def test_the_keys_microsoft_entra_adds_to_a_patch_are_ignored(): assert patch_op.operations[0].unknown_attributes == {"name": "addMember"} +# Unknown attributes in a PATCH path + + +def _pathed_patch(operations, policy): + return PatchOp[User].model_validate( + {"Operations": operations}, + scim_ctx=Context.RESOURCE_PATCH_REQUEST, + scim_policy=policy, + ) + + +def _pathed_user(): + return User( + user_name="bjensen", emails=[{"value": "b@example.com", "type": "work"}] + ) + + +UNDECLARED_PATHS = [ + pytest.param("unknownAttr", id="attribute"), + pytest.param("name.unknownAttr", id="sub-attribute"), + pytest.param("urn:example:2.0:Unmodelled:attr", id="extension"), + pytest.param( + 'emails[type eq "work"].unknownAttr', id="sub-attribute of a selection" + ), +] + + +def test_an_undeclared_path_is_refused_by_default(): + """The interoperability profile asks a service provider to reject what it does not define.""" + with pytest.raises(ValidationError) as raised: + _pathed_patch([{"op": "replace", "path": "unknownAttr", "value": "x"}], None) + + assert raised.value.errors()[0]["type"] == "scim_invalidPath" + + +@pytest.mark.parametrize( + "policy", [IGNORE, ScimPolicy(unknown=ScimPolicy.Unknown.keep)] +) +@pytest.mark.parametrize("op", ["add", "replace", "remove"]) +@pytest.mark.parametrize("path", UNDECLARED_PATHS) +def test_an_undeclared_path_is_dropped_under_a_tolerant_policy(policy, op, path): + """An attribute the policy drops from a value is also dropped from a path, since it has no field to write to.""" + operation = {"op": op, "path": path} + if op != "remove": + operation["value"] = "x" + user = _pathed_user() + before = user.model_dump() + + assert not _pathed_patch([operation], policy).patch(user, scim_policy=policy) + assert user.model_dump() == before + + +def test_the_operations_beside_an_undeclared_path_are_applied(): + """Dropping one operation leaves the others of the patch to apply.""" + user = _pathed_user() + patch = _pathed_patch( + [ + {"op": "replace", "path": "unknownAttr", "value": "x"}, + {"op": "replace", "path": "nickName", "value": "Babs"}, + ], + IGNORE, + ) + + assert patch.patch(user, scim_policy=IGNORE) + assert user.nick_name == "Babs" + + +def test_a_filter_comparing_an_undeclared_sub_attribute_is_invalid_under_any_policy(): + """The policy applies to attribute names, not to filter expressions.""" + patch = _pathed_patch( + [{"op": "replace", "path": 'emails[unknownAttr eq "x"].value', "value": "x"}], + IGNORE, + ) + + with pytest.raises(InvalidFilterException): + patch.patch(_pathed_user(), scim_policy=IGNORE) + + +def test_a_malformed_path_is_invalid_under_any_policy(): + """A path the grammar rejects has no attribute the policy could drop.""" + with pytest.raises(ValidationError) as raised: + _pathed_patch([{"op": "replace", "path": "unknown attr", "value": "x"}], IGNORE) + + assert raised.value.errors()[0]["type"] == "scim_invalidPath" + + # Unknown attributes, carried back @@ -465,8 +558,8 @@ def test_a_pathless_patch_operation_keeps_the_unknown_attributes_it_merges_over( assert user.unknown_attributes == {"unknownAttr": "x"} -def test_replacing_a_complex_attribute_drops_the_unknowns_it_carried(): - """The whole attribute is replaced, so what was unknown in it goes with the rest.""" +def test_replacing_a_complex_attribute_keeps_the_unknowns_it_carried(): + """RFC7644 §3.5.2.3 keeps the sub-attributes a replace does not specify, unknown ones included.""" with KEEP: user = User.model_validate( unknown_payload(name={"familyName": "Jensen", "bogusSub": 1}) @@ -476,8 +569,9 @@ def test_replacing_a_complex_attribute_drops_the_unknowns_it_carried(): ) PatchOp[User](operations=[operation]).patch(user) - assert user.name.family_name is None - assert user.name.unknown_attributes == {} + assert user.name.given_name == "Barbara" + assert user.name.family_name == "Jensen" + assert user.name.unknown_attributes == {"bogusSub": 1} # A remove operation carrying a value @@ -589,6 +683,14 @@ def test_a_selection_that_matches_nothing_is_a_success(): assert len(group.members) == 2 +def test_a_selection_listing_no_entry_removes_nothing(): + """An empty list selects no member, so none is removed.""" + group = group_with_members() + + assert entra_remove([]).patch(group, scim_policy=APPLY) is False + assert len(group.members) == 2 + + def test_a_path_that_already_selects_refuses_a_value_even_under_apply(): """Two selections are two intentions, and nothing says which one to honour.""" patch_op = PatchOp[Group]( @@ -619,3 +721,222 @@ def test_an_ambient_policy_reaches_a_remove_carrying_a_value(): entra_remove([{"value": "s-foobar"}]).patch(group) assert [member.value for member in group.members] == ["autre"] + + +# A path filter matching nothing + + +CREATE = ScimPolicy(unmatched_path_filter=ScimPolicy.UnmatchedPathFilter.create) + + +class Tag(ComplexAttribute): + kind: str | None = None + stamp: Annotated[str | None, Mutability.read_only] = None + + +class Tagged(Resource): + __schema__ = URN("urn:example:2.0:Tagged") + + tags: list[Tag] | None = None + sealed: Annotated[list[Tag] | None, Mutability.immutable] = None + labels: list[str] | None = None + + +class Tagging(Extension): + __schema__ = URN("urn:example:2.0:Tagging") + + tags: list[Tag] | None = None + + +def unmatched_patch(model: Any, *operations: dict[str, Any]) -> "PatchOp[Any]": + """Build a patch from raw operations on the given resource type.""" + return PatchOp[model].model_validate({"Operations": list(operations)}) + + +def test_an_add_whose_filter_matches_nothing_has_no_target_by_default(): + """Table 9 of RFC7644 §3.12 defines noTarget for a filter that yields no match.""" + patch = unmatched_patch( + User, + {"op": "add", "path": 'emails[type eq "work"].value', "value": "w@example.com"}, + ) + + with pytest.raises(NoTargetException): + patch.patch(User(user_name="bjensen")) + + +@pytest.mark.parametrize("op", ["add", "replace"]) +def test_the_entry_a_filter_describes_is_created_under_create(op): + """Microsoft Entra fills an absent work email through a filter, with add or replace.""" + user = User(user_name="bjensen") + patch = unmatched_patch( + User, + {"op": op, "path": 'emails[type eq "work"].value', "value": "w@example.com"}, + ) + + assert patch.patch(user, scim_policy=CREATE) + + assert [(email.type, email.value) for email in user.emails] == [ + ("work", "w@example.com") + ] + + +def test_the_operations_with_the_same_filter_reach_the_created_entry(): + """Entra spreads one entry over several operations that share a filter.""" + user = User(user_name="bjensen") + patch = unmatched_patch( + User, + {"op": "add", "path": 'emails[type eq "work"].value', "value": "w@example.com"}, + {"op": "add", "path": 'emails[type eq "work"].display', "value": "Work"}, + ) + + assert patch.patch(user, scim_policy=CREATE) + + assert len(user.emails) == 1 + assert user.emails[0].display == "Work" + + +def test_a_value_is_merged_into_the_created_entry(): + """The created entry also takes the sub-attributes in the value.""" + user = User(user_name="bjensen") + patch = unmatched_patch( + User, + { + "op": "add", + "path": 'emails[type eq "work"]', + "value": {"value": "w@example.com"}, + }, + ) + + assert patch.patch(user, scim_policy=CREATE) + + assert (user.emails[0].type, user.emails[0].value) == ("work", "w@example.com") + + +def test_a_created_primary_entry_becomes_the_only_primary(): + """The filter literal is read as ScimFilter compares it, and primary stays unique.""" + user = User( + user_name="bjensen", + emails=[{"type": "home", "value": "h@example.com", "primary": True}], + ) + patch = unmatched_patch( + User, + { + "op": "replace", + "path": 'emails[type eq "work" and primary eq "True"].value', + "value": "w@example.com", + }, + ) + + assert patch.patch(user, scim_policy=CREATE) + + assert [(email.type, email.primary) for email in user.emails] == [ + ("home", False), + ("work", True), + ] + + +def test_a_value_contradicting_the_filter_is_refused(): + """The created entry must match its filter, for a later operation to reach it.""" + user = User(user_name="bjensen") + patch = unmatched_patch( + User, + { + "op": "add", + "path": 'emails[type eq "work"]', + "value": {"type": "home", "value": "h@example.com"}, + }, + ) + + with pytest.raises(InvalidValueException): + patch.patch(user, scim_policy=CREATE) + assert user.emails is None + + +@pytest.mark.parametrize( + "path", + [ + 'emails[type eq "work" or type eq "home"].value', + 'emails[type ne "work"].value', + 'emails[not (type eq "work")].value', + 'emails[display co "Work"].value', + "emails[type pr].value", + ], +) +def test_a_filter_describing_no_entry_still_has_no_target(path): + """Only eq comparisons joined by and describe the entry to create.""" + patch = unmatched_patch(User, {"op": "add", "path": path, "value": "w@example.com"}) + + with pytest.raises(NoTargetException): + patch.patch(User(user_name="bjensen"), scim_policy=CREATE) + + +def test_a_filter_over_simple_values_still_has_no_target(): + """A multi-valued attribute without sub-attributes has no entry to describe.""" + patch = unmatched_patch( + Tagged, {"op": "replace", "path": 'labels[value eq "red"]', "value": "green"} + ) + + with pytest.raises(NoTargetException): + patch.patch(Tagged(labels=["blue"]), scim_policy=CREATE) + + +def test_a_remove_whose_filter_matches_nothing_creates_nothing(): + """Per RFC7644 §3.5.2.2, a remove that selects nothing succeeds without change.""" + user = User(user_name="bjensen") + patch = unmatched_patch(User, {"op": "remove", "path": 'emails[type eq "work"]'}) + + assert patch.patch(user, scim_policy=CREATE) is False + assert user.emails is None + + +def test_a_created_entry_cannot_set_a_read_only_sub_attribute(): + """The created entry goes through the same checks as an add of that entry.""" + patch = unmatched_patch( + Tagged, {"op": "add", "path": 'tags[stamp eq "x"].kind', "value": "k"} + ) + + with pytest.raises(MutabilityException): + patch.patch(Tagged(), scim_policy=CREATE) + + +def test_no_entry_is_created_in_an_immutable_attribute_holding_values(): + """An immutable multi-valued attribute takes no new entry once assigned.""" + patch = unmatched_patch( + Tagged, {"op": "add", "path": 'sealed[kind eq "b"].kind', "value": "b"} + ) + + with pytest.raises(MutabilityException): + patch.patch(Tagged(sealed=[{"kind": "a"}]), scim_policy=CREATE) + + +def test_an_unassigned_extension_is_created_with_the_entry(): + """An extension attribute is selected into as a core one is.""" + resource = Tagged[Tagging]() + patch = unmatched_patch( + Tagged[Tagging], + {"op": "add", "path": 'urn:example:2.0:Tagging:tags[kind eq "b"]', "value": {}}, + ) + + assert patch.patch(resource, scim_policy=CREATE) + + assert resource[Tagging].tags == [Tag(kind="b")] + + +def test_an_ambient_policy_reaches_the_creation(): + """A server may state the policy once per request with a block.""" + user = User(user_name="bjensen") + patch = unmatched_patch( + User, + {"op": "add", "path": 'emails[type eq "work"].value', "value": "w@example.com"}, + ) + + with CREATE: + assert patch.patch(user) + + +def test_path_set_creates_nothing_under_create(): + """The policy only applies to PATCH, and Path.set still reports the missing target.""" + with CREATE, pytest.raises(NoTargetException): + Path[User]('emails[type eq "work"].value').set( + User(user_name="bjensen"), "w@example.com" + )