From 22403a53bcef4f7ef0aa0a9abc906a54cf163a0a Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Tue, 17 Dec 2024 14:05:05 -0500 Subject: [PATCH 01/26] remove unneeded provider methods --- gateway/api/views/files.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/gateway/api/views/files.py b/gateway/api/views/files.py index a10af8661..b36976935 100644 --- a/gateway/api/views/files.py +++ b/gateway/api/views/files.py @@ -154,22 +154,6 @@ def user_has_access_to_provider_function(self, user, function: Program) -> bool: return has_access # Provider methods, these will be migrated to a Repository class - def list_user_providers(self, user): - """list provider names that the user in""" - provider_list = [] - providers = Provider.objects.all() - for instance in providers: - user_groups = user.groups.all() - admin_groups = instance.admin_groups.all() - provider_found = any(group in admin_groups for group in user_groups) - if provider_found: - provider_list.append(instance.name) - return provider_list - - def check_user_has_provider(self, user, provider_name): - """check if user has the provider""" - return provider_name in self.list_user_providers(user) - def user_has_provider_access(self, user, provider_name: str) -> bool: """ This method returns True or False if the user has access to the provider or not. From 377e9cf4e4fc56423c65fdd6fc6eb23daa02d988 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 10:30:58 -0500 Subject: [PATCH 02/26] create access policies file --- gateway/api/access_policies/__init__.py | 0 gateway/api/access_policies/programs.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 gateway/api/access_policies/__init__.py create mode 100644 gateway/api/access_policies/programs.py diff --git a/gateway/api/access_policies/__init__.py b/gateway/api/access_policies/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/gateway/api/access_policies/programs.py b/gateway/api/access_policies/programs.py new file mode 100644 index 000000000..e69de29bb From 22979507c961a487d8d093e8d34b8854ba58fa38 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 10:32:50 -0500 Subject: [PATCH 03/26] refactor get_functions repository method --- gateway/api/repositories/programs.py | 2 +- gateway/api/views/programs.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/gateway/api/repositories/programs.py b/gateway/api/repositories/programs.py index f7e1aae74..4fa849f60 100644 --- a/gateway/api/repositories/programs.py +++ b/gateway/api/repositories/programs.py @@ -19,7 +19,7 @@ class ProgramRepository: The main objective of this class is to manage the access to the model """ - def get_functions(self, author) -> List[Program] | Any: + def get_functions_with_view_permissions(self, author) -> List[Program] | Any: """ Returns all the functions available to the user. This means: - User functions where the user is the author diff --git a/gateway/api/views/programs.py b/gateway/api/views/programs.py index dd6eba04f..4d873be6c 100644 --- a/gateway/api/views/programs.py +++ b/gateway/api/views/programs.py @@ -167,7 +167,9 @@ def list(self, request): ) else: # If filter is not applied we return author and providers functions together - functions = self.program_repository.get_functions(author) + functions = self.program_repository.get_functions_with_view_permissions( + author + ) serializer = self.get_serializer(functions, many=True) From ddb8d3bff4a3c6a65d207beae3bce417de9a551a Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:36:02 -0500 Subject: [PATCH 04/26] refactor groups and repositories --- gateway/api/access_policies/programs.py | 49 ++++++++++ gateway/api/repositories/groups.py | 23 +++++ gateway/api/repositories/programs.py | 114 ++++++++++++++---------- gateway/api/views/programs.py | 2 +- 4 files changed, 139 insertions(+), 49 deletions(-) create mode 100644 gateway/api/repositories/groups.py diff --git a/gateway/api/access_policies/programs.py b/gateway/api/access_policies/programs.py index e69de29bb..1c86b0588 100644 --- a/gateway/api/access_policies/programs.py +++ b/gateway/api/access_policies/programs.py @@ -0,0 +1,49 @@ +""" +Access policies implementation for Program access +""" +import logging + +from django.contrib.auth.models import Group, Permission +from django.db.models import Q + + +from api.models import RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION, Program + + +logger = logging.getLogger("gateway") + + +class ProgramAccessPolicy: + @staticmethod + def can_view(user, user_view_groups, function: Program) -> bool: + if function.author.id == user.id: + return True + + instances = function.instances.all() + has_access = any(group in instances for group in user_view_groups) + # TODO: the message must be different if the function has a provider or not + if not has_access: + logger.error( + "User [%s] has no access to function [%s/%s].", + user.id, + function.provider.name, + function.title, + ) + return has_access + + @staticmethod + def can_run(user, user_run_groups, function: Program) -> bool: + if function.author.id == user.id: + return True + + instances = function.instances.all() + has_access = any(group in instances for group in user_run_groups) + # TODO: the message must be different if the function has a provider or not + if not has_access: + logger.error( + "User [%s] has no access to function [%s/%s].", + user.id, + function.provider.name, + function.title, + ) + return has_access diff --git a/gateway/api/repositories/groups.py b/gateway/api/repositories/groups.py new file mode 100644 index 000000000..6bb2c6d5e --- /dev/null +++ b/gateway/api/repositories/groups.py @@ -0,0 +1,23 @@ +from typing import List +from django.contrib.auth.models import Group, Permission +from django.db.models import Q + +from api.models import RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION + + +class GroupRepository: + def get_groups_with_view_permissions_from_user(self, user) -> List[Group]: + view_program_permission = Permission.objects.get( + codename=VIEW_PROGRAM_PERMISSION + ) + user_criteria = Q(user=user) + view_permission_criteria = Q(permissions=view_program_permission) + + return Group.objects.filter(user_criteria & view_permission_criteria) + + def get_groups_with_run_permissions_from_user(self, user) -> List[Group]: + run_program_permission = Permission.objects.get(codename=RUN_PROGRAM_PERMISSION) + user_criteria = Q(user=user) + run_permission_criteria = Q(permissions=run_program_permission) + + return Group.objects.filter(user_criteria & run_permission_criteria) diff --git a/gateway/api/repositories/programs.py b/gateway/api/repositories/programs.py index 4fa849f60..d642dc78d 100644 --- a/gateway/api/repositories/programs.py +++ b/gateway/api/repositories/programs.py @@ -1,14 +1,15 @@ """ -Repository implementatio for Programs model +Repository implementation for Programs model """ import logging -from typing import Any, List +from typing import List from django.db.models import Q from django.contrib.auth.models import Group, Permission from api.models import RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION, Program +from api.repositories.groups import GroupRepository logger = logging.getLogger("gateway") @@ -19,7 +20,12 @@ class ProgramRepository: The main objective of this class is to manage the access to the model """ - def get_functions_with_view_permissions(self, author) -> List[Program] | Any: + # This repository should be in the use case implementatio + # but this class is not ready yet so it will live here + # in the meantime + group_repository = GroupRepository() + + def get_functions_with_view_permissions(self, author) -> List[Program]: """ Returns all the functions available to the user. This means: - User functions where the user is the author @@ -32,21 +38,11 @@ def get_functions_with_view_permissions(self, author) -> List[Program] | Any: List[Program] | Any: all the functions available to the user """ - view_program_permission = Permission.objects.get( - codename=VIEW_PROGRAM_PERMISSION + view_groups = self.group_repository.get_groups_with_view_permissions_from_user( + user=author ) - - user_criteria = Q(user=author) - view_permission_criteria = Q(permissions=view_program_permission) - - author_groups_with_view_permissions = Group.objects.filter( - user_criteria & view_permission_criteria - ) - + author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) author_criteria = Q(author=author) - author_groups_with_view_permissions_criteria = Q( - instances__in=author_groups_with_view_permissions - ) result_queryset = Program.objects.filter( author_criteria | author_groups_with_view_permissions_criteria @@ -57,7 +53,7 @@ def get_functions_with_view_permissions(self, author) -> List[Program] | Any: return result_queryset - def get_user_functions(self, author) -> List[Program] | Any: + def get_user_functions(self, author) -> List[Program]: """ Returns the user functions available to the user. This means: - User functions where the user is the author @@ -82,9 +78,7 @@ def get_user_functions(self, author) -> List[Program] | Any: return result_queryset - def get_provider_functions_with_run_permissions( - self, author - ) -> List[Program] | Any: + def get_provider_functions_with_run_permissions(self, author) -> List[Program]: """ Returns the provider functions available to the user. This means: - Provider functions where the user has run permissions @@ -97,18 +91,10 @@ def get_provider_functions_with_run_permissions( List[Program] | Any: providers functions available to the user """ - run_program_permission = Permission.objects.get(codename=RUN_PROGRAM_PERMISSION) - - user_criteria = Q(user=author) - run_permission_criteria = Q(permissions=run_program_permission) - author_groups_with_run_permissions = Group.objects.filter( - user_criteria & run_permission_criteria + run_groups = self.group_repository.get_groups_with_run_permissions_from_user( + user=author ) - - author_groups_with_run_permissions_criteria = Q( - instances__in=author_groups_with_run_permissions - ) - + author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) provider_exists_criteria = ~Q(provider=None) result_queryset = Program.objects.filter( @@ -120,7 +106,7 @@ def get_provider_functions_with_run_permissions( return result_queryset - def get_user_function_by_title(self, author, title: str) -> Program | Any: + def get_user_function_by_title(self, author, title: str) -> Program | None: """ Returns the user function associated to a title: @@ -148,9 +134,9 @@ def get_user_function_by_title(self, author, title: str) -> Program | Any: return result_queryset - def get_provider_function_by_title( + def get_provider_function_by_title_with_view_permissions( self, author, title: str, provider_name: str - ) -> Program | Any: + ) -> Program | None: """ Returns the provider function associated to: - A Function title @@ -163,31 +149,63 @@ def get_provider_function_by_title( provider: Provider associated to the function Returns: - Program | Any: provider function with the specific + Program | None: provider function with the specific title and provider """ - view_program_permission = Permission.objects.get( - codename=VIEW_PROGRAM_PERMISSION + # This access should be checked in the use-case but how we don't + # have it implemented yet we will do the check by now in the + # repository call + view_groups = self.group_repository.get_groups_with_view_permissions_from_user( + user=author ) + author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) + title_criteria = Q(title=title, provider__name=provider_name) - user_criteria = Q(user=author) - view_permission_criteria = Q(permissions=view_program_permission) + result_queryset = Program.objects.filter( + author_groups_with_view_permissions_criteria & title_criteria + ).first() - author_groups_with_view_permissions = Group.objects.filter( - user_criteria & view_permission_criteria - ) + if result_queryset is None: + logger.warning( + "Function [%s/%s] was not found or author [%s] doesn't have access to it", + provider_name, + title, + author.id, + ) - author_criteria = Q(author=author) - author_groups_with_view_permissions_criteria = Q( - instances__in=author_groups_with_view_permissions - ) + return result_queryset + + def get_provider_function_by_title_with_run_permissions( + self, author, title: str, provider_name: str + ) -> Program | None: + """ + Returns the provider function associated to: + - A Function title + - A Provider + - Author must have run permission to execute it or be the author + Args: + author: Django author from who retrieve the function + title: Title that the function must have to find it + provider: Provider associated to the function + + Returns: + Program | None: provider function with the specific + title and provider + """ + + # This access should be checked in the use-case but how we don't + # have it implemented yet we will do the check by now in the + # repository call + run_groups = self.group_repository.get_groups_with_run_permissions_from_user( + user=author + ) + author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) title_criteria = Q(title=title, provider__name=provider_name) result_queryset = Program.objects.filter( - (author_criteria | author_groups_with_view_permissions_criteria) - & title_criteria + author_groups_with_run_permissions_criteria & title_criteria ).first() if result_queryset is None: diff --git a/gateway/api/views/programs.py b/gateway/api/views/programs.py index 4d873be6c..2f8d9159b 100644 --- a/gateway/api/views/programs.py +++ b/gateway/api/views/programs.py @@ -309,7 +309,7 @@ def get_by_title(self, request, title): ) if provider_name: - function = self.program_repository.get_provider_function_by_title( + function = self.program_repository.get_provider_function_by_title_with_view_permissions( author=author, title=function_title, provider_name=provider_name ) else: From 909eab5ca1ccc4eeca951c3c87d8bb815fd4cb92 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 13:10:26 -0500 Subject: [PATCH 05/26] repository refactor from files --- gateway/api/access_policies/programs.py | 4 +- gateway/api/access_policies/providers.py | 26 ++++ gateway/api/repositories/providers.py | 32 +++++ gateway/api/views/files.py | 158 +++++++---------------- 4 files changed, 107 insertions(+), 113 deletions(-) create mode 100644 gateway/api/access_policies/providers.py create mode 100644 gateway/api/repositories/providers.py diff --git a/gateway/api/access_policies/programs.py b/gateway/api/access_policies/programs.py index 1c86b0588..d7d07e4e2 100644 --- a/gateway/api/access_policies/programs.py +++ b/gateway/api/access_policies/programs.py @@ -23,7 +23,7 @@ def can_view(user, user_view_groups, function: Program) -> bool: has_access = any(group in instances for group in user_view_groups) # TODO: the message must be different if the function has a provider or not if not has_access: - logger.error( + logger.warning( "User [%s] has no access to function [%s/%s].", user.id, function.provider.name, @@ -40,7 +40,7 @@ def can_run(user, user_run_groups, function: Program) -> bool: has_access = any(group in instances for group in user_run_groups) # TODO: the message must be different if the function has a provider or not if not has_access: - logger.error( + logger.warning( "User [%s] has no access to function [%s/%s].", user.id, function.provider.name, diff --git a/gateway/api/access_policies/providers.py b/gateway/api/access_policies/providers.py new file mode 100644 index 000000000..8aa0476db --- /dev/null +++ b/gateway/api/access_policies/providers.py @@ -0,0 +1,26 @@ +""" +Access policies implementation for Program access +""" +import logging + +from django.contrib.auth.models import Group, Permission +from django.db.models import Q + + +from api.models import Provider + + +logger = logging.getLogger("gateway") + + +class ProviderAccessPolicy: + @staticmethod + def can_access(user, provider: Provider) -> bool: + user_groups = user.groups.all() + admin_groups = provider.admin_groups.all() + has_access = any(group in admin_groups for group in user_groups) + if not has_access: + logger.warning( + "User [%s] has no access to provider [%s].", user.id, provider.name + ) + return has_access diff --git a/gateway/api/repositories/providers.py b/gateway/api/repositories/providers.py new file mode 100644 index 000000000..7d1687681 --- /dev/null +++ b/gateway/api/repositories/providers.py @@ -0,0 +1,32 @@ +""" +Repository implementation for Provider model +""" +import logging + +from api.models import Provider + + +logger = logging.getLogger("gateway") + + +class ProviderRepository: + """ + The main objective of this class is to manage the access to the model + """ + + def get_provider_by_name(self, name: str) -> Provider | None: + """ + Returns the provider associated with a name. + + Args: + name: provider name + + Returns: + Provider | None: returns the specific provider if it exists + """ + + provider = Provider.objects.filter(name=name).first() + if provider is None: + logger.warning("Provider [%s] does not exist.", name) + + return provider diff --git a/gateway/api/views/files.py b/gateway/api/views/files.py index b36976935..1790bc8c1 100644 --- a/gateway/api/views/files.py +++ b/gateway/api/views/files.py @@ -19,9 +19,11 @@ from rest_framework.decorators import action from rest_framework.response import Response +from api.access_policies.providers import ProviderAccessPolicy +from api.repositories.programs import ProgramRepository +from api.repositories.providers import ProviderRepository from api.services.file_storage import FileStorage, WorkingDir from api.utils import sanitize_file_name, sanitize_name -from api.models import Provider, Program # pylint: disable=duplicate-code logger = logging.getLogger("gateway") @@ -49,7 +51,9 @@ class FilesViewSet(viewsets.ViewSet): BASE_NAME = "files" - # Functions methods, these will be migrated to a Repository class + program_repository = ProgramRepository() + provider_repository = ProviderRepository() + def get_function( self, user, function_title: str, provider_name: str | None ) -> None: @@ -66,112 +70,12 @@ def get_function( """ if not provider_name: - return self.get_user_function(user=user, function_title=function_title) - - function = self.get_provider_function( - provider_name=provider_name, function_title=function_title - ) - if function and self.user_has_access_to_provider_function( - user=user, function=function - ): - return function - - return None - - def get_user_function(self, user, function_title: str) -> Program | None: - """ - This method returns the specified function. - - Args: - user: Django user to identify the author of the function - function_title (str): title of the function - - Returns: - Program | None: returns the function if it exists - """ - - function = Program.objects.filter(title=function_title, author=user).first() - if function is None: - logger.error( - "Function [%s] does not exist for the author [%s]", - function_title, - user.id, - ) - return None - return function - - def get_provider_function( - self, provider_name: str, function_title: str - ) -> Program | None: - """ - This method returns the specified function from a provider. - - Args: - provider_name (str): name of the provider - function_title (str): title of the function - - Returns: - Program | None: returns the function if it exists - """ - - provider = Provider.objects.filter(name=provider_name).first() - if provider is None: - logger.error("Provider [%s] does not exist.", provider_name) - return None - - function = Program.objects.filter( - title=function_title, provider=provider - ).first() - if function is None: - logger.error( - "Function [%s/%s] does not exist.", provider_name, function_title - ) - return None - return function - - def user_has_access_to_provider_function(self, user, function: Program) -> bool: - """ - This method returns True or False if the user has access to the provider function. - - Args: - user: Django user that is being checked - function (Program): the Qiskit Function that is going to be checked - - Returns: - bool: boolean value that verifies if the user has access or not to the function - """ - - instances = function.instances.all() - user_groups = user.groups.all() - has_access = any(group in instances for group in user_groups) - if not has_access: - logger.error( - "User [%s] has no access to function [%s/%s].", - user.id, - function.provider.name, - function.title, + return self.program_repository.get_provider_function_by_title_with_run_permissions( + author=user, title=function_title, provider_name=provider_name ) - return has_access - - # Provider methods, these will be migrated to a Repository class - def user_has_provider_access(self, user, provider_name: str) -> bool: - """ - This method returns True or False if the user has access to the provider or not. - """ - - provider = Provider.objects.filter(name=provider_name).first() - if provider is None: - logger.error("Provider [%s] does not exist.", provider_name) - return False - - user_groups = user.groups.all() - admin_groups = provider.admin_groups.all() - has_access = any(group in admin_groups for group in user_groups) - if not has_access: - logger.error( - "User [%s] has no access to provider [%s].", user.id, provider_name - ) - return has_access + return self.program_repository.get_user_function_by_title( + author=user, title=function_title + ) def list(self, request): """ @@ -240,7 +144,15 @@ def provider_list(self, request): status=status.HTTP_400_BAD_REQUEST, ) - if not self.user_has_provider_access(request.user, provider_name): + provider = self.provider_repository.get_provider_by_name(name=provider_name) + if provider is None: + return Response( + {"message": f"Provider {provider_name} doesn't exist."}, + status=status.HTTP_404_NOT_FOUND, + ) + if not ProviderAccessPolicy.can_access( + user=request.user, provider=provider + ): return Response( {"message": f"Provider {provider_name} doesn't exist."}, status=status.HTTP_404_NOT_FOUND, @@ -356,7 +268,15 @@ def provider_download(self, request): status=status.HTTP_400_BAD_REQUEST, ) - if not self.user_has_provider_access(request.user, provider_name): + provider = self.provider_repository.get_provider_by_name(name=provider_name) + if provider is None: + return Response( + {"message": f"Provider {provider_name} doesn't exist."}, + status=status.HTTP_404_NOT_FOUND, + ) + if not ProviderAccessPolicy.can_access( + user=request.user, provider=provider + ): return Response( {"message": f"Provider {provider_name} doesn't exist."}, status=status.HTTP_404_NOT_FOUND, @@ -469,7 +389,15 @@ def provider_delete(self, request): status=status.HTTP_400_BAD_REQUEST, ) - if not self.user_has_provider_access(request.user, provider_name): + provider = self.provider_repository.get_provider_by_name(name=provider_name) + if provider is None: + return Response( + {"message": f"Provider {provider_name} doesn't exist."}, + status=status.HTTP_404_NOT_FOUND, + ) + if not ProviderAccessPolicy.can_access( + user=request.user, provider=provider + ): return Response( {"message": f"Provider {provider_name} doesn't exist."}, status=status.HTTP_404_NOT_FOUND, @@ -577,7 +505,15 @@ def provider_upload(self, request): status=status.HTTP_400_BAD_REQUEST, ) - if not self.user_has_provider_access(request.user, provider_name): + provider = self.provider_repository.get_provider_by_name(name=provider_name) + if provider is None: + return Response( + {"message": f"Provider {provider_name} doesn't exist."}, + status=status.HTTP_404_NOT_FOUND, + ) + if not ProviderAccessPolicy.can_access( + user=request.user, provider=provider + ): return Response( {"message": f"Provider {provider_name} doesn't exist."}, status=status.HTTP_404_NOT_FOUND, From 3f0a43162d7d5b9eb4c128eef6b22607d56320ea Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 14:33:22 -0500 Subject: [PATCH 06/26] fix some linter problems --- gateway/api/access_policies/programs.py | 39 ++++++++++++++++--- gateway/api/access_policies/providers.py | 22 ++++++++--- gateway/api/repositories/groups.py | 28 +++++++++++++ gateway/api/repositories/programs.py | 11 +++--- .../test_user_2/artifact_delete.tar | 1 + 5 files changed, 85 insertions(+), 16 deletions(-) create mode 100644 gateway/tests/resources/fake_media/test_user_2/artifact_delete.tar diff --git a/gateway/api/access_policies/programs.py b/gateway/api/access_policies/programs.py index d7d07e4e2..46b1c562c 100644 --- a/gateway/api/access_policies/programs.py +++ b/gateway/api/access_policies/programs.py @@ -3,19 +3,34 @@ """ import logging -from django.contrib.auth.models import Group, Permission -from django.db.models import Q - - -from api.models import RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION, Program +from api.models import Program logger = logging.getLogger("gateway") class ProgramAccessPolicy: + """ + The main objective of this class is to manage the access for the user + to the Program entities. + """ + @staticmethod def can_view(user, user_view_groups, function: Program) -> bool: + """ + Checks if the user has view access to a Function: + - If it's the author it will always have access + - a view group is in the Program.instances + + Args: + user: Django user from the request + user_view_groups: view groups from a user + function: Program instance against to check the access + + Returns: + bool: True or False in case the user has access + """ + if function.author.id == user.id: return True @@ -33,6 +48,20 @@ def can_view(user, user_view_groups, function: Program) -> bool: @staticmethod def can_run(user, user_run_groups, function: Program) -> bool: + """ + Checks if the user has run access to a Function: + - If it's the author it will always have access + - a run group is in the Program.instances + + Args: + user: Django user from the request + user_run_groups: run groups from a user + function: Program instance against to check the access + + Returns: + bool: True or False in case the user has access + """ + if function.author.id == user.id: return True diff --git a/gateway/api/access_policies/providers.py b/gateway/api/access_policies/providers.py index 8aa0476db..1580d1ae8 100644 --- a/gateway/api/access_policies/providers.py +++ b/gateway/api/access_policies/providers.py @@ -1,12 +1,8 @@ """ -Access policies implementation for Program access +Access policies implementation for Provider access """ import logging -from django.contrib.auth.models import Group, Permission -from django.db.models import Q - - from api.models import Provider @@ -14,8 +10,24 @@ class ProviderAccessPolicy: + """ + The main objective of this class is to manage the access for the user + to the Provider entities. + """ + @staticmethod def can_access(user, provider: Provider) -> bool: + """ + Checks if the user has access to a Provider: + + Args: + user: Django user from the request + provider: Provider instance against to check the access + + Returns: + bool: True or False in case the user has access + """ + user_groups = user.groups.all() admin_groups = provider.admin_groups.all() has_access = any(group in admin_groups for group in user_groups) diff --git a/gateway/api/repositories/groups.py b/gateway/api/repositories/groups.py index 6bb2c6d5e..a911ec15d 100644 --- a/gateway/api/repositories/groups.py +++ b/gateway/api/repositories/groups.py @@ -1,3 +1,7 @@ +""" +Repository implementation for Groups model +""" + from typing import List from django.contrib.auth.models import Group, Permission from django.db.models import Q @@ -6,7 +10,21 @@ class GroupRepository: + """ + The main objective of this class is to manage the access to the model + """ + def get_groups_with_view_permissions_from_user(self, user) -> List[Group]: + """ + Returns all the groups with view permissions available to the user. + + Args: + user: Django user from the request + + Returns: + List[Group]: all the groups available to the user + """ + view_program_permission = Permission.objects.get( codename=VIEW_PROGRAM_PERMISSION ) @@ -16,6 +34,16 @@ def get_groups_with_view_permissions_from_user(self, user) -> List[Group]: return Group.objects.filter(user_criteria & view_permission_criteria) def get_groups_with_run_permissions_from_user(self, user) -> List[Group]: + """ + Returns all the groups with run permissions available to the user. + + Args: + user: Django user from the request + + Returns: + List[Group]: all the groups available to the user + """ + run_program_permission = Permission.objects.get(codename=RUN_PROGRAM_PERMISSION) user_criteria = Q(user=user) run_permission_criteria = Q(permissions=run_program_permission) diff --git a/gateway/api/repositories/programs.py b/gateway/api/repositories/programs.py index d642dc78d..440435898 100644 --- a/gateway/api/repositories/programs.py +++ b/gateway/api/repositories/programs.py @@ -6,9 +6,8 @@ from typing import List from django.db.models import Q -from django.contrib.auth.models import Group, Permission -from api.models import RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION, Program +from api.models import Program from api.repositories.groups import GroupRepository @@ -35,7 +34,7 @@ def get_functions_with_view_permissions(self, author) -> List[Program]: author: Django author from who retrieve the functions Returns: - List[Program] | Any: all the functions available to the user + List[Program]: all the functions available to the user """ view_groups = self.group_repository.get_groups_with_view_permissions_from_user( @@ -63,7 +62,7 @@ def get_user_functions(self, author) -> List[Program]: author: Django author from who retrieve the functions Returns: - List[Program] | Any: user functions available to the user + List[Program]: user functions available to the user """ author_criteria = Q(author=author) @@ -88,7 +87,7 @@ def get_provider_functions_with_run_permissions(self, author) -> List[Program]: author: Django author from who retrieve the functions Returns: - List[Program] | Any: providers functions available to the user + List[Program]: providers functions available to the user """ run_groups = self.group_repository.get_groups_with_run_permissions_from_user( @@ -115,7 +114,7 @@ def get_user_function_by_title(self, author, title: str) -> Program | None: title: Title that the function must have to find it Returns: - Program | Any: user function with the specific title + Program | None: user function with the specific title """ author_criteria = Q(author=author) diff --git a/gateway/tests/resources/fake_media/test_user_2/artifact_delete.tar b/gateway/tests/resources/fake_media/test_user_2/artifact_delete.tar new file mode 100644 index 000000000..24773bca6 --- /dev/null +++ b/gateway/tests/resources/fake_media/test_user_2/artifact_delete.tar @@ -0,0 +1 @@ +This is first line \ No newline at end of file From 833068fc3d6cdf2f55c7979a87e19fb32c176525 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 14:50:59 -0500 Subject: [PATCH 07/26] fixed a bug when the user retrieves a function --- gateway/api/repositories/programs.py | 8 ++++++-- gateway/api/views/files.py | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/gateway/api/repositories/programs.py b/gateway/api/repositories/programs.py index 440435898..f1208546b 100644 --- a/gateway/api/repositories/programs.py +++ b/gateway/api/repositories/programs.py @@ -159,10 +159,12 @@ def get_provider_function_by_title_with_view_permissions( user=author ) author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) + author_criteria = Q(author=author) title_criteria = Q(title=title, provider__name=provider_name) result_queryset = Program.objects.filter( - author_groups_with_view_permissions_criteria & title_criteria + (author_criteria | author_groups_with_view_permissions_criteria) + & title_criteria ).first() if result_queryset is None: @@ -201,10 +203,12 @@ def get_provider_function_by_title_with_run_permissions( user=author ) author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) + author_criteria = Q(author=author) title_criteria = Q(title=title, provider__name=provider_name) result_queryset = Program.objects.filter( - author_groups_with_run_permissions_criteria & title_criteria + (author_criteria | author_groups_with_run_permissions_criteria) + & title_criteria ).first() if result_queryset is None: diff --git a/gateway/api/views/files.py b/gateway/api/views/files.py index 1790bc8c1..b80d55a19 100644 --- a/gateway/api/views/files.py +++ b/gateway/api/views/files.py @@ -69,7 +69,7 @@ def get_function( Program | None: returns the function if it exists """ - if not provider_name: + if provider_name: return self.program_repository.get_provider_function_by_title_with_run_permissions( author=user, title=function_title, provider_name=provider_name ) From 2f760ee5d06a1a130b90e5a000f8cd2302852024 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 14:53:39 -0500 Subject: [PATCH 08/26] fix lint --- gateway/api/access_policies/programs.py | 4 ++-- gateway/api/access_policies/providers.py | 2 +- gateway/api/repositories/providers.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gateway/api/access_policies/programs.py b/gateway/api/access_policies/programs.py index 46b1c562c..38e36faa6 100644 --- a/gateway/api/access_policies/programs.py +++ b/gateway/api/access_policies/programs.py @@ -36,7 +36,7 @@ def can_view(user, user_view_groups, function: Program) -> bool: instances = function.instances.all() has_access = any(group in instances for group in user_view_groups) - # TODO: the message must be different if the function has a provider or not + # the message must be different if the function has a provider or not if not has_access: logger.warning( "User [%s] has no access to function [%s/%s].", @@ -67,7 +67,7 @@ def can_run(user, user_run_groups, function: Program) -> bool: instances = function.instances.all() has_access = any(group in instances for group in user_run_groups) - # TODO: the message must be different if the function has a provider or not + # the message must be different if the function has a provider or not if not has_access: logger.warning( "User [%s] has no access to function [%s/%s].", diff --git a/gateway/api/access_policies/providers.py b/gateway/api/access_policies/providers.py index 1580d1ae8..18a019cf0 100644 --- a/gateway/api/access_policies/providers.py +++ b/gateway/api/access_policies/providers.py @@ -9,7 +9,7 @@ logger = logging.getLogger("gateway") -class ProviderAccessPolicy: +class ProviderAccessPolicy: # pylint: disable=too-few-public-methods """ The main objective of this class is to manage the access for the user to the Provider entities. diff --git a/gateway/api/repositories/providers.py b/gateway/api/repositories/providers.py index 7d1687681..d931a4c10 100644 --- a/gateway/api/repositories/providers.py +++ b/gateway/api/repositories/providers.py @@ -9,7 +9,7 @@ logger = logging.getLogger("gateway") -class ProviderRepository: +class ProviderRepository: # pylint: disable=too-few-public-methods """ The main objective of this class is to manage the access to the model """ From 7b89093eab922609cdf421d4677762926666a5a5 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 15:01:24 -0500 Subject: [PATCH 09/26] refactor of get_function method --- gateway/api/repositories/programs.py | 23 ++++++ gateway/api/views/files.py | 103 +++++++++++++-------------- 2 files changed, 71 insertions(+), 55 deletions(-) diff --git a/gateway/api/repositories/programs.py b/gateway/api/repositories/programs.py index f1208546b..d15fbb441 100644 --- a/gateway/api/repositories/programs.py +++ b/gateway/api/repositories/programs.py @@ -220,3 +220,26 @@ def get_provider_function_by_title_with_run_permissions( ) return result_queryset + + def get_function_by_title_with_run_permissions( + self, user, function_title: str, provider_name: str | None + ) -> None: + """ + This method returns the specified function if the user is + the author of the function or it has run permissions. + + Args: + user: Django user of the function that wants to get it + function_title (str): title of the function + provider_name (str | None): name of the provider owner of the function + + Returns: + Program | None: returns the function if it exists + """ + + if provider_name: + return self.get_provider_function_by_title_with_run_permissions( + author=user, title=function_title, provider_name=provider_name + ) + + return self.get_user_function_by_title(author=user, title=function_title) diff --git a/gateway/api/views/files.py b/gateway/api/views/files.py index b80d55a19..88c65bcdc 100644 --- a/gateway/api/views/files.py +++ b/gateway/api/views/files.py @@ -54,29 +54,6 @@ class FilesViewSet(viewsets.ViewSet): program_repository = ProgramRepository() provider_repository = ProviderRepository() - def get_function( - self, user, function_title: str, provider_name: str | None - ) -> None: - """ - This method returns the specified function. - - Args: - user: Django user of the function that wants to get it - function_title (str): title of the function - provider_name (str | None): name of the provider owner of the function - - Returns: - Program | None: returns the function if it exists - """ - - if provider_name: - return self.program_repository.get_provider_function_by_title_with_run_permissions( - author=user, title=function_title, provider_name=provider_name - ) - return self.program_repository.get_user_function_by_title( - author=user, title=function_title - ) - def list(self, request): """ It returns a list with the names of available files for the user directory: @@ -97,10 +74,12 @@ def list(self, request): status=status.HTTP_400_BAD_REQUEST, ) - function = self.get_function( - user=request.user, - function_title=function_title, - provider_name=provider_name, + function = ( + self.program_repository.get_function_by_title_with_run_permissions( + user=request.user, + function_title=function_title, + provider_name=provider_name, + ) ) if not function: if provider_name: @@ -158,10 +137,12 @@ def provider_list(self, request): status=status.HTTP_404_NOT_FOUND, ) - function = self.get_function( - user=request.user, - function_title=function_title, - provider_name=provider_name, + function = ( + self.program_repository.get_function_by_title_with_run_permissions( + user=request.user, + function_title=function_title, + provider_name=provider_name, + ) ) if not function: return Response( @@ -205,10 +186,12 @@ def download(self, request): status=status.HTTP_400_BAD_REQUEST, ) - function = self.get_function( - user=request.user, - function_title=function_title, - provider_name=provider_name, + function = ( + self.program_repository.get_function_by_title_with_run_permissions( + user=request.user, + function_title=function_title, + provider_name=provider_name, + ) ) if not function: if provider_name: @@ -282,10 +265,12 @@ def provider_download(self, request): status=status.HTTP_404_NOT_FOUND, ) - function = self.get_function( - user=request.user, - function_title=function_title, - provider_name=provider_name, + function = ( + self.program_repository.get_function_by_title_with_run_permissions( + user=request.user, + function_title=function_title, + provider_name=provider_name, + ) ) if not function: return Response( @@ -336,10 +321,12 @@ def delete(self, request): status=status.HTTP_400_BAD_REQUEST, ) - function = self.get_function( - user=request.user, - function_title=function_title, - provider_name=provider_name, + function = ( + self.program_repository.get_function_by_title_with_run_permissions( + user=request.user, + function_title=function_title, + provider_name=provider_name, + ) ) if not function: @@ -403,10 +390,12 @@ def provider_delete(self, request): status=status.HTTP_404_NOT_FOUND, ) - function = self.get_function( - user=request.user, - function_title=function_title, - provider_name=provider_name, + function = ( + self.program_repository.get_function_by_title_with_run_permissions( + user=request.user, + function_title=function_title, + provider_name=provider_name, + ) ) if not function: @@ -456,10 +445,12 @@ def upload(self, request): status=status.HTTP_400_BAD_REQUEST, ) - function = self.get_function( - user=request.user, - function_title=function_title, - provider_name=provider_name, + function = ( + self.program_repository.get_function_by_title_with_run_permissions( + user=request.user, + function_title=function_title, + provider_name=provider_name, + ) ) if not function: if provider_name: @@ -519,10 +510,12 @@ def provider_upload(self, request): status=status.HTTP_404_NOT_FOUND, ) - function = self.get_function( - user=request.user, - function_title=function_title, - provider_name=provider_name, + function = ( + self.program_repository.get_function_by_title_with_run_permissions( + user=request.user, + function_title=function_title, + provider_name=provider_name, + ) ) if not function: return Response( From fd042d00712ab536a6cf5d8ae795d6aa2fff1730 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Wed, 18 Dec 2024 15:10:02 -0500 Subject: [PATCH 10/26] remove artifact test file --- .../tests/resources/fake_media/test_user_2/artifact_delete.tar | 1 - 1 file changed, 1 deletion(-) delete mode 100644 gateway/tests/resources/fake_media/test_user_2/artifact_delete.tar diff --git a/gateway/tests/resources/fake_media/test_user_2/artifact_delete.tar b/gateway/tests/resources/fake_media/test_user_2/artifact_delete.tar deleted file mode 100644 index 24773bca6..000000000 --- a/gateway/tests/resources/fake_media/test_user_2/artifact_delete.tar +++ /dev/null @@ -1 +0,0 @@ -This is first line \ No newline at end of file From 512d6409e16db5149ee7ced1d7a2d09432788968 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:25:10 -0500 Subject: [PATCH 11/26] remove programs access policies --- gateway/api/access_policies/programs.py | 78 ------------------------- 1 file changed, 78 deletions(-) delete mode 100644 gateway/api/access_policies/programs.py diff --git a/gateway/api/access_policies/programs.py b/gateway/api/access_policies/programs.py deleted file mode 100644 index 38e36faa6..000000000 --- a/gateway/api/access_policies/programs.py +++ /dev/null @@ -1,78 +0,0 @@ -""" -Access policies implementation for Program access -""" -import logging - -from api.models import Program - - -logger = logging.getLogger("gateway") - - -class ProgramAccessPolicy: - """ - The main objective of this class is to manage the access for the user - to the Program entities. - """ - - @staticmethod - def can_view(user, user_view_groups, function: Program) -> bool: - """ - Checks if the user has view access to a Function: - - If it's the author it will always have access - - a view group is in the Program.instances - - Args: - user: Django user from the request - user_view_groups: view groups from a user - function: Program instance against to check the access - - Returns: - bool: True or False in case the user has access - """ - - if function.author.id == user.id: - return True - - instances = function.instances.all() - has_access = any(group in instances for group in user_view_groups) - # the message must be different if the function has a provider or not - if not has_access: - logger.warning( - "User [%s] has no access to function [%s/%s].", - user.id, - function.provider.name, - function.title, - ) - return has_access - - @staticmethod - def can_run(user, user_run_groups, function: Program) -> bool: - """ - Checks if the user has run access to a Function: - - If it's the author it will always have access - - a run group is in the Program.instances - - Args: - user: Django user from the request - user_run_groups: run groups from a user - function: Program instance against to check the access - - Returns: - bool: True or False in case the user has access - """ - - if function.author.id == user.id: - return True - - instances = function.instances.all() - has_access = any(group in instances for group in user_run_groups) - # the message must be different if the function has a provider or not - if not has_access: - logger.warning( - "User [%s] has no access to function [%s/%s].", - user.id, - function.provider.name, - function.title, - ) - return has_access From d88c180ffa64d4b72c16313003591b05d43bd63c Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Fri, 20 Dec 2024 12:12:31 -0500 Subject: [PATCH 12/26] refactor programs references to functions --- .../{programs.py => functions.py} | 30 +++++++++---------- gateway/api/views/files.py | 28 ++++++++--------- gateway/api/views/programs.py | 4 +-- 3 files changed, 29 insertions(+), 33 deletions(-) rename gateway/api/repositories/{programs.py => functions.py} (92%) diff --git a/gateway/api/repositories/programs.py b/gateway/api/repositories/functions.py similarity index 92% rename from gateway/api/repositories/programs.py rename to gateway/api/repositories/functions.py index d15fbb441..e8959307d 100644 --- a/gateway/api/repositories/programs.py +++ b/gateway/api/repositories/functions.py @@ -7,14 +7,14 @@ from django.db.models import Q -from api.models import Program +from api.models import Program as Function from api.repositories.groups import GroupRepository logger = logging.getLogger("gateway") -class ProgramRepository: +class FunctionRepository: """ The main objective of this class is to manage the access to the model """ @@ -24,7 +24,7 @@ class ProgramRepository: # in the meantime group_repository = GroupRepository() - def get_functions_with_view_permissions(self, author) -> List[Program]: + def get_functions_with_view_permissions(self, author) -> List[Function]: """ Returns all the functions available to the user. This means: - User functions where the user is the author @@ -34,7 +34,7 @@ def get_functions_with_view_permissions(self, author) -> List[Program]: author: Django author from who retrieve the functions Returns: - List[Program]: all the functions available to the user + List[Function]: all the functions available to the user """ view_groups = self.group_repository.get_groups_with_view_permissions_from_user( @@ -43,7 +43,7 @@ def get_functions_with_view_permissions(self, author) -> List[Program]: author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) author_criteria = Q(author=author) - result_queryset = Program.objects.filter( + result_queryset = Function.objects.filter( author_criteria | author_groups_with_view_permissions_criteria ).distinct() @@ -52,7 +52,7 @@ def get_functions_with_view_permissions(self, author) -> List[Program]: return result_queryset - def get_user_functions(self, author) -> List[Program]: + def get_user_functions(self, author) -> List[Function]: """ Returns the user functions available to the user. This means: - User functions where the user is the author @@ -68,7 +68,7 @@ def get_user_functions(self, author) -> List[Program]: author_criteria = Q(author=author) provider_criteria = Q(provider=None) - result_queryset = Program.objects.filter( + result_queryset = Function.objects.filter( author_criteria & provider_criteria ).distinct() @@ -77,7 +77,7 @@ def get_user_functions(self, author) -> List[Program]: return result_queryset - def get_provider_functions_with_run_permissions(self, author) -> List[Program]: + def get_provider_functions_with_run_permissions(self, author) -> List[Function]: """ Returns the provider functions available to the user. This means: - Provider functions where the user has run permissions @@ -96,7 +96,7 @@ def get_provider_functions_with_run_permissions(self, author) -> List[Program]: author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) provider_exists_criteria = ~Q(provider=None) - result_queryset = Program.objects.filter( + result_queryset = Function.objects.filter( author_groups_with_run_permissions_criteria & provider_exists_criteria ).distinct() @@ -105,7 +105,7 @@ def get_provider_functions_with_run_permissions(self, author) -> List[Program]: return result_queryset - def get_user_function_by_title(self, author, title: str) -> Program | None: + def get_user_function_by_title(self, author, title: str) -> Function | None: """ Returns the user function associated to a title: @@ -120,7 +120,7 @@ def get_user_function_by_title(self, author, title: str) -> Program | None: author_criteria = Q(author=author) title_criteria = Q(title=title) - result_queryset = Program.objects.filter( + result_queryset = Function.objects.filter( author_criteria & title_criteria ).first() @@ -135,7 +135,7 @@ def get_user_function_by_title(self, author, title: str) -> Program | None: def get_provider_function_by_title_with_view_permissions( self, author, title: str, provider_name: str - ) -> Program | None: + ) -> Function | None: """ Returns the provider function associated to: - A Function title @@ -162,7 +162,7 @@ def get_provider_function_by_title_with_view_permissions( author_criteria = Q(author=author) title_criteria = Q(title=title, provider__name=provider_name) - result_queryset = Program.objects.filter( + result_queryset = Function.objects.filter( (author_criteria | author_groups_with_view_permissions_criteria) & title_criteria ).first() @@ -179,7 +179,7 @@ def get_provider_function_by_title_with_view_permissions( def get_provider_function_by_title_with_run_permissions( self, author, title: str, provider_name: str - ) -> Program | None: + ) -> Function | None: """ Returns the provider function associated to: - A Function title @@ -206,7 +206,7 @@ def get_provider_function_by_title_with_run_permissions( author_criteria = Q(author=author) title_criteria = Q(title=title, provider__name=provider_name) - result_queryset = Program.objects.filter( + result_queryset = Function.objects.filter( (author_criteria | author_groups_with_run_permissions_criteria) & title_criteria ).first() diff --git a/gateway/api/views/files.py b/gateway/api/views/files.py index 88c65bcdc..8bec08080 100644 --- a/gateway/api/views/files.py +++ b/gateway/api/views/files.py @@ -20,7 +20,7 @@ from rest_framework.response import Response from api.access_policies.providers import ProviderAccessPolicy -from api.repositories.programs import ProgramRepository +from api.repositories.functions import FunctionRepository from api.repositories.providers import ProviderRepository from api.services.file_storage import FileStorage, WorkingDir from api.utils import sanitize_file_name, sanitize_name @@ -51,7 +51,7 @@ class FilesViewSet(viewsets.ViewSet): BASE_NAME = "files" - program_repository = ProgramRepository() + function_repository = FunctionRepository() provider_repository = ProviderRepository() def list(self, request): @@ -75,7 +75,7 @@ def list(self, request): ) function = ( - self.program_repository.get_function_by_title_with_run_permissions( + self.function_repository.get_function_by_title_with_run_permissions( user=request.user, function_title=function_title, provider_name=provider_name, @@ -138,7 +138,7 @@ def provider_list(self, request): ) function = ( - self.program_repository.get_function_by_title_with_run_permissions( + self.function_repository.get_function_by_title_with_run_permissions( user=request.user, function_title=function_title, provider_name=provider_name, @@ -187,7 +187,7 @@ def download(self, request): ) function = ( - self.program_repository.get_function_by_title_with_run_permissions( + self.function_repository.get_function_by_title_with_run_permissions( user=request.user, function_title=function_title, provider_name=provider_name, @@ -266,7 +266,7 @@ def provider_download(self, request): ) function = ( - self.program_repository.get_function_by_title_with_run_permissions( + self.function_repository.get_function_by_title_with_run_permissions( user=request.user, function_title=function_title, provider_name=provider_name, @@ -303,12 +303,10 @@ def provider_download(self, request): @action(methods=["DELETE"], detail=False) def delete(self, request): - """Deletes file uploaded or produced by the programs,""" - # default response for file not found, overwritten if file is found + """Deletes file uploaded or produced by the functions""" tracer = trace.get_tracer("gateway.tracer") ctx = TraceContextTextMapPropagator().extract(carrier=request.headers) with tracer.start_as_current_span("gateway.files.delete", context=ctx): - # look for file in user's folder username = request.user.username file_name = sanitize_file_name(request.query_params.get("file", None)) provider_name = sanitize_name(request.query_params.get("provider")) @@ -322,7 +320,7 @@ def delete(self, request): ) function = ( - self.program_repository.get_function_by_title_with_run_permissions( + self.function_repository.get_function_by_title_with_run_permissions( user=request.user, function_title=function_title, provider_name=provider_name, @@ -358,12 +356,10 @@ def delete(self, request): @action(methods=["DELETE"], detail=False, url_path="provider/delete") def provider_delete(self, request): - """Deletes file uploaded or produced by the programs,""" - # default response for file not found, overwritten if file is found + """Deletes file uploaded or produced by the functions""" tracer = trace.get_tracer("gateway.tracer") ctx = TraceContextTextMapPropagator().extract(carrier=request.headers) with tracer.start_as_current_span("gateway.files.delete", context=ctx): - # look for file in user's folder username = request.user.username file_name = sanitize_file_name(request.query_params.get("file")) provider_name = sanitize_name(request.query_params.get("provider")) @@ -391,7 +387,7 @@ def provider_delete(self, request): ) function = ( - self.program_repository.get_function_by_title_with_run_permissions( + self.function_repository.get_function_by_title_with_run_permissions( user=request.user, function_title=function_title, provider_name=provider_name, @@ -446,7 +442,7 @@ def upload(self, request): ) function = ( - self.program_repository.get_function_by_title_with_run_permissions( + self.function_repository.get_function_by_title_with_run_permissions( user=request.user, function_title=function_title, provider_name=provider_name, @@ -511,7 +507,7 @@ def provider_upload(self, request): ) function = ( - self.program_repository.get_function_by_title_with_run_permissions( + self.function_repository.get_function_by_title_with_run_permissions( user=request.user, function_title=function_title, provider_name=provider_name, diff --git a/gateway/api/views/programs.py b/gateway/api/views/programs.py index 2f8d9159b..e2407e6b3 100644 --- a/gateway/api/views/programs.py +++ b/gateway/api/views/programs.py @@ -20,7 +20,7 @@ from rest_framework import viewsets, status from rest_framework.response import Response -from api.repositories.programs import ProgramRepository +from api.repositories.functions import FunctionRepository from api.utils import sanitize_name from api.serializers import ( JobConfigSerializer, @@ -56,7 +56,7 @@ class ProgramViewSet(viewsets.GenericViewSet): BASE_NAME = "programs" - program_repository = ProgramRepository() + program_repository = FunctionRepository() @staticmethod def get_serializer_job_config(*args, **kwargs): From 99a0fd53c141f9ef746682ff28a3919285e6a303 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Fri, 20 Dec 2024 12:21:26 -0500 Subject: [PATCH 13/26] group repository refactor --- gateway/api/repositories/functions.py | 22 ++++++++++------- gateway/api/repositories/groups.py | 34 ++++++--------------------- 2 files changed, 20 insertions(+), 36 deletions(-) diff --git a/gateway/api/repositories/functions.py b/gateway/api/repositories/functions.py index e8959307d..517b9c5ca 100644 --- a/gateway/api/repositories/functions.py +++ b/gateway/api/repositories/functions.py @@ -7,7 +7,11 @@ from django.db.models import Q -from api.models import Program as Function +from api.models import ( + RUN_PROGRAM_PERMISSION, + VIEW_PROGRAM_PERMISSION, + Program as Function, +) from api.repositories.groups import GroupRepository @@ -37,8 +41,8 @@ def get_functions_with_view_permissions(self, author) -> List[Function]: List[Function]: all the functions available to the user """ - view_groups = self.group_repository.get_groups_with_view_permissions_from_user( - user=author + view_groups = self.group_repository.get_groups_by_permissions( + user=author, permission_name=VIEW_PROGRAM_PERMISSION ) author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) author_criteria = Q(author=author) @@ -90,8 +94,8 @@ def get_provider_functions_with_run_permissions(self, author) -> List[Function]: List[Program]: providers functions available to the user """ - run_groups = self.group_repository.get_groups_with_run_permissions_from_user( - user=author + run_groups = self.group_repository.get_groups_by_permissions( + user=author, permission_name=RUN_PROGRAM_PERMISSION ) author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) provider_exists_criteria = ~Q(provider=None) @@ -155,8 +159,8 @@ def get_provider_function_by_title_with_view_permissions( # This access should be checked in the use-case but how we don't # have it implemented yet we will do the check by now in the # repository call - view_groups = self.group_repository.get_groups_with_view_permissions_from_user( - user=author + view_groups = self.group_repository.get_groups_by_permissions( + user=author, permission_name=VIEW_PROGRAM_PERMISSION ) author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) author_criteria = Q(author=author) @@ -199,8 +203,8 @@ def get_provider_function_by_title_with_run_permissions( # This access should be checked in the use-case but how we don't # have it implemented yet we will do the check by now in the # repository call - run_groups = self.group_repository.get_groups_with_run_permissions_from_user( - user=author + run_groups = self.group_repository.get_groups_by_permissions( + user=author, permission_name=RUN_PROGRAM_PERMISSION ) author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) author_criteria = Q(author=author) diff --git a/gateway/api/repositories/groups.py b/gateway/api/repositories/groups.py index a911ec15d..c6b7ab144 100644 --- a/gateway/api/repositories/groups.py +++ b/gateway/api/repositories/groups.py @@ -6,46 +6,26 @@ from django.contrib.auth.models import Group, Permission from django.db.models import Q -from api.models import RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION - -class GroupRepository: +class GroupRepository: # pylint: disable=too-few-public-methods """ The main objective of this class is to manage the access to the model """ - def get_groups_with_view_permissions_from_user(self, user) -> List[Group]: - """ - Returns all the groups with view permissions available to the user. - - Args: - user: Django user from the request - - Returns: - List[Group]: all the groups available to the user - """ - - view_program_permission = Permission.objects.get( - codename=VIEW_PROGRAM_PERMISSION - ) - user_criteria = Q(user=user) - view_permission_criteria = Q(permissions=view_program_permission) - - return Group.objects.filter(user_criteria & view_permission_criteria) - - def get_groups_with_run_permissions_from_user(self, user) -> List[Group]: + def get_groups_by_permissions(self, user, permission_name: str) -> List[Group]: """ - Returns all the groups with run permissions available to the user. + Returns all the groups associated to a permission available in the user. Args: user: Django user from the request + permission Returns: List[Group]: all the groups available to the user """ - run_program_permission = Permission.objects.get(codename=RUN_PROGRAM_PERMISSION) + function_permission = Permission.objects.get(codename=permission_name) user_criteria = Q(user=user) - run_permission_criteria = Q(permissions=run_program_permission) + permission_criteria = Q(permissions=function_permission) - return Group.objects.filter(user_criteria & run_permission_criteria) + return Group.objects.filter(user_criteria & permission_criteria) From d50a4d297d139620a6add82cf3d6a40358cb7567 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Fri, 20 Dec 2024 12:27:21 -0500 Subject: [PATCH 14/26] rename groups repository into user repository --- gateway/api/repositories/functions.py | 12 ++++++------ gateway/api/repositories/{groups.py => users.py} | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) rename gateway/api/repositories/{groups.py => users.py} (93%) diff --git a/gateway/api/repositories/functions.py b/gateway/api/repositories/functions.py index 517b9c5ca..3aa17f03f 100644 --- a/gateway/api/repositories/functions.py +++ b/gateway/api/repositories/functions.py @@ -12,7 +12,7 @@ VIEW_PROGRAM_PERMISSION, Program as Function, ) -from api.repositories.groups import GroupRepository +from api.repositories.users import UserRepository logger = logging.getLogger("gateway") @@ -26,7 +26,7 @@ class FunctionRepository: # This repository should be in the use case implementatio # but this class is not ready yet so it will live here # in the meantime - group_repository = GroupRepository() + user_repository = UserRepository() def get_functions_with_view_permissions(self, author) -> List[Function]: """ @@ -41,7 +41,7 @@ def get_functions_with_view_permissions(self, author) -> List[Function]: List[Function]: all the functions available to the user """ - view_groups = self.group_repository.get_groups_by_permissions( + view_groups = self.user_repository.get_groups_by_permissions( user=author, permission_name=VIEW_PROGRAM_PERMISSION ) author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) @@ -94,7 +94,7 @@ def get_provider_functions_with_run_permissions(self, author) -> List[Function]: List[Program]: providers functions available to the user """ - run_groups = self.group_repository.get_groups_by_permissions( + run_groups = self.user_repository.get_groups_by_permissions( user=author, permission_name=RUN_PROGRAM_PERMISSION ) author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) @@ -159,7 +159,7 @@ def get_provider_function_by_title_with_view_permissions( # This access should be checked in the use-case but how we don't # have it implemented yet we will do the check by now in the # repository call - view_groups = self.group_repository.get_groups_by_permissions( + view_groups = self.user_repository.get_groups_by_permissions( user=author, permission_name=VIEW_PROGRAM_PERMISSION ) author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) @@ -203,7 +203,7 @@ def get_provider_function_by_title_with_run_permissions( # This access should be checked in the use-case but how we don't # have it implemented yet we will do the check by now in the # repository call - run_groups = self.group_repository.get_groups_by_permissions( + run_groups = self.user_repository.get_groups_by_permissions( user=author, permission_name=RUN_PROGRAM_PERMISSION ) author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) diff --git a/gateway/api/repositories/groups.py b/gateway/api/repositories/users.py similarity index 93% rename from gateway/api/repositories/groups.py rename to gateway/api/repositories/users.py index c6b7ab144..a6b3952a4 100644 --- a/gateway/api/repositories/groups.py +++ b/gateway/api/repositories/users.py @@ -7,7 +7,7 @@ from django.db.models import Q -class GroupRepository: # pylint: disable=too-few-public-methods +class UserRepository: # pylint: disable=too-few-public-methods """ The main objective of this class is to manage the access to the model """ From 1fb6ba9211c6a4747ac9d7833a3c18980924bae8 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Fri, 20 Dec 2024 13:33:08 -0500 Subject: [PATCH 15/26] simplified get_function methods --- gateway/api/repositories/functions.py | 71 +++++---------------- gateway/api/views/files.py | 89 ++++++++++++--------------- gateway/api/views/programs.py | 11 ++-- 3 files changed, 65 insertions(+), 106 deletions(-) diff --git a/gateway/api/repositories/functions.py b/gateway/api/repositories/functions.py index 3aa17f03f..cedde324f 100644 --- a/gateway/api/repositories/functions.py +++ b/gateway/api/repositories/functions.py @@ -23,7 +23,7 @@ class FunctionRepository: The main objective of this class is to manage the access to the model """ - # This repository should be in the use case implementatio + # This repository should be in the use case implementation # but this class is not ready yet so it will live here # in the meantime user_repository = UserRepository() @@ -109,7 +109,7 @@ def get_provider_functions_with_run_permissions(self, author) -> List[Function]: return result_queryset - def get_user_function_by_title(self, author, title: str) -> Function | None: + def get_user_function(self, author, title: str) -> Function | None: """ Returns the user function associated to a title: @@ -137,8 +137,8 @@ def get_user_function_by_title(self, author, title: str) -> Function | None: return result_queryset - def get_provider_function_by_title_with_view_permissions( - self, author, title: str, provider_name: str + def get_provider_function_by_permission( + self, author, permission_name: str, title: str, provider_name: str ) -> Function | None: """ Returns the provider function associated to: @@ -160,7 +160,7 @@ def get_provider_function_by_title_with_view_permissions( # have it implemented yet we will do the check by now in the # repository call view_groups = self.user_repository.get_groups_by_permissions( - user=author, permission_name=VIEW_PROGRAM_PERMISSION + user=author, permission_name=permission_name ) author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) author_criteria = Q(author=author) @@ -181,52 +181,12 @@ def get_provider_function_by_title_with_view_permissions( return result_queryset - def get_provider_function_by_title_with_run_permissions( - self, author, title: str, provider_name: str - ) -> Function | None: - """ - Returns the provider function associated to: - - A Function title - - A Provider - - Author must have run permission to execute it or be the author - - Args: - author: Django author from who retrieve the function - title: Title that the function must have to find it - provider: Provider associated to the function - - Returns: - Program | None: provider function with the specific - title and provider - """ - - # This access should be checked in the use-case but how we don't - # have it implemented yet we will do the check by now in the - # repository call - run_groups = self.user_repository.get_groups_by_permissions( - user=author, permission_name=RUN_PROGRAM_PERMISSION - ) - author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) - author_criteria = Q(author=author) - title_criteria = Q(title=title, provider__name=provider_name) - - result_queryset = Function.objects.filter( - (author_criteria | author_groups_with_run_permissions_criteria) - & title_criteria - ).first() - - if result_queryset is None: - logger.warning( - "Function [%s/%s] was not found or author [%s] doesn't have access to it", - provider_name, - title, - author.id, - ) - - return result_queryset - - def get_function_by_title_with_run_permissions( - self, user, function_title: str, provider_name: str | None + def get_function_by_permission( + self, + user, + permission_name: str, + function_title: str, + provider_name: str | None, ) -> None: """ This method returns the specified function if the user is @@ -242,8 +202,11 @@ def get_function_by_title_with_run_permissions( """ if provider_name: - return self.get_provider_function_by_title_with_run_permissions( - author=user, title=function_title, provider_name=provider_name + return self.get_provider_function_by_permission( + author=user, + permission_name=permission_name, + title=function_title, + provider_name=provider_name, ) - return self.get_user_function_by_title(author=user, title=function_title) + return self.get_user_function(author=user, title=function_title) diff --git a/gateway/api/views/files.py b/gateway/api/views/files.py index 8bec08080..0c9e44c33 100644 --- a/gateway/api/views/files.py +++ b/gateway/api/views/files.py @@ -20,6 +20,7 @@ from rest_framework.response import Response from api.access_policies.providers import ProviderAccessPolicy +from api.models import RUN_PROGRAM_PERMISSION from api.repositories.functions import FunctionRepository from api.repositories.providers import ProviderRepository from api.services.file_storage import FileStorage, WorkingDir @@ -74,12 +75,11 @@ def list(self, request): status=status.HTTP_400_BAD_REQUEST, ) - function = ( - self.function_repository.get_function_by_title_with_run_permissions( - user=request.user, - function_title=function_title, - provider_name=provider_name, - ) + function = self.function_repository.get_function_by_permission( + user=request.user, + permission_name=RUN_PROGRAM_PERMISSION, + function_title=function_title, + provider_name=provider_name, ) if not function: if provider_name: @@ -137,12 +137,11 @@ def provider_list(self, request): status=status.HTTP_404_NOT_FOUND, ) - function = ( - self.function_repository.get_function_by_title_with_run_permissions( - user=request.user, - function_title=function_title, - provider_name=provider_name, - ) + function = self.function_repository.get_function_by_permission( + user=request.user, + permission_name=RUN_PROGRAM_PERMISSION, + function_title=function_title, + provider_name=provider_name, ) if not function: return Response( @@ -186,12 +185,11 @@ def download(self, request): status=status.HTTP_400_BAD_REQUEST, ) - function = ( - self.function_repository.get_function_by_title_with_run_permissions( - user=request.user, - function_title=function_title, - provider_name=provider_name, - ) + function = self.function_repository.get_function_by_permission( + user=request.user, + permission_name=RUN_PROGRAM_PERMISSION, + function_title=function_title, + provider_name=provider_name, ) if not function: if provider_name: @@ -265,12 +263,11 @@ def provider_download(self, request): status=status.HTTP_404_NOT_FOUND, ) - function = ( - self.function_repository.get_function_by_title_with_run_permissions( - user=request.user, - function_title=function_title, - provider_name=provider_name, - ) + function = self.function_repository.get_function_by_permission( + user=request.user, + permission_name=RUN_PROGRAM_PERMISSION, + function_title=function_title, + provider_name=provider_name, ) if not function: return Response( @@ -319,12 +316,11 @@ def delete(self, request): status=status.HTTP_400_BAD_REQUEST, ) - function = ( - self.function_repository.get_function_by_title_with_run_permissions( - user=request.user, - function_title=function_title, - provider_name=provider_name, - ) + function = self.function_repository.get_function_by_permission( + user=request.user, + permission_name=RUN_PROGRAM_PERMISSION, + function_title=function_title, + provider_name=provider_name, ) if not function: @@ -386,12 +382,11 @@ def provider_delete(self, request): status=status.HTTP_404_NOT_FOUND, ) - function = ( - self.function_repository.get_function_by_title_with_run_permissions( - user=request.user, - function_title=function_title, - provider_name=provider_name, - ) + function = self.function_repository.get_function_by_permission( + user=request.user, + permission_name=RUN_PROGRAM_PERMISSION, + function_title=function_title, + provider_name=provider_name, ) if not function: @@ -441,12 +436,11 @@ def upload(self, request): status=status.HTTP_400_BAD_REQUEST, ) - function = ( - self.function_repository.get_function_by_title_with_run_permissions( - user=request.user, - function_title=function_title, - provider_name=provider_name, - ) + function = self.function_repository.get_function_by_permission( + user=request.user, + permission_name=RUN_PROGRAM_PERMISSION, + function_title=function_title, + provider_name=provider_name, ) if not function: if provider_name: @@ -506,12 +500,11 @@ def provider_upload(self, request): status=status.HTTP_404_NOT_FOUND, ) - function = ( - self.function_repository.get_function_by_title_with_run_permissions( - user=request.user, - function_title=function_title, - provider_name=provider_name, - ) + function = self.function_repository.get_function_by_permission( + user=request.user, + permission_name=RUN_PROGRAM_PERMISSION, + function_title=function_title, + provider_name=provider_name, ) if not function: return Response( diff --git a/gateway/api/views/programs.py b/gateway/api/views/programs.py index e2407e6b3..e0ca9c048 100644 --- a/gateway/api/views/programs.py +++ b/gateway/api/views/programs.py @@ -29,7 +29,7 @@ RunProgramSerializer, UploadProgramSerializer, ) -from api.models import RUN_PROGRAM_PERMISSION, Program, Job +from api.models import RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION, Program, Job from api.views.enums.type_filter import TypeFilter # pylint: disable=duplicate-code @@ -309,11 +309,14 @@ def get_by_title(self, request, title): ) if provider_name: - function = self.program_repository.get_provider_function_by_title_with_view_permissions( - author=author, title=function_title, provider_name=provider_name + function = self.program_repository.get_provider_function_by_permission( + author=author, + permission_name=VIEW_PROGRAM_PERMISSION, + title=function_title, + provider_name=provider_name, ) else: - function = self.program_repository.get_user_function_by_title( + function = self.program_repository.get_user_function( author=author, title=function_title ) From 1869acaddd2bf3f8ad76fb2c7510f64807f7056b Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Fri, 20 Dec 2024 13:37:17 -0500 Subject: [PATCH 16/26] fix query --- gateway/api/repositories/functions.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gateway/api/repositories/functions.py b/gateway/api/repositories/functions.py index cedde324f..0306bddac 100644 --- a/gateway/api/repositories/functions.py +++ b/gateway/api/repositories/functions.py @@ -99,9 +99,11 @@ def get_provider_functions_with_run_permissions(self, author) -> List[Function]: ) author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) provider_exists_criteria = ~Q(provider=None) + author_criteria = Q(author=author) result_queryset = Function.objects.filter( - author_groups_with_run_permissions_criteria & provider_exists_criteria + (author_groups_with_run_permissions_criteria & provider_exists_criteria) + | (author_criteria & provider_exists_criteria) ).distinct() count = result_queryset.count() From 82aef0d74d64b0e1a286bb13d3289cf82c03d3dd Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Fri, 20 Dec 2024 13:42:13 -0500 Subject: [PATCH 17/26] adapt get_functions methods --- gateway/api/repositories/functions.py | 19 ++++++++++--------- gateway/api/views/programs.py | 8 ++++---- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/gateway/api/repositories/functions.py b/gateway/api/repositories/functions.py index 0306bddac..e77771f2f 100644 --- a/gateway/api/repositories/functions.py +++ b/gateway/api/repositories/functions.py @@ -7,11 +7,8 @@ from django.db.models import Q -from api.models import ( - RUN_PROGRAM_PERMISSION, - VIEW_PROGRAM_PERMISSION, - Program as Function, -) +from api.models import Program as Function + from api.repositories.users import UserRepository @@ -28,7 +25,9 @@ class FunctionRepository: # in the meantime user_repository = UserRepository() - def get_functions_with_view_permissions(self, author) -> List[Function]: + def get_functions_by_permission( + self, author, permission_name: str + ) -> List[Function]: """ Returns all the functions available to the user. This means: - User functions where the user is the author @@ -42,7 +41,7 @@ def get_functions_with_view_permissions(self, author) -> List[Function]: """ view_groups = self.user_repository.get_groups_by_permissions( - user=author, permission_name=VIEW_PROGRAM_PERMISSION + user=author, permission_name=permission_name ) author_groups_with_view_permissions_criteria = Q(instances__in=view_groups) author_criteria = Q(author=author) @@ -81,7 +80,9 @@ def get_user_functions(self, author) -> List[Function]: return result_queryset - def get_provider_functions_with_run_permissions(self, author) -> List[Function]: + def get_provider_functions_by_permission( + self, author, permission_name: str + ) -> List[Function]: """ Returns the provider functions available to the user. This means: - Provider functions where the user has run permissions @@ -95,7 +96,7 @@ def get_provider_functions_with_run_permissions(self, author) -> List[Function]: """ run_groups = self.user_repository.get_groups_by_permissions( - user=author, permission_name=RUN_PROGRAM_PERMISSION + user=author, permission_name=permission_name ) author_groups_with_run_permissions_criteria = Q(instances__in=run_groups) provider_exists_criteria = ~Q(provider=None) diff --git a/gateway/api/views/programs.py b/gateway/api/views/programs.py index e0ca9c048..e3682ff29 100644 --- a/gateway/api/views/programs.py +++ b/gateway/api/views/programs.py @@ -161,14 +161,14 @@ def list(self, request): # Catalog filter only returns providers functions that user has access: # author has view permissions and the function has a provider assigned functions = ( - self.program_repository.get_provider_functions_with_run_permissions( - author + self.program_repository.get_provider_functions_by_permission( + author, permission_name=RUN_PROGRAM_PERMISSION ) ) else: # If filter is not applied we return author and providers functions together - functions = self.program_repository.get_functions_with_view_permissions( - author + functions = self.program_repository.get_functions_by_permission( + author, permission_name=VIEW_PROGRAM_PERMISSION ) serializer = self.get_serializer(functions, many=True) From da9bf35f1f310947ffc58692d565870262665826 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Fri, 20 Dec 2024 14:03:43 -0500 Subject: [PATCH 18/26] updated comments --- gateway/api/repositories/functions.py | 25 +++++++++++++++++-------- gateway/api/repositories/providers.py | 4 ++-- gateway/api/repositories/users.py | 2 +- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/gateway/api/repositories/functions.py b/gateway/api/repositories/functions.py index e77771f2f..33020f8e7 100644 --- a/gateway/api/repositories/functions.py +++ b/gateway/api/repositories/functions.py @@ -30,11 +30,13 @@ def get_functions_by_permission( ) -> List[Function]: """ Returns all the functions available to the user. This means: - - User functions where the user is the author - - Provider functions with view permissions + - User functions where the user is the author + - Provider functions with the permission specified Args: author: Django author from who retrieve the functions + permission_name (str): name of the permission. Values accepted + RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION Returns: List[Function]: all the functions available to the user @@ -86,10 +88,13 @@ def get_provider_functions_by_permission( """ Returns the provider functions available to the user. This means: - Provider functions where the user has run permissions + - Provider functions where the user is the author - Provider is NOT None Args: author: Django author from who retrieve the functions + permission_name (str): name of the permission. Values accepted + RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION Returns: List[Program]: providers functions available to the user @@ -118,7 +123,7 @@ def get_user_function(self, author, title: str) -> Function | None: Args: author: Django author from who retrieve the function - title: Title that the function must have to find it + title (str): Title that the function must have to find it Returns: Program | None: user function with the specific title @@ -147,16 +152,18 @@ def get_provider_function_by_permission( Returns the provider function associated to: - A Function title - A Provider - - Author must have view permission to see it or be the author + - Author must have a permission to see it or be the author Args: author: Django author from who retrieve the function - title: Title that the function must have to find it - provider: Provider associated to the function + permission_name (str): name of the permission. Values accepted + RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION + title (str): Title that the function must have to find it + provider (str): the name of the provider Returns: Program | None: provider function with the specific - title and provider + title and provider """ # This access should be checked in the use-case but how we don't @@ -193,10 +200,12 @@ def get_function_by_permission( ) -> None: """ This method returns the specified function if the user is - the author of the function or it has run permissions. + the author of the function or it has a permission. Args: user: Django user of the function that wants to get it + permission_name (str): name of the permission. Values accepted + RUN_PROGRAM_PERMISSION, VIEW_PROGRAM_PERMISSION function_title (str): title of the function provider_name (str | None): name of the provider owner of the function diff --git a/gateway/api/repositories/providers.py b/gateway/api/repositories/providers.py index d931a4c10..cc3e31125 100644 --- a/gateway/api/repositories/providers.py +++ b/gateway/api/repositories/providers.py @@ -19,10 +19,10 @@ def get_provider_by_name(self, name: str) -> Provider | None: Returns the provider associated with a name. Args: - name: provider name + - name: provider name Returns: - Provider | None: returns the specific provider if it exists + - Provider | None: returns the specific provider if it exists """ provider = Provider.objects.filter(name=name).first() diff --git a/gateway/api/repositories/users.py b/gateway/api/repositories/users.py index a6b3952a4..5827e4478 100644 --- a/gateway/api/repositories/users.py +++ b/gateway/api/repositories/users.py @@ -18,7 +18,7 @@ def get_groups_by_permissions(self, user, permission_name: str) -> List[Group]: Args: user: Django user from the request - permission + permission_name (str): name of the permission by look for Returns: List[Group]: all the groups available to the user From 81896a2f463a3227395d0234988c5afd5f8b24cc Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 26 Dec 2024 11:19:26 -0500 Subject: [PATCH 19/26] create path if doesn't exist --- gateway/api/services/file_storage.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/gateway/api/services/file_storage.py b/gateway/api/services/file_storage.py index bdf6cdf73..0e2fe8deb 100644 --- a/gateway/api/services/file_storage.py +++ b/gateway/api/services/file_storage.py @@ -82,7 +82,12 @@ def __get_user_path(self, function_title: str, provider_name: str | None) -> str settings.MEDIA_ROOT, self.username, provider_name, function_title ) - return sanitize_file_path(path) + sanitized_path = sanitize_file_path(path) + + # Create directory if it doesn't exist + os.makedirs(os.path.dirname(sanitized_path), exist_ok=True) + + return sanitized_path def __get_provider_path(self, function_title: str, provider_name: str) -> str: """ @@ -99,7 +104,12 @@ def __get_provider_path(self, function_title: str, provider_name: str) -> str: """ path = os.path.join(settings.MEDIA_ROOT, provider_name, function_title) - return sanitize_file_path(path) + sanitized_path = sanitize_file_path(path) + + # Create directory if it doesn't exist + os.makedirs(os.path.dirname(sanitized_path), exist_ok=True) + + return sanitized_path def get_files(self) -> list[str]: """ From 305775b80c29a90455c40f9985dc25d54656988e Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 26 Dec 2024 11:28:53 -0500 Subject: [PATCH 20/26] remove some unused code --- gateway/api/services/file_storage.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/gateway/api/services/file_storage.py b/gateway/api/services/file_storage.py index 0e2fe8deb..d7b7653bc 100644 --- a/gateway/api/services/file_storage.py +++ b/gateway/api/services/file_storage.py @@ -121,14 +121,6 @@ def get_files(self) -> list[str]: list[str]: list of file names """ - if not os.path.exists(self.file_path): - logger.warning( - "Directory %s does not exist for %s.", - self.file_path, - self.username, - ) - return [] - return [ os.path.basename(path) for path in glob.glob(f"{self.file_path}/*") @@ -155,9 +147,9 @@ def get_file(self, file_name: str) -> Optional[Tuple[FileWrapper, str, int]]: if not os.path.exists(path_to_file): logger.warning( - "Directory %s does not exist for file %s.", - path_to_file, + "File %s not found in %s.", file_name_path, + path_to_file, ) return None @@ -214,9 +206,9 @@ def remove_file(self, file_name: str) -> bool: os.remove(path_to_file) except FileNotFoundError: logger.warning( - "Directory %s does not exist for file %s.", - path_to_file, + "File %s not found in %s.", file_name_path, + path_to_file, ) return False except OSError as ex: From bcf947c2359a1b7070fa231492392c64f126d6f6 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 26 Dec 2024 11:38:56 -0500 Subject: [PATCH 21/26] fix files client --- .../core/clients/serverless_client.py | 8 +++--- client/qiskit_serverless/core/files.py | 26 ++++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/client/qiskit_serverless/core/clients/serverless_client.py b/client/qiskit_serverless/core/clients/serverless_client.py index 79e335c9b..7bc4f2468 100644 --- a/client/qiskit_serverless/core/clients/serverless_client.py +++ b/client/qiskit_serverless/core/clients/serverless_client.py @@ -437,14 +437,14 @@ def provider_file_delete(self, file: str, function: QiskitFunction, provider: st return self._files_client.provider_delete(file, function, provider) def file_upload( - self, file: str, function: QiskitFunction, provider: Optional[str] = None + self, file: str, function: QiskitFunction ): """Upload file.""" - return self._files_client.upload(file, function, provider) + return self._files_client.upload(file, function) - def provider_file_upload(self, file: str, function: QiskitFunction, provider: str): + def provider_file_upload(self, file: str, function: QiskitFunction): """Upload file.""" - return self._files_client.provider_upload(file, function, provider) + return self._files_client.provider_upload(file, function) class IBMServerlessClient(ServerlessClient): diff --git a/client/qiskit_serverless/core/files.py b/client/qiskit_serverless/core/files.py index ef5a42522..682e70032 100644 --- a/client/qiskit_serverless/core/files.py +++ b/client/qiskit_serverless/core/files.py @@ -133,14 +133,14 @@ def provider_download( @_trace def upload( - self, file: str, function: QiskitFunction, provider: Optional[str] = None + self, file: str, function: QiskitFunction ) -> Optional[str]: """Uploads file.""" with open(file, "rb") as f: with requests.post( os.path.join(self._files_url, "upload/"), files={"file": f}, - params={"provider": provider, "function": function.title}, + params={"provider": function.provider, "function": function.title}, stream=True, headers={"Authorization": f"Bearer {self._token}"}, timeout=REQUESTS_STREAMING_TIMEOUT, @@ -152,14 +152,17 @@ def upload( @_trace def provider_upload( - self, file: str, function: QiskitFunction, provider: str + self, file: str, function: QiskitFunction ) -> Optional[str]: + if not function.provider: + raise QiskitServerlessException("`function` doesn't have a provider.") + """Uploads file to provider/function file storage.""" with open(file, "rb") as f: with requests.post( os.path.join(self._files_url, "upload/"), files={"file": f}, - params={"provider": provider, "function": function.title}, + params={"provider": function.provider, "function": function.title}, stream=True, headers={"Authorization": f"Bearer {self._token}"}, timeout=REQUESTS_STREAMING_TIMEOUT, @@ -175,7 +178,7 @@ def list(self, function: QiskitFunction) -> List[str]: response_data = safe_json_request_as_dict( request=lambda: requests.get( self._files_url, - params={"function": function.title}, + params={"function": function.title, "provider": function.provider}, headers={"Authorization": f"Bearer {self._token}"}, timeout=REQUESTS_TIMEOUT, ) @@ -191,7 +194,7 @@ def provider_list(self, function: QiskitFunction) -> List[str]: response_data = safe_json_request_as_dict( request=lambda: requests.get( os.path.join(self._files_url, "provider"), - params={"provider": function.provider, "function": function.title}, + params={"function": function.title, "provider": function.provider}, headers={"Authorization": f"Bearer {self._token}"}, timeout=REQUESTS_TIMEOUT, ) @@ -200,7 +203,7 @@ def provider_list(self, function: QiskitFunction) -> List[str]: @_trace def delete( - self, file: str, function: QiskitFunction, provider: Optional[str] = None + self, file: str, function: QiskitFunction ) -> Optional[str]: """Deletes file uploaded or produced by the programs,""" response_data = safe_json_request_as_dict( @@ -209,7 +212,7 @@ def delete( params={ "file": file, "function": function.title, - "provider": provider, + "provider": function.provider, }, headers={ "Authorization": f"Bearer {self._token}", @@ -222,8 +225,11 @@ def delete( @_trace def provider_delete( - self, file: str, function: QiskitFunction, provider: str + self, file: str, function: QiskitFunction ) -> Optional[str]: + if not function.provider: + raise QiskitServerlessException("`function` doesn't have a provider.") + """Deletes file uploaded or produced by the programs,""" response_data = safe_json_request_as_dict( request=lambda: requests.delete( @@ -231,7 +237,7 @@ def provider_delete( params={ "file": file, "function": function.title, - "provider": provider, + "provider": function.provider, }, headers={ "Authorization": f"Bearer {self._token}", From d426ec4aaed503846247759f57b34e615c9c8fdb Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 26 Dec 2024 11:43:56 -0500 Subject: [PATCH 22/26] fix typos --- .../core/clients/serverless_client.py | 14 +++++-------- client/qiskit_serverless/core/files.py | 20 ++++++------------- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/client/qiskit_serverless/core/clients/serverless_client.py b/client/qiskit_serverless/core/clients/serverless_client.py index 7bc4f2468..b34a9155e 100644 --- a/client/qiskit_serverless/core/clients/serverless_client.py +++ b/client/qiskit_serverless/core/clients/serverless_client.py @@ -426,19 +426,15 @@ def provider_file_download( file, download_location, function, target_name ) - def file_delete( - self, file: str, function: QiskitFunction, provider: Optional[str] = None - ): + def file_delete(self, file: str, function: QiskitFunction): """Deletes file uploaded or produced by the programs,""" - return self._files_client.delete(file, function, provider) + return self._files_client.delete(file, function) - def provider_file_delete(self, file: str, function: QiskitFunction, provider: str): + def provider_file_delete(self, file: str, function: QiskitFunction): """Deletes file uploaded or produced by the programs,""" - return self._files_client.provider_delete(file, function, provider) + return self._files_client.provider_delete(file, function) - def file_upload( - self, file: str, function: QiskitFunction - ): + def file_upload(self, file: str, function: QiskitFunction): """Upload file.""" return self._files_client.upload(file, function) diff --git a/client/qiskit_serverless/core/files.py b/client/qiskit_serverless/core/files.py index 682e70032..f4770d94f 100644 --- a/client/qiskit_serverless/core/files.py +++ b/client/qiskit_serverless/core/files.py @@ -132,9 +132,7 @@ def provider_download( ) @_trace - def upload( - self, file: str, function: QiskitFunction - ) -> Optional[str]: + def upload(self, file: str, function: QiskitFunction) -> Optional[str]: """Uploads file.""" with open(file, "rb") as f: with requests.post( @@ -151,13 +149,11 @@ def upload( return "Can not open file" @_trace - def provider_upload( - self, file: str, function: QiskitFunction - ) -> Optional[str]: + def provider_upload(self, file: str, function: QiskitFunction) -> Optional[str]: + """Uploads file to provider/function file storage.""" if not function.provider: raise QiskitServerlessException("`function` doesn't have a provider.") - """Uploads file to provider/function file storage.""" with open(file, "rb") as f: with requests.post( os.path.join(self._files_url, "upload/"), @@ -202,9 +198,7 @@ def provider_list(self, function: QiskitFunction) -> List[str]: return response_data.get("results", []) @_trace - def delete( - self, file: str, function: QiskitFunction - ) -> Optional[str]: + def delete(self, file: str, function: QiskitFunction) -> Optional[str]: """Deletes file uploaded or produced by the programs,""" response_data = safe_json_request_as_dict( request=lambda: requests.delete( @@ -224,13 +218,11 @@ def delete( return response_data.get("message", "") @_trace - def provider_delete( - self, file: str, function: QiskitFunction - ) -> Optional[str]: + def provider_delete(self, file: str, function: QiskitFunction) -> Optional[str]: + """Deletes a file uploaded or produced by the Qiskit Functions""" if not function.provider: raise QiskitServerlessException("`function` doesn't have a provider.") - """Deletes file uploaded or produced by the programs,""" response_data = safe_json_request_as_dict( request=lambda: requests.delete( os.path.join(self._files_url, "provider", "delete"), From f51f3459e6d6aa1a62ad4030e89b8798791121c2 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 26 Dec 2024 12:33:15 -0500 Subject: [PATCH 23/26] fixed the creation of the directory --- gateway/api/services/file_storage.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gateway/api/services/file_storage.py b/gateway/api/services/file_storage.py index d7b7653bc..06a00ca60 100644 --- a/gateway/api/services/file_storage.py +++ b/gateway/api/services/file_storage.py @@ -85,7 +85,9 @@ def __get_user_path(self, function_title: str, provider_name: str | None) -> str sanitized_path = sanitize_file_path(path) # Create directory if it doesn't exist - os.makedirs(os.path.dirname(sanitized_path), exist_ok=True) + if not os.path.exists(sanitized_path): + os.makedirs(sanitized_path, exist_ok=True) + logger.debug("Path %s was created.", sanitized_path) return sanitized_path @@ -107,7 +109,9 @@ def __get_provider_path(self, function_title: str, provider_name: str) -> str: sanitized_path = sanitize_file_path(path) # Create directory if it doesn't exist - os.makedirs(os.path.dirname(sanitized_path), exist_ok=True) + if not os.path.exists(sanitized_path): + os.makedirs(sanitized_path, exist_ok=True) + logger.debug("Path %s was created.", sanitized_path) return sanitized_path From d3b48cb673d273ff13b11290dbb6305c9810ebaf Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 26 Dec 2024 16:48:02 -0500 Subject: [PATCH 24/26] added a test for the provider end-points --- tests/docker/test_docker_experimental.py | 46 ++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/tests/docker/test_docker_experimental.py b/tests/docker/test_docker_experimental.py index 60ba11d0f..681799c47 100644 --- a/tests/docker/test_docker_experimental.py +++ b/tests/docker/test_docker_experimental.py @@ -77,7 +77,7 @@ def test_file_consumer(self, serverless_client: ServerlessClient): assert (file_count - len(serverless_client.files(functionTitle))) == 1 @mark.order(1) - def test_upload_download_delete(self, serverless_client: ServerlessClient): + def test_list_upload_download_delete(self, serverless_client: ServerlessClient): """Integration test for upload files.""" function = serverless_client.function("hello-world") @@ -112,7 +112,7 @@ def test_upload_download_delete(self, serverless_client: ServerlessClient): assert (file_count - len(files)) == 1 - def test_provider_upload_download_delete(self, serverless_client: ServerlessClient): + def test_list_upload_download_delete_with_provider_function(self, serverless_client: ServerlessClient): """Integration test for upload files.""" function = QiskitFunction( title="provider-function", @@ -153,3 +153,45 @@ def test_provider_upload_download_delete(self, serverless_client: ServerlessClie print(files) assert (file_count - len(files)) == 1 + + def test_provider_list_upload_download_delete(self, serverless_client: ServerlessClient): + """Integration test for upload files.""" + function = QiskitFunction( + title="provider-function", + provider="mockprovider", + image="test-local-provider-function:latest", + ) + serverless_client.upload(function) + + function = serverless_client.function("mockprovider/provider-function") + + print("::: Provider file_upload :::") + print(serverless_client.provider_file_upload(filename_path, function)) + + files = serverless_client.provider_files(function) + print("::: Provider files :::") + print(files) + + file_count = len(files) + print("::: Provider file_count :::") + print(file_count) + + assert file_count == 1 + + print("::: Provider file_download :::") + assert serverless_client.provider_file_download(filename, function) is not None + + files = serverless_client.provider_files(function) + print("::: Provider files after download :::") + print(files) + + assert file_count == len(files) + + print("::: Provider file_delete :::") + print(serverless_client.provider_file_delete(filename, function)) + + print("::: Provider files after delete:::") + files = serverless_client.provider_files(function) + print(files) + + assert (file_count - len(files)) == 1 \ No newline at end of file From 8a53989703db722954f4f4fe3042d7d492e3095d Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 26 Dec 2024 16:48:17 -0500 Subject: [PATCH 25/26] fix some typos from the provider end-points --- client/qiskit_serverless/core/clients/serverless_client.py | 4 ++-- client/qiskit_serverless/core/files.py | 2 +- gateway/api/views/files.py | 2 +- gateway/tests/api/test_v1_files.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client/qiskit_serverless/core/clients/serverless_client.py b/client/qiskit_serverless/core/clients/serverless_client.py index b34a9155e..2b0ff8733 100644 --- a/client/qiskit_serverless/core/clients/serverless_client.py +++ b/client/qiskit_serverless/core/clients/serverless_client.py @@ -427,11 +427,11 @@ def provider_file_download( ) def file_delete(self, file: str, function: QiskitFunction): - """Deletes file uploaded or produced by the programs,""" + """Deletes file uploaded or produced by the Qiskit function""" return self._files_client.delete(file, function) def provider_file_delete(self, file: str, function: QiskitFunction): - """Deletes file uploaded or produced by the programs,""" + """Deletes file uploaded or produced by the Qiskit Function""" return self._files_client.provider_delete(file, function) def file_upload(self, file: str, function: QiskitFunction): diff --git a/client/qiskit_serverless/core/files.py b/client/qiskit_serverless/core/files.py index f4770d94f..5ace9b300 100644 --- a/client/qiskit_serverless/core/files.py +++ b/client/qiskit_serverless/core/files.py @@ -156,7 +156,7 @@ def provider_upload(self, file: str, function: QiskitFunction) -> Optional[str]: with open(file, "rb") as f: with requests.post( - os.path.join(self._files_url, "upload/"), + os.path.join(self._files_url, "provider", "upload/"), files={"file": f}, params={"provider": function.provider, "function": function.title}, stream=True, diff --git a/gateway/api/views/files.py b/gateway/api/views/files.py index 0c9e44c33..8ad1431b3 100644 --- a/gateway/api/views/files.py +++ b/gateway/api/views/files.py @@ -360,7 +360,7 @@ def provider_delete(self, request): file_name = sanitize_file_name(request.query_params.get("file")) provider_name = sanitize_name(request.query_params.get("provider")) function_title = sanitize_name(request.query_params.get("function", None)) - working_dir = WorkingDir.USER_STORAGE + working_dir = WorkingDir.PROVIDER_STORAGE if not all([file_name, function_title, provider_name]): return Response( diff --git a/gateway/tests/api/test_v1_files.py b/gateway/tests/api/test_v1_files.py index bf8b98399..de1ee58b1 100644 --- a/gateway/tests/api/test_v1_files.py +++ b/gateway/tests/api/test_v1_files.py @@ -344,7 +344,7 @@ def test_provider_file_delete(self): function = "Program" file = "artifact_delete.tar" username = "test_user_2" - functionPath = os.path.join(media_root, username, provider, function) + functionPath = os.path.join(media_root, provider, function) if not os.path.exists(functionPath): os.makedirs(functionPath) From 64720eed4d696f19b2d0f6b6d707298fbb7cb1e4 Mon Sep 17 00:00:00 2001 From: David <9059044+Tansito@users.noreply.github.com> Date: Thu, 26 Dec 2024 16:52:30 -0500 Subject: [PATCH 26/26] fix black on tests --- tests/docker/test_docker_experimental.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/docker/test_docker_experimental.py b/tests/docker/test_docker_experimental.py index 681799c47..d2bfa192c 100644 --- a/tests/docker/test_docker_experimental.py +++ b/tests/docker/test_docker_experimental.py @@ -112,7 +112,9 @@ def test_list_upload_download_delete(self, serverless_client: ServerlessClient): assert (file_count - len(files)) == 1 - def test_list_upload_download_delete_with_provider_function(self, serverless_client: ServerlessClient): + def test_list_upload_download_delete_with_provider_function( + self, serverless_client: ServerlessClient + ): """Integration test for upload files.""" function = QiskitFunction( title="provider-function", @@ -154,7 +156,9 @@ def test_list_upload_download_delete_with_provider_function(self, serverless_cli assert (file_count - len(files)) == 1 - def test_provider_list_upload_download_delete(self, serverless_client: ServerlessClient): + def test_provider_list_upload_download_delete( + self, serverless_client: ServerlessClient + ): """Integration test for upload files.""" function = QiskitFunction( title="provider-function", @@ -194,4 +198,4 @@ def test_provider_list_upload_download_delete(self, serverless_client: Serverles files = serverless_client.provider_files(function) print(files) - assert (file_count - len(files)) == 1 \ No newline at end of file + assert (file_count - len(files)) == 1