Skip to content

Commit 61618e1

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 275afed commit 61618e1

15 files changed

Lines changed: 591 additions & 436 deletions

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')

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)

api/tests/test_app.py

Lines changed: 63 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99
import json
1010
import os.path
1111

12-
from django.test import TestCase
1312
from django.conf import settings
13+
from django.contrib.auth.models import User
14+
from django.test import TestCase
15+
from rest_framework.authtoken.models import Token
1416

1517
from api.models import App
1618

@@ -21,8 +23,8 @@ class AppTest(TestCase):
2123
fixtures = ['tests.json']
2224

2325
def setUp(self):
24-
self.assertTrue(
25-
self.client.login(username='autotest', password='password'))
26+
self.user = User.objects.get(username='autotest')
27+
self.token = Token.objects.get(user=self.user).key
2628
# provide mock authentication used for run commands
2729
settings.SSH_PRIVATE_KEY = '<some-ssh-private-key>'
2830

@@ -35,39 +37,46 @@ def test_app(self):
3537
Test that a user can create, read, update and delete an application
3638
"""
3739
url = '/api/apps'
38-
response = self.client.post(url)
40+
response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
3941
self.assertEqual(response.status_code, 201)
4042
app_id = response.data['id'] # noqa
4143
self.assertIn('id', response.data)
4244
self.assertIn('url', response.data)
4345
self.assertEqual(response.data['url'], '{app_id}.deisapp.local'.format(**locals()))
44-
response = self.client.get('/api/apps')
46+
response = self.client.get('/api/apps',
47+
HTTP_AUTHORIZATION='token {}'.format(self.token))
4548
self.assertEqual(response.status_code, 200)
4649
self.assertEqual(len(response.data['results']), 1)
4750
url = '/api/apps/{app_id}'.format(**locals())
48-
response = self.client.get(url)
51+
response = self.client.get(url,
52+
HTTP_AUTHORIZATION='token {}'.format(self.token))
4953
self.assertEqual(response.status_code, 200)
5054
body = {'id': 'new'}
51-
response = self.client.patch(url, json.dumps(body), content_type='application/json')
55+
response = self.client.patch(url, json.dumps(body), content_type='application/json',
56+
HTTP_AUTHORIZATION='token {}'.format(self.token))
5257
self.assertEqual(response.status_code, 405)
53-
response = self.client.delete(url)
58+
response = self.client.delete(url,
59+
HTTP_AUTHORIZATION='token {}'.format(self.token))
5460
self.assertEqual(response.status_code, 204)
5561

5662
def test_app_override_id(self):
5763
body = {'id': 'myid'}
5864
response = self.client.post('/api/apps', json.dumps(body),
59-
content_type='application/json')
65+
content_type='application/json',
66+
HTTP_AUTHORIZATION='token {}'.format(self.token))
6067
self.assertEqual(response.status_code, 201)
6168
body = {'id': response.data['id']}
6269
response = self.client.post('/api/apps', json.dumps(body),
63-
content_type='application/json')
70+
content_type='application/json',
71+
HTTP_AUTHORIZATION='token {}'.format(self.token))
6472
self.assertContains(response, 'App with this Id already exists.', status_code=400)
6573
return response
6674

6775
def test_app_actions(self):
6876
url = '/api/apps'
6977
body = {'id': 'autotest'}
70-
response = self.client.post(url, json.dumps(body), content_type='application/json')
78+
response = self.client.post(url, json.dumps(body), content_type='application/json',
79+
HTTP_AUTHORIZATION='token {}'.format(self.token))
7180
self.assertEqual(response.status_code, 201)
7281
app_id = response.data['id'] # noqa
7382
# test logs
@@ -78,20 +87,23 @@ def test_app_actions(self):
7887
if os.path.exists(path):
7988
os.remove(path)
8089
url = '/api/apps/{app_id}/logs'.format(**locals())
81-
response = self.client.get(url)
90+
response = self.client.get(url,
91+
HTTP_AUTHORIZATION='token {}'.format(self.token))
8292
self.assertEqual(response.status_code, 204)
8393
self.assertEqual(response.data, 'No logs for {}'.format(app_id))
8494
# write out some fake log data and try again
8595
with open(path, 'a') as f:
8696
f.write(FAKE_LOG_DATA)
87-
response = self.client.get(url)
97+
response = self.client.get(url,
98+
HTTP_AUTHORIZATION='token {}'.format(self.token))
8899
self.assertEqual(response.status_code, 200)
89100
self.assertEqual(response.data, FAKE_LOG_DATA)
90101
os.remove(path)
91102
# test run
92103
url = '/api/apps/{app_id}/run'.format(**locals())
93104
body = {'command': 'ls -al'}
94-
response = self.client.post(url, json.dumps(body), content_type='application/json')
105+
response = self.client.post(url, json.dumps(body), content_type='application/json',
106+
HTTP_AUTHORIZATION='token {}'.format(self.token))
95107
self.assertEqual(response.status_code, 200)
96108
self.assertEqual(response.data[0], 0)
97109
# delete file for future runs
@@ -101,12 +113,14 @@ def test_app_release_notes_in_logs(self):
101113
"""Verifies that an app's release summary is dumped into the logs."""
102114
url = '/api/apps'
103115
body = {'id': 'autotest'}
104-
response = self.client.post(url, json.dumps(body), content_type='application/json')
116+
response = self.client.post(url, json.dumps(body), content_type='application/json',
117+
HTTP_AUTHORIZATION='token {}'.format(self.token))
105118
self.assertEqual(response.status_code, 201)
106119
app_id = response.data['id'] # noqa
107120
path = os.path.join(settings.DEIS_LOG_DIR, app_id + '.log')
108121
url = '/api/apps/{app_id}/logs'.format(**locals())
109-
response = self.client.get(url)
122+
response = self.client.get(url,
123+
HTTP_AUTHORIZATION='token {}'.format(self.token))
110124
self.assertIn('autotest created initial release', response.data)
111125
self.assertEqual(response.status_code, 200)
112126
# delete file for future runs
@@ -116,28 +130,33 @@ def test_app_errors(self):
116130
app_id = 'autotest-errors'
117131
url = '/api/apps'
118132
body = {'id': 'camelCase'}
119-
response = self.client.post(url, json.dumps(body), content_type='application/json')
133+
response = self.client.post(url, json.dumps(body), content_type='application/json',
134+
HTTP_AUTHORIZATION='token {}'.format(self.token))
120135
self.assertContains(response, 'App IDs can only contain [a-z0-9-]', status_code=400)
121136
url = '/api/apps'
122137
body = {'id': 'deis'}
123-
response = self.client.post(url, json.dumps(body), content_type='application/json')
138+
response = self.client.post(url, json.dumps(body), content_type='application/json',
139+
HTTP_AUTHORIZATION='token {}'.format(self.token))
124140
self.assertContains(response, "App IDs cannot be 'deis'", status_code=400)
125141
body = {'id': app_id}
126-
response = self.client.post(url, json.dumps(body), content_type='application/json')
142+
response = self.client.post(url, json.dumps(body), content_type='application/json',
143+
HTTP_AUTHORIZATION='token {}'.format(self.token))
127144
self.assertEqual(response.status_code, 201)
128145
app_id = response.data['id'] # noqa
129146
url = '/api/apps/{app_id}'.format(**locals())
130-
response = self.client.delete(url)
147+
response = self.client.delete(url,
148+
HTTP_AUTHORIZATION='token {}'.format(self.token))
131149
self.assertEquals(response.status_code, 204)
132150
for endpoint in ('containers', 'config', 'releases', 'builds'):
133151
url = '/api/apps/{app_id}/{endpoint}'.format(**locals())
134-
response = self.client.get(url)
152+
response = self.client.get(url,
153+
HTTP_AUTHORIZATION='token {}'.format(self.token))
135154
self.assertEquals(response.status_code, 404)
136155

137156
def test_app_structure_is_valid_json(self):
138157
"""Application structures should be valid JSON objects."""
139158
url = '/api/apps'
140-
response = self.client.post(url)
159+
response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
141160
self.assertEqual(response.status_code, 201)
142161
app_id = response.data['id']
143162
self.assertIn('structure', response.data)
@@ -146,57 +165,59 @@ def test_app_structure_is_valid_json(self):
146165
app.structure = {'web': 1}
147166
app.save()
148167
url = '/api/apps/{}'.format(app_id)
149-
response = self.client.get(url)
168+
response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
150169
self.assertIn('structure', response.data)
151170
self.assertEqual(response.data['structure'], {"web": 1})
152171

153172
def test_admin_can_manage_other_apps(self):
154173
"""Administrators of Deis should be able to manage all applications.
155174
"""
156175
# log in as non-admin user and create an app
157-
self.assertTrue(
158-
self.client.login(username='autotest2', password='password'))
176+
user = User.objects.get(username='autotest2')
177+
token = Token.objects.get(user=user)
159178
app_id = 'autotest'
160179
url = '/api/apps'
161180
body = {'id': app_id}
162-
response = self.client.post(url, json.dumps(body), content_type='application/json')
181+
response = self.client.post(url, json.dumps(body), content_type='application/json',
182+
HTTP_AUTHORIZATION='token {}'.format(token))
163183
# log in as admin, check to see if they have access
164-
self.assertTrue(
165-
self.client.login(username='autotest', password='password'))
166184
url = '/api/apps/{}'.format(app_id)
167-
response = self.client.get(url)
185+
response = self.client.get(url,
186+
HTTP_AUTHORIZATION='token {}'.format(self.token))
168187
self.assertEqual(response.status_code, 200)
169188
# check app logs
170189
url = '/api/apps/{app_id}/logs'.format(**locals())
171-
response = self.client.get(url)
190+
response = self.client.get(url,
191+
HTTP_AUTHORIZATION='token {}'.format(self.token))
172192
self.assertEqual(response.status_code, 200)
173193
self.assertIn('autotest2 created initial release', response.data)
174194
# run one-off commands
175195
url = '/api/apps/{app_id}/run'.format(**locals())
176196
body = {'command': 'ls -al'}
177-
response = self.client.post(url, json.dumps(body), content_type='application/json')
197+
response = self.client.post(url, json.dumps(body), content_type='application/json',
198+
HTTP_AUTHORIZATION='token {}'.format(self.token))
178199
self.assertEqual(response.status_code, 200)
179200
self.assertEqual(response.data[0], 0)
180201
# delete the app
181202
url = '/api/apps/{}'.format(app_id)
182-
response = self.client.delete(url)
203+
response = self.client.delete(url,
204+
HTTP_AUTHORIZATION='token {}'.format(self.token))
183205
self.assertEqual(response.status_code, 204)
184206

185207
def test_admin_can_see_other_apps(self):
186208
"""If a user creates an application, the administrator should be able
187209
to see it.
188210
"""
189211
# log in as non-admin user and create an app
190-
self.assertTrue(
191-
self.client.login(username='autotest2', password='password'))
212+
user = User.objects.get(username='autotest2')
213+
token = Token.objects.get(user=user)
192214
app_id = 'autotest'
193215
url = '/api/apps'
194216
body = {'id': app_id}
195-
response = self.client.post(url, json.dumps(body), content_type='application/json')
217+
response = self.client.post(url, json.dumps(body), content_type='application/json',
218+
HTTP_AUTHORIZATION='token {}'.format(token))
196219
# log in as admin
197-
self.assertTrue(
198-
self.client.login(username='autotest', password='password'))
199-
response = self.client.get(url)
220+
response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
200221
self.assertEqual(response.data['count'], 1)
201222

202223
def test_run_without_auth(self):
@@ -205,13 +226,15 @@ def test_run_without_auth(self):
205226
settings.SSH_PRIVATE_KEY = ''
206227
url = '/api/apps'
207228
body = {'id': 'autotest'}
208-
response = self.client.post(url, json.dumps(body), content_type='application/json')
229+
response = self.client.post(url, json.dumps(body), content_type='application/json',
230+
HTTP_AUTHORIZATION='token {}'.format(self.token))
209231
self.assertEqual(response.status_code, 201)
210232
app_id = response.data['id'] # noqa
211233
# test run
212234
url = '/api/apps/{app_id}/run'.format(**locals())
213235
body = {'command': 'ls -al'}
214-
response = self.client.post(url, json.dumps(body), content_type='application/json')
236+
response = self.client.post(url, json.dumps(body), content_type='application/json',
237+
HTTP_AUTHORIZATION='token {}'.format(self.token))
215238
self.assertEquals(response.status_code, 400)
216239
self.assertEquals(response.data, 'Support for admin commands is not configured')
217240

0 commit comments

Comments
 (0)