Merge ~blake-rouse/maas:login-return-json into maas:master

[Blake Rouse]

Return JSON for login and logout when the HTTP includes application/json header.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b login-return-json lp:~blake-rouse/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6724/console
COMMIT: 21c758992b5895df44258a07e5d4f72ec971a6eb

review: Needs Fixing

52d63b2... by Blake Rouse on 2019-10-14

Fix check-imports to allow mimeparse as its already a dependency of piston.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b login-return-json lp:~blake-rouse/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 52d63b2ea1de204d7c4b8910ec9e9ce5807a232d

review: Approve

[Alberto Donato (ack)]

LGTM with a couple of comments inline

review: Approve

a021cd4... by Blake Rouse on 2019-10-15

Add python3-mimeparse as a direct dependency. Fix LoginJSONView class doc.

diff --git a/debian/control b/debian/control
index 394235a..a1e0e0f 100644
--- a/debian/control
+++ b/debian/control
@@ -85,6 +85,7 @@ Depends: bind9 (>= 1:9.10.3.dfsg.P2-5~),
          python3-djorm-ext-pgarray,
          python3-maas-provisioningserver (= ${binary:Version}),
          python3-macaroonbakery (>= 1.1.3),
+         python3-mimeparse,
          python3-petname,
          python3-requests,
          ubuntu-keyring,
diff --git a/required-packages/base b/required-packages/base
index 1675b14..db44002 100644
--- a/required-packages/base
+++ b/required-packages/base
@@ -38,6 +38,7 @@ python3-jsonschema
 python3-lxml
 python3-macaroonbakery
 python3-markupsafe
+python3-mimeparse
 python3-novaclient
 python3-netaddr
 python3-netifaces
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index 7a61edc..4045ed6 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -113,6 +113,7 @@ parts:
       - python3-httplib2
       - python3-jsonschema
       - python3-lxml
+      - python3-mimeparse
       - python3-netaddr
       - python3-netifaces
       - python3-paramiko
diff --git a/src/maasserver/views/account.py b/src/maasserver/views/account.py
index dc90f83..2bce574 100644
--- a/src/maasserver/views/account.py
+++ b/src/maasserver/views/account.py
@@ -5,14 +5,20 @@
 
 __all__ = ["authenticate", "login", "logout"]
 
+import json
+import mimeparse
+
 from django import forms
 from django.conf import settings as django_settings
 from django.contrib.auth import (
     authenticate as dj_authenticate,
     REDIRECT_FIELD_NAME,
 )
-from django.contrib.auth.views import login as dj_login, logout as dj_logout
+from django.contrib.auth.views import LoginView, logout as dj_logout
+from django.contrib.auth.forms import AuthenticationForm
 from django.http import (
+    HttpResponse,
+    HttpResponseBadRequest,
     HttpResponseForbidden,
     HttpResponseNotAllowed,
     HttpResponseRedirect,
@@ -30,6 +36,32 @@ from maasserver.utils.django_urls import reverse
 from provisioningserver.events import EVENT_TYPES
 
 
+def wants_json_response(request):
+    """Return True when the requestor wants a JSON response."""
+    try:
+        return ("application", "json") == mimeparse.parse_mime_type(
+            request.META.get("HTTP_ACCEPT", "")
+        )[:2]
+    except mimeparse.MimeTypeParseException:
+        return False
+
+
+class LoginJSONView(LoginView):
+    """A `LoginView` that conditionally returns JSON instead of a HTML view."""
+
+    def form_valid(self, form):
+        if wants_json_response(self.request):
+            return HttpResponse(status=204)
+        return super().form_valid(form)
+
+    def form_invalid(self, form):
+        if wants_json_response(self.request):
+            return HttpResponseBadRequest(
+                json.dumps(form.errors), content_type="application/json"
+            )
+        return super().form_invalid(form)
+
+
 @csrf_exempt
 def login(request):
     extra_context = {
@@ -38,6 +70,8 @@ def login(request):
         "external_auth_url": Config.objects.get_config("external_auth_url"),
     }
     if request.user.is_authenticated:
+        if wants_json_response(request):
+            return HttpResponse(status=204)
         return HttpResponseRedirect(reverse("index"))
     else:
         redirect_url = request.GET.get(
@@ -47,11 +81,13 @@ def login(request):
             redirect_field_name = None  # Ignore next page.
         else:
             redirect_field_name = REDIRECT_FIELD_NAME
-        result = dj_login(
-            request,
+        result = LoginJSONView.as_view(
+            template_name="registration/login.html",
             redirect_field_name=redirect_field_name,
+            form_class=AuthenticationForm,
             extra_context=extra_context,
-        )
+            redirect_authenticated_user=False,
+        )(request)
         if request.user.is_authenticated:
             create_audit_event(
                 EVENT_TYPES.AUTHORISATION,
@@ -88,10 +124,19 @@ def logout(request):
                     % ("admin" if request.user.is_superuser else "user")
                 ),
             )
-            return dj_logout(request, next_page=reverse("login"))
+            response = dj_logout(request, next_page=reverse("login"))
+            if wants_json_response(request):
+                return HttpResponse(status=204)
+            return response
+        elif wants_json_response(request):
+            return HttpResponseBadRequest(
+                json.dumps(form.errors), content_type="application/json"
+            )
     else:
         form = LogoutForm()
 
+    if wants_json_response(request):
+        return HttpResponseNotAllowed([request.method])
     return render(request, "maasserver/logout_confirm.html", {"form": form})
 
 
diff --git a/src/maasserver/views/tests/test_account.py b/src/maasserver/views/tests/test_account.py
index 2dba63a..7e84764 100644
--- a/src/maasserver/views/tests/test_account.py
+++ b/src/maasserver/views/tests/test_account.py
@@ -50,6 +50,16 @@ class TestLogin(MAASServerTestCase):
         response = self.client.get(reverse("login"))
         self.assertEqual(reverse("index"), extract_redirect(response))
 
+    def test_login_json_returns_204_when_already_authenticated(self):
+        password = factory.make_string()
+        user = factory.make_User(password=password)
+        self.client.handler.enforce_csrf_checks = True
+        self.client.login(username=user.username, password=password)
+        response = self.client.get(
+            reverse("login"), HTTP_ACCEPT="application/json"
+        )
+        self.assertThat(response, HasStatusCode(http.client.NO_CONTENT))
+
     def test_login_doesnt_redirect_to_logout_GET(self):
         password = factory.make_string()
         user = factory.make_User(password=password)
@@ -100,6 +110,31 @@ class TestLogin(MAASServerTestCase):
         )
         self.assertEqual(reverse("prefs"), extract_redirect(response))
 
+    def test_login_json_returns_204_on_authentication(self):
+        password = factory.make_string()
+        user = factory.make_User(password=password)
+        self.client.handler.enforce_csrf_checks = True
+        response = self.client.post(
+            reverse("login"),
+            {"username": user.username, "password": password},
+            HTTP_ACCEPT="application/json",
+        )
+        self.assertThat(response, HasStatusCode(http.client.NO_CONTENT))
+
+    def test_login_json_returns_400_on_bad_authentication(self):
+        password = factory.make_string()
+        user = factory.make_User(password=password)
+        self.client.handler.enforce_csrf_checks = True
+        response = self.client.post(
+            reverse("login"),
+            {
+                "username": factory.make_name("username"),
+                "password": factory.make_name("password"),
+            },
+            HTTP_ACCEPT="application/json",
+        )
+        self.assertThat(response, HasStatusCode(http.client.BAD_REQUEST))
+
     def test_login_sets_autocomplete_off_in_production(self):
         self.patch(settings, "DEBUG", False)
         factory.make_User()
@@ -161,10 +196,10 @@ class TestLogout(MAASServerTestCase):
             logout_link, get_content_links(response, element="#user-options")
         )
 
-    def test_loggout_uses_POST(self):
+    def test_logout_uses_POST(self):
         # Using POST for logging out, along with Django's csrf_token
         # tag, guarantees that we're protected against CSRF attacks on
-        # the loggout page.
+        # the logout page.
         password = factory.make_string()
         user = factory.make_User(password=password)
         self.client.handler.enforce_csrf_checks = True
@@ -173,6 +208,31 @@ class TestLogout(MAASServerTestCase):
         self.client.post(reverse("logout"))
         self.assertNotIn(SESSION_KEY, self.client.session)
 
+    def test_logout_json_returns_204(self):
+        password = factory.make_string()
+        user = factory.make_User(password=password)
+        self.client.handler.enforce_csrf_checks = True
+        self.client.login(username=user.username, password=password)
+        self.client.handler.enforce_csrf_checks = False
+        response = self.client.post(
+            reverse("logout"), HTTP_ACCEPT="application/json"
+        )
+        self.assertThat(response, HasStatusCode(http.client.NO_CONTENT))
+        self.assertNotIn(SESSION_KEY, self.client.session)
+
+    def test_logout_json_GET_returns_405(self):
+        password = factory.make_string()
+        user = factory.make_User(password=password)
+        self.client.handler.enforce_csrf_checks = True
+        self.client.login(username=user.username, password=password)
+        self.client.handler.enforce_csrf_checks = False
+        response = self.client.get(
+            reverse("logout"), HTTP_ACCEPT="application/json"
+        )
+        self.assertThat(
+            response, HasStatusCode(http.client.METHOD_NOT_ALLOWED)
+        )
+
     def test_logout_creates_audit_event(self):
         password = factory.make_string()
         user = factory.make_User(password=password)
diff --git a/utilities/check-imports b/utilities/check-imports
index 338a4ca..56f37c6 100755
--- a/utilities/check-imports
+++ b/utilities/check-imports
@@ -268,6 +268,7 @@ RegionControllerRule = Rule(
     Allow("macaroonbakery|macaroonbakery.**"),
     Allow("markupsafe|markupsafe.**"),
     Allow("metadataserver|metadataserver.**"),
+    Allow("mimeparse"),
     Allow("netaddr|netaddr.**"),
     Allow("oauth|oauth.**"),
     Allow("OpenSSL|OpenSSL.**"),