Handle "registration_enabled" parameter for CAS

matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.

https://matrix-org.github.io/synapse

Apache License 2.0

11.82k stars 2.13k forks source link

Handle "registration_enabled" parameter for CAS #16262

Closed agrimpard closed 1 year ago

agrimpard commented 1 year ago

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] Pull request includes a sign off
[x] Code style is correct (run the linters)

Signed-off-by: Aurélien Grimpard aurelien@grimpard.net

This PR is related to this issue : fixes #15775

clokep commented 1 year ago

@agrimpard Please sign-off on your changes.

clokep commented 1 year ago

It would be good to update test_cas.py with a test for this while we're at it!

agrimpard commented 1 year ago

@agrimpard Please sign-off on your changes.

Just did ! I'm looking into this /synapse/tests/handlers/test_cas.py that i'm not familiar with ...

clokep commented 1 year ago

The changes made in #14978 for OIDC are similar, hopefully they'll lead you in the right direction?

agrimpard commented 1 year ago

This commit needs a review, i'm really not familliar with devops : https://github.com/matrix-org/synapse/pull/16262/commits/c42c8f639a7292f9fea4baaa4da549b98cb172c1

Help also welcome for this failing test : https://github.com/matrix-org/synapse/actions/runs/6096949144/job/16543605894

agrimpard commented 1 year ago

tests/handlers/test_cas.py:212: error: "CasHandlerTestCase" has no attribute "assertRenderedError" [attr-defined]

I assume that it retains the parameters added by the previous test test_required_attributes, which asks for CAS attributes. How do I tell it that it should go back to the default configuration without the parameters from the previous test?

agrimpard commented 1 year ago

Even if the tests didn't fail, you'd really have to look closely at the test function added to the /synapse/tests/handlers/test_cas.py file before merging. Again, I'm not comfortable with devops.

clokep commented 1 year ago

Thanks! 🎉