Make DomainSpecificString an attrs class

changelog.d/9875.misc

@@ -0,0 +1 @@
+Make `DomainSpecificString` an `attrs` class.

synapse/handlers/oidc.py

@@ -957,6 +957,11 @@ async def grandfather_existing_users() -> Optional[str]:
                 # and attempt to match it.
                 attributes = await oidc_response_to_user_attributes(failures=0)
 
+                if attributes.localpart is None:
+                    # If no localpart is returned then we will generate one, so
+                    # there is no need to search for existing users.
+                    return None
+
                 user_id = UserID(attributes.localpart, self._server_name).to_string()
                 users = await self._store.get_users_by_id_case_insensitive(user_id)
                 if users:

synapse/rest/synapse/client/new_user_consent.py

@@ -61,6 +61,15 @@ async def _async_render_GET(self, request: Request) -> None:
             self._sso_handler.render_error(request, "bad_session", e.msg, code=e.code)
             return
 
+        # It should be impossible to get here without having first been through
+        # the pick-a-username step, which ensures chosen_localpart gets set.
+        if not session.chosen_localpart:
+            logger.warning("Session has no user name selected")
+            self._sso_handler.render_error(
+                request, "no_user", "No user name has been selected.", code=400
+            )
+            return
+
         user_id = UserID(session.chosen_localpart, self._server_name)
         user_profile = {
             "display_name": session.display_name,

synapse/types.py

@@ -199,9 +199,8 @@ def get_localpart_from_id(string):
 DS = TypeVar("DS", bound="DomainSpecificString")
 
 
-class DomainSpecificString(
-    namedtuple("DomainSpecificString", ("localpart", "domain")), metaclass=abc.ABCMeta
-):
+@attr.s(slots=True, frozen=True)
+class DomainSpecificString(metaclass=abc.ABCMeta):
     """Common base class among ID/name strings that have a local part and a
     domain name, prefixed with a sigil.
 
@@ -213,11 +212,8 @@ class DomainSpecificString(
 
     SIGIL = abc.abstractproperty()  # type: str  # type: ignore
 
-    # Deny iteration because it will bite you if you try to create a singleton
-    # set by:
-    #    users = set(user)
-    def __iter__(self):
-        raise ValueError("Attempted to iterate a %s" % (type(self).__name__,))
+    localpart = attr.ib(type=str)
+    domain = attr.ib(type=str)
 
     # Because this class is a namedtuple of strings and booleans, it is deeply
     # immutable.
@@ -272,30 +268,35 @@ def is_valid(cls: Type[DS], s: str) -> bool:
     __repr__ = to_string
 
 
+@attr.s(slots=True, frozen=True)
 class UserID(DomainSpecificString):
     """Structure representing a user ID."""
 
     SIGIL = "@"
 
 
+@attr.s(slots=True, frozen=True)
 class RoomAlias(DomainSpecificString):
     """Structure representing a room name."""
 
     SIGIL = "#"
 
 
+@attr.s(slots=True, frozen=True)
 class RoomID(DomainSpecificString):
     """Structure representing a room id. """
 
     SIGIL = "!"
 
 
+@attr.s(slots=True, frozen=True)
 class EventID(DomainSpecificString):
     """Structure representing an event id. """
 
     SIGIL = "$"
 
 
+@attr.s(slots=True, frozen=True)
 class GroupID(DomainSpecificString):
     """Structure representing a group ID."""