Merge lp:~allenap/maas/version-without-auth--bug-1583715--1.9 into lp:maas/1.9

[Gavin Panella]

Backport r5039 from lp:maas. Ensure that restricted resources also perform meaningful authentication of clients.

[Gavin Panella (allenap)]

Backport; self-reviewing.

review: Approve

=== modified file 'src/maasserver/api/support.py'
--- src/maasserver/api/support.py	2016-04-13 16:34:23 +0000
+++ src/maasserver/api/support.py	2016-05-26 14:12:20 +0000
@@ -25,6 +25,7 @@
 from django.http import Http404
 from maasserver.api.doc import get_api_description_hash
 from maasserver.exceptions import MAASAPIBadRequest
+from piston.authentication import NoAuthentication
 from piston.emitters import Emitter
 from piston.handler import (
     AnonymousBaseHandler,
@@ -66,10 +67,35 @@
         else:
             raise
 
+    @property
+    def is_authentication_attempted(self):
+        """Will use of this resource attempt to authenticate the client?
+
+        For example, `None`, ``[]``, and :class:`NoAuthentication` are all
+        examples of authentication handlers that do *not* count.
+        """
+        return len(self.authentication) != 0 and not any(
+            isinstance(auth, NoAuthentication) for auth in self.authentication)
+
 
 class RestrictedResource(OperationsResource):
     """A resource that's restricted to active users."""
 
+    def __init__(self, handler, authentication):
+        """A value for `authentication` MUST be provided AND be meaningful.
+
+        This prevents the situation where none of the following are restricted
+        at all::
+
+          handler = RestrictedResource(HandlerClass)
+          handler = RestrictedResource(HandlerClass, authentication=None)
+          handler = RestrictedResource(HandlerClass, authentication=[])
+
+        """
+        super(RestrictedResource, self).__init__(handler, authentication)
+        if not self.is_authentication_attempted:
+            raise AssertionError("Authentication must be attempted.")
+
     def authenticate(self, request, rm):
         actor, anonymous = super(
             RestrictedResource, self).authenticate(request, rm)

=== modified file 'src/maasserver/api/tests/test_support.py'
--- src/maasserver/api/tests/test_support.py	2015-04-21 22:51:42 +0000
+++ src/maasserver/api/tests/test_support.py	2016-05-26 14:12:20 +0000
@@ -23,7 +23,10 @@
 from maasserver.api.doc import get_api_description_hash
 from maasserver.api.support import (
     admin_method,
+    AdminRestrictedResource,
     OperationsHandlerMixin,
+    OperationsResource,
+    RestrictedResource,
 )
 from maasserver.models.config import (
     Config,
@@ -37,7 +40,17 @@
     Mock,
     sentinel,
 )
-from testtools.matchers import Equals
+from piston.authentication import NoAuthentication
+from testtools.matchers import (
+    Equals,
+    Is,
+)
+
+
+class StubHandler:
+    """A stub handler class that breaks when called."""
+    def __call__(self, request):
+        raise AssertionError("Do not call the stub handler.")
 
 
 class TestOperationsResource(APITestCase):
@@ -76,6 +89,59 @@
             response["X-MAAS-API-Hash"],
             Equals(get_api_description_hash()))
 
+    def test_authenticated_is_False_when_no_authentication_provided(self):
+        resource = OperationsResource(StubHandler)
+        self.assertThat(resource.is_authentication_attempted, Is(False))
+
+    def test_authenticated_is_False_when_authentication_is_empty(self):
+        resource = OperationsResource(StubHandler, authentication=[])
+        self.assertThat(resource.is_authentication_attempted, Is(False))
+
+    def test_authenticated_is_False_when_authentication_is_NoAuthn(self):
+        resource = OperationsResource(
+            StubHandler, authentication=NoAuthentication())
+        self.assertThat(resource.is_authentication_attempted, Is(False))
+
+    def test_authenticated_is_True_when_authentication_is_provided(self):
+        resource = OperationsResource(
+            StubHandler, authentication=sentinel.authentication)
+        self.assertThat(resource.is_authentication_attempted, Is(True))
+
+
+class TestRestrictedResources(APITestCase):
+    """Tests for `RestrictedResource` and `AdminRestrictedResource`."""
+
+    scenarios = (
+        ("user", dict(resource_type=RestrictedResource)),
+        ("admin", dict(resource_type=AdminRestrictedResource)),
+    )
+
+    def test_authentication_must_not_be_None(self):
+        error = self.assertRaises(
+            AssertionError, self.resource_type, StubHandler,
+            authentication=None)
+        self.assertThat(unicode(error), Equals(
+            "Authentication must be attempted."))
+
+    def test_authentication_must_be_non_empty(self):
+        error = self.assertRaises(
+            AssertionError, self.resource_type, StubHandler,
+            authentication=[])
+        self.assertThat(unicode(error), Equals(
+            "Authentication must be attempted."))
+
+    def test_authentication_must_be_meaningful(self):
+        error = self.assertRaises(
+            AssertionError, self.resource_type, StubHandler,
+            authentication=NoAuthentication())
+        self.assertThat(unicode(error), Equals(
+            "Authentication must be attempted."))
+
+    def test_authentication_is_okay(self):
+        resource = self.resource_type(
+            StubHandler, authentication=sentinel.authentication)
+        self.assertThat(resource.is_authentication_attempted, Is(True))
+
 
 class TestAdminMethodDecorator(APITestCase):
 

=== modified file 'src/maasserver/api/tests/test_version.py'
--- src/maasserver/api/tests/test_version.py	2015-06-24 14:20:57 +0000
+++ src/maasserver/api/tests/test_version.py	2016-05-26 14:12:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test maasserver API version."""
@@ -18,19 +18,31 @@
 import httplib
 import json
 
+from django.contrib.auth.models import AnonymousUser
 from django.core.urlresolvers import reverse
 from maasserver.api.version import API_CAPABILITIES_LIST
+from maasserver.testing.api import MultipleUsersScenarios
+from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils import version as version_module
 
 
-class TestFindingResources(MAASServerTestCase):
-    """Tests for /version/ API."""
+class TestVersionAPIBasics(MAASServerTestCase):
+    """Basic tests for /version/ API."""
 
     def test_handler_path(self):
         self.assertEqual(
             '/api/1.0/version/', reverse('version_handler'))
 
+
+class TestVersionAPI(MultipleUsersScenarios, MAASServerTestCase):
+    """Tests for /version/ API."""
+
+    scenarios = [
+        ('anon', dict(userfactory=AnonymousUser)),
+        ('user', dict(userfactory=factory.make_User)),
+    ]
+
     def test_GET_returns_details(self):
         mock_apt = self.patch(version_module, "get_version_from_apt")
         mock_apt.return_value = "1.8.0~alpha4+bzr356-0ubuntu1"

=== modified file 'src/maasserver/urls_api.py'
--- src/maasserver/urls_api.py	2015-12-07 14:25:30 +0000
+++ src/maasserver/urls_api.py	2016-05-26 14:12:20 +0000
@@ -132,6 +132,7 @@
 )
 from maasserver.api.support import (
     AdminRestrictedResource,
+    OperationsResource,
     RestrictedResource,
 )
 from maasserver.api.tags import (
@@ -219,7 +220,7 @@
     BootImagesHandler, authentication=api_auth)
 tag_handler = RestrictedResource(TagHandler, authentication=api_auth)
 tags_handler = RestrictedResource(TagsHandler, authentication=api_auth)
-version_handler = RestrictedResource(VersionHandler)
+version_handler = OperationsResource(VersionHandler)
 node_results_handler = RestrictedResource(
     NodeResultsHandler, authentication=api_auth)
 sshkey_handler = RestrictedResource(SSHKeyHandler, authentication=api_auth)