Skip to content

Commit

Permalink
Fix email verification redirection (#16761)
Browse files Browse the repository at this point in the history
Previously, the response status of `HTMLResource` was hardcoded as
`200`. However, for proper redirection after the user verifies their
email, we require the status to be `302`. This PR addresses that issue
by using `code` as response status.
  • Loading branch information
FadhlanR authored Jan 2, 2024
1 parent 0f535f2 commit eabedd9
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 5 deletions.
1 change: 1 addition & 0 deletions changelog.d/16761.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix email verification redirection. Contributed by Fadhlan Ridhwanallah.
2 changes: 1 addition & 1 deletion synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ def _send_response(
assert isinstance(response_object, bytes)
html_bytes = response_object

respond_with_html_bytes(request, 200, html_bytes)
respond_with_html_bytes(request, code, html_bytes)

def _send_error_response(
self,
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/synapse/client/password_reset.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ async def _async_render_POST(self, request: Request) -> Tuple[int, bytes]:
return (
302,
(
b'You are being redirected to <a src="%s">%s</a>.'
b'You are being redirected to <a href="%s">%s</a>.'
% (next_link_bytes, next_link_bytes)
),
)
Expand Down
43 changes: 40 additions & 3 deletions tests/rest/client/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,16 +328,49 @@ def test_password_reset_bad_email_inhibit_error(self) -> None:

self.assertIsNotNone(session_id)

def test_password_reset_redirection(self) -> None:
"""Test basic password reset flow"""
old_password = "monkey"

user_id = self.register_user("kermit", old_password)
self.login("kermit", old_password)

email = "test@example.com"

# Add a threepid
self.get_success(
self.store.user_add_threepid(
user_id=user_id,
medium="email",
address=email,
validated_at=0,
added_at=0,
)
)

client_secret = "foobar"
next_link = "http://example.com"
self._request_token(email, client_secret, "127.0.0.1", next_link)

self.assertEqual(len(self.email_attempts), 1)
link = self._get_link_from_email()

self._validate_token(link, next_link)

def _request_token(
self,
email: str,
client_secret: str,
ip: str = "127.0.0.1",
next_link: Optional[str] = None,
) -> str:
body = {"client_secret": client_secret, "email": email, "send_attempt": 1}
if next_link is not None:
body["next_link"] = next_link
channel = self.make_request(
"POST",
b"account/password/email/requestToken",
{"client_secret": client_secret, "email": email, "send_attempt": 1},
body,
client_ip=ip,
)

Expand All @@ -350,7 +383,7 @@ def _request_token(

return channel.json_body["sid"]

def _validate_token(self, link: str) -> None:
def _validate_token(self, link: str, next_link: Optional[str] = None) -> None:
# Remove the host
path = link.replace("https://example.com", "")

Expand Down Expand Up @@ -378,7 +411,11 @@ def _validate_token(self, link: str) -> None:
shorthand=False,
content_is_form=True,
)
self.assertEqual(HTTPStatus.OK, channel.code, channel.result)
self.assertEqual(
HTTPStatus.OK if next_link is None else HTTPStatus.FOUND,
channel.code,
channel.result,
)

def _get_link_from_email(self) -> str:
assert self.email_attempts, "No emails have been sent"
Expand Down

0 comments on commit eabedd9

Please sign in to comment.