SAML2 Improvements and redirect stuff #5316
Conversation
Signed-off-by: Alexander Trost <galexrt@googlemail.com>
|
This is awesome. Thank you so much for doing and sharing this work. It turns out that I need to get this going in a hurry, so I'm going to make a branch based on it to fix up the global SAML2Client and logging issues you've already identified. |
| @@ -75,6 +75,7 @@ def default_config(self, config_dir_path, server_name, **kwargs): | |||
| # override them. | |||
| # | |||
| #saml2_config: | |||
| # enabled: true | |||
There was a problem hiding this comment.
tbh I think this is redundant. The default is for 'enabled' to be considered True: the only reason that the config parser checks it is to avoid breaking people who have enabled: false in their config files.
| @@ -727,6 +727,9 @@ def validate_login(self, username, login_submission): | |||
| if canonical_user_id: | |||
| defer.returnValue((canonical_user_id, None)) | |||
|
|
|||
| if login_type == LoginType.SSO: | |||
There was a problem hiding this comment.
can you share the thinking behind this change? it looks like it's turning a 400 into a 403, but which endpoint is this for?
There was a problem hiding this comment.
If I remember correctly, because of this
synapse/synapse/rest/client/v1/login.py
Lines 251 to 254 in dc3e586
Due to no if condition being met it would fail because SSO is not a valid login type.
| @@ -56,6 +56,7 @@ var show_login = function() { | |||
| } | |||
|
|
|||
| if (matrixLogin.serverAcceptsSso) { | |||
| $("#sso_form").attr("action", "/_matrix/client/r0/login/sso/redirect"); | |||
There was a problem hiding this comment.
In which point the default? This change was needed as otherwise no SSO redirect would have happened.
There was a problem hiding this comment.
The default action for #sso_form is /_matrix/client/r0/login/sso/redirect: https://github.com/matrix-org/synapse/blob/master/synapse/static/client/login/index.html#L22
|
@galexrt nice, thank you. I will test and give feedback! |
This is a "draft" right now and doesn't really comply with the guidelines of this repository / project (will work on the PR changelog file when I refined my changes). I just want to put out my changes to get SAML2 SSO working for me, so that other people can enjoy SAML2 with Matrix.
There are still things to fix:
NOTE This basically takes code from older PRs #200 and #4267 to get it working.
Please let me know what would needs to be improved to get this PR ready for review and / or merge.
Thanks!
Could potentially close #5130 when a SSO logout endpoint has been proposed and implemented?
Pull Request Checklist