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

Make DomainSpecificString an attrs class #9875

Merged
merged 4 commits into from
Apr 23, 2021
Merged
Show file tree
Hide file tree
Changes from all 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/9875.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make `DomainSpecificString` an `attrs` class.
5 changes: 5 additions & 0 deletions synapse/handlers/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +960 to +963
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this previously worked by there hopefully not being an @None:foo user?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I believe so


user_id = UserID(attributes.localpart, self._server_name).to_string()
users = await self._store.get_users_by_id_case_insensitive(user_id)
if users:
Expand Down
9 changes: 9 additions & 0 deletions synapse/rest/synapse/client/new_user_consent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking for not asserting here is that I assume its possible for a client that is being stupid to get here, and so we don't want to get stack traces/sentry alerts?


user_id = UserID(session.chosen_localpart, self._server_name)
user_profile = {
"display_name": session.display_name,
Expand Down
17 changes: 9 additions & 8 deletions synapse/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, repr=False)
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.

Expand All @@ -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.
Expand Down Expand Up @@ -272,30 +268,35 @@ def is_valid(cls: Type[DS], s: str) -> bool:
__repr__ = to_string


@attr.s(slots=True, frozen=True, repr=False)
class UserID(DomainSpecificString):
"""Structure representing a user ID."""

SIGIL = "@"


@attr.s(slots=True, frozen=True, repr=False)
class RoomAlias(DomainSpecificString):
"""Structure representing a room name."""

SIGIL = "#"


@attr.s(slots=True, frozen=True, repr=False)
class RoomID(DomainSpecificString):
"""Structure representing a room id. """

SIGIL = "!"


@attr.s(slots=True, frozen=True, repr=False)
class EventID(DomainSpecificString):
"""Structure representing an event id. """

SIGIL = "$"


@attr.s(slots=True, frozen=True, repr=False)
class GroupID(DomainSpecificString):
"""Structure representing a group ID."""

Expand Down