Skip to content

Commit

Permalink
[fix] Resolve permissions based on localfile storage connection (DEV-…
Browse files Browse the repository at this point in the history
…1296) (HumanSignal#1808)

* Resolve permissions based on localfile storage connection

* Fix first()

* Fix full path name

* Check all storages

* Fix startswith

* Temp fix ti turn off permissions

* Remove storage link

* Fix s3 resolve with not string

* Fix misspell

* Add local storage tests (DEV-1296)

* Fix document_root check for windows (DEV-1296)

* Fix tests for windows (DEV-1296)

* Skip local storage tests on windows (DEV-1296)

* Add more info in validation. Fix packaged in requirements

* Update requirements.txt

* Update conftest.py

Co-authored-by: nik <[email protected]>
Co-authored-by: makseq-ubnt <[email protected]>
Co-authored-by: Sergei Ivashchenko <[email protected]>
  • Loading branch information
4 people authored Dec 18, 2021
1 parent 8bc6974 commit 1cfd9e9
Show file tree
Hide file tree
Showing 10 changed files with 421 additions and 14 deletions.
14 changes: 12 additions & 2 deletions deploy/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,18 @@ azure-storage-blob>=12.6.0
boto==2.49.0
boto3==1.16.28
botocore==1.19.28
google-cloud-storage==1.28.1
google-cloud-logging==2.6.0

google-api-core==1.31.5
google-auth==1.35.0
google-cloud-appengine-logging==1.1.0
google-cloud-audit-log==0.2.0
google-cloud-core==1.5.0
google-cloud-logging==2.7.0
google-cloud-storage==1.29.0
google-resumable-media==0.5.1
googleapis-common-protos==1.52.0
grpc-google-iam-v1==0.12.3

Django==3.1.13
django_annoying==0.10.6
django_debug_toolbar==3.2.1
Expand Down
1 change: 1 addition & 0 deletions label_studio/core/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@

ENABLE_LOCAL_FILES_STORAGE = get_bool_env('ENABLE_LOCAL_FILES_STORAGE', default=True)
LOCAL_FILES_SERVING_ENABLED = get_bool_env('LOCAL_FILES_SERVING_ENABLED', default=False)
LOCAL_FILES_DOCUMENT_ROOT = get_env('LOCAL_FILES_DOCUMENT_ROOT', default=os.path.abspath(os.sep))

""" React Libraries: do not forget to change this dir in /etc/nginx/nginx.conf """
# EDITOR = label-studio-frontend repository
Expand Down
20 changes: 16 additions & 4 deletions label_studio/core/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
from wsgiref.util import FileWrapper
from rest_framework.views import APIView
from drf_yasg.utils import swagger_auto_schema
from django.db.models import Value, F, CharField

from core import utils
from core.utils.io import find_file
from core.utils.params import get_env
from core.label_config import generate_time_series_json
from core.utils.common import collect_versions
from io_storages.localfiles.models import LocalFilesImportStorageLink
from io_storages.localfiles.models import LocalFilesImportStorageLink, LocalFilesImportStorage

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -166,18 +167,29 @@ def samples_paragraphs(request):

def localfiles_data(request):
"""Serving files for LocalFilesImportStorage"""
user = request.user
path = request.GET.get('d')
if settings.LOCAL_FILES_SERVING_ENABLED is False:
return HttpResponseForbidden("Serving local files can be dangerous, so it's disabled by default. "
'You can enable it with LOCAL_FILES_SERVING_ENABLED environment variable, '
'please check docs: https://labelstud.io/guide/storage.html#Local-storage')

local_serving_document_root = get_env('LOCAL_FILES_DOCUMENT_ROOT', default='/')
local_serving_document_root = settings.LOCAL_FILES_DOCUMENT_ROOT
if path and request.user.is_authenticated:
path = posixpath.normpath(path).lstrip('/')
full_path = Path(safe_join(local_serving_document_root, path))
link = LocalFilesImportStorageLink.objects.filter(key=str(full_path)).first()
if link and link.has_permission(request.user) and os.path.exists(full_path):
user_has_permissions = False

# Try to find Local File Storage connection based prefix:
# storage.path=/home/user, full_path=/home/user/a/b/c/1.jpg =>
# full_path.startswith(path) => True
localfiles_storage = LocalFilesImportStorage.objects \
.annotate(_full_path=Value(os.path.dirname(full_path), output_field=CharField())) \
.filter(_full_path__startswith=F('path'))
if localfiles_storage.exists():
user_has_permissions = any(storage.project.has_permission(user) for storage in localfiles_storage)

if user_has_permissions and os.path.exists(full_path):
content_type, encoding = mimetypes.guess_type(str(full_path))
content_type = content_type or 'application/octet-stream'
return RangedFileResponse(request, open(full_path, mode='rb'), content_type)
Expand Down
9 changes: 6 additions & 3 deletions label_studio/io_storages/all_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@
def _get_common_storage_list():
storage_list = get_storage_list()
if settings.ENABLE_LOCAL_FILES_STORAGE:
storage_list += [{'name': 'localfiles', 'title': 'Local files', 'import_list_api': LocalFilesImportStorageListAPI, 'export_list_api': LocalFilesExportStorageListAPI}]
storage_list += [{
'name': 'localfiles',
'title': 'Local files',
'import_list_api': LocalFilesImportStorageListAPI,
'export_list_api': LocalFilesExportStorageListAPI
}]

return storage_list

Expand Down Expand Up @@ -84,6 +89,4 @@ def _get_response(self, api, request, *args, **kwargs):
def list(self, request, *args, **kwargs):
list_responses = sum([
self._get_response(s['export_list_api'], request, *args, **kwargs) for s in _common_storage_list], [])
if settings.ENABLE_LOCAL_FILES_STORAGE:
list_responses.extend(self._get_response(LocalFilesExportStorageListAPI, request, *args, **kwargs))
return Response(list_responses)
12 changes: 9 additions & 3 deletions label_studio/io_storages/localfiles/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,13 @@ class LocalFilesMixin(models.Model):

def validate_connection(self):
path = Path(self.path)
document_root = Path(settings.LOCAL_FILES_DOCUMENT_ROOT)
if not path.exists():
raise ValidationError(f'Path {self.path} does not exist')
if document_root not in path.parents:
raise ValidationError(f'Path {self.path} must start with '
f'LOCAL_FILES_DOCUMENT_ROOT={settings.LOCAL_FILES_DOCUMENT_ROOT} '
f'and must be a child, e.g.: {Path(settings.LOCAL_FILES_DOCUMENT_ROOT) / "abc"}')
if settings.LOCAL_FILES_SERVING_ENABLED is False:
raise ValidationError("Serving local files can be dangerous, so it's disabled by default. "
'You can enable it with LOCAL_FILES_SERVING_ENABLED environment variable, '
Expand All @@ -64,10 +69,11 @@ def iterkeys(self):
def get_data(self, key):
path = Path(key)
if self.use_blob_urls:
# include self-hosted links pointed to local resources via {settings.HOSTNAME}/data/local-files?d=<path/to/local/dir>
document_root = Path(get_env('LOCAL_FILES_DOCUMENT_ROOT', default='/'))
# include self-hosted links pointed to local resources via
# {settings.HOSTNAME}/data/local-files?d=<path/to/local/dir>
document_root = Path(settings.LOCAL_FILES_DOCUMENT_ROOT)
relative_path = str(path.relative_to(document_root))
return {settings.DATA_UNDEFINED_NAME: f'{settings.HOSTNAME}/data/local-files/?d={relative_path}'}
return {settings.DATA_UNDEFINED_NAME: f'{settings.HOSTNAME}/data/local-files/?d={str(relative_path)}'}

try:
with open(path, encoding='utf8') as f:
Expand Down
4 changes: 3 additions & 1 deletion label_studio/tasks/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ def _get_storage_by_url(self, url):
for storage_class in get_storage_classes('import'):
storage_objects = storage_class.objects.filter(project=self.project)
for storage_object in storage_objects:
if storage_object.can_resolve_url(url):
# check url is string because task can have int, float, dict, list
# and 'can_resolve_url' will fail
if isinstance(url, str) and storage_object.can_resolve_url(url):
return storage_object

@property
Expand Down
29 changes: 28 additions & 1 deletion label_studio/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,21 @@
import re
import boto3
import logging
import shutil
import tempfile

from moto import mock_s3
from copy import deepcopy
from pathlib import Path

from django.conf import settings
from projects.models import Project
from tasks.models import Task
from users.models import User
from organizations.models import Organization
from types import SimpleNamespace

# if we haven't this package, pytest.ini::env doesn't work
# if we haven't this package, pytest.ini::env doesn't work
try:
import pytest_env.plugin
except ImportError:
Expand Down Expand Up @@ -392,3 +396,26 @@ def configured_project(business_client, annotator_client):
@pytest.fixture(name="django_live_url")
def get_server_url(live_server):
yield live_server.url


@pytest.fixture(name="local_files_storage")
def local_files_storage(settings):
settings.LOCAL_FILES_SERVING_ENABLED = True
tempdir = Path(tempfile.gettempdir()) / Path('files')
subdir = tempdir / Path('subdir')
os.makedirs(str(subdir), exist_ok=True)
test_image = Path(*'tests/test_suites/samples/test_image.png'.split('/'))
shutil.copyfile(str(test_image), str(tempdir / Path('test_image1.png')))
shutil.copyfile(str(test_image), str(subdir / Path('test_image2.png')))


@pytest.fixture(name="local_files_document_root_tempdir")
def local_files_document_root_tempdir(settings):
tempdir = Path(tempfile.gettempdir())
settings.LOCAL_FILES_DOCUMENT_ROOT = tempdir.root


@pytest.fixture(name="local_files_document_root_subdir")
def local_files_document_root_subdir(settings):
tempdir = Path(tempfile.gettempdir()) / Path('files')
settings.LOCAL_FILES_DOCUMENT_ROOT = str(tempdir)
Loading

0 comments on commit 1cfd9e9

Please sign in to comment.