From e8ad08f5047c8aef437e3453d76607f176830ce5 Mon Sep 17 00:00:00 2001 From: Ben Hearsum Date: Tue, 24 May 2016 10:38:27 -0400 Subject: [PATCH 1/2] Unquote necessary url parts during routing. --- auslib/admin/base.py | 35 +++++++++++++++++++-- auslib/test/admin/views/test_permissions.py | 31 ++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/auslib/admin/base.py b/auslib/admin/base.py index 7fac0347de..a9d41b9ff6 100644 --- a/auslib/admin/base.py +++ b/auslib/admin/base.py @@ -1,6 +1,10 @@ +import urllib + from flask import Flask, request from flask_compress import Compress +from werkzeug.routing import BaseConverter, PathConverter + import logging log = logging.getLogger(__name__) @@ -37,6 +41,31 @@ def add_security_headers(response): Compress(app) + +# When running under uwsgi, paths will not get decoded before hitting the app. +# We need to handle this ourselves in certain fields, and adding converters +# for them is the best way to do this. +class UrlquotingConverter(BaseConverter): + def to_python(self, value): + return urllib.unquote(value) + + def to_url(self, value): + return urllib.quote(value) + + +class UrlquotingPathConverter(PathConverter): + def to_python(self, value): + value = super(UrlquotingPathConverter, self).to_python(value) + return urllib.unquote(value) + + def to_url(self, value): + value = super(UrlquotingPathConverter, self).to_url(value) + return urllib.quote(value) + + +app.url_map.converters["urlquote"] = UrlquotingConverter +app.url_map.converters["urlquotepath"] = UrlquotingPathConverter + # Endpoints required for the Balrog 2.0 UI. # In the Mozilla deployments of Balrog, both the the admin API (these endpoints) # and the static admin UI are hosted on the same domain. This API wsgi app is @@ -44,10 +73,10 @@ def add_security_headers(response): # these requests. app.add_url_rule("/csrf_token", view_func=CSRFView.as_view("csrf")) app.add_url_rule("/users", view_func=UsersView.as_view("users")) -app.add_url_rule("/users//permissions", view_func=PermissionsView.as_view("user_permissions")) -app.add_url_rule("/users//permissions/", view_func=SpecificPermissionView.as_view("specific_permission")) +app.add_url_rule("/users//permissions", view_func=PermissionsView.as_view("user_permissions")) +app.add_url_rule("/users//permissions/", view_func=SpecificPermissionView.as_view("specific_permission")) # Some permissions may start with a slash, and the converter won"t match them, so we need an extra rule to cope. -app.add_url_rule("/users//permissions//", view_func=SpecificPermissionView.as_view("specific_permission2")) +app.add_url_rule("/users//permissions//", view_func=SpecificPermissionView.as_view("specific_permission2")) app.add_url_rule("/rules", view_func=RulesAPIView.as_view("rules")) # Normal operations (get/update/delete) on rules can be done by id or alias... app.add_url_rule("/rules/", view_func=SingleRuleView.as_view("rule")) diff --git a/auslib/test/admin/views/test_permissions.py b/auslib/test/admin/views/test_permissions.py index 07115535f6..7b0362e7ac 100644 --- a/auslib/test/admin/views/test_permissions.py +++ b/auslib/test/admin/views/test_permissions.py @@ -36,6 +36,37 @@ def testPermissionPut(self): query = query.where(dbo.permissions.permission == 'admin') self.assertEqual(query.execute().fetchone(), ('admin', 'bob', None, 1)) + def testPermissionPutWithEmail(self): + ret = self._put('/users/bob@bobsworld.com/permissions/admin') + self.assertStatusCode(ret, 201) + self.assertEqual(ret.data, json.dumps(dict(new_data_version=1)), "Data: %s" % ret.data) + query = dbo.permissions.t.select() + query = query.where(dbo.permissions.username == 'bob@bobsworld.com') + query = query.where(dbo.permissions.permission == 'admin') + self.assertEqual(query.execute().fetchone(), ('admin', 'bob@bobsworld.com', None, 1)) + + # This test is meant to verify that the app properly unquotes URL parts + # as part of routing, because it is required when running under uwsgi. + # Unfortunately, Werkzeug's test Client will unquote URL parts before + # the app sees them, so this test doesn't actually verify that case... + def testPermissionPutWithQuotedEmail(self): + ret = self._put('/users/bob%40bobsworld.com/permissions/admin') + self.assertStatusCode(ret, 201) + self.assertEqual(ret.data, json.dumps(dict(new_data_version=1)), "Data: %s" % ret.data) + query = dbo.permissions.t.select() + query = query.where(dbo.permissions.username == 'bob@bobsworld.com') + query = query.where(dbo.permissions.permission == 'admin') + self.assertEqual(query.execute().fetchone(), ('admin', 'bob@bobsworld.com', None, 1)) + + def testPermissionPutWithQuotedUrl(self): + ret = self._put('/users/bob/permissions/%2frules') + self.assertStatusCode(ret, 201) + self.assertEqual(ret.data, json.dumps(dict(new_data_version=1)), "Data: %s" % ret.data) + query = dbo.permissions.t.select() + query = query.where(dbo.permissions.username == 'bob') + query = query.where(dbo.permissions.permission == '/rules') + self.assertEqual(query.execute().fetchone(), ('/rules', 'bob', None, 1)) + def testPermissionsPostWithHttpRemoteUser(self): ret = self._httpRemoteUserPost('/users/bill/permissions/admin', username="bob", data=dict(options="", data_version=1)) self.assertEqual(ret.status_code, 200, "Status Code: %d" % ret.status_code) From b905edaad8ad7b8797c19704540a98ce1d6c33df Mon Sep 17 00:00:00 2001 From: Ben Hearsum Date: Wed, 25 May 2016 10:56:32 -0400 Subject: [PATCH 2/2] Use middleware to unquote paths instead of dealing with it in routing. --- auslib/admin/base.py | 47 +++++++++++++++++--------------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/auslib/admin/base.py b/auslib/admin/base.py index a9d41b9ff6..5033303a65 100644 --- a/auslib/admin/base.py +++ b/auslib/admin/base.py @@ -3,8 +3,6 @@ from flask import Flask, request from flask_compress import Compress -from werkzeug.routing import BaseConverter, PathConverter - import logging log = logging.getLogger(__name__) @@ -24,6 +22,21 @@ create_dockerflow_endpoints(app) +# When running under uwsgi, paths will not get decoded before hitting the app. +# We need to handle this ourselves in certain fields, and adding converters +# for them is the best way to do this. +class UnquotingMiddleware(object): + def __init__(self, app): + self.app = app + + def __call__(self, environ, start_response): + environ["PATH_INFO"] = urllib.unquote(environ["PATH_INFO"]) + return self.app(environ, start_response) + + +app.wsgi_app = UnquotingMiddleware(app.wsgi_app) + + @app.errorhandler(500) def ise(error): log.error("Caught ISE 500 error.") @@ -42,30 +55,6 @@ def add_security_headers(response): Compress(app) -# When running under uwsgi, paths will not get decoded before hitting the app. -# We need to handle this ourselves in certain fields, and adding converters -# for them is the best way to do this. -class UrlquotingConverter(BaseConverter): - def to_python(self, value): - return urllib.unquote(value) - - def to_url(self, value): - return urllib.quote(value) - - -class UrlquotingPathConverter(PathConverter): - def to_python(self, value): - value = super(UrlquotingPathConverter, self).to_python(value) - return urllib.unquote(value) - - def to_url(self, value): - value = super(UrlquotingPathConverter, self).to_url(value) - return urllib.quote(value) - - -app.url_map.converters["urlquote"] = UrlquotingConverter -app.url_map.converters["urlquotepath"] = UrlquotingPathConverter - # Endpoints required for the Balrog 2.0 UI. # In the Mozilla deployments of Balrog, both the the admin API (these endpoints) # and the static admin UI are hosted on the same domain. This API wsgi app is @@ -73,10 +62,10 @@ def to_url(self, value): # these requests. app.add_url_rule("/csrf_token", view_func=CSRFView.as_view("csrf")) app.add_url_rule("/users", view_func=UsersView.as_view("users")) -app.add_url_rule("/users//permissions", view_func=PermissionsView.as_view("user_permissions")) -app.add_url_rule("/users//permissions/", view_func=SpecificPermissionView.as_view("specific_permission")) +app.add_url_rule("/users//permissions", view_func=PermissionsView.as_view("user_permissions")) +app.add_url_rule("/users//permissions/", view_func=SpecificPermissionView.as_view("specific_permission")) # Some permissions may start with a slash, and the converter won"t match them, so we need an extra rule to cope. -app.add_url_rule("/users//permissions//", view_func=SpecificPermissionView.as_view("specific_permission2")) +app.add_url_rule("/users//permissions//", view_func=SpecificPermissionView.as_view("specific_permission2")) app.add_url_rule("/rules", view_func=RulesAPIView.as_view("rules")) # Normal operations (get/update/delete) on rules can be done by id or alias... app.add_url_rule("/rules/", view_func=SingleRuleView.as_view("rule"))