From 59a95450e523865ce8b2ffe78c9bdb29f6204f22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 18 May 2017 15:30:33 +0200 Subject: [PATCH] Updated Eve, Flask, and Werkzeug. Adjusted code to make Pillar work again. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eve : 0.6.3 → 0.7.3 Flask : 0.10.1 → 0.12.2 Werkzeug: 0.11.10 → 0.11.15 Also updated some secondary requirements. --- pillar/__init__.py | 55 +++++++++++++++---- pillar/api/eve_settings.py | 2 +- pillar/api/nodes/custom/comment.py | 15 +++--- pillar/cli.py | 12 ++--- pillar/tests/__init__.py | 2 +- requirements.txt | 16 +++--- tests/test_api/test_auth.py | 64 +++++++++-------------- tests/test_api/test_nodes.py | 8 ++- tests/test_api/test_project_management.py | 27 ++++------ 9 files changed, 104 insertions(+), 97 deletions(-) diff --git a/pillar/__init__.py b/pillar/__init__.py index 1bf0e551..426fdb21 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -1,6 +1,7 @@ """Pillar server.""" import collections +import contextlib import copy import json import logging @@ -519,28 +520,32 @@ class PillarServer(Eve): """Workaround for Eve issue https://github.com/nicolaiarocci/eve/issues/810""" from eve.methods.post import post_internal - with self.test_request_context(method='POST', path='%s/%s' % (self.api_prefix, resource)): - return post_internal(resource, payl=payl, skip_validation=skip_validation) + url = self.config['URLS'][resource] + path = '%s/%s' % (self.api_prefix, url) + with self.__fake_request_url_rule('POST', path): + return post_internal(resource, payl=payl, skip_validation=skip_validation)[:4] def put_internal(self, resource, payload=None, concurrency_check=False, skip_validation=False, **lookup): """Workaround for Eve issue https://github.com/nicolaiarocci/eve/issues/810""" from eve.methods.put import put_internal - path = '%s/%s/%s' % (self.api_prefix, resource, lookup['_id']) - with self.test_request_context(method='PUT', path=path): + url = self.config['URLS'][resource] + path = '%s/%s/%s' % (self.api_prefix, url, lookup['_id']) + with self.__fake_request_url_rule('PUT', path): return put_internal(resource, payload=payload, concurrency_check=concurrency_check, - skip_validation=skip_validation, **lookup) + skip_validation=skip_validation, **lookup)[:4] def patch_internal(self, resource, payload=None, concurrency_check=False, skip_validation=False, **lookup): """Workaround for Eve issue https://github.com/nicolaiarocci/eve/issues/810""" from eve.methods.patch import patch_internal - path = '%s/%s/%s' % (self.api_prefix, resource, lookup['_id']) - with self.test_request_context(method='PATCH', path=path): + url = self.config['URLS'][resource] + path = '%s/%s/%s' % (self.api_prefix, url, lookup['_id']) + with self.__fake_request_url_rule('PATCH', path): return patch_internal(resource, payload=payload, concurrency_check=concurrency_check, - skip_validation=skip_validation, **lookup) + skip_validation=skip_validation, **lookup)[:4] def _list_routes(self): from pprint import pprint @@ -558,11 +563,15 @@ class PillarServer(Eve): # and rules that require parameters if "GET" in rule.methods and has_no_empty_params(rule): url = url_for(rule.endpoint, **(rule.defaults or {})) - links.append((url, rule.endpoint)) + links.append((url, rule.endpoint, rule.methods)) + if "PATCH" in rule.methods: + args = {arg: arg for arg in rule.arguments} + url = url_for(rule.endpoint, **args) + links.append((url, rule.endpoint, rule.methods)) - links.sort(key=lambda t: len(t[0]) + 100 * ('/api/' in t[0])) + links.sort(key=lambda t: (('/api/' in t[0]), len(t[0]))) - pprint(links) + pprint(links, width=300) def db(self, collection_name: str = None) \ -> typing.Union[pymongo.collection.Collection, pymongo.database.Database]: @@ -583,3 +592,27 @@ class PillarServer(Eve): return jinja2.Markup(''.join(ext.sidebar_links(project) for ext in self.pillar_extensions.values())) + + @contextlib.contextmanager + def __fake_request_url_rule(self, method: str, url_path: str): + """Tries to force-set the request URL rule. + + This is required by Eve (since 0.70) to be able to construct a + Location HTTP header that points to the resource item. + + See post_internal, put_internal and patch_internal. + """ + + import werkzeug.exceptions as wz_exceptions + + with self.test_request_context(method=method, path=url_path) as ctx: + try: + rule, _ = ctx.url_adapter.match(url_path, method=method, return_rule=True) + except (wz_exceptions.MethodNotAllowed, wz_exceptions.NotFound): + # We're POSTing things that we haven't told Eve are POSTable. Try again using the + # GET method. + rule, _ = ctx.url_adapter.match(url_path, method='GET', return_rule=True) + current_request = request._get_current_object() + current_request.url_rule = rule + + yield ctx diff --git a/pillar/api/eve_settings.py b/pillar/api/eve_settings.py index 8d46630e..6b9198bf 100644 --- a/pillar/api/eve_settings.py +++ b/pillar/api/eve_settings.py @@ -718,7 +718,7 @@ users = { 'cache_expires': 10, 'resource_methods': ['GET'], - 'item_methods': ['GET', 'PUT', 'PATCH'], + 'item_methods': ['GET', 'PUT'], 'public_item_methods': ['GET'], 'schema': users_schema diff --git a/pillar/api/nodes/custom/comment.py b/pillar/api/nodes/custom/comment.py index 5d0a708a..b4897883 100644 --- a/pillar/api/nodes/custom/comment.py +++ b/pillar/api/nodes/custom/comment.py @@ -2,7 +2,6 @@ import logging -from eve.methods.patch import patch_internal from flask import current_app import werkzeug.exceptions as wz_exceptions @@ -145,13 +144,13 @@ def edit_comment(user_id, node_id, patch): raise wz_exceptions.Forbidden('You can only edit your own comments.') # Use Eve to PATCH this node, as that also updates the etag. - r, _, _, status = patch_internal('nodes', - {'properties.content': patch['content'], - 'project': node['project'], - 'user': node['user'], - 'node_type': node['node_type']}, - concurrency_check=False, - _id=node_id) + r, _, _, status = current_app.patch_internal('nodes', + {'properties.content': patch['content'], + 'project': node['project'], + 'user': node['user'], + 'node_type': node['node_type']}, + concurrency_check=False, + _id=node_id) if status != 200: log.error('Error %i editing comment %s for user %s: %s', status, node_id, user_id, r) diff --git a/pillar/cli.py b/pillar/cli.py index 40c8821f..e8efbcfa 100644 --- a/pillar/cli.py +++ b/pillar/cli.py @@ -7,8 +7,6 @@ import copy import logging from bson.objectid import ObjectId, InvalidId -from eve.methods.put import put_internal -from eve.methods.post import post_internal from flask import current_app from flask_script import Manager @@ -565,7 +563,7 @@ def replace_pillar_node_type_schemas(proj_url=None, all_projects=False): # Use Eve to PUT, so we have schema checking. db_proj = remove_private_keys(project) - r, _, _, status = put_internal('projects', db_proj, _id=project['_id']) + r, _, _, status = current_app.put_internal('projects', db_proj, _id=project['_id']) if status != 200: log.error('Error %i storing altered project %s %s', status, project['_id'], r) raise SystemExit('Error storing project, see log.') @@ -686,7 +684,7 @@ def upgrade_attachment_schema(proj_url=None, all_projects=False): # Use Eve to PUT, so we have schema checking. db_proj = remove_private_keys(project) - r, _, _, status = put_internal('projects', db_proj, _id=project['_id']) + r, _, _, status = current_app.put_internal('projects', db_proj, _id=project['_id']) if status != 200: log.error('Error %i storing altered project %s %s', status, project['_id'], r) raise SystemExit('Error storing project, see log.') @@ -716,7 +714,7 @@ def upgrade_attachment_schema(proj_url=None, all_projects=False): # Use Eve to PUT, so we have schema checking. db_node = remove_private_keys(node) - r, _, _, status = put_internal('nodes', db_node, _id=node['_id']) + r, _, _, status = current_app.put_internal('nodes', db_node, _id=node['_id']) if status != 200: log.error('Error %i storing altered node %s %s', status, node['_id'], r) raise SystemExit('Error storing node; see log.') @@ -760,7 +758,7 @@ def create_blog(proj_url): replace_existing=False) proj_id = proj['_id'] - r, _, _, status = put_internal('projects', remove_private_keys(proj), _id=proj_id) + r, _, _, status = current_app.put_internal('projects', remove_private_keys(proj), _id=proj_id) if status != 200: log.error('Error %i storing altered project %s %s', status, proj_id, r) return 4 @@ -777,7 +775,7 @@ def create_blog(proj_url): 'properties': {}, 'project': proj_id, } - r, _, _, status = post_internal('nodes', blog) + r, _, _, status = current_app.post_internal('nodes', blog) if status != 201: log.error('Error %i storing blog node: %s', status, r) return 4 diff --git a/pillar/tests/__init__.py b/pillar/tests/__init__.py index 0eeab9b2..84be00e0 100644 --- a/pillar/tests/__init__.py +++ b/pillar/tests/__init__.py @@ -410,7 +410,7 @@ class AbstractPillarTest(TestMinimal): headers['Content-Type'] = 'application/json' if etag is not None: - if method == 'PUT': + if method in {'PUT', 'PATCH', 'DELETE'}: headers['If-Match'] = etag elif method == 'GET': headers['If-None-Match'] = etag diff --git a/requirements.txt b/requirements.txt index 6b264437..334b15ff 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,11 +7,9 @@ bcrypt==3.1.3 blinker==1.4 bugsnag==2.3.1 bleach==1.4.3 -Cerberus==0.9.2 commonmark==0.7.2 -Eve==0.6.3 -Events==0.2.1 -Flask==0.10.1 +Eve==0.7.3 +Flask==0.12.2 Flask-Cache==0.13.1 Flask-Script==2.0.5 Flask-Login==0.3.2 @@ -26,27 +24,29 @@ Pillow==2.8.1 pycrypto==2.6.1 python-dateutil==2.5.3 redis==2.10.5 -simplejson==3.8.2 WebOb==1.5.0 wheel==0.29.0 zencoder==0.6.5 # Secondary requirements Flask-PyMongo==0.4.1 -Jinja2==2.8 -Werkzeug==0.11.10 CommonMark==0.7.2 +cerberus==0.9.2 +events==0.2.2 future==0.15.2 html5lib==0.9999999 googleapis-common-protos==1.1.0 itsdangerous==0.24 +jinja2==2.9.6 oauth2client==2.0.2 oauthlib==2.0.1 protobuf==3.0.0b2.post2 protorpc==0.11.1 pyasn1-modules==0.0.8 -pymongo==3.2.2 +pymongo==3.4.0 requests-oauthlib==0.7.0 rsa==3.4.2 +simplejson==3.10.0 six==1.10.0 WTForms==2.1 +werkzeug==0.11.15 diff --git a/tests/test_api/test_auth.py b/tests/test_api/test_auth.py index 0e1abb97..f8328698 100644 --- a/tests/test_api/test_auth.py +++ b/tests/test_api/test_auth.py @@ -127,54 +127,40 @@ class AuthenticationTests(AbstractPillarTest): from pillar.api.utils import authentication as auth from pillar.api.utils import PillarJSONEncoder, remove_private_keys - user_id = self.create_user(roles=['subscriber']) + user_id = self.create_user(roles=['subscriber'], token='token') - now = datetime.datetime.now(tz_util.utc) - future = now + datetime.timedelta(days=1) - - with self.app.test_request_context(): - auth.store_token(user_id, 'nonexpired-main', future, None) - - with self.app.test_request_context( - headers={'Authorization': self.make_header('nonexpired-main')}): - self.assertTrue(auth.validate_token()) - - users = self.app.data.driver.db['users'] - db_user = users.find_one(user_id) + def fetch_user(): + with self.app.test_request_context(): + users_coll = self.app.db('users') + return users_coll.find_one(user_id) + db_user = fetch_user() updated_fields = remove_private_keys(db_user) updated_fields['roles'] = ['admin', 'subscriber', 'demo'] # Try to elevate our roles. # POSTing updated info to a specific user URL is not allowed by Eve. - resp = self.client.post('/api/users/%s' % user_id, - data=json.dumps(updated_fields, cls=PillarJSONEncoder), - headers={'Authorization': self.make_header('nonexpired-main'), - 'Content-Type': 'application/json'}) - self.assertEqual(405, resp.status_code) + self.post('/api/users/%s' % user_id, + json=updated_fields, + auth_token='token', + expected_status=405) - # PUT and PATCH should not be allowed. - resp = self.client.put('/api/users/%s' % user_id, - data=json.dumps(updated_fields, cls=PillarJSONEncoder), - headers={'Authorization': self.make_header('nonexpired-main'), - 'Content-Type': 'application/json'}) - self.assertEqual(403, resp.status_code) + # PUT is allowed, but shouldn't change roles. + self.put('/api/users/%s' % user_id, + json=updated_fields, + auth_token='token', + etag=db_user['_etag']) + db_user = fetch_user() + self.assertEqual(['subscriber'], db_user['roles']) + # PATCH should not be allowed. updated_fields = {'roles': ['admin', 'subscriber', 'demo']} - resp = self.client.patch('/api/users/%s' % user_id, - data=json.dumps(updated_fields, cls=PillarJSONEncoder), - headers={'Authorization': self.make_header('nonexpired-main'), - 'Content-Type': 'application/json'}) - self.assertEqual(403, resp.status_code) - - # After all of this, the roles should be the same. - with self.app.test_request_context( - headers={'Authorization': self.make_header('nonexpired-main')}): - self.assertTrue(auth.validate_token()) - - users = self.app.data.driver.db['users'] - db_user = users.find_one(user_id) - - self.assertEqual(['subscriber'], db_user['roles']) + self.patch('/api/users/%s' % user_id, + json=updated_fields, + auth_token='token', + etag=db_user['_etag'], + expected_status=405) + db_user = fetch_user() + self.assertEqual(['subscriber'], db_user['roles']) def test_token_expiry(self): """Expired tokens should be deleted from the database.""" diff --git a/tests/test_api/test_nodes.py b/tests/test_api/test_nodes.py index 26f5f063..f71573d7 100644 --- a/tests/test_api/test_nodes.py +++ b/tests/test_api/test_nodes.py @@ -2,8 +2,6 @@ import json import pillar.tests.common_test_data as ctd from bson import ObjectId -from eve.methods.post import post_internal -from eve.methods.put import put_internal from flask import g from mock import mock from pillar.tests import AbstractPillarTest @@ -47,7 +45,7 @@ class NodeContentTypeTest(AbstractPillarTest): nodes = self.app.data.driver.db['nodes'] # Create the node. - r, _, _, status = post_internal('nodes', node_doc) + r, _, _, status = self.app.post_internal('nodes', node_doc) self.assertEqual(status, 201, r) node_id = r['_id'] @@ -56,12 +54,12 @@ class NodeContentTypeTest(AbstractPillarTest): self.assertNotIn('content_type', db_node['properties']) # PUT it again, without a file -- should be blocked. - self.assertRaises(UnprocessableEntity, put_internal, 'nodes', node_doc, + self.assertRaises(UnprocessableEntity, self.app.put_internal, 'nodes', node_doc, _id=node_id) # PUT it with a file. node_doc['properties']['file'] = str(file_id) - r, _, _, status = put_internal('nodes', node_doc, _id=node_id) + r, _, _, status = self.app.put_internal('nodes', node_doc, _id=node_id) self.assertEqual(status, 200, r) # Get from database to test the final node. diff --git a/tests/test_api/test_project_management.py b/tests/test_api/test_project_management.py index 09ba337d..04ada87a 100644 --- a/tests/test_api/test_project_management.py +++ b/tests/test_api/test_project_management.py @@ -164,19 +164,17 @@ class ProjectEditTest(AbstractProjectTest): project_info = self._create_user_and_project(['subscriber']) project_url = '/api/projects/%(_id)s' % project_info - resp = self.client.get(project_url, - headers={'Authorization': self.make_header('token')}) - project = json.loads(resp.data.decode('utf-8')) + project = self.get(project_url, auth_token='token').json() # Create another user we can try and assign the project to. other_user_id = 'f00dd00df00dd00df00dd00d' self._create_user_with_token(['subscriber'], 'other-token', user_id=other_user_id) # Unauthenticated should be forbidden - resp = self.client.put('/api/projects/%s' % project['_id'], - data=dumps(remove_private_keys(project)), - headers={'Content-Type': 'application/json'}) - self.assertEqual(403, resp.status_code) + self.put('/api/projects/%s' % project['_id'], + json=remove_private_keys(project), + etag=project['_etag'], + expected_status=403) # Regular user should be able to PUT, but only be able to edit certain fields. put_project = remove_private_keys(project) @@ -191,20 +189,15 @@ class ProjectEditTest(AbstractProjectTest): # Try making the project public. This should update is_private as well. put_project['permissions']['world'] = ['GET'] - - resp = self.client.put(project_url, - data=dumps(put_project), - headers={'Authorization': self.make_header('token'), - 'Content-Type': 'application/json', - 'If-Match': project['_etag']}) - self.assertEqual(200, resp.status_code, resp.data) + self.put(project_url, + json=put_project, + auth_token='token', + etag=project['_etag']) # Re-fetch from database to see which fields actually made it there. # equal to put_project -> changed in DB # equal to project -> not changed in DB - resp = self.client.get(project_url, - headers={'Authorization': self.make_header('token')}) - db_proj = json.loads(resp.data) + db_proj = self.get(project_url, auth_token='token').json() self.assertEqual(project['url'], db_proj['url']) self.assertEqual(put_project['description'], db_proj['description']) self.assertEqual(put_project['name'], db_proj['name'])