From 16da615f54a2782584c36faf517ae3d85975719b Mon Sep 17 00:00:00 2001 From: wei-kuochen Date: Fri, 10 Jan 2025 17:48:49 +0900 Subject: [PATCH 1/8] weko#48603 fix community admin role issue --- modules/weko-workflow/weko_workflow/api.py | 28 +++++++++------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/modules/weko-workflow/weko_workflow/api.py b/modules/weko-workflow/weko_workflow/api.py index 7ef1f2c466..29133c0005 100644 --- a/modules/weko-workflow/weko_workflow/api.py +++ b/modules/weko-workflow/weko_workflow/api.py @@ -1529,21 +1529,16 @@ def query_activities_by_tab_is_wait(query): def query_activities_by_tab_is_all( query, is_community_admin, - community_user_ids, ): """Query activities by tab is all. :param query: :param is_community_admin: - :param community_user_ids: :return: """ self_user_id = int(current_user.get_id()) self_group_ids = [role.id for role in current_user.roles] - if is_community_admin: - query = query \ - .filter(_Activity.activity_login_user.in_(community_user_ids)) - else: + if not is_community_admin: query = query.filter( or_( and_( @@ -1601,12 +1596,13 @@ def check_current_user_role(): return is_admin, is_community_admin @staticmethod - def query_activities_by_tab_is_todo(query, is_admin): + def query_activities_by_tab_is_todo(query, is_admin, is_community_admin): """ Query activities by tab is todo. :param query: :param is_admin: + :param is_community_admin: :return: """ query = query \ @@ -1614,7 +1610,7 @@ def query_activities_by_tab_is_todo(query, is_admin): == ActivityStatusPolicy.ACTIVITY_BEGIN, _Activity.activity_status == ActivityStatusPolicy.ACTIVITY_MAKING)) - if not is_admin or current_app.config[ + if (not is_admin and not is_community_admin) or current_app.config[ 'WEKO_WORKFLOW_ENABLE_SHOW_ACTIVITY']: self_user_id = int(current_user.get_id()) self_group_ids = [role.id for role in current_user.roles] @@ -1639,7 +1635,7 @@ def query_activities_by_tab_is_todo(query, is_admin): )\ .filter(_FlowAction.action_id == _Activity.action_id) \ .filter(_FlowAction.action_order == _Activity.action_order) - if is_admin: + if is_admin or is_community_admin: query = query.filter( ActivityAction.action_handler.in_([-1, self_user_id]) ) @@ -1773,7 +1769,8 @@ def __format_activity_data_to_show_on_workflow(self, activities, activity_data.role_name = role_name if role_name else '' if is_community_admin: - if not self._check_community_permission(activity_data, index_ids): + if activity_data.activity_login_user != int(current_user.get_id()) and \ + not self._check_community_permission(activity_data, index_ids): continue # Append to do and action activities into the master list activities.append(activity_data) @@ -1845,11 +1842,9 @@ def get_activity_list(self, community_id=None, conditions=None, if size_all and size_all[0].isnumeric(): size = size_all[0] if not is_admin: - community_user_ids = self.__get_community_user_ids() query_action_activities = self \ .query_activities_by_tab_is_all( - query_action_activities, is_community_admin, - community_user_ids + query_action_activities, is_community_admin ) # query activities by tab is todo elif tab == WEKO_WORKFLOW_TODO_TAB: @@ -1860,15 +1855,13 @@ def get_activity_list(self, community_id=None, conditions=None, if size_todo and size_todo[0].isnumeric(): size = size_todo[0] if not is_admin: - community_user_ids = self.__get_community_user_ids() query_action_activities = self\ .query_activities_by_tab_is_all( - query_action_activities, is_community_admin, - community_user_ids + query_action_activities, is_community_admin ) query_action_activities = self.query_activities_by_tab_is_todo( - query_action_activities, is_admin + query_action_activities, is_admin, is_community_admin ) # Filter conditions @@ -1916,6 +1909,7 @@ def __get_activity_list_per_page( query_action_activities = query_action_activities.limit( size).offset(offset) action_activities = query_action_activities.all() + current_app.logger.debug('========= action_activities: {}'.format(action_activities)) if action_activities: # Format activities self.__format_activity_data_to_show_on_workflow( From e9314aaf57b61a60b06031b93e47527700d94c02 Mon Sep 17 00:00:00 2001 From: wei-kuochen Date: Fri, 10 Jan 2025 18:15:08 +0900 Subject: [PATCH 2/8] weko#48603 delete comment message --- modules/weko-workflow/weko_workflow/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/weko-workflow/weko_workflow/api.py b/modules/weko-workflow/weko_workflow/api.py index 29133c0005..a38040b3de 100644 --- a/modules/weko-workflow/weko_workflow/api.py +++ b/modules/weko-workflow/weko_workflow/api.py @@ -1909,7 +1909,6 @@ def __get_activity_list_per_page( query_action_activities = query_action_activities.limit( size).offset(offset) action_activities = query_action_activities.all() - current_app.logger.debug('========= action_activities: {}'.format(action_activities)) if action_activities: # Format activities self.__format_activity_data_to_show_on_workflow( From 4d0b910b164441aac9599f182784879ad9b4949c Mon Sep 17 00:00:00 2001 From: wei-kuochen Date: Tue, 14 Jan 2025 17:14:07 +0900 Subject: [PATCH 3/8] weko#48603 change the activity list specifications --- modules/weko-workflow/weko_workflow/api.py | 183 ++++++++++----------- 1 file changed, 83 insertions(+), 100 deletions(-) diff --git a/modules/weko-workflow/weko_workflow/api.py b/modules/weko-workflow/weko_workflow/api.py index a38040b3de..84e79d74cf 100644 --- a/modules/weko-workflow/weko_workflow/api.py +++ b/modules/weko-workflow/weko_workflow/api.py @@ -1528,47 +1528,44 @@ def query_activities_by_tab_is_wait(query): @staticmethod def query_activities_by_tab_is_all( query, - is_community_admin, ): """Query activities by tab is all. :param query: - :param is_community_admin: :return: """ self_user_id = int(current_user.get_id()) self_group_ids = [role.id for role in current_user.roles] - if not is_community_admin: - query = query.filter( - or_( - and_( - _Activity.activity_login_user == self_user_id, - _Activity.activity_status - == ActivityStatusPolicy.ACTIVITY_FINALLY - ), - and_( - ActivityAction.action_handler == self_user_id - ), - and_( - _Activity.approval1 == current_user.email - ), - and_( - _Activity.approval2 == current_user.email - ), - and_( - _FlowActionRole.action_user == self_user_id, - _FlowActionRole.action_user_exclude == '0' - ), - and_( - _FlowActionRole.action_role.in_(self_group_ids), - _FlowActionRole.action_role_exclude == '0' - ), - and_( - _Activity.shared_user_id == self_user_id, - _FlowAction.action_id != 4 - ), - ) + query = query.filter( + or_( + and_( + _Activity.activity_login_user == self_user_id, + _Activity.activity_status + == ActivityStatusPolicy.ACTIVITY_FINALLY + ), + and_( + ActivityAction.action_handler == self_user_id + ), + and_( + _Activity.approval1 == current_user.email + ), + and_( + _Activity.approval2 == current_user.email + ), + and_( + _FlowActionRole.action_user == self_user_id, + _FlowActionRole.action_user_exclude == '0' + ), + and_( + _FlowActionRole.action_role.in_(self_group_ids), + _FlowActionRole.action_role_exclude == '0' + ), + and_( + _Activity.shared_user_id == self_user_id, + _FlowAction.action_id != 4 + ), ) + ) return query @@ -1610,52 +1607,52 @@ def query_activities_by_tab_is_todo(query, is_admin, is_community_admin): == ActivityStatusPolicy.ACTIVITY_BEGIN, _Activity.activity_status == ActivityStatusPolicy.ACTIVITY_MAKING)) - if (not is_admin and not is_community_admin) or current_app.config[ - 'WEKO_WORKFLOW_ENABLE_SHOW_ACTIVITY']: - self_user_id = int(current_user.get_id()) - self_group_ids = [role.id for role in current_user.roles] + self_user_id = int(current_user.get_id()) + self_group_ids = [role.id for role in current_user.roles] + query = query \ + .filter(_FlowAction.action_id == _Activity.action_id) \ + .filter(_FlowAction.action_order == _Activity.action_order) + if is_admin or is_community_admin: query = query \ - .filter( - or_( - and_( - _FlowActionRole.action_user == self_user_id, - _FlowActionRole.action_user_exclude == '0' - ), - and_( - _FlowActionRole.action_role.in_(self_group_ids), - _FlowActionRole.action_role_exclude == '0' - ), - and_( - _FlowActionRole.id.is_(None) - ), - and_( - _Activity.shared_user_id == self_user_id, - ), - ) - )\ - .filter(_FlowAction.action_id == _Activity.action_id) \ - .filter(_FlowAction.action_order == _Activity.action_order) - if is_admin or is_community_admin: - query = query.filter( - ActivityAction.action_handler.in_([-1, self_user_id]) + .filter( + ActivityAction.action_handler.in_([-1, self_user_id]) + ) + else: + query = query \ + .filter( + or_( + and_( + _FlowActionRole.action_user == self_user_id, + _FlowActionRole.action_user_exclude == '0' + ), + and_( + _FlowActionRole.action_role.in_(self_group_ids), + _FlowActionRole.action_role_exclude == '0' + ), + and_( + _FlowActionRole.id.is_(None) + ), + and_( + _Activity.shared_user_id == self_user_id, + ), ) - else: - query = query.filter( - or_( - ActivityAction.action_handler == self_user_id, - and_( - _FlowActionRole.action_user == self_user_id, - _FlowActionRole.action_user_exclude == '0' - ), - and_( - _FlowActionRole.action_role.in_(self_group_ids), - _FlowActionRole.action_role_exclude == '0' - ), - and_( - _Activity.shared_user_id == self_user_id, - ), - ) + ) \ + .filter( + or_( + ActivityAction.action_handler == self_user_id, + and_( + _FlowActionRole.action_user == self_user_id, + _FlowActionRole.action_user_exclude == '0' + ), + and_( + _FlowActionRole.action_role.in_(self_group_ids), + _FlowActionRole.action_role_exclude == '0' + ), + and_( + _Activity.shared_user_id == self_user_id, + ), ) + ) return query @@ -1689,28 +1686,14 @@ def __common_query_activity_list(): ActivityAction.activity_id == _Activity.activity_id, ActivityAction.action_id == _Activity.action_id, ) - ) - - if current_app.config['WEKO_WORKFLOW_ENABLE_SHOW_ACTIVITY'] or \ - current_app.config['WEKO_ITEMS_UI_MULTIPLE_APPROVALS']: - common_query = common_query \ - .outerjoin( - User, - and_( - _Activity.activity_login_user == User.id, - ) - ) \ - .outerjoin( - userrole, and_(User.id == userrole.c.user_id) - ).outerjoin(Role, and_(userrole.c.role_id == Role.id)) - else: - common_query = common_query \ - .outerjoin( - User, - and_( - _Activity.activity_update_user == User.id, - ) + ).outerjoin( + User, + and_( + _Activity.activity_login_user == User.id, ) + ).outerjoin(userrole, and_(User.id == userrole.c.user_id) + ).outerjoin(Role, and_(userrole.c.role_id == Role.id)) + return common_query @staticmethod @@ -1841,10 +1824,10 @@ def get_activity_list(self, community_id=None, conditions=None, page = page_all[0] if size_all and size_all[0].isnumeric(): size = size_all[0] - if not is_admin: + if not is_admin and not is_community_admin: query_action_activities = self \ .query_activities_by_tab_is_all( - query_action_activities, is_community_admin + query_action_activities ) # query activities by tab is todo elif tab == WEKO_WORKFLOW_TODO_TAB: @@ -1854,10 +1837,10 @@ def get_activity_list(self, community_id=None, conditions=None, page = page_todo[0] if size_todo and size_todo[0].isnumeric(): size = size_todo[0] - if not is_admin: + if not is_admin and not is_community_admin: query_action_activities = self\ .query_activities_by_tab_is_all( - query_action_activities, is_community_admin + query_action_activities ) query_action_activities = self.query_activities_by_tab_is_todo( From bc7f177304990abff8682e18bda2b41e8e18f730 Mon Sep 17 00:00:00 2001 From: wei-kuochen Date: Wed, 15 Jan 2025 14:54:15 +0900 Subject: [PATCH 4/8] weko#48603 added test code --- modules/weko-workflow/tests/conftest.py | 4 +- modules/weko-workflow/tests/test_api.py | 226 ++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 2 deletions(-) diff --git a/modules/weko-workflow/tests/conftest.py b/modules/weko-workflow/tests/conftest.py index 2b75943081..743bc69414 100644 --- a/modules/weko-workflow/tests/conftest.py +++ b/modules/weko-workflow/tests/conftest.py @@ -743,8 +743,8 @@ def users(app, db): index = Index() db.session.add(index) db.session.commit() - comm = Community.create(community_id="comm01", role_id=sysadmin_role.id, - id_user=sysadmin.id, title="test community", + comm = Community.create(community_id="comm01", role_id=comadmin_role.id, + id_user=comadmin.id, title="test community", description=("this is test community"), root_node_id=index.id) db.session.commit() diff --git a/modules/weko-workflow/tests/test_api.py b/modules/weko-workflow/tests/test_api.py index 8707c6f781..40979f62d3 100644 --- a/modules/weko-workflow/tests/test_api.py +++ b/modules/weko-workflow/tests/test_api.py @@ -1,5 +1,9 @@ +import uuid +from mock import patch +from datetime import datetime from flask_login.utils import login_user +from weko_workflow.models import Activity, ActivityAction from weko_workflow.api import Flow, WorkActivity # .tox/c1/bin/pytest --cov=weko_workflow tests/test_api.py::test_Flow_action -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp @@ -134,3 +138,225 @@ def test_WorkActivity_get_corresponding_usage_activities(app, db_register): usage_application_list, output_report_list = activity.get_corresponding_usage_activities(1) assert usage_application_list == {'activity_data_type': {}, 'activity_ids': []} assert output_report_list == {'activity_data_type': {}, 'activity_ids': []} + + +# .tox/c1/bin/pytest --cov=weko_workflow tests/test_api.py::test_WorkActivity_get_activity_list_todo -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp +def test_WorkActivity_get_activity_list_todo(app, db, workflow, users): + wa = WorkActivity() + activity1 = Activity( + status='N', + activity_id='A-00000001-00001', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + action_id=5, + action_status = 'M', + activity_login_user=users[2]['id'], + activity_update_user=users[2]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_system', + shared_user_id=-1, + extra_info={}, + action_order=4 + ) + activity2 = Activity( + status='N', + activity_id='A-00000001-00002', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + action_id=5, + action_status = 'M', + activity_login_user=users[1]['id'], + activity_update_user=users[1]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_repo', + shared_user_id=-1, + extra_info={}, + action_order=4 + ) + activity3 = Activity( + status='N', + activity_id='A-00000001-00003', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + action_id=5, + action_status = 'M', + activity_login_user=users[3]['id'], + activity_update_user=users[3]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_comm', + shared_user_id=-1, + extra_info={}, + action_order=4 + ) + activity4 = Activity( + status='N', + activity_id='A-00000001-00004', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + action_id=5, + action_status = 'M', + activity_login_user=users[0]['id'], + activity_update_user=users[0]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_contributor', + shared_user_id=-1, + extra_info={}, + action_order=4 + ) + activity5 = Activity( + status='N', + activity_id='A-00000001-00005', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + item_id=uuid.uuid4(), + action_id=4, + action_status = 'M', + activity_login_user=users[3]['id'], + activity_update_user=users[3]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_comm_approval', + shared_user_id=-1, + extra_info={}, + action_order=6 + ) + activity6 = Activity( + status='N', + activity_id='A-00000001-00006', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + item_id=uuid.uuid4(), + action_id=4, + action_status = 'M', + activity_login_user=users[0]['id'], + activity_update_user=users[0]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_contributor_approval', + shared_user_id=-1, + extra_info={}, + action_order=6 + ) + activity7 = Activity( + status='N', + activity_id='A-00000001-00007', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + action_id=5, + action_status = 'C', + activity_login_user=users[1]['id'], + activity_update_user=users[1]['id'], + activity_status='C', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_repo_cancel', + shared_user_id=-1, + extra_info={}, + action_order=4 + ) + + with db.session.begin_nested(): + db.session.add(activity1) + db.session.add(activity2) + db.session.add(activity3) + db.session.add(activity4) + db.session.add(activity5) + db.session.add(activity6) + db.session.add(activity7) + db.session.commit() + + activity_action1 = ActivityAction( + activity_id=activity1.activity_id, + action_id=5, + action_status="M", + action_handler=users[2]['id'], + action_order=4 + ) + activity_action2 = ActivityAction( + activity_id=activity2.activity_id, + action_id=5, + action_status="M", + action_handler=users[1]['id'], + action_order=4 + ) + activity_action3 = ActivityAction( + activity_id=activity3.activity_id, + action_id=5, + action_status="M", + action_handler=users[3]['id'], + action_order=4 + ) + activity_action4 = ActivityAction( + activity_id=activity4.activity_id, + action_id=5, + action_status="M", + action_handler=users[0]['id'], + action_order=4 + ) + activity_action5 = ActivityAction( + activity_id=activity5.activity_id, + action_id=4, + action_status="M", + action_handler=-1, + action_order=6 + ) + activity_action6 = ActivityAction( + activity_id=activity6.activity_id, + action_id=4, + action_status="M", + action_handler=-1, + action_order=6 + ) + activity_action7 = ActivityAction( + activity_id=activity7.activity_id, + action_id=5, + action_status="C", + action_handler=-1, + action_order=4 + ) + + with db.session.begin_nested(): + db.session.add(activity_action1) + db.session.add(activity_action2) + db.session.add(activity_action3) + db.session.add(activity_action4) + db.session.add(activity_action5) + db.session.add(activity_action6) + db.session.add(activity_action7) + db.session.commit() + + with app.test_request_context(): + #system admin + login_user(users[2]["obj"]) + activities, max_page, size, page, name_param = wa.get_activity_list( + None, {}, False, False) + assert len(activities) == 3 + + # repo admin + login_user(users[1]["obj"]) + activities, max_page, size, page, name_param = wa.get_activity_list( + None, {}, False, False) + assert len(activities) == 3 + + # comm admin + login_user(users[3]["obj"]) + with patch('weko_workflow.api.WekoDeposit.get_record', return_value={'path': [1]}): + activities, max_page, size, page, name_param = wa.get_activity_list( + None, {}, False, False) + assert len(activities) == 3 + + # contributor admin + login_user(users[0]["obj"]) + activities, max_page, size, page, name_param = wa.get_activity_list( + None, {}, False, False) + assert len(activities) == 1 \ No newline at end of file From 3e0c950c9041ee77a8b738236d91d7a435866782 Mon Sep 17 00:00:00 2001 From: ivis-nakagawa Date: Thu, 16 Jan 2025 15:05:02 +0900 Subject: [PATCH 5/8] weko#48601 Fix to use User model --- modules/weko-items-ui/tests/conftest.py | 3 +- modules/weko-items-ui/weko_items_ui/utils.py | 84 ++++++-------------- 2 files changed, 25 insertions(+), 62 deletions(-) diff --git a/modules/weko-items-ui/tests/conftest.py b/modules/weko-items-ui/tests/conftest.py index ae3791bcf8..1cfb0f9cec 100644 --- a/modules/weko-items-ui/tests/conftest.py +++ b/modules/weko-items-ui/tests/conftest.py @@ -184,7 +184,8 @@ def base_app(instance_path): # WEKO_ITEMS_UI_INDEX_TEMPLATE= 'weko_items_ui/item_index.html', CACHE_TYPE="redis", ACCOUNTS_SESSION_REDIS_DB_NO=1, - REDIS_PORT="6379"CACHE_REDIS_URL=os.environ.get("CACHE_REDIS_URL", "redis://redis:6379/0"), + REDIS_PORT="6379", + CACHE_REDIS_URL=os.environ.get("CACHE_REDIS_URL", "redis://redis:6379/0"), WEKO_BUCKET_QUOTA_SIZE=50 * 1024 * 1024 * 1024, WEKO_MAX_FILE_SIZE=50 * 1024 * 1024 * 1024, SEARCH_ELASTIC_HOSTS=os.environ.get("SEARCH_ELASTIC_HOSTS", "elasticsearch"), diff --git a/modules/weko-items-ui/weko_items_ui/utils.py b/modules/weko-items-ui/weko_items_ui/utils.py index cbc80c1984..7f70dd9ff0 100644 --- a/modules/weko-items-ui/weko_items_ui/utils.py +++ b/modules/weko-items-ui/weko_items_ui/utils.py @@ -42,7 +42,7 @@ url_for,jsonify from flask_babelex import gettext as _ from flask_login import current_user -from invenio_accounts.models import Role, userrole +from invenio_accounts.models import Role, User, userrole from invenio_db import db from invenio_i18n.ext import current_i18n from invenio_indexer.api import RecordIndexer @@ -51,7 +51,6 @@ from invenio_pidstore.models import PersistentIdentifier, PIDStatus from invenio_pidstore.errors import PIDDoesNotExistError from invenio_records.api import RecordBase -from invenio_accounts.models import User from invenio_search import RecordsSearch from invenio_stats.utils import QueryRankingHelper, QuerySearchReportHelper from invenio_stats.views import QueryRecordViewCount as _QueryRecordViewCount @@ -156,21 +155,12 @@ def get_user_info_by_username(username): user = UserProfile.get_by_username(username) user_id = user.user_id - metadata = MetaData() - metadata.reflect(bind=db.engine) - table_name = 'accounts_user' - - user_table = Table(table_name, metadata) - record = db.session.query(user_table) - - data = record.all() - - for item in data: - if item[0] == user_id: - result['username'] = username - result['user_id'] = user_id - result['email'] = item[1] - return result + data = User.query.filter(User.id == user_id).first() + if data: + result['username'] = username + result['user_id'] = user_id + result['email'] = data.email + return result return None except Exception as e: result['error'] = str(e) @@ -201,19 +191,9 @@ def validate_user(username, email): user = UserProfile.get_by_username(username) user_id = 0 - metadata = MetaData() - metadata.reflect(bind=db.engine) - table_name = 'accounts_user' - - user_table = Table(table_name, metadata) - record = db.session.query(user_table) - - data = record.all() - - for item in data: - if item[1] == email: - user_id = item[0] - break + data = User.query.filter(User.email == email).first() + if data: + user_id = data.id if user.user_id == user_id: user_info = dict() @@ -243,24 +223,16 @@ def get_user_info_by_email(email): """ result = dict() try: - metadata = MetaData() - metadata.reflect(bind=db.engine) - table_name = 'accounts_user' - - user_table = Table(table_name, metadata) - record = db.session.query(user_table) - - data = record.all() - for item in data: - if item[1] == email: - user = UserProfile.get_by_userid(item[0]) - if user is None: - result['username'] = "" - else: - result['username'] = user.get_username - result['user_id'] = item[0] - result['email'] = email - return result + data = User.query.filter(User.email == email).first() + if data: + user = UserProfile.get_by_userid(data.id) + if user is None: + result['username'] = "" + else: + result['username'] = user.get_username + result['user_id'] = data.id + result['email'] = email + return result return None except Exception as e: result['error'] = str(e) @@ -288,19 +260,9 @@ def get_user_information(user_id): result['username'] = user_info.get_username result['fullname'] = user_info.fullname - metadata = MetaData() - metadata.reflect(bind=db.engine) - table_name = 'accounts_user' - - user_table = Table(table_name, metadata) - record = db.session.query(user_table) - - data = record.all() - - for item in data: - if item[0] == user_id: - result['email'] = item[1] - return result + data = User.query.filter(User.id == user_id).first() + if data: + result['email'] = data.email return result From 8ee46aa7ffe4cc4571954c14b635befec7435898 Mon Sep 17 00:00:00 2001 From: ivis-nakagawa Date: Thu, 16 Jan 2025 16:50:44 +0900 Subject: [PATCH 6/8] weko#48601 Fix activity list query for shared_user --- modules/weko-workflow/tests/test_api.py | 103 ++++++++++++++++++--- modules/weko-workflow/weko_workflow/api.py | 76 ++++++++------- 2 files changed, 132 insertions(+), 47 deletions(-) diff --git a/modules/weko-workflow/tests/test_api.py b/modules/weko-workflow/tests/test_api.py index 40979f62d3..d793689df0 100644 --- a/modules/weko-workflow/tests/test_api.py +++ b/modules/weko-workflow/tests/test_api.py @@ -264,15 +264,72 @@ def test_WorkActivity_get_activity_list_todo(app, db, workflow, users): extra_info={}, action_order=4 ) - + # 代理登録 repo → system + activity8 = Activity( + status='N', + activity_id='A-00000001-00008', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + action_id=5, + action_status = 'M', + activity_login_user=users[1]['id'], + activity_update_user=users[1]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_share_repo_to_system', + shared_user_id=users[2]['id'], + extra_info={}, + action_order=4 + ) + # 代理登録 comm → repo + activity9 = Activity( + status='N', + activity_id='A-00000001-00009', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + action_id=5, + action_status = 'M', + activity_login_user=users[3]['id'], + activity_update_user=users[3]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_share_comm_to_repo', + shared_user_id=users[1]['id'], + extra_info={}, + action_order=4 + ) + # 代理登録 contributor → comm + activity10 = Activity( + status='N', + activity_id='A-00000001-00010', + workflow_id=workflow['workflow'].id, + flow_id=workflow['flow'].id, + action_id=5, + action_status = 'M', + activity_login_user=users[0]['id'], + activity_update_user=users[0]['id'], + activity_status='M', + activity_start=datetime.strptime('2022/04/14 3:01:53.931', '%Y/%m/%d %H:%M:%S.%f'), + activity_confirm_term_of_use=True, + title='test_share_con_to_comm', + shared_user_id=users[3]['id'], + extra_info={}, + action_order=4 + ) + with db.session.begin_nested(): - db.session.add(activity1) - db.session.add(activity2) - db.session.add(activity3) - db.session.add(activity4) - db.session.add(activity5) - db.session.add(activity6) + db.session.add(activity1) + db.session.add(activity2) + db.session.add(activity3) + db.session.add(activity4) + db.session.add(activity5) + db.session.add(activity6) db.session.add(activity7) + db.session.add(activity8) + db.session.add(activity9) + db.session.add(activity10) db.session.commit() activity_action1 = ActivityAction( @@ -324,6 +381,27 @@ def test_WorkActivity_get_activity_list_todo(app, db, workflow, users): action_handler=-1, action_order=4 ) + activity_action8 = ActivityAction( + activity_id=activity8.activity_id, + action_id=5, + action_status="M", + action_handler=users[1]['id'], + action_order=4 + ) + activity_action9 = ActivityAction( + activity_id=activity9.activity_id, + action_id=5, + action_status="M", + action_handler=users[3]['id'], + action_order=4 + ) + activity_action10 = ActivityAction( + activity_id=activity10.activity_id, + action_id=5, + action_status="M", + action_handler=users[0]['id'], + action_order=4 + ) with db.session.begin_nested(): db.session.add(activity_action1) @@ -333,6 +411,9 @@ def test_WorkActivity_get_activity_list_todo(app, db, workflow, users): db.session.add(activity_action5) db.session.add(activity_action6) db.session.add(activity_action7) + db.session.add(activity_action8) + db.session.add(activity_action9) + db.session.add(activity_action10) db.session.commit() with app.test_request_context(): @@ -340,23 +421,23 @@ def test_WorkActivity_get_activity_list_todo(app, db, workflow, users): login_user(users[2]["obj"]) activities, max_page, size, page, name_param = wa.get_activity_list( None, {}, False, False) - assert len(activities) == 3 + assert len(activities) == 4 # repo admin login_user(users[1]["obj"]) activities, max_page, size, page, name_param = wa.get_activity_list( None, {}, False, False) - assert len(activities) == 3 + assert len(activities) == 5 # comm admin login_user(users[3]["obj"]) with patch('weko_workflow.api.WekoDeposit.get_record', return_value={'path': [1]}): activities, max_page, size, page, name_param = wa.get_activity_list( None, {}, False, False) - assert len(activities) == 3 + assert len(activities) == 5 # contributor admin login_user(users[0]["obj"]) activities, max_page, size, page, name_param = wa.get_activity_list( None, {}, False, False) - assert len(activities) == 1 \ No newline at end of file + assert len(activities) == 2 \ No newline at end of file diff --git a/modules/weko-workflow/weko_workflow/api.py b/modules/weko-workflow/weko_workflow/api.py index 84e79d74cf..37fe090c9a 100644 --- a/modules/weko-workflow/weko_workflow/api.py +++ b/modules/weko-workflow/weko_workflow/api.py @@ -1614,45 +1614,48 @@ def query_activities_by_tab_is_todo(query, is_admin, is_community_admin): .filter(_FlowAction.action_order == _Activity.action_order) if is_admin or is_community_admin: query = query \ - .filter( - ActivityAction.action_handler.in_([-1, self_user_id]) - ) + .filter( + or_( + ActivityAction.action_handler.in_([-1, self_user_id]), + _Activity.shared_user_id == self_user_id + ) + ) else: query = query \ - .filter( - or_( - and_( - _FlowActionRole.action_user == self_user_id, - _FlowActionRole.action_user_exclude == '0' - ), - and_( - _FlowActionRole.action_role.in_(self_group_ids), - _FlowActionRole.action_role_exclude == '0' - ), - and_( - _FlowActionRole.id.is_(None) - ), - and_( - _Activity.shared_user_id == self_user_id, - ), - ) - ) \ - .filter( - or_( - ActivityAction.action_handler == self_user_id, - and_( - _FlowActionRole.action_user == self_user_id, - _FlowActionRole.action_user_exclude == '0' - ), - and_( - _FlowActionRole.action_role.in_(self_group_ids), - _FlowActionRole.action_role_exclude == '0' - ), - and_( - _Activity.shared_user_id == self_user_id, - ), + .filter( + or_( + and_( + _FlowActionRole.action_user == self_user_id, + _FlowActionRole.action_user_exclude == '0' + ), + and_( + _FlowActionRole.action_role.in_(self_group_ids), + _FlowActionRole.action_role_exclude == '0' + ), + and_( + _FlowActionRole.id.is_(None) + ), + and_( + _Activity.shared_user_id == self_user_id, + ), + ) + ) \ + .filter( + or_( + ActivityAction.action_handler == self_user_id, + and_( + _FlowActionRole.action_user == self_user_id, + _FlowActionRole.action_user_exclude == '0' + ), + and_( + _FlowActionRole.action_role.in_(self_group_ids), + _FlowActionRole.action_role_exclude == '0' + ), + and_( + _Activity.shared_user_id == self_user_id, + ), + ) ) - ) return query @@ -1753,6 +1756,7 @@ def __format_activity_data_to_show_on_workflow(self, activities, if is_community_admin: if activity_data.activity_login_user != int(current_user.get_id()) and \ + activity_data.shared_user_id != int(current_user.get_id()) and \ not self._check_community_permission(activity_data, index_ids): continue # Append to do and action activities into the master list From d9f5c59c54eb9ae981c9b78070769d714f115b2b Mon Sep 17 00:00:00 2001 From: ivis-nakagawa Date: Fri, 17 Jan 2025 10:57:36 +0900 Subject: [PATCH 7/8] weko#48601 Fix authority check logic in Action Role --- modules/weko-workflow/tests/conftest.py | 16 ++++++++-------- modules/weko-workflow/tests/test_views.py | 17 +++++++---------- modules/weko-workflow/weko_workflow/views.py | 4 ++-- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/modules/weko-workflow/tests/conftest.py b/modules/weko-workflow/tests/conftest.py index 743bc69414..813fafa8fd 100644 --- a/modules/weko-workflow/tests/conftest.py +++ b/modules/weko-workflow/tests/conftest.py @@ -700,14 +700,14 @@ def users(app, db): ActionRoles(action='files-rest-object-delete-version', role=repoadmin_role), ActionRoles(action='files-rest-object-read', role=repoadmin_role), ActionRoles(action='search-access', role=repoadmin_role), - ActionRoles(action='detail-page-acces', role=repoadmin_role), + ActionRoles(action='detail-page-access', role=repoadmin_role), ActionRoles(action='download-original-pdf-access', role=repoadmin_role), ActionRoles(action='author-access', role=repoadmin_role), ActionRoles(action='items-autofill', role=repoadmin_role), ActionRoles(action='stats-api-access', role=repoadmin_role), ActionRoles(action='read-style-action', role=repoadmin_role), ActionRoles(action='update-style-action', role=repoadmin_role), - ActionRoles(action='detail-page-acces', role=repoadmin_role), + ActionRoles(action='detail-page-access', role=repoadmin_role), ActionRoles(action='admin-access', role=comadmin_role), ActionRoles(action='index-tree-access', role=comadmin_role), @@ -718,12 +718,12 @@ def users(app, db): ActionRoles(action='files-rest-object-delete-version', role=comadmin_role), ActionRoles(action='files-rest-object-read', role=comadmin_role), ActionRoles(action='search-access', role=comadmin_role), - ActionRoles(action='detail-page-acces', role=comadmin_role), + ActionRoles(action='detail-page-access', role=comadmin_role), ActionRoles(action='download-original-pdf-access', role=comadmin_role), ActionRoles(action='author-access', role=comadmin_role), ActionRoles(action='items-autofill', role=comadmin_role), - ActionRoles(action='detail-page-acces', role=comadmin_role), - ActionRoles(action='detail-page-acces', role=comadmin_role), + ActionRoles(action='detail-page-access', role=comadmin_role), + ActionRoles(action='detail-page-access', role=comadmin_role), ActionRoles(action='item-access', role=contributor_role), ActionRoles(action='files-rest-bucket-update', role=contributor_role), @@ -731,12 +731,12 @@ def users(app, db): ActionRoles(action='files-rest-object-delete-version', role=contributor_role), ActionRoles(action='files-rest-object-read', role=contributor_role), ActionRoles(action='search-access', role=contributor_role), - ActionRoles(action='detail-page-acces', role=contributor_role), + ActionRoles(action='detail-page-access', role=contributor_role), ActionRoles(action='download-original-pdf-access', role=contributor_role), ActionRoles(action='author-access', role=contributor_role), ActionRoles(action='items-autofill', role=contributor_role), - ActionRoles(action='detail-page-acces', role=contributor_role), - ActionRoles(action='detail-page-acces', role=contributor_role), + ActionRoles(action='detail-page-access', role=contributor_role), + ActionRoles(action='detail-page-access', role=contributor_role), ] db.session.add_all(action_roles) db.session.commit() diff --git a/modules/weko-workflow/tests/test_views.py b/modules/weko-workflow/tests/test_views.py index 69f874c662..7fb071d459 100644 --- a/modules/weko-workflow/tests/test_views.py +++ b/modules/weko-workflow/tests/test_views.py @@ -3980,7 +3980,7 @@ class MockUser: # .tox/c1/bin/pytest --cov=weko_workflow tests/test_views.py::test_check_authority_action -v -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp def test_check_authority_action(app,db,users,db_register,db_records): - # no authentiated + # no authenticated with app.test_request_context(): result = check_authority_action() assert result == 1 @@ -4012,14 +4012,14 @@ def test_check_authority_action(app,db,users,db_register,db_records): with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): result = check_authority_action() assert result == 1 - - # role not in allow - rs = {"allow":[1,2],"deny":[]} + + # role in allow + rs = {"allow":[2,3],"deny":[]} us = {"allow":[],"deny":[]} with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): result = check_authority_action() - assert result == 1 - + assert result == 0 + rs = {"allow":[], "deny":[]} us = {"allow":[], "deny":[]} with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): @@ -4028,10 +4028,7 @@ def test_check_authority_action(app,db,users,db_register,db_records): # cur_user == activity_login_user result = check_authority_action(activity_id=activity.activity_id) assert result == 0 - - rs = {"allow":[3], "deny":[]} - us = {"allow":[], "deny":[]} - with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): + with patch("flask_login.utils._get_user", return_value=users[0]["obj"]): activity = db_register["activities"][1] # item_metadata.json[shared_user_id]=cur_user diff --git a/modules/weko-workflow/weko_workflow/views.py b/modules/weko-workflow/weko_workflow/views.py index cb91a63fe5..6c40c3d386 100644 --- a/modules/weko-workflow/weko_workflow/views.py +++ b/modules/weko-workflow/weko_workflow/views.py @@ -1188,8 +1188,8 @@ def check_authority_action(activity_id='0', action_id=0, for role in cur_role: if roles['deny'] and role.id in roles['deny']: return 1 - if roles['allow'] and role.id not in roles['allow']: - return 1 + if roles['allow'] and role.id in roles['allow']: + return 0 # If action_roles is not set # or action roles does not contain any role of current_user: From 3b8d40737e263214d14850ee42f86a6e0d6404b4 Mon Sep 17 00:00:00 2001 From: ivis-nakagawa Date: Thu, 23 Jan 2025 11:21:13 +0900 Subject: [PATCH 8/8] weko#48601 Fix authority check logic in Action Role at @check_authority decorator --- modules/weko-workflow/tests/test_views.py | 67 +++++++++++++++++++- modules/weko-workflow/weko_workflow/views.py | 14 +++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/modules/weko-workflow/tests/test_views.py b/modules/weko-workflow/tests/test_views.py index 7fb071d459..37c28c532e 100644 --- a/modules/weko-workflow/tests/test_views.py +++ b/modules/weko-workflow/tests/test_views.py @@ -48,7 +48,7 @@ from flask_security import login_user from invenio_accounts.testutils import login_user_via_session as login from weko_workflow.models import ActionStatusPolicy, ActionFeedbackMail, ActionJournal, ActionIdentifier, Activity, ActivityHistory, ActionStatus, Action, WorkFlow, FlowDefine, FlowAction,FlowActionRole, ActivityAction, GuestActivity -from weko_workflow.views import unlock_activity, check_approval, get_feedback_maillist, save_activity, previous_action,_generate_download_url,check_authority_action +from weko_workflow.views import unlock_activity, check_approval, get_feedback_maillist, save_activity, previous_action,_generate_download_url,check_authority,check_authority_action from marshmallow.exceptions import ValidationError from weko_records_ui.models import FileOnetimeDownload, FilePermission from weko_records.models import ItemMetadata, ItemReference @@ -3978,6 +3978,71 @@ class MockUser: res = client.post(url, query_string=input) mock_render_template.assert_called() + +# .tox/c1/bin/pytest --cov=weko_workflow tests/test_views.py::test_check_authority -v -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp +def test_check_authority(app,db,users,db_register): + + @check_authority + def test_function(activity_id, action_id): + return jsonify(code=200) + + with patch("flask_login.utils._get_user", return_value=users[2]["obj"]): + # user has admin role + with patch("weko_workflow.views.check_authority_by_admin", return_value=True): + result = test_function(activity_id='1', action_id=1).get_json() + assert result['code'] == 200 + + with patch("flask_login.utils._get_user", return_value=users[0]["obj"]): + # action role and action user are empty + rs = {"allow":[],"deny":[]} + us = {"allow":[],"deny":[]} + with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): + result = test_function(activity_id='1', action_id=1).get_json() + assert result['code'] == 200 + + # user in deny + rs = {"allow":[],"deny":[]} + us = {"allow":[],"deny":[2]} + with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): + result = test_function(activity_id='1', action_id=1).get_json() + assert result['code'] == 403 + + # user not in allow + rs = {"allow":[],"deny":[]} + us = {"allow":[1],"deny":[]} + with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): + result = test_function(activity_id='1', action_id=1).get_json() + assert result['code'] == 403 + + # user in allow + rs = {"allow":[],"deny":[]} + us = {"allow":[2],"deny":[]} + with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): + result = test_function(activity_id='1', action_id=1).get_json() + assert result['code'] == 200 + + # role in deny + rs = {"allow":[],"deny":[3]} + us = {"allow":[],"deny":[]} + with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): + result = test_function(activity_id='1', action_id=1).get_json() + assert result['code'] == 403 + + # role not in allow + rs = {"allow":[1,2],"deny":[]} + us = {"allow":[],"deny":[]} + with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): + result = test_function(activity_id='1', action_id=1).get_json() + assert result['code'] == 403 + + # role in allow + rs = {"allow":[2,3],"deny":[]} + us = {"allow":[],"deny":[]} + with patch("weko_workflow.views.WorkActivity.get_activity_action_role",return_value=(rs,us)): + result = test_function(activity_id='1', action_id=1).get_json() + assert result['code'] == 200 + + # .tox/c1/bin/pytest --cov=weko_workflow tests/test_views.py::test_check_authority_action -v -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp def test_check_authority_action(app,db,users,db_register,db_records): # no authenticated diff --git a/modules/weko-workflow/weko_workflow/views.py b/modules/weko-workflow/weko_workflow/views.py index 6c40c3d386..ddc05274aa 100644 --- a/modules/weko-workflow/weko_workflow/views.py +++ b/modules/weko-workflow/weko_workflow/views.py @@ -1147,11 +1147,23 @@ def decorated_function(*args, **kwargs): return jsonify(code=403, msg=error_msg) if users['allow'] and int(cur_user) not in users['allow']: return jsonify(code=403, msg=error_msg) + + role_flg = 0 for role in cur_role: + # If current_role is in denied action_role if roles['deny'] and role.id in roles['deny']: return jsonify(code=403, msg=error_msg) - if roles['allow'] and role.id not in roles['allow']: + if roles['allow']: + # If current_role is in allowed action_role + if role.id in roles['allow']: + break + else: + role_flg = 1 + else: + # If allowed action_role does not contain current_role + if role_flg: return jsonify(code=403, msg=error_msg) + return func(*args, **kwargs) return decorated_function