From 199ffbbfdf36c9560e734b5bad49380d9a3ad0e2 Mon Sep 17 00:00:00 2001 From: Piero Toffanin Date: Wed, 8 Feb 2017 12:30:11 -0500 Subject: [PATCH 1/3] Added JWT auth, fixed processing lock issue wiof removal of tasks from nodes that are offline --- README.md | 4 ++++ app/models.py | 24 ++++++++++++------------ nodeodm/exceptions.py | 8 ++++++++ nodeodm/models.py | 30 ++++++++++++++++-------------- nodeodm/tests.py | 16 ++++++++-------- requirements.txt | 2 ++ webodm/settings.py | 5 +++++ 7 files changed, 55 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 37b0f516..7b79112c 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,10 @@ Have you had other issues? Please [report them](https://github.com/OpenDroneMap/ WebODM can be linked to one or more processing nodes running [node-OpenDroneMap](https://github.com/pierotofy/node-OpenDroneMap). The default configuration already includes a "node-odm-1" processing node which runs on the same machine as WebODM, just to help you get started. As you become more familiar with WebODM, you might want to install processing nodes on separate machines. +### Security + +If you want to run WebODM in production, make sure to change the `SECRET_KEY` variable in `webodm/settings.py`, as well as any other relevant setting as indicated in the [Django Deployment Checklist](https://docs.djangoproject.com/en/1.10/howto/deployment/checklist/). + ## Run it natively If you want to run WebODM natively, you will need to install: diff --git a/app/models.py b/app/models.py index b1cdb4a9..1427c6bb 100644 --- a/app/models.py +++ b/app/models.py @@ -20,7 +20,7 @@ from guardian.shortcuts import get_perms_for_model, assign_perm from app import pending_actions from app.postgis import OffDbRasterField from nodeodm import status_codes -from nodeodm.exceptions import ProcessingException +from nodeodm.exceptions import ProcessingError, ProcessingTimeout, ProcessingException from nodeodm.models import ProcessingNode from webodm import settings @@ -149,7 +149,7 @@ class Task(models.Model): def __str__(self): name = self.name if self.name is not None else "unnamed" - return 'Task {} ({})'.format(name, self.id) + return 'Task [{}] ({})'.format(name, self.id) def save(self, *args, **kwargs): # Autovalidate on save @@ -209,21 +209,21 @@ class Task(models.Model): # TODO: log process has started processing - except ProcessingException as e: + except ProcessingError as e: self.set_failure(str(e)) if self.pending_action is not None: try: if self.pending_action == pending_actions.CANCEL: # Do we need to cancel the task on the processing node? - logger.info("Canceling task {}".format(self)) + logger.info("Canceling {}".format(self)) if self.processing_node and self.uuid: self.processing_node.cancel_task(self.uuid) self.pending_action = None self.status = None self.save() else: - raise ProcessingException("Cannot cancel a task that has no processing node or UUID") + raise ProcessingError("Cannot cancel a task that has no processing node or UUID") elif self.pending_action == pending_actions.RESTART: logger.info("Restarting {}".format(self)) @@ -237,7 +237,7 @@ class Task(models.Model): try: info = self.processing_node.get_task_info(self.uuid) uuid_still_exists = info['uuid'] == self.uuid - except ProcessingException: + except ProcessingError: pass if uuid_still_exists: @@ -257,10 +257,10 @@ class Task(models.Model): self.pending_action = None self.save() else: - raise ProcessingException("Cannot restart a task that has no processing node or UUID") + raise ProcessingError("Cannot restart a task that has no processing node or UUID") elif self.pending_action == pending_actions.REMOVE: - logger.info("Removing task {}".format(self)) + logger.info("Removing {}".format(self)) if self.processing_node and self.uuid: # Attempt to delete the resources on the processing node # We don't care if this fails, as resources on processing nodes @@ -276,7 +276,7 @@ class Task(models.Model): # Stop right here! return - except ProcessingException as e: + except ProcessingError as e: self.last_error = str(e) self.save() @@ -336,7 +336,7 @@ class Task(models.Model): logger.info("Imported orthophoto {} for {}".format(orthophoto_4326_path, self)) self.save() - except ProcessingException as e: + except ProcessingError as e: self.set_failure(str(e)) else: # FAILED, CANCELED @@ -344,11 +344,11 @@ class Task(models.Model): else: # Still waiting... self.save() - except ProcessingException as e: + except ProcessingError as e: self.set_failure(str(e)) except (ConnectionRefusedError, ConnectionError) as e: logger.warning("{} cannot communicate with processing node: {}".format(self, str(e))) - except requests.exceptions.ConnectTimeout as e: + except ProcessingTimeout as e: logger.warning("{} timed out with error: {}. We'll try reprocessing at the next tick.".format(self, str(e))) diff --git a/nodeodm/exceptions.py b/nodeodm/exceptions.py index 77c36115..313e5fbd 100644 --- a/nodeodm/exceptions.py +++ b/nodeodm/exceptions.py @@ -1,2 +1,10 @@ class ProcessingException(Exception): pass + + +class ProcessingError(ProcessingException): + pass + + +class ProcessingTimeout(ProcessingException): + pass \ No newline at end of file diff --git a/nodeodm/models.py b/nodeodm/models.py index 174e1c70..17d5ab7e 100644 --- a/nodeodm/models.py +++ b/nodeodm/models.py @@ -12,7 +12,7 @@ from .api_client import ApiClient import json from django.db.models import signals from requests.exceptions import ConnectionError -from .exceptions import ProcessingException +from .exceptions import ProcessingError, ProcessingTimeout import simplejson def api(func): """ @@ -23,7 +23,9 @@ def api(func): try: return func(*args, **kwargs) except (json.decoder.JSONDecodeError, simplejson.JSONDecodeError) as e: - raise ProcessingException(str(e)) + raise ProcessingError(str(e)) + except (requests.exceptions.ConnectTimeout, requests.exceptions.ConnectionError) as e: + raise ProcessingTimeout(str(e)) return wrapper @@ -83,20 +85,20 @@ class ProcessingNode(models.Model): :returns UUID of the newly created task """ - if len(images) < 2: raise ProcessingException("Need at least 2 images") + if len(images) < 2: raise ProcessingError("Need at least 2 images") api_client = self.api_client() try: result = api_client.new_task(images, name, options) except requests.exceptions.ConnectionError as e: - raise ProcessingException(e) + raise ProcessingError(e) if isinstance(result, dict) and 'uuid' in result: return result['uuid'] elif isinstance(result, dict) and 'error' in result: - raise ProcessingException(result['error']) + raise ProcessingError(result['error']) else: - raise ProcessingException("Unexpected answer from server: {}".format(result)) + raise ProcessingError("Unexpected answer from server: {}".format(result)) @api def get_task_info(self, uuid): @@ -110,9 +112,9 @@ class ProcessingNode(models.Model): if isinstance(result, dict) and 'uuid' in result: return result elif isinstance(result, dict) and 'error' in result: - raise ProcessingException(result['error']) + raise ProcessingError(result['error']) else: - raise ProcessingException("Unknown result from task info: {}".format(result)) + raise ProcessingError("Unknown result from task info: {}".format(result)) @api def get_task_console_output(self, uuid, line): @@ -123,11 +125,11 @@ class ProcessingNode(models.Model): api_client = self.api_client() result = api_client.task_output(uuid, line) if isinstance(result, dict) and 'error' in result: - raise ProcessingException(result['error']) + raise ProcessingError(result['error']) elif isinstance(result, list): return "".join(result) else: - raise ProcessingException("Unknown response for console output: {}".format(result)) + raise ProcessingError("Unknown response for console output: {}".format(result)) @api def cancel_task(self, uuid): @@ -153,7 +155,7 @@ class ProcessingNode(models.Model): api_client = self.api_client() res = api_client.task_download(uuid, asset) if isinstance(res, dict) and 'error' in res: - raise ProcessingException(res['error']) + raise ProcessingError(res['error']) else: return res @@ -174,11 +176,11 @@ class ProcessingNode(models.Model): :return: True on success, raises ProcessingException otherwise """ if isinstance(result, dict) and 'error' in result: - raise ProcessingException(result['error']) + raise ProcessingError(result['error']) elif isinstance(result, dict) and 'success' in result: return True else: - raise ProcessingException("Unknown response: {}".format(result)) + raise ProcessingError("Unknown response: {}".format(result)) class Meta: permissions = ( @@ -192,7 +194,7 @@ def auto_update_node_info(sender, instance, created, **kwargs): if created: try: instance.update_node_info() - except ProcessingException: + except ProcessingError: pass class ProcessingNodeUserObjectPermission(UserObjectPermissionBase): diff --git a/nodeodm/tests.py b/nodeodm/tests.py index c6e46921..3ff740ac 100644 --- a/nodeodm/tests.py +++ b/nodeodm/tests.py @@ -6,7 +6,7 @@ from os import path from .models import ProcessingNode from .api_client import ApiClient from requests.exceptions import ConnectionError -from .exceptions import ProcessingException +from .exceptions import ProcessingError from . import status_codes current_dir = path.dirname(path.realpath(__file__)) @@ -80,7 +80,7 @@ class TestClientApi(TestCase): task_info = api.task_info(uuid) if task_info['status']['code'] == status: return True - except ProcessingException: + except ProcessingError: pass time.sleep(0.5) @@ -120,25 +120,25 @@ class TestClientApi(TestCase): self.assertTrue(isinstance(api.task_output(uuid, 0), list)) self.assertTrue(isinstance(online_node.get_task_console_output(uuid, 0), str)) - self.assertRaises(ProcessingException, online_node.get_task_console_output, "wrong-uuid", 0) + self.assertRaises(ProcessingError, online_node.get_task_console_output, "wrong-uuid", 0) # Can restart task self.assertTrue(online_node.restart_task(uuid)) - self.assertRaises(ProcessingException, online_node.restart_task, "wrong-uuid") + self.assertRaises(ProcessingError, online_node.restart_task, "wrong-uuid") wait_for_status(api, uuid, status_codes.COMPLETED, 10, "Could not restart task") # Can cancel task (should work even if we completed the task) self.assertTrue(online_node.cancel_task(uuid)) - self.assertRaises(ProcessingException, online_node.cancel_task, "wrong-uuid") + self.assertRaises(ProcessingError, online_node.cancel_task, "wrong-uuid") # Wait for task to be canceled wait_for_status(api, uuid, status_codes.CANCELED, 5, "Could not remove task") self.assertTrue(online_node.remove_task(uuid)) - self.assertRaises(ProcessingException, online_node.remove_task, "wrong-uuid") + self.assertRaises(ProcessingError, online_node.remove_task, "wrong-uuid") # Cannot delete task again - self.assertRaises(ProcessingException, online_node.remove_task, uuid) + self.assertRaises(ProcessingError, online_node.remove_task, uuid) # Task has been deleted - self.assertRaises(ProcessingException, online_node.get_task_info, uuid) \ No newline at end of file + self.assertRaises(ProcessingError, online_node.get_task_info, uuid) \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index d5b73920..b9dc38bb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,6 +9,7 @@ django-guardian==1.4.6 django-rest-swagger==2.1.0 django-webpack-loader==0.3.3 djangorestframework==3.5.1 +djangorestframework-jwt==1.9.0 drf-nested-routers==0.11.1 funcsigs==1.0.2 futures==3.0.5 @@ -19,6 +20,7 @@ packaging==16.8 Pillow==3.3.1 pip-autoremove==0.9.0 psycopg2==2.6.2 +PyJWT==1.4.2 pyparsing==2.1.10 pytz==2016.6.1 requests==2.11.1 diff --git a/webodm/settings.py b/webodm/settings.py index a91cb304..4983d013 100644 --- a/webodm/settings.py +++ b/webodm/settings.py @@ -220,6 +220,11 @@ REST_FRAMEWORK = { 'rest_framework.filters.DjangoFilterBackend', 'rest_framework.filters.OrderingFilter', ], + 'DEFAULT_AUTHENTICATION_CLASSES': ( + 'rest_framework.authentication.SessionAuthentication', + 'rest_framework.authentication.BasicAuthentication', + 'rest_framework_jwt.authentication.JSONWebTokenAuthentication', + ), 'PAGE_SIZE': 10, } From 75153c18f680eb267eb1e8859c51e63203e98d96 Mon Sep 17 00:00:00 2001 From: Piero Toffanin Date: Wed, 8 Feb 2017 14:14:24 -0500 Subject: [PATCH 2/3] Test access tokens, lengthened expiration time --- app/api/urls.py | 2 ++ app/tests/test_api.py | 49 +++++++++++++++++++++++++++++++++++-------- webodm/settings.py | 6 ++++++ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/app/api/urls.py b/app/api/urls.py index 4ce21dd5..831fa202 100644 --- a/app/api/urls.py +++ b/app/api/urls.py @@ -3,6 +3,7 @@ from .projects import ProjectViewSet from .tasks import TaskViewSet, TaskTiles, TaskTilesJson, TaskDownloads, TaskAssets from .processingnodes import ProcessingNodeViewSet from rest_framework_nested import routers +from rest_framework_jwt.views import obtain_jwt_token router = routers.DefaultRouter() router.register(r'projects', ProjectViewSet) @@ -21,4 +22,5 @@ urlpatterns = [ url(r'projects/(?P[^/.]+)/tasks/(?P[^/.]+)/assets/(?P.+)$', TaskAssets.as_view()), url(r'^auth/', include('rest_framework.urls')), + url(r'^token-auth/', obtain_jwt_token), ] \ No newline at end of file diff --git a/app/tests/test_api.py b/app/tests/test_api.py index 5dd83120..38d57b47 100644 --- a/app/tests/test_api.py +++ b/app/tests/test_api.py @@ -1,18 +1,15 @@ import datetime -import subprocess +from django.contrib.auth.models import User from guardian.shortcuts import assign_perm +from rest_framework import status +from rest_framework.test import APIClient +from rest_framework_jwt.settings import api_settings from app import pending_actions -from nodeodm import status_codes -from .classes import BootTestCase -from rest_framework.test import APIClient -from rest_framework import status -import time, os - -from app.models import Project, Task, ImageUpload +from app.models import Project, Task from nodeodm.models import ProcessingNode -from django.contrib.auth.models import User +from .classes import BootTestCase class TestApi(BootTestCase): @@ -310,3 +307,37 @@ class TestApi(BootTestCase): self.assertTrue(len(res.data) == 2) self.assertTrue(res.data[1]["port"] == 1000) + def test_token_auth(self): + client = APIClient() + + pnode = ProcessingNode.objects.create( + hostname="localhost", + port=999 + ) + + # Cannot access resources + res = client.get('/api/processingnodes/') + self.assertEqual(res.status_code, status.HTTP_403_FORBIDDEN) + + # Cannot generate token with invalid credentials + res = client.post('/api/token-auth/', { + 'username': 'testuser', + 'password': 'wrongpwd' + }) + self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST) + + # Can generate token with invalid credentials + res = client.post('/api/token-auth/', { + 'username': 'testuser', + 'password': 'test1234' + }) + self.assertEqual(res.status_code, status.HTTP_200_OK) + + token = res.data['token'] + self.assertTrue(len(token) > 0) + + # Can access resources by passing token + client = APIClient(HTTP_AUTHORIZATION="{0} {1}".format(api_settings.JWT_AUTH_HEADER_PREFIX, token)) + res = client.get('/api/processingnodes/') + self.assertEqual(res.status_code, status.HTTP_200_OK) + diff --git a/webodm/settings.py b/webodm/settings.py index 4983d013..4ac7fe0f 100644 --- a/webodm/settings.py +++ b/webodm/settings.py @@ -11,6 +11,8 @@ https://docs.djangoproject.com/en/1.10/ref/settings/ """ import os, sys + +import datetime from django.contrib.messages import constants as messages # Build paths inside the project like this: os.path.join(BASE_DIR, ...) @@ -228,6 +230,10 @@ REST_FRAMEWORK = { 'PAGE_SIZE': 10, } +JWT_AUTH = { + 'JWT_EXPIRATION_DELTA': datetime.timedelta(hours=6), +} + TESTING = sys.argv[1:2] == ['test'] if TESTING: MEDIA_ROOT = os.path.join(BASE_DIR, 'app', 'media_test') From ab4020e5193e2f0308a89fc61ddae075bedb1bad Mon Sep 17 00:00:00 2001 From: Piero Toffanin Date: Wed, 8 Feb 2017 14:18:17 -0500 Subject: [PATCH 3/3] Typo --- app/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/tests/test_api.py b/app/tests/test_api.py index 38d57b47..28681137 100644 --- a/app/tests/test_api.py +++ b/app/tests/test_api.py @@ -326,7 +326,7 @@ class TestApi(BootTestCase): }) self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST) - # Can generate token with invalid credentials + # Can generate token with valid credentials res = client.post('/api/token-auth/', { 'username': 'testuser', 'password': 'test1234'