From 0e027e6858022589ace11218ce102ce57499e5e6 Mon Sep 17 00:00:00 2001
From: "Kali Kaneko (leap communications)" <kali@leap.se>
Date: Mon, 12 Dec 2016 17:37:30 +0100
Subject: [feature] better param error handling in rest api

---
 src/leap/bitmask/core/dispatcher.py | 31 +++++++++++++++++++++++++------
 src/leap/bitmask/core/dummy.py      | 10 ++++++++--
 src/leap/bitmask/core/web/api.py    |  6 ++++++
 3 files changed, 39 insertions(+), 8 deletions(-)

(limited to 'src')

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):
-- 
cgit v1.2.3