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

fix bug where preserved threepid user comes to sign up and server is … #3777

Merged
merged 7 commits into from
Aug 31, 2018

Conversation

neilisfragile
Copy link
Contributor

…mau blocked

I have omitted saml2 support and a subset of v1/register

@@ -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):
Copy link
Member

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]:
Copy link
Member

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)
Copy link
Member

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)
Copy link
Member

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
Copy link
Member

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?

@@ -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")
Copy link
Member

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.

@erikjohnston erikjohnston removed their assignment Aug 31, 2018
@neilisfragile neilisfragile merged commit 99178f8 into develop Aug 31, 2018
@hawkowl hawkowl deleted the neilj/fix_register_user_registration branch September 20, 2018 14:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants