Merge lp:~elachuni/piston-mini-client/oauthlib-fix into lp:piston-mini-client

[Anthony Lenton]

Fixed two issues related to OAuth authentication with oauthlib.

This branch fixes two issues related to OAuth authentication with oauthlib:
 - oauthlib refuses to sign requests with a urlencoded body if the Content-Type header isn't set to 'application/x-www-form-urlencoded'. This check is case-sensitive, so the header can't be 'Content-type' or 'content-type'. Submitted this issue as https://github.com/idan/oauthlib/issues/121

 - oauthlib also refuses to sign requests with a Content-Type header set to 'application/x-www-form-urlencoded' if the body is None. Submitted this issue as https://github.com/idan/oauthlib/issues/122 thinking that the issue was if the body was blank (''), but it turns out that it *only* works with '' iff Content-Type is 'application/x-www-form-urlencoded', otherwise it needs to be None.

[Barry Warsaw (barry)]

<barry> one suggestion: use example.{com,net,org} urls instead of
        bathsandco.com since the formers are actually reserved for exactly
        that purpose (i.e. they will never resolve)
<achuni> true [16:31]
<achuni> will do, even though we're mocking out httplib and will never attempt
         to resolve
<achuni> httplib2
* barry nods

review: Approve

[lp:~elachuni/piston-mini-client/oauthlib-fix]

72. By Anthony Lenton on 2013-02-21

Use example.com for tests, and work with 0.3.0

=== modified file '.bzrignore'
--- .bzrignore	2012-06-13 13:25:54 +0000
+++ .bzrignore	2013-02-21 21:37:22 +0000
@@ -5,5 +5,6 @@
 .tox
 build/
 dist/
+piston_mini_client.egg-info/
 piston_mini_client/__pycache__/
 piston_mini_client/tests/__pycache__/

=== modified file 'piston_mini_client/__init__.py'
--- piston_mini_client/__init__.py	2013-02-01 14:25:10 +0000
+++ piston_mini_client/__init__.py	2013-02-21 21:37:22 +0000
@@ -371,7 +371,7 @@
         """
         headers = {}
         if content_type:
-            headers['Content-type'] = content_type
+            headers['Content-Type'] = content_type
         if self._extra_headers is not None:
             headers.update(self._extra_headers)
         if extra_headers is not None:

=== modified file 'piston_mini_client/auth.py'
--- piston_mini_client/auth.py	2012-11-21 15:37:34 +0000
+++ piston_mini_client/auth.py	2013-02-21 21:37:22 +0000
@@ -43,10 +43,15 @@
         # would treat the empty string as "no body", but python-oauthlib
         # requires None.
         if not body:
-            body = None
+            content_type = headers.get('Content-Type')
+            if content_type == 'application/x-www-form-urlencoded':
+                body = ''
+            else:
+                body = None
         # Import oauthlib here so that you don't need it if you're not going
         # to use it.  Plan B: move this out into a separate oauth module.
-        from oauthlib.oauth1 import Client, SIGNATURE_PLAINTEXT
+        from oauthlib.oauth1 import Client
+        from oauthlib.oauth1.rfc5849 import SIGNATURE_PLAINTEXT
         oauth_client = Client(self.consumer_key, self.consumer_secret,
                               self.token_key, self.token_secret,
                               signature_method=SIGNATURE_PLAINTEXT,

=== modified file 'piston_mini_client/tests/test_auth.py'
--- piston_mini_client/tests/test_auth.py	2013-02-01 14:25:10 +0000
+++ piston_mini_client/tests/test_auth.py	2013-02-21 21:37:22 +0000
@@ -2,8 +2,11 @@
 # Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Lesser General Public License version 3 (see the file LICENSE).
 
+from mock import patch
+from unittest import TestCase
+from piston_mini_client import PistonAPI, returns_json
+from piston_mini_client.validators import oauth_protected
 from piston_mini_client.auth import OAuthAuthorizer, BasicAuthorizer
-from unittest import TestCase
 
 
 class BasicAuthorizerTestCase(TestCase):
@@ -33,3 +36,58 @@
         auth.sign_request(url=url, method='GET', body='', headers=headers)
         self.assertTrue('Authorization' in headers)
         self.assertTrue(headers['Authorization'].startswith('OAuth '))
+
+    @patch('httplib2.Http.request')
+    def test_body_is_signed_if_urlencoded(self, mock_request):
+        formencoded = 'application/x-www-form-urlencoded'
+
+        class BathAPI(PistonAPI):
+            default_content_type = formencoded
+            default_service_root = 'http://example.com'
+
+            @returns_json
+            @oauth_protected
+            def soak(self):
+                return self._post('/soak/', data={'time': 900})
+
+        mock_request.return_value = {'status': '200'}, '"done"'
+
+        auth = OAuthAuthorizer('tkey', 'tsecret', 'ckey', 'csecret')
+        api = BathAPI(auth=auth)
+
+        response = api.soak()
+
+        self.assertEqual('done', response)
+        self.assertEqual(1, mock_request.call_count)
+        args, kwargs = mock_request.call_args
+        self.assertEqual(formencoded, kwargs['headers']['Content-Type'])
+        auth_header = kwargs['headers']['Authorization']
+        self.assertTrue(auth_header.startswith('OAuth '))
+
+    @patch('httplib2.Http.request')
+    def test_post_works_with_no_body(self, mock_request):
+        cases = [
+            'application/x-www-form-urlencoded',
+            'application/json',
+        ]
+
+        class ShowerAPI(PistonAPI):
+            default_service_root = 'http://example.com'
+
+            @returns_json
+            @oauth_protected
+            def soak(self):
+                return self._post('/noop/', data='')
+        auth = OAuthAuthorizer('tkey', 'tsecret', 'ckey', 'csecret')
+        for content_type in cases:
+            mock_request.return_value = {'status': '200'}, '"done"'
+            api = ShowerAPI(auth=auth)
+            api.default_content_type = content_type
+
+            response = api.soak()
+
+            self.assertEqual('done', response)
+            args, kwargs = mock_request.call_args
+            self.assertEqual(content_type, kwargs['headers']['Content-Type'])
+            auth_header = kwargs['headers']['Authorization']
+            self.assertTrue(auth_header.startswith('OAuth '))

=== modified file 'piston_mini_client/tests/test_failhandlers.py'
--- piston_mini_client/tests/test_failhandlers.py	2012-09-20 17:48:10 +0000
+++ piston_mini_client/tests/test_failhandlers.py	2013-02-21 21:37:22 +0000
@@ -234,7 +234,7 @@
         mock_request.return_value = self.response, self.body
         self.expected['method'] = 'POST'
         self.expected['request_body'] = '{"plants": "all"}'
-        self.expected['request_headers'] = {'Content-type': 'application/json'}
+        self.expected['request_headers'] = {'Content-Type': 'application/json'}
         self.expected['url'] = 'http://localhost:12345/grow'
 
         self.assertEqual(self.expected, self.api.grow())

=== modified file 'piston_mini_client/tests/test_log_to_file.py'
--- piston_mini_client/tests/test_log_to_file.py	2012-11-27 16:39:05 +0000
+++ piston_mini_client/tests/test_log_to_file.py	2013-02-21 21:37:22 +0000
@@ -41,7 +41,7 @@
         self.assertTrue(lines[0].endswith(
             'Request: POST http://test.info/api/1.0/poke/'))
         self.assertEqual(
-            set(['Content-type: application/json',
+            set(['Content-Type: application/json',
                  'location: ribs', '', '[1, 2, 3]']),
             set(lines[1:5]))
         self.assertTrue(lines[5].endswith('Response: 201'))

=== modified file 'piston_mini_client/tests/test_resource.py'
--- piston_mini_client/tests/test_resource.py	2012-07-30 17:09:26 +0000
+++ piston_mini_client/tests/test_resource.py	2013-02-21 21:37:22 +0000
@@ -97,7 +97,7 @@
         api = self.CoffeeAPI()
         api._post('/serve', data={'foo': 'bar'})
         kwargs = mock_request.call_args[1]
-        self.assertEqual(kwargs['headers']['Content-type'], 'application/json')
+        self.assertEqual(kwargs['headers']['Content-Type'], 'application/json')
         self.assertEqual(kwargs['method'], 'POST')
 
     @patch('httplib2.Http.request')
@@ -109,7 +109,7 @@
         api = self.CoffeeAPI()
         api._post('/serve', data=MyCoffeeRequest(strength='mild'))
         kwargs = mock_request.call_args[1]
-        self.assertEqual(kwargs['headers']['Content-type'], 'application/json')
+        self.assertEqual(kwargs['headers']['Content-Type'], 'application/json')
         self.assertEqual(kwargs['method'], 'POST')
 
     @patch('httplib2.Http.request')
@@ -119,7 +119,7 @@
         api._post('/serve', data={'foo': 'bar'},
                   content_type='application/x-www-form-urlencoded')
         kwargs = mock_request.call_args[1]
-        self.assertEqual(kwargs['headers']['Content-type'],
+        self.assertEqual(kwargs['headers']['Content-Type'],
                          'application/x-www-form-urlencoded')
         self.assertEqual(kwargs['method'], 'POST')
 
@@ -205,7 +205,7 @@
         api._post('/foo', scheme='https')
         mock_request.assert_called_with(
             'https://localhost:12345/foo',
-            body='null', headers={'Content-type': 'application/json'},
+            body='null', headers={'Content-Type': 'application/json'},
             method='POST')
 
     @patch('httplib2.Http.request')
@@ -223,7 +223,7 @@
         api = self.CoffeeAPI()
         api._put('/serve', data={'foo': 'bar'})
         kwargs = mock_request.call_args[1]
-        self.assertEqual(kwargs['headers']['Content-type'], 'application/json')
+        self.assertEqual(kwargs['headers']['Content-Type'], 'application/json')
         self.assertEqual(kwargs['method'], 'PUT')
 
     @patch('httplib2.Http.request')
@@ -235,7 +235,7 @@
         api = self.CoffeeAPI()
         api._put('/serve', data=MyCoffeeRequest(strength='mild'))
         kwargs = mock_request.call_args[1]
-        self.assertEqual(kwargs['headers']['Content-type'], 'application/json')
+        self.assertEqual(kwargs['headers']['Content-Type'], 'application/json')
         self.assertEqual(kwargs['method'], 'PUT')
 
     @patch('httplib2.Http.request')
@@ -254,7 +254,7 @@
         api._put('/foo', scheme='https')
         mock_request.assert_called_with(
             'https://localhost:12345/foo',
-            body='null', headers={'Content-type': 'application/json'},
+            body='null', headers={'Content-Type': 'application/json'},
             method='PUT')
 
     @patch('httplib2.Http.request')
@@ -265,7 +265,7 @@
                  content_type='application/x-www-form-urlencoded')
         kwargs = mock_request.call_args[1]
         self.assertEqual(kwargs['body'], 'foo=bar')
-        self.assertEqual(kwargs['headers']['Content-type'],
+        self.assertEqual(kwargs['headers']['Content-Type'],
                          'application/x-www-form-urlencoded')
         self.assertEqual(kwargs['method'], 'PUT')
 
@@ -283,7 +283,7 @@
         mock_request.assert_called_with(
             'http://localhost:12345/foo',
             body='', headers=expected_headers, method='DELETE')
-        expected_headers['Content-type'] = 'application/json'
+        expected_headers['Content-Type'] = 'application/json'
         api._post('/foo')
         mock_request.assert_called_with(
             'http://localhost:12345/foo',
@@ -306,7 +306,7 @@
         mock_request.assert_called_with(
             'http://localhost:12345/foo',
             body='', headers=expected_headers, method='DELETE')
-        expected_headers['Content-type'] = 'application/json'
+        expected_headers['Content-Type'] = 'application/json'
         api._post('/foo', extra_headers={'X-Foo': 'bar'})
         mock_request.assert_called_with(
             'http://localhost:12345/foo',
@@ -330,13 +330,13 @@
         api._post('/foo', data=[])
         mock_request.assert_called_with(
             'http://localhost:12345/foo',
-            body=expected, headers={'Content-type': 'application/json'},
+            body=expected, headers={'Content-Type': 'application/json'},
             method='POST')
 
         api._put('/foo', data=None)
         mock_request.assert_called_with(
             'http://localhost:12345/foo',
-            body=expected, headers={'Content-type': 'application/json'},
+            body=expected, headers={'Content-Type': 'application/json'},
             method='PUT')
 
     @patch('httplib2.Http.request')