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

[Gavin Panella]

Ensure that restricted resources also perform meaningful authentication of clients.

Also demonstrate that /api/2.0/versions/ is available anonymously.

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

=== modified file 'src/maasserver/api/support.py'
--- src/maasserver/api/support.py	2016-04-28 01:11:50 +0000
+++ src/maasserver/api/support.py	2016-05-20 15:20:50 +0000
@@ -16,6 +16,7 @@
 from django.http import Http404
 from maasserver.api.doc import get_api_description_hash
 from maasserver.exceptions import MAASAPIBadRequest
+from piston3.authentication import NoAuthentication
 from piston3.emitters import Emitter
 from piston3.handler import (
     AnonymousBaseHandler,
@@ -67,10 +68,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	2016-05-12 19:07:37 +0000
+++ src/maasserver/api/tests/test_support.py	2016-05-20 15:20:50 +0000
@@ -19,7 +19,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,
@@ -28,7 +31,17 @@
 from maasserver.testing.api import APITestCase
 from maasserver.testing.factory import factory
 from maastesting.testcase import MAASTestCase
-from testtools.matchers import Equals
+from piston3.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):
@@ -67,6 +80,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(str(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(str(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(str(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	2016-04-15 16:33:02 +0000
+++ src/maasserver/api/tests/test_version.py	2016-05-20 15:20:50 +0000
@@ -10,19 +10,31 @@
 import json
 
 from django.conf import settings
+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/2.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	2016-04-12 17:36:35 +0000
+++ src/maasserver/urls_api.py	2016-05-20 15:20:50 +0000
@@ -238,7 +238,7 @@
     InterfacesHandler, authentication=api_auth)
 tag_handler = OperationsResource(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)