Skip to content
48 changes: 36 additions & 12 deletions server/portal/apps/datafiles/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from requests.exceptions import HTTPError
from tapipy.errors import InternalServerError, UnauthorizedError
from portal.views.base import BaseApiView
from portal.utils import get_client_ip
from portal.utils import check_group_membership, get_client_ip
from portal.libs.agave.utils import service_account
from portal.apps.datafiles.handlers.tapis_handlers import (tapis_get_handler,
tapis_put_handler,
Expand All @@ -34,6 +34,28 @@
METRICS = logging.getLogger(f"metrics.{__name__}")


def check_project_admin_group(user):
"""Check whether a user belongs to the project admin group."""
return check_group_membership(user, settings.PROJECT_ADMIN_GROUP)


def is_project_system(system):
"""Return whether a Tapis system is managed as a portal project system."""
project_prefixes = (
settings.PORTAL_PROJECTS_SYSTEM_PREFIX,
settings.PORTAL_PROJECTS_REVIEW_SYSTEM_PREFIX,
settings.PORTAL_PROJECTS_PUBLISHED_SYSTEM_PREFIX,
)
return bool(system and system.startswith(project_prefixes))


def get_tapis_client(user, system=None):
"""Return the correct Tapis client for a user and optional project system."""
if is_project_system(system) and check_project_admin_group(user):
return service_account()
return user.tapis_oauth.client


class SystemListingView(BaseApiView):
"""System Listing View"""

Expand Down Expand Up @@ -72,7 +94,7 @@ class SystemDefinitionView(BaseApiView):

def get(self, request, systemId):
try:
client = request.user.tapis_oauth.client
client = get_tapis_client(request.user, systemId)
except AttributeError:
# Make sure that we only let unauth'd users see public systems
public_sys = next((sys for sys in settings.PORTAL_DATAFILES_STORAGE_SYSTEMS if sys['scheme'] == 'public'), None)
Expand All @@ -96,7 +118,7 @@ class TapisFilesView(BaseApiView):
@retry(UnauthorizedError, tries=3, max_time=15)
def get(self, request, operation=None, scheme=None, system=None, path='/'):
try:
client = request.user.tapis_oauth.client
client = get_tapis_client(request.user, system)
except AttributeError:
# Make sure that we only let unauth'd users see public systems
public_sys = next((sys for sys in settings.PORTAL_DATAFILES_STORAGE_SYSTEMS if sys['scheme'] == 'public'), None)
Expand Down Expand Up @@ -189,7 +211,7 @@ def put(self, request, operation=None, scheme=None,
handler=None, system=None, path='/'):
body = json.loads(request.body)
try:
client = request.user.tapis_oauth.client
client = get_tapis_client(request.user, system)
except AttributeError:
return HttpResponseForbidden("This data requires authentication to view.")

Expand Down Expand Up @@ -224,7 +246,7 @@ def post(self, request, operation=None, scheme=None,
body = request.FILES.dict()

try:
client = request.user.tapis_oauth.client
client = get_tapis_client(request.user, system)
except AttributeError:
return HttpResponseForbidden("This data requires authentication to upload.")

Expand Down Expand Up @@ -305,23 +327,25 @@ def put(self, request, operation=None, scheme=None,
return JsonResponse({"data": response})


def get_client(user, api):
def get_client(user, api, system=None):
client_mappings = {
'tapis': 'tapis_oauth',
'shared': 'tapis_oauth',
'googledrive': 'googledrive_user_token',
'box': 'box_user_token',
'dropbox': 'dropbox_user_token'
}
if api in ('tapis', 'shared') and is_project_system(system) and check_project_admin_group(user):
return service_account()
return getattr(user, client_mappings[api]).client


class TransferFilesView(BaseApiView):
def put(self, request, filetype):
body = json.loads(request.body)

src_client = get_client(request.user, body['src_api'])
dest_client = get_client(request.user, body['dest_api'])
src_client = get_client(request.user, body['src_api'], body.get('src_system'))
dest_client = get_client(request.user, body['dest_api'], body.get('dest_system'))
METRICS.info('Data Files',
extra={
'user': request.user.username,
Expand Down Expand Up @@ -351,7 +375,7 @@ def put(self, request, filetype):
class LinkView(BaseApiView):
def create_postit(self, request, scheme, system, path):

client = request.user.tapis_oauth.client
client = get_tapis_client(request.user, system)

# Create postit for unlimited use with a valid period of 1 year
postit = client.files.createPostIt(systemId=system, path=path, allowedUses=-1, validSeconds=31536000)
Expand All @@ -364,8 +388,8 @@ def create_postit(self, request, scheme, system, path):
)
return {"data": postit_redeem_url, "expiration": postit.expiration}

def delete_link(self, request, link):
client = request.user.tapis_oauth.client
def delete_link(self, request, system, link):
client = get_tapis_client(request.user, system)

postitId = link.get_uuid()

Expand Down Expand Up @@ -417,7 +441,7 @@ def delete(self, request, scheme, system, path):
link = Link.objects.get(tapis_uri=f"{system}/{path}")
except Link.DoesNotExist:
raise ApiException("Post-it does not exist")
response = self.delete_link(request, link)
response = self.delete_link(request, system, link)
return JsonResponse({"data": response})

def post(self, request, scheme, system, path):
Expand Down
16 changes: 16 additions & 0 deletions server/portal/apps/datafiles/views_unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
import os

import pytest
from django.contrib.auth.models import Group
from django.conf import settings
from mock import MagicMock, patch
from tapipy.errors import InternalServerError, UnauthorizedError
from tapipy.tapis import TapisResult

from portal.apps.datafiles.models import Link
from portal.apps.datafiles.views import get_tapis_client
pytestmark = pytest.mark.django_db


Expand All @@ -31,6 +33,20 @@ def get_user_data(mocker):
yield mock


def test_get_tapis_client_uses_service_account_for_project_admin(authenticated_user, mocker):
group = Group.objects.create(name=settings.PROJECT_ADMIN_GROUP)
authenticated_user.groups.add(group)
mock_service_account = mocker.patch('portal.apps.datafiles.views.service_account')

client = get_tapis_client(
authenticated_user,
f'{settings.PORTAL_PROJECTS_SYSTEM_PREFIX}.PRJ-123',
)

assert client == mock_service_account.return_value
mock_service_account.assert_called_once_with()


def test_get_no_allocation(client, authenticated_user, mocker, monkeypatch, mock_tapis_client):
mock_tapis_get = mocker.patch('portal.apps.datafiles.views.tapis_get_handler')
mock_error = InternalServerError()
Expand Down
3 changes: 1 addition & 2 deletions server/portal/apps/projects/models/project_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ def sync_users(self):
return
prj_users = [user.get("username") for user in self.value.get("users", [])]
project_user_query = models.Q(username__in=prj_users)
admin_user_query = models.Q(username__in=settings.PROJECT_ADMIN_USERS)
self.users.set(user_model.objects.filter(project_user_query | admin_user_query))
self.users.set(user_model.objects.filter(project_user_query))

def save(self, *args, **kwargs):
if self.name == constants.PROJECT:
Expand Down
52 changes: 38 additions & 14 deletions server/portal/apps/projects/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from django.http import JsonResponse
from django.utils.decorators import method_decorator
from portal.libs.agave.utils import service_account
from portal.utils import get_client_ip
from portal.utils import check_group_membership, get_client_ip
from portal.utils.decorators import agave_jwt_login
from portal.exceptions.api import ApiException
from portal.views.base import BaseApiView
Expand Down Expand Up @@ -49,6 +49,26 @@ def validate_project_metadata(metadata):
validated_model = schema.model_validate(metadata)
return validated_model.model_dump(exclude_none=True)


def check_project_admin_group(user):
"""Check whether a user belongs to the project admin group."""
return check_group_membership(user, settings.PROJECT_ADMIN_GROUP)


def get_project_client(user):
"""Return a Tapis client with project access for this user."""
if check_project_admin_group(user):
return service_account()
return user.tapis_oauth.client


def get_project_for_user(project_id, user):
"""Return project metadata if the user can access the project."""
project_query = models.Q(uuid=project_id) | models.Q(value__projectId=project_id)
if check_project_admin_group(user):
return ProjectMetadata.objects.get(project_query)
return user.projects.get(project_query)
Comment thread
van-go marked this conversation as resolved.
Outdated

@method_decorator(agave_jwt_login, name='dispatch')
@method_decorator(login_required, name='dispatch')
class ProjectsApiView(BaseApiView):
Expand Down Expand Up @@ -127,12 +147,12 @@ def get(self, request, root_system=None):
listing = []
# Filter search results to projects specific to user
if hits:
client = request.user.tapis_oauth.client
client = get_project_client(request.user)
listing = list_projects(client, root_system)
filtered_list = filter(lambda prj: prj['id'] in hits, listing)
listing = list(filtered_list)
else:
client = request.user.tapis_oauth.client
client = get_project_client(request.user)
listing = list_projects(client, root_system)

for project in listing:
Expand Down Expand Up @@ -226,7 +246,7 @@ def get(self, request, project_id=None, system_id=None):
if system_id and system_id.startswith(settings.PORTAL_PROJECTS_PUBLISHED_SYSTEM_PREFIX):
client = service_account()
else:
client = request.user.tapis_oauth.client
client = get_project_client(request.user)

prj = get_project(client, project_id)

Expand Down Expand Up @@ -330,11 +350,11 @@ def patch(
},
)

client = request.user.tapis_oauth.client
client = get_project_client(request.user)

system_details = client.systems.getSystem(systemId=project_id_full)

if (system_details.owner == request.user.username):
if system_details.owner == request.user.username or check_project_admin_group(request.user):
workspace_def = update_project(client, project_id, title, description, keywords)
else:
workspace_def = get_project(client, project_id)
Expand Down Expand Up @@ -499,7 +519,7 @@ def change_system_role(self, request, project_Id, **data):
@login_required
def get_project_role(request, project_id, username):
role = None
client = request.user.tapis_oauth.client
client = get_project_client(request.user)

METRICS.info(
"Projects",
Expand All @@ -520,7 +540,7 @@ def get_project_role(request, project_id, username):

@login_required
def get_system_role(request, project_id, username):
client = request.user.tapis_oauth.client
client = get_project_client(request.user)

METRICS.info(
"Projects",
Expand All @@ -542,10 +562,16 @@ class ProjectEntityView(BaseApiView):

def patch(self, request: HttpRequest, project_id: str):

client = request.user.tapis_oauth.client

if not request.user.is_authenticated:
raise ApiException("Unauthenticated user", status=401)

client = get_project_client(request.user)
try:
get_project_for_user(project_id, request.user)
except ProjectMetadata.DoesNotExist as exc:
raise ApiException(
"User does not have access to the requested project", status=403
) from exc

req_body = json.loads(request.body)
value = req_body.get("value", {})
Expand Down Expand Up @@ -573,14 +599,12 @@ def patch(self, request: HttpRequest, project_id: str):
def post(self, request: HttpRequest, project_id: str):
"""Add a new entity to a project"""

client = request.user.tapis_oauth.client
if not request.user.is_authenticated:
raise ApiException("Unauthenticated user", status=401)
client = get_project_client(request.user)

try:
project: ProjectMetadata = ProjectMetadata.objects.get(
models.Q(uuid=project_id) | models.Q(value__projectId=project_id)
)
project: ProjectMetadata = get_project_for_user(project_id, request.user)
except ProjectMetadata.DoesNotExist as exc:
raise ApiException(
"User does not have access to the requested project", status=403
Expand Down
28 changes: 28 additions & 0 deletions server/portal/apps/projects/views_unit_test.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import pytest
from hashlib import sha256
from portal.apps.projects.managers.base import ProjectsManager
from portal.apps.projects.models.project_metadata import ProjectMetadata
from portal.apps.projects.views import get_project_client, get_project_for_user
from portal.apps.search.tasks import tapis_project_listing_indexer
from portal.libs.elasticsearch.indexes import IndexedProject
from mock import MagicMock
import json
from tapipy.tapis import TapisResult
from django.conf import settings
from django.contrib.auth.models import Group
from django.test import override_settings
from portal.apps._custom.drp import constants


@pytest.fixture
Expand Down Expand Up @@ -106,6 +110,30 @@ def project_list(authenticated_user):
}


def test_get_project_client_uses_service_account_for_project_admin(authenticated_user, mocker):
group = Group.objects.create(name=settings.PROJECT_ADMIN_GROUP)
authenticated_user.groups.add(group)
mock_service_account = mocker.patch('portal.apps.projects.views.service_account')

client = get_project_client(authenticated_user)

assert client == mock_service_account.return_value
mock_service_account.assert_called_once_with()


def test_get_project_for_user_allows_project_admin(authenticated_user):
group = Group.objects.create(name=settings.PROJECT_ADMIN_GROUP)
authenticated_user.groups.add(group)
project = ProjectMetadata.objects.create(
name=constants.PROJECT,
value={'projectId': f'{settings.PORTAL_PROJECTS_SYSTEM_PREFIX}.PRJ-123', 'users': []},
)

result = get_project_for_user(project.project_id, authenticated_user)

assert result == project


def test_projects_get(
authenticated_user,
client,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ def create_shared_workspace(client: Tapis, title: str, description: str, keyword
"writer")

# User creates the system and adds their credential
system_id = create_workspace_system(client, workspace_id, title, description, keywords)
system_id = create_workspace_system(
client, workspace_id, title, description, keywords, owner=owner
)

# Give portal admin full permissions
portal_admin = settings.PORTAL_ADMIN_USERNAME
Expand Down
Loading
Loading