-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix bug where preserved threepid user comes to sign up and server is … #3777
Conversation
@@ -775,7 +775,7 @@ def check_in_room_or_world_readable(self, room_id, user_id): | |||
) | |||
|
|||
@defer.inlineCallbacks | |||
def check_auth_blocking(self, user_id=None): | |||
def check_auth_blocking(self, user_id=None, threepid=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please doc threepid arg
@@ -281,12 +281,20 @@ def _do_password(self, request, register_json, session): | |||
register_json["user"].encode("utf-8") | |||
if "user" in register_json else None | |||
) | |||
threepid = None | |||
if session[LoginType.EMAIL_IDENTITY]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this throw if the session doesn't have the email login step?
# Necessary due to auth checks prior to the threepid being | ||
# written to the db | ||
if self.store.is_threepid_reserved(threepid): | ||
self.store.upsert_monthly_active_user(registered_user_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a yield
# Necessary due to auth checks prior to the threepid being | ||
# written to the db | ||
if self.store.is_threepid_reserved(threepid): | ||
self.store.upsert_monthly_active_user(registered_user_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yield
and threepid['address'] == tp['address']): | ||
return True | ||
else: | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this in store
rather than e.g. in the config object?
synapse/api/auth.py
Outdated
@@ -797,6 +803,10 @@ def check_auth_blocking(self, user_id=None): | |||
limit_type=self.hs.config.hs_disabled_limit_type | |||
) | |||
if self.hs.config.limit_usage_by_mau is True: | |||
|
|||
if user_id and threepid: | |||
logger.warn("Called with both user_id and threepid, this shoudn't happen") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I'd assert not (user_id and threepid)
or something so that if someone does do it it gets picked up in tests.
…mau blocked
I have omitted saml2 support and a subset of v1/register