-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Send password reset from HS: Sending the email #5345
Changes from 27 commits
dbdebc2
9567c60
ed35302
094c351
899219c
309943f
354d749
62e1ec0
a0e2a10
a862f2a
752dbee
177f024
6d2d3c9
6394715
fe0af29
91eac88
c9573ca
4c406f5
70b161d
79bc668
f522cde
a4c0907
efa1a56
6a9588c
78ca92a
12ed769
6efb301
3478213
a37a2f1
cd4f4a2
92090d3
7168dee
828cdbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add ability to perform password reset via email without trusting the identity server. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -74,14 +74,75 @@ def read_config(self, config): | |||||
"account_validity", {}, | ||||||
).get("renew_at") | ||||||
|
||||||
if self.email_enable_notifs or account_validity_renewal_enabled: | ||||||
self.email_enable_password_reset_from_is = email_config.get( | ||||||
"enable_password_reset_from_is", False, | ||||||
) | ||||||
self.enable_password_resets = ( | ||||||
self.email_enable_password_reset_from_is | ||||||
or (not self.email_enable_password_reset_from_is and email_config != {}) | ||||||
) | ||||||
if email_config == {} and not self.email_enable_password_reset_from_is: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
logger.warn( | ||||||
"User password resets have been disabled due to lack of email config." | ||||||
) | ||||||
|
||||||
self.email_validation_token_lifetime = email_config.get( | ||||||
"validation_token_lifetime", 15 * 60, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, should be in milliseconds for consistency |
||||||
) | ||||||
|
||||||
if ( | ||||||
self.email_enable_notifs | ||||||
or account_validity_renewal_enabled | ||||||
or (self.enable_password_resets | ||||||
and self.email_enable_password_reset_from_is) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
No? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes |
||||||
): | ||||||
# make sure we can import the required deps | ||||||
import jinja2 | ||||||
import bleach | ||||||
# prevent unused warnings | ||||||
jinja2 | ||||||
bleach | ||||||
|
||||||
if self.enable_password_resets and not self.email_enable_password_reset_from_is: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think at this point we change |
||||||
required = [ | ||||||
"smtp_host", | ||||||
"smtp_port", | ||||||
"notif_from", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably call this something else.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but, backwards compatibility. |
||||||
] | ||||||
|
||||||
missing = [] | ||||||
for k in required: | ||||||
if k not in email_config: | ||||||
missing.append(k) | ||||||
|
||||||
if (len(missing) > 0): | ||||||
raise RuntimeError( | ||||||
"email.enable_password_reset_from_is is False " | ||||||
"but required keys are missing: %s" % | ||||||
(", ".join(["email." + k for k in missing]),) | ||||||
) | ||||||
|
||||||
# Templates for password reset emails | ||||||
self.email_password_reset_template_html = email_config.get( | ||||||
"password_reset_template_html", "password_reset.html", | ||||||
) | ||||||
self.email_password_reset_template_text = email_config.get( | ||||||
"password_reset_template_text", "password_reset.txt", | ||||||
) | ||||||
|
||||||
# Check templates exist | ||||||
for f in [self.email_password_reset_template_html, | ||||||
self.email_password_reset_template_text]: | ||||||
p = os.path.join(self.email_template_dir, f) | ||||||
if not os.path.isfile(p): | ||||||
raise ConfigError("Unable to find template file %s" % (p, )) | ||||||
|
||||||
if config.get("public_baseurl") is None: | ||||||
raise RuntimeError( | ||||||
"email.enable_password_reset_from_is is False but no " | ||||||
"public_baseurl is set" | ||||||
) | ||||||
|
||||||
if self.email_enable_notifs: | ||||||
required = [ | ||||||
"smtp_host", | ||||||
|
@@ -139,33 +200,79 @@ def read_config(self, config): | |||||
if not os.path.isfile(p): | ||||||
raise ConfigError("Unable to find email template file %s" % (p, )) | ||||||
|
||||||
def _get_template_content(self, template_dir, path): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's already a |
||||||
fullpath = os.path.join(template_dir, path) | ||||||
|
||||||
try: | ||||||
with open(fullpath) as f: | ||||||
return f.read() | ||||||
except Exception as e: | ||||||
raise ConfigError( | ||||||
"Unable to read content of template: %s - %s", fullpath, e, | ||||||
) | ||||||
|
||||||
def default_config(self, config_dir_path, server_name, **kwargs): | ||||||
return """ | ||||||
# Enable sending emails for notification events or expiry notices | ||||||
# Defining a custom URL for Riot is only needed if email notifications | ||||||
# should contain links to a self-hosted installation of Riot; when set | ||||||
# the "app_name" setting is ignored. | ||||||
# Enable sending emails for password resets, notification events or | ||||||
# account expiry notices. | ||||||
# | ||||||
# If your SMTP server requires authentication, the optional smtp_user & | ||||||
# smtp_pass variables should be used | ||||||
# | ||||||
#email: | ||||||
# enable_notifs: false | ||||||
# smtp_host: "localhost" | ||||||
# smtp_port: 25 | ||||||
# smtp_port: 25 # SSL: 465, STARTTLS: 587 | ||||||
# smtp_user: "exampleusername" | ||||||
# smtp_pass: "examplepassword" | ||||||
# require_transport_security: False | ||||||
# notif_from: "Your Friendly %(app)s Home Server <noreply@example.com>" | ||||||
# app_name: Matrix | ||||||
# # if template_dir is unset, uses the example templates that are part of | ||||||
# # the Synapse distribution. | ||||||
# | ||||||
# # Enable sending email notifications for new chat messages | ||||||
# # | ||||||
# enable_notifs: False | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we not document these stuff in this PR please, its a bit confusing |
||||||
# | ||||||
# # Enable email notifications by default | ||||||
# notif_for_new_users: True | ||||||
# | ||||||
# # Defining a custom URL for Riot is only needed if email notifications | ||||||
# # should contain links to a self-hosted installation of Riot; when set | ||||||
# # the "app_name" setting is ignored | ||||||
# riot_base_url: "http://localhost/riot" | ||||||
# | ||||||
# # Disable sending password reset emails via the configured, trusted | ||||||
# # identity servers | ||||||
# # | ||||||
# # IMPORTANT! This will give a malicious or overtaken identity server | ||||||
# # the ability to reset passwords for your users! Make absolutely sure | ||||||
# # that you want to do this! It is strongly recommended that password | ||||||
# # reset emails be sent by the homeserver instead | ||||||
# # | ||||||
# #enable_password_reset_from_is: False | ||||||
# | ||||||
# # Configure the time in seconds that a validation email or text | ||||||
# # message code will expire after sending | ||||||
# # | ||||||
# # This is currently used for password resets | ||||||
# #validation_token_lifetime: 900 # 15 minutes | ||||||
# | ||||||
# # Template directory. All template files should be stored within this | ||||||
# # directory | ||||||
# # | ||||||
# #template_dir: res/templates | ||||||
# | ||||||
# # Templates for email notifications | ||||||
# # | ||||||
# notif_template_html: notif_mail.html | ||||||
# notif_template_text: notif_mail.txt | ||||||
# # Templates for account expiry notices. | ||||||
# | ||||||
# # Templates for account expiry notices | ||||||
# # | ||||||
# expiry_template_html: notice_expiry.html | ||||||
# expiry_template_text: notice_expiry.txt | ||||||
# notif_for_new_users: True | ||||||
# riot_base_url: "http://localhost/riot" | ||||||
# | ||||||
# # Templates for password reset emails sent by the homeserver | ||||||
# # | ||||||
# #password_reset_template_html: password_reset.html | ||||||
# #password_reset_template_text: password_reset.txt | ||||||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,10 +80,10 @@ | |
|
||
|
||
class Mailer(object): | ||
def __init__(self, hs, app_name, notif_template_html, notif_template_text): | ||
def __init__(self, hs, app_name, template_html, template_text): | ||
self.hs = hs | ||
self.notif_template_html = notif_template_html | ||
self.notif_template_text = notif_template_text | ||
self.template_html = template_html | ||
self.template_text = template_text | ||
|
||
self.sendmail = self.hs.get_sendmail() | ||
self.store = self.hs.get_datastore() | ||
|
@@ -94,21 +94,48 @@ def __init__(self, hs, app_name, notif_template_html, notif_template_text): | |
logger.info("Created Mailer for app_name %s" % app_name) | ||
|
||
@defer.inlineCallbacks | ||
def send_notification_mail(self, app_id, user_id, email_address, | ||
push_actions, reason): | ||
try: | ||
from_string = self.hs.config.email_notif_from % { | ||
"app": self.app_name | ||
} | ||
except TypeError: | ||
from_string = self.hs.config.email_notif_from | ||
def send_password_reset_mail( | ||
self, | ||
email_address, | ||
token, | ||
client_secret, | ||
sid, | ||
): | ||
"""Send an email with a password reset link to a user | ||
|
||
Args: | ||
email_address (str): Email address we're sending the password | ||
reset to | ||
token (str): Unique token generated by the server to verify | ||
password reset email was received | ||
client_secret (str): Unique token generated by the client to | ||
group together multiple email sending attempts | ||
sid (str): The generated session ID | ||
""" | ||
if email.utils.parseaddr(email_address)[1] == '': | ||
raise RuntimeError("Invalid 'to' email address") | ||
|
||
link = ( | ||
self.hs.config.public_baseurl + | ||
"_matrix/identity/api/v1/validate/email/submitToken" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will make it hard/impossible to run an identity server on the same domain as the HS. Given this should be an opaque link(?), let's just have something like |
||
"?token=%s&client_secret=%s&sid=%s" % | ||
(token, client_secret, sid) | ||
) | ||
|
||
raw_from = email.utils.parseaddr(from_string)[1] | ||
raw_to = email.utils.parseaddr(email_address)[1] | ||
template_vars = { | ||
"link": link, | ||
} | ||
|
||
if raw_to == '': | ||
raise RuntimeError("Invalid 'to' address") | ||
yield self.send_email( | ||
email_address, | ||
"[%s] Password Reset Email" % self.hs.config.server_name, | ||
template_vars, | ||
) | ||
|
||
@defer.inlineCallbacks | ||
def send_notification_mail(self, app_id, user_id, email_address, | ||
push_actions, reason): | ||
"""Send email regarding a user's room notifications""" | ||
rooms_in_order = deduped_ordered_list( | ||
[pa['room_id'] for pa in push_actions] | ||
) | ||
|
@@ -176,14 +203,36 @@ def _fetch_room_state(room_id): | |
"reason": reason, | ||
} | ||
|
||
html_text = self.notif_template_html.render(**template_vars) | ||
yield self.send_email( | ||
email_address, | ||
"[%s] %s" % (self.app_name, summary_text), | ||
template_vars, | ||
) | ||
|
||
@defer.inlineCallbacks | ||
def send_email(self, email_address, subject, template_vars): | ||
"""Send an email with the given information and template text""" | ||
try: | ||
from_string = self.hs.config.email_notif_from % { | ||
"app": self.app_name | ||
} | ||
except TypeError: | ||
from_string = self.hs.config.email_notif_from | ||
|
||
raw_from = email.utils.parseaddr(from_string)[1] | ||
raw_to = email.utils.parseaddr(email_address)[1] | ||
|
||
if raw_to == '': | ||
raise RuntimeError("Invalid 'to' address") | ||
|
||
html_text = self.template_html.render(**template_vars) | ||
html_part = MIMEText(html_text, "html", "utf8") | ||
|
||
plain_text = self.notif_template_text.render(**template_vars) | ||
plain_text = self.template_text.render(**template_vars) | ||
text_part = MIMEText(plain_text, "plain", "utf8") | ||
|
||
multipart_msg = MIMEMultipart('alternative') | ||
multipart_msg['Subject'] = "[%s] %s" % (self.app_name, summary_text) | ||
multipart_msg['Subject'] = subject | ||
multipart_msg['From'] = from_string | ||
multipart_msg['To'] = email_address | ||
multipart_msg['Date'] = email.utils.formatdate() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,7 +77,7 @@ | |
] | ||
|
||
CONDITIONAL_REQUIREMENTS = { | ||
"email.enable_notifs": ["Jinja2>=2.9", "bleach>=1.4.2"], | ||
"email": ["Jinja2>=2.9", "bleach>=1.4.2"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was changed as multiple options (all relating to email) now require these dependencies. If some documentation lists the different pip [...] options, we'll need to update them. |
||
"matrix-synapse-ldap3": ["matrix-synapse-ldap3>=0.1"], | ||
|
||
# we use execute_batch, which arrived in psycopg 2.7. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<html> | ||
<body> | ||
<p>A password reset request has been received for your Matrix account. If this was you, please click the link below to confirm resetting your password:</p> | ||
|
||
<a href="{{ link }}">{{ link }}</a> | ||
|
||
<p>If this was not you, please disregard this email and contact your server administrator. Thank you.</p> | ||
</body> | ||
</html> |
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.
Isn't this just
self.enable_password_resets = self.email_enable_password_reset_from_is or email_config != {}
?