Skip to content

Commit 5e76335

Browse files
author
Matthew Fisher
committed
ref(controller): switch to token-based authn
When pulling out CSRF tokens from the authentication requirements, we are still relying on cookies. This is great from a CSRF attacker's standpoint as attackers can easily `<iframe>` the site that utilizes the API, generate a POST request and re-use the existing cookie. Using token-based authn fixes this because there will be no existing authn cookie in the request. With this change, we are also not vulnerable to XSRF attacks either because we do not store a validated login cookie in the browser (yay!).
1 parent 0d9b17f commit 5e76335

18 files changed

Lines changed: 609 additions & 507 deletions

client/deis.py

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
from __future__ import print_function
4242
from collections import namedtuple
4343
from collections import OrderedDict
44-
from cookielib import MozillaCookieJar
4544
from datetime import datetime
4645
from getpass import getpass
4746
from itertools import cycle
@@ -82,21 +81,14 @@ class Session(requests.Session):
8281
def __init__(self):
8382
super(Session, self).__init__()
8483
self.trust_env = False
85-
cookie_file = os.path.expanduser('~/.deis/cookies.txt')
86-
cookie_dir = os.path.dirname(cookie_file)
87-
self.cookies = MozillaCookieJar(cookie_file)
84+
config_dir = os.path.expanduser('~/.deis')
8885
self.proxies = {
8986
"http": os.getenv("http_proxy"),
9087
"https": os.getenv("https_proxy")
9188
}
9289
# Create the $HOME/.deis dir if it doesn't exist
93-
if not os.path.isdir(cookie_dir):
94-
os.mkdir(cookie_dir, 0700)
95-
# Load existing cookies if the cookies.txt exists
96-
if os.path.isfile(cookie_file):
97-
self.cookies.load()
98-
self.cookies.clear_expired_cookies()
99-
self.cookies.save()
90+
if not os.path.isdir(config_dir):
91+
os.mkdir(config_dir, 0700)
10092

10193
@property
10294
def app(self):
@@ -152,17 +144,14 @@ def _get_name_from_git_remote(self, git_root):
152144

153145
def request(self, *args, **kwargs):
154146
"""
155-
Issue an HTTP request with proper cookie handling
147+
Issue an HTTP request
156148
"""
157149
url = args[1]
158150
if 'headers' in kwargs:
159151
kwargs['headers']['Referer'] = url
160152
else:
161153
kwargs['headers'] = {'Referer': url}
162154
response = super(Session, self).request(*args, **kwargs)
163-
self.cookies.save()
164-
# set ~/.deis/cookies.txt readable only by its owner
165-
os.chmod(self.cookies.filename, 0600)
166155
return response
167156

168157

@@ -388,16 +377,18 @@ def _dispatch(self, method, path, body=None, **kwargs):
388377
"""
389378
Dispatch an API request to the active Deis controller
390379
"""
391-
headers = {
392-
'content-type': 'application/json',
393-
'X-Deis-Version': __version__.rsplit('.', 1)[0],
394-
}
395380
func = getattr(self._session, method.lower())
396381
controller = self._settings.get('controller')
397-
if not controller:
382+
token = self._settings.get('token')
383+
if not token:
398384
raise EnvironmentError(
399-
'No active controller. Use `deis login` or `deis register` to get started.')
385+
'Could not find token. Use `deis login` or `deis register` to get started.')
400386
url = urlparse.urljoin(controller, path, **kwargs)
387+
headers = {
388+
'content-type': 'application/json',
389+
'X-Deis-Version': __version__.rsplit('.', 1)[0],
390+
'Authorization': 'token {}'.format(token)
391+
}
401392
response = func(url, data=body, headers=headers)
402393
return response
403394

@@ -711,9 +702,6 @@ def auth_register(self, args):
711702
email = raw_input('email: ')
712703
url = urlparse.urljoin(controller, '/api/auth/register')
713704
payload = {'username': username, 'password': password, 'email': email}
714-
# Clear any existing cookies
715-
self._session.cookies.clear()
716-
self._session.cookies.save()
717705
response = self._session.post(url, data=payload, allow_redirects=False)
718706
if response.status_code == requests.codes.created: # @UndefinedVariable
719707
self._settings['controller'] = controller
@@ -744,9 +732,8 @@ def auth_cancel(self, args):
744732
confirm = raw_input("Cancel account \"{}\" at {}? (y/n) ".format(username, controller))
745733
if confirm == 'y':
746734
self._dispatch('delete', '/api/auth/cancel')
747-
self._session.cookies.clear()
748-
self._session.cookies.save()
749735
self._settings['controller'] = None
736+
self._settings['token'] = None
750737
self._settings.save()
751738
self._logger.info('Account cancelled')
752739
else:
@@ -780,16 +767,13 @@ def auth_login(self, args):
780767
password = getpass('password: ')
781768
url = urlparse.urljoin(controller, '/api/auth/login/')
782769
payload = {'username': username, 'password': password}
783-
# clear any existing cookies
784-
self._session.cookies.clear()
785-
self._session.cookies.save()
786-
# prime cookies for login
787-
self._session.get(url, headers=headers)
788770
# post credentials to the login URL
789771
response = self._session.post(url, data=payload, allow_redirects=False)
790-
if response.status_code == requests.codes.found: # @UndefinedVariable
772+
if response.status_code == requests.codes.ok: # @UndefinedVariable
773+
# retrieve and save the API token for future requests
791774
self._settings['controller'] = controller
792775
self._settings['username'] = username
776+
self._settings['token'] = response.json()['token']
793777
self._settings.save()
794778
self._logger.info("Logged in as {}".format(username))
795779
return username
@@ -802,16 +786,9 @@ def auth_logout(self, args):
802786
803787
Usage: deis auth:logout
804788
"""
805-
controller = self._settings.get('controller')
806-
if controller:
807-
try:
808-
self._dispatch('get', '/api/auth/logout/')
809-
except requests.exceptions.ConnectionError:
810-
pass
811-
self._session.cookies.clear()
812-
self._session.cookies.save()
813789
self._settings['controller'] = None
814790
self._settings['username'] = None
791+
self._settings['token'] = None
815792
self._settings.save()
816793
self._logger.info('Logged out')
817794

controller/api/models.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,19 @@
1515
import threading
1616

1717
from django.conf import settings
18-
from django.contrib.auth.models import User
18+
from django.contrib.auth import get_user_model
1919
from django.core.exceptions import ValidationError
2020
from django.db import models
2121
from django.db.models import Max
22-
from django.db.models.signals import post_delete
23-
from django.db.models.signals import post_save
22+
from django.db.models.signals import post_delete, post_save
23+
from django.dispatch import receiver
2424
from django.utils.encoding import python_2_unicode_compatible
2525
from django_fsm import FSMField, transition
2626
from django_fsm.signals import post_transition
2727
from docker.utils import utils
2828
from json_field.fields import JSONField
2929
import requests
30+
from rest_framework.authtoken.models import Token
3031

3132
from api import fields
3233
from registry import publish_release
@@ -830,6 +831,13 @@ def _etcd_purge_domains(**kwargs):
830831
post_delete.connect(_log_domain_removed, sender=Domain, dispatch_uid='api.models.log')
831832

832833

834+
# automatically generate a new token on creation
835+
@receiver(post_save, sender=get_user_model())
836+
def create_auth_token(sender, instance=None, created=False, **kwargs):
837+
if created:
838+
Token.objects.create(user=instance)
839+
840+
833841
# save FSM transitions as they happen
834842
def _save_transition(**kwargs):
835843
kwargs['instance'].save()
@@ -851,7 +859,7 @@ def _save_transition(**kwargs):
851859
if _etcd_client:
852860
post_save.connect(_etcd_publish_key, sender=Key, dispatch_uid='api.models')
853861
post_delete.connect(_etcd_purge_key, sender=Key, dispatch_uid='api.models')
854-
post_delete.connect(_etcd_purge_user, sender=User, dispatch_uid='api.models')
862+
post_delete.connect(_etcd_purge_user, sender=get_user_model(), dispatch_uid='api.models')
855863
post_save.connect(_etcd_publish_domains, sender=Domain, dispatch_uid='api.models')
856864
post_delete.connect(_etcd_purge_domains, sender=Domain, dispatch_uid='api.models')
857865
post_save.connect(_etcd_create_app, sender=App, dispatch_uid='api.models')

controller/api/tests/test_api_middleware.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
"""
66
from __future__ import unicode_literals
77

8+
from django.contrib.auth.models import User
89
from django.test import TestCase
10+
from rest_framework.authtoken.models import Token
911

1012
from deis import __version__
1113

@@ -17,16 +19,17 @@ class APIMiddlewareTest(TestCase):
1719
fixtures = ['tests.json']
1820

1921
def setUp(self):
20-
self.assertTrue(
21-
self.client.login(username='autotest', password='password'))
22+
self.user = User.objects.get(username='autotest')
23+
self.token = Token.objects.get(user=self.user).key
2224

2325
def test_x_deis_version_header_good(self):
2426
"""
2527
Test that when the version header is sent, the request is accepted.
2628
"""
2729
response = self.client.get(
2830
'/api/apps',
29-
HTTP_X_DEIS_VERSION=__version__.rsplit('.', 1)[0]
31+
HTTP_X_DEIS_VERSION=__version__.rsplit('.', 1)[0],
32+
HTTP_AUTHORIZATION='token {}'.format(self.token),
3033
)
3134
self.assertEqual(response.status_code, 200)
3235

@@ -36,13 +39,15 @@ def test_x_deis_version_header_bad(self):
3639
"""
3740
response = self.client.get(
3841
'/api/apps',
39-
HTTP_X_DEIS_VERSION='1234.5678'
42+
HTTP_X_DEIS_VERSION='1234.5678',
43+
HTTP_AUTHORIZATION='token {}'.format(self.token),
4044
)
4145
self.assertEqual(response.status_code, 405)
4246

4347
def test_x_deis_version_header_not_present(self):
4448
"""
4549
Test that when the version header is not present, the request is accepted.
4650
"""
47-
response = self.client.get('/api/apps')
51+
response = self.client.get('/api/apps',
52+
HTTP_AUTHORIZATION='token {}'.format(self.token))
4853
self.assertEqual(response.status_code, 200)

0 commit comments

Comments
 (0)