Improve SSO/delegated authentication worker docs#19752
Conversation
benbz
changed the title
| Additionally, the following endpoints should be included if Synapse is configured | ||
| to use SSO (you only need to include the ones for whichever SSO provider you're | ||
| using): | ||
| using) and delegated authentication isn't enabled: |
There was a problem hiding this comment.
I've made the assertion here that none of these paths are useful in delegated auth world (be it experimental config or stabilised)
| # Stabilised Delegated Authentication support (`matrix_authentication_service.enabled: true`) | ||
| ^/_synapse/mas/ |
There was a problem hiding this comment.
Documented up here rather than next to the experimental_features.msc3861.enabled paths as AFAICT it can be handled by any worker rather than a worker with a single process only
| @@ -0,0 +1 @@ | |||
| Improve documentation around endpoints that can be enabled with MSC3861. | |||
There was a problem hiding this comment.
Where is this change spawning from? I assume you ran into this foot-gun somewhere
There was a problem hiding this comment.
element-hq/ess-helm#1278 and backlinks in both that issue and here
| Do note that these endpoints can't be handled by workers if the stabilised delegated | ||
| authentication support is enabled (`matrix_authentication_service.enabled` set to | ||
| `true`). |
There was a problem hiding this comment.
How do you know?
Why is this the case?
There was a problem hiding this comment.
From #19752 (comment), it seems like it doesn't matter whether these are handled. It just isn't necessary.
Which means the phrasing of these updates should change. Or perhaps, isn't necessary to call this out at all. You can handle whatever with workers.
There was a problem hiding this comment.
How do you know?
Why is this the case?
https://github.com/element-hq/synapse/pull/18759/changes#diff-21eb1b3b6455b7011ccbffc74b0279d87d5c69752d62d0196479b1bbc0bdbee3R274-R278 and discussion with @sandhose
From #19752 (comment), it seems like it doesn't matter whether these are handled. It just isn't necessary.
Which means the phrasing of these updates should change. Or perhaps, isn't necessary to call this out at all. You can handle whatever with workers.
My comment above is about the main block of paths. I think that the sentence I'm commenting is accurate and the additional assertion I've made (not useful with delegated auth) holds given Synapse won't be receiving any OIDC/SAML/CAS callbacks or doing any of the SSO dance.
These paths here can't be be put on a worker at all if the stabilised MAS configuration is used
There was a problem hiding this comment.
These paths here can't be be put on a worker at all if the stabilised MAS configuration is used
I feel like it doesn't matter. As far as I know, when MAS is enabled, it disables all of the relevant endpoints.
There was a problem hiding this comment.
https://github.com/element-hq/synapse/blob/v1.153.0/synapse/rest/admin/__init__.py#L391-L401 these still appear enabled on workers with the experimental MAS configuration. Just not with the stable MAS configuration (or without MAS).
| @@ -0,0 +1 @@ | |||
| Document the paths that can be handled on workers with stabilised delegated authentication. | |||
There was a problem hiding this comment.
I think this PR should be reviewed by someone else from @element-hq/mas-maintainers for accuracy.
I can't seem to select the team as a reviewer
2 participants
Improve SSO/delegated authentication (MSC3861) worker docs
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.Documenting the state of the world as of #18759. It is unclear to me whether these endpoints were excluded from normal delegated auth support as they won't work or just because they weren't needed.
Also the
/devicesendpoint(s) had the wrong version and were incomplete as per https://github.com/element-hq/synapse/blob/v1.152.0/synapse/rest/admin/devices.py#L47 and https://github.com/element-hq/synapse/blob/v1.152.0/synapse/rest/admin/devices.py#L121