Use better fix

element-hq · Nov 28, 2024 · 439aefb · 439aefb
1 parent 4ea1a51
commit 439aefb
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 33 deletions.
diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py
@@ -76,12 +76,7 @@
 
 
 # synapse server name: used to populate public_baseurl in some tests
-#
-# Because we can only specify the URI path when doing a `make_request(...)`, this needs
-# to match the hostname/port that's used in the `make_request(...)` calls; because the
-# `SsoRedirectServlet` expects us to use the "canonical URL" for the homeserver in order
-# for the cookies to be visible.
-SYNAPSE_SERVER_PUBLIC_HOSTNAME = "127.0.0.1:8888"
+SYNAPSE_SERVER_PUBLIC_HOSTNAME = "synapse"
 
 # public_baseurl for some tests. It uses an http:// scheme because
 # FakeChannel.isSecure() returns False, so synapse will see the requested uri as
@@ -759,14 +754,16 @@ def test_multi_sso_redirect_to_cas(self) -> None:
         )
 
         # follow the redirect
-        #
-        # We have to make this relative to be compatible with `make_request(...)`
-        relative_sso_login_redirect_uri = get_relative_uri_from_absolute_uri(
-            sso_login_redirect_uri
-        )
         channel = self.make_request(
             "GET",
-            relative_sso_login_redirect_uri,
+            # We have to make this relative to be compatible with `make_request(...)`
+            get_relative_uri_from_absolute_uri(sso_login_redirect_uri),
+            # We have to set the Host header to match the `public_baseurl` to avoid
+            # the extra redirect in the `SsoRedirectServlet` in order for the
+            # cookies to be visible.
+            custom_headers=[
+                ("Host", SYNAPSE_SERVER_PUBLIC_HOSTNAME),
+            ],
         )
 
         self.assertEqual(channel.code, 302, channel.result)
@@ -808,14 +805,16 @@ def test_multi_sso_redirect_to_saml(self) -> None:
         )
 
         # follow the redirect
-        #
-        # We have to make this relative to be compatible with `make_request(...)`
-        relative_sso_login_redirect_uri = get_relative_uri_from_absolute_uri(
-            sso_login_redirect_uri
-        )
         channel = self.make_request(
             "GET",
-            relative_sso_login_redirect_uri,
+            # We have to make this relative to be compatible with `make_request(...)`
+            get_relative_uri_from_absolute_uri(sso_login_redirect_uri),
+            # We have to set the Host header to match the `public_baseurl` to avoid
+            # the extra redirect in the `SsoRedirectServlet` in order for the
+            # cookies to be visible.
+            custom_headers=[
+                ("Host", SYNAPSE_SERVER_PUBLIC_HOSTNAME),
+            ],
         )
 
         self.assertEqual(channel.code, 302, channel.result)
@@ -858,14 +857,16 @@ def test_login_via_oidc(self) -> None:
 
         with fake_oidc_server.patch_homeserver(hs=self.hs):
             # follow the redirect
-            #
-            # We have to make this relative to be compatible with `make_request(...)`
-            relative_sso_login_redirect_uri = get_relative_uri_from_absolute_uri(
-                sso_login_redirect_uri
-            )
             channel = self.make_request(
                 "GET",
-                relative_sso_login_redirect_uri,
+                # We have to make this relative to be compatible with `make_request(...)`
+                get_relative_uri_from_absolute_uri(sso_login_redirect_uri),
+                # We have to set the Host header to match the `public_baseurl` to avoid
+                # the extra redirect in the `SsoRedirectServlet` in order for the
+                # cookies to be visible.
+                custom_headers=[
+                    ("Host", SYNAPSE_SERVER_PUBLIC_HOSTNAME),
+                ],
             )
 
         self.assertEqual(channel.code, 302, channel.result)
@@ -948,14 +949,16 @@ def test_multi_sso_redirect_to_unknown(self) -> None:
         )
 
         # follow the redirect
-        #
-        # We have to make this relative to be compatible with `make_request(...)`
-        relative_sso_login_redirect_uri = get_relative_uri_from_absolute_uri(
-            sso_login_redirect_uri
-        )
         channel = self.make_request(
             "GET",
-            relative_sso_login_redirect_uri,
+            # We have to make this relative to be compatible with `make_request(...)`
+            get_relative_uri_from_absolute_uri(sso_login_redirect_uri),
+            # We have to set the Host header to match the `public_baseurl` to avoid
+            # the extra redirect in the `SsoRedirectServlet` in order for the
+            # cookies to be visible.
+            custom_headers=[
+                ("Host", SYNAPSE_SERVER_PUBLIC_HOSTNAME),
+            ],
         )
 
         self.assertEqual(channel.code, 404, channel.result)

diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py
@@ -889,7 +889,7 @@ def initiate_sso_login(
             "GET",
             uri,
         )
-        assert channel.code == 302
+        assert channel.code == 302, f"Expected 302 for {uri}, got {channel.code}"
 
         # hit the redirect url again with the right Host header, which should now issue
         # a cookie and redirect to the SSO provider.
@@ -901,17 +901,18 @@ def get_location(channel: FakeChannel) -> str:
 
         location = get_location(channel)
         parts = urllib.parse.urlsplit(location)
+        next_uri = urllib.parse.urlunsplit(("", "") + parts[2:])
         channel = make_request(
             self.reactor,
             self.site,
             "GET",
-            urllib.parse.urlunsplit(("", "") + parts[2:]),
+            next_uri,
             custom_headers=[
                 ("Host", parts[1]),
             ],
         )
 
-        assert channel.code == 302
+        assert channel.code == 302, f"Expected 302 for {next_uri}, got {channel.code}"
         channel.extract_cookies(cookies)
         return get_location(channel)