launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/api_get_allocated into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/api_get_allocated/+merge/98578

make_oauth_header now lives in the factory, and extract_oauth_key now lives in maasserver/api.py
-- 
https://code.launchpad.net/~julian-edwards/maas/api_get_allocated/+merge/98578
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/api_get_allocated into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-03-19 10:25:32 +0000
+++ src/maasserver/api.py	2012-03-21 05:59:22 +0000
@@ -11,6 +11,7 @@
 __metaclass__ = type
 __all__ = [
     "api_doc",
+    "extract_oauth_key",
     "generate_api_doc",
     "AccountHandler",
     "AnonNodesHandler",
@@ -240,6 +241,22 @@
         return value
 
 
+def extract_oauth_key(auth_data):
+    """Extract the oauth key from auth data in HTTP header.
+
+    :param auth_data: {string} The HTTP Authorization header.
+
+    :return: The oauth key from the header, or None.
+    """
+    for entry in auth_data.split():
+        key_value = entry.split('=', 1)
+        if len(key_value) == 2:
+            key, value = key_value
+            if key == 'oauth_token':
+                return value.rstrip(',').strip('"')
+    return None
+
+
 NODE_FIELDS = (
     'system_id', 'hostname', ('macaddress_set', ('mac_address',)),
     'architecture', 'status')

=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2012-03-15 16:51:15 +0000
+++ src/maasserver/testing/factory.py	2012-03-21 05:59:22 +0000
@@ -15,6 +15,7 @@
 
 from io import BytesIO
 import random
+import time
 
 from django.contrib.auth.models import User
 from maasserver.models import (
@@ -95,6 +96,26 @@
 
         return FileStorage.objects.save_file(filename, BytesIO(data))
 
+    def make_oauth_header(self, **kwargs):
+        """Fake an OAuth authorization header.
+
+        This will use arbitrary values.  Pass as keyword arguments any
+        header items that you wish to override.
+        """
+        items = {
+            'realm': self.getRandomString(),
+            'oauth_nonce': random.randint(0, 99999),
+            'oauth_timestamp': time.time(),
+            'oauth_consumer_key': self.getRandomString(18),
+            'oauth_signature_method': 'PLAINTEXT',
+            'oauth_version': '1.0',
+            'oauth_token': self.getRandomString(18),
+            'oauth_signature': "%%26%s" % self.getRandomString(32),
+        }
+        items.update(kwargs)
+        return "OAuth " + ", ".join([
+            '%s="%s"' % (key, value) for key, value in items.items()])
+
 
 # Create factory singleton.
 factory = Factory()

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-03-19 17:39:39 +0000
+++ src/maasserver/tests/test_api.py	2012-03-21 05:59:22 +0000
@@ -18,6 +18,7 @@
 import shutil
 
 from django.conf import settings
+from maasserver.api import extract_oauth_key
 from maasserver.models import (
     ARCHITECTURE_CHOICES,
     Config,
@@ -55,6 +56,18 @@
         return api_root + path
 
 
+class TestModuleHelpers(TestCase):
+
+    def test_extract_oauth_key_extracts_oauth_token_from_oauth_header(self):
+        token = factory.getRandomString(18)
+        self.assertEqual(
+            token,
+            extract_oauth_key(factory.make_oauth_header(oauth_token=token)))
+
+    def test_extract_oauth_key_returns_None_without_oauth_key(self):
+        self.assertIs(None, extract_oauth_key(''))
+
+
 class AnonymousEnlistmentAPITest(APIv10TestMixin, TestCase):
     # Nodes can be enlisted anonymously.
 

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2012-03-15 13:58:32 +0000
+++ src/metadataserver/api.py	2012-03-21 05:59:22 +0000
@@ -17,6 +17,7 @@
     ]
 
 from django.http import HttpResponse
+from maasserver.api import extract_oauth_key
 from maasserver.exceptions import (
     MAASAPINotFound,
     PermissionDenied,
@@ -37,23 +38,14 @@
     """Not a known node."""
 
 
-def extract_oauth_key(auth_data):
-    """Extract the oauth key from auth data in HTTP header."""
-    for entry in auth_data.split():
-        key_value = entry.split('=', 1)
-        if len(key_value) == 2:
-            key, value = key_value
-            if key == 'oauth_token':
-                return value.rstrip(',').strip('"')
-    raise Unauthorized("No oauth token found for metadata request.")
-
-
 def get_node_for_request(request):
     """Return the `Node` that `request` is authorized to query for."""
     auth_header = request.META.get('HTTP_AUTHORIZATION')
     if auth_header is None:
         raise Unauthorized("No authorization header received.")
     key = extract_oauth_key(auth_header)
+    if key is None:
+        raise Unauthorized("No oauth token found for metadata request.")
     try:
         return NodeKey.objects.get_node_for_key(key)
     except NodeKey.DoesNotExist:

=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2012-03-17 12:24:08 +0000
+++ src/metadataserver/tests/test_api.py	2012-03-21 05:59:22 +0000
@@ -13,8 +13,6 @@
 
 from collections import namedtuple
 import httplib
-from random import randint
-from time import time
 
 from maasserver.exceptions import Unauthorized
 from maasserver.testing.factory import factory
@@ -23,7 +21,6 @@
 from maastesting.testcase import TestCase
 from metadataserver.api import (
     check_version,
-    extract_oauth_key,
     get_node_for_request,
     make_list_response,
     make_text_response,
@@ -40,26 +37,6 @@
 class TestHelpers(TestCase):
     """Tests for the API helper functions."""
 
-    def make_oauth_header(self, **kwargs):
-        """Fake an OAuth authorization header.
-
-        This will use arbitrary values.  Pass as keyword arguments any
-        header items that you wish to override.
-        """
-        items = {
-            'realm': factory.getRandomString(),
-            'oauth_nonce': randint(0, 99999),
-            'oauth_timestamp': time(),
-            'oauth_consumer_key': factory.getRandomString(18),
-            'oauth_signature_method': 'PLAINTEXT',
-            'oauth_version': '1.0',
-            'oauth_token': factory.getRandomString(18),
-            'oauth_signature': "%%26%s" % factory.getRandomString(32),
-        }
-        items.update(kwargs)
-        return "OAuth " + ", ".join([
-            '%s="%s"' % (key, value) for key, value in items.items()])
-
     def fake_request(self, **kwargs):
         """Produce a cheap fake request, fresh from the sweat shop.
 
@@ -86,21 +63,13 @@
     def test_check_version_reports_unknown_version(self):
         self.assertRaises(UnknownMetadataVersion, check_version, '1.0')
 
-    def test_extract_oauth_key_extracts_oauth_token_from_oauth_header(self):
-        token = factory.getRandomString(18)
-        self.assertEqual(
-            token,
-            extract_oauth_key(self.make_oauth_header(oauth_token=token)))
-
-    def test_extract_oauth_key_rejects_auth_without_oauth_key(self):
-        self.assertRaises(Unauthorized, extract_oauth_key, '')
-
     @uses_rabbit_fixture
     def test_get_node_for_request_finds_node(self):
         node = factory.make_node()
         token = NodeKey.objects.get_token_for_node(node)
         request = self.fake_request(
-            HTTP_AUTHORIZATION=self.make_oauth_header(oauth_token=token.key))
+            HTTP_AUTHORIZATION=factory.make_oauth_header(
+                oauth_token=token.key))
         self.assertEqual(node, get_node_for_request(request))
 
     def test_get_node_for_request_reports_missing_auth_header(self):