Implement MSC4098: SCIM provisioning

[azmeuk]

This is an implementation of MSC4098. It implements a subset of the SCIM provisioning protocol defined in RFC7643 and RFC7644.

It contains:

• A SCIM servlet implementing the minimal SCIM endpoints.
  • The data edition/retrieval part largely takes inspiration (and shameless copied) from synapse/rest/admin/users.py.
  • The SCIM payload validation and production is achieved with scim2-models, a library based on pydantic which I maintain.
• Unit tests for those endpoints.
• Documentation on the state of the SCIM implementation, and examples of requests and response payloads.

The SCIM requires needs python 3.9+ (because of the use of typing.Anotated in scim2-models) and pydantic 2.7.0+

It seems ./scripts-dev/check_pydantic_models.py breaks because of some models in scim2-models, but I am not really sure what to do about this.

SCIM implementation details

Only a subset of the SCIM endpoints are implemented:

What's implemented:

• The main endpoints:
  • /Users (GET, POST)
  • /Users/<user_id> (GET, PUT, DELETE)
  • /ServiceProviderConfig (GET)
  • /Schemas (GET)
  • /Schemas/<schema_id> (GET)
  • /ResourceTypes (GET)
  • /ResourceTypes/<resource_type_id>
• pagination
• The user attributes:
  • userName
  • password
  • emails
  • phoneNumbers
  • displayName
  • photos (as a MXC URI)
  • active

What is defined in the SCIM specs but not implemented here:

• A few endpoints:
  • /Users (PATCH)
  • /Me (GET, POST, PUT, PATCH, DELETE)
  • /Groups (GET, POST, PUT, PATCH) (but this no relevant)
  • /Bulk (POST)
  • /.search (POST)
• filtering
• sorting
• attributes selection
• ETags

What do you think?

Pull Request Checklist

• [x] Pull request is based on the develop branch
• [x] Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• [x] Code style is correct (run the linters)

[erikjohnston]

(I've taken this out of the review queue as its in draft, let us know if you want feedback)

[azmeuk]

Hi @erikjohnston Thank you for your feedback offering. Indeed this is a draft, but I hope to take back the development soon.

There is one design question though. I see that there is a dependency to pydantic in synapse, and I recently published scim2-models that is a library that helps to parse and serialize SCIM2 payloads using pydantic. I think the SCIM implementation would greatly benefit from using scim2-models, as a big part of the specification compliance would be delegated to the library.

Would it be acceptable to add a dependency towards scim2-models in synapse, or should I continue checking and building SCIM2 payloads manually?

[azmeuk]

Hi @erikjohnston I think the PR can be reviewed now. I edited the OP to detail what's in there. I am available on #synapse-dev too if there are things to discuss.

[azmeuk]

I fixed the typing issues, ~but I need to fix type annotations and add a py.typed file in scim2-models as well.~

[azmeuk]

mypy issues are fixed, however the check_pydantic_models.py raises a TypeError. I am not sure why. Is it due to my modifications or is it a bug in the script?

Traceback (most recent call last):
  File "/home/eloi/dev/matrix/synapse/scripts-dev/check_pydantic_models.py", line 495, in <module>
    sys.exit(lint())
             ^^^^^^
  File "/home/eloi/dev/matrix/synapse/scripts-dev/check_pydantic_models.py", line 226, in lint
    failures = do_lint()
               ^^^^^^^^^
  File "/home/eloi/dev/matrix/synapse/scripts-dev/check_pydantic_models.py", line 251, in do_lint
    module_infos = list(
                   ^^^^^
  File "/usr/lib/python3.12/pkgutil.py", line 78, in walk_packages
    __import__(info.name)
  File "/home/eloi/dev/matrix/synapse/synapse/module_api/__init__.py", line 122, in <module>
    from synapse.rest.client.login import LoginResponse
  File "/home/eloi/dev/matrix/synapse/synapse/rest/__init__.py", line 25, in <module>
    from synapse.rest import admin, scim
  File "/home/eloi/dev/matrix/synapse/synapse/rest/scim.py", line 32, in <module>
    from scim2_models import (
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/scim2_models/__init__.py", line 1, in <module>
    from .base import BaseModel
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/scim2_models/base.py", line 327, in <module>
    class BaseModel(PydanticBaseModel):
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/pydantic/v1/main.py", line 221, in __new__
    inferred = ModelField.infer(
               ^^^^^^^^^^^^^^^^^
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/pydantic/v1/fields.py", line 504, in infer
    return cls(
           ^^^^
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/pydantic/v1/fields.py", line 434, in __init__
    self.prepare()
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/pydantic/v1/fields.py", line 544, in prepare
    self._set_default_and_type()
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/pydantic/v1/fields.py", line 568, in _set_default_and_type
    default_value = self.get_default()
                    ^^^^^^^^^^^^^^^^^^
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/pydantic/v1/fields.py", line 437, in get_default
    return smart_deepcopy(self.default) if self.default_factory is None else self.default_factory()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/eloi/.cache/pypoetry/virtualenvs/matrix-synapse-jPdyJ-JW-py3.12/lib/python3.12/site-packages/pydantic/v1/utils.py", line 693, in smart_deepcopy
    return deepcopy(obj)  # slowest way when we actually might need a deepcopy
           ^^^^^^^^^^^^^
  File "/usr/lib/python3.12/copy.py", line 162, in deepcopy
    y = _reconstruct(x, memo, *rv)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/copy.py", line 259, in _reconstruct
    state = deepcopy(state, memo)
            ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/copy.py", line 136, in deepcopy
    y = copier(x, memo)
        ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/copy.py", line 221, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
                             ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/copy.py", line 151, in deepcopy
    rv = reductor(4)
         ^^^^^^^^^^^
TypeError: cannot pickle 'classmethod' object

[azmeuk]

I opened #17667 to fix the check_pydantic_models.py issue.

[azmeuk]

Finally, the CI is green! :tada: @erikjohnston this is entirely for review now.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[MatMaul]

I think this should be moved under /_synapse/admin, it is clearly not a client API I believe, and more an already standardized way of provisioning users. Hence I don't think it really needs a MSC.

[azmeuk]

Thank you both for your reviews.

@reivilibre I will address your suggestions soon and ping you for a second round.

@MatMaul, would /_synapse/admin/scim/v2 be fine, or should there be a mention that the feature is unstable?

[MatMaul]

@MatMaul, would /_synapse/admin/scim/v2 be fine, or should there be a mention that the feature is unstable?

I think it's fine, the unstable prefix is for features that are not yet in the spec, but this doesn't really need a spec update.

You should probably remove mention of MSC4098 in the title of the PR, the changelog entry, and strike through This is an implementation of MSC4098 in the body, so we keep a ref perhaps.

[azmeuk]

@reivilibre I think all your comments have been addressed now. Most of them have been fixed in the code and I responded to some with new questions. The implementation have been tested against Keycloak with the keycloak-scim extension.

I am sorry for the many commits, the PR looks like a mess. If it is easier for the review, I can close it and open a new one. Let me know.