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

Remove bind_email parameter from register_user#7336

Closed
anoadragon453 wants to merge 2 commits into
developfrom
anoa/remove_bind_email
Closed

Remove bind_email parameter from register_user#7336
anoadragon453 wants to merge 2 commits into
developfrom
anoa/remove_bind_email

Conversation

@anoadragon453
Copy link
Copy Markdown
Member

As far as I can tell, this was only used by the bind_email parameter which was removed in #5964.

Emails added during registration still happen in post_registration_actions:

if auth_result and LoginType.EMAIL_IDENTITY in auth_result:
threepid = auth_result[LoginType.EMAIL_IDENTITY]
# Necessary due to auth checks prior to the threepid being
# written to the db
if is_threepid_reserved(
self.hs.config.mau_limits_reserved_threepids, threepid
):
yield self.store.upsert_monthly_active_user(user_id)
yield self._register_email_threepid(user_id, threepid, access_token)

@anoadragon453 anoadragon453 requested a review from a team April 23, 2020 15:33
@anoadragon453 anoadragon453 self-assigned this Apr 23, 2020
Copy link
Copy Markdown
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks OK. I think we need to think about changing the ModuleApi object a bit more?

Comment thread synapse/handlers/register.py

@defer.inlineCallbacks
def register(self, localpart, displayname=None, emails=[]):
def register(self, localpart, displayname=None):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably note in the changelog that his changed (or leave it for backwards compatibility).

Do we know if any external modules called this with an iterable of emails?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Aha! Good spot. Ok, I was under the impression that modules like matrix-synapse-ldap3 bound emails in another manner (through the check_3pid_auth module method).

But you're right!

Which probably makes this whole PR defunct I suppose.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, yeah looks like this is still used then. :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh well!

@anoadragon453
Copy link
Copy Markdown
Member Author

Closing as this argument is still used by modules. Your editor may not show it though as it won't know about a separate codebase's method calls :)

@anoadragon453 anoadragon453 deleted the anoa/remove_bind_email branch April 23, 2020 18:35
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