Merge pull request #2136 from bacongobbler/remove_csrf_token

ref(controller): remove csrf token; switch to token-based authn

Author: Matthew Fisher

client/deis.py

@@ -41,7 +41,6 @@
 from __future__ import print_function
 from collections import namedtuple
 from collections import OrderedDict
-from cookielib import MozillaCookieJar
 from datetime import datetime
 from getpass import getpass
 from itertools import cycle
@@ -82,21 +81,14 @@ class Session(requests.Session):
     def __init__(self):
         super(Session, self).__init__()
         self.trust_env = False
-        cookie_file = os.path.expanduser('~/.deis/cookies.txt')
-        cookie_dir = os.path.dirname(cookie_file)
-        self.cookies = MozillaCookieJar(cookie_file)
+        config_dir = os.path.expanduser('~/.deis')
         self.proxies = {
             "http": os.getenv("http_proxy"),
             "https": os.getenv("https_proxy")
         }
         # Create the $HOME/.deis dir if it doesn't exist
-        if not os.path.isdir(cookie_dir):
-            os.mkdir(cookie_dir, 0700)
-        # Load existing cookies if the cookies.txt exists
-        if os.path.isfile(cookie_file):
-            self.cookies.load()
-            self.cookies.clear_expired_cookies()
-            self.cookies.save()
+        if not os.path.isdir(config_dir):
+            os.mkdir(config_dir, 0700)
 
     @property
     def app(self):
@@ -152,25 +144,14 @@ def _get_name_from_git_remote(self, git_root):
 
     def request(self, *args, **kwargs):
         """
-        Issue an HTTP request with proper cookie handling including
-        `Django CSRF tokens <https://docs.djangoproject.com/en/dev/ref/contrib/csrf/>`
+        Issue an HTTP request
         """
-        for cookie in self.cookies:
-            if cookie.name == 'csrftoken':
-                if 'headers' in kwargs:
-                    kwargs['headers']['X-CSRFToken'] = cookie.value
-                else:
-                    kwargs['headers'] = {'X-CSRFToken': cookie.value}
-                break
         url = args[1]
         if 'headers' in kwargs:
             kwargs['headers']['Referer'] = url
         else:
             kwargs['headers'] = {'Referer': url}
         response = super(Session, self).request(*args, **kwargs)
-        self.cookies.save()
-        # set ~/.deis/cookies.txt readable only by its owner
-        os.chmod(self.cookies.filename, 0600)
         return response
 
 
@@ -395,16 +376,18 @@ def _dispatch(self, method, path, body=None, **kwargs):
         """
         Dispatch an API request to the active Deis controller
         """
-        headers = {
-            'content-type': 'application/json',
-            'X-Deis-Version': __version__.rsplit('.', 1)[0],
-        }
         func = getattr(self._session, method.lower())
         controller = self._settings.get('controller')
-        if not controller:
+        token = self._settings.get('token')
+        if not token:
             raise EnvironmentError(
-                'No active controller. Use `deis login` or `deis register` to get started.')
+                'Could not find token. Use `deis login` or `deis register` to get started.')
         url = urlparse.urljoin(controller, path, **kwargs)
+        headers = {
+            'content-type': 'application/json',
+            'X-Deis-Version': __version__.rsplit('.', 1)[0],
+            'Authorization': 'token {}'.format(token)
+        }
         response = func(url, data=body, headers=headers)
         return response
 
@@ -718,9 +701,6 @@ def auth_register(self, args):
             email = raw_input('email: ')
         url = urlparse.urljoin(controller, '/api/auth/register')
         payload = {'username': username, 'password': password, 'email': email}
-        # Clear any existing cookies
-        self._session.cookies.clear()
-        self._session.cookies.save()
         response = self._session.post(url, data=payload, allow_redirects=False)
         if response.status_code == requests.codes.created:  # @UndefinedVariable
             self._settings['controller'] = controller
@@ -751,9 +731,8 @@ def auth_cancel(self, args):
             confirm = raw_input("Cancel account \"{}\" at {}? (y/n) ".format(username, controller))
             if confirm == 'y':
                 self._dispatch('delete', '/api/auth/cancel')
-                self._session.cookies.clear()
-                self._session.cookies.save()
                 self._settings['controller'] = None
+                self._settings['token'] = None
                 self._settings.save()
                 self._logger.info('Account cancelled')
             else:
@@ -787,16 +766,13 @@ def auth_login(self, args):
             password = getpass('password: ')
         url = urlparse.urljoin(controller, '/api/auth/login/')
         payload = {'username': username, 'password': password}
-        # clear any existing cookies
-        self._session.cookies.clear()
-        self._session.cookies.save()
-        # prime cookies for login
-        self._session.get(url, headers=headers)
         # post credentials to the login URL
         response = self._session.post(url, data=payload, allow_redirects=False)
-        if response.status_code == requests.codes.found:  # @UndefinedVariable
+        if response.status_code == requests.codes.ok:  # @UndefinedVariable
+            # retrieve and save the API token for future requests
             self._settings['controller'] = controller
             self._settings['username'] = username
+            self._settings['token'] = response.json()['token']
             self._settings.save()
             self._logger.info("Logged in as {}".format(username))
             return username
@@ -809,16 +785,9 @@ def auth_logout(self, args):
 
         Usage: deis auth:logout
         """
-        controller = self._settings.get('controller')
-        if controller:
-            try:
-                self._dispatch('get', '/api/auth/logout/')
-            except requests.exceptions.ConnectionError:
-                pass
-        self._session.cookies.clear()
-        self._session.cookies.save()
         self._settings['controller'] = None
         self._settings['username'] = None
+        self._settings['token'] = None
         self._settings.save()
         self._logger.info('Logged out')
 

controller/api/models.py

@@ -15,18 +15,19 @@
 import threading
 
 from django.conf import settings
-from django.contrib.auth.models import User
+from django.contrib.auth import get_user_model
 from django.core.exceptions import ValidationError
 from django.db import models
 from django.db.models import Max
-from django.db.models.signals import post_delete
-from django.db.models.signals import post_save
+from django.db.models.signals import post_delete, post_save
+from django.dispatch import receiver
 from django.utils.encoding import python_2_unicode_compatible
 from django_fsm import FSMField, transition
 from django_fsm.signals import post_transition
 from docker.utils import utils
 from json_field.fields import JSONField
 import requests
+from rest_framework.authtoken.models import Token
 
 from api import fields
 from registry import publish_release
@@ -831,6 +832,13 @@ def _etcd_purge_domains(**kwargs):
 post_delete.connect(_log_domain_removed, sender=Domain, dispatch_uid='api.models.log')
 
 
+# automatically generate a new token on creation
+@receiver(post_save, sender=get_user_model())
+def create_auth_token(sender, instance=None, created=False, **kwargs):
+    if created:
+        Token.objects.create(user=instance)
+
+
 # save FSM transitions as they happen
 def _save_transition(**kwargs):
     kwargs['instance'].save()
@@ -852,7 +860,7 @@ def _save_transition(**kwargs):
 if _etcd_client:
     post_save.connect(_etcd_publish_key, sender=Key, dispatch_uid='api.models')
     post_delete.connect(_etcd_purge_key, sender=Key, dispatch_uid='api.models')
-    post_delete.connect(_etcd_purge_user, sender=User, dispatch_uid='api.models')
+    post_delete.connect(_etcd_purge_user, sender=get_user_model(), dispatch_uid='api.models')
     post_save.connect(_etcd_publish_domains, sender=Domain, dispatch_uid='api.models')
     post_delete.connect(_etcd_purge_domains, sender=Domain, dispatch_uid='api.models')
     post_save.connect(_etcd_create_app, sender=App, dispatch_uid='api.models')

controller/api/tests/test_api_middleware.py

@@ -5,7 +5,9 @@
 """
 from __future__ import unicode_literals
 
+from django.contrib.auth.models import User
 from django.test import TestCase
+from rest_framework.authtoken.models import Token
 
 from deis import __version__
 
@@ -17,16 +19,17 @@ class APIMiddlewareTest(TestCase):
     fixtures = ['tests.json']
 
     def setUp(self):
-        self.assertTrue(
-            self.client.login(username='autotest', password='password'))
+        self.user = User.objects.get(username='autotest')
+        self.token = Token.objects.get(user=self.user).key
 
     def test_x_deis_version_header_good(self):
         """
         Test that when the version header is sent, the request is accepted.
         """
         response = self.client.get(
             '/api/apps',
-            HTTP_X_DEIS_VERSION=__version__.rsplit('.', 1)[0]
+            HTTP_X_DEIS_VERSION=__version__.rsplit('.', 1)[0],
+            HTTP_AUTHORIZATION='token {}'.format(self.token),
         )
         self.assertEqual(response.status_code, 200)
 
@@ -36,13 +39,15 @@ def test_x_deis_version_header_bad(self):
         """
         response = self.client.get(
             '/api/apps',
-            HTTP_X_DEIS_VERSION='1234.5678'
+            HTTP_X_DEIS_VERSION='1234.5678',
+            HTTP_AUTHORIZATION='token {}'.format(self.token),
         )
         self.assertEqual(response.status_code, 405)
 
     def test_x_deis_version_header_not_present(self):
         """
         Test that when the version header is not present, the request is accepted.
         """
-        response = self.client.get('/api/apps')
+        response = self.client.get('/api/apps',
+                                   HTTP_AUTHORIZATION='token {}'.format(self.token))
         self.assertEqual(response.status_code, 200)