From a10255c8d37515fc4d02e212c36a268ea01157c2 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Tue, 17 Aug 2021 19:38:05 -0400 Subject: [PATCH 01/33] WIP --- multinet/api/utils/arango.py | 8 ++++++-- multinet/api/views/workspace.py | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index 7e0c13b..9acae1f 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -23,8 +23,10 @@ def arango_client(): return ArangoClient(hosts=settings.MULTINET_ARANGO_URL, http_client=NoTimeoutHttpClient()) -def db(name: str): - return arango_client().db(name, username='root', password=settings.MULTINET_ARANGO_PASSWORD) +def db(name: str, username: Optional[str] = None, password: Optional[str] = None): + username = username or 'root' + password = password or settings.MULTINET_ARANGO_PASSWORD + return arango_client().db(name, username=username, password=password) @lru_cache() @@ -57,6 +59,8 @@ def __init__( db: StandardDatabase, query_str: Optional[str] = None, bind_vars: Optional[Dict[str, str]] = None, + time_limit_secs: int = 30, + memory_limit_mb: int = 200, ) -> None: self.db = db self.query_str = query_str diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 628150b..4ef269f 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -4,6 +4,7 @@ from django.db.models import Q from django.shortcuts import get_object_or_404 from django_filters import rest_framework as filters +from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema from rest_framework import status from rest_framework.decorators import action @@ -173,3 +174,17 @@ def put_workspace_permissions(self, request, name: str): workspace.set_owner(new_owner) return Response(PermissionsReturnSerializer(workspace).data, status=status.HTTP_200_OK) + + @swagger_auto_schema(manual_parameters=[openapi.Parameter('query', 'query', type='string')]) + @action(detail=True) + @require_workspace_permission(WorkspaceRoleChoice.READER) + def aql(self, request, name: str): + """Execute AQL in a workspace.""" + query = request.query_params.get('query') + if query is None: + return Response(None) + + workspace: Workspace = get_object_or_404(Workspace, name=name) + + # FIX + return Response(None, status=status.HTTP_200_OK) From a3915c524875c9ff537fceba40b17624ec471109 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Mon, 23 Aug 2021 17:28:37 -0400 Subject: [PATCH 02/33] Implement AQL endpoint (WIP) Initial implementation of AQL endpoint. Currently doesn't quite work due to generators not being serializable. Also adds environment variables for the readonly arango user. --- dev/.env.docker-compose | 1 + dev/.env.docker-compose-native | 1 + multinet/api/models/workspace.py | 10 +++++++++- multinet/api/utils/arango.py | 30 ++++++++++++++++++++++++++++-- multinet/api/views/workspace.py | 19 ++++++++++++++----- multinet/settings.py | 1 + 6 files changed, 54 insertions(+), 8 deletions(-) diff --git a/dev/.env.docker-compose b/dev/.env.docker-compose index 5234ae7..29dd5f7 100644 --- a/dev/.env.docker-compose +++ b/dev/.env.docker-compose @@ -8,3 +8,4 @@ DJANGO_STORAGE_BUCKET_NAME=django-storage DJANGO_MINIO_STORAGE_MEDIA_URL=http://localhost:9000/django-storage DJANGO_MULTINET_ARANGO_URL=http://arangodb:8529 DJANGO_MULTINET_ARANGO_PASSWORD=letmein +DJANGO_MULTINET_ARANGO_READONLY_PASSWORD=letmein diff --git a/dev/.env.docker-compose-native b/dev/.env.docker-compose-native index c80606f..04c2ae7 100644 --- a/dev/.env.docker-compose-native +++ b/dev/.env.docker-compose-native @@ -7,3 +7,4 @@ DJANGO_MINIO_STORAGE_SECRET_KEY=minioSecretKey DJANGO_STORAGE_BUCKET_NAME=django-storage DJANGO_MULTINET_ARANGO_URL=http://localhost:8529 DJANGO_MULTINET_ARANGO_PASSWORD=letmein +DJANGO_MULTINET_ARANGO_READONLY_PASSWORD=letmein diff --git a/multinet/api/models/workspace.py b/multinet/api/models/workspace.py index b1cf279..4d97f99 100644 --- a/multinet/api/models/workspace.py +++ b/multinet/api/models/workspace.py @@ -10,7 +10,12 @@ from django.dispatch import receiver from django_extensions.db.models import TimeStampedModel -from multinet.api.utils.arango import ensure_db_created, ensure_db_deleted, get_or_create_db +from multinet.api.utils.arango import ( + ensure_db_created, + ensure_db_deleted, + get_or_create_db, + get_or_create_db_readonly, +) def create_default_arango_db_name(): @@ -168,6 +173,9 @@ def set_user_permissions_bulk( def get_arango_db(self) -> StandardDatabase: return get_or_create_db(self.arango_db_name) + def get_arango_db_readonly(self) -> StandardDatabase: + return get_or_create_db_readonly(self.arango_db_name) + def __str__(self) -> str: return self.name diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index 9acae1f..0aa3879 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -1,8 +1,9 @@ from __future__ import annotations from functools import lru_cache -from typing import Dict, List, Optional +from typing import Dict, Generator, List, Optional import uuid +import json from arango import ArangoClient from arango.cursor import Cursor @@ -51,6 +52,21 @@ def get_or_create_db(name: str) -> StandardDatabase: return db(name) +def get_or_create_db_readonly(name: str) -> StandardDatabase: + ensure_db_created(name) + return db(name, 'readonly', settings.MULTINET_ARANGO_READONLY_PASSWORD) + + +def generate_query_result(cursor: Cursor) -> Generator: + + # comma = "" + for row in cursor: + # yield f"{comma}{json.dumps(row)}" + print(type(row)) + yield f'{json.dumps(row)}' + # comma = "," + + class ArangoQuery: """A class to represent an AQL query.""" @@ -60,11 +76,13 @@ def __init__( query_str: Optional[str] = None, bind_vars: Optional[Dict[str, str]] = None, time_limit_secs: int = 30, - memory_limit_mb: int = 200, + memory_limit_bytes: int = 20000000, # 20MB ) -> None: self.db = db self.query_str = query_str self.bind_vars = bind_vars + self.time_limit_secs = time_limit_secs + self.memory_limit_bytes = memory_limit_bytes @staticmethod def from_collections(db: StandardDatabase, collections: List[str]) -> ArangoQuery: @@ -136,4 +154,12 @@ def execute(self, **kwargs) -> Cursor: Accepts the same keyword arguments as `arango.database.StandardDatabase.aql.execute`. """ + + # Use time and memory limit of the query object unless different values + # are explicitly passed. + if 'max_runtime' not in kwargs: + kwargs['max_runtime'] = self.time_limit_secs + if 'memory_limit' not in kwargs: + kwargs['memory_limit'] = self.memory_limit_bytes + return self.db.aql.execute(query=self.query_str, bind_vars=self.bind_vars, **kwargs) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 4ef269f..1d62447 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -1,5 +1,7 @@ from typing import OrderedDict +import json +from arango.cursor import Cursor from django.contrib.auth.models import User from django.db.models import Q from django.shortcuts import get_object_or_404 @@ -13,6 +15,7 @@ from rest_framework.viewsets import ReadOnlyModelViewSet from multinet.api.models import Workspace, WorkspaceRole, WorkspaceRoleChoice +from multinet.api.utils.arango import ArangoQuery, generate_query_result from multinet.api.views.serializers import ( PermissionsCreateSerializer, PermissionsReturnSerializer, @@ -180,11 +183,17 @@ def put_workspace_permissions(self, request, name: str): @require_workspace_permission(WorkspaceRoleChoice.READER) def aql(self, request, name: str): """Execute AQL in a workspace.""" - query = request.query_params.get('query') - if query is None: + query_str = request.query_params.get('query') + if query_str is None: return Response(None) workspace: Workspace = get_object_or_404(Workspace, name=name) - - # FIX - return Response(None, status=status.HTTP_200_OK) + database = workspace.get_arango_db_readonly() + query = ArangoQuery(database, query_str) + cursor: Cursor = query.execute() + + return Response( + json.dumps(generate_query_result(cursor)), + content_type='application/json', + status=status.HTTP_200_OK, + ) diff --git a/multinet/settings.py b/multinet/settings.py index 55a3ba1..9b7a644 100644 --- a/multinet/settings.py +++ b/multinet/settings.py @@ -38,6 +38,7 @@ def before_binding(configuration: ComposedConfiguration) -> None: MULTINET_ARANGO_URL = values.Value(environ_required=True) MULTINET_ARANGO_PASSWORD = values.Value(environ_required=True) + MULTINET_ARANGO_READONLY_PASSWORD = values.Value(environ_required=True) SWAGGER_SETTINGS = { 'DEFAULT_AUTO_SCHEMA_CLASS': 'multinet.api.utils.swagger.ImprovedAutoSchema' } From ff8017c26f6c4430bd026f66551976e3e731d1bb Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 24 Aug 2021 11:49:20 -0400 Subject: [PATCH 03/33] Create wrapper class to stream cursor --- multinet/api/utils/arango.py | 19 ++++++++++--------- multinet/api/views/workspace.py | 6 +++--- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index 0aa3879..b9c5363 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -1,9 +1,8 @@ from __future__ import annotations from functools import lru_cache -from typing import Dict, Generator, List, Optional +from typing import Dict, List, Optional import uuid -import json from arango import ArangoClient from arango.cursor import Cursor @@ -57,14 +56,16 @@ def get_or_create_db_readonly(name: str) -> StandardDatabase: return db(name, 'readonly', settings.MULTINET_ARANGO_READONLY_PASSWORD) -def generate_query_result(cursor: Cursor) -> Generator: +class QueryStream: + def __init__(self, cursor: Cursor): + self.cursor = cursor - # comma = "" - for row in cursor: - # yield f"{comma}{json.dumps(row)}" - print(type(row)) - yield f'{json.dumps(row)}' - # comma = "," + def __iter__(self): + for row in self.cursor: + yield row + + def __len__(self): + self.cursor.count() class ArangoQuery: diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 1d62447..6bde19b 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -1,5 +1,4 @@ from typing import OrderedDict -import json from arango.cursor import Cursor from django.contrib.auth.models import User @@ -15,7 +14,7 @@ from rest_framework.viewsets import ReadOnlyModelViewSet from multinet.api.models import Workspace, WorkspaceRole, WorkspaceRoleChoice -from multinet.api.utils.arango import ArangoQuery, generate_query_result +from multinet.api.utils.arango import ArangoQuery, QueryStream from multinet.api.views.serializers import ( PermissionsCreateSerializer, PermissionsReturnSerializer, @@ -191,9 +190,10 @@ def aql(self, request, name: str): database = workspace.get_arango_db_readonly() query = ArangoQuery(database, query_str) cursor: Cursor = query.execute() + query_stream = QueryStream(cursor) return Response( - json.dumps(generate_query_result(cursor)), + query_stream, content_type='application/json', status=status.HTTP_200_OK, ) From e0c704e9c80cb10f13a2da7c276639f341dab3a3 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 24 Aug 2021 11:50:53 -0400 Subject: [PATCH 04/33] Pass cursor directly to response --- multinet/api/utils/arango.py | 12 ------------ multinet/api/views/workspace.py | 5 ++--- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index b9c5363..c2b8f8f 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -56,18 +56,6 @@ def get_or_create_db_readonly(name: str) -> StandardDatabase: return db(name, 'readonly', settings.MULTINET_ARANGO_READONLY_PASSWORD) -class QueryStream: - def __init__(self, cursor: Cursor): - self.cursor = cursor - - def __iter__(self): - for row in self.cursor: - yield row - - def __len__(self): - self.cursor.count() - - class ArangoQuery: """A class to represent an AQL query.""" diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 6bde19b..58cbf27 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -14,7 +14,7 @@ from rest_framework.viewsets import ReadOnlyModelViewSet from multinet.api.models import Workspace, WorkspaceRole, WorkspaceRoleChoice -from multinet.api.utils.arango import ArangoQuery, QueryStream +from multinet.api.utils.arango import ArangoQuery from multinet.api.views.serializers import ( PermissionsCreateSerializer, PermissionsReturnSerializer, @@ -190,10 +190,9 @@ def aql(self, request, name: str): database = workspace.get_arango_db_readonly() query = ArangoQuery(database, query_str) cursor: Cursor = query.execute() - query_stream = QueryStream(cursor) return Response( - query_stream, + cursor, content_type='application/json', status=status.HTTP_200_OK, ) From 328b93cb8726c3a883eb67036823e9956bd3ac44 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 24 Aug 2021 12:09:45 -0400 Subject: [PATCH 05/33] Remove extraneous blank line --- multinet/api/utils/arango.py | 1 - 1 file changed, 1 deletion(-) diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index c2b8f8f..d506809 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -143,7 +143,6 @@ def execute(self, **kwargs) -> Cursor: Accepts the same keyword arguments as `arango.database.StandardDatabase.aql.execute`. """ - # Use time and memory limit of the query object unless different values # are explicitly passed. if 'max_runtime' not in kwargs: From 586e311958805c6c5ce5d8385b245d860a7adcce Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 24 Aug 2021 12:48:16 -0400 Subject: [PATCH 06/33] Handle AQLQueryExecuteErrors --- multinet/api/views/workspace.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 58cbf27..8afefd2 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -1,6 +1,7 @@ from typing import OrderedDict from arango.cursor import Cursor +from arango.exceptions import AQLQueryExecuteError from django.contrib.auth.models import User from django.db.models import Q from django.shortcuts import get_object_or_404 @@ -189,10 +190,16 @@ def aql(self, request, name: str): workspace: Workspace = get_object_or_404(Workspace, name=name) database = workspace.get_arango_db_readonly() query = ArangoQuery(database, query_str) - cursor: Cursor = query.execute() - return Response( - cursor, - content_type='application/json', - status=status.HTTP_200_OK, - ) + try: + cursor: Cursor = query.execute() + return Response( + cursor, + content_type='application/json', + status=status.HTTP_200_OK, + ) + except AQLQueryExecuteError: + return Response( + 'Cannot perform and invalid or mutating AQL query', + status=status.HTTP_400_BAD_REQUEST, + ) From fc5e4a71eb0de2af233d9fd05f18a6549cebdca9 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 24 Aug 2021 13:01:59 -0400 Subject: [PATCH 07/33] Add environment variables to tox and circleci AQL queries run by making an API request to the new endpoint at GET /api/workspaces/{workspace}/aql/ are run by the Arango readonly user. New environment variables are added to hold the password for this user. --- .circleci/config.yml | 1 + tox.ini | 2 ++ 2 files changed, 3 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 27eb017..bd02218 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -31,6 +31,7 @@ jobs: DJANGO_MINIO_STORAGE_SECRET_KEY: minioSecretKey DJANGO_MULTINET_ARANGO_URL: http://localhost:8529 DJANGO_MULTINET_ARANGO_PASSWORD: letmein + DJANGO_MULTINET_ARANGO_READONLY_PASSWORD: letmein workflows: version: 2 ci: diff --git a/tox.ini b/tox.ini index 000451f..278ba87 100644 --- a/tox.ini +++ b/tox.ini @@ -47,6 +47,7 @@ passenv = DJANGO_MINIO_STORAGE_SECRET_KEY DJANGO_MULTINET_ARANGO_URL DJANGO_MULTINET_ARANGO_PASSWORD + DJANGO_MULTINET_ARANGO_READONLY_PASSWORD extras = dev deps = @@ -70,6 +71,7 @@ passenv = DJANGO_MINIO_STORAGE_SECRET_KEY DJANGO_MULTINET_ARANGO_URL DJANGO_MULTINET_ARANGO_PASSWORD + DJANGO_MULTINET_ARANGO_READONLY_PASSWORD commands = {envpython} ./manage.py makemigrations --check --dry-run From 923bd5f239e1d58ab825b706689ba5230d7f3d11 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 24 Aug 2021 14:22:29 -0400 Subject: [PATCH 08/33] Improve error handling in AQL endpoint --- multinet/api/views/workspace.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 8afefd2..a387b61 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -1,7 +1,7 @@ from typing import OrderedDict from arango.cursor import Cursor -from arango.exceptions import AQLQueryExecuteError +from arango.exceptions import AQLQueryExecuteError, ArangoServerError from django.contrib.auth.models import User from django.db.models import Q from django.shortcuts import get_object_or_404 @@ -198,8 +198,15 @@ def aql(self, request, name: str): content_type='application/json', status=status.HTTP_200_OK, ) - except AQLQueryExecuteError: + except AQLQueryExecuteError as err: + # Invalid query, too much memory, invalid permissions return Response( - 'Cannot perform and invalid or mutating AQL query', + err.error_message, status=status.HTTP_400_BAD_REQUEST, ) + except ArangoServerError as err: + # Arango server errors unrelated to the client's query + return Response( + err.error_message, + status=err.http_code, + ) From 30478cb8326485d3067932c4b78964a5f026a110 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 24 Aug 2021 17:30:04 -0400 Subject: [PATCH 09/33] Create wrapper class to serialize cursor --- multinet/api/tests/test_workspace.py | 50 ++++++++++++++++++++++++++++ multinet/api/utils/arango.py | 12 +++++++ multinet/api/views/workspace.py | 6 ++-- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/multinet/api/tests/test_workspace.py b/multinet/api/tests/test_workspace.py index 910a535..de17ee9 100644 --- a/multinet/api/tests/test_workspace.py +++ b/multinet/api/tests/test_workspace.py @@ -1,4 +1,6 @@ from typing import Dict, List +import json +from arango.cursor import Cursor from django.contrib.auth.models import User from faker import Faker @@ -14,6 +16,7 @@ from multinet.api.tests.utils import create_users_with_permissions from multinet.api.utils.arango import arango_system_db +from .conftest import populated_table from .fuzzy import TIMESTAMP_RE, workspace_re @@ -415,3 +418,50 @@ def test_workspace_rest_get_user_permission_public( 'permission': WorkspaceRoleChoice.READER.value, 'permission_label': WorkspaceRoleChoice.READER.label, } + + +@pytest.mark.django_db +@pytest.mark.parametrize( + 'permission,is_owner,status_code,success', + [ + (None, False, 404, False), + (WorkspaceRoleChoice.READER, False, 200, True), + (WorkspaceRoleChoice.WRITER, False, 200, True), + (WorkspaceRoleChoice.MAINTAINER, False, 200, True), + (None, True, 200, True), + ], +) +def test_workspace_rest_aql( + workspace: Workspace, + user: User, + authenticated_api_client: APIClient, + permission: WorkspaceRoleChoice, + is_owner: bool, + status_code: int, + success: bool, +): + if permission is not None: + workspace.set_user_permission(user, WorkspaceRoleChoice.READER) + elif is_owner: + workspace.set_owner(user) + + # get some data going to test + node_table = populated_table(workspace, False) + nodes: Cursor = node_table.get_rows() + nodes_list = [] + while not nodes.empty(): + nodes_list.append(nodes.next()) + # try and execute a valid non-mutating query on the data + query = f'FOR document IN {node_table.name} RETURN document' + r = authenticated_api_client.get( + f'/api/workspaces/{workspace.name}/aql/', data={'query': query} + ) + assert r.status_code == status_code + + if success: + results = json.loads(r.data) + for node in nodes_list: + assert node in results + + +# test invalid query diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index d506809..d0aaa6a 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -56,6 +56,18 @@ def get_or_create_db_readonly(name: str) -> StandardDatabase: return db(name, 'readonly', settings.MULTINET_ARANGO_READONLY_PASSWORD) +class QueryStream(list): + def __init__(self, cursor: Cursor): + self.cursor = cursor + + def __iter__(self): + for row in self.cursor: + yield row + + def __len__(self): + return self.cursor.count() + + class ArangoQuery: """A class to represent an AQL query.""" diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index a387b61..71d31c9 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -1,4 +1,5 @@ from typing import OrderedDict +import json from arango.cursor import Cursor from arango.exceptions import AQLQueryExecuteError, ArangoServerError @@ -15,7 +16,7 @@ from rest_framework.viewsets import ReadOnlyModelViewSet from multinet.api.models import Workspace, WorkspaceRole, WorkspaceRoleChoice -from multinet.api.utils.arango import ArangoQuery +from multinet.api.utils.arango import ArangoQuery, QueryStream from multinet.api.views.serializers import ( PermissionsCreateSerializer, PermissionsReturnSerializer, @@ -193,8 +194,9 @@ def aql(self, request, name: str): try: cursor: Cursor = query.execute() + stream: QueryStream = QueryStream(cursor) return Response( - cursor, + json.dumps(stream), content_type='application/json', status=status.HTTP_200_OK, ) From ef40598b9b70569e7f2e0ff31b03a1b35c8fdd06 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 09:54:01 -0400 Subject: [PATCH 10/33] Add test for mutating query Also fix syntax in circleci config file. --- .circleci/config.yml | 2 +- multinet/api/tests/test_workspace.py | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bd02218..34abae6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -15,7 +15,7 @@ jobs: MINIO_SECRET_KEY: minioSecretKey - image: arangodb/arangodb:3.5.6 environment: - ARANGO_ROOT_PASSWORD=letmein + ARANGO_ROOT_PASSWORD: letmein steps: - checkout - run: diff --git a/multinet/api/tests/test_workspace.py b/multinet/api/tests/test_workspace.py index de17ee9..e0de51d 100644 --- a/multinet/api/tests/test_workspace.py +++ b/multinet/api/tests/test_workspace.py @@ -445,7 +445,6 @@ def test_workspace_rest_aql( elif is_owner: workspace.set_owner(user) - # get some data going to test node_table = populated_table(workspace, False) nodes: Cursor = node_table.get_rows() nodes_list = [] @@ -464,4 +463,17 @@ def test_workspace_rest_aql( assert node in results -# test invalid query +@pytest.mark.django_db +def test_workspace_rest_aql_mutating_query( + workspace: Workspace, user: User, authenticated_api_client: APIClient +): + workspace.set_user_permission(user, WorkspaceRoleChoice.READER) + fake = Faker() + + node_table = populated_table(workspace, False) + # Mutating query + query = f'INSERT {{ \'name\': {fake.pystr()} }} INTO {node_table.name}' + r = authenticated_api_client.get( + f'/api/workspaces/{workspace.name}/aql/', data={'query': query} + ) + assert r.status_code == 400 From cbfec503d4ca63ef8ce09fff00222b69eb983e3a Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 13:36:34 -0400 Subject: [PATCH 11/33] Pass cursor object directly to Response --- multinet/api/tests/test_workspace.py | 3 +-- multinet/api/utils/arango.py | 12 ------------ multinet/api/views/workspace.py | 8 +++----- 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/multinet/api/tests/test_workspace.py b/multinet/api/tests/test_workspace.py index e0de51d..51e5b6d 100644 --- a/multinet/api/tests/test_workspace.py +++ b/multinet/api/tests/test_workspace.py @@ -1,5 +1,4 @@ from typing import Dict, List -import json from arango.cursor import Cursor from django.contrib.auth.models import User @@ -458,7 +457,7 @@ def test_workspace_rest_aql( assert r.status_code == status_code if success: - results = json.loads(r.data) + results = r.json() for node in nodes_list: assert node in results diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index d0aaa6a..d506809 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -56,18 +56,6 @@ def get_or_create_db_readonly(name: str) -> StandardDatabase: return db(name, 'readonly', settings.MULTINET_ARANGO_READONLY_PASSWORD) -class QueryStream(list): - def __init__(self, cursor: Cursor): - self.cursor = cursor - - def __iter__(self): - for row in self.cursor: - yield row - - def __len__(self): - return self.cursor.count() - - class ArangoQuery: """A class to represent an AQL query.""" diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 71d31c9..687fc21 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -1,5 +1,4 @@ from typing import OrderedDict -import json from arango.cursor import Cursor from arango.exceptions import AQLQueryExecuteError, ArangoServerError @@ -16,7 +15,7 @@ from rest_framework.viewsets import ReadOnlyModelViewSet from multinet.api.models import Workspace, WorkspaceRole, WorkspaceRoleChoice -from multinet.api.utils.arango import ArangoQuery, QueryStream +from multinet.api.utils.arango import ArangoQuery from multinet.api.views.serializers import ( PermissionsCreateSerializer, PermissionsReturnSerializer, @@ -194,10 +193,9 @@ def aql(self, request, name: str): try: cursor: Cursor = query.execute() - stream: QueryStream = QueryStream(cursor) + # stream: QueryStream = QueryStream(cursor) return Response( - json.dumps(stream), - content_type='application/json', + cursor, status=status.HTTP_200_OK, ) except AQLQueryExecuteError as err: From 62d6826f0d557f5aaf568a1a58a15f83dd7d459f Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 14:54:44 -0400 Subject: [PATCH 12/33] Create command to create a 'readonly' use for Arango Our Django API uses an assumed 'readonly' user to execute arbitrary AQL queries on an Arango database. This new manage.py command allows for easy creation of that user using environment variables. --- multinet/api/management/__init__.py | 0 multinet/api/management/commands/__init__.py | 0 .../commands/createarangoreadonlyuser.py | 25 +++++++++++++++++++ 3 files changed, 25 insertions(+) create mode 100644 multinet/api/management/__init__.py create mode 100644 multinet/api/management/commands/__init__.py create mode 100644 multinet/api/management/commands/createarangoreadonlyuser.py diff --git a/multinet/api/management/__init__.py b/multinet/api/management/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/multinet/api/management/commands/__init__.py b/multinet/api/management/commands/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/multinet/api/management/commands/createarangoreadonlyuser.py b/multinet/api/management/commands/createarangoreadonlyuser.py new file mode 100644 index 0000000..f9d811e --- /dev/null +++ b/multinet/api/management/commands/createarangoreadonlyuser.py @@ -0,0 +1,25 @@ +from arango import ArangoClient +from arango.database import StandardDatabase +from arango.http import DefaultHTTPClient +from django.conf import settings +from django.core.management.base import BaseCommand + +READONLY_USERNAME = 'readonly' + + +class Command(BaseCommand): + help = ( + 'Ensure the existence of a user with universal read-only priveleges on the Arango DB server' + ) + + def handle(self, *args, **kwargs): + arango_client: ArangoClient = ArangoClient( + hosts=settings.MULTINET_ARANGO_URL, http_client=DefaultHTTPClient() + ) + system_db: StandardDatabase = arango_client.db(password=settings.MULTINET_ARANGO_PASSWORD) + readonly_user_exists = system_db.has_user(READONLY_USERNAME) + + if not readonly_user_exists: + system_db.create_user(READONLY_USERNAME, settings.MULTINET_ARANGO_READONLY_PASSWORD) + + system_db.update_permission(username=READONLY_USERNAME, permission='ro', database='*') From c4874c84f4a3f659cf82bfdff3f7027356fcb8a3 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 15:30:22 -0400 Subject: [PATCH 13/33] Add success and failure messages to command --- .../commands/createarangoreadonlyuser.py | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/multinet/api/management/commands/createarangoreadonlyuser.py b/multinet/api/management/commands/createarangoreadonlyuser.py index f9d811e..cdea26c 100644 --- a/multinet/api/management/commands/createarangoreadonlyuser.py +++ b/multinet/api/management/commands/createarangoreadonlyuser.py @@ -1,5 +1,6 @@ from arango import ArangoClient from arango.database import StandardDatabase +from arango.exceptions import ArangoError from arango.http import DefaultHTTPClient from django.conf import settings from django.core.management.base import BaseCommand @@ -13,13 +14,29 @@ class Command(BaseCommand): ) def handle(self, *args, **kwargs): - arango_client: ArangoClient = ArangoClient( - hosts=settings.MULTINET_ARANGO_URL, http_client=DefaultHTTPClient() - ) - system_db: StandardDatabase = arango_client.db(password=settings.MULTINET_ARANGO_PASSWORD) - readonly_user_exists = system_db.has_user(READONLY_USERNAME) + try: + arango_client: ArangoClient = ArangoClient( + hosts=settings.MULTINET_ARANGO_URL, http_client=DefaultHTTPClient() + ) + system_db: StandardDatabase = arango_client.db( + password=settings.MULTINET_ARANGO_PASSWORD + ) + readonly_user_exists = system_db.has_user(READONLY_USERNAME) - if not readonly_user_exists: - system_db.create_user(READONLY_USERNAME, settings.MULTINET_ARANGO_READONLY_PASSWORD) + if not readonly_user_exists: + system_db.create_user(READONLY_USERNAME, settings.MULTINET_ARANGO_READONLY_PASSWORD) + self.stdout.write( + self.style.SUCCESS(f'Successfully created user: \'{READONLY_USERNAME}\'') + ) - system_db.update_permission(username=READONLY_USERNAME, permission='ro', database='*') + system_db.update_permission(username=READONLY_USERNAME, permission='ro', database='*') + self.stdout.write( + self.style.SUCCESS( + 'Successfully set universal read-only permission for user: ' + f'\'{READONLY_USERNAME}\'' + ) + ) + except ArangoError: + self.stderr.write(self.style.ERROR('Failed to communicate with the Arango server')) + except Exception: + self.stderr.write(self.style.ERROR(f'Failed to create user: {READONLY_USERNAME}')) From 850d521c9e103dc27f6e5cc9eb13155ed60a170b Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 16:02:29 -0400 Subject: [PATCH 14/33] Add test for createarangoreadonlyuser --- .../commands/createarangoreadonlyuser.py | 16 ++++++---------- multinet/api/tests/test_command.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 multinet/api/tests/test_command.py diff --git a/multinet/api/management/commands/createarangoreadonlyuser.py b/multinet/api/management/commands/createarangoreadonlyuser.py index cdea26c..4f71b20 100644 --- a/multinet/api/management/commands/createarangoreadonlyuser.py +++ b/multinet/api/management/commands/createarangoreadonlyuser.py @@ -1,10 +1,9 @@ -from arango import ArangoClient -from arango.database import StandardDatabase from arango.exceptions import ArangoError -from arango.http import DefaultHTTPClient from django.conf import settings from django.core.management.base import BaseCommand +from multinet.api.utils.arango import arango_system_db + READONLY_USERNAME = 'readonly' @@ -15,16 +14,13 @@ class Command(BaseCommand): def handle(self, *args, **kwargs): try: - arango_client: ArangoClient = ArangoClient( - hosts=settings.MULTINET_ARANGO_URL, http_client=DefaultHTTPClient() - ) - system_db: StandardDatabase = arango_client.db( - password=settings.MULTINET_ARANGO_PASSWORD - ) + system_db = arango_system_db() readonly_user_exists = system_db.has_user(READONLY_USERNAME) if not readonly_user_exists: - system_db.create_user(READONLY_USERNAME, settings.MULTINET_ARANGO_READONLY_PASSWORD) + system_db.create_user( + READONLY_USERNAME, settings.MULTINET_ARANGO_READONLY_PASSWORD, active=True + ) self.stdout.write( self.style.SUCCESS(f'Successfully created user: \'{READONLY_USERNAME}\'') ) diff --git a/multinet/api/tests/test_command.py b/multinet/api/tests/test_command.py new file mode 100644 index 0000000..acb9dcb --- /dev/null +++ b/multinet/api/tests/test_command.py @@ -0,0 +1,17 @@ +from multinet.api.management.commands.createarangoreadonlyuser import READONLY_USERNAME +from django.core.management import call_command +from multinet.api.utils.arango import arango_system_db + + +def test_createarangoreadonlyuser(): + system_db = arango_system_db() + + if system_db.has_user(READONLY_USERNAME): + system_db.delete_user(READONLY_USERNAME) + assert not system_db.has_user(READONLY_USERNAME) + + call_command('createarangoreadonlyuser') + + assert system_db.has_user(READONLY_USERNAME) + readonly_permissions = system_db.permission(READONLY_USERNAME, '*') + assert readonly_permissions == 'ro' From 18faa2f79c4424f8d1e51db923f065a921e5235d Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 16:16:21 -0400 Subject: [PATCH 15/33] Add CircleCI step to create readonly user --- .circleci/config.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 34abae6..8148c46 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,6 +21,13 @@ jobs: - run: name: Install tox command: sudo pip install tox + - run: + name: Create readonly Arango user + command: ./manage.py createarangoreadonlyuser + environment: + DJANGO_MULTINET_ARANGO_URL: http://localhost:8529 + DJANGO_MULTINET_ARANGO_PASSWORD: letmein + DJANGO_MULTINET_ARANGO_READONLY_PASSWORD: letmein - run: name: Run tests command: tox From 4942ffcde973f42b74b2b4a29e59e0ac7512b2cc Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 16:37:38 -0400 Subject: [PATCH 16/33] Fix linting issues --- .../api/management/commands/createarangoreadonlyuser.py | 4 ++-- multinet/api/tests/test_command.py | 3 ++- multinet/api/tests/test_workspace.py | 8 +++----- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/multinet/api/management/commands/createarangoreadonlyuser.py b/multinet/api/management/commands/createarangoreadonlyuser.py index 4f71b20..066cf45 100644 --- a/multinet/api/management/commands/createarangoreadonlyuser.py +++ b/multinet/api/management/commands/createarangoreadonlyuser.py @@ -22,14 +22,14 @@ def handle(self, *args, **kwargs): READONLY_USERNAME, settings.MULTINET_ARANGO_READONLY_PASSWORD, active=True ) self.stdout.write( - self.style.SUCCESS(f'Successfully created user: \'{READONLY_USERNAME}\'') + self.style.SUCCESS(f'Successfully created user: {READONLY_USERNAME}') ) system_db.update_permission(username=READONLY_USERNAME, permission='ro', database='*') self.stdout.write( self.style.SUCCESS( 'Successfully set universal read-only permission for user: ' - f'\'{READONLY_USERNAME}\'' + f'{READONLY_USERNAME}' ) ) except ArangoError: diff --git a/multinet/api/tests/test_command.py b/multinet/api/tests/test_command.py index acb9dcb..33fdb52 100644 --- a/multinet/api/tests/test_command.py +++ b/multinet/api/tests/test_command.py @@ -1,5 +1,6 @@ -from multinet.api.management.commands.createarangoreadonlyuser import READONLY_USERNAME from django.core.management import call_command + +from multinet.api.management.commands.createarangoreadonlyuser import READONLY_USERNAME from multinet.api.utils.arango import arango_system_db diff --git a/multinet/api/tests/test_workspace.py b/multinet/api/tests/test_workspace.py index 51e5b6d..a666de7 100644 --- a/multinet/api/tests/test_workspace.py +++ b/multinet/api/tests/test_workspace.py @@ -1,6 +1,6 @@ from typing import Dict, List -from arango.cursor import Cursor +from arango.cursor import Cursor from django.contrib.auth.models import User from faker import Faker import pytest @@ -446,9 +446,7 @@ def test_workspace_rest_aql( node_table = populated_table(workspace, False) nodes: Cursor = node_table.get_rows() - nodes_list = [] - while not nodes.empty(): - nodes_list.append(nodes.next()) + nodes_list = list(nodes) # try and execute a valid non-mutating query on the data query = f'FOR document IN {node_table.name} RETURN document' r = authenticated_api_client.get( @@ -471,7 +469,7 @@ def test_workspace_rest_aql_mutating_query( node_table = populated_table(workspace, False) # Mutating query - query = f'INSERT {{ \'name\': {fake.pystr()} }} INTO {node_table.name}' + query = f"INSERT {{ 'name': {fake.pystr()} }} INTO {node_table.name}" r = authenticated_api_client.get( f'/api/workspaces/{workspace.name}/aql/', data={'query': query} ) From 5abbee85b6ee3e55f2a39aa4039d7ffb8edcbef5 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 16:46:07 -0400 Subject: [PATCH 17/33] Use docker-compose to run management command --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8148c46..edd723a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -23,7 +23,7 @@ jobs: command: sudo pip install tox - run: name: Create readonly Arango user - command: ./manage.py createarangoreadonlyuser + command: docker-compose run --rm ./manage.py createarangoreadonlyuser environment: DJANGO_MULTINET_ARANGO_URL: http://localhost:8529 DJANGO_MULTINET_ARANGO_PASSWORD: letmein From 3e16a3d58ed64d55efc18ac406e2312b4b08d18e Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 17:03:02 -0400 Subject: [PATCH 18/33] Remove use of docker-compose for circleci --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index edd723a..8148c46 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -23,7 +23,7 @@ jobs: command: sudo pip install tox - run: name: Create readonly Arango user - command: docker-compose run --rm ./manage.py createarangoreadonlyuser + command: ./manage.py createarangoreadonlyuser environment: DJANGO_MULTINET_ARANGO_URL: http://localhost:8529 DJANGO_MULTINET_ARANGO_PASSWORD: letmein From e61eff82de7e5a1adcb68c614df4fe81f6ee1f2e Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 17:24:38 -0400 Subject: [PATCH 19/33] Create readonly user in tox setup --- .circleci/config.yml | 7 ------- tox.ini | 2 ++ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8148c46..34abae6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,13 +21,6 @@ jobs: - run: name: Install tox command: sudo pip install tox - - run: - name: Create readonly Arango user - command: ./manage.py createarangoreadonlyuser - environment: - DJANGO_MULTINET_ARANGO_URL: http://localhost:8529 - DJANGO_MULTINET_ARANGO_PASSWORD: letmein - DJANGO_MULTINET_ARANGO_READONLY_PASSWORD: letmein - run: name: Run tests command: tox diff --git a/tox.ini b/tox.ini index 278ba87..4ca0e4b 100644 --- a/tox.ini +++ b/tox.ini @@ -57,6 +57,8 @@ deps = pytest-django pytest-factoryboy pytest-mock +commands_pre = + ./manage.py createarangoreadonlyuser commands = pytest {posargs} From 8a29bb334b6daae29a1f6f7e8cc342c37c387fed Mon Sep 17 00:00:00 2001 From: naglepuff Date: Wed, 25 Aug 2021 17:35:21 -0400 Subject: [PATCH 20/33] Set TestingConfiguration for tests --- tox.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tox.ini b/tox.ini index 4ca0e4b..15536e8 100644 --- a/tox.ini +++ b/tox.ini @@ -39,6 +39,8 @@ commands = black {posargs:.} [testenv:test] +setenv = + DJANGO_CONFIGURATION = TestingConfiguration passenv = DJANGO_CELERY_BROKER_URL DJANGO_DATABASE_URL From 5a5b7e6181647ea43f6003f3973123eedde73f6b Mon Sep 17 00:00:00 2001 From: naglepuff Date: Thu, 26 Aug 2021 09:05:49 -0400 Subject: [PATCH 21/33] Remove redundant config in tox In order to properly run the commands_pre commands in the testing environment, we are now seeing the environment directly there. This makes setting the environment under [pytest] redundant. --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index 15536e8..49b275a 100644 --- a/tox.ini +++ b/tox.ini @@ -94,7 +94,6 @@ ignore = [pytest] DJANGO_SETTINGS_MODULE = multinet.settings -DJANGO_CONFIGURATION = TestingConfiguration addopts = --strict-markers --showlocals --verbose filterwarnings = ignore::DeprecationWarning:minio From 43b4bd929659fe8bab96e327b2f32f0d6f09e28a Mon Sep 17 00:00:00 2001 From: naglepuff Date: Thu, 26 Aug 2021 09:37:40 -0400 Subject: [PATCH 22/33] Update release command to create readonly user On release, in addition to running database migrations on the Postgres database, ensure that there exists a 'readonly' user for the Arango database, with universal read-only permissions. This user is used by application code for certain actions like running arbitrary AQL queries via the GET /api/workspaces/{workspace}/aql/ endpoint. --- Procfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Procfile b/Procfile index 8f7a9e8..ce9c55e 100644 --- a/Procfile +++ b/Procfile @@ -1,3 +1,3 @@ -release: ./manage.py migrate +release: ./manage.py migrate && ./manage.py createarangoreadonlyuser web: gunicorn --bind 0.0.0.0:$PORT multinet.wsgi worker: REMAP_SIGTERM=SIGQUIT celery --app multinet.celery worker --loglevel INFO --without-heartbeat From 8e16602f4cda26a3f87fee650a442957502a5b82 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Thu, 26 Aug 2021 10:13:42 -0400 Subject: [PATCH 23/33] Provide error message if no query param --- multinet/api/views/workspace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 687fc21..abc4771 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -185,7 +185,7 @@ def aql(self, request, name: str): """Execute AQL in a workspace.""" query_str = request.query_params.get('query') if query_str is None: - return Response(None) + return Response('No query provided', status=status.HTTP_400_BAD_REQUEST) workspace: Workspace = get_object_or_404(Workspace, name=name) database = workspace.get_arango_db_readonly() From 613d99d389d4930e1b63c9e40f6da96647157ed7 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Thu, 26 Aug 2021 14:08:28 -0400 Subject: [PATCH 24/33] Allow readonly or editable Arango db instances Instead of two functions (one to retrieve a workspace's DB as read-only, and another for an editable DB), use one function with a new parameter to drive read-only state. This involved adding the parameter when retrieving the database for purposes of creating or deleting databases, collections, and documents. --- multinet/api/models/network.py | 6 +++--- multinet/api/models/table.py | 12 ++++++------ multinet/api/models/workspace.py | 8 ++------ multinet/api/utils/arango.py | 19 +++++++++---------- multinet/api/views/workspace.py | 2 +- 5 files changed, 21 insertions(+), 26 deletions(-) diff --git a/multinet/api/models/network.py b/multinet/api/models/network.py index 93d66ea..2e8dbd7 100644 --- a/multinet/api/models/network.py +++ b/multinet/api/models/network.py @@ -68,7 +68,7 @@ def create_with_edge_definition( ) -> Network: """Create a network with an edge definition, using the provided arguments.""" # Create graph in arango before creating the Network object here - workspace.get_arango_db().create_graph( + workspace.get_arango_db(readonly=False).create_graph( name, edge_definitions=[ { @@ -93,7 +93,7 @@ def __str__(self) -> str: def arango_graph_save(sender: Type[Network], instance: Network, **kwargs): workspace: Workspace = instance.workspace - db = workspace.get_arango_db() + db = workspace.get_arango_db(readonly=False) if not db.has_graph(instance.name): db.create_graph(instance.name) @@ -102,6 +102,6 @@ def arango_graph_save(sender: Type[Network], instance: Network, **kwargs): def arango_graph_delete(sender: Type[Network], instance: Network, **kwargs): workspace: Workspace = instance.workspace - db = workspace.get_arango_db() + db = workspace.get_arango_db(readonly=False) if db.has_graph(instance.name): db.delete_graph(instance.name) diff --git a/multinet/api/models/table.py b/multinet/api/models/table.py index 511ace4..32a0874 100644 --- a/multinet/api/models/table.py +++ b/multinet/api/models/table.py @@ -43,9 +43,9 @@ class Meta: def count(self) -> int: return self.get_arango_collection().count() - def get_arango_collection(self) -> StandardCollection: + def get_arango_collection(self, readonly=True) -> StandardCollection: workspace: Workspace = self.workspace - return workspace.get_arango_db().collection(self.name) + return workspace.get_arango_db(readonly=readonly).collection(self.name) def get_row(self, query: Optional[Dict] = None) -> Cursor: """Return a specific document.""" @@ -58,7 +58,7 @@ def get_rows(self, limit: Optional[int] = None, offset: Optional[int] = None) -> def put_rows(self, rows: List[Dict]) -> RowInsertionResponse: """Insert/update rows in the underlying arangodb collection.""" - res = self.get_arango_collection().insert_many(rows, overwrite=True) + res = self.get_arango_collection(readonly=False).insert_many(rows, overwrite=True) errors = [ RowModifyError(index=i, message=doc.error_message) for i, doc in enumerate(res) @@ -70,7 +70,7 @@ def put_rows(self, rows: List[Dict]) -> RowInsertionResponse: def delete_rows(self, rows: List[Dict]) -> RowDeletionResponse: """Delete rows in the underlying arangodb collection.""" - res = self.get_arango_collection().delete_many(rows) + res = self.get_arango_collection(readonly=False).delete_many(rows) errors = [ RowModifyError(index=i, message=doc.error_message) for i, doc in enumerate(res) @@ -127,7 +127,7 @@ def __str__(self) -> str: def arango_coll_save(sender: Type[Table], instance: Table, **kwargs): workspace: Workspace = instance.workspace - db = workspace.get_arango_db() + db = workspace.get_arango_db(readonly=False) if not db.has_collection(instance.name): db.create_collection(instance.name, edge=instance.edge) @@ -136,6 +136,6 @@ def arango_coll_save(sender: Type[Table], instance: Table, **kwargs): def arango_coll_delete(sender: Type[Table], instance: Table, **kwargs): workspace: Workspace = instance.workspace - db = workspace.get_arango_db() + db = workspace.get_arango_db(readonly=False) if db.has_collection(instance.name): db.delete_collection(instance.name) diff --git a/multinet/api/models/workspace.py b/multinet/api/models/workspace.py index 4d97f99..9a51427 100644 --- a/multinet/api/models/workspace.py +++ b/multinet/api/models/workspace.py @@ -14,7 +14,6 @@ ensure_db_created, ensure_db_deleted, get_or_create_db, - get_or_create_db_readonly, ) @@ -170,11 +169,8 @@ def set_user_permissions_bulk( [*new_reader_roles, *new_writer_roles, *new_maintainer_roles] ) - def get_arango_db(self) -> StandardDatabase: - return get_or_create_db(self.arango_db_name) - - def get_arango_db_readonly(self) -> StandardDatabase: - return get_or_create_db_readonly(self.arango_db_name) + def get_arango_db(self, readonly=True) -> StandardDatabase: + return get_or_create_db(self.arango_db_name, readonly) def __str__(self) -> str: return self.name diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index d506809..1215e72 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -23,9 +23,13 @@ def arango_client(): return ArangoClient(hosts=settings.MULTINET_ARANGO_URL, http_client=NoTimeoutHttpClient()) -def db(name: str, username: Optional[str] = None, password: Optional[str] = None): - username = username or 'root' - password = password or settings.MULTINET_ARANGO_PASSWORD +def db(name: str, readonly=False): + username = 'readonly' if readonly else 'root' + password = ( + settings.MULTINET_ARANGO_READONLY_PASSWORD + if readonly + else settings.MULTINET_ARANGO_PASSWORD + ) return arango_client().db(name, username=username, password=password) @@ -46,14 +50,9 @@ def ensure_db_deleted(name: str) -> None: sysdb.delete_database(name) -def get_or_create_db(name: str) -> StandardDatabase: +def get_or_create_db(name: str, readonly=True) -> StandardDatabase: ensure_db_created(name) - return db(name) - - -def get_or_create_db_readonly(name: str) -> StandardDatabase: - ensure_db_created(name) - return db(name, 'readonly', settings.MULTINET_ARANGO_READONLY_PASSWORD) + return db(name, readonly=readonly) class ArangoQuery: diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index abc4771..a83c1f3 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -188,7 +188,7 @@ def aql(self, request, name: str): return Response('No query provided', status=status.HTTP_400_BAD_REQUEST) workspace: Workspace = get_object_or_404(Workspace, name=name) - database = workspace.get_arango_db_readonly() + database = workspace.get_arango_db() query = ArangoQuery(database, query_str) try: From c42f079ad505aeb454153a57e178a34d7ae645ac Mon Sep 17 00:00:00 2001 From: naglepuff Date: Thu, 26 Aug 2021 14:49:05 -0400 Subject: [PATCH 25/33] Update createarangoreadonlyuser command Specifically, make error handling more precise, and explicitly replace the 'readonly' user if it already exists to ensure the password for that user matches the current environment variable. Also fix some linting issues. --- .../commands/createarangoreadonlyuser.py | 36 ++++++++++++++----- multinet/api/models/workspace.py | 6 +--- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/multinet/api/management/commands/createarangoreadonlyuser.py b/multinet/api/management/commands/createarangoreadonlyuser.py index 066cf45..a10f996 100644 --- a/multinet/api/management/commands/createarangoreadonlyuser.py +++ b/multinet/api/management/commands/createarangoreadonlyuser.py @@ -1,4 +1,9 @@ -from arango.exceptions import ArangoError +from arango.exceptions import ( + ArangoClientError, + ArangoServerError, + PermissionUpdateError, + UserCreateError, +) from django.conf import settings from django.core.management.base import BaseCommand @@ -9,21 +14,26 @@ class Command(BaseCommand): help = ( - 'Ensure the existence of a user with universal read-only priveleges on the Arango DB server' + 'Ensure the existence of a user with universal read-only priveleges on the' + 'Arango DB server. Replaces the readonly user with a new one if it already exists.' ) def handle(self, *args, **kwargs): try: + password = settings.MULTINET_ARANGO_READONLY_PASSWORD system_db = arango_system_db() readonly_user_exists = system_db.has_user(READONLY_USERNAME) if not readonly_user_exists: - system_db.create_user( - READONLY_USERNAME, settings.MULTINET_ARANGO_READONLY_PASSWORD, active=True - ) + system_db.create_user(READONLY_USERNAME, password, active=True) self.stdout.write( self.style.SUCCESS(f'Successfully created user: {READONLY_USERNAME}') ) + else: + system_db.replace_user(READONLY_USERNAME, password, active=True) + self.stdout.write( + self.style.SUCCESS(f'Successfully replaced user: {READONLY_USERNAME}') + ) system_db.update_permission(username=READONLY_USERNAME, permission='ro', database='*') self.stdout.write( @@ -32,7 +42,17 @@ def handle(self, *args, **kwargs): f'{READONLY_USERNAME}' ) ) - except ArangoError: - self.stderr.write(self.style.ERROR('Failed to communicate with the Arango server')) - except Exception: + except AttributeError: + self.stderr.write( + self.style.ERROR( + 'Environment variable MULTINET_ARANGO_READONLY_PASSWORD is required.' + ) + ) + except UserCreateError: self.stderr.write(self.style.ERROR(f'Failed to create user: {READONLY_USERNAME}')) + except PermissionUpdateError: + self.stderr.write( + self.style.ERROR(f'Failed to set permissions for user: {READONLY_USERNAME}') + ) + except (ArangoClientError, ArangoServerError) as error: + self.stderr.write(self.style.ERROR(str(error))) diff --git a/multinet/api/models/workspace.py b/multinet/api/models/workspace.py index 9a51427..55783ec 100644 --- a/multinet/api/models/workspace.py +++ b/multinet/api/models/workspace.py @@ -10,11 +10,7 @@ from django.dispatch import receiver from django_extensions.db.models import TimeStampedModel -from multinet.api.utils.arango import ( - ensure_db_created, - ensure_db_deleted, - get_or_create_db, -) +from multinet.api.utils.arango import ensure_db_created, ensure_db_deleted, get_or_create_db def create_default_arango_db_name(): From ee5f2aa757823e818072e028fcc67b3d55ae440a Mon Sep 17 00:00:00 2001 From: Michael Nagler Date: Thu, 26 Aug 2021 14:50:48 -0400 Subject: [PATCH 26/33] Remove commented out code Co-authored-by: Jacob Nesbitt --- multinet/api/views/workspace.py | 1 - 1 file changed, 1 deletion(-) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index abc4771..004c8f5 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -193,7 +193,6 @@ def aql(self, request, name: str): try: cursor: Cursor = query.execute() - # stream: QueryStream = QueryStream(cursor) return Response( cursor, status=status.HTTP_200_OK, From e194e04e73b8bd9d994b4a97561bf6956f4e8807 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Thu, 26 Aug 2021 15:05:18 -0400 Subject: [PATCH 27/33] Update AQL query error handling Default to HTTP 500 response code instead of forwarding the error code from the ArangoDB server. Also clarify some comments. --- multinet/api/views/workspace.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index ff186d6..e39390b 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -198,14 +198,12 @@ def aql(self, request, name: str): status=status.HTTP_200_OK, ) except AQLQueryExecuteError as err: - # Invalid query, too much memory, invalid permissions + # Invalid query, time/memory limit reached, or + # attempt to run a mutating query as the readonly user return Response( err.error_message, status=status.HTTP_400_BAD_REQUEST, ) except ArangoServerError as err: # Arango server errors unrelated to the client's query - return Response( - err.error_message, - status=err.http_code, - ) + return Response(err.error_message, status=status.HTTP_500_INTERNAL_SERVER_ERROR) From 6fd45d44c58a2c66f2377a7dfa2daddb3eee47f7 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 30 Aug 2021 12:16:58 -0400 Subject: [PATCH 28/33] Remove use of get_or_create_db --- multinet/api/models/workspace.py | 10 ++++++++-- multinet/api/utils/arango.py | 5 ----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/multinet/api/models/workspace.py b/multinet/api/models/workspace.py index 55783ec..036ec6a 100644 --- a/multinet/api/models/workspace.py +++ b/multinet/api/models/workspace.py @@ -10,7 +10,7 @@ from django.dispatch import receiver from django_extensions.db.models import TimeStampedModel -from multinet.api.utils.arango import ensure_db_created, ensure_db_deleted, get_or_create_db +from multinet.api.utils.arango import ensure_db_created, ensure_db_deleted, db def create_default_arango_db_name(): @@ -166,7 +166,13 @@ def set_user_permissions_bulk( ) def get_arango_db(self, readonly=True) -> StandardDatabase: - return get_or_create_db(self.arango_db_name, readonly) + """ + Return the arango database associated with this workspace. + + The workspace must be saved before accessing this function. Otherwise, this will result in + errors, as the arango database is created on model save. + """ + return db(self.arango_db_name, readonly) def __str__(self) -> str: return self.name diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index 1215e72..c63572a 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -50,11 +50,6 @@ def ensure_db_deleted(name: str) -> None: sysdb.delete_database(name) -def get_or_create_db(name: str, readonly=True) -> StandardDatabase: - ensure_db_created(name) - return db(name, readonly=readonly) - - class ArangoQuery: """A class to represent an AQL query.""" From b883e2675803a1649cabf334ddb9d1eda16c54b8 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 30 Aug 2021 12:29:05 -0400 Subject: [PATCH 29/33] Remove default for db, add for arango_system_db --- .../management/commands/createarangoreadonlyuser.py | 2 +- multinet/api/tests/conftest.py | 5 +++-- multinet/api/tests/test_command.py | 2 +- multinet/api/utils/arango.py | 10 +++++----- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/multinet/api/management/commands/createarangoreadonlyuser.py b/multinet/api/management/commands/createarangoreadonlyuser.py index a10f996..e71b686 100644 --- a/multinet/api/management/commands/createarangoreadonlyuser.py +++ b/multinet/api/management/commands/createarangoreadonlyuser.py @@ -21,7 +21,7 @@ class Command(BaseCommand): def handle(self, *args, **kwargs): try: password = settings.MULTINET_ARANGO_READONLY_PASSWORD - system_db = arango_system_db() + system_db = arango_system_db(readonly=False) readonly_user_exists = system_db.has_user(READONLY_USERNAME) if not readonly_user_exists: diff --git a/multinet/api/tests/conftest.py b/multinet/api/tests/conftest.py index b4f05ad..c11a279 100644 --- a/multinet/api/tests/conftest.py +++ b/multinet/api/tests/conftest.py @@ -88,9 +88,10 @@ def pytest_sessionfinish(session, exitstatus): # `pytest.mark.django_db` decorator doesn't run the model save/delete methods, meaning the sync # between arangodb and django doesn't happen. - for db in arango_system_db().databases(): + system_db = arango_system_db(readonly=False) + for db in system_db.databases(): if db not in pytest.before_session_arango_databases: - arango_system_db().delete_database(db, ignore_missing=True) + system_db.delete_database(db, ignore_missing=True) register(UserFactory) diff --git a/multinet/api/tests/test_command.py b/multinet/api/tests/test_command.py index 33fdb52..51d8795 100644 --- a/multinet/api/tests/test_command.py +++ b/multinet/api/tests/test_command.py @@ -5,7 +5,7 @@ def test_createarangoreadonlyuser(): - system_db = arango_system_db() + system_db = arango_system_db(readonly=False) if system_db.has_user(READONLY_USERNAME): system_db.delete_user(READONLY_USERNAME) diff --git a/multinet/api/utils/arango.py b/multinet/api/utils/arango.py index c63572a..c0a1729 100644 --- a/multinet/api/utils/arango.py +++ b/multinet/api/utils/arango.py @@ -23,7 +23,7 @@ def arango_client(): return ArangoClient(hosts=settings.MULTINET_ARANGO_URL, http_client=NoTimeoutHttpClient()) -def db(name: str, readonly=False): +def db(name: str, readonly): username = 'readonly' if readonly else 'root' password = ( settings.MULTINET_ARANGO_READONLY_PASSWORD @@ -34,18 +34,18 @@ def db(name: str, readonly=False): @lru_cache() -def arango_system_db(): - return db('_system') +def arango_system_db(readonly=True): + return db('_system', readonly) def ensure_db_created(name: str) -> None: - sysdb = arango_system_db() + sysdb = arango_system_db(readonly=False) if not sysdb.has_database(name): sysdb.create_database(name) def ensure_db_deleted(name: str) -> None: - sysdb = arango_system_db() + sysdb = arango_system_db(readonly=False) if sysdb.has_database(name): sysdb.delete_database(name) From e2b6fdc63c420c48e4570158e4625fb509312656 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 30 Aug 2021 12:40:53 -0400 Subject: [PATCH 30/33] Fix linting --- multinet/api/models/workspace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multinet/api/models/workspace.py b/multinet/api/models/workspace.py index 036ec6a..6e5fa3a 100644 --- a/multinet/api/models/workspace.py +++ b/multinet/api/models/workspace.py @@ -10,7 +10,7 @@ from django.dispatch import receiver from django_extensions.db.models import TimeStampedModel -from multinet.api.utils.arango import ensure_db_created, ensure_db_deleted, db +from multinet.api.utils.arango import db, ensure_db_created, ensure_db_deleted def create_default_arango_db_name(): From ef0705ff12b721507631775f92367f082c37cbab Mon Sep 17 00:00:00 2001 From: naglepuff Date: Mon, 30 Aug 2021 14:03:56 -0400 Subject: [PATCH 31/33] Create query_serializer for AQL endpoint This removes the need to use openapi.manualParameter. --- multinet/api/views/serializers.py | 4 ++++ multinet/api/views/workspace.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/multinet/api/views/serializers.py b/multinet/api/views/serializers.py index 329440d..0010843 100644 --- a/multinet/api/views/serializers.py +++ b/multinet/api/views/serializers.py @@ -88,6 +88,10 @@ class SingleUserWorkspacePermissionSerializer(serializers.Serializer): permission_label = serializers.CharField(allow_null=True) +class AqlQuerySerializer(serializers.Serializer): + query = serializers.CharField() + + class LimitOffsetSerializer(serializers.Serializer): limit = serializers.IntegerField(required=False) offset = serializers.IntegerField(required=False) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index 0ea402d..ee4b4e8 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -6,7 +6,6 @@ from django.db.models import Q from django.shortcuts import get_object_or_404 from django_filters import rest_framework as filters -from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema from rest_framework import status from rest_framework.decorators import action @@ -18,6 +17,7 @@ from multinet.api.models import Workspace, WorkspaceRole, WorkspaceRoleChoice from multinet.api.utils.arango import ArangoQuery from multinet.api.views.serializers import ( + AqlQuerySerializer, PermissionsCreateSerializer, PermissionsReturnSerializer, SingleUserWorkspacePermissionSerializer, @@ -178,7 +178,7 @@ def put_workspace_permissions(self, request, name: str): return Response(PermissionsReturnSerializer(workspace).data, status=status.HTTP_200_OK) - @swagger_auto_schema(manual_parameters=[openapi.Parameter('query', 'query', type='string')]) + @swagger_auto_schema(query_serializer=AqlQuerySerializer()) @action(detail=True) @require_workspace_permission(WorkspaceRoleChoice.READER) def aql(self, request, name: str): From b66f40161001ea2162497c9d8364c03bbaf6cdbe Mon Sep 17 00:00:00 2001 From: Michael Nagler Date: Mon, 30 Aug 2021 14:18:38 -0400 Subject: [PATCH 32/33] Use serializer to check for query string Co-authored-by: Jacob Nesbitt --- multinet/api/views/workspace.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index ee4b4e8..ab54487 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -183,10 +183,9 @@ def put_workspace_permissions(self, request, name: str): @require_workspace_permission(WorkspaceRoleChoice.READER) def aql(self, request, name: str): """Execute AQL in a workspace.""" - query_str = request.query_params.get('query') - if query_str is None: - return Response('No query provided', status=status.HTTP_400_BAD_REQUEST) - + serializer = AqlQuerySerailizer(data=request.query_params) + serializer.is_valid(raise_exception=True) + query_str = serializer.validated_data['query'] workspace: Workspace = get_object_or_404(Workspace, name=name) database = workspace.get_arango_db() query = ArangoQuery(database, query_str) From 3f2216e92eb46595edb4a7d86afca1f60797020e Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 30 Aug 2021 14:21:27 -0400 Subject: [PATCH 33/33] Fix typos --- multinet/api/views/workspace.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index ab54487..db86202 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -183,9 +183,9 @@ def put_workspace_permissions(self, request, name: str): @require_workspace_permission(WorkspaceRoleChoice.READER) def aql(self, request, name: str): """Execute AQL in a workspace.""" - serializer = AqlQuerySerailizer(data=request.query_params) - serializer.is_valid(raise_exception=True) - query_str = serializer.validated_data['query'] + serializer = AqlQuerySerializer(data=request.query_params) + serializer.is_valid(raise_exception=True) + query_str = serializer.validated_data['query'] workspace: Workspace = get_object_or_404(Workspace, name=name) database = workspace.get_arango_db() query = ArangoQuery(database, query_str)