Skip to content

Commit

Permalink
Require X-Requested-By header to prevent 3rd party javascript from
Browse files Browse the repository at this point in the history
forging requests.
  • Loading branch information
adomate committed Aug 13, 2013
1 parent 00a4a08 commit 69d4e78
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 40 deletions.
10 changes: 10 additions & 0 deletions COMMIT_MSG
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Now with eager load on __include and a fix for eager loading of Events.
# Please enter the description for your pull request. Lines starting
# with '#' will be ignored, and an empty message aborts the request.
#
# The first line should be a short feature summary, no longer than
# 72 characters.
#
# The subsequent lines should be a longer description of the feature,
# including a summary of backwards-compatibility breaks, wrapped at
# 80 characters.
1 change: 1 addition & 0 deletions src/ggrc/assets/javascripts/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ jQuery.extend(GGRC, {
var etags = {};
$.ajaxPrefilter(function( options, originalOptions, jqXHR ) {
var data;
jqXHR.setRequestHeader("X-Requested-By", "gGRC");
if ( /^\/api\//.test(options.url) && /PUT|POST|DELETE/.test(options.type.toUpperCase())) {
data = originalOptions.data;
options.dataType = "json";
Expand Down
2 changes: 2 additions & 0 deletions src/ggrc/services/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,8 @@ class Resource(ModelView):
)

def dispatch_request(self, *args, **kwargs):
if 'X-Requested-By' not in request.headers:
raise BadRequest('X-Requested-By header is REQUIRED.')
method = request.method

if method == 'GET':
Expand Down
6 changes: 5 additions & 1 deletion src/service_specs/steps/ggrc_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ def define_current_user(context, user_json):
# logout current user
response = requests.get(
context.base_url+'/logout',
headers={'Accept': 'text/html'},
headers={
'Accept': 'text/html',
'X-Requested-By': 'Reciprocity Behave Tests',
},
cookies=getattr(context, 'cookies', {})
)
assert response.status_code == 200, 'Failed to logout!!'
Expand Down Expand Up @@ -259,6 +262,7 @@ def delete_resource(context, url, resource):
'Content-Type': 'application/json',
'If-Match': resource.response.headers['Etag'],
'If-Unmodified-Since': resource.response.headers['Last-Modified'],
'X-Requested-By': 'Reciprocity Behave Tests',
}
if hasattr(context, 'current_user_json'):
headers['X-ggrc-user'] = context.current_user_json
Expand Down
17 changes: 14 additions & 3 deletions src/tests/ggrc/behave/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ def handle_get_resource_and_name_it(context, url, name):
setattr(context, name, response.json())

def get_resource(context, url):
headers={'Accept': 'application/json',}
headers={
'Accept': 'application/json',
'X-Requested-By': 'Reciprocity Behave Tests',
}
if hasattr(context, 'current_user_json'):
headers['X-ggrc-user'] = context.current_user_json
response = requests.get(
Expand All @@ -71,6 +74,7 @@ def put_resource(context, url, resource):
'Content-Type': 'application/json',
'If-Match': resource.response.headers['Etag'],
'If-Unmodified-Since': resource.response.headers['Last-Modified'],
'X-Requested-By': 'Reciprocity Behave Tests',
}
if hasattr(context, 'current_user_json'):
headers['X-ggrc-user'] = context.current_user_json
Expand Down Expand Up @@ -117,7 +121,10 @@ def handle_post_named_example(context, name, expected_status=201):
def post_to_endpoint(context, endpoint, data, url=None):
if url is None:
url = get_service_endpoint_url(context, endpoint)
headers = {'Content-Type': 'application/json',}
headers = {
'Content-Type': 'application/json',
'X-Requested-By': 'Reciprocity Behave Tests',
}
response = requests.post(
context.base_url+url,
data=data,
Expand All @@ -127,7 +134,10 @@ def post_to_endpoint(context, endpoint, data, url=None):
)
if response.status_code == 302:
# deal with login redirect, expect noop
headers={'Accept': 'text/html',}
headers={
'Accept': 'text/html',
'X-Requested-By': 'Reciprocity Behave Tests',
}
if hasattr(context, 'current_user_json'):
headers['X-ggrc-user'] = context.current_user_json
response = requests.get(
Expand All @@ -141,6 +151,7 @@ def post_to_endpoint(context, endpoint, data, url=None):
data=data,
headers={
'Content-Type': 'application/json',
'X-Requested-By': 'Reciprocity Behave Tests',
},
cookies=context.cookies,
)
Expand Down
92 changes: 56 additions & 36 deletions src/tests/ggrc/services/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,22 @@ def assertOptions(self, response, allowed):
self.assertIn('Allow', response.headers)
self.assertItemsEqual(allowed, response.headers['Allow'].split(', '))

def test_empty_collection_get(self):
def headers(self, *args, **kwargs):
ret = list(args)
ret.append(('X-Requested-By', 'Unit Tests'))
ret.extend([(k,v) for k,v in kwargs.items()])
return ret

def test_X_Requested_By_required(self):
response = self.client.get(self.mock_url())
self.assert400(response)

def test_empty_collection_get(self):
response = self.client.get(self.mock_url(), headers=self.headers())
self.assert200(response)

def test_missing_resource_get(self):
response = self.client.get(self.mock_url('foo'))
response = self.client.get(self.mock_url('foo'), headers=self.headers())
self.assert404(response)

def test_collection_get(self):
Expand All @@ -133,7 +143,7 @@ def test_collection_get(self):
modified_by_id=42, created_at=date1, updated_at=date1)
mock2 = self.mock_model(
modified_by_id=43, created_at=date2, updated_at=date2)
response = self.client.get(self.mock_url())
response = self.client.get(self.mock_url(), headers=self.headers())
self.assert200(response)
self.assertRequiredHeaders(
response,
Expand All @@ -153,7 +163,7 @@ def test_resource_get(self):
date1 = datetime(2013, 4, 17, 0, 0, 0, 0)
mock1 = self.mock_model(
modified_by_id=42, created_at=date1, updated_at=date1)
response = self.client.get(self.mock_url(mock1.id))
response = self.client.get(self.mock_url(mock1.id), headers=self.headers())
self.assert200(response)
self.assertRequiredHeaders(
response,
Expand All @@ -164,11 +174,14 @@ def test_resource_get(self):
self.assertDictEqual(self.mock_json(mock1), response.json['services_test_mock_model'])

def test_collection_put(self):
self.assertAllow(self.client.put(URL_MOCK_COLLECTION), COLLECTION_ALLOWED)
self.assertAllow(
self.client.put(URL_MOCK_COLLECTION, headers=self.headers()),
COLLECTION_ALLOWED)

def test_collection_delete(self):
self.assertAllow(
self.client.delete(URL_MOCK_COLLECTION), COLLECTION_ALLOWED)
self.client.delete(URL_MOCK_COLLECTION, headers=self.headers()),
COLLECTION_ALLOWED)

def test_collection_post_successful(self):
data = json.dumps(
Expand All @@ -177,18 +190,20 @@ def test_collection_post_successful(self):
URL_MOCK_COLLECTION,
content_type='application/json',
data=data,
headers=self.headers(),
)
self.assertStatus(response, 201)
self.assertIn('Location', response.headers)
response = self.client.get(self.get_location(response))
response = self.client.get(
self.get_location(response), headers=self.headers())
self.assert200(response)
self.assertIn('Content-Type', response.headers)
self.assertEqual('application/json', response.headers['Content-Type'])
self.assertIn('services_test_mock_model', response.json)
self.assertIn('foo', response.json['services_test_mock_model'])
self.assertEqual('bar', response.json['services_test_mock_model']['foo'])
# check the collection, too
response = self.client.get(URL_MOCK_COLLECTION)
response = self.client.get(URL_MOCK_COLLECTION, headers=self.headers())
self.assert200(response)
self.assertEqual(
1, len(response.json['test_model_collection']['test_model']))
Expand All @@ -200,6 +215,7 @@ def test_collection_post_bad_request(self):
URL_MOCK_COLLECTION,
content_type='application/json',
data='This is most definitely not valid content.',
headers=self.headers(),
)
self.assert400(response)

Expand All @@ -208,12 +224,13 @@ def test_collection_post_bad_content_type(self):
URL_MOCK_COLLECTION,
content_type='text/plain',
data="Doesn't matter, now does it?",
headers=self.headers(),
)
self.assertStatus(response, 415)

def test_put_successful(self):
mock = self.mock_model(foo='buzz')
response = self.client.get(self.mock_url(mock.id))
response = self.client.get(self.mock_url(mock.id), headers=self.headers())
self.assert200(response)
self.assertRequiredHeaders(response)
obj = response.json
Expand All @@ -228,14 +245,14 @@ def test_put_successful(self):
response = self.client.put(
url,
data=json.dumps(obj),
headers=[
headers=self.headers(
('If-Unmodified-Since', original_headers['Last-Modified']),
('If-Match', original_headers['Etag']),
],
),
content_type='application/json',
)
self.assert200(response)
response = self.client.get(url)
response = self.client.get(url, headers=self.headers())
self.assert200(response)
self.assertNotEqual(
original_headers['Last-Modified'], response.headers['Last-Modified'])
Expand All @@ -245,24 +262,24 @@ def test_put_successful(self):

def test_put_bad_request(self):
mock = self.mock_model(foo='tough')
response = self.client.get(self.mock_url(mock.id))
response = self.client.get(self.mock_url(mock.id), headers=self.headers())
self.assert200(response)
self.assertRequiredHeaders(response)
url = urlparse(response.json['services_test_mock_model']['selfLink']).path
response = self.client.put(
url,
content_type='application/json',
data='This is most definitely not valid content.',
headers=[
headers=self.headers(
('If-Unmodified-Since', response.headers['Last-Modified']),
('If-Match', response.headers['Etag']),
],
),
)
self.assert400(response)

def test_put_and_delete_conflict(self):
mock = self.mock_model(foo='mudder')
response = self.client.get(self.mock_url(mock.id))
response = self.client.get(self.mock_url(mock.id), headers=self.headers())
self.assert200(response)
self.assertRequiredHeaders(response)
obj = response.json
Expand All @@ -277,26 +294,26 @@ def test_put_and_delete_conflict(self):
response = self.client.put(
url,
data=json.dumps(obj),
headers=[
headers=self.headers(
('If-Unmodified-Since', original_headers['Last-Modified']),
('If-Match', original_headers['Etag']),
],
),
content_type='application/json',
)
self.assertStatus(response, 409)
response = self.client.delete(
url,
headers=[
headers=self.headers(
('If-Unmodified-Since', original_headers['Last-Modified']),
('If-Match', original_headers['Etag']),
],
),
content_type='application/json',
)
self.assertStatus(response, 409)

def test_put_and_delete_missing_precondition(self):
mock = self.mock_model(foo='tricky')
response = self.client.get(self.mock_url(mock.id))
response = self.client.get(self.mock_url(mock.id), headers=self.headers())
self.assert200(response)
obj = response.json
obj['services_test_mock_model']['foo'] = 'strings'
Expand All @@ -305,42 +322,45 @@ def test_put_and_delete_missing_precondition(self):
url,
data=json.dumps(obj),
content_type='application/json',
headers=self.headers(),
)
self.assertStatus(response, 428)
response = self.client.delete(url)
response = self.client.delete(url, headers=self.headers())
self.assertStatus(response, 428)

def test_delete_successful(self):
mock = self.mock_model(foo='delete me')
response = self.client.get(self.mock_url(mock.id))
response = self.client.get(self.mock_url(mock.id), headers=self.headers())
self.assert200(response)
url = urlparse(response.json['services_test_mock_model']['selfLink']).path
response = self.client.delete(
url,
headers=[
headers=self.headers(
('If-Unmodified-Since', response.headers['Last-Modified']),
('If-Match', response.headers['Etag']),
],
),
)
self.assert200(response)
response = self.client.get(url)
response = self.client.get(url, headers=self.headers())
#410 would be nice! But, requires a tombstone.
self.assert404(response)

def test_options(self):
mock = self.mock_model()
response = self.client.open(self.mock_url(mock.id), method='OPTIONS')
response = self.client.open(
self.mock_url(mock.id), method='OPTIONS', headers=self.headers())
self.assertOptions(response, RESOURCE_ALLOWED)

def test_collection_options(self):
response = self.client.open(self.mock_url(), method='OPTIONS')
response = self.client.open(
self.mock_url(), method='OPTIONS', headers=self.headers())
self.assertOptions(response, COLLECTION_ALLOWED)

def test_get_bad_accept(self):
mock1 = self.mock_model(foo='baz')
response = self.client.get(
self.mock_url(mock1.id),
headers=[('Accept', 'text/plain')],
headers=self.headers(('Accept', 'text/plain')),
)
self.assertStatus(response, 406)
self.assertEqual('text/plain', response.headers.get('Content-Type'))
Expand All @@ -349,7 +369,7 @@ def test_get_bad_accept(self):
def test_collection_get_bad_accept(self):
response = self.client.get(
URL_MOCK_COLLECTION,
headers=[('Accept', 'text/plain')],
headers=self.headers(('Accept', 'text/plain')),
)
self.assertStatus(response, 406)
self.assertEqual('text/plain', response.headers.get('Content-Type'))
Expand All @@ -359,16 +379,16 @@ def test_get_if_none_match(self):
mock1 = self.mock_model(foo='baz')
response = self.client.get(
self.mock_url(mock1.id),
headers=[('Accept', 'application/json')],
headers=self.headers(('Accept', 'application/json')),
)
self.assert200(response)
previous_headers = dict(response.headers)
response = self.client.get(
self.mock_url(mock1.id),
headers=[
headers=self.headers(
('Accept', 'application/json'),
('If-None-Match', previous_headers['Etag']),
]
),
)
self.assertStatus(response, 304)
self.assertIn('Etag', response.headers)
Expand All @@ -377,16 +397,16 @@ def test_collection_get_if_non_match(self):
self.mock_model(foo='baz')
response = self.client.get(
URL_MOCK_COLLECTION,
headers=[('Accept', 'application/json')],
headers=self.headers(('Accept', 'application/json')),
)
self.assert200(response)
previous_headers = dict(response.headers)
response = self.client.get(
URL_MOCK_COLLECTION,
headers=[
headers=self.headers(
('Accept', 'application/json'),
('If-None-Match', previous_headers['Etag']),
]
),
)
self.assertStatus(response, 304)
self.assertIn('Etag', response.headers)

0 comments on commit 69d4e78

Please # to comment.