SCIM provisioning API basic implementation

[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+

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

• Pull request is based on the develop branch
• 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.
• 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]

@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.

[reivilibre]

this is looking generally good, but a few comments (and some still open from last time).

The CI is also unhappy but hopefully that's nothing too difficult?

[reivilibre]

I don't think this was noted on the documentation?

[reivilibre]

I suspect this is no longer functional, given authenticated media means you can't just hotlink to media like this anymore.

[azmeuk]

@reivilibre
After all those months I finally took time to solve the remaining issues and merge the current develop branch. I have more availability now so it should not take as long if there other things to do.

The DELETE http call is documented here. I don't know why this is not displayed by GH here.

I think everything should be solved except the enable_authenticated_media question, see the conversation for more details.

[wrjlewis]

Hey @azmeuk

Thanks for the update. Unfortunately, we won't be able to continue work on this PR at this time for a couple of reasons:

1. We want to limit features like this in Synapse not least to reduce our maintenance burden, but mainly this category of auth work is now being directed to MAS instead, as the natural home for it.

2. Which begs the question 'what about if someone contributed this to MAS instead?'. For full transparency, in this case we see it as an enterprise-level feature and therefore again wouldn't wish to pursue it in open-souce.

Thanks very much for taking the time to develop this PR and get it to this stage - I just want to acknowledge this isn't an ideal situation to come to this conclusion at this stage and really appreciate the effort you've put in!

[ara4n]

To clarify (speaking as element ceo/cto), there are three considerations here:

• It's not clear that this should go in Synapse rather than MAS - given it's managing user identities, MAS is surely a better bet than Synapse (and would then benefit any homeserver which uses MAS). I'm a bit confused on how we've been managing to review this for 16 months without spotting this; it feels like we might have been missing the bigger picture.
• That said, we barely have bandwidth at Element to maintain our own stuff right now, let alone major new features like this (especially if it's not stuff we're using in production ourselves)
• we don't have any policy against accepting enterprisey PRs: instead, we take them case by case; the key question is whether we'd have the bandwidth to maintain it going forwards. If a FOSS PR conflicts with an existing proprietary product, then we'd figure out which direction we'd want to take - i.e. whether to adopt the contribution or not.

So, my recommendation here would be to maintain this out-of-tree, either in Synapse (given that's where the work has happened already), or as a third-party Synapse module, or even better to do it on MAS (although I can't guarantee that we'd have bandwidth to accept it there either. plus I guess MAS being in Rust might be an obstacle).

I can only apologise that we (I) didn't spot this back in May 2024 and warn that this might not be a feature we'd be able to merge.

[azmeuk]

Hi.
This is bad news but I understand your reasons.
@ara4n no hard feelings, @reivilibre warned me last year on #synapse-dev that it could end this way, even before I started the development.

I'll see how much work it would ask to maintain this out of synapse, but I fear this would double the maintenance burden.

I might tackle the implementation on MAS some day, but certainly not anytime soon. I would not benevolently contributing to a proprietary feature though, and on the other hand I understand that you would not be interested in paying for a feature you did not even ask in the first place. So for this to happen, this should probably be funded by someone else.