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

Tests for user consent resource #4140

Merged
merged 10 commits into from
Nov 5, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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/4140.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Generating the user consent URI no longer fails on Python 3.
2 changes: 1 addition & 1 deletion synapse/rest/consent/consent_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ def _check_hash(self, userid, userhmac):
key=self._hmac_secret,
msg=userid.encode('utf-8'),
digestmod=sha256,
).hexdigest()
).hexdigest().encode('ascii')

if not compare_digest(want_mac, userhmac):
raise SynapseError(http_client.FORBIDDEN, "HMAC incorrect")
99 changes: 99 additions & 0 deletions tests/rest/client/test_consent.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# -*- coding: utf-8 -*-
# Copyright 2018 New Vector
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import os

from synapse.api.urls import ConsentURIBuilder
from synapse.rest.client.v1 import admin, login, room
from synapse.rest.consent import consent_resource

from tests import unittest
from tests.server import render


class RoomSearchTestCase(unittest.HomeserverTestCase):
turt2live marked this conversation as resolved.
Show resolved Hide resolved
servlets = [
admin.register_servlets,
room.register_servlets,
login.register_servlets,
]
user_id = True
hijack_auth = False

def make_homeserver(self, reactor, clock):

config = self.default_config()
config.user_consent_version = "1"
config.public_baseurl = ""
config.form_secret = "123abc"

# Make some temporary templates...
temp_consent_path = self.mktemp()
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to be nice and delete this directory when we tear down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, since trial's mktemp makes it in the _trial_temp dir, which is cleaned up automatically (or should be).

Copy link
Member

Choose a reason for hiding this comment

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

Cunning!!!

os.mkdir(temp_consent_path)
os.mkdir(os.path.join(temp_consent_path, 'en'))
config.user_consent_template_dir = os.path.abspath(temp_consent_path)

with open(os.path.join(temp_consent_path, "en/1.html"), 'w') as f:
f.write("{{version}}")

with open(os.path.join(temp_consent_path, "en/success.html"), 'w') as f:
f.write("yay!")

hs = self.setup_test_homeserver(config=config)
return hs

def test_accept_consent(self):
"""
A user can use the consent form to accept the terms.
"""
uri_builder = ConsentURIBuilder(self.hs.config)
store = self.hs.get_datastore()
resource = consent_resource.ConsentResource(self.hs)

# Register a user
user_id = self.register_user("user", "pass")
access_token = self.login("user", "pass")

# They haven't consented, so they'll have a consent version of None
user_data = self.get_success(store.get_user_by_id(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.

I'd prefer it if we're doing an API test case to do the checks against the API, rather than against the data store. It a) makes it feel cleaner due to working at one level, and b) means that if we change the storage APIs we don't randomly break tests that aren't actually testing the storage APIs.

OTOH, I don't know much about the consent API, so if there isn't a nice way of doing this then that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, done

Copy link
Member

Choose a reason for hiding this comment

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

Do we even need to check the datastore at this point then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, fixed

self.assertIs(user_data["consent_version"], None)

# Fetch the consent page, to get the consent version
consent_uri = (
uri_builder.build_user_consent_uri(user_id).replace("_matrix/", "")
+ "&u=user"
)
request, channel = self.make_request(
"GET", consent_uri, access_token=access_token, shorthand=False
)
render(request, resource, self.reactor)
self.assertEqual(channel.code, 200)

# Get the version from the body
version = channel.result["body"].decode('ascii')

# POST to the consent page, saying we've agreed
request, channel = self.make_request(
"POST",
consent_uri + "&v=" + version,
access_token=access_token,
shorthand=False,
)
render(request, resource, self.reactor)
self.assertEqual(channel.code, 200)

# Now we've consented!
user_data = self.get_success(store.get_user_by_id(user_id))
self.assertEqual(user_data["consent_version"], "1")
8 changes: 5 additions & 3 deletions tests/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ def info(self, *args, **kwargs):
return FakeLogger()


def make_request(method, path, content=b"", access_token=None, request=SynapseRequest):
def make_request(
method, path, content=b"", access_token=None, request=SynapseRequest, shorthand=True
):
Copy link
Member

Choose a reason for hiding this comment

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

Can haz docs for shorthand pls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiine

"""
Make a web request using the given method and path, feed it the
content, and return the Request and the Channel underneath.
Expand All @@ -115,8 +117,8 @@ def make_request(method, path, content=b"", access_token=None, request=SynapseRe
if not isinstance(path, bytes):
path = path.encode('ascii')

# Decorate it to be the full path
if not path.startswith(b"/_matrix"):
# Decorate it to be the full path, if we're using shorthand
if shorthand and not path.startswith(b"/_matrix"):
path = b"/_matrix/client/r0/" + path
path = path.replace(b"//", b"/")

Expand Down
10 changes: 8 additions & 2 deletions tests/unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,13 @@ def prepare(self, reactor, clock, homeserver):
"""

def make_request(
self, method, path, content=b"", access_token=None, request=SynapseRequest
self,
method,
path,
content=b"",
access_token=None,
request=SynapseRequest,
shorthand=True,
Copy link
Member

Choose a reason for hiding this comment

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

Can haz docs for shorthand pls?

):
"""
Create a SynapseRequest at the path using the method and containing the
Expand All @@ -277,7 +283,7 @@ def make_request(
if isinstance(content, dict):
content = json.dumps(content).encode('utf8')

return make_request(method, path, content, access_token, request)
return make_request(method, path, content, access_token, request, shorthand)

def render(self, request):
"""
Expand Down