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

Do not require the CAS service URL setting (use public_baseurl instead). #9199

Merged
merged 3 commits into from
Jan 26, 2021
Merged

Do not require the CAS service URL setting (use public_baseurl instead). #9199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1913982
Do not require the CAS service URL.
clokep Jan 21, 2021
e6932de
Consistency with the OIDC handler.
clokep Jan 21, 2021
237469e
Newsfragment.
clokep Jan 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9199.removal
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The `service_url` parameter in `cas_config` is deprecated in favor of `public_baseurl`.
4 changes: 0 additions & 4 deletions docs/sample_config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1878,10 +1878,6 @@ cas_config:
#
#server_url: "https://cas-server.com"

# The public URL of the homeserver.
#
#service_url: "https://homeserver.domain.com:8448"

# The attribute of the CAS response to use as the display name.
#
# If unset, no displayname will be set.
Expand Down
12 changes: 7 additions & 5 deletions synapse/config/cas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,13 @@ def read_config(self, config, **kwargs):

if self.cas_enabled:
self.cas_server_url = cas_config["server_url"]
self.cas_service_url = cas_config["service_url"]
public_base_url = cas_config.get("service_url") or self.public_baseurl
if public_base_url[-1] != "/":
public_base_url += "/"
Copy link
Member

Choose a reason for hiding this comment

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

This will change self.public_baseurl, which may end up confusing people?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should not since strings are immutable: public_base_url doesn't end up as a reference to self.public_baseurl unless there's additional magic here that I don't know?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, yeah, this is python where public_base_url += will not mutate the strings

Copy link
Member Author

Choose a reason for hiding this comment

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

Also in reality this code won't run if self.public_baseurl is used:

synapse/synapse/config/server.py

Lines 167 to 168 in 0cd2938

if self.public_baseurl[-1] != "/":
self.public_baseurl += "/"

# TODO Update this to a _synapse URL.
self.cas_service_url = (
public_base_url + "_matrix/client/r0/login/cas/ticket"
)
self.cas_displayname_attribute = cas_config.get("displayname_attribute")
self.cas_required_attributes = cas_config.get("required_attributes") or {}
else:
Expand All @@ -53,10 +59,6 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
#
#server_url: "https://cas-server.com"

# The public URL of the homeserver.
#
#service_url: "https://homeserver.domain.com:8448"

# The attribute of the CAS response to use as the display name.
#
# If unset, no displayname will be set.
Expand Down
3 changes: 1 addition & 2 deletions synapse/config/oidc_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ def read_config(self, config, **kwargs):
"Multiple OIDC providers have the idp_id %r." % idp_id
)

public_baseurl = self.public_baseurl
self.oidc_callback_url = public_baseurl + "_synapse/oidc/callback"
self.oidc_callback_url = self.public_baseurl + "_synapse/oidc/callback"

@property
def oidc_enabled(self) -> bool:
Expand Down
6 changes: 1 addition & 5 deletions synapse/handlers/cas_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,7 @@ def _build_service_param(self, args: Dict[str, str]) -> str:
Returns:
The URL to use as a "service" parameter.
"""
return "%s%s?%s" % (
self._cas_service_url,
"/_matrix/client/r0/login/cas/ticket",
urllib.parse.urlencode(args),
)
return "%s?%s" % (self._cas_service_url, urllib.parse.urlencode(args),)

async def _validate_ticket(
self, ticket: str, service_args: Dict[str, str]
Expand Down