From a45de9e8d01266fac367a5a487271b383116aaf7 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 3 Sep 2021 12:08:03 +0100
Subject: [PATCH] Incorporate review

---
 synapse/handlers/account_validity.py |  4 +++-
 synapse/handlers/identity.py         | 12 ++++++------
 synapse/util/stringutils.py          |  6 ------
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py
index ce92a8f831..4e02bd5a53 100644
--- a/synapse/handlers/account_validity.py
+++ b/synapse/handlers/account_validity.py
@@ -430,7 +430,9 @@ class AccountValidityHandler:
             # Show the user in the directory again by setting them to active
             # We get the profile handler here so that we don't cause a cyclic dependency
             # at startup.
-            # FIXME: this doesn't work with the new account validity module stuff
+            # FIXME: this doesn't work with the new account validity module stuff, in
+            #  that this code is only run when no account validity module implements the
+            #  on_legacy_renew callback - which synapse-email-account-validity does.
             await self.hs.get_profile_handler().set_active(
                 [UserID.from_string(user_id)], True, True
             )
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 79e070f12d..6242b6b323 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -202,17 +202,17 @@ class IdentityHandler(BaseHandler):
         if id_access_token is None:
             use_v2 = False
 
-        # if we have a rewrite rule set for the identity server,
-        # apply it now, but only for sending the request (not
-        # storing in the database).
-        id_server_url = self.rewrite_id_server_url(id_server, add_https=True)
-
-        if not valid_id_server_location(id_server_url):
+        if not valid_id_server_location(id_server):
             raise SynapseError(
                 400,
                 "id_server must be a valid hostname with optional port and path components",
             )
 
+        # if we have a rewrite rule set for the identity server,
+        # apply it now, but only for sending the request (not
+        # storing in the database).
+        id_server_url = self.rewrite_id_server_url(id_server, add_https=True)
+
         # Decide which API endpoint URLs to use
         headers = {}
         bind_data = {"sid": sid, "client_secret": client_secret, "mxid": mxid}
diff --git a/synapse/util/stringutils.py b/synapse/util/stringutils.py
index 6ed0580a52..da216242c4 100644
--- a/synapse/util/stringutils.py
+++ b/synapse/util/stringutils.py
@@ -147,12 +147,6 @@ def valid_id_server_location(id_server: str) -> bool:
     Returns:
         True if valid, False otherwise.
     """
-
-    # A previous call to rewrite_id_server_url might have added a protocol scheme to the
-    # server's address - we don't care about that so remove it just for these checks.
-    if id_server.startswith("https://"):
-        id_server = id_server[8:]
-
     components = id_server.split("/", 1)
 
     host = components[0]