Merge lp:~canonical-isd-hackers/canonical-identity-provider/1013494 into lp:canonical-identity-provider/release

[Simon Davy]

Fix for #1013494, XSS vuln.

Fix for #1013494, XSS vuln.

[Ricardo Kirkner (ricardokirkner)]

LGTM. Thanks!

review: Approve

=== modified file 'identityprovider/api10/handlers.py'
--- identityprovider/api10/handlers.py	2012-01-15 15:52:38 +0000
+++ identityprovider/api10/handlers.py	2012-06-15 14:49:20 +0000
@@ -51,6 +51,11 @@
 from oauth_backend.models import Token
 
 
+def api_error(cls, msg):
+    """Ensure 4xx response content in plain text to avoid XSS"""
+    return cls(msg, content_type='text/plain')
+
+
 class LazrRestfulEmitter(Emitter):
     """JSON emitter, lazr.restful flavoured.
 
@@ -90,7 +95,7 @@
 
     def create(self, request):
         if not 'ws.op' in request.POST:
-            return HttpResponseBadRequest('No operation name given.')
+            return api_error(HttpResponseBadRequest, 'No operation name given.')
         return self.named_operation(request, request.POST)
 
     def named_operation(self, request, serialized):
@@ -98,7 +103,7 @@
         method = getattr(self, method_name, None)
         is_named_operation = getattr(method, 'is_named_operation', False)
         if not is_named_operation:
-            return HttpResponseBadRequest('No such operation: %s' %
+            return api_error(HttpResponseBadRequest, 'No such operation: %s' %
                 method_name)
         request.data = self.lazr_restful_deserialize(serialized)
         return method(request)
@@ -232,7 +237,7 @@
 
         account = Account.objects.get_by_email(email)
         if account is None or not account.can_reset_password:
-            return HttpResponseForbidden(
+            return api_error(HttpResponseForbidden,
                 "CanNotResetPasswordError: Can't reset password for this "
                 "account.")
 
@@ -255,7 +260,7 @@
                 email=data['email'], token=data['token'],
                 token_type=LoginTokenType.PASSWORDRECOVERY)
         except AuthToken.DoesNotExist:
-            return HttpResponseForbidden(
+            return api_error(HttpResponseForbidden,
                 "CanNotSetPasswordError: Invalid token.")
         if not token.requester:
             token.delete()

=== modified file 'identityprovider/tests/unit/test_handlers.py'
--- identityprovider/tests/unit/test_handlers.py	2012-01-17 16:16:01 +0000
+++ identityprovider/tests/unit/test_handlers.py	2012-06-15 14:49:20 +0000
@@ -1,6 +1,8 @@
 # encoding: utf-8
 import base64
 import json
+from unittest import TestCase
+
 from django.utils import simplejson
 from mock import patch
 from oauth.oauth import (
@@ -11,7 +13,10 @@
     )
 from lazr.restfulclient.errors import HTTPError
 
-from identityprovider.api10.handlers import AuthenticationHandler
+from identityprovider.api10.handlers import (
+    AuthenticationHandler,
+    LazrRestfulHandler,
+)
 from identityprovider.tests.utils import AnonAPITestCase, TestCaseWithFactory
 from identityprovider.models import (
     Account,
@@ -35,6 +40,21 @@
         self.environ = {'REMOTE_ADDR': '127.0.0.1'}
 
 
+class TestLazrRestfulHandler(TestCase):
+    handler = LazrRestfulHandler()
+
+    def test_named_operation_error_is_plaintext(self):
+        script = '<script>alert("boo");</script>'
+        response = self.handler.named_operation(None, {'ws.op': script})
+        self.assertEqual(response['content-type'], 'text/plain')
+
+    def test_create_error_is_plaintext(self):
+        request = MockRequest()
+        request.POST = {}
+        response = self.handler.create(request)
+        self.assertEqual(response['content-type'], 'text/plain')
+
+
 class RegistrationHandlerTestCase(AnonAPITestCase):
 
     def test_authtoken_without_requester_is_handled_properly(self):
@@ -53,6 +73,7 @@
                 email='[e-mail]')
         except HTTPError, e:
             self.assertEqual(e.response.status, 403)
+            self.assertEquals(e.response['content-type'], "text/plain")
             self.assertTrue(e.content.startswith("CanNotResetPasswordError:"))
 
     def test_reset_token_when_account_is_disabled(self):
@@ -66,6 +87,7 @@
                 email=email_address)
         except HTTPError, e:
             self.assertEqual(e.response.status, 403)
+            self.assertEquals(e.response['content-type'], "text/plain")
             self.assertTrue(e.content.startswith("CanNotResetPasswordError:"))
 
     def test_reset_password_when_not_existing_token_is_passed(self):
@@ -76,6 +98,7 @@
                 new_password='password')
         except HTTPError, e:
             self.assertEqual(e.response.status, 403)
+            self.assertEqual(e.response['content-type'], "text/plain")
             self.assertTrue(e.content.startswith(
                 "CanNotSetPasswordError: Invalid token."))