From a1968edd216617786da41aa9d5fa3c6d45ef6508 Mon Sep 17 00:00:00 2001 From: sshugsc Date: Thu, 29 Jan 2026 12:27:54 -0800 Subject: [PATCH 01/11] checking for project membership before creating the report --- pori_python/ipr/connection.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index dca4687d..cc126c4a 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -107,6 +107,10 @@ def upload_report( projects = self.get("project") project_names = [item["name"] for item in projects] + project_users = { + item["name"]: [user["username"] for user in item.get("users", [])] + for item in projects + } # if project is not exist, create one if content["project"] not in project_names: @@ -118,6 +122,9 @@ def upload_report( except Exception as err: raise Exception(f"Project creation failed due to {err}") + if self.username not in project_users[content["project"]]: + raise Exception(f"User have no permission to create report in project {content['project']}") + if ignore_extra_fields: initial_result = self.post("reports-async?ignore_extra_fields=true", content) else: From be3add72f8502b74160e2bd5cecc22d0b1ab8bd1 Mon Sep 17 00:00:00 2001 From: sshugsc Date: Fri, 30 Jan 2026 12:01:43 -0800 Subject: [PATCH 02/11] lint --- pori_python/ipr/connection.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index cc126c4a..1cfd260a 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -123,8 +123,10 @@ def upload_report( raise Exception(f"Project creation failed due to {err}") if self.username not in project_users[content["project"]]: - raise Exception(f"User have no permission to create report in project {content['project']}") - + raise Exception( + f"User have no permission to create report in project {content['project']}" + ) + if ignore_extra_fields: initial_result = self.post("reports-async?ignore_extra_fields=true", content) else: From 221b52e077c0f145f539593cf067b5e546e868b5 Mon Sep 17 00:00:00 2001 From: Shirley Shu <147874967+sshugsc@users.noreply.github.com> Date: Tue, 24 Mar 2026 09:59:41 -0700 Subject: [PATCH 03/11] make it more readable --- pori_python/ipr/connection.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index 1cfd260a..92c5c7a0 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -108,7 +108,10 @@ def upload_report( projects = self.get("project") project_names = [item["name"] for item in projects] project_users = { - item["name"]: [user["username"] for user in item.get("users", [])] + item["name"]: [ + user["username"] + for user in item.get("users", []) + ] for item in projects } From cf84f961c76839517549ec3af466e4b3acbd8b92 Mon Sep 17 00:00:00 2001 From: sshugsc Date: Tue, 24 Mar 2026 10:05:07 -0700 Subject: [PATCH 04/11] lint --- pori_python/ipr/connection.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index e40ba0d4..317ae90b 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -105,13 +105,10 @@ def upload_report( # or 'report'. jobStatus is no longer available once the report is successfully # uploaded. - projects = self.get("project") - project_names = [item["name"] for item in projects] + projects = self.get('project') + project_names = [item['name'] for item in projects] project_users = { - item["name"]: [ - user["username"] - for user in item.get("users", []) - ] + item['name']: [user['username'] for user in item.get('users', [])] for item in projects } @@ -125,9 +122,9 @@ def upload_report( except Exception as err: raise Exception(f'Project creation failed due to {err}') - if self.username not in project_users[content["project"]]: + if self.username not in project_users[content['project']]: raise Exception( - f"User have no permission to create report in project {content['project']}" + f'User have no permission to create report in project {content["project"]}' ) if ignore_extra_fields: From 1c8153c32362eecfefda138eb9ee951539b795ca Mon Sep 17 00:00:00 2001 From: sshugsc Date: Mon, 20 Apr 2026 15:13:38 -0700 Subject: [PATCH 05/11] typo --- pori_python/ipr/connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index 317ae90b..4847c9ee 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -124,7 +124,7 @@ def upload_report( if self.username not in project_users[content['project']]: raise Exception( - f'User have no permission to create report in project {content["project"]}' + f'User has no permission to create report in project {content["project"]}' ) if ignore_extra_fields: From 8ab54b96bed33cd4b7c8d25bf71c348f675302cb Mon Sep 17 00:00:00 2001 From: Eleanor Lewis Date: Mon, 20 Apr 2026 15:44:11 -0700 Subject: [PATCH 06/11] check permission first, increase mins_to_wait --- pori_python/ipr/connection.py | 69 +++++++++++------ pori_python/ipr/main.py | 8 +- tests/test_ipr/test_connection.py | 123 ++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 25 deletions(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index 317ae90b..53eb2827 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -93,10 +93,55 @@ def delete(self, uri: str, data: Dict = {}, **kwargs) -> Dict: **kwargs, ) + def check_upload_permission(self, project_name: str) -> None: + """Check that the current user has permission to upload to the given project. + + Fetches all projects and the current user info (including groups and + projects) up front. Checks for admin, manager, create report access, + all projects access, and project membership. Creates the project if + it does not yet exist. + """ + projects = self.get('project') + project_exists = any(p['name'] == project_name for p in projects) + + user = self.get('user/me') + user_groups = user.get('groups', []) if isinstance(user, dict) else [] + group_names = { + group.get('name', '').strip().lower() + if isinstance(group, dict) + else group.strip().lower() + for group in user_groups + } + + is_admin = 'admin' in group_names + is_manager = 'manager' in group_names + has_create_report_access = 'create report access' in group_names + has_all_projects_access = 'all projects access' in group_names + + # admins and managers can always create reports + can_create_report = is_admin or is_manager or has_create_report_access + + user_projects = user.get('projects', []) if isinstance(user, dict) else [] + has_project_access = ( + is_admin + or has_all_projects_access + or any(isinstance(p, dict) and p.get('name') == project_name for p in user_projects) + ) + + if not can_create_report: + raise Exception( + f'User does not have report creation permission' + ) + + if not has_project_access: + raise Exception( + f'User has no permission to create report in project {project_name}' + ) + def upload_report( self, content: Dict, - mins_to_wait: int = 5, + mins_to_wait: int = 10, async_upload: bool = False, ignore_extra_fields: bool = False, ) -> Dict: @@ -105,28 +150,6 @@ def upload_report( # or 'report'. jobStatus is no longer available once the report is successfully # uploaded. - projects = self.get('project') - project_names = [item['name'] for item in projects] - project_users = { - item['name']: [user['username'] for user in item.get('users', [])] - for item in projects - } - - # if project is not exist, create one - if content['project'] not in project_names: - logger.info( - f'Project not found - attempting to create project {content["project"]}' - ) - try: - self.post('project', {'name': content['project']}) - except Exception as err: - raise Exception(f'Project creation failed due to {err}') - - if self.username not in project_users[content['project']]: - raise Exception( - f'User have no permission to create report in project {content["project"]}' - ) - if ignore_extra_fields: initial_result = self.post('reports-async?ignore_extra_fields=true', content) else: diff --git a/pori_python/ipr/main.py b/pori_python/ipr/main.py index 3319c811..0d95efe9 100644 --- a/pori_python/ipr/main.py +++ b/pori_python/ipr/main.py @@ -152,7 +152,7 @@ def command_interface() -> None: ) parser.add_argument( '--mins_to_wait', - default=5, + default=10, action='store', help='is using reports-async, number of minutes to wait before throwing error', ) @@ -337,7 +337,7 @@ def ipr_report( match_germline: bool = False, custom_kb_match_filter: Optional[Callable] = None, async_upload: bool = False, - mins_to_wait: int = 5, + mins_to_wait: int = 10, include_ipr_variant_text: bool = True, include_nonspecific_disease: bool = False, include_nonspecific_project: bool = False, @@ -396,6 +396,10 @@ def ipr_report( else: logger.warning('No ipr_url given') + # Verify upload permission before doing any expensive processing + if ipr_upload and ipr_conn: + ipr_conn.check_upload_permission(content['project']) + if validate_json: if not ipr_conn: raise ValueError('ipr_url required to validate json') diff --git a/tests/test_ipr/test_connection.py b/tests/test_ipr/test_connection.py index d83ac79a..3cd851a9 100644 --- a/tests/test_ipr/test_connection.py +++ b/tests/test_ipr/test_connection.py @@ -95,3 +95,126 @@ def request(*args, **kwargs): ) }, ) + + +class TestCheckUploadPermission: + def _user_response(self, groups=None, projects=None): + return { + 'groups': [{'name': g} for g in (groups or [])], + 'projects': [{'name': p} for p in (projects or [])], + } + + def test_rejects_user_without_create_report_access(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[[{'name': 'TEST'}], self._user_response(projects=['TEST'])] + ) + conn.post = mock.MagicMock() + + with pytest.raises(Exception, match='User does not have report creation permission'): + conn.check_upload_permission('TEST') + + conn.post.assert_not_called() + + def test_rejects_user_without_project_access(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[ + [{'name': 'TEST'}], + self._user_response(groups=['create report access'], projects=['OTHER']), + ] + ) + conn.post = mock.MagicMock() + + with pytest.raises(Exception, match='User has no permission to create report in project TEST'): + conn.check_upload_permission('TEST') + + conn.post.assert_not_called() + + def test_allows_user_with_project_and_create_report_access(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[ + [{'name': 'TEST'}], + self._user_response(groups=['create report access'], projects=['TEST']), + ] + ) + conn.post = mock.MagicMock() + + conn.check_upload_permission('TEST') + + conn.post.assert_not_called() + + def test_manager_has_implicit_create_report_access(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[ + [{'name': 'TEST'}], + self._user_response(groups=['manager'], projects=['TEST']), + ] + ) + conn.post = mock.MagicMock() + + conn.check_upload_permission('TEST') + + conn.post.assert_not_called() + + def test_admin_bypasses_all_checks(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[ + [{'name': 'TEST'}], + self._user_response(groups=['admin'], projects=[]), + ] + ) + conn.post = mock.MagicMock() + + conn.check_upload_permission('TEST') + + conn.post.assert_not_called() + + def test_admin_creates_missing_project(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[ + [{'name': 'OTHER'}], + self._user_response(groups=['admin'], projects=[]), + ] + ) + conn.post = mock.MagicMock() + + conn.check_upload_permission('TEST') + + conn.post.assert_called_once_with('project', {'name': 'TEST'}) + + def test_all_projects_access_without_project_membership(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[ + [{'name': 'TEST'}], + self._user_response( + groups=['create report access', 'all projects access'], projects=[] + ), + ] + ) + conn.post = mock.MagicMock() + + conn.check_upload_permission('TEST') + + conn.post.assert_not_called() + + def test_creates_missing_project_for_all_projects_access_user(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[ + [{'name': 'OTHER'}], + self._user_response( + groups=['create report access', 'all projects access'], projects=[] + ), + ] + ) + conn.post = mock.MagicMock() + + conn.check_upload_permission('TEST') + + conn.post.assert_called_once_with('project', {'name': 'TEST'}) From acb3f8ee135390882804a713bc064d55040cec9e Mon Sep 17 00:00:00 2001 From: Eleanor Lewis Date: Mon, 20 Apr 2026 15:44:55 -0700 Subject: [PATCH 07/11] format --- pori_python/ipr/connection.py | 8 ++------ tests/test_ipr/test_connection.py | 4 +++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index 53eb2827..88a4caa3 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -129,14 +129,10 @@ def check_upload_permission(self, project_name: str) -> None: ) if not can_create_report: - raise Exception( - f'User does not have report creation permission' - ) + raise Exception(f'User does not have report creation permission') if not has_project_access: - raise Exception( - f'User has no permission to create report in project {project_name}' - ) + raise Exception(f'User has no permission to create report in project {project_name}') def upload_report( self, diff --git a/tests/test_ipr/test_connection.py b/tests/test_ipr/test_connection.py index 3cd851a9..b5156f29 100644 --- a/tests/test_ipr/test_connection.py +++ b/tests/test_ipr/test_connection.py @@ -126,7 +126,9 @@ def test_rejects_user_without_project_access(self): ) conn.post = mock.MagicMock() - with pytest.raises(Exception, match='User has no permission to create report in project TEST'): + with pytest.raises( + Exception, match='User has no permission to create report in project TEST' + ): conn.check_upload_permission('TEST') conn.post.assert_not_called() From 39469e69b094eb0b4351d4658e0fe57b7246374a Mon Sep 17 00:00:00 2001 From: Eleanor Lewis Date: Mon, 20 Apr 2026 16:18:37 -0700 Subject: [PATCH 08/11] fix: create missing project in check_upload_permission and add user/me mock to tests --- pori_python/ipr/connection.py | 3 +++ tests/test_ipr/test_main.py | 5 +++++ tests/test_ipr/test_probe.py | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index 88a4caa3..cc21a993 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -134,6 +134,9 @@ def check_upload_permission(self, project_name: str) -> None: if not has_project_access: raise Exception(f'User has no permission to create report in project {project_name}') + if not project_exists and can_create_report and has_project_access: + self.post('project', {'name': project_name}) + def upload_report( self, content: Dict, diff --git a/tests/test_ipr/test_main.py b/tests/test_ipr/test_main.py index fd8e8bb8..3dbd0aa5 100644 --- a/tests/test_ipr/test_main.py +++ b/tests/test_ipr/test_main.py @@ -110,6 +110,11 @@ def side_effect_function(*args, **kwargs): return [{'name': 'genomic', 'ident': '001'}] elif args[0] == 'project': return [{'name': 'TEST', 'ident': '001'}] + elif args[0] == 'user/me': + return { + 'groups': [{'name': 'admin'}], + 'projects': [{'name': 'TEST'}], + } else: return [] diff --git a/tests/test_ipr/test_probe.py b/tests/test_ipr/test_probe.py index 43ead9f1..ec93599c 100644 --- a/tests/test_ipr/test_probe.py +++ b/tests/test_ipr/test_probe.py @@ -25,6 +25,11 @@ def side_effect_function(*args, **kwargs): return [{'name': 'genomic', 'ident': '001'}] elif args[0] == 'project': return [{'name': 'TEST', 'ident': '001'}] + elif args[0] == 'user/me': + return { + 'groups': [{'name': 'admin'}], + 'projects': [{'name': 'TEST'}], + } else: return [] From 37ced1ee570360cc1baa6f2d289838c4181d2201 Mon Sep 17 00:00:00 2001 From: Eleanor Lewis Date: Mon, 20 Apr 2026 16:48:32 -0700 Subject: [PATCH 09/11] remove report creation option --- pori_python/ipr/connection.py | 5 +---- tests/test_ipr/test_connection.py | 30 ------------------------------ 2 files changed, 1 insertion(+), 34 deletions(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index cc21a993..c7d444d1 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -98,8 +98,7 @@ def check_upload_permission(self, project_name: str) -> None: Fetches all projects and the current user info (including groups and projects) up front. Checks for admin, manager, create report access, - all projects access, and project membership. Creates the project if - it does not yet exist. + all projects access, and project membership. """ projects = self.get('project') project_exists = any(p['name'] == project_name for p in projects) @@ -134,8 +133,6 @@ def check_upload_permission(self, project_name: str) -> None: if not has_project_access: raise Exception(f'User has no permission to create report in project {project_name}') - if not project_exists and can_create_report and has_project_access: - self.post('project', {'name': project_name}) def upload_report( self, diff --git a/tests/test_ipr/test_connection.py b/tests/test_ipr/test_connection.py index b5156f29..aea50d38 100644 --- a/tests/test_ipr/test_connection.py +++ b/tests/test_ipr/test_connection.py @@ -175,20 +175,6 @@ def test_admin_bypasses_all_checks(self): conn.post.assert_not_called() - def test_admin_creates_missing_project(self): - conn = IprConnection('user', 'pass') - conn.get = mock.MagicMock( - side_effect=[ - [{'name': 'OTHER'}], - self._user_response(groups=['admin'], projects=[]), - ] - ) - conn.post = mock.MagicMock() - - conn.check_upload_permission('TEST') - - conn.post.assert_called_once_with('project', {'name': 'TEST'}) - def test_all_projects_access_without_project_membership(self): conn = IprConnection('user', 'pass') conn.get = mock.MagicMock( @@ -204,19 +190,3 @@ def test_all_projects_access_without_project_membership(self): conn.check_upload_permission('TEST') conn.post.assert_not_called() - - def test_creates_missing_project_for_all_projects_access_user(self): - conn = IprConnection('user', 'pass') - conn.get = mock.MagicMock( - side_effect=[ - [{'name': 'OTHER'}], - self._user_response( - groups=['create report access', 'all projects access'], projects=[] - ), - ] - ) - conn.post = mock.MagicMock() - - conn.check_upload_permission('TEST') - - conn.post.assert_called_once_with('project', {'name': 'TEST'}) From a788ee4aae5035f6162a62fb15294da153aee4cd Mon Sep 17 00:00:00 2001 From: Eleanor Lewis Date: Mon, 20 Apr 2026 16:48:50 -0700 Subject: [PATCH 10/11] format --- pori_python/ipr/connection.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index c7d444d1..a9010864 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -133,7 +133,6 @@ def check_upload_permission(self, project_name: str) -> None: if not has_project_access: raise Exception(f'User has no permission to create report in project {project_name}') - def upload_report( self, content: Dict, From aa3b8849d54241ca01c2e7a60ca33009235586d2 Mon Sep 17 00:00:00 2001 From: Eleanor Lewis Date: Wed, 13 May 2026 15:29:36 -0700 Subject: [PATCH 11/11] add project_exists check and tests --- pori_python/ipr/connection.py | 6 +++++- tests/test_ipr/test_connection.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/pori_python/ipr/connection.py b/pori_python/ipr/connection.py index a9010864..75458d41 100644 --- a/pori_python/ipr/connection.py +++ b/pori_python/ipr/connection.py @@ -102,6 +102,10 @@ def check_upload_permission(self, project_name: str) -> None: """ projects = self.get('project') project_exists = any(p['name'] == project_name for p in projects) + if not project_exists: + raise Exception( + f'Project {project_name} does not exist or user does not have permission to view it' + ) user = self.get('user/me') user_groups = user.get('groups', []) if isinstance(user, dict) else [] @@ -128,7 +132,7 @@ def check_upload_permission(self, project_name: str) -> None: ) if not can_create_report: - raise Exception(f'User does not have report creation permission') + raise Exception('User does not have report creation permission') if not has_project_access: raise Exception(f'User has no permission to create report in project {project_name}') diff --git a/tests/test_ipr/test_connection.py b/tests/test_ipr/test_connection.py index aea50d38..e2f8d4f5 100644 --- a/tests/test_ipr/test_connection.py +++ b/tests/test_ipr/test_connection.py @@ -147,7 +147,34 @@ def test_allows_user_with_project_and_create_report_access(self): conn.post.assert_not_called() - def test_manager_has_implicit_create_report_access(self): + def test_project_not_found_raises(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock(side_effect=[[{'name': 'OTHER'}]]) + conn.post = mock.MagicMock() + + with pytest.raises(Exception, match='Project TEST does not exist'): + conn.check_upload_permission('TEST') + + conn.post.assert_not_called() + + def test_manager_without_project_membership_raises(self): + conn = IprConnection('user', 'pass') + conn.get = mock.MagicMock( + side_effect=[ + [{'name': 'TEST'}], + self._user_response(groups=['manager'], projects=[]), + ] + ) + conn.post = mock.MagicMock() + + with pytest.raises( + Exception, match='User has no permission to create report in project TEST' + ): + conn.check_upload_permission('TEST') + + conn.post.assert_not_called() + + def test_manager_with_project_membership_allowed(self): conn = IprConnection('user', 'pass') conn.get = mock.MagicMock( side_effect=[