Support for form_post in OIDC responses (#9376)

Apple want to POST the OIDC auth response back to us rather than using query-params; add the necessary support to make that work.
pull/9433/head
Richard van der Hoff 2021-02-17 10:15:14 +00:00 committed by GitHub
parent 33f64ca7d6
commit e1071fd625
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 78 additions and 36 deletions

1
changelog.d/9376.feature Normal file
View File

@ -0,0 +1 @@
Add support for receiving OpenID Connect authentication responses via form `POST`s rather than `GET`s.

View File

@ -48,7 +48,26 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__)
SESSION_COOKIE_NAME = b"oidc_session"
# we want the cookie to be returned to us even when the request is the POSTed
# result of a form on another domain, as is used with `response_mode=form_post`.
#
# Modern browsers will not do so unless we set SameSite=None; however *older*
# browsers (including all versions of Safari on iOS 12?) don't support
# SameSite=None, and interpret it as SameSite=Strict:
# https://bugs.webkit.org/show_bug.cgi?id=198181
#
# As a rather painful workaround, we set *two* cookies, one with SameSite=None
# and one with no SameSite, in the hope that at least one of them will get
# back to us.
#
# Secure is necessary for SameSite=None (and, empirically, also breaks things
# on iOS 12.)
#
# 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"),
]
#: A token exchanged from the token endpoint, as per RFC6749 sec 5.1. and
#: OpenID.Core sec 3.1.3.3.
@ -149,26 +168,33 @@ class OidcHandler:
# otherwise, it is presumably a successful response. see:
# https://tools.ietf.org/html/rfc6749#section-4.1.2
# Fetch the session cookie
session = request.getCookie(SESSION_COOKIE_NAME) # type: Optional[bytes]
if session is None:
# Fetch the session cookie. See the comments on SESSION_COOKIES for why there
# are two.
for cookie_name, _ in _SESSION_COOKIES:
session = request.getCookie(cookie_name) # type: Optional[bytes]
if session is not None:
break
else:
logger.info("Received OIDC callback, with no session cookie")
self._sso_handler.render_error(
request, "missing_session", "No session cookie found"
)
return
# Remove the cookie. There is a good chance that if the callback failed
# Remove the cookies. There is a good chance that if the callback failed
# once, it will fail next time and the code will already be exchanged.
# Removing it early avoids spamming the provider with token requests.
request.addCookie(
SESSION_COOKIE_NAME,
b"",
path="/_synapse/oidc",
expires="Thu, Jan 01 1970 00:00:00 UTC",
httpOnly=True,
sameSite="lax",
)
# Removing the cookies early avoids spamming the provider with token requests.
#
# we have to build the header by hand rather than calling request.addCookie
# because the latter does not support SameSite=None
# (https://twistedmatrix.com/trac/ticket/10088)
for cookie_name, options in _SESSION_COOKIES:
request.cookies.append(
b"%s=; Expires=Thu, Jan 01 1970 00:00:00 UTC; %s"
% (cookie_name, options)
)
# Check for the state query parameter
if b"state" not in request.args:
@ -722,14 +748,18 @@ class OidcProvider:
ui_auth_session_id=ui_auth_session_id,
),
)
request.addCookie(
SESSION_COOKIE_NAME,
cookie,
path="/_synapse/client/oidc",
max_age="3600",
httpOnly=True,
sameSite="lax",
)
# Set the cookies. See the comments on _SESSION_COOKIES for why there are two.
#
# we have to build the header by hand rather than calling request.addCookie
# because the latter does not support SameSite=None
# (https://twistedmatrix.com/trac/ticket/10088)
for cookie_name, options in _SESSION_COOKIES:
request.cookies.append(
b"%s=%s; Max-Age=3600; %s"
% (cookie_name, cookie.encode("utf-8"), options)
)
metadata = await self.load_metadata()
authorization_endpoint = metadata.get("authorization_endpoint")

View File

@ -12,19 +12,30 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from typing import TYPE_CHECKING
from synapse.http.server import DirectServeHtmlResource
if TYPE_CHECKING:
from synapse.server import HomeServer
logger = logging.getLogger(__name__)
class OIDCCallbackResource(DirectServeHtmlResource):
isLeaf = 1
def __init__(self, hs):
def __init__(self, hs: "HomeServer"):
super().__init__()
self._oidc_handler = hs.get_oidc_handler()
async def _async_render_GET(self, request):
await self._oidc_handler.handle_oidc_callback(request)
async def _async_render_POST(self, request):
# the auth response can be returned via an x-www-form-urlencoded form instead
# of GET params, as per
# https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html.
await self._oidc_handler.handle_oidc_callback(request)

View File

@ -327,7 +327,9 @@ class OidcHandlerTestCase(HomeserverTestCase):
def test_redirect_request(self):
"""The redirect request has the right arguments & generates a valid session cookie."""
req = Mock(spec=["addCookie"])
req = Mock(spec=["cookies"])
req.cookies = []
url = self.get_success(
self.provider.handle_redirect_request(req, b"http://client/redirect")
)
@ -346,19 +348,16 @@ class OidcHandlerTestCase(HomeserverTestCase):
self.assertEqual(len(params["state"]), 1)
self.assertEqual(len(params["nonce"]), 1)
# Check what is in the cookie
# note: python3.5 mock does not have the .called_once() method
calls = req.addCookie.call_args_list
self.assertEqual(len(calls), 1) # called once
# For some reason, call.args does not work with python3.5
args = calls[0][0]
kwargs = calls[0][1]
# Check what is in the cookies
self.assertEqual(len(req.cookies), 2) # two cookies
cookie_header = req.cookies[0]
# The cookie name and path don't really matter, just that it has to be coherent
# between the callback & redirect handlers.
self.assertEqual(args[0], b"oidc_session")
self.assertEqual(kwargs["path"], "/_synapse/client/oidc")
cookie = args[1]
parts = [p.strip() for p in cookie_header.split(b";")]
self.assertIn(b"Path=/_synapse/client/oidc", parts)
name, cookie = parts[0].split(b"=")
self.assertEqual(name, b"oidc_session")
macaroon = pymacaroons.Macaroon.deserialize(cookie)
state = self.handler._token_generator._get_value_from_macaroon(
@ -489,7 +488,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
def test_callback_session(self):
"""The callback verifies the session presence and validity"""
request = Mock(spec=["args", "getCookie", "addCookie"])
request = Mock(spec=["args", "getCookie", "cookies"])
# Missing cookie
request.args = {}
@ -943,13 +942,14 @@ def _build_callback_request(
spec=[
"args",
"getCookie",
"addCookie",
"cookies",
"requestHeaders",
"getClientIP",
"getHeader",
]
)
request.cookies = []
request.getCookie.return_value = session
request.args = {}
request.args[b"code"] = [code.encode("utf-8")]