Skip to content

Commit

Permalink
Only accept 'token' in POST fields (stop using get_arg())
Browse files Browse the repository at this point in the history
  • Loading branch information
meejah authored and warner committed Apr 25, 2016
1 parent afb7718 commit 01b09f3
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
28 changes: 23 additions & 5 deletions src/allmydata/test/test_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -5905,12 +5905,19 @@ def beforeRender(self, ctx):
raise CompletelyUnhandledError("whoops")


# XXX FIXME when we introduce "mock" as a dependency, these can
# probably just be Mock instances
@implementer(IRequest)
class FakeRequest(object):
def __init__(self):
self.method = "POST"
self.fields = dict()
self.args = dict()
self.fields = []


class FakeField(object):
def __init__(self, *values):
self.value = list(values)


class FakeClientWithToken(object):
Expand Down Expand Up @@ -5945,10 +5952,21 @@ def test_missing_token(self):
self.assertEquals(exc.text, "Missing token")
self.assertEquals(exc.code, 401)

def test_token_in_get_args(self):
req = FakeRequest()
req.args['token'] = 'z' * 32

exc = self.assertRaises(
common.WebError,
self.page.renderHTTP, req,
)
self.assertEquals(exc.text, "Do not pass 'token' as URL argument")
self.assertEquals(exc.code, 400)

def test_invalid_token(self):
wrong_token = 'b' * 32
req = FakeRequest()
req.args['token'] = [wrong_token]
req.fields['token'] = FakeField(wrong_token)

exc = self.assertRaises(
common.WebError,
Expand All @@ -5959,7 +5977,7 @@ def test_invalid_token(self):

def test_valid_token_no_t_arg(self):
req = FakeRequest()
req.args['token'] = [self.client.token]
req.fields['token'] = FakeField(self.client.token)

with self.assertRaises(common.WebError) as exc:
self.page.renderHTTP(req)
Expand All @@ -5968,7 +5986,7 @@ def test_valid_token_no_t_arg(self):

def test_valid_token_invalid_t_arg(self):
req = FakeRequest()
req.args['token'] = [self.client.token]
req.fields['token'] = FakeField(self.client.token)
req.args['t'] = 'not at all json'

with self.assertRaises(common.WebError) as exc:
Expand All @@ -5978,7 +5996,7 @@ def test_valid_token_invalid_t_arg(self):

def test_valid(self):
req = FakeRequest()
req.args['token'] = [self.client.token]
req.fields['token'] = FakeField(self.client.token)
req.args['t'] = ['json']

result = self.page.renderHTTP(req)
Expand Down
9 changes: 7 additions & 2 deletions src/allmydata/web/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,13 @@ def renderHTTP(self, ctx):
req = IRequest(ctx)
if req.method != 'POST':
raise server.UnsupportedMethod(('POST',))

token = get_arg(req, "token", None)
if req.args.get('token', False):
raise WebError("Do not pass 'token' as URL argument", http.BAD_REQUEST)
# not using get_arg() here because we *don't* want the token
# argument to work if you passed it as a GET-style argument
token = None
if req.fields and 'token' in req.fields:
token = req.fields['token'].value[0]
if not token:
raise WebError("Missing token", http.UNAUTHORIZED)
if not timing_safe_compare(token, self.client.get_auth_token()):
Expand Down

0 comments on commit 01b09f3

Please sign in to comment.