From 0e027e6858022589ace11218ce102ce57499e5e6 Mon Sep 17 00:00:00 2001 From: "Kali Kaneko (leap communications)" Date: Mon, 12 Dec 2016 17:37:30 +0100 Subject: [feature] better param error handling in rest api --- docs/core/index.rst | 40 ++++++++++++++++++++++++++++++------- src/leap/bitmask/core/dispatcher.py | 31 ++++++++++++++++++++++------ src/leap/bitmask/core/dummy.py | 10 ++++++++-- src/leap/bitmask/core/web/api.py | 6 ++++++ tests/unit/core/test_web_api.py | 36 +++++++++++++++++++++++---------- 5 files changed, 98 insertions(+), 25 deletions(-) diff --git a/docs/core/index.rst b/docs/core/index.rst index 5b33d82..b17a432 100644 --- a/docs/core/index.rst +++ b/docs/core/index.rst @@ -12,7 +12,7 @@ The bitmask core daemon can be launched like this:: bitmaskd -The command-line program, ``bitmaskctl``, and the GUI, will launch the +The command-line program ``bitmaskctl`` and the GUI, will launch the daemon when needed. Starting the API server @@ -59,7 +59,7 @@ means that it does not need an authentication header. +------------------------------------+---------------------------------+ | ``POST`` :ref:`cmd_user_create` * | Create a new user | +------------------------------------+---------------------------------+ -| ``POST`` :ref:`cmd_user_update` | Update an user | +| ``POST`` :ref:`cmd_user_update` | Change the user password | +------------------------------------+---------------------------------+ | ``POST`` :ref:`cmd_user_auth` * | Authenticate an user | +------------------------------------+---------------------------------+ @@ -93,6 +93,7 @@ JSON-encoded data to the POST. **Example request**:: curl -X POST localhost:7070/API/core/version + **Example response**:: @@ -155,7 +156,8 @@ JSON-encoded data to the POST. **Example request**:: - curl -X POST localhost:7070/API/bonafide/provider/read -d '["dev.bitmask.net"]' + + curl -X POST localhost:7070/API/bonafide/provider/read -d '["dev.bitmask.net"]' **Example response**:: @@ -224,8 +226,6 @@ JSON-encoded data to the POST. List all the users known to the local backend. **Form parameters**: - * ``foo`` *(required)* - foo bar. - * ``bar`` *(optional)* - foo bar. **Status codes**: * ``200`` - no error @@ -247,7 +247,15 @@ JSON-encoded data to the POST. Create a new user. **Form parameters**: - * ``foo`` *(required)* - foo bar. + * ``username`` *(required)* - in the form user@provider. + * ``pass`` *(required)* - the username passphrase + * ``invitecode`` *(optional)* - an optional invitecode, to be used if + the provider requires it for creating a new account. + * ``autoconf`` *(optional)* - whether to autoconfigure the provider, if + we don't have seen it before. + + **Status codes**: + * ``200`` - no error .. _cmd_user_update: @@ -255,7 +263,15 @@ JSON-encoded data to the POST. --------------------- **POST /bonafide/user/update** - Update a given user. + Change the user password. + + **Form parameters**: + * ``username`` *(required)* - in the form user@provider + * ``oldpass`` *(required)* - current password + * ``newpass`` *(required)* - new password + + **Status codes**: + * ``200`` - no error .. _cmd_user_auth: @@ -265,6 +281,16 @@ JSON-encoded data to the POST. Authenticate an user. + **Form parameters**: + + * ``username`` *(required)* - in the form user@provider + * ``pass`` *(required)* - passphrase + * ``autoconf`` *(optional)* - whether to autoconfigure the provider, if + we don't have seen it before. + + **Status codes**: + * ``200`` - no error + .. _cmd_user_logout: /bonafide/user/logout diff --git a/src/leap/bitmask/core/dispatcher.py b/src/leap/bitmask/core/dispatcher.py index 3a5b5f1..f96bd45 100644 --- a/src/leap/bitmask/core/dispatcher.py +++ b/src/leap/bitmask/core/dispatcher.py @@ -108,7 +108,12 @@ class UserCmd(SubCommand): @register_method("{'srp_token': unicode, 'uuid': unicode " "'lcl_token': unicode}") def do_AUTHENTICATE(self, bonafide, *parts): - user, password = parts[2], parts[3] + try: + user, password = parts[2], parts[3] + except IndexError: + raise DispatchError( + 'wrong number of arguments: expected 2 or 3, got %s' % ( + len(parts[2:]))) autoconf = False if len(parts) > 4: if parts[4] == 'true': @@ -117,17 +122,21 @@ class UserCmd(SubCommand): # FIXME We still SHOULD pass a local token # even if the SRP authentication times out!!! def add_local_token(result): - result['lcl_token'] = bonafide.local_tokens[user] + result['lcl_token'] = bonafide.local_tokens.get(user) return result - d = bonafide.do_authenticate(user, password, autoconf) + d = defer.maybeDeferred( + bonafide.do_authenticate, user, password, autoconf) d.addCallback(add_local_token) return d @register_method("{'signup': 'ok', 'user': str}") def do_CREATE(self, bonafide, *parts): if len(parts) < 5: - raise DispatchError('Not enough parameters passed') + raise DispatchError( + 'wrong number of arguments: expected min 3, got %s' % ( + len(parts[2:]))) + # params are: [user, create, full_id, password, invite, autoconf] user, password, invite = parts[2], parts[3], parts[4] @@ -142,7 +151,12 @@ class UserCmd(SubCommand): @register_method("{'logout': 'ok'}") def do_LOGOUT(self, bonafide, *parts): - user = parts[2] + try: + user = parts[2] + except IndexError: + raise DispatchError( + 'wrong number of arguments: expected 1, got %s' % ( + len(parts[2:]))) return bonafide.do_logout(user) @register_method("[{'userid': str, 'authenticated': bool}]") @@ -151,7 +165,12 @@ class UserCmd(SubCommand): @register_method("{'update': 'ok'}") def do_UPDATE(self, bonafide, *parts): - user, current_password, new_password = parts[2], parts[3], parts[4] + try: + user, current_password, new_password = parts[2], parts[3], parts[4] + except IndexError: + raise DispatchError( + 'wrong number of arguments: expected 3, got %s' % ( + len(parts[2:]))) return bonafide.do_change_password( user, current_password, new_password) diff --git a/src/leap/bitmask/core/dummy.py b/src/leap/bitmask/core/dummy.py index 2037b81..51eec1a 100644 --- a/src/leap/bitmask/core/dummy.py +++ b/src/leap/bitmask/core/dummy.py @@ -37,6 +37,7 @@ class CannedData: class bonafide: auth = { + u'lcl_token': u'deadbeef', u'srp_token': u'deadbeef123456789012345678901234567890123', u'uuid': u'01234567890abcde01234567890abcde'} signup = { @@ -48,6 +49,8 @@ class CannedData: logout = { 'logout': 'ok'} get_active_user = 'dummyuser@provider.example.org' + change_password = { + 'update': 'ok'} class BackendCommands(object): @@ -90,10 +93,10 @@ class BonafideService(HookableService): def __init__(self, basedir): self.canned = CannedData - def do_authenticate(self, user, password): + def do_authenticate(self, user, password, autoconf): return self.canned.bonafide.auth - def do_signup(self, user, password): + def do_signup(self, user, password, invite, autoconf): return self.canned.bonafide.signup def do_list_users(self): @@ -104,3 +107,6 @@ class BonafideService(HookableService): def do_get_active_user(self): return self.canned.bonafide.get_active_user + + def do_change_password(self, username, old, new): + return self.canned.bonafide.change_password diff --git a/src/leap/bitmask/core/web/api.py b/src/leap/bitmask/core/web/api.py index e8bd21e..6c386b5 100644 --- a/src/leap/bitmask/core/web/api.py +++ b/src/leap/bitmask/core/web/api.py @@ -2,6 +2,9 @@ import json from twisted.web.server import NOT_DONE_YET from twisted.web.resource import Resource +from twisted.logger import Logger + +log = Logger() class Api(Resource): @@ -16,6 +19,8 @@ class Api(Resource): command = request.uri.split('/')[2:] params = request.content.getvalue() if params: + # TODO sanitize this + # json.loads returns unicode strings and the rest of the code # expects strings. This 'str(param)' conversion can be removed # if we move to python3 @@ -24,6 +29,7 @@ class Api(Resource): d = self.dispatcher.dispatch(command) d.addCallback(self._write_response, request) + d.addErrback(log.error) return NOT_DONE_YET def _write_response(self, response, request): diff --git a/tests/unit/core/test_web_api.py b/tests/unit/core/test_web_api.py index 02708f6..6d61686 100644 --- a/tests/unit/core/test_web_api.py +++ b/tests/unit/core/test_web_api.py @@ -132,7 +132,9 @@ class RESTApiTests(unittest.TestCase): def setUp(self): dispatcher = dummyDispatcherFactory() api = web.api.Api(dispatcher) - plainSite = Site(api) + root = resource.Resource() + root.putChild(b"API", api) + plainSite = Site(root) self.plainPort = reactor.listenTCP(0, plainSite, interface="127.0.0.1") self.plainPortno = self.plainPort.getHost().port self.canned = CannedData @@ -161,17 +163,23 @@ class RESTApiTests(unittest.TestCase): @defer.inlineCallbacks def test_bonafide_user_create(self): - call = yield self.makeAPICall('bonafide/user/create') - self.assertCall(call, self.canned.bonafide.auth) + params = ['user@provider', 'pass', 'invitecode'] + call = yield self.makeAPICall('bonafide/user/create', + params=params) + self.assertCall(call, self.canned.bonafide.signup) @defer.inlineCallbacks def test_bonafide_user_update(self): - call = yield self.makeAPICall('bonafide/user/update') - self.assertCall(call, self.canned.bonafide.update) + params = ['user@provider', 'oldpass', 'newpass'] + call = yield self.makeAPICall('bonafide/user/update', + params=params) + self.assertCall(call, self.canned.bonafide.change_password) @defer.inlineCallbacks def test_bonafide_user_authenticate(self): - call = yield self.makeAPICall('bonafide/user/authenticate') + params = ['user@provider', 'pass', 'false'] + call = yield self.makeAPICall('bonafide/user/authenticate', + params=params) self.assertCall(call, self.canned.bonafide.auth) @defer.inlineCallbacks @@ -181,13 +189,20 @@ class RESTApiTests(unittest.TestCase): @defer.inlineCallbacks def test_bonafide_user_logout(self): - call = yield self.makeAPICall('bonafide/user/logout') + params = ['user@provider'] + call = yield self.makeAPICall('bonafide/user/logout', + params=params) self.assertCall(call, self.canned.bonafide.logout) - def makeAPICall(self, path, method="POST"): - uri = networkString("http://127.0.0.1:%d/%s" % ( + def makeAPICall(self, path, method="POST", params=None): + if params: + postdata = json.dumps(params) + else: + postdata = None + uri = networkString("http://127.0.0.1:%d/API/%s" % ( self.plainPortno, path)) - return client.getPage(uri, method=method, timeout=1) + return client.getPage( + uri, method=method, timeout=1, postdata=postdata) def assertCall(self, returned, expected): data = json.loads(returned) @@ -219,6 +234,7 @@ class DummyCore(service.MultiService): self.init('mail', mail) self.core_cmds = BackendCommands(self) + self.tokens = {} def init(self, label, service, *args, **kw): s = service(*args, **kw) -- cgit v1.2.3