Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add configuration setting for CAS protocol version #15816

Merged
merged 12 commits into from
Aug 24, 2023
Merged

Add configuration setting for CAS protocol version #15816

merged 12 commits into from
Aug 24, 2023

Conversation

agrimpard
Copy link
Contributor

@agrimpard agrimpard commented Jun 21, 2023 •
edited by MadLittleMods
Loading

Add configuration setting for CAS protocol version

I made a new pull request, taking into account Clokep's comment. I've added the possibility of configuring the CAS protocol version and, in the case of version 3, the CAS ticket validation url is modified to allow CAS attributes to be retrieved, which is not possible with version 2.

Fix #15807

Old PR : #15811

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

Signed-off-by: Aurélien Grimpard [email protected]

@agrimpard
Update cas.py
9fa253b 
Manage configuration for CAS protocol version.
@agrimpard
Update cas.py
e04ede7 
Manage configuration for CAS protocol version.
@agrimpard
Create 15807.feature
f3d22cc
@agrimpard
Merge branch 'matrix-org:develop' into develop
00553f6
@agrimpard
Update config_documentation.md
eb0a7ab 
Update documentation
@agrimpard agrimpard requested a review from a team as a code owner June 21, 2023 13:44
@agrimpard
Copy link
Contributor Author

agrimpard commented Jun 21, 2023 •
edited
Loading

I made a new pull request, taking into account Clokep's comment. I've added the possibility of configuring the CAS protocol version and, in the case of version 3, the CAS ticket validation url is modified to allow CAS attributes to be retrieved, which is not possible with version 2.

It solve this issue : #15807

Old PR : #15811

MadLittleMods
MadLittleMods reviewed Jun 22, 2023
View reviewed changes
changelog.d/15816.feature
@@ -0,0 +1 @@
Add configuration setting for CAS protocol version. Contributed by Aurélien Grimpard.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helpful docs for anyone looking at this: https://github.com/matrix-org/synapse/blob/d53547352063c3e66302f7bdc40d7ac7270e605c/docs/development/cas.md

@MadLittleMods MadLittleMods requested a review from a team June 22, 2023 20:01
@MadLittleMods MadLittleMods added the z-auth (Deprecated Label) label Jun 26, 2023
MadLittleMods
MadLittleMods reviewed Jun 27, 2023
View reviewed changes
synapse/handlers/cas.py
@@ -120,7 +121,10 @@ async def _validate_ticket(
Returns:
The parsed CAS response.
"""
uri = self._cas_server_url + "/proxyValidate"
if self._cas_protocol_version == 3:
uri = self._cas_server_url + "/p3/proxyValidate"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems legit -> spec docs for CAS protocol version 3: https://apereo.github.io/cas/6.6.x/protocol/CAS-Protocol-Specification.html#29-p3proxyvalidate-cas-30

2.9. /p3/proxyValidate [CAS 3.0]

/p3/proxyValidate MUST perform the same validation tasks as /p3/serviceValidate and additionally validate proxy tickets. See Section 2.8.

2.8. /p3/serviceValidate [CAS 3.0]

/p3/serviceValidate MUST perform the same validation tasks as /serviceValidate and additionally return user attributes in the CAS response. See Section 2.5 and Section 2.5.7 for details.

2.5.7 Example response with custom attributes

<cas:serviceResponse xmlns:cas="http://www.yale.edu/tp/cas">
  <cas:authenticationSuccess>
    <cas:user>username</cas:user>
    <cas:attributes>
      <cas:firstname>John</cas:firstname>
      <cas:lastname>Doe</cas:lastname>
      <cas:title>Mr.</cas:title>
      <cas:email>[email protected]</cas:email>
      <cas:affiliation>staff</cas:affiliation>
      <cas:affiliation>faculty</cas:affiliation>
    </cas:attributes>
    <cas:proxyGrantingTicket>PGTIOU-84678-8a9d...</cas:proxyGrantingTicket>
  </cas:authenticationSuccess>
</cas:serviceResponse>

@clokep
Copy link
Member

clokep commented Aug 24, 2023

@agrimpard Are you interested in finishing this up?

@agrimpard
Copy link
Contributor Author

agrimpard commented Aug 24, 2023 •
edited
Loading

@agrimpard Are you interested in finishing this up?

I'm sorry i'm a bit new there, what should be done ?

I'm interested to have this PR in prod :)

@clokep
Copy link
Member

clokep commented Aug 24, 2023

I'm interested to have this PR in prod :)

There's a comment left above which needs to be handled: #15816 (comment)

You should be able to just push additional commits to this branch (or perhaps just commit what was suggested via the GitHub UI, if you agree with it).

clokep
clokep reviewed Aug 24, 2023
View reviewed changes
synapse/config/cas.py Outdated Show resolved Hide resolved
clokep
clokep approved these changes Aug 24, 2023
View reviewed changes
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good if CI is happy! 👍 Thanks for sticking w/ this.

@clokep clokep merged commit aeeca2a into matrix-org:develop Aug 24, 2023
39 checks passed
@clokep
Copy link
Member

clokep commented Aug 24, 2023

Thanks for your contribution! 🎉 It should be part of Synapse 1.92.0.

hughns pushed a commit that referenced this pull request Sep 4, 2023
@agrimpard @hughns
Add configuration setting for CAS protocol version (#15816)
a0adf71
DMRobertson pushed a commit that referenced this pull request Sep 5, 2023
Merge tag 'v1.92.0rc1' into develop
02bc590 
- Add configuration setting for CAS protocol version. Contributed by Aurélien Grimpard. ([\#15816](#15816))
- Suppress notifications from message edits per [MSC3958](matrix-org/matrix-spec-proposals#3958). ([\#16113](#16113))
- Return a `Retry-After` with `M_LIMIT_EXCEEDED` error responses. ([\#16136](#16136))
- Add `last_seen_ts` to the [admin users API](https://matrix-org.github.io/synapse/latest/admin_api/user_admin_api.html). ([\#16218](#16218))
- Improve resource usage when sending data to a large number of remote hosts that are marked as "down". ([\#16223](#16223))

- Fix IPv6-related bugs on SMTP settings, adding groundwork to fix similar issues. Contributed by @evilham and @telmich (ungleich.ch). ([\#16155](#16155))
- Fix a spec compliance issue where requests to the `/publicRooms` federation API would specify `include_all_networks` as a string. ([\#16185](#16185))
- Fix inaccurate error message while attempting to ban or unban a user with the same or higher PL by spliting the conditional statements. Contributed by @leviosacz. ([\#16205](#16205))
- Fix a rare bug that broke looping calls, which could lead to e.g. linearly increasing memory usage. Introduced in v1.90.0. ([\#16210](#16210))
- Fix a long-standing bug where uploading images would fail if we could not generate thumbnails for them. ([\#16211](#16211))
- Fix a long-standing bug where we did not correctly back off from servers that had "gone" if they returned 4xx series error codes. ([\#16221](#16221))

- Update links to the [matrix.org blog](https://matrix.org/blog/). ([\#16008](#16008))
- Document which [admin APIs](https://matrix-org.github.io/synapse/latest/usage/administration/admin_api/index.html) are disabled when experimental [MSC3861](matrix-org/matrix-spec-proposals#3861) support is enabled. ([\#16168](#16168))
- Document [`exclude_rooms_from_sync`](https://matrix-org.github.io/synapse/v1.92/usage/configuration/config_documentation.html#exclude_rooms_from_sync) configuration option. ([\#16178](#16178))

- Prepare unit tests for Python 3.12. ([\#16099](#16099))
- Fix nightly CI jobs. ([\#16121](#16121), [\#16213](#16213))
- Describe which rate limiter was hit in logs. ([\#16135](#16135))
- Simplify presence code when using workers. ([\#16170](#16170))
- Track per-device information in the presence code. ([\#16171](#16171), [\#16172](#16172))
- Stop using the `event_txn_id` table. ([\#16175](#16175))
- Use `AsyncMock` instead of custom code. ([\#16179](#16179), [\#16180](#16180))
- Improve error reporting of invalid data passed to `/_matrix/key/v2/query`. ([\#16183](#16183))
- Task scheduler: add replication notify for new task to launch ASAP. ([\#16184](#16184))
- Improve type hints. ([\#16186](#16186), [\#16188](#16188), [\#16201](#16201))
- Bump black version to 23.7.0. ([\#16187](#16187))
- Log the details of background update failures. ([\#16212](#16212))
- Cache device resync requests over replication. ([\#16241](#16241))

* Bump anyhow from 1.0.72 to 1.0.75. ([\#16141](#16141))
* Bump furo from 2023.7.26 to 2023.8.19. ([\#16238](#16238))
* Bump phonenumbers from 8.13.18 to 8.13.19. ([\#16237](#16237))
* Bump psycopg2 from 2.9.6 to 2.9.7. ([\#16196](#16196))
* Bump regex from 1.9.3 to 1.9.4. ([\#16195](#16195))
* Bump ruff from 0.0.277 to 0.0.286. ([\#16198](#16198))
* Bump sentry-sdk from 1.29.2 to 1.30.0. ([\#16236](#16236))
* Bump serde from 1.0.184 to 1.0.188. ([\#16194](#16194))
* Bump serde_json from 1.0.104 to 1.0.105. ([\#16140](#16140))
* Bump types-psycopg2 from 2.9.21.10 to 2.9.21.11. ([\#16200](#16200))
* Bump types-pyyaml from 6.0.12.10 to 6.0.12.11. ([\#16199](#16199))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MadLittleMods MadLittleMods MadLittleMods left review comments

@clokep clokep clokep approved these changes

Assignees
No one assigned
Labels
z-auth (Deprecated Label)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Synapse doesn't retrieve CAS attributes when using CAS Protocol 3.0
3 participants
@agrimpard @clokep @MadLittleMods