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

Allow OIDC cookies to work on non-root public baseurls #9726

Merged
merged 6 commits into from
Apr 23, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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/9726.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes the OIDC SSO flow when using a `public_baseurl` value including a non-root URL path.
8 changes: 4 additions & 4 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,11 @@ def read_config(self, config, **kwargs):
self.print_pidfile = config.get("print_pidfile")
self.user_agent_suffix = config.get("user_agent_suffix")
self.use_frozen_dicts = config.get("use_frozen_dicts", False)

self.public_baseurl = config.get("public_baseurl")
if self.public_baseurl is not None:
if self.public_baseurl[-1] != "/":
self.public_baseurl += "/"

# Whether to enable user presence.
presence_config = config.get("presence") or {}
Expand Down Expand Up @@ -407,10 +411,6 @@ def read_config(self, config, **kwargs):
config_path=("federation_ip_range_blacklist",),
)

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

# (undocumented) option for torturing the worker-mode replication a bit,
# for testing. The value defines the number of milliseconds to pause before
# sending out any replication updates.
Expand Down
23 changes: 18 additions & 5 deletions synapse/handlers/oidc_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import inspect
import logging
from typing import TYPE_CHECKING, Dict, Generic, List, Optional, TypeVar, Union
from urllib.parse import urlencode
from urllib.parse import urlencode, urlparse

import attr
import pymacaroons
Expand Down Expand Up @@ -71,8 +71,8 @@
#
# Here we have the names of the cookies, and the options we use to set them.
_SESSION_COOKIES = [
(b"oidc_session", b"Path=/_synapse/client/oidc; HttpOnly; Secure; SameSite=None"),
(b"oidc_session_no_samesite", b"Path=/_synapse/client/oidc; HttpOnly"),
(b"oidc_session", b"HttpOnly; Secure; SameSite=None"),
(b"oidc_session_no_samesite", b"HttpOnly"),
]

#: A token exchanged from the token endpoint, as per RFC6749 sec 5.1. and
Expand Down Expand Up @@ -282,6 +282,13 @@ def __init__(
self._config = provider
self._callback_url = hs.config.oidc_callback_url # type: str

# TODO: This is pretty much a hack to get the path specified by public_baseurl.
# It'd probably be nicer to have a config option that lets you specify a custom
# path, which we'd then use here.
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
self._callback_path_prefix = urlparse(hs.config.public_baseurl).path
if self._callback_path_prefix.endswith("/"):
self._callback_path_prefix = self._callback_path_prefix[:-1]

self._oidc_attribute_requirements = provider.attribute_requirements
self._scopes = provider.scopes
self._user_profile_method = provider.user_profile_method
Expand Down Expand Up @@ -782,8 +789,14 @@ async def handle_redirect_request(

for cookie_name, options in _SESSION_COOKIES:
request.cookies.append(
b"%s=%s; Max-Age=3600; %s"
% (cookie_name, cookie.encode("utf-8"), options)
b"%s=%s; Max-Age=3600; Path=%s; %s"
% (
cookie_name,
cookie.encode("utf-8"),
self._callback_path_prefix.encode("utf-8")
+ b"/_synapse/client/oidc",
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
options,
)
)

metadata = await self.load_metadata()
Expand Down