Skip to content

fix: 権限チェック追加に追従していなかったユニットテストを直す #1942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mhaya wants to merge 2 commits into
base: develop_v2.1.0
Choose a base branch
from
Open

fix: 権限チェック追加に追従していなかったユニットテストを直す #1942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c961fc5
fix(invenio-resourcesyncserver): _export_item が {} を返したときに処理を打ち切る
Oct 18, 2026
be17daa
test: 権限チェック追加にテストが追従しておらず CI で落ちていたのを直す
Oct 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions modules/invenio-resourcesyncserver/invenio_resourcesyncserver/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,9 @@ def get_record_content_file(self, record_id):
include_contents,
record_path,
)
if not exported_item:
# _export_item は閲覧権限のないレコードを {} で返す
return None

result['items'].append(exported_item)

Expand Down Expand Up @@ -1222,6 +1225,9 @@ def get_record_content_file(self, record_id):
include_contents,
record_path,
)
if not exported_item:
# _export_item は閲覧権限のないレコードを {} で返す
return None

result['items'].append(exported_item)

Expand Down
14 changes: 11 additions & 3 deletions modules/invenio-resourcesyncserver/tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,10 @@ def get_resource_dump_manifest(item):
# Exception coverage
assert not test.get_record_content_file(1)

# 閲覧権限のないレコードは _export_item が {} を返す
with patch("invenio_resourcesyncserver.api._export_item", return_value=({}, {})):
assert test.get_record_content_file(1) is None

@sourcery-ai sourcery-ai Bot Oct 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium · Empty-export regression goes undetected

When the empty-export guard is removed, removing the new empty-export guard still lets this assertion pass: ItemTypes.get_by_id(None) is followed by an attribute access that raises, and the handler catches that exception and returns None. The test therefore does not catch a regression to the exception-and-error-log path it is intended to prevent.

Also assert that the downstream item-type lookup is not reached for an empty export.

Prompt for AI agents 
In `modules/invenio-resourcesyncserver/tests/test_api.py` at line 377:

**Empty-export regression goes undetected**

When the empty-export guard is removed, removing the new empty-export guard still lets this assertion pass: `ItemTypes.get_by_id(None)` is followed by an attribute access that raises, and the handler catches that exception and returns `None`. The test therefore does not catch a regression to the exception-and-error-log path it is intended to prevent.

Also assert that the downstream item-type lookup is not reached for an empty export.



# class ChangeListHandler(object):
# def __init__(self, **kwargs):
Expand Down Expand Up @@ -803,12 +807,16 @@ def get_change_dump_manifest_xml(key):

test_str._is_record_in_index = _is_record_in_index

# Exception coverage
# es_records のレコードは非公開なので、匿名では書き出されない
assert not test_str.get_record_content_file(record_id)

test_str.get_change_dump_manifest_xml = get_change_dump_manifest_xml
with patch("weko_items_ui.utils.check_publish_status", return_value=True):
# Exception coverage
assert not test_str.get_record_content_file(record_id)

test_str.get_change_dump_manifest_xml = get_change_dump_manifest_xml

assert test_str.get_record_content_file(record_id)
assert test_str.get_record_content_file(record_id)


# def get_capability_content(cls):
Expand Down
4 changes: 2 additions & 2 deletions modules/weko-groups/tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ def users(app, db_2):
comadmin = User.query.filter_by(email='comadmin@test.org').first()
repoadmin = User.query.filter_by(email='repoadmin@test.org').first()
sysadmin = User.query.filter_by(email='sysadmin@test.org').first()
generaluser = User.query.filter_by(email='generaluser@test.org')
generaluser = User.query.filter_by(email='generaluser@test.org').first()
originalroleuser = create_test_user(email='originalroleuser@test.org')
originalroleuser2 = create_test_user(email='originalroleuser2@test.org')

Expand Down Expand Up @@ -278,7 +278,7 @@ def users(app, db_2):
{'email': repoadmin.email, 'id': repoadmin.id, 'obj': repoadmin},
{'email': sysadmin.email, 'id': sysadmin.id, 'obj': sysadmin},
{'email': comadmin.email, 'id': comadmin.id, 'obj': comadmin},
{'email': generaluser.email, 'id': generaluser.id, 'obj': sysadmin},
{'email': generaluser.email, 'id': generaluser.id, 'obj': generaluser},
{'email': originalroleuser.email, 'id': originalroleuser.id, 'obj': originalroleuser},
{'email': originalroleuser2.email, 'id': originalroleuser2.id, 'obj': originalroleuser2},
{'email': user.email, 'id': user.id, 'obj': user},
Expand Down
5 changes: 3 additions & 2 deletions modules/weko-groups/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -975,10 +975,11 @@ def test_can_invite_others(example_group):
assert group.can_invite_others(member)
assert group.can_invite_others(non_member)

# 招待できるのは OPEN のときだけ。APPROVAL では管理者のみ
group.subscription_policy = SubscriptionPolicy.APPROVAL
assert group.can_invite_others(admin)
assert group.can_invite_others(member)
assert group.can_invite_others(non_member)
assert not group.can_invite_others(member)
assert not group.can_invite_others(non_member)

group.subscription_policy = SubscriptionPolicy.CLOSED
assert group.can_invite_others(admin)
Expand Down
22 changes: 21 additions & 1 deletion modules/weko-groups/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,13 @@ def validate_on_submit_False():
with app_2.test_request_context():
with app_2.test_client() as client:
with patch("flask_login.utils._get_user", return_value=users[id]["obj"]):
if status_code == 403:
# roles_required で弾かれる。要求後にセッションが片付き
# fixture のユーザが detach されるため、続けて要求しない
res = client.get(url_for('weko_groups.new'))
assert res.status_code == status_code
return

with patch("weko_groups.views.GroupForm", return_value=form):
# "Encountered unknown tag 'assets'. Jinja was looking for the following tags: 'endblock'. The innermost block that needs to be closed is 'block'.",
# But upon testing on the actual url on the browser "https://localhost/accounts/settings/groups/new" there is no problem
Expand Down Expand Up @@ -555,11 +562,24 @@ def test_new_member_3(app_2, users):
with app_2.test_request_context():
group = Group.create(name="name", is_managed=True)
with app_2.test_client() as client:
with patch("flask_login.utils._get_user", return_value=users[7]["obj"]):
# 管理者ロールがあっても is_managed のグループには招待できず戻される
with patch("flask_login.utils._get_user", return_value=users[3]["obj"]):
res = client.get(url_for('weko_groups.new_member', group_id=group.id))
assert res.status_code == 302


def test_new_member_4(app_2, users):
# 1要求ごとにセッションが片付き fixture のユーザが detach されるため、
# 別ユーザでの確認は別のテストに分ける
with app_2.test_request_context():
group = Group.create(name="name", is_managed=True)
with app_2.test_client() as client:
# 管理者ロールを持たないユーザは roles_required で弾かれる
with patch("flask_login.utils._get_user", return_value=users[7]["obj"]):
res = client.get(url_for('weko_groups.new_member', group_id=group.id))
assert res.status_code == 403


# def remove_csrf(form):
def test_remove_csrf(app):
form = MagicMock()
Expand Down
3 changes: 2 additions & 1 deletion modules/weko-groups/tests/test_weko_groups.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,6 @@ def test_view(app):
with app.test_client() as client:
res = client.get('/accounts/settings/groups/groupcount')
assert 200 == res.status_code
# grouplist は login_required。未ログインはログイン画面へ飛ばされる
res = client.get('/accounts/settings/groups/grouplist')
assert 200 == res.status_code
assert 302 == res.status_code
2 changes: 1 addition & 1 deletion modules/weko-search-ui/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def test_opensearch_description_acl_guest(app,client_api,db_register2,index_styl
# def journal_detail(index_id=0):
def test_journal_detail(i18n_app, users, indices):
with patch("flask_login.utils._get_user", return_value=users[3]['obj']):
assert journal_detail(33)
assert journal_detail(index_id=33)


def test_journal_detail_forbidden(client, users, indices):
Expand Down
10 changes: 3 additions & 7 deletions modules/weko-workflow/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4365,14 +4365,10 @@ def test_get_feedback_maillist_acl_users(


# .tox/c1/bin/pytest --cov=weko_workflow tests/test_views.py::test_get_feedback_maillist -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp
# check_authority で弾かれずに本体まで届くよう、アクティビティの申請者ではない
# ユーザのうち管理者(sysadmin)で確認する。権限の境界は *_acl_* のテストで見る。
@pytest.mark.parametrize('users_index, status_code', [
(0, 200),
#(1, 200),
#(2, 200),
#(3, 200),
#(4, 200),
#(5, 200),
#(6, 200),
(2, 200),
])
def test_get_feedback_maillist(client, users, db_register_full_action, users_index, status_code):
login(client=client, email=users[users_index]['email'])
Expand Down
Loading