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

Fix an internal server error when viewing the public privacy policy #4184

Merged
merged 3 commits into from
Nov 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/4184.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Include flags to optionally add `m.login.terms` to the registration flow when consent tracking is enabled.
13 changes: 8 additions & 5 deletions synapse/rest/consent/consent_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,10 @@ def _async_render_GET(self, request):
userhmac = None
has_consented = False
public_version = username == ""
if not public_version or not self.hs.config.user_consent_at_registration:
Copy link
Member

Choose a reason for hiding this comment

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

Taking off the config flag causes a different set of errors. If the policy template doesn't correctly block usage of the missing variables, it will explode.

I'd highly recommend maintaining the flag.

Copy link
Member

Choose a reason for hiding this comment

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

I don't follow I'm afraid. That if statement doesn't block calls to _render_template?

Copy link
Member

Choose a reason for hiding this comment

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

Github helpfully wasn't showing me that userhmac is defined above here and therefore fine (I think).

The problem is more of one where undefined variables can make it through _render_template and cause problems when the template tries to use them. When I was testing the template stuff originally, the thing exploded if stuff like userhmac wasn't defined at template time if the template didn't have an !is_public if statement around it.

userhmac = parse_string(request, "h", required=True, encoding=None)
if not public_version:
userhmac_bytes = parse_string(request, "h", required=True, encoding=None)

self._check_hash(username, userhmac)
self._check_hash(username, userhmac_bytes)

if username.startswith('@'):
qualified_user_id = username
Expand All @@ -155,15 +155,18 @@ def _async_render_GET(self, request):
u = yield self.store.get_user_by_id(qualified_user_id)
if u is None:
raise NotFoundError("Unknown user")

has_consented = u["consent_version"] == version
userhmac = userhmac_bytes.decode("ascii")

try:
self._render_template(
request, "%s.html" % (version,),
user=username,
userhmac=userhmac.decode('ascii'),
userhmac=userhmac,
version=version,
has_consented=has_consented, public_version=public_version,
has_consented=has_consented,
public_version=public_version,
)
except TemplateNotFound:
raise NotFoundError("Unknown policy version")
Expand Down
7 changes: 7 additions & 0 deletions tests/rest/client/test_consent.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ def make_homeserver(self, reactor, clock):
hs = self.setup_test_homeserver(config=config)
return hs

def test_render_public_consent(self):
"""You can observe the terms form without specifying a user"""
resource = consent_resource.ConsentResource(self.hs)
request, channel = self.make_request("GET", "/consent?v=1", shorthand=False)
render(request, resource, self.reactor)
self.assertEqual(channel.code, 200)

def test_accept_consent(self):
"""
A user can use the consent form to accept the terms.
Expand Down